Catch name clashes in the OM parser.

Make sure we're catching all the cases in the client too.
This means you can't have a foo counter and a foo_created gauge.

Signed-off-by: Brian Brazil <[email protected]>

Author: brian-brazil

prometheus_client/openmetrics/parser.py

@@ -453,14 +453,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 +530,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

@@ -771,6 +771,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)