Parallelize CI Visibility settings requests (#8299)

Author: nikita-tkachenko-datadog

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

@@ -18,7 +18,7 @@ public CiVisibilitySettings getSettings(TracerEnvironment tracerEnvironment) {
 
         @Override
         public SkippableTests getSkippableTests(TracerEnvironment tracerEnvironment) {
-          return new SkippableTests(null, Collections.emptyMap(), null);
+          return SkippableTests.EMPTY;
         }
 
         @Override

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

@@ -29,6 +29,7 @@
 import java.util.Base64;
 import java.util.BitSet;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
@@ -191,7 +192,9 @@ public SkippableTests getSkippableTests(TracerEnvironment tracerEnvironment) thr
 
     String correlationId = response.meta != null ? response.meta.correlation_id : null;
     Map<String, BitSet> coveredLinesByRelativeSourcePath =
-        response.meta != null ? response.meta.coverage : null;
+        response.meta != null && response.meta.coverage != null
+            ? response.meta.coverage
+            : Collections.emptyMap();
     return new SkippableTests(
         correlationId, testIdentifiersByModule, coveredLinesByRelativeSourcePath);
   }

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

@@ -40,7 +40,7 @@ public class ExecutionSettings {
   @Nonnull private final EarlyFlakeDetectionSettings earlyFlakeDetectionSettings;
   @Nullable private final String itrCorrelationId;
   @Nonnull private final Map<TestIdentifier, TestMetadata> skippableTests;
-  @Nullable private final Map<String, BitSet> skippableTestsCoverage;
+  @Nonnull private final Map<String, BitSet> skippableTestsCoverage;
   @Nullable private final Collection<TestIdentifier> flakyTests;
   @Nullable private final Collection<TestIdentifier> knownTests;
   @Nonnull private final Diff pullRequestDiff;
@@ -54,7 +54,7 @@ public ExecutionSettings(
       @Nonnull EarlyFlakeDetectionSettings earlyFlakeDetectionSettings,
       @Nullable String itrCorrelationId,
       @Nonnull Map<TestIdentifier, TestMetadata> skippableTests,
-      @Nullable Map<String, BitSet> skippableTestsCoverage,
+      @Nonnull Map<String, BitSet> skippableTestsCoverage,
       @Nullable Collection<TestIdentifier> flakyTests,
       @Nullable Collection<TestIdentifier> knownTests,
       @Nonnull Diff pullRequestDiff) {
@@ -107,7 +107,7 @@ public String getItrCorrelationId() {
   }
 
   /** A bit vector of covered lines by relative source file path. */
-  @Nullable
+  @Nonnull
   public Map<String, BitSet> getSkippableTestsCoverage() {
     return skippableTestsCoverage;
   }
@@ -126,6 +126,10 @@ public Collection<TestIdentifier> getKnownTests() {
     return knownTests;
   }
 
+  /**
+   * @return the list of flaky tests for the given module (can be empty), or {@code null} if flaky
+   *     tests could not be obtained
+   */
   @Nullable
   public Collection<TestIdentifier> getFlakyTests() {
     return flakyTests;

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

@@ -4,7 +4,6 @@
 import datadog.trace.api.civisibility.CIConstants;
 import datadog.trace.api.civisibility.CiVisibilityWellKnownTags;
 import datadog.trace.api.civisibility.config.TestIdentifier;
-import datadog.trace.api.civisibility.config.TestMetadata;
 import datadog.trace.api.git.GitInfo;
 import datadog.trace.api.git.GitInfoProvider;
 import datadog.trace.civisibility.ci.PullRequestInfo;
@@ -14,15 +13,19 @@
 import datadog.trace.civisibility.git.tree.GitClient;
 import datadog.trace.civisibility.git.tree.GitDataUploader;
 import datadog.trace.civisibility.git.tree.GitRepoUnshallow;
-import java.nio.file.Path;
 import java.nio.file.Paths;
-import java.util.BitSet;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Set;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
+import java.util.concurrent.ThreadFactory;
 import java.util.concurrent.TimeUnit;
 import java.util.function.Function;
 import javax.annotation.Nonnull;
@@ -33,8 +36,11 @@
 public class ExecutionSettingsFactoryImpl implements ExecutionSettingsFactory {
 
   private static final Logger LOGGER = LoggerFactory.getLogger(ExecutionSettingsFactoryImpl.class);
+
   private static final String TEST_CONFIGURATION_TAG_PREFIX = "test.configuration.";
 
+  private static final ThreadFactory THREAD_FACTORY = r -> new Thread(null, r, "dd-ci-vis-config");
+
   /**
    * A workaround for bulk-requesting module settings. For any module that has no settings that are
    * exclusive to it (i.e. that has no skippable/flaky/known tests), the settings will be under this
@@ -113,9 +119,31 @@ private TracerEnvironment buildTracerEnvironment(
         .build();
   }
 
-  private @Nonnull Map<String, ExecutionSettings> create(TracerEnvironment tracerEnvironment) {
+  @Nonnull
+  private Map<String, ExecutionSettings> create(TracerEnvironment tracerEnvironment) {
     CiVisibilitySettings settings = getCiVisibilitySettings(tracerEnvironment);
+    ExecutorService settingsExecutor = Executors.newCachedThreadPool(THREAD_FACTORY);
+    try {
+      return doCreate(tracerEnvironment, settings, settingsExecutor);
+
+    } catch (InterruptedException e) {
+      Thread.currentThread().interrupt();
+      LOGGER.error("Interrupted while creating execution settings");
+      return Collections.singletonMap(DEFAULT_SETTINGS, ExecutionSettings.EMPTY);
 
+    } catch (ExecutionException e) {
+      LOGGER.error("Error while creating execution settings", e);
+      return Collections.singletonMap(DEFAULT_SETTINGS, ExecutionSettings.EMPTY);
+
+    } finally {
+      settingsExecutor.shutdownNow();
+    }
+  }
+
+  @Nonnull
+  private Map<String, ExecutionSettings> doCreate(
+      TracerEnvironment tracerEnvironment, CiVisibilitySettings settings, ExecutorService executor)
+      throws InterruptedException, ExecutionException {
     boolean itrEnabled =
         isFeatureEnabled(
             settings, CiVisibilitySettings::isItrEnabled, Config::isCiVisibilityItrEnabled);
@@ -134,7 +162,7 @@ private TracerEnvironment buildTracerEnvironment(
             settings,
             CiVisibilitySettings::isFlakyTestRetriesEnabled,
             Config::isCiVisibilityFlakyRetryEnabled);
-    boolean impactedTestsDetectionEnabled =
+    boolean impactedTestsEnabled =
         isFeatureEnabled(
             settings,
             CiVisibilitySettings::isImpactedTestsDetectionEnabled,
@@ -167,46 +195,27 @@ private TracerEnvironment buildTracerEnvironment(
         codeCoverageEnabled,
         testSkippingEnabled,
         earlyFlakeDetectionEnabled,
-        impactedTestsDetectionEnabled,
+        impactedTestsEnabled,
         knownTestsRequest,
         flakyTestRetriesEnabled);
 
-    String itrCorrelationId = null;
-    Map<String, Map<TestIdentifier, TestMetadata>> skippableTestIdentifiers =
-        Collections.emptyMap();
-    Map<String, BitSet> skippableTestsCoverage = null;
-    if (itrEnabled && repositoryRoot != null) {
-      SkippableTests skippableTests =
-          getSkippableTests(Paths.get(repositoryRoot), tracerEnvironment);
-      if (skippableTests != null) {
-        itrCorrelationId = skippableTests.getCorrelationId();
-        skippableTestIdentifiers = skippableTests.getIdentifiersByModule();
-        skippableTestsCoverage = skippableTests.getCoveredLinesByRelativeSourcePath();
-      }
-    }
-
-    Map<String, Collection<TestIdentifier>> flakyTestsByModule =
-        flakyTestRetriesEnabled && config.isCiVisibilityFlakyRetryOnlyKnownFlakes()
-                || CIConstants.FAIL_FAST_TEST_ORDER.equalsIgnoreCase(
-                    config.getCiVisibilityTestOrder())
-            ? getFlakyTestsByModule(tracerEnvironment)
-            : null;
-
-    Map<String, Collection<TestIdentifier>> knownTestsByModule =
-        knownTestsRequest ? getKnownTestsByModule(tracerEnvironment) : null;
-
-    Set<String> moduleNames = new HashSet<>(Collections.singleton(DEFAULT_SETTINGS));
-    moduleNames.addAll(skippableTestIdentifiers.keySet());
-    if (flakyTestsByModule != null) {
-      moduleNames.addAll(flakyTestsByModule.keySet());
-    }
-    if (knownTestsByModule != null) {
-      moduleNames.addAll(knownTestsByModule.keySet());
-    }
+    Future<SkippableTests> skippableTestsFuture =
+        executor.submit(() -> getSkippableTests(tracerEnvironment, itrEnabled));
+    Future<Map<String, Collection<TestIdentifier>>> flakyTestsFuture =
+        executor.submit(() -> getFlakyTestsByModule(tracerEnvironment, flakyTestRetriesEnabled));
+    Future<Map<String, Collection<TestIdentifier>>> knownTestsFuture =
+        executor.submit(() -> getKnownTestsByModule(tracerEnvironment, knownTestsRequest));
+    Future<Diff> pullRequestDiffFuture =
+        executor.submit(() -> getPullRequestDiff(tracerEnvironment, impactedTestsEnabled));
 
-    Diff pullRequestDiff = getPullRequestDiff(impactedTestsDetectionEnabled, tracerEnvironment);
+    SkippableTests skippableTests = skippableTestsFuture.get();
+    Map<String, Collection<TestIdentifier>> flakyTestsByModule = flakyTestsFuture.get();
+    Map<String, Collection<TestIdentifier>> knownTestsByModule = knownTestsFuture.get();
+    Diff pullRequestDiff = pullRequestDiffFuture.get();
 
     Map<String, ExecutionSettings> settingsByModule = new HashMap<>();
+    Set<String> moduleNames =
+        getModuleNames(skippableTests, flakyTestsByModule, knownTestsByModule);
     for (String moduleName : moduleNames) {
       settingsByModule.put(
           moduleName,
@@ -215,13 +224,15 @@ private TracerEnvironment buildTracerEnvironment(
               codeCoverageEnabled,
               testSkippingEnabled,
               flakyTestRetriesEnabled,
-              impactedTestsDetectionEnabled,
+              impactedTestsEnabled,
               earlyFlakeDetectionEnabled
                   ? settings.getEarlyFlakeDetectionSettings()
                   : EarlyFlakeDetectionSettings.DEFAULT,
-              itrCorrelationId,
-              skippableTestIdentifiers.getOrDefault(moduleName, Collections.emptyMap()),
-              skippableTestsCoverage,
+              skippableTests.getCorrelationId(),
+              skippableTests
+                  .getIdentifiersByModule()
+                  .getOrDefault(moduleName, Collections.emptyMap()),
+              skippableTests.getCoveredLinesByRelativeSourcePath(),
               flakyTestsByModule != null
                   ? flakyTestsByModule.getOrDefault(moduleName, Collections.emptyList())
                   : null,
@@ -258,47 +269,66 @@ private boolean isFeatureEnabled(
     return remoteSetting.apply(ciVisibilitySettings) && killSwitch.apply(config);
   }
 
-  @Nullable
+  @Nonnull
   private SkippableTests getSkippableTests(
-      Path repositoryRoot, TracerEnvironment tracerEnvironment) {
+      TracerEnvironment tracerEnvironment, boolean itrEnabled) {
+    if (!itrEnabled || repositoryRoot == null) {
+      return SkippableTests.EMPTY;
+    }
     try {
       // ensure git data upload is finished before asking for tests
       gitDataUploader
           .startOrObserveGitDataUpload()
           .get(config.getCiVisibilityGitUploadTimeoutMillis(), TimeUnit.MILLISECONDS);
 
       SkippableTests skippableTests = configurationApi.getSkippableTests(tracerEnvironment);
-      LOGGER.debug(
-          "Received {} skippable tests in total for {}",
-          skippableTests.getIdentifiersByModule().size(),
-          repositoryRoot);
+
+      if (LOGGER.isDebugEnabled()) {
+        int totalSkippableTests =
+            skippableTests.getIdentifiersByModule().values().stream()
+                .filter(Objects::nonNull)
+                .mapToInt(Map::size)
+                .sum();
+        LOGGER.debug(
+            "Received {} skippable tests in total for {}",
+            totalSkippableTests,
+            Paths.get(repositoryRoot));
+      }
 
       return skippableTests;
 
     } catch (InterruptedException e) {
       Thread.currentThread().interrupt();
       LOGGER.error("Interrupted while waiting for git data upload", e);
-      return null;
+      return SkippableTests.EMPTY;
 
     } catch (Exception e) {
       LOGGER.error("Could not obtain list of skippable tests, will proceed without skipping", e);
-      return null;
+      return SkippableTests.EMPTY;
     }
   }
 
+  @Nullable
   private Map<String, Collection<TestIdentifier>> getFlakyTestsByModule(
-      TracerEnvironment tracerEnvironment) {
+      TracerEnvironment tracerEnvironment, boolean flakyTestRetriesEnabled) {
+    if (!(flakyTestRetriesEnabled && config.isCiVisibilityFlakyRetryOnlyKnownFlakes())
+        && !CIConstants.FAIL_FAST_TEST_ORDER.equalsIgnoreCase(config.getCiVisibilityTestOrder())) {
+      return null;
+    }
     try {
       return configurationApi.getFlakyTestsByModule(tracerEnvironment);
-
     } catch (Exception e) {
       LOGGER.error("Could not obtain list of flaky tests", e);
-      return Collections.emptyMap();
+      return null;
     }
   }
 
+  @Nullable
   private Map<String, Collection<TestIdentifier>> getKnownTestsByModule(
-      TracerEnvironment tracerEnvironment) {
+      TracerEnvironment tracerEnvironment, boolean knownTestsRequest) {
+    if (!knownTestsRequest) {
+      return null;
+    }
     try {
       return configurationApi.getKnownTestsByModule(tracerEnvironment);
 
@@ -310,7 +340,7 @@ private Map<String, Collection<TestIdentifier>> getKnownTestsByModule(
 
   @Nonnull
   private Diff getPullRequestDiff(
-      boolean impactedTestsDetectionEnabled, TracerEnvironment tracerEnvironment) {
+      TracerEnvironment tracerEnvironment, boolean impactedTestsDetectionEnabled) {
     if (!impactedTestsDetectionEnabled) {
       return LineDiff.EMPTY;
     }
@@ -362,4 +392,20 @@ private Diff getPullRequestDiff(
 
     return LineDiff.EMPTY;
   }
+
+  @Nonnull
+  private static Set<String> getModuleNames(
+      SkippableTests skippableTests,
+      Map<String, Collection<TestIdentifier>> flakyTestsByModule,
+      Map<String, Collection<TestIdentifier>> knownTestsByModule) {
+    Set<String> moduleNames = new HashSet<>(Collections.singleton(DEFAULT_SETTINGS));
+    moduleNames.addAll(skippableTests.getIdentifiersByModule().keySet());
+    if (flakyTestsByModule != null) {
+      moduleNames.addAll(flakyTestsByModule.keySet());
+    }
+    if (knownTestsByModule != null) {
+      moduleNames.addAll(knownTestsByModule.keySet());
+    }
+    return moduleNames;
+  }
 }

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

@@ -3,19 +3,24 @@
 import datadog.trace.api.civisibility.config.TestIdentifier;
 import datadog.trace.api.civisibility.config.TestMetadata;
 import java.util.BitSet;
+import java.util.Collections;
 import java.util.Map;
+import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 
 public class SkippableTests {
 
+  public static final SkippableTests EMPTY =
+      new SkippableTests(null, Collections.emptyMap(), Collections.emptyMap());
+
   private final String correlationId;
   private final Map<String, Map<TestIdentifier, TestMetadata>> identifiersByModule;
   private final Map<String, BitSet> coveredLinesByRelativeSourcePath;
 
   public SkippableTests(
       @Nullable String correlationId,
-      Map<String, Map<TestIdentifier, TestMetadata>> identifiersByModule,
-      @Nullable Map<String, BitSet> coveredLinesByRelativeSourcePath) {
+      @Nonnull Map<String, Map<TestIdentifier, TestMetadata>> identifiersByModule,
+      @Nonnull Map<String, BitSet> coveredLinesByRelativeSourcePath) {
     this.correlationId = correlationId;
     this.identifiersByModule = identifiersByModule;
     this.coveredLinesByRelativeSourcePath = coveredLinesByRelativeSourcePath;
@@ -26,11 +31,12 @@ public String getCorrelationId() {
     return correlationId;
   }
 
+  @Nonnull
   public Map<String, Map<TestIdentifier, TestMetadata>> getIdentifiersByModule() {
     return identifiersByModule;
   }
 
-  @Nullable
+  @Nonnull
   public Map<String, BitSet> getCoveredLinesByRelativeSourcePath() {
     return coveredLinesByRelativeSourcePath;
   }

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

@@ -165,10 +165,7 @@ private void addModuleLayout(BuildModuleLayout moduleLayout) {
   }
 
   /** Handles skipped tests' coverage data received from the backend */
-  private void addBackendCoverageData(@Nullable Map<String, BitSet> skippableTestsCoverage) {
-    if (skippableTestsCoverage == null) {
-      return;
-    }
+  private void addBackendCoverageData(@Nonnull Map<String, BitSet> skippableTestsCoverage) {
     synchronized (coverageDataLock) {
       for (Map.Entry<String, BitSet> e : skippableTestsCoverage.entrySet()) {
         backendCoverageData.merge(e.getKey(), e.getValue(), JacocoCoverageCalculator::mergeBitSets);