From 313f12f0da7fbae9133bd50ad8d75320117752b9 Mon Sep 17 00:00:00 2001 From: Brian Brazil Date: Thu, 20 Sep 2018 13:25:58 +0100 Subject: [PATCH 01/13] 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 7577d64941f8dd5dc33044c2e24dca8e4566c84a Mon Sep 17 00:00:00 2001 From: Lee Calcote Date: Tue, 25 Sep 2018 18:50:10 -0500 Subject: [PATCH 02/13] checks to validate labelvalues as utf-8 Signed-off-by: Lee Calcote --- prometheus_client/openmetrics/parser.py | 6 ++++++ tests/openmetrics/test_parser.py | 12 ++++++++++++ 2 files changed, 18 insertions(+) diff --git a/prometheus_client/openmetrics/parser.py b/prometheus_client/openmetrics/parser.py index 6517cfe8..59c4f579 100644 --- a/prometheus_client/openmetrics/parser.py +++ b/prometheus_client/openmetrics/parser.py @@ -111,6 +111,8 @@ def _parse_labels(it, text): labelvalue = [] state = 'endoflabelvalue' else: + if not _validate_utf8(char): + raise ValueError("Invalid line: " + text) labelvalue.append(char) elif state == 'endoflabelvalue': if char == ',': @@ -128,6 +130,8 @@ def _parse_labels(it, text): elif char == '"': labelvalue.append('"') else: + if not _validate_utf8(char): + raise ValueError("Invalid line: " + text) labelvalue.append('\\' + char) elif state == 'endoflabels': if char == ' ': @@ -136,6 +140,8 @@ def _parse_labels(it, text): raise ValueError("Invalid line: " + text) return labels +def _validate_utf8(char): + return ord(char) < 65536 def _parse_sample(text): name = [] diff --git a/tests/openmetrics/test_parser.py b/tests/openmetrics/test_parser.py index e05ea869..356a0546 100644 --- a/tests/openmetrics/test_parser.py +++ b/tests/openmetrics/test_parser.py @@ -206,6 +206,18 @@ def test_labels_with_curly_braces(self): metric_family.add_metric(["bar", "b{a}z"], 1) self.assertEqual([metric_family], list(families)) + def test_labels_with_invalid_utf8_values(self): + try: + families = text_string_to_metric_families('''# TYPE a counter +# HELP a help +a_total{foo="'''+u'\U00010000'+'''",bar="baz"} 1 +# EOF +''') + for f in families: pass + assert False + except ValueError: + assert True + def test_empty_help(self): families = text_string_to_metric_families("""# TYPE a counter # HELP a From 2b485d3ecfe1d0e73f09813fe0f2d7d8fde9d896 Mon Sep 17 00:00:00 2001 From: Lee Calcote Date: Fri, 28 Sep 2018 21:00:44 -0500 Subject: [PATCH 03/13] switching to encode('utf-8') test Signed-off-by: Lee Calcote --- prometheus_client/openmetrics/parser.py | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/prometheus_client/openmetrics/parser.py b/prometheus_client/openmetrics/parser.py index 59c4f579..50e9a02d 100644 --- a/prometheus_client/openmetrics/parser.py +++ b/prometheus_client/openmetrics/parser.py @@ -106,13 +106,16 @@ def _parse_labels(it, text): elif char == '"': if not core._METRIC_LABEL_NAME_RE.match(''.join(labelname)): raise ValueError("Invalid line: " + text) - labels[''.join(labelname)] = ''.join(labelvalue) + utf8_str = ''.join(labelvalue) + try: + utf8_str.encode('utf') + except: + raise ValueError("Invalid line: " + text) + labels[''.join(labelname)] = utf8_str labelname = [] labelvalue = [] state = 'endoflabelvalue' - else: - if not _validate_utf8(char): - raise ValueError("Invalid line: " + text) + else: labelvalue.append(char) elif state == 'endoflabelvalue': if char == ',': @@ -129,9 +132,7 @@ def _parse_labels(it, text): labelvalue.append('\n') elif char == '"': labelvalue.append('"') - else: - if not _validate_utf8(char): - raise ValueError("Invalid line: " + text) + else: labelvalue.append('\\' + char) elif state == 'endoflabels': if char == ' ': @@ -140,9 +141,6 @@ def _parse_labels(it, text): raise ValueError("Invalid line: " + text) return labels -def _validate_utf8(char): - return ord(char) < 65536 - def _parse_sample(text): name = [] value = [] From 53bcb8953ef51bff7c5a82148c7fe4df41532dc3 Mon Sep 17 00:00:00 2001 From: Lee Calcote Date: Sun, 30 Sep 2018 08:37:12 -0500 Subject: [PATCH 04/13] update from encode('utf') to 'utf-8' Signed-off-by: Lee Calcote --- prometheus_client/openmetrics/parser.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/prometheus_client/openmetrics/parser.py b/prometheus_client/openmetrics/parser.py index 50e9a02d..9a63c98a 100644 --- a/prometheus_client/openmetrics/parser.py +++ b/prometheus_client/openmetrics/parser.py @@ -108,7 +108,7 @@ def _parse_labels(it, text): raise ValueError("Invalid line: " + text) utf8_str = ''.join(labelvalue) try: - utf8_str.encode('utf') + utf8_str.encode('utf-8') except: raise ValueError("Invalid line: " + text) labels[''.join(labelname)] = utf8_str From bf70f586f0e45c1c84d4b4e82a1bdeb87886c75c Mon Sep 17 00:00:00 2001 From: Lee Calcote Date: Wed, 24 Oct 2018 11:09:20 -0500 Subject: [PATCH 05/13] Identified a potential invalid utf-8 character - u'\uD802 Signed-off-by: Lee Calcote --- .gitignore | 1 + tests/openmetrics/test_parser.py | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index 043223b4..1ab25e2d 100644 --- a/.gitignore +++ b/.gitignore @@ -6,3 +6,4 @@ dist .coverage.* .coverage .tox +.vscode diff --git a/tests/openmetrics/test_parser.py b/tests/openmetrics/test_parser.py index 356a0546..416e7db4 100644 --- a/tests/openmetrics/test_parser.py +++ b/tests/openmetrics/test_parser.py @@ -210,7 +210,7 @@ def test_labels_with_invalid_utf8_values(self): try: families = text_string_to_metric_families('''# TYPE a counter # HELP a help -a_total{foo="'''+u'\U00010000'+'''",bar="baz"} 1 +a_total{foo="'''+u'\uD802'+'''",bar="baz"} 1 # EOF ''') for f in families: pass From 1012e6fd5abcfc065495d48510d32e769a16b239 Mon Sep 17 00:00:00 2001 From: Lee Calcote Date: Wed, 24 Oct 2018 11:50:25 -0500 Subject: [PATCH 06/13] catching specific error type Signed-off-by: Lee Calcote --- prometheus_client/openmetrics/parser.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/prometheus_client/openmetrics/parser.py b/prometheus_client/openmetrics/parser.py index ff6ac1cd..0c067565 100644 --- a/prometheus_client/openmetrics/parser.py +++ b/prometheus_client/openmetrics/parser.py @@ -114,7 +114,7 @@ def _parse_labels(it, text): utf8_str = ''.join(labelvalue) try: utf8_str.encode('utf-8') - except: + except UnicodeEncodeError: raise ValueError("Invalid line: " + text) labels[''.join(labelname)] = utf8_str labelname = [] From 3b028128a51ed3622a3c1c7e182d9ed51c8147f3 Mon Sep 17 00:00:00 2001 From: Lee Calcote Date: Wed, 24 Oct 2018 18:58:17 -0500 Subject: [PATCH 07/13] added python2.6 compatibility Signed-off-by: Lee Calcote --- prometheus_client/openmetrics/parser.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/prometheus_client/openmetrics/parser.py b/prometheus_client/openmetrics/parser.py index 0c067565..317c2112 100644 --- a/prometheus_client/openmetrics/parser.py +++ b/prometheus_client/openmetrics/parser.py @@ -3,6 +3,7 @@ from __future__ import unicode_literals import math +import sys try: import StringIO @@ -113,7 +114,10 @@ def _parse_labels(it, text): raise ValueError("Invalid line: " + text) utf8_str = ''.join(labelvalue) try: - utf8_str.encode('utf-8') + if sys.version_info >= (3,): + utf8_str.encode('utf-8') + else: + utf8_str.decode('utf-8') except UnicodeEncodeError: raise ValueError("Invalid line: " + text) labels[''.join(labelname)] = utf8_str From 244ba2e24a90980c5bc63cbb120b4da7f320df6b Mon Sep 17 00:00:00 2001 From: Brian Brazil Date: Mon, 5 Nov 2018 13:37:28 +0000 Subject: [PATCH 08/13] Check for negative counter-like and guage histogram values. Signed-off-by: Brian Brazil --- prometheus_client/openmetrics/parser.py | 4 +++- tests/openmetrics/test_parser.py | 18 ++++++++++++++---- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/prometheus_client/openmetrics/parser.py b/prometheus_client/openmetrics/parser.py index 718e8fe4..21fbfaf9 100644 --- a/prometheus_client/openmetrics/parser.py +++ b/prometheus_client/openmetrics/parser.py @@ -432,8 +432,10 @@ def build_metric(name, documentation, typ, unit, samples): 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): + if sample.name[len(name):] in ['_total', '_sum', '_count', '_bucket', '_gcount', '_gsum'] and math.isnan(sample.value): raise ValueError("Counter-like samples cannot be NaN: " + line) + if sample.name[len(name):] in ['_total', '_sum', '_count', '_bucket', '_gcount', '_gsum'] and sample.value < 0: + raise ValueError("Counter-like samples cannot be negative: " + 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 22c18efc..a4228156 100644 --- a/tests/openmetrics/test_parser.py +++ b/tests/openmetrics/test_parser.py @@ -256,13 +256,13 @@ def test_empty_help(self): self.assertEqual([CounterMetricFamily("a", "", value=1)], list(families)) def test_labels_and_infinite(self): - families = text_string_to_metric_families("""# TYPE a counter + families = text_string_to_metric_families("""# TYPE a gauge # HELP a help -a_total{foo="bar"} +Inf -a_total{foo="baz"} -Inf +a{foo="bar"} +Inf +a{foo="baz"} -Inf # EOF """) - metric_family = CounterMetricFamily("a", "help", labels=["foo"]) + metric_family = GaugeMetricFamily("a", "help", labels=["foo"]) metric_family.add_metric(["bar"], float('inf')) metric_family.add_metric(["baz"], float('-inf')) self.assertEqual([metric_family], list(families)) @@ -535,12 +535,22 @@ def test_invalid_input(self): ('# TYPE a stateset\na 0\n# EOF\n'), # Bad counter values. ('# TYPE a counter\na_total NaN\n# EOF\n'), + ('# TYPE a counter\na_total -1\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{le="+Inf"} NaN\n# EOF\n'), + ('# TYPE a histogram\na_sum -1\n# EOF\n'), + ('# TYPE a histogram\na_count -1\n# EOF\n'), + ('# TYPE a histogram\na_bucket{le="+Inf"} -1\n# EOF\n'), ('# TYPE a gaugehistogram\na_bucket{le="+Inf"} NaN\n# EOF\n'), + ('# TYPE a gaugehistogram\na_bucket{le="+Inf"} -1\na_gcount -1\n# EOF\n'), + ('# TYPE a gaugehistogram\na_bucket{le="+Inf"} -1\n# EOF\n'), + ('# TYPE a gaugehistogram\na_bucket{le="+Inf"} 1\na_gsum -1\n# EOF\n'), + ('# TYPE a gaugehistogram\na_bucket{le="+Inf"} 1\na_gsum NaN\n# EOF\n'), ('# TYPE a summary\na_sum NaN\n# EOF\n'), ('# TYPE a summary\na_count NaN\n# EOF\n'), + ('# TYPE a summary\na_sum -1\n# EOF\n'), + ('# TYPE a summary\na_count -1\n# EOF\n'), # Bad histograms. ('# TYPE a histogram\na_sum 1\n# EOF\n'), ('# TYPE a gaugehistogram\na_gsum 1\n# EOF\n'), From 3bdeed2d5180dd24dfed463b77d494d837120315 Mon Sep 17 00:00:00 2001 From: Girish Ranganathan Date: Wed, 7 Nov 2018 13:50:31 -0500 Subject: [PATCH 09/13] trying to only use encode Signed-off-by: Girish Ranganathan --- prometheus_client/openmetrics/parser.py | 7 ++----- tests/openmetrics/test_parser.py | 6 +++++- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/prometheus_client/openmetrics/parser.py b/prometheus_client/openmetrics/parser.py index 317c2112..52ffcc99 100644 --- a/prometheus_client/openmetrics/parser.py +++ b/prometheus_client/openmetrics/parser.py @@ -114,11 +114,8 @@ def _parse_labels(it, text): raise ValueError("Invalid line: " + text) utf8_str = ''.join(labelvalue) try: - if sys.version_info >= (3,): - utf8_str.encode('utf-8') - else: - utf8_str.decode('utf-8') - except UnicodeEncodeError: + utf8_str.encode('utf-8') + except UnicodeError: raise ValueError("Invalid line: " + text) labels[''.join(labelname)] = utf8_str labelname = [] diff --git a/tests/openmetrics/test_parser.py b/tests/openmetrics/test_parser.py index 78d05235..a1bc7475 100644 --- a/tests/openmetrics/test_parser.py +++ b/tests/openmetrics/test_parser.py @@ -252,9 +252,13 @@ def test_labels_with_curly_braces(self): def test_labels_with_invalid_utf8_values(self): try: + if sys.version_info < (2, 7): + inj = '\xc3\x83' + else: + inj = u'\uD802' families = text_string_to_metric_families('''# TYPE a counter # HELP a help -a_total{foo="'''+u'\uD802'+'''",bar="baz"} 1 +a_total{foo="'''+inj+'''",bar="baz"} 1 # EOF ''') for f in families: pass From 6708b0ca6994f806b5f5a869aa6e75fc5b54536d Mon Sep 17 00:00:00 2001 From: Girish Ranganathan Date: Tue, 20 Nov 2018 11:27:27 -0500 Subject: [PATCH 10/13] Merge branch 'openmetrics' into valid-utf8 Signed-off-by: Girish Ranganathan --- .gitignore | 2 -- tests/openmetrics/test_parser.py | 6 +++--- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/.gitignore b/.gitignore index dbe69f12..fe6a1845 100644 --- a/.gitignore +++ b/.gitignore @@ -7,5 +7,3 @@ dist .coverage .tox .*cache -htmlcov -.vscode diff --git a/tests/openmetrics/test_parser.py b/tests/openmetrics/test_parser.py index 65ecb560..3776184a 100644 --- a/tests/openmetrics/test_parser.py +++ b/tests/openmetrics/test_parser.py @@ -249,10 +249,10 @@ def test_labels_with_curly_braces(self): def test_labels_with_invalid_utf8_values(self): try: - if sys.version_info < (2, 7): - inj = '\xc3\x83' - else: + if sys.version_info >= (3,): inj = u'\uD802' + else: + inj = '\xc3\x83' families = text_string_to_metric_families('''# TYPE a counter # HELP a help a_total{foo="'''+inj+'''",bar="baz"} 1 From 1f5bee3f661f1a2a8a7e30522091b8e77d62be05 Mon Sep 17 00:00:00 2001 From: Girish Ranganathan Date: Tue, 20 Nov 2018 12:22:20 -0500 Subject: [PATCH 11/13] trying another sequence for negative test with python2 Signed-off-by: Girish Ranganathan --- tests/openmetrics/test_parser.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/openmetrics/test_parser.py b/tests/openmetrics/test_parser.py index 3776184a..1df78ff5 100644 --- a/tests/openmetrics/test_parser.py +++ b/tests/openmetrics/test_parser.py @@ -252,7 +252,7 @@ def test_labels_with_invalid_utf8_values(self): if sys.version_info >= (3,): inj = u'\uD802' else: - inj = '\xc3\x83' + inj = '\xf8\xa1\xa1\xa1\xa1' families = text_string_to_metric_families('''# TYPE a counter # HELP a help a_total{foo="'''+inj+'''",bar="baz"} 1 From 8c38c1d3badbea5ff6e5422bc4b7037dbf1a089b Mon Sep 17 00:00:00 2001 From: Girish Ranganathan Date: Tue, 20 Nov 2018 12:34:45 -0500 Subject: [PATCH 12/13] trying another sequence for negative test with python2 Signed-off-by: Girish Ranganathan --- tests/openmetrics/test_parser.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/openmetrics/test_parser.py b/tests/openmetrics/test_parser.py index 1df78ff5..b55b2475 100644 --- a/tests/openmetrics/test_parser.py +++ b/tests/openmetrics/test_parser.py @@ -252,7 +252,7 @@ def test_labels_with_invalid_utf8_values(self): if sys.version_info >= (3,): inj = u'\uD802' else: - inj = '\xf8\xa1\xa1\xa1\xa1' + inj = '\xfc' families = text_string_to_metric_families('''# TYPE a counter # HELP a help a_total{foo="'''+inj+'''",bar="baz"} 1 From e26b1efe61ba61e1ca6b9f33e6dff589c888153a Mon Sep 17 00:00:00 2001 From: Girish Ranganathan Date: Tue, 20 Nov 2018 14:44:07 -0500 Subject: [PATCH 13/13] trying a binary string for python2 Signed-off-by: Girish Ranganathan --- tests/openmetrics/test_parser.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/openmetrics/test_parser.py b/tests/openmetrics/test_parser.py index b55b2475..47e844f2 100644 --- a/tests/openmetrics/test_parser.py +++ b/tests/openmetrics/test_parser.py @@ -252,12 +252,13 @@ def test_labels_with_invalid_utf8_values(self): if sys.version_info >= (3,): inj = u'\uD802' else: - inj = '\xfc' - families = text_string_to_metric_families('''# TYPE a counter + inj = b'\xef' + str = '''# TYPE a counter # HELP a help a_total{foo="'''+inj+'''",bar="baz"} 1 # EOF -''') +''' + families = text_string_to_metric_families(str) for f in families: pass assert False except ValueError: