Skip to content

kafkatopicsobserver: use configkafka.ClientConfig #38597

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

Conversation

axw
Copy link
Contributor

@axw axw commented Mar 13, 2025

Description

Remove the duplicated client config from the Config struct, and embed configkafka.ClientConfig. Remove the duplicated cluster admin client creation code, and use kafka.NewSaramaClientConfig.

As a result of this change:

  • protocol_version config is no longer required, it has a default like the other Kafka components do
  • client_id config is now supported, and defaults to "otel-collector"
  • metadata config is now supported
  • validation of client config has been removed from this package, since it's covered by validation in the configkafka types

Link to tracking issue

Part of #38411

Testing

Ran the unit tests.

Documentation

Updated README.

Remove the duplicated client config from the Config struct,
and embed configkafka.ClientConfig. Remove the duplicated
cluster admin client creation code, and use kafka.NewSaramaClientConfig.

As a result of this change:
 - protocol_version is no longer required, it has a default
   like the other Kafka components do (their READMEs lie, I'll fix
   those in other PRs)
 - client_id is now supported, and defaults to "otel-collector"
 - metadata config is now supported
 - validation of client config has been removed from this package,
   since it's covered by validation in the configkafka types
@MovieStoreGuy MovieStoreGuy merged commit 8505ccf into open-telemetry:main Mar 17, 2025
175 checks passed
@github-actions github-actions bot added this to the next release milestone Mar 17, 2025
@axw axw deleted the kafkatopicsobserver-embed-clientconfig.yaml branch March 17, 2025 04:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants