fix(metrics): terminate classic histogram buckets with +Inf - #260
Merged
TheJokr merged 1 commit intoAug 26, 2026
Merged
Conversation
`Histogram` and `TimeHistogram` appended a finite `f64::MAX` bucket as the catch-all bound, and the classic buckets borrowed from `prometheus_client` for a `NativeHistogram` carry the same sentinel. The text encoder rewrote it to `+Inf` on the way out, but `encode_to_protobuf` did not, so the two encoders disagreed and every classic bucket set scraped over protobuf gained a phantom `le="1.7976931348623157e+308"` series. Prometheus treats an infinite bound as the terminator of a classic bucket list (`IsInf` in `protobufparse.go`) and appends a synthetic `+Inf` bucket carrying the sample count when it does not find one. A finite sentinel therefore scrapes as an extra series duplicating `+Inf`, which inflates series counts and, worse, lets `histogram_quantile` interpolate between the last real bound and `f64::MAX`: a p99 above the top configured bucket returned ~1.8e308 seconds instead of being clamped to that bucket. Use `f64::INFINITY` at the source instead, and translate the sentinel where upstream's buckets enter our protobuf model. The comment claiming the finite bound was needed for the protobuf data model was wrong: `upper_bound` is an IEEE 754 double and Prometheus depends on `+Inf` appearing there. This also fixes an observation being dropped: with a finite terminal bound an `f64::INFINITY` observation matched no bucket while still incrementing the count, leaving the catch-all bucket below `_count`. The text encoder no longer rewrites the sentinel. Nothing in the crate emits it any more, and tolerating it in one encoder but not the other is what hid this. Note for downstream: tests asserting `f64::MAX` as the encoded terminal bound, or reading it back from `HistogramSnapshot::buckets`, now see `f64::INFINITY`.
TheJokr
self-requested a review
August 26, 2026 12:36
MichaHoffmann
approved these changes
Aug 26, 2026
TheJokr
approved these changes
Aug 26, 2026
Merged
dotjs
pushed a commit
that referenced
this pull request
Aug 26, 2026
### Fixed - #258: With `foundations-metrics-backend` enabled, the `process` metrics implementation relied on an accessor in the `prometheus` crate that is not present when the `protobuf` feature is enabled. This change enables the `protobuf` feature in `prometheus` instead and uses a different accessor. - #260: `NativeHistogram`, when scraped in protobuf format, now correctly exposes an upper bound of `f64::INFINITY` rather than `f64::MAX`.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Classic histogram bucket lists terminated with a finite
f64::MAXinstead off64::INFINITY.encode_to_textrewrote it to+Inf;encode_to_protobufdid not, so scrapes gained a phantomle="1.7976931348623157e+308"series andhistogram_quantileinterpolated against the sentinel (p99 returned ~1.8e308rather than clamping to the top bucket).Changes:
metrics/histogram.rs–Histogram::newandTimeHistogram::newappendf64::INFINITY.metrics/native_histogram.rs– newclassic_upper_boundtranslates the sentinel whereprometheus_client's buckets enter our protobuf model. Needed separately: upstream appendsf64::MAXitself, soNativeHistogramisn't covered by the change above.encoding/text.rs– dropped the rewrite. Nothing emits the sentinel now, and its comment wrongly claimed the finite bound was required by the protobuf data model.tests/histogram_terminal_bucket.rs– asserts the terminal bound across both encoders for classic, time and native+classic histograms.Also fixes
observe(f64::INFINITY)matching no bucket while still incrementing the count.Breaking for consumers reading the terminal bound:
HistogramSnapshot::buckets()and the encoded value are nowf64::INFINITY.cloudchamberd-rs/src/rpc_metrics.rsasserts the old value.