Do not gather coverage for skippable tests

dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/config/ModuleExecutionSettingsFactoryImpl.java

@@ -60,24 +60,31 @@ public ModuleExecutionSettings create(JvmInfo jvmInfo, @Nullable String moduleNa
         buildTracerEnvironment(repositoryRoot, jvmInfo, moduleName);
     CiVisibilitySettings ciVisibilitySettings = getCiVisibilitySettings(tracerEnvironment);
 
-    boolean codeCoverageEnabled = isCodeCoverageEnabled(ciVisibilitySettings);
     boolean itrEnabled = isItrEnabled(ciVisibilitySettings);
+    boolean codeCoverageEnabled = isCodeCoverageEnabled(ciVisibilitySettings);
+    boolean testSkippingEnabled = isTestSkippingEnabled(ciVisibilitySettings);
     boolean flakyTestRetriesEnabled = isFlakyTestRetriesEnabled(ciVisibilitySettings);
     boolean earlyFlakeDetectionEnabled = isEarlyFlakeDetectionEnabled(ciVisibilitySettings);
     Map<String, String> systemProperties =
         getPropertiesPropagatedToChildProcess(
-            codeCoverageEnabled, itrEnabled, flakyTestRetriesEnabled, earlyFlakeDetectionEnabled);
+            itrEnabled,
+            codeCoverageEnabled,
+            testSkippingEnabled,
+            flakyTestRetriesEnabled,
+            earlyFlakeDetectionEnabled);
 
     LOGGER.info(
         "CI Visibility settings ({}, {}):\n"
-            + "Per-test code coverage - {},\n"
             + "Intelligent Test Runner - {},\n"
+            + "Per-test code coverage - {},\n"
+            + "Tests skipping - {},\n"
             + "Early flakiness detection - {},\n"
-            + "Flaky test retries - {}",
+            + "Auto test retries - {}",
         repositoryRoot,
         jvmInfo,
-        codeCoverageEnabled,
         itrEnabled,
+        codeCoverageEnabled,
+        testSkippingEnabled,
         earlyFlakeDetectionEnabled,
         flakyTestRetriesEnabled);
 
@@ -104,8 +111,9 @@ public ModuleExecutionSettings create(JvmInfo jvmInfo, @Nullable String moduleNa
 
     List<String> coverageEnabledPackages = getCoverageEnabledPackages(codeCoverageEnabled);
     return new ModuleExecutionSettings(
-        codeCoverageEnabled,
         itrEnabled,
+        codeCoverageEnabled,
+        testSkippingEnabled,
         flakyTestRetriesEnabled,
         // knownTests being null covers the following cases:
         //  - early flake detection is disabled in remote settings
@@ -177,7 +185,12 @@ private CiVisibilitySettings getCiVisibilitySettings(TracerEnvironment tracerEnv
   }
 
   private boolean isItrEnabled(CiVisibilitySettings ciVisibilitySettings) {
-    return ciVisibilitySettings.isTestsSkippingEnabled() && config.isCiVisibilityItrEnabled();
+    return ciVisibilitySettings.isItrEnabled() && config.isCiVisibilityItrEnabled();
+  }
+
+  private boolean isTestSkippingEnabled(CiVisibilitySettings ciVisibilitySettings) {
+    return ciVisibilitySettings.isTestsSkippingEnabled()
+        && config.isCiVisibilityTestSkippingEnabled();
   }
 
   private boolean isCodeCoverageEnabled(CiVisibilitySettings ciVisibilitySettings) {
@@ -196,8 +209,9 @@ private boolean isEarlyFlakeDetectionEnabled(CiVisibilitySettings ciVisibilitySe
   }
 
   private Map<String, String> getPropertiesPropagatedToChildProcess(
-      boolean codeCoverageEnabled,
       boolean itrEnabled,
+      boolean codeCoverageEnabled,
+      boolean testSkippingEnabled,
       boolean flakyTestRetriesEnabled,
       boolean earlyFlakeDetectionEnabled) {
     Map<String, String> propagatedSystemProperties = new HashMap<>();
@@ -212,14 +226,19 @@ private Map<String, String> getPropertiesPropagatedToChildProcess(
       }
     }
 
+    propagatedSystemProperties.put(
+        Strings.propertyNameToSystemPropertyName(CiVisibilityConfig.CIVISIBILITY_ITR_ENABLED),
+        Boolean.toString(itrEnabled));
+
     propagatedSystemProperties.put(
         Strings.propertyNameToSystemPropertyName(
             CiVisibilityConfig.CIVISIBILITY_CODE_COVERAGE_ENABLED),
         Boolean.toString(codeCoverageEnabled));
 
     propagatedSystemProperties.put(
-        Strings.propertyNameToSystemPropertyName(CiVisibilityConfig.CIVISIBILITY_ITR_ENABLED),
-        Boolean.toString(itrEnabled));
+        Strings.propertyNameToSystemPropertyName(
+            CiVisibilityConfig.CIVISIBILITY_TEST_SKIPPING_ENABLED),
+        Boolean.toString(testSkippingEnabled));
 
     propagatedSystemProperties.put(
         Strings.propertyNameToSystemPropertyName(

dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/config/ModuleExecutionSettingsSerializer.java

@@ -11,17 +11,19 @@
 
 public class ModuleExecutionSettingsSerializer {
 
-  private static final int CODE_COVERAGE_ENABLED_FLAG = 1;
-  private static final int ITR_ENABLED_FLAG = 2;
-  private static final int FLAKY_TEST_RETRIES_ENABLED_FLAG = 4;
+  private static final int ITR_ENABLED_FLAG = 1;
+  private static final int CODE_COVERAGE_ENABLED_FLAG = 2;
+  private static final int TEST_SKIPPING_ENABLED_FLAG = 4;
+  private static final int FLAKY_TEST_RETRIES_ENABLED_FLAG = 8;
 
   public static ByteBuffer serialize(ModuleExecutionSettings settings) {
     Serializer s = new Serializer();
 
     byte flags =
         (byte)
-            ((settings.isCodeCoverageEnabled() ? CODE_COVERAGE_ENABLED_FLAG : 0)
-                | (settings.isItrEnabled() ? ITR_ENABLED_FLAG : 0)
+            ((settings.isItrEnabled() ? ITR_ENABLED_FLAG : 0)
+                | (settings.isCodeCoverageEnabled() ? CODE_COVERAGE_ENABLED_FLAG : 0)
+                | (settings.isTestSkippingEnabled() ? TEST_SKIPPING_ENABLED_FLAG : 0)
                 | (settings.isFlakyTestRetriesEnabled() ? FLAKY_TEST_RETRIES_ENABLED_FLAG : 0));
     s.write(flags);
 
@@ -45,8 +47,9 @@ public static ByteBuffer serialize(ModuleExecutionSettings settings) {
 
   public static ModuleExecutionSettings deserialize(ByteBuffer buffer) {
     byte flags = Serializer.readByte(buffer);
-    boolean codeCoverageEnabled = (flags & CODE_COVERAGE_ENABLED_FLAG) != 0;
     boolean itrEnabled = (flags & ITR_ENABLED_FLAG) != 0;
+    boolean codeCoverageEnabled = (flags & CODE_COVERAGE_ENABLED_FLAG) != 0;
+    boolean testSkippingEnabled = (flags & TEST_SKIPPING_ENABLED_FLAG) != 0;
     boolean flakyTestRetriesEnabled = (flags & FLAKY_TEST_RETRIES_ENABLED_FLAG) != 0;
 
     EarlyFlakeDetectionSettings earlyFlakeDetectionSettings =
@@ -69,8 +72,9 @@ public static ModuleExecutionSettings deserialize(ByteBuffer buffer) {
     List<String> codeCoveragePackages = Serializer.readStringList(buffer);
 
     return new ModuleExecutionSettings(
-        codeCoverageEnabled,
         itrEnabled,
+        codeCoverageEnabled,
+        testSkippingEnabled,
         flakyTestRetriesEnabled,
         earlyFlakeDetectionSettings,
         systemProperties,

dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/coverage/CoverageProbeStoreFactory.java

@@ -1,8 +1,9 @@
 package datadog.trace.civisibility.coverage;
 
+import datadog.trace.api.civisibility.config.TestIdentifier;
 import datadog.trace.api.civisibility.coverage.CoverageProbeStore;
 import datadog.trace.civisibility.source.SourcePathResolver;
 
 public interface CoverageProbeStoreFactory extends CoverageProbeStore.Registry {
-  CoverageProbeStore create(SourcePathResolver sourcePathResolver);
+  CoverageProbeStore create(TestIdentifier testIdentifier, SourcePathResolver sourcePathResolver);
 }

dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/coverage/NoopCoverageProbeStore.java

@@ -1,12 +1,13 @@
 package datadog.trace.civisibility.coverage;
 
+import datadog.trace.api.civisibility.config.TestIdentifier;
 import datadog.trace.api.civisibility.coverage.CoverageProbeStore;
 import datadog.trace.api.civisibility.coverage.TestReport;
 import datadog.trace.civisibility.source.SourcePathResolver;
 import javax.annotation.Nullable;
 
 public class NoopCoverageProbeStore implements CoverageProbeStore {
-  private static final CoverageProbeStore INSTANCE = new NoopCoverageProbeStore();
+  public static final CoverageProbeStore INSTANCE = new NoopCoverageProbeStore();
 
   @Override
   public void record(Class<?> clazz) {}
@@ -33,7 +34,8 @@ public static final class NoopCoverageProbeStoreFactory implements CoverageProbe
     public void setTotalProbeCount(String className, int totalProbeCount) {}
 
     @Override
-    public CoverageProbeStore create(SourcePathResolver sourcePathResolver) {
+    public CoverageProbeStore create(
+        TestIdentifier testIdentifier, SourcePathResolver sourcePathResolver) {
       return INSTANCE;
     }
   }

dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/coverage/SegmentlessTestProbes.java

@@ -1,5 +1,6 @@
 package datadog.trace.civisibility.coverage;
 
+import datadog.trace.api.civisibility.config.TestIdentifier;
 import datadog.trace.api.civisibility.coverage.CoverageProbeStore;
 import datadog.trace.api.civisibility.coverage.TestReport;
 import datadog.trace.api.civisibility.coverage.TestReportFileEntry;
@@ -178,7 +179,8 @@ public void setTotalProbeCount(String className, int totalProbeCount) {
     }
 
     @Override
-    public CoverageProbeStore create(SourcePathResolver sourcePathResolver) {
+    public CoverageProbeStore create(
+        TestIdentifier testIdentifier, SourcePathResolver sourcePathResolver) {
       return new SegmentlessTestProbes(sourcePathResolver, metricCollector);
     }
   }

dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/coverage/SkippableAwareCoverageProbeStoreFactory.java

@@ -0,0 +1,38 @@
+package datadog.trace.civisibility.coverage;
+
+import datadog.trace.api.civisibility.config.TestIdentifier;
+import datadog.trace.api.civisibility.coverage.CoverageProbeStore;
+import datadog.trace.civisibility.source.SourcePathResolver;
+import java.util.Collection;
+
+/**
+ * Coverage store factory returns no-op stores for skippable tests. This is done to reduce coverage
+ * overhead. The idea is that if a test is skippable then it means none of the files it covers were
+ * changed. If none of the files were changed then gathering coverage for the test make no sense,
+ * because it will be the same as previously gathered coverage that the backend already has.
+ */
+public class SkippableAwareCoverageProbeStoreFactory implements CoverageProbeStoreFactory {
+  private final Collection<TestIdentifier> skippableTests;
+  private final CoverageProbeStoreFactory delegate;
+
+  public SkippableAwareCoverageProbeStoreFactory(
+      Collection<TestIdentifier> skippableTests, CoverageProbeStoreFactory delegate) {
+    this.skippableTests = skippableTests;
+    this.delegate = delegate;
+  }
+
+  @Override
+  public CoverageProbeStore create(
+      TestIdentifier testIdentifier, SourcePathResolver sourcePathResolver) {
+    if (skippableTests.contains(testIdentifier)) {
+      return NoopCoverageProbeStore.INSTANCE;
+    } else {
+      return delegate.create(testIdentifier, sourcePathResolver);
+    }
+  }
+
+  @Override
+  public void setTotalProbeCount(String className, int totalProbeCount) {
+    delegate.setTotalProbeCount(className, totalProbeCount);
+  }
+}

dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/coverage/TestProbes.java

@@ -1,5 +1,6 @@
 package datadog.trace.civisibility.coverage;
 
+import datadog.trace.api.civisibility.config.TestIdentifier;
 import datadog.trace.api.civisibility.coverage.CoverageProbeStore;
 import datadog.trace.api.civisibility.coverage.TestReport;
 import datadog.trace.api.civisibility.coverage.TestReportFileEntry;
@@ -235,7 +236,8 @@ public void setTotalProbeCount(String className, int totalProbeCount) {
     }
 
     @Override
-    public CoverageProbeStore create(SourcePathResolver sourcePathResolver) {
+    public CoverageProbeStore create(
+        TestIdentifier testIdentifier, SourcePathResolver sourcePathResolver) {
       return new TestProbes(sourcePathResolver, metricCollector);
     }
   }

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

@@ -12,7 +12,6 @@
 import datadog.trace.bootstrap.instrumentation.api.Tags;
 import datadog.trace.civisibility.InstrumentationType;
 import datadog.trace.civisibility.codeowners.Codeowners;
-import datadog.trace.civisibility.coverage.CoverageProbeStoreFactory;
 import datadog.trace.civisibility.decorator.TestDecorator;
 import datadog.trace.civisibility.source.MethodLinesResolver;
 import datadog.trace.civisibility.source.SourcePathResolver;
@@ -30,7 +29,6 @@ public abstract class AbstractTestModule {
   protected final SourcePathResolver sourcePathResolver;
   protected final Codeowners codeowners;
   protected final MethodLinesResolver methodLinesResolver;
-  protected final CoverageProbeStoreFactory coverageProbeStoreFactory;
   private final Consumer<AgentSpan> onSpanFinish;
 
   public AbstractTestModule(
@@ -45,7 +43,6 @@ public AbstractTestModule(
       SourcePathResolver sourcePathResolver,
       Codeowners codeowners,
       MethodLinesResolver methodLinesResolver,
-      CoverageProbeStoreFactory coverageProbeStoreFactory,
       Consumer<AgentSpan> onSpanFinish) {
     this.sessionId = sessionId;
     this.moduleName = moduleName;
@@ -55,7 +52,6 @@ public AbstractTestModule(
     this.sourcePathResolver = sourcePathResolver;
     this.codeowners = codeowners;
     this.methodLinesResolver = methodLinesResolver;
-    this.coverageProbeStoreFactory = coverageProbeStoreFactory;
     this.onSpanFinish = onSpanFinish;
 
     if (startTime != null) {

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

@@ -15,14 +15,6 @@ TestSuiteImpl testSuiteStart(
       boolean parallelized,
       TestFrameworkInstrumentation instrumentation);
 
-  /**
-   * Checks if a given test can be skipped with Intelligent Test Runner or not
-   *
-   * @param test Test to be checked
-   * @return {@code true} if the test can be skipped, {@code false} otherwise
-   */
-  boolean isSkippable(TestIdentifier test);
-
   /**
    * Checks if a given test is "new" or not. A test is considered "new" if the backend has no
    * information about it.
@@ -33,6 +25,14 @@ TestSuiteImpl testSuiteStart(
    */
   boolean isNew(TestIdentifier test);
 
+  /**
+   * Checks if a given test should be skipped with Intelligent Test Runner or not
+   *
+   * @param test Test to be checked
+   * @return {@code true} if the test can be skipped, {@code false} otherwise
+   */
+  boolean shouldBeSkipped(TestIdentifier test);
+
   /**
    * Checks if a given test can be skipped with Intelligent Test Runner or not. If the test is
    * considered skippable, the count of skippable tests is incremented.

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

@@ -8,6 +8,7 @@
 import datadog.trace.api.civisibility.DDTest;
 import datadog.trace.api.civisibility.InstrumentationBridge;
 import datadog.trace.api.civisibility.InstrumentationTestBridge;
+import datadog.trace.api.civisibility.config.TestIdentifier;
 import datadog.trace.api.civisibility.coverage.CoverageBridge;
 import datadog.trace.api.civisibility.coverage.CoverageProbeStore;
 import datadog.trace.api.civisibility.domain.TestContext;
@@ -56,6 +57,7 @@ public TestImpl(
       String moduleName,
       String testSuiteName,
       String testName,
+      @Nullable String testParameters,
       @Nullable String itrCorrelationId,
       @Nullable Long startTime,
       @Nullable Class<?> testClass,
@@ -76,7 +78,9 @@ public TestImpl(
     this.suiteId = suiteId;
     this.onSpanFinish = onSpanFinish;
 
-    CoverageProbeStore probeStore = coverageProbeStoreFactory.create(sourcePathResolver);
+    TestIdentifier identifier = new TestIdentifier(testSuiteName, testName, testParameters, null);
+    CoverageProbeStore probeStore =
+        coverageProbeStoreFactory.create(identifier, sourcePathResolver);
     CoverageBridge.setThreadLocalCoverageProbeStore(probeStore);
 
     this.context = new TestContextImpl(probeStore);

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

@@ -38,7 +38,7 @@ public class TestSuiteImpl implements DDTestSuite {
   private final InstrumentationType instrumentationType;
   private final TestFrameworkInstrumentation instrumentation;
   private final Config config;
-  CiVisibilityMetricCollector metricCollector;
+  private final CiVisibilityMetricCollector metricCollector;
   private final TestDecorator testDecorator;
   private final SourcePathResolver sourcePathResolver;
   private final Codeowners codeowners;
@@ -189,13 +189,22 @@ public void end(@Nullable Long endTime) {
   @Override
   public TestImpl testStart(
       String testName, @Nullable Method testMethod, @Nullable Long startTime) {
+    return testStart(testName, null, testMethod, startTime);
+  }
+
+  public TestImpl testStart(
+      String testName,
+      @Nullable String testParameters,
+      @Nullable Method testMethod,
+      @Nullable Long startTime) {
     return new TestImpl(
         sessionId,
         moduleId,
         span.getSpanId(),
         moduleName,
         testSuiteName,
         testName,
+        testParameters,
         itrCorrelationId,
         startTime,
         testClass,