Prometheus +Inf bucket should always match count (fixes micrometer-me…

…trics#249)
pavel-golub · Nov 24, 2017 · edde666 · edde666
1 parent b277445
commit edde666
Show file tree

Hide file tree

Showing 3 changed files with 10 additions and 13 deletions.
diff --git a/...r-registry-prometheus/src/main/java/io/micrometer/prometheus/PrometheusMeterRegistry.java b/...r-registry-prometheus/src/main/java/io/micrometer/prometheus/PrometheusMeterRegistry.java
@@ -185,15 +185,16 @@ protected io.micrometer.core.instrument.Timer newTimer(Meter.Id id, HistogramCon
                 // satisfies https://prometheus.io/docs/concepts/metric_types/#histogram
                 for (CountAtValue c : histogramCounts) {
                     final List<String> histogramValues = new LinkedList<>(tagValues);
-                    if (c.value() == Long.MAX_VALUE) {
-                        histogramValues.add("+Inf");
-                    } else {
-                        histogramValues.add(Collector.doubleToGoString(c.value(TimeUnit.SECONDS)));
-                    }
-
+                    histogramValues.add(Collector.doubleToGoString(c.value(TimeUnit.SECONDS)));
                     samples.add(new Collector.MetricFamilySamples.Sample(
                         conventionName + "_bucket", histogramKeys, histogramValues, c.count()));
                 }
+
+                // the +Inf bucket should always equal `count`
+                final List<String> histogramValues = new LinkedList<>(tagValues);
+                histogramValues.add("+Inf");
+                samples.add(new Collector.MetricFamilySamples.Sample(
+                    conventionName + "_bucket", histogramKeys, histogramValues, snapshot.count()));
             }
 
             samples.add(new Collector.MetricFamilySamples.Sample(

diff --git a/...gistry-prometheus/src/test/java/io/micrometer/prometheus/PrometheusMeterRegistryTest.java b/...gistry-prometheus/src/test/java/io/micrometer/prometheus/PrometheusMeterRegistryTest.java
@@ -159,17 +159,14 @@ void percentileTimersAreClampedByDefault() {
     @Issue("#127")
     @Test
     void percentileHistogramsAccumulateToInfinityEvenWhenClamped() {
-        // the underlying histogram is clamped implicitly by HdrHistogram
-        DistributionSummary widthSizes = DistributionSummary.builder("screen.width.pixels")
-            .tags("page", "home")
-            .description("Distribution of screen 'width'")
+        Timer t = Timer.builder("t1")
             .publishPercentileHistogram()
             .register(registry);
 
-        widthSizes.record(1024);
+        t.record(106, TimeUnit.SECONDS);
 
         assertThat(registry.scrape())
-            .contains("screen_width_pixels_bucket{page=\"home\",le=\"+Inf\",} 1.0");
+            .contains("t1_duration_seconds_bucket{le=\"+Inf\",} 1.0");
     }
 
     @Issue("#127")

diff --git a/micrometer-core/src/main/java/io/micrometer/core/instrument/histogram/HistogramConfig.java b/micrometer-core/src/main/java/io/micrometer/core/instrument/histogram/HistogramConfig.java
@@ -67,7 +67,6 @@ public NavigableSet<Long> getHistogramBuckets(boolean supportsAggregablePercenti
             buckets.addAll(PercentileHistogramBuckets.buckets(this));
             buckets.add(minimumExpectedValue);
             buckets.add(maximumExpectedValue);
-            buckets.add(Long.MAX_VALUE);
         }
 
         for (long slaBoundary : sla) {