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

[exporter/clickhouse] Add client info for identifying exporter in system.query_log #37146

Open
wants to merge 27 commits into
base: main
Choose a base branch
from

Conversation

SpencerTorres
Copy link
Member

@SpencerTorres SpencerTorres commented Jan 10, 2025

Description:
(reopen from #34915)

This change will include information about the collector version and OS/Arch for every INSERT. Users can then track the origin of these queries in system.query_log. For an official build this will look something like otelcol/v0.114.0.

This is implemented by setting client_info_product in the DSN. The underlying ClickHouse Go driver will then attach it upon connection. Users can already add their own version info in endpoint or connection_params, but this change will now safely set or append to that value with the binary's information.

Documentation:

  • Added notes to README for client_product_info

Testing:

  • Add/updated unit tests

Copy link
Contributor

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions bot added the Stale label Jan 25, 2025
@SpencerTorres
Copy link
Member Author

not stale 👉

@github-actions github-actions bot removed the Stale label Jan 26, 2025
@SpencerTorres
Copy link
Member Author

@dmitryax I have updated the code to use exporter.Settings.BuildInfo.Version. Note that this may not include the architecture/os like the original implementation, although I don't think it's necessary. By default the build version is latest which may be misleading if the user's build doesn't include a specific version number.

Overall the code is simpler though, let me know if anything else stands out. Thanks!

Copy link
Contributor

@atoulme atoulme left a comment

Choose a reason for hiding this comment

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

the changes look ok to me.

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