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 e71a8a5d8c9..2d2a8261d64 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 @@ -15,7 +15,6 @@ import datadog.trace.api.civisibility.config.LibraryCapability import datadog.trace.api.civisibility.config.TestFQN import datadog.trace.api.civisibility.config.TestIdentifier import datadog.trace.api.civisibility.config.TestMetadata -import datadog.trace.api.civisibility.coverage.CoveragePerTestBridge import datadog.trace.api.civisibility.events.TestEventsHandler import datadog.trace.api.civisibility.telemetry.CiVisibilityMetricCollector import datadog.trace.api.civisibility.telemetry.tag.Provider @@ -201,8 +200,6 @@ abstract class CiVisibilityInstrumentationTest extends InstrumentationSpecificat InstrumentationBridge.registerBuildEventsHandlerFactory { decorator -> new BuildEventsHandlerImpl<>(buildSystemSessionFactory, new JvmInfoFactoryImpl()) } - - CoveragePerTestBridge.registerCoverageStoreRegistry(coverageStoreFactory) } private static final class MockExecutionSettingsFactory implements ExecutionSettingsFactory { diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/CiVisibilitySystem.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/CiVisibilitySystem.java index 246ebe2b336..9372b203156 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/CiVisibilitySystem.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/CiVisibilitySystem.java @@ -8,7 +8,6 @@ import datadog.trace.api.civisibility.DDTestSuite; import datadog.trace.api.civisibility.InstrumentationBridge; import datadog.trace.api.civisibility.config.LibraryCapability; -import datadog.trace.api.civisibility.coverage.CoveragePerTestBridge; import datadog.trace.api.civisibility.events.BuildEventsHandler; import datadog.trace.api.civisibility.events.TestEventsHandler; import datadog.trace.api.civisibility.telemetry.CiVisibilityMetricCollector; @@ -118,7 +117,6 @@ public static void start(Instrumentation inst, SharedCommunicationObjects sco) { TestEventsHandlerFactory testEventsHandlerFactory = new TestEventsHandlerFactory(services, repoServices, coverageServices, executionSettings); InstrumentationBridge.registerTestEventsHandlerFactory(testEventsHandlerFactory); - CoveragePerTestBridge.registerCoverageStoreRegistry(coverageServices.coverageStoreFactory); AgentTracer.TracerAPI tracerAPI = AgentTracer.get(); tracerAPI.addShutdownListener(testEventsHandlerFactory::shutdown); diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/coverage/SkippableAwareCoverageStoreFactory.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/coverage/SkippableAwareCoverageStoreFactory.java index be50bf82d7c..90de26c1be8 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/coverage/SkippableAwareCoverageStoreFactory.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/coverage/SkippableAwareCoverageStoreFactory.java @@ -31,9 +31,4 @@ public CoverageStore create(@Nullable TestIdentifier testIdentifier) { return delegate.create(testIdentifier); } } - - @Override - public void setTotalProbeCount(String className, int totalProbeCount) { - delegate.setTotalProbeCount(className, totalProbeCount); - } } 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 6de1354e7c7..dfd4017a70a 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 @@ -118,10 +118,5 @@ public CoverageStore create(@Nullable TestIdentifier testIdentifier) { private FileProbes createProbes(boolean isTestThread) { return new FileProbes(metrics, isTestThread); } - - @Override - public void setTotalProbeCount(String className, int totalProbeCount) { - // no op - } } } diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/coverage/file/FileProbes.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/coverage/file/FileProbes.java index ac53ecda24a..47192376abd 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/coverage/file/FileProbes.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/coverage/file/FileProbes.java @@ -31,11 +31,6 @@ public class FileProbes implements CoverageProbes { nonCodeResources = isTestThread ? new HashMap<>() : new ConcurrentHashMap<>(); } - @Override - public void record(Class clazz, long classId, int probeId) { - record(clazz); - } - @Override public void record(Class clazz) { try { diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/coverage/line/ClassCoverageModel.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/coverage/line/ClassCoverageModel.java new file mode 100644 index 00000000000..51b35464157 --- /dev/null +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/coverage/line/ClassCoverageModel.java @@ -0,0 +1,615 @@ +package datadog.trace.civisibility.coverage.line; + +import java.util.ArrayList; +import java.util.BitSet; +import java.util.Collections; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import org.jacoco.core.analysis.ISourceNode; +import org.jacoco.core.internal.analysis.filter.Filters; +import org.jacoco.core.internal.analysis.filter.IFilter; +import org.jacoco.core.internal.analysis.filter.IFilterContext; +import org.jacoco.core.internal.analysis.filter.IFilterOutput; +import org.jacoco.core.internal.analysis.filter.Replacements; +import org.jacoco.core.internal.flow.ClassProbesAdapter; +import org.jacoco.core.internal.flow.ClassProbesVisitor; +import org.jacoco.core.internal.flow.IFrame; +import org.jacoco.core.internal.flow.LabelInfo; +import org.jacoco.core.internal.flow.MethodProbesVisitor; +import org.objectweb.asm.ClassReader; +import org.objectweb.asm.Handle; +import org.objectweb.asm.Label; +import org.objectweb.asm.MethodVisitor; +import org.objectweb.asm.tree.AbstractInsnNode; +import org.objectweb.asm.tree.MethodNode; +import org.objectweb.asm.tree.TryCatchBlockNode; + +/** + * Probe-value-independent structural model of a class' line coverage. + * + *

Jacoco's {@link org.jacoco.core.analysis.Analyzer} re-parses and re-analyzes a class' bytecode + * for every test that covers it, which dominates the cost of line-coverage reporting. The structure + * that maps probes to covered lines, however, depends only on the bytecode — not on which probes a + * given test executed. This model captures that structure once (mirroring Jacoco's own analysis but + * carrying symbolic per-branch probe sets instead of concrete booleans) so that resolving the + * covered lines for a test becomes a cheap set intersection: a line is covered iff any probe that + * structurally covers it was executed. + * + *

Correctness is not assumed: {@link #matches} lets the caller verify the model against Jacoco's + * real analysis for an observed probe array before trusting it, falling back otherwise. + */ +public final class ClassCoverageModel { + + /** + * Sentinel cached for classes that must not use a model (build failed or the model did not + * reproduce Jacoco). Callers compare by identity and fall back to Jacoco's analysis. Having a + * single cache with this sentinel (rather than a separate "unmodellable" set) keeps the + * first-encounter decision atomic and avoids a class being treated as both modelled and not. + */ + static final ClassCoverageModel UNMODELLABLE = new ClassCoverageModel(Collections.emptyMap()); + + // For each source line, the set of probe ids that cover it. The line is covered by a test iff the + // test executed at least one of these probes. + private final Map lineProbes; + + private ClassCoverageModel(Map lineProbes) { + this.lineProbes = lineProbes; + } + + /** Resolves the lines covered by a test given the probes it executed. */ + public BitSet coveredLines(boolean[] probes) { + BitSet covered = new BitSet(); + for (Map.Entry e : lineProbes.entrySet()) { + BitSet probeSet = e.getValue(); + for (int p = probeSet.nextSetBit(0); p >= 0; p = probeSet.nextSetBit(p + 1)) { + if (p < probes.length && probes[p]) { + covered.set(e.getKey()); + break; + } + } + } + return covered; + } + + /** + * Verifies the model reproduces Jacoco's covered lines for a given probe array. Used to guard the + * optimization: a mismatch (e.g. an unmodelled filter interaction) means the caller should fall + * back to Jacoco's analysis for this class rather than trust the model. + */ + public boolean matches(boolean[] probes, BitSet jacocoCoveredLines) { + return coveredLines(probes).equals(jacocoCoveredLines); + } + + /** Builds the model by analyzing the class bytecode once. */ + public static ClassCoverageModel build(byte[] classBytes) { + Map lineProbes = new HashMap<>(); + ClassReader reader = new ClassReader(classBytes); + reader.accept(new ClassProbesAdapter(new ModelClassAnalyzer(lineProbes), false), 0); + return new ClassCoverageModel(lineProbes); + } + + /** A single bytecode instruction with, per outgoing branch, the set of probes that cover it. */ + private static final class ProbeInstruction { + private final int line; + private int branches; + private final Map branchProbes = new HashMap<>(); + private ProbeInstruction predecessor; + private int predecessorBranch; + + ProbeInstruction(int line) { + this.line = line; + } + + /** Structural CFG edge to a target instruction (mirrors Jacoco addBranch(Instruction,int)). */ + void addBranch(ProbeInstruction target, int branch) { + branches++; + target.predecessor = this; + target.predecessorBranch = branch; + BitSet targetProbes = target.coveringProbes(); + for (int p = targetProbes.nextSetBit(0); p >= 0; p = targetProbes.nextSetBit(p + 1)) { + propagate(this, branch, p); + } + } + + /** Branch whose coverage is directly gated by a probe (mirrors addBranch(boolean,int)). */ + void addProbeBranch(int probeId, int branch) { + branches++; + propagate(this, branch, probeId); + } + + // Propagates "covered by probeId along this branch" up the predecessor chain. Unlike Jacoco's + // boolean propagation (which stops as soon as an instruction is covered by anything), this + // continues until the instruction already carries this specific probe, so every probe that can + // reach an instruction is recorded — needed to answer the query for any probe array. + private static void propagate(ProbeInstruction insn, int branch, int probeId) { + while (insn != null) { + BitSet bp = insn.branchProbes.get(branch); + if (bp == null) { + bp = new BitSet(); + insn.branchProbes.put(branch, bp); + } + if (bp.get(probeId)) { + break; + } + bp.set(probeId); + branch = insn.predecessorBranch; + insn = insn.predecessor; + } + } + + /** Probes covering this instruction (union over its branches). */ + BitSet coveringProbes() { + BitSet union = new BitSet(); + for (BitSet bp : branchProbes.values()) { + union.or(bp); + } + return union; + } + + ProbeInstruction merge(ProbeInstruction other) { + ProbeInstruction result = new ProbeInstruction(line); + result.branches = branches; + for (Map.Entry e : branchProbes.entrySet()) { + result.branchProbes.put(e.getKey(), (BitSet) e.getValue().clone()); + } + for (Map.Entry e : other.branchProbes.entrySet()) { + result.branchProbes.computeIfAbsent(e.getKey(), k -> new BitSet()).or(e.getValue()); + } + return result; + } + + ProbeInstruction replaceBranches(Replacements replacements, Mapper mapper) { + ProbeInstruction result = new ProbeInstruction(line); + int branchIndex = 0; + for (java.util.Collection newBranch : replacements.values()) { + BitSet probes = new BitSet(); + for (Replacements.InstructionBranch from : newBranch) { + BitSet fromProbes = mapper.apply(from.instruction).branchProbes.get(from.branch); + if (fromProbes != null) { + probes.or(fromProbes); + } + } + if (!probes.isEmpty()) { + result.branchProbes.put(branchIndex, probes); + } + branchIndex++; + } + result.branches = branchIndex; + return result; + } + + int getLine() { + return line; + } + } + + /** {@code Function}. */ + private interface Mapper { + ProbeInstruction apply(AbstractInsnNode node); + } + + /** Builds {@link ProbeInstruction}s of a method (mirrors Jacoco InstructionsBuilder). */ + private static final class InstructionsBuilder { + private int currentLine = ISourceNode.UNKNOWN_LINE; + private ProbeInstruction currentInsn; + private final Map instructions = new HashMap<>(); + private final Map labelToInsn = new HashMap<>(); + private final List

The per-test array is allocated when a method of the class is entered (so the probe array + * can be swapped in), which can happen even if no probe ends up firing (e.g. the method throws + * before reaching its first probe). Returning whether any probe was actually covered lets the + * caller skip such classes and avoid emitting empty coverage entries. + * + * @return {@code true} if at least one probe was covered by the test + */ + boolean mergeIntoJacocoProbes() { + boolean covered = false; + for (int i = 0; i < probeActivations.length; i++) { + if (probeActivations[i]) { + jacocoProbes[i] = true; + covered = true; + } + } + return covered; + } + ExecutionData toExecutionData() { return new ExecutionData(classId, className, probeActivations); } 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 647f28ca181..1d97e6117f0 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 @@ -12,6 +12,8 @@ import datadog.trace.civisibility.coverage.ConcurrentCoverageStore; import datadog.trace.civisibility.source.SourcePathResolver; import datadog.trace.civisibility.source.Utils; +import java.io.ByteArrayOutputStream; +import java.io.IOException; import java.io.InputStream; import java.util.ArrayList; import java.util.BitSet; @@ -25,6 +27,7 @@ import java.util.function.Function; import javax.annotation.Nullable; import org.jacoco.core.analysis.Analyzer; +import org.jacoco.core.data.ExecutionData; import org.jacoco.core.data.ExecutionDataStore; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -37,16 +40,32 @@ public class LineCoverageStore extends ConcurrentCoverageStore { private static final Logger log = LoggerFactory.getLogger(LineCoverageStore.class); + /** + * Upper bound on the number of cached class models. Coverage stays correct beyond it (the class + * is just analyzed by Jacoco each time), this only guards memory for pathologically large suites. + */ + private static final int MAX_MODEL_CACHE_ENTRIES = 50_000; + private final CiVisibilityMetricCollector metrics; private final SourcePathResolver sourcePathResolver; + // Module-wide, shared across tests: class id -> structural model, or the UNMODELLABLE sentinel + // for + // classes that must fall back to Jacoco. Building a model parses the class once; resolving a + // test's covered lines against it is a cheap set intersection, so a class covered by many tests + // is + // parsed by Jacoco only once (on first encounter, to verify the model) rather than re-running + // Jacoco's Analyzer per (test, class). + private final Map modelCache; private LineCoverageStore( Function probesFactory, CiVisibilityMetricCollector metrics, - SourcePathResolver sourcePathResolver) { + SourcePathResolver sourcePathResolver, + Map modelCache) { super(probesFactory); this.metrics = metrics; this.sourcePathResolver = sourcePathResolver; + this.modelCache = modelCache; } @Nullable @@ -70,6 +89,12 @@ protected TestReport report( Map coveredLinesBySourcePath = new HashMap<>(); for (Map.Entry, ExecutionDataAdapter> e : combinedExecutionData.entrySet()) { ExecutionDataAdapter executionDataAdapter = e.getValue(); + // Back-fill Jacoco's aggregate coverage (total coverage percentage and report uploads). Skip + // classes with no covered probes: the per-test array is allocated on method entry, so a + // method that throws before its first probe fires would otherwise yield an empty entry. + if (!executionDataAdapter.mergeIntoJacocoProbes()) { + continue; + } String className = executionDataAdapter.getClassName(); Class clazz = e.getKey(); @@ -83,24 +108,9 @@ protected TestReport report( } 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); + BitSet coveredLines = analyzeClass(clazz, executionDataAdapter); + if (coveredLines != null) { + coveredLinesBySourcePath.computeIfAbsent(sourcePath, key -> new BitSet()).or(coveredLines); } } @@ -132,12 +142,156 @@ protected TestReport report( return report; } - public static final class Factory implements CoverageStore.Factory { + /** + * Resolves the covered lines for a class given a test's probe activations. Parsing the class with + * Jacoco's {@link Analyzer} is the dominant cost of reporting, and the probe-to-line structure + * depends only on the bytecode (not on which probes a test executed). So a class is parsed once + * into a {@link ClassCoverageModel}; subsequent tests resolve their covered lines against the + * model with a cheap set intersection. + * + *

Correctness of the model logic is established offline (see {@code + * LineCoverageModelOracleTest}, which differential-tests it against Jacoco). As runtime + * defense-in-depth, on first encounter the class is analyzed by Jacoco (authoritative, and the + * result returned for that test) and the model checked against Jacoco for the empty, full, and + * observed probe arrays; if it disagrees — or building it throws — the class is cached as {@link + * ClassCoverageModel#UNMODELLABLE} and always analyzed by Jacoco thereafter. Model build/verify + * failures never discard Jacoco's already-computed result. Note the battery is a sanity check, + * not a proof of equality for every array — that assurance comes from the offline oracle. + * + * @return the covered lines, or {@code null} if the class could not be analyzed by Jacoco + */ + @Nullable + private BitSet analyzeClass(Class clazz, ExecutionDataAdapter executionDataAdapter) { + long classId = executionDataAdapter.getClassId(); + boolean[] probes = executionDataAdapter.getProbeActivations(); + + ClassCoverageModel model = modelCache.get(classId); + if (model != null && model != ClassCoverageModel.UNMODELLABLE) { + return model.coveredLines(probes); + } + + byte[] classBytes = readClassBytes(clazz); + if (classBytes == null) { + log.debug( + "Skipping coverage reporting for {} because its bytecode is unavailable", + executionDataAdapter.getClassName()); + metrics.add(CiVisibilityCountMetric.CODE_COVERAGE_ERRORS, 1); + return null; + } + + // Jacoco is authoritative: compute (and return) its result regardless of what happens with the + // model, so a model build/verify failure can never lose coverage Jacoco handled successfully. + BitSet jacocoCoveredLines; + try { + jacocoCoveredLines = + analyzeWithJacoco(classBytes, classId, executionDataAdapter.getClassName(), probes); + } catch (Exception exception) { + log.debug( + "Skipping coverage reporting for {} because of error", + executionDataAdapter.getClassName(), + exception); + metrics.add(CiVisibilityCountMetric.CODE_COVERAGE_ERRORS, 1); + return null; + } + + // First encounter (not the UNMODELLABLE sentinel): decide once, atomically, whether this class + // gets a model. computeIfAbsent makes the decision happen exactly once per class id even under + // concurrent reports, so a class can never end up both modelled and unmodellable. + if (model == null && modelCache.size() < MAX_MODEL_CACHE_ENTRIES) { + String className = executionDataAdapter.getClassName(); + modelCache.computeIfAbsent( + classId, k -> decideModel(classBytes, classId, className, probes, jacocoCoveredLines)); + } + return jacocoCoveredLines; + } + + /** + * Builds and verifies a model for a class; returns it if trustworthy, else {@link + * ClassCoverageModel#UNMODELLABLE}. Runs under {@code computeIfAbsent}, so it executes once per + * class id. + */ + private static ClassCoverageModel decideModel( + byte[] classBytes, + long classId, + String className, + boolean[] observed, + BitSet jacocoObserved) { + try { + ClassCoverageModel built = ClassCoverageModel.build(classBytes); + if (modelMatchesJacoco(built, classBytes, classId, className, observed, jacocoObserved)) { + return built; + } + log.debug( + "Coverage model did not match Jacoco for {}, falling back to Jacoco analysis", className); + } catch (Exception exception) { + log.debug( + "Could not build coverage model for {}, falling back to Jacoco analysis", className); + } + return ClassCoverageModel.UNMODELLABLE; + } - private final Map probeCounts = new ConcurrentHashMap<>(); + /** + * Checks the model against Jacoco for the observed probe array plus the empty and full arrays. + * The full array pins the exact set of coverable lines and the observed array a real case; this + * is a cheap gross-error guard, complementing the exhaustive offline oracle. + */ + private static boolean modelMatchesJacoco( + ClassCoverageModel built, + byte[] classBytes, + long classId, + String className, + boolean[] observed, + BitSet jacocoObserved) { + try { + if (!built.matches(observed, jacocoObserved)) { + return false; + } + boolean[] all = new boolean[observed.length]; + java.util.Arrays.fill(all, true); + if (!built.matches(all, analyzeWithJacoco(classBytes, classId, className, all))) { + return false; + } + boolean[] none = new boolean[observed.length]; + return built.matches(none, analyzeWithJacoco(classBytes, classId, className, none)); + } catch (Exception e) { + return false; + } + } + + /** Runs Jacoco's {@link Analyzer} for a class and probe array, returning the covered lines. */ + private static BitSet analyzeWithJacoco( + byte[] classBytes, long classId, String className, boolean[] probes) throws IOException { + BitSet coveredLines = new BitSet(); + ExecutionDataStore store = new ExecutionDataStore(); + store.put(new ExecutionData(classId, className, probes)); + Analyzer analyzer = new Analyzer(store, new SourceAnalyzer(coveredLines)); + analyzer.analyzeClass(classBytes, className); + return coveredLines; + } + + @Nullable + private static byte[] readClassBytes(Class clazz) { + try (InputStream is = Utils.getClassStream(clazz)) { + if (is == null) { + return null; + } + ByteArrayOutputStream bos = new ByteArrayOutputStream(); + byte[] buf = new byte[8192]; + int n; + while ((n = is.read(buf)) > 0) { + bos.write(buf, 0, n); + } + return bos.toByteArray(); + } catch (IOException e) { + return null; + } + } + + public static final class Factory implements CoverageStore.Factory { private final CiVisibilityMetricCollector metrics; private final SourcePathResolver sourcePathResolver; + private final Map modelCache = new ConcurrentHashMap<>(); public Factory(CiVisibilityMetricCollector metrics, SourcePathResolver sourcePathResolver) { this.metrics = metrics; @@ -146,16 +300,11 @@ public Factory(CiVisibilityMetricCollector metrics, SourcePathResolver sourcePat @Override public CoverageStore create(@Nullable TestIdentifier testIdentifier) { - return new LineCoverageStore(this::createProbes, metrics, sourcePathResolver); + return new LineCoverageStore(this::createProbes, metrics, sourcePathResolver, modelCache); } private LineProbes createProbes(boolean isTestThread) { - return new LineProbes(metrics, probeCounts, isTestThread); - } - - @Override - public void setTotalProbeCount(String className, int totalProbeCount) { - probeCounts.put(className.replace('/', '.'), totalProbeCount); + return new LineProbes(metrics, isTestThread); } } } diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/coverage/line/LineProbes.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/coverage/line/LineProbes.java index f99c78e2c29..f96c0b5f435 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/coverage/line/LineProbes.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/coverage/line/LineProbes.java @@ -19,7 +19,6 @@ public class LineProbes implements CoverageProbes { private final CiVisibilityMetricCollector metrics; - private final Map probeCounts; private final Map, ExecutionDataAdapter> executionData; private final Map nonCodeResources; @@ -27,10 +26,8 @@ public class LineProbes implements CoverageProbes { private Class lastCoveredClass; private ExecutionDataAdapter lastCoveredExecutionData; - LineProbes( - CiVisibilityMetricCollector metrics, Map probeCounts, boolean isTestThread) { + LineProbes(CiVisibilityMetricCollector metrics, boolean isTestThread) { this.metrics = metrics; - this.probeCounts = probeCounts; executionData = isTestThread ? new IdentityHashMap<>() : new ConcurrentHashMap<>(); nonCodeResources = isTestThread ? new HashMap<>() : new ConcurrentHashMap<>(); } @@ -41,20 +38,21 @@ public void record(Class clazz) { } @Override - public void record(Class clazz, long classId, int probeId) { + public boolean[] resolveProbeArray(Class clazz, long classId, boolean[] jacocoProbes) { try { if (lastCoveredClass != clazz) { - // optimization to avoid map lookup if activating several probes for same class in a row + // optimization to avoid map lookup if resolving the array for the same class in a row lastCoveredExecutionData = executionData.computeIfAbsent( lastCoveredClass = clazz, - k -> new ExecutionDataAdapter(classId, k.getName(), probeCounts.get(k.getName()))); + k -> new ExecutionDataAdapter(classId, k.getName(), jacocoProbes)); } - lastCoveredExecutionData.record(probeId); + return lastCoveredExecutionData.getProbeActivations(); } catch (Exception e) { metrics.add(CiVisibilityCountMetric.CODE_COVERAGE_ERRORS, 1, CoverageErrorType.RECORD); - throw e; + // fall back to Jacoco's shared array so coverage is still recorded + return jacocoProbes; } } diff --git a/dd-java-agent/agent-ci-visibility/src/test/java/datadog/trace/civisibility/coverage/line/ExecutionDataAdapterTest.java b/dd-java-agent/agent-ci-visibility/src/test/java/datadog/trace/civisibility/coverage/line/ExecutionDataAdapterTest.java new file mode 100644 index 00000000000..44d5708fcf2 --- /dev/null +++ b/dd-java-agent/agent-ci-visibility/src/test/java/datadog/trace/civisibility/coverage/line/ExecutionDataAdapterTest.java @@ -0,0 +1,71 @@ +package datadog.trace.civisibility.coverage.line; + +import static org.junit.jupiter.api.Assertions.assertArrayEquals; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import org.junit.jupiter.api.Test; + +class ExecutionDataAdapterTest { + + @Test + void probeActivationsAreSizedFromJacocoArrayAndStartEmpty() { + boolean[] jacocoProbes = new boolean[4]; + ExecutionDataAdapter adapter = new ExecutionDataAdapter(1L, "com/example/Foo", jacocoProbes); + + boolean[] probeActivations = adapter.getProbeActivations(); + assertEquals(4, probeActivations.length); + assertArrayEquals(new boolean[4], probeActivations); + } + + @Test + void mergeIntoJacocoProbesOrsPerTestBitsBack() { + boolean[] jacocoProbes = new boolean[4]; + // a bit that was already set on the shared array (e.g. covered outside any test) + jacocoProbes[0] = true; + ExecutionDataAdapter adapter = new ExecutionDataAdapter(1L, "com/example/Foo", jacocoProbes); + + // simulate Jacoco's native probes writing into the per-test array + adapter.getProbeActivations()[2] = true; + + adapter.mergeIntoJacocoProbes(); + + // existing shared bit is preserved, per-test bit is folded back, untouched probes stay false + assertArrayEquals(new boolean[] {true, false, true, false}, jacocoProbes); + } + + @Test + void mergeIntoJacocoProbesNeverClearsBits() { + boolean[] jacocoProbes = new boolean[] {true, true, true, true}; + ExecutionDataAdapter adapter = new ExecutionDataAdapter(1L, "com/example/Foo", jacocoProbes); + // per-test array is all false; merging it back must not clear any shared bits + adapter.mergeIntoJacocoProbes(); + assertArrayEquals(new boolean[] {true, true, true, true}, jacocoProbes); + } + + @Test + void mergeCombinesProbeActivationsFromAnotherAdapter() { + boolean[] jacocoProbes = new boolean[4]; + ExecutionDataAdapter a = new ExecutionDataAdapter(1L, "com/example/Foo", jacocoProbes); + ExecutionDataAdapter b = new ExecutionDataAdapter(1L, "com/example/Foo", jacocoProbes); + a.getProbeActivations()[1] = true; + b.getProbeActivations()[3] = true; + + ExecutionDataAdapter merged = a.merge(b); + + assertTrue(merged.getProbeActivations()[1]); + assertTrue(merged.getProbeActivations()[3]); + assertFalse(merged.getProbeActivations()[0]); + } + + @Test + void toExecutionDataExposesPerTestProbes() { + boolean[] jacocoProbes = new boolean[3]; + ExecutionDataAdapter adapter = new ExecutionDataAdapter(7L, "com/example/Foo", jacocoProbes); + adapter.getProbeActivations()[1] = true; + + assertEquals(7L, adapter.toExecutionData().getId()); + assertArrayEquals(new boolean[] {false, true, false}, adapter.toExecutionData().getProbes()); + } +} diff --git a/dd-java-agent/agent-ci-visibility/src/test/java/datadog/trace/civisibility/coverage/line/LineCoverageModelOracleTest.java b/dd-java-agent/agent-ci-visibility/src/test/java/datadog/trace/civisibility/coverage/line/LineCoverageModelOracleTest.java new file mode 100644 index 00000000000..b66777232ae --- /dev/null +++ b/dd-java-agent/agent-ci-visibility/src/test/java/datadog/trace/civisibility/coverage/line/LineCoverageModelOracleTest.java @@ -0,0 +1,430 @@ +package datadog.trace.civisibility.coverage.line; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.io.ByteArrayOutputStream; +import java.io.File; +import java.io.IOException; +import java.io.InputStream; +import java.util.ArrayList; +import java.util.BitSet; +import java.util.Enumeration; +import java.util.List; +import java.util.Random; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; +import java.util.jar.JarEntry; +import java.util.jar.JarFile; +import org.jacoco.core.analysis.Analyzer; +import org.jacoco.core.data.ExecutionData; +import org.jacoco.core.data.ExecutionDataStore; +import org.jacoco.core.internal.data.CRC64; +import org.junit.jupiter.api.Test; + +/** + * Oracle for the per-class structural coverage model ({@link ClassCoverageModel}, SDTEST-3847 + * follow-up #1). + * + *

Covered lines computed by Jacoco's real {@link Analyzer} are the source of truth. The + * structural model (parse a class once, evaluate per-test probe arrays cheaply) must produce + * identical results for every probe array. Both are monotone "OR over a fixed probe set" + * per line, so they agree on all arrays iff their per-line probe sets are equal — which is why the + * runtime single-array check is only a sanity guard and this offline differential test is the real + * correctness gate. It pins the equality via: + * + *

+ */ +class LineCoverageModelOracleTest { + + /** Sample with varied control flow so the reference exercises diverse Jacoco analysis. */ + @SuppressWarnings("unused") + static final class Sample { + int sequential(int a) { + int x = a + 1; + return x * 2; + } + + int branch(int a) { + if (a > 0) { + return a; + } else { + return -a; + } + } + + int loop(int n) { + int sum = 0; + for (int i = 0; i < n; i++) { + sum += i; + } + return sum; + } + + int switchStmt(int a) { + switch (a) { + case 1: + return 10; + case 2: + return 20; + default: + return 0; + } + } + + int tryCatch(int a) { + try { + return 100 / a; + } catch (ArithmeticException e) { + return -1; + } + } + } + + /** Constructs that trigger Jacoco's filters (synchronized, try-with-resources, string switch). */ + @SuppressWarnings("unused") + static final class FilteredSample { + private final Lock lock = new ReentrantLock(); + + int synchronizedBlock(int a) { + lock.lock(); + try { + return a * 2; + } finally { + lock.unlock(); + } + } + + int tryWithResources(InputStream in) throws IOException { + try (InputStream stream = in) { + return stream.read(); + } + } + + int stringSwitch(String s) { + switch (s) { + case "a": + return 1; + case "b": + return 2; + default: + return 0; + } + } + + int ternaryAndBoolean(int a, int b) { + boolean r = a > 0 && b > 0 || a == b; + return r ? a : b; + } + + Runnable lambda(int a) { + return () -> { + int x = a + 1; + System.out.println(x); + }; + } + } + + /** Further constructs with special Jacoco flow/filter handling. */ + @SuppressWarnings("unused") + static final class MoreConstructs { + enum Color { + RED, + GREEN, + BLUE + } + + int enumSwitch(Color c) { + switch (c) { + case RED: + return 1; + case GREEN: + return 2; + default: + return 3; + } + } + + int finallyWithReturn(int a) { + try { + if (a < 0) { + return -1; + } + return a; + } finally { + System.out.println("done"); + } + } + + int nestedTryCatch(int a) { + try { + try { + return 10 / a; + } catch (ArithmeticException e) { + return -1; + } + } finally { + System.out.println("outer"); + } + } + + int multiCatch(String s) { + try { + return Integer.parseInt(s) + s.charAt(5); + } catch (NumberFormatException | IndexOutOfBoundsException e) { + return -1; + } + } + + int tryWithMultipleResources(InputStream a, InputStream b) throws IOException { + try (InputStream x = a; + InputStream y = b) { + return x.read() + y.read(); + } + } + + int labeledBreak(int[][] grid, int target) { + int found = -1; + outer: + for (int i = 0; i < grid.length; i++) { + for (int j = 0; j < grid[i].length; j++) { + if (grid[i][j] == target) { + found = i; + break outer; + } + } + } + return found; + } + + int withAssert(int a) { + assert a >= 0 : "negative"; + return a * 2; + } + + int enhancedFor(List items) { + int sum = 0; + for (int v : items) { + sum += v; + } + return sum; + } + + Runnable nestedCapturingLambda(int a) { + return () -> { + Runnable inner = + () -> { + int x = a + 1; + System.out.println(x); + }; + inner.run(); + }; + } + + synchronized int synchronizedMethod(int a) { + return a + 1; + } + } + + // Generously oversized: Jacoco's analysis reads probes[id] for id < the class' real probe count + // and ignores the rest, so the exact count is not needed for the reference computation. + private static final int PROBES = 1024; + + private static final List> SAMPLES = new ArrayList<>(); + + static { + SAMPLES.add(Sample.class); + SAMPLES.add(FilteredSample.class); + SAMPLES.add(MoreConstructs.class); + } + + /** Reference covered-lines for class bytes given a probe array, via Jacoco's {@link Analyzer}. */ + static BitSet referenceCoveredLines(byte[] bytes, String vmName, boolean[] probes) + throws Exception { + long classId = CRC64.classId(bytes); + ExecutionDataStore store = new ExecutionDataStore(); + store.put(new ExecutionData(classId, vmName, probes.clone())); + BitSet coveredLines = new BitSet(); + new Analyzer(store, new SourceAnalyzer(coveredLines)).analyzeClass(bytes, vmName); + return coveredLines; + } + + static BitSet referenceCoveredLines(Class clazz, boolean[] probes) throws Exception { + return referenceCoveredLines(readBytecode(clazz), clazz.getName().replace('.', '/'), probes); + } + + private static byte[] readBytecode(Class clazz) throws Exception { + try (InputStream is = + clazz.getResourceAsStream("/" + clazz.getName().replace('.', '/') + ".class")) { + return readAll(is); + } + } + + private static byte[] readAll(InputStream is) throws IOException { + ByteArrayOutputStream bos = new ByteArrayOutputStream(); + byte[] buf = new byte[8192]; + int n; + while ((n = is.read(buf)) > 0) { + bos.write(buf, 0, n); + } + return bos.toByteArray(); + } + + private static boolean[] randomProbes(Random random) { + boolean[] p = new boolean[PROBES]; + for (int i = 0; i < p.length; i++) { + p[i] = random.nextBoolean(); + } + return p; + } + + private static boolean[] filled(boolean value) { + boolean[] p = new boolean[PROBES]; + java.util.Arrays.fill(p, value); + return p; + } + + @Test + void modelMatchesJacocoForAllProbeArrays() throws Exception { + Random random = new Random(123); + for (Class clazz : SAMPLES) { + byte[] bytes = readBytecode(clazz); + String vmName = clazz.getName().replace('.', '/'); + ClassCoverageModel model = ClassCoverageModel.build(bytes); + + assertEquals( + referenceCoveredLines(bytes, vmName, filled(false)), + model.coveredLines(filled(false)), + () -> clazz.getName() + " (no probes)"); + assertEquals( + referenceCoveredLines(bytes, vmName, filled(true)), + model.coveredLines(filled(true)), + () -> clazz.getName() + " (all probes)"); + + for (int trial = 0; trial < 200; trial++) { + boolean[] probes = randomProbes(random); + assertEquals( + referenceCoveredLines(bytes, vmName, probes), + model.coveredLines(probes), + () -> clazz.getName() + " (random probes)"); + } + } + } + + @Test + void modelMatchesJacocoForUnitProbeArrays() throws Exception { + // Per-probe (unit) arrays pin the exact per-line probe-set membership, not just line presence. + for (Class clazz : SAMPLES) { + byte[] bytes = readBytecode(clazz); + String vmName = clazz.getName().replace('.', '/'); + ClassCoverageModel model = ClassCoverageModel.build(bytes); + for (int i = 0; i < 128; i++) { + boolean[] unit = new boolean[PROBES]; + unit[i] = true; + final int probeId = i; + assertEquals( + referenceCoveredLines(bytes, vmName, unit), + model.coveredLines(unit), + () -> clazz.getName() + " (unit probe " + probeId + ")"); + } + } + } + + @Test + void modelMatchesJacocoAcrossJacocoCoreJar() throws Exception { + File jar = jarContaining(Analyzer.class); + org.junit.jupiter.api.Assumptions.assumeTrue( + jar != null && jar.isFile(), "jacoco-core is not a jar on the classpath"); + + Random random = new Random(2027); + int tested = 0; + int skipped = 0; + try (JarFile jarFile = new JarFile(jar)) { + Enumeration entries = jarFile.entries(); + while (entries.hasMoreElements()) { + JarEntry entry = entries.nextElement(); + String name = entry.getName(); + if (!name.endsWith(".class") + || name.endsWith("module-info.class") + || name.endsWith("package-info.class")) { + continue; + } + byte[] bytes; + try (InputStream is = jarFile.getInputStream(entry)) { + bytes = readAll(is); + } + String vmName = name.substring(0, name.length() - ".class".length()); + + ClassCoverageModel model; + BitSet refAll; + try { + model = ClassCoverageModel.build(bytes); + // all-true both anchors the coverable-line universe and confirms the class is analyzable + refAll = referenceCoveredLines(bytes, vmName, filled(true)); + } catch (Exception e) { + // classes Jacoco or the model cannot process are handled by runtime fallback; skip here + skipped++; + continue; + } + + assertEquals(refAll, model.coveredLines(filled(true)), () -> vmName + " (all probes)"); + assertEquals( + referenceCoveredLines(bytes, vmName, filled(false)), + model.coveredLines(filled(false)), + () -> vmName + " (no probes)"); + for (int trial = 0; trial < 8; trial++) { + boolean[] probes = randomProbes(random); + assertEquals( + referenceCoveredLines(bytes, vmName, probes), + model.coveredLines(probes), + () -> vmName + " (random probes)"); + } + tested++; + } + } + assertTrue(tested > 50, "expected to differential-test many real classes, got " + tested); + } + + @Test + void allProbesCoverLines() throws Exception { + assertFalse( + referenceCoveredLines(Sample.class, filled(true)).isEmpty(), + "a class with executable code should report covered lines when all probes fire"); + } + + @Test + void coverageIsMonotonicInProbeSet() throws Exception { + Random random = new Random(42); + for (int trial = 0; trial < 50; trial++) { + boolean[] subset = randomProbes(random); + boolean[] superset = subset.clone(); + for (int i = 0; i < superset.length; i++) { + superset[i] |= random.nextBoolean(); + } + BitSet subCovered = referenceCoveredLines(Sample.class, subset); + BitSet superCovered = referenceCoveredLines(Sample.class, superset); + + BitSet intersection = (BitSet) subCovered.clone(); + intersection.and(superCovered); + assertEquals( + subCovered, + intersection, + "lines covered by a probe subset must remain covered by a superset"); + } + } + + private static File jarContaining(Class clazz) { + try { + return new File(clazz.getProtectionDomain().getCodeSource().getLocation().toURI()); + } catch (Exception e) { + return null; + } + } +} diff --git a/dd-java-agent/agent-ci-visibility/src/test/java/datadog/trace/civisibility/coverage/line/LineCoverageStoreTest.java b/dd-java-agent/agent-ci-visibility/src/test/java/datadog/trace/civisibility/coverage/line/LineCoverageStoreTest.java new file mode 100644 index 00000000000..b36facd8b6f --- /dev/null +++ b/dd-java-agent/agent-ci-visibility/src/test/java/datadog/trace/civisibility/coverage/line/LineCoverageStoreTest.java @@ -0,0 +1,69 @@ +package datadog.trace.civisibility.coverage.line; + +import static java.util.Collections.emptyList; +import static java.util.Collections.singletonList; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import datadog.trace.api.DDTraceId; +import datadog.trace.api.civisibility.coverage.CoverageProbes; +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 LineCoverageStoreTest { + + private static final class CoveredClass {} + + @Test + void reportFoldsPerTestCoverageBackIntoJacocoSharedArray() { + CiVisibilityMetricCollector metrics = mock(CiVisibilityMetricCollector.class); + SourcePathResolver sourcePathResolver = mock(SourcePathResolver.class); + // source path resolution is irrelevant here: the aggregate back-fill happens regardless + when(sourcePathResolver.getSourcePaths(any())).thenReturn(emptyList()); + + CoverageStore store = new LineCoverageStore.Factory(metrics, sourcePathResolver).create(null); + CoverageProbes probes = store.getProbes(); + + boolean[] jacocoProbes = new boolean[5]; + boolean[] perTest = probes.resolveProbeArray(CoveredClass.class, 42L, jacocoProbes); + // simulate Jacoco's native probes recording coverage into the per-test array + perTest[3] = true; + + // before report, Jacoco's shared (aggregate) array is untouched + assertFalse(jacocoProbes[3]); + + store.report(DDTraceId.ONE, 1L, 1L); + + // after report the per-test coverage is folded back so Jacoco's aggregate stays accurate + assertTrue(jacocoProbes[3]); + } + + @Test + void reportSkipsClassesWhoseProbeArrayWasResolvedButNeverWritten() { + CiVisibilityMetricCollector metrics = mock(CiVisibilityMetricCollector.class); + SourcePathResolver sourcePathResolver = mock(SourcePathResolver.class); + // resolve the source path so the class would be reported if it were not skipped + when(sourcePathResolver.getSourcePaths(any())) + .thenReturn(singletonList("src/test/java/datadog/smoke/CoveredClass.java")); + + CoverageStore store = new LineCoverageStore.Factory(metrics, sourcePathResolver).create(null); + CoverageProbes probes = store.getProbes(); + // a method was entered (probe array resolved) but it threw before any probe fired + probes.resolveProbeArray(CoveredClass.class, 42L, new boolean[5]); + + boolean coverageGathered = store.report(DDTraceId.ONE, 1L, 1L); + + assertFalse( + coverageGathered, "a test that covered no probes must not produce a coverage report"); + TestReport report = store.getReport(); + if (report != null) { + assertTrue(report.getTestReportFileEntries().isEmpty()); + } + } +} diff --git a/dd-java-agent/agent-ci-visibility/src/test/java/datadog/trace/civisibility/coverage/line/LineProbesTest.java b/dd-java-agent/agent-ci-visibility/src/test/java/datadog/trace/civisibility/coverage/line/LineProbesTest.java new file mode 100644 index 00000000000..7ea3bb5a65e --- /dev/null +++ b/dd-java-agent/agent-ci-visibility/src/test/java/datadog/trace/civisibility/coverage/line/LineProbesTest.java @@ -0,0 +1,67 @@ +package datadog.trace.civisibility.coverage.line; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotSame; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.mock; + +import datadog.trace.api.civisibility.telemetry.CiVisibilityMetricCollector; +import org.junit.jupiter.api.Test; + +class LineProbesTest { + + private static final class ClassA {} + + private static final class ClassB {} + + private final CiVisibilityMetricCollector metrics = mock(CiVisibilityMetricCollector.class); + + @Test + void resolvesAPerTestArraySizedFromJacocoArray() { + LineProbes probes = new LineProbes(metrics, true); + boolean[] jacocoProbes = new boolean[6]; + + boolean[] perTest = probes.resolveProbeArray(ClassA.class, 1L, jacocoProbes); + + assertNotSame(jacocoProbes, perTest, "should not record into Jacoco's shared array"); + assertEquals(jacocoProbes.length, perTest.length); + } + + @Test + void returnsTheSameArrayForRepeatedResolutionsOfTheSameClass() { + LineProbes probes = new LineProbes(metrics, true); + boolean[] jacocoProbes = new boolean[3]; + + boolean[] first = probes.resolveProbeArray(ClassA.class, 1L, jacocoProbes); + boolean[] second = probes.resolveProbeArray(ClassA.class, 1L, jacocoProbes); + + assertSame(first, second); + } + + @Test + void keepsSeparateArraysPerClass() { + LineProbes probes = new LineProbes(metrics, true); + + boolean[] a = probes.resolveProbeArray(ClassA.class, 1L, new boolean[2]); + boolean[] b = probes.resolveProbeArray(ClassB.class, 2L, new boolean[2]); + + assertNotSame(a, b); + assertEquals(2, probes.getExecutionData().size()); + assertTrue(probes.getExecutionData().containsKey(ClassA.class)); + assertTrue(probes.getExecutionData().containsKey(ClassB.class)); + } + + @Test + void perTestWritesDoNotLeakIntoJacocoArrayBeforeReport() { + LineProbes probes = new LineProbes(metrics, true); + boolean[] jacocoProbes = new boolean[4]; + + boolean[] perTest = probes.resolveProbeArray(ClassA.class, 1L, jacocoProbes); + perTest[1] = true; + + // the shared array is only updated at report time via + // ExecutionDataAdapter#mergeIntoJacocoProbes + assertEquals(false, jacocoProbes[1]); + } +} diff --git a/dd-java-agent/instrumentation/jacoco-0.8.9/src/main/java/datadog/trace/instrumentation/jacoco/ClassInstrumenterInstrumentation.java b/dd-java-agent/instrumentation/jacoco-0.8.9/src/main/java/datadog/trace/instrumentation/jacoco/ClassInstrumenterInstrumentation.java deleted file mode 100644 index 7960a0ff40e..00000000000 --- a/dd-java-agent/instrumentation/jacoco-0.8.9/src/main/java/datadog/trace/instrumentation/jacoco/ClassInstrumenterInstrumentation.java +++ /dev/null @@ -1,57 +0,0 @@ -package datadog.trace.instrumentation.jacoco; - -import static datadog.trace.agent.tooling.bytebuddy.matcher.NameMatchers.named; -import static net.bytebuddy.matcher.ElementMatchers.isMethod; -import static net.bytebuddy.matcher.ElementMatchers.nameEndsWith; -import static net.bytebuddy.matcher.ElementMatchers.nameStartsWith; - -import com.google.auto.service.AutoService; -import datadog.trace.agent.tooling.Instrumenter; -import datadog.trace.agent.tooling.InstrumenterModule; -import datadog.trace.api.Config; -import datadog.trace.api.civisibility.coverage.CoveragePerTestBridge; -import net.bytebuddy.asm.Advice; -import net.bytebuddy.description.type.TypeDescription; -import net.bytebuddy.matcher.ElementMatcher; - -@AutoService(InstrumenterModule.class) -public class ClassInstrumenterInstrumentation extends InstrumenterModule.CiVisibility - implements Instrumenter.ForTypeHierarchy, Instrumenter.HasMethodAdvice { - public ClassInstrumenterInstrumentation() { - super("jacoco"); - } - - @Override - public boolean isEnabled() { - return super.isEnabled() && Config.get().isCiVisibilityCoverageLinesEnabled(); - } - - @Override - public String hierarchyMarkerType() { - return "org.jacoco.agent.rt.IAgent"; - } - - @Override - public ElementMatcher hierarchyMatcher() { - // The jacoco javaagent jar that is published relocates internal classes to an "obfuscated" - // package name ex. org.jacoco.agent.rt.internal_72ddf3b.core.internal.instr.ClassInstrumenter - return nameStartsWith("org.jacoco.agent.rt.internal") - .and(nameEndsWith(".core.internal.instr.ClassInstrumenter")); - } - - @Override - public void methodAdvice(MethodTransformer transformer) { - transformer.applyAdvice( - isMethod().and(named("visitTotalProbeCount")), - getClass().getName() + "$VisitTotalProbeCountAdvice"); - } - - public static class VisitTotalProbeCountAdvice { - @Advice.OnMethodEnter(suppress = Throwable.class) - static void enter( - @Advice.FieldValue(value = "className") final String className, - @Advice.Argument(0) int count) { - CoveragePerTestBridge.setTotalProbeCount(className, count); - } - } -} diff --git a/dd-java-agent/instrumentation/jacoco-0.8.9/src/main/java/datadog/trace/instrumentation/jacoco/MethodVisitorWrapper.java b/dd-java-agent/instrumentation/jacoco-0.8.9/src/main/java/datadog/trace/instrumentation/jacoco/MethodVisitorWrapper.java index 8b80d00ce30..53b394ab997 100644 --- a/dd-java-agent/instrumentation/jacoco-0.8.9/src/main/java/datadog/trace/instrumentation/jacoco/MethodVisitorWrapper.java +++ b/dd-java-agent/instrumentation/jacoco-0.8.9/src/main/java/datadog/trace/instrumentation/jacoco/MethodVisitorWrapper.java @@ -17,6 +17,7 @@ public class MethodVisitorWrapper { private static final MethodHandle visitMethodInsnHandle; private static final MethodHandle visitInsnHandle; private static final MethodHandle visitIntInsnHandle; + private static final MethodHandle visitVarInsnHandle; private static final MethodHandle visitLdcInsnHandle; private static final MethodHandle getTypeHandle; @@ -43,6 +44,8 @@ public class MethodVisitorWrapper { visitInsnHandle = accessMethod(lookup, shadedMethodVisitorClass, "visitInsn", int.class); visitIntInsnHandle = accessMethod(lookup, shadedMethodVisitorClass, "visitIntInsn", int.class, int.class); + visitVarInsnHandle = + accessMethod(lookup, shadedMethodVisitorClass, "visitVarInsn", int.class, int.class); visitLdcInsnHandle = accessMethod(lookup, shadedMethodVisitorClass, "visitLdcInsn", Object.class); @@ -97,6 +100,10 @@ public void visitMethodInsn(int opcode, String owner, String name, String desc, visitMethodInsnHandle.invoke(mv, opcode, owner, name, desc, itf); } + public void visitVarInsn(int opcode, int var) throws Throwable { + visitVarInsnHandle.invoke(mv, opcode, var); + } + public void visitLdcInsn(Object cst) throws Throwable { visitLdcInsnHandle.invoke(mv, cst); } diff --git a/dd-java-agent/instrumentation/jacoco-0.8.9/src/main/java/datadog/trace/instrumentation/jacoco/ProbeInserterInstrumentation.java b/dd-java-agent/instrumentation/jacoco-0.8.9/src/main/java/datadog/trace/instrumentation/jacoco/ProbeInserterInstrumentation.java index 695b659d6e1..ad7929de302 100644 --- a/dd-java-agent/instrumentation/jacoco-0.8.9/src/main/java/datadog/trace/instrumentation/jacoco/ProbeInserterInstrumentation.java +++ b/dd-java-agent/instrumentation/jacoco-0.8.9/src/main/java/datadog/trace/instrumentation/jacoco/ProbeInserterInstrumentation.java @@ -112,29 +112,17 @@ public ElementMatcher hierarchyMatcher() { @Override public void methodAdvice(MethodTransformer transformer) { transformer.applyAdvice( - isMethod().and(named("visitMaxs")).and(takesArguments(2)).and(takesArgument(0, int.class)), - getClass().getName() + "$VisitMaxsAdvice"); - transformer.applyAdvice( - isMethod() - .and(named("insertProbe")) - .and(takesArguments(1)) - .and(takesArgument(0, int.class)), - getClass().getName() + "$InsertProbeAdvice"); - } - - public static class VisitMaxsAdvice { - @Advice.OnMethodEnter(suppress = Throwable.class) - static void enter(@Advice.Argument(value = 0, readOnly = false) int maxStack) { - maxStack = maxStack + 2; - } + isMethod().and(named("visitCode")).and(takesArguments(0)), + getClass().getName() + "$VisitCodeAdvice"); } - public static class InsertProbeAdvice { - @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) + public static class VisitCodeAdvice { + @Advice.OnMethodExit(suppress = Throwable.class) static void exit( @Advice.FieldValue(value = "mv") final Object mv, @Advice.FieldValue(value = "arrayStrategy") final Object arrayStrategy, - @Advice.Argument(0) final int id) + @Advice.FieldValue(value = "variable") final int variable, + @Advice.FieldValue(value = "accessorStackSize", readOnly = false) int accessorStackSize) throws Throwable { Field classNameField = arrayStrategy.getClass().getDeclaredField("className"); classNameField.setAccessible(true); @@ -167,16 +155,24 @@ static void exit( MethodVisitorWrapper methodVisitor = MethodVisitorWrapper.wrap(mv); + // Jacoco's storeInstance() has just stored the class' shared probe array into local variable + // `variable`. Swap it for the per-test array so Jacoco's own probe writes + // (probes[id] = true) record per-test coverage with no per-probe overhead. When no test is + // active the bridge returns the shared array unchanged, preserving Jacoco's aggregate. + methodVisitor.visitVarInsn(Opcodes.ALOAD, variable); methodVisitor.pushClass(className); methodVisitor.visitLdcInsn(classId); - methodVisitor.push(id); - methodVisitor.visitMethodInsn( Opcodes.INVOKESTATIC, "datadog/trace/api/civisibility/coverage/CoveragePerTestBridge", - "recordCoverage", - "(Ljava/lang/Class;JI)V", + "resolveProbeArray", + "([ZLjava/lang/Class;J)[Z", false); + methodVisitor.visitVarInsn(Opcodes.ASTORE, variable); + + // the swap leaves 4 slots on the stack (boolean[] + Class + long); Jacoco sizes the method's + // max stack as max(maxStack + 3, accessorStackSize) in visitMaxs + accessorStackSize = Math.max(accessorStackSize, 4); } } } diff --git a/internal-api/src/main/java/datadog/trace/api/civisibility/coverage/CoveragePerTestBridge.java b/internal-api/src/main/java/datadog/trace/api/civisibility/coverage/CoveragePerTestBridge.java index c8f52293eec..744b50b6ac7 100644 --- a/internal-api/src/main/java/datadog/trace/api/civisibility/coverage/CoveragePerTestBridge.java +++ b/internal-api/src/main/java/datadog/trace/api/civisibility/coverage/CoveragePerTestBridge.java @@ -2,73 +2,14 @@ import datadog.trace.api.civisibility.InstrumentationTestBridge; import datadog.trace.api.civisibility.domain.TestContext; -import java.util.ArrayDeque; -import java.util.Queue; -import javax.annotation.Nonnull; -import javax.annotation.concurrent.GuardedBy; public abstract class CoveragePerTestBridge { private static final ThreadLocal COVERAGE_PROBES = new ThreadLocal<>(); - private static volatile CoverageStore.Registry COVERAGE_STORE_REGISTRY; - private static final Object COVERAGE_STORE_REGISTRY_LOCK = new Object(); - - @GuardedBy("COVERAGE_STORE_REGISTRY_LOCK") - private static final Queue DEFERRED_PROBE_COUNTS = new ArrayDeque<>(); - - public static void registerCoverageStoreRegistry( - @Nonnull CoverageStore.Registry coverageStoreRegistry) { - synchronized (COVERAGE_STORE_REGISTRY_LOCK) { - while (!DEFERRED_PROBE_COUNTS.isEmpty()) { - TotalProbeCount c = DEFERRED_PROBE_COUNTS.poll(); - coverageStoreRegistry.setTotalProbeCount(c.className, c.count); - } - COVERAGE_STORE_REGISTRY = coverageStoreRegistry; - } - } - - /** - * {@link #COVERAGE_STORE_REGISTRY} is set when CI Visibility is initialized. It is possible, that - * core/internal JDK classes are loaded and transformed by Jacoco before this happens. As the - * result this method may be called when {@link #COVERAGE_STORE_REGISTRY} is still {@code null}. - * - *

While instrumenting core/internal JDK classes with Jacoco makes little sense, we do not - * always have the control over the users' Jacoco {@code includes} setting, therefore we have to - * account for this case and support it. - * - *

If this method finds {@link #COVERAGE_STORE_REGISTRY} to be {@code null}, the probe counts - * are saved in {@link #DEFERRED_PROBE_COUNTS} to be processed when {@link - * #COVERAGE_STORE_REGISTRY} is set. - */ - public static void setTotalProbeCount(String className, int totalProbeCount) { - if (COVERAGE_STORE_REGISTRY != null) { - COVERAGE_STORE_REGISTRY.setTotalProbeCount(className, totalProbeCount); - return; - } - - synchronized (COVERAGE_STORE_REGISTRY_LOCK) { - if (COVERAGE_STORE_REGISTRY != null) { - COVERAGE_STORE_REGISTRY.setTotalProbeCount(className, totalProbeCount); - } else { - DEFERRED_PROBE_COUNTS.offer(new TotalProbeCount(className, totalProbeCount)); - } - } - } - - private static final class TotalProbeCount { - private final String className; - private final int count; - - private TotalProbeCount(String className, int count) { - this.className = className; - this.count = count; - } - } - - /* This method is referenced by name in bytecode added in jacoco instrumentation module (see datadog.trace.instrumentation.jacoco.ProbeInserterInstrumentation.InsertProbeAdvice) */ - public static void recordCoverage(Class clazz, long classId, int probeId) { - getCurrentCoverageProbes().record(clazz, classId, probeId); + /* This method is referenced by name in bytecode added in jacoco instrumentation module (see datadog.trace.instrumentation.jacoco.ProbeInserterInstrumentation.VisitCodeAdvice) */ + public static boolean[] resolveProbeArray(boolean[] jacocoProbes, Class clazz, long classId) { + return getCurrentCoverageProbes().resolveProbeArray(clazz, classId, jacocoProbes); } /* This method is referenced by name in bytecode added by coverage probes (see datadog.trace.civisibility.coverage.instrumentation.CoverageUtils#insertCoverageProbe) */ diff --git a/internal-api/src/main/java/datadog/trace/api/civisibility/coverage/CoverageProbes.java b/internal-api/src/main/java/datadog/trace/api/civisibility/coverage/CoverageProbes.java index 2bdb5ca2371..54535de4bc6 100644 --- a/internal-api/src/main/java/datadog/trace/api/civisibility/coverage/CoverageProbes.java +++ b/internal-api/src/main/java/datadog/trace/api/civisibility/coverage/CoverageProbes.java @@ -3,7 +3,20 @@ public interface CoverageProbes { void record(Class clazz); - void record(Class clazz, long classId, int probeId); + /** + * Resolves the probe array that Jacoco's instrumentation writes into at runtime. Called once per + * instrumented method invocation, allowing per-test coverage to be captured by swapping Jacoco's + * shared probe array for a test-scoped one. The default returns {@code jacocoProbes} unchanged so + * that, when no per-test store is active, Jacoco's own aggregate coverage keeps working. + * + * @param clazz the class being executed + * @param classId Jacoco's class identifier + * @param jacocoProbes Jacoco's shared probe array for the class + * @return the probe array to record coverage into + */ + default boolean[] resolveProbeArray(Class clazz, long classId, boolean[] jacocoProbes) { + return jacocoProbes; + } void recordNonCodeResource(String absolutePath); } diff --git a/internal-api/src/main/java/datadog/trace/api/civisibility/coverage/CoverageStore.java b/internal-api/src/main/java/datadog/trace/api/civisibility/coverage/CoverageStore.java index a779c699281..cc42d28aa98 100644 --- a/internal-api/src/main/java/datadog/trace/api/civisibility/coverage/CoverageStore.java +++ b/internal-api/src/main/java/datadog/trace/api/civisibility/coverage/CoverageStore.java @@ -13,11 +13,7 @@ public interface CoverageStore extends TestReportHolder { */ boolean report(DDTraceId testSessionId, Long testSuiteId, long testSpanId); - interface Factory extends Registry { + interface Factory { CoverageStore create(@Nullable TestIdentifier testIdentifier); } - - interface Registry { - void setTotalProbeCount(String className, int totalProbeCount); - } } diff --git a/internal-api/src/main/java/datadog/trace/api/civisibility/coverage/NoOpCoverageStore.java b/internal-api/src/main/java/datadog/trace/api/civisibility/coverage/NoOpCoverageStore.java index c2b677f416f..7c709eaff1e 100644 --- a/internal-api/src/main/java/datadog/trace/api/civisibility/coverage/NoOpCoverageStore.java +++ b/internal-api/src/main/java/datadog/trace/api/civisibility/coverage/NoOpCoverageStore.java @@ -31,10 +31,5 @@ public static final class Factory implements CoverageStore.Factory { public CoverageStore create(@Nullable TestIdentifier testIdentifier) { return INSTANCE; } - - @Override - public void setTotalProbeCount(String className, int totalProbeCount) { - // no op - } } } diff --git a/internal-api/src/main/java/datadog/trace/api/civisibility/coverage/NoOpProbes.java b/internal-api/src/main/java/datadog/trace/api/civisibility/coverage/NoOpProbes.java index cdc2142e398..dfd55855250 100644 --- a/internal-api/src/main/java/datadog/trace/api/civisibility/coverage/NoOpProbes.java +++ b/internal-api/src/main/java/datadog/trace/api/civisibility/coverage/NoOpProbes.java @@ -8,9 +8,6 @@ private NoOpProbes() {} @Override public void record(Class clazz) {} - @Override - public void record(Class clazz, long classId, int probeId) {} - @Override public void recordNonCodeResource(String absolutePath) {} }