Merge branch 'master' into dougqh/strings-improvements

Author: dougqh

.github/workflows/create-release-branch.yaml

@@ -1,4 +1,4 @@
-name: Create Release Branch
+name: Create Release Branch and Pin System Tests
 
 on:
   push:
@@ -37,9 +37,9 @@ jobs:
         id: define-release-branch
         run: |
           TAG=${{ steps.determine-tag.outputs.tag }}
-          echo "branch=test/${TAG%.0}.x" >> "$GITHUB_OUTPUT" # TODO: change back to release/ branch after testing
+          echo "branch=release/${TAG%.0}.x" >> "$GITHUB_OUTPUT"
 
-      - name: Checkout dd-trace-java at tag
+      - name: Check out repo at tag
         uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # 6.0.2
         with:
           ref: ${{ steps.determine-tag.outputs.tag }}
@@ -75,7 +75,7 @@ jobs:
           scope: DataDog/dd-trace-java
           policy: self.pin-system-tests.create-pr
 
-      - name: Checkout dd-trace-java at release branch
+      - name: Check out repo at release branch
         uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # 6.0.2
         with:
           ref: ${{ needs.create-release-branch.outputs.release-branch-name }}
@@ -85,7 +85,7 @@ jobs:
         run: |
           echo "sha=$(git rev-parse HEAD)" >> $GITHUB_OUTPUT
 
-      - name: Define pin-system-tests branch name
+      - name: Define pin-system-tests branch name from date
         id: define-pin-branch
         run: echo "branch=ci/pin-system-tests-$(date +'%Y%m%d')" >> $GITHUB_OUTPUT
 

.github/workflows/pin-system-tests.yaml

@@ -29,7 +29,7 @@ variables:
   GRADLE_VERSION: "8.14.4" # must match gradle-wrapper.properties
   MAVEN_REPOSITORY_PROXY: "https://depot-read-api-java.us1.ddbuild.io/magicmirror/magicmirror/@current/"
   GRADLE_PLUGIN_PROXY: "https://depot-read-api-java.us1.ddbuild.io/magicmirror/magicmirror/@current/"
-  BUILDER_IMAGE_VERSION_PREFIX: "v26.01-" # use either an empty string (e.g. "") for latest images or a version followed by a hyphen (e.g. "v25.05-")
+  BUILDER_IMAGE_VERSION_PREFIX: "v26.02-" # use either an empty string (e.g. "") for latest images or a version followed by a hyphen (e.g. "v25.05-")
   REPO_NOTIFICATION_CHANNEL: "#apm-java-escalations"
   DEFAULT_TEST_JVMS: /^(8|11|17|21|25|stable)$/ # the latest "stable" version is 26
   PROFILE_TESTS:

.gitlab-ci.yml

@@ -1,12 +1,32 @@
 package datadog.trace.bootstrap.instrumentation.decorator;
 
+import datadog.trace.api.TagMap;
 import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
 import datadog.trace.bootstrap.instrumentation.api.Tags;
 
 public abstract class ClientDecorator extends BaseDecorator {
+  // Deliberately not volatile, reading a stale null and creating an extra Entry is safe
+  private TagMap.Entry cachedSpanKindEntry = null;
 
   protected abstract String service();
 
+  /** Caches span kind entry to reduce allocation */
+  private final TagMap.Entry spanKindEntry() {
+    // DQH - I considered moving the creation of the TagMap.Entry into a ClientDecorator
+    // constructor, but that introduces a subtle ordering requirement.
+
+    // If the spanKind method refers to a static that isn't yet initialized,
+    // then spanKind will return null when the Decorator singleton is being constructed.
+
+    // Such an ordering problem did occur with similar changes in BaseDecorator, so I've
+    // decided to be cautious here, too.
+    TagMap.Entry kindEntry = cachedSpanKindEntry;
+    if (kindEntry == null) {
+      cachedSpanKindEntry = kindEntry = TagMap.Entry.create(Tags.SPAN_KIND, spanKind());
+    }
+    return kindEntry;
+  }
+
   protected String spanKind() {
     return Tags.SPAN_KIND_CLIENT;
   }
@@ -17,7 +37,7 @@ public AgentSpan afterStart(final AgentSpan span) {
     if (service != null) {
       span.setServiceName(service, component());
     }
-    span.setTag(Tags.SPAN_KIND, spanKind());
+    span.setTag(spanKindEntry());
 
     // Generate metrics for all client spans.
     span.setMeasured(true);

dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/instrumentation/decorator/ClientDecorator.java

@@ -26,11 +26,12 @@ class ClientDecoratorTest extends BaseDecoratorTest {
     1 * span.setTag(TagMap.Entry.create(Tags.COMPONENT, "test-component"))
     1 * span.context() >> spanContext
     1 * spanContext.setIntegrationName("test-component")
-    1 * span.setTag(Tags.SPAN_KIND, "client")
+    1 * span.setTag(TagMap.Entry.create(Tags.SPAN_KIND, "client"))
     1 * span.setSpanType(decorator.spanType())
     1 * span.setMetric(TagMap.Entry.create(DDTags.ANALYTICS_SAMPLE_RATE, 1.0))
     _ * span.setTag(_)
     _ * span.setTag(_, _) // Want to allow other calls from child implementations.
+    _ * span.setTag(_)
     _ * span.setServiceName(_)
     _ * span.setOperationName(_)
     0 * _

dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/ClientDecoratorTest.groovy

@@ -1,6 +1,7 @@
 package datadog.trace.bootstrap.instrumentation.decorator
 
 import datadog.trace.api.DDTags
+import datadog.trace.api.TagMap
 import datadog.trace.bootstrap.instrumentation.api.AgentSpan
 import datadog.trace.bootstrap.instrumentation.api.Tags
 import datadog.trace.bootstrap.instrumentation.api.UTF8BytesString
@@ -26,7 +27,7 @@ class DBTypeProcessingDatabaseClientDecoratorTest extends ClientDecoratorTest {
     1 * span.setTag(Tags.COMPONENT, "test-component")
     1 * span.context() >> spanContext
     1 * spanContext.setIntegrationName("test-component")
-    1 * span.setTag(Tags.SPAN_KIND, "client")
+    1 * span.setTag(TagMap.Entry.create(Tags.SPAN_KIND, "client"))
     1 * span.setSpanType("test-type")
     1 * span.setServiceName("test-db")
     1 * span.setOperationName(UTF8BytesString.create("test-db.query"))

dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/DBTypeProcessingDatabaseClientDecoratorTest.groovy

@@ -30,7 +30,7 @@ class DatabaseClientDecoratorTest extends ClientDecoratorTest {
     1 * span.setTag(TagMap.Entry.create(Tags.COMPONENT, "test-component"))
     1 * span.context() >> spanContext
     1 * spanContext.setIntegrationName("test-component")
-    1 * span.setTag(Tags.SPAN_KIND, "client")
+    1 * span.setTag(TagMap.Entry.create(Tags.SPAN_KIND, "client"))
     1 * span.setSpanType("test-type")
     1 * span.setMetric(TagMap.Entry.create(DDTags.ANALYTICS_SAMPLE_RATE, 1.0))
     0 * _

dd-java-agent/agent-bootstrap/src/test/groovy/datadog/trace/bootstrap/instrumentation/decorator/DatabaseClientDecoratorTest.groovy

@@ -269,7 +269,7 @@ public void onTestFinish(
       if (outcome.lastExecution()) {
         test.setTag(Tags.TEST_FINAL_STATUS, outcome.finalStatus());
 
-        if (outcome.failedAllRetries()) {
+        if (retryReason != null && outcome.failedAllRetries()) {
           test.setTag(Tags.TEST_HAS_FAILED_ALL_RETRIES, true);
         }
 

dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/events/TestEventsHandlerImpl.java

@@ -4,29 +4,37 @@
 import datadog.trace.api.civisibility.execution.TestStatus;
 import datadog.trace.api.civisibility.telemetry.tag.RetryReason;
 import datadog.trace.civisibility.config.ExecutionsByDuration;
+import datadog.trace.civisibility.execution.exit.EarlyExitPolicy;
 import java.util.List;
 
-/** Runs a test case N times (N depends on test duration) regardless of success or failure. */
+/**
+ * Runs a test case N times (N depends on test duration) regardless of success or failure. The
+ * execution can also be terminated early if its ExitPolicy evaluates to {@code true}.
+ */
 public class RunNTimes implements TestExecutionPolicy {
 
   private final boolean suppressFailures;
   private final List<ExecutionsByDuration> executionsByDuration;
   private int executions;
   private int maxExecutions;
   private int successfulExecutionsSeen;
+  private int failedExecutionsSeen;
   private final RetryReason retryReason;
   private TestStatus lastStatus;
+  private final EarlyExitPolicy exitPolicy;
 
   public RunNTimes(
       List<ExecutionsByDuration> executionsByDuration,
       boolean suppressFailures,
-      RetryReason retryReason) {
+      RetryReason retryReason,
+      EarlyExitPolicy exitPolicy) {
     this.suppressFailures = suppressFailures;
     this.executionsByDuration = executionsByDuration;
     this.executions = 0;
     this.maxExecutions = getExecutions(0);
     this.successfulExecutionsSeen = 0;
     this.retryReason = retryReason;
+    this.exitPolicy = exitPolicy;
   }
 
   @Override
@@ -35,20 +43,21 @@ public ExecutionOutcome registerExecution(TestStatus status, long durationMillis
     ++executions;
     if (status != TestStatus.fail) {
       ++successfulExecutionsSeen;
+    } else {
+      ++failedExecutionsSeen;
     }
     int maxExecutionsForGivenDuration = getExecutions(durationMillis);
     maxExecutions = Math.min(maxExecutions, maxExecutionsForGivenDuration);
 
     boolean lastExecution = !retriesLeft();
     boolean retry = executions > 1; // first execution is not a retry
     boolean failureSuppressed = status == TestStatus.fail && suppressFailures();
-    boolean succeededAllRetries = lastExecution && successfulExecutionsSeen == executions;
-
+    boolean succeededAllExecutions = successfulExecutionsSeen == executions;
     TestStatus finalStatus = null;
     if (lastExecution) {
       // final status will only be "pass" if all retries pass (or the failures were suppressed)
       // also, the `suppressFailures()` call works because its value cannot change between retries
-      if (succeededAllRetries || suppressFailures()) {
+      if (succeededAllExecutions || suppressFailures()) {
         finalStatus = TestStatus.pass;
       } else {
         finalStatus = TestStatus.fail;
@@ -58,15 +67,17 @@ public ExecutionOutcome registerExecution(TestStatus status, long durationMillis
     return new ExecutionOutcomeImpl(
         failureSuppressed,
         lastExecution,
-        lastExecution && successfulExecutionsSeen == 0,
-        succeededAllRetries,
+        lastExecution && failedExecutionsSeen == executions,
+        lastExecution && succeededAllExecutions,
         retry ? retryReason : null,
         finalStatus);
   }
 
   private boolean retriesLeft() {
     // skipped tests (either by the framework or DD) should not be retried
-    return lastStatus != TestStatus.skip && executions < maxExecutions;
+    return lastStatus != TestStatus.skip
+        && executions < maxExecutions
+        && !exitPolicy.evaluate(failedExecutionsSeen != 0, successfulExecutionsSeen != 0);
   }
 
   @Override

dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/execution/RunNTimes.java

@@ -0,0 +1,9 @@
+package datadog.trace.civisibility.execution.exit;
+
+public interface EarlyExitPolicy {
+
+  /**
+   * @return {@code true} if the policy indicates that the test should not be retried anymore.
+   */
+  boolean evaluate(boolean hasFailedExecutions, boolean hasPassedExecutions);
+}