Refactor AgentTestRunner: extract a common interface, convert to Java class (#2223)

gradle/spotbugs-exclude.xml

@@ -43,6 +43,12 @@
     <Bug pattern="SE_BAD_FIELD_INNER_CLASS"/>
   </Match>
 
+  <Match>
+    <!-- writing to static field in a non-static method (clearAllExportedData()) -->
+    <Class name="io.opentelemetry.instrumentation.testing.LibraryTestRunner"/>
+    <Bug pattern="ST_WRITE_TO_STATIC_FROM_INSTANCE_METHOD"/>
+  </Match>
+
   <Match>
     <Class name="io.opentelemetry.instrumentation.api.tracer.utils.NetPeerUtils"/>
     <Method name="setNetPeer"

testing-common/src/main/groovy/io/opentelemetry/instrumentation/test/AgentTestRunner.java

@@ -1,62 +0,0 @@
-/*
- * Copyright The OpenTelemetry Authors
- * SPDX-License-Identifier: Apache-2.0
- */
-
-package io.opentelemetry.instrumentation.test;
-
-import ch.qos.logback.classic.Level;
-import ch.qos.logback.classic.Logger;
-import groovy.lang.Closure;
-import groovy.lang.DelegatesTo;
-import groovy.transform.stc.ClosureParams;
-import groovy.transform.stc.SimpleType;
-import io.opentelemetry.api.trace.Span;
-import io.opentelemetry.instrumentation.test.asserts.InMemoryExporterAssert;
-import io.opentelemetry.javaagent.testing.common.AgentTestingExporterAccess;
-import io.opentelemetry.javaagent.testing.common.TestAgentListenerAccess;
-import org.slf4j.LoggerFactory;
-
-public final class AgentTestRunner {
-  /**
-   * For test runs, agent's global tracer will report to this list writer.
-   *
-   * <p>Before the start of each test the reported traces will be reset.
-   */
-  public static final InMemoryExporter TEST_WRITER = new InMemoryExporter();
-
-  static {
-    ((Logger) LoggerFactory.getLogger(Logger.ROOT_LOGGER_NAME)).setLevel(Level.WARN);
-    ((Logger) LoggerFactory.getLogger("io.opentelemetry")).setLevel(Level.DEBUG);
-  }
-
-  public void setupBeforeTests() {
-    TestAgentListenerAccess.reset();
-  }
-
-  public void beforeTest() {
-    assert !Span.current().getSpanContext().isValid()
-        : "Span is active before test has started: " + Span.current();
-    AgentTestingExporterAccess.reset();
-  }
-
-  public static synchronized void agentCleanup() {
-    // Cleanup before assertion.
-    assert TestAgentListenerAccess.getInstrumentationErrorCount() == 0
-        : TestAgentListenerAccess.getInstrumentationErrorCount()
-            + " Instrumentation errors during test";
-    assert TestAgentListenerAccess.getIgnoredButTransformedClassNames().isEmpty()
-        : "Transformed classes match global libraries ignore matcher: "
-            + TestAgentListenerAccess.getIgnoredButTransformedClassNames();
-  }
-
-  public static void assertTraces(
-      int size,
-      @ClosureParams(
-              value = SimpleType.class,
-              options = "io.opentelemetry.instrumentation.test.asserts.ListWriterAssert")
-          @DelegatesTo(value = InMemoryExporterAssert.class, strategy = Closure.DELEGATE_FIRST)
-          Closure spec) {
-    InMemoryExporterAssert.assertTraces(AgentTestingExporterAccess::getExportedSpans, size, spec);
-  }
-}

testing-common/src/main/groovy/io/opentelemetry/instrumentation/test/AgentTestTrait.groovy

@@ -5,10 +5,8 @@
 
 package io.opentelemetry.instrumentation.test
 
-
-import groovy.transform.stc.ClosureParams
-import groovy.transform.stc.SimpleType
-import io.opentelemetry.instrumentation.test.asserts.InMemoryExporterAssert
+import io.opentelemetry.instrumentation.testing.AgentTestRunner
+import io.opentelemetry.instrumentation.testing.InstrumentationTestRunner
 
 /**
  * A trait which initializes agent tests, including bytecode manipulation and a test span exporter.
@@ -16,31 +14,10 @@ import io.opentelemetry.instrumentation.test.asserts.InMemoryExporterAssert
  */
 trait AgentTestTrait {
 
-  static AgentTestRunner agentTestRunner
-  static InMemoryExporter testWriter
-
-  void runnerSetupSpec() {
-    agentTestRunner = new AgentTestRunner()
-    testWriter = AgentTestRunner.TEST_WRITER
+  static InstrumentationTestRunner agentTestRunner = AgentTestRunner.instance()
+  static InMemoryExporter testWriter = new InMemoryExporter()
 
-    agentTestRunner.setupBeforeTests()
+  InstrumentationTestRunner testRunner() {
+    agentTestRunner
   }
-
-  void runnerSetup() {
-    agentTestRunner.beforeTest()
-  }
-
-  void runnerCleanupSpec() {
-    AgentTestRunner.agentCleanup()
-  }
-
-  void assertTraces(final int size,
-                    @ClosureParams(
-                      value = SimpleType,
-                      options = "io.opentelemetry.instrumentation.test.asserts.ListWriterAssert")
-                    @DelegatesTo(value = InMemoryExporterAssert, strategy = Closure.DELEGATE_FIRST)
-                    final Closure spec) {
-    AgentTestRunner.assertTraces(size, spec)
-  }
-
 }

testing-common/src/main/groovy/io/opentelemetry/instrumentation/test/InstrumentationSpecification.groovy

@@ -5,10 +5,11 @@
 
 package io.opentelemetry.instrumentation.test
 
-
 import groovy.transform.stc.ClosureParams
 import groovy.transform.stc.SimpleType
+import io.opentelemetry.api.trace.Span
 import io.opentelemetry.instrumentation.test.asserts.InMemoryExporterAssert
+import io.opentelemetry.instrumentation.testing.InstrumentationTestRunner
 import spock.lang.Specification
 
 /**
@@ -17,29 +18,35 @@ import spock.lang.Specification
  * {@link LibraryTestTrait}.
  */
 abstract class InstrumentationSpecification extends Specification {
+  abstract InstrumentationTestRunner testRunner()
+
   def setupSpec() {
-    runnerSetupSpec()
+    testRunner().beforeTestClass()
   }
 
-  abstract void runnerSetupSpec()
-
+  /**
+   * Clears all data exported during a test.
+   */
   def setup() {
-    runnerSetup()
+    assert !Span.current().getSpanContext().isValid(): "Span is active before test has started: " + Span.current()
+    testRunner().clearAllExportedData()
   }
 
-  abstract void runnerSetup()
-
   def cleanupSpec() {
-    runnerCleanupSpec()
+    testRunner().afterTestClass()
   }
 
-  abstract void runnerCleanupSpec()
+  boolean forceFlushCalled() {
+    return testRunner().forceFlushCalled()
+  }
 
-  abstract void assertTraces(
+  void assertTraces(
     final int size,
     @ClosureParams(
       value = SimpleType,
       options = "io.opentelemetry.instrumentation.test.asserts.ListWriterAssert")
     @DelegatesTo(value = InMemoryExporterAssert, strategy = Closure.DELEGATE_FIRST)
-    final Closure spec)
+    final Closure spec) {
+    InMemoryExporterAssert.assertTraces({ testRunner().getExportedSpans() }, size, spec)
+  }
 }

testing-common/src/main/groovy/io/opentelemetry/instrumentation/test/InstrumentationTestRunner.groovy

@@ -1,90 +0,0 @@
-/*
- * Copyright The OpenTelemetry Authors
- * SPDX-License-Identifier: Apache-2.0
- */
-
-package io.opentelemetry.instrumentation.test
-
-import groovy.transform.stc.ClosureParams
-import groovy.transform.stc.SimpleType
-import io.opentelemetry.api.trace.propagation.W3CTraceContextPropagator
-import io.opentelemetry.context.Context
-import io.opentelemetry.context.propagation.ContextPropagators
-import io.opentelemetry.instrumentation.test.asserts.InMemoryExporterAssert
-import io.opentelemetry.sdk.OpenTelemetrySdk
-import io.opentelemetry.sdk.common.CompletableResultCode
-import io.opentelemetry.sdk.testing.exporter.InMemorySpanExporter
-import io.opentelemetry.sdk.trace.ReadWriteSpan
-import io.opentelemetry.sdk.trace.ReadableSpan
-import io.opentelemetry.sdk.trace.SdkTracerProvider
-import io.opentelemetry.sdk.trace.SpanProcessor
-import io.opentelemetry.sdk.trace.export.SimpleSpanProcessor
-import org.junit.Before
-import spock.lang.Specification
-/**
- * A spock test runner which automatically initializes an in-memory exporter that can be used to
- * verify traces.
- */
-abstract class InstrumentationTestRunner extends Specification {
-
-  protected static final InMemorySpanExporter testExporter
-
-  private static boolean forceFlushCalled
-
-  static {
-    testExporter = InMemorySpanExporter.create()
-    OpenTelemetrySdk.builder()
-      .setTracerProvider(SdkTracerProvider.builder()
-        .addSpanProcessor(new FlushTrackingSpanProcessor())
-        .addSpanProcessor(SimpleSpanProcessor.create(testExporter))
-        .build())
-      .setPropagators(ContextPropagators.create(W3CTraceContextPropagator.getInstance()))
-      .buildAndRegisterGlobal()
-  }
-
-  @Before
-  void beforeTest() {
-    testExporter.reset()
-    forceFlushCalled = false
-  }
-
-  protected static boolean forceFlushCalled() {
-    return forceFlushCalled
-  }
-
-  protected static void assertTraces(
-    final int size,
-    @ClosureParams(
-      value = SimpleType,
-      options = "io.opentelemetry.instrumentation.test.asserts.ListWriterAssert")
-    @DelegatesTo(value = InMemoryExporterAssert, strategy = Closure.DELEGATE_FIRST)
-    final Closure spec) {
-    InMemoryExporterAssert.assertTraces({ testExporter.getFinishedSpanItems() }, size, spec)
-  }
-
-  static class FlushTrackingSpanProcessor implements SpanProcessor {
-    @Override
-    void onStart(Context parentContext, ReadWriteSpan span) {
-    }
-
-    @Override
-    boolean isStartRequired() {
-      return false
-    }
-
-    @Override
-    void onEnd(ReadableSpan span) {
-    }
-
-    @Override
-    boolean isEndRequired() {
-      return false
-    }
-
-    @Override
-    CompletableResultCode forceFlush() {
-      forceFlushCalled = true
-      return CompletableResultCode.ofSuccess()
-    }
-  }
-}

testing-common/src/main/groovy/io/opentelemetry/instrumentation/test/LibraryTestTrait.groovy

@@ -5,11 +5,8 @@
 
 package io.opentelemetry.instrumentation.test
 
-
-import groovy.transform.stc.ClosureParams
-import groovy.transform.stc.SimpleType
-import io.opentelemetry.instrumentation.test.asserts.InMemoryExporterAssert
-import io.opentelemetry.sdk.testing.exporter.InMemorySpanExporter
+import io.opentelemetry.instrumentation.testing.InstrumentationTestRunner
+import io.opentelemetry.instrumentation.testing.LibraryTestRunner
 
 /**
  * A trait which initializes instrumentation library tests, including a test span exporter. All
@@ -17,33 +14,13 @@ import io.opentelemetry.sdk.testing.exporter.InMemorySpanExporter
  */
 trait LibraryTestTrait {
 
-  static InstrumentationTestRunner instrumentationTestRunner
-  static InMemorySpanExporter testWriter
-
-  void runnerSetupSpec() {
-    instrumentationTestRunner = new InstrumentationTestRunnerImpl()
-    testWriter = InstrumentationTestRunner.testExporter
-  }
-
-  void runnerSetup() {
-    instrumentationTestRunner.beforeTest()
-  }
-
-  void runnerCleanupSpec() {
-  }
+  static InstrumentationTestRunner instrumentationTestRunner = LibraryTestRunner.instance()
 
-  boolean forceFlushCalled() {
-    return instrumentationTestRunner.forceFlushCalled()
+  static {
+    instrumentationTestRunner = LibraryTestRunner.instance()
   }
 
-  void assertTraces(final int size,
-                    @ClosureParams(
-                      value = SimpleType,
-                      options = "io.opentelemetry.instrumentation.test.asserts.ListWriterAssert")
-                    @DelegatesTo(value = InMemoryExporterAssert, strategy = Closure.DELEGATE_FIRST)
-                    final Closure spec) {
-    instrumentationTestRunner.assertTraces(size, spec)
+  InstrumentationTestRunner testRunner() {
+    instrumentationTestRunner
   }
-
-  static class InstrumentationTestRunnerImpl extends InstrumentationTestRunner {}
 }

testing-common/src/main/java/io/opentelemetry/instrumentation/testing/AgentTestRunner.java

@@ -0,0 +1,72 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.instrumentation.testing;
+
+import ch.qos.logback.classic.Level;
+import ch.qos.logback.classic.Logger;
+import io.opentelemetry.javaagent.testing.common.AgentTestingExporterAccess;
+import io.opentelemetry.javaagent.testing.common.TestAgentListenerAccess;
+import io.opentelemetry.sdk.metrics.data.MetricData;
+import io.opentelemetry.sdk.trace.data.SpanData;
+import java.util.List;
+import org.slf4j.LoggerFactory;
+
+/**
+ * An implementation of {@link InstrumentationTestRunner} that delegates most of its calls to the
+ * OpenTelemetry Javaagent that this process runs with. It uses the {@link
+ * AgentTestingExporterAccess} bridge class to retrieve exported traces and metrics data from the
+ * agent classloader.
+ */
+public final class AgentTestRunner implements InstrumentationTestRunner {
+  static {
+    ((Logger) LoggerFactory.getLogger(Logger.ROOT_LOGGER_NAME)).setLevel(Level.WARN);
+    ((Logger) LoggerFactory.getLogger("io.opentelemetry")).setLevel(Level.DEBUG);
+  }
+
+  private static final AgentTestRunner INSTANCE = new AgentTestRunner();
+
+  public static InstrumentationTestRunner instance() {
+    return INSTANCE;
+  }
+
+  @Override
+  public void beforeTestClass() {
+    TestAgentListenerAccess.reset();
+  }
+
+  @Override
+  public void afterTestClass() {
+    // Cleanup before assertion.
+    assert TestAgentListenerAccess.getInstrumentationErrorCount() == 0
+        : TestAgentListenerAccess.getInstrumentationErrorCount()
+            + " Instrumentation errors during test";
+    assert TestAgentListenerAccess.getIgnoredButTransformedClassNames().isEmpty()
+        : "Transformed classes match global libraries ignore matcher: "
+            + TestAgentListenerAccess.getIgnoredButTransformedClassNames();
+  }
+
+  @Override
+  public void clearAllExportedData() {
+    AgentTestingExporterAccess.reset();
+  }
+
+  @Override
+  public List<SpanData> getExportedSpans() {
+    return AgentTestingExporterAccess.getExportedSpans();
+  }
+
+  @Override
+  public List<MetricData> getExportedMetrics() {
+    return AgentTestingExporterAccess.getExportedMetrics();
+  }
+
+  @Override
+  public boolean forceFlushCalled() {
+    return AgentTestingExporterAccess.forceFlushCalled();
+  }
+
+  private AgentTestRunner() {}
+}

testing-common/src/main/java/io/opentelemetry/instrumentation/testing/InstrumentationTestRunner.java

@@ -0,0 +1,31 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.instrumentation.testing;
+
+import io.opentelemetry.sdk.metrics.data.MetricData;
+import io.opentelemetry.sdk.trace.data.SpanData;
+import java.util.List;
+
+/**
+ * This interface defines a common set of operations for interaction with OpenTelemetry SDK and
+ * traces & metrics exporters.
+ *
+ * @see LibraryTestRunner
+ * @see AgentTestRunner
+ */
+public interface InstrumentationTestRunner {
+  void beforeTestClass();
+
+  void afterTestClass();
+
+  void clearAllExportedData();
+
+  List<SpanData> getExportedSpans();
+
+  List<MetricData> getExportedMetrics();
+
+  boolean forceFlushCalled();
+}

testing-common/src/main/java/io/opentelemetry/instrumentation/testing/LibraryTestRunner.java

@@ -0,0 +1,103 @@
+/*
+ * Copyright The OpenTelemetry Authors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package io.opentelemetry.instrumentation.testing;
+
+import io.opentelemetry.api.trace.propagation.W3CTraceContextPropagator;
+import io.opentelemetry.context.Context;
+import io.opentelemetry.context.propagation.ContextPropagators;
+import io.opentelemetry.sdk.OpenTelemetrySdk;
+import io.opentelemetry.sdk.common.CompletableResultCode;
+import io.opentelemetry.sdk.metrics.data.MetricData;
+import io.opentelemetry.sdk.testing.exporter.InMemorySpanExporter;
+import io.opentelemetry.sdk.trace.ReadWriteSpan;
+import io.opentelemetry.sdk.trace.ReadableSpan;
+import io.opentelemetry.sdk.trace.SdkTracerProvider;
+import io.opentelemetry.sdk.trace.SpanProcessor;
+import io.opentelemetry.sdk.trace.data.SpanData;
+import io.opentelemetry.sdk.trace.export.SimpleSpanProcessor;
+import java.util.Collections;
+import java.util.List;
+
+/**
+ * An implementation of {@link InstrumentationTestRunner} that initializes OpenTelemetry SDK and
+ * uses in-memory exporter to collect traces and metrics.
+ */
+public final class LibraryTestRunner implements InstrumentationTestRunner {
+
+  protected static final InMemorySpanExporter testExporter;
+  private static boolean forceFlushCalled;
+  private static final LibraryTestRunner INSTANCE = new LibraryTestRunner();
+
+  static {
+    testExporter = InMemorySpanExporter.create();
+    OpenTelemetrySdk.builder()
+        .setTracerProvider(
+            SdkTracerProvider.builder()
+                .addSpanProcessor(new FlushTrackingSpanProcessor())
+                .addSpanProcessor(SimpleSpanProcessor.create(testExporter))
+                .build())
+        .setPropagators(ContextPropagators.create(W3CTraceContextPropagator.getInstance()))
+        .buildAndRegisterGlobal();
+  }
+
+  public static InstrumentationTestRunner instance() {
+    return INSTANCE;
+  }
+
+  @Override
+  public void beforeTestClass() {}
+
+  @Override
+  public void afterTestClass() {}
+
+  @Override
+  public void clearAllExportedData() {
+    testExporter.reset();
+    forceFlushCalled = false;
+  }
+
+  @Override
+  public List<SpanData> getExportedSpans() {
+    return testExporter.getFinishedSpanItems();
+  }
+
+  @Override
+  public List<MetricData> getExportedMetrics() {
+    // no metrics support yet
+    return Collections.emptyList();
+  }
+
+  @Override
+  public boolean forceFlushCalled() {
+    return forceFlushCalled;
+  }
+
+  private LibraryTestRunner() {}
+
+  private static class FlushTrackingSpanProcessor implements SpanProcessor {
+    @Override
+    public void onStart(Context parentContext, ReadWriteSpan span) {}
+
+    @Override
+    public boolean isStartRequired() {
+      return false;
+    }
+
+    @Override
+    public void onEnd(ReadableSpan span) {}
+
+    @Override
+    public boolean isEndRequired() {
+      return false;
+    }
+
+    @Override
+    public CompletableResultCode forceFlush() {
+      forceFlushCalled = true;
+      return CompletableResultCode.ofSuccess();
+    }
+  }
+}