Skip to content

Commit

Permalink
Fix quantile value for DistributionSummary for OtlpMeterRegistry
Browse files Browse the repository at this point in the history 
Co-authored-by: Johnny Lim <[email protected]>
  • Loading branch information
@jonatan-ivanov @izeye
jonatan-ivanov and izeye committed Jun 21, 2024
2 parents 1d45f62 + 9eec355 commit 9500ee5
Show file tree
Hide file tree
Showing 4 changed files with 80 additions and 10 deletions.
7 changes: 4 additions & 3 deletions ...crometer-registry-otlp/src/main/java/io/micrometer/registry/otlp/OtlpMetricConverter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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) {
Expand All @@ -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);
Expand Down
23 changes: 22 additions & 1 deletion ...istry-otlp/src/test/java/io/micrometer/registry/otlp/OtlpCumulativeMeterRegistryTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ void distributionSummary() {
}

@Test
void distributionSummaryWithHistogramBuckets() {
void distributionSummaryWithHistogram() {
DistributionSummary size = DistributionSummary.builder("http.request.size")
.baseUnit(BaseUnits.BYTES)
.publishPercentileHistogram()
Expand Down Expand Up @@ -473,6 +473,27 @@ void distributionSummaryWithHistogramBuckets() {
}
}

@Test
void distributionSummaryWithPercentiles() {
DistributionSummary size = DistributionSummary.builder("http.response.size")
.baseUnit(BaseUnits.BYTES)
.publishPercentiles(0.5, 0.9, 0.99)
.register(registry);
size.record(100);
size.record(15);
size.record(2233);
clock.add(otlpConfig().step());
size.record(204);

assertThat(writeToMetric(size).toString())
.isEqualTo("name: \"http.response.size\"\n" + "unit: \"bytes\"\n" + "summary {\n" + " data_points {\n"
+ " start_time_unix_nano: 1000000\n" + " time_unix_nano: 60001000000\n" + " count: 4\n"
+ " sum: 2552.0\n" + " quantile_values {\n" + " quantile: 0.5\n" + " value: 200.0\n"
+ " }\n" + " quantile_values {\n" + " quantile: 0.9\n" + " value: 200.0\n"
+ " }\n" + " quantile_values {\n" + " quantile: 0.99\n" + " value: 200.0\n"
+ " }\n" + " }\n" + "}\n");
}

private double extractValue(String line) {
return Double.parseDouble(line.substring(line.lastIndexOf(' ')));
}
Expand Down
35 changes: 34 additions & 1 deletion ...r-registry-otlp/src/test/java/io/micrometer/registry/otlp/OtlpDeltaMeterRegistryTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@
import io.opentelemetry.proto.metrics.v1.HistogramDataPoint;
import io.opentelemetry.proto.metrics.v1.Metric;
import io.opentelemetry.proto.metrics.v1.NumberDataPoint;
import io.opentelemetry.proto.metrics.v1.SummaryDataPoint;
import io.opentelemetry.proto.metrics.v1.SummaryDataPoint.ValueAtQuantile;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.testcontainers.shaded.com.google.common.util.concurrent.AtomicDouble;
Expand All @@ -32,6 +34,7 @@
import java.util.ArrayDeque;
import java.util.Arrays;
import java.util.Deque;
import java.util.List;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.ExecutionException;
Expand Down Expand Up @@ -255,9 +258,9 @@ void distributionSummary() {
@Test
void distributionSummaryWithHistogram() {
DistributionSummary ds = DistributionSummary.builder(METER_NAME)
.baseUnit(BaseUnits.BYTES)
.description(METER_DESCRIPTION)
.tags(Tags.of(meterTag))
.baseUnit(BaseUnits.BYTES)
.serviceLevelObjectives(10, 50, 100, 500)
.register(registry);

Expand Down Expand Up @@ -302,6 +305,36 @@ void distributionSummaryWithHistogram() {
assertThat(histogramDataPoint.getBucketCounts(3)).isZero();
}

@Test
void distributionSummaryWithPercentiles() {
DistributionSummary size = DistributionSummary.builder(METER_NAME)
.baseUnit(BaseUnits.BYTES)
.description(METER_DESCRIPTION)
.tags(Tags.of(meterTag))
.publishPercentiles(0.5, 0.9, 0.99)
.register(registry);
size.record(100);
size.record(15);
size.record(2233);
stepOverNStep(1);
size.record(204);

Metric metric = writeToMetric(size);
assertThat(metric.getName()).isEqualTo(METER_NAME);
assertThat(metric.getDescription()).isEqualTo(METER_DESCRIPTION);
assertThat(metric.getUnit()).isEqualTo(BaseUnits.BYTES);
List<SummaryDataPoint> dataPoints = metric.getSummary().getDataPointsList();
assertThat(dataPoints).hasSize(1);
List<ValueAtQuantile> quantiles = dataPoints.get(0).getQuantileValuesList();
assertThat(quantiles).hasSize(3);
assertThat(quantiles.get(0)).satisfies(quantile -> assertThat(quantile.getQuantile()).isEqualTo(0.5))
.satisfies(quantile -> assertThat(quantile.getValue()).isEqualTo(200));
assertThat(quantiles.get(1)).satisfies(quantile -> assertThat(quantile.getQuantile()).isEqualTo(0.9))
.satisfies(quantile -> assertThat(quantile.getValue()).isEqualTo(200));
assertThat(quantiles.get(2)).satisfies(quantile -> assertThat(quantile.getQuantile()).isEqualTo(0.99))
.satisfies(quantile -> assertThat(quantile.getValue()).isEqualTo(200));
}

@Test
void longTaskTimer() {
LongTaskTimer taskTimer = LongTaskTimer.builder(METER_NAME)
Expand Down
25 changes: 20 additions & 5 deletions ...eter-registry-otlp/src/test/java/io/micrometer/registry/otlp/OtlpMetricConverterTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,7 @@
*/
package io.micrometer.registry.otlp;

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.*;
import io.micrometer.core.instrument.config.NamingConvention;
import io.opentelemetry.proto.metrics.v1.Metric;
import org.junit.jupiter.api.BeforeEach;
Expand All @@ -32,6 +29,8 @@

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");
Expand All @@ -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);
}
Expand Down Expand Up @@ -179,4 +178,20 @@ void applyCustomNamingConvention() {
});
}

@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))));
}

}

0 comments on commit 9500ee5

Please sign in to comment.