[Fleet] Improve validation for dynamic Kafka topics #212422

Supplementing · 2025-02-25T17:21:39Z

Closes #206194

Summary

Screen.Recording.2025-02-26.at.7.30.18.AM.mov

Removed hardcoded wrapping of user-entered topics with %{[]} to fix issues arising from the user pre-wrapping, and also allow greater flexibility in naming
Added validation rules to check for unclosed brackets & brackets with missing % preceding
Added the auto-wrapping to the value field of items chosen from the dropdown to ensure they were always wrapped as intended
Added test coverage for validating dynamic topics

Checklist

Check the PR satisfies following conditions.

Reviewers should verify this PR satisfies this list as well.

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
If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the docker list
This was checked for breaking HTTP API changes, and any breaking changes have been approved by the breaking-change committee. The release_note:breaking label should be applied in these situations.
Flaky Test Runner was used on any tests changed
The PR description includes the appropriate Release Notes section, and the correct release_note:* label is applied per the guidelines

Identify risks

n/a

… and added wrapper to combobox items

elasticmachine · 2025-02-25T17:23:00Z

Pinging @elastic/fleet (Team:Fleet)

Supplementing · 2025-02-25T17:23:22Z

@elasticmachine merge upstream

...pplications/fleet/sections/settings/components/edit_output_flyout/output_form_validators.tsx

…coverage

…upplementing/kibana into fix-kafka-dynamic-topic-output

kpollich · 2025-02-26T17:32:48Z

Looking at the dropdown I wonder if we can change the "not recommended" text. I don't think we need to explicitly discourage the dynamic topic syntax like that.

Supplementing · 2025-02-26T17:36:10Z

Looking at the dropdown I wonder if we can change the "not recommended" text. I don't think we need to explicitly discourage the dynamic topic syntax like that.

I agree, it does make it feel like you're 'doing something wrong', when its really just a more 'advanced option' than inherently wrong, or to be discouraged.

Supplementing · 2025-02-26T17:39:16Z

@kpollich Also, do you think its worth maybe linking to the docs somewhere in the UI with it being a little bit more advanced and prone to user error? If so, @criamico mentioned it in the issue, but the docs dont explicitly lay out the multi-format string as being an option, so we likely need to get those up-to-date as well.

kpollich · 2025-02-26T19:01:12Z

@kpollich Also, do you think its worth maybe linking to the docs somewhere in the UI with it being a little bit more advanced and prone to user error? If so, @criamico mentioned it in the issue, but the docs dont explicitly lay out the multi-format string as being an option, so we likely need to get those up-to-date as well.

Yeah we can link to those docs and probably file an issue in https://github.com/elastic/ingest-docs to get those filebeat docs updated to go into more detail about the dynamic topic syntax.

kpollich

One last thing to update and this will be good to go from me

...lications/fleet/sections/settings/components/edit_output_flyout/output_form_kafka_topics.tsx

kpollich

Awesome work 🚀

kilfoyle · 2025-02-27T16:25:37Z

Yeah we can link to those docs and probably file an issue in https://github.com/elastic/ingest-docs to get those filebeat docs updated to go into more detail about the dynamic topic syntax.

@kpollich I hope it's okay: I transferred this docs issue into the beats repo (elastic/beats#42932) since (as far as I know) the ingest-docs repo has been used only for Fleet & Elastic Agent updates. This may all change soon though with the docs migration.

Thanks for opening it @Supplementing!

Supplementing · 2025-02-28T02:45:59Z

@elasticmachine merge upstream

elasticmachine · 2025-02-28T04:31:01Z

💛 Build succeeded, but was flaky

Buildkite Build
Commit: 42145e7

Failed CI Steps

Fleet Cypress Tests #1

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
`aiAssistantManagementSelection`	92.5KB	92.6KB	+65.0B
`fleet`	1.7MB	1.7MB	+797.0B
`lists`	136.5KB	136.6KB	+65.0B
total			+927.0B

Page load bundle

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

id	before	after	diff
`core`	474.6KB	474.7KB	+65.0B

History

💚 Build #279732 succeeded bfccfdb
💔 Build #279720 failed 44c3a46
💔 Build #279691 failed fdd2cd5
💔 Build #279573 failed d99c2b7
💚 Build #279252 succeeded 91da3c7
💚 Build #279193 succeeded 76f1af5

...ations/fleet/sections/settings/components/edit_output_flyout/output_form_validators.test.tsx

src/platform/packages/shared/kbn-doc-links/src/get_doc_links.ts

kilfoyle

LGTM! 🚢

Just one super small suggestion for the error text.

…upplementing/kibana into fix-kafka-dynamic-topic-output

Closes elastic#206194 ## Summary - Removed hardcoded wrapping of user-entered topics with `%{[]}` to fix issues arising from the user pre-wrapping, and also allow greater flexibility in naming - Added validation rules to check for unclosed brackets & brackets with missing `%` preceding - Added the auto-wrapping to the `value` field of items chosen from the dropdown to ensure they were always wrapped as intended ### Checklist Check the PR satisfies following conditions. Reviewers should verify this PR satisfies this list as well. - [ ] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/src/platform/packages/shared/kbn-i18n/README.md) - [ ] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials - [ ] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [ ] If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the [docker list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker) - [ ] This was checked for breaking HTTP API changes, and any breaking changes have been approved by the breaking-change committee. The `release_note:breaking` label should be applied in these situations. - [ ] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed - [ ] The PR description includes the appropriate Release Notes section, and the correct `release_note:*` label is applied per the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) ### Identify risks n/a --------- Co-authored-by: Elastic Machine <[email protected]>

This reverts commit 5903c7a.

Reverts #212422 We don't want to surface this Beats-specific syntax for dynamic topics because it greatly harms our ability to migrate users to OTel. The recommended way to handle complex multi-field interpolations for topic names is to use an `add_field` Beats processor to add the desired topic name as a distinct field, and then use that field for the dynamic topic name in the Fleet-managed Kafka output. I'll be filing a docs issue to get this in our public docs as well.

…#219415) Reverts elastic#212422 We don't want to surface this Beats-specific syntax for dynamic topics because it greatly harms our ability to migrate users to OTel. The recommended way to handle complex multi-field interpolations for topic names is to use an `add_field` Beats processor to add the desired topic name as a distinct field, and then use that field for the dynamic topic name in the Fleet-managed Kafka output. I'll be filing a docs issue to get this in our public docs as well.

removed hardcoded wrapper, adjusted validation for matching brackets,…

e655377

… and added wrapper to combobox items

Supplementing requested a review from a team as a code owner February 25, 2025 17:21

Supplementing added release_note:skip Skip the PR/issue when compiling release notes backport:skip This PR does not require backporting Team:Fleet Team label for Observability Data Collection Fleet team labels Feb 25, 2025

Merge branch 'main' into fix-kafka-dynamic-topic-output

76f1af5

kpollich requested changes Feb 25, 2025

View reviewed changes

Supplementing added 2 commits February 25, 2025 14:01

cleaned up variable names, hoisted function to top level, added test …

734d346

…coverage

Merge branch 'fix-kafka-dynamic-topic-output' of https://github.com/S…

91da3c7

…upplementing/kibana into fix-kafka-dynamic-topic-output

Supplementing requested a review from kpollich February 25, 2025 21:12

Supplementing enabled auto-merge (squash) February 25, 2025 21:15

Supplementing added 2 commits February 26, 2025 14:05

removed verbiage around not recommending, added doc link

c006413

fixed verbiage and message id

d99c2b7

Supplementing mentioned this pull request Feb 26, 2025

[REQUEST]: Kafka output configuraton docs needs more information regarding the dynamic topic syntax. elastic/beats#42932

Open

kpollich requested changes Feb 27, 2025

View reviewed changes

...lications/fleet/sections/settings/components/edit_output_flyout/output_form_kafka_topics.tsx Outdated Show resolved Hide resolved

...lications/fleet/sections/settings/components/edit_output_flyout/output_form_kafka_topics.tsx Show resolved Hide resolved

updated harcoded url to use doc links service

fdd2cd5

Supplementing requested a review from a team as a code owner February 27, 2025 12:37

Supplementing requested a review from kpollich February 27, 2025 12:39

Supplementing added 3 commits February 27, 2025 06:44

fixed translation error?

1e16d78

missed a period, maybe fixed translation now?

44c3a46

ran translation fix for embedded link

bfccfdb

kpollich approved these changes Feb 27, 2025

View reviewed changes

Merge branch 'main' into fix-kafka-dynamic-topic-output

42145e7

kilfoyle reviewed Mar 4, 2025

View reviewed changes

...ations/fleet/sections/settings/components/edit_output_flyout/output_form_validators.test.tsx Outdated Show resolved Hide resolved

kilfoyle reviewed Mar 4, 2025

View reviewed changes

src/platform/packages/shared/kbn-doc-links/src/get_doc_links.ts Show resolved Hide resolved

kilfoyle approved these changes Mar 4, 2025

View reviewed changes

Supplementing and others added 3 commits March 4, 2025 08:13

updated verbiage per docs team recommendation on error text/tests

29d7fc7

Merge branch 'fix-kafka-dynamic-topic-output' of https://github.com/S…

2e9b856

…upplementing/kibana into fix-kafka-dynamic-topic-output

Merge branch 'main' into fix-kafka-dynamic-topic-output

440fa36

Supplementing merged commit 5903c7a into elastic:main Mar 4, 2025
9 checks passed

kibanamachine added the v9.1.0 label Mar 4, 2025

kpollich added a commit that referenced this pull request Apr 28, 2025

Revert "[Fleet] Improve validation for dynamic Kafka topics (#212422)"

eeab9d6

This reverts commit 5903c7a.

This was referenced Apr 28, 2025

Revert "[Fleet] Improve validation for dynamic Kafka topics" #219415

Merged

Fleet : Kafka output : Dynamic Topic : Kibana wraps the custom topic name with a leading '%{[' and trailing ']}' pattern #206194

Closed

[Fleet] Improve validation for dynamic Kafka topics #212422

[Fleet] Improve validation for dynamic Kafka topics #212422

Uh oh!

Conversation

Supplementing commented Feb 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Checklist

Identify risks

Uh oh!

elasticmachine commented Feb 25, 2025

Uh oh!

Supplementing commented Feb 25, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kpollich commented Feb 26, 2025

Uh oh!

Supplementing commented Feb 26, 2025

Uh oh!

Supplementing commented Feb 26, 2025

Uh oh!

kpollich commented Feb 26, 2025

Uh oh!

kpollich left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

kpollich left a comment

Choose a reason for hiding this comment

Uh oh!

kilfoyle commented Feb 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Supplementing commented Feb 28, 2025

Uh oh!

elasticmachine commented Feb 28, 2025

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

Async chunks

Page load bundle

History

Uh oh!

Uh oh!

Uh oh!

kilfoyle left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Supplementing commented Feb 25, 2025 •

edited

Loading

kilfoyle commented Feb 27, 2025 •

edited

Loading