Merge branch 'master' into nicholas.hulston/dynamodb-span-pointers

Author: nhulston

dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestFrameworkModule.java

@@ -29,7 +29,7 @@ TestSuiteImpl testSuiteStart(
 
   boolean isFlaky(@Nonnull TestIdentifier test);
 
-  boolean isModified(TestSourceData testSourceData);
+  boolean isModified(@Nonnull TestSourceData testSourceData);
 
   boolean isQuarantined(TestIdentifier test);
 
@@ -49,5 +49,11 @@ TestSuiteImpl testSuiteStart(
   @Nonnull
   TestExecutionPolicy executionPolicy(TestIdentifier test, TestSourceData testSource);
 
+  /**
+   * Returns the priority of the test execution that can be used for ordering tests. The higher the
+   * value, the higher the priority, meaning that the test should be executed earlier.
+   */
+  int executionPriority(@Nullable TestIdentifier test, @Nonnull TestSourceData testSource);
+
   void end(Long startTime);
 }

dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/buildsystem/ProxyTestModule.java

@@ -100,7 +100,7 @@ public boolean isFlaky(@Nonnull TestIdentifier test) {
   }
 
   @Override
-  public boolean isModified(TestSourceData testSourceData) {
+  public boolean isModified(@Nonnull TestSourceData testSourceData) {
     return executionStrategy.isModified(testSourceData);
   }
 
@@ -131,6 +131,11 @@ public TestExecutionPolicy executionPolicy(TestIdentifier test, TestSourceData t
     return executionStrategy.executionPolicy(test, testSource);
   }
 
+  @Override
+  public int executionPriority(@Nullable TestIdentifier test, @Nonnull TestSourceData testSource) {
+    return executionStrategy.executionPriority(test, testSource);
+  }
+
   @Override
   public void end(@Nullable Long endTime) {
     // we have no span locally,

dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/headless/HeadlessTestModule.java

@@ -85,7 +85,7 @@ public boolean isFlaky(@Nonnull TestIdentifier test) {
   }
 
   @Override
-  public boolean isModified(TestSourceData testSourceData) {
+  public boolean isModified(@Nonnull TestSourceData testSourceData) {
     return executionStrategy.isModified(testSourceData);
   }
 
@@ -116,6 +116,11 @@ public TestExecutionPolicy executionPolicy(TestIdentifier test, TestSourceData t
     return executionStrategy.executionPolicy(test, testSource);
   }
 
+  @Override
+  public int executionPriority(@Nullable TestIdentifier test, @Nonnull TestSourceData testSource) {
+    return executionStrategy.executionPriority(test, testSource);
+  }
+
   @Override
   public void end(@Nullable Long endTime) {
     ExecutionSettings executionSettings = executionStrategy.getExecutionSettings();

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

@@ -114,6 +114,12 @@ public boolean isFlaky(@Nonnull TestIdentifier test) {
     return false;
   }
 
+  @Override
+  public int executionPriority(
+      @Nullable TestIdentifier test, @Nonnull TestSourceData testSourceData) {
+    return 0;
+  }
+
   @Override
   public void close() {
     // do nothing

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

@@ -299,6 +299,11 @@ public TestExecutionPolicy executionPolicy(TestIdentifier test, TestSourceData t
     return testModule.executionPolicy(test, testSource);
   }
 
+  @Override
+  public int executionPriority(@Nullable TestIdentifier test, @Nonnull TestSourceData testSource) {
+    return testModule.executionPriority(test, testSource);
+  }
+
   @Override
   public boolean isNew(@Nonnull TestIdentifier test) {
     return testModule.isNew(test);

dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/test/ExecutionStrategy.java

@@ -187,7 +187,7 @@ public boolean isEFDLimitReached() {
     return detectionsUsed > threshold;
   }
 
-  public boolean isModified(TestSourceData testSourceData) {
+  public boolean isModified(@Nonnull TestSourceData testSourceData) {
     Class<?> testClass = testSourceData.getTestClass();
     if (testClass == null) {
       return false;
@@ -219,4 +219,28 @@ private LinesResolver.Lines getLines(Method testMethod) {
       return linesResolver.getMethodLines(testMethod);
     }
   }
+
+  /**
+   * Returns the priority of the test execution that can be used for ordering tests. The higher the
+   * value, the higher the priority, meaning that the test should be executed earlier.
+   */
+  public int executionPriority(@Nullable TestIdentifier test, @Nonnull TestSourceData testSource) {
+    if (test == null) {
+      return 0;
+    }
+    if (isNew(test)) {
+      // execute new tests first
+      return 300;
+    }
+    if (isModified(testSource)) {
+      // then modified tests
+      return 200;
+    }
+    if (isFlaky(test)) {
+      // then tests known to be flaky
+      return 100;
+    }
+    // then the rest
+    return 0;
+  }
 }

dd-java-agent/instrumentation/akka-http/akka-http-10.0/src/main/java/datadog/trace/instrumentation/akkahttp/DatadogServerRequestResponseFlowWrapper.java

@@ -1,6 +1,6 @@
 package datadog.trace.instrumentation.akkahttp;
 
-import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activeScope;
+import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activeSpan;
 
 import akka.http.scaladsl.model.HttpRequest;
 import akka.http.scaladsl.model.HttpResponse;
@@ -142,11 +142,11 @@ public void onPush() throws Exception {
                     response = newResponse;
                   }
                   DatadogWrapperHelper.finishSpan(span, response);
-                  // Check if the active scope is still the scope from when the request came in,
+                  // Check if the active span matches the scope from when the request came in,
                   // and close it. If it's not, then it will be cleaned up actor message
                   // processing instrumentation that drives this state machine
-                  AgentScope activeScope = activeScope();
-                  if (activeScope == scope) {
+                  AgentSpan activeSpan = activeSpan();
+                  if (activeSpan == scope.span()) {
                     scope.close();
                   }
                 }

dd-java-agent/instrumentation/junit-5.3/junit-5.8/src/main/java/datadog/trace/instrumentation/junit5/order/FailFastClassOrderer.java

@@ -1,6 +1,7 @@
 package datadog.trace.instrumentation.junit5.order;
 
 import datadog.trace.api.civisibility.config.TestIdentifier;
+import datadog.trace.api.civisibility.config.TestSourceData;
 import datadog.trace.api.civisibility.events.TestEventsHandler;
 import datadog.trace.instrumentation.junit5.JUnitPlatformUtils;
 import java.util.Comparator;
@@ -15,41 +16,44 @@ public class FailFastClassOrderer implements ClassOrderer {
 
   private final TestEventsHandler<TestDescriptor, TestDescriptor> testEventsHandler;
   private final @Nullable ClassOrderer delegate;
-  private final Comparator<ClassDescriptor> knownTestSuitesComparator;
+  private final Comparator<ClassDescriptor> executionOrderComparator;
 
   public FailFastClassOrderer(
       TestEventsHandler<TestDescriptor, TestDescriptor> testEventsHandler,
       @Nullable ClassOrderer delegate) {
     this.testEventsHandler = testEventsHandler;
     this.delegate = delegate;
-    this.knownTestSuitesComparator = Comparator.comparing(this::knownAndStableTestsPercentage);
+    this.executionOrderComparator = Comparator.comparing(this::classExecutionPriority).reversed();
   }
 
-  private int knownAndStableTestsPercentage(ClassDescriptor classDescriptor) {
+  private int classExecutionPriority(ClassDescriptor classDescriptor) {
     TestDescriptor testDescriptor = JUnit5OrderUtils.getTestDescriptor(classDescriptor);
-    return 100 - unknownAndFlakyTestsPercentage(testDescriptor);
+    return executionPriority(testDescriptor);
   }
 
-  private int unknownAndFlakyTestsPercentage(TestDescriptor testDescriptor) {
+  private int executionPriority(TestDescriptor testDescriptor) {
     if (testDescriptor == null) {
       return 0;
     }
+
     if (testDescriptor.isTest() || JUnitPlatformUtils.isParameterizedTest(testDescriptor)) {
       TestIdentifier testIdentifier = JUnitPlatformUtils.toTestIdentifier(testDescriptor);
-      return testEventsHandler.isNew(testIdentifier) || testEventsHandler.isFlaky(testIdentifier)
-          ? 100
-          : 0;
+      TestSourceData testSourceData = JUnitPlatformUtils.toTestSourceData(testDescriptor);
+      return testEventsHandler.executionPriority(testIdentifier, testSourceData);
     }
+
     Set<? extends TestDescriptor> children = testDescriptor.getChildren();
     if (children.isEmpty()) {
       return 0;
     }
 
-    int uknownTestsPercentage = 0;
+    // there's no specific meaning to the average of children priorities,
+    // it is just a heuristic to try to favor classes with higher percentage of failure-prone tests
+    int childrenPrioritySum = 0;
     for (TestDescriptor child : children) {
-      uknownTestsPercentage += unknownAndFlakyTestsPercentage(child);
+      childrenPrioritySum += executionPriority(child);
     }
-    return uknownTestsPercentage / children.size();
+    return childrenPrioritySum / children.size();
   }
 
   @Override
@@ -60,6 +64,6 @@ public void orderClasses(ClassOrdererContext classOrdererContext) {
     }
     // then apply our ordering (since sorting is stable, delegate's ordering will be preserved for
     // classes with the same "known/stable" status)
-    classOrdererContext.getClassDescriptors().sort(knownTestSuitesComparator);
+    classOrdererContext.getClassDescriptors().sort(executionOrderComparator);
   }
 }

dd-java-agent/instrumentation/junit-5.3/junit-5.8/src/main/java/datadog/trace/instrumentation/junit5/order/FailFastMethodOrderer.java

@@ -1,6 +1,7 @@
 package datadog.trace.instrumentation.junit5.order;
 
 import datadog.trace.api.civisibility.config.TestIdentifier;
+import datadog.trace.api.civisibility.config.TestSourceData;
 import datadog.trace.api.civisibility.events.TestEventsHandler;
 import datadog.trace.instrumentation.junit5.JUnitPlatformUtils;
 import java.util.Comparator;
@@ -14,23 +15,24 @@ public class FailFastMethodOrderer implements MethodOrderer {
 
   private final TestEventsHandler<TestDescriptor, TestDescriptor> testEventsHandler;
   private final @Nullable MethodOrderer delegate;
-  private final Comparator<MethodDescriptor> knownTestMethodsComparator;
+  private final Comparator<MethodDescriptor> executionOrderComparator;
 
   public FailFastMethodOrderer(
       TestEventsHandler<TestDescriptor, TestDescriptor> testEventsHandler,
       @Nullable MethodOrderer delegate) {
     this.testEventsHandler = testEventsHandler;
     this.delegate = delegate;
-    this.knownTestMethodsComparator = Comparator.comparing(this::isKnownAndStable);
+    this.executionOrderComparator = Comparator.comparing(this::executionPriority).reversed();
   }
 
-  private boolean isKnownAndStable(MethodDescriptor methodDescriptor) {
+  private int executionPriority(MethodDescriptor methodDescriptor) {
     TestDescriptor testDescriptor = JUnit5OrderUtils.getTestDescriptor(methodDescriptor);
     if (testDescriptor == null) {
-      return true;
+      return 0;
     }
     TestIdentifier testIdentifier = JUnitPlatformUtils.toTestIdentifier(testDescriptor);
-    return !testEventsHandler.isNew(testIdentifier) && !testEventsHandler.isFlaky(testIdentifier);
+    TestSourceData testSourceData = JUnitPlatformUtils.toTestSourceData(testDescriptor);
+    return testEventsHandler.executionPriority(testIdentifier, testSourceData);
   }
 
   @Override
@@ -41,6 +43,6 @@ public void orderMethods(MethodOrdererContext methodOrdererContext) {
     }
     // then apply our ordering (since sorting is stable, delegate's ordering will be preserved for
     // methods with the same "known/stable" status)
-    methodOrdererContext.getMethodDescriptors().sort(knownTestMethodsComparator);
+    methodOrdererContext.getMethodDescriptors().sort(executionOrderComparator);
   }
 }

dd-java-agent/instrumentation/pekko-http-1.0/src/main/java/datadog/trace/instrumentation/pekkohttp/DatadogServerRequestResponseFlowWrapper.java

@@ -1,8 +1,9 @@
 package datadog.trace.instrumentation.pekkohttp;
 
-import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activeScope;
+import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.activeSpan;
 
 import datadog.trace.bootstrap.instrumentation.api.AgentScope;
+import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
 import java.util.Queue;
 import java.util.concurrent.ArrayBlockingQueue;
 import org.apache.pekko.http.scaladsl.model.HttpRequest;
@@ -113,11 +114,11 @@ public void onPush() throws Exception {
                 final AgentScope scope = scopes.poll();
                 if (scope != null) {
                   DatadogWrapperHelper.finishSpan(scope.span(), response);
-                  // Check if the active scope is still the scope from when the request came in,
+                  // Check if the active span matches the scope from when the request came in,
                   // and close it. If it's not, then it will be cleaned up actor message
                   // processing instrumentation that drives this state machine
-                  AgentScope activeScope = activeScope();
-                  if (activeScope == scope) {
+                  AgentSpan activeSpan = activeSpan();
+                  if (activeSpan == scope.span()) {
                     scope.close();
                   }
                 }