Prefer local Git metadata over CI provider values

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

@@ -54,7 +54,7 @@ private Path getGitPath(String repositoryPath) {
 
   @Override
   public int order() {
-    return 2;
+    return 3;
   }
 
   @Override

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

@@ -32,7 +32,7 @@ public GitInfo build(@Nullable String repositoryPath) {
 
   @Override
   public int order() {
-    return 1;
+    return 2;
   }
 
   @Override

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

@@ -1,12 +1,15 @@
 package datadog.trace.civisibility.git;
 
+import static java.util.Collections.emptyList;
+
 import datadog.trace.api.Config;
 import datadog.trace.api.civisibility.telemetry.tag.GitProviderDiscrepant;
 import datadog.trace.api.civisibility.telemetry.tag.GitProviderExpected;
 import datadog.trace.api.git.CommitInfo;
 import datadog.trace.api.git.GitInfo;
 import datadog.trace.api.git.GitInfoBuilder;
 import datadog.trace.civisibility.git.tree.GitClient;
+import datadog.trace.civisibility.utils.ShellCommandExecutor;
 import java.util.List;
 import javax.annotation.Nullable;
 import org.slf4j.Logger;
@@ -33,14 +36,23 @@ public GitInfo build(@Nullable String repositoryPath) {
     GitClient gitClient = gitClientFactory.create(repositoryPath);
     try {
       String remoteName = config.getCiVisibilityGitRemoteName();
-      String remoteUrl = gitClient.getRemoteUrl(remoteName);
+      String remoteUrl = null;
+      try {
+        remoteUrl = gitClient.getRemoteUrl(remoteName);
+      } catch (ShellCommandExecutor.ShellCommandFailedException e) {
+        LOGGER.debug("Could not read Git remote URL from {}", repositoryPath, e);
+      }
       String branch = gitClient.getCurrentBranch();
-      List<String> tags = gitClient.getTags(GitClient.HEAD);
-      String tag = !tags.isEmpty() ? tags.iterator().next() : null;
+      List<String> tags =
+          branch == null || branch.isEmpty() ? gitClient.getTags(GitClient.HEAD) : emptyList();
+      String tag = tags.size() == 1 ? tags.get(0) : null;
 
       CommitInfo commitInfo = gitClient.getCommitInfo(GitClient.HEAD, false);
       return new GitInfo(remoteUrl, branch, tag, commitInfo);
 
+    } catch (InterruptedException e) {
+      Thread.currentThread().interrupt();
+      return GitInfo.NOOP;
     } catch (Exception e) {
       LOGGER.debug("Error while getting Git data from {}", repositoryPath, e);
       LOGGER.warn("Error while getting Git data by executing shell commands");
@@ -50,7 +62,7 @@ public GitInfo build(@Nullable String repositoryPath) {
 
   @Override
   public int order() {
-    return 3;
+    return 1;
   }
 
   @Override

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

@@ -48,6 +48,10 @@ CommitInfo getCommitInfo(String commit, boolean fetchIfNotPresent)
   @Nonnull
   List<String> getLatestCommits() throws IOException, TimeoutException, InterruptedException;
 
+  @Nonnull
+  List<String> getLatestCommits(String commit)
+      throws IOException, TimeoutException, InterruptedException;
+
   @Nonnull
   List<String> getObjects(Collection<String> commitsToSkip, Collection<String> commitsToInclude)
       throws IOException, TimeoutException, InterruptedException;

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

@@ -96,7 +96,15 @@ private void uploadGitData() {
 
       GitInfo gitInfo = gitInfoProvider.getGitInfo(repoRoot);
       String remoteUrl = gitInfo.getRepositoryURL();
-      List<String> latestCommits = gitClient.getLatestCommits();
+      String selectedCommit = gitInfo.getCommit().getSha();
+      List<String> latestCommits = gitClient.getLatestCommits(selectedCommit);
+      boolean unshallowed = false;
+      if (latestCommits.isEmpty() && config.isCiVisibilityGitUnshallowDefer()) {
+        unshallowed = gitRepoUnshallow.unshallow();
+        if (unshallowed) {
+          latestCommits = gitClient.getLatestCommits(selectedCommit);
+        }
+      }
       if (latestCommits.isEmpty()) {
         LOGGER.debug("No commits in the last month, skipping git data upload");
         callback.complete(null);
@@ -112,8 +120,10 @@ private void uploadGitData() {
         return;
       }
 
-      if (config.isCiVisibilityGitUnshallowDefer() && gitRepoUnshallow.unshallow()) {
-        latestCommits = gitClient.getLatestCommits();
+      if (config.isCiVisibilityGitUnshallowDefer()
+          && !unshallowed
+          && gitRepoUnshallow.unshallow()) {
+        latestCommits = gitClient.getLatestCommits(selectedCommit);
         commitsToSkip = gitDataApi.searchCommits(remoteUrl, latestCommits);
       }
 
@@ -132,7 +142,7 @@ private void uploadGitData() {
         return;
       }
 
-      String currentCommit = latestCommits.get(0);
+      String currentCommit = gitInfo.getCommit().getSha();
 
       Path packFilesDirectory = gitClient.createPackFiles(objectHashes);
       try {

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

@@ -74,6 +74,11 @@ public CommitInfo getCommitInfo(String commit, boolean fetchIfNotPresent) {
   }
 
   @Nonnull
+  @Override
+  public List<String> getLatestCommits(String commit) {
+    return Collections.emptyList();
+  }
+
   @Override
   public List<String> getLatestCommits() {
     return Collections.emptyList();

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

@@ -24,6 +24,7 @@
 import java.util.Collection;
 import java.util.Collections;
 import java.util.Comparator;
+import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Objects;
 import java.util.concurrent.TimeoutException;
@@ -45,7 +46,7 @@ public class ShellGitClient implements GitClient {
       Arrays.asList("release/", "hotfix/");
   private static final Pattern WHITESPACE_PATTERN = Pattern.compile("\\s+");
   private static final String ORIGIN = "origin";
-  private static final Pattern COMMIT_INFO_SPLIT = Pattern.compile("\",\"");
+  private static final Pattern COMMIT_INFO_SPLIT = Pattern.compile("\\x00");
 
   private final CiVisibilityMetricCollector metricCollector;
   private final String repoRoot;
@@ -323,10 +324,9 @@ public List<String> getTags(String commit)
         () -> {
           try {
             return commandExecutor.executeCommand(
-                IOUtils::readLines, buildGitCommand("describe", "--tags", "--exact-match", commit));
+                IOUtils::readLines, buildGitCommand("tag", "--no-column", "--points-at", commit));
           } catch (ShellCommandExecutor.ShellCommandFailedException e) {
-            // if provided commit is not tagged,
-            // command will fail because "--exact-match" is specified
+            // An unavailable revision cannot have local tags.
             return Collections.emptyList();
           }
         });
@@ -438,14 +438,15 @@ public CommitInfo getCommitInfo(String commit, boolean fetchIfNotPresent)
                             "show",
                             commit,
                             "-s",
-                            "--format=%H\",\"%an\",\"%ae\",\"%aI\",\"%cn\",\"%ce\",\"%cI\",\"%B"))
+                            "--no-show-signature",
+                            "--format=%H%x00%an%x00%ae%x00%aI%x00%cn%x00%ce%x00%cI%x00%B"))
                     .trim();
           } catch (ShellCommandExecutor.ShellCommandFailedException e) {
             LOGGER.error("Failed to fetch commit info", e);
             return CommitInfo.NOOP;
           }
 
-          String[] fields = COMMIT_INFO_SPLIT.split(info);
+          String[] fields = COMMIT_INFO_SPLIT.split(info, 8);
           if (fields.length < 8) {
             LOGGER.error("Could not parse commit info: {}", info);
             return CommitInfo.NOOP;
@@ -472,17 +473,42 @@ public CommitInfo getCommitInfo(String commit, boolean fetchIfNotPresent)
   @Override
   public List<String> getLatestCommits()
       throws IOException, TimeoutException, InterruptedException {
-    return executeCommand(
-        Command.GET_LOCAL_COMMITS,
-        () ->
-            commandExecutor.executeCommand(
-                IOUtils::readLines,
-                buildGitCommand(
-                    "log",
-                    "--format=%H",
-                    "-n",
-                    String.valueOf(latestCommitsLimit),
-                    String.format("--since='%s'", latestCommitsSince))));
+    return getLatestCommits(HEAD);
+  }
+
+  @Nonnull
+  @Override
+  public List<String> getLatestCommits(String commit)
+      throws IOException, TimeoutException, InterruptedException {
+    if (GitUtils.isNotValidCommit(commit)) {
+      return Collections.emptyList();
+    }
+    List<String> commits;
+    try {
+      commits =
+          executeCommand(
+              Command.GET_LOCAL_COMMITS,
+              () ->
+                  commandExecutor.executeCommand(
+                      IOUtils::readLines,
+                      buildGitCommand(
+                          "log",
+                          "--format=%H",
+                          "-n",
+                          String.valueOf(latestCommitsLimit),
+                          String.format("--since='%s'", latestCommitsSince),
+                          commit,
+                          "--")));
+    } catch (ShellCommandExecutor.ShellCommandFailedException e) {
+      if (!isCommitPresent(commit)) {
+        return Collections.emptyList();
+      }
+      throw e;
+    }
+    if (!HEAD.equals(commit) && !commits.contains(commit) && isCommitPresent(commit)) {
+      commits.add(0, commit);
+    }
+    return commits;
   }
 
   /**
@@ -505,22 +531,45 @@ public List<String> getObjects(
     return executeCommand(
         Command.GET_OBJECTS,
         () -> {
-          String[] gitArgs = new String[5 + commitsToSkip.size() + commitsToInclude.size()];
+          String[] gitArgs = new String[6 + commitsToSkip.size() + commitsToInclude.size()];
           gitArgs[0] = "rev-list";
           gitArgs[1] = "--objects";
           gitArgs[2] = "--no-object-names";
           gitArgs[3] = "--filter=blob:none";
           gitArgs[4] = String.format("--since='%s'", latestCommitsSince);
+          gitArgs[5] = "--max-count=" + latestCommitsLimit;
 
-          int count = 5;
+          int count = 6;
           for (String commitToSkip : commitsToSkip) {
             gitArgs[count++] = "^" + commitToSkip;
           }
           for (String commitToInclude : commitsToInclude) {
             gitArgs[count++] = commitToInclude;
           }
 
-          return commandExecutor.executeCommand(IOUtils::readLines, buildGitCommand(gitArgs));
+          List<String> objects =
+              commandExecutor.executeCommand(IOUtils::readLines, buildGitCommand(gitArgs));
+          LinkedHashSet<String> outsideWindow = new LinkedHashSet<>(commitsToInclude);
+          for (String object : objects) {
+            outsideWindow.remove(object);
+          }
+          if (!outsideWindow.isEmpty()) {
+            List<String> selectedArgs = new ArrayList<>();
+            Collections.addAll(
+                selectedArgs,
+                "rev-list",
+                "--objects",
+                "--no-object-names",
+                "--filter=blob:none",
+                "--no-walk=unsorted");
+            selectedArgs.addAll(outsideWindow);
+            LinkedHashSet<String> combined = new LinkedHashSet<>(objects);
+            combined.addAll(
+                commandExecutor.executeCommand(
+                    IOUtils::readLines, buildGitCommand(selectedArgs.toArray(new String[0]))));
+            return new ArrayList<>(combined);
+          }
+          return objects;
         });
   }
 

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

@@ -52,6 +52,27 @@ class GitClientGitInfoBuilderTest extends Specification {
       "* Changes based on the review."
   }
 
+  def "keeps checkout metadata when no remote is configured"() {
+    given:
+    givenGitRepo()
+    Files.delete(tempDir.resolve(".git/config"))
+    def metricCollector = Stub(CiVisibilityMetricCollectorImpl)
+    def config = Stub(Config) {
+      getCiVisibilityGitRemoteName() >> "origin"
+      getCiVisibilityGitCommandTimeoutMillis() >> GIT_COMMAND_TIMEOUT_MILLIS
+    }
+    def builder = new GitClientGitInfoBuilder(config, new ShellGitClient.Factory(config, metricCollector))
+
+    when:
+    def gitInfo = builder.build(tempDir.toString())
+
+    then:
+    gitInfo.repositoryURL == null
+    gitInfo.branch == "master"
+    gitInfo.commit.sha == "5b6f3a6dab5972d73a56dff737bd08d995255c08"
+    gitInfo.commit.author.name == "Tony Redondo"
+  }
+
   private void givenGitRepo() {
     givenGitRepo("ci/git/with_pack/git")
   }

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

@@ -2,6 +2,7 @@ package datadog.trace.civisibility.git.tree
 
 import datadog.trace.api.Config
 import datadog.trace.civisibility.telemetry.CiVisibilityMetricCollectorImpl
+import datadog.trace.api.git.CommitInfo
 import datadog.trace.api.git.GitInfo
 import datadog.trace.api.git.GitInfoProvider
 import datadog.communication.util.IOUtils
@@ -37,7 +38,7 @@ class GitDataUploaderImplTest extends Specification {
     }
     def api = Mock(GitDataApi)
     def gitInfoProvider = Stub(GitInfoProvider)
-    gitInfoProvider.getGitInfo(repoRoot) >> new GitInfo(repoUrl, null, null, null)
+    gitInfoProvider.getGitInfo(repoRoot) >> new GitInfo(repoUrl, null, null, new CommitInfo("5b6f3a6dab5972d73a56dff737bd08d995255c08"))
 
     def gitClient = new ShellGitClient(metricCollector, repoRoot, "25 years ago", 3, TIMEOUT_MILLIS)
     def unshallow = new GitRepoUnshallow(config, gitClient)

dd-java-agent/agent-ci-visibility/src/test/java/datadog/trace/civisibility/git/GitClientGitInfoBuilderUnitTest.java

@@ -0,0 +1,63 @@
+package datadog.trace.civisibility.git;
+
+import static java.util.Arrays.asList;
+import static java.util.Collections.singletonList;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import datadog.trace.api.Config;
+import datadog.trace.api.git.CommitInfo;
+import datadog.trace.api.git.GitInfo;
+import datadog.trace.civisibility.git.tree.GitClient;
+import java.util.concurrent.TimeoutException;
+import org.junit.jupiter.api.Test;
+
+class GitClientGitInfoBuilderUnitTest {
+  @Test
+  void interruptedRemoteLookupAbortsAndPreservesInterruption() throws Exception {
+    GitClient client = mock(GitClient.class);
+    when(client.getRemoteUrl("origin")).thenThrow(new InterruptedException());
+    try {
+      assertEquals(GitInfo.NOOP, builder(client).build("/repo"));
+      assertTrue(Thread.currentThread().isInterrupted());
+      verify(client, never()).getCurrentBranch();
+    } finally {
+      Thread.interrupted();
+    }
+    assertFalse(Thread.currentThread().isInterrupted());
+  }
+
+  @Test
+  void timedOutRemoteLookupAborts() throws Exception {
+    GitClient client = mock(GitClient.class);
+    when(client.getRemoteUrl("origin")).thenThrow(new TimeoutException());
+    assertEquals(GitInfo.NOOP, builder(client).build("/repo"));
+    verify(client, never()).getCurrentBranch();
+  }
+
+  @Test
+  void tagsAreOnlyInferredForAnUnambiguousDetachedCheckout() throws Exception {
+    GitClient client = mock(GitClient.class);
+    when(client.getCommitInfo(GitClient.HEAD, false)).thenReturn(new CommitInfo("local-sha"));
+    when(client.getCurrentBranch()).thenReturn("main", "", "");
+    when(client.getTags(GitClient.HEAD)).thenReturn(singletonList("v1"), asList("v1", "stable"));
+    GitClientGitInfoBuilder builder = builder(client);
+    assertNull(builder.build("/repo").getTag());
+    assertEquals("v1", builder.build("/repo").getTag());
+    assertNull(builder.build("/repo").getTag());
+  }
+
+  private GitClientGitInfoBuilder builder(GitClient client) {
+    Config config = mock(Config.class);
+    when(config.getCiVisibilityGitRemoteName()).thenReturn("origin");
+    GitClient.Factory factory = mock(GitClient.Factory.class);
+    when(factory.create("/repo")).thenReturn(client);
+    return new GitClientGitInfoBuilder(config, factory);
+  }
+}