Catch name clashes in the OM parser.

prometheus_client/metrics_core.py

@@ -216,11 +216,11 @@ def add_metric(self, labels, buckets, sum_value, timestamp=None):
                 timestamp,
                 exemplar,
             ))
-        # +Inf is last and provides the count value.
-        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.
+        # Don't include sum and thus count if there's negative buckets.
         if float(buckets[0][0]) >= 0 and sum_value is not None:
+            # +Inf is last and provides the count value.
+            self.samples.append(
+                Sample(self.name + '_count', dict(zip(self._labelnames, labels)), buckets[-1][1], timestamp))
             self.samples.append(
                 Sample(self.name + '_sum', dict(zip(self._labelnames, labels)), sum_value, timestamp))
 

prometheus_client/openmetrics/parser.py

@@ -25,7 +25,7 @@ def text_string_to_metric_families(text):
         yield metric_family
 
 
-_CANONICAL_NUMBERS = set([i / 1000.0 for i in range(10000)] + [10.0**i for i in range(-10, 11)] + [float("inf")])
+_CANONICAL_NUMBERS = set([float("inf")])
 
 
 def _isUncanonicalNumber(s):
@@ -360,7 +360,7 @@ def _parse_remaining_text(text):
     exemplar = None
     if exemplar_labels is not None:
         exemplar_length = sum([len(k) + len(v) for k, v in exemplar_labels.items()])
-        if exemplar_length > 64:
+        if exemplar_length > 128:
             raise ValueError("Exmplar labels are too long: " + text)
         exemplar = Exemplar(
             exemplar_labels,
@@ -399,6 +399,12 @@ 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_sum and count is None:
+            raise ValueError("_count must be present if _sum is present: " + name)
+        if has_gsum and count is None:
+            raise ValueError("_gcount must be present if _gsum is present: " + name)
+        if not (has_sum or has_gsum) and count is not None:
+            raise ValueError("_sum/_gsum must be present if _count is present: " + 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:
@@ -414,6 +420,7 @@ def do_checks():
             bucket = None
             has_negative_buckets = False
             has_sum = False
+            has_gsum = False
             has_negative_gsum = False
             value = 0
         group = g
@@ -433,8 +440,10 @@ def do_checks():
             count = s.value
         elif suffix in ['_sum']:
             has_sum = True
-        elif suffix in ['_gsum'] and s.value < 0:
-            has_negative_gsum = True
+        elif suffix in ['_gsum']:
+            has_gsum = True
+            if s.value < 0:
+                has_negative_gsum = True
 
     if group is not None:
         do_checks()
@@ -453,14 +462,22 @@ def text_fd_to_metric_families(fd):
     allowed_names = []
     eof = False
 
-    seen_metrics = set()
+    seen_names = set()
+    type_suffixes = {
+        'counter': ['_total', '_created'],
+        'summary': ['', '_count', '_sum', '_created'],
+        'histogram': ['_count', '_sum', '_bucket', '_created'],
+        'gaugehistogram': ['_gcount', '_gsum', '_bucket'],
+        'info': ['_info'],
+    }
 
     def build_metric(name, documentation, typ, unit, samples):
-        if name in seen_metrics:
-            raise ValueError("Duplicate metric: " + name)
-        seen_metrics.add(name)
         if typ is None:
             typ = 'unknown'
+        for suffix in set(type_suffixes.get(typ, []) + [""]):
+            if name + suffix in seen_names:
+                raise ValueError("Clashing name: " + name + suffix)
+            seen_names.add(name + suffix)
         if documentation is None:
             documentation = ''
         if unit is None:
@@ -522,14 +539,7 @@ def build_metric(name, documentation, typ, unit, samples):
                 typ = parts[3]
                 if typ == 'untyped':
                     raise ValueError("Invalid TYPE for metric: " + line)
-                allowed_names = {
-                    'counter': ['_total', '_created'],
-                    'summary': ['_count', '_sum', '', '_created'],
-                    'histogram': ['_count', '_sum', '_bucket', '_created'],
-                    'gaugehistogram': ['_gcount', '_gsum', '_bucket'],
-                    'info': ['_info'],
-                }.get(typ, [''])
-                allowed_names = [name + n for n in allowed_names]
+                allowed_names = [name + n for n in type_suffixes.get(typ, [''])]
             elif parts[1] == 'UNIT':
                 if unit is not None:
                     raise ValueError("More than one UNIT for metric: " + line)

prometheus_client/registry.py

@@ -41,7 +41,7 @@ def unregister(self, collector):
             del self._collector_to_names[collector]
 
     def _get_names(self, collector):
-        """Get names of timeseries the collector produces."""
+        """Get names of timeseries the collector produces and clashes with."""
         desc_func = None
         # If there's a describe function, use it.
         try:
@@ -58,13 +58,14 @@ def _get_names(self, collector):
         result = []
         type_suffixes = {
             'counter': ['_total', '_created'],
-            'summary': ['', '_sum', '_count', '_created'],
+            'summary': ['_sum', '_count', '_created'],
             'histogram': ['_bucket', '_sum', '_count', '_created'],
             'gaugehistogram': ['_bucket', '_gsum', '_gcount'],
             'info': ['_info'],
         }
         for metric in desc_func():
-            for suffix in type_suffixes.get(metric.type, ['']):
+            result.append(metric.name)
+            for suffix in type_suffixes.get(metric.type, []):
                 result.append(metric.name + suffix)
         return result
 

tests/openmetrics/test_parser.py

@@ -102,12 +102,14 @@ def test_summary_quantiles(self):
 a_count 1
 a_sum 2
 a{quantile="0.5"} 0.7
+a{quantile="1"} 0.8
 # EOF
 """)
         # The Python client doesn't support quantiles, but we
         # still need to be able to parse them.
         metric_family = SummaryMetricFamily("a", "help", count_value=1, sum_value=2)
         metric_family.add_sample("a", {"quantile": "0.5"}, 0.7)
+        metric_family.add_sample("a", {"quantile": "1"}, 0.8)
         self.assertEqual([metric_family], list(families))
 
     def test_simple_histogram(self):
@@ -125,9 +127,16 @@ def test_simple_histogram(self):
     def test_histogram_noncanonical(self):
         families = text_string_to_metric_families("""# TYPE a histogram
 # HELP a help
+a_bucket{le="0"} 0
 a_bucket{le="0.00000000001"} 0
+a_bucket{le="0.0000000001"} 0
+a_bucket{le="1e-04"} 0
 a_bucket{le="1.1e-4"} 0
 a_bucket{le="1.1e-3"} 0
+a_bucket{le="1.1e-2"} 0
+a_bucket{le="1"} 0
+a_bucket{le="1e+05"} 0
+a_bucket{le="10000000000"} 0
 a_bucket{le="100000000000.0"} 0
 a_bucket{le="+Inf"} 3
 a_count 3
@@ -142,7 +151,6 @@ def test_negative_bucket_histogram(self):
 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)])],
@@ -153,14 +161,14 @@ def test_histogram_exemplars(self):
 # HELP a help
 a_bucket{le="1.0"} 0 # {a="b"} 0.5
 a_bucket{le="2.0"} 2 # {a="c"} 0.5
-a_bucket{le="+Inf"} 3 # {a="1234567890123456789012345678901234567890123456789012345678"} 4 123
+a_bucket{le="+Inf"} 3 # {a="2345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678"} 4 123
 # EOF
 """)
         hfm = HistogramMetricFamily("a", "help")
         hfm.add_sample("a_bucket", {"le": "1.0"}, 0.0, None, Exemplar({"a": "b"}, 0.5))
         hfm.add_sample("a_bucket", {"le": "2.0"}, 2.0, None, Exemplar({"a": "c"}, 0.5)),
         hfm.add_sample("a_bucket", {"le": "+Inf"}, 3.0, None,
-                       Exemplar({"a": "1234567890123456789012345678901234567890123456789012345678"}, 4,
+                       Exemplar({"a": "2345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678"}, 4,
                                 Timestamp(123, 0)))
         self.assertEqual([hfm], list(families))
 
@@ -695,7 +703,7 @@ def test_invalid_input(self):
             ('# TYPE a histogram\na_bucket{le="+Inf"} 1 # {} 1 \n# EOF\n'),
             ('# TYPE a histogram\na_bucket{le="+Inf"} 1 # {} 1 1 \n# EOF\n'),
             ('# TYPE a histogram\na_bucket{le="+Inf"} 1 # '
-             '{a="2345678901234567890123456789012345678901234567890123456789012345"} 1 1\n# EOF\n'),
+             '{a="23456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789"} 1 1\n# EOF\n'),
             ('# TYPE a histogram\na_bucket{le="+Inf"} 1 # {} 0x1p-3\n# EOF\n'),
             ('# TYPE a histogram\na_bucket{le="+Inf"} 1 # {} 1 0x1p-3\n# EOF\n'),
             # Exemplars on unallowed samples.
@@ -718,7 +726,6 @@ def test_invalid_input(self):
             ('# TYPE a summary\na{quantile="foo"} 0\n# EOF\n'),
             ('# TYPE a summary\na{quantile="1.01"} 0\n# EOF\n'),
             ('# TYPE a summary\na{quantile="NaN"} 0\n# EOF\n'),
-            ('# TYPE a summary\na{quantile="1"} 0\n# EOF\n'),
             ('# TYPE a histogram\na_bucket 0\n# EOF\n'),
             ('# TYPE a gaugehistogram\na_bucket 0\n# EOF\n'),
             ('# TYPE a stateset\na 0\n# EOF\n'),
@@ -745,16 +752,13 @@ def test_invalid_input(self):
             ('# TYPE a summary\na{quantile="0.5"} -1\n# EOF\n'),
             # Bad histograms.
             ('# TYPE a histogram\na_sum 1\n# EOF\n'),
+            ('# TYPE a histogram\na_bucket{le="+Inf"} 0\n#a_sum 0\n# EOF\n'),
+            ('# TYPE a histogram\na_bucket{le="+Inf"} 0\n#a_count 0\n# EOF\n'),
             ('# TYPE a gaugehistogram\na_gsum 1\n# EOF\n'),
+            ('# TYPE a gaugehistogram\na_bucket{le="+Inf"} 0\na_gsum 0\n# EOF\n'),
+            ('# TYPE a gaugehistogram\na_bucket{le="+Inf"} 0\na_gcount 0\n# EOF\n'),
             ('# TYPE a histogram\na_count 1\na_bucket{le="+Inf"} 0\n# EOF\n'),
             ('# TYPE a histogram\na_bucket{le="+Inf"} 0\na_count 1\n# EOF\n'),
-            ('# TYPE a histogram\na_bucket{le="0"} 0\na_bucket{le="+Inf"} 0\n# EOF\n'),
-            ('# TYPE a histogram\na_bucket{le="1"} 0\na_bucket{le="+Inf"} 0\n# EOF\n'),
-            ('# TYPE a histogram\na_bucket{le="0.0000000001"} 0\na_bucket{le="+Inf"} 0\n# EOF\n'),
-            ('# TYPE a histogram\na_bucket{le="1.1e-2"} 0\na_bucket{le="+Inf"} 0\n# EOF\n'),
-            ('# TYPE a histogram\na_bucket{le="1e-04"} 0\na_bucket{le="+Inf"} 0\n# EOF\n'),
-            ('# TYPE a histogram\na_bucket{le="1e+05"} 0\na_bucket{le="+Inf"} 0\n# EOF\n'),
-            ('# TYPE a histogram\na_bucket{le="10000000000"} 0\na_bucket{le="+Inf"} 0\n# EOF\n'),
             ('# TYPE a histogram\na_bucket{le="+INF"} 0\n# EOF\n'),
             ('# TYPE a histogram\na_bucket{le="2"} 0\na_bucket{le="1"} 0\na_bucket{le="+Inf"} 0\n# EOF\n'),
             ('# TYPE a histogram\na_bucket{le="1"} 1\na_bucket{le="2"} 1\na_bucket{le="+Inf"} 0\n# EOF\n'),
@@ -771,6 +775,10 @@ def test_invalid_input(self):
             ('# TYPE a gauge\na 0 1\na 0 0\n# EOF\n'),
             ('# TYPE a gauge\na 0\na 0 0\n# EOF\n'),
             ('# TYPE a gauge\na 0 0\na 0\n# EOF\n'),
+            # Clashing names.
+            ('# TYPE a counter\n# TYPE a counter\n# EOF\n'),
+            ('# TYPE a info\n# TYPE a counter\n# EOF\n'),
+            ('# TYPE a_created gauge\n# TYPE a counter\n# EOF\n'),
         ]:
             with self.assertRaises(ValueError, msg=case):
                 list(text_string_to_metric_families(case))

tests/test_core.py

@@ -528,9 +528,8 @@ def test_no_units_for_info_enum(self):
         self.assertRaises(ValueError, Enum, 'foo', 'help', unit="x")
 
     def test_name_cleanup_before_unit_append(self):
-        self.assertEqual(self.counter._name, 'c')
-        self.c = Counter('c_total', 'help', unit="total", labelnames=['l'], registry=self.registry)
-        self.assertEqual(self.c._name, 'c_total')
+        c = Counter('b_total', 'help', unit="total", labelnames=['l'], registry=self.registry)
+        self.assertEqual(c._name, 'b_total')
 
 
 class TestMetricFamilies(unittest.TestCase):
@@ -717,8 +716,8 @@ def test_duplicate_metrics_raises(self):
         self.assertRaises(ValueError, Gauge, 'h_sum', 'help', registry=registry)
         self.assertRaises(ValueError, Gauge, 'h_bucket', 'help', registry=registry)
         self.assertRaises(ValueError, Gauge, 'h_created', 'help', registry=registry)
-        # The name of the histogram itself isn't taken.
-        Gauge('h', 'help', registry=registry)
+        # The name of the histogram itself is also taken.
+        self.assertRaises(ValueError, Gauge, 'h', 'help', registry=registry)
 
         Info('i', 'help', registry=registry)
         self.assertRaises(ValueError, Gauge, 'i_info', 'help', registry=registry)