From a40988ea1a37cadda17567404b944396782ee6cd Mon Sep 17 00:00:00 2001 From: Jonas Kunz Date: Wed, 20 Sep 2023 13:53:23 +0200 Subject: [PATCH] OpenTelemetry metrics exporter instrumentation breaks with instrument=false (#3326) * OpenTelemetry metrics exporter instrumentation should be enabled even with instrument=false * Added changelog * Reproduce failure in tests --- CHANGELOG.asciidoc | 1 + ...dkMeterProviderBuilderInstrumentation.java | 9 +++- .../AbstractOtelMetricsTest.java | 44 +++++++++++++++++-- 3 files changed, 49 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index f8a602aa51..1db9649ac0 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -47,6 +47,7 @@ Use subheadings with the "=====" level for adding notes for unreleased changes: * Fix micrometer histogram serialization - {pull}3290[#3290], {pull}3304[#3304] * Fix transactions not being correctly handled in certain edge cases - {pull}3294[#3294] * Fixed JDBC instrumentation for DB2 - {pull}3313[#3313] +* Fixed OpenTelemetry metrics export breaking when `instrument=false` is configured - {pull}3326[#3326] [[release-notes-1.x]] === Java Agent version 1.x diff --git a/apm-agent-plugins/apm-opentelemetry/apm-opentelemetry-metricsdk-plugin/src/main/java/co/elastic/apm/agent/otelmetricsdk/SdkMeterProviderBuilderInstrumentation.java b/apm-agent-plugins/apm-opentelemetry/apm-opentelemetry-metricsdk-plugin/src/main/java/co/elastic/apm/agent/otelmetricsdk/SdkMeterProviderBuilderInstrumentation.java index 91b982a349..2cc64c554c 100644 --- a/apm-agent-plugins/apm-opentelemetry/apm-opentelemetry-metricsdk-plugin/src/main/java/co/elastic/apm/agent/otelmetricsdk/SdkMeterProviderBuilderInstrumentation.java +++ b/apm-agent-plugins/apm-opentelemetry/apm-opentelemetry-metricsdk-plugin/src/main/java/co/elastic/apm/agent/otelmetricsdk/SdkMeterProviderBuilderInstrumentation.java @@ -19,13 +19,13 @@ package co.elastic.apm.agent.otelmetricsdk; import co.elastic.apm.agent.impl.ElasticApmTracer; -import co.elastic.apm.agent.tracer.GlobalTracer; import co.elastic.apm.agent.sdk.ElasticApmInstrumentation; +import co.elastic.apm.agent.sdk.internal.util.LoggerUtils; import co.elastic.apm.agent.sdk.logging.Logger; import co.elastic.apm.agent.sdk.logging.LoggerFactory; import co.elastic.apm.agent.sdk.weakconcurrent.WeakConcurrent; import co.elastic.apm.agent.sdk.weakconcurrent.WeakSet; -import co.elastic.apm.agent.sdk.internal.util.LoggerUtils; +import co.elastic.apm.agent.tracer.GlobalTracer; import io.opentelemetry.sdk.metrics.SdkMeterProviderBuilder; import io.opentelemetry.sdk.metrics.export.MetricExporter; import net.bytebuddy.asm.Advice; @@ -55,6 +55,11 @@ public Collection getInstrumentationGroupNames() { return Arrays.asList("opentelemetry", "opentelemetry-metrics", "experimental"); } + @Override + public final boolean includeWhenInstrumentationIsDisabled() { + return true; + } + @Override public String getAdviceClassName() { return getClass().getName() + "$SdkMeterProviderBuilderAdvice"; 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 ef93c11b3b..53c3b29819 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 @@ -18,9 +18,14 @@ */ package co.elastic.apm.agent.otelmetricsdk; -import co.elastic.apm.agent.AbstractInstrumentationTest; +import co.elastic.apm.agent.MockReporter; +import co.elastic.apm.agent.MockTracer; +import co.elastic.apm.agent.bci.ElasticApmAgent; import co.elastic.apm.agent.common.util.WildcardMatcher; +import co.elastic.apm.agent.configuration.CoreConfiguration; import co.elastic.apm.agent.configuration.MetricsConfiguration; +import co.elastic.apm.agent.configuration.SpyConfiguration; +import co.elastic.apm.agent.impl.ElasticApmTracer; import co.elastic.apm.agent.report.ReporterConfiguration; import co.elastic.apm.agent.util.AtomicDouble; import io.opentelemetry.api.common.Attributes; @@ -41,8 +46,13 @@ import io.opentelemetry.api.metrics.ObservableLongGauge; import io.opentelemetry.api.metrics.ObservableLongMeasurement; import io.opentelemetry.api.metrics.ObservableLongUpDownCounter; +import net.bytebuddy.agent.ByteBuddyAgent; +import org.junit.AfterClass; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.stagemonitor.configuration.ConfigurationRegistry; import javax.annotation.Nullable; import java.time.Instant; @@ -63,7 +73,12 @@ import static io.opentelemetry.api.common.AttributeKey.stringKey; import static org.mockito.Mockito.doReturn; -public abstract class AbstractOtelMetricsTest extends AbstractInstrumentationTest { +public abstract class AbstractOtelMetricsTest { + + protected static ElasticApmTracer tracer; + protected static MockReporter reporter; + protected static ConfigurationRegistry config; + private ReporterConfiguration reporterConfig; @@ -74,9 +89,32 @@ public abstract class AbstractOtelMetricsTest extends AbstractInstrumentationTes @Nullable private MeterProvider meterProvider; + + @BeforeAll + public static synchronized void beforeAll() { + MockTracer.MockInstrumentationSetup mockInstrumentationSetup = MockTracer.createMockInstrumentationSetup(); + config = mockInstrumentationSetup.getConfig(); + tracer = mockInstrumentationSetup.getTracer(); + reporter = mockInstrumentationSetup.getReporter(); + + //Metrics export should work even with instrument=false + CoreConfiguration coreConfig = config.getConfig(CoreConfiguration.class); + doReturn(false).when(coreConfig).isInstrument(); + + assertThat(tracer.isRunning()).isTrue(); + ElasticApmAgent.initInstrumentation(tracer, ByteBuddyAgent.install()); + } + + @AfterAll + @AfterClass + public static synchronized void afterAll() { + ElasticApmAgent.reset(); + } + @BeforeEach public void setup() { - reporterConfig = tracer.getConfig(ReporterConfiguration.class); + SpyConfiguration.reset(config); + reporterConfig = config.getConfig(ReporterConfiguration.class); // we use explicit flush in tests instead of periodic reporting to prevent flakyness doReturn(1_000_000L).when(reporterConfig).getMetricsIntervalMs(); meterProvider = null;