Ensure git repo is not shallow before calculating git diff

Author: nikita-tkachenko-datadog

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

@@ -23,6 +23,7 @@
 import datadog.trace.civisibility.git.tree.GitDataApi;
 import datadog.trace.civisibility.git.tree.GitDataUploader;
 import datadog.trace.civisibility.git.tree.GitDataUploaderImpl;
+import datadog.trace.civisibility.git.tree.GitRepoUnshallow;
 import datadog.trace.civisibility.ipc.ExecutionSettingsRequest;
 import datadog.trace.civisibility.ipc.ExecutionSettingsResponse;
 import datadog.trace.civisibility.ipc.SignalClient;
@@ -72,12 +73,16 @@ public class CiVisibilityRepoServices {
     moduleName = getModuleName(services.config, repoRoot, path);
     ciTags = new CITagsProvider().getCiTags(ciInfo, pullRequestInfo);
 
+    GitClient gitClient = services.gitClientFactory.create(repoRoot);
+    GitRepoUnshallow gitRepoUnshallow = new GitRepoUnshallow(services.config, gitClient);
+
     gitDataUploader =
         buildGitDataUploader(
             services.config,
             services.metricCollector,
             services.gitInfoProvider,
-            services.gitClientFactory,
+            gitClient,
+            gitRepoUnshallow,
             services.backendApi,
             repoRoot);
     repoIndexProvider = services.repoIndexProviderFactory.create(repoRoot);
@@ -93,7 +98,8 @@ public class CiVisibilityRepoServices {
               services.config,
               services.metricCollector,
               services.backendApi,
-              services.gitClientFactory,
+              gitClient,
+              gitRepoUnshallow,
               gitDataUploader,
               pullRequestInfo,
               repoRoot);
@@ -165,7 +171,8 @@ private static ExecutionSettingsFactory buildExecutionSettingsFactory(
       Config config,
       CiVisibilityMetricCollector metricCollector,
       BackendApi backendApi,
-      GitClient.Factory gitClientFactory,
+      GitClient gitClient,
+      GitRepoUnshallow gitRepoUnshallow,
       GitDataUploader gitDataUploader,
       PullRequestInfo pullRequestInfo,
       String repoRoot) {
@@ -180,7 +187,13 @@ private static ExecutionSettingsFactory buildExecutionSettingsFactory(
 
     ExecutionSettingsFactoryImpl factory =
         new ExecutionSettingsFactoryImpl(
-            config, configurationApi, gitClientFactory, gitDataUploader, pullRequestInfo, repoRoot);
+            config,
+            configurationApi,
+            gitClient,
+            gitRepoUnshallow,
+            gitDataUploader,
+            pullRequestInfo,
+            repoRoot);
     if (processHierarchy.isHeadless()) {
       return factory;
     } else {
@@ -192,7 +205,8 @@ private static GitDataUploader buildGitDataUploader(
       Config config,
       CiVisibilityMetricCollector metricCollector,
       GitInfoProvider gitInfoProvider,
-      GitClient.Factory gitClientFactory,
+      GitClient gitClient,
+      GitRepoUnshallow gitRepoUnshallow,
       BackendApi backendApi,
       String repoRoot) {
     if (!config.isCiVisibilityGitUploadEnabled()) {
@@ -213,9 +227,15 @@ private static GitDataUploader buildGitDataUploader(
 
     String remoteName = config.getCiVisibilityGitRemoteName();
     GitDataApi gitDataApi = new GitDataApi(backendApi, metricCollector);
-    GitClient gitClient = gitClientFactory.create(repoRoot);
     return new GitDataUploaderImpl(
-        config, metricCollector, gitDataApi, gitClient, gitInfoProvider, repoRoot, remoteName);
+        config,
+        metricCollector,
+        gitDataApi,
+        gitClient,
+        gitRepoUnshallow,
+        gitInfoProvider,
+        repoRoot,
+        remoteName);
   }
 
   private static SourcePathResolver buildSourcePathResolver(

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

@@ -12,6 +12,7 @@
 import datadog.trace.civisibility.diff.FileDiff;
 import datadog.trace.civisibility.git.tree.GitClient;
 import datadog.trace.civisibility.git.tree.GitDataUploader;
+import datadog.trace.civisibility.git.tree.GitRepoUnshallow;
 import java.io.IOException;
 import java.nio.file.Path;
 import java.nio.file.Paths;
@@ -43,21 +44,24 @@ public class ExecutionSettingsFactoryImpl implements ExecutionSettingsFactory {
 
   private final Config config;
   private final ConfigurationApi configurationApi;
-  private final GitClient.Factory gitClientFactory;
+  private final GitClient gitClient;
+  private final GitRepoUnshallow gitRepoUnshallow;
   private final GitDataUploader gitDataUploader;
   private final PullRequestInfo pullRequestInfo;
   private final String repositoryRoot;
 
   public ExecutionSettingsFactoryImpl(
       Config config,
       ConfigurationApi configurationApi,
-      GitClient.Factory gitClientFactory,
+      GitClient gitClient,
+      GitRepoUnshallow gitRepoUnshallow,
       GitDataUploader gitDataUploader,
       PullRequestInfo pullRequestInfo,
       String repositoryRoot) {
     this.config = config;
     this.configurationApi = configurationApi;
-    this.gitClientFactory = gitClientFactory;
+    this.gitClient = gitClient;
+    this.gitRepoUnshallow = gitRepoUnshallow;
     this.gitDataUploader = gitDataUploader;
     this.pullRequestInfo = pullRequestInfo;
     this.repositoryRoot = repositoryRoot;
@@ -310,7 +314,8 @@ private Diff getPullRequestDiff(
 
     try {
       if (repositoryRoot != null && pullRequestInfo.isNotEmpty()) {
-        GitClient gitClient = gitClientFactory.create(repositoryRoot);
+        // ensure repo is not shallow before attempting to get git diff
+        gitRepoUnshallow.unshallow();
         return gitClient.getGitDiff(
             pullRequestInfo.getPullRequestBaseBranchSha(), pullRequestInfo.getGitCommitHeadSha());
       }

dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/GitDataUploaderImpl.java

@@ -7,9 +7,7 @@
 import datadog.trace.api.git.GitInfoProvider;
 import datadog.trace.api.telemetry.LogCollector;
 import datadog.trace.civisibility.utils.FileUtils;
-import datadog.trace.civisibility.utils.ShellCommandExecutor;
 import datadog.trace.util.AgentThreadFactory;
-import java.io.IOException;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.ArrayList;
@@ -31,6 +29,7 @@ public class GitDataUploaderImpl implements GitDataUploader {
   private final CiVisibilityMetricCollector metricCollector;
   private final GitDataApi gitDataApi;
   private final GitClient gitClient;
+  private final GitRepoUnshallow gitRepoUnshallow;
   private final GitInfoProvider gitInfoProvider;
   private final String repoRoot;
   private final String remoteName;
@@ -42,13 +41,15 @@ public GitDataUploaderImpl(
       CiVisibilityMetricCollector metricCollector,
       GitDataApi gitDataApi,
       GitClient gitClient,
+      GitRepoUnshallow gitRepoUnshallow,
       GitInfoProvider gitInfoProvider,
       String repoRoot,
       String remoteName) {
     this.config = config;
     this.metricCollector = metricCollector;
     this.gitDataApi = gitDataApi;
     this.gitClient = gitClient;
+    this.gitRepoUnshallow = gitRepoUnshallow;
     this.gitInfoProvider = gitInfoProvider;
     this.repoRoot = repoRoot;
     this.remoteName = remoteName;
@@ -89,10 +90,8 @@ private void uploadGitData() {
     try {
       LOGGER.debug("Starting git data upload, {}", gitClient);
 
-      if (config.isCiVisibilityGitUnshallowEnabled()
-          && !config.isCiVisibilityGitUnshallowDefer()
-          && gitClient.isShallow()) {
-        unshallowRepository();
+      if (!config.isCiVisibilityGitUnshallowDefer()) {
+        gitRepoUnshallow.unshallow();
       }
 
       GitInfo gitInfo = gitInfoProvider.getGitInfo(repoRoot);
@@ -113,10 +112,7 @@ private void uploadGitData() {
         return;
       }
 
-      if (config.isCiVisibilityGitUnshallowEnabled()
-          && config.isCiVisibilityGitUnshallowDefer()
-          && gitClient.isShallow()) {
-        unshallowRepository();
+      if (config.isCiVisibilityGitUnshallowDefer() && gitRepoUnshallow.unshallow()) {
         latestCommits = gitClient.getLatestCommits();
         commitsToSkip = gitDataApi.searchCommits(remoteUrl, latestCommits);
       }
@@ -184,29 +180,6 @@ private void removeShutdownHook() {
     }
   }
 
-  private void unshallowRepository() throws IOException, TimeoutException, InterruptedException {
-    long unshallowStart = System.currentTimeMillis();
-    try {
-      gitClient.unshallow(GitClient.HEAD);
-      return;
-    } catch (ShellCommandExecutor.ShellCommandFailedException e) {
-      LOGGER.debug(
-          "Could not unshallow using HEAD - assuming HEAD points to a local commit that does not exist in the remote repo",
-          e);
-    }
-
-    try {
-      String upstreamBranch = gitClient.getUpstreamBranchSha();
-      gitClient.unshallow(upstreamBranch);
-    } catch (ShellCommandExecutor.ShellCommandFailedException e) {
-      LOGGER.debug(
-          "Could not unshallow using upstream branch - assuming currently checked out local branch does not track any remote branch",
-          e);
-      gitClient.unshallow(null);
-    }
-    LOGGER.debug("Repository unshallowing took {} ms", System.currentTimeMillis() - unshallowStart);
-  }
-
   private void waitForUploadToFinish() {
     try {
       long uploadTimeoutMillis = config.getCiVisibilityGitUploadTimeoutMillis();

dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/GitRepoUnshallow.java

@@ -0,0 +1,48 @@
+package datadog.trace.civisibility.git.tree;
+
+import datadog.trace.api.Config;
+import datadog.trace.civisibility.utils.ShellCommandExecutor;
+import java.io.IOException;
+import java.util.concurrent.TimeoutException;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class GitRepoUnshallow {
+
+  private static final Logger LOGGER = LoggerFactory.getLogger(GitRepoUnshallow.class);
+
+  private final Config config;
+  private final GitClient gitClient;
+
+  public GitRepoUnshallow(Config config, GitClient gitClient) {
+    this.config = config;
+    this.gitClient = gitClient;
+  }
+
+  public boolean unshallow() throws IOException, InterruptedException, TimeoutException {
+    if (!config.isCiVisibilityGitUnshallowEnabled() || !gitClient.isShallow()) {
+      return false;
+    }
+
+    long unshallowStart = System.currentTimeMillis();
+    try {
+      gitClient.unshallow(GitClient.HEAD);
+    } catch (ShellCommandExecutor.ShellCommandFailedException e) {
+      LOGGER.debug(
+          "Could not unshallow using HEAD - assuming HEAD points to a local commit that does not exist in the remote repo",
+          e);
+    }
+
+    try {
+      String upstreamBranch = gitClient.getUpstreamBranchSha();
+      gitClient.unshallow(upstreamBranch);
+    } catch (ShellCommandExecutor.ShellCommandFailedException e) {
+      LOGGER.debug(
+          "Could not unshallow using upstream branch - assuming currently checked out local branch does not track any remote branch",
+          e);
+      gitClient.unshallow(null);
+    }
+    LOGGER.debug("Repository unshallowing took {} ms", System.currentTimeMillis() - unshallowStart);
+    return true;
+  }
+}

dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/git/tree/GitDataUploaderImplTest.groovy

@@ -40,7 +40,8 @@ class GitDataUploaderImplTest extends Specification {
     gitInfoProvider.getGitInfo(repoRoot) >> new GitInfo(repoUrl, null, null, null)
 
     def gitClient = new GitClient(metricCollector, repoRoot, "25 years ago", 3, TIMEOUT_MILLIS)
-    def uploader = new GitDataUploaderImpl(config, metricCollector, api, gitClient, gitInfoProvider, repoRoot, "origin")
+    def unshallow = new GitRepoUnshallow(config, gitClient)
+    def uploader = new GitDataUploaderImpl(config, metricCollector, api, gitClient, unshallow, gitInfoProvider, repoRoot, "origin")
 
     when:
     def future = uploader.startOrObserveGitDataUpload()