diff --git a/apm-agent-plugins/apm-opentelemetry/apm-opentelemetry-embedded-metrics-sdk/src/main/java/co/elastic/apm/agent/embeddedotel/proxy/ProxyDoubleHistogramBuilder.java b/apm-agent-plugins/apm-opentelemetry/apm-opentelemetry-embedded-metrics-sdk/src/main/java/co/elastic/apm/agent/embeddedotel/proxy/ProxyDoubleHistogramBuilder.java index 91127d6736..5d4b0034ec 100644 --- a/apm-agent-plugins/apm-opentelemetry/apm-opentelemetry-embedded-metrics-sdk/src/main/java/co/elastic/apm/agent/embeddedotel/proxy/ProxyDoubleHistogramBuilder.java +++ b/apm-agent-plugins/apm-opentelemetry/apm-opentelemetry-embedded-metrics-sdk/src/main/java/co/elastic/apm/agent/embeddedotel/proxy/ProxyDoubleHistogramBuilder.java @@ -18,9 +18,9 @@ */ package co.elastic.apm.agent.embeddedotel.proxy; -import io.opentelemetry.api.metrics.DoubleHistogram; +import co.elastic.apm.agent.configuration.MetricsConfiguration; +import co.elastic.apm.agent.tracer.GlobalTracer; import io.opentelemetry.api.metrics.DoubleHistogramBuilder; -import io.opentelemetry.api.metrics.LongHistogramBuilder; import java.util.List; @@ -30,6 +30,9 @@ public class ProxyDoubleHistogramBuilder { public ProxyDoubleHistogramBuilder(DoubleHistogramBuilder delegate) { this.delegate = delegate; + //apply default bucket boundaries + List boundaries = GlobalTracer.get().getConfig(MetricsConfiguration.class).getCustomMetricsHistogramBoundaries(); + delegate.setExplicitBucketBoundariesAdvice(boundaries); } public DoubleHistogramBuilder getDelegate() { diff --git a/apm-agent-plugins/apm-opentelemetry/apm-opentelemetry-embedded-metrics-sdk/src/main/java/co/elastic/apm/agent/embeddedotel/proxy/ProxyLongHistogramBuilder.java b/apm-agent-plugins/apm-opentelemetry/apm-opentelemetry-embedded-metrics-sdk/src/main/java/co/elastic/apm/agent/embeddedotel/proxy/ProxyLongHistogramBuilder.java index 210ea31b60..6f21a53441 100644 --- a/apm-agent-plugins/apm-opentelemetry/apm-opentelemetry-embedded-metrics-sdk/src/main/java/co/elastic/apm/agent/embeddedotel/proxy/ProxyLongHistogramBuilder.java +++ b/apm-agent-plugins/apm-opentelemetry/apm-opentelemetry-embedded-metrics-sdk/src/main/java/co/elastic/apm/agent/embeddedotel/proxy/ProxyLongHistogramBuilder.java @@ -18,9 +18,11 @@ */ package co.elastic.apm.agent.embeddedotel.proxy; -import io.opentelemetry.api.metrics.LongHistogram; +import co.elastic.apm.agent.configuration.MetricsConfiguration; +import co.elastic.apm.agent.tracer.GlobalTracer; import io.opentelemetry.api.metrics.LongHistogramBuilder; +import java.util.ArrayList; import java.util.List; public class ProxyLongHistogramBuilder { @@ -29,6 +31,21 @@ public class ProxyLongHistogramBuilder { public ProxyLongHistogramBuilder(LongHistogramBuilder delegate) { this.delegate = delegate; + //apply default bucket boundaries + List boundaries = GlobalTracer.get().getConfig(MetricsConfiguration.class).getCustomMetricsHistogramBoundaries(); + delegate.setExplicitBucketBoundariesAdvice(convertToLongBoundaries(boundaries)); + } + + private List convertToLongBoundaries(List boundaries) { + List result = new ArrayList<>(); + for(double val : boundaries) { + long rounded = Math.round(val); + //Do not add the same boundary twice + if(rounded > 0 && (result.isEmpty() || result.get(result.size() - 1) != rounded)) { + result.add(rounded); + } + } + return result; } public LongHistogramBuilder getDelegate() { diff --git a/apm-agent-plugins/apm-opentelemetry/apm-opentelemetry-metricsdk-plugin/src/main/java/co/elastic/apm/agent/otelmetricsdk/ElasticOtelMetricsExporter.java b/apm-agent-plugins/apm-opentelemetry/apm-opentelemetry-metricsdk-plugin/src/main/java/co/elastic/apm/agent/otelmetricsdk/ElasticOtelMetricsExporter.java index 941cf14f05..d07abdbe53 100644 --- a/apm-agent-plugins/apm-opentelemetry/apm-opentelemetry-metricsdk-plugin/src/main/java/co/elastic/apm/agent/otelmetricsdk/ElasticOtelMetricsExporter.java +++ b/apm-agent-plugins/apm-opentelemetry/apm-opentelemetry-metricsdk-plugin/src/main/java/co/elastic/apm/agent/otelmetricsdk/ElasticOtelMetricsExporter.java @@ -25,6 +25,7 @@ import co.elastic.apm.agent.sdk.internal.util.ExecutorUtils; import co.elastic.apm.agent.tracer.configuration.MetricsConfiguration; import co.elastic.apm.agent.tracer.configuration.ReporterConfiguration; +import io.opentelemetry.api.metrics.DoubleHistogramBuilder; import io.opentelemetry.sdk.common.CompletableResultCode; import io.opentelemetry.sdk.metrics.Aggregation; import io.opentelemetry.sdk.metrics.InstrumentType; @@ -37,6 +38,7 @@ import java.time.Duration; import java.util.Collection; +import java.util.List; public class ElasticOtelMetricsExporter implements MetricExporter { @@ -44,6 +46,8 @@ public class ElasticOtelMetricsExporter implements MetricExporter { private static final AggregationTemporalitySelector TEMPORALITY_SELECTOR = AggregationTemporalitySelector.deltaPreferred(); + private static final boolean API_SUPPORTS_BUCKET_ADVICE = checkOtelApiSupportsHistogramBucketAdvice(); + private final Aggregation defaultHistogramAggregation; private final OtelMetricSerializer serializer; @@ -103,10 +107,22 @@ public AggregationTemporality getAggregationTemporality(InstrumentType instrumen @Override public Aggregation getDefaultAggregation(InstrumentType instrumentType) { - if (instrumentType == InstrumentType.HISTOGRAM) { + // Unfortunately advices are not applied when a non-default aggregation is returned here + // When instrumenting API-usages, we now apply the default histogram boundaries via the + // ProxyLongHistogramBuilder and ProxyDoubleHistogramBuilder + if (instrumentType == InstrumentType.HISTOGRAM && !API_SUPPORTS_BUCKET_ADVICE) { return defaultHistogramAggregation; } else { return Aggregation.defaultAggregation(); } } + + private static boolean checkOtelApiSupportsHistogramBucketAdvice() { + try { + DoubleHistogramBuilder.class.getMethod("setExplicitBucketBoundariesAdvice", List.class); + return true; + } catch (NoSuchMethodException e) { + return false; + } + } } diff --git a/apm-agent-plugins/apm-opentelemetry/apm-opentelemetry-metricsdk-plugin/src/test/java/co/elastic/apm/agent/otelmetricsdk/AbstractOtelMetricsTest.java b/apm-agent-plugins/apm-opentelemetry/apm-opentelemetry-metricsdk-plugin/src/test/java/co/elastic/apm/agent/otelmetricsdk/AbstractOtelMetricsTest.java index 53c3b29819..185e47ad99 100644 --- a/apm-agent-plugins/apm-opentelemetry/apm-opentelemetry-metricsdk-plugin/src/test/java/co/elastic/apm/agent/otelmetricsdk/AbstractOtelMetricsTest.java +++ b/apm-agent-plugins/apm-opentelemetry/apm-opentelemetry-metricsdk-plugin/src/test/java/co/elastic/apm/agent/otelmetricsdk/AbstractOtelMetricsTest.java @@ -32,6 +32,7 @@ import io.opentelemetry.api.metrics.DoubleCounter; import io.opentelemetry.api.metrics.DoubleGaugeBuilder; import io.opentelemetry.api.metrics.DoubleHistogram; +import io.opentelemetry.api.metrics.DoubleHistogramBuilder; import io.opentelemetry.api.metrics.DoubleUpDownCounter; import io.opentelemetry.api.metrics.LongCounter; import io.opentelemetry.api.metrics.LongHistogram; @@ -588,44 +589,44 @@ public void testUpDownCounter() { @Test public void testHistogram() { MetricsConfiguration metricsConfig = config.getConfig(MetricsConfiguration.class); - doReturn(List.of(1.0, 5.0)).when(metricsConfig).getCustomMetricsHistogramBoundaries(); + doReturn(List.of(5d, 10d, 25d)).when(metricsConfig).getCustomMetricsHistogramBoundaries(); Meter testMeter = createMeter("test"); DoubleHistogram doubleHisto = testMeter.histogramBuilder("double_histo").build(); LongHistogram longHisto = testMeter.histogramBuilder("long_histo").ofLongs().build(); doubleHisto.record(0.5); - doubleHisto.record(1.5); - doubleHisto.record(1.5); - doubleHisto.record(5.5); - doubleHisto.record(5.5); - doubleHisto.record(5.5); + doubleHisto.record(6.5); + doubleHisto.record(6.5); + doubleHisto.record(10.5); + doubleHisto.record(10.5); + doubleHisto.record(10.5); - longHisto.record(0); - longHisto.record(2); - longHisto.record(2); - longHisto.record(6); + longHisto.record(1); longHisto.record(6); longHisto.record(6); + longHisto.record(11); + longHisto.record(11); + longHisto.record(11); resetReporterAndFlushMetrics(); assertThatMetricSets(reporter.getBytes()) .hasMetricsetCount(1) .first() - .containsHistogramMetric("double_histo", List.of(0.5, 3.0, 5.0), List.of(1L, 2L, 3L)) - .containsHistogramMetric("long_histo", List.of(0.5, 3.0, 5.0), List.of(1L, 2L, 3L)) + .containsHistogramMetric("double_histo", List.of(2.5, 7.5, 17.5), List.of(1L, 2L, 3L)) + .containsHistogramMetric("long_histo", List.of(2.5, 7.5, 17.5), List.of(1L, 2L, 3L)) .hasMetricsCount(2); //make sure only delta is reported and empty buckets are omitted - doubleHisto.record(1.5); - longHisto.record(2); + doubleHisto.record(6.5); + longHisto.record(6); resetReporterAndFlushMetrics(); assertThatMetricSets(reporter.getBytes()) .hasMetricsetCount(1) .first() - .containsHistogramMetric("double_histo", List.of(3.0), List.of(1L)) - .containsHistogramMetric("long_histo", List.of(3.0), List.of(1L)) + .containsHistogramMetric("double_histo", List.of(7.5), List.of(1L)) + .containsHistogramMetric("long_histo", List.of(7.5), List.of(1L)) .hasMetricsCount(2); //empty histograms must not be exported @@ -664,9 +665,56 @@ public void testDefaultHistogramBuckets() { .hasMetricsetCount(1) .first() .metricSatisfies("double_histo", - metric -> assertThat(metric.counts.stream().mapToLong(Long::longValue).sum()).isEqualTo(totalSumFinal)) + metric -> { + assertThat(metric.counts).hasSizeGreaterThan(20); + assertThat(metric.counts.stream().mapToLong(Long::longValue).sum()).isEqualTo(totalSumFinal); + }) .metricSatisfies("long_histo", - metric -> assertThat(metric.counts.stream().mapToLong(Long::longValue).sum()).isEqualTo(totalSumFinal)) + metric -> { + assertThat(metric.counts).hasSizeGreaterThan(20); + assertThat(metric.counts.stream().mapToLong(Long::longValue).sum()).isEqualTo(totalSumFinal); + }) + .hasMetricsCount(2); + } + + + @Test + public void testHistogramAdviceAPI() { + try { + DoubleHistogramBuilder.class.getMethod("setExplicitBucketBoundariesAdvice", List.class); + } catch (NoSuchMethodException expected) { + //we are in an integration test where .setExplicitBucketBoundariesAdvice() doesn't exist, skip this test + return; + } + + Meter testMeter = createMeter("test"); + DoubleHistogram doubleHisto = testMeter.histogramBuilder("double_histo") + .setExplicitBucketBoundariesAdvice(List.of(2.0 ,6.0)) + .build(); + LongHistogram longHisto = testMeter.histogramBuilder("long_histo").ofLongs() + .setExplicitBucketBoundariesAdvice(List.of(2L ,6L)) + .build(); + + doubleHisto.record(0.5); + doubleHisto.record(2.5); + doubleHisto.record(2.5); + doubleHisto.record(7.5); + doubleHisto.record(7.5); + doubleHisto.record(7.5); + + longHisto.record(0); + longHisto.record(3); + longHisto.record(3); + longHisto.record(7); + longHisto.record(7); + longHisto.record(7); + + resetReporterAndFlushMetrics(); + assertThatMetricSets(reporter.getBytes()) + .hasMetricsetCount(1) + .first() + .containsHistogramMetric("double_histo", List.of(1.0, 4.0, 6.0), List.of(1L, 2L, 3L)) + .containsHistogramMetric("long_histo", List.of(1.0, 4.0, 6.0), List.of(1L, 2L, 3L)) .hasMetricsCount(2); } diff --git a/apm-agent-plugins/apm-opentelemetry/apm-opentelemetry-metricsdk-plugin/src/test/java/co/elastic/apm/agent/otelmetricsdk/PrivateUserSdkOtelMetricsTest.java b/apm-agent-plugins/apm-opentelemetry/apm-opentelemetry-metricsdk-plugin/src/test/java/co/elastic/apm/agent/otelmetricsdk/PrivateUserSdkOtelMetricsTest.java index f31884653b..5bd9e711cf 100644 --- a/apm-agent-plugins/apm-opentelemetry/apm-opentelemetry-metricsdk-plugin/src/test/java/co/elastic/apm/agent/otelmetricsdk/PrivateUserSdkOtelMetricsTest.java +++ b/apm-agent-plugins/apm-opentelemetry/apm-opentelemetry-metricsdk-plugin/src/test/java/co/elastic/apm/agent/otelmetricsdk/PrivateUserSdkOtelMetricsTest.java @@ -20,6 +20,7 @@ import co.elastic.apm.agent.configuration.MetricsConfiguration; import io.opentelemetry.api.metrics.DoubleHistogram; +import io.opentelemetry.api.metrics.DoubleHistogramBuilder; import io.opentelemetry.api.metrics.Meter; import io.opentelemetry.api.metrics.MeterProvider; import io.opentelemetry.sdk.metrics.Aggregation; @@ -95,6 +96,23 @@ protected void invokeSdkForceFlush() { ((SdkMeterProvider) getMeterProvider()).forceFlush(); } + + @Test + @Override + public void testDefaultHistogramBuckets() { + // Unfortunately default histogram buckets don't work with user-provided SDKs of version 1.32.0 or newer + // The reason is that a default aggregation provided by the exporter would override + // bucket boundaries set via DoubleHistogramBuilder.setExplicitBucketBoundaries + // We decided to instead respect the bucket boundaries provided by the API + try { + DoubleHistogramBuilder.class.getMethod("setExplicitBucketBoundariesAdvice", List.class); + //Method exists, default bucket boundaries are not supported + //Don't execute test for that reason + } catch (NoSuchMethodException e) { + super.testDefaultHistogramBuckets(); + } + } + @Test public void testCustomHistogramView() { sdkCustomizer = builder -> builder.registerView( diff --git a/apm-agent-plugins/apm-opentelemetry/apm-opentelemetry-test/src/test/java/co/elastic/apm/agent/opentelemetry/OpenTelemetryVersionIT.java b/apm-agent-plugins/apm-opentelemetry/apm-opentelemetry-test/src/test/java/co/elastic/apm/agent/opentelemetry/OpenTelemetryVersionIT.java index d617ad5f0f..7dc9ec7d6f 100644 --- a/apm-agent-plugins/apm-opentelemetry/apm-opentelemetry-test/src/test/java/co/elastic/apm/agent/opentelemetry/OpenTelemetryVersionIT.java +++ b/apm-agent-plugins/apm-opentelemetry/apm-opentelemetry-test/src/test/java/co/elastic/apm/agent/opentelemetry/OpenTelemetryVersionIT.java @@ -65,25 +65,18 @@ void testTracingVersion(String version) throws Exception { } @ParameterizedTest - @ValueSource(strings = { - "1.10.0", - //"1.11.0", - //"1.12.0", - //"1.13.0", - "1.14.0", - "1.15.0", - //"1.16.0", - //"1.17.0", - //"1.18.0", - //"1.19.0", - //"1.20.0", - "1.21.0" - }) - void testAgentProvidedMetricsSdkForApiVersion(String version) throws Exception { + @CsvSource(value = { + "1.10.0|io.opentelemetry:opentelemetry-semconv:1.10.0-alpha", + "1.14.0|io.opentelemetry:opentelemetry-semconv:1.14.0-alpha", + "1.15.0|io.opentelemetry:opentelemetry-semconv:1.15.0-alpha", + "1.21.0|io.opentelemetry:opentelemetry-semconv:1.21.0-alpha", + "1.31.0|io.opentelemetry.semconv:opentelemetry-semconv:1.22.0-alpha", + }, delimiterString = "|") + void testAgentProvidedMetricsSdkForApiVersion(String version, String semConvDep) throws Exception { List dependencies = List.of( "io.opentelemetry:opentelemetry-api:" + version, "io.opentelemetry:opentelemetry-context:" + version, - "io.opentelemetry:opentelemetry-semconv:" + version + "-alpha"); + semConvDep); TestClassWithDependencyRunner runner = new TestClassWithDependencyRunner(dependencies, "co.elastic.apm.agent.opentelemetry.metrics.AgentProvidedSdkOtelMetricsTest", "co.elastic.apm.agent.otelmetricsdk.AbstractOtelMetricsTest", @@ -93,21 +86,18 @@ void testAgentProvidedMetricsSdkForApiVersion(String version) throws Exception { } @ParameterizedTest - @ValueSource(strings = { - "1.16.0", - //"1.17.0", - //"1.18.0", - //"1.19.0", - //"1.20.0", - "1.21.0" - }) - void testUserProvidedMetricsSdkVersion(String version) throws Exception { + @CsvSource(value = { + "1.16.0|io.opentelemetry:opentelemetry-semconv:1.16.0-alpha", + "1.21.0|io.opentelemetry:opentelemetry-semconv:1.21.0-alpha", + "1.31.0|io.opentelemetry.semconv:opentelemetry-semconv:1.22.0-alpha", + }, delimiterString = "|") + void testUserProvidedMetricsSdkVersion(String version, String semConvDep) throws Exception { List dependencies = List.of( "io.opentelemetry:opentelemetry-api:" + version, "io.opentelemetry:opentelemetry-sdk-metrics:" + version, "io.opentelemetry:opentelemetry-sdk-common:" + version, "io.opentelemetry:opentelemetry-context:" + version, - "io.opentelemetry:opentelemetry-semconv:" + version + "-alpha"); + semConvDep); TestClassWithDependencyRunner runner = new TestClassWithDependencyRunner(dependencies, "co.elastic.apm.agent.otelmetricsdk.PrivateUserSdkOtelMetricsTest", "co.elastic.apm.agent.otelmetricsdk.AbstractOtelMetricsTest",