8000 Catch name clashes in the OM parser. · the-chieftain/client_python@5c6af2d · GitHub
[go: up one dir, main page]
Skip to content
8000

Commit 5c6af2d

Browse files
brian-brazilbrian-brazil
committed
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 <brian.brazil@robustperception.io>
1 parent 02be07a commit 5c6af2d

File tree

4 files changed

+25
-20
lines changed

4 files changed

+25
-20
lines changed

‎prometheus_client/openmetrics/parser.py

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -453,14 +453,22 @@ def text_fd_to_metric_families(fd):
453453
allowed_names = []
454454
eof = False
455455

456-
seen_metrics = set()
456+
seen_names = set()
457+
type_suffixes = {
458+
'counter': ['_total', '_created'],
459+
'summary': ['', '_count', '_sum', '_created'],
460+
'histogram': ['_count', '_sum', '_bucket', '_created'],
461+
'gaugehistogram': ['_gcount', '_gsum', '_bucket'],
462+
'info': ['_info'],
463+
}
457464

458465
def build_metric(name, documentation, typ, unit, samples):
459-
if name in seen_metrics:
460-
raise ValueError("Duplicate metric: " + name)
461-
seen_metrics.add(name)
462466
if typ is None:
463467
typ = 'unknown'
468+
for suffix in set(type_suffixes.get(typ, []) + [""]):
469+
if name + suffix in seen_names:
470+
raise ValueError("Clashing name: " + name + suffix)
471+
seen_names.add(name + suffix)
464472
if documentation is None:
465473
documentation = ''
466474
if unit is None:
@@ -522,14 +530,7 @@ def build_metric(name, documentation, typ, unit, samples):
522530
typ = parts[3]
523531
if typ == 'untyped':
524532
raise ValueError("Invalid TYPE for metric: " + line)
525-
allowed_names = {
526-
'counter': ['_total', '_created'],
527-
'summary': ['_count', '_sum', '', '_created'],
528-
'histogram': ['_count', '_sum', '_bucket', '_created'],
529-
'gaugehistogram': ['_gcount', '_gsum', '_bucket'],
530-
'info': ['_info'],
531-
}.get(typ, [''])
532-
allowed_names = [name + n for n in allowed_names]
533+
allowed_names = [name + n for n in type_suffixes.get(typ, [''])]
533534
elif parts[1] == 'UNIT':
534535
if unit is not None:
535536
raise ValueError("More than one UNIT for metric: " + line)

‎prometheus_client/registry.py

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ def unregister(self, collector):
4141
del self._collector_to_names[collector]
4242

4343
def _get_names(self, collector):
44-
"""Get names of timeseries the collector produces."""
44+
"""Get names of timeseries the collector produces and clashes with."""
4545
desc_func = None
4646
# If there's a describe function, use it.
4747
try:
@@ -58,13 +58,14 @@ def _get_names(self, collector):
5858
result = []
5959
type_suffixes = {
6060
'counter': ['_total', '_created'],
61-
'summary': ['', '_sum', '_count', '_created'],
61+
'summary': ['_sum', '_count', '_created'],
6262
'histogram': ['_bucket', '_sum', '_count', '_created'],
6363
'gaugehistogram': ['_bucket', '_gsum', '_gcount'],
6464
'info': ['_info'],
6565
}
6666
for metric in desc_func():
67-
for suffix in type_suffixes.get(metric.type, ['']):
67+
result.append(metric.name)
68+
for suffix in type_suffixes.get(metric.type, []):
6869
result.append(metric.name + suffix)
6970
return result
7071

‎tests/openmetrics/test_parser.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -771,6 +771,10 @@ def test_invalid_input(self):
771771
('# TYPE a gauge\na 0 1\na 0 0\n# EOF\n'),
772772
('# TYPE a gauge\na 0\na 0 0\n# EOF\n'),
773773
('# TYPE a gauge\na 0 0\na 0\n# EOF\n'),
774+
# Clashing names.
775+
('# TYPE a counter\n# TYPE a counter\n# EOF\n'),
776+
('# TYPE a info\n# TYPE a counter\n# EOF\n'),
777+
('# TYPE a_created gauge\n# TYPE a counter\n# EOF\n'),
774778
]:
775779
with self.assertRaises(ValueError, msg=case):
776780
list(text_string_to_metric_families(case))

‎tests/test_core.py

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -528,9 +528,8 @@ def test_no_units_for_info_enum(self):
528528
self.assertRaises(ValueError, Enum, 'foo', 'help', unit="x")
529529

530530
def test_name_cleanup_before_unit_append(self):
531-
self.assertEqual(self.counter._name, 'c')
532-
self.c = Counter('c_total', 'help', unit="total", labelnames=['l'], registry=self.registry)
533-
self.assertEqual(self.c._name, 'c_total')
531+
c = Counter('b_total', 'help', unit="total", labelnames=['l'], registry=self.registry)
532+
self.assertEqual(c._name, 'b_total')
534533

535534

536535
class TestMetricFamilies(unittest.TestCase):
@@ -717,8 +716,8 @@ def test_duplicate_metrics_raises(self):
717716
self.assertRaises(ValueError, Gauge, 'h_sum', 'help', registry=registry)
718717
self.assertRaises(ValueError, Gauge, 'h_bucket', 'help', registry=registry)
719718
self.assertRaises(ValueError, Gauge, 'h_created', 'help', registry=registry)
720-
# The name of the histogram itself isn't taken.
721-
Gauge('h', 'help', registry=registry)
719+
# The name of the histogram itself is also taken.
720+
self.assertRaises(ValueError, Gauge, 'h', 'help', registry=registry)
722721

723722
Info('i', 'help', registry=registry)
724723
self.assertRaises(ValueError, Gauge, 'i_info', 'help', registry=registry)

0 commit comments

Comments
 (0)
0