Skip to content

Commit

Permalink
Added tests for bucket boundary advice API, changes application of de…
Browse files Browse the repository at this point in the history 
…fault bucket boundaries.
  • Loading branch information
@JonasKunz
JonasKunz committed Nov 29, 2023
1 parent 713adaf commit aea41d0
Show file tree
Hide file tree
Showing 6 changed files with 140 additions and 48 deletions.
7 changes: 5 additions & 2 deletions ...dk/src/main/java/co/elastic/apm/agent/embeddedotel/proxy/ProxyDoubleHistogramBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -30,6 +30,9 @@ public class ProxyDoubleHistogramBuilder {

public ProxyDoubleHistogramBuilder(DoubleHistogramBuilder delegate) {
this.delegate = delegate;
//apply default bucket boundaries
List<Double> boundaries = GlobalTracer.get().getConfig(MetricsConfiguration.class).getCustomMetricsHistogramBoundaries();
delegate.setExplicitBucketBoundariesAdvice(boundaries);
}

public DoubleHistogramBuilder getDelegate() {
Expand Down
19 changes: 18 additions & 1 deletion ...-sdk/src/main/java/co/elastic/apm/agent/embeddedotel/proxy/ProxyLongHistogramBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -29,6 +31,21 @@ public class ProxyLongHistogramBuilder {

public ProxyLongHistogramBuilder(LongHistogramBuilder delegate) {
this.delegate = delegate;
//apply default bucket boundaries
List<Double> boundaries = GlobalTracer.get().getConfig(MetricsConfiguration.class).getCustomMetricsHistogramBoundaries();
delegate.setExplicitBucketBoundariesAdvice(convertToLongBoundaries(boundaries));
}

private List<Long> convertToLongBoundaries(List<Double> boundaries) {
List<Long> 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() {
Expand Down
18 changes: 17 additions & 1 deletion ...k-plugin/src/main/java/co/elastic/apm/agent/otelmetricsdk/ElasticOtelMetricsExporter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -37,13 +38,16 @@

import java.time.Duration;
import java.util.Collection;
import java.util.List;

public class ElasticOtelMetricsExporter implements MetricExporter {

private static final Logger logger = LoggerFactory.getLogger(ElasticOtelMetricsExporter.class);

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;
Expand Down Expand Up @@ -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;
}
}
}
84 changes: 66 additions & 18 deletions ...csdk-plugin/src/test/java/co/elastic/apm/agent/otelmetricsdk/AbstractOtelMetricsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
}

Expand Down
18 changes: 18 additions & 0 deletions ...lugin/src/test/java/co/elastic/apm/agent/otelmetricsdk/PrivateUserSdkOtelMetricsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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(
Expand Down
42 changes: 16 additions & 26 deletions ...lemetry-test/src/test/java/co/elastic/apm/agent/opentelemetry/OpenTelemetryVersionIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<String> 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",
Expand All @@ -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<String> 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",
Expand Down

0 comments on commit aea41d0

Please sign in to comment.