Fix quantile value for DistributionSummary for OtlpMeterRegistry

izeye · May 20, 2024 · 34c9d89 · 34c9d89
1 parent aa4b7c2
commit 34c9d89
Show file tree

Hide file tree

Showing 2 changed files with 24 additions and 8 deletions.
diff --git a/...crometer-registry-otlp/src/main/java/io/micrometer/registry/otlp/OtlpMetricConverter.java b/...crometer-registry-otlp/src/main/java/io/micrometer/registry/otlp/OtlpMetricConverter.java
@@ -132,7 +132,7 @@ private void writeHistogramSupport(HistogramSupport histogramSupport) {
 
         // if percentiles configured, use summary
         if (histogramSnapshot.percentileValues().length != 0) {
-            buildSummaryDataPoint(histogramSupport, tags, startTimeNanos, total, count, histogramSnapshot);
+            buildSummaryDataPoint(histogramSupport, tags, startTimeNanos, total, count, isTimeBased, histogramSnapshot);
         }
         else {
             buildHistogramDataPoint(histogramSupport, tags, startTimeNanos, total, count, isTimeBased,
@@ -192,7 +192,7 @@ private void buildHistogramDataPoint(final HistogramSupport histogramSupport, fi
     }
 
     private void buildSummaryDataPoint(final HistogramSupport histogramSupport, final Iterable<KeyValue> tags,
-            final long startTimeNanos, final double total, final long count,
+            final long startTimeNanos, final double total, final long count, boolean isTimeBased,
             final HistogramSnapshot histogramSnapshot) {
         Metric.Builder metricBuilder = getOrCreateMetricBuilder(histogramSupport.getId(), DataCase.SUMMARY);
         if (metricBuilder != null) {
@@ -203,9 +203,10 @@ private void buildSummaryDataPoint(final HistogramSupport histogramSupport, fina
                 .setSum(total)
                 .setCount(count);
             for (ValueAtPercentile percentile : histogramSnapshot.percentileValues()) {
+                double value = percentile.value();
                 summaryDataPoint.addQuantileValues(SummaryDataPoint.ValueAtQuantile.newBuilder()
                     .setQuantile(percentile.percentile())
-                    .setValue(TimeUtils.convert(percentile.value(), TimeUnit.NANOSECONDS, baseTimeUnit)));
+                    .setValue(isTimeBased ? TimeUtils.convert(value, TimeUnit.NANOSECONDS, baseTimeUnit) : value));
             }
 
             setSummaryDataPoint(metricBuilder, summaryDataPoint);

diff --git a/...eter-registry-otlp/src/test/java/io/micrometer/registry/otlp/OtlpMetricConverterTest.java b/...eter-registry-otlp/src/test/java/io/micrometer/registry/otlp/OtlpMetricConverterTest.java
@@ -21,17 +21,16 @@
 import java.util.List;
 import java.util.concurrent.TimeUnit;
 
+import io.micrometer.core.instrument.*;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
-import io.micrometer.core.instrument.Gauge;
-import io.micrometer.core.instrument.MockClock;
-import io.micrometer.core.instrument.Tags;
-import io.micrometer.core.instrument.Timer;
 import io.micrometer.core.instrument.config.NamingConvention;
 import io.opentelemetry.proto.metrics.v1.Metric;
 
 class OtlpMetricConverterTest {
 
+    private static final Duration STEP = Duration.ofMillis(1);
+
     private static final Tags FIRST_TAG = Tags.of("key", "1");
 
     private static final Tags SECOND_TAG = Tags.of("key", "2");
@@ -45,7 +44,7 @@ class OtlpMetricConverterTest {
     @BeforeEach
     void setUp() {
         mockClock = new MockClock();
-        otlpMetricConverter = new OtlpMetricConverter(mockClock, Duration.ofMillis(1), TimeUnit.MILLISECONDS,
+        otlpMetricConverter = new OtlpMetricConverter(mockClock, STEP, TimeUnit.MILLISECONDS,
                 AggregationTemporality.CUMULATIVE, NamingConvention.dot);
         otlpMeterRegistry = new OtlpMeterRegistry(OtlpConfig.DEFAULT, mockClock);
     }
@@ -160,4 +159,20 @@ void timerWithSummaryAndHistogramShouldBeMultipleMetrics() {
         });
     }
 
+    @Test
+    void addMeterWithDistributionSummary() {
+        DistributionSummary summary = DistributionSummary.builder("test.summary")
+            .publishPercentiles(0.5)
+            .register(otlpMeterRegistry);
+
+        summary.record(5);
+        mockClock.add(STEP);
+
+        otlpMetricConverter.addMeter(summary);
+        assertThat(otlpMetricConverter.getAllMetrics()).singleElement()
+            .satisfies(metric -> assertThat(metric.getSummary().getDataPointsList()).singleElement()
+                .satisfies(dataPoint -> assertThat(dataPoint.getQuantileValuesList()).singleElement()
+                    .satisfies(valueAtQuantile -> assertThat(valueAtQuantile.getValue()).isEqualTo(5))));
+    }
+
 }