diff --git a/prometheus_client/metrics_core.py b/prometheus_client/metrics_core.py index a962077d..4fd0fcd7 100644 --- a/prometheus_client/metrics_core.py +++ b/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)) diff --git a/prometheus_client/openmetrics/parser.py b/prometheus_client/openmetrics/parser.py index 4bbab93a..f1d6cecc 100644 --- a/prometheus_client/openmetrics/parser.py +++ b/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) diff --git a/prometheus_client/registry.py b/prometheus_client/registry.py index 7f86a000..fff1f98c 100644 --- a/prometheus_client/registry.py +++ b/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 diff --git a/tests/openmetrics/test_parser.py b/tests/openmetrics/test_parser.py index 3d64c1c7..d19f525a 100644 --- a/tests/openmetrics/test_parser.py +++ b/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)) diff --git a/tests/test_core.py b/tests/test_core.py index d1ae1150..5a81f122 100644 --- a/tests/test_core.py +++ b/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)