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
elastic · Sep 20, 2023 · a40988e · a40988e
1 parent e968cf3
commit a40988e
Show file tree

Hide file tree

Showing 3 changed files with 49 additions and 5 deletions.
diff --git 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/.../main/java/co/elastic/apm/agent/otelmetricsdk/SdkMeterProviderBuilderInstrumentation.java b/.../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<String> 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/...csdk-plugin/src/test/java/co/elastic/apm/agent/otelmetricsdk/AbstractOtelMetricsTest.java b/...csdk-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;