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 7ebbe41c005..d3c0280a84b 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 @@ -35,7 +35,6 @@ import datadog.trace.civisibility.source.index.RepoIndexProvider; import datadog.trace.civisibility.source.index.RepoIndexSourcePathResolver; import datadog.trace.util.Strings; -import java.io.File; import java.nio.file.Path; import java.nio.file.Paths; import java.util.Map; @@ -72,7 +71,7 @@ public class CiVisibilityRepoServices { LOGGER.info("PR detected: {}", pullRequestInfo); } - repoRoot = appendSlashIfNeeded(getRepoRoot(ciInfo, services.gitClientFactory)); + repoRoot = getRepoRoot(ciInfo, services.gitClientFactory); moduleName = getModuleName(services.config, repoRoot, path); ciTags = new CITagsProvider().getCiTags(ciInfo, pullRequestInfo); @@ -126,7 +125,7 @@ private static PullRequestInfo buildPullRequestInfo( } private static String getRepoRoot(CIInfo ciInfo, GitClient.Factory gitClientFactory) { - String ciWorkspace = ciInfo.getNormalizedCiWorkspace(); + String ciWorkspace = ciInfo.getCiWorkspace(); if (Strings.isNotBlank(ciWorkspace)) { return ciWorkspace; @@ -146,14 +145,6 @@ private static String getRepoRoot(CIInfo ciInfo, GitClient.Factory gitClientFact } } - private static String appendSlashIfNeeded(String repoRoot) { - if (repoRoot != null && !repoRoot.endsWith(File.separator)) { - return repoRoot + File.separator; - } else { - return repoRoot; - } - } - static String getModuleName(Config config, @Nullable String repoRoot, Path path) { // if parent process is instrumented, it will provide build system's module name String parentModuleName = config.getCiVisibilityModuleName(); diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/ci/CIInfo.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/ci/CIInfo.java index 9fb2bbfb81f..a2d221197bc 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/ci/CIInfo.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/ci/CIInfo.java @@ -170,7 +170,7 @@ public CIInfo( this.ciPipelineNumber = ciPipelineNumber; this.ciPipelineUrl = ciPipelineUrl; this.ciJobUrl = ciJobUrl; - this.ciWorkspace = ciWorkspace; + this.ciWorkspace = sanitizeWorkspace(ciWorkspace); this.ciNodeName = ciNodeName; this.ciNodeLabels = ciNodeLabels; this.ciEnvVars = ciEnvVars; @@ -209,20 +209,18 @@ public String getCiJobUrl() { return ciJobUrl; } - /** - * @deprecated This method is here only to satisfy CI spec tests. Use {@link - * #getNormalizedCiWorkspace()} - */ - @Deprecated - public String getCiWorkspace() { - return ciWorkspace; + private String sanitizeWorkspace(String workspace) { + String realCiWorkspace = FileUtils.toRealPath(workspace); + return (realCiWorkspace == null + || !realCiWorkspace.endsWith(File.separator) + || realCiWorkspace.length() == 1) // root path "/" + ? realCiWorkspace + : (realCiWorkspace.substring(0, realCiWorkspace.length() - 1)); } - public String getNormalizedCiWorkspace() { - String realCiWorkspace = FileUtils.toRealPath(ciWorkspace); - return (realCiWorkspace == null || realCiWorkspace.endsWith(File.separator)) - ? realCiWorkspace - : (realCiWorkspace + File.separator); + /** @return Workspace path without the trailing separator */ + public String getCiWorkspace() { + return ciWorkspace; } public String getCiNodeName() { 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 a41bbb28af9..53cedefd702 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 @@ -22,7 +22,7 @@ public CITagsProvider() { } public Map getCiTags(CIInfo ciInfo, PullRequestInfo pullRequestInfo) { - String repoRoot = ciInfo.getNormalizedCiWorkspace(); + String repoRoot = ciInfo.getCiWorkspace(); GitInfo gitInfo = gitInfoProvider.getGitInfo(repoRoot); return new CITagsBuilder() diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/source/CompilerAidedSourcePathResolver.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/source/CompilerAidedSourcePathResolver.java index 73b51d78e70..c2aa8831780 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/source/CompilerAidedSourcePathResolver.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/source/CompilerAidedSourcePathResolver.java @@ -1,6 +1,7 @@ package datadog.trace.civisibility.source; import datadog.compiler.utils.CompilerUtils; +import java.io.File; import javax.annotation.Nonnull; import javax.annotation.Nullable; @@ -9,7 +10,7 @@ public class CompilerAidedSourcePathResolver implements SourcePathResolver { private final String repoRoot; public CompilerAidedSourcePathResolver(String repoRoot) { - this.repoRoot = repoRoot; + this.repoRoot = repoRoot.endsWith(File.separator) ? repoRoot : repoRoot + File.separator; } @Nullable diff --git a/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/ci/CIInfoTest.groovy b/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/ci/CIInfoTest.groovy new file mode 100644 index 00000000000..a8686c3483d --- /dev/null +++ b/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/ci/CIInfoTest.groovy @@ -0,0 +1,22 @@ +package datadog.trace.civisibility.ci + + +import spock.lang.Specification + +class CIInfoTest extends Specification { + + def "test ci workspace is correctly sanitized #iterationIndex"() { + def builder = CIInfo.builder(null) + builder.ciWorkspace(workspacePath) + def info = builder.build() + + info.ciWorkspace == sanitizedPath + + where: + workspacePath | sanitizedPath + null | null + "/" | "/" + "/repo/path" | "/repo/path" + "/repo/path/" | "/repo/path" + } +} diff --git a/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/codeowners/CodeownersProviderTest.groovy b/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/codeowners/CodeownersProviderTest.groovy index dde2cb02748..27f578276e0 100644 --- a/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/codeowners/CodeownersProviderTest.groovy +++ b/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/codeowners/CodeownersProviderTest.groovy @@ -8,7 +8,7 @@ import java.nio.file.Files class CodeownersProviderTest extends Specification { - private static final String REPO_ROOT = "/repo/root/" + private static final String REPO_ROOT = "/repo/root" def "test codeowners loading: #path"() { setup: @@ -30,10 +30,10 @@ class CodeownersProviderTest extends Specification { where: path << [ - REPO_ROOT + "CODEOWNERS", - REPO_ROOT + ".github/CODEOWNERS", - REPO_ROOT + ".gitlab/CODEOWNERS", - REPO_ROOT + "docs/CODEOWNERS" + REPO_ROOT + "/CODEOWNERS", + REPO_ROOT + "/.github/CODEOWNERS", + REPO_ROOT + "/.gitlab/CODEOWNERS", + REPO_ROOT + "/docs/CODEOWNERS" ] } } diff --git a/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/source/CompilerAidedSourcePathResolverTest.groovy b/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/source/CompilerAidedSourcePathResolverTest.groovy index 581d2d95671..77b563fd96c 100644 --- a/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/source/CompilerAidedSourcePathResolverTest.groovy +++ b/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/source/CompilerAidedSourcePathResolverTest.groovy @@ -5,7 +5,7 @@ import spock.lang.Specification class CompilerAidedSourcePathResolverTest extends Specification { - public static final String REPO_ROOT = "/repo/root/" + public static final String REPO_ROOT = "/repo/root" public static final String SOURCE_PATH_VALUE = "/repo/root/path/to/AClassWithSourceInfoInjected.java" public static final String SOURCE_PATH_OUTSIDE_REPO_VALUE = "/outside/path/to/AClassWithSourceInfoInjected.java" diff --git a/internal-api/src/main/java/datadog/trace/api/git/GitInfoProvider.java b/internal-api/src/main/java/datadog/trace/api/git/GitInfoProvider.java index 1eb767d33c3..9550a16236e 100644 --- a/internal-api/src/main/java/datadog/trace/api/git/GitInfoProvider.java +++ b/internal-api/src/main/java/datadog/trace/api/git/GitInfoProvider.java @@ -55,6 +55,7 @@ public GitInfo getGitInfo(@Nullable String repositoryPath) { if (repositoryPath == null) { repositoryPath = NULL_PATH_STRING; } + return gitInfoCache.computeIfAbsent(repositoryPath, this::buildGitInfo); }