diff --git a/prometheus_client/metrics.py b/prometheus_client/metrics.py index 218bf705..4094449f 100644 --- a/prometheus_client/metrics.py +++ b/prometheus_client/metrics.py @@ -566,7 +566,8 @@ def _child_samples(self): acc += self._buckets[i].get() samples.append(('_bucket', {'le': floatToGoString(bound)}, acc)) samples.append(('_count', {}, acc)) - samples.append(('_sum', {}, self._sum.get())) + if self._upper_bounds[0] >= 0: + samples.append(('_sum', {}, self._sum.get())) samples.append(('_created', {}, self._created)) return tuple(samples) diff --git a/prometheus_client/metrics_core.py b/prometheus_client/metrics_core.py index d8873557..503d7fc5 100644 --- a/prometheus_client/metrics_core.py +++ b/prometheus_client/metrics_core.py @@ -183,8 +183,8 @@ class HistogramMetricFamily(Metric): def __init__(self, name, documentation, buckets=None, sum_value=None, labels=None, unit=''): Metric.__init__(self, name, documentation, 'histogram', unit) - if (sum_value is None) != (buckets is None): - raise ValueError('buckets and sum_value must be provided together.') + if sum_value is not None and buckets is None: + raise ValueError('sum value cannot be provided without buckets.') if labels is not None and buckets is not None: raise ValueError('Can only specify at most one of buckets and labels.') if labels is None: @@ -217,10 +217,13 @@ def add_metric(self, labels, buckets, sum_value, timestamp=None): exemplar, )) # +Inf is last and provides the count value. - self.samples.extend([ - Sample(self.name + '_count', dict(zip(self._labelnames, labels)), buckets[-1][1], timestamp), - Sample(self.name + '_sum', dict(zip(self._labelnames, labels)), sum_value, timestamp), - ]) + self.samples.append( + Sample(self.name + '_count', dict(zip(self._labelnames, labels)), buckets[-1][1], timestamp)) + # Don't iunclude sum if there's negative buckets. + if float(buckets[0][0]) >= 0 and sum_value is not None: + self.samples.append( + Sample(self.name + '_sum', dict(zip(self._labelnames, labels)), sum_value, timestamp)) + class GaugeHistogramMetricFamily(Metric): diff --git a/prometheus_client/openmetrics/parser.py b/prometheus_client/openmetrics/parser.py index 000b55bc..a167ae6e 100644 --- a/prometheus_client/openmetrics/parser.py +++ b/prometheus_client/openmetrics/parser.py @@ -389,6 +389,10 @@ def do_checks(): raise ValueError("+Inf bucket missing: " + name) if count is not None and value != count: raise ValueError("Count does not match +Inf value: " + name) + if has_negative_buckets and has_sum: + raise ValueError("Cannot have _sum with negative buckets: " + name) + if not has_negative_buckets and has_negative_gsum: + raise ValueError("Cannot have negative _gsum with non-negative buckets: " + name) for s in samples: suffix = s.name[len(name):] @@ -397,14 +401,19 @@ def do_checks(): if group is not None: do_checks() count = None - bucket = -1 + bucket = None + has_negative_buckets = False + has_sum = False + has_negative_gsum = False value = 0 group = g timestamp = s.timestamp if suffix == '_bucket': b = float(s.labels['le']) - if b <= bucket: + if b < 0: + has_negative_buckets = True + if bucket is not None and b <= bucket: raise ValueError("Buckets out of order: " + name) if s.value < value: raise ValueError("Bucket values out of order: " + name) @@ -412,6 +421,11 @@ def do_checks(): value = s.value elif suffix in ['_count', '_gcount']: count = s.value + elif suffix in ['_sum']: + has_sum = True + elif suffix in ['_gsum'] and s.value < 0: + has_negative_gsum = True + if group is not None: do_checks() @@ -529,7 +543,7 @@ def build_metric(name, documentation, typ, unit, samples): if typ == 'stateset' and name not in sample.labels: raise ValueError("Stateset missing label: " + line) if (typ in ['histogram', 'gaugehistogram'] and name + '_bucket' == sample.name - and (float(sample.labels.get('le', -1)) < 0 + and (sample.labels.get('le', "NaN") == "NaN" or sample.labels['le'] != floatToGoString(sample.labels['le']))): raise ValueError("Invalid le label: " + line) if (typ == 'summary' and name == sample.name @@ -567,8 +581,7 @@ def build_metric(name, documentation, typ, unit, samples): if sample.name[len(name):] in ['_total', '_sum', '_count', '_bucket', '_gcount', '_gsum'] and math.isnan( sample.value): raise ValueError("Counter-like samples cannot be NaN: " + line) - if sample.name[len(name):] in ['_total', '_sum', '_count', '_bucket', '_gcount', - '_gsum'] and sample.value < 0: + if sample.name[len(name):] in ['_total', '_sum', '_count', '_bucket', '_gcount'] and sample.value < 0: raise ValueError("Counter-like samples cannot be negative: " + line) if sample.exemplar and not ( (typ in ['histogram', 'gaugehistogram'] and sample.name.endswith('_bucket')) diff --git a/prometheus_client/utils.py b/prometheus_client/utils.py index a9c9cd21..1ff77c3c 100644 --- a/prometheus_client/utils.py +++ b/prometheus_client/utils.py @@ -2,6 +2,7 @@ INF = float("inf") MINUS_INF = float("-inf") +NaN = float("NaN") def floatToGoString(d): diff --git a/tests/openmetrics/test_exposition.py b/tests/openmetrics/test_exposition.py index 502a45e0..05791de1 100644 --- a/tests/openmetrics/test_exposition.py +++ b/tests/openmetrics/test_exposition.py @@ -89,6 +89,22 @@ def test_histogram(self): hh_sum 0.05 hh_created 123.456 # EOF +""", generate_latest(self.registry)) + + def test_histogram_negative_buckets(self): + s = Histogram('hh', 'A histogram', buckets=[-1, -0.5, 0, 0.5, 1], registry=self.registry) + s.observe(-0.5) + 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_created 123.456 +# EOF """, generate_latest(self.registry)) def test_histogram_exemplar(self): @@ -148,6 +164,18 @@ def test_gaugehistogram(self): gh_gcount 5.0 gh_gsum 7.0 # EOF +""", generate_latest(self.registry)) + + def test_gaugehistogram_negative_buckets(self): + self.custom_collector( + GaugeHistogramMetricFamily('gh', 'help', buckets=[('-1.0', 4), ('+Inf', (5))], gsum_value=-7)) + self.assertEqual(b"""# HELP gh help +# TYPE gh gaugehistogram +gh_bucket{le="-1.0"} 4.0 +gh_bucket{le="+Inf"} 5.0 +gh_gcount 5.0 +gh_gsum -7.0 +# EOF """, generate_latest(self.registry)) def test_info(self): diff --git a/tests/openmetrics/test_parser.py b/tests/openmetrics/test_parser.py index b5585762..abdf1816 100644 --- a/tests/openmetrics/test_parser.py +++ b/tests/openmetrics/test_parser.py @@ -122,6 +122,18 @@ def test_simple_histogram(self): self.assertEqual([HistogramMetricFamily("a", "help", sum_value=2, buckets=[("1.0", 0.0), ("+Inf", 3.0)])], list(families)) + def test_negative_bucket_histogram(self): + families = text_string_to_metric_families("""# TYPE a histogram +# HELP a help +a_bucket{le="-1.0"} 0 +a_bucket{le="1.0"} 1 +a_bucket{le="+Inf"} 3 +a_count 3 +# EOF +""") + self.assertEqual([HistogramMetricFamily("a", "help", buckets=[("-1.0", 0.0), ("1.0", 1.0), ("+Inf", 3.0)])], + list(families)) + def test_histogram_exemplars(self): families = text_string_to_metric_families("""# TYPE a histogram # HELP a help @@ -150,6 +162,19 @@ def test_simple_gaugehistogram(self): self.assertEqual([GaugeHistogramMetricFamily("a", "help", gsum_value=2, buckets=[("1.0", 0.0), ("+Inf", 3.0)])], list(families)) + def test_negative_bucket_gaugehistogram(self): + families = text_string_to_metric_families("""# TYPE a gaugehistogram +# HELP a help +a_bucket{le="-1.0"} 1 +a_bucket{le="1.0"} 2 +a_bucket{le="+Inf"} 3 +a_gcount 3 +a_gsum -5 +# EOF +""") + self.assertEqual([GaugeHistogramMetricFamily("a", "help", gsum_value=-5, buckets=[("-1.0", 1.0), ("1.0", 2.0), ("+Inf", 3.0)])], + list(families)) + def test_gaugehistogram_exemplars(self): families = text_string_to_metric_families("""# TYPE a gaugehistogram # HELP a help @@ -689,6 +714,8 @@ def test_invalid_input(self): ('# TYPE a histogram\na_sum -1\n# EOF\n'), ('# TYPE a histogram\na_count -1\n# EOF\n'), ('# TYPE a histogram\na_bucket{le="+Inf"} -1\n# EOF\n'), + ('# TYPE a histogram\na_bucket{le="-1.0"} 1\na_bucket{le="+Inf"} 2\na_sum -1\n# EOF\n'), + ('# TYPE a histogram\na_bucket{le="-1.0"} 1\na_bucket{le="+Inf"} 2\na_sum 1\n# EOF\n'), ('# TYPE a gaugehistogram\na_bucket{le="+Inf"} NaN\n# EOF\n'), ('# TYPE a gaugehistogram\na_bucket{le="+Inf"} -1\na_gcount -1\n# EOF\n'), ('# TYPE a gaugehistogram\na_bucket{le="+Inf"} -1\n# EOF\n'), diff --git a/tests/test_core.py b/tests/test_core.py index 4d658a68..2d6f83f4 100644 --- a/tests/test_core.py +++ b/tests/test_core.py @@ -657,7 +657,7 @@ def test_bad_constructors(self): self.assertRaises(ValueError, SummaryMetricFamily, 's', 'help', count_value=1, sum_value=1, labels=['a']) self.assertRaises(ValueError, HistogramMetricFamily, 'h', 'help', sum_value=1) - self.assertRaises(ValueError, HistogramMetricFamily, 'h', 'help', buckets={}) + self.assertRaises(KeyError, HistogramMetricFamily, 'h', 'help', buckets={}) self.assertRaises(ValueError, HistogramMetricFamily, 'h', 'help', sum_value=1, labels=['a']) self.assertRaises(ValueError, HistogramMetricFamily, 'h', 'help', buckets={}, labels=['a']) self.assertRaises(ValueError, HistogramMetricFamily, 'h', 'help', buckets={}, sum_value=1, labels=['a']) diff --git a/tests/test_exposition.py b/tests/test_exposition.py index 44b197a1..db73e237 100644 --- a/tests/test_exposition.py +++ b/tests/test_exposition.py @@ -386,7 +386,6 @@ def test_summary_metric_family(registry, count_value, sum_value, error): @pytest.mark.parametrize('MetricFamily', [ - core.HistogramMetricFamily, core.GaugeHistogramMetricFamily, ]) @pytest.mark.parametrize('buckets,sum_value,error', [