Repository: prometheus/client_python
Pull request #184 (2017-08-05 17:56:03)

Convert MetricFamily labels -> list
On Python 3 the labels of the `python_info` metric is a reference to the `dict_keys` of the data object used to create it. This PR ensures all labelnames in MetricFamilies it are a static tuple instead.

This PR is motivated by some broken tests in https://github.com/korfuri/django-prometheus with recent versions of prometheus-client: https://github.com/korfuri/django-prometheus/pull/46#issuecomment-320453360

Although the docs do not say Metrics should be serializable this change seems more in keeping with the intention that labels should be owned by the Metric and not a reference to an external object.

brian-brazil: This doesn't seem like the right fix, this should be down in GaugeMetricFamily I think. Also a unittest would be good.
manics: Do you mean GaugeMetricFamily should ensure labels is a list of strings in __init__?: https://github.com/prometheus/client_python/blob/master/prometheus_client/core.py#L215
brian-brazil: Yes
manics: @brian-brazil I've force-pushed away the original commit, and converted labelnames to a tuple in all MetricFamilies

Patch:
diff --git a/prometheus_client/core.py b/prometheus_client/core.py
index 6456749..8e51ba2 100644
--- a/prometheus_client/core.py
+++ b/prometheus_client/core.py
@@ -187,7 +187,7 @@ class CounterMetricFamily(Metric):
             raise ValueError('Can only specify at most one of value and labels.')
         if labels is None:
           labels = []
-        self._labelnames = labels
+        self._labelnames = tuple(labels)
         if value is not None:
           self.add_metric([], value)
 
@@ -212,7 +212,7 @@ class GaugeMetricFamily(Metric):
             raise ValueError('Can only specify at most one of value and labels.')
         if labels is None:
           labels = []
-        self._labelnames = labels
+        self._labelnames = tuple(labels)
         if value is not None:
           self.add_metric([], value)
 
@@ -239,7 +239,7 @@ class SummaryMetricFamily(Metric):
             raise ValueError('Can only specify at most one of value and labels.')
         if labels is None:
           labels = []
-        self._labelnames = labels
+        self._labelnames = tuple(labels)
         if count_value is not None:
           self.add_metric([], count_value, sum_value)
 
@@ -268,7 +268,7 @@ class HistogramMetricFamily(Metric):
             raise ValueError('Can only specify at most one of buckets and labels.')
         if labels is None:
           labels = []
-        self._labelnames = labels
+        self._labelnames = tuple(labels)
         if buckets is not None:
           self.add_metric([], buckets, sum_value)
 


Test patch:
diff --git a/tests/test_core.py b/tests/test_core.py
index f0eca1f..6242537 100644
--- a/tests/test_core.py
+++ b/tests/test_core.py
@@ -389,6 +389,16 @@ class TestMetricFamilies(unittest.TestCase):
         self.assertRaises(ValueError, HistogramMetricFamily, 'h', 'help', buckets={}, sum_value=1, labels=['a'])
         self.assertRaises(KeyError, HistogramMetricFamily, 'h', 'help', buckets={}, sum_value=1)
 
+    def test_labelnames(self):
+        cmf = CounterMetricFamily('c', 'help', labels=iter(['a']))
+        self.assertEqual(('a',), cmf._labelnames)
+        gmf = GaugeMetricFamily('g', 'help', labels=iter(['a']))
+        self.assertEqual(('a',), gmf._labelnames)
+        smf = SummaryMetricFamily('s', 'help', labels=iter(['a']))
+        self.assertEqual(('a',), smf._labelnames)
+        hmf = HistogramMetricFamily('h', 'help', labels=iter(['a']))
+        self.assertEqual(('a',), hmf._labelnames)
+
 
 class TestCollectorRegistry(unittest.TestCase):
     def test_duplicate_metrics_raises(self):