Skip to content

Improve Git commit info building #9210

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 13 commits into from
Jul 24, 2025
Merged

Improve Git commit info building #9210

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
6b3cf19
feat: add single command to obtain full commit info
daniel-mohedano Jul 22, 2025
bef1740
feat: update soft unshallow to use --no-write-fetch
daniel-mohedano Jul 22, 2025
25dbcbb
chore: remove unused getAuthor..., getCommitter..., and getFullMessage
daniel-mohedano Jul 22, 2025
815a095
style: spotless
daniel-mohedano Jul 22, 2025
d4526f8
chore: renamed field to headCommit for clarity
daniel-mohedano Jul 22, 2025
50f0421
feat: add pr number parsing for github actions file
daniel-mohedano Jul 22, 2025
13a176f
fix: treat pr.number as `Number` for github actions parsing
daniel-mohedano Jul 22, 2025
72f9b91
fix: simplify pr.number extraction
daniel-mohedano Jul 23, 2025
0f0698d
refactor: rename and refactor to clarify the unshallow execution paths
daniel-mohedano Jul 23, 2025
9c4c478
feat: add check for presence of commit before unshallowing
daniel-mohedano Jul 23, 2025
ef249c8
refactor: separate fetching and unshallowing functionality
daniel-mohedano Jul 23, 2025
fa81bea
renaming
daniel-mohedano Jul 23, 2025
004fc73
refactor: move commitFetch and isCommitPresent to private methods
daniel-mohedano Jul 23, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 3 additions & 16 deletions ...gent-ci-visibility/src/main/java/datadog/trace/civisibility/CiVisibilityRepoServices.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -127,23 +126,11 @@ 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.getHeadCommit().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));
CommitInfo commitInfo =
new CommitInfo(headSha, authorInfo, committerInfo, gitClient.getFullMessage(headSha));
CommitInfo commitInfo = gitClient.getCommitInfo(headSha, true);
return PullRequestInfo.merge(ciInfo, new PullRequestInfo(null, null, commitInfo, null));
} catch (Exception ignored) {
}
Expand Down
17 changes: 8 additions & 9 deletions ...agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/ci/CITagsProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
8 changes: 7 additions & 1 deletion ...nt/agent-ci-visibility/src/main/java/datadog/trace/civisibility/ci/GithubActionsInfo.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ public PullRequestInfo buildPullRequestInfo() {

String baseSha = null;
String headSha = null;
String prNumber = null;

Map<String, Object> pullRequest = (Map<String, Object>) eventJson.get("pull_request");
if (pullRequest != null) {
Expand All @@ -121,9 +122,14 @@ public PullRequestInfo buildPullRequestInfo() {
if (base != null) {
baseSha = (String) base.get("sha");
}

Double number = (Double) pullRequest.get("number");
if (number != null) {
prNumber = String.valueOf(number.intValue());
}
}

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);
Expand Down
24 changes: 12 additions & 12 deletions ...gent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/ci/PullRequestInfo.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand All @@ -35,8 +35,8 @@ public String getPullRequestBaseBranchSha() {
}

@Nonnull
public CommitInfo getGitCommitHead() {
return gitCommitHead;
public CommitInfo getHeadCommit() {
return headCommit;
}

public String getPullRequestNumber() {
Expand All @@ -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);
}

Expand All @@ -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);
Expand All @@ -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
Expand All @@ -108,8 +108,8 @@ public String toString() {
+ ", baseSHA='"
+ pullRequestBaseBranchSha
+ '\''
+ ", commitSHA='"
+ gitCommitHead
+ ", headCommit='"
+ headCommit
+ '\''
+ ", prNumber='"
+ pullRequestNumber
Expand Down
13 changes: 6 additions & 7 deletions ...ibility/src/main/java/datadog/trace/civisibility/config/ExecutionSettingsFactoryImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -396,12 +396,12 @@ private Map<TestSetting, Map<String, Collection<TestFQN>>> 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());
Expand All @@ -421,16 +421,15 @@ 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();

String baseCommitSha = pullRequestInfo.getPullRequestBaseBranchSha();
if (baseCommitSha == null) {
baseCommitSha =
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;
}
Expand Down
16 changes: 1 addition & 15 deletions ...t-ci-visibility/src/main/java/datadog/trace/civisibility/git/GitClientGitInfoBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -39,20 +38,7 @@ public GitInfo build(@Nullable String repositoryPath) {
List<String> 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, false);
return new GitInfo(remoteUrl, branch, tag, commitInfo);

} catch (Exception e) {
Expand Down
25 changes: 4 additions & 21 deletions ...gent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/GitClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -16,7 +17,7 @@ public interface GitClient {

boolean isShallow() throws IOException, TimeoutException, InterruptedException;

void unshallow(@Nullable String remoteCommitReference, boolean parentOnly)
void unshallow(@Nullable String remoteCommitReference)
throws IOException, TimeoutException, InterruptedException;

@Nullable
Expand All @@ -40,28 +41,10 @@ void unshallow(@Nullable String remoteCommitReference, boolean parentOnly)
@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)
@Nonnull
CommitInfo getCommitInfo(String commit, boolean fetchIfNotPresent)
throws IOException, TimeoutException, InterruptedException;

@Nullable
String getCommitterDate(String commit) throws IOException, TimeoutException, InterruptedException;

@Nonnull
List<String> getLatestCommits() throws IOException, TimeoutException, InterruptedException;

Expand Down
4 changes: 2 additions & 2 deletions ...-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/GitDataUploaderImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ private void uploadGitData() {
LOGGER.debug("Starting git data upload, {}", gitClient);

if (!config.isCiVisibilityGitUnshallowDefer()) {
gitRepoUnshallow.unshallow(false);
gitRepoUnshallow.unshallow();
}

GitInfo gitInfo = gitInfoProvider.getGitInfo(repoRoot);
Expand All @@ -112,7 +112,7 @@ private void uploadGitData() {
return;
}

if (config.isCiVisibilityGitUnshallowDefer() && gitRepoUnshallow.unshallow(false)) {
if (config.isCiVisibilityGitUnshallowDefer() && gitRepoUnshallow.unshallow()) {
latestCommits = gitClient.getLatestCommits();
commitsToSkip = gitDataApi.searchCommits(remoteUrl, latestCommits);
}
Expand Down
8 changes: 4 additions & 4 deletions ...ent-ci-visibility/src/main/java/datadog/trace/civisibility/git/tree/GitRepoUnshallow.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,15 @@ public GitRepoUnshallow(Config config, GitClient gitClient) {
this.gitClient = gitClient;
}

public synchronized boolean unshallow(boolean parentOnly)
public synchronized boolean unshallow()
throws IOException, InterruptedException, TimeoutException {
if (!config.isCiVisibilityGitUnshallowEnabled() || !gitClient.isShallow()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if the gitClient.isShallow() needs to be updated to check if the provided commit SHA is present. Also, what happens if we request unshallow with a commit SHA, and then request "full unshallow" with a null SHA?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will look into updating the logic to check isShallow for a specific commit if the argument is provided. Could avoid additional git cmd calls that are unnecessary if it is called repeatedly. Regarding the two calls, AFAIK the second "full unshallow" would just be executed as regular, unshallowing to the shallow-since limit, given that the isShallow check would be true.

return false;
}

long unshallowStart = System.currentTimeMillis();
try {
gitClient.unshallow(GitClient.HEAD, parentOnly);
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",
Expand All @@ -36,12 +36,12 @@ public synchronized boolean unshallow(boolean parentOnly)

try {
String upstreamBranch = gitClient.getUpstreamBranchSha();
gitClient.unshallow(upstreamBranch, parentOnly);
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, parentOnly);
gitClient.unshallow(null);
}
LOGGER.debug("Repository unshallowing took {} ms", System.currentTimeMillis() - unshallowStart);
return true;
Expand Down
Loading