Exporter/Prometheus: Upgrade version and fix buckets and labels. (census-instrumentation#492)

- Upgrade to latest Prometheus client.
- Use `UnknownMetricFamily` instead of `UntypedMetricFamily`, as the latter is now deprecated.
- Use ordered list for histogram buckets. Previously we use unordered map and that led to unpredictable behavior.
- Label keys and values must match when uploading metrics.
- Update unit tests to test against more precise results.

Author: songy23

CHANGELOG.md

@@ -1,4 +1,7 @@
 # Changelog
+- Fix bugs in Prometheus exporter. Use ordered list for histogram buckets.
+  Use `UnknownMetricFamily` for `SumData` instead of `UntypedMetricFamily`.
+  Check if label keys and values match before exporting.
 
 ## Unreleased
 

opencensus/stats/exporters/prometheus_exporter.py

@@ -18,7 +18,7 @@
 from prometheus_client.core import GaugeMetricFamily
 from prometheus_client.core import HistogramMetricFamily
 from prometheus_client.core import REGISTRY
-from prometheus_client.core import UntypedMetricFamily
+from prometheus_client.core import UnknownMetricFamily
 
 from opencensus.common.transports import sync
 from opencensus.stats import aggregation_data as aggregation_data_module
@@ -158,14 +158,15 @@ def to_metric(self, desc, tag_values, agg_data):
 
         :rtype: :class:`~prometheus_client.core.CounterMetricFamily` or
                 :class:`~prometheus_client.core.HistogramMetricFamily` or
-                :class:`~prometheus_client.core.UntypedMetricFamily` or
+                :class:`~prometheus_client.core.UnknownMetricFamily` or
                 :class:`~prometheus_client.core.GaugeMetricFamily`
         :returns: A Prometheus metric object
         """
         metric_name = desc['name']
         metric_description = desc['documentation']
         label_keys = desc['labels']
 
+        assert(len(tag_values) == len(label_keys))
         # Prometheus requires that all tag values be strings hence
         # the need to cast none to the empty string before exporting. See
         # https://github.com/census-instrumentation/opencensus-python/issues/480
@@ -183,26 +184,30 @@ def to_metric(self, desc, tag_values, agg_data):
                         aggregation_data_module.DistributionAggregationData):
 
             assert(agg_data.bounds == sorted(agg_data.bounds))
-            points = {}
-            cum_count = 0
+            # buckets are a list of buckets. Each bucket is another list with
+            # a pair of bucket name and value, or a triple of bucket name,
+            # value, and exemplar. buckets need to be in order.
+            buckets = []
+            cum_count = 0  # Prometheus buckets expect cumulative count.
             for ii, bound in enumerate(agg_data.bounds):
                 cum_count += agg_data.counts_per_bucket[ii]
-                points[str(bound)] = cum_count
+                bucket = [str(bound), cum_count]
+                buckets.append(bucket)
             # Prometheus requires buckets to be sorted, and +Inf present.
             # In OpenCensus we don't have +Inf in the bucket bonds so need to
             # append it here.
-            points["+Inf"] = agg_data.count_data
+            buckets.append(["+Inf", agg_data.count_data])
             metric = HistogramMetricFamily(name=metric_name,
                                            documentation=metric_description,
                                            labels=label_keys)
             metric.add_metric(labels=tag_values,
-                              buckets=list(points.items()),
+                              buckets=buckets,
                               sum_value=agg_data.sum,)
             return metric
 
         elif isinstance(agg_data,
                         aggregation_data_module.SumAggregationDataFloat):
-            metric = UntypedMetricFamily(name=metric_name,
+            metric = UnknownMetricFamily(name=metric_name,
                                          documentation=metric_description,
                                          labels=label_keys)
             metric.add_metric(labels=tag_values,

requirements-test.txt

@@ -15,4 +15,4 @@ SQLAlchemy==1.1.14
 WebOb==1.7.3
 wrapt==1.10.11
 thrift==0.10.0
-prometheus_client==0.3.1
+prometheus_client==0.5.0

tests/system/stats/prometheus/prometheus_stats_test.py

@@ -74,11 +74,11 @@ def test_prometheus_stats(self):
             import urllib2
             contents = urllib2.urlopen("http://localhost:9303/metrics").read()
 
-        self.assertIn(b'# TYPE opencensus_request_count_view counter',
+        self.assertIn(b'# TYPE opencensus_request_count_view_total counter',
                       contents)
-        self.assertIn(b'opencensus_request_count_view'
+        self.assertIn(b'opencensus_request_count_view_total'
                       b'{method="some method"} 1.0',
                       contents)
-        self.assertIn(b'opencensus_request_count_view'
+        self.assertIn(b'opencensus_request_count_view_total'
                       b'{method="some other method"} 2.0',
                       contents)

tests/unit/stats/exporters/test_prometheus_stats.py

@@ -13,9 +13,12 @@
 # limitations under the License.
 
 from datetime import datetime
+import copy
 import mock
 import unittest
 
+from prometheus_client.core import Sample
+
 from opencensus.stats import aggregation as aggregation_module
 from opencensus.stats import measure as measure_module
 from opencensus.stats import stats as stats_module
@@ -26,7 +29,6 @@
 from opencensus.tags import tag_map as tag_map_module
 from opencensus.tags import tag_value as tag_value_module
 
-
 MiB = 1 << 20
 FRONTEND_KEY = tag_key_module.TagKey("myorg_keys_frontend")
 FRONTEND_KEY_FLOAT = tag_key_module.TagKey("myorg_keys_frontend_FLOAT")
@@ -138,7 +140,7 @@ def test_collector_to_metric_count(self):
         collector.register_view(view)
         desc = collector.registered_views[list(REGISTERED_VIEW)[0]]
         metric = collector.to_metric(
-            desc=desc, tag_values=[], agg_data=agg.aggregation_data)
+            desc=desc, tag_values=[None], agg_data=agg.aggregation_data)
 
         self.assertEqual(desc['name'], metric.name)
         self.assertEqual(desc['documentation'], metric.documentation)
@@ -156,11 +158,11 @@ def test_collector_to_metric_sum(self):
         collector.register_view(view)
         desc = collector.registered_views[list(REGISTERED_VIEW)[0]]
         metric = collector.to_metric(
-            desc=desc, tag_values=[], agg_data=agg.aggregation_data)
+            desc=desc, tag_values=[None], agg_data=agg.aggregation_data)
 
         self.assertEqual(desc['name'], metric.name)
         self.assertEqual(desc['documentation'], metric.documentation)
-        self.assertEqual('untyped', metric.type)
+        self.assertEqual('unknown', metric.type)
         self.assertEqual(1, len(metric.samples))
 
     def test_collector_to_metric_last_value(self):
@@ -174,7 +176,7 @@ def test_collector_to_metric_last_value(self):
         collector.register_view(view)
         desc = collector.registered_views[list(REGISTERED_VIEW)[0]]
         metric = collector.to_metric(
-            desc=desc, tag_values=[], agg_data=agg.aggregation_data)
+            desc=desc, tag_values=[None], agg_data=agg.aggregation_data)
 
         self.assertEqual(desc['name'], metric.name)
         self.assertEqual(desc['documentation'], metric.documentation)
@@ -187,15 +189,31 @@ def test_collector_to_metric_histogram(self):
         collector = prometheus.Collector(options=options)
         collector.register_view(VIDEO_SIZE_VIEW)
         desc = collector.registered_views[list(REGISTERED_VIEW)[0]]
+        distribution = copy.deepcopy(VIDEO_SIZE_DISTRIBUTION.aggregation_data)
+        distribution.add_sample(280.0 * MiB, None, None)
         metric = collector.to_metric(
             desc=desc,
-            tag_values=[],
-            agg_data=VIDEO_SIZE_DISTRIBUTION.aggregation_data)
+            tag_values=[tag_value_module.TagValue("ios")],
+            agg_data=distribution)
 
         self.assertEqual(desc['name'], metric.name)
         self.assertEqual(desc['documentation'], metric.documentation)
         self.assertEqual('histogram', metric.type)
-        self.assertEqual(5, len(metric.samples))
+        expected_samples = [
+            Sample(metric.name + '_bucket',
+                   {"myorg_keys_frontend": "ios", "le": str(16.0 * MiB)},
+                   0),
+            Sample(metric.name + '_bucket',
+                   {"myorg_keys_frontend": "ios", "le": str(256.0 * MiB)},
+                   0),
+            Sample(metric.name + '_bucket',
+                   {"myorg_keys_frontend": "ios", "le": "+Inf"},
+                   1),
+            Sample(metric.name + '_count', {"myorg_keys_frontend": "ios"}, 1),
+            Sample(metric.name + '_sum',
+                   {"myorg_keys_frontend": "ios"},
+                   280.0 * MiB)]
+        self.assertEqual(expected_samples, metric.samples)
 
     def test_collector_to_metric_invalid_dist(self):
         agg = mock.Mock()
@@ -211,7 +229,7 @@ def test_collector_to_metric_invalid_dist(self):
         with self.assertRaisesRegexp(
                 ValueError,
                 'unsupported aggregation type <class \'mock.mock.Mock\'>'):
-            collector.to_metric(desc=desc, tag_values=[], agg_data=agg)
+            collector.to_metric(desc=desc, tag_values=[None], agg_data=agg)
 
     def test_collector_collect(self):
         agg = aggregation_module.LastValueAggregation(256)
@@ -222,23 +240,17 @@ def test_collector_collect(self):
         collector = prometheus.Collector(options=options)
         collector.register_view(view)
         desc = collector.registered_views['test2_new_view']
-        collector.to_metric(
-            desc=desc, tag_values=[], agg_data=agg.aggregation_data)
-
-        registry = mock.Mock()
-        options = prometheus.Options("test1", 8001, "localhost", registry)
-        collector = prometheus.Collector(options=options)
-        collector.register_view(VIDEO_SIZE_VIEW)
-        desc = collector.registered_views[list(REGISTERED_VIEW)[0]]
         metric = collector.to_metric(
             desc=desc,
-            tag_values=[],
-            agg_data=VIDEO_SIZE_DISTRIBUTION.aggregation_data)
+            tag_values=[tag_value_module.TagValue("value")],
+            agg_data=agg.aggregation_data)
 
         self.assertEqual(desc['name'], metric.name)
         self.assertEqual(desc['documentation'], metric.documentation)
-        self.assertEqual('histogram', metric.type)
-        self.assertEqual(5, len(metric.samples))
+        self.assertEqual('gauge', metric.type)
+        expected_samples = [
+            Sample(metric.name, {"myorg_keys_frontend": "value"}, 256)]
+        self.assertEqual(expected_samples, metric.samples)
 
     def test_collector_collect_with_none_label_value(self):
         agg = aggregation_module.LastValueAggregation(256)