Set bucket bounds as le labels for Prometheus exporter. (#1167)

Closes #1164.

Previously we don't set the bucket bounds for Prometheus Samples,
since bucket bounds is a built-in feature for Prometheus Histogram.
However, as #1164 suggested, we can manually set the bucket bounds
with the same label le that the built-in Histogram uses,
to get the same output as the built-in Prometheus Histogram Samples.

(cherry picked from commit 3838c4c)

Author: songy23

CHANGELOG.md

@@ -1,5 +1,6 @@
 ## Unreleased
 - Fix a typo on displaying Aggregation Type for a View on StatsZ page.
+- Set bucket bounds as "le" labels for Prometheus Stats exporter.
 
 ## 0.13.0 - 2018-04-27
 - Support building with Java 9.

exporters/stats/prometheus/README.md

@@ -78,4 +78,4 @@ and [Bridges](https://github.com/prometheus/client_java#bridges).
 
 Java 7 or above is required for using this exporter.
 
-## FAQ
+## FAQ

exporters/stats/prometheus/src/main/java/io/opencensus/exporter/stats/prometheus/PrometheusExportUtils.java

@@ -16,6 +16,8 @@
 
 package io.opencensus.exporter.stats.prometheus;
 
+import static io.prometheus.client.Collector.doubleToGoString;
+
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Preconditions;
 import com.google.common.collect.Lists;
@@ -86,17 +88,20 @@ final class PrometheusExportUtils {
   @VisibleForTesting static final String SAMPLE_SUFFIX_BUCKET = "_bucket";
   @VisibleForTesting static final String SAMPLE_SUFFIX_COUNT = "_count";
   @VisibleForTesting static final String SAMPLE_SUFFIX_SUM = "_sum";
+  @VisibleForTesting static final String LABEL_NAME_BUCKET_BOUND = "le";
 
   // Converts a ViewData to a Prometheus MetricFamilySamples.
   static MetricFamilySamples createMetricFamilySamples(ViewData viewData) {
     View view = viewData.getView();
     String name =
         Collector.sanitizeMetricName(OPENCENSUS_NAMESPACE + '_' + view.getName().asString());
     Type type = getType(view.getAggregation(), view.getWindow());
+    List<String> labelNames = convertToLabelNames(view.getColumns());
     List<Sample> samples = Lists.newArrayList();
     for (Entry<List</*@Nullable*/ TagValue>, AggregationData> entry :
         viewData.getAggregationMap().entrySet()) {
-      samples.addAll(getSamples(name, view.getColumns(), entry.getKey(), entry.getValue()));
+      samples.addAll(
+          getSamples(name, labelNames, entry.getKey(), entry.getValue(), view.getAggregation()));
     }
     return new MetricFamilySamples(
         name, type, OPENCENSUS_HELP_MSG + view.getDescription(), samples);
@@ -108,6 +113,13 @@ static MetricFamilySamples createDescribableMetricFamilySamples(View view) {
     String name =
         Collector.sanitizeMetricName(OPENCENSUS_NAMESPACE + '_' + view.getName().asString());
     Type type = getType(view.getAggregation(), view.getWindow());
+    List<String> labelNames = convertToLabelNames(view.getColumns());
+    if (containsDisallowedLeLabelForHistogram(labelNames, type)) {
+      throw new IllegalStateException(
+          "Prometheus Histogram cannot have a label named 'le', "
+              + "because it is a reserved label for bucket boundaries. "
+              + "Please remove this tag key from your view.");
+    }
     return new MetricFamilySamples(
         name, type, OPENCENSUS_HELP_MSG + view.getDescription(), Collections.<Sample>emptyList());
   }
@@ -133,20 +145,19 @@ public Type apply(Aggregation arg) {
         });
   }
 
+  // Converts a row in ViewData (a.k.a Entry<List<TagValue>, AggregationData>) to a list of
+  // Prometheus Samples.
   @VisibleForTesting
   static List<Sample> getSamples(
       final String name,
-      List<TagKey> tagKeys,
+      final List<String> labelNames,
       List</*@Nullable*/ TagValue> tagValues,
-      AggregationData aggregationData) {
+      AggregationData aggregationData,
+      final Aggregation aggregation) {
     Preconditions.checkArgument(
-        tagKeys.size() == tagValues.size(), "Tag keys and tag values have different sizes.");
+        labelNames.size() == tagValues.size(), "Label names and tag values have different sizes.");
     final List<Sample> samples = Lists.newArrayList();
-    final List<String> labelNames = new ArrayList<String>(tagKeys.size());
     final List<String> labelValues = new ArrayList<String>(tagValues.size());
-    for (TagKey tagKey : tagKeys) {
-      labelNames.add(Collector.sanitizeMetricName(tagKey.getName()));
-    }
     for (TagValue tagValue : tagValues) {
       String labelValue = tagValue == null ? "" : tagValue.asString();
       labelValues.add(labelValue);
@@ -177,11 +188,29 @@ public Void apply(CountData arg) {
         new Function<DistributionData, Void>() {
           @Override
           public Void apply(DistributionData arg) {
-            for (long bucketCount : arg.getBucketCounts()) {
+            // For histogram buckets, manually add the bucket boundaries as "le" labels. See
+            // https://github.com/prometheus/client_java/commit/ed184d8e50c82e98bb2706723fff764424840c3a#diff-c505abbde72dd6bf36e89917b3469404R241
+            @SuppressWarnings("unchecked")
+            Distribution distribution = (Distribution) aggregation;
+            List<Double> boundaries = distribution.getBucketBoundaries().getBoundaries();
+            List<String> labelNamesWithLe = new ArrayList<String>(labelNames);
+            labelNamesWithLe.add(LABEL_NAME_BUCKET_BOUND);
+            for (int i = 0; i < arg.getBucketCounts().size(); i++) {
+              List<String> labelValuesWithLe = new ArrayList<String>(labelValues);
+              // The label value of "le" is the upper inclusive bound.
+              // For the last bucket, it should be "+Inf".
+              String bucketBoundary =
+                  doubleToGoString(
+                      i < boundaries.size() ? boundaries.get(i) : Double.POSITIVE_INFINITY);
+              labelValuesWithLe.add(bucketBoundary);
               samples.add(
                   new MetricFamilySamples.Sample(
-                      name + SAMPLE_SUFFIX_BUCKET, labelNames, labelValues, bucketCount));
+                      name + SAMPLE_SUFFIX_BUCKET,
+                      labelNamesWithLe,
+                      labelValuesWithLe,
+                      arg.getBucketCounts().get(i)));
             }
+
             samples.add(
                 new MetricFamilySamples.Sample(
                     name + SAMPLE_SUFFIX_COUNT, labelNames, labelValues, arg.getCount()));
@@ -234,5 +263,30 @@ public Void apply(AggregationData arg) {
     return samples;
   }
 
+  // Converts the list of tag keys to a list of string label names. Also sanitizes the tag keys.
+  @VisibleForTesting
+  static List<String> convertToLabelNames(List<TagKey> tagKeys) {
+    final List<String> labelNames = new ArrayList<String>(tagKeys.size());
+    for (TagKey tagKey : tagKeys) {
+      labelNames.add(Collector.sanitizeMetricName(tagKey.getName()));
+    }
+    return labelNames;
+  }
+
+  // Returns true if there is an "le" label name in histogram label names, returns false otherwise.
+  // Similar check to
+  // https://github.com/prometheus/client_java/commit/ed184d8e50c82e98bb2706723fff764424840c3a#diff-c505abbde72dd6bf36e89917b3469404R78
+  static boolean containsDisallowedLeLabelForHistogram(List<String> labelNames, Type type) {
+    if (!Type.HISTOGRAM.equals(type)) {
+      return false;
+    }
+    for (String label : labelNames) {
+      if (LABEL_NAME_BUCKET_BOUND.equals(label)) {
+        return true;
+      }
+    }
+    return false;
+  }
+
   private PrometheusExportUtils() {}
 }

exporters/stats/prometheus/src/main/java/io/opencensus/exporter/stats/prometheus/PrometheusStatsCollector.java

@@ -16,6 +16,10 @@
 
 package io.opencensus.exporter.stats.prometheus;
 
+import static io.opencensus.exporter.stats.prometheus.PrometheusExportUtils.containsDisallowedLeLabelForHistogram;
+import static io.opencensus.exporter.stats.prometheus.PrometheusExportUtils.convertToLabelNames;
+import static io.opencensus.exporter.stats.prometheus.PrometheusExportUtils.getType;
+
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Lists;
@@ -39,6 +43,7 @@
  *
  * @since 0.12
  */
+@SuppressWarnings("deprecation")
 public final class PrometheusStatsCollector extends Collector implements Collector.Describable {
 
   private static final Logger logger = Logger.getLogger(PrometheusStatsCollector.class.getName());
@@ -90,20 +95,27 @@ public List<MetricFamilySamples> collect() {
     span.addAnnotation("Collect Prometheus Metric Samples.");
     try (Scope scope = tracer.withSpan(span)) {
       for (View view : viewManager.getAllExportedViews()) {
-        ViewData viewData = viewManager.getView(view.getName());
-        if (viewData == null) {
-          continue;
-        } else {
-          samples.add(PrometheusExportUtils.createMetricFamilySamples(viewData));
+        if (containsDisallowedLeLabelForHistogram(
+            convertToLabelNames(view.getColumns()),
+            getType(view.getAggregation(), view.getWindow()))) {
+          continue; // silently skip Distribution views with "le" tag key
+        }
+        try {
+          ViewData viewData = viewManager.getView(view.getName());
+          if (viewData == null) {
+            continue;
+          } else {
+            samples.add(PrometheusExportUtils.createMetricFamilySamples(viewData));
+          }
+        } catch (Throwable e) {
+          logger.log(Level.WARNING, "Exception thrown when collecting metric samples.", e);
+          span.setStatus(
+              Status.UNKNOWN.withDescription(
+                  "Exception thrown when collecting Prometheus Metric Samples: "
+                      + exceptionMessage(e)));
         }
       }
       span.addAnnotation("Finish collecting Prometheus Metric Samples.");
-    } catch (Throwable e) {
-      logger.log(Level.WARNING, "Exception thrown when collecting metric samples.", e);
-      span.setStatus(
-          Status.UNKNOWN.withDescription(
-              "Exception thrown when collecting Prometheus Metric Samples: "
-                  + exceptionMessage(e)));
     } finally {
       span.end();
     }
@@ -118,14 +130,16 @@ public List<MetricFamilySamples> describe() {
     span.addAnnotation("Describe Prometheus Metrics.");
     try (Scope scope = tracer.withSpan(span)) {
       for (View view : viewManager.getAllExportedViews()) {
-        samples.add(PrometheusExportUtils.createDescribableMetricFamilySamples(view));
+        try {
+          samples.add(PrometheusExportUtils.createDescribableMetricFamilySamples(view));
+        } catch (Throwable e) {
+          logger.log(Level.WARNING, "Exception thrown when describing metrics.", e);
+          span.setStatus(
+              Status.UNKNOWN.withDescription(
+                  "Exception thrown when describing Prometheus Metrics: " + exceptionMessage(e)));
+        }
       }
       span.addAnnotation("Finish describing Prometheus Metrics.");
-    } catch (Throwable e) {
-      logger.log(Level.WARNING, "Exception thrown when describing metrics.", e);
-      span.setStatus(
-          Status.UNKNOWN.withDescription(
-              "Exception thrown when describing Prometheus Metrics: " + exceptionMessage(e)));
     } finally {
       span.end();
     }