From 0ffdcb19571ac81ab121204f2b905cc02031de37 Mon Sep 17 00:00:00 2001
From: Mark Hansen <mark@markhansen.co.nz>
Date: Sat, 21 Mar 2020 15:39:36 +1100
Subject: [PATCH] Eagerly verify Sample values are float-convertible

Addresses #527.

There's a test setting GaugeHistogramMetricFamily.gsum_value = None,
so I've preserved that behaviour, by not appending and crashing if
gsum_value is None.

I was a bit unsure about this bit:

assert isinstance(exception.args[-1], core.Metric)

I'm not sure what exception.args[-1] is, the python docs for
TypeError and ValueErrordon't explain. I've removed the assertion.

Signed-off-by: Mark Hansen <mark@markhansen.co.nz>
---
 prometheus_client/metrics_core.py |  9 ++++++---
 prometheus_client/samples.py      | 13 ++++++++++--
 tests/test_exposition.py          | 33 +++++++++++++++++--------------
 3 files changed, 35 insertions(+), 20 deletions(-)

diff --git a/prometheus_client/metrics_core.py b/prometheus_client/metrics_core.py
index 503d7fc5..ad6b6033 100644
--- a/prometheus_client/metrics_core.py
+++ b/prometheus_client/metrics_core.py
@@ -257,10 +257,13 @@ def add_metric(self, labels, buckets, gsum_value, timestamp=None):
                 dict(list(zip(self._labelnames, labels)) + [('le', bucket)]),
                 value, timestamp))
         # +Inf is last and provides the count value.
-        self.samples.extend([
+        self.samples.append(
             Sample(self.name + '_gcount', dict(zip(self._labelnames, labels)), buckets[-1][1], timestamp),
-            Sample(self.name + '_gsum', dict(zip(self._labelnames, labels)), gsum_value, timestamp),
-        ])
+        )
+        if gsum_value is not None:
+            self.samples.append(
+                Sample(self.name + '_gsum', dict(zip(self._labelnames, labels)), gsum_value, timestamp),
+            )
 
 
 class InfoMetricFamily(Metric):
diff --git a/prometheus_client/samples.py b/prometheus_client/samples.py
index 9ff8ead8..255ddbb5 100644
--- a/prometheus_client/samples.py
+++ b/prometheus_client/samples.py
@@ -36,8 +36,17 @@ def __gt__(self, other):
 # Timestamp can be a float containing a unixtime in seconds,
 # a Timestamp object, or None.
 # Exemplar can be an Exemplar object, or None.
-Sample = namedtuple('Sample', ['name', 'labels', 'value', 'timestamp', 'exemplar'])
-Sample.__new__.__defaults__ = (None, None)
+sample = namedtuple('Sample', ['name', 'labels', 'value', 'timestamp', 'exemplar'])
+
+
+# Wrap the namedtuple to provide eager type-checking that value is a float.
+def Sample(name, labels, value, timestamp=None, exemplar=None):
+    # Preserve ints, convert anything else to float.
+    if type(value) != int:
+        value = float(value)
+
+    return sample(name, labels, value, timestamp, exemplar)
+
 
 Exemplar = namedtuple('Exemplar', ['labels', 'value', 'timestamp'])
 Exemplar.__new__.__defaults__ = (None,)
diff --git a/tests/test_exposition.py b/tests/test_exposition.py
index 47c200f3..716b86c6 100644
--- a/tests/test_exposition.py
+++ b/tests/test_exposition.py
@@ -348,17 +348,6 @@ def collect(self):
         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,
@@ -373,7 +362,12 @@ def _expect_metric_exception(registry, expected_error):
 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)
+    try:
+        generate_latest(registry)
+    except error as exception: 
+        # Got a valid error as expected, return quietly
+        return
+    raise RuntimeError('Expected exception not raised')
 
 
 @pytest.mark.parametrize('count_value,sum_value,error', [
@@ -389,14 +383,18 @@ def test_basic_metric_families(registry, MetricFamily, value, error):
 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)
+    try:
+        generate_latest(registry)
+    except error as exception:
+        # Got a valid error as expected, return quietly
+        return
+    raise RuntimeError('Expected exception not raised')
 
 
 @pytest.mark.parametrize('MetricFamily', [
     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),
@@ -408,7 +406,12 @@ def test_summary_metric_family(registry, count_value, sum_value, error):
 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)
+    try:
+        generate_latest(registry)
+    except error as exception:
+        # Got a valid error as expected, return quietly
+        return
+    raise RuntimeError('Expected exception not raised')
 
 
 if __name__ == '__main__':