Added metrics to MetricFamily exceptions to fix prometheus#362 (prometheus#364)

Signed-off-by: Rick van Hattem <Wolph@wol.ph>

Author: wolph

prometheus_client/exposition.py

@@ -87,36 +87,41 @@ def sample_line(s):
 
     output = []
     for metric in registry.collect():
-        mname = metric.name
-        mtype = metric.type
-        # Munging from OpenMetrics into Prometheus format.
-        if mtype == 'counter':
-            mname = mname + '_total'
-        elif mtype == 'info':
-            mname = mname + '_info'
-            mtype = 'gauge'
-        elif mtype == 'stateset':
-            mtype = 'gauge'
-        elif mtype == 'gaugehistogram':
-            # A gauge histogram is really a gauge,
-            # but this captures the strucutre better.
-            mtype = 'histogram'
-        elif mtype == 'unknown':
-            mtype = 'untyped'
-
-        output.append('# HELP {0} {1}\n'.format(
-            mname, metric.documentation.replace('\\', r'\\').replace('\n', r'\n')))
-        output.append('# TYPE {0} {1}\n'.format(mname, mtype))
-
-        om_samples = {}
-        for s in metric.samples:
-            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:
-                output.append(sample_line(s))
+        try:
+            mname = metric.name
+            mtype = metric.type
+            # Munging from OpenMetrics into Prometheus format.
+            if mtype == 'counter':
+                mname = mname + '_total'
+            elif mtype == 'info':
+                mname = mname + '_info'
+                mtype = 'gauge'
+            elif mtype == 'stateset':
+                mtype = 'gauge'
+            elif mtype == 'gaugehistogram':
+                # A gauge histogram is really a gauge,
+                # but this captures the strucutre better.
+                mtype = 'histogram'
+            elif mtype == 'unknown':
+                mtype = 'untyped'
+
+            output.append('# HELP {0} {1}\n'.format(
+                mname, metric.documentation.replace('\\', r'\\').replace('\n', r'\n')))
+            output.append('# TYPE {0} {1}\n'.format(mname, mtype))
+
+            om_samples = {}
+            for s in metric.samples:
+                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:
+                    output.append(sample_line(s))
+        except Exception as exception:
+            exception.args = (exception.args or ('',)) + (metric,)
+            raise
+                    
         for suffix, lines in sorted(om_samples.items()):
             output.append('# TYPE {0}{1} gauge\n'.format(metric.name, suffix))
             output.extend(lines)

prometheus_client/openmetrics/exposition.py

@@ -12,49 +12,54 @@ def generate_latest(registry):
     '''Returns the metrics from the registry in latest text format as a string.'''
     output = []
     for metric in registry.collect():
-        mname = metric.name
-        output.append('# HELP {0} {1}\n'.format(
-            mname, metric.documentation.replace('\\', r'\\').replace('\n', r'\n').replace('"', r'\"')))
-        output.append('# TYPE {0} {1}\n'.format(mname, metric.type))
-        if metric.unit:
-            output.append('# UNIT {0} {1}\n'.format(mname, metric.unit))
-        for s in metric.samples:
-            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 = ''
-            if s.exemplar:
-                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(
-                     k, v.replace('\\', r'\\').replace('\n', r'\n').replace('"', r'\"'))
-                     for k, v in sorted(s.exemplar.labels.items())]))
-                if s.exemplar.timestamp is not None:
-                    exemplarstr = ' # {0} {1} {2}'.format(
-                        labels,
-                        floatToGoString(s.exemplar.value),
-                        s.exemplar.timestamp,
-                    )
+        try:
+            mname = metric.name
+            output.append('# HELP {0} {1}\n'.format(
+                mname, metric.documentation.replace('\\', r'\\').replace('\n', r'\n').replace('"', r'\"')))
+            output.append('# TYPE {0} {1}\n'.format(mname, metric.type))
+            if metric.unit:
+                output.append('# UNIT {0} {1}\n'.format(mname, metric.unit))
+            for s in metric.samples:
+                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:
-                    exemplarstr = ' # {0} {1}'.format(
-                        labels,
-                        floatToGoString(s.exemplar.value),
-                    )
-            else:
-                exemplarstr = ''
-            timestamp = ''
-            if s.timestamp is not None:
-                timestamp = ' {0}'.format(s.timestamp)
-            output.append('{0}{1} {2}{3}{4}\n'.format(
-                s.name,
-                labelstr,
-                floatToGoString(s.value),
-                timestamp,
-                exemplarstr,
-            ))
+                    labelstr = ''
+                if s.exemplar:
+                    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(
+                         k, v.replace('\\', r'\\').replace('\n', r'\n').replace('"', r'\"'))
+                         for k, v in sorted(s.exemplar.labels.items())]))
+                    if s.exemplar.timestamp is not None:
+                        exemplarstr = ' # {0} {1} {2}'.format(
+                            labels,
+                            floatToGoString(s.exemplar.value),
+                            s.exemplar.timestamp,
+                        )
+                    else:
+                        exemplarstr = ' # {0} {1}'.format(
+                            labels,
+                            floatToGoString(s.exemplar.value),
+                        )
+                else:
+                    exemplarstr = ''
+                timestamp = ''
+                if s.timestamp is not None:
+                    timestamp = ' {0}'.format(s.timestamp)
+                output.append('{0}{1} {2}{3}{4}\n'.format(
+                    s.name,
+                    labelstr,
+                    floatToGoString(s.value),
+                    timestamp,
+                    exemplarstr,
+                ))
+        except Exception as exception:
+            exception.args = (exception.args or ('',)) + (metric,)
+            raise
+                    
     output.append('# EOF\n')
     return ''.join(output).encode('utf-8')

tests/test_exposition.py

@@ -3,6 +3,7 @@
 import sys
 import threading
 import time
+import pytest
 
 from prometheus_client import (
     CollectorRegistry, CONTENT_TYPE_LATEST, Counter, delete_from_gateway, Enum,
@@ -11,8 +12,9 @@
 )
 from prometheus_client.core import GaugeHistogramMetricFamily, Timestamp
 from prometheus_client.exposition import (
-    basic_auth_handler, default_handler, MetricsHandler,
+    basic_auth_handler, default_handler, MetricsHandler, generate_latest,
 )
+from prometheus_client import core
 
 if sys.version_info < (2, 7):
     # We need the skip decorators from unittest2 on Python 2.6.
@@ -303,5 +305,84 @@ def test_metrics_handler_subclassing(self):
         self.assertTrue(issubclass(handler, (MetricsHandler, subclass)))
 
 
+@pytest.fixture
+def registry():
+    return core.CollectorRegistry()
+
+
+class Collector:
+    def __init__(self, metric_family, *values):
+        self.metric_family = metric_family
+        self.values = values
+
+    def collect(self):
+        self.metric_family.add_metric([], *self.values)
+        return [self.metric_family]
+
+
+def _expect_metric_exception(registry, expected_error):
+    try:
+        generate_latest(registry)
+    except expected_error as exception:
+        assert isinstance(exception.args[-1], core.Metric)
+        # Got a valid error as expected, return quietly
+        return
+
+    raise RuntimeError('Expected exception not raised')
+
+
+@pytest.mark.parametrize('MetricFamily', [
+    core.CounterMetricFamily,
+    core.GaugeMetricFamily,
+])
+@pytest.mark.parametrize('value,error', [
+    (None, TypeError),
+    ('', ValueError),
+    ('x', ValueError),
+    ([], TypeError),
+    ({}, TypeError),
+])
+def test_basic_metric_families(registry, MetricFamily, value, error):
+    metric_family = MetricFamily(MetricFamily.__name__, 'help')
+    registry.register(Collector(metric_family, value))
+    _expect_metric_exception(registry, error)
+
+
+@pytest.mark.parametrize('count_value,sum_value,error', [
+    (None, 0, TypeError),
+    (0, None, TypeError),
+    ('', 0, ValueError),
+    (0, '', ValueError),
+    ([], 0, TypeError),
+    (0, [], TypeError),
+    ({}, 0, TypeError),
+    (0, {}, TypeError),
+])
+def test_summary_metric_family(registry, count_value, sum_value, error):
+    metric_family = core.SummaryMetricFamily('summary', 'help')
+    registry.register(Collector(metric_family, count_value, sum_value))
+    _expect_metric_exception(registry, error)
+
+
+@pytest.mark.parametrize('MetricFamily', [
+    core.HistogramMetricFamily,
+    core.GaugeHistogramMetricFamily,
+])
+@pytest.mark.parametrize('buckets,sum_value,error', [
+    ([('spam', 0), ('eggs', 0)], None, TypeError),
+    ([('spam', 0), ('eggs', None)], 0, TypeError),
+    ([('spam', 0), (None, 0)], 0, AttributeError),
+    ([('spam', None), ('eggs', 0)], 0, TypeError),
+    ([(None, 0), ('eggs', 0)], 0, AttributeError),
+    ([('spam', 0), ('eggs', 0)], '', ValueError),
+    ([('spam', 0), ('eggs', '')], 0, ValueError),
+    ([('spam', ''), ('eggs', 0)], 0, ValueError),
+])
+def test_histogram_metric_families(MetricFamily, registry, buckets, sum_value, error):
+    metric_family = MetricFamily(MetricFamily.__name__, 'help')
+    registry.register(Collector(metric_family, buckets, sum_value))
+    _expect_metric_exception(registry, error)
+
+
 if __name__ == '__main__':
     unittest.main()