Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Fleet] Remove deprecated topics property for kafka output in favor of topic #199226

Merged

Conversation

nchaulet
Copy link
Member

@nchaulet nchaulet commented Nov 6, 2024

Description

Related to #189746

Remove deprecated property topics from output APIs in response and requests (GET|POST|PUT) /api/fleet/outputs in favor of the topic property in 9.0.

The property was still used in Fleet UI and had to made a few change there.

kibanamachine and others added 6 commits November 6, 2024 20:45
…t --include-path /api/status --include-path /api/alerting/rule/ --include-path /api/alerting/rules --include-path /api/actions --include-path /api/security/role --include-path /api/spaces --include-path /api/fleet --update'
… src/core/server/integration_tests/ci_checks'
…nchaulet/kibana into feature-remove-deprecated-kafka-topics
@nchaulet nchaulet self-assigned this Nov 7, 2024
@nchaulet nchaulet added release_note:breaking backport:skip This commit does not require backporting Team:Fleet Team label for Observability Data Collection Fleet team labels Nov 7, 2024
@nchaulet nchaulet marked this pull request as ready for review November 7, 2024 13:09
@nchaulet nchaulet requested review from a team as code owners November 7, 2024 13:09
@elasticmachine
Copy link
Contributor

Pinging @elastic/fleet (Team:Fleet)

@nchaulet nchaulet requested a review from criamico November 7, 2024 13:10
@SiddharthMantri SiddharthMantri self-requested a review November 7, 2024 13:18
Copy link
Contributor

@juliaElastic juliaElastic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

outputDoc.attributes.topic = outputDoc.attributes.topics?.filter((t) => !t.when)?.[0]?.topic;
}

delete outputDoc.attributes.topics;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fact that we're removing this field makes it non-BWC. I wonder if we can perform the migration in 2 steps:

  1. Maintains both fields
  2. (In a couple of weeks) Deletes the deprecated topics array

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@afharo since kibana 8.13 we already handle the fact that topics could not be here and the topic field is present, so I think we should not have any BWC issues

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let me know if you think this could still be an issue

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh! if 8.13+ already uses the single topic field, then all good on my end. Approving

@nchaulet nchaulet requested a review from afharo November 7, 2024 14:54
Copy link
Contributor

@criamico criamico left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I checked the changes for the dynamic topic, back when I implemented it I wasn't aware that topic was being deprecated. Thanks for taking care of it! LGTM 🚀

@nchaulet nchaulet enabled auto-merge (squash) November 8, 2024 13:34
@nchaulet
Copy link
Member Author

nchaulet commented Nov 8, 2024

@elasticmachine merge upstream

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

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.7MB 1.7MB -276.0B

History

cc @nchaulet

@nchaulet nchaulet merged commit 4a56896 into elastic:main Nov 8, 2024
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting release_note:breaking Team:Fleet Team label for Observability Data Collection Fleet team v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants