Skip to content

Include all candidate paths for duplicate keys in coverage reports #10997

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
gh-worker-dd-mergequeue-cf854d merged 9 commits into from
Apr 6, 2026
Merged

Include all candidate paths for duplicate keys in coverage reports #10997

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
b177da4
fix: report duplicate entries in coverage instead of nulling the report
daniel-mohedano Mar 30, 2026
74ea9f5
fix: refactor source path resolvers to always return collections
daniel-mohedano Mar 30, 2026
0367988
fix: remove unused variable
daniel-mohedano Mar 30, 2026
51cb4ce
fix: pre-size structures
daniel-mohedano Mar 30, 2026
4f10a8a
nit: add @Nonnull annotations to overriden methods
daniel-mohedano Mar 30, 2026
78672aa
style: spotless
daniel-mohedano Mar 30, 2026
a3d3997
fix: remove unused logging
daniel-mohedano Mar 31, 2026
e6eca50
Merge branch 'master' into daniel.mohedano/coverage-report-duplicate-…
daniel-mohedano Apr 1, 2026
a819948
Merge branch 'master' into daniel.mohedano/coverage-report-duplicate-…
daniel-mohedano Apr 6, 2026
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
1 change: 1 addition & 0 deletions dd-java-agent/agent-ci-visibility/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
6 changes: 3 additions & 3 deletions ...ixtures/src/main/groovy/datadog/trace/civisibility/CiVisibilityInstrumentationTest.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
12 changes: 3 additions & 9 deletions ...visibility/src/main/java/datadog/trace/civisibility/coverage/ConcurrentCoverageStore.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<T> probes)
throws SourceResolutionException;
DDTraceId testSessionId, Long testSuiteId, long testSpanId, Collection<T> probes);

@Nullable
@Override
Expand Down
88 changes: 40 additions & 48 deletions ...-visibility/src/main/java/datadog/trace/civisibility/coverage/file/FileCoverageStore.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -47,61 +46,54 @@ private FileCoverageStore(
@Nullable
@Override
protected TestReport report(
DDTraceId testSessionId, Long testSuiteId, long testSpanId, Collection<FileProbes> probes)
throws SourceResolutionException {
try {
Set<Class<?>> combinedClasses = Collections.newSetFromMap(new IdentityHashMap<>());
Collection<String> combinedNonCodeResources = new HashSet<>();

for (FileProbes probe : probes) {
combinedClasses.addAll(probe.getCoveredClasses());
combinedNonCodeResources.addAll(probe.getNonCodeResources());
}
DDTraceId testSessionId, Long testSuiteId, long testSpanId, Collection<FileProbes> probes) {
Set<Class<?>> combinedClasses = Collections.newSetFromMap(new IdentityHashMap<>());
Collection<String> combinedNonCodeResources = new HashSet<>();

if (combinedClasses.isEmpty() && combinedNonCodeResources.isEmpty()) {
return null;
}
for (FileProbes probe : probes) {
combinedClasses.addAll(probe.getCoveredClasses());
combinedNonCodeResources.addAll(probe.getNonCodeResources());
}

Set<String> 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<String> coveredPaths = set(combinedClasses.size() + combinedNonCodeResources.size());
for (Class<?> clazz : combinedClasses) {
Collection<String> 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<TestReportFileEntry> fileEntries = new ArrayList<>(coveredPaths.size());
for (String path : coveredPaths) {
fileEntries.add(new TestReportFileEntry(path, null));
for (String nonCodeResource : combinedNonCodeResources) {
Collection<String> 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<TestReportFileEntry> 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 <T> Set<T> set(int size) {
Expand Down
143 changes: 69 additions & 74 deletions ...-visibility/src/main/java/datadog/trace/civisibility/coverage/line/LineCoverageStore.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -53,88 +52,84 @@ private LineCoverageStore(
@Nullable
@Override
protected TestReport report(
DDTraceId testSessionId, Long testSuiteId, long testSpanId, Collection<LineProbes> probes)
throws SourceResolutionException {
try {
Map<Class<?>, ExecutionDataAdapter> combinedExecutionData = new IdentityHashMap<>();
Collection<String> combinedNonCodeResources = new HashSet<>();

for (LineProbes probe : probes) {
for (Map.Entry<Class<?>, ExecutionDataAdapter> e : probe.getExecutionData().entrySet()) {
combinedExecutionData.merge(e.getKey(), e.getValue(), ExecutionDataAdapter::merge);
}
combinedNonCodeResources.addAll(probe.getNonCodeResources());
}
DDTraceId testSessionId, Long testSuiteId, long testSpanId, Collection<LineProbes> probes) {
Map<Class<?>, ExecutionDataAdapter> combinedExecutionData = new IdentityHashMap<>();
Collection<String> combinedNonCodeResources = new HashSet<>();

if (combinedExecutionData.isEmpty() && combinedNonCodeResources.isEmpty()) {
return null;
for (LineProbes probe : probes) {
for (Map.Entry<Class<?>, ExecutionDataAdapter> e : probe.getExecutionData().entrySet()) {
combinedExecutionData.merge(e.getKey(), e.getValue(), ExecutionDataAdapter::merge);
}
combinedNonCodeResources.addAll(probe.getNonCodeResources());
}

Map<String, BitSet> coveredLinesBySourcePath = new HashMap<>();
for (Map.Entry<Class<?>, 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<TestReportFileEntry> fileEntries = new ArrayList<>(coveredLinesBySourcePath.size());
for (Map.Entry<String, BitSet> e : coveredLinesBySourcePath.entrySet()) {
String sourcePath = e.getKey();
BitSet coveredLines = e.getValue();
fileEntries.add(new TestReportFileEntry(sourcePath, coveredLines));
Map<String, BitSet> coveredLinesBySourcePath = new HashMap<>();
for (Map.Entry<Class<?>, ExecutionDataAdapter> e : combinedExecutionData.entrySet()) {
ExecutionDataAdapter executionDataAdapter = e.getValue();
String className = executionDataAdapter.getClassName();

Class<?> clazz = e.getKey();
Collection<String> 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<TestReportFileEntry> fileEntries = new ArrayList<>(coveredLinesBySourcePath.size());
for (Map.Entry<String, BitSet> 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<String> 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 {
Expand Down
32 changes: 12 additions & 20 deletions ...ity/src/main/java/datadog/trace/civisibility/coverage/report/JacocoCoverageProcessor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<String> 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)));
}
}

Expand Down Expand Up @@ -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<String> 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,
Expand Down
Loading
Loading