[Fleet] Support Elasticsearch output performance presets #172359

kpollich · 2023-12-01T13:34:57Z

Summary

Closes #166870
Closes #172525

Adds a new preset field to output saved objects
Updates REST spec payloads to allow preset field in POST/PUT requests to the /api/fleet/outputs endpoint
Adds logic to set default preset to balanced or custom based on whether a reserved key exists in output.config_yaml
Adds UI to the output settings flyout for providing a preset
Adds backfill logic to Fleet setup that updates all existing outputs + redeploys their associated policies to ensure the proper preset is provided on all policies

To do

Fix failing tests
Add a lot of tests + testing instructions
Allow preconfigured outputs to specify a preset
Update OpenAPI spec for outputs API
Disable EuiSelect when output is managed
Add in-product link to performance preset docs once they exist (might have to be a follow-up? (Follow up: [Fleet] Add docs link for performance presets to Fleet output flyout #172523)
Parse YML box contents instead of using basic string lookup for forcing custom preset (Follow up: [Fleet] Make custom performance preset logic more robust when checking for reserved fields in custom YAML box #172525)

How to test

Create a new Elasticsearch output
Observe the Performance preset dropdown defaults to balanced
Add a performance setting to the custom YAML box e.g. bulk_max_size: 1000
Note the callout with the list of reserved keys
Note that the dropdown switches to Custom and is now disabled
Remove the offending key
Note the dropdown returns to its normal state
Save the output
Edit the output and observe the same behaviors

For the backfill

Create a local environment with multiple elasticsearch outputs on main
Stop Kibana
Checkout this PR branch
Restart Kibana
Observe the ES outputs have been updated to include the appropriate preset value

Screenshots + Screen recordings

Screen.Recording.2023-12-01.at.8.35.59.AM.mov

apmmachine · 2023-12-01T13:35:12Z

🤖 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!)

…te --fix'

packages/kbn-check-mappings-update-cli/current_mappings.json

juliaElastic · 2023-12-04T12:16:17Z

I tested that the preset field is added to the agent policy in .fleet-policies when the output is added to the policy. The preset field is also present when I check in agent diagnostics, computed-config.yaml, so that looks good.

Tested that the API populates the preset as expected: balanced if custom_yaml is not provided, custom if provided.

// response has preset: balanced
POST kbn:/api/fleet/outputs
{"name":"local2","type":"elasticsearch","hosts":["http://localhost:9200"],"is_default":false,"is_default_monitoring":false,"config_yaml":"","proxy_id":null}

// response has preset: custom
POST kbn:/api/fleet/outputs
{"name":"local2","type":"elasticsearch","hosts":["http://localhost:9200"],"is_default":false,"is_default_monitoring":false,"config_yaml":"queue.mem.events: 3650","proxy_id":null}

When I edit an output with preset:balanced, and I add a value in the custom yaml box, should we change to preset:custom automatically? Or at least show a callout that values will be ignored unless the user sets preset:custom.

x-pack/plugins/fleet/common/types/models/output.ts

...ns/fleet/public/applications/fleet/sections/settings/components/edit_output_flyout/index.tsx

kpollich · 2023-12-04T12:55:12Z

When I edit an output with preset:balanced, and I add a value in the custom yaml box, should we change to preset:custom automatically? Or at least show a callout that values will be ignored unless the user sets preset:custom.

Yes we should show the callout.

One thing to note is that we're only doing a rudimentary string match on reserved keys, not parsing the YAML and checking the resulting key contents. So there will be some non-obvious behavior for now, e.g.

# Successfully forces `custom` preset, display callout
queue.mem.events: 1000

# Doesn't force the preset or show the callout, but would still be ignored by agent
queue:
  mem:
    events: 1000

I think this is an okay corner to cut for now, but if we have time it would be good to get a more sophisticated parsing pattern in place where we

Parse the YAML with safeLoad
Check the resulting objects' keys for a given dot-separated path

This might not be too hard with some combination of the js-yaml library's defaults and some lodash magic, so I'll give it a shot after tests are written and CI is happier. If it's taking a while, though, I think we could punt to an enhancement.

kpollich · 2023-12-04T21:08:02Z

Created follow up issues for docs + better YAML parsing:

#172523

#172525

kpollich · 2023-12-05T17:24:41Z

One thing to note is that we're only doing a rudimentary string match on reserved keys, not parsing the YAML and checking the resulting key contents. So there will be some non-obvious behavior for now, e.g.

Successfully forces custom preset, display callout

queue.mem.events: 1000

Doesn't force the preset or show the callout, but would still be ignored by agent

queue:
mem:
events: 1000

The second format here is the one most commonly used and the one used in our queue documentation in Beats (technically it is a slightly different third format): https://www.elastic.co/guide/en/beats/filebeat/current/configuring-internal-queue.html
queue.mem:
  events: 4096
  flush.min_events: 512
  flush.timeout: 5s
I think this is going to cause some confusion. If we can fix this we should. Beats and Agent YAML has supported this format for a long time so I'd expect it's the style most people would write even if we were to update our reference examples.

It doesn't have to be fixed in this PR, but we should avoid having this gap live for very long.

@cmacknz I have added actual YML parsing to the validation here + unit tests. Hence, this will now close #172525.

kpollich · 2023-12-05T17:28:04Z

x-pack/plugins/fleet/common/services/output_helpers.ts

+export function outputYmlIncludesReservedPerformanceKey(configYml: string) {
+  if (!configYml || configYml === '') {
+    return false;
+  }
+
+  const parsedYml = yaml.safeLoad(configYml);
+
+  if (!isObject(parsedYml)) {
+    return RESERVED_CONFIG_YML_KEYS.some((key) => parsedYml.includes(key));
+  }
+
+  const flattenedYml = isObject(parsedYml) ? getFlattenedObject(parsedYml) : {};
+
+  return RESERVED_CONFIG_YML_KEYS.some((key) => Object.keys(flattenedYml).includes(key));
+}


This function now parses YML if possible when checking for reserved keys

kpollich · 2023-12-05T17:28:35Z

x-pack/plugins/fleet/common/services/output_helpers.test.ts

+describe('outputYmlIncludesReservedPerformanceKey', () => {
+  describe('dot notation', () => {
+    it('returns true when reserved key is present', () => {
+      const configYml = `queue.mem.events: 1000`;
+
+      expect(outputYmlIncludesReservedPerformanceKey(configYml)).toBe(true);
+    });
+
+    it('returns false when no reserved key is present', () => {
+      const configYml = `some.random.key: 1000`;
+
+      expect(outputYmlIncludesReservedPerformanceKey(configYml)).toBe(false);
+    });
+  });
+
+  describe('object notation', () => {
+    it('returns true when reserved key is present', () => {
+      const configYml = `
+      queue:
+          mem:
+            events: 1000
+    `;
+
+      expect(outputYmlIncludesReservedPerformanceKey(configYml)).toBe(true);
+    });
+
+    it('returns false when no reserved key is present', () => {
+      const configYml = `
+        some:
+          random:
+            key: 1000
+      `;
+
+      expect(outputYmlIncludesReservedPerformanceKey(configYml)).toBe(false);
+    });
+  });
+
+  describe('plain string', () => {
+    it('returns false when no reserved key is present', () => {
+      const configYml = `bulk_max_size`;
+
+      expect(outputYmlIncludesReservedPerformanceKey(configYml)).toBe(true);
+    });
+
+    it('returns true when reserved key is present', () => {
+      const configYml = `just a string`;
+
+      expect(outputYmlIncludesReservedPerformanceKey(configYml)).toBe(false);
+    });
+  });
+});


Relevant YML parsing test cases here - we also still check plain strings even if the YML is invalid just to be sure.

kpollich · 2023-12-05T21:01:53Z

@elasticmachine merge upstream

kpollich · 2023-12-06T01:03:01Z

Bundle size blew up because of the js-yaml import in client code. Pushed 08f3d12 to lazy load the output flyout (probably something we could do for every flyout or portal, though most of them wouldn't make a large difference in bundle size)

kpollich · 2023-12-06T01:03:11Z

@elasticmachine merge upstream

kpollich · 2023-12-06T11:48:22Z

js-yaml is accounting for the massive increase in bundle size here, and doesn't support tree-shaking: nodeca/js-yaml#661. Going to see if I can come up with an alternative.

kpollich · 2023-12-06T11:52:00Z

Hmm maybe my interpretation of the webpack analyzer is wrong or I'm missing something. We import js-yaml plenty of other places in our React code so I don't see how it would have blown up the bundle size like this, e.g. we use the same `import { safeLoad } from 'js-yaml' module import in: https://github.com/elastic/kibana/blob/main/x-pack/plugins/fleet/public/applications/fleet/sections/agent_policy/create_package_policy_page/multi_page_layout/components/page_steps/add_integration.tsx

kibana-ci · 2023-12-06T14:52:10Z

💚 Build Succeeded

Buildkite Build
Commit: 3f985ac

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`fleet`	1.2MB	1.2MB	+1.9KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`fleet`	156.0KB	156.7KB	+755.0B

Unknown metric groups

ESLint disabled line counts

id	before	after	diff
`fleet`	45	44	-1

Total ESLint disabled count

id	before	after	diff
`fleet`	56	55	-1

History

💔 Build #182114 failed bd9979e
💔 Build #182047 failed 76c75b8
💔 Build #182044 failed 06f4ecd
💔 Build #181995 failed e85edf3
💔 Build #181950 failed 0e6bf4d
💔 Build #181916 failed c39d310

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

cc @kpollich

Add initial logic for output presets

a0d0024

kpollich added the Team:Fleet Team label for Observability Data Collection Fleet team label Dec 1, 2023

kpollich self-assigned this Dec 1, 2023

kpollich and others added 5 commits December 1, 2023 10:26

Disable dropdown + fix full agent policy logic

e9ceb17

Fix some tests

1f3501d

Update SO snapshot

efaa9e7

Fix FTR assertion

1a2fe05

[CI] Auto-commit changed files from 'node scripts/check_mappings_upda…

3b1cc9b

…te --fix'

juliaElastic reviewed Dec 4, 2023

View reviewed changes

packages/kbn-check-mappings-update-cli/current_mappings.json Show resolved Hide resolved

juliaElastic reviewed Dec 4, 2023

View reviewed changes

x-pack/plugins/fleet/common/types/models/output.ts Outdated Show resolved Hide resolved

juliaElastic reviewed Dec 4, 2023

View reviewed changes

...ns/fleet/public/applications/fleet/sections/settings/components/edit_output_flyout/index.tsx Outdated Show resolved Hide resolved

juliaElastic reviewed Dec 4, 2023

View reviewed changes

...ns/fleet/public/applications/fleet/sections/settings/components/edit_output_flyout/index.tsx Outdated Show resolved Hide resolved

kpollich added 10 commits December 4, 2023 08:43

Don't show UI or attempt to backfill non-ES outputs

7979402

Fix AAD exclusion + preset defaulting when updating

8d279dd

Fix cloud preconfiguration assertion

759fa06

Fix typo + FTR assertions

3965d6d

Fix output service tests

7b0440c

Add unit tests for output service

9f59844

Add cypress tests

3c24b1b

Add FTR tests

b3a19bb

Update OpenAPI spec

264ec72

Fix openAPI

4da753d

kpollich mentioned this pull request Dec 4, 2023

[Fleet] Make custom performance preset logic more robust when checking for reserved fields in custom YAML box #172525

Closed

Merge branch 'main' into fleet/166870-output-presets

1679582

kpollich marked this pull request as ready for review December 4, 2023 21:11

kpollich requested a review from a team as a code owner December 4, 2023 21:11

Fix type

5f8de5c

kpollich enabled auto-merge (squash) December 5, 2023 16:50

kpollich disabled auto-merge December 5, 2023 16:58

kpollich added 3 commits December 5, 2023 12:09

Parse YML when validating

3217e0d

Fix type again

ebee7b9

Fix YML validation on plain strings

d78a6f7

Update OpenAPI

61c4d3b

kpollich commented Dec 5, 2023

View reviewed changes

kpollich added 2 commits December 5, 2023 12:29

Fix test case names

c39d310

Fix output service test

0e6bf4d

kpollich enabled auto-merge (squash) December 5, 2023 19:06

kibanamachine and others added 2 commits December 5, 2023 16:01

Merge branch 'main' into fleet/166870-output-presets

e85edf3

Lazy load output flyout to save bundle size

08f3d12

kibanamachine and others added 3 commits December 5, 2023 20:03

Merge branch 'main' into fleet/166870-output-presets

06f4ecd

Fix missing suspense boundary

76c75b8

Try only importing safeload

6bb7594

Revert lazy conversion to stop creating separate chunk

bd9979e

kpollich disabled auto-merge December 6, 2023 12:39

kpollich added 2 commits December 6, 2023 08:29

Add DI for safeload in output helpers

0b90335

Fix tests

3f985ac

kpollich merged commit 6fe6cdd into elastic:main Dec 6, 2023
38 checks passed

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Fleet] Support Elasticsearch output performance presets #172359

[Fleet] Support Elasticsearch output performance presets #172359

kpollich commented Dec 1, 2023 •

edited

apmmachine commented Dec 1, 2023

juliaElastic commented Dec 4, 2023 •

edited

kpollich commented Dec 4, 2023

kpollich commented Dec 4, 2023 •

edited

kpollich commented Dec 5, 2023

Successfully forces `custom` preset, display callout

Doesn't force the preset or show the callout, but would still be ignored by agent

kpollich Dec 5, 2023

kpollich Dec 5, 2023

kpollich commented Dec 5, 2023

kpollich commented Dec 6, 2023

kpollich commented Dec 6, 2023

kpollich commented Dec 6, 2023

kpollich commented Dec 6, 2023

kibana-ci commented Dec 6, 2023

ESLint disabled line counts

Total ESLint disabled count

[Fleet] Support Elasticsearch output performance presets #172359

[Fleet] Support Elasticsearch output performance presets #172359

Conversation

kpollich commented Dec 1, 2023 • edited

Summary

To do

How to test

Screenshots + Screen recordings

apmmachine commented Dec 1, 2023

🤖 GitHub comments

juliaElastic commented Dec 4, 2023 • edited

kpollich commented Dec 4, 2023

kpollich commented Dec 4, 2023 • edited

kpollich commented Dec 5, 2023

Successfully forces custom preset, display callout

Doesn't force the preset or show the callout, but would still be ignored by agent

kpollich Dec 5, 2023

Choose a reason for hiding this comment

kpollich Dec 5, 2023

Choose a reason for hiding this comment

kpollich commented Dec 5, 2023

kpollich commented Dec 6, 2023

kpollich commented Dec 6, 2023

kpollich commented Dec 6, 2023

kpollich commented Dec 6, 2023

kibana-ci commented Dec 6, 2023

💚 Build Succeeded

Metrics [docs]

Async chunks

Page load bundle

ESLint disabled line counts

Total ESLint disabled count

History

kpollich commented Dec 1, 2023 •

edited

juliaElastic commented Dec 4, 2023 •

edited

kpollich commented Dec 4, 2023 •

edited

Successfully forces `custom` preset, display callout