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

workload: fix quantile values in openmetrics exporter #135252

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sambhav-jain-16
Copy link
Contributor

@sambhav-jain-16 sambhav-jain-16 commented Nov 15, 2024

hdrHistogram library has quantiles in percentages rather than absolute
values. Openmetrics standard forces the quantiles values in summary
metric to be absolute values b/w 0 and 1 inclusive.
https://github.com/prometheus/OpenMetrics/blob/296468bc2359ebac83f24301b54a0871f2268016/specification/OpenMetrics.md?plain=1#L294

Earlier the exporter would export the percentage value, this change
intends to make it absolute to adhere to openmetrics standard

Epic: none

Release note: None

@sambhav-jain-16 sambhav-jain-16 requested a review from a team as a code owner November 15, 2024 10:53
@sambhav-jain-16 sambhav-jain-16 requested review from herkolategan and vidit-bhat and removed request for a team November 15, 2024 10:53
@cockroach-teamcity
Copy link
Member

This change is Reviewable

Copy link
Collaborator

@herkolategan herkolategan left a comment

Choose a reason for hiding this comment

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

Which other tests use 50.0 vs. 0.5? Curious as the OpenMetrics standard seems to suggest this format: https://github.com/prometheus/OpenMetrics/blob/main/specification/OpenMetrics.md#summary-1

@sambhav-jain-16
Copy link
Contributor Author

Which other tests use 50.0 vs. 0.5? Curious as the OpenMetrics standard seems to suggest this format: https://github.com/prometheus/OpenMetrics/blob/main/specification/OpenMetrics.md#summary-1

Every other tests using the workload binary and exporting the histogram summaries from it eg tpcc, ycsb etc.
Actually I checked and looks like this needs to be fixed in the openmetrics exporter itself rather than here.

I'm using expfmt.MetricFamilyToOpenMetrics from prometheus library so I assumed it will emit in right format. Looks like I was wrong.
I'll push the fixes in the workload binary in this PR itself. Thanks @herkolategan for pointing out.

@sambhav-jain-16 sambhav-jain-16 changed the title roachtest/tests: fix quantile value in tpce workload: fix quantile values in openmetrics exporter Nov 15, 2024
`hdrHistogram` library has quantiles in percentages rather than absolute
values. Openmetrics standard forces the quantiles values in `summary`
metric to be absolute values b/w 0 and 1 inclusive.
https://github.com/prometheus/OpenMetrics/blob/296468bc2359ebac83f24301b54a0871f2268016/specification/OpenMetrics.md?plain=1#L294

Earlier the exporter would export the percentage value, this change
intends to make it absolute to adhere to openmetrics standard

Epic: none

Release note: None
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.

3 participants