[Fleet] Support dynamic_template mappings from object field by nchaulet · Pull Request #137772 · elastic/kibana

nchaulet · 2022-08-01T19:03:37Z

Summary

Related to #129344

Fleet generate mappings from fields yaml provided by integrations, we were not handling field with object and object_type correctly as these fields should not result in properties mappings but in dynamic templates.

That PR fix that, if a field is of type object and contains an object_type we generate a dynamic template, if the path do not contains a * we add .* at the end of the path.

For example this fields:

- name: cockroachdb.status.*.histogram
  type: object
  object_type: histogram
  object_type_mapping_type: '*'
  description: >-
    Prometheus histogram metric

Will result in the following mappings

"dynamic_templates": [{
      "cockroachdb.status.*.histogram": {
        "mapping": {
          "type": "histogram"
        },
        "match_mapping_type": "*",
        "path_match": "cockroachdb.status.*.histogram"
      }
    }
  ]

How this was implemented in beats https://github.com/elastic/beats/blob/ea207346d651448b8917b0791b2b117b9f9b9212/libbeat/template/processor.go#L444

In a second PR, we will introduce a way to generate dynamic template just by adding a * in the property name.

@jsoriano Does it make sense to you?

I tested this by installing all the integrations (to verify all the integrations still install) and looked a few mappings to see if they seemed correct to me.

Should we target 8.4 with that change?

nchaulet · 2022-08-02T14:07:22Z

x-pack/plugins/fleet/server/services/epm/elasticsearch/template/mappings.ts

@@ -0,0 +1,48 @@
+/*


I created that file to start moving mapping generation to his own module.

elasticmachine · 2022-08-02T14:10:21Z

Pinging @elastic/fleet (Team:Fleet)

jsoriano · 2022-08-02T16:02:18Z

That PR fix that, if a field is of type object and contains an object_type we generate a dynamic template, if the path do not contains a * we add .* at the end of the path.

How this was implemented in beats https://github.com/elastic/beats/blob/ea207346d651448b8917b0791b2b117b9f9b9212/libbeat/template/processor.go#L444

In a second PR, we will introduce a way to generate dynamic template just by adding a * in the property name.

@jsoriano Does it make sense to you?

Makes sense, yes, keep this separated as this is something new. Thanks!

jsoriano · 2022-08-02T16:07:01Z

...plugins/fleet/server/services/epm/elasticsearch/template/__snapshots__/template.test.ts.snap

+            "labels": {
+              "properties": {}
+            },
+            "*": {


This static field shouldn't be here, right? This creates a mapping for a field called literally cockroachdb.status.labels.*.

Yes you are correct, I just pushed a fix for that we do not create mappings is there only dynamic template for that path.

kpollich

Not an area of the Fleet codebase I'm too familiar with, but I think I understand everything happening here for the most part. 🚀

x-pack/plugins/fleet/server/services/epm/elasticsearch/template/template.ts

nchaulet · 2022-08-02T17:18:34Z

@kpollich Do you think we should backport this one to 8.4?

kpollich · 2022-08-02T17:20:01Z

@nchaulet - Yes let's backport to 8.4

nchaulet · 2022-08-02T19:08:13Z

@elasticmachine merge upstream

nchaulet · 2022-08-02T22:47:17Z

@elasticmachine merge upstream

kibana-ci · 2022-08-03T00:37:50Z

💚 Build Succeeded

Buildkite Build
Commit: 3be2c5e

Metrics [docs]

✅ unchanged

History

💚 Build #62613 succeeded 4589dd9
💔 Build #62597 failed 2a22f69a2f1ed0fb261efcace8f69c89c184521f
💚 Build #62567 succeeded 54a6c4e
💔 Build #62553 failed 6c8cb71
💔 Build #62520 failed d988ac5
💚 Build #62449 succeeded 7e6084c

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @nchaulet

…137772) (cherry picked from commit 76c55a2)

kibanamachine · 2022-08-03T00:53:04Z

💚 All backports created successfully

Status	Branch	Result
✅	8.4

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

…#137929) (cherry picked from commit 76c55a2) Co-authored-by: Nicolas Chaulet <nicolas.chaulet@elastic.co>

jsoriano · 2022-08-03T08:36:12Z

Thanks!

andrewkroh · 2022-08-17T14:13:03Z

This is adding features and behavior that is not defined in the package-spec which undermines the value of working from a specification as our source of truth. Please go through the process to update package-spec so that everyone knows how to benefit from these useful changes.

jsoriano · 2022-08-18T08:51:11Z

Thanks @andrewkroh for raising this up, I have created an issue to follow on this elastic/package-spec#393.

nchaulet added release_note:skip Skip the PR/issue when compiling release notes Team:Fleet Team label for Observability Data Collection Fleet team labels Aug 1, 2022

nchaulet self-assigned this Aug 1, 2022

nchaulet force-pushed the feature-dynamic-template-mappings branch 2 times, most recently from 0cfbb09 to b658868 Compare August 1, 2022 20:22

[Fleet] Support dynamic_template mappings from object field

3fad1e9

nchaulet force-pushed the feature-dynamic-template-mappings branch from b658868 to 3fad1e9 Compare August 2, 2022 14:03

nchaulet commented Aug 2, 2022

View reviewed changes

nchaulet added v8.5.0 bug Fixes for quality problems that affect the customer experience labels Aug 2, 2022

nchaulet marked this pull request as ready for review August 2, 2022 14:10

nchaulet requested a review from a team as a code owner August 2, 2022 14:10

kpollich self-requested a review August 2, 2022 14:29

Update snapshot

7e6084c

jsoriano reviewed Aug 2, 2022

View reviewed changes

kpollich approved these changes Aug 2, 2022

View reviewed changes

x-pack/plugins/fleet/server/services/epm/elasticsearch/template/template.ts Outdated Show resolved Hide resolved

nchaulet added 2 commits August 2, 2022 13:15

Fix empty properties

811e843

Fix after review

d988ac5

nchaulet added the v8.4.0 label Aug 2, 2022

Update snapshot

6c8cb71

Merge branch 'main' into feature-dynamic-template-mappings

54a6c4e

jen-huang added the backport:prev-minor label Aug 2, 2022

Fix merge existing dynamic_templates

4589dd9

nchaulet force-pushed the feature-dynamic-template-mappings branch from 2a22f69 to 4589dd9 Compare August 2, 2022 21:18

Merge branch 'main' into feature-dynamic-template-mappings

3be2c5e

nchaulet merged commit 76c55a2 into elastic:main Aug 3, 2022

nchaulet deleted the feature-dynamic-template-mappings branch August 3, 2022 00:49

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Aug 3, 2022

[Fleet] Support dynamic_template mappings from object field (elastic#…

9b8b7b5

…137772) (cherry picked from commit 76c55a2)

kibanamachine mentioned this pull request Aug 3, 2022

[8.4] [Fleet] Support dynamic_template mappings from object field (#137772) #137929

Merged

kibanamachine added a commit that referenced this pull request Aug 3, 2022

[Fleet] Support dynamic_template mappings from object field (#137772) (…

91337f4

…#137929) (cherry picked from commit 76c55a2) Co-authored-by: Nicolas Chaulet <nicolas.chaulet@elastic.co>

nchaulet mentioned this pull request Aug 3, 2022

[Fleet] Generate dynamic template mappings for field with wildcard in name #137978

Merged

jsoriano mentioned this pull request Aug 18, 2022

[Change Proposal] Properly support dynamic template mappings elastic/package-spec#393

Closed

jsoriano mentioned this pull request Sep 14, 2022

Document and validate object_type elastic/package-spec#417

Merged

jsoriano mentioned this pull request Dec 7, 2022

Temporarily import hard-coded list of common fields elastic/elastic-package#1018

Closed

3 tasks

jsoriano mentioned this pull request Oct 2, 2023

[Validation] Detect incorrectly defined dynamic mappings elastic/package-spec#624

Closed

Conversation

nchaulet commented Aug 1, 2022 • edited by kibanamachine Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Uh oh!

nchaulet Aug 2, 2022

Choose a reason for hiding this comment

Uh oh!

elasticmachine commented Aug 2, 2022

Uh oh!

jsoriano commented Aug 2, 2022

Uh oh!

jsoriano Aug 2, 2022

Choose a reason for hiding this comment

Uh oh!

nchaulet Aug 2, 2022

Choose a reason for hiding this comment

Uh oh!

kpollich left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nchaulet commented Aug 2, 2022

Uh oh!

kpollich commented Aug 2, 2022

Uh oh!

nchaulet commented Aug 2, 2022

Uh oh!

nchaulet commented Aug 2, 2022

Uh oh!

kibana-ci commented Aug 3, 2022

💚 Build Succeeded

Metrics [docs]

History

Uh oh!

kibanamachine commented Aug 3, 2022

💚 All backports created successfully

Questions ?

Uh oh!

jsoriano commented Aug 3, 2022

Uh oh!

andrewkroh commented Aug 17, 2022

Uh oh!

jsoriano commented Aug 18, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

nchaulet commented Aug 1, 2022 •

edited by kibanamachine

Loading