diff --git a/docs/content/instrumenting/histogram.md b/docs/content/instrumenting/histogram.md index fa0ffe1a..37916d3f 100644 --- a/docs/content/instrumenting/histogram.md +++ b/docs/content/instrumenting/histogram.md @@ -18,6 +18,10 @@ A Histogram exposes three time series per metric: - `_sum` — sum of all observed values - `_count` — total number of observations +If the lowest bucket bound is negative, OpenMetrics 1.0 output leaves `_sum` +out, because OpenMetrics 1.0 does not allow a sum for histograms with negative +buckets. Other formats, including OpenMetrics 2.0, keep it. + ## Constructor ```python diff --git a/prometheus_client/metrics.py b/prometheus_client/metrics.py index 79c3438c..a0ad9b92 100644 --- a/prometheus_client/metrics.py +++ b/prometheus_client/metrics.py @@ -712,8 +712,7 @@ def _child_samples(self) -> Iterable[Sample]: acc += self._buckets[i].get() samples.append(Sample('_bucket', {'le': floatToGoString(bound)}, acc, None, self._buckets[i].get_exemplar())) samples.append(Sample('_count', {}, acc, None, None)) - if self._upper_bounds[0] >= 0: - samples.append(Sample('_sum', {}, self._sum.get(), None, None)) + samples.append(Sample('_sum', {}, self._sum.get(), None, None)) if _use_created: samples.append(Sample('_created', {}, self._created, None, None)) return tuple(samples) diff --git a/prometheus_client/openmetrics/exposition.py b/prometheus_client/openmetrics/exposition.py index 5d276e2f..05054f25 100644 --- a/prometheus_client/openmetrics/exposition.py +++ b/prometheus_client/openmetrics/exposition.py @@ -66,7 +66,16 @@ def generate_latest(registry, escaping=UNDERSCORES, version="1.0.0"): output.append(f'# TYPE {escape_metric_name(mname, escaping)} {metric.type}\n') if metric.unit: output.append(f'# UNIT {escape_metric_name(mname, escaping)} {metric.unit}\n') + # OpenMetrics 1.0 doesn't allow _sum on a histogram with negative buckets. + negative_bucket_labels = set() + if metric.type == 'histogram' and parse_version(version) < (2, 0, 0): + for s in metric.samples: + if s.name == mname + '_bucket' and float(s.labels['le']) < 0: + negative_bucket_labels.add(frozenset((k, v) for k, v in s.labels.items() if k != 'le')) for s in metric.samples: + if (negative_bucket_labels and s.name == mname + '_sum' + and frozenset(s.labels.items()) in negative_bucket_labels): + continue if escaping == ALLOWUTF8 and not _is_valid_legacy_metric_name(s.name): labelstr = escape_metric_name(s.name, escaping) if s.labels: diff --git a/tests/openmetrics/test_exposition.py b/tests/openmetrics/test_exposition.py index 5abc0423..d998fc0c 100644 --- a/tests/openmetrics/test_exposition.py +++ b/tests/openmetrics/test_exposition.py @@ -318,6 +318,19 @@ def test_histogram_negative_buckets(self) -> None: hh_count 1.0 hh_created 123.456 # EOF +""", generate_latest(self.registry, version="1.0.0")) + self.assertEqual(b"""# HELP hh A histogram +# TYPE hh histogram +hh_bucket{le="-1.0"} 0.0 +hh_bucket{le="-0.5"} 1.0 +hh_bucket{le="0.0"} 1.0 +hh_bucket{le="0.5"} 1.0 +hh_bucket{le="1.0"} 1.0 +hh_bucket{le="+Inf"} 1.0 +hh_count 1.0 +hh_sum -0.5 +hh_created 123.456 +# EOF """, generate_latest(self.registry, version="2.0.0")) def test_histogram_exemplar(self) -> None: diff --git a/tests/test_multiprocess.py b/tests/test_multiprocess.py index c0acfadd..9a9efeda 100644 --- a/tests/test_multiprocess.py +++ b/tests/test_multiprocess.py @@ -15,6 +15,8 @@ from prometheus_client.multiprocess import ( mark_process_dead, MultiProcessCollector, ) +from prometheus_client.openmetrics.exposition import \ + generate_latest as openmetrics_generate_latest from prometheus_client.values import ( get_value_class, MultiProcessValue, MutexValue, ) @@ -123,6 +125,21 @@ def test_histogram_nan(self): self.assertEqual(2, self.registry.get_sample_value('h_count')) self.assertTrue(math.isnan(self.registry.get_sample_value('h_sum'))) + def test_histogram_negative_buckets(self): + h1 = Histogram('h', 'help', labelnames=['l'], buckets=[-1, 0, 1], registry=None) + values.ValueClass = MultiProcessValue(lambda: 456) + h2 = Histogram('h', 'help', labelnames=['l'], buckets=[-1, 0, 1], registry=None) + h1.labels('x').observe(-0.5) + h2.labels('x').observe(2) + + self.assertEqual(1, self.registry.get_sample_value('h_bucket', {'l': 'x', 'le': '0.0'})) + self.assertEqual(2, self.registry.get_sample_value('h_bucket', {'l': 'x', 'le': '+Inf'})) + self.assertEqual(2, self.registry.get_sample_value('h_count', {'l': 'x'})) + self.assertEqual(1.5, self.registry.get_sample_value('h_sum', {'l': 'x'})) + # OpenMetrics 1.0 doesn't allow _sum with negative buckets, 2.0 does. + self.assertNotIn(b'h_sum', openmetrics_generate_latest(self.registry, version='1.0.0')) + self.assertIn(b'h_sum{l="x"} 1.5', openmetrics_generate_latest(self.registry, version='2.0.0')) + def test_gauge_all(self): g1 = Gauge('g', 'help', registry=None) values.ValueClass = MultiProcessValue(lambda: 456)