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

fix(metrics): use f64::MAX as histogram bound to display +Inf properly #16701

Merged
merged 3 commits into from
Oct 29, 2024

Conversation

flaneur2020
Copy link
Member

@flaneur2020 flaneur2020 commented Oct 28, 2024

I hereby agree to the terms of the CLA available at: https://docs.databend.com/dev/policies/cla/

Summary

on rendering the prometheus metrics, prometheus-client checks the upper bound with f64::MAX, and renders it as +Inf, which is a marker in calculating histogram_quantile on monitoring.

if +Inf is not displayed properly, it seems that histogram_quantile will return nothing.

the source look likes this:

            self.write_prefix_name_unit()?;
            self.write_suffix("bucket")?;

            if *upper_bound == f64::MAX {
                self.encode_labels(Some(&[("le", "+Inf")]))?;
            } else {
                self.encode_labels(Some(&[("le", *upper_bound)]))?;
            }

            self.writer.write_str(" ")?;
            self.writer
                .write_str(itoa::Buffer::new().format(cummulative))?;

to display +Inf properly, we need let the upper bound with f64::MAX.

Tests

  • Unit test

Type of change

  • Bug Fix (non-breaking change which fixes an issue)

This change is Reviewable

@github-actions github-actions bot added the pr-bugfix this PR patches a bug in codebase label Oct 28, 2024
@flaneur2020 flaneur2020 requested a review from zhang2014 October 28, 2024 10:38
@zhang2014 zhang2014 added this pull request to the merge queue Oct 29, 2024
@zhang2014 zhang2014 removed this pull request from the merge queue due to a manual request Oct 29, 2024
@zhang2014 zhang2014 added the ci-cloud Build docker image for cloud test label Oct 29, 2024
Copy link
Contributor

Docker Image for PR

  • tag: pr-16701-e2f19db-1730194908

note: this image tag is only available for internal use,
please check the internal doc for more details.

@zhang2014
Copy link
Member

Cloud test passed

@zhang2014 zhang2014 added this pull request to the merge queue Oct 29, 2024
@BohuTANG BohuTANG removed this pull request from the merge queue due to a manual request Oct 29, 2024
@BohuTANG BohuTANG merged commit 00393c6 into databendlabs:main Oct 29, 2024
80 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci-cloud Build docker image for cloud test pr-bugfix this PR patches a bug in codebase
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants