diff --git a/dd-java-agent/agent-ci-visibility/build.gradle b/dd-java-agent/agent-ci-visibility/build.gradle index 7811fa39140..323553b8c03 100644 --- a/dd-java-agent/agent-ci-visibility/build.gradle +++ b/dd-java-agent/agent-ci-visibility/build.gradle @@ -37,6 +37,7 @@ dependencies { testImplementation group: 'org.skyscreamer', name: 'jsonassert', version: '1.5.1' testImplementation group: 'org.freemarker', name: 'freemarker', version: '2.3.31' testImplementation group: 'org.msgpack', name: 'jackson-dataformat-msgpack', version: '0.9.6' + testImplementation libs.bundles.mockito } tasks.named("shadowJar", ShadowJar) { diff --git a/dd-java-agent/agent-ci-visibility/civisibility-instrumentation-test-fixtures/src/main/groovy/datadog/trace/civisibility/CiVisibilityInstrumentationTest.groovy b/dd-java-agent/agent-ci-visibility/civisibility-instrumentation-test-fixtures/src/main/groovy/datadog/trace/civisibility/CiVisibilityInstrumentationTest.groovy index c46a2b2781b..e71a8a5d8c9 100644 --- a/dd-java-agent/agent-ci-visibility/civisibility-instrumentation-test-fixtures/src/main/groovy/datadog/trace/civisibility/CiVisibilityInstrumentationTest.groovy +++ b/dd-java-agent/agent-ci-visibility/civisibility-instrumentation-test-fixtures/src/main/groovy/datadog/trace/civisibility/CiVisibilityInstrumentationTest.groovy @@ -128,9 +128,9 @@ abstract class CiVisibilityInstrumentationTest extends InstrumentationSpecificat def metricCollector = Stub(CiVisibilityMetricCollectorImpl) def sourcePathResolver = Stub(SourcePathResolver) - sourcePathResolver.getSourcePath(_ as Class) >> DUMMY_SOURCE_PATH - sourcePathResolver.getResourcePath(_ as String) >> { - String path -> path + sourcePathResolver.getSourcePaths(_ as Class) >> [DUMMY_SOURCE_PATH] + sourcePathResolver.getResourcePaths(_ as String) >> { + String path -> [path] } def codeowners = Stub(Codeowners) diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/coverage/ConcurrentCoverageStore.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/coverage/ConcurrentCoverageStore.java index 29b420f3b02..3a68c61f40d 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/coverage/ConcurrentCoverageStore.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/coverage/ConcurrentCoverageStore.java @@ -4,7 +4,6 @@ import datadog.trace.api.civisibility.coverage.CoverageProbes; import datadog.trace.api.civisibility.coverage.CoverageStore; import datadog.trace.api.civisibility.coverage.TestReport; -import datadog.trace.civisibility.source.SourceResolutionException; import java.util.Collection; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; @@ -37,18 +36,13 @@ private T create(Thread thread) { @Override public boolean report(DDTraceId testSessionId, Long testSuiteId, long testSpanId) { - try { - report = report(testSessionId, testSuiteId, testSpanId, probes.values()); - return report != null && report.isNotEmpty(); - } catch (SourceResolutionException e) { - return false; - } + report = report(testSessionId, testSuiteId, testSpanId, probes.values()); + return report != null && report.isNotEmpty(); } @Nullable protected abstract TestReport report( - DDTraceId testSessionId, Long testSuiteId, long testSpanId, Collection probes) - throws SourceResolutionException; + DDTraceId testSessionId, Long testSuiteId, long testSpanId, Collection probes); @Nullable @Override diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/coverage/file/FileCoverageStore.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/coverage/file/FileCoverageStore.java index 21d6d601926..6de1354e7c7 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/coverage/file/FileCoverageStore.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/coverage/file/FileCoverageStore.java @@ -11,7 +11,6 @@ import datadog.trace.api.civisibility.telemetry.tag.CoverageErrorType; import datadog.trace.civisibility.coverage.ConcurrentCoverageStore; import datadog.trace.civisibility.source.SourcePathResolver; -import datadog.trace.civisibility.source.SourceResolutionException; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; @@ -47,61 +46,54 @@ private FileCoverageStore( @Nullable @Override protected TestReport report( - DDTraceId testSessionId, Long testSuiteId, long testSpanId, Collection probes) - throws SourceResolutionException { - try { - Set> combinedClasses = Collections.newSetFromMap(new IdentityHashMap<>()); - Collection combinedNonCodeResources = new HashSet<>(); - - for (FileProbes probe : probes) { - combinedClasses.addAll(probe.getCoveredClasses()); - combinedNonCodeResources.addAll(probe.getNonCodeResources()); - } + DDTraceId testSessionId, Long testSuiteId, long testSpanId, Collection probes) { + Set> combinedClasses = Collections.newSetFromMap(new IdentityHashMap<>()); + Collection combinedNonCodeResources = new HashSet<>(); - if (combinedClasses.isEmpty() && combinedNonCodeResources.isEmpty()) { - return null; - } + for (FileProbes probe : probes) { + combinedClasses.addAll(probe.getCoveredClasses()); + combinedNonCodeResources.addAll(probe.getNonCodeResources()); + } - Set coveredPaths = set(combinedClasses.size() + combinedNonCodeResources.size()); - for (Class clazz : combinedClasses) { - String sourcePath = sourcePathResolver.getSourcePath(clazz); - if (sourcePath == null) { - log.debug( - "Skipping coverage reporting for {} because source path could not be determined", - clazz); - metrics.add(CiVisibilityCountMetric.CODE_COVERAGE_ERRORS, 1, CoverageErrorType.PATH); - continue; - } - coveredPaths.add(sourcePath); - } + if (combinedClasses.isEmpty() && combinedNonCodeResources.isEmpty()) { + return null; + } - for (String nonCodeResource : combinedNonCodeResources) { - String resourcePath = sourcePathResolver.getResourcePath(nonCodeResource); - if (resourcePath == null) { - log.debug( - "Skipping coverage reporting for {} because resource path could not be determined", - nonCodeResource); - metrics.add(CiVisibilityCountMetric.CODE_COVERAGE_ERRORS, 1, CoverageErrorType.PATH); - continue; - } - coveredPaths.add(resourcePath); + Set coveredPaths = set(combinedClasses.size() + combinedNonCodeResources.size()); + for (Class clazz : combinedClasses) { + Collection sourcePaths = sourcePathResolver.getSourcePaths(clazz); + if (sourcePaths.isEmpty()) { + log.debug( + "Skipping coverage reporting for {} because source path could not be determined", + clazz); + metrics.add(CiVisibilityCountMetric.CODE_COVERAGE_ERRORS, 1, CoverageErrorType.PATH); + continue; } + coveredPaths.addAll(sourcePaths); + } - List fileEntries = new ArrayList<>(coveredPaths.size()); - for (String path : coveredPaths) { - fileEntries.add(new TestReportFileEntry(path, null)); + for (String nonCodeResource : combinedNonCodeResources) { + Collection resourcePaths = sourcePathResolver.getResourcePaths(nonCodeResource); + if (resourcePaths.isEmpty()) { + log.debug( + "Skipping coverage reporting for {} because resource path could not be determined", + nonCodeResource); + metrics.add(CiVisibilityCountMetric.CODE_COVERAGE_ERRORS, 1, CoverageErrorType.PATH); + continue; } + coveredPaths.addAll(resourcePaths); + } - TestReport report = new TestReport(testSessionId, testSuiteId, testSpanId, fileEntries); - metrics.add( - CiVisibilityDistributionMetric.CODE_COVERAGE_FILES, - report.getTestReportFileEntries().size()); - return report; - - } catch (Exception e) { - metrics.add(CiVisibilityCountMetric.CODE_COVERAGE_ERRORS, 1); - throw e; + List fileEntries = new ArrayList<>(coveredPaths.size()); + for (String path : coveredPaths) { + fileEntries.add(new TestReportFileEntry(path, null)); } + + TestReport report = new TestReport(testSessionId, testSuiteId, testSpanId, fileEntries); + metrics.add( + CiVisibilityDistributionMetric.CODE_COVERAGE_FILES, + report.getTestReportFileEntries().size()); + return report; } private static Set set(int size) { diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/coverage/line/LineCoverageStore.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/coverage/line/LineCoverageStore.java index ae811c35de9..647f28ca181 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/coverage/line/LineCoverageStore.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/coverage/line/LineCoverageStore.java @@ -11,7 +11,6 @@ import datadog.trace.api.civisibility.telemetry.tag.CoverageErrorType; import datadog.trace.civisibility.coverage.ConcurrentCoverageStore; import datadog.trace.civisibility.source.SourcePathResolver; -import datadog.trace.civisibility.source.SourceResolutionException; import datadog.trace.civisibility.source.Utils; import java.io.InputStream; import java.util.ArrayList; @@ -53,88 +52,84 @@ private LineCoverageStore( @Nullable @Override protected TestReport report( - DDTraceId testSessionId, Long testSuiteId, long testSpanId, Collection probes) - throws SourceResolutionException { - try { - Map, ExecutionDataAdapter> combinedExecutionData = new IdentityHashMap<>(); - Collection combinedNonCodeResources = new HashSet<>(); - - for (LineProbes probe : probes) { - for (Map.Entry, ExecutionDataAdapter> e : probe.getExecutionData().entrySet()) { - combinedExecutionData.merge(e.getKey(), e.getValue(), ExecutionDataAdapter::merge); - } - combinedNonCodeResources.addAll(probe.getNonCodeResources()); - } + DDTraceId testSessionId, Long testSuiteId, long testSpanId, Collection probes) { + Map, ExecutionDataAdapter> combinedExecutionData = new IdentityHashMap<>(); + Collection combinedNonCodeResources = new HashSet<>(); - if (combinedExecutionData.isEmpty() && combinedNonCodeResources.isEmpty()) { - return null; + for (LineProbes probe : probes) { + for (Map.Entry, ExecutionDataAdapter> e : probe.getExecutionData().entrySet()) { + combinedExecutionData.merge(e.getKey(), e.getValue(), ExecutionDataAdapter::merge); } + combinedNonCodeResources.addAll(probe.getNonCodeResources()); + } - Map coveredLinesBySourcePath = new HashMap<>(); - for (Map.Entry, ExecutionDataAdapter> e : combinedExecutionData.entrySet()) { - ExecutionDataAdapter executionDataAdapter = e.getValue(); - String className = executionDataAdapter.getClassName(); - - Class clazz = e.getKey(); - String sourcePath = sourcePathResolver.getSourcePath(clazz); - if (sourcePath == null) { - log.debug( - "Skipping coverage reporting for {} because source path could not be determined", - className); - metrics.add(CiVisibilityCountMetric.CODE_COVERAGE_ERRORS, 1, CoverageErrorType.PATH); - continue; - } - - try (InputStream is = Utils.getClassStream(clazz)) { - BitSet coveredLines = - coveredLinesBySourcePath.computeIfAbsent(sourcePath, key -> new BitSet()); - ExecutionDataStore store = new ExecutionDataStore(); - store.put(executionDataAdapter.toExecutionData()); - - // TODO optimize this part to avoid parsing - // the same class multiple times for different test cases - Analyzer analyzer = new Analyzer(store, new SourceAnalyzer(coveredLines)); - analyzer.analyzeClass(is, null); - - } catch (Exception exception) { - log.debug( - "Skipping coverage reporting for {} ({}) because of error", - className, - sourcePath, - exception); - metrics.add(CiVisibilityCountMetric.CODE_COVERAGE_ERRORS, 1); - } - } + if (combinedExecutionData.isEmpty() && combinedNonCodeResources.isEmpty()) { + return null; + } - List fileEntries = new ArrayList<>(coveredLinesBySourcePath.size()); - for (Map.Entry e : coveredLinesBySourcePath.entrySet()) { - String sourcePath = e.getKey(); - BitSet coveredLines = e.getValue(); - fileEntries.add(new TestReportFileEntry(sourcePath, coveredLines)); + Map coveredLinesBySourcePath = new HashMap<>(); + for (Map.Entry, ExecutionDataAdapter> e : combinedExecutionData.entrySet()) { + ExecutionDataAdapter executionDataAdapter = e.getValue(); + String className = executionDataAdapter.getClassName(); + + Class clazz = e.getKey(); + Collection sourcePaths = sourcePathResolver.getSourcePaths(clazz); + if (sourcePaths.size() != 1) { + log.debug( + "Skipping coverage reporting for {} because source path could not be determined", + className); + metrics.add(CiVisibilityCountMetric.CODE_COVERAGE_ERRORS, 1, CoverageErrorType.PATH); + continue; } - - for (String nonCodeResource : combinedNonCodeResources) { - String resourcePath = sourcePathResolver.getResourcePath(nonCodeResource); - if (resourcePath == null) { - log.debug( - "Skipping coverage reporting for {} because resource path could not be determined", - nonCodeResource); - metrics.add(CiVisibilityCountMetric.CODE_COVERAGE_ERRORS, 1, CoverageErrorType.PATH); - continue; - } - fileEntries.add(new TestReportFileEntry(resourcePath, null)); + String sourcePath = sourcePaths.iterator().next(); + + try (InputStream is = Utils.getClassStream(clazz)) { + BitSet coveredLines = + coveredLinesBySourcePath.computeIfAbsent(sourcePath, key -> new BitSet()); + ExecutionDataStore store = new ExecutionDataStore(); + store.put(executionDataAdapter.toExecutionData()); + + // TODO optimize this part to avoid parsing + // the same class multiple times for different test cases + Analyzer analyzer = new Analyzer(store, new SourceAnalyzer(coveredLines)); + analyzer.analyzeClass(is, null); + + } catch (Exception exception) { + log.debug( + "Skipping coverage reporting for {} ({}) because of error", + className, + sourcePath, + exception); + metrics.add(CiVisibilityCountMetric.CODE_COVERAGE_ERRORS, 1); } + } - TestReport report = new TestReport(testSessionId, testSuiteId, testSpanId, fileEntries); - metrics.add( - CiVisibilityDistributionMetric.CODE_COVERAGE_FILES, - report.getTestReportFileEntries().size()); - return report; + List fileEntries = new ArrayList<>(coveredLinesBySourcePath.size()); + for (Map.Entry e : coveredLinesBySourcePath.entrySet()) { + String sourcePath = e.getKey(); + BitSet coveredLines = e.getValue(); + fileEntries.add(new TestReportFileEntry(sourcePath, coveredLines)); + } - } catch (Exception e) { - metrics.add(CiVisibilityCountMetric.CODE_COVERAGE_ERRORS, 1); - throw e; + for (String nonCodeResource : combinedNonCodeResources) { + Collection resourcePaths = sourcePathResolver.getResourcePaths(nonCodeResource); + if (resourcePaths.isEmpty()) { + log.debug( + "Skipping coverage reporting for {} because resource path could not be determined", + nonCodeResource); + metrics.add(CiVisibilityCountMetric.CODE_COVERAGE_ERRORS, 1, CoverageErrorType.PATH); + continue; + } + for (String resourcePath : resourcePaths) { + fileEntries.add(new TestReportFileEntry(resourcePath, null)); + } } + + TestReport report = new TestReport(testSessionId, testSuiteId, testSpanId, fileEntries); + metrics.add( + CiVisibilityDistributionMetric.CODE_COVERAGE_FILES, + report.getTestReportFileEntries().size()); + return report; } public static final class Factory implements CoverageStore.Factory { diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/coverage/report/JacocoCoverageProcessor.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/coverage/report/JacocoCoverageProcessor.java index db630801e84..0514dde7ab1 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/coverage/report/JacocoCoverageProcessor.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/coverage/report/JacocoCoverageProcessor.java @@ -9,7 +9,6 @@ import datadog.trace.civisibility.ipc.ModuleCoverageDataJacoco; import datadog.trace.civisibility.ipc.SignalResponse; import datadog.trace.civisibility.ipc.SignalType; -import datadog.trace.civisibility.source.SourceResolutionException; import datadog.trace.civisibility.source.index.RepoIndex; import datadog.trace.civisibility.source.index.RepoIndexProvider; import datadog.trace.util.Strings; @@ -355,19 +354,15 @@ private RepoIndexFileLocator(RepoIndex repoIndex, @Nonnull String repoRoot) { @Override protected InputStream getSourceStream(String path) throws IOException { - try { - String relativePath = repoIndex.getSourcePath(path); - if (relativePath == null) { - return null; - } - String absolutePath = - repoRoot + (!repoRoot.endsWith(File.separator) ? File.separator : "") + relativePath; - return new BufferedInputStream(Files.newInputStream(Paths.get(absolutePath))); - - } catch (SourceResolutionException e) { - LOGGER.debug("Could not resolve source for path {}", path, e); + Collection relativePaths = repoIndex.getSourcePaths(path); + if (relativePaths.size() != 1) { + LOGGER.debug("Could not resolve source for path {}", path); return null; } + String relativePath = relativePaths.iterator().next(); + String absolutePath = + repoRoot + (!repoRoot.endsWith(File.separator) ? File.separator : "") + relativePath; + return new BufferedInputStream(Files.newInputStream(Paths.get(absolutePath))); } } @@ -437,19 +432,16 @@ private long mergeAndUploadCoverageReport(IBundleCoverage coverageBundle) { String fileName = sourceFile.getName(); String pathRelativeToSourceRoot = (Strings.isNotBlank(packageName) ? packageName + "/" : "") + fileName; - String pathRelativeToIndexRoot; - try { - pathRelativeToIndexRoot = repoIndex.getSourcePath(pathRelativeToSourceRoot); - } catch (SourceResolutionException e) { - LOGGER.debug("Could not resolve source for path {}", pathRelativeToSourceRoot, e); - continue; - } + Collection pathsRelativeToIndexRoot = + repoIndex.getSourcePaths(pathRelativeToSourceRoot); - if (pathRelativeToIndexRoot == null) { + if (pathsRelativeToIndexRoot.size() != 1) { LOGGER.debug("Could not resolve source for path {}", pathRelativeToSourceRoot); continue; } + String pathRelativeToIndexRoot = pathsRelativeToIndexRoot.iterator().next(); + LinesCoverage linesCoverage = getLinesCoverage(sourceFile); // backendCoverageData contains data for all modules in the repo, // but coverageBundle bundle only has source files that are relevant for the given module, diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestImpl.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestImpl.java index 19a4650ea79..e1e87f67765 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestImpl.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestImpl.java @@ -44,7 +44,6 @@ import datadog.trace.civisibility.decorator.TestDecorator; import datadog.trace.civisibility.source.LinesResolver; import datadog.trace.civisibility.source.SourcePathResolver; -import datadog.trace.civisibility.source.SourceResolutionException; import datadog.trace.civisibility.test.ExecutionResults; import java.lang.reflect.Method; import java.util.Collection; @@ -179,17 +178,13 @@ private void populateSourceDataTags( return; } - String sourcePath; - try { - sourcePath = sourcePathResolver.getSourcePath(testClass); - if (sourcePath == null || sourcePath.isEmpty()) { - return; - } - } catch (SourceResolutionException e) { - log.debug("Could not populate source path for {}", testClass, e); + Collection sourcePaths = sourcePathResolver.getSourcePaths(testClass); + if (sourcePaths.size() != 1) { + log.debug("Could not populate source path for {}", testClass); return; } + String sourcePath = sourcePaths.iterator().next(); span.setTag(Tags.TEST_SOURCE_FILE, sourcePath); if (testMethod != null) { diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestSuiteImpl.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestSuiteImpl.java index 2b09f0cd0a7..ed853e9576e 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestSuiteImpl.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/domain/TestSuiteImpl.java @@ -23,7 +23,6 @@ import datadog.trace.civisibility.decorator.TestDecorator; import datadog.trace.civisibility.source.LinesResolver; import datadog.trace.civisibility.source.SourcePathResolver; -import datadog.trace.civisibility.source.SourceResolutionException; import datadog.trace.civisibility.test.ExecutionResults; import java.lang.reflect.Method; import java.util.Collection; @@ -157,17 +156,13 @@ private void populateSourceDataTags( return; } - String sourcePath; - try { - sourcePath = sourcePathResolver.getSourcePath(testClass); - if (sourcePath == null || sourcePath.isEmpty()) { - return; - } - } catch (SourceResolutionException e) { - log.debug("Could not populate source path for {}", testClass, e); + Collection sourcePaths = sourcePathResolver.getSourcePaths(testClass); + if (sourcePaths.size() != 1) { + log.debug("Could not populate source path for {}", testClass); return; } + String sourcePath = sourcePaths.iterator().next(); span.setTag(Tags.TEST_SOURCE_FILE, sourcePath); LinesResolver.Lines testClassLines = linesResolver.getClassLines(testClass); diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/source/BestEffortSourcePathResolver.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/source/BestEffortSourcePathResolver.java index 3302f5c4723..814104cc3a9 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/source/BestEffortSourcePathResolver.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/source/BestEffortSourcePathResolver.java @@ -1,5 +1,7 @@ package datadog.trace.civisibility.source; +import java.util.Collection; +import java.util.Collections; import javax.annotation.Nonnull; import javax.annotation.Nullable; @@ -11,24 +13,22 @@ public BestEffortSourcePathResolver(SourcePathResolver... delegates) { this.delegates = delegates; } - @Nullable @Override - public String getSourcePath(@Nonnull Class c) throws SourceResolutionException { + public Collection getSourcePaths(@Nonnull Class c) { for (SourcePathResolver delegate : delegates) { - String sourcePath = delegate.getSourcePath(c); - if (sourcePath != null) { - return sourcePath; + Collection sourcePaths = delegate.getSourcePaths(c); + if (!sourcePaths.isEmpty()) { + return sourcePaths; } } - return null; + return Collections.emptyList(); } - @Nullable @Override - public String getResourcePath(@Nullable String relativePath) throws SourceResolutionException { + public Collection getResourcePaths(@Nullable String relativePath) { for (SourcePathResolver delegate : delegates) { - String resourcePath = delegate.getResourcePath(relativePath); - if (resourcePath != null) { + Collection resourcePath = delegate.getResourcePaths(relativePath); + if (!resourcePath.isEmpty()) { return resourcePath; } } 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 c2aa8831780..35cef73d1d6 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 @@ -2,6 +2,8 @@ import datadog.compiler.utils.CompilerUtils; import java.io.File; +import java.util.Collection; +import java.util.Collections; import javax.annotation.Nonnull; import javax.annotation.Nullable; @@ -13,20 +15,19 @@ public CompilerAidedSourcePathResolver(String repoRoot) { this.repoRoot = repoRoot.endsWith(File.separator) ? repoRoot : repoRoot + File.separator; } - @Nullable + @Nonnull @Override - public String getSourcePath(@Nonnull Class c) { + public Collection getSourcePaths(@Nonnull Class c) { String absoluteSourcePath = CompilerUtils.getSourcePath(c); if (absoluteSourcePath != null && absoluteSourcePath.startsWith(repoRoot)) { - return absoluteSourcePath.substring(repoRoot.length()); + return Collections.singletonList(absoluteSourcePath.substring(repoRoot.length())); } else { - return null; + return Collections.emptyList(); } } - @Nullable @Override - public String getResourcePath(String relativePath) { - return null; + public @Nullable Collection getResourcePaths(String relativePath) { + return Collections.emptyList(); } } diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/source/NoOpSourcePathResolver.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/source/NoOpSourcePathResolver.java index a7b9e18f7bf..fb77c9cd87f 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/source/NoOpSourcePathResolver.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/source/NoOpSourcePathResolver.java @@ -1,5 +1,7 @@ package datadog.trace.civisibility.source; +import java.util.Collection; +import java.util.Collections; import javax.annotation.Nonnull; import javax.annotation.Nullable; @@ -7,15 +9,13 @@ public class NoOpSourcePathResolver implements SourcePathResolver { public static final SourcePathResolver INSTANCE = new NoOpSourcePathResolver(); - @Nullable @Override - public String getSourcePath(@Nonnull Class c) { - return null; + public Collection getSourcePaths(@Nonnull Class c) { + return Collections.emptyList(); } - @Nullable @Override - public String getResourcePath(@Nullable String relativePath) { - return null; + public Collection getResourcePaths(@Nullable String relativePath) { + return Collections.emptyList(); } } diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/source/SourcePathResolver.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/source/SourcePathResolver.java index e76ed076c5f..e12705ba6b5 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/source/SourcePathResolver.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/source/SourcePathResolver.java @@ -1,20 +1,21 @@ package datadog.trace.civisibility.source; +import java.util.Collection; import javax.annotation.Nonnull; import javax.annotation.Nullable; public interface SourcePathResolver { + /** - * @return path to the source file corresponding to the provided class, relative to repository - * root. {@code null} is returned if the path could not be resolved + * @return paths to the source files corresponding to the provided class, relative to repository + * root. Returns all candidate paths when multiple matches exist (e.g. duplicate trie keys in + * repo index approach). Empty collection is returned if no paths could be resolved. */ - @Nullable - String getSourcePath(@Nonnull Class c) throws SourceResolutionException; + Collection getSourcePaths(@Nonnull Class c); /** * @param relativePath Path to a resource in current run's repository, relative to a resource root - * @return Path relative to repository root + * @return Candidate paths relative to repository root */ - @Nullable - String getResourcePath(@Nullable String relativePath) throws SourceResolutionException; + Collection getResourcePaths(@Nullable String relativePath); } diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/source/SourceResolutionException.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/source/SourceResolutionException.java deleted file mode 100644 index 657b46d5031..00000000000 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/source/SourceResolutionException.java +++ /dev/null @@ -1,8 +0,0 @@ -package datadog.trace.civisibility.source; - -public class SourceResolutionException extends Exception { - - public SourceResolutionException(String message) { - super(message); - } -} diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/source/index/RepoIndex.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/source/index/RepoIndex.java index 60a6b969e54..86f47c55eed 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/source/index/RepoIndex.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/source/index/RepoIndex.java @@ -4,7 +4,6 @@ import datadog.trace.api.Config; import datadog.trace.api.civisibility.domain.Language; import datadog.trace.civisibility.ipc.serialization.Serializer; -import datadog.trace.civisibility.source.SourceResolutionException; import datadog.trace.civisibility.source.Utils; import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; @@ -16,6 +15,7 @@ import java.util.Collection; import java.util.Collections; import java.util.List; +import java.util.Map; import java.util.Objects; import javax.annotation.Nonnull; import javax.annotation.Nullable; @@ -27,20 +27,20 @@ public class RepoIndex { static final RepoIndex EMPTY = new RepoIndex( ClassNameTrie.Builder.EMPTY_TRIE, - Collections.emptyList(), + Collections.emptyMap(), Collections.emptyList(), Collections.emptyList()); private static final Logger log = LoggerFactory.getLogger(RepoIndex.class); private final ClassNameTrie trie; - private final Collection duplicateTrieKeys; + private final Map> duplicateTrieKeys; private final List sourceRoots; private final List rootPackages; RepoIndex( ClassNameTrie trie, - Collection duplicateTrieKeys, + Map> duplicateTrieKeys, List sourceRoots, List rootPackages) { this.trie = trie; @@ -53,11 +53,10 @@ public List getRootPackages() { return rootPackages; } - @Nullable - public String getSourcePath(@Nonnull Class c) throws SourceResolutionException { + public Collection getSourcePaths(@Nonnull Class c) { String topLevelClassName = Utils.stripNestedClassNames(c.getName()); - String sourcePath = doGetSourcePath(topLevelClassName); - return sourcePath != null ? sourcePath : getFallbackSourcePath(c); + Collection sourcePaths = doGetAllSourcePaths(topLevelClassName); + return !sourcePaths.isEmpty() ? sourcePaths : getFallbackSourcePaths(c); } /** @@ -65,55 +64,51 @@ public String getSourcePath(@Nonnull Class c) throws SourceResolutionExceptio * name does not necessarily correspond to the source file name, so source file name needs to be * retrieved from the bytecode. */ - @Nullable - private String getFallbackSourcePath(@Nonnull Class c) throws SourceResolutionException { + private Collection getFallbackSourcePaths(@Nonnull Class c) { try { String fileName = Utils.getFileName(c); if (fileName == null) { log.debug("Could not retrieve file name for class {}", c.getName()); - return null; + return Collections.emptyList(); } String fileNameWithoutExtension = Utils.stripExtension(fileName); Package classPackage = c.getPackage(); String packageName = classPackage != null ? classPackage.getName() : ""; String key = packageName + '.' + fileNameWithoutExtension; - return doGetSourcePath(key); + return doGetAllSourcePaths(key); } catch (IOException e) { log.error("Error while trying to retrieve file name for class {}", c.getName(), e); - return null; + return Collections.emptyList(); } } - @Nullable - public String getSourcePath(@Nullable String pathRelativeToSourceRoot) - throws SourceResolutionException { + public Collection getSourcePaths(@Nullable String pathRelativeToSourceRoot) { if (pathRelativeToSourceRoot == null) { - return null; + return Collections.emptyList(); } String key = Utils.toTrieKey(pathRelativeToSourceRoot); - return doGetSourcePath(key); + return doGetAllSourcePaths(key); } - @Nullable - private String doGetSourcePath(String key) throws SourceResolutionException { - if (Config.get().isCiVisibilityRepoIndexDuplicateKeyCheckEnabled()) { - if (!duplicateTrieKeys.isEmpty() && duplicateTrieKeys.contains(key)) { - throw new SourceResolutionException("There are multiple repo index entries for " + key); - } + private Collection doGetAllSourcePaths(String key) { + if (Config.get().isCiVisibilityRepoIndexDuplicateKeyCheckEnabled() + && !duplicateTrieKeys.isEmpty() + && duplicateTrieKeys.containsKey(key)) { + List paths = duplicateTrieKeys.get(key); + log.debug( + "Duplicate trie key {} resolved to {} candidate paths: {}", key, paths.size(), paths); + return paths; } int sourceRootIdx = trie.apply(key); if (sourceRootIdx < 0) { log.debug("Could not find source root for {}", key); - return null; + return Collections.emptyList(); } SourceRoot sourceRoot = sourceRoots.get(sourceRootIdx); - return sourceRoot.relativePath - + File.separatorChar - + key.replace('.', File.separatorChar) - + sourceRoot.language.getExtension(); + return Collections.singletonList(sourceRoot.resolveSourcePath(key)); } public ByteBuffer serialize() { @@ -129,7 +124,7 @@ public ByteBuffer serialize() { Serializer s = new Serializer(); s.write(serializedTrie); - s.write(duplicateTrieKeys); + s.write(duplicateTrieKeys, Serializer::write, (ser, paths) -> ser.write(paths)); s.write(sourceRoots, SourceRoot::serialize); s.write(rootPackages); return s.flush(); @@ -152,7 +147,8 @@ public static RepoIndex deserialize(ByteBuffer buffer) { } } - Collection duplicateTrieKeys = Serializer.readSet(buffer, Serializer::readString); + Map> duplicateTrieKeys = + Serializer.readMap(buffer, Serializer::readString, Serializer::readStringList); List sourceRoots = Serializer.readList(buffer, SourceRoot::deserialize); List rootPackages = Serializer.readStringList(buffer); return new RepoIndex(trie, duplicateTrieKeys, sourceRoots, rootPackages); @@ -169,6 +165,14 @@ static final class SourceRoot { this.language = language; } + /** Resolves a trie key (dot-separated) to a full source path relative to the source root. */ + String resolveSourcePath(String trieKey) { + return relativePath + + File.separatorChar + + trieKey.replace('.', File.separatorChar) + + language.getExtension(); + } + static void serialize(Serializer s, SourceRoot sourceRoot) { s.write(sourceRoot.relativePath); s.write(sourceRoot.language.ordinal()); diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/source/index/RepoIndexBuilder.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/source/index/RepoIndexBuilder.java index 4f645028cbb..2c223c34f90 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/source/index/RepoIndexBuilder.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/source/index/RepoIndexBuilder.java @@ -12,11 +12,11 @@ import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.attribute.BasicFileAttributes; +import java.util.ArrayList; import java.util.Arrays; -import java.util.Collection; import java.util.EnumSet; import java.util.HashMap; -import java.util.HashSet; +import java.util.List; import java.util.Map; import java.util.concurrent.atomic.AtomicInteger; import javax.annotation.Nonnull; @@ -97,8 +97,8 @@ private static final class RepoIndexingFileVisitor implements FileVisitor private final PackageResolver packageResolver; private final ResourceResolver resourceResolver; private final ClassNameTrie.Builder trieBuilder; - private final Map trieKeyToPath; - private final Collection duplicateTrieKeys; + private final Map trieKeyToSourceRootIdx; + private final Map> duplicateSourceRootIndices; private final Map sourceRoots; private final PackageTree packageTree; private final RepoIndexingStats indexingStats; @@ -115,8 +115,8 @@ private RepoIndexingFileVisitor( this.resourceResolver = resourceResolver; this.repoRoot = repoRoot; trieBuilder = new ClassNameTrie.Builder(); - trieKeyToPath = new HashMap<>(); - duplicateTrieKeys = new HashSet<>(); + trieKeyToSourceRootIdx = new HashMap<>(); + duplicateSourceRootIndices = new HashMap<>(); sourceRoots = new HashMap<>(); packageTree = new PackageTree(config); indexingStats = new RepoIndexingStats(); @@ -124,6 +124,7 @@ private RepoIndexingFileVisitor( followSymlinks = config.isCiVisibilityRepoIndexFollowSymlinks(); } + @Nonnull @Override public FileVisitResult preVisitDirectory(Path dir, BasicFileAttributes attrs) { if (Files.isSymbolicLink(dir)) { @@ -151,6 +152,7 @@ private static Path readSymbolicLink(Path path) { } } + @Nonnull @Override public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) { indexingStats.filesVisited++; @@ -177,10 +179,18 @@ public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) { String key = Utils.toTrieKey(relativePath); trieBuilder.put(key, sourceRootIdx); - String existingPath = trieKeyToPath.put(key, file.toString()); - if (existingPath != null) { - log.debug("Duplicate repo index key: {} - {}", existingPath, file); - duplicateTrieKeys.add(key); + Integer existingSourceRootIdx = trieKeyToSourceRootIdx.put(key, sourceRootIdx); + if (existingSourceRootIdx != null) { + log.debug("Duplicate repo index key: {}", key); + duplicateSourceRootIndices + .computeIfAbsent( + key, + k -> { + List indices = new ArrayList<>(); + indices.add(existingSourceRootIdx); // Initialize with original source root + return indices; + }) + .add(sourceRootIdx); } } } @@ -220,6 +230,7 @@ private Path getNonCodeSourceRoot(Path file) throws IOException { return resourceResolver.getResourceRoot(file); } + @Nonnull @Override public FileVisitResult visitFileFailed(Path file, IOException exc) { if (exc != null) { @@ -228,6 +239,7 @@ public FileVisitResult visitFileFailed(Path file, IOException exc) { return FileVisitResult.CONTINUE; } + @Nonnull @Override public FileVisitResult postVisitDirectory(Path dir, IOException exc) { if (exc != null) { @@ -242,8 +254,23 @@ public RepoIndex getIndex() { roots[e.getValue()] = e.getKey(); } + Map> duplicateTrieKeyPaths = + new HashMap<>(duplicateSourceRootIndices.size() * 4 / 3); + for (Map.Entry> entry : duplicateSourceRootIndices.entrySet()) { + String key = entry.getKey(); + List paths = new ArrayList<>(entry.getValue().size()); + for (int idx : entry.getValue()) { + RepoIndex.SourceRoot sr = roots[idx]; + paths.add(sr.resolveSourcePath(key)); + } + duplicateTrieKeyPaths.put(key, paths); + } + return new RepoIndex( - trieBuilder.buildTrie(), duplicateTrieKeys, Arrays.asList(roots), packageTree.asList()); + trieBuilder.buildTrie(), + duplicateTrieKeyPaths, + Arrays.asList(roots), + packageTree.asList()); } } diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/source/index/RepoIndexSourcePathResolver.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/source/index/RepoIndexSourcePathResolver.java index f58de66307f..52ae4db3109 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/source/index/RepoIndexSourcePathResolver.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/source/index/RepoIndexSourcePathResolver.java @@ -1,7 +1,7 @@ package datadog.trace.civisibility.source.index; import datadog.trace.civisibility.source.SourcePathResolver; -import datadog.trace.civisibility.source.SourceResolutionException; +import java.util.Collection; import javax.annotation.Nonnull; import javax.annotation.Nullable; @@ -13,15 +13,13 @@ public RepoIndexSourcePathResolver(RepoIndexProvider indexProvider) { this.indexProvider = indexProvider; } - @Nullable @Override - public String getSourcePath(@Nonnull Class c) throws SourceResolutionException { - return indexProvider.getIndex().getSourcePath(c); + public Collection getSourcePaths(@Nonnull Class c) { + return indexProvider.getIndex().getSourcePaths(c); } - @Nullable @Override - public String getResourcePath(@Nullable String relativePath) throws SourceResolutionException { - return indexProvider.getIndex().getSourcePath(relativePath); + public Collection getResourcePaths(@Nullable String relativePath) { + return indexProvider.getIndex().getSourcePaths(relativePath); } } diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/test/ExecutionStrategy.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/test/ExecutionStrategy.java index 7dcea65c6f2..77fe7f354f9 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/test/ExecutionStrategy.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/test/ExecutionStrategy.java @@ -197,10 +197,11 @@ public boolean isModified(@Nonnull TestSourceData testSourceData) { return false; } try { - String sourcePath = sourcePathResolver.getSourcePath(testClass); - if (sourcePath == null) { + Collection sourcePaths = sourcePathResolver.getSourcePaths(testClass); + if (sourcePaths.size() != 1) { return false; } + String sourcePath = sourcePaths.iterator().next(); LinesResolver.Lines lines = getLines(testSourceData.getTestMethod()); return executionSettings diff --git a/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/domain/TestSuiteImplTest.groovy b/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/domain/TestSuiteImplTest.groovy index c78510db52d..3059fe6e8f2 100644 --- a/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/domain/TestSuiteImplTest.groovy +++ b/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/domain/TestSuiteImplTest.groovy @@ -62,7 +62,7 @@ class TestSuiteImplTest extends SpanWriterTest { linesResolver.getClassLines(MyClass) >> classLines def resolver = Stub(SourcePathResolver) - resolver.getSourcePath(MyClass) >> "MyClass.java" + resolver.getSourcePaths(MyClass) >> ["MyClass.java"] def codeowners = Stub(NoCodeowners) codeowners.getOwners("MyClass.java") >> ["@global-owner1", "@global-owner2"] diff --git a/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/source/BestEffortSourcePathResolverTest.groovy b/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/source/BestEffortSourcePathResolverTest.groovy index 23ae4fd48e5..4568b0e6104 100644 --- a/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/source/BestEffortSourcePathResolverTest.groovy +++ b/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/source/BestEffortSourcePathResolverTest.groovy @@ -5,54 +5,54 @@ import spock.lang.Specification class BestEffortSourcePathResolverTest extends Specification { - def "test get source info from delegate"() { + def "test get source paths from first delegate"() { setup: - def expectedPath = "source/path/TestClass.java" + def expectedPaths = ["source/path/TestClass.java"] def delegate = Stub(SourcePathResolver) def secondDelegate = Stub(SourcePathResolver) def resolver = new BestEffortSourcePathResolver(delegate, secondDelegate) - delegate.getSourcePath(TestClass) >> expectedPath - secondDelegate.getSourcePath(TestClass) >> null + delegate.getSourcePaths(TestClass) >> expectedPaths + secondDelegate.getSourcePaths(TestClass) >> [] when: - def path = resolver.getSourcePath(TestClass) + def paths = resolver.getSourcePaths(TestClass) then: - path == expectedPath + paths == expectedPaths } - def "test get source info from second delegate"() { + def "test get source paths from second delegate when first returns empty"() { setup: - def expectedPath = "source/path/TestClass.java" + def expectedPaths = ["debug/path/TestClass.java", "release/path/TestClass.java"] def delegate = Stub(SourcePathResolver) def secondDelegate = Stub(SourcePathResolver) def resolver = new BestEffortSourcePathResolver(delegate, secondDelegate) - delegate.getSourcePath(TestClass) >> null - secondDelegate.getSourcePath(TestClass) >> expectedPath + delegate.getSourcePaths(TestClass) >> [] + secondDelegate.getSourcePaths(TestClass) >> expectedPaths when: - def path = resolver.getSourcePath(TestClass) + def paths = resolver.getSourcePaths(TestClass) then: - path == expectedPath + paths == expectedPaths } - def "test failed to get info from both delegates"() { + def "test failed to get source paths from both delegates"() { setup: def delegate = Stub(SourcePathResolver) def secondDelegate = Stub(SourcePathResolver) def resolver = new BestEffortSourcePathResolver(delegate, secondDelegate) - delegate.getSourcePath(TestClass) >> null - secondDelegate.getSourcePath(TestClass) >> null + delegate.getSourcePaths(TestClass) >> [] + secondDelegate.getSourcePaths(TestClass) >> [] when: - def path = resolver.getSourcePath(TestClass) + def paths = resolver.getSourcePaths(TestClass) then: - path == null + paths.isEmpty() } private static final class TestClass {} 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 77b563fd96c..c1d516b2256 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 @@ -14,16 +14,17 @@ class CompilerAidedSourcePathResolverTest extends Specification { def sourcePathResolver = new CompilerAidedSourcePathResolver(REPO_ROOT) when: - def path = sourcePathResolver.getSourcePath(clazz) + def path = sourcePathResolver.getSourcePaths(clazz) then: - path == expectedPath + path.size() == expectedPath.size() + path.containsAll(expectedPath) where: clazz | expectedPath - AClassWithNoSourceInfoInjected | null - AClassWithSourceInfoInjected | "path/to/AClassWithSourceInfoInjected.java" - AClassWithSourceOutsideRepository | null + AClassWithNoSourceInfoInjected | [] + AClassWithSourceInfoInjected | ["path/to/AClassWithSourceInfoInjected.java"] + AClassWithSourceOutsideRepository | [] } private static final class AClassWithNoSourceInfoInjected {} diff --git a/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/source/index/RepoIndexSourcePathResolverTest.groovy b/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/source/index/RepoIndexSourcePathResolverTest.groovy index 108c0020065..df61ba11170 100644 --- a/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/source/index/RepoIndexSourcePathResolverTest.groovy +++ b/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/source/index/RepoIndexSourcePathResolverTest.groovy @@ -4,7 +4,6 @@ import com.google.common.jimfs.Configuration import com.google.common.jimfs.Jimfs import datadog.trace.api.Config import datadog.trace.api.civisibility.domain.Language -import datadog.trace.civisibility.source.SourceResolutionException import groovy.transform.PackageScope import spock.lang.Specification @@ -25,9 +24,11 @@ class RepoIndexSourcePathResolverTest extends Specification { when: def sourcePathResolver = new RepoIndexSourcePathResolver(new RepoIndexBuilder(config, repoRoot, packageResolver, resourceResolver, fileSystem)) + def sourcePaths = sourcePathResolver.getSourcePaths(RepoIndexSourcePathResolverTest) then: - sourcePathResolver.getSourcePath(RepoIndexSourcePathResolverTest) == expectedSourcePath + sourcePaths.size() == 1 + sourcePaths.contains(expectedSourcePath) } def "test source path resolution for inner class"() { @@ -36,9 +37,11 @@ class RepoIndexSourcePathResolverTest extends Specification { when: def sourcePathResolver = new RepoIndexSourcePathResolver(new RepoIndexBuilder(config, repoRoot, packageResolver, resourceResolver, fileSystem)) + def sourcePaths = sourcePathResolver.getSourcePaths(InnerClass) then: - sourcePathResolver.getSourcePath(InnerClass) == expectedSourcePath + sourcePaths.size() == 1 + sourcePaths.contains(expectedSourcePath) } def "test source path resolution for nested inner class"() { @@ -47,9 +50,11 @@ class RepoIndexSourcePathResolverTest extends Specification { when: def sourcePathResolver = new RepoIndexSourcePathResolver(new RepoIndexBuilder(config, repoRoot, packageResolver, resourceResolver, fileSystem)) + def sourcePaths = sourcePathResolver.getSourcePaths(InnerClass.NestedInnerClass) then: - sourcePathResolver.getSourcePath(InnerClass.NestedInnerClass) == expectedSourcePath + sourcePaths.size() == 1 + sourcePaths.contains(expectedSourcePath) } def "test source path resolution for anonymous class"() { @@ -61,9 +66,11 @@ class RepoIndexSourcePathResolverTest extends Specification { def r = new Runnable() { void run() {} } + def sourcePaths = sourcePathResolver.getSourcePaths(r.getClass()) then: - sourcePathResolver.getSourcePath(r.getClass()) == expectedSourcePath + sourcePaths.size() == 1 + sourcePaths.contains(expectedSourcePath) } def "test source path resolution for package-private class"() { @@ -72,9 +79,11 @@ class RepoIndexSourcePathResolverTest extends Specification { when: def sourcePathResolver = new RepoIndexSourcePathResolver(new RepoIndexBuilder(config, repoRoot, packageResolver, resourceResolver, fileSystem)) + def sourcePaths = sourcePathResolver.getSourcePaths(PackagePrivateClass) then: - sourcePathResolver.getSourcePath(PackagePrivateClass) == expectedSourcePath + sourcePaths.size() == 1 + sourcePaths.contains(expectedSourcePath) } def "test source path resolution for class nested into package-private class"() { @@ -83,9 +92,11 @@ class RepoIndexSourcePathResolverTest extends Specification { when: def sourcePathResolver = new RepoIndexSourcePathResolver(new RepoIndexBuilder(config, repoRoot, packageResolver, resourceResolver, fileSystem)) + def sourcePaths = sourcePathResolver.getSourcePaths(PackagePrivateClass.NestedIntoPackagePrivateClass) then: - sourcePathResolver.getSourcePath(PackagePrivateClass.NestedIntoPackagePrivateClass) == expectedSourcePath + sourcePaths.size() == 1 + sourcePaths.contains(expectedSourcePath) } def "test source path resolution for non-java class whose file name is different from class name"() { @@ -94,9 +105,11 @@ class RepoIndexSourcePathResolverTest extends Specification { when: def sourcePathResolver = new RepoIndexSourcePathResolver(new RepoIndexBuilder(config, repoRoot, packageResolver, resourceResolver, fileSystem)) + def sourcePaths = sourcePathResolver.getSourcePaths(PublicClassWhoseNameDoesNotCorrespondToFileName) then: - sourcePathResolver.getSourcePath(PublicClassWhoseNameDoesNotCorrespondToFileName) == expectedSourcePath + sourcePaths.size() == 1 + sourcePaths.contains(expectedSourcePath) } def "test source path for non-indexed class"() { @@ -106,7 +119,7 @@ class RepoIndexSourcePathResolverTest extends Specification { def sourcePathResolver = new RepoIndexSourcePathResolver(new RepoIndexBuilder(config, repoRoot, packageResolver, resourceResolver, fileSystem)) then: - sourcePathResolver.getSourcePath(RepoIndexSourcePathResolver) == null + sourcePathResolver.getSourcePaths(RepoIndexSourcePathResolver).isEmpty() } def "test source path for non-indexed package-private class"() { @@ -116,7 +129,7 @@ class RepoIndexSourcePathResolverTest extends Specification { def sourcePathResolver = new RepoIndexSourcePathResolver(new RepoIndexBuilder(config, repoRoot, packageResolver, resourceResolver, fileSystem)) then: - sourcePathResolver.getSourcePath(PackagePrivateClass) == null + sourcePathResolver.getSourcePaths(PackagePrivateClass).isEmpty() } def "test file-indexing failure"() { @@ -131,7 +144,7 @@ class RepoIndexSourcePathResolverTest extends Specification { def sourcePathResolver = new RepoIndexSourcePathResolver(new RepoIndexBuilder(config, repoRoot, packageResolver, resourceResolver, fileSystem)) then: - sourcePathResolver.getSourcePath(RepoIndexSourcePathResolverTest) == null + sourcePathResolver.getSourcePaths(RepoIndexSourcePathResolverTest).isEmpty() } def "test source path resolution for repo with multiple files"() { @@ -143,25 +156,32 @@ class RepoIndexSourcePathResolverTest extends Specification { when: def sourcePathResolver = new RepoIndexSourcePathResolver(new RepoIndexBuilder(config, repoRoot, packageResolver, resourceResolver, fileSystem)) + def sourcePathsOne = sourcePathResolver.getSourcePaths(RepoIndexSourcePathResolverTest) + def sourcePathsTwo = sourcePathResolver.getSourcePaths(InnerClass) + def sourcePathsThree = sourcePathResolver.getSourcePaths(RepoIndexSourcePathResolver) then: - sourcePathResolver.getSourcePath(RepoIndexSourcePathResolverTest) == expectedSourcePathOne - sourcePathResolver.getSourcePath(InnerClass) == expectedSourcePathOne - sourcePathResolver.getSourcePath(RepoIndexSourcePathResolver) == expectedSourcePathTwo + sourcePathsOne.size() == 1 + sourcePathsOne.contains(expectedSourcePathOne) + sourcePathsTwo.size() == 1 + sourcePathsTwo.contains(expectedSourcePathOne) + sourcePathsThree.size() == 1 + sourcePathsThree.contains(expectedSourcePathTwo) } - def "test trying to resolve a duplicate key throws exception"() { + def "test trying to resolve a duplicate key returns both candidates"() { setup: - givenSourceFile(RepoIndexSourcePathResolverTest, repoRoot + "/src/java") - givenSourceFile(RepoIndexSourcePathResolverTest, repoRoot + "/src/scala") + def expectedJavaPath = givenSourceFile(RepoIndexSourcePathResolverTest, repoRoot + "/src/java") + def expectedScalaPath = givenSourceFile(RepoIndexSourcePathResolverTest, repoRoot + "/src/scala") def sourcePathResolver = new RepoIndexSourcePathResolver(new RepoIndexBuilder(config, repoRoot, packageResolver, resourceResolver, fileSystem)) when: - sourcePathResolver.getSourcePath(RepoIndexSourcePathResolverTest) + def sourcePaths = sourcePathResolver.getSourcePaths(RepoIndexSourcePathResolverTest) then: - thrown SourceResolutionException + sourcePaths.size() == 2 + sourcePaths.containsAll([expectedJavaPath, expectedScalaPath]) } private String givenSourceFile(Class c, String sourceRoot, Language language = Language.GROOVY) { diff --git a/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/source/index/RepoIndexTest.groovy b/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/source/index/RepoIndexTest.groovy index 5a4763e5a53..70da9c5780a 100644 --- a/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/source/index/RepoIndexTest.groovy +++ b/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/source/index/RepoIndexTest.groovy @@ -2,7 +2,6 @@ package datadog.trace.civisibility.source.index import datadog.instrument.utils.ClassNameTrie import datadog.trace.api.civisibility.domain.Language -import datadog.trace.civisibility.source.SourceResolutionException import spock.lang.Specification class RepoIndexTest extends Specification { @@ -21,26 +20,95 @@ class RepoIndexTest extends Specification { new RepoIndex.SourceRoot("myClassSourceRoot", Language.GROOVY), new RepoIndex.SourceRoot("myOtherClassSourceRoot", Language.GROOVY)) - def repoIndex = new RepoIndex(trie, Collections.emptyList(), sourceRoots, Collections.emptyList()) + def repoIndex = new RepoIndex(trie, Collections.emptyMap(), sourceRoots, Collections.emptyList()) when: def serialized = repoIndex.serialize() def deserialized = RepoIndex.deserialize(serialized) then: - deserialized.getSourcePath(RepoIndexTest) == "myClassSourceRoot/" + myClassName.replace('.' as char, File.separatorChar) + Language.GROOVY.extension - deserialized.getSourcePath(RepoIndexSourcePathResolverTest) == "myOtherClassSourceRoot/" + myOtherClassName.replace('.' as char, File.separatorChar) + Language.GROOVY.extension + deserialized.getSourcePaths(RepoIndexTest).size() == 1 + deserialized.getSourcePaths(RepoIndexTest) .contains("myClassSourceRoot/" + myClassName.replace('.' as char, File.separatorChar) + Language.GROOVY.extension) + deserialized.getSourcePaths(RepoIndexSourcePathResolverTest).size() == 1 + deserialized.getSourcePaths(RepoIndexSourcePathResolverTest).contains("myOtherClassSourceRoot/" + myOtherClassName.replace('.' as char, File.separatorChar) + Language.GROOVY.extension) } - def "test trying to resolve a duplicate key throws exception"() { + def "test serialization and deserialization with duplicate keys"() { given: - def duplicateKeys = [RepoIndexTest.name] - def repoIndex = new RepoIndex(new ClassNameTrie.Builder().buildTrie(), duplicateKeys, Collections.emptyList(), Collections.emptyList()) + def myClassName = RepoIndexTest.name + + def trieBuilder = new ClassNameTrie.Builder() + trieBuilder.put(myClassName, 0) + def trie = trieBuilder.buildTrie() + + def sourceRoots = Arrays.asList( + new RepoIndex.SourceRoot("sourceRoot1", Language.GROOVY), + new RepoIndex.SourceRoot("sourceRoot2", Language.GROOVY)) + + def duplicateKeys = [(myClassName): [ + "sourceRoot1/" + myClassName.replace('.' as char, File.separatorChar) + Language.GROOVY.extension, + "sourceRoot2/" + myClassName.replace('.' as char, File.separatorChar) + Language.GROOVY.extension + ]] + + def repoIndex = new RepoIndex(trie, duplicateKeys, sourceRoots, Collections.emptyList()) + + when: + def serialized = repoIndex.serialize() + def deserialized = RepoIndex.deserialize(serialized) + + then: + def paths = deserialized.getSourcePaths(RepoIndexTest) + paths.size() == 2 + paths.containsAll([ + "sourceRoot1/" + myClassName.replace('.' as char, File.separatorChar) + Language.GROOVY.extension, + "sourceRoot2/" + myClassName.replace('.' as char, File.separatorChar) + Language.GROOVY.extension + ]) + } + + def "test getSourcePaths returns all paths for duplicate key"() { + given: + def myClassName = RepoIndexTest.name + + def trieBuilder = new ClassNameTrie.Builder() + trieBuilder.put(myClassName, 0) + def trie = trieBuilder.buildTrie() + + def sourceRoots = Arrays.asList( + new RepoIndex.SourceRoot("debug", Language.GROOVY), + new RepoIndex.SourceRoot("release", Language.GROOVY)) + + def expectedPath1 = "debug/" + myClassName.replace('.' as char, File.separatorChar) + Language.GROOVY.extension + def expectedPath2 = "release/" + myClassName.replace('.' as char, File.separatorChar) + Language.GROOVY.extension + def duplicateKeys = [(myClassName): [expectedPath1, expectedPath2]] + + def repoIndex = new RepoIndex(trie, duplicateKeys, sourceRoots, Collections.emptyList()) + + when: + def paths = repoIndex.getSourcePaths(RepoIndexTest) + + then: + paths.size() == 2 + paths.containsAll([expectedPath1, expectedPath2]) + } + + def "test getSourcePaths returns single path for non-duplicate key"() { + given: + def myClassName = RepoIndexTest.name + + def trieBuilder = new ClassNameTrie.Builder() + trieBuilder.put(myClassName, 0) + def trie = trieBuilder.buildTrie() + + def sourceRoots = Arrays.asList( + new RepoIndex.SourceRoot("src/main/groovy", Language.GROOVY)) + + def repoIndex = new RepoIndex(trie, Collections.emptyMap(), sourceRoots, Collections.emptyList()) when: - repoIndex.getSourcePath(RepoIndexTest) + def paths = repoIndex.getSourcePaths(RepoIndexTest) then: - thrown SourceResolutionException + paths.size() == 1 + paths.first() == "src/main/groovy/" + myClassName.replace('.' as char, File.separatorChar) + Language.GROOVY.extension } } diff --git a/dd-java-agent/agent-ci-visibility/src/test/java/datadog/trace/civisibility/coverage/file/FileCoverageStoreTest.java b/dd-java-agent/agent-ci-visibility/src/test/java/datadog/trace/civisibility/coverage/file/FileCoverageStoreTest.java new file mode 100644 index 00000000000..fef10b98639 --- /dev/null +++ b/dd-java-agent/agent-ci-visibility/src/test/java/datadog/trace/civisibility/coverage/file/FileCoverageStoreTest.java @@ -0,0 +1,97 @@ +package datadog.trace.civisibility.coverage.file; + +import static java.util.Arrays.asList; +import static java.util.Collections.emptyList; +import static java.util.Collections.singletonList; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import datadog.trace.api.DDTraceId; +import datadog.trace.api.civisibility.coverage.CoverageStore; +import datadog.trace.api.civisibility.coverage.TestReport; +import datadog.trace.api.civisibility.telemetry.CiVisibilityMetricCollector; +import datadog.trace.civisibility.source.SourcePathResolver; +import org.junit.jupiter.api.Test; + +class FileCoverageStoreTest { + + private static final class ResolvableClassA {} + + private static final class DuplicateKeyClass {} + + private static final class ResolvableClassC {} + + @Test + void duplicateKeyClassReturnsAllCandidatePathsInCoverageReport() { + CiVisibilityMetricCollector metrics = mock(CiVisibilityMetricCollector.class); + SourcePathResolver sourcePathResolver = mock(SourcePathResolver.class); + when(sourcePathResolver.getSourcePaths(ResolvableClassA.class)) + .thenReturn(singletonList("src/main/java/com/example/ClassA.java")); + when(sourcePathResolver.getSourcePaths(DuplicateKeyClass.class)) + .thenReturn( + asList( + "src/debug/java/com/example/DuplicateKeyClass.java", + "src/release/java/com/example/DuplicateKeyClass.java")); + when(sourcePathResolver.getSourcePaths(ResolvableClassC.class)) + .thenReturn(singletonList("src/main/java/com/example/ClassC.java")); + + CoverageStore store = new FileCoverageStore.Factory(metrics, sourcePathResolver).create(null); + store.getProbes().record(ResolvableClassA.class); + store.getProbes().record(DuplicateKeyClass.class); + store.getProbes().record(ResolvableClassC.class); + + boolean result = store.report(DDTraceId.ONE, 1L, 1L); + + assertTrue(result); + TestReport report = store.getReport(); + assertNotNull(report); + assertEquals(4, report.getTestReportFileEntries().size()); + } + + @Test + void coverageReportSucceedsForNonDuplicateClasses() { + CiVisibilityMetricCollector metrics = mock(CiVisibilityMetricCollector.class); + SourcePathResolver sourcePathResolver = mock(SourcePathResolver.class); + when(sourcePathResolver.getSourcePaths(ResolvableClassA.class)) + .thenReturn(singletonList("src/main/java/com/example/ClassA.java")); + when(sourcePathResolver.getSourcePaths(ResolvableClassC.class)) + .thenReturn(singletonList("src/main/java/com/example/ClassC.java")); + + CoverageStore store = new FileCoverageStore.Factory(metrics, sourcePathResolver).create(null); + store.getProbes().record(ResolvableClassA.class); + store.getProbes().record(ResolvableClassC.class); + + boolean result = store.report(DDTraceId.ONE, 1L, 1L); + + assertTrue(result); + TestReport report = store.getReport(); + assertNotNull(report); + assertEquals(2, report.getTestReportFileEntries().size()); + } + + @Test + void emptySourcePathsForOneClassDoesNotKillCoverageReport() { + CiVisibilityMetricCollector metrics = mock(CiVisibilityMetricCollector.class); + SourcePathResolver sourcePathResolver = mock(SourcePathResolver.class); + when(sourcePathResolver.getSourcePaths(ResolvableClassA.class)) + .thenReturn(singletonList("src/main/java/com/example/ClassA.java")); + when(sourcePathResolver.getSourcePaths(DuplicateKeyClass.class)).thenReturn(emptyList()); + when(sourcePathResolver.getSourcePaths(ResolvableClassC.class)) + .thenReturn(singletonList("src/main/java/com/example/ClassC.java")); + + CoverageStore store = new FileCoverageStore.Factory(metrics, sourcePathResolver).create(null); + store.getProbes().record(ResolvableClassA.class); + store.getProbes().record(DuplicateKeyClass.class); + store.getProbes().record(ResolvableClassC.class); + + boolean result = store.report(DDTraceId.ONE, 1L, 1L); + + assertTrue(result); + TestReport report = store.getReport(); + assertNotNull(report); + assertEquals(2, report.getTestReportFileEntries().size()); + } +}