Merge branch 'master' into alexeyk/protocol-v1-1

# Conflicts:
#	dd-trace-core/src/main/java/datadog/trace/core/DDSpanContext.java

Author: AlexeyKuznetsov-DD

.gitlab-ci.yml

@@ -4,10 +4,16 @@ include:
   - local: ".gitlab/macrobenchmarks.yml"
   - local: ".gitlab/exploration-tests.yml"
   - local: ".gitlab/ci-visibility-tests.yml"
+  - project: 'DataDog/apm-reliability/apm-sdks-benchmarks'
+    file: '.gitlab/ci-java-spring-petclinic-parallel.yml'
+    ref: 'main'
 
 stages:
   - build
   - publish
+  # These benchmarks are intended to replace the legacy benchmarks in the future
+  - java-spring-petclinic-parallel
+  - java-spring-petclinic-parallel-slo
   - shared-pipeline
   - benchmarks
   - macrobenchmarks

dd-java-agent/appsec/src/main/java/com/datadog/appsec/config/AppSecConfigServiceImpl.java

@@ -422,7 +422,7 @@ public void maybeSubscribeConfigPolling() {
       } else {
         subscribeConfigurationPoller();
       }
-    } else {
+    } else if (!tracerConfig.isAwsServerless()) {
       log.info("Remote config is disabled; AppSec will not be able to use it");
     }
   }

dd-java-agent/instrumentation-testing/src/main/groovy/datadog/trace/agent/test/base/HttpServerTest.groovy

@@ -1170,6 +1170,7 @@ abstract class HttpServerTest<SERVER> extends WithHttpServer<SERVER> {
     }
 
     response.body().contentLength() < 1 || redirectHasBody()
+    response.close()
 
     and:
     assertTraces(1) {
@@ -1210,6 +1211,8 @@ abstract class HttpServerTest<SERVER> extends WithHttpServer<SERVER> {
     if (bubblesResponse()) {
       assert response.body().string().contains(ERROR.body)
       assert response.code() == ERROR.status
+    } else {
+      response.close()
     }
 
     and:
@@ -1255,6 +1258,8 @@ abstract class HttpServerTest<SERVER> extends WithHttpServer<SERVER> {
     response.code() == EXCEPTION.status
     if (testExceptionBody()) {
       assert response.body().string() == EXCEPTION.body
+    } else {
+      response.close()
     }
 
     and:
@@ -1299,6 +1304,7 @@ abstract class HttpServerTest<SERVER> extends WithHttpServer<SERVER> {
 
     expect:
     response.code() == NOT_FOUND.status
+    response.close()
 
     and:
     assertTraces(1) {
@@ -1836,6 +1842,7 @@ abstract class HttpServerTest<SERVER> extends WithHttpServer<SERVER> {
     response.code() == 301
     response.header('location') == 'https://www.google.com/'
     !handlerRan
+    response.close()
 
     when:
     TEST_WRITER.waitForTraces(1)
@@ -2117,6 +2124,7 @@ abstract class HttpServerTest<SERVER> extends WithHttpServer<SERVER> {
     }
     response.code() == 301
     response.header("Location") == 'https://www.google.com/'
+    response.close()
     TEST_WRITER.waitForTraces(1)
     def trace = TEST_WRITER.get(0)
 

dd-java-agent/instrumentation-testing/src/main/java/datadog/trace/agent/test/AbstractInstrumentationTest.java

@@ -14,7 +14,7 @@
 import datadog.trace.api.Config;
 import datadog.trace.api.IdGenerationStrategy;
 import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
-import datadog.trace.bootstrap.instrumentation.api.AgentTracer;
+import datadog.trace.bootstrap.instrumentation.api.AgentTracer.TracerAPI;
 import datadog.trace.common.writer.ListWriter;
 import datadog.trace.core.CoreTracer;
 import datadog.trace.core.DDSpan;
@@ -30,48 +30,56 @@
 import java.util.function.Function;
 import java.util.function.Predicate;
 import net.bytebuddy.agent.ByteBuddyAgent;
+import org.junit.jupiter.api.AfterAll;
 import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.extension.ExtendWith;
 import org.opentest4j.AssertionFailedError;
 
 /**
- * This class is an experimental base to run instrumentation tests using JUnit Jupiter. It is still
- * early development, and the overall API is expected to change to leverage its extension model. The
- * current implementation is inspired and kept close to it Groovy / Spock counterpart, the {@code
- * InstrumentationSpecification}.
+ * Base class for instrumentation tests using JUnit Jupiter.
+ *
+ * <p>It is still early development, and the overall API might change to leverage its extension
+ * model. The current implementation is inspired and kept close to its Groovy / Spock counterpart,
+ * the {@code InstrumentationSpecification}.
+ *
+ * <ul>
+ *   <li>{@code @BeforeAll}: Installs the agent and creates a shared tracer
+ *   <li>{@code @BeforeEach}: Flushes and resets the writer
+ *   <li>{@code @AfterEach}: Flushes the tracer
+ *   <li>{@code @AfterAll}: Closes the tracer and removes the agent transformer
+ * </ul>
  */
 @ExtendWith({TestClassShadowingExtension.class, AllowContextTestingExtension.class})
 public abstract class AbstractInstrumentationTest {
   static final Instrumentation INSTRUMENTATION = ByteBuddyAgent.getInstrumentation();
 
   static final long TIMEOUT_MILLIS = TimeUnit.SECONDS.toMillis(20);
 
-  protected AgentTracer.TracerAPI tracer;
+  protected static final InstrumentationTestConfig testConfig = new InstrumentationTestConfig();
 
-  protected ListWriter writer;
+  protected static TracerAPI tracer;
+  protected static ListWriter writer;
+  private static ClassFileTransformer activeTransformer;
+  private static ClassFileTransformerListener transformerListener;
 
-  protected ClassFileTransformer activeTransformer;
-  protected ClassFileTransformerListener transformerLister;
-
-  @BeforeEach
-  public void init() {
+  @BeforeAll
+  static void initAll() {
     // If this fails, it's likely the result of another test loading Config before it can be
     // injected into the bootstrap classpath.
-    // If one test extends AgentTestRunner in a module, all tests must extend
     assertNull(Config.class.getClassLoader(), "Config must load on the bootstrap classpath.");
 
-    // Initialize test tracer
-    this.writer = new ListWriter();
-    // Initialize test tracer
-    CoreTracer tracer =
+    // Create shared test writer and tracer
+    writer = new ListWriter();
+    CoreTracer coreTracer =
         CoreTracer.builder()
-            .writer(this.writer)
-            .idGenerationStrategy(IdGenerationStrategy.fromName(idGenerationStrategyName()))
-            .strictTraceWrites(useStrictTraceWrites())
+            .writer(writer)
+            .idGenerationStrategy(IdGenerationStrategy.fromName(testConfig.idGenerationStrategy))
+            .strictTraceWrites(testConfig.strictTraceWrites)
             .build();
-    TracerInstaller.forceInstallGlobalTracer(tracer);
-    this.tracer = tracer;
+    TracerInstaller.forceInstallGlobalTracer(coreTracer);
+    tracer = coreTracer;
 
     ClassInjector.enableClassInjection(INSTRUMENTATION);
 
@@ -85,33 +93,43 @@ public void init() {
             .iterator()
             .hasNext(),
         "No instrumentation found");
-    this.transformerLister = new ClassFileTransformerListener();
-    this.activeTransformer =
+    transformerListener = new ClassFileTransformerListener();
+    activeTransformer =
         AgentInstaller.installBytebuddyAgent(
-            INSTRUMENTATION, true, AgentInstaller.getEnabledSystems(), this.transformerLister);
-  }
-
-  protected String idGenerationStrategyName() {
-    return "SEQUENTIAL";
+            INSTRUMENTATION, true, AgentInstaller.getEnabledSystems(), transformerListener);
   }
 
-  private boolean useStrictTraceWrites() {
-    return true;
+  @BeforeEach
+  public void init() {
+    tracer.flush();
+    writer.start();
   }
 
   @AfterEach
   public void tearDown() {
-    this.tracer.close();
-    this.writer.close();
-    if (this.activeTransformer != null) {
-      INSTRUMENTATION.removeTransformer(this.activeTransformer);
-      this.activeTransformer = null;
-    }
+    tracer.flush();
+  }
 
-    // All cleanups should happen before these assertions.
+  @AfterAll
+  static void tearDownAll() {
+    if (tracer != null) {
+      tracer.close();
+      tracer = null;
+    }
+    if (writer != null) {
+      writer.close();
+      writer = null;
+    }
+    if (activeTransformer != null) {
+      INSTRUMENTATION.removeTransformer(activeTransformer);
+      activeTransformer = null;
+    }
+    // All cleanups should happen before this verify call.
     // If not, a failing assertion may prevent cleanup
-    this.transformerLister.verify();
-    this.transformerLister = null;
+    if (transformerListener != null) {
+      transformerListener.verify();
+      transformerListener = null;
+    }
   }
 
   /**
@@ -134,11 +152,11 @@ protected void assertTraces(
       TraceMatcher... matchers) {
     int expectedTraceCount = matchers.length;
     try {
-      this.writer.waitForTraces(expectedTraceCount);
+      writer.waitForTraces(expectedTraceCount);
     } catch (InterruptedException | TimeoutException e) {
       throw new AssertionFailedError("Timeout while waiting for traces", e);
     }
-    TraceAssertions.assertTraces(this.writer, options, matchers);
+    TraceAssertions.assertTraces(writer, options, matchers);
   }
 
   /**
@@ -149,7 +167,7 @@ protected void assertTraces(
    */
   protected void blockUntilTracesMatch(Predicate<List<List<DDSpan>>> predicate) {
     long deadline = System.currentTimeMillis() + TIMEOUT_MILLIS;
-    while (!predicate.test(this.writer)) {
+    while (!predicate.test(writer)) {
       if (System.currentTimeMillis() > deadline) {
         throw new RuntimeException(new TimeoutException("Timed out waiting for traces/spans."));
       }
@@ -161,8 +179,8 @@ protected void blockUntilTracesMatch(Predicate<List<List<DDSpan>>> predicate) {
     }
   }
 
-  protected void blockUntilChildSpansFinished(final int numberOfSpans) {
-    blockUntilChildSpansFinished(this.tracer.activeSpan(), numberOfSpans);
+  protected void blockUntilChildSpansFinished(int numberOfSpans) {
+    blockUntilChildSpansFinished(tracer.activeSpan(), numberOfSpans);
   }
 
   static void blockUntilChildSpansFinished(AgentSpan span, int numberOfSpans) {
@@ -190,4 +208,20 @@ static void blockUntilChildSpansFinished(AgentSpan span, int numberOfSpans) {
       }
     }
   }
+
+  /** Configuration for {@link AbstractInstrumentationTest}. */
+  protected static class InstrumentationTestConfig {
+    private String idGenerationStrategy = "SEQUENTIAL";
+    private boolean strictTraceWrites = true;
+
+    public InstrumentationTestConfig idGenerationStrategy(String strategy) {
+      this.idGenerationStrategy = strategy;
+      return this;
+    }
+
+    public InstrumentationTestConfig strictTraceWrites(boolean strict) {
+      this.strictTraceWrites = strict;
+      return this;
+    }
+  }
 }

dd-java-agent/instrumentation-testing/src/main/java/datadog/trace/agent/test/assertions/Matchers.java

@@ -1,9 +1,10 @@
 package datadog.trace.agent.test.assertions;
 
+import static org.junit.jupiter.api.AssertionFailureBuilder.assertionFailure;
+
 import java.util.Optional;
 import java.util.function.Predicate;
 import java.util.regex.Pattern;
-import org.opentest4j.AssertionFailedError;
 
 /** This class is a utility class to create generic matchers. */
 public final class Matchers {
@@ -103,12 +104,11 @@ public static <T> Matcher<T> any() {
   static <T> void assertValue(Matcher<T> matcher, T value, String message) {
     if (matcher != null && !matcher.test(value)) {
       Optional<T> expected = matcher.expected();
-      if (expected.isPresent()) {
-        throw new AssertionFailedError(
-            message + ". " + matcher.failureReason(), expected.get(), value);
-      } else {
-        throw new AssertionFailedError(message + ": " + value + ". " + matcher.failureReason());
-      }
+      assertionFailure()
+          .message(message + ". " + matcher.failureReason())
+          .expected(expected.orElse(null))
+          .actual(value)
+          .buildAndThrow();
     }
   }
 }

dd-java-agent/instrumentation-testing/src/main/java/datadog/trace/agent/test/assertions/SpanMatcher.java

@@ -10,6 +10,7 @@
 import static datadog.trace.agent.test.assertions.Matchers.validates;
 import static datadog.trace.core.DDSpanAccessor.spanLinks;
 import static java.time.Duration.ofNanos;
+import static org.junit.jupiter.api.AssertionFailureBuilder.assertionFailure;
 
 import datadog.trace.api.TagMap;
 import datadog.trace.bootstrap.instrumentation.api.AgentSpanLink;
@@ -322,7 +323,7 @@ private void assertSpanTags(TagMap tags) {
           if (matcher == null) {
             uncheckedTagNames.add(key);
           } else {
-            assertValue(matcher, value, "Unexpected " + key + " tag value.");
+            assertValue(matcher, value, "Unexpected " + key + " tag value");
           }
         });
     // Remove matchers that accept missing tags
@@ -344,10 +345,18 @@ private void assertSpanTags(TagMap tags) {
    * It might evolve into partial link collection testing, matching links using TID/SIP.
    */
   private void assertSpanLinks(List<AgentSpanLink> links) {
+    // Check if links should be asserted at all
+    if (this.linkMatchers == null) {
+      return;
+    }
     int linkCount = links == null ? 0 : links.size();
-    int expectedLinkCount = this.linkMatchers == null ? 0 : this.linkMatchers.length;
+    int expectedLinkCount = this.linkMatchers.length;
     if (linkCount != expectedLinkCount) {
-      throw new AssertionFailedError("Unexpected span link count", expectedLinkCount, linkCount);
+      assertionFailure()
+          .message("Unexpected span link count")
+          .expected(expectedLinkCount)
+          .actual(linkCount)
+          .buildAndThrow();
     }
     for (int i = 0; i < expectedLinkCount; i++) {
       SpanLinkMatcher linkMatcher = this.linkMatchers[expectedLinkCount];