From ff5595aee6236d3ebcbc5d4a001cd4d9e42f4143 Mon Sep 17 00:00:00 2001 From: Jack Berg Date: Tue, 28 Nov 2023 15:35:07 -0600 Subject: [PATCH 1/2] Convert histogram measurements to double before passing recording exemplar reservoir --- .../internal/exemplar/ExemplarReservoir.java | 14 +++ .../LongToDoubleExemplarReservoir.java | 35 +++++++ .../Base2ExponentialHistogramAggregation.java | 9 +- .../ExplicitBucketHistogramAggregation.java | 5 +- .../sdk/metrics/SdkDoubleHistogramTest.java | 95 +++++++++++++++++++ .../sdk/metrics/SdkLongHistogramTest.java | 95 +++++++++++++++++++ .../LongToDoubleExemplarReservoirTest.java | 39 ++++++++ 7 files changed, 286 insertions(+), 6 deletions(-) create mode 100644 sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/internal/exemplar/LongToDoubleExemplarReservoir.java create mode 100644 sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/internal/exemplar/LongToDoubleExemplarReservoirTest.java diff --git a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/internal/exemplar/ExemplarReservoir.java b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/internal/exemplar/ExemplarReservoir.java index cebd4683fbb..935c30dc143 100644 --- a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/internal/exemplar/ExemplarReservoir.java +++ b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/internal/exemplar/ExemplarReservoir.java @@ -8,6 +8,7 @@ import io.opentelemetry.api.common.Attributes; import io.opentelemetry.context.Context; import io.opentelemetry.sdk.common.Clock; +import io.opentelemetry.sdk.metrics.Aggregation; import io.opentelemetry.sdk.metrics.data.DoubleExemplarData; import io.opentelemetry.sdk.metrics.data.ExemplarData; import io.opentelemetry.sdk.metrics.data.LongExemplarData; @@ -25,6 +26,19 @@ */ public interface ExemplarReservoir { + /** + * Wraps a {@link ExemplarReservoir}, casting calls to {@link + * ExemplarReservoir#offerLongMeasurement(long, Attributes, Context)} to {@link + * ExemplarReservoir#offerDoubleMeasurement(double, Attributes, Context)} such that {@link + * ExemplarReservoir#collectAndReset(Attributes)} only returns {@link DoubleExemplarData}. + * + *

This is used for {@link Aggregation#explicitBucketHistogram()} and {@link + * Aggregation#base2ExponentialBucketHistogram()} which only support double measurements. + */ + static ExemplarReservoir longToDouble(ExemplarReservoir delegate) { + return new LongToDoubleExemplarReservoir<>(delegate); + } + /** Wraps a {@link ExemplarReservoir} with a measurement pre-filter. */ static ExemplarReservoir filtered( ExemplarFilter filter, ExemplarReservoir original) { diff --git a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/internal/exemplar/LongToDoubleExemplarReservoir.java b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/internal/exemplar/LongToDoubleExemplarReservoir.java new file mode 100644 index 00000000000..c3a6b98fced --- /dev/null +++ b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/internal/exemplar/LongToDoubleExemplarReservoir.java @@ -0,0 +1,35 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.sdk.metrics.internal.exemplar; + +import io.opentelemetry.api.common.Attributes; +import io.opentelemetry.context.Context; +import io.opentelemetry.sdk.metrics.data.ExemplarData; +import java.util.List; + +class LongToDoubleExemplarReservoir implements ExemplarReservoir { + + private final ExemplarReservoir delegate; + + LongToDoubleExemplarReservoir(ExemplarReservoir delegate) { + this.delegate = delegate; + } + + @Override + public void offerDoubleMeasurement(double value, Attributes attributes, Context context) { + delegate.offerDoubleMeasurement(value, attributes, context); + } + + @Override + public void offerLongMeasurement(long value, Attributes attributes, Context context) { + offerDoubleMeasurement((double) value, attributes, context); + } + + @Override + public List collectAndReset(Attributes pointAttributes) { + return delegate.collectAndReset(pointAttributes); + } +} diff --git a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/internal/view/Base2ExponentialHistogramAggregation.java b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/internal/view/Base2ExponentialHistogramAggregation.java index a0408bc32fd..0604f591327 100644 --- a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/internal/view/Base2ExponentialHistogramAggregation.java +++ b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/internal/view/Base2ExponentialHistogramAggregation.java @@ -72,10 +72,11 @@ public Aggregator createAggr () -> ExemplarReservoir.filtered( exemplarFilter, - ExemplarReservoir.doubleFixedSizeReservoir( - Clock.getDefault(), - Runtime.getRuntime().availableProcessors(), - RandomSupplier.platformDefault())), + ExemplarReservoir.longToDouble( + ExemplarReservoir.doubleFixedSizeReservoir( + Clock.getDefault(), + Runtime.getRuntime().availableProcessors(), + RandomSupplier.platformDefault()))), maxBuckets, maxScale); } diff --git a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/internal/view/ExplicitBucketHistogramAggregation.java b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/internal/view/ExplicitBucketHistogramAggregation.java index 1ec3867dc7e..1ad32b698ba 100644 --- a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/internal/view/ExplicitBucketHistogramAggregation.java +++ b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/internal/view/ExplicitBucketHistogramAggregation.java @@ -57,8 +57,9 @@ public Aggregator createAggr () -> ExemplarReservoir.filtered( exemplarFilter, - ExemplarReservoir.histogramBucketReservoir( - Clock.getDefault(), bucketBoundaries))); + ExemplarReservoir.longToDouble( + ExemplarReservoir.histogramBucketReservoir( + Clock.getDefault(), bucketBoundaries)))); } @Override diff --git a/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/SdkDoubleHistogramTest.java b/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/SdkDoubleHistogramTest.java index dd060736ed6..dd210faad46 100644 --- a/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/SdkDoubleHistogramTest.java +++ b/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/SdkDoubleHistogramTest.java @@ -14,12 +14,16 @@ import io.opentelemetry.api.common.Attributes; import io.opentelemetry.api.metrics.DoubleHistogram; import io.opentelemetry.api.metrics.Meter; +import io.opentelemetry.api.trace.Span; +import io.opentelemetry.api.trace.Tracer; +import io.opentelemetry.context.Scope; import io.opentelemetry.internal.testing.slf4j.SuppressLogger; import io.opentelemetry.sdk.common.InstrumentationScopeInfo; import io.opentelemetry.sdk.metrics.internal.state.DefaultSynchronousMetricStorage; import io.opentelemetry.sdk.resources.Resource; import io.opentelemetry.sdk.testing.exporter.InMemoryMetricReader; import io.opentelemetry.sdk.testing.time.TestClock; +import io.opentelemetry.sdk.trace.SdkTracerProvider; import java.time.Duration; import java.util.Arrays; import java.util.Collections; @@ -275,6 +279,97 @@ void doubleHistogramRecord_NaN() { assertThat(sdkMeterReader.collectAllMetrics()).hasSize(0); } + @Test + void collectMetrics_ExemplarsWithExponentialHistogram() { + InMemoryMetricReader reader = InMemoryMetricReader.create(); + SdkMeterProvider sdkMeterProvider = + SdkMeterProvider.builder() + .setClock(testClock) + .setResource(RESOURCE) + .registerView( + InstrumentSelector.builder().setType(InstrumentType.HISTOGRAM).build(), + View.builder() + .setAggregation(Aggregation.base2ExponentialBucketHistogram()) + .setAttributeFilter(Collections.emptySet()) + .build()) + .registerMetricReader(reader) + .build(); + Meter sdkMeter = sdkMeterProvider.get(getClass().getName()); + DoubleHistogram histogram = sdkMeter.histogramBuilder("testHistogram").build(); + + SdkTracerProvider tracerProvider = SdkTracerProvider.builder().build(); + Tracer tracer = tracerProvider.get("foo"); + + Span span = tracer.spanBuilder("span").startSpan(); + try (Scope unused = span.makeCurrent()) { + histogram.record(10, Attributes.builder().put("key", "value").build()); + } + + assertThat(reader.collectAllMetrics()) + .satisfiesExactly( + metric -> + assertThat(metric) + .hasExponentialHistogramSatisfying( + exponentialHistogram -> + exponentialHistogram.hasPointsSatisfying( + point -> + point + .hasSum(10.0) + .hasAttributes(Attributes.empty()) + .hasExemplarsSatisfying( + exemplar -> + exemplar + .hasValue(10.0) + .hasFilteredAttributes( + Attributes.builder() + .put("key", "value") + .build()))))); + } + + @Test + void collectMetrics_ExemplarsWithExplicitBucketHistogram() { + InMemoryMetricReader reader = InMemoryMetricReader.create(); + SdkMeterProvider sdkMeterProvider = + SdkMeterProvider.builder() + .setClock(testClock) + .setResource(RESOURCE) + .registerView( + InstrumentSelector.builder().setName("*").build(), + View.builder().setAttributeFilter(Collections.emptySet()).build()) + .registerMetricReader(reader) + .build(); + Meter sdkMeter = sdkMeterProvider.get(getClass().getName()); + DoubleHistogram histogram = sdkMeter.histogramBuilder("testHistogram").build(); + + SdkTracerProvider tracerProvider = SdkTracerProvider.builder().build(); + Tracer tracer = tracerProvider.get("foo"); + + Span span = tracer.spanBuilder("span").startSpan(); + try (Scope unused = span.makeCurrent()) { + histogram.record(10, Attributes.builder().put("key", "value").build()); + } + + assertThat(reader.collectAllMetrics()) + .satisfiesExactly( + metric -> + assertThat(metric) + .hasHistogramSatisfying( + explicitHistogram -> + explicitHistogram.hasPointsSatisfying( + point -> + point + .hasSum(10) + .hasAttributes(Attributes.empty()) + .hasExemplarsSatisfying( + exemplar -> + exemplar + .hasValue(10.0) + .hasFilteredAttributes( + Attributes.builder() + .put("key", "value") + .build()))))); + } + @Test void stressTest() { DoubleHistogram doubleHistogram = sdkMeter.histogramBuilder("testHistogram").build(); diff --git a/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/SdkLongHistogramTest.java b/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/SdkLongHistogramTest.java index 09e33faf2cc..dd802ff613f 100644 --- a/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/SdkLongHistogramTest.java +++ b/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/SdkLongHistogramTest.java @@ -14,11 +14,15 @@ import io.opentelemetry.api.common.Attributes; import io.opentelemetry.api.metrics.LongHistogram; import io.opentelemetry.api.metrics.Meter; +import io.opentelemetry.api.trace.Span; +import io.opentelemetry.api.trace.Tracer; +import io.opentelemetry.context.Scope; import io.opentelemetry.internal.testing.slf4j.SuppressLogger; import io.opentelemetry.sdk.common.InstrumentationScopeInfo; import io.opentelemetry.sdk.resources.Resource; import io.opentelemetry.sdk.testing.exporter.InMemoryMetricReader; import io.opentelemetry.sdk.testing.time.TestClock; +import io.opentelemetry.sdk.trace.SdkTracerProvider; import java.time.Duration; import java.util.Arrays; import java.util.Collections; @@ -435,6 +439,97 @@ void longHistogramRecord_NonNegativeCheck() { "Histograms can only record non-negative values. Instrument testHistogram has recorded a negative value."); } + @Test + void collectMetrics_ExemplarsWithExponentialHistogram() { + InMemoryMetricReader reader = InMemoryMetricReader.create(); + SdkMeterProvider sdkMeterProvider = + SdkMeterProvider.builder() + .setClock(testClock) + .setResource(RESOURCE) + .registerView( + InstrumentSelector.builder().setType(InstrumentType.HISTOGRAM).build(), + View.builder() + .setAggregation(Aggregation.base2ExponentialBucketHistogram()) + .setAttributeFilter(Collections.emptySet()) + .build()) + .registerMetricReader(reader) + .build(); + Meter sdkMeter = sdkMeterProvider.get(getClass().getName()); + LongHistogram histogram = sdkMeter.histogramBuilder("testHistogram").ofLongs().build(); + + SdkTracerProvider tracerProvider = SdkTracerProvider.builder().build(); + Tracer tracer = tracerProvider.get("foo"); + + Span span = tracer.spanBuilder("span").startSpan(); + try (Scope unused = span.makeCurrent()) { + histogram.record(10, Attributes.builder().put("key", "value").build()); + } + + assertThat(reader.collectAllMetrics()) + .satisfiesExactly( + metric -> + assertThat(metric) + .hasExponentialHistogramSatisfying( + exponentialHistogram -> + exponentialHistogram.hasPointsSatisfying( + point -> + point + .hasSum(10.0) + .hasAttributes(Attributes.empty()) + .hasExemplarsSatisfying( + exemplar -> + exemplar + .hasValue(10.0) + .hasFilteredAttributes( + Attributes.builder() + .put("key", "value") + .build()))))); + } + + @Test + void collectMetrics_ExemplarsWithExplicitBucketHistogram() { + InMemoryMetricReader reader = InMemoryMetricReader.create(); + SdkMeterProvider sdkMeterProvider = + SdkMeterProvider.builder() + .setClock(testClock) + .setResource(RESOURCE) + .registerView( + InstrumentSelector.builder().setName("*").build(), + View.builder().setAttributeFilter(Collections.emptySet()).build()) + .registerMetricReader(reader) + .build(); + Meter sdkMeter = sdkMeterProvider.get(getClass().getName()); + LongHistogram histogram = sdkMeter.histogramBuilder("testHistogram").ofLongs().build(); + + SdkTracerProvider tracerProvider = SdkTracerProvider.builder().build(); + Tracer tracer = tracerProvider.get("foo"); + + Span span = tracer.spanBuilder("span").startSpan(); + try (Scope unused = span.makeCurrent()) { + histogram.record(10, Attributes.builder().put("key", "value").build()); + } + + assertThat(reader.collectAllMetrics()) + .satisfiesExactly( + metric -> + assertThat(metric) + .hasHistogramSatisfying( + explicitHistogram -> + explicitHistogram.hasPointsSatisfying( + point -> + point + .hasSum(10) + .hasAttributes(Attributes.empty()) + .hasExemplarsSatisfying( + exemplar -> + exemplar + .hasValue(10.0) + .hasFilteredAttributes( + Attributes.builder() + .put("key", "value") + .build()))))); + } + @Test void stressTest() { LongHistogram longHistogram = sdkMeter.histogramBuilder("testHistogram").ofLongs().build(); diff --git a/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/internal/exemplar/LongToDoubleExemplarReservoirTest.java b/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/internal/exemplar/LongToDoubleExemplarReservoirTest.java new file mode 100644 index 00000000000..fc7a5a3c07a --- /dev/null +++ b/sdk/metrics/src/test/java/io/opentelemetry/sdk/metrics/internal/exemplar/LongToDoubleExemplarReservoirTest.java @@ -0,0 +1,39 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.sdk.metrics.internal.exemplar; + +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyLong; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; + +import io.opentelemetry.api.common.Attributes; +import io.opentelemetry.context.Context; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +@ExtendWith(MockitoExtension.class) +class LongToDoubleExemplarReservoirTest { + @Mock ExemplarReservoir delegate; + + @Test + void offerDoubleMeasurement() { + ExemplarReservoir filtered = new LongToDoubleExemplarReservoir<>(delegate); + filtered.offerDoubleMeasurement(1.0, Attributes.empty(), Context.root()); + verify(delegate).offerDoubleMeasurement(1.0, Attributes.empty(), Context.root()); + verify(delegate, never()).offerLongMeasurement(anyLong(), any(), any()); + } + + @Test + void offerLongMeasurement() { + ExemplarReservoir filtered = new LongToDoubleExemplarReservoir<>(delegate); + filtered.offerLongMeasurement(1L, Attributes.empty(), Context.root()); + verify(delegate).offerDoubleMeasurement(1.0, Attributes.empty(), Context.root()); + verify(delegate, never()).offerLongMeasurement(anyLong(), any(), any()); + } +} From c40a4cb042c894beb53cb7560f2b77a8403e5249 Mon Sep 17 00:00:00 2001 From: jack-berg <34418638+jack-berg@users.noreply.github.com> Date: Thu, 30 Nov 2023 10:00:58 -0600 Subject: [PATCH 2/2] Update sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/internal/exemplar/ExemplarReservoir.java Co-authored-by: Trask Stalnaker --- .../sdk/metrics/internal/exemplar/ExemplarReservoir.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/internal/exemplar/ExemplarReservoir.java b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/internal/exemplar/ExemplarReservoir.java index 935c30dc143..c0de6e0df6a 100644 --- a/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/internal/exemplar/ExemplarReservoir.java +++ b/sdk/metrics/src/main/java/io/opentelemetry/sdk/metrics/internal/exemplar/ExemplarReservoir.java @@ -27,7 +27,7 @@ public interface ExemplarReservoir { /** - * Wraps a {@link ExemplarReservoir}, casting calls to {@link + * Wraps an {@link ExemplarReservoir}, casting calls from {@link * ExemplarReservoir#offerLongMeasurement(long, Attributes, Context)} to {@link * ExemplarReservoir#offerDoubleMeasurement(double, Attributes, Context)} such that {@link * ExemplarReservoir#collectAndReset(Attributes)} only returns {@link DoubleExemplarData}.