Allow for negtive histogram buckets. (#496)

Per OM discussions. I also extended this to _gsum,
where as it's a gauge we can allow a negative value
if there's a negative bucket.

Signed-off-by: Brian Brazil <brian.brazil@robustperception.io>

Author: brian-brazil

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)
 

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):

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,21 +401,31 @@ 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)
             bucket = b
             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'))

prometheus_client/utils.py

@@ -2,6 +2,7 @@
 
 INF = float("inf")
 MINUS_INF = float("-inf")
+NaN = float("NaN")
 
 
 def floatToGoString(d):

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):

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'),

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'])

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', [