From 80851d80964832bd187f4b13922d88f9f5a2858c Mon Sep 17 00:00:00 2001 From: Gheorghita MUTU Date: Wed, 9 Sep 2026 16:59:01 +0300 Subject: [PATCH 1/3] fix(encoding): use +Inf as the implicit histogram overflow bucket bound Signed-off-by: Gheorghita MUTU --- src/encoding/prometheus_protobuf.rs | 6 ++++- src/encoding/text.rs | 2 +- src/metrics/histogram.rs | 40 ++++++++++++++++++++++++++++- 3 files changed, 45 insertions(+), 3 deletions(-) diff --git a/src/encoding/prometheus_protobuf.rs b/src/encoding/prometheus_protobuf.rs index 5ad0fad2..621dbfcc 100644 --- a/src/encoding/prometheus_protobuf.rs +++ b/src/encoding/prometheus_protobuf.rs @@ -803,7 +803,11 @@ mod tests { assert_eq!(11, histogram.bucket.len()); assert_eq!(1, histogram.bucket[0].cumulative_count); assert_eq!(1.0, histogram.bucket[0].upper_bound); - assert_eq!(f64::MAX, histogram.bucket.last().unwrap().upper_bound); + assert_eq!( + f64::INFINITY, + histogram.bucket.last().unwrap().upper_bound, + "the implicit overflow bucket must be exposed as +Inf" + ); } #[test] diff --git a/src/encoding/text.rs b/src/encoding/text.rs index 53377cdc..4b8bd0bd 100644 --- a/src/encoding/text.rs +++ b/src/encoding/text.rs @@ -426,7 +426,7 @@ impl MetricEncoder<'_> { self.write_prefix_name_unit()?; self.write_suffix("bucket")?; - if *upper_bound == f64::MAX { + if *upper_bound == f64::INFINITY { self.encode_labels(Some(&[("le", "+Inf")]))?; } else { self.encode_labels(Some(&[("le", *upper_bound)]))?; diff --git a/src/metrics/histogram.rs b/src/metrics/histogram.rs index 9b29402b..aa985ea2 100644 --- a/src/metrics/histogram.rs +++ b/src/metrics/histogram.rs @@ -323,7 +323,7 @@ impl Histogram { count: Default::default(), buckets: buckets .into_iter() - .chain(once(f64::MAX)) + .chain(once(f64::INFINITY)) .map(|upper_bound| (upper_bound, 0)) .collect(), native: None, @@ -2227,3 +2227,41 @@ mod tests { ); } } + +#[cfg(test)] +mod overflow_bucket_tests { + use super::Histogram; + + /// An `+Inf` observation must land in the overflow bucket. + /// + /// `observe_classic` routes NaN to the last bucket explicitly, but everything else through + /// `find(|(upper_bound, _)| upper_bound >= &v)`. With `f64::MAX` as the sentinel, + /// `f64::MAX >= f64::INFINITY` is false, so an infinite observation incremented `sum` and + /// `count` while incrementing NO bucket — leaving the overflow bucket short of `_count`. + /// Prometheus only hid this because it ignored the finite sentinel and synthesised its own + /// `+Inf` series from `sample_count`. + #[test] + fn infinite_observation_lands_in_the_overflow_bucket() { + let histogram = Histogram::new([1.0, 2.0]); + histogram.observe(0.5); + histogram.observe(f64::INFINITY); + histogram.observe(f64::NAN); + + let inner = histogram.inner.lock(); + let cumulative: u64 = inner.buckets.iter().map(|(_, count)| count).sum(); + assert_eq!( + cumulative, inner.count, + "every observation must be in some bucket; buckets={:?} count={}", + inner.buckets, inner.count + ); + let (last_bound, last_count) = *inner.buckets.last().expect("no buckets"); + assert!( + last_bound.is_infinite(), + "overflow bound is {last_bound:e}, not +Inf" + ); + assert_eq!( + last_count, 2, + "the +Inf and NaN observations belong to the overflow bucket" + ); + } +} From 22b59b7be6ceb6b535a57d18e68c30fc20558344 Mon Sep 17 00:00:00 2001 From: Gheorghita MUTU Date: Fri, 11 Sep 2026 17:05:53 +0300 Subject: [PATCH 2/3] fix(encoding): drop non-finite bucket bounds in Histogram::new The overflow bucket is implicit and now genuinely +Inf, so an explicitly configured f64::INFINITY bound produced two le="+Inf" series for the same metric, which is invalid exposition. client_golang strips an explicitly-configured +Inf bound for the same reason. NaN and -Inf bounds are dropped as well: observe_classic matches with upper_bound >= v, and every comparison against NaN is false, so a NaN bound could never receive an observation. Regression test asserts a single +Inf bucket both on the bucket list and on the rendered text exposition. It fails without the filter with buckets=[(1.0, 1), (inf, 0), (NaN, 0), (-inf, 0), (inf, 0)]. Signed-off-by: Gheorghita MUTU --- src/metrics/histogram.rs | 49 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/src/metrics/histogram.rs b/src/metrics/histogram.rs index aa985ea2..a4e53a49 100644 --- a/src/metrics/histogram.rs +++ b/src/metrics/histogram.rs @@ -316,6 +316,11 @@ impl Histogram { /// # use prometheus_client::metrics::histogram::Histogram; /// let histogram = Histogram::new([10.0, 100.0, 1_000.0]); /// ``` + /// + /// The overflow bucket is implicit and always present, so non-finite bounds in `buckets` are + /// dropped: passing `f64::INFINITY` explicitly would otherwise produce two `le="+Inf"` series + /// for the same metric. `client_golang` strips an explicitly-configured `+Inf` bound for the + /// same reason. A `NaN` bound is dropped too, since no observation can ever match it. pub fn new(buckets: impl IntoIterator) -> Self { Self { inner: Arc::new(Mutex::new(Inner { @@ -323,6 +328,7 @@ impl Histogram { count: Default::default(), buckets: buckets .into_iter() + .filter(|upper_bound| upper_bound.is_finite()) .chain(once(f64::INFINITY)) .map(|upper_bound| (upper_bound, 0)) .collect(), @@ -2240,6 +2246,49 @@ mod overflow_bucket_tests { /// `count` while incrementing NO bucket — leaving the overflow bucket short of `_count`. /// Prometheus only hid this because it ignored the finite sentinel and synthesised its own /// `+Inf` series from `sample_count`. + /// An explicitly-configured `+Inf` bound must not duplicate the implicit overflow bucket. + /// + /// Two buckets with the same `le` is invalid exposition, so `Histogram::new` drops non-finite + /// bounds, as `client_golang` does. + #[test] + fn explicit_infinite_bound_does_not_duplicate_the_overflow_bucket() { + use crate::encoding::text::encode; + use crate::registry::Registry; + + let histogram = Histogram::new([1.0, f64::INFINITY, f64::NAN, f64::NEG_INFINITY]); + histogram.observe(0.5); + + { + let inner = histogram.inner.lock(); + let infinite = inner + .buckets + .iter() + .filter(|(upper_bound, _)| upper_bound.is_infinite()) + .count(); + assert_eq!( + infinite, 1, + "exactly one infinite bucket expected, got buckets={:?}", + inner.buckets + ); + assert_eq!( + inner.buckets.len(), + 2, + "only the finite 1.0 bound plus the implicit overflow bucket should remain, got {:?}", + inner.buckets + ); + } + + let mut registry = Registry::default(); + registry.register("my_histogram", "My histogram", histogram); + let mut encoded = String::new(); + encode(&mut encoded, ®istry).expect("encode"); + assert_eq!( + encoded.matches(r#"le="+Inf""#).count(), + 1, + "the exposition must contain exactly one +Inf bucket:\n{encoded}" + ); + } + #[test] fn infinite_observation_lands_in_the_overflow_bucket() { let histogram = Histogram::new([1.0, 2.0]); From 73d647b3702978417797a1ae029d1a78c5da1424 Mon Sep 17 00:00:00 2001 From: Gheorghita MUTU Date: Fri, 11 Sep 2026 17:40:37 +0300 Subject: [PATCH 3/3] test(histogram): order the overflow-bucket tests to match their doc comments Signed-off-by: Gheorghita MUTU --- src/metrics/histogram.rs | 64 +++++++++++++++++----------------------- 1 file changed, 27 insertions(+), 37 deletions(-) diff --git a/src/metrics/histogram.rs b/src/metrics/histogram.rs index a4e53a49..7deaed45 100644 --- a/src/metrics/histogram.rs +++ b/src/metrics/histogram.rs @@ -2238,18 +2238,33 @@ mod tests { mod overflow_bucket_tests { use super::Histogram; - /// An `+Inf` observation must land in the overflow bucket. - /// - /// `observe_classic` routes NaN to the last bucket explicitly, but everything else through - /// `find(|(upper_bound, _)| upper_bound >= &v)`. With `f64::MAX` as the sentinel, - /// `f64::MAX >= f64::INFINITY` is false, so an infinite observation incremented `sum` and - /// `count` while incrementing NO bucket — leaving the overflow bucket short of `_count`. - /// Prometheus only hid this because it ignored the finite sentinel and synthesised its own - /// `+Inf` series from `sample_count`. - /// An explicitly-configured `+Inf` bound must not duplicate the implicit overflow bucket. - /// - /// Two buckets with the same `le` is invalid exposition, so `Histogram::new` drops non-finite - /// bounds, as `client_golang` does. + /// Every observation, including `+Inf` and `NaN`, must be counted in some bucket. + #[test] + fn infinite_observation_lands_in_the_overflow_bucket() { + let histogram = Histogram::new([1.0, 2.0]); + histogram.observe(0.5); + histogram.observe(f64::INFINITY); + histogram.observe(f64::NAN); + + let inner = histogram.inner.lock(); + let cumulative: u64 = inner.buckets.iter().map(|(_, count)| count).sum(); + assert_eq!( + cumulative, inner.count, + "every observation must be in some bucket; buckets={:?} count={}", + inner.buckets, inner.count + ); + let (last_bound, last_count) = *inner.buckets.last().expect("no buckets"); + assert!( + last_bound.is_infinite(), + "overflow bound is {last_bound:e}, not +Inf" + ); + assert_eq!( + last_count, 2, + "the +Inf and NaN observations belong to the overflow bucket" + ); + } + + /// An explicit `+Inf` bound must not duplicate the implicit overflow bucket. #[test] fn explicit_infinite_bound_does_not_duplicate_the_overflow_bucket() { use crate::encoding::text::encode; @@ -2288,29 +2303,4 @@ mod overflow_bucket_tests { "the exposition must contain exactly one +Inf bucket:\n{encoded}" ); } - - #[test] - fn infinite_observation_lands_in_the_overflow_bucket() { - let histogram = Histogram::new([1.0, 2.0]); - histogram.observe(0.5); - histogram.observe(f64::INFINITY); - histogram.observe(f64::NAN); - - let inner = histogram.inner.lock(); - let cumulative: u64 = inner.buckets.iter().map(|(_, count)| count).sum(); - assert_eq!( - cumulative, inner.count, - "every observation must be in some bucket; buckets={:?} count={}", - inner.buckets, inner.count - ); - let (last_bound, last_count) = *inner.buckets.last().expect("no buckets"); - assert!( - last_bound.is_infinite(), - "overflow bound is {last_bound:e}, not +Inf" - ); - assert_eq!( - last_count, 2, - "the +Inf and NaN observations belong to the overflow bucket" - ); - } }