Fleet: add support for subobjects: false #171826

zmoog · 2023-11-23T08:49:36Z

Summary

Update the Fleet plugin to support the subobjects setting on the object type mapping.

This PR supports the subobjects setting on a per-field basis. We will add support for subobjects setting at the data stream level when Elasticsearch will automatically flatten the mappings in elastic/elasticsearch#99860.

The PR add deals with the following user cases found in the integration packages and add support for a few of them.

Case A

- name: a.labels
  type: object
  subobjects: false

The use case A is invalid on package-spec v3 and it's not supported.

Case B

- name: b.labels.*
  type: object
  object_type: keyword
  subobjects: false

that _generateMappings() should map to:

{
  dynamic_templates: [{
    "b.labels.*": {
      path_match: "b.labels.*",
      match_mapping_type: "string",
      mapping: {
        type: "keyword"
      }
    }
  }],
  properties: {
    b: {
      type: 'object',
      dynamic: true,
      properties: {
        labels: {
          dynamic: true,
          type: 'object',
          subobjects: false,
        }
      }
    },
  },
}

Case C

- name: prometheus.c.labels.*
  type: object
  subobjects: false

The use case C is considered invalid and it's not supported.

Case D

- name: d.labels
  type: object
  object_type: keyword
  subobjects: false

that _generateMappings() should map to:

{
  dynamic_templates: [{
    "d.labels": {
      path_match: "d.labels.*",
      match_mapping_type: "string",
      mapping: {
        type: "keyword"
      }
    }
  }],
  properties: {
    d: {
      type: 'object',
      dynamic: true,
      properties: {
        labels: {
          dynamic: true,
          type: 'object',
          subobjects: false,
        }
      }
    },
  },
}

Checklist

Delete any items that are not applicable to this PR.

~~[ ] Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support~~
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios
~~[ ] Any UI touched in this PR is usable by keyboard only (learn more about keyboard accessibility)~~
~~[ ] Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)~~
~~[ ] If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the docker list~~
~~[ ] This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)~~
~~[ ] This was checked for cross-browser compatibility~~

Risk Matrix

Delete this section if it is not applicable to this PR.

Before closing this PR, invite QA, stakeholders, and other developers to identify risks that should be tested prior to the change/feature release.

When forming the risk matrix, consider some of the following examples and how they may potentially impact the change:

Risk	Probability	Severity	Mitigation/Notes
Multiple Spaces—unexpected behavior in non-default Kibana Space.	Low	High	Integration tests will verify that all features are still supported in non-default Kibana Space and when user switches between spaces.
Multiple nodes—Elasticsearch polling might have race conditions when multiple Kibana nodes are polling for the same tasks.	High	Low	Tasks are idempotent, so executing them multiple times will not result in logical error, but will degrade performance. To test for this case we add plenty of unit tests around this logic and document manual testing procedure.
Code should gracefully handle cases when feature X or plugin Y are disabled.	Medium	High	Unit tests will verify that any feature flag or plugin combination still results in our service operational.
See more potential risk examples

For maintainers

This was checked for breaking API changes and was labeled appropriately

apmmachine · 2023-11-23T08:49:52Z

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

/oblt-deploy : Deploy a Kibana instance using the Observability test environments.
/oblt-deploy-serverless : Deploy a serverless Kibana instance using the Observability test environments.
run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

jsoriano

Looks mostly good to me. We should confirm first how this is going to be supported in package-spec, to know what cases should be supported.
Lets continue with elastic/package-spec#573 in parallel.

jsoriano · 2023-11-30T15:35:54Z

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

+    const objectFieldWithPropertyReversedLiteralYml = `
+- name: a.labels
+  type: object
+  subobjects: false


I think that this mapping is not allowed in the package spec. It would not define a mapping for the sub fields of a.labels.

Yep, I agree.

I kept this case around if we found a compelling use case to resurrect it. There isn't one.

So, since it is not allowed in package-spec v3, I will drop this mapping from the PR.

jsoriano · 2023-11-30T15:40:06Z

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

+    const mappings = generateMappings(processedFields);
+    expect(mappings).toEqual(objectFieldWithPropertyReversedMapping);
+  });
+


Should we add a test case where subobjects is used in an object without dynamic mappings? Something like this.

- name: a type: object subobjects: false fields: - name: b.c type: keyword - name: x.y type: keyword

For completeness, more than because I have some use case in mind 🙂

Makes sense!

I added the test case in the template.test.ts file:

- name: a type: object subobjects: false fields: - name: b.c type: keyword - name: x.y type: keyword

And I am getting the following result:

{ properties: { a: { type: 'object', subobjects: false, }, }, }

The result is weird, but I think it's because to have fields, the type should be group, right?

I tried to add this to the good_v3 test package, and it complains about the type value:

type must be one of the following: "group", "nested"

Since the type is not group the content of fields is probably ignored.

Ah yes, you are right, it should be type: group, not object, can you try to change it? Or well, maybe we don't need to support this use case, at least by now.

Yeah, looking to what we will likely do in the spec we don't need to support the case of the object with subfields.

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

elasticmachine · 2023-11-30T15:56:21Z

Pinging @elastic/fleet (Team:Fleet)

zmoog · 2023-12-05T08:12:29Z

@jsoriano, do you think we're close to the finish line, or does this PR need more work? It would be great to have it in 8.12, if ready.

jsoriano

I am sorry it won't probably reach 8.12. Lets get this in and in the package spec so we can start testing with actual packages.

jsoriano · 2023-12-07T13:34:59Z

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

+    expect(mappings).toEqual(objectFieldWithPropertyReversedMapping);
+  });
+
+  it('tests processing object field with subobjects without dynamic mappings', () => {


This test can be probably removed as this won't be supported by the spec, as very well noticed in https://github.com/elastic/kibana/pull/171826/files#r1411644741

zmoog · 2023-12-11T08:56:39Z

@elasticmachine merge upstream

zmoog · 2023-12-11T14:01:01Z

@elasticmachine merge upstream

Add support for the basic case: - name: b.labels.* type: object object_type: keyword subjects: false that `_generateMappings()` should map to: { "properties": { "a": { "properties": { "labels": { "type": "object", "subobjects": false } } } } }

…-fix'

We consider case C invalid and we'll not support it.

…-fix'

Use case A is non allowed in package-spec v3.

We decided [^1] this is not a supported use case, dropping it. [^1]: elastic#171826 (comment)

kibana-ci · 2023-12-11T18:31:09Z

💛 Build succeeded, but was flaky

Buildkite Build
Commit: 522baa5

Failed CI Steps

Metrics [docs]

✅ unchanged

History

💔 Build #182944 failed 6318ce9
💔 Build #182877 failed 33ae2ba
💚 Build #180773 succeeded aa0c9b6
💚 Build #180431 succeeded 93a4249
💔 Build #179896 failed 78169c0

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

cc @zmoog

zmoog self-assigned this Nov 23, 2023

zmoog force-pushed the zmoog/subobjects-false branch 2 times, most recently from 78169c0 to 93a4249 Compare November 30, 2023 10:59

zmoog changed the title ~~Fleet: Add mapping for subobjects: false~~ Fleet: add support for subobjects: false Nov 30, 2023

zmoog marked this pull request as ready for review November 30, 2023 14:23

zmoog requested a review from a team as a code owner November 30, 2023 14:23

jsoriano reviewed Nov 30, 2023

View reviewed changes

botelastic bot added the Team:Fleet Team label for Observability Data Collection Fleet team label Nov 30, 2023

zmoog force-pushed the zmoog/subobjects-false branch from 93a4249 to aa0c9b6 Compare December 1, 2023 05:13

zmoog added the release_note:feature Makes this part of the condensed release notes label Dec 4, 2023

jsoriano approved these changes Dec 7, 2023

View reviewed changes

zmoog mentioned this pull request Dec 11, 2023

Add support for subobjects: false elastic/package-spec#349

Closed

zmoog and others added 8 commits December 11, 2023 18:13

Add static mapping for subobjects: false

dc45937

Add support for the basic case: - name: b.labels.* type: object object_type: keyword subjects: false that `_generateMappings()` should map to: { "properties": { "a": { "properties": { "labels": { "type": "object", "subobjects": false } } } } }

[CI] Auto-commit changed files from 'node scripts/eslint --no-cache -…

930b93d

…-fix'

Add support for cases B and D

59e7954

We consider case C invalid and we'll not support it.

[CI] Auto-commit changed files from 'node scripts/eslint --no-cache -…

7d90437

…-fix'

Remove use case A

02a572f

Use case A is non allowed in package-spec v3.

Better explain the WHY in a if statement

91336a5

Add a test case without dynamic mapping

ed3dbd6

Remove unsupported use case

522baa5

We decided [^1] this is not a supported use case, dropping it. [^1]: elastic#171826 (comment)

zmoog force-pushed the zmoog/subobjects-false branch from 4cfd6c5 to 522baa5 Compare December 11, 2023 17:18

zmoog merged commit e6e10b5 into elastic:main Dec 12, 2023
45 checks passed

zmoog deleted the zmoog/subobjects-false branch December 12, 2023 04:17

kibanamachine added the v8.13.0 label Dec 12, 2023

kibanamachine added the backport:skip This commit does not require backporting label Dec 12, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fleet: add support for subobjects: false #171826

Fleet: add support for subobjects: false #171826

zmoog commented Nov 23, 2023 •

edited

apmmachine commented Nov 23, 2023

jsoriano left a comment

jsoriano Nov 30, 2023

zmoog Dec 1, 2023

jsoriano Nov 30, 2023

zmoog Dec 1, 2023

zmoog Dec 1, 2023

jsoriano Dec 5, 2023

jsoriano Dec 7, 2023

elasticmachine commented Nov 30, 2023

zmoog commented Dec 5, 2023

jsoriano left a comment

jsoriano Dec 7, 2023

zmoog commented Dec 11, 2023

zmoog commented Dec 11, 2023

kibana-ci commented Dec 11, 2023

Fleet: add support for subobjects: false #171826

Fleet: add support for subobjects: false #171826

Conversation

zmoog commented Nov 23, 2023 • edited

Summary

Case A

Case B

Case C

Case D

Checklist

Risk Matrix

For maintainers

apmmachine commented Nov 23, 2023

🤖 GitHub comments

jsoriano left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elasticmachine commented Nov 30, 2023

zmoog commented Dec 5, 2023

jsoriano left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zmoog commented Dec 11, 2023

zmoog commented Dec 11, 2023

kibana-ci commented Dec 11, 2023

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

History

zmoog commented Nov 23, 2023 •

edited