From 6b3cf19191b95f103fa3d39e8b883a21f4bc74c1 Mon Sep 17 00:00:00 2001 From: Daniel Mohedano Date: Tue, 22 Jul 2025 12:10:21 +0200 Subject: [PATCH 01/13] feat: add single command to obtain full commit info --- .../civisibility/git/tree/GitClient.java | 5 ++ .../civisibility/git/tree/NoOpGitClient.java | 7 +++ .../civisibility/git/tree/ShellGitClient.java | 46 +++++++++++++++++++ .../git/tree/GitClientTest.groovy | 30 ++++++++++-- 4 files changed, 84 insertions(+), 4 deletions(-) diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/GitClient.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/GitClient.java index b427a26288a..57bf72839ad 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/GitClient.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/GitClient.java @@ -1,5 +1,6 @@ package datadog.trace.civisibility.git.tree; +import datadog.trace.api.git.CommitInfo; import datadog.trace.civisibility.diff.LineDiff; import java.io.IOException; import java.nio.file.Path; @@ -62,6 +63,10 @@ String getCommitterEmail(String commit) @Nullable String getCommitterDate(String commit) throws IOException, TimeoutException, InterruptedException; + @Nonnull + CommitInfo getCommitInfo(String commit) + throws IOException, TimeoutException, InterruptedException; + @Nonnull List getLatestCommits() throws IOException, TimeoutException, InterruptedException; diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/NoOpGitClient.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/NoOpGitClient.java index 52f959ae9a5..a8639097a82 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/NoOpGitClient.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/NoOpGitClient.java @@ -1,5 +1,6 @@ package datadog.trace.civisibility.git.tree; +import datadog.trace.api.git.CommitInfo; import datadog.trace.civisibility.diff.LineDiff; import java.nio.file.Path; import java.util.Collection; @@ -108,6 +109,12 @@ public String getCommitterDate(String commit) { return null; } + @Nonnull + @Override + public CommitInfo getCommitInfo(String commit) { + return CommitInfo.NOOP; + } + @Nonnull @Override public List getLatestCommits() { diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/ShellGitClient.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/ShellGitClient.java index 45a73449268..49e54ae3448 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/ShellGitClient.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/ShellGitClient.java @@ -6,7 +6,9 @@ import datadog.trace.api.civisibility.telemetry.CiVisibilityDistributionMetric; import datadog.trace.api.civisibility.telemetry.CiVisibilityMetricCollector; import datadog.trace.api.civisibility.telemetry.tag.Command; +import datadog.trace.api.git.CommitInfo; import datadog.trace.api.git.GitUtils; +import datadog.trace.api.git.PersonInfo; import datadog.trace.civisibility.diff.LineDiff; import datadog.trace.civisibility.utils.ShellCommandExecutor; import datadog.trace.util.Strings; @@ -43,6 +45,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 final CiVisibilityMetricCollector metricCollector; private final String repoRoot; @@ -499,6 +502,49 @@ public String getCommitterDate(String commit) .trim()); } + /** + * Returns commit information for the provided commit + * + * @param commit Commit SHA or reference (HEAD, branch name, etc) to check + * @return commit info (sha, author info, committer info, full message) + * @throws IOException If an error was encountered while writing command input or reading output + * @throws TimeoutException If timeout was reached while waiting for Git command to finish + * @throws InterruptedException If current thread was interrupted while waiting for Git command to + * finish + */ + @Nonnull + @Override + public CommitInfo getCommitInfo(String commit) + throws IOException, InterruptedException, TimeoutException { + if (GitUtils.isNotValidCommit(commit)) { + return CommitInfo.NOOP; + } + return executeCommand( + Command.OTHER, + () -> { + String info = + commandExecutor + .executeCommand( + IOUtils::readFully, + "git", + "show", + commit, + "-s", + "--format=%H\",\"%an\",\"%ae\",\"%aI\",\"%cn\",\"%ce\",\"%cI\",\"%B") + .trim(); + String[] fields = COMMIT_INFO_SPLIT.split(info); + if (fields.length < 8) { + LOGGER.error("Could not parse commit info: {}", info); + return CommitInfo.NOOP; + } + return new CommitInfo( + fields[0], + new PersonInfo(fields[1], fields[2], fields[3]), + new PersonInfo(fields[4], fields[5], fields[6]), + fields[7]); + }); + } + /** * Returns SHAs of the latest commits in the current branch. Maximum number of commits and how far * into the past to look are configured when the client is created. diff --git a/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/git/tree/GitClientTest.groovy b/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/git/tree/GitClientTest.groovy index ee3fed3b9d4..87581d0a8dc 100644 --- a/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/git/tree/GitClientTest.groovy +++ b/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/git/tree/GitClientTest.groovy @@ -94,10 +94,10 @@ class GitClientTest extends Specification { where: remoteSha | parentOnly | isShallow | numCommits - GitClient.HEAD | false | false | 10 - null | false | false | 10 - GitClient.HEAD | true | true | 2 - null | true | true | 2 + GitClient.HEAD | false | false | 10 + null | false | false | 10 + GitClient.HEAD | true | true | 2 + null | true | true | 2 } def "test get git folder"() { @@ -247,6 +247,28 @@ class GitClientTest extends Specification { authorDate == "2021-02-26T19:32:13+01:00" } + def "test get commit info"() { + given: + givenGitRepo() + + when: + def gitClient = givenGitClient() + def commitInfo = gitClient.getCommitInfo(GitClient.HEAD) + + then: + commitInfo.sha == "5b6f3a6dab5972d73a56dff737bd08d995255c08" + commitInfo.author.name == "Tony Redondo" + commitInfo.author.email == "tony.redondo@datadoghq.com" + commitInfo.author.iso8601Date == "2021-02-26T19:32:13+01:00" + commitInfo.committer.name == "GitHub" + commitInfo.committer.email == "noreply@github.com" + commitInfo.committer.iso8601Date == "2021-02-26T19:32:13+01:00" + commitInfo.fullMessage == "Adding Git information to test spans (#1242)\n\n" + + "* Initial basic GitInfo implementation.\r\n\r\n" + + "* Adds Author, Committer and Message git parser.\r\n\r\n" + + "* Changes based on the review." + } + def "test get latest commits"() { given: givenGitRepo() From bef1740144fe88bcf84fb9a42de62faa6203f647 Mon Sep 17 00:00:00 2001 From: Daniel Mohedano Date: Tue, 22 Jul 2025 13:19:49 +0200 Subject: [PATCH 02/13] feat: update soft unshallow to use --no-write-fetch --- .../CiVisibilityRepoServices.java | 19 ++------ .../config/ExecutionSettingsFactoryImpl.java | 2 +- .../civisibility/git/tree/GitClient.java | 2 +- .../git/tree/GitDataUploaderImpl.java | 4 +- .../git/tree/GitRepoUnshallow.java | 43 +++++++++++-------- .../civisibility/git/tree/NoOpGitClient.java | 2 +- .../civisibility/git/tree/ShellGitClient.java | 31 +++++-------- .../git/tree/GitClientTest.groovy | 14 +++--- 8 files changed, 53 insertions(+), 64 deletions(-) diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/CiVisibilityRepoServices.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/CiVisibilityRepoServices.java index 3fc0eb482cc..e33172ac4fa 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/CiVisibilityRepoServices.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/CiVisibilityRepoServices.java @@ -6,7 +6,6 @@ import datadog.trace.api.civisibility.telemetry.tag.Provider; import datadog.trace.api.git.CommitInfo; import datadog.trace.api.git.GitInfoProvider; -import datadog.trace.api.git.PersonInfo; import datadog.trace.civisibility.ci.CIInfo; import datadog.trace.civisibility.ci.CIProviderInfo; import datadog.trace.civisibility.ci.CITagsProvider; @@ -127,23 +126,13 @@ static PullRequestInfo buildPullRequestInfo( // complete with CI vars if user didn't provide all information PullRequestInfo ciInfo = PullRequestInfo.merge(userInfo, ciProviderInfo.buildPullRequestInfo()); - if (Strings.isNotBlank(ciInfo.getGitCommitHead().getSha())) { + String headSha = ciInfo.getGitCommitHead().getSha(); + if (Strings.isNotBlank(headSha)) { // if head sha present try to populate author, committer and message info through git client try { - gitRepoUnshallow.unshallow(true); - String headSha = ciInfo.getGitCommitHead().getSha(); - PersonInfo authorInfo = - new PersonInfo( - gitClient.getAuthorName(headSha), - gitClient.getAuthorEmail(headSha), - gitClient.getAuthorDate(headSha)); - PersonInfo committerInfo = - new PersonInfo( - gitClient.getCommitterName(headSha), - gitClient.getCommitterEmail(headSha), - gitClient.getCommitterDate(headSha)); + gitRepoUnshallow.unshallow(headSha); CommitInfo commitInfo = - new CommitInfo(headSha, authorInfo, committerInfo, gitClient.getFullMessage(headSha)); + gitClient.getCommitInfo(headSha); return PullRequestInfo.merge(ciInfo, new PullRequestInfo(null, null, commitInfo, null)); } catch (Exception ignored) { } diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/config/ExecutionSettingsFactoryImpl.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/config/ExecutionSettingsFactoryImpl.java index 3a2855ada83..572b62e2fa2 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/config/ExecutionSettingsFactoryImpl.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/config/ExecutionSettingsFactoryImpl.java @@ -421,7 +421,7 @@ private Diff getPullRequestDiff(boolean impactedTestsDetectionEnabled, String de try { if (repositoryRoot != null) { // ensure repo is not shallow before attempting to get git diff - gitRepoUnshallow.unshallow(false); + gitRepoUnshallow.unshallow(null); String baseCommitSha = pullRequestInfo.getPullRequestBaseBranchSha(); if (baseCommitSha == null) { diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/GitClient.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/GitClient.java index 57bf72839ad..9039edf065f 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/GitClient.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/GitClient.java @@ -17,7 +17,7 @@ public interface GitClient { boolean isShallow() throws IOException, TimeoutException, InterruptedException; - void unshallow(@Nullable String remoteCommitReference, boolean parentOnly) + void unshallow(@Nullable String remoteCommitReference, boolean onlyFetchCommit) throws IOException, TimeoutException, InterruptedException; @Nullable diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/GitDataUploaderImpl.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/GitDataUploaderImpl.java index 4878f7cf8af..f3485e60774 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/GitDataUploaderImpl.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/GitDataUploaderImpl.java @@ -91,7 +91,7 @@ private void uploadGitData() { LOGGER.debug("Starting git data upload, {}", gitClient); if (!config.isCiVisibilityGitUnshallowDefer()) { - gitRepoUnshallow.unshallow(false); + gitRepoUnshallow.unshallow(null); } GitInfo gitInfo = gitInfoProvider.getGitInfo(repoRoot); @@ -112,7 +112,7 @@ private void uploadGitData() { return; } - if (config.isCiVisibilityGitUnshallowDefer() && gitRepoUnshallow.unshallow(false)) { + if (config.isCiVisibilityGitUnshallowDefer() && gitRepoUnshallow.unshallow(null)) { latestCommits = gitClient.getLatestCommits(); commitsToSkip = gitDataApi.searchCommits(remoteUrl, latestCommits); } diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/GitRepoUnshallow.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/GitRepoUnshallow.java index f9dc3f28750..c521552bba8 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/GitRepoUnshallow.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/GitRepoUnshallow.java @@ -4,6 +4,7 @@ import datadog.trace.civisibility.utils.ShellCommandExecutor; import java.io.IOException; import java.util.concurrent.TimeoutException; +import javax.annotation.Nullable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -19,29 +20,37 @@ public GitRepoUnshallow(Config config, GitClient gitClient) { this.gitClient = gitClient; } - public synchronized boolean unshallow(boolean parentOnly) + public synchronized boolean unshallow(@Nullable String commit) throws IOException, InterruptedException, TimeoutException { if (!config.isCiVisibilityGitUnshallowEnabled() || !gitClient.isShallow()) { return false; } long unshallowStart = System.currentTimeMillis(); - try { - gitClient.unshallow(GitClient.HEAD, parentOnly); - } 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, parentOnly); - } 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, parentOnly); + if (commit == null) { + try { + gitClient.unshallow(GitClient.HEAD, false); + } 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, false); + } 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, false); + } + } else { + try { + gitClient.unshallow(commit, true); + } catch (ShellCommandExecutor.ShellCommandFailedException e) { + LOGGER.debug("Could not unshallow specific commit {}", commit, e); + } } LOGGER.debug("Repository unshallowing took {} ms", System.currentTimeMillis() - unshallowStart); return true; diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/NoOpGitClient.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/NoOpGitClient.java index a8639097a82..f0a0a6e8c89 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/NoOpGitClient.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/NoOpGitClient.java @@ -21,7 +21,7 @@ public boolean isShallow() { } @Override - public void unshallow(@Nullable String remoteCommitReference, boolean parentOnly) { + public void unshallow(@Nullable String remoteCommitReference, boolean onlyFetchCommit) { // no op } diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/ShellGitClient.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/ShellGitClient.java index 49e54ae3448..d36f4e19ea0 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/ShellGitClient.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/ShellGitClient.java @@ -129,55 +129,46 @@ public String getUpstreamBranchSha() throws IOException, TimeoutException, Inter * * @param remoteCommitReference The commit to fetch from the remote repository, so local repo will * be updated with this commit and its ancestors. If {@code null}, everything will be fetched. - * @param parentOnly If only the parent commit should be unshallowed or the full {@code - * latestCommitsSince} + * @param onlyFetchCommit Only fetch the specific commit provided. * @throws IOException If an error was encountered while writing command input or reading output * @throws TimeoutException If timeout was reached while waiting for Git command to finish * @throws InterruptedException If current thread was interrupted while waiting for Git command to * finish */ @Override - public void unshallow(@Nullable String remoteCommitReference, boolean parentOnly) + public void unshallow(@Nullable String remoteCommitReference, boolean onlyFetchCommit) throws IOException, TimeoutException, InterruptedException { executeCommand( Command.UNSHALLOW, () -> { - String remote = - commandExecutor - .executeCommand( - IOUtils::readFully, - "git", - "config", - "--default", - "origin", - "--get", - "clone.defaultRemoteName") - .trim(); + String remote = getRemoteName(); // refetch data from the server for the given period of time - String depth = - parentOnly ? "--deepen=1" : String.format("--shallow-since='%s'", latestCommitsSince); if (remoteCommitReference != null && GitUtils.isValidRef(remoteCommitReference)) { - String headSha = getSha(remoteCommitReference); + String onlyFetchCommitArg = + onlyFetchCommit + ? "--no-write-fetch-head" + : String.format("--shallow-since='%s'", latestCommitsSince); + String commitSha = getSha(remoteCommitReference); commandExecutor.executeCommand( ShellCommandExecutor.OutputParser.IGNORE, "git", "fetch", - depth, "--update-shallow", "--filter=blob:none", "--recurse-submodules=no", + onlyFetchCommitArg, remote, - headSha); + commitSha); } else { commandExecutor.executeCommand( ShellCommandExecutor.OutputParser.IGNORE, "git", "fetch", - depth, "--update-shallow", "--filter=blob:none", "--recurse-submodules=no", + String.format("--shallow-since='%s'", latestCommitsSince), remote); } diff --git a/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/git/tree/GitClientTest.groovy b/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/git/tree/GitClientTest.groovy index 87581d0a8dc..1fcaacd4af0 100644 --- a/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/git/tree/GitClientTest.groovy +++ b/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/git/tree/GitClientTest.groovy @@ -70,7 +70,7 @@ class GitClientTest extends Specification { upstreamBranch == "98b944cc44f18bfb78e3021de2999cdcda8efdf6" } - def "test unshallow: sha-#remoteSha parentOnly-#parentOnly"() { + def "test unshallow: sha-#remoteSha fetchOnlyCommit-#fetchOnlyCommit"() { given: givenGitRepo("ci/git/shallow/git") @@ -84,7 +84,7 @@ class GitClientTest extends Specification { commits.size() == 1 when: - gitClient.unshallow(remoteSha, parentOnly) + gitClient.unshallow(remoteSha, fetchOnlyCommit) shallow = gitClient.isShallow() commits = gitClient.getLatestCommits() @@ -93,11 +93,11 @@ class GitClientTest extends Specification { commits.size() == numCommits where: - remoteSha | parentOnly | isShallow | numCommits - GitClient.HEAD | false | false | 10 - null | false | false | 10 - GitClient.HEAD | true | true | 2 - null | true | true | 2 + remoteSha | fetchOnlyCommit | isShallow | numCommits + GitClient.HEAD | false | false | 10 + null | false | false | 10 + "f4377e97f10c2d58696192b170b2fef2a8464b04" | true | true | 1 + null | true | false | 10 } def "test get git folder"() { From 25dbcbb6eca3455a4a00fc6a99085cce8e91fa6f Mon Sep 17 00:00:00 2001 From: Daniel Mohedano Date: Tue, 22 Jul 2025 13:20:34 +0200 Subject: [PATCH 03/13] chore: remove unused getAuthor..., getCommitter..., and getFullMessage --- .../git/GitClientGitInfoBuilder.java | 15 +- .../civisibility/git/tree/GitClient.java | 22 --- .../civisibility/git/tree/NoOpGitClient.java | 42 ----- .../civisibility/git/tree/ShellGitClient.java | 175 ------------------ .../CiVisibilityRepoServicesTest.groovy | 8 +- .../git/tree/GitClientTest.groovy | 93 +--------- 6 files changed, 5 insertions(+), 350 deletions(-) diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/GitClientGitInfoBuilder.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/GitClientGitInfoBuilder.java index 7553cf40318..80aafda60dd 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/GitClientGitInfoBuilder.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/GitClientGitInfoBuilder.java @@ -39,20 +39,7 @@ public GitInfo build(@Nullable String repositoryPath) { List tags = gitClient.getTags(GitClient.HEAD); String tag = !tags.isEmpty() ? tags.iterator().next() : null; - String currentCommitSha = gitClient.getSha(GitClient.HEAD); - String fullMessage = gitClient.getFullMessage(GitClient.HEAD); - - String authorName = gitClient.getAuthorName(GitClient.HEAD); - String authorEmail = gitClient.getAuthorEmail(GitClient.HEAD); - String authorDate = gitClient.getAuthorDate(GitClient.HEAD); - PersonInfo author = new PersonInfo(authorName, authorEmail, authorDate); - - String committerName = gitClient.getCommitterName(GitClient.HEAD); - String committerEmail = gitClient.getCommitterEmail(GitClient.HEAD); - String committerDate = gitClient.getCommitterDate(GitClient.HEAD); - PersonInfo committer = new PersonInfo(committerName, committerEmail, committerDate); - - CommitInfo commitInfo = new CommitInfo(currentCommitSha, author, committer, fullMessage); + CommitInfo commitInfo = gitClient.getCommitInfo(GitClient.HEAD); return new GitInfo(remoteUrl, branch, tag, commitInfo); } catch (Exception e) { diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/GitClient.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/GitClient.java index 9039edf065f..b00fe394f84 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/GitClient.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/GitClient.java @@ -41,28 +41,6 @@ void unshallow(@Nullable String remoteCommitReference, boolean onlyFetchCommit) @Nullable String getSha(String reference) throws IOException, TimeoutException, InterruptedException; - @Nullable - String getFullMessage(String commit) throws IOException, TimeoutException, InterruptedException; - - @Nullable - String getAuthorName(String commit) throws IOException, TimeoutException, InterruptedException; - - @Nullable - String getAuthorEmail(String commit) throws IOException, TimeoutException, InterruptedException; - - @Nullable - String getAuthorDate(String commit) throws IOException, TimeoutException, InterruptedException; - - @Nullable - String getCommitterName(String commit) throws IOException, TimeoutException, InterruptedException; - - @Nullable - String getCommitterEmail(String commit) - throws IOException, TimeoutException, InterruptedException; - - @Nullable - String getCommitterDate(String commit) throws IOException, TimeoutException, InterruptedException; - @Nonnull CommitInfo getCommitInfo(String commit) throws IOException, TimeoutException, InterruptedException; diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/NoOpGitClient.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/NoOpGitClient.java index f0a0a6e8c89..ce8cc208e5e 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/NoOpGitClient.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/NoOpGitClient.java @@ -67,48 +67,6 @@ public String getSha(String reference) { return null; } - @Nullable - @Override - public String getFullMessage(String commit) { - return null; - } - - @Nullable - @Override - public String getAuthorName(String commit) { - return null; - } - - @Nullable - @Override - public String getAuthorEmail(String commit) { - return null; - } - - @Nullable - @Override - public String getAuthorDate(String commit) { - return null; - } - - @Nullable - @Override - public String getCommitterName(String commit) { - return null; - } - - @Nullable - @Override - public String getCommitterEmail(String commit) { - return null; - } - - @Nullable - @Override - public String getCommitterDate(String commit) { - return null; - } - @Nonnull @Override public CommitInfo getCommitInfo(String commit) { diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/ShellGitClient.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/ShellGitClient.java index d36f4e19ea0..e977818b210 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/ShellGitClient.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/ShellGitClient.java @@ -318,181 +318,6 @@ public String getSha(String reference) .trim()); } - /** - * Returns full message of the provided commit - * - * @param commit Commit SHA or reference (HEAD, branch name, etc) to check - * @return full message of the provided commit - * @throws IOException If an error was encountered while writing command input or reading output - * @throws TimeoutException If timeout was reached while waiting for Git command to finish - * @throws InterruptedException If current thread was interrupted while waiting for Git command to - * finish - */ - @Nullable - @Override - public String getFullMessage(String commit) - throws IOException, TimeoutException, InterruptedException { - if (GitUtils.isNotValidCommit(commit)) { - return null; - } - return executeCommand( - Command.OTHER, - () -> - commandExecutor - .executeCommand(IOUtils::readFully, "git", "log", "-n", "1", "--format=%B", commit) - .trim()); - } - - /** - * Returns author name for the provided commit - * - * @param commit Commit SHA or reference (HEAD, branch name, etc) to check - * @return author name for the provided commit - * @throws IOException If an error was encountered while writing command input or reading output - * @throws TimeoutException If timeout was reached while waiting for Git command to finish - * @throws InterruptedException If current thread was interrupted while waiting for Git command to - * finish - */ - @Nullable - @Override - public String getAuthorName(String commit) - throws IOException, TimeoutException, InterruptedException { - if (GitUtils.isNotValidCommit(commit)) { - return null; - } - return executeCommand( - Command.OTHER, - () -> - commandExecutor - .executeCommand(IOUtils::readFully, "git", "log", "-n", "1", "--format=%an", commit) - .trim()); - } - - /** - * Returns author email for the provided commit - * - * @param commit Commit SHA or reference (HEAD, branch name, etc) to check - * @return author email for the provided commit - * @throws IOException If an error was encountered while writing command input or reading output - * @throws TimeoutException If timeout was reached while waiting for Git command to finish - * @throws InterruptedException If current thread was interrupted while waiting for Git command to - * finish - */ - @Nullable - @Override - public String getAuthorEmail(String commit) - throws IOException, TimeoutException, InterruptedException { - if (GitUtils.isNotValidCommit(commit)) { - return null; - } - return executeCommand( - Command.OTHER, - () -> - commandExecutor - .executeCommand(IOUtils::readFully, "git", "log", "-n", "1", "--format=%ae", commit) - .trim()); - } - - /** - * Returns author date in strict ISO 8601 format for the provided commit - * - * @param commit Commit SHA or reference (HEAD, branch name, etc) to check - * @return author date in strict ISO 8601 format - * @throws IOException If an error was encountered while writing command input or reading output - * @throws TimeoutException If timeout was reached while waiting for Git command to finish - * @throws InterruptedException If current thread was interrupted while waiting for Git command to - * finish - */ - @Nullable - @Override - public String getAuthorDate(String commit) - throws IOException, TimeoutException, InterruptedException { - if (GitUtils.isNotValidCommit(commit)) { - return null; - } - return executeCommand( - Command.OTHER, - () -> - commandExecutor - .executeCommand(IOUtils::readFully, "git", "log", "-n", "1", "--format=%aI", commit) - .trim()); - } - - /** - * Returns committer name for the provided commit - * - * @param commit Commit SHA or reference (HEAD, branch name, etc) to check - * @return committer name for the provided commit - * @throws IOException If an error was encountered while writing command input or reading output - * @throws TimeoutException If timeout was reached while waiting for Git command to finish - * @throws InterruptedException If current thread was interrupted while waiting for Git command to - * finish - */ - @Nullable - @Override - public String getCommitterName(String commit) - throws IOException, TimeoutException, InterruptedException { - if (GitUtils.isNotValidCommit(commit)) { - return null; - } - return executeCommand( - Command.OTHER, - () -> - commandExecutor - .executeCommand(IOUtils::readFully, "git", "log", "-n", "1", "--format=%cn", commit) - .trim()); - } - - /** - * Returns committer email for the provided commit - * - * @param commit Commit SHA or reference (HEAD, branch name, etc) to check - * @return committer email for the provided commit - * @throws IOException If an error was encountered while writing command input or reading output - * @throws TimeoutException If timeout was reached while waiting for Git command to finish - * @throws InterruptedException If current thread was interrupted while waiting for Git command to - * finish - */ - @Nullable - @Override - public String getCommitterEmail(String commit) - throws IOException, TimeoutException, InterruptedException { - if (GitUtils.isNotValidCommit(commit)) { - return null; - } - return executeCommand( - Command.OTHER, - () -> - commandExecutor - .executeCommand(IOUtils::readFully, "git", "log", "-n", "1", "--format=%ce", commit) - .trim()); - } - - /** - * Returns committer date in strict ISO 8601 format for the provided commit - * - * @param commit Commit SHA or reference (HEAD, branch name, etc) to check - * @return committer date in strict ISO 8601 format - * @throws IOException If an error was encountered while writing command input or reading output - * @throws TimeoutException If timeout was reached while waiting for Git command to finish - * @throws InterruptedException If current thread was interrupted while waiting for Git command to - * finish - */ - @Nullable - @Override - public String getCommitterDate(String commit) - throws IOException, TimeoutException, InterruptedException { - if (GitUtils.isNotValidCommit(commit)) { - return null; - } - return executeCommand( - Command.OTHER, - () -> - commandExecutor - .executeCommand(IOUtils::readFully, "git", "log", "-n", "1", "--format=%cI", commit) - .trim()); - } - /** * Returns commit information for the provided commit * diff --git a/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/CiVisibilityRepoServicesTest.groovy b/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/CiVisibilityRepoServicesTest.groovy index a683da60369..e8881b69bc2 100644 --- a/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/CiVisibilityRepoServicesTest.groovy +++ b/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/CiVisibilityRepoServicesTest.groovy @@ -59,13 +59,7 @@ class CiVisibilityRepoServicesTest extends Specification { def gitClient = Stub(GitClient) gitClient.getMergeBase("targetSha", "sourceSha") >> expectedInfo.getPullRequestBaseBranchSha() - gitClient.getAuthorName("sourceSha") >> expectedInfo.getGitCommitHead().getAuthor().getName() - gitClient.getAuthorEmail("sourceSha") >> expectedInfo.getGitCommitHead().getAuthor().getEmail() - gitClient.getAuthorDate("sourceSha") >> expectedInfo.getGitCommitHead().getAuthor().getIso8601Date() - gitClient.getCommitterName("sourceSha") >> expectedInfo.getGitCommitHead().getCommitter().getName() - gitClient.getCommitterEmail("sourceSha") >> expectedInfo.getGitCommitHead().getCommitter().getEmail() - gitClient.getCommitterDate("sourceSha") >> expectedInfo.getGitCommitHead().getCommitter().getIso8601Date() - gitClient.getFullMessage("sourceSha") >> expectedInfo.getGitCommitHead().getFullMessage() + gitClient.getCommitInfo("sourceSha") >> expectedInfo.getGitCommitHead() expect: CiVisibilityRepoServices.buildPullRequestInfo(config, environment, ciProviderInfo, gitClient, repoUnshallow) == expectedInfo diff --git a/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/git/tree/GitClientTest.groovy b/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/git/tree/GitClientTest.groovy index 1fcaacd4af0..b32b412df32 100644 --- a/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/git/tree/GitClientTest.groovy +++ b/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/git/tree/GitClientTest.groovy @@ -160,93 +160,6 @@ class GitClientTest extends Specification { sha == "5b6f3a6dab5972d73a56dff737bd08d995255c08" } - def "test get full message"() { - given: - givenGitRepo() - - when: - def gitClient = givenGitClient() - def message = gitClient.getFullMessage(GitClient.HEAD) - - then: - message == "Adding Git information to test spans (#1242)\n\n" + - "* Initial basic GitInfo implementation.\r\n\r\n" + - "* Adds Author, Committer and Message git parser.\r\n\r\n" + - "* Changes based on the review." - } - - def "test get author name"() { - given: - givenGitRepo() - - when: - def gitClient = givenGitClient() - def authorName = gitClient.getAuthorName(GitClient.HEAD) - - then: - authorName == "Tony Redondo" - } - - def "test get author email"() { - given: - givenGitRepo() - - when: - def gitClient = givenGitClient() - def authorEmail = gitClient.getAuthorEmail(GitClient.HEAD) - - then: - authorEmail == "tony.redondo@datadoghq.com" - } - - def "test get author date"() { - given: - givenGitRepo() - - when: - def gitClient = givenGitClient() - def authorDate = gitClient.getAuthorDate(GitClient.HEAD) - - then: - authorDate == "2021-02-26T19:32:13+01:00" - } - - def "test get committer name"() { - given: - givenGitRepo() - - when: - def gitClient = givenGitClient() - def authorName = gitClient.getCommitterName(GitClient.HEAD) - - then: - authorName == "GitHub" - } - - def "test get committer email"() { - given: - givenGitRepo() - - when: - def gitClient = givenGitClient() - def authorEmail = gitClient.getCommitterEmail(GitClient.HEAD) - - then: - authorEmail == "noreply@github.com" - } - - def "test get committer date"() { - given: - givenGitRepo() - - when: - def gitClient = givenGitClient() - def authorDate = gitClient.getCommitterDate(GitClient.HEAD) - - then: - authorDate == "2021-02-26T19:32:13+01:00" - } - def "test get commit info"() { given: givenGitRepo() @@ -264,9 +177,9 @@ class GitClientTest extends Specification { commitInfo.committer.email == "noreply@github.com" commitInfo.committer.iso8601Date == "2021-02-26T19:32:13+01:00" commitInfo.fullMessage == "Adding Git information to test spans (#1242)\n\n" + - "* Initial basic GitInfo implementation.\r\n\r\n" + - "* Adds Author, Committer and Message git parser.\r\n\r\n" + - "* Changes based on the review." + "* Initial basic GitInfo implementation.\r\n\r\n" + + "* Adds Author, Committer and Message git parser.\r\n\r\n" + + "* Changes based on the review." } def "test get latest commits"() { From 815a0955206ac9908bb783393e1f79bcc8017a15 Mon Sep 17 00:00:00 2001 From: Daniel Mohedano Date: Tue, 22 Jul 2025 13:24:49 +0200 Subject: [PATCH 04/13] style: spotless --- .../trace/civisibility/CiVisibilityRepoServices.java | 3 +-- .../trace/civisibility/git/GitClientGitInfoBuilder.java | 1 - .../trace/civisibility/git/tree/GitClientTest.groovy | 6 +++--- 3 files changed, 4 insertions(+), 6 deletions(-) diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/CiVisibilityRepoServices.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/CiVisibilityRepoServices.java index e33172ac4fa..3892f353ee9 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/CiVisibilityRepoServices.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/CiVisibilityRepoServices.java @@ -131,8 +131,7 @@ static PullRequestInfo buildPullRequestInfo( // if head sha present try to populate author, committer and message info through git client try { gitRepoUnshallow.unshallow(headSha); - CommitInfo commitInfo = - gitClient.getCommitInfo(headSha); + CommitInfo commitInfo = gitClient.getCommitInfo(headSha); return PullRequestInfo.merge(ciInfo, new PullRequestInfo(null, null, commitInfo, null)); } catch (Exception ignored) { } diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/GitClientGitInfoBuilder.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/GitClientGitInfoBuilder.java index 80aafda60dd..d10a2cc27f8 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/GitClientGitInfoBuilder.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/GitClientGitInfoBuilder.java @@ -6,7 +6,6 @@ import datadog.trace.api.git.CommitInfo; import datadog.trace.api.git.GitInfo; import datadog.trace.api.git.GitInfoBuilder; -import datadog.trace.api.git.PersonInfo; import datadog.trace.civisibility.git.tree.GitClient; import java.util.List; import javax.annotation.Nullable; diff --git a/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/git/tree/GitClientTest.groovy b/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/git/tree/GitClientTest.groovy index b32b412df32..ed4641fc2c3 100644 --- a/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/git/tree/GitClientTest.groovy +++ b/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/git/tree/GitClientTest.groovy @@ -177,9 +177,9 @@ class GitClientTest extends Specification { commitInfo.committer.email == "noreply@github.com" commitInfo.committer.iso8601Date == "2021-02-26T19:32:13+01:00" commitInfo.fullMessage == "Adding Git information to test spans (#1242)\n\n" + - "* Initial basic GitInfo implementation.\r\n\r\n" + - "* Adds Author, Committer and Message git parser.\r\n\r\n" + - "* Changes based on the review." + "* Initial basic GitInfo implementation.\r\n\r\n" + + "* Adds Author, Committer and Message git parser.\r\n\r\n" + + "* Changes based on the review." } def "test get latest commits"() { From d4526f86ebcb5a85b75c3da81e4a5939e1f11f31 Mon Sep 17 00:00:00 2001 From: Daniel Mohedano Date: Tue, 22 Jul 2025 14:57:47 +0200 Subject: [PATCH 05/13] chore: renamed field to headCommit for clarity --- .../CiVisibilityRepoServices.java | 2 +- .../trace/civisibility/ci/CITagsProvider.java | 17 +++++++------ .../civisibility/ci/PullRequestInfo.java | 24 +++++++++---------- .../config/ExecutionSettingsFactoryImpl.java | 11 ++++----- .../CiVisibilityRepoServicesTest.groovy | 4 ++-- 5 files changed, 28 insertions(+), 30 deletions(-) diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/CiVisibilityRepoServices.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/CiVisibilityRepoServices.java index 3892f353ee9..3ff199d11fd 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/CiVisibilityRepoServices.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/CiVisibilityRepoServices.java @@ -126,7 +126,7 @@ static PullRequestInfo buildPullRequestInfo( // complete with CI vars if user didn't provide all information PullRequestInfo ciInfo = PullRequestInfo.merge(userInfo, ciProviderInfo.buildPullRequestInfo()); - String headSha = ciInfo.getGitCommitHead().getSha(); + String headSha = ciInfo.getHeadCommit().getSha(); if (Strings.isNotBlank(headSha)) { // if head sha present try to populate author, committer and message info through git client try { diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/ci/CITagsProvider.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/ci/CITagsProvider.java index a7d7bd0ccdc..c3d2aae75f5 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/ci/CITagsProvider.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/ci/CITagsProvider.java @@ -140,48 +140,47 @@ public CITagsBuilder withPullRequestBaseBranchSha(final PullRequestInfo pullRequ } public CITagsBuilder withGitCommitHeadSha(final PullRequestInfo pullRequestInfo) { - return putTagValue(Tags.GIT_COMMIT_HEAD_SHA, pullRequestInfo.getGitCommitHead().getSha()); + return putTagValue(Tags.GIT_COMMIT_HEAD_SHA, pullRequestInfo.getHeadCommit().getSha()); } public CITagsBuilder withGitCommitHeadAuthorName(final PullRequestInfo pullRequestInfo) { return putTagValue( - Tags.GIT_COMMIT_HEAD_AUTHOR_NAME, - pullRequestInfo.getGitCommitHead().getAuthor().getName()); + Tags.GIT_COMMIT_HEAD_AUTHOR_NAME, pullRequestInfo.getHeadCommit().getAuthor().getName()); } public CITagsBuilder withGitCommitHeadAuthorEmail(final PullRequestInfo pullRequestInfo) { return putTagValue( Tags.GIT_COMMIT_HEAD_AUTHOR_EMAIL, - pullRequestInfo.getGitCommitHead().getAuthor().getEmail()); + pullRequestInfo.getHeadCommit().getAuthor().getEmail()); } public CITagsBuilder withGitCommitHeadAuthorDate(final PullRequestInfo pullRequestInfo) { return putTagValue( Tags.GIT_COMMIT_HEAD_AUTHOR_DATE, - pullRequestInfo.getGitCommitHead().getAuthor().getIso8601Date()); + pullRequestInfo.getHeadCommit().getAuthor().getIso8601Date()); } public CITagsBuilder withGitCommitHeadCommitterName(final PullRequestInfo pullRequestInfo) { return putTagValue( Tags.GIT_COMMIT_HEAD_COMMITTER_NAME, - pullRequestInfo.getGitCommitHead().getCommitter().getName()); + pullRequestInfo.getHeadCommit().getCommitter().getName()); } public CITagsBuilder withGitCommitHeadCommitterEmail(final PullRequestInfo pullRequestInfo) { return putTagValue( Tags.GIT_COMMIT_HEAD_COMMITTER_EMAIL, - pullRequestInfo.getGitCommitHead().getCommitter().getEmail()); + pullRequestInfo.getHeadCommit().getCommitter().getEmail()); } public CITagsBuilder withGitCommitHeadCommitterDate(final PullRequestInfo pullRequestInfo) { return putTagValue( Tags.GIT_COMMIT_HEAD_COMMITTER_DATE, - pullRequestInfo.getGitCommitHead().getCommitter().getIso8601Date()); + pullRequestInfo.getHeadCommit().getCommitter().getIso8601Date()); } public CITagsBuilder withGitCommitHeadMessage(final PullRequestInfo pullRequestInfo) { return putTagValue( - Tags.GIT_COMMIT_HEAD_MESSAGE, pullRequestInfo.getGitCommitHead().getFullMessage()); + Tags.GIT_COMMIT_HEAD_MESSAGE, pullRequestInfo.getHeadCommit().getFullMessage()); } public CITagsBuilder withPullRequestNumber(final PullRequestInfo pullRequestInfo) { diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/ci/PullRequestInfo.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/ci/PullRequestInfo.java index f58151c08d9..75a18d3ebb2 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/ci/PullRequestInfo.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/ci/PullRequestInfo.java @@ -12,17 +12,17 @@ public class PullRequestInfo { private final String pullRequestBaseBranch; private final String pullRequestBaseBranchSha; - @Nonnull private final CommitInfo gitCommitHead; + @Nonnull private final CommitInfo headCommit; private final String pullRequestNumber; public PullRequestInfo( String pullRequestBaseBranch, String pullRequestBaseBranchSha, - @Nonnull CommitInfo gitCommitHead, + @Nonnull CommitInfo headCommit, String pullRequestNumber) { this.pullRequestBaseBranch = pullRequestBaseBranch; this.pullRequestBaseBranchSha = pullRequestBaseBranchSha; - this.gitCommitHead = gitCommitHead; + this.headCommit = headCommit; this.pullRequestNumber = pullRequestNumber; } @@ -35,8 +35,8 @@ public String getPullRequestBaseBranchSha() { } @Nonnull - public CommitInfo getGitCommitHead() { - return gitCommitHead; + public CommitInfo getHeadCommit() { + return headCommit; } public String getPullRequestNumber() { @@ -46,14 +46,14 @@ public String getPullRequestNumber() { public boolean isEmpty() { return Strings.isBlank(pullRequestBaseBranch) && Strings.isBlank(pullRequestBaseBranchSha) - && gitCommitHead.isEmpty() + && headCommit.isEmpty() && Strings.isBlank(pullRequestNumber); } public boolean isComplete() { return Strings.isNotBlank(pullRequestBaseBranch) && Strings.isNotBlank(pullRequestBaseBranchSha) - && gitCommitHead.isComplete() + && headCommit.isComplete() && Strings.isNotBlank(pullRequestNumber); } @@ -72,7 +72,7 @@ public static PullRequestInfo merge(PullRequestInfo info, PullRequestInfo fallba Strings.isNotBlank(info.pullRequestBaseBranchSha) ? info.pullRequestBaseBranchSha : fallback.pullRequestBaseBranchSha, - CommitInfo.merge(info.gitCommitHead, fallback.gitCommitHead), + CommitInfo.merge(info.headCommit, fallback.headCommit), Strings.isNotBlank(info.pullRequestNumber) ? info.pullRequestNumber : fallback.pullRequestNumber); @@ -89,14 +89,14 @@ public boolean equals(Object o) { PullRequestInfo that = (PullRequestInfo) o; return Objects.equals(pullRequestBaseBranch, that.pullRequestBaseBranch) && Objects.equals(pullRequestBaseBranchSha, that.pullRequestBaseBranchSha) - && Objects.equals(gitCommitHead, that.gitCommitHead) + && Objects.equals(headCommit, that.headCommit) && Objects.equals(pullRequestNumber, that.pullRequestNumber); } @Override public int hashCode() { return Objects.hash( - pullRequestBaseBranch, pullRequestBaseBranchSha, gitCommitHead, pullRequestNumber); + pullRequestBaseBranch, pullRequestBaseBranchSha, headCommit, pullRequestNumber); } @Override @@ -108,8 +108,8 @@ public String toString() { + ", baseSHA='" + pullRequestBaseBranchSha + '\'' - + ", commitSHA='" - + gitCommitHead + + ", headCommit='" + + headCommit + '\'' + ", prNumber='" + pullRequestNumber diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/config/ExecutionSettingsFactoryImpl.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/config/ExecutionSettingsFactoryImpl.java index 572b62e2fa2..13adf44fd3a 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/config/ExecutionSettingsFactoryImpl.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/config/ExecutionSettingsFactoryImpl.java @@ -396,12 +396,12 @@ private Map>> getTestManagementTest return Collections.emptyMap(); } try { - if (Strings.isNotBlank(pullRequestInfo.getGitCommitHead().getSha()) - && Strings.isNotBlank(pullRequestInfo.getGitCommitHead().getFullMessage())) { + if (Strings.isNotBlank(pullRequestInfo.getHeadCommit().getSha()) + && Strings.isNotBlank(pullRequestInfo.getHeadCommit().getFullMessage())) { return configurationApi.getTestManagementTestsByModule( tracerEnvironment, - pullRequestInfo.getGitCommitHead().getSha(), - pullRequestInfo.getGitCommitHead().getFullMessage()); + pullRequestInfo.getHeadCommit().getSha(), + pullRequestInfo.getHeadCommit().getFullMessage()); } else { return configurationApi.getTestManagementTestsByModule( tracerEnvironment, tracerEnvironment.getSha(), tracerEnvironment.getCommitMessage()); @@ -429,8 +429,7 @@ private Diff getPullRequestDiff(boolean impactedTestsDetectionEnabled, String de gitClient.getBaseCommitSha(pullRequestInfo.getPullRequestBaseBranch(), defaultBranch); } - Diff diff = - gitClient.getGitDiff(baseCommitSha, pullRequestInfo.getGitCommitHead().getSha()); + Diff diff = gitClient.getGitDiff(baseCommitSha, pullRequestInfo.getHeadCommit().getSha()); if (diff != null) { return diff; } diff --git a/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/CiVisibilityRepoServicesTest.groovy b/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/CiVisibilityRepoServicesTest.groovy index e8881b69bc2..c57b80830c6 100644 --- a/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/CiVisibilityRepoServicesTest.groovy +++ b/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/CiVisibilityRepoServicesTest.groovy @@ -51,7 +51,7 @@ class CiVisibilityRepoServicesTest extends Specification { def environment = Stub(CiEnvironment) environment.get(Constants.DDCI_PULL_REQUEST_TARGET_SHA) >> "targetSha" - environment.get(Constants.DDCI_PULL_REQUEST_SOURCE_SHA) >> expectedInfo.getGitCommitHead().getSha() + environment.get(Constants.DDCI_PULL_REQUEST_SOURCE_SHA) >> expectedInfo.getHeadCommit().getSha() def repoUnshallow = Stub(GitRepoUnshallow) def ciProviderInfo = Stub(CIProviderInfo) @@ -59,7 +59,7 @@ class CiVisibilityRepoServicesTest extends Specification { def gitClient = Stub(GitClient) gitClient.getMergeBase("targetSha", "sourceSha") >> expectedInfo.getPullRequestBaseBranchSha() - gitClient.getCommitInfo("sourceSha") >> expectedInfo.getGitCommitHead() + gitClient.getCommitInfo("sourceSha") >> expectedInfo.getHeadCommit() expect: CiVisibilityRepoServices.buildPullRequestInfo(config, environment, ciProviderInfo, gitClient, repoUnshallow) == expectedInfo From 50f04214ff37e7a9cf0a525a4461b49f1068260a Mon Sep 17 00:00:00 2001 From: Daniel Mohedano Date: Tue, 22 Jul 2025 14:58:31 +0200 Subject: [PATCH 06/13] feat: add pr number parsing for github actions file --- .../trace/civisibility/ci/GithubActionsInfo.java | 11 ++++++++++- .../civisibility/ci/GithubActionsInfoTest.groovy | 3 ++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/ci/GithubActionsInfo.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/ci/GithubActionsInfo.java index 0be6c3e0b70..fb26e590445 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/ci/GithubActionsInfo.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/ci/GithubActionsInfo.java @@ -109,6 +109,7 @@ public PullRequestInfo buildPullRequestInfo() { String baseSha = null; String headSha = null; + String prNumber = null; Map pullRequest = (Map) eventJson.get("pull_request"); if (pullRequest != null) { @@ -121,9 +122,17 @@ public PullRequestInfo buildPullRequestInfo() { if (base != null) { baseSha = (String) base.get("sha"); } + + Object number = pullRequest.get("number"); + if (number != null) { + prNumber = + number instanceof Double + ? String.valueOf(((Double) number).intValue()) + : String.valueOf(number); + } } - return new PullRequestInfo(baseRef, baseSha, new CommitInfo(headSha), null); + return new PullRequestInfo(baseRef, baseSha, new CommitInfo(headSha), prNumber); } catch (Exception e) { LOGGER.warn("Error while parsing GitHub event", e); diff --git a/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/ci/GithubActionsInfoTest.groovy b/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/ci/GithubActionsInfoTest.groovy index 6ffdf46166e..0be0b908c7b 100644 --- a/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/ci/GithubActionsInfoTest.groovy +++ b/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/ci/GithubActionsInfoTest.groovy @@ -42,6 +42,7 @@ class GithubActionsInfoTest extends CITagsProviderTest { then: pullRequestInfo.getPullRequestBaseBranch() == "base-ref" pullRequestInfo.getPullRequestBaseBranchSha() == "52e0974c74d41160a03d59ddc73bb9f5adab054b" - pullRequestInfo.getGitCommitHead().getSha() == "df289512a51123083a8e6931dd6f57bb3883d4c4" + pullRequestInfo.getHeadCommit().getSha() == "df289512a51123083a8e6931dd6f57bb3883d4c4" + pullRequestInfo.getPullRequestNumber() == "1" } } From 13a176f2c2ec95a102fb1764ac79f760f66a5c5f Mon Sep 17 00:00:00 2001 From: Daniel Mohedano Date: Tue, 22 Jul 2025 16:49:36 +0200 Subject: [PATCH 07/13] fix: treat pr.number as `Number` for github actions parsing --- .../datadog/trace/civisibility/ci/GithubActionsInfo.java | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/ci/GithubActionsInfo.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/ci/GithubActionsInfo.java index fb26e590445..41a99629090 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/ci/GithubActionsInfo.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/ci/GithubActionsInfo.java @@ -124,11 +124,8 @@ public PullRequestInfo buildPullRequestInfo() { } Object number = pullRequest.get("number"); - if (number != null) { - prNumber = - number instanceof Double - ? String.valueOf(((Double) number).intValue()) - : String.valueOf(number); + if (number instanceof Number) { + prNumber = String.valueOf(((Number) number).intValue()); } } From 72f9b91d45612f3daadd9c5307242e08c7702ad0 Mon Sep 17 00:00:00 2001 From: Daniel Mohedano Date: Wed, 23 Jul 2025 10:33:51 +0200 Subject: [PATCH 08/13] fix: simplify pr.number extraction --- .../datadog/trace/civisibility/ci/GithubActionsInfo.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/ci/GithubActionsInfo.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/ci/GithubActionsInfo.java index 41a99629090..7077ad17bbf 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/ci/GithubActionsInfo.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/ci/GithubActionsInfo.java @@ -123,9 +123,9 @@ public PullRequestInfo buildPullRequestInfo() { baseSha = (String) base.get("sha"); } - Object number = pullRequest.get("number"); - if (number instanceof Number) { - prNumber = String.valueOf(((Number) number).intValue()); + Double number = (Double) pullRequest.get("number"); + if (number != null) { + prNumber = String.valueOf(number.intValue()); } } From 0f0698d63990cdd0fac81874af463056c45413bd Mon Sep 17 00:00:00 2001 From: Daniel Mohedano Date: Wed, 23 Jul 2025 10:34:15 +0200 Subject: [PATCH 09/13] refactor: rename and refactor to clarify the unshallow execution paths --- .../civisibility/git/tree/GitClient.java | 2 +- .../git/tree/GitRepoUnshallow.java | 23 ++++++++++++------- .../civisibility/git/tree/NoOpGitClient.java | 2 +- .../civisibility/git/tree/ShellGitClient.java | 16 ++++++++----- 4 files changed, 27 insertions(+), 16 deletions(-) diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/GitClient.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/GitClient.java index b00fe394f84..8d9b93fb351 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/GitClient.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/GitClient.java @@ -17,7 +17,7 @@ public interface GitClient { boolean isShallow() throws IOException, TimeoutException, InterruptedException; - void unshallow(@Nullable String remoteCommitReference, boolean onlyFetchCommit) + void unshallow(@Nullable String remoteCommitReference, boolean shallowUntilCommit) throws IOException, TimeoutException, InterruptedException; @Nullable diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/GitRepoUnshallow.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/GitRepoUnshallow.java index c521552bba8..b48bbaf5f13 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/GitRepoUnshallow.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/GitRepoUnshallow.java @@ -20,14 +20,27 @@ public GitRepoUnshallow(Config config, GitClient gitClient) { this.gitClient = gitClient; } - public synchronized boolean unshallow(@Nullable String commit) + /** + * Unshallows git repo up to a specific boundary commit, if provided, or up to the time limit + * configured in the git client if not. + * + * @param boundaryCommitSha used as boundary for the unshallowing if provided. + * @return false if unshallowing is not enabled or unnecessary, true otherwise + */ + public synchronized boolean unshallow(@Nullable String boundaryCommitSha) throws IOException, InterruptedException, TimeoutException { if (!config.isCiVisibilityGitUnshallowEnabled() || !gitClient.isShallow()) { return false; } long unshallowStart = System.currentTimeMillis(); - if (commit == null) { + if (boundaryCommitSha != null) { + try { + gitClient.unshallow(boundaryCommitSha, true); + } catch (ShellCommandExecutor.ShellCommandFailedException e) { + LOGGER.debug("Could not unshallow to specific boundary {}", boundaryCommitSha, e); + } + } else { try { gitClient.unshallow(GitClient.HEAD, false); } catch (ShellCommandExecutor.ShellCommandFailedException e) { @@ -45,12 +58,6 @@ public synchronized boolean unshallow(@Nullable String commit) e); gitClient.unshallow(null, false); } - } else { - try { - gitClient.unshallow(commit, true); - } catch (ShellCommandExecutor.ShellCommandFailedException e) { - LOGGER.debug("Could not unshallow specific commit {}", commit, e); - } } LOGGER.debug("Repository unshallowing took {} ms", System.currentTimeMillis() - unshallowStart); return true; diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/NoOpGitClient.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/NoOpGitClient.java index ce8cc208e5e..4ef87239884 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/NoOpGitClient.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/NoOpGitClient.java @@ -21,7 +21,7 @@ public boolean isShallow() { } @Override - public void unshallow(@Nullable String remoteCommitReference, boolean onlyFetchCommit) { + public void unshallow(@Nullable String remoteCommitReference, boolean shallowUntilCommit) { // no op } diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/ShellGitClient.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/ShellGitClient.java index e977818b210..c33e5659752 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/ShellGitClient.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/ShellGitClient.java @@ -129,14 +129,15 @@ public String getUpstreamBranchSha() throws IOException, TimeoutException, Inter * * @param remoteCommitReference The commit to fetch from the remote repository, so local repo will * be updated with this commit and its ancestors. If {@code null}, everything will be fetched. - * @param onlyFetchCommit Only fetch the specific commit provided. + * @param shallowUntilCommit Only unshallow up until the commit provided, or use the time limit + * configured if not. Ignored if no reference is provided. * @throws IOException If an error was encountered while writing command input or reading output * @throws TimeoutException If timeout was reached while waiting for Git command to finish * @throws InterruptedException If current thread was interrupted while waiting for Git command to * finish */ @Override - public void unshallow(@Nullable String remoteCommitReference, boolean onlyFetchCommit) + public void unshallow(@Nullable String remoteCommitReference, boolean shallowUntilCommit) throws IOException, TimeoutException, InterruptedException { executeCommand( Command.UNSHALLOW, @@ -144,9 +145,12 @@ public void unshallow(@Nullable String remoteCommitReference, boolean onlyFetchC String remote = getRemoteName(); // refetch data from the server for the given period of time - if (remoteCommitReference != null && GitUtils.isValidRef(remoteCommitReference)) { - String onlyFetchCommitArg = - onlyFetchCommit + if (remoteCommitReference != null) { + if (!GitUtils.isValidRef(remoteCommitReference)) { + return (Void) null; + } + String boundaryArg = + shallowUntilCommit ? "--no-write-fetch-head" : String.format("--shallow-since='%s'", latestCommitsSince); String commitSha = getSha(remoteCommitReference); @@ -157,7 +161,7 @@ public void unshallow(@Nullable String remoteCommitReference, boolean onlyFetchC "--update-shallow", "--filter=blob:none", "--recurse-submodules=no", - onlyFetchCommitArg, + boundaryArg, remote, commitSha); } else { From 9c4c478f74eee5ba54fd23695ade01804021fd8e Mon Sep 17 00:00:00 2001 From: Daniel Mohedano Date: Wed, 23 Jul 2025 11:10:57 +0200 Subject: [PATCH 10/13] feat: add check for presence of commit before unshallowing --- .../civisibility/git/tree/GitClient.java | 5 ++- .../git/tree/GitRepoUnshallow.java | 7 +++- .../civisibility/git/tree/NoOpGitClient.java | 7 +++- .../civisibility/git/tree/ShellGitClient.java | 38 +++++++++++++++++-- .../git/tree/GitClientTest.groovy | 36 ++++++++++++++---- 5 files changed, 79 insertions(+), 14 deletions(-) diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/GitClient.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/GitClient.java index 8d9b93fb351..570d59746fd 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/GitClient.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/GitClient.java @@ -17,7 +17,10 @@ public interface GitClient { boolean isShallow() throws IOException, TimeoutException, InterruptedException; - void unshallow(@Nullable String remoteCommitReference, boolean shallowUntilCommit) + boolean isCommitPresent(String commitReference) + throws IOException, TimeoutException, InterruptedException; + + void unshallow(@Nullable String remoteCommitReference, boolean unshallowUntilCommit) throws IOException, TimeoutException, InterruptedException; @Nullable diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/GitRepoUnshallow.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/GitRepoUnshallow.java index b48bbaf5f13..ccdfaf30fc6 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/GitRepoUnshallow.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/GitRepoUnshallow.java @@ -22,14 +22,17 @@ public GitRepoUnshallow(Config config, GitClient gitClient) { /** * Unshallows git repo up to a specific boundary commit, if provided, or up to the time limit - * configured in the git client if not. + * configured in the git client if not. Won't perform an unshallow action when a boundary is + * provided and the object is already present, or if the repository is already unshallowed. * * @param boundaryCommitSha used as boundary for the unshallowing if provided. * @return false if unshallowing is not enabled or unnecessary, true otherwise */ public synchronized boolean unshallow(@Nullable String boundaryCommitSha) throws IOException, InterruptedException, TimeoutException { - if (!config.isCiVisibilityGitUnshallowEnabled() || !gitClient.isShallow()) { + if (!config.isCiVisibilityGitUnshallowEnabled() + || (boundaryCommitSha != null && gitClient.isCommitPresent(boundaryCommitSha)) + || !gitClient.isShallow()) { return false; } diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/NoOpGitClient.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/NoOpGitClient.java index 4ef87239884..73ee7f7cde9 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/NoOpGitClient.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/NoOpGitClient.java @@ -21,7 +21,12 @@ public boolean isShallow() { } @Override - public void unshallow(@Nullable String remoteCommitReference, boolean shallowUntilCommit) { + public boolean isCommitPresent(String commitReference) { + return false; + } + + @Override + public void unshallow(@Nullable String remoteCommitReference, boolean unshallowUntilCommit) { // no op } diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/ShellGitClient.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/ShellGitClient.java index c33e5659752..5c64825e326 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/ShellGitClient.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/ShellGitClient.java @@ -99,6 +99,38 @@ public boolean isShallow() throws IOException, TimeoutException, InterruptedExce }); } + /** + * Checks whether the provided reference object is present or not. + * + * @param commitReference to check presence of + * @return {@code true} if object is present, {@code false} otherwise + * @throws IOException If an error was encountered while writing command input or reading output + * @throws TimeoutException If timeout was reached while waiting for Git command to finish + * @throws InterruptedException If current thread was interrupted while waiting for Git command to + */ + @Override + public boolean isCommitPresent(String commitReference) + throws IOException, TimeoutException, InterruptedException { + if (!GitUtils.isValidRef(commitReference)) { + return false; + } + return executeCommand( + Command.OTHER, + () -> { + try { + commandExecutor.executeCommand( + ShellCommandExecutor.OutputParser.IGNORE, + "git", + "cat-file", + "-e", + commitReference + "^{commit}"); + return true; + } catch (ShellCommandExecutor.ShellCommandFailedException ignored) { + return false; + } + }); + } + /** * Returns the SHA of the head commit of the upstream (remote tracking) branch for the currently * checked-out local branch. If the local branch is not tracking any remote branches, a {@link @@ -129,7 +161,7 @@ public String getUpstreamBranchSha() throws IOException, TimeoutException, Inter * * @param remoteCommitReference The commit to fetch from the remote repository, so local repo will * be updated with this commit and its ancestors. If {@code null}, everything will be fetched. - * @param shallowUntilCommit Only unshallow up until the commit provided, or use the time limit + * @param unshallowUntilCommit Only unshallow up until the commit provided, or use the time limit * configured if not. Ignored if no reference is provided. * @throws IOException If an error was encountered while writing command input or reading output * @throws TimeoutException If timeout was reached while waiting for Git command to finish @@ -137,7 +169,7 @@ public String getUpstreamBranchSha() throws IOException, TimeoutException, Inter * finish */ @Override - public void unshallow(@Nullable String remoteCommitReference, boolean shallowUntilCommit) + public void unshallow(@Nullable String remoteCommitReference, boolean unshallowUntilCommit) throws IOException, TimeoutException, InterruptedException { executeCommand( Command.UNSHALLOW, @@ -150,7 +182,7 @@ public void unshallow(@Nullable String remoteCommitReference, boolean shallowUnt return (Void) null; } String boundaryArg = - shallowUntilCommit + unshallowUntilCommit ? "--no-write-fetch-head" : String.format("--shallow-since='%s'", latestCommitsSince); String commitSha = getSha(remoteCommitReference); diff --git a/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/git/tree/GitClientTest.groovy b/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/git/tree/GitClientTest.groovy index ed4641fc2c3..d7bdd6b4e96 100644 --- a/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/git/tree/GitClientTest.groovy +++ b/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/git/tree/GitClientTest.groovy @@ -70,7 +70,7 @@ class GitClientTest extends Specification { upstreamBranch == "98b944cc44f18bfb78e3021de2999cdcda8efdf6" } - def "test unshallow: sha-#remoteSha fetchOnlyCommit-#fetchOnlyCommit"() { + def "test unshallow: sha-#remoteSha"() { given: givenGitRepo("ci/git/shallow/git") @@ -84,7 +84,7 @@ class GitClientTest extends Specification { commits.size() == 1 when: - gitClient.unshallow(remoteSha, fetchOnlyCommit) + gitClient.unshallow(remoteSha, false) shallow = gitClient.isShallow() commits = gitClient.getLatestCommits() @@ -93,11 +93,33 @@ class GitClientTest extends Specification { commits.size() == numCommits where: - remoteSha | fetchOnlyCommit | isShallow | numCommits - GitClient.HEAD | false | false | 10 - null | false | false | 10 - "f4377e97f10c2d58696192b170b2fef2a8464b04" | true | true | 1 - null | true | false | 10 + remoteSha | isShallow | numCommits + GitClient.HEAD | false | 10 + null | false | 10 + } + + def "test unshallow using commit as boundary"() { + given: + givenGitRepo("ci/git/shallow/git") + + when: + def commit = "f4377e97f10c2d58696192b170b2fef2a8464b04" + def gitClient = givenGitClient() + def shallow = gitClient.isShallow() + def isPresent = gitClient.isCommitPresent(commit) + + then: + shallow + !isPresent + + when: + gitClient.unshallow(commit, true) + shallow = gitClient.isShallow() + isPresent = gitClient.isCommitPresent(commit) + + then: + shallow + isPresent } def "test get git folder"() { From ef249c8921427cebb83ca17dd419ecf1f1c92e06 Mon Sep 17 00:00:00 2001 From: Daniel Mohedano Date: Wed, 23 Jul 2025 11:59:56 +0200 Subject: [PATCH 11/13] refactor: separate fetching and unshallowing functionality --- .../CiVisibilityRepoServices.java | 4 +- .../config/ExecutionSettingsFactoryImpl.java | 2 +- .../civisibility/git/tree/GitClient.java | 5 +- .../git/tree/GitDataUploaderImpl.java | 4 +- .../git/tree/GitRepoUnshallow.java | 53 ++++++------------- .../civisibility/git/tree/NoOpGitClient.java | 2 +- .../civisibility/git/tree/ShellGitClient.java | 49 ++++++++++++----- .../git/tree/GitClientTest.groovy | 18 +++---- 8 files changed, 71 insertions(+), 66 deletions(-) diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/CiVisibilityRepoServices.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/CiVisibilityRepoServices.java index 3ff199d11fd..10b89ef0e30 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/CiVisibilityRepoServices.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/CiVisibilityRepoServices.java @@ -130,7 +130,9 @@ static PullRequestInfo buildPullRequestInfo( if (Strings.isNotBlank(headSha)) { // if head sha present try to populate author, committer and message info through git client try { - gitRepoUnshallow.unshallow(headSha); + if (!gitClient.isCommitPresent(headSha)) { + gitClient.fetchCommit(headSha); + } CommitInfo commitInfo = gitClient.getCommitInfo(headSha); return PullRequestInfo.merge(ciInfo, new PullRequestInfo(null, null, commitInfo, null)); } catch (Exception ignored) { diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/config/ExecutionSettingsFactoryImpl.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/config/ExecutionSettingsFactoryImpl.java index 13adf44fd3a..4382931defc 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/config/ExecutionSettingsFactoryImpl.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/config/ExecutionSettingsFactoryImpl.java @@ -421,7 +421,7 @@ private Diff getPullRequestDiff(boolean impactedTestsDetectionEnabled, String de try { if (repositoryRoot != null) { // ensure repo is not shallow before attempting to get git diff - gitRepoUnshallow.unshallow(null); + gitRepoUnshallow.unshallow(); String baseCommitSha = pullRequestInfo.getPullRequestBaseBranchSha(); if (baseCommitSha == null) { diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/GitClient.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/GitClient.java index 570d59746fd..d407bde4e66 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/GitClient.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/GitClient.java @@ -20,7 +20,10 @@ public interface GitClient { boolean isCommitPresent(String commitReference) throws IOException, TimeoutException, InterruptedException; - void unshallow(@Nullable String remoteCommitReference, boolean unshallowUntilCommit) + void unshallow(@Nullable String remoteCommitReference) + throws IOException, TimeoutException, InterruptedException; + + void fetchCommit(String remoteCommitReference) throws IOException, TimeoutException, InterruptedException; @Nullable diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/GitDataUploaderImpl.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/GitDataUploaderImpl.java index f3485e60774..5cbc46a51c9 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/GitDataUploaderImpl.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/GitDataUploaderImpl.java @@ -91,7 +91,7 @@ private void uploadGitData() { LOGGER.debug("Starting git data upload, {}", gitClient); if (!config.isCiVisibilityGitUnshallowDefer()) { - gitRepoUnshallow.unshallow(null); + gitRepoUnshallow.unshallow(); } GitInfo gitInfo = gitInfoProvider.getGitInfo(repoRoot); @@ -112,7 +112,7 @@ private void uploadGitData() { return; } - if (config.isCiVisibilityGitUnshallowDefer() && gitRepoUnshallow.unshallow(null)) { + if (config.isCiVisibilityGitUnshallowDefer() && gitRepoUnshallow.unshallow()) { latestCommits = gitClient.getLatestCommits(); commitsToSkip = gitDataApi.searchCommits(remoteUrl, latestCommits); } diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/GitRepoUnshallow.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/GitRepoUnshallow.java index ccdfaf30fc6..489e2a4d377 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/GitRepoUnshallow.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/GitRepoUnshallow.java @@ -4,7 +4,6 @@ import datadog.trace.civisibility.utils.ShellCommandExecutor; import java.io.IOException; import java.util.concurrent.TimeoutException; -import javax.annotation.Nullable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -20,47 +19,29 @@ public GitRepoUnshallow(Config config, GitClient gitClient) { this.gitClient = gitClient; } - /** - * Unshallows git repo up to a specific boundary commit, if provided, or up to the time limit - * configured in the git client if not. Won't perform an unshallow action when a boundary is - * provided and the object is already present, or if the repository is already unshallowed. - * - * @param boundaryCommitSha used as boundary for the unshallowing if provided. - * @return false if unshallowing is not enabled or unnecessary, true otherwise - */ - public synchronized boolean unshallow(@Nullable String boundaryCommitSha) + public synchronized boolean unshallow() throws IOException, InterruptedException, TimeoutException { - if (!config.isCiVisibilityGitUnshallowEnabled() - || (boundaryCommitSha != null && gitClient.isCommitPresent(boundaryCommitSha)) - || !gitClient.isShallow()) { + if (!config.isCiVisibilityGitUnshallowEnabled() || !gitClient.isShallow()) { return false; } long unshallowStart = System.currentTimeMillis(); - if (boundaryCommitSha != null) { - try { - gitClient.unshallow(boundaryCommitSha, true); - } catch (ShellCommandExecutor.ShellCommandFailedException e) { - LOGGER.debug("Could not unshallow to specific boundary {}", boundaryCommitSha, e); - } - } else { - try { - gitClient.unshallow(GitClient.HEAD, false); - } 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 { + 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, false); - } 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, false); - } + 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; diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/NoOpGitClient.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/NoOpGitClient.java index 73ee7f7cde9..7aab71e758f 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/NoOpGitClient.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/NoOpGitClient.java @@ -26,7 +26,7 @@ public boolean isCommitPresent(String commitReference) { } @Override - public void unshallow(@Nullable String remoteCommitReference, boolean unshallowUntilCommit) { + public void unshallow(@Nullable String remoteCommitReference) { // no op } diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/ShellGitClient.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/ShellGitClient.java index 5c64825e326..2d50bf7ba7b 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/ShellGitClient.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/ShellGitClient.java @@ -161,15 +161,13 @@ public String getUpstreamBranchSha() throws IOException, TimeoutException, Inter * * @param remoteCommitReference The commit to fetch from the remote repository, so local repo will * be updated with this commit and its ancestors. If {@code null}, everything will be fetched. - * @param unshallowUntilCommit Only unshallow up until the commit provided, or use the time limit - * configured if not. Ignored if no reference is provided. * @throws IOException If an error was encountered while writing command input or reading output * @throws TimeoutException If timeout was reached while waiting for Git command to finish * @throws InterruptedException If current thread was interrupted while waiting for Git command to * finish */ @Override - public void unshallow(@Nullable String remoteCommitReference, boolean unshallowUntilCommit) + public void unshallow(@Nullable String remoteCommitReference) throws IOException, TimeoutException, InterruptedException { executeCommand( Command.UNSHALLOW, @@ -177,14 +175,7 @@ public void unshallow(@Nullable String remoteCommitReference, boolean unshallowU String remote = getRemoteName(); // refetch data from the server for the given period of time - if (remoteCommitReference != null) { - if (!GitUtils.isValidRef(remoteCommitReference)) { - return (Void) null; - } - String boundaryArg = - unshallowUntilCommit - ? "--no-write-fetch-head" - : String.format("--shallow-since='%s'", latestCommitsSince); + if (remoteCommitReference != null && GitUtils.isValidRef(remoteCommitReference)) { String commitSha = getSha(remoteCommitReference); commandExecutor.executeCommand( ShellCommandExecutor.OutputParser.IGNORE, @@ -193,7 +184,7 @@ public void unshallow(@Nullable String remoteCommitReference, boolean unshallowU "--update-shallow", "--filter=blob:none", "--recurse-submodules=no", - boundaryArg, + String.format("--shallow-since='%s'", latestCommitsSince), remote, commitSha); } else { @@ -212,6 +203,40 @@ public void unshallow(@Nullable String remoteCommitReference, boolean unshallowU }); } + /** + * Fetches provided commit object from the server. + * + * @param commitReference Commit to fetch from the remote repository + * @throws IOException If an error was encountered while writing command input or reading output + * @throws TimeoutException If timeout was reached while waiting for Git command to finish + * @throws InterruptedException If current thread was interrupted while waiting for Git command to + */ + @Override + public void fetchCommit(String commitReference) + throws IOException, TimeoutException, InterruptedException { + if (!GitUtils.isValidRef(commitReference)) { + return; + } + executeCommand( + Command.OTHER, + () -> { + String remote = getRemoteName(); + + commandExecutor.executeCommand( + ShellCommandExecutor.OutputParser.IGNORE, + "git", + "fetch", + "--update-shallow", + "--filter=blob:none", + "--recurse-submodules=no", + "--no-write-fetch-head", + remote, + commitReference); + + return (Void) null; + }); + } + /** * Returns the absolute path of the .git directory. * diff --git a/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/git/tree/GitClientTest.groovy b/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/git/tree/GitClientTest.groovy index d7bdd6b4e96..ad9cbb8989b 100644 --- a/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/git/tree/GitClientTest.groovy +++ b/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/git/tree/GitClientTest.groovy @@ -84,41 +84,35 @@ class GitClientTest extends Specification { commits.size() == 1 when: - gitClient.unshallow(remoteSha, false) + gitClient.unshallow(remoteSha) shallow = gitClient.isShallow() commits = gitClient.getLatestCommits() then: - shallow == isShallow - commits.size() == numCommits + !shallow + commits.size() == 10 where: - remoteSha | isShallow | numCommits - GitClient.HEAD | false | 10 - null | false | 10 + remoteSha << [GitClient.HEAD, null] } - def "test unshallow using commit as boundary"() { + def "test commit fetch"() { given: givenGitRepo("ci/git/shallow/git") when: def commit = "f4377e97f10c2d58696192b170b2fef2a8464b04" def gitClient = givenGitClient() - def shallow = gitClient.isShallow() def isPresent = gitClient.isCommitPresent(commit) then: - shallow !isPresent when: - gitClient.unshallow(commit, true) - shallow = gitClient.isShallow() + gitClient.fetchCommit(commit) isPresent = gitClient.isCommitPresent(commit) then: - shallow isPresent } From fa81bea1fdb2a255d0ae5423846c274073c45ad9 Mon Sep 17 00:00:00 2001 From: Daniel Mohedano Date: Wed, 23 Jul 2025 12:05:21 +0200 Subject: [PATCH 12/13] renaming --- .../trace/civisibility/git/tree/NoOpGitClient.java | 5 +++++ .../trace/civisibility/git/tree/ShellGitClient.java | 9 ++++----- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/NoOpGitClient.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/NoOpGitClient.java index 7aab71e758f..ea8fc720b2e 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/NoOpGitClient.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/NoOpGitClient.java @@ -30,6 +30,11 @@ public void unshallow(@Nullable String remoteCommitReference) { // no op } + @Override + public void fetchCommit(String remoteCommitReference) { + // no op + } + @Nullable @Override public String getGitFolder() { diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/ShellGitClient.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/ShellGitClient.java index 2d50bf7ba7b..f8ca67dd188 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/ShellGitClient.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/ShellGitClient.java @@ -206,22 +206,21 @@ public void unshallow(@Nullable String remoteCommitReference) /** * Fetches provided commit object from the server. * - * @param commitReference Commit to fetch from the remote repository + * @param remoteCommitReference Commit to fetch from the remote repository * @throws IOException If an error was encountered while writing command input or reading output * @throws TimeoutException If timeout was reached while waiting for Git command to finish * @throws InterruptedException If current thread was interrupted while waiting for Git command to */ @Override - public void fetchCommit(String commitReference) + public void fetchCommit(String remoteCommitReference) throws IOException, TimeoutException, InterruptedException { - if (!GitUtils.isValidRef(commitReference)) { + if (!GitUtils.isValidRef(remoteCommitReference)) { return; } executeCommand( Command.OTHER, () -> { String remote = getRemoteName(); - commandExecutor.executeCommand( ShellCommandExecutor.OutputParser.IGNORE, "git", @@ -231,7 +230,7 @@ public void fetchCommit(String commitReference) "--recurse-submodules=no", "--no-write-fetch-head", remote, - commitReference); + remoteCommitReference); return (Void) null; }); From 004fc73ce9c23c9445f0638a2a19517bdcc76e4b Mon Sep 17 00:00:00 2001 From: Daniel Mohedano Date: Wed, 23 Jul 2025 15:37:07 +0200 Subject: [PATCH 13/13] refactor: move commitFetch and isCommitPresent to private methods --- .../CiVisibilityRepoServices.java | 5 +- .../git/GitClientGitInfoBuilder.java | 2 +- .../civisibility/git/tree/GitClient.java | 8 +- .../civisibility/git/tree/NoOpGitClient.java | 12 +- .../civisibility/git/tree/ShellGitClient.java | 148 +++++++++--------- .../CiVisibilityRepoServicesTest.groovy | 2 +- .../git/tree/GitClientTest.groovy | 49 ++---- .../civisibility/telemetry/tag/Command.java | 1 + 8 files changed, 95 insertions(+), 132 deletions(-) diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/CiVisibilityRepoServices.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/CiVisibilityRepoServices.java index 10b89ef0e30..ec6ffb74885 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/CiVisibilityRepoServices.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/CiVisibilityRepoServices.java @@ -130,10 +130,7 @@ static PullRequestInfo buildPullRequestInfo( if (Strings.isNotBlank(headSha)) { // if head sha present try to populate author, committer and message info through git client try { - if (!gitClient.isCommitPresent(headSha)) { - gitClient.fetchCommit(headSha); - } - CommitInfo commitInfo = gitClient.getCommitInfo(headSha); + CommitInfo commitInfo = gitClient.getCommitInfo(headSha, true); return PullRequestInfo.merge(ciInfo, new PullRequestInfo(null, null, commitInfo, null)); } catch (Exception ignored) { } diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/GitClientGitInfoBuilder.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/GitClientGitInfoBuilder.java index d10a2cc27f8..a12646a8f67 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/GitClientGitInfoBuilder.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/GitClientGitInfoBuilder.java @@ -38,7 +38,7 @@ public GitInfo build(@Nullable String repositoryPath) { List tags = gitClient.getTags(GitClient.HEAD); String tag = !tags.isEmpty() ? tags.iterator().next() : null; - CommitInfo commitInfo = gitClient.getCommitInfo(GitClient.HEAD); + CommitInfo commitInfo = gitClient.getCommitInfo(GitClient.HEAD, false); return new GitInfo(remoteUrl, branch, tag, commitInfo); } catch (Exception e) { diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/GitClient.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/GitClient.java index d407bde4e66..140aa3999ab 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/GitClient.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/GitClient.java @@ -17,15 +17,9 @@ public interface GitClient { boolean isShallow() throws IOException, TimeoutException, InterruptedException; - boolean isCommitPresent(String commitReference) - throws IOException, TimeoutException, InterruptedException; - void unshallow(@Nullable String remoteCommitReference) throws IOException, TimeoutException, InterruptedException; - void fetchCommit(String remoteCommitReference) - throws IOException, TimeoutException, InterruptedException; - @Nullable String getGitFolder() throws IOException, TimeoutException, InterruptedException; @@ -48,7 +42,7 @@ void fetchCommit(String remoteCommitReference) String getSha(String reference) throws IOException, TimeoutException, InterruptedException; @Nonnull - CommitInfo getCommitInfo(String commit) + CommitInfo getCommitInfo(String commit, boolean fetchIfNotPresent) throws IOException, TimeoutException, InterruptedException; @Nonnull diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/NoOpGitClient.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/NoOpGitClient.java index ea8fc720b2e..0a77fb72163 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/NoOpGitClient.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/NoOpGitClient.java @@ -20,21 +20,11 @@ public boolean isShallow() { return false; } - @Override - public boolean isCommitPresent(String commitReference) { - return false; - } - @Override public void unshallow(@Nullable String remoteCommitReference) { // no op } - @Override - public void fetchCommit(String remoteCommitReference) { - // no op - } - @Nullable @Override public String getGitFolder() { @@ -79,7 +69,7 @@ public String getSha(String reference) { @Nonnull @Override - public CommitInfo getCommitInfo(String commit) { + public CommitInfo getCommitInfo(String commit, boolean fetchIfNotPresent) { return CommitInfo.NOOP; } diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/ShellGitClient.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/ShellGitClient.java index f8ca67dd188..f353b05c0b9 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/ShellGitClient.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/ShellGitClient.java @@ -99,38 +99,6 @@ public boolean isShallow() throws IOException, TimeoutException, InterruptedExce }); } - /** - * Checks whether the provided reference object is present or not. - * - * @param commitReference to check presence of - * @return {@code true} if object is present, {@code false} otherwise - * @throws IOException If an error was encountered while writing command input or reading output - * @throws TimeoutException If timeout was reached while waiting for Git command to finish - * @throws InterruptedException If current thread was interrupted while waiting for Git command to - */ - @Override - public boolean isCommitPresent(String commitReference) - throws IOException, TimeoutException, InterruptedException { - if (!GitUtils.isValidRef(commitReference)) { - return false; - } - return executeCommand( - Command.OTHER, - () -> { - try { - commandExecutor.executeCommand( - ShellCommandExecutor.OutputParser.IGNORE, - "git", - "cat-file", - "-e", - commitReference + "^{commit}"); - return true; - } catch (ShellCommandExecutor.ShellCommandFailedException ignored) { - return false; - } - }); - } - /** * Returns the SHA of the head commit of the upstream (remote tracking) branch for the currently * checked-out local branch. If the local branch is not tracking any remote branches, a {@link @@ -203,39 +171,6 @@ public void unshallow(@Nullable String remoteCommitReference) }); } - /** - * Fetches provided commit object from the server. - * - * @param remoteCommitReference Commit to fetch from the remote repository - * @throws IOException If an error was encountered while writing command input or reading output - * @throws TimeoutException If timeout was reached while waiting for Git command to finish - * @throws InterruptedException If current thread was interrupted while waiting for Git command to - */ - @Override - public void fetchCommit(String remoteCommitReference) - throws IOException, TimeoutException, InterruptedException { - if (!GitUtils.isValidRef(remoteCommitReference)) { - return; - } - executeCommand( - Command.OTHER, - () -> { - String remote = getRemoteName(); - commandExecutor.executeCommand( - ShellCommandExecutor.OutputParser.IGNORE, - "git", - "fetch", - "--update-shallow", - "--filter=blob:none", - "--recurse-submodules=no", - "--no-write-fetch-head", - remote, - remoteCommitReference); - - return (Void) null; - }); - } - /** * Returns the absolute path of the .git directory. * @@ -378,10 +313,58 @@ public String getSha(String reference) .trim()); } + /** Checks whether the provided reference object is present or not. */ + private boolean isCommitPresent(String commitReference) + throws IOException, TimeoutException, InterruptedException { + if (GitUtils.isNotValidCommit(commitReference)) { + return false; + } + return executeCommand( + Command.OTHER, + () -> { + try { + commandExecutor.executeCommand( + ShellCommandExecutor.OutputParser.IGNORE, + "git", + "cat-file", + "-e", + commitReference + "^{commit}"); + return true; + } catch (ShellCommandExecutor.ShellCommandFailedException ignored) { + return false; + } + }); + } + + /** Fetches provided commit object from the server. */ + private void fetchCommit(String remoteCommitReference) + throws IOException, TimeoutException, InterruptedException { + if (GitUtils.isNotValidCommit(remoteCommitReference)) { + return; + } + executeCommand( + Command.FETCH_COMMIT, + () -> { + String remote = getRemoteName(); + commandExecutor.executeCommand( + ShellCommandExecutor.OutputParser.IGNORE, + "git", + "fetch", + "--filter=blob:none", + "--recurse-submodules=no", + "--no-write-fetch-head", + remote, + remoteCommitReference); + + return (Void) null; + }); + } + /** * Returns commit information for the provided commit * * @param commit Commit SHA or reference (HEAD, branch name, etc) to check + * @param fetchIfNotPresent If the commit should be fetched from server if not present locally * @return commit info (sha, author info, committer info, full message) * @throws IOException If an error was encountered while writing command input or reading output * @throws TimeoutException If timeout was reached while waiting for Git command to finish @@ -390,24 +373,37 @@ public String getSha(String reference) */ @Nonnull @Override - public CommitInfo getCommitInfo(String commit) + public CommitInfo getCommitInfo(String commit, boolean fetchIfNotPresent) throws IOException, InterruptedException, TimeoutException { if (GitUtils.isNotValidCommit(commit)) { return CommitInfo.NOOP; } + if (fetchIfNotPresent) { + boolean isPresent = isCommitPresent(commit); + if (!isPresent) { + fetchCommit(commit); + } + } return executeCommand( Command.OTHER, () -> { - String info = - commandExecutor - .executeCommand( - IOUtils::readFully, - "git", - "show", - commit, - "-s", - "--format=%H\",\"%an\",\"%ae\",\"%aI\",\"%cn\",\"%ce\",\"%cI\",\"%B") - .trim(); + String info = ""; + try { + info = + commandExecutor + .executeCommand( + IOUtils::readFully, + "git", + "show", + commit, + "-s", + "--format=%H\",\"%an\",\"%ae\",\"%aI\",\"%cn\",\"%ce\",\"%cI\",\"%B") + .trim(); + } catch (ShellCommandExecutor.ShellCommandFailedException e) { + LOGGER.error("Failed to fetch commit info", e); + return CommitInfo.NOOP; + } + String[] fields = COMMIT_INFO_SPLIT.split(info); if (fields.length < 8) { LOGGER.error("Could not parse commit info: {}", info); diff --git a/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/CiVisibilityRepoServicesTest.groovy b/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/CiVisibilityRepoServicesTest.groovy index c57b80830c6..a3ddb6f4f1e 100644 --- a/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/CiVisibilityRepoServicesTest.groovy +++ b/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/CiVisibilityRepoServicesTest.groovy @@ -59,7 +59,7 @@ class CiVisibilityRepoServicesTest extends Specification { def gitClient = Stub(GitClient) gitClient.getMergeBase("targetSha", "sourceSha") >> expectedInfo.getPullRequestBaseBranchSha() - gitClient.getCommitInfo("sourceSha") >> expectedInfo.getHeadCommit() + gitClient.getCommitInfo("sourceSha", true) >> expectedInfo.getHeadCommit() expect: CiVisibilityRepoServices.buildPullRequestInfo(config, environment, ciProviderInfo, gitClient, repoUnshallow) == expectedInfo diff --git a/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/git/tree/GitClientTest.groovy b/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/git/tree/GitClientTest.groovy index ad9cbb8989b..479c3ce922c 100644 --- a/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/git/tree/GitClientTest.groovy +++ b/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/git/tree/GitClientTest.groovy @@ -3,6 +3,7 @@ package datadog.trace.civisibility.git.tree import static datadog.trace.civisibility.TestUtils.lines import datadog.communication.util.IOUtils +import datadog.trace.api.git.CommitInfo import datadog.trace.civisibility.git.GitObject import datadog.trace.civisibility.git.pack.V2PackGitInfoExtractor import datadog.trace.civisibility.telemetry.CiVisibilityMetricCollectorImpl @@ -96,26 +97,6 @@ class GitClientTest extends Specification { remoteSha << [GitClient.HEAD, null] } - def "test commit fetch"() { - given: - givenGitRepo("ci/git/shallow/git") - - when: - def commit = "f4377e97f10c2d58696192b170b2fef2a8464b04" - def gitClient = givenGitClient() - def isPresent = gitClient.isCommitPresent(commit) - - then: - !isPresent - - when: - gitClient.fetchCommit(commit) - isPresent = gitClient.isCommitPresent(commit) - - then: - isPresent - } - def "test get git folder"() { given: givenGitRepo() @@ -176,26 +157,30 @@ class GitClientTest extends Specification { sha == "5b6f3a6dab5972d73a56dff737bd08d995255c08" } - def "test get commit info"() { + def "test get commit info with fetching"() { given: - givenGitRepo() + givenGitRepo("ci/git/shallow/git") when: + def commit = "f4377e97f10c2d58696192b170b2fef2a8464b04" def gitClient = givenGitClient() - def commitInfo = gitClient.getCommitInfo(GitClient.HEAD) + def commitInfo = gitClient.getCommitInfo(commit, false) + + then: + commitInfo == CommitInfo.NOOP + + when: + commitInfo = gitClient.getCommitInfo(commit, true) then: - commitInfo.sha == "5b6f3a6dab5972d73a56dff737bd08d995255c08" - commitInfo.author.name == "Tony Redondo" - commitInfo.author.email == "tony.redondo@datadoghq.com" - commitInfo.author.iso8601Date == "2021-02-26T19:32:13+01:00" + commitInfo.sha == commit + commitInfo.author.name == "sullis" + commitInfo.author.email == "github@seansullivan.com" + commitInfo.author.iso8601Date == "2023-05-30T07:07:35-07:00" commitInfo.committer.name == "GitHub" commitInfo.committer.email == "noreply@github.com" - commitInfo.committer.iso8601Date == "2021-02-26T19:32:13+01:00" - commitInfo.fullMessage == "Adding Git information to test spans (#1242)\n\n" + - "* Initial basic GitInfo implementation.\r\n\r\n" + - "* Adds Author, Committer and Message git parser.\r\n\r\n" + - "* Changes based on the review." + commitInfo.committer.iso8601Date == "2023-05-30T07:07:35-07:00" + commitInfo.fullMessage == "brotli4j 1.12.0 (#1592)" } def "test get latest commits"() { diff --git a/internal-api/src/main/java/datadog/trace/api/civisibility/telemetry/tag/Command.java b/internal-api/src/main/java/datadog/trace/api/civisibility/telemetry/tag/Command.java index 0413066d100..5ea133cb7e3 100644 --- a/internal-api/src/main/java/datadog/trace/api/civisibility/telemetry/tag/Command.java +++ b/internal-api/src/main/java/datadog/trace/api/civisibility/telemetry/tag/Command.java @@ -13,6 +13,7 @@ public enum Command implements TagValue { PACK_OBJECTS, DIFF, BASE_COMMIT_SHA, + FETCH_COMMIT, OTHER; private final String s;