From 8a4a65c4d2a844a2c2a072abb14a930cfe7ca207 Mon Sep 17 00:00:00 2001 From: Brian Brazil Date: Thu, 20 Sep 2018 13:25:58 +0100 Subject: [PATCH 01/14] Add gsum/gcount to GaugeHistogram. Allow gsum, gcount, and created to be sanely returned in Prometheus format. Extend openmetrics parser unittests to cover Info and StateSet. Signed-off-by: Brian Brazil --- prometheus_client/core.py | 13 +++-- prometheus_client/exposition.py | 45 +++++++++++------ prometheus_client/openmetrics/exposition.py | 3 +- prometheus_client/openmetrics/parser.py | 3 +- tests/openmetrics/test_exposition.py | 4 +- tests/openmetrics/test_parser.py | 45 +++++++++++++++++ tests/test_core.py | 4 +- tests/test_exposition.py | 55 +++++++++++++++++---- 8 files changed, 137 insertions(+), 35 deletions(-) diff --git a/prometheus_client/core.py b/prometheus_client/core.py index cb1e7c5b..ccb41348 100644 --- a/prometheus_client/core.py +++ b/prometheus_client/core.py @@ -122,6 +122,7 @@ def _get_names(self, collector): 'counter': ['_total', '_created'], 'summary': ['', '_sum', '_count', '_created'], 'histogram': ['_bucket', '_sum', '_count', '_created'], + 'gaugehistogram': ['_bucket', '_gsum', '_gcount'], 'info': ['_info'], } for metric in desc_func(): @@ -391,7 +392,7 @@ class GaugeHistogramMetricFamily(Metric): For use by custom collectors. ''' - def __init__(self, name, documentation, buckets=None, labels=None, unit=''): + def __init__(self, name, documentation, buckets=None, gsum_value=None, labels=None, unit=''): Metric.__init__(self, name, documentation, 'gaugehistogram', unit) if labels is not None and buckets is not None: raise ValueError('Can only specify at most one of buckets and labels.') @@ -399,21 +400,25 @@ def __init__(self, name, documentation, buckets=None, labels=None, unit=''): labels = [] self._labelnames = tuple(labels) if buckets is not None: - self.add_metric([], buckets) + self.add_metric([], buckets, gsum_value) - def add_metric(self, labels, buckets, timestamp=None): + def add_metric(self, labels, buckets, gsum_value, timestamp=None): '''Add a metric to the metric family. Args: labels: A list of label values buckets: A list of pairs of bucket names and values. The buckets must be sorted, and +Inf present. + gsum_value: The sum value of the metric. ''' for bucket, value in buckets: self.samples.append(Sample( self.name + '_bucket', dict(list(zip(self._labelnames, labels)) + [('le', bucket)]), value, timestamp)) + # +Inf is last and provides the count value. + self.samples.append(Sample(self.name + '_gcount', dict(zip(self._labelnames, labels)), buckets[-1][1], timestamp)) + self.samples.append(Sample(self.name + '_gsum', dict(zip(self._labelnames, labels)), gsum_value, timestamp)) class InfoMetricFamily(Metric): @@ -465,7 +470,7 @@ def add_metric(self, labels, value, timestamp=None): value: A dict of string state names to booleans ''' labels = tuple(labels) - for state, enabled in value.items(): + for state, enabled in sorted(value.items()): v = (1 if enabled else 0) self.samples.append(Sample(self.name, dict(zip(self._labelnames + (self.name,), labels + (state,))), v, timestamp)) diff --git a/prometheus_client/exposition.py b/prometheus_client/exposition.py index 1ebeba29..08686cae 100644 --- a/prometheus_client/exposition.py +++ b/prometheus_client/exposition.py @@ -67,6 +67,22 @@ def start_wsgi_server(port, addr='', registry=core.REGISTRY): def generate_latest(registry=core.REGISTRY): '''Returns the metrics from the registry in latest text format as a string.''' + + def sample_line(s): + if s.labels: + labelstr = '{{{0}}}'.format(','.join( + ['{0}="{1}"'.format( + k, v.replace('\\', r'\\').replace('\n', r'\n').replace('"', r'\"')) + for k, v in sorted(s.labels.items())])) + else: + labelstr = '' + timestamp = '' + if s.timestamp is not None: + # Convert to milliseconds. + timestamp = ' {0:d}'.format(int(float(s.timestamp) * 1000)) + return '{0}{1} {2}{3}\n'.format( + s.name, labelstr, core._floatToGoString(s.value), timestamp) + output = [] for metric in registry.collect(): mname = metric.name @@ -86,25 +102,22 @@ def generate_latest(registry=core.REGISTRY): elif mtype == 'unknown': mtype = 'untyped' - output.append('# HELP {0} {1}'.format( + output.append('# HELP {0} {1}\n'.format( mname, metric.documentation.replace('\\', r'\\').replace('\n', r'\n'))) - output.append('\n# TYPE {0} {1}\n'.format(mname, mtype)) + output.append('# TYPE {0} {1}\n'.format(mname, mtype)) + + om_samples = {} for s in metric.samples: - if s.name == metric.name + '_created': - continue # Ignore OpenMetrics specific sample. TODO: Make these into a gauge. - if s.labels: - labelstr = '{{{0}}}'.format(','.join( - ['{0}="{1}"'.format( - k, v.replace('\\', r'\\').replace('\n', r'\n').replace('"', r'\"')) - for k, v in sorted(s.labels.items())])) + for suffix in ['_created', '_gsum', '_gcount']: + if s.name == metric.name + suffix: + # OpenMetrics specific sample, put in a gauge at the end. + om_samples.setdefault(suffix, []).append(sample_line(s)) + break else: - labelstr = '' - timestamp = '' - if s.timestamp is not None: - # Convert to milliseconds. - timestamp = ' {0:d}'.format(int(float(s.timestamp) * 1000)) - output.append('{0}{1} {2}{3}\n'.format( - s.name, labelstr, core._floatToGoString(s.value), timestamp)) + output.append(sample_line(s)) + for suffix, lines in sorted(om_samples.items()): + output.append('# TYPE {0}{1} gauge\n'.format(metric.name, suffix)) + output.extend(lines) return ''.join(output).encode('utf-8') diff --git a/prometheus_client/openmetrics/exposition.py b/prometheus_client/openmetrics/exposition.py index 8b37867e..c07578b2 100644 --- a/prometheus_client/openmetrics/exposition.py +++ b/prometheus_client/openmetrics/exposition.py @@ -26,7 +26,7 @@ def generate_latest(registry): else: labelstr = '' if s.exemplar: - if metric.type != 'histogram' or not s.name.endswith('_bucket'): + if metric.type not in ('histogram', 'gaugehistogram') or not s.name.endswith('_bucket'): raise ValueError("Metric {0} has exemplars, but is not a histogram bucket".format(metric.name)) labels = '{{{0}}}'.format(','.join( ['{0}="{1}"'.format( @@ -42,7 +42,6 @@ def generate_latest(registry): exemplarstr = '' timestamp = '' if s.timestamp is not None: - # Convert to milliseconds. timestamp = ' {0}'.format(s.timestamp) output.append('{0}{1} {2}{3}{4}\n'.format(s.name, labelstr, core._floatToGoString(s.value), timestamp, exemplarstr)) diff --git a/prometheus_client/openmetrics/parser.py b/prometheus_client/openmetrics/parser.py index 8edf3a64..6517cfe8 100644 --- a/prometheus_client/openmetrics/parser.py +++ b/prometheus_client/openmetrics/parser.py @@ -302,7 +302,8 @@ def build_metric(name, documentation, typ, unit, samples): 'counter': ['_total', '_created'], 'summary': ['_count', '_sum', '', '_created'], 'histogram': ['_count', '_sum', '_bucket', 'created'], - 'gaugehistogram': ['_bucket'], + 'gaugehistogram': ['_gcount', '_gsum', '_bucket'], + 'info': ['_info'], }.get(typ, ['']) allowed_names = [name + n for n in allowed_names] elif parts[1] == 'UNIT': diff --git a/tests/openmetrics/test_exposition.py b/tests/openmetrics/test_exposition.py index 56462722..b4b08961 100644 --- a/tests/openmetrics/test_exposition.py +++ b/tests/openmetrics/test_exposition.py @@ -134,11 +134,13 @@ def collect(self): generate_latest(self.registry) def test_gaugehistogram(self): - self.custom_collector(GaugeHistogramMetricFamily('gh', 'help', buckets=[('1.0', 4), ('+Inf', (5))])) + 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)) diff --git a/tests/openmetrics/test_parser.py b/tests/openmetrics/test_parser.py index 300965af..e05ea869 100644 --- a/tests/openmetrics/test_parser.py +++ b/tests/openmetrics/test_parser.py @@ -13,10 +13,13 @@ CollectorRegistry, CounterMetricFamily, Exemplar, + GaugeHistogramMetricFamily, GaugeMetricFamily, HistogramMetricFamily, + InfoMetricFamily, Metric, Sample, + StateSetMetricFamily, SummaryMetricFamily, Timestamp, ) @@ -120,6 +123,48 @@ def test_histogram_exemplars(self): hfm.add_sample("a_bucket", {"le": "+Inf"}, 3.0, None, Exemplar({"a": "d"}, 4, Timestamp(123, 0))) self.assertEqual([hfm], list(families)) + def test_simple_gaugehistogram(self): + families = text_string_to_metric_families("""# TYPE a gaugehistogram +# HELP a help +a_bucket{le="1"} 0 +a_bucket{le="+Inf"} 3 +a_gcount 3 +a_gsum 2 +# EOF +""") + self.assertEqual([GaugeHistogramMetricFamily("a", "help", gsum_value=2, buckets=[("1", 0.0), ("+Inf", 3.0)])], list(families)) + + def test_histogram_exemplars(self): + families = text_string_to_metric_families("""# TYPE a gaugehistogram +# HELP a help +a_bucket{le="1"} 0 # {a="b"} 0.5 +a_bucket{le="2"} 2 123 # {a="c"} 0.5 +a_bucket{le="+Inf"} 3 # {a="d"} 4 123 +# EOF +""") + hfm = GaugeHistogramMetricFamily("a", "help") + hfm.add_sample("a_bucket", {"le": "1"}, 0.0, None, Exemplar({"a": "b"}, 0.5)) + hfm.add_sample("a_bucket", {"le": "2"}, 2.0, Timestamp(123, 0), Exemplar({"a": "c"}, 0.5)), + hfm.add_sample("a_bucket", {"le": "+Inf"}, 3.0, None, Exemplar({"a": "d"}, 4, Timestamp(123, 0))) + self.assertEqual([hfm], list(families)) + + def test_simple_info(self): + families = text_string_to_metric_families("""# TYPE a info +# HELP a help +a_info{foo="bar"} 1 +# EOF +""") + self.assertEqual([InfoMetricFamily("a", "help", {'foo': 'bar'})], list(families)) + + def test_simple_stateset(self): + families = text_string_to_metric_families("""# TYPE a stateset +# HELP a help +a{a="bar"} 0 +a{a="foo"} 1 +# EOF +""") + self.assertEqual([StateSetMetricFamily("a", "help", {'foo': True, 'bar': False})], list(families)) + def test_no_metadata(self): families = text_string_to_metric_families("""a 1 # EOF diff --git a/tests/test_core.py b/tests/test_core.py index fceca7ae..923b8bd7 100644 --- a/tests/test_core.py +++ b/tests/test_core.py @@ -569,10 +569,12 @@ def test_gaugehistogram(self): def test_gaugehistogram_labels(self): cmf = GaugeHistogramMetricFamily('h', 'help', labels=['a']) - cmf.add_metric(['b'], buckets=[('0', 1), ('+Inf', 2)]) + cmf.add_metric(['b'], buckets=[('0', 1), ('+Inf', 2)], gsum_value=3) self.custom_collector(cmf) self.assertEqual(1, self.registry.get_sample_value('h_bucket', {'a': 'b', 'le': '0'})) self.assertEqual(2, self.registry.get_sample_value('h_bucket', {'a': 'b', 'le': '+Inf'})) + self.assertEqual(2, self.registry.get_sample_value('h_gcount', {'a': 'b'})) + self.assertEqual(3, self.registry.get_sample_value('h_gsum', {'a': 'b'})) def test_info(self): self.custom_collector(InfoMetricFamily('i', 'help', value={'a': 'b'})) diff --git a/tests/test_exposition.py b/tests/test_exposition.py index cd5e7ebc..7e41a500 100644 --- a/tests/test_exposition.py +++ b/tests/test_exposition.py @@ -2,6 +2,7 @@ import sys import threading +import time if sys.version_info < (2, 7): # We need the skip decorators from unittest2 on Python 2.6. @@ -29,6 +30,13 @@ class TestGenerateText(unittest.TestCase): def setUp(self): self.registry = CollectorRegistry() + # Mock time so _created values are fixed. + self.old_time = time.time + time.time = lambda: 123.456 + + def tearDown(self): + time.time = self.old_time + def custom_collector(self, metric_family): class CustomCollector(object): def collect(self): @@ -38,12 +46,23 @@ def collect(self): def test_counter(self): c = Counter('cc', 'A counter', registry=self.registry) c.inc() - self.assertEqual(b'# HELP cc_total A counter\n# TYPE cc_total counter\ncc_total 1.0\n', generate_latest(self.registry)) + self.assertEqual(b'''# HELP cc_total A counter +# TYPE cc_total counter +cc_total 1.0 +# TYPE cc_created gauge +cc_created 123.456 +''', generate_latest(self.registry)) def test_counter_total(self): c = Counter('cc_total', 'A counter', registry=self.registry) c.inc() - self.assertEqual(b'# HELP cc_total A counter\n# TYPE cc_total counter\ncc_total 1.0\n', generate_latest(self.registry)) + self.assertEqual(b'''# HELP cc_total A counter +# TYPE cc_total counter +cc_total 1.0 +# TYPE cc_created gauge +cc_created 123.456 +''', generate_latest(self.registry)) + def test_gauge(self): g = Gauge('gg', 'A gauge', registry=self.registry) g.set(17) @@ -52,7 +71,13 @@ def test_gauge(self): def test_summary(self): s = Summary('ss', 'A summary', ['a', 'b'], registry=self.registry) s.labels('c', 'd').observe(17) - self.assertEqual(b'# HELP ss A summary\n# TYPE ss summary\nss_count{a="c",b="d"} 1.0\nss_sum{a="c",b="d"} 17.0\n', generate_latest(self.registry)) + self.assertEqual(b'''# HELP ss A summary +# TYPE ss summary +ss_count{a="c",b="d"} 1.0 +ss_sum{a="c",b="d"} 17.0 +# TYPE ss_created gauge +ss_created{a="c",b="d"} 123.456 +''', generate_latest(self.registry)) @unittest.skipIf(sys.version_info < (2, 7), "Test requires Python 2.7+.") def test_histogram(self): @@ -77,11 +102,21 @@ def test_histogram(self): hh_bucket{le="+Inf"} 1.0 hh_count 1.0 hh_sum 0.05 +# TYPE hh_created gauge +hh_created 123.456 ''', generate_latest(self.registry)) def test_gaugehistogram(self): - self.custom_collector(GaugeHistogramMetricFamily('gh', 'help', buckets=[('1.0', 4), ('+Inf', (5))])) - self.assertEqual(b'''# HELP gh help\n# TYPE gh histogram\ngh_bucket{le="1.0"} 4.0\ngh_bucket{le="+Inf"} 5.0\n''', generate_latest(self.registry)) + self.custom_collector(GaugeHistogramMetricFamily('gh', 'help', buckets=[('1.0', 4), ('+Inf', 5)], gsum_value=7)) + self.assertEqual(b'''# HELP gh help +# TYPE gh histogram +gh_bucket{le="1.0"} 4.0 +gh_bucket{le="+Inf"} 5.0 +# TYPE gh_gcount gauge +gh_gcount 5.0 +# TYPE gh_gsum gauge +gh_gsum 7.0 +''', generate_latest(self.registry)) def test_info(self): i = Info('ii', 'A info', ['a', 'b'], registry=self.registry) @@ -94,14 +129,14 @@ def test_enum(self): self.assertEqual(b'# HELP ee An enum\n# TYPE ee gauge\nee{a="c",b="d",ee="foo"} 0.0\nee{a="c",b="d",ee="bar"} 1.0\n', generate_latest(self.registry)) def test_unicode(self): - c = Counter('cc', '\u4500', ['l'], registry=self.registry) + c = Gauge('cc', '\u4500', ['l'], registry=self.registry) c.labels('\u4500').inc() - self.assertEqual(b'# HELP cc_total \xe4\x94\x80\n# TYPE cc_total counter\ncc_total{l="\xe4\x94\x80"} 1.0\n', generate_latest(self.registry)) + self.assertEqual(b'# HELP cc \xe4\x94\x80\n# TYPE cc gauge\ncc{l="\xe4\x94\x80"} 1.0\n', generate_latest(self.registry)) def test_escaping(self): - c = Counter('cc', 'A\ncount\\er', ['a'], registry=self.registry) - c.labels('\\x\n"').inc(1) - self.assertEqual(b'# HELP cc_total A\\ncount\\\\er\n# TYPE cc_total counter\ncc_total{a="\\\\x\\n\\""} 1.0\n', generate_latest(self.registry)) + g = Gauge('cc', 'A\ngaug\\e', ['a'], registry=self.registry) + g.labels('\\x\n"').inc(1) + self.assertEqual(b'# HELP cc A\\ngaug\\\\e\n# TYPE cc gauge\ncc{a="\\\\x\\n\\""} 1.0\n', generate_latest(self.registry)) def test_nonnumber(self): From 95c7285415edf1086a1c762fa20d13d8735dd54f Mon Sep 17 00:00:00 2001 From: Brian Brazil Date: Mon, 1 Oct 2018 15:17:51 +0100 Subject: [PATCH 02/14] Check exemplars are only where they're meant to be. Signed-off-by: Brian Brazil --- prometheus_client/openmetrics/parser.py | 11 +++++++---- tests/openmetrics/test_parser.py | 6 +++++- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/prometheus_client/openmetrics/parser.py b/prometheus_client/openmetrics/parser.py index 6517cfe8..63413761 100644 --- a/prometheus_client/openmetrics/parser.py +++ b/prometheus_client/openmetrics/parser.py @@ -250,7 +250,6 @@ def build_metric(name, documentation, typ, unit, samples): raise ValueError("Units not allowed for this metric type: " + name) metric = core.Metric(name, documentation, typ, unit) # TODO: check labelvalues are valid utf8 - # TODO: check only histogram buckets have exemplars. # TODO: check samples are appropriately grouped and ordered # TODO: check info/stateset values are 1/0 # TODO: check for metadata in middle of samples @@ -320,18 +319,22 @@ def build_metric(name, documentation, typ, unit, samples): raise ValueError("Invalid line: " + line) else: sample = _parse_sample(line) - if sample[0] not in allowed_names: + if sample.name not in allowed_names: if name != '': yield build_metric(name, documentation, typ, unit, samples) # Start an untyped metric. - name = sample[0] + name = sample.name documentation = '' unit = '' typ = 'untyped' samples = [sample] - allowed_names = [sample[0]] + allowed_names = [sample.name] else: samples.append(sample) + if sample.exemplar and not ( + typ in ['histogram', 'gaugehistogram'] + and sample.name.endswith('_bucket')): + raise ValueError("Invalid line only histogram/gaugehistogram buckets can have exemplars: " + line) if name != '': yield build_metric(name, documentation, typ, unit, samples) diff --git a/tests/openmetrics/test_parser.py b/tests/openmetrics/test_parser.py index e05ea869..d094eb3f 100644 --- a/tests/openmetrics/test_parser.py +++ b/tests/openmetrics/test_parser.py @@ -134,7 +134,7 @@ def test_simple_gaugehistogram(self): """) self.assertEqual([GaugeHistogramMetricFamily("a", "help", gsum_value=2, buckets=[("1", 0.0), ("+Inf", 3.0)])], list(families)) - def test_histogram_exemplars(self): + def test_gaugehistogram_exemplars(self): families = text_string_to_metric_families("""# TYPE a gaugehistogram # HELP a help a_bucket{le="1"} 0 # {a="b"} 0.5 @@ -451,6 +451,10 @@ 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 \n# EOF\n'), ('# TYPE a histogram\na_bucket{le="+Inf"} 1 # {} 1 1 \n# EOF\n'), + # Exemplars on unallowed samples. + ('# TYPE a histogram\na_sum 1 # {a="b"} 0.5\n# EOF\n'), + ('# TYPE a gaugehistogram\na_sum 1 # {a="b"} 0.5\n# EOF\n'), + ('# TYPE a_bucket gauge\na_bucket 1 # {a="b"} 0.5\n# EOF\n'), ]: with self.assertRaises(ValueError): list(text_string_to_metric_families(case)) From 0b4e983147ada3c9c9836ed86934cfb5308c68ad Mon Sep 17 00:00:00 2001 From: Brian Brazil Date: Mon, 1 Oct 2018 15:49:33 +0100 Subject: [PATCH 03/14] Catch repeated metadata, and metadata after samples. Signed-off-by: Brian Brazil --- prometheus_client/openmetrics/parser.py | 52 ++++++++++++------------- tests/openmetrics/test_parser.py | 9 +++++ 2 files changed, 34 insertions(+), 27 deletions(-) diff --git a/prometheus_client/openmetrics/parser.py b/prometheus_client/openmetrics/parser.py index 63413761..d80405b5 100644 --- a/prometheus_client/openmetrics/parser.py +++ b/prometheus_client/openmetrics/parser.py @@ -244,6 +244,12 @@ 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 = 'untyped' + if documentation is None: + documentation = '' + if unit is None: + unit = '' if unit and not name.endswith("_" + unit): raise ValueError("Unit does not match metric name: " + name) if unit and typ in ['info', 'stateset']: @@ -252,9 +258,7 @@ def build_metric(name, documentation, typ, unit, samples): # TODO: check labelvalues are valid utf8 # TODO: check samples are appropriately grouped and ordered # TODO: check info/stateset values are 1/0 - # TODO: check for metadata in middle of samples # TODO: Check histogram bucket rules being followed - # TODO: Check for duplicate metrics # TODO: Check for dupliate samples # TODO: Check for decresing timestamps metric.samples = samples @@ -273,29 +277,29 @@ def build_metric(name, documentation, typ, unit, samples): parts = line.split(' ', 3) if len(parts) < 4: raise ValueError("Invalid line: " + line) + if parts[2] == name and samples: + raise ValueError("Received metadata after samples: " + line) + if parts[2] != name: + if name != '': + yield build_metric(name, documentation, typ, unit, samples) + # New metric + name = parts[2] + unit = None + typ = None + documentation = None + samples = [] + allowed_names = [parts[2]] + if parts[1] == 'HELP': - if parts[2] != name: - if name != '': - yield build_metric(name, documentation, typ, unit, samples) - # New metric - name = parts[2] - unit = '' - typ = 'untyped' - samples = [] - allowed_names = [parts[2]] + if documentation is not None: + raise ValueError("More than one HELP for metric: " + line) if len(parts) == 4: documentation = _unescape_help(parts[3]) elif len(parts) == 3: raise ValueError("Invalid line: " + line) elif parts[1] == 'TYPE': - if parts[2] != name: - if name != '': - yield build_metric(name, documentation, typ, unit, samples) - # New metric - name = parts[2] - documentation = '' - unit = '' - samples = [] + if typ is not None: + raise ValueError("More than one TYPE for metric: " + line) typ = parts[3] allowed_names = { 'counter': ['_total', '_created'], @@ -306,14 +310,8 @@ def build_metric(name, documentation, typ, unit, samples): }.get(typ, ['']) allowed_names = [name + n for n in allowed_names] elif parts[1] == 'UNIT': - if parts[2] != name: - if name != '': - yield build_metric(name, documentation, typ, unit, samples) - # New metric - name = parts[2] - typ = 'untyped' - samples = [] - allowed_names = [parts[2]] + if unit is not None: + raise ValueError("More than one UNIT for metric: " + line) unit = parts[3] else: raise ValueError("Invalid line: " + line) diff --git a/tests/openmetrics/test_parser.py b/tests/openmetrics/test_parser.py index d094eb3f..67a26487 100644 --- a/tests/openmetrics/test_parser.py +++ b/tests/openmetrics/test_parser.py @@ -431,6 +431,15 @@ def test_invalid_input(self): ('# UNIT a_seconds seconds \n# EOF\n'), ('# TYPE x_u info\n# UNIT x_u u\n# EOF\n'), ('# TYPE x_u stateset\n# UNIT x_u u\n# EOF\n'), + # Metadata in wrong place. + ('# HELP a x\na 1\n# TYPE a gauge\n# EOF\n'), + ('# TYPE a gauge\na 1\n# HELP a gauge\n# EOF\n'), + ('# TYPE a_s gauge\na_s 1\n# UNIT a_s s\n# EOF\n'), + # Repeated metadata. + ('# HELP a \n# HELP a \n# EOF\n'), + ('# HELP a x\n# HELP a x\n# EOF\n'), + ('# TYPE a untyped\n# TYPE a untyped\n# EOF\n'), + ('# UNIT a_s s\n# UNIT a_s s\n# EOF\n'), # Bad metric names. ('0a 1\n# EOF\n'), ('a.b 1\n# EOF\n'), From addb28cbd8959ce9a4ce30e96a6c3ce70c2625c4 Mon Sep 17 00:00:00 2001 From: Brian Brazil Date: Mon, 1 Oct 2018 15:59:23 +0100 Subject: [PATCH 04/14] Check for invalid info/stateset values. Signed-off-by: Brian Brazil --- prometheus_client/openmetrics/parser.py | 5 ++++- tests/openmetrics/test_parser.py | 7 ++++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/prometheus_client/openmetrics/parser.py b/prometheus_client/openmetrics/parser.py index d80405b5..1e81f3f3 100644 --- a/prometheus_client/openmetrics/parser.py +++ b/prometheus_client/openmetrics/parser.py @@ -257,7 +257,6 @@ def build_metric(name, documentation, typ, unit, samples): metric = core.Metric(name, documentation, typ, unit) # TODO: check labelvalues are valid utf8 # TODO: check samples are appropriately grouped and ordered - # TODO: check info/stateset values are 1/0 # TODO: Check histogram bucket rules being followed # TODO: Check for dupliate samples # TODO: Check for decresing timestamps @@ -329,6 +328,10 @@ def build_metric(name, documentation, typ, unit, samples): allowed_names = [sample.name] else: samples.append(sample) + if typ == 'stateset' and sample.value not in [0, 1]: + raise ValueError("Stateset samples can only have values zero and one: " + line) + if typ == 'info' and sample.value != 1: + raise ValueError("Info samples can only have value one: " + line) if sample.exemplar and not ( typ in ['histogram', 'gaugehistogram'] and sample.name.endswith('_bucket')): diff --git a/tests/openmetrics/test_parser.py b/tests/openmetrics/test_parser.py index 67a26487..d2dfa71e 100644 --- a/tests/openmetrics/test_parser.py +++ b/tests/openmetrics/test_parser.py @@ -160,7 +160,7 @@ def test_simple_stateset(self): families = text_string_to_metric_families("""# TYPE a stateset # HELP a help a{a="bar"} 0 -a{a="foo"} 1 +a{a="foo"} 1.0 # EOF """) self.assertEqual([StateSetMetricFamily("a", "help", {'foo': True, 'bar': False})], list(families)) @@ -464,6 +464,11 @@ def test_invalid_input(self): ('# TYPE a histogram\na_sum 1 # {a="b"} 0.5\n# EOF\n'), ('# TYPE a gaugehistogram\na_sum 1 # {a="b"} 0.5\n# EOF\n'), ('# TYPE a_bucket gauge\na_bucket 1 # {a="b"} 0.5\n# EOF\n'), + # Bad stateset/info values. + ('# TYPE a stateset\na 2\n# EOF\n'), + ('# TYPE a info\na 2\n# EOF\n'), + ('# TYPE a stateset\na 2.0\n# EOF\n'), + ('# TYPE a info\na 2.0\n# EOF\n'), ]: with self.assertRaises(ValueError): list(text_string_to_metric_families(case)) From c8c5a027668afe752acbb5b88518205c286eefba Mon Sep 17 00:00:00 2001 From: Brian Brazil Date: Mon, 1 Oct 2018 16:06:20 +0100 Subject: [PATCH 05/14] Add test for empty metadata, and the null byte. Signed-off-by: Brian Brazil --- tests/openmetrics/test_parser.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tests/openmetrics/test_parser.py b/tests/openmetrics/test_parser.py index d2dfa71e..04bd08fb 100644 --- a/tests/openmetrics/test_parser.py +++ b/tests/openmetrics/test_parser.py @@ -173,6 +173,14 @@ def test_no_metadata(self): metric_family.add_sample("a", {}, 1) self.assertEqual([metric_family], list(families)) + def test_empty_metadata(self): + families = text_string_to_metric_families("""# HELP a +# UNIT a +# EOF +""") + metric_family = Metric("a", "", "untyped") + self.assertEqual([metric_family], list(families)) + def test_untyped(self): # https://github.com/prometheus/client_python/issues/79 families = text_string_to_metric_families("""# HELP redis_connected_clients Redis connected clients @@ -315,6 +323,14 @@ def test_escaping(self): metric_family.add_metric(["b\\a\\z"], 2) self.assertEqual([metric_family], list(families)) + def test_null_byte(self): + families = text_string_to_metric_families("""# TYPE a counter +# HELP a he\0lp +# EOF +""") + metric_family = CounterMetricFamily("a", "he\0lp") + self.assertEqual([metric_family], list(families)) + def test_timestamps(self): families = text_string_to_metric_families("""# TYPE a counter # HELP a help From e8c3daa6acb7b3d9d4dd1b0c8f3169659a07b4bf Mon Sep 17 00:00:00 2001 From: Brian Brazil Date: Mon, 1 Oct 2018 16:15:05 +0100 Subject: [PATCH 06/14] Check for NaN counter values Signed-off-by: Brian Brazil --- prometheus_client/openmetrics/parser.py | 5 +++++ tests/openmetrics/test_parser.py | 16 ++++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/prometheus_client/openmetrics/parser.py b/prometheus_client/openmetrics/parser.py index 1e81f3f3..05ff1a6b 100644 --- a/prometheus_client/openmetrics/parser.py +++ b/prometheus_client/openmetrics/parser.py @@ -2,6 +2,8 @@ from __future__ import unicode_literals +import math + try: import StringIO except ImportError: @@ -328,10 +330,13 @@ def build_metric(name, documentation, typ, unit, samples): allowed_names = [sample.name] else: samples.append(sample) + if typ == 'stateset' and sample.value not in [0, 1]: raise ValueError("Stateset samples can only have values zero and one: " + line) if typ == 'info' and sample.value != 1: raise ValueError("Info samples can only have value one: " + line) + if sample.name[len(name):] in ['_total', '_sum', '_count', '_bucket'] and math.isnan(sample.value): + raise ValueError("Counter-like samples cannot be NaN: " + line) if sample.exemplar and not ( typ in ['histogram', 'gaugehistogram'] and sample.name.endswith('_bucket')): diff --git a/tests/openmetrics/test_parser.py b/tests/openmetrics/test_parser.py index 04bd08fb..6c2dce43 100644 --- a/tests/openmetrics/test_parser.py +++ b/tests/openmetrics/test_parser.py @@ -65,6 +65,14 @@ def test_float_gauge(self): """) self.assertEqual([GaugeMetricFamily("a", "help", value=1.2)], list(families)) + def test_nan_gauge(self): + families = text_string_to_metric_families("""# TYPE a gauge +# HELP a help +a NaN +# EOF +""") + self.assertTrue(math.isnan(list(families)[0].samples[0].value)) + def test_unit_gauge(self): families = text_string_to_metric_families("""# TYPE a_seconds gauge # UNIT a_seconds seconds @@ -485,6 +493,14 @@ def test_invalid_input(self): ('# TYPE a info\na 2\n# EOF\n'), ('# TYPE a stateset\na 2.0\n# EOF\n'), ('# TYPE a info\na 2.0\n# EOF\n'), + # Bad counter values. + ('# TYPE a counter\na_total NaN\n# EOF\n'), + ('# TYPE a histogram\na_sum NaN\n# EOF\n'), + ('# TYPE a histogram\na_count NaN\n# EOF\n'), + ('# TYPE a histogram\na_bucket NaN\n# EOF\n'), + ('# TYPE a gaugehistogram\na_bucket NaN\n# EOF\n'), + ('# TYPE a summary\na_sum NaN\n# EOF\n'), + ('# TYPE a summary\na_count NaN\n# EOF\n'), ]: with self.assertRaises(ValueError): list(text_string_to_metric_families(case)) From 61480b20bb1a9c2d6fef9cd24db8b3b1019b2ad9 Mon Sep 17 00:00:00 2001 From: Brian Brazil Date: Mon, 1 Oct 2018 16:25:51 +0100 Subject: [PATCH 07/14] Check for exemplars being too long. Signed-off-by: Brian Brazil --- prometheus_client/openmetrics/parser.py | 3 +++ tests/openmetrics/test_parser.py | 5 +++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/prometheus_client/openmetrics/parser.py b/prometheus_client/openmetrics/parser.py index 05ff1a6b..ba498e87 100644 --- a/prometheus_client/openmetrics/parser.py +++ b/prometheus_client/openmetrics/parser.py @@ -217,6 +217,9 @@ def _parse_sample(text): ts = _parse_timestamp(timestamp) exemplar = None if exemplar_labels is not None: + exemplar_length = sum([len(k) + len(v) + 3 for k, v in exemplar_labels.items()]) + 2 + if exemplar_length > 64: + raise ValueError("Exmplar labels are too long: " + text) exemplar = core.Exemplar(exemplar_labels, _parse_value(exemplar_value), _parse_timestamp(exemplar_timestamp)) diff --git a/tests/openmetrics/test_parser.py b/tests/openmetrics/test_parser.py index 6c2dce43..8df91833 100644 --- a/tests/openmetrics/test_parser.py +++ b/tests/openmetrics/test_parser.py @@ -122,13 +122,13 @@ def test_histogram_exemplars(self): # HELP a help a_bucket{le="1"} 0 # {a="b"} 0.5 a_bucket{le="2"} 2 123 # {a="c"} 0.5 -a_bucket{le="+Inf"} 3 # {a="d"} 4 123 +a_bucket{le="+Inf"} 3 # {a="1234567890123456789012345678901234567890123456789012345678"} 4 123 # EOF """) hfm = HistogramMetricFamily("a", "help") hfm.add_sample("a_bucket", {"le": "1"}, 0.0, None, Exemplar({"a": "b"}, 0.5)) hfm.add_sample("a_bucket", {"le": "2"}, 2.0, Timestamp(123, 0), Exemplar({"a": "c"}, 0.5)), - hfm.add_sample("a_bucket", {"le": "+Inf"}, 3.0, None, Exemplar({"a": "d"}, 4, Timestamp(123, 0))) + hfm.add_sample("a_bucket", {"le": "+Inf"}, 3.0, None, Exemplar({"a": "1234567890123456789012345678901234567890123456789012345678"}, 4, Timestamp(123, 0))) self.assertEqual([hfm], list(families)) def test_simple_gaugehistogram(self): @@ -484,6 +484,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 \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="12345678901234567890123456789012345678901234567890123456789"} 1 1\n# EOF\n'), # Exemplars on unallowed samples. ('# TYPE a histogram\na_sum 1 # {a="b"} 0.5\n# EOF\n'), ('# TYPE a gaugehistogram\na_sum 1 # {a="b"} 0.5\n# EOF\n'), From afe09a356fa81acc6d06448c859ba3ed76f2195a Mon Sep 17 00:00:00 2001 From: Brian Brazil Date: Tue, 2 Oct 2018 12:15:44 +0100 Subject: [PATCH 08/14] Check for non-real timestamps Signed-off-by: Brian Brazil --- prometheus_client/openmetrics/parser.py | 5 ++++- tests/openmetrics/test_parser.py | 4 ++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/prometheus_client/openmetrics/parser.py b/prometheus_client/openmetrics/parser.py index ba498e87..654a4d34 100644 --- a/prometheus_client/openmetrics/parser.py +++ b/prometheus_client/openmetrics/parser.py @@ -75,7 +75,10 @@ def _parse_timestamp(timestamp): return core.Timestamp(int(parts[0]), int(parts[1][:9].ljust(9, "0"))) except ValueError: # Float. - return float(timestamp) + ts = float(timestamp) + if math.isnan(ts) or math.isinf(ts): + raise ValueError("Invalid timestamp: {0!r}".format(timestamp)) + return ts def _parse_labels(it, text): diff --git a/tests/openmetrics/test_parser.py b/tests/openmetrics/test_parser.py index 8df91833..ecc0088c 100644 --- a/tests/openmetrics/test_parser.py +++ b/tests/openmetrics/test_parser.py @@ -477,6 +477,10 @@ def test_invalid_input(self): ('a 1 z\n# EOF\n'), ('a 1 1z\n# EOF\n'), ('a 1 1.1.1\n# EOF\n'), + ('a 1 NaN\n# EOF\n'), + ('a 1 Inf\n# EOF\n'), + ('a 1 +Inf\n# EOF\n'), + ('a 1 -Inf\n# EOF\n'), # Bad exemplars. ('# TYPE a histogram\na_bucket{le="+Inf"} 1 #\n# EOF\n'), ('# TYPE a histogram\na_bucket{le="+Inf"} 1# {} 1\n# EOF\n'), From c7bfebc73471c706160b9cff198d99f2ead1a157 Mon Sep 17 00:00:00 2001 From: Brian Brazil Date: Tue, 2 Oct 2018 14:07:22 +0100 Subject: [PATCH 09/14] Check samples are grouped, untyped isn't used, and for invalid/missing le/quantile values. Signed-off-by: Brian Brazil --- prometheus_client/openmetrics/parser.py | 62 +++++++++++++++++++++---- tests/openmetrics/test_parser.py | 21 +++++++-- 2 files changed, 70 insertions(+), 13 deletions(-) diff --git a/prometheus_client/openmetrics/parser.py b/prometheus_client/openmetrics/parser.py index 654a4d34..df082b43 100644 --- a/prometheus_client/openmetrics/parser.py +++ b/prometheus_client/openmetrics/parser.py @@ -228,7 +228,26 @@ def _parse_sample(text): _parse_timestamp(exemplar_timestamp)) return core.Sample(''.join(name), labels, val, ts, exemplar) - + + +def _group_for_sample(sample, name, typ): + if typ == 'info': + # We can't distinguish between groups for info metrics. + return {} + if typ == 'summary' and sample.name == name: + d = sample.labels.copy() + del d['quantile'] + return d + if typ == 'stateset': + d = sample.labels.copy() + del d[name] + return d + if typ in ['histogram', 'gaugehistogram'] and sample.name == name + '_bucket': + d = sample.labels.copy() + del d['le'] + return d + return sample.labels + def text_fd_to_metric_families(fd): """Parse Prometheus text format from a file descriptor. @@ -240,9 +259,11 @@ def text_fd_to_metric_families(fd): Yields core.Metric's. """ name = '' - documentation = '' - typ = 'untyped' - unit = '' + documentation = None + typ = None + unit = None + group = None + seen_groups = set() samples = [] allowed_names = [] eof = False @@ -253,7 +274,7 @@ def build_metric(name, documentation, typ, unit, samples): raise ValueError("Duplicate metric: " + name) seen_metrics.add(name) if typ is None: - typ = 'untyped' + typ = 'unknown' if documentation is None: documentation = '' if unit is None: @@ -264,7 +285,7 @@ def build_metric(name, documentation, typ, unit, samples): raise ValueError("Units not allowed for this metric type: " + name) metric = core.Metric(name, documentation, typ, unit) # TODO: check labelvalues are valid utf8 - # TODO: check samples are appropriately grouped and ordered + # TODO: check samples are appropriately ordered # TODO: Check histogram bucket rules being followed # TODO: Check for dupliate samples # TODO: Check for decresing timestamps @@ -294,6 +315,8 @@ def build_metric(name, documentation, typ, unit, samples): unit = None typ = None documentation = None + group = None + seen_groups = set() samples = [] allowed_names = [parts[2]] @@ -308,6 +331,8 @@ def build_metric(name, documentation, typ, unit, samples): if typ is not None: raise ValueError("More than one TYPE for metric: " + line) typ = parts[3] + if typ == 'untyped': + raise ValueError("Invalid TYPE for metric: " + line) allowed_names = { 'counter': ['_total', '_created'], 'summary': ['_count', '_sum', '', '_created'], @@ -327,16 +352,33 @@ def build_metric(name, documentation, typ, unit, samples): if sample.name not in allowed_names: if name != '': yield build_metric(name, documentation, typ, unit, samples) - # Start an untyped metric. + # Start an unknown metric. name = sample.name - documentation = '' - unit = '' - typ = 'untyped' + documentation = None + unit = None + typ = 'unknown' samples = [sample] + group = None + seen_groups = set() allowed_names = [sample.name] else: samples.append(sample) + 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): + raise ValueError("Invalid le label: " + line) + if (typ == 'summary' and name == sample.name + and not (0 <= float(sample.labels.get('quantile', -1)) <= 1)): + raise ValueError("Invalid quantile label: " + line) + + g = tuple(sorted(_group_for_sample(sample, name, typ).items())) + if group is not None and g != group and g in seen_groups: + raise ValueError("Invalid metric group ordering: " + line) + group = g + seen_groups.add(g) + if typ == 'stateset' and sample.value not in [0, 1]: raise ValueError("Stateset samples can only have values zero and one: " + line) if typ == 'info' and sample.value != 1: diff --git a/tests/openmetrics/test_parser.py b/tests/openmetrics/test_parser.py index ecc0088c..01a8f87d 100644 --- a/tests/openmetrics/test_parser.py +++ b/tests/openmetrics/test_parser.py @@ -192,7 +192,7 @@ def test_empty_metadata(self): def test_untyped(self): # https://github.com/prometheus/client_python/issues/79 families = text_string_to_metric_families("""# HELP redis_connected_clients Redis connected clients -# TYPE redis_connected_clients untyped +# TYPE redis_connected_clients unknown redis_connected_clients{instance="rough-snowflake-web",port="6380"} 10.0 redis_connected_clients{instance="rough-snowflake-web",port="6381"} 12.0 # EOF @@ -446,6 +446,7 @@ def test_invalid_input(self): ('# TYPE a meh\n# EOF\n'), ('# TYPE a meh \n# EOF\n'), ('# TYPE a gauge \n# EOF\n'), + ('# TYPE a untyped\n# EOF\n'), # Bad UNIT. ('# UNIT\n# EOF\n'), ('# UNIT \n# EOF\n'), @@ -498,14 +499,28 @@ def test_invalid_input(self): ('# TYPE a info\na 2\n# EOF\n'), ('# TYPE a stateset\na 2.0\n# EOF\n'), ('# TYPE a info\na 2.0\n# EOF\n'), + # Missing or invalid labels for a type. + ('# TYPE a summary\na 0\n# EOF\n'), + ('# TYPE a summary\na{quantile="-1"} 0\n# EOF\n'), + ('# 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 histogram\na_bucket 0\n# EOF\n'), + ('# TYPE a gaugehistogram\na_bucket 0\n# EOF\n'), + ('# TYPE a stateset\na 0\n# EOF\n'), # Bad counter values. ('# TYPE a counter\na_total NaN\n# EOF\n'), ('# TYPE a histogram\na_sum NaN\n# EOF\n'), ('# TYPE a histogram\na_count NaN\n# EOF\n'), - ('# TYPE a histogram\na_bucket NaN\n# EOF\n'), - ('# TYPE a gaugehistogram\na_bucket NaN\n# EOF\n'), + ('# TYPE a histogram\na_bucket{le="+Inf"} NaN\n# EOF\n'), + ('# TYPE a gaugehistogram\na_bucket{le="+Inf"} NaN\n# EOF\n'), ('# TYPE a summary\na_sum NaN\n# EOF\n'), ('# TYPE a summary\na_count NaN\n# EOF\n'), + # Bad grouping. + ('# TYPE a histogram\na_sum{a="1"} 0\na_sum{a="2"} 0\na_count{a="1"} 0\n# EOF\n'), + ('# TYPE a histogram\na_bucket{a="1",le="1"} 0\na_bucket{a="2",le="+Inf""} 0\na_bucket{a="1",le="+Inf"} 0\n# EOF\n'), + ('# TYPE a gaugehistogram\na_gsum{a="1"} 0\na_gsum{a="2"} 0\na_gcount{a="1"} 0\n# EOF\n'), + ('# TYPE a summary\nquantile{quantile="0"} 0\na_sum{a="1"} 0\nquantile{quantile="1"} 0\n# EOF\n'), ]: with self.assertRaises(ValueError): list(text_string_to_metric_families(case)) From 1ebdaf91a4993e426e8d48a750b482c5a943f3c2 Mon Sep 17 00:00:00 2001 From: Brian Brazil Date: Tue, 2 Oct 2018 14:50:26 +0100 Subject: [PATCH 10/14] Check ordering within groups. Signed-off-by: Brian Brazil --- prometheus_client/core.py | 5 ++++ prometheus_client/openmetrics/parser.py | 15 ++++++++--- tests/openmetrics/test_parser.py | 33 +++++++++++++++++++------ 3 files changed, 42 insertions(+), 11 deletions(-) diff --git a/prometheus_client/core.py b/prometheus_client/core.py index ccb41348..16ef4f0e 100644 --- a/prometheus_client/core.py +++ b/prometheus_client/core.py @@ -49,6 +49,8 @@ class Timestamp(object): def __init__(self, sec, nsec): if nsec < 0 or nsec >= 1e9: raise ValueError("Invalid value for nanoseconds in Timestamp: {}".format(nsec)) + if sec < 0: + nsec = -nsec self.sec = int(sec) self.nsec = int(nsec) @@ -64,6 +66,9 @@ def __float__(self): def __eq__(self, other): return type(self) == type(other) and self.sec == other.sec and self.nsec == other.nsec + def __gt__(self, other): + return self.sec > other.sec or self.nsec > other.nsec + Exemplar = namedtuple('Exemplar', ['labels', 'value', 'timestamp']) Exemplar.__new__.__defaults__ = (None, ) diff --git a/prometheus_client/openmetrics/parser.py b/prometheus_client/openmetrics/parser.py index df082b43..4bfb7a4b 100644 --- a/prometheus_client/openmetrics/parser.py +++ b/prometheus_client/openmetrics/parser.py @@ -264,6 +264,7 @@ def text_fd_to_metric_families(fd): unit = None group = None seen_groups = set() + group_timestamp = None samples = [] allowed_names = [] eof = False @@ -285,10 +286,8 @@ def build_metric(name, documentation, typ, unit, samples): raise ValueError("Units not allowed for this metric type: " + name) metric = core.Metric(name, documentation, typ, unit) # TODO: check labelvalues are valid utf8 - # TODO: check samples are appropriately ordered # TODO: Check histogram bucket rules being followed - # TODO: Check for dupliate samples - # TODO: Check for decresing timestamps + # TODO: Check for duplicate samples metric.samples = samples return metric @@ -317,6 +316,7 @@ def build_metric(name, documentation, typ, unit, samples): documentation = None group = None seen_groups = set() + group_timestamp = None samples = [] allowed_names = [parts[2]] @@ -359,6 +359,7 @@ def build_metric(name, documentation, typ, unit, samples): typ = 'unknown' samples = [sample] group = None + group_timestamp = None seen_groups = set() allowed_names = [sample.name] else: @@ -375,8 +376,14 @@ def build_metric(name, documentation, typ, unit, samples): g = tuple(sorted(_group_for_sample(sample, name, typ).items())) if group is not None and g != group and g in seen_groups: - raise ValueError("Invalid metric group ordering: " + line) + raise ValueError("Invalid metric grouping: " + line) + if group is not None and g == group: + if (sample.timestamp is None) != (group_timestamp is None): + raise ValueError("Mix of timestamp presence within a group: " + line) + if group_timestamp is not None and group_timestamp > sample.timestamp and typ != 'info': + raise ValueError("Timestamps went backwards within a group: " + line) group = g + group_timestamp = sample.timestamp seen_groups.add(g) if typ == 'stateset' and sample.value not in [0, 1]: diff --git a/tests/openmetrics/test_parser.py b/tests/openmetrics/test_parser.py index 01a8f87d..24eb5236 100644 --- a/tests/openmetrics/test_parser.py +++ b/tests/openmetrics/test_parser.py @@ -121,13 +121,13 @@ def test_histogram_exemplars(self): families = text_string_to_metric_families("""# TYPE a histogram # HELP a help a_bucket{le="1"} 0 # {a="b"} 0.5 -a_bucket{le="2"} 2 123 # {a="c"} 0.5 +a_bucket{le="2"} 2 # {a="c"} 0.5 a_bucket{le="+Inf"} 3 # {a="1234567890123456789012345678901234567890123456789012345678"} 4 123 # EOF """) hfm = HistogramMetricFamily("a", "help") hfm.add_sample("a_bucket", {"le": "1"}, 0.0, None, Exemplar({"a": "b"}, 0.5)) - hfm.add_sample("a_bucket", {"le": "2"}, 2.0, Timestamp(123, 0), Exemplar({"a": "c"}, 0.5)), + hfm.add_sample("a_bucket", {"le": "2"}, 2.0, None, Exemplar({"a": "c"}, 0.5)), hfm.add_sample("a_bucket", {"le": "+Inf"}, 3.0, None, Exemplar({"a": "1234567890123456789012345678901234567890123456789012345678"}, 4, Timestamp(123, 0))) self.assertEqual([hfm], list(families)) @@ -145,15 +145,15 @@ def test_simple_gaugehistogram(self): def test_gaugehistogram_exemplars(self): families = text_string_to_metric_families("""# TYPE a gaugehistogram # HELP a help -a_bucket{le="1"} 0 # {a="b"} 0.5 +a_bucket{le="1"} 0 123 # {a="b"} 0.5 a_bucket{le="2"} 2 123 # {a="c"} 0.5 -a_bucket{le="+Inf"} 3 # {a="d"} 4 123 +a_bucket{le="+Inf"} 3 123 # {a="d"} 4 123 # EOF """) hfm = GaugeHistogramMetricFamily("a", "help") - hfm.add_sample("a_bucket", {"le": "1"}, 0.0, None, Exemplar({"a": "b"}, 0.5)) + hfm.add_sample("a_bucket", {"le": "1"}, 0.0, Timestamp(123, 0), Exemplar({"a": "b"}, 0.5)) hfm.add_sample("a_bucket", {"le": "2"}, 2.0, Timestamp(123, 0), Exemplar({"a": "c"}, 0.5)), - hfm.add_sample("a_bucket", {"le": "+Inf"}, 3.0, None, Exemplar({"a": "d"}, 4, Timestamp(123, 0))) + hfm.add_sample("a_bucket", {"le": "+Inf"}, 3.0, Timestamp(123, 0), Exemplar({"a": "d"}, 4, Timestamp(123, 0))) self.assertEqual([hfm], list(families)) def test_simple_info(self): @@ -164,6 +164,18 @@ def test_simple_info(self): """) self.assertEqual([InfoMetricFamily("a", "help", {'foo': 'bar'})], list(families)) + def test_info_timestamps(self): + families = text_string_to_metric_families("""# TYPE a info +# HELP a help +a_info{a="1",foo="bar"} 1 1 +a_info{a="2",foo="bar"} 1 0 +# EOF +""") + imf = InfoMetricFamily("a", "help") + imf.add_sample("a_info", {"a": "1", "foo": "bar"}, 1, Timestamp(1, 0)) + imf.add_sample("a_info", {"a": "2", "foo": "bar"}, 1, Timestamp(0, 0)) + self.assertEqual([imf], list(families)) + def test_simple_stateset(self): families = text_string_to_metric_families("""# TYPE a stateset # HELP a help @@ -516,11 +528,18 @@ def test_invalid_input(self): ('# TYPE a gaugehistogram\na_bucket{le="+Inf"} NaN\n# EOF\n'), ('# TYPE a summary\na_sum NaN\n# EOF\n'), ('# TYPE a summary\na_count NaN\n# EOF\n'), - # Bad grouping. + # Bad grouping or ordering. ('# TYPE a histogram\na_sum{a="1"} 0\na_sum{a="2"} 0\na_count{a="1"} 0\n# EOF\n'), ('# TYPE a histogram\na_bucket{a="1",le="1"} 0\na_bucket{a="2",le="+Inf""} 0\na_bucket{a="1",le="+Inf"} 0\n# EOF\n'), ('# TYPE a gaugehistogram\na_gsum{a="1"} 0\na_gsum{a="2"} 0\na_gcount{a="1"} 0\n# EOF\n'), ('# TYPE a summary\nquantile{quantile="0"} 0\na_sum{a="1"} 0\nquantile{quantile="1"} 0\n# EOF\n'), + ('# TYPE a gauge\na 0 -1\na 0 -2\n# EOF\n'), + ('# TYPE a gauge\na 0 -1\na 0 -1.1\n# EOF\n'), + ('# TYPE a gauge\na 0 1\na 0 -1\n# EOF\n'), + ('# TYPE a gauge\na 0 1.1\na 0 1\n# EOF\n'), + ('# 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'), ]: with self.assertRaises(ValueError): list(text_string_to_metric_families(case)) From a61233d166c913f915d0f9b61ea23c35d88fd9d3 Mon Sep 17 00:00:00 2001 From: Brian Brazil Date: Tue, 2 Oct 2018 17:02:07 +0100 Subject: [PATCH 11/14] Check bucket rules are being followed Signed-off-by: Brian Brazil --- prometheus_client/core.py | 3 ++ prometheus_client/openmetrics/parser.py | 39 ++++++++++++++++++++++++- tests/openmetrics/test_parser.py | 8 +++++ 3 files changed, 49 insertions(+), 1 deletion(-) diff --git a/prometheus_client/core.py b/prometheus_client/core.py index 16ef4f0e..dfd814e3 100644 --- a/prometheus_client/core.py +++ b/prometheus_client/core.py @@ -66,6 +66,9 @@ def __float__(self): def __eq__(self, other): return type(self) == type(other) and self.sec == other.sec and self.nsec == other.nsec + def __ne__(self, other): + return not self == other + def __gt__(self, other): return self.sec > other.sec or self.nsec > other.nsec diff --git a/prometheus_client/openmetrics/parser.py b/prometheus_client/openmetrics/parser.py index 4bfb7a4b..4a08b565 100644 --- a/prometheus_client/openmetrics/parser.py +++ b/prometheus_client/openmetrics/parser.py @@ -249,6 +249,42 @@ def _group_for_sample(sample, name, typ): return sample.labels +def _check_histogram(samples, name): + group = None + timestamp = None + + def do_checks(): + if bucket != float('+Inf'): + raise ValueError("+Inf bucket missing: " + name) + if count is not None and value != count: + raise ValueError("Count does not match +Inf value: " + name) + + for s in samples: + suffix = s.name[len(name):] + g = _group_for_sample(s, name, 'histogram') + if g != group or s.timestamp != timestamp: + if group is not None: + do_checks() + count = None + bucket = -1 + value = 0 + group = g + timestamp = s.timestamp + + if suffix == '_bucket': + b = float(s.labels['le']) + if 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 + if group is not None: + do_checks() + + def text_fd_to_metric_families(fd): """Parse Prometheus text format from a file descriptor. @@ -284,9 +320,10 @@ def build_metric(name, documentation, typ, unit, samples): raise ValueError("Unit does not match metric name: " + name) if unit and typ in ['info', 'stateset']: raise ValueError("Units not allowed for this metric type: " + name) + if typ in ['histogram', 'gaugehistogram']: + _check_histogram(samples, name) metric = core.Metric(name, documentation, typ, unit) # TODO: check labelvalues are valid utf8 - # TODO: Check histogram bucket rules being followed # TODO: Check for duplicate samples metric.samples = samples return metric diff --git a/tests/openmetrics/test_parser.py b/tests/openmetrics/test_parser.py index 24eb5236..64b7e152 100644 --- a/tests/openmetrics/test_parser.py +++ b/tests/openmetrics/test_parser.py @@ -528,6 +528,14 @@ def test_invalid_input(self): ('# TYPE a gaugehistogram\na_bucket{le="+Inf"} NaN\n# EOF\n'), ('# TYPE a summary\na_sum NaN\n# EOF\n'), ('# TYPE a summary\na_count NaN\n# EOF\n'), + # Bad histograms. + ('# TYPE a histogram\na_sum 1\n# EOF\n'), + ('# TYPE a gaugehistogram\na_gsum 1\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="+Inf"} 0\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'), # Bad grouping or ordering. ('# TYPE a histogram\na_sum{a="1"} 0\na_sum{a="2"} 0\na_count{a="1"} 0\n# EOF\n'), ('# TYPE a histogram\na_bucket{a="1",le="1"} 0\na_bucket{a="2",le="+Inf""} 0\na_bucket{a="1",le="+Inf"} 0\n# EOF\n'), From f4c08cddbbda8976ed67ee6bf3c76f97738ba156 Mon Sep 17 00:00:00 2001 From: Brian Brazil Date: Wed, 3 Oct 2018 13:04:10 +0100 Subject: [PATCH 12/14] Simplify code a bit Signed-off-by: Brian Brazil --- prometheus_client/openmetrics/parser.py | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/prometheus_client/openmetrics/parser.py b/prometheus_client/openmetrics/parser.py index 4a08b565..ca483f9e 100644 --- a/prometheus_client/openmetrics/parser.py +++ b/prometheus_client/openmetrics/parser.py @@ -294,14 +294,7 @@ def text_fd_to_metric_families(fd): Yields core.Metric's. """ - name = '' - documentation = None - typ = None - unit = None - group = None - seen_groups = set() - group_timestamp = None - samples = [] + name = None allowed_names = [] eof = False @@ -344,7 +337,7 @@ def build_metric(name, documentation, typ, unit, samples): if parts[2] == name and samples: raise ValueError("Received metadata after samples: " + line) if parts[2] != name: - if name != '': + if name is not None: yield build_metric(name, documentation, typ, unit, samples) # New metric name = parts[2] @@ -387,7 +380,7 @@ def build_metric(name, documentation, typ, unit, samples): else: sample = _parse_sample(line) if sample.name not in allowed_names: - if name != '': + if name is not None: yield build_metric(name, documentation, typ, unit, samples) # Start an unknown metric. name = sample.name @@ -434,7 +427,7 @@ def build_metric(name, documentation, typ, unit, samples): and sample.name.endswith('_bucket')): raise ValueError("Invalid line only histogram/gaugehistogram buckets can have exemplars: " + line) - if name != '': + if name is not None: yield build_metric(name, documentation, typ, unit, samples) if not eof: From 5c5c3e2db6f83b4f08bcac990fb565b7f10e5754 Mon Sep 17 00:00:00 2001 From: Brian Brazil Date: Wed, 3 Oct 2018 14:12:31 +0100 Subject: [PATCH 13/14] Drop samples with duplicate timestamps, presumably due to truncation Signed-off-by: Brian Brazil --- prometheus_client/openmetrics/parser.py | 16 ++++++++++++---- tests/openmetrics/test_parser.py | 17 ++++++++++++++++- 2 files changed, 28 insertions(+), 5 deletions(-) diff --git a/prometheus_client/openmetrics/parser.py b/prometheus_client/openmetrics/parser.py index ca483f9e..d7793a99 100644 --- a/prometheus_client/openmetrics/parser.py +++ b/prometheus_client/openmetrics/parser.py @@ -317,7 +317,6 @@ def build_metric(name, documentation, typ, unit, samples): _check_histogram(samples, name) metric = core.Metric(name, documentation, typ, unit) # TODO: check labelvalues are valid utf8 - # TODO: Check for duplicate samples metric.samples = samples return metric @@ -347,6 +346,7 @@ def build_metric(name, documentation, typ, unit, samples): group = None seen_groups = set() group_timestamp = None + group_timestamp_samples = set() samples = [] allowed_names = [parts[2]] @@ -387,13 +387,12 @@ def build_metric(name, documentation, typ, unit, samples): documentation = None unit = None typ = 'unknown' - samples = [sample] + samples = [] group = None group_timestamp = None + group_timestamp_samples = set() seen_groups = set() allowed_names = [sample.name] - else: - samples.append(sample) if typ == 'stateset' and name not in sample.labels: raise ValueError("Stateset missing label: " + line) @@ -412,6 +411,15 @@ def build_metric(name, documentation, typ, unit, samples): raise ValueError("Mix of timestamp presence within a group: " + line) if group_timestamp is not None and group_timestamp > sample.timestamp and typ != 'info': raise ValueError("Timestamps went backwards within a group: " + line) + else: + group_timestamp_samples = set() + + series_id = (sample.name, tuple(sorted(sample.labels.items()))) + if sample.timestamp != group_timestamp or series_id not in group_timestamp_samples: + # Not a duplicate due to timestamp truncation. + samples.append(sample) + group_timestamp_samples.add(series_id) + group = g group_timestamp = sample.timestamp seen_groups.add(g) diff --git a/tests/openmetrics/test_parser.py b/tests/openmetrics/test_parser.py index 64b7e152..7dc042c1 100644 --- a/tests/openmetrics/test_parser.py +++ b/tests/openmetrics/test_parser.py @@ -185,6 +185,22 @@ def test_simple_stateset(self): """) self.assertEqual([StateSetMetricFamily("a", "help", {'foo': True, 'bar': False})], list(families)) + def test_duplicate_timestamps(self): + families = text_string_to_metric_families("""# TYPE a gauge +# HELP a help +a{a="1",foo="bar"} 1 0.0000000000 +a{a="1",foo="bar"} 2 0.0000000001 +a{a="1",foo="bar"} 3 0.0000000010 +a{a="2",foo="bar"} 4 0.0000000000 +a{a="2",foo="bar"} 5 0.0000000001 +# EOF +""") + imf = GaugeMetricFamily("a", "help") + imf.add_sample("a", {"a": "1", "foo": "bar"}, 1, Timestamp(0, 0)) + imf.add_sample("a", {"a": "1", "foo": "bar"}, 3, Timestamp(0, 1)) + imf.add_sample("a", {"a": "2", "foo": "bar"}, 4, Timestamp(0, 0)) + self.assertEqual([imf], list(families)) + def test_no_metadata(self): families = text_string_to_metric_families("""a 1 # EOF @@ -533,7 +549,6 @@ def test_invalid_input(self): ('# TYPE a gaugehistogram\na_gsum 1\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="+Inf"} 0\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'), # Bad grouping or ordering. From 283ba23c8d75d820913355f4beb6bfefcba14dbc Mon Sep 17 00:00:00 2001 From: Brian Brazil Date: Wed, 3 Oct 2018 14:37:33 +0100 Subject: [PATCH 14/14] Update OM content type. Per https://github.com/OpenObservability/OpenMetrics/issues/79 Signed-off-by: Brian Brazil --- prometheus_client/exposition.py | 2 +- prometheus_client/openmetrics/exposition.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/prometheus_client/exposition.py b/prometheus_client/exposition.py index 08686cae..5083fabe 100644 --- a/prometheus_client/exposition.py +++ b/prometheus_client/exposition.py @@ -124,7 +124,7 @@ def sample_line(s): def choose_encoder(accept_header): accept_header = accept_header or '' for accepted in accept_header.split(','): - if accepted == 'text/openmetrics; version=0.0.1': + if accepted == 'application/openmetrics-text; version=0.0.1': return (openmetrics.exposition.generate_latest, openmetrics.exposition.CONTENT_TYPE_LATEST) return (generate_latest, CONTENT_TYPE_LATEST) diff --git a/prometheus_client/openmetrics/exposition.py b/prometheus_client/openmetrics/exposition.py index c07578b2..d9e77856 100644 --- a/prometheus_client/openmetrics/exposition.py +++ b/prometheus_client/openmetrics/exposition.py @@ -4,7 +4,7 @@ from .. import core -CONTENT_TYPE_LATEST = str('text/openmetrics; version=0.0.1; charset=utf-8') +CONTENT_TYPE_LATEST = str('application/openmetrics-text; version=0.0.1; charset=utf-8') '''Content type of the latest OpenMetrics text format''' def generate_latest(registry):