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

Add kafka.request.time.avg #1135

Merged
merged 1 commit into from
Jan 4, 2024
Merged

Conversation

mackjmr
Copy link
Member

@mackjmr mackjmr commented Dec 15, 2023

Description:
This PR adds metric kafka.request.time.avg for kafka.network:type=RequestMetrics,name=TotalTimeMs,request={Produce,FetchConsumer,FetchFollower}. It does so by collecting from the Mean attribute.

Existing Issue(s):
This is a small addition so didn't open an issue.

Testing:
Added test case in KafkaIntegrationTest.java

Documentation:
Added documentation in kafka.md

Outstanding items:

< Anything that these changes are intentionally missing
that will be needed or can be helpful in the future. >

This PR adds metric `kafka.request.time.avg` for `kafka.network:type=RequestMetrics,name=TotalTimeMs,request={Produce,FetchConsumer,FetchFollower}`. It does so by collecting from the `Mean` attribute.`
Copy link
Contributor

@breedx-splk breedx-splk left a comment

Choose a reason for hiding this comment

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

Thanks @mackjmr . I'm ok with this approach. It's not your fault, but I'm a little bummed that each of these aggregations is split out into its own metric...but it's the existing pattern, and it's what we have for now.

@mackjmr
Copy link
Member Author

mackjmr commented Dec 21, 2023

Thanks for the review @breedx-splk, is this good to be merged ?

@breedx-splk
Copy link
Contributor

@open-telemetry/java-contrib-maintainers little help?

@trask trask merged commit 8fe0252 into open-telemetry:main Jan 4, 2024
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants