Skip to content

kafkareceiver: Improve internal telemetry #40846

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

1pkg
Copy link
Contributor

@1pkg 1pkg commented Jun 20, 2025

Description

This PR adds the following internal telemetry metrics to improve the receiver telemetry to make it easier to operate.

Click to expand
Name Description Type
kafka_receiver_bytes The number of received bytes, accounting for client compression sum
kafka_receiver_bytes_uncompressed The number of received uncompressed bytes sum
kafka_receiver_latency Time it took to receive a batch of records histogram
kafka_broker_connects The number of connections opened sum
kafka_broker_disconnects The number of connections closed sum
kafka_broker_throttling_duration The throttling duration imposed by the broker when receiving records histogram

Link to tracking issue

Fixes

#40816

Testing

Unit tests are updated and tested manually.

Documentation

Documentation is autogenerated with mdatagen.

Copy link
Contributor

@axw axw left a comment

Choose a reason for hiding this comment

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

Thanks! Looks good. There are a couple of breaking changes here, we'll need to be clear about that in the changelog.

telemetry:
metrics:
kafka_receiver_messages:
Copy link
Contributor

Choose a reason for hiding this comment

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

This would be a breaking change. For the receiver we should either add a new "records" metric and keep the "messages" one but deprecate it; or alternatively update the exporter to use "messages" instead of "records". Sorry, I forgot we had this one.

I'd probably update the exporter metric for now, and we could deprecate/rename later. WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm okay with either options, I agree that using records should be more "correct" way to call the metric. But if the preferred way is to introduce less breaking changes I will update the exporter PR to use messages for the metric name.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I updated both the receiver and the exporter PRs to use consistent naming for messages to not cause any more breaking changes for now. Please take a look.

@github-actions github-actions bot requested a review from pavolloffay June 25, 2025 18:28
@1pkg 1pkg requested a review from axw June 25, 2025 18:29
Copy link
Contributor

@axw axw left a comment

Choose a reason for hiding this comment

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

Looks great. Thank you!

@MovieStoreGuy MovieStoreGuy merged commit 18da435 into open-telemetry:main Jun 26, 2025
177 checks passed
@github-actions github-actions bot added this to the next release milestone Jun 26, 2025
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.

4 participants