From 388eb39986bf94e0c8e42bc968f86c2d61e0d1a4 Mon Sep 17 00:00:00 2001 From: Nikita Tkachenko Date: Mon, 23 Mar 2026 14:47:36 +0100 Subject: [PATCH 01/19] Phase 1: instrumentation, coverage collection and logging --- .../java/datadog/trace/bootstrap/Agent.java | 45 +++- .../agent-code-coverage/build.gradle | 29 +++ .../codecoverage/CodeCoverageCollector.java | 147 ++++++++++++ .../codecoverage/CodeCoverageFilter.java | 84 +++++++ .../codecoverage/CodeCoverageSender.java | 19 ++ .../codecoverage/CodeCoverageSystem.java | 62 +++++ .../codecoverage/CodeCoverageTransformer.java | 213 ++++++++++++++++++ .../LoggingCodeCoverageSender.java | 24 ++ dd-java-agent/build.gradle | 1 + .../trace/api/config/CodeCoverageConfig.java | 14 ++ .../main/java/datadog/trace/api/Config.java | 39 ++++ .../datadog/trace/api/InstrumenterConfig.java | 11 + settings.gradle.kts | 5 + 13 files changed, 691 insertions(+), 2 deletions(-) create mode 100644 dd-java-agent/agent-code-coverage/build.gradle create mode 100644 dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageCollector.java create mode 100644 dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageFilter.java create mode 100644 dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageSender.java create mode 100644 dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageSystem.java create mode 100644 dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageTransformer.java create mode 100644 dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/LoggingCodeCoverageSender.java create mode 100644 dd-trace-api/src/main/java/datadog/trace/api/config/CodeCoverageConfig.java diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java index 5288f92dbe3..2c013b934a2 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java @@ -26,6 +26,7 @@ import datadog.instrument.utils.ClassLoaderValue; import datadog.metrics.api.statsd.StatsDClientManager; import datadog.trace.api.Config; +import datadog.trace.api.InstrumenterConfig; import datadog.trace.api.Platform; import datadog.trace.api.WithGlobalTracer; import datadog.trace.api.appsec.AppSecEventTracker; @@ -336,6 +337,11 @@ public static void start( StaticEventLogger.end("crashtracking"); } + Object codeCoverageTransformer = null; + if (InstrumenterConfig.get().isCodeCoverageEnabled()) { + codeCoverageTransformer = maybeStartCodeCoverage(inst); + } + startDatadogAgent(initTelemetry, inst); final EnumSet libraries = detectLibraries(log); @@ -390,7 +396,8 @@ public static void start( } InstallDatadogTracerCallback installDatadogTracerCallback = - new InstallDatadogTracerCallback(initTelemetry, inst, okHttpDelayMillis); + new InstallDatadogTracerCallback( + initTelemetry, inst, okHttpDelayMillis, codeCoverageTransformer); if (waitForJUL) { log.debug("Custom logger detected. Delaying Datadog Tracer initialization."); registerLogManagerCallback(installDatadogTracerCallback); @@ -645,11 +652,14 @@ protected static class InstallDatadogTracerCallback extends ClassLoadCallBack { private final Object sco; private final Class scoClass; private final int okHttpDelayMillis; + private final Object codeCoverageTransformer; public InstallDatadogTracerCallback( InitializationTelemetry initTelemetry, Instrumentation instrumentation, - int okHttpDelayMillis) { + int okHttpDelayMillis, + Object codeCoverageTransformer) { + this.codeCoverageTransformer = codeCoverageTransformer; this.okHttpDelayMillis = okHttpDelayMillis; this.instrumentation = instrumentation; try { @@ -696,6 +706,10 @@ public void execute() { if (flareEnabled) { startFlarePoller(scoClass, sco); } + + if (codeCoverageTransformer != null) { + startCodeCoverageCollector(codeCoverageTransformer); + } } private void resumeRemoteComponents() { @@ -1124,6 +1138,33 @@ private static void maybeStartCiVisibility(Instrumentation inst, Class scoCla } } + private static Object maybeStartCodeCoverage(Instrumentation inst) { + StaticEventLogger.begin("Code Coverage"); + + try { + final Class systemClass = + AGENT_CLASSLOADER.loadClass("datadog.trace.codecoverage.CodeCoverageSystem"); + final Method startMethod = systemClass.getMethod("start", Instrumentation.class); + return startMethod.invoke(null, inst); + } catch (final Throwable e) { + log.warn("Not starting Code Coverage subsystem", e); + return null; + } finally { + StaticEventLogger.end("Code Coverage"); + } + } + + private static void startCodeCoverageCollector(Object transformer) { + try { + final Class systemClass = + AGENT_CLASSLOADER.loadClass("datadog.trace.codecoverage.CodeCoverageSystem"); + final Method startCollectorMethod = systemClass.getMethod("startCollector", Object.class); + startCollectorMethod.invoke(null, transformer); + } catch (final Throwable e) { + log.warn("Not starting Code Coverage collector", e); + } + } + private static void maybeStartLLMObs(Instrumentation inst, Class scoClass, Object sco) { if (llmObsEnabled) { StaticEventLogger.begin("LLM Observability"); diff --git a/dd-java-agent/agent-code-coverage/build.gradle b/dd-java-agent/agent-code-coverage/build.gradle new file mode 100644 index 00000000000..90969b02e3d --- /dev/null +++ b/dd-java-agent/agent-code-coverage/build.gradle @@ -0,0 +1,29 @@ +import com.github.jengelman.gradle.plugins.shadow.tasks.ShadowJar + +plugins { + id 'com.gradleup.shadow' +} + +apply from: "$rootDir/gradle/java.gradle" +apply from: "$rootDir/gradle/version.gradle" + +minimumBranchCoverage = 0.0 +minimumInstructionCoverage = 0.0 + +dependencies { + api libs.slf4j + + implementation group: 'org.jacoco', name: 'org.jacoco.core', version: '0.8.14' + + implementation project(':internal-api') + + testImplementation project(':dd-java-agent:testing') +} + +tasks.named("shadowJar", ShadowJar) { + dependencies deps.excludeShared +} + +tasks.named("jar", Jar) { + archiveClassifier = 'unbundled' +} diff --git a/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageCollector.java b/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageCollector.java new file mode 100644 index 00000000000..2e9872f8ead --- /dev/null +++ b/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageCollector.java @@ -0,0 +1,147 @@ +package datadog.trace.codecoverage; + +import java.io.File; +import java.io.IOException; +import java.util.ArrayList; +import java.util.BitSet; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.concurrent.Executors; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.TimeUnit; +import org.jacoco.core.analysis.Analyzer; +import org.jacoco.core.analysis.CoverageBuilder; +import org.jacoco.core.analysis.IClassCoverage; +import org.jacoco.core.analysis.ICounter; +import org.jacoco.core.data.ExecutionDataStore; +import org.jacoco.core.data.SessionInfoStore; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Periodically collects code coverage probe data, resolves it to covered source lines using + * JaCoCo's analysis pipeline, and sends the results via a {@link CodeCoverageSender}. + */ +public final class CodeCoverageCollector { + + private static final Logger log = LoggerFactory.getLogger(CodeCoverageCollector.class); + + private final CodeCoverageTransformer transformer; + private final CodeCoverageSender sender; + private final int intervalSeconds; + private final String explicitClasspath; + private volatile ScheduledExecutorService scheduler; + + /** + * @param transformer the transformer that holds runtime probe data + * @param sender the sender to deliver coverage results to + * @param intervalSeconds interval between collection cycles + * @param explicitClasspath explicit classpath override (nullable; if null, auto-detected) + */ + public CodeCoverageCollector( + CodeCoverageTransformer transformer, + CodeCoverageSender sender, + int intervalSeconds, + String explicitClasspath) { + this.transformer = transformer; + this.sender = sender; + this.intervalSeconds = intervalSeconds; + this.explicitClasspath = explicitClasspath; + } + + /** Starts the periodic collection scheduler. */ + public void start() { + scheduler = + Executors.newSingleThreadScheduledExecutor( + r -> { + Thread t = new Thread(r, "dd-code-coverage"); + t.setDaemon(true); + return t; + }); + scheduler.scheduleAtFixedRate(this::collect, intervalSeconds, intervalSeconds, TimeUnit.SECONDS); + log.debug( + "Code coverage collector started with interval of {} seconds", + intervalSeconds); + } + + /** Stops the periodic collection scheduler. */ + public void stop() { + ScheduledExecutorService s = scheduler; + if (s != null) { + s.shutdownNow(); + } + } + + /** Performs a single collection cycle: collect probes, analyze, and send. */ + void collect() { + try { + // 1. Collect and reset probes + ExecutionDataStore execStore = new ExecutionDataStore(); + SessionInfoStore sessionStore = new SessionInfoStore(); + transformer.collectAndReset(execStore, sessionStore); + + // 2. Resolve classpath entries + List classpathEntries = resolveClasspath(); + + // 3. Analyze: map probes to source lines using original class files + CoverageBuilder builder = new CoverageBuilder(); + Analyzer analyzer = new Analyzer(execStore, builder); + for (File entry : classpathEntries) { + if (entry.exists()) { + try { + analyzer.analyzeAll(entry); + } catch (IOException e) { + log.debug("Failed to analyze classpath entry: {}", entry, e); + } + } + } + + // 4. Build coverage map: source file -> covered line numbers + Map coverage = new HashMap<>(); + for (IClassCoverage cc : builder.getClasses()) { + if (cc.getSourceFileName() == null) { + continue; + } + String sourceFile = cc.getPackageName() + "/" + cc.getSourceFileName(); + BitSet lines = coverage.computeIfAbsent(sourceFile, k -> new BitSet()); + for (int line = cc.getFirstLine(); line <= cc.getLastLine(); line++) { + int status = cc.getLine(line).getStatus(); + if (status == ICounter.PARTLY_COVERED || status == ICounter.FULLY_COVERED) { + lines.set(line); + } + } + } + + // 5. Send if there is data + if (!coverage.isEmpty()) { + sender.send(coverage); + } + } catch (Exception e) { + log.debug("Error during code coverage collection", e); + } + } + + /** + * Resolves classpath entries to analyze. If an explicit classpath is configured, it takes + * precedence. Otherwise, falls back to {@code java.class.path} system property. + */ + private List resolveClasspath() { + String cp; + if (explicitClasspath != null && !explicitClasspath.isEmpty()) { + cp = explicitClasspath; + } else { + cp = System.getProperty("java.class.path"); + } + List entries = new ArrayList<>(); + if (cp != null && !cp.isEmpty()) { + for (String path : cp.split(File.pathSeparator)) { + String trimmed = path.trim(); + if (!trimmed.isEmpty()) { + entries.add(new File(trimmed)); + } + } + } + return entries; + } +} diff --git a/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageFilter.java b/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageFilter.java new file mode 100644 index 00000000000..204335a6b8b --- /dev/null +++ b/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageFilter.java @@ -0,0 +1,84 @@ +package datadog.trace.codecoverage; + +import java.util.function.Predicate; + +/** + * Determines whether a class should be instrumented for production code coverage based on + * include/exclude patterns. + */ +public final class CodeCoverageFilter implements Predicate { + + private final String[] includePrefixes; + private final String[] excludePrefixes; + private final boolean includeAll; + + /** + * @param includes include patterns (e.g. {@code ["com.example.*", "*"]}). A single {@code "*"} + * means include everything. + * @param excludes exclude patterns (e.g. {@code ["com.example.internal.*"]}) + */ + public CodeCoverageFilter(String[] includes, String[] excludes) { + this.includeAll = includes.length == 1 && "*".equals(includes[0]); + this.includePrefixes = toVmPrefixes(includes); + this.excludePrefixes = toVmPrefixes(excludes); + } + + /** + * @param className class name in VM format (e.g. {@code "com/example/MyClass"}) + * @return {@code true} if the class should be instrumented + */ + @Override + public boolean test(String className) { + // Always reject agent internals + if (className.startsWith("datadog/")) { + return false; + } + + // Check excludes first + for (String excludePrefix : excludePrefixes) { + if (className.startsWith(excludePrefix)) { + return false; + } + } + + if (includeAll) { + return true; + } + + // Check includes + for (String includePrefix : includePrefixes) { + if (className.startsWith(includePrefix)) { + return true; + } + } + + return false; + } + + /** + * Converts dot-separated patterns like {@code "com.example.*"} to VM-format prefixes like {@code + * "com/example/"}. + */ + private static String[] toVmPrefixes(String[] patterns) { + if (patterns == null || patterns.length == 0) { + return new String[0]; + } + String[] prefixes = new String[patterns.length]; + for (int i = 0; i < patterns.length; i++) { + String pattern = patterns[i].trim(); + if ("*".equals(pattern)) { + prefixes[i] = ""; + continue; + } + // Strip trailing wildcard + if (pattern.endsWith(".*") || pattern.endsWith("/*")) { + pattern = pattern.substring(0, pattern.length() - 1); + } else if (pattern.endsWith("*")) { + pattern = pattern.substring(0, pattern.length() - 1); + } + // Convert dots to slashes + prefixes[i] = pattern.replace('.', '/'); + } + return prefixes; + } +} diff --git a/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageSender.java b/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageSender.java new file mode 100644 index 00000000000..987575408ef --- /dev/null +++ b/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageSender.java @@ -0,0 +1,19 @@ +package datadog.trace.codecoverage; + +import java.util.BitSet; +import java.util.Map; + +/** + * Interface for sending collected code coverage data to a backend. Phase 1 uses a logging stub; + * future phases will implement real sending to Datadog. + */ +public interface CodeCoverageSender { + + /** + * Sends coverage data. + * + * @param coverage map from source file path (e.g. {@code "com/example/MyClass.java"}) to set of + * covered line numbers + */ + void send(Map coverage); +} diff --git a/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageSystem.java b/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageSystem.java new file mode 100644 index 00000000000..0d96f6a68b0 --- /dev/null +++ b/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageSystem.java @@ -0,0 +1,62 @@ +package datadog.trace.codecoverage; + +import datadog.trace.api.Config; +import java.lang.instrument.Instrumentation; +import java.util.function.Predicate; + +/** + * Entry point for the production code coverage product module. + * + *

Follows the tracer's standard product system pattern with a two-phase start: + * + *

    + *
  1. {@link #start(Instrumentation)} — called during premain, before ByteBuddy's + * transformer is registered. Must not use logging, NIO, or JMX. + *
  2. {@link #startCollector(Object)} — called from a deferred callback after premain, when + * logging and thread scheduling are safe. + *
+ */ +public final class CodeCoverageSystem { + + /** + * Phase 1: registers the coverage {@link java.lang.instrument.ClassFileTransformer}. + * + *

Called during premain, synchronously, before ByteBuddy. The returned object is an opaque + * handle to the transformer, passed to {@link #startCollector(Object)} later. + * + * @param inst the JVM instrumentation service + * @return the transformer instance (opaque; passed to {@link #startCollector}) + * @throws Exception if JaCoCo runtime initialization fails + */ + public static Object start(Instrumentation inst) throws Exception { + Config config = Config.get(); + String[] includes = config.getCodeCoverageIncludes(); + String[] excludes = config.getCodeCoverageExcludes(); + Predicate filter = new CodeCoverageFilter(includes, excludes); + CodeCoverageTransformer transformer = new CodeCoverageTransformer(inst, filter); + inst.addTransformer(transformer); + return transformer; + } + + /** + * Phase 2: starts the periodic coverage collector. + * + *

Called from a deferred callback after premain. Safe to use logging and thread scheduling. + * + * @param transformerObj the opaque transformer handle returned by {@link #start} + */ + public static void startCollector(Object transformerObj) { + CodeCoverageTransformer transformer = (CodeCoverageTransformer) transformerObj; + Config config = Config.get(); + CodeCoverageSender sender = new LoggingCodeCoverageSender(); + CodeCoverageCollector collector = + new CodeCoverageCollector( + transformer, + sender, + config.getCodeCoverageReportIntervalSeconds(), + config.getCodeCoverageClasspath()); + collector.start(); + } + + private CodeCoverageSystem() {} +} diff --git a/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageTransformer.java b/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageTransformer.java new file mode 100644 index 00000000000..a83fa3611db --- /dev/null +++ b/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageTransformer.java @@ -0,0 +1,213 @@ +package datadog.trace.codecoverage; + +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.InputStream; +import java.lang.instrument.ClassFileTransformer; +import java.lang.instrument.Instrumentation; +import java.security.ProtectionDomain; +import java.util.Collections; +import java.util.HashSet; +import java.util.Map; +import java.util.Set; +import java.util.function.Predicate; +import org.jacoco.core.data.ExecutionDataReader; +import org.jacoco.core.data.ExecutionDataStore; +import org.jacoco.core.data.ExecutionDataWriter; +import org.jacoco.core.data.SessionInfoStore; +import org.jacoco.core.instr.Instrumenter; +import org.jacoco.core.runtime.IRuntime; +import org.jacoco.core.runtime.InjectedClassRuntime; +import org.jacoco.core.runtime.RuntimeData; + +/** + * A {@link ClassFileTransformer} that uses JaCoCo's {@link Instrumenter} to insert boolean probes + * into class bytecode at load time. + * + *

Must be registered before ByteBuddy's transformer so that JaCoCo sees original class + * bytes (CRC64 must match the {@code .class} files on disk for analysis to work). + */ +public final class CodeCoverageTransformer implements ClassFileTransformer { + + private final RuntimeData runtimeData; + private final Instrumenter instrumenter; + private final Predicate filter; + + /** + * Initializes the JaCoCo runtime and instrumenter. + * + *

This replicates the logic from JaCoCo's {@code AgentModule} and {@code PreMain}: it creates + * an isolated classloader, opens {@code java.lang} to it via {@code + * Instrumentation.redefineModule}, loads {@link InjectedClassRuntime} in that module, and starts + * the runtime. + * + * @param inst the JVM instrumentation service + * @param filter predicate that decides which classes to instrument (VM class name format) + * @throws Exception if the JaCoCo runtime cannot be initialized + */ + public CodeCoverageTransformer(Instrumentation inst, Predicate filter) throws Exception { + this.filter = filter; + this.runtimeData = new RuntimeData(); + + // Replicate AgentModule logic: create isolated classloader and open java.lang to it + Set scope = new HashSet<>(); + addToScopeWithInnerClasses(InjectedClassRuntime.class, scope); + + // Use the classloader that has the (shaded) JaCoCo classes as the resource source and parent. + // The parent provides access to AbstractRuntime, IRuntime, RuntimeData, ASM classes, etc. + // Scoped classes (InjectedClassRuntime and its inner classes) are re-defined in the isolated + // classloader so they belong to its distinct unnamed module — which has java.lang opened to it. + ClassLoader agentLoader = CodeCoverageTransformer.class.getClassLoader(); + + ClassLoader isolatedLoader = + new ClassLoader(agentLoader) { + @Override + protected Class loadClass(String name, boolean resolve) + throws ClassNotFoundException { + if (!scope.contains(name)) { + return super.loadClass(name, resolve); + } + InputStream resourceStream = + agentLoader.getResourceAsStream(name.replace('.', '/') + ".class"); + if (resourceStream == null) { + throw new ClassNotFoundException(name); + } + byte[] bytes; + try { + bytes = readAllBytes(resourceStream); + } catch (IOException e) { + throw new RuntimeException(e); + } + return defineClass( + name, bytes, 0, bytes.length, CodeCoverageTransformer.class.getProtectionDomain()); + } + }; + + // Open java.lang package to the isolated classloader's unnamed module + openPackage(inst, Object.class, isolatedLoader); + + // Load InjectedClassRuntime in the isolated module + @SuppressWarnings("unchecked") + Class rtClass = + (Class) isolatedLoader.loadClass(InjectedClassRuntime.class.getName()); + + IRuntime runtime = + rtClass + .getConstructor(Class.class, String.class) + .newInstance(Object.class, "$DDCov"); + + runtime.startup(runtimeData); + this.instrumenter = new Instrumenter(runtime); + } + + @Override + public byte[] transform( + ClassLoader loader, + String className, + Class classBeingRedefined, + ProtectionDomain pd, + byte[] classfileBuffer) { + if (classBeingRedefined != null) { + return null; // retransformation not supported (schema change) + } + if (className == null || loader == null) { + return null; // skip bootstrap classes and unnamed classes + } + if (!filter.test(className)) { + return null; + } + try { + return instrumenter.instrument(classfileBuffer, className); + } catch (Exception e) { + return null; + } + } + + /** + * Collects current probe data and resets all probes to {@code false}. + * + *

Uses a serialize/deserialize round-trip to capture probe values before reset. This is + * necessary because {@code RuntimeData.collect()} passes references to the live {@code boolean[]} + * probe arrays to the visitor. If we passed an {@code ExecutionDataStore} directly, it would store + * references to the same arrays that {@code reset()} then zeroes out — destroying the collected + * data. The byte-stream approach (same as JaCoCo's own {@code Agent.getExecutionData()}) captures + * probe values into the stream before the reset runs. + * + * @param target store to receive the execution data + * @param sessionTarget store to receive session info + */ + public void collectAndReset(ExecutionDataStore target, SessionInfoStore sessionTarget) { + try { + // Serialize probe data to bytes (captures values before reset) + ByteArrayOutputStream buffer = new ByteArrayOutputStream(); + ExecutionDataWriter writer = new ExecutionDataWriter(buffer); + runtimeData.collect(writer, writer, true); + + // Deserialize into the target stores + ExecutionDataReader reader = + new ExecutionDataReader(new java.io.ByteArrayInputStream(buffer.toByteArray())); + reader.setExecutionDataVisitor(target); + reader.setSessionInfoVisitor(sessionTarget); + reader.read(); + } catch (IOException e) { + throw new RuntimeException("Failed to collect coverage data", e); + } + } + + /** + * Opens the package of {@code classInPackage} to the unnamed module of {@code targetLoader}. + * + *

This uses {@code Instrumentation.redefineModule} reflectively (same approach as JaCoCo's + * {@code AgentModule.openPackage}). + */ + private static void openPackage( + Instrumentation inst, Class classInPackage, ClassLoader targetLoader) throws Exception { + // module of the package to open (e.g. java.base for java.lang) + Object module = Class.class.getMethod("getModule").invoke(classInPackage); + + // unnamed module of the isolated classloader + Object unnamedModule = ClassLoader.class.getMethod("getUnnamedModule").invoke(targetLoader); + + Class moduleClass = Class.forName("java.lang.Module"); + + // Instrumentation.redefineModule(Module, Set, Map, Map>, Set, Map) + Instrumentation.class + .getMethod( + "redefineModule", + moduleClass, + Set.class, + Map.class, + Map.class, + Set.class, + Map.class) + .invoke( + inst, + module, // module to modify + Collections.emptySet(), // extraReads + Collections.emptyMap(), // extraExports + Collections.singletonMap( + classInPackage.getPackage().getName(), + Collections.singleton(unnamedModule)), // extraOpens + Collections.emptySet(), // extraUses + Collections.emptyMap()); // extraProvides + } + + /** Recursively adds the given class and all its declared inner classes to the scope set. */ + private static void addToScopeWithInnerClasses(Class clazz, Set scope) { + scope.add(clazz.getName()); + for (Class inner : clazz.getDeclaredClasses()) { + addToScopeWithInnerClasses(inner, scope); + } + } + + /** Reads all bytes from an input stream. */ + private static byte[] readAllBytes(InputStream is) throws IOException { + byte[] buf = new byte[1024]; + ByteArrayOutputStream out = new ByteArrayOutputStream(); + int r; + while ((r = is.read(buf)) != -1) { + out.write(buf, 0, r); + } + return out.toByteArray(); + } +} diff --git a/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/LoggingCodeCoverageSender.java b/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/LoggingCodeCoverageSender.java new file mode 100644 index 00000000000..ea82f7915e0 --- /dev/null +++ b/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/LoggingCodeCoverageSender.java @@ -0,0 +1,24 @@ +package datadog.trace.codecoverage; + +import java.util.BitSet; +import java.util.Map; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Phase 1 stub sender that logs coverage summaries. Will be replaced by a real backend sender in + * future phases. + */ +public final class LoggingCodeCoverageSender implements CodeCoverageSender { + + private static final Logger log = LoggerFactory.getLogger(LoggingCodeCoverageSender.class); + + @Override + public void send(Map coverage) { + int totalLines = 0; + for (BitSet lines : coverage.values()) { + totalLines += lines.cardinality(); + } + log.info("Code coverage collected: {} files, {} lines covered", coverage.size(), totalLines); + } +} diff --git a/dd-java-agent/build.gradle b/dd-java-agent/build.gradle index a34b233cea9..8712defde15 100644 --- a/dd-java-agent/build.gradle +++ b/dd-java-agent/build.gradle @@ -229,6 +229,7 @@ includeSubprojShadowJar(project(':dd-java-agent:agent-aiguard'), 'aiguard', incl includeSubprojShadowJar(project(':dd-java-agent:agent-iast'), 'iast', includedJarFileTree) includeSubprojShadowJar(project(':dd-java-agent:agent-debugger'), 'debugger', includedJarFileTree) includeSubprojShadowJar(project(':dd-java-agent:agent-ci-visibility'), 'ci-visibility', includedJarFileTree) +includeSubprojShadowJar(project(':dd-java-agent:agent-code-coverage'), 'code-coverage', includedJarFileTree) includeSubprojShadowJar(project(':dd-java-agent:agent-llmobs'), 'llm-obs', includedJarFileTree) includeSubprojShadowJar(project(':dd-java-agent:agent-logs-intake'), 'logs-intake', includedJarFileTree) includeSubprojShadowJar(project(':dd-java-agent:cws-tls'), 'cws-tls', includedJarFileTree) diff --git a/dd-trace-api/src/main/java/datadog/trace/api/config/CodeCoverageConfig.java b/dd-trace-api/src/main/java/datadog/trace/api/config/CodeCoverageConfig.java new file mode 100644 index 00000000000..cee50521510 --- /dev/null +++ b/dd-trace-api/src/main/java/datadog/trace/api/config/CodeCoverageConfig.java @@ -0,0 +1,14 @@ +package datadog.trace.api.config; + +/** Constant with names of configuration options for production code coverage. */ +public final class CodeCoverageConfig { + + public static final String CODE_COVERAGE_ENABLED = "code.coverage.enabled"; + public static final String CODE_COVERAGE_INCLUDES = "code.coverage.includes"; + public static final String CODE_COVERAGE_EXCLUDES = "code.coverage.excludes"; + public static final String CODE_COVERAGE_REPORT_INTERVAL_SECONDS = + "code.coverage.report.interval.seconds"; + public static final String CODE_COVERAGE_CLASSPATH = "code.coverage.classpath"; + + private CodeCoverageConfig() {} +} diff --git a/internal-api/src/main/java/datadog/trace/api/Config.java b/internal-api/src/main/java/datadog/trace/api/Config.java index fcaac7a9b55..6375241f53f 100644 --- a/internal-api/src/main/java/datadog/trace/api/Config.java +++ b/internal-api/src/main/java/datadog/trace/api/Config.java @@ -294,6 +294,10 @@ import static datadog.trace.api.config.CiVisibilityConfig.TEST_MANAGEMENT_ATTEMPT_TO_FIX_RETRIES; import static datadog.trace.api.config.CiVisibilityConfig.TEST_MANAGEMENT_ENABLED; import static datadog.trace.api.config.CiVisibilityConfig.TEST_SESSION_NAME; +import static datadog.trace.api.config.CodeCoverageConfig.CODE_COVERAGE_CLASSPATH; +import static datadog.trace.api.config.CodeCoverageConfig.CODE_COVERAGE_EXCLUDES; +import static datadog.trace.api.config.CodeCoverageConfig.CODE_COVERAGE_INCLUDES; +import static datadog.trace.api.config.CodeCoverageConfig.CODE_COVERAGE_REPORT_INTERVAL_SECONDS; import static datadog.trace.api.config.CrashTrackingConfig.CRASH_TRACKING_AGENTLESS; import static datadog.trace.api.config.CrashTrackingConfig.CRASH_TRACKING_AGENTLESS_DEFAULT; import static datadog.trace.api.config.CrashTrackingConfig.CRASH_TRACKING_ERRORS_INTAKE_ENABLED; @@ -1248,6 +1252,11 @@ public static String getHostName() { private final boolean cwsEnabled; private final int cwsTlsRefresh; + private final String[] codeCoverageIncludes; + private final String[] codeCoverageExcludes; + private final int codeCoverageReportIntervalSeconds; + private final String codeCoverageClasspath; + private final boolean dataJobsOpenLineageEnabled; private final boolean dataJobsOpenLineageTimeoutEnabled; private final boolean dataJobsParseSparkPlanEnabled; @@ -2816,6 +2825,20 @@ PROFILING_DATADOG_PROFILER_ENABLED, isDatadogProfilerSafeInCurrentEnvironment()) cwsEnabled = configProvider.getBoolean(CWS_ENABLED, DEFAULT_CWS_ENABLED); cwsTlsRefresh = configProvider.getInteger(CWS_TLS_REFRESH, DEFAULT_CWS_TLS_REFRESH); + { + List includesList = configProvider.getList(CODE_COVERAGE_INCLUDES); + codeCoverageIncludes = + includesList == null || includesList.isEmpty() + ? new String[] {"*"} + : includesList.toArray(new String[0]); + List excludesList = configProvider.getList(CODE_COVERAGE_EXCLUDES); + codeCoverageExcludes = + excludesList == null ? new String[0] : excludesList.toArray(new String[0]); + } + codeCoverageReportIntervalSeconds = + configProvider.getInteger(CODE_COVERAGE_REPORT_INTERVAL_SECONDS, 900); + codeCoverageClasspath = configProvider.getString(CODE_COVERAGE_CLASSPATH); + dataJobsOpenLineageEnabled = configProvider.getBoolean( DATA_JOBS_OPENLINEAGE_ENABLED, DEFAULT_DATA_JOBS_OPENLINEAGE_ENABLED); @@ -4677,6 +4700,22 @@ public boolean isCwsEnabled() { return cwsEnabled; } + public String[] getCodeCoverageIncludes() { + return codeCoverageIncludes; + } + + public String[] getCodeCoverageExcludes() { + return codeCoverageExcludes; + } + + public int getCodeCoverageReportIntervalSeconds() { + return codeCoverageReportIntervalSeconds; + } + + public String getCodeCoverageClasspath() { + return codeCoverageClasspath; + } + public int getCwsTlsRefresh() { return cwsTlsRefresh; } diff --git a/internal-api/src/main/java/datadog/trace/api/InstrumenterConfig.java b/internal-api/src/main/java/datadog/trace/api/InstrumenterConfig.java index 00dd43f7197..d679b2812c4 100644 --- a/internal-api/src/main/java/datadog/trace/api/InstrumenterConfig.java +++ b/internal-api/src/main/java/datadog/trace/api/InstrumenterConfig.java @@ -28,6 +28,7 @@ import static datadog.trace.api.config.AppSecConfig.APPSEC_ENABLED; import static datadog.trace.api.config.AppSecConfig.APPSEC_RASP_ENABLED; import static datadog.trace.api.config.CiVisibilityConfig.CIVISIBILITY_ENABLED; +import static datadog.trace.api.config.CodeCoverageConfig.CODE_COVERAGE_ENABLED; import static datadog.trace.api.config.GeneralConfig.AGENTLESS_LOG_SUBMISSION_ENABLED; import static datadog.trace.api.config.GeneralConfig.APP_LOGS_COLLECTION_ENABLED; import static datadog.trace.api.config.GeneralConfig.DATA_JOBS_ENABLED; @@ -215,6 +216,8 @@ public class InstrumenterConfig { private final boolean appLogsCollectionEnabled; private final boolean legacyContextManagerEnabled; + private final boolean codeCoverageEnabled; + static { // Bind telemetry collector to config module before initializing ConfigProvider OtelEnvMetricCollectorProvider.register(OtelEnvMetricCollectorImpl.getInstance()); @@ -367,6 +370,8 @@ private InstrumenterConfig() { configProvider.getBoolean(APP_LOGS_COLLECTION_ENABLED, DEFAULT_APP_LOGS_COLLECTION_ENABLED); legacyContextManagerEnabled = configProvider.getBoolean(LEGACY_CONTEXT_MANAGER_ENABLED, true); + + codeCoverageEnabled = configProvider.getBoolean(CODE_COVERAGE_ENABLED, false); } public boolean isCodeOriginEnabled() { @@ -690,6 +695,10 @@ public boolean isLegacyContextManagerEnabled() { return legacyContextManagerEnabled; } + public boolean isCodeCoverageEnabled() { + return codeCoverageEnabled; + } + // This has to be placed after all other static fields to give them a chance to initialize private static final InstrumenterConfig INSTANCE = new InstrumenterConfig( @@ -811,6 +820,8 @@ public String toString() { + apiSecurityEndpointCollectionEnabled + ", legacyContextManagerEnabled=" + legacyContextManagerEnabled + + ", codeCoverageEnabled=" + + codeCoverageEnabled + '}'; } } diff --git a/settings.gradle.kts b/settings.gradle.kts index dbe66b33670..8ade33eee3f 100644 --- a/settings.gradle.kts +++ b/settings.gradle.kts @@ -128,6 +128,11 @@ include( ":dd-java-agent:agent-ci-visibility:civisibility-instrumentation-test-fixtures", ) +// code-coverage +include( + ":dd-java-agent:agent-code-coverage", +) + // llm-observability include( ":dd-java-agent:agent-llmobs", From bde7b531e9424da744f81158958e099999205ef8 Mon Sep 17 00:00:00 2001 From: Nikita Tkachenko Date: Mon, 23 Mar 2026 16:11:58 +0100 Subject: [PATCH 02/19] Implement sending coverage report --- .../java/datadog/trace/bootstrap/Agent.java | 9 +- .../agent-ci-visibility/build.gradle | 1 + .../CiVisibilityCoverageServices.java | 25 +++-- .../report/JacocoCoverageProcessor.java | 3 + .../agent-code-coverage/build.gradle | 2 + .../codecoverage/CodeCoverageCollector.java | 23 +++-- .../codecoverage/CodeCoverageLcovSender.java | 30 ++++++ .../codecoverage/CodeCoverageSender.java | 19 ---- .../codecoverage/CodeCoverageSystem.java | 91 ++++++++++++++++++- .../LoggingCodeCoverageSender.java | 24 ----- gradle.properties | 2 +- settings.gradle.kts | 1 + utils/coverage-utils/build.gradle.kts | 13 +++ .../coverage}/CoverageReportUploader.java | 31 ++----- .../trace/coverage}/LcovReportWriter.java | 2 +- .../trace/coverage}/LinesCoverage.java | 2 +- .../CoverageReportUploaderTest.groovy | 6 +- .../coverage}/LcovReportWriterTest.groovy | 2 +- 18 files changed, 189 insertions(+), 97 deletions(-) create mode 100644 dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageLcovSender.java delete mode 100644 dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageSender.java delete mode 100644 dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/LoggingCodeCoverageSender.java create mode 100644 utils/coverage-utils/build.gradle.kts rename {dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/coverage/report => utils/coverage-utils/src/main/java/datadog/trace/coverage}/CoverageReportUploader.java (69%) rename {dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/coverage/report => utils/coverage-utils/src/main/java/datadog/trace/coverage}/LcovReportWriter.java (97%) rename {dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/coverage/report => utils/coverage-utils/src/main/java/datadog/trace/coverage}/LinesCoverage.java (76%) rename {dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/coverage/report => utils/coverage-utils/src/test/groovy/datadog/trace/coverage}/CoverageReportUploaderTest.groovy (93%) rename {dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/coverage/report => utils/coverage-utils/src/test/groovy/datadog/trace/coverage}/LcovReportWriterTest.groovy (98%) diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java index 2c013b934a2..21f2260897f 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/Agent.java @@ -708,7 +708,7 @@ public void execute() { } if (codeCoverageTransformer != null) { - startCodeCoverageCollector(codeCoverageTransformer); + startCodeCoverageCollector(codeCoverageTransformer, sco); } } @@ -1154,12 +1154,13 @@ private static Object maybeStartCodeCoverage(Instrumentation inst) { } } - private static void startCodeCoverageCollector(Object transformer) { + private static void startCodeCoverageCollector(Object transformer, Object sco) { try { final Class systemClass = AGENT_CLASSLOADER.loadClass("datadog.trace.codecoverage.CodeCoverageSystem"); - final Method startCollectorMethod = systemClass.getMethod("startCollector", Object.class); - startCollectorMethod.invoke(null, transformer); + final Method startCollectorMethod = + systemClass.getMethod("startCollector", Object.class, Object.class); + startCollectorMethod.invoke(null, transformer, sco); } catch (final Throwable e) { log.warn("Not starting Code Coverage collector", e); } diff --git a/dd-java-agent/agent-ci-visibility/build.gradle b/dd-java-agent/agent-ci-visibility/build.gradle index 7811fa39140..070c6e5c06b 100644 --- a/dd-java-agent/agent-ci-visibility/build.gradle +++ b/dd-java-agent/agent-ci-visibility/build.gradle @@ -27,6 +27,7 @@ dependencies { implementation project(':components:json') implementation project(':internal-api') implementation project(':internal-api:internal-api-9') + implementation project(':utils:coverage-utils') testImplementation project(':dd-java-agent:testing') testImplementation("com.google.jimfs:jimfs:1.1") // an in-memory file system for testing code that works with files diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/CiVisibilityCoverageServices.java b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/CiVisibilityCoverageServices.java index c83beb14283..0fa7dc81d4f 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/CiVisibilityCoverageServices.java +++ b/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/CiVisibilityCoverageServices.java @@ -10,9 +10,13 @@ import datadog.trace.civisibility.coverage.SkippableAwareCoverageStoreFactory; import datadog.trace.civisibility.coverage.file.FileCoverageStore; import datadog.trace.civisibility.coverage.line.LineCoverageStore; +import datadog.communication.http.OkHttpUtils; +import datadog.trace.api.civisibility.telemetry.CiVisibilityCountMetric; +import datadog.trace.api.civisibility.telemetry.CiVisibilityDistributionMetric; +import datadog.trace.civisibility.communication.TelemetryListener; import datadog.trace.civisibility.coverage.report.CoverageProcessor; -import datadog.trace.civisibility.coverage.report.CoverageReportUploader; import datadog.trace.civisibility.coverage.report.JacocoCoverageProcessor; +import datadog.trace.coverage.CoverageReportUploader; import datadog.trace.civisibility.coverage.report.child.ChildProcessCoverageReporter; import datadog.trace.civisibility.coverage.report.child.JacocoChildProcessCoverageReporter; import datadog.trace.civisibility.domain.buildsystem.ModuleSignalRouter; @@ -34,11 +38,20 @@ static class Parent { ExecutionSettings executionSettings = repoServices.executionSettingsFactory.create(JvmInfo.CURRENT_JVM, null); - CoverageReportUploader coverageReportUploader = - executionSettings.isCodeCoverageReportUploadEnabled() - ? new CoverageReportUploader( - services.ciIntake, repoServices.ciTags, services.metricCollector) - : null; + CoverageReportUploader coverageReportUploader; + if (executionSettings.isCodeCoverageReportUploadEnabled()) { + OkHttpUtils.CustomListener telemetryListener = + new TelemetryListener.Builder(services.metricCollector) + .requestCount(CiVisibilityCountMetric.COVERAGE_UPLOAD_REQUEST) + .requestBytes(CiVisibilityDistributionMetric.COVERAGE_UPLOAD_REQUEST_BYTES) + .requestErrors(CiVisibilityCountMetric.COVERAGE_UPLOAD_REQUEST_ERRORS) + .requestDuration(CiVisibilityDistributionMetric.COVERAGE_UPLOAD_REQUEST_MS) + .build(); + coverageReportUploader = + new CoverageReportUploader(services.ciIntake, repoServices.ciTags, telemetryListener); + } else { + coverageReportUploader = null; + } coverageProcessorFactory = new JacocoCoverageProcessor.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..c87ca9a0d8b 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 @@ -5,6 +5,9 @@ import datadog.trace.api.civisibility.domain.SourceSet; import datadog.trace.civisibility.config.ExecutionSettings; import datadog.trace.civisibility.domain.buildsystem.ModuleSignalRouter; +import datadog.trace.coverage.CoverageReportUploader; +import datadog.trace.coverage.LcovReportWriter; +import datadog.trace.coverage.LinesCoverage; import datadog.trace.civisibility.ipc.AckResponse; import datadog.trace.civisibility.ipc.ModuleCoverageDataJacoco; import datadog.trace.civisibility.ipc.SignalResponse; diff --git a/dd-java-agent/agent-code-coverage/build.gradle b/dd-java-agent/agent-code-coverage/build.gradle index 90969b02e3d..6c86afcf1f7 100644 --- a/dd-java-agent/agent-code-coverage/build.gradle +++ b/dd-java-agent/agent-code-coverage/build.gradle @@ -16,6 +16,8 @@ dependencies { implementation group: 'org.jacoco', name: 'org.jacoco.core', version: '0.8.14' implementation project(':internal-api') + implementation project(':communication') + implementation project(':utils:coverage-utils') testImplementation project(':dd-java-agent:testing') } diff --git a/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageCollector.java b/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageCollector.java index 2e9872f8ead..de08ef8eb46 100644 --- a/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageCollector.java +++ b/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageCollector.java @@ -1,9 +1,9 @@ package datadog.trace.codecoverage; +import datadog.trace.coverage.LinesCoverage; import java.io.File; import java.io.IOException; import java.util.ArrayList; -import java.util.BitSet; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -21,14 +21,14 @@ /** * Periodically collects code coverage probe data, resolves it to covered source lines using - * JaCoCo's analysis pipeline, and sends the results via a {@link CodeCoverageSender}. + * JaCoCo's analysis pipeline, and sends the results via a {@link CodeCoverageLcovSender}. */ public final class CodeCoverageCollector { private static final Logger log = LoggerFactory.getLogger(CodeCoverageCollector.class); private final CodeCoverageTransformer transformer; - private final CodeCoverageSender sender; + private final CodeCoverageLcovSender sender; private final int intervalSeconds; private final String explicitClasspath; private volatile ScheduledExecutorService scheduler; @@ -41,7 +41,7 @@ public final class CodeCoverageCollector { */ public CodeCoverageCollector( CodeCoverageTransformer transformer, - CodeCoverageSender sender, + CodeCoverageLcovSender sender, int intervalSeconds, String explicitClasspath) { this.transformer = transformer; @@ -97,25 +97,28 @@ void collect() { } } - // 4. Build coverage map: source file -> covered line numbers - Map coverage = new HashMap<>(); + // 4. Build coverage map: source file -> lines coverage + Map coverage = new HashMap<>(); for (IClassCoverage cc : builder.getClasses()) { if (cc.getSourceFileName() == null) { continue; } String sourceFile = cc.getPackageName() + "/" + cc.getSourceFileName(); - BitSet lines = coverage.computeIfAbsent(sourceFile, k -> new BitSet()); + LinesCoverage lc = coverage.computeIfAbsent(sourceFile, k -> new LinesCoverage()); for (int line = cc.getFirstLine(); line <= cc.getLastLine(); line++) { int status = cc.getLine(line).getStatus(); - if (status == ICounter.PARTLY_COVERED || status == ICounter.FULLY_COVERED) { - lines.set(line); + if (status != ICounter.EMPTY) { + lc.executableLines.set(line); + if (status == ICounter.PARTLY_COVERED || status == ICounter.FULLY_COVERED) { + lc.coveredLines.set(line); + } } } } // 5. Send if there is data if (!coverage.isEmpty()) { - sender.send(coverage); + sender.upload(coverage); } } catch (Exception e) { log.debug("Error during code coverage collection", e); diff --git a/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageLcovSender.java b/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageLcovSender.java new file mode 100644 index 00000000000..b223f8dee6e --- /dev/null +++ b/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageLcovSender.java @@ -0,0 +1,30 @@ +package datadog.trace.codecoverage; + +import datadog.trace.coverage.CoverageReportUploader; +import datadog.trace.coverage.LcovReportWriter; +import datadog.trace.coverage.LinesCoverage; +import java.io.ByteArrayInputStream; +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.util.Map; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +public final class CodeCoverageLcovSender { + private static final Logger log = LoggerFactory.getLogger(CodeCoverageLcovSender.class); + private final CoverageReportUploader uploader; + + public CodeCoverageLcovSender(CoverageReportUploader uploader) { + this.uploader = uploader; + } + + public void upload(Map coverage) { + String lcov = LcovReportWriter.toString(coverage); + try { + uploader.upload( + "lcov", new ByteArrayInputStream(lcov.getBytes(StandardCharsets.UTF_8))); + } catch (IOException e) { + log.debug("Failed to upload code coverage report", e); + } + } +} diff --git a/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageSender.java b/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageSender.java deleted file mode 100644 index 987575408ef..00000000000 --- a/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageSender.java +++ /dev/null @@ -1,19 +0,0 @@ -package datadog.trace.codecoverage; - -import java.util.BitSet; -import java.util.Map; - -/** - * Interface for sending collected code coverage data to a backend. Phase 1 uses a logging stub; - * future phases will implement real sending to Datadog. - */ -public interface CodeCoverageSender { - - /** - * Sends coverage data. - * - * @param coverage map from source file path (e.g. {@code "com/example/MyClass.java"}) to set of - * covered line numbers - */ - void send(Map coverage); -} diff --git a/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageSystem.java b/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageSystem.java index 0d96f6a68b0..7aca7e1aca2 100644 --- a/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageSystem.java +++ b/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageSystem.java @@ -1,8 +1,21 @@ package datadog.trace.codecoverage; +import datadog.communication.BackendApi; +import datadog.communication.BackendApiFactory; +import datadog.communication.ddagent.SharedCommunicationObjects; import datadog.trace.api.Config; +import datadog.trace.api.git.CommitInfo; +import datadog.trace.api.git.GitInfo; +import datadog.trace.api.git.GitInfoProvider; +import datadog.trace.api.git.PersonInfo; +import datadog.trace.api.intake.Intake; +import datadog.trace.coverage.CoverageReportUploader; import java.lang.instrument.Instrumentation; +import java.util.HashMap; +import java.util.Map; import java.util.function.Predicate; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * Entry point for the production code coverage product module. @@ -12,17 +25,19 @@ *

    *
  1. {@link #start(Instrumentation)} — called during premain, before ByteBuddy's * transformer is registered. Must not use logging, NIO, or JMX. - *
  2. {@link #startCollector(Object)} — called from a deferred callback after premain, when - * logging and thread scheduling are safe. + *
  3. {@link #startCollector(Object, Object)} — called from a deferred callback after premain, + * when logging and thread scheduling are safe. *
*/ public final class CodeCoverageSystem { + private static final Logger log = LoggerFactory.getLogger(CodeCoverageSystem.class); + /** * Phase 1: registers the coverage {@link java.lang.instrument.ClassFileTransformer}. * *

Called during premain, synchronously, before ByteBuddy. The returned object is an opaque - * handle to the transformer, passed to {@link #startCollector(Object)} later. + * handle to the transformer, passed to {@link #startCollector(Object, Object)} later. * * @param inst the JVM instrumentation service * @return the transformer instance (opaque; passed to {@link #startCollector}) @@ -44,11 +59,35 @@ public static Object start(Instrumentation inst) throws Exception { *

Called from a deferred callback after premain. Safe to use logging and thread scheduling. * * @param transformerObj the opaque transformer handle returned by {@link #start} + * @param scoObj the SharedCommunicationObjects instance for backend communication */ - public static void startCollector(Object transformerObj) { + public static void startCollector(Object transformerObj, Object scoObj) { CodeCoverageTransformer transformer = (CodeCoverageTransformer) transformerObj; Config config = Config.get(); - CodeCoverageSender sender = new LoggingCodeCoverageSender(); + + // Build event tags from git info + Map tags = buildGitTags(); + if (!tags.containsKey("git.commit.sha")) { + log.warn( + "DD_GIT_COMMIT_SHA is not set; " + + "code coverage reports cannot be uploaded without a commit SHA"); + return; + } + + // Create BackendApi for coverage uploads + BackendApiFactory factory = + new BackendApiFactory(config, (SharedCommunicationObjects) scoObj); + BackendApi backendApi = factory.createBackendApi(Intake.CI_INTAKE); + if (backendApi == null) { + log.warn( + "Cannot create backend API for code coverage uploads; " + + "agent may not support EVP proxy"); + return; + } + + CoverageReportUploader uploader = new CoverageReportUploader(backendApi, tags, null); + CodeCoverageLcovSender sender = new CodeCoverageLcovSender(uploader); + CodeCoverageCollector collector = new CodeCoverageCollector( transformer, @@ -58,5 +97,47 @@ public static void startCollector(Object transformerObj) { collector.start(); } + private static Map buildGitTags() { + Map tags = new HashMap<>(); + GitInfo gitInfo = GitInfoProvider.INSTANCE.getGitInfo(); + CommitInfo commit = gitInfo.getCommit(); + if (commit != null && commit.getSha() != null) { + tags.put("git.commit.sha", commit.getSha()); + } + if (gitInfo.getRepositoryURL() != null) { + tags.put("git.repository_url", gitInfo.getRepositoryURL()); + } + if (gitInfo.getBranch() != null) { + tags.put("git.branch", gitInfo.getBranch()); + } + // Add author/committer info if available + if (commit != null) { + PersonInfo author = commit.getAuthor(); + if (author.getName() != null) { + tags.put("git.commit.author.name", author.getName()); + } + if (author.getEmail() != null) { + tags.put("git.commit.author.email", author.getEmail()); + } + if (author.getIso8601Date() != null) { + tags.put("git.commit.author.date", author.getIso8601Date()); + } + PersonInfo committer = commit.getCommitter(); + if (committer.getName() != null) { + tags.put("git.commit.committer.name", committer.getName()); + } + if (committer.getEmail() != null) { + tags.put("git.commit.committer.email", committer.getEmail()); + } + if (committer.getIso8601Date() != null) { + tags.put("git.commit.committer.date", committer.getIso8601Date()); + } + if (commit.getFullMessage() != null) { + tags.put("git.commit.message", commit.getFullMessage()); + } + } + return tags; + } + private CodeCoverageSystem() {} } diff --git a/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/LoggingCodeCoverageSender.java b/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/LoggingCodeCoverageSender.java deleted file mode 100644 index ea82f7915e0..00000000000 --- a/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/LoggingCodeCoverageSender.java +++ /dev/null @@ -1,24 +0,0 @@ -package datadog.trace.codecoverage; - -import java.util.BitSet; -import java.util.Map; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -/** - * Phase 1 stub sender that logs coverage summaries. Will be replaced by a real backend sender in - * future phases. - */ -public final class LoggingCodeCoverageSender implements CodeCoverageSender { - - private static final Logger log = LoggerFactory.getLogger(LoggingCodeCoverageSender.class); - - @Override - public void send(Map coverage) { - int totalLines = 0; - for (BitSet lines : coverage.values()) { - totalLines += lines.cardinality(); - } - log.info("Code coverage collected: {} files, {} lines covered", coverage.size(), totalLines); - } -} diff --git a/gradle.properties b/gradle.properties index 48d5ceb5b49..0c75ce4e3a4 100644 --- a/gradle.properties +++ b/gradle.properties @@ -1,6 +1,6 @@ org.gradle.parallel=true org.gradle.caching=true -org.gradle.jvmargs=-XX:MaxMetaspaceSize=1g +org.gradle.jvmargs=-Xmx2g -XX:MaxMetaspaceSize=1g # Toggle on to get more details during IJ sync #org.gradle.logging.level=info diff --git a/settings.gradle.kts b/settings.gradle.kts index 8ade33eee3f..497fe614654 100644 --- a/settings.gradle.kts +++ b/settings.gradle.kts @@ -164,6 +164,7 @@ include( ":dd-java-agent:testing", ":utils:config-utils", ":utils:container-utils", + ":utils:coverage-utils", ":utils:filesystem-utils", ":utils:flare-utils", ":utils:logging-utils", diff --git a/utils/coverage-utils/build.gradle.kts b/utils/coverage-utils/build.gradle.kts new file mode 100644 index 00000000000..900b9da3aef --- /dev/null +++ b/utils/coverage-utils/build.gradle.kts @@ -0,0 +1,13 @@ +plugins { + `java-library` +} + +apply(from = "$rootDir/gradle/java.gradle") + +dependencies { + // For CoverageReportUploader + implementation(project(":communication")) + implementation(project(":internal-api")) + + testImplementation(project(":dd-java-agent:testing")) +} diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/coverage/report/CoverageReportUploader.java b/utils/coverage-utils/src/main/java/datadog/trace/coverage/CoverageReportUploader.java similarity index 69% rename from dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/coverage/report/CoverageReportUploader.java rename to utils/coverage-utils/src/main/java/datadog/trace/coverage/CoverageReportUploader.java index 0d0ffef37c9..ae40ac13c71 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/coverage/report/CoverageReportUploader.java +++ b/utils/coverage-utils/src/main/java/datadog/trace/coverage/CoverageReportUploader.java @@ -1,4 +1,4 @@ -package datadog.trace.civisibility.coverage.report; +package datadog.trace.coverage; import static datadog.communication.http.OkHttpUtils.jsonRequestBodyOf; @@ -7,10 +7,6 @@ import com.squareup.moshi.Types; import datadog.communication.BackendApi; import datadog.communication.http.OkHttpUtils; -import datadog.trace.api.civisibility.telemetry.CiVisibilityCountMetric; -import datadog.trace.api.civisibility.telemetry.CiVisibilityDistributionMetric; -import datadog.trace.api.civisibility.telemetry.CiVisibilityMetricCollector; -import datadog.trace.civisibility.communication.TelemetryListener; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import java.io.IOException; import java.io.InputStream; @@ -19,6 +15,7 @@ import java.util.HashMap; import java.util.Map; import java.util.zip.GZIPOutputStream; +import javax.annotation.Nullable; import okhttp3.MediaType; import okhttp3.MultipartBody; import okhttp3.RequestBody; @@ -27,17 +24,17 @@ public class CoverageReportUploader { private final BackendApi backendApi; - private final Map ciTags; - private final CiVisibilityMetricCollector metricCollector; + private final Map tags; + @Nullable private final OkHttpUtils.CustomListener requestListener; private final JsonAdapter> eventAdapter; public CoverageReportUploader( BackendApi backendApi, - Map ciTags, - CiVisibilityMetricCollector metricCollector) { + Map tags, + @Nullable OkHttpUtils.CustomListener requestListener) { this.backendApi = backendApi; - this.ciTags = ciTags; - this.metricCollector = metricCollector; + this.tags = tags; + this.requestListener = requestListener; Moshi moshi = new Moshi.Builder().build(); Type type = Types.newParameterizedType(Map.class, String.class, String.class); @@ -45,7 +42,7 @@ public CoverageReportUploader( } public void upload(String format, InputStream reportStream) throws IOException { - Map event = new HashMap<>(ciTags); + Map event = new HashMap<>(tags); event.put("format", format); event.put("type", "coverage_report"); String eventJson = eventAdapter.toJson(event); @@ -60,15 +57,7 @@ public void upload(String format, InputStream reportStream) throws IOException { .addFormDataPart("event", "event.json", eventBody) .build(); - OkHttpUtils.CustomListener telemetryListener = - new TelemetryListener.Builder(metricCollector) - .requestCount(CiVisibilityCountMetric.COVERAGE_UPLOAD_REQUEST) - .requestBytes(CiVisibilityDistributionMetric.COVERAGE_UPLOAD_REQUEST_BYTES) - .requestErrors(CiVisibilityCountMetric.COVERAGE_UPLOAD_REQUEST_ERRORS) - .requestDuration(CiVisibilityDistributionMetric.COVERAGE_UPLOAD_REQUEST_MS) - .build(); - - backendApi.post("cicovreprt", multipartBody, responseStream -> null, telemetryListener, false); + backendApi.post("cicovreprt", multipartBody, responseStream -> null, requestListener, false); } /** Request body that compresses a form data part */ diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/coverage/report/LcovReportWriter.java b/utils/coverage-utils/src/main/java/datadog/trace/coverage/LcovReportWriter.java similarity index 97% rename from dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/coverage/report/LcovReportWriter.java rename to utils/coverage-utils/src/main/java/datadog/trace/coverage/LcovReportWriter.java index b7fc48c8873..4146c2d42ed 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/coverage/report/LcovReportWriter.java +++ b/utils/coverage-utils/src/main/java/datadog/trace/coverage/LcovReportWriter.java @@ -1,4 +1,4 @@ -package datadog.trace.civisibility.coverage.report; +package datadog.trace.coverage; import java.io.IOException; import java.io.StringWriter; diff --git a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/coverage/report/LinesCoverage.java b/utils/coverage-utils/src/main/java/datadog/trace/coverage/LinesCoverage.java similarity index 76% rename from dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/coverage/report/LinesCoverage.java rename to utils/coverage-utils/src/main/java/datadog/trace/coverage/LinesCoverage.java index 962467439a1..b587f3a0f7e 100644 --- a/dd-java-agent/agent-ci-visibility/src/main/java/datadog/trace/civisibility/coverage/report/LinesCoverage.java +++ b/utils/coverage-utils/src/main/java/datadog/trace/coverage/LinesCoverage.java @@ -1,4 +1,4 @@ -package datadog.trace.civisibility.coverage.report; +package datadog.trace.coverage; import java.util.BitSet; diff --git a/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/coverage/report/CoverageReportUploaderTest.groovy b/utils/coverage-utils/src/test/groovy/datadog/trace/coverage/CoverageReportUploaderTest.groovy similarity index 93% rename from dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/coverage/report/CoverageReportUploaderTest.groovy rename to utils/coverage-utils/src/test/groovy/datadog/trace/coverage/CoverageReportUploaderTest.groovy index d1322d91c3b..0644a641e27 100644 --- a/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/coverage/report/CoverageReportUploaderTest.groovy +++ b/utils/coverage-utils/src/test/groovy/datadog/trace/coverage/CoverageReportUploaderTest.groovy @@ -1,11 +1,10 @@ -package datadog.trace.civisibility.coverage.report +package datadog.trace.coverage import com.fasterxml.jackson.databind.ObjectMapper import datadog.communication.BackendApi import datadog.communication.IntakeApi import datadog.communication.http.HttpRetryPolicy import datadog.communication.http.OkHttpUtils -import datadog.trace.api.civisibility.telemetry.CiVisibilityMetricCollector import datadog.trace.api.intake.Intake import datadog.trace.test.util.MultipartRequestParser import okhttp3.HttpUrl @@ -70,8 +69,7 @@ class CoverageReportUploaderTest extends Specification { def "test upload coverage report"() { setup: def backendApi = givenIntakeApi() - def metricCollector = Stub(CiVisibilityMetricCollector) - def uploader = new CoverageReportUploader(backendApi, [(CI_TAG_KEY):CI_TAG_VALUE], metricCollector) + def uploader = new CoverageReportUploader(backendApi, [(CI_TAG_KEY):CI_TAG_VALUE], null) def report = new ByteArrayInputStream(COVERAGE_REPORT_BODY.getBytes()) expect: diff --git a/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/coverage/report/LcovReportWriterTest.groovy b/utils/coverage-utils/src/test/groovy/datadog/trace/coverage/LcovReportWriterTest.groovy similarity index 98% rename from dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/coverage/report/LcovReportWriterTest.groovy rename to utils/coverage-utils/src/test/groovy/datadog/trace/coverage/LcovReportWriterTest.groovy index a1c2d3c0478..9bc2b145199 100644 --- a/dd-java-agent/agent-ci-visibility/src/test/groovy/datadog/trace/civisibility/coverage/report/LcovReportWriterTest.groovy +++ b/utils/coverage-utils/src/test/groovy/datadog/trace/coverage/LcovReportWriterTest.groovy @@ -1,4 +1,4 @@ -package datadog.trace.civisibility.coverage.report +package datadog.trace.coverage import spock.lang.Specification From 632c3a9b7c7f5b80e8fa37fa965ffcc0f3c81154 Mon Sep 17 00:00:00 2001 From: Nikita Tkachenko Date: Mon, 23 Mar 2026 19:08:16 +0100 Subject: [PATCH 03/19] Implement probe-to-line cache --- .../trace/codecoverage/ClassProbeMapping.java | 22 +++ .../ClassProbeMappingBuilder.java | 98 ++++++++++++ .../codecoverage/CodeCoverageCollector.java | 68 ++++---- .../trace/codecoverage/ProbeMappingCache.java | 148 ++++++++++++++++++ gradle.properties | 2 +- 5 files changed, 305 insertions(+), 33 deletions(-) create mode 100644 dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/ClassProbeMapping.java create mode 100644 dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/ClassProbeMappingBuilder.java create mode 100644 dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/ProbeMappingCache.java diff --git a/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/ClassProbeMapping.java b/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/ClassProbeMapping.java new file mode 100644 index 00000000000..cc952b85129 --- /dev/null +++ b/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/ClassProbeMapping.java @@ -0,0 +1,22 @@ +package datadog.trace.codecoverage; + +import java.util.BitSet; + +/** + * Cached mapping from probe IDs to source lines for a single class. Built once per class version + * (identified by CRC64) and reused across collection cycles. + */ +final class ClassProbeMapping { + final long classId; + final String sourceFile; // "package/SourceFile.java" + final BitSet executableLines; + final int[][] probeToLines; // probeToLines[probeId] = sorted line numbers + + ClassProbeMapping( + long classId, String sourceFile, BitSet executableLines, int[][] probeToLines) { + this.classId = classId; + this.sourceFile = sourceFile; + this.executableLines = executableLines; + this.probeToLines = probeToLines; + } +} diff --git a/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/ClassProbeMappingBuilder.java b/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/ClassProbeMappingBuilder.java new file mode 100644 index 00000000000..74b48bb6244 --- /dev/null +++ b/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/ClassProbeMappingBuilder.java @@ -0,0 +1,98 @@ +package datadog.trace.codecoverage; + +import java.io.IOException; +import java.util.BitSet; +import org.jacoco.core.analysis.Analyzer; +import org.jacoco.core.analysis.CoverageBuilder; +import org.jacoco.core.analysis.IClassCoverage; +import org.jacoco.core.analysis.ICounter; +import org.jacoco.core.data.ExecutionData; +import org.jacoco.core.data.ExecutionDataStore; + +/** + * Builds a {@link ClassProbeMapping} by running JaCoCo's {@link Analyzer} with controlled probe + * configurations. + * + *

For a class with N probes, this runs N+1 analyzer passes: one with all probes false (to + * determine executable lines), then one per probe (to determine which lines each probe covers). + */ +final class ClassProbeMappingBuilder { + + /** + * Builds a {@link ClassProbeMapping} from raw class bytes. + * + * @param classId CRC64 of the class bytes + * @param className VM class name (e.g. "com/example/MyClass") + * @param probeCount number of probes in this class + * @param classBytes original class file bytes (must match classId) + * @return the mapping, never null + * @throws IOException if the class bytes cannot be analyzed + */ + static ClassProbeMapping build( + long classId, String className, int probeCount, byte[] classBytes) throws IOException { + + // 1. Get executable lines (analyze with all probes false) + BitSet executableLines = new BitSet(); + String sourceFile = null; + { + ExecutionDataStore store = new ExecutionDataStore(); + store.put(new ExecutionData(classId, className, probeCount)); // all false + CoverageBuilder builder = new CoverageBuilder(); + Analyzer analyzer = new Analyzer(store, builder); + analyzer.analyzeClass(classBytes, className); + for (IClassCoverage cc : builder.getClasses()) { + if (cc.getSourceFileName() != null) { + sourceFile = cc.getPackageName() + "/" + cc.getSourceFileName(); + } + for (int line = cc.getFirstLine(); line <= cc.getLastLine(); line++) { + if (cc.getLine(line).getStatus() != ICounter.EMPTY) { + executableLines.set(line); + } + } + } + } + + if (sourceFile == null) { + // No source info -- create a mapping with no lines + return new ClassProbeMapping(classId, null, executableLines, new int[probeCount][]); + } + + // 2. Build per-probe line mapping + int[][] probeToLines = new int[probeCount][]; + for (int probeId = 0; probeId < probeCount; probeId++) { + boolean[] probes = new boolean[probeCount]; + probes[probeId] = true; + + ExecutionDataStore store = new ExecutionDataStore(); + store.put(new ExecutionData(classId, className, probes)); + CoverageBuilder builder = new CoverageBuilder(); + Analyzer analyzer = new Analyzer(store, builder); + analyzer.analyzeClass(classBytes, className); + + // Collect covered lines for this probe + BitSet coveredByProbe = new BitSet(); + for (IClassCoverage cc : builder.getClasses()) { + for (int line = cc.getFirstLine(); line <= cc.getLastLine(); line++) { + int status = cc.getLine(line).getStatus(); + if (status == ICounter.PARTLY_COVERED || status == ICounter.FULLY_COVERED) { + coveredByProbe.set(line); + } + } + } + + // Convert BitSet to sorted int[] + probeToLines[probeId] = bitSetToArray(coveredByProbe); + } + + return new ClassProbeMapping(classId, sourceFile, executableLines, probeToLines); + } + + private static int[] bitSetToArray(BitSet bs) { + int[] result = new int[bs.cardinality()]; + int idx = 0; + for (int bit = bs.nextSetBit(0); bit >= 0; bit = bs.nextSetBit(bit + 1)) { + result[idx++] = bit; + } + return result; + } +} diff --git a/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageCollector.java b/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageCollector.java index de08ef8eb46..d13510fdf49 100644 --- a/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageCollector.java +++ b/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageCollector.java @@ -2,26 +2,27 @@ import datadog.trace.coverage.LinesCoverage; import java.io.File; -import java.io.IOException; import java.util.ArrayList; +import java.util.Collection; import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.concurrent.Executors; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; -import org.jacoco.core.analysis.Analyzer; -import org.jacoco.core.analysis.CoverageBuilder; -import org.jacoco.core.analysis.IClassCoverage; -import org.jacoco.core.analysis.ICounter; +import org.jacoco.core.data.ExecutionData; import org.jacoco.core.data.ExecutionDataStore; import org.jacoco.core.data.SessionInfoStore; import org.slf4j.Logger; import org.slf4j.LoggerFactory; /** - * Periodically collects code coverage probe data, resolves it to covered source lines using - * JaCoCo's analysis pipeline, and sends the results via a {@link CodeCoverageLcovSender}. + * Periodically collects code coverage probe data, resolves it to covered source lines using a + * cached probe-to-line mapping, and sends the results via a {@link CodeCoverageLcovSender}. + * + *

On the first collection cycle (or when new classes appear), a classpath scan builds the + * cache. Subsequent cycles simply iterate boolean probe arrays and set bits -- no JaCoCo {@code + * Analyzer} pass is needed. */ public final class CodeCoverageCollector { @@ -31,6 +32,7 @@ public final class CodeCoverageCollector { private final CodeCoverageLcovSender sender; private final int intervalSeconds; private final String explicitClasspath; + private final ProbeMappingCache probeCache = new ProbeMappingCache(); private volatile ScheduledExecutorService scheduler; /** @@ -73,7 +75,7 @@ public void stop() { } } - /** Performs a single collection cycle: collect probes, analyze, and send. */ + /** Performs a single collection cycle: collect probes, resolve via cache, and send. */ void collect() { try { // 1. Collect and reset probes @@ -81,35 +83,37 @@ void collect() { SessionInfoStore sessionStore = new SessionInfoStore(); transformer.collectAndReset(execStore, sessionStore); - // 2. Resolve classpath entries - List classpathEntries = resolveClasspath(); - - // 3. Analyze: map probes to source lines using original class files - CoverageBuilder builder = new CoverageBuilder(); - Analyzer analyzer = new Analyzer(execStore, builder); - for (File entry : classpathEntries) { - if (entry.exists()) { - try { - analyzer.analyzeAll(entry); - } catch (IOException e) { - log.debug("Failed to analyze classpath entry: {}", entry, e); - } + // 2. Separate cache hits from misses + Collection allEntries = execStore.getContents(); + List cacheMisses = new ArrayList<>(); + for (ExecutionData ed : allEntries) { + if (probeCache.get(ed.getId()) == null) { + cacheMisses.add(ed); } } - // 4. Build coverage map: source file -> lines coverage + // 3. Build cache entries for misses (scans classpath) + if (!cacheMisses.isEmpty()) { + List classpathEntries = resolveClasspath(); + probeCache.buildMissing(cacheMisses, classpathEntries); + log.debug("Built cache entries for {} new classes", cacheMisses.size()); + } + + // 4. Build coverage from cache Map coverage = new HashMap<>(); - for (IClassCoverage cc : builder.getClasses()) { - if (cc.getSourceFileName() == null) { - continue; + for (ExecutionData ed : allEntries) { + ClassProbeMapping mapping = probeCache.get(ed.getId()); + if (mapping == null || mapping.sourceFile == null) { + continue; // no mapping available } - String sourceFile = cc.getPackageName() + "/" + cc.getSourceFileName(); - LinesCoverage lc = coverage.computeIfAbsent(sourceFile, k -> new LinesCoverage()); - for (int line = cc.getFirstLine(); line <= cc.getLastLine(); line++) { - int status = cc.getLine(line).getStatus(); - if (status != ICounter.EMPTY) { - lc.executableLines.set(line); - if (status == ICounter.PARTLY_COVERED || status == ICounter.FULLY_COVERED) { + + LinesCoverage lc = coverage.computeIfAbsent(mapping.sourceFile, k -> new LinesCoverage()); + lc.executableLines.or(mapping.executableLines); + + boolean[] probes = ed.getProbes(); + for (int p = 0; p < probes.length && p < mapping.probeToLines.length; p++) { + if (probes[p]) { + for (int line : mapping.probeToLines[p]) { lc.coveredLines.set(line); } } diff --git a/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/ProbeMappingCache.java b/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/ProbeMappingCache.java new file mode 100644 index 00000000000..b08d86707e9 --- /dev/null +++ b/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/ProbeMappingCache.java @@ -0,0 +1,148 @@ +package datadog.trace.codecoverage; + +import java.io.ByteArrayOutputStream; +import java.io.File; +import java.io.FileInputStream; +import java.io.IOException; +import java.io.InputStream; +import java.util.BitSet; +import java.util.Collection; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.zip.ZipEntry; +import java.util.zip.ZipInputStream; +import org.jacoco.core.data.ExecutionData; +import org.jacoco.core.internal.data.CRC64; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Maintains a cache of {@link ClassProbeMapping} entries, keyed by CRC64 class ID. Builds entries + * lazily from classpath analysis when cache misses occur. + */ +final class ProbeMappingCache { + + private static final Logger log = LoggerFactory.getLogger(ProbeMappingCache.class); + + private final Map cache = new HashMap<>(); + + /** Returns the cached mapping for the given class ID, or null if not cached. */ + ClassProbeMapping get(long classId) { + return cache.get(classId); + } + + /** + * Populates cache entries for all classes in {@code missingClasses} by scanning the given + * classpath entries. Each classpath entry is a jar or directory. + * + *

For each class file found, computes CRC64 and checks if it matches any missing class. If + * so, builds a ClassProbeMapping and adds it to the cache. + * + * @param missingClasses execution data entries that have no cached mapping + * @param classpathEntries jars/directories to scan for class files + */ + void buildMissing(Collection missingClasses, List classpathEntries) { + // Build a lookup: classId -> ExecutionData for the missing entries + Map needed = new HashMap<>(); + for (ExecutionData ed : missingClasses) { + needed.put(ed.getId(), ed); + } + + for (File entry : classpathEntries) { + if (needed.isEmpty()) { + break; + } + if (!entry.exists()) { + continue; + } + + try { + if (entry.isDirectory()) { + scanDirectory(entry, needed); + } else if (entry.getName().endsWith(".jar") || entry.getName().endsWith(".zip")) { + scanJar(entry, needed); + } + } catch (IOException e) { + log.debug("Failed to scan classpath entry for cache building: {}", entry, e); + } + } + + // Any remaining entries in 'needed' couldn't be found on the classpath. + // Mark them in the cache with a sentinel so we don't rescan for them. + for (Map.Entry e : needed.entrySet()) { + cache.put( + e.getKey(), new ClassProbeMapping(e.getKey(), null, new BitSet(), new int[0][])); + log.debug( + "Class {} (id {}) not found on classpath; skipping", + e.getValue().getName(), + Long.toHexString(e.getKey())); + } + } + + private void scanDirectory(File dir, Map needed) throws IOException { + File[] files = dir.listFiles(); + if (files == null) { + return; + } + for (File f : files) { + if (needed.isEmpty()) { + return; + } + if (f.isDirectory()) { + scanDirectory(f, needed); + } else if (f.getName().endsWith(".class")) { + // Use try-with-resources to avoid leaking the FileInputStream + try (FileInputStream fis = new FileInputStream(f)) { + byte[] bytes = readAllBytes(fis); + tryBuildMapping(bytes, needed); + } + } + } + } + + private void scanJar(File jarFile, Map needed) throws IOException { + try (ZipInputStream zis = new ZipInputStream(new FileInputStream(jarFile))) { + ZipEntry entry; + while ((entry = zis.getNextEntry()) != null && !needed.isEmpty()) { + if (entry.getName().endsWith(".class") && !entry.isDirectory()) { + // Do NOT close the ZipInputStream here -- readAllBytes reads without closing + byte[] bytes = readAllBytes(zis); + tryBuildMapping(bytes, needed); + } + } + } + } + + private void tryBuildMapping(byte[] classBytes, Map needed) { + long crc = CRC64.classId(classBytes); + ExecutionData ed = needed.get(crc); + if (ed == null) { + return; // this class isn't one we're looking for + } + + try { + ClassProbeMapping mapping = + ClassProbeMappingBuilder.build( + ed.getId(), ed.getName(), ed.getProbes().length, classBytes); + cache.put(ed.getId(), mapping); + needed.remove(crc); + } catch (Exception e) { + log.debug("Failed to build probe mapping for class {}", ed.getName(), e); + } + } + + /** + * Reads all bytes from an input stream WITHOUT closing it (important for ZipInputStream where + * closing the stream would close the zip). + */ + private static byte[] readAllBytes(InputStream is) throws IOException { + ByteArrayOutputStream out = new ByteArrayOutputStream(); + byte[] buf = new byte[4096]; + int r; + while ((r = is.read(buf)) != -1) { + out.write(buf, 0, r); + } + return out.toByteArray(); + } +} diff --git a/gradle.properties b/gradle.properties index 0c75ce4e3a4..c51c2eadd4d 100644 --- a/gradle.properties +++ b/gradle.properties @@ -1,6 +1,6 @@ org.gradle.parallel=true org.gradle.caching=true -org.gradle.jvmargs=-Xmx2g -XX:MaxMetaspaceSize=1g +org.gradle.jvmargs=-Xmx4g -XX:MaxMetaspaceSize=1g # Toggle on to get more details during IJ sync #org.gradle.logging.level=info From d0bf9306e56e05713645d28f3669441f5780dc47 Mon Sep 17 00:00:00 2001 From: Nikita Tkachenko Date: Tue, 24 Mar 2026 18:39:38 +0100 Subject: [PATCH 04/19] Replace LCOV with compact binary coverage protocol Implement Coverage Binary Protocol v1 (CoverageBinaryEncoder) using two bit vectors per record for executable and covered lines. Switch from LCOV text format to the new "ddcov" binary format for coverage uploads. Add className to ClassProbeMapping, key coverage data by CoverageKey (sourceFile + className), and include language/env tags in upload metadata. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../trace/codecoverage/ClassProbeMapping.java | 10 ++- .../ClassProbeMappingBuilder.java | 12 ++-- .../codecoverage/CodeCoverageCollector.java | 14 ++-- ...covSender.java => CodeCoverageSender.java} | 18 ++--- .../codecoverage/CodeCoverageSystem.java | 10 ++- .../codecoverage/CoverageBinaryEncoder.java | 68 +++++++++++++++++++ .../trace/codecoverage/ProbeMappingCache.java | 3 +- .../datadog/trace/coverage/CoverageKey.java | 27 ++++++++ 8 files changed, 136 insertions(+), 26 deletions(-) rename dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/{CodeCoverageLcovSender.java => CodeCoverageSender.java} (53%) create mode 100644 dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CoverageBinaryEncoder.java create mode 100644 utils/coverage-utils/src/main/java/datadog/trace/coverage/CoverageKey.java diff --git a/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/ClassProbeMapping.java b/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/ClassProbeMapping.java index cc952b85129..71c20302ed4 100644 --- a/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/ClassProbeMapping.java +++ b/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/ClassProbeMapping.java @@ -8,13 +8,19 @@ */ final class ClassProbeMapping { final long classId; - final String sourceFile; // "package/SourceFile.java" + final String className; // "com/example/MyClass" + final String sourceFile; // "SourceFile.java" final BitSet executableLines; final int[][] probeToLines; // probeToLines[probeId] = sorted line numbers ClassProbeMapping( - long classId, String sourceFile, BitSet executableLines, int[][] probeToLines) { + long classId, + String className, + String sourceFile, + BitSet executableLines, + int[][] probeToLines) { this.classId = classId; + this.className = className; this.sourceFile = sourceFile; this.executableLines = executableLines; this.probeToLines = probeToLines; diff --git a/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/ClassProbeMappingBuilder.java b/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/ClassProbeMappingBuilder.java index 74b48bb6244..ce3d7e6a49c 100644 --- a/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/ClassProbeMappingBuilder.java +++ b/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/ClassProbeMappingBuilder.java @@ -41,9 +41,7 @@ static ClassProbeMapping build( Analyzer analyzer = new Analyzer(store, builder); analyzer.analyzeClass(classBytes, className); for (IClassCoverage cc : builder.getClasses()) { - if (cc.getSourceFileName() != null) { - sourceFile = cc.getPackageName() + "/" + cc.getSourceFileName(); - } + sourceFile = cc.getSourceFileName(); for (int line = cc.getFirstLine(); line <= cc.getLastLine(); line++) { if (cc.getLine(line).getStatus() != ICounter.EMPTY) { executableLines.set(line); @@ -52,9 +50,9 @@ static ClassProbeMapping build( } } - if (sourceFile == null) { - // No source info -- create a mapping with no lines - return new ClassProbeMapping(classId, null, executableLines, new int[probeCount][]); + if (executableLines.isEmpty()) { + return new ClassProbeMapping( + classId, className, sourceFile, executableLines, new int[probeCount][]); } // 2. Build per-probe line mapping @@ -84,7 +82,7 @@ static ClassProbeMapping build( probeToLines[probeId] = bitSetToArray(coveredByProbe); } - return new ClassProbeMapping(classId, sourceFile, executableLines, probeToLines); + return new ClassProbeMapping(classId, className, sourceFile, executableLines, probeToLines); } private static int[] bitSetToArray(BitSet bs) { diff --git a/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageCollector.java b/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageCollector.java index d13510fdf49..b00d27e8d3a 100644 --- a/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageCollector.java +++ b/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageCollector.java @@ -1,5 +1,6 @@ package datadog.trace.codecoverage; +import datadog.trace.coverage.CoverageKey; import datadog.trace.coverage.LinesCoverage; import java.io.File; import java.util.ArrayList; @@ -18,7 +19,7 @@ /** * Periodically collects code coverage probe data, resolves it to covered source lines using a - * cached probe-to-line mapping, and sends the results via a {@link CodeCoverageLcovSender}. + * cached probe-to-line mapping, and sends the results via a {@link CodeCoverageSender}. * *

On the first collection cycle (or when new classes appear), a classpath scan builds the * cache. Subsequent cycles simply iterate boolean probe arrays and set bits -- no JaCoCo {@code @@ -29,7 +30,7 @@ public final class CodeCoverageCollector { private static final Logger log = LoggerFactory.getLogger(CodeCoverageCollector.class); private final CodeCoverageTransformer transformer; - private final CodeCoverageLcovSender sender; + private final CodeCoverageSender sender; private final int intervalSeconds; private final String explicitClasspath; private final ProbeMappingCache probeCache = new ProbeMappingCache(); @@ -43,7 +44,7 @@ public final class CodeCoverageCollector { */ public CodeCoverageCollector( CodeCoverageTransformer transformer, - CodeCoverageLcovSender sender, + CodeCoverageSender sender, int intervalSeconds, String explicitClasspath) { this.transformer = transformer; @@ -100,14 +101,15 @@ void collect() { } // 4. Build coverage from cache - Map coverage = new HashMap<>(); + Map coverage = new HashMap<>(); for (ExecutionData ed : allEntries) { ClassProbeMapping mapping = probeCache.get(ed.getId()); - if (mapping == null || mapping.sourceFile == null) { + if (mapping == null || mapping.className == null) { continue; // no mapping available } - LinesCoverage lc = coverage.computeIfAbsent(mapping.sourceFile, k -> new LinesCoverage()); + CoverageKey key = new CoverageKey(mapping.sourceFile, mapping.className); + LinesCoverage lc = coverage.computeIfAbsent(key, k -> new LinesCoverage()); lc.executableLines.or(mapping.executableLines); boolean[] probes = ed.getProbes(); diff --git a/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageLcovSender.java b/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageSender.java similarity index 53% rename from dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageLcovSender.java rename to dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageSender.java index b223f8dee6e..4178a906f12 100644 --- a/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageLcovSender.java +++ b/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageSender.java @@ -1,28 +1,28 @@ package datadog.trace.codecoverage; +import datadog.trace.coverage.CoverageKey; import datadog.trace.coverage.CoverageReportUploader; -import datadog.trace.coverage.LcovReportWriter; import datadog.trace.coverage.LinesCoverage; import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; import java.io.IOException; -import java.nio.charset.StandardCharsets; import java.util.Map; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -public final class CodeCoverageLcovSender { - private static final Logger log = LoggerFactory.getLogger(CodeCoverageLcovSender.class); +public final class CodeCoverageSender { + private static final Logger log = LoggerFactory.getLogger(CodeCoverageSender.class); private final CoverageReportUploader uploader; - public CodeCoverageLcovSender(CoverageReportUploader uploader) { + public CodeCoverageSender(CoverageReportUploader uploader) { this.uploader = uploader; } - public void upload(Map coverage) { - String lcov = LcovReportWriter.toString(coverage); + public void upload(Map coverage) { try { - uploader.upload( - "lcov", new ByteArrayInputStream(lcov.getBytes(StandardCharsets.UTF_8))); + ByteArrayOutputStream buf = new ByteArrayOutputStream(); + CoverageBinaryEncoder.encode(coverage, buf); + uploader.upload("ddcov", new ByteArrayInputStream(buf.toByteArray())); } catch (IOException e) { log.debug("Failed to upload code coverage report", e); } diff --git a/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageSystem.java b/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageSystem.java index 7aca7e1aca2..8243fa7b304 100644 --- a/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageSystem.java +++ b/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageSystem.java @@ -4,11 +4,13 @@ import datadog.communication.BackendApiFactory; import datadog.communication.ddagent.SharedCommunicationObjects; import datadog.trace.api.Config; +import datadog.trace.api.DDTags; import datadog.trace.api.git.CommitInfo; import datadog.trace.api.git.GitInfo; import datadog.trace.api.git.GitInfoProvider; import datadog.trace.api.git.PersonInfo; import datadog.trace.api.intake.Intake; +import datadog.trace.bootstrap.instrumentation.api.Tags; import datadog.trace.coverage.CoverageReportUploader; import java.lang.instrument.Instrumentation; import java.util.HashMap; @@ -85,8 +87,14 @@ public static void startCollector(Object transformerObj, Object scoObj) { return; } + tags.put(DDTags.LANGUAGE_TAG_KEY, DDTags.LANGUAGE_TAG_VALUE); + String env = config.getEnv(); + if (env != null && !env.isEmpty()) { + tags.put(Tags.ENV, env); + } + CoverageReportUploader uploader = new CoverageReportUploader(backendApi, tags, null); - CodeCoverageLcovSender sender = new CodeCoverageLcovSender(uploader); + CodeCoverageSender sender = new CodeCoverageSender(uploader); CodeCoverageCollector collector = new CodeCoverageCollector( diff --git a/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CoverageBinaryEncoder.java b/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CoverageBinaryEncoder.java new file mode 100644 index 00000000000..3a7dff733c3 --- /dev/null +++ b/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CoverageBinaryEncoder.java @@ -0,0 +1,68 @@ +package datadog.trace.codecoverage; + +import datadog.trace.coverage.CoverageKey; +import datadog.trace.coverage.LinesCoverage; +import java.io.IOException; +import java.io.OutputStream; +import java.nio.charset.StandardCharsets; +import java.util.BitSet; +import java.util.Map; + +/** + * Encodes coverage data into the Coverage Binary Protocol v1 + */ +public final class CoverageBinaryEncoder { + + private static final int VERSION = 1; + private static final int NUM_EXTRA_FIELDS = 1; // className + + public static void encode(Map coverage, OutputStream out) + throws IOException { + out.write(VERSION); + writeUvarint(NUM_EXTRA_FIELDS, out); + writeUvarint(coverage.size(), out); + for (Map.Entry entry : coverage.entrySet()) { + writeRecord(entry.getKey(), entry.getValue(), out); + } + } + + private static void writeRecord(CoverageKey key, LinesCoverage lines, OutputStream out) + throws IOException { + writeString(key.sourceFile, out); + writeString(key.className, out); + + int maxLine = + Math.max(lines.executableLines.length(), lines.coveredLines.length()) - 1; + if (maxLine < 0) { + writeUvarint(0, out); + return; + } + int byteCount = (maxLine >> 3) + 1; + writeUvarint(byteCount, out); + writeBitVector(lines.executableLines, byteCount, out); + writeBitVector(lines.coveredLines, byteCount, out); + } + + private static void writeBitVector(BitSet bits, int byteCount, OutputStream out) + throws IOException { + byte[] data = bits.toByteArray(); + out.write(data, 0, Math.min(data.length, byteCount)); + for (int i = data.length; i < byteCount; i++) { + out.write(0); + } + } + + private static void writeString(String s, OutputStream out) throws IOException { + byte[] bytes = s.getBytes(StandardCharsets.UTF_8); + writeUvarint(bytes.length, out); + out.write(bytes); + } + + static void writeUvarint(int value, OutputStream out) throws IOException { + while (value >= 0x80) { + out.write((value & 0x7F) | 0x80); + value >>>= 7; + } + out.write(value); + } +} diff --git a/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/ProbeMappingCache.java b/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/ProbeMappingCache.java index b08d86707e9..39a629e92e3 100644 --- a/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/ProbeMappingCache.java +++ b/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/ProbeMappingCache.java @@ -72,7 +72,8 @@ void buildMissing(Collection missingClasses, List classpath // Mark them in the cache with a sentinel so we don't rescan for them. for (Map.Entry e : needed.entrySet()) { cache.put( - e.getKey(), new ClassProbeMapping(e.getKey(), null, new BitSet(), new int[0][])); + e.getKey(), + new ClassProbeMapping(e.getKey(), null, null, new BitSet(), new int[0][])); log.debug( "Class {} (id {}) not found on classpath; skipping", e.getValue().getName(), diff --git a/utils/coverage-utils/src/main/java/datadog/trace/coverage/CoverageKey.java b/utils/coverage-utils/src/main/java/datadog/trace/coverage/CoverageKey.java new file mode 100644 index 00000000000..560361f5960 --- /dev/null +++ b/utils/coverage-utils/src/main/java/datadog/trace/coverage/CoverageKey.java @@ -0,0 +1,27 @@ +package datadog.trace.coverage; + +import java.util.Objects; + +public class CoverageKey { + public final String sourceFile; + public final String className; + + public CoverageKey(String sourceFile, String className) { + this.sourceFile = sourceFile; + this.className = className; + } + + @Override + public boolean equals(Object o) { + if (o == null || getClass() != o.getClass()) { + return false; + } + CoverageKey that = (CoverageKey) o; + return Objects.equals(sourceFile, that.sourceFile) && Objects.equals(className, that.className); + } + + @Override + public int hashCode() { + return Objects.hash(sourceFile, className); + } +} From d9e68ea5ff228ea8059f52f95d9bb9f3b28e76a1 Mon Sep 17 00:00:00 2001 From: Nikita Tkachenko Date: Tue, 24 Mar 2026 18:41:12 +0100 Subject: [PATCH 05/19] Add tests for the binary cov data encoder --- .../CoverageBinaryEncoderTest.java | 515 ++++++++++++++++++ 1 file changed, 515 insertions(+) create mode 100644 dd-java-agent/agent-code-coverage/src/test/java/datadog/trace/codecoverage/CoverageBinaryEncoderTest.java diff --git a/dd-java-agent/agent-code-coverage/src/test/java/datadog/trace/codecoverage/CoverageBinaryEncoderTest.java b/dd-java-agent/agent-code-coverage/src/test/java/datadog/trace/codecoverage/CoverageBinaryEncoderTest.java new file mode 100644 index 00000000000..1b6a26f8a35 --- /dev/null +++ b/dd-java-agent/agent-code-coverage/src/test/java/datadog/trace/codecoverage/CoverageBinaryEncoderTest.java @@ -0,0 +1,515 @@ +package datadog.trace.codecoverage; + +import static org.junit.jupiter.api.Assertions.assertArrayEquals; +import static org.junit.jupiter.api.Assertions.assertEquals; + +import datadog.trace.coverage.CoverageKey; +import datadog.trace.coverage.LinesCoverage; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.util.LinkedHashMap; +import java.util.Map; +import org.junit.jupiter.api.Test; + +class CoverageBinaryEncoderTest { + + // --- uvarint encoding --- + + @Test + void uvarintZero() throws IOException { + assertUvarint(0, new byte[] {0x00}); + } + + @Test + void uvarintSingleByte() throws IOException { + assertUvarint(1, new byte[] {0x01}); + assertUvarint(0x7F, new byte[] {0x7F}); + } + + @Test + void uvarintTwoBytes() throws IOException { + // 128 = 0x80 → low 7 bits = 0x00 with continuation, then 0x01 + assertUvarint(128, new byte[] {(byte) 0x80, 0x01}); + // 16383 = 0x3FFF → 0xFF, 0x7F + assertUvarint(16383, new byte[] {(byte) 0xFF, 0x7F}); + } + + @Test + void uvarintThreeBytes() throws IOException { + // 16384 = 0x4000 → 0x80, 0x80, 0x01 + assertUvarint(16384, new byte[] {(byte) 0x80, (byte) 0x80, 0x01}); + } + + @Test + void uvarintLargeValue() throws IOException { + // 300 = 0x12C → low 7: 0x2C | 0x80 = 0xAC, remaining 2 → 0x02 + assertUvarint(300, new byte[] {(byte) 0xAC, 0x02}); + } + + // --- Empty coverage map --- + + @Test + void emptyMapProducesHeaderOnly() throws IOException { + Map coverage = new LinkedHashMap<>(); + byte[] result = encode(coverage); + // version=1, num_extra_fields=1, num_records=0 + assertArrayEquals(new byte[] {0x01, 0x01, 0x00}, result); + } + + // --- Single record with empty BitSets --- + + @Test + void singleRecordEmptyLines() throws IOException { + Map coverage = new LinkedHashMap<>(); + coverage.put(new CoverageKey("A.java", "A"), new LinesCoverage()); + + byte[] result = encode(coverage); + ByteArrayOutputStream expected = new ByteArrayOutputStream(); + expected.write(0x01); // version + expected.write(0x01); // num_extra_fields + expected.write(0x01); // num_records = 1 + writeExpectedString("A.java", expected); + writeExpectedString("A", expected); + expected.write(0x00); // bitvec_byte_count = 0, no bit vector data + + assertArrayEquals(expected.toByteArray(), result); + } + + // --- Bit vector encoding --- + + @Test + void singleLineSet() throws IOException { + // Line 1 only: byte_count = (1>>3)+1 = 1, exec byte 0 = 0x02, cov byte 0 = 0x02 + Map coverage = new LinkedHashMap<>(); + LinesCoverage lc = new LinesCoverage(); + lc.executableLines.set(1); + lc.coveredLines.set(1); + coverage.put(new CoverageKey("X.java", "X"), lc); + + byte[] result = encode(coverage); + ByteArrayOutputStream expected = new ByteArrayOutputStream(); + expected.write(0x01); // version + expected.write(0x01); // num_extra_fields + expected.write(0x01); // num_records + writeExpectedString("X.java", expected); + writeExpectedString("X", expected); + expected.write(0x01); // bitvec_byte_count = 1 + expected.write(0x02); // executable: line 1 → bit 1 of byte 0 + expected.write(0x02); // covered: line 1 → bit 1 of byte 0 + + assertArrayEquals(expected.toByteArray(), result); + } + + @Test + void linesSpanMultipleBytes() throws IOException { + // Lines {1, 8}: max_line=8, byte_count=(8>>3)+1=2 + // Line 1: byte 0, bit 1 → 0x02 + // Line 8: byte 1, bit 0 → 0x01 + Map coverage = new LinkedHashMap<>(); + LinesCoverage lc = new LinesCoverage(); + lc.executableLines.set(1); + lc.executableLines.set(8); + lc.coveredLines.set(8); + coverage.put(new CoverageKey("F.java", "F"), lc); + + byte[] result = encode(coverage); + ByteArrayOutputStream expected = new ByteArrayOutputStream(); + expected.write(0x01); + expected.write(0x01); + expected.write(0x01); + writeExpectedString("F.java", expected); + writeExpectedString("F", expected); + expected.write(0x02); // bitvec_byte_count = 2 + expected.write(0x02); // exec byte 0: line 1 + expected.write(0x01); // exec byte 1: line 8 + expected.write(0x00); // cov byte 0: no lines + expected.write(0x01); // cov byte 1: line 8 + + assertArrayEquals(expected.toByteArray(), result); + } + + @Test + void coveredLinesBitVectorPaddedWithZeros() throws IOException { + // executable has line 15 (byte 1), covered has only line 1 (byte 0) + // Both bit vectors must be 2 bytes (covered padded to match) + Map coverage = new LinkedHashMap<>(); + LinesCoverage lc = new LinesCoverage(); + lc.executableLines.set(1); + lc.executableLines.set(15); + lc.coveredLines.set(1); + coverage.put(new CoverageKey("P.java", "P"), lc); + + byte[] result = encode(coverage); + ByteArrayOutputStream expected = new ByteArrayOutputStream(); + expected.write(0x01); + expected.write(0x01); + expected.write(0x01); + writeExpectedString("P.java", expected); + writeExpectedString("P", expected); + expected.write(0x02); // bitvec_byte_count = 2 (max line 15: (15>>3)+1=2) + expected.write(0x02); // exec byte 0: line 1 + expected.write((byte) 0x80); // exec byte 1: line 15 → bit 7 + expected.write(0x02); // cov byte 0: line 1 + expected.write(0x00); // cov byte 1: padding + + assertArrayEquals(expected.toByteArray(), result); + } + + @Test + void executableLinesBitVectorPaddedWhenCoveredHasHigherLine() throws IOException { + // covered has line 10 (higher than executable's max of 3) + // This violates the spec constraint but encoder should still handle it + Map coverage = new LinkedHashMap<>(); + LinesCoverage lc = new LinesCoverage(); + lc.executableLines.set(3); + lc.coveredLines.set(10); + coverage.put(new CoverageKey("Q.java", "Q"), lc); + + byte[] result = encode(coverage); + ByteArrayOutputStream expected = new ByteArrayOutputStream(); + expected.write(0x01); + expected.write(0x01); + expected.write(0x01); + writeExpectedString("Q.java", expected); + writeExpectedString("Q", expected); + expected.write(0x02); // bitvec_byte_count = (10>>3)+1 = 2 + expected.write(0x08); // exec byte 0: line 3 → bit 3 + expected.write(0x00); // exec byte 1: padding + expected.write(0x00); // cov byte 0: no lines in lower byte + expected.write(0x04); // cov byte 1: line 10 → byte 1, bit 2 + + assertArrayEquals(expected.toByteArray(), result); + } + + // --- Multiple records --- + + @Test + void multipleRecords() throws IOException { + Map coverage = new LinkedHashMap<>(); + + LinesCoverage lc1 = new LinesCoverage(); + lc1.executableLines.set(1); + lc1.coveredLines.set(1); + coverage.put(new CoverageKey("A.java", "A"), lc1); + + LinesCoverage lc2 = new LinesCoverage(); + lc2.executableLines.set(2); + coverage.put(new CoverageKey("B.java", "B"), lc2); + + byte[] result = encode(coverage); + ByteArrayOutputStream expected = new ByteArrayOutputStream(); + expected.write(0x01); // version + expected.write(0x01); // num_extra_fields + expected.write(0x02); // num_records = 2 + + // Record 1 + writeExpectedString("A.java", expected); + writeExpectedString("A", expected); + expected.write(0x01); // bitvec_byte_count = 1 + expected.write(0x02); // exec: line 1 + expected.write(0x02); // cov: line 1 + + // Record 2 + writeExpectedString("B.java", expected); + writeExpectedString("B", expected); + expected.write(0x01); // bitvec_byte_count = 1 + expected.write(0x04); // exec: line 2 + expected.write(0x00); // cov: none + + assertArrayEquals(expected.toByteArray(), result); + } + + // --- String encoding --- + + @Test + void emptyStringEncoding() throws IOException { + Map coverage = new LinkedHashMap<>(); + coverage.put(new CoverageKey("", ""), new LinesCoverage()); + + byte[] result = encode(coverage); + ByteArrayOutputStream expected = new ByteArrayOutputStream(); + expected.write(0x01); // version + expected.write(0x01); // num_extra_fields + expected.write(0x01); // num_records + expected.write(0x00); // file_name: length 0 + expected.write(0x00); // extra_fields[0]: length 0 + expected.write(0x00); // bitvec_byte_count = 0 + + assertArrayEquals(expected.toByteArray(), result); + } + + @Test + void utf8MultiByteStringEncoding() throws IOException { + // UTF-8 multi-byte: "Ñ" is 2 bytes (0xC3 0x91) + Map coverage = new LinkedHashMap<>(); + coverage.put(new CoverageKey("Ñ.java", "Ñ"), new LinesCoverage()); + + byte[] result = encode(coverage); + byte[] fileName = "Ñ.java".getBytes(StandardCharsets.UTF_8); + byte[] className = "Ñ".getBytes(StandardCharsets.UTF_8); + + ByteArrayOutputStream expected = new ByteArrayOutputStream(); + expected.write(0x01); + expected.write(0x01); + expected.write(0x01); + // file_name length is byte count, not char count + writeExpectedUvarint(fileName.length, expected); + expected.write(fileName); + writeExpectedUvarint(className.length, expected); + expected.write(className); + expected.write(0x00); // bitvec_byte_count + + assertArrayEquals(expected.toByteArray(), result); + // Verify byte length != char length + assertEquals(7, fileName.length); // "Ñ" is 2 bytes + ".java" is 5 bytes + } + + // --- Spec example --- + + @Test + void specExampleTwoRecords() throws IOException { + Map coverage = new LinkedHashMap<>(); + + // Record 1: com/example/Foo.java, com.example.Foo, exec={1,2,3,5,8}, cov={1,3,5} + LinesCoverage lc1 = new LinesCoverage(); + for (int line : new int[] {1, 2, 3, 5, 8}) { + lc1.executableLines.set(line); + } + for (int line : new int[] {1, 3, 5}) { + lc1.coveredLines.set(line); + } + coverage.put(new CoverageKey("com/example/Foo.java", "com.example.Foo"), lc1); + + // Record 2: com/example/Bar.java, com.example.Bar, exec={2,4,6}, cov={4} + LinesCoverage lc2 = new LinesCoverage(); + for (int line : new int[] {2, 4, 6}) { + lc2.executableLines.set(line); + } + lc2.coveredLines.set(4); + coverage.put(new CoverageKey("com/example/Bar.java", "com.example.Bar"), lc2); + + byte[] result = encode(coverage); + + // Expected byte sequence from the spec + byte[] expected = { + 0x01, 0x01, 0x02, // header: version=1, extra_fields=1, records=2 + 0x14, // file_name length = 20 + 0x63, 0x6F, 0x6D, 0x2F, 0x65, 0x78, 0x61, 0x6D, // "com/exam" + 0x70, 0x6C, 0x65, 0x2F, 0x46, 0x6F, 0x6F, 0x2E, // "ple/Foo." + 0x6A, 0x61, 0x76, 0x61, // "java" + 0x0F, // extra_fields[0] length = 15 + 0x63, 0x6F, 0x6D, 0x2E, 0x65, 0x78, 0x61, 0x6D, // "com.exam" + 0x70, 0x6C, 0x65, 0x2E, 0x46, 0x6F, 0x6F, // "ple.Foo" + 0x02, // bitvec_byte_count = 2 + 0x2E, 0x01, // executable_lines + 0x2A, 0x00, // covered_lines + 0x14, // file_name length = 20 + 0x63, 0x6F, 0x6D, 0x2F, 0x65, 0x78, 0x61, 0x6D, // "com/exam" + 0x70, 0x6C, 0x65, 0x2F, 0x42, 0x61, 0x72, 0x2E, // "ple/Bar." + 0x6A, 0x61, 0x76, 0x61, // "java" + 0x0F, // extra_fields[0] length = 15 + 0x63, 0x6F, 0x6D, 0x2E, 0x65, 0x78, 0x61, 0x6D, // "com.exam" + 0x70, 0x6C, 0x65, 0x2E, 0x42, 0x61, 0x72, // "ple.Bar" + 0x01, // bitvec_byte_count = 1 + 0x54, // executable_lines + 0x10 // covered_lines + }; + + assertArrayEquals(expected, result); + } + + // --- Edge cases --- + + @Test + void highLineNumber() throws IOException { + // Line 1000: byte index = 1000>>3 = 125, bit = 1000&7 = 0 + // byte_count = (1000>>3)+1 = 126 + Map coverage = new LinkedHashMap<>(); + LinesCoverage lc = new LinesCoverage(); + lc.executableLines.set(1000); + lc.coveredLines.set(1000); + coverage.put(new CoverageKey("H.java", "H"), lc); + + byte[] result = encode(coverage); + + // Parse manually: skip header and strings, check bitvec + int offset = 0; + assertEquals(0x01, result[offset++] & 0xFF); // version + assertEquals(0x01, result[offset++] & 0xFF); // num_extra_fields + assertEquals(0x01, result[offset++] & 0xFF); // num_records + + // Skip file_name "H.java" (length 6) + assertEquals(0x06, result[offset++] & 0xFF); + offset += 6; + + // Skip extra_field "H" (length 1) + assertEquals(0x01, result[offset++] & 0xFF); + offset += 1; + + // bitvec_byte_count = 126 → varint encoding: (126 & 0x7F) = 0x7E, fits in 1 byte + assertEquals(126, result[offset++] & 0xFF); + + // executable_lines: 126 bytes, only byte 125 has bit 0 set + for (int i = 0; i < 126; i++) { + int expectedByte = (i == 125) ? 0x01 : 0x00; + assertEquals(expectedByte, result[offset + i] & 0xFF, "exec byte " + i); + } + offset += 126; + + // covered_lines: same pattern + for (int i = 0; i < 126; i++) { + int expectedByte = (i == 125) ? 0x01 : 0x00; + assertEquals(expectedByte, result[offset + i] & 0xFF, "cov byte " + i); + } + } + + @Test + void line7SetsHighBitOfByte0() throws IOException { + // Line 7: byte 0, bit 7 → 0x80 + Map coverage = new LinkedHashMap<>(); + LinesCoverage lc = new LinesCoverage(); + lc.executableLines.set(7); + coverage.put(new CoverageKey("S.java", "S"), lc); + + byte[] result = encode(coverage); + ByteArrayOutputStream expected = new ByteArrayOutputStream(); + expected.write(0x01); + expected.write(0x01); + expected.write(0x01); + writeExpectedString("S.java", expected); + writeExpectedString("S", expected); + expected.write(0x01); // bitvec_byte_count = 1 + expected.write((byte) 0x80); // exec: line 7 → bit 7 + expected.write(0x00); // cov: none + + assertArrayEquals(expected.toByteArray(), result); + } + + @Test + void allLinesInOneByte() throws IOException { + // Lines {1,2,3,4,5,6,7}: all in byte 0 + Map coverage = new LinkedHashMap<>(); + LinesCoverage lc = new LinesCoverage(); + for (int i = 1; i <= 7; i++) { + lc.executableLines.set(i); + } + lc.coveredLines.set(4); + coverage.put(new CoverageKey("Z.java", "Z"), lc); + + byte[] result = encode(coverage); + ByteArrayOutputStream expected = new ByteArrayOutputStream(); + expected.write(0x01); + expected.write(0x01); + expected.write(0x01); + writeExpectedString("Z.java", expected); + writeExpectedString("Z", expected); + expected.write(0x01); // bitvec_byte_count = 1 + // exec: bits 1-7 set → 0xFE + expected.write((byte) 0xFE); + // cov: bit 4 → 0x10 + expected.write(0x10); + + assertArrayEquals(expected.toByteArray(), result); + } + + @Test + void onlyExecutableLinesNoCoverage() throws IOException { + Map coverage = new LinkedHashMap<>(); + LinesCoverage lc = new LinesCoverage(); + lc.executableLines.set(1); + lc.executableLines.set(5); + coverage.put(new CoverageKey("N.java", "N"), lc); + + byte[] result = encode(coverage); + ByteArrayOutputStream expected = new ByteArrayOutputStream(); + expected.write(0x01); + expected.write(0x01); + expected.write(0x01); + writeExpectedString("N.java", expected); + writeExpectedString("N", expected); + expected.write(0x01); // bitvec_byte_count = 1 + expected.write(0x22); // exec: line 1 (0x02) | line 5 (0x20) = 0x22 + expected.write(0x00); // cov: empty + + assertArrayEquals(expected.toByteArray(), result); + } + + @Test + void stringLengthRequiresMultiByteUvarint() throws IOException { + // Create a string longer than 127 bytes so its length needs 2 uvarint bytes + StringBuilder sb = new StringBuilder(); + for (int i = 0; i < 130; i++) { + sb.append('a'); + } + String longName = sb.toString(); + + Map coverage = new LinkedHashMap<>(); + coverage.put(new CoverageKey(longName, "C"), new LinesCoverage()); + + byte[] result = encode(coverage); + + // Check the uvarint encoding of 130: (130 & 0x7F) | 0x80 = 0x82, 130 >> 7 = 1 → 0x01 + int offset = 3; // skip version + num_extra_fields + num_records + assertEquals((byte) 0x82, result[offset]); // low 7 bits of 130 with continuation + assertEquals((byte) 0x01, result[offset + 1]); // remaining bits + offset += 2; + // Verify string data + for (int i = 0; i < 130; i++) { + assertEquals((byte) 'a', result[offset + i]); + } + } + + @Test + void outputSizeMatchesExpectedForSpecExample() throws IOException { + // The spec says total message size is 85 bytes + Map coverage = new LinkedHashMap<>(); + + LinesCoverage lc1 = new LinesCoverage(); + for (int line : new int[] {1, 2, 3, 5, 8}) { + lc1.executableLines.set(line); + } + for (int line : new int[] {1, 3, 5}) { + lc1.coveredLines.set(line); + } + coverage.put(new CoverageKey("com/example/Foo.java", "com.example.Foo"), lc1); + + LinesCoverage lc2 = new LinesCoverage(); + for (int line : new int[] {2, 4, 6}) { + lc2.executableLines.set(line); + } + lc2.coveredLines.set(4); + coverage.put(new CoverageKey("com/example/Bar.java", "com.example.Bar"), lc2); + + byte[] result = encode(coverage); + assertEquals(85, result.length); + } + + // --- Helpers --- + + private static byte[] encode(Map coverage) throws IOException { + ByteArrayOutputStream out = new ByteArrayOutputStream(); + CoverageBinaryEncoder.encode(coverage, out); + return out.toByteArray(); + } + + private static void assertUvarint(int value, byte[] expected) throws IOException { + ByteArrayOutputStream out = new ByteArrayOutputStream(); + CoverageBinaryEncoder.writeUvarint(value, out); + assertArrayEquals(expected, out.toByteArray(), "uvarint(" + value + ")"); + } + + private static void writeExpectedUvarint(int value, ByteArrayOutputStream out) { + while (value >= 0x80) { + out.write((value & 0x7F) | 0x80); + value >>>= 7; + } + out.write(value); + } + + private static void writeExpectedString(String s, ByteArrayOutputStream out) throws IOException { + byte[] bytes = s.getBytes(StandardCharsets.UTF_8); + writeExpectedUvarint(bytes.length, out); + out.write(bytes); + } +} From ba20380a21ff3eb78091034621cc2a1d98c5a817 Mon Sep 17 00:00:00 2001 From: Nikita Tkachenko Date: Tue, 24 Mar 2026 21:46:16 +0100 Subject: [PATCH 06/19] Fix NPE in probe cache for classes with no executable lines `new int[probeCount][]` creates an array of null references. When the collector iterates `probeToLines[p]`, it NPEs on classes that have probes but no executable lines (no debug info, interfaces). Initialize each entry to `new int[0]` instead. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../trace/codecoverage/ClassProbeMappingBuilder.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/ClassProbeMappingBuilder.java b/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/ClassProbeMappingBuilder.java index ce3d7e6a49c..e1fc1784a67 100644 --- a/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/ClassProbeMappingBuilder.java +++ b/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/ClassProbeMappingBuilder.java @@ -1,6 +1,7 @@ package datadog.trace.codecoverage; import java.io.IOException; +import java.util.Arrays; import java.util.BitSet; import org.jacoco.core.analysis.Analyzer; import org.jacoco.core.analysis.CoverageBuilder; @@ -51,8 +52,11 @@ static ClassProbeMapping build( } if (executableLines.isEmpty()) { + // No executable lines — return mapping with empty (not null) arrays for each probe + int[][] emptyProbeToLines = new int[probeCount][]; + Arrays.fill(emptyProbeToLines, new int[0]); return new ClassProbeMapping( - classId, className, sourceFile, executableLines, new int[probeCount][]); + classId, className, sourceFile, executableLines, emptyProbeToLines); } // 2. Build per-probe line mapping From e465278ec4edb69ef9fce1f626c7196c20bc00f6 Mon Sep 17 00:00:00 2001 From: Nikita Tkachenko Date: Tue, 24 Mar 2026 22:05:47 +0100 Subject: [PATCH 07/19] Skip classes with null sourceFile, log instrumentation failures - Skip classes without a SourceFile attribute in the collector to prevent NPE in the binary encoder (null is not a valid string in the coverage binary protocol) - Log instrumentation failures at debug level so they are visible when troubleshooting Co-Authored-By: Claude Opus 4.6 (1M context) --- .../datadog/trace/codecoverage/CodeCoverageCollector.java | 2 +- .../datadog/trace/codecoverage/CodeCoverageTransformer.java | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageCollector.java b/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageCollector.java index b00d27e8d3a..fcf7ff3e5c0 100644 --- a/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageCollector.java +++ b/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageCollector.java @@ -104,7 +104,7 @@ void collect() { Map coverage = new HashMap<>(); for (ExecutionData ed : allEntries) { ClassProbeMapping mapping = probeCache.get(ed.getId()); - if (mapping == null || mapping.className == null) { + if (mapping == null || mapping.className == null || mapping.sourceFile == null) { continue; // no mapping available } diff --git a/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageTransformer.java b/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageTransformer.java index a83fa3611db..7ad06a4e831 100644 --- a/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageTransformer.java +++ b/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageTransformer.java @@ -19,6 +19,8 @@ import org.jacoco.core.runtime.IRuntime; import org.jacoco.core.runtime.InjectedClassRuntime; import org.jacoco.core.runtime.RuntimeData; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * A {@link ClassFileTransformer} that uses JaCoCo's {@link Instrumenter} to insert boolean probes @@ -29,6 +31,8 @@ */ public final class CodeCoverageTransformer implements ClassFileTransformer { + private static final Logger log = LoggerFactory.getLogger(CodeCoverageTransformer.class); + private final RuntimeData runtimeData; private final Instrumenter instrumenter; private final Predicate filter; @@ -119,6 +123,7 @@ public byte[] transform( try { return instrumenter.instrument(classfileBuffer, className); } catch (Exception e) { + log.debug("Failed to instrument class {}", className, e); return null; } } From f3c6408ca54fff4a8566363e9faa6b97aa24f6b2 Mon Sep 17 00:00:00 2001 From: Nikita Tkachenko Date: Wed, 25 Mar 2026 16:47:49 +0100 Subject: [PATCH 08/19] Replace N+1 Analyzer passes with single-pass CFG walking for probe cache ClassProbeMappingBuilder previously ran JaCoCo's Analyzer N+1 times per class (once per probe + once for executable lines). For a class with 200 probes, that meant 201 full ASM parses of the same bytecode. The new implementation parses the class once using ClassProbesAdapter, builds a simplified instruction graph (ProbeNode with predecessor links), and walks predecessor chains to determine which lines each probe covers. ~200x faster for large classes. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../ClassProbeMappingBuilder.java | 406 ++++++++++++++---- 1 file changed, 332 insertions(+), 74 deletions(-) diff --git a/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/ClassProbeMappingBuilder.java b/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/ClassProbeMappingBuilder.java index e1fc1784a67..0ba0c734f99 100644 --- a/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/ClassProbeMappingBuilder.java +++ b/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/ClassProbeMappingBuilder.java @@ -1,100 +1,358 @@ package datadog.trace.codecoverage; -import java.io.IOException; -import java.util.Arrays; +import java.util.ArrayList; import java.util.BitSet; -import org.jacoco.core.analysis.Analyzer; -import org.jacoco.core.analysis.CoverageBuilder; -import org.jacoco.core.analysis.IClassCoverage; -import org.jacoco.core.analysis.ICounter; -import org.jacoco.core.data.ExecutionData; -import org.jacoco.core.data.ExecutionDataStore; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +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.jacoco.core.internal.instr.InstrSupport; +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; /** - * Builds a {@link ClassProbeMapping} by running JaCoCo's {@link Analyzer} with controlled probe - * configurations. + * Builds a {@link ClassProbeMapping} by parsing the class bytecode once using JaCoCo's {@link + * ClassProbesAdapter}, building a simplified instruction graph, and walking predecessor chains to + * determine which lines each probe covers. * - *

For a class with N probes, this runs N+1 analyzer passes: one with all probes false (to - * determine executable lines), then one per probe (to determine which lines each probe covers). + *

This replaces the previous N+1 pass approach (one {@code Analyzer} pass per probe plus one for + * executable lines) with a single-pass design that is significantly faster for classes with many + * probes. */ final class ClassProbeMappingBuilder { + static ClassProbeMapping build( + long classId, String className, int probeCount, byte[] classBytes) { + ClassReader reader = InstrSupport.classReaderFor(classBytes); + ProbeMappingVisitor visitor = new ProbeMappingVisitor(); + ClassProbesAdapter adapter = new ClassProbesAdapter(visitor, false); + reader.accept(adapter, 0); + return visitor.toMapping(classId, className, probeCount); + } + + /** Simplified instruction node with a line number and a single predecessor link. */ + static final class ProbeNode { + final int line; + ProbeNode predecessor; + + ProbeNode(int line) { + this.line = line; + } + } + + /** A deferred jump from a source instruction to a target label. */ + static final class Jump { + final ProbeNode source; + final Label target; + final int branch; + + Jump(ProbeNode source, Label target, int branch) { + this.source = source; + this.target = target; + this.branch = branch; + } + } + /** - * Builds a {@link ClassProbeMapping} from raw class bytes. + * Class-level visitor that collects source file info and delegates method visiting to {@link + * MethodMapper}. + */ + private static final class ProbeMappingVisitor extends ClassProbesVisitor { + private String sourceFile; + private final BitSet executableLines = new BitSet(); + private final Map probeToLines = new HashMap<>(); + + @Override + public void visitSource(String source, String debug) { + sourceFile = source; + } + + @Override + public MethodProbesVisitor visitMethod( + int access, String name, String desc, String signature, String[] exceptions) { + return new MethodMapper(executableLines, probeToLines); + } + + @Override + public void visitTotalProbeCount(int count) { + // no-op; we get probeCount from the caller + } + + ClassProbeMapping toMapping(long classId, String className, int probeCount) { + int[][] probeToLinesArray = new int[probeCount][]; + for (int p = 0; p < probeCount; p++) { + BitSet lines = probeToLines.get(p); + probeToLinesArray[p] = (lines != null) ? bitSetToArray(lines) : new int[0]; + } + return new ClassProbeMapping(classId, className, sourceFile, executableLines, probeToLinesArray); + } + + private static int[] bitSetToArray(BitSet bs) { + int[] result = new int[bs.cardinality()]; + int idx = 0; + for (int bit = bs.nextSetBit(0); bit >= 0; bit = bs.nextSetBit(bit + 1)) { + result[idx++] = bit; + } + return result; + } + } + + /** + * Method-level visitor that builds a simplified instruction graph (with predecessor links) and + * records probe-to-instruction associations. After all instructions are replayed, jump targets are + * wired and predecessor chains are walked to collect covered lines per probe. * - * @param classId CRC64 of the class bytes - * @param className VM class name (e.g. "com/example/MyClass") - * @param probeCount number of probes in this class - * @param classBytes original class file bytes (must match classId) - * @return the mapping, never null - * @throws IOException if the class bytes cannot be analyzed + *

This replicates the logic of JaCoCo's {@code InstructionsBuilder} and {@code + * MethodAnalyzer}, which are package-private and cannot be used directly from this package. */ - static ClassProbeMapping build( - long classId, String className, int probeCount, byte[] classBytes) throws IOException { - - // 1. Get executable lines (analyze with all probes false) - BitSet executableLines = new BitSet(); - String sourceFile = null; - { - ExecutionDataStore store = new ExecutionDataStore(); - store.put(new ExecutionData(classId, className, probeCount)); // all false - CoverageBuilder builder = new CoverageBuilder(); - Analyzer analyzer = new Analyzer(store, builder); - analyzer.analyzeClass(classBytes, className); - for (IClassCoverage cc : builder.getClasses()) { - sourceFile = cc.getSourceFileName(); - for (int line = cc.getFirstLine(); line <= cc.getLastLine(); line++) { - if (cc.getLine(line).getStatus() != ICounter.EMPTY) { - executableLines.set(line); + private static final class MethodMapper extends MethodProbesVisitor { + private static final int UNKNOWN_LINE = -1; + + private final BitSet executableLines; + private final Map probeToLines; + + // Per-method state + private int currentLine = UNKNOWN_LINE; + private ProbeNode currentInsn; + private final List

For each class file found, computes CRC64 and checks if it matches any missing class. If - * so, builds a ClassProbeMapping and adds it to the cache. + * Populates cache entries for all classes in {@code missingClasses}. First attempts targeted + * lookup via the context classloader's {@code getResourceAsStream} (O(1) per class). Any classes + * that can't be resolved this way fall back to a full classpath scan. * * @param missingClasses execution data entries that have no cached mapping - * @param classpathEntries jars/directories to scan for class files + * @param classpathEntries jars/directories to scan as fallback */ void buildMissing(Collection missingClasses, List classpathEntries) { // Build a lookup: classId -> ExecutionData for the missing entries @@ -49,6 +48,94 @@ void buildMissing(Collection missingClasses, List classpath needed.put(ed.getId(), ed); } + // Phase 1: targeted classloader lookup (fast path) + resolveViaClassloader(needed); + + // Phase 2: fall back to classpath scan for anything still unresolved + if (!needed.isEmpty()) { + resolveViaClasspathScan(needed, classpathEntries); + } + + // Any remaining entries couldn't be found anywhere. + // Mark them with a sentinel so we don't retry on subsequent cycles. + for (Map.Entry e : needed.entrySet()) { + cache.put( + e.getKey(), + new ClassProbeMapping(e.getKey(), null, null, new BitSet(), new int[0][])); + log.debug( + "Class {} (id {}) not found on classpath; skipping", + e.getValue().getName(), + Long.toHexString(e.getKey())); + } + } + + /** + * Attempts to resolve missing classes via the context classloader's resource lookup. This is O(1) + * per class — the classloader already knows where each class file lives. CRC64 is verified after + * reading to ensure the bytes match what JaCoCo instrumented. + */ + private void resolveViaClassloader(Map needed) { + // Try the system classloader (application classpath) first, then the context classloader. + // The dd-code-coverage thread inherits the agent's context classloader, which typically + // can't find application classes. The system classloader is the standard app classloader. + ClassLoader systemCl = ClassLoader.getSystemClassLoader(); + ClassLoader contextCl = Thread.currentThread().getContextClassLoader(); + + // Iterate over a copy since we modify 'needed' during iteration + for (ExecutionData ed : new ArrayList<>(needed.values())) { + String resource = ed.getName() + ".class"; + InputStream is = findResource(resource, systemCl, contextCl); + if (is == null) { + continue; // not found via any classloader — will try classpath scan + } + try (InputStream stream = is) { + byte[] bytes = readAllBytes(stream); + long crc = CRC64.classId(bytes); + if (crc != ed.getId()) { + // CRC64 mismatch — classloader returned different bytes than what was instrumented. + // Fall through to classpath scan. + log.debug( + "CRC64 mismatch for {} via classloader (expected {}, got {}); will try classpath scan", + ed.getName(), + Long.toHexString(ed.getId()), + Long.toHexString(crc)); + continue; + } + ClassProbeMapping mapping = + ClassProbeMappingBuilder.build( + ed.getId(), ed.getName(), ed.getProbes().length, bytes); + cache.put(ed.getId(), mapping); + needed.remove(ed.getId()); + } catch (Exception e) { + log.debug("Failed to resolve class {} via classloader", ed.getName(), e); + } + } + } + + /** + * Tries to find a class resource using the given classloaders, returning the first non-null + * InputStream. Returns null if no classloader can find the resource. + */ + private static InputStream findResource( + String resource, ClassLoader... classLoaders) { + for (ClassLoader cl : classLoaders) { + if (cl == null) { + continue; + } + InputStream is = cl.getResourceAsStream(resource); + if (is != null) { + return is; + } + } + return null; + } + + /** + * Falls back to scanning classpath jars/directories for classes that couldn't be resolved via the + * classloader. + */ + private void resolveViaClasspathScan( + Map needed, List classpathEntries) { for (File entry : classpathEntries) { if (needed.isEmpty()) { break; @@ -67,18 +154,6 @@ void buildMissing(Collection missingClasses, List classpath log.debug("Failed to scan classpath entry for cache building: {}", entry, e); } } - - // Any remaining entries in 'needed' couldn't be found on the classpath. - // Mark them in the cache with a sentinel so we don't rescan for them. - for (Map.Entry e : needed.entrySet()) { - cache.put( - e.getKey(), - new ClassProbeMapping(e.getKey(), null, null, new BitSet(), new int[0][])); - log.debug( - "Class {} (id {}) not found on classpath; skipping", - e.getValue().getName(), - Long.toHexString(e.getKey())); - } } private void scanDirectory(File dir, Map needed) throws IOException { From 566c54366b34fb557211255d1d5531068420f7e0 Mon Sep 17 00:00:00 2001 From: Nikita Tkachenko Date: Wed, 25 Mar 2026 16:56:42 +0100 Subject: [PATCH 10/19] Set language/env/flag tags --- .../trace/codecoverage/CodeCoverageSystem.java | 14 +++++++++----- .../trace/coverage/CoverageReportUploader.java | 10 +++++----- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageSystem.java b/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageSystem.java index 8243fa7b304..59707c105a0 100644 --- a/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageSystem.java +++ b/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageSystem.java @@ -10,9 +10,9 @@ import datadog.trace.api.git.GitInfoProvider; import datadog.trace.api.git.PersonInfo; import datadog.trace.api.intake.Intake; -import datadog.trace.bootstrap.instrumentation.api.Tags; import datadog.trace.coverage.CoverageReportUploader; import java.lang.instrument.Instrumentation; +import java.util.Collections; import java.util.HashMap; import java.util.Map; import java.util.function.Predicate; @@ -68,7 +68,7 @@ public static void startCollector(Object transformerObj, Object scoObj) { Config config = Config.get(); // Build event tags from git info - Map tags = buildGitTags(); + Map tags = buildGitTags(); if (!tags.containsKey("git.commit.sha")) { log.warn( "DD_GIT_COMMIT_SHA is not set; " @@ -90,7 +90,11 @@ public static void startCollector(Object transformerObj, Object scoObj) { tags.put(DDTags.LANGUAGE_TAG_KEY, DDTags.LANGUAGE_TAG_VALUE); String env = config.getEnv(); if (env != null && !env.isEmpty()) { - tags.put(Tags.ENV, env); + tags.put("runtime.env", env); + } + String serviceName = config.getServiceName(); + if (serviceName != null && !serviceName.isEmpty()) { + tags.put("report.flags", Collections.singletonList("service:" + serviceName)); } CoverageReportUploader uploader = new CoverageReportUploader(backendApi, tags, null); @@ -105,8 +109,8 @@ public static void startCollector(Object transformerObj, Object scoObj) { collector.start(); } - private static Map buildGitTags() { - Map tags = new HashMap<>(); + private static Map buildGitTags() { + Map tags = new HashMap<>(); GitInfo gitInfo = GitInfoProvider.INSTANCE.getGitInfo(); CommitInfo commit = gitInfo.getCommit(); if (commit != null && commit.getSha() != null) { diff --git a/utils/coverage-utils/src/main/java/datadog/trace/coverage/CoverageReportUploader.java b/utils/coverage-utils/src/main/java/datadog/trace/coverage/CoverageReportUploader.java index ae40ac13c71..9a60e477bf1 100644 --- a/utils/coverage-utils/src/main/java/datadog/trace/coverage/CoverageReportUploader.java +++ b/utils/coverage-utils/src/main/java/datadog/trace/coverage/CoverageReportUploader.java @@ -24,25 +24,25 @@ public class CoverageReportUploader { private final BackendApi backendApi; - private final Map tags; + private final Map tags; @Nullable private final OkHttpUtils.CustomListener requestListener; - private final JsonAdapter> eventAdapter; + private final JsonAdapter> eventAdapter; public CoverageReportUploader( BackendApi backendApi, - Map tags, + Map tags, @Nullable OkHttpUtils.CustomListener requestListener) { this.backendApi = backendApi; this.tags = tags; this.requestListener = requestListener; Moshi moshi = new Moshi.Builder().build(); - Type type = Types.newParameterizedType(Map.class, String.class, String.class); + Type type = Types.newParameterizedType(Map.class, String.class, Object.class); eventAdapter = moshi.adapter(type); } public void upload(String format, InputStream reportStream) throws IOException { - Map event = new HashMap<>(tags); + Map event = new HashMap<>(tags); event.put("format", format); event.put("type", "coverage_report"); String eventJson = eventAdapter.toJson(event); From 9e26d80b452ad15e2ee6a3a2cc8604bb4443a42b Mon Sep 17 00:00:00 2001 From: Nikita Tkachenko Date: Wed, 25 Mar 2026 17:00:48 +0100 Subject: [PATCH 11/19] Use AgentTaskScheduler for coverage collection thread Replace custom ScheduledExecutorService with a dedicated AgentTaskScheduler(CODE_COVERAGE) instance, following the same pattern as Profiler, Debugger, Remote Config, and Tracer Flare. This gives us proper daemon thread in the dd-trace-java thread group, null context classloader, uncaught exception handler, and shutdown hook handling for free. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../codecoverage/CodeCoverageCollector.java | 25 ++++++------------- .../trace/util/AgentThreadFactory.java | 4 ++- 2 files changed, 11 insertions(+), 18 deletions(-) diff --git a/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageCollector.java b/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageCollector.java index fcf7ff3e5c0..9fbcfc8b454 100644 --- a/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageCollector.java +++ b/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageCollector.java @@ -1,15 +1,16 @@ package datadog.trace.codecoverage; +import static datadog.trace.util.AgentThreadFactory.AgentThread.CODE_COVERAGE; + import datadog.trace.coverage.CoverageKey; import datadog.trace.coverage.LinesCoverage; +import datadog.trace.util.AgentTaskScheduler; import java.io.File; import java.util.ArrayList; import java.util.Collection; import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.concurrent.Executors; -import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; import org.jacoco.core.data.ExecutionData; import org.jacoco.core.data.ExecutionDataStore; @@ -34,7 +35,7 @@ public final class CodeCoverageCollector { private final int intervalSeconds; private final String explicitClasspath; private final ProbeMappingCache probeCache = new ProbeMappingCache(); - private volatile ScheduledExecutorService scheduler; + private final AgentTaskScheduler scheduler = new AgentTaskScheduler(CODE_COVERAGE); /** * @param transformer the transformer that holds runtime probe data @@ -55,25 +56,15 @@ public CodeCoverageCollector( /** Starts the periodic collection scheduler. */ public void start() { - scheduler = - Executors.newSingleThreadScheduledExecutor( - r -> { - Thread t = new Thread(r, "dd-code-coverage"); - t.setDaemon(true); - return t; - }); - scheduler.scheduleAtFixedRate(this::collect, intervalSeconds, intervalSeconds, TimeUnit.SECONDS); + scheduler.scheduleAtFixedRate( + this::collect, intervalSeconds, intervalSeconds, TimeUnit.SECONDS); log.debug( - "Code coverage collector started with interval of {} seconds", - intervalSeconds); + "Code coverage collector started with interval of {} seconds", intervalSeconds); } /** Stops the periodic collection scheduler. */ public void stop() { - ScheduledExecutorService s = scheduler; - if (s != null) { - s.shutdownNow(); - } + scheduler.shutdown(5, TimeUnit.SECONDS); } /** Performs a single collection cycle: collect probes, resolve via cache, and send. */ diff --git a/internal-api/src/main/java/datadog/trace/util/AgentThreadFactory.java b/internal-api/src/main/java/datadog/trace/util/AgentThreadFactory.java index 4aa40e06522..02364ec4d41 100644 --- a/internal-api/src/main/java/datadog/trace/util/AgentThreadFactory.java +++ b/internal-api/src/main/java/datadog/trace/util/AgentThreadFactory.java @@ -63,7 +63,9 @@ public enum AgentThread { LLMOBS_EVALS_PROCESSOR("dd-llmobs-evals-processor"), - FEATURE_FLAG_EXPOSURE_PROCESSOR("dd-ffe-exposure-processor"); + FEATURE_FLAG_EXPOSURE_PROCESSOR("dd-ffe-exposure-processor"), + + CODE_COVERAGE("dd-code-coverage"); public final String threadName; From b5017ef03f6066ae0c34f8c857ae90d89d18b017 Mon Sep 17 00:00:00 2001 From: Nikita Tkachenko Date: Wed, 25 Mar 2026 17:15:34 +0100 Subject: [PATCH 12/19] Minor tweak --- .../trace/codecoverage/CodeCoverageTransformer.java | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageTransformer.java b/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageTransformer.java index 7ad06a4e831..62d8efc1fa1 100644 --- a/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageTransformer.java +++ b/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageTransformer.java @@ -71,13 +71,12 @@ protected Class loadClass(String name, boolean resolve) if (!scope.contains(name)) { return super.loadClass(name, resolve); } - InputStream resourceStream = - agentLoader.getResourceAsStream(name.replace('.', '/') + ".class"); - if (resourceStream == null) { - throw new ClassNotFoundException(name); - } byte[] bytes; - try { + try (InputStream resourceStream = + agentLoader.getResourceAsStream(name.replace('.', '/') + ".class")) { + if (resourceStream == null) { + throw new ClassNotFoundException(name); + } bytes = readAllBytes(resourceStream); } catch (IOException e) { throw new RuntimeException(e); From d7a063a97646ccf52de691ac2b50d2cab41f3109 Mon Sep 17 00:00:00 2001 From: Nikita Tkachenko Date: Thu, 26 Mar 2026 17:22:52 +0100 Subject: [PATCH 13/19] Moved a couple of methods around --- .../codecoverage/CodeCoverageTransformer.java | 113 +++++++++--------- 1 file changed, 57 insertions(+), 56 deletions(-) diff --git a/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageTransformer.java b/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageTransformer.java index 62d8efc1fa1..933319baf1f 100644 --- a/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageTransformer.java +++ b/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageTransformer.java @@ -103,6 +103,63 @@ protected Class loadClass(String name, boolean resolve) this.instrumenter = new Instrumenter(runtime); } + /** Recursively adds the given class and all its declared inner classes to the scope set. */ + private static void addToScopeWithInnerClasses(Class clazz, Set scope) { + scope.add(clazz.getName()); + for (Class inner : clazz.getDeclaredClasses()) { + addToScopeWithInnerClasses(inner, scope); + } + } + + /** Reads all bytes from an input stream. */ + private static byte[] readAllBytes(InputStream is) throws IOException { + byte[] buf = new byte[1024]; + ByteArrayOutputStream out = new ByteArrayOutputStream(); + int r; + while ((r = is.read(buf)) != -1) { + out.write(buf, 0, r); + } + return out.toByteArray(); + } + + /** + * Opens the package of {@code classInPackage} to the unnamed module of {@code targetLoader}. + * + *

This uses {@code Instrumentation.redefineModule} reflectively (same approach as JaCoCo's + * {@code AgentModule.openPackage}). + */ + private static void openPackage( + Instrumentation inst, Class classInPackage, ClassLoader targetLoader) throws Exception { + // module of the package to open (e.g. java.base for java.lang) + Object module = Class.class.getMethod("getModule").invoke(classInPackage); + + // unnamed module of the isolated classloader + Object unnamedModule = ClassLoader.class.getMethod("getUnnamedModule").invoke(targetLoader); + + Class moduleClass = Class.forName("java.lang.Module"); + + // Instrumentation.redefineModule(Module, Set, Map, Map>, Set, Map) + Instrumentation.class + .getMethod( + "redefineModule", + moduleClass, + Set.class, + Map.class, + Map.class, + Set.class, + Map.class) + .invoke( + inst, + module, // module to modify + Collections.emptySet(), // extraReads + Collections.emptyMap(), // extraExports + Collections.singletonMap( + classInPackage.getPackage().getName(), + Collections.singleton(unnamedModule)), // extraOpens + Collections.emptySet(), // extraUses + Collections.emptyMap()); // extraProvides + } + @Override public byte[] transform( ClassLoader loader, @@ -158,60 +215,4 @@ public void collectAndReset(ExecutionDataStore target, SessionInfoStore sessionT } } - /** - * Opens the package of {@code classInPackage} to the unnamed module of {@code targetLoader}. - * - *

This uses {@code Instrumentation.redefineModule} reflectively (same approach as JaCoCo's - * {@code AgentModule.openPackage}). - */ - private static void openPackage( - Instrumentation inst, Class classInPackage, ClassLoader targetLoader) throws Exception { - // module of the package to open (e.g. java.base for java.lang) - Object module = Class.class.getMethod("getModule").invoke(classInPackage); - - // unnamed module of the isolated classloader - Object unnamedModule = ClassLoader.class.getMethod("getUnnamedModule").invoke(targetLoader); - - Class moduleClass = Class.forName("java.lang.Module"); - - // Instrumentation.redefineModule(Module, Set, Map, Map>, Set, Map) - Instrumentation.class - .getMethod( - "redefineModule", - moduleClass, - Set.class, - Map.class, - Map.class, - Set.class, - Map.class) - .invoke( - inst, - module, // module to modify - Collections.emptySet(), // extraReads - Collections.emptyMap(), // extraExports - Collections.singletonMap( - classInPackage.getPackage().getName(), - Collections.singleton(unnamedModule)), // extraOpens - Collections.emptySet(), // extraUses - Collections.emptyMap()); // extraProvides - } - - /** Recursively adds the given class and all its declared inner classes to the scope set. */ - private static void addToScopeWithInnerClasses(Class clazz, Set scope) { - scope.add(clazz.getName()); - for (Class inner : clazz.getDeclaredClasses()) { - addToScopeWithInnerClasses(inner, scope); - } - } - - /** Reads all bytes from an input stream. */ - private static byte[] readAllBytes(InputStream is) throws IOException { - byte[] buf = new byte[1024]; - ByteArrayOutputStream out = new ByteArrayOutputStream(); - int r; - while ((r = is.read(buf)) != -1) { - out.write(buf, 0, r); - } - return out.toByteArray(); - } } From 6034289ef1d862db4fc83996466d698e683220a6 Mon Sep 17 00:00:00 2001 From: Nikita Tkachenko Date: Thu, 26 Mar 2026 17:51:41 +0100 Subject: [PATCH 14/19] Replace ClassProbeMappingBuilder with JaCoCo Analyzer delegation The single-pass reimplementation had incorrect probe semantics for control flow: it unconditionally nulled currentInsn after every probe, but JaCoCo only does that for standalone visitProbe. This broke fall-through edges after probed conditional jumps and silently dropped later probed switch targets. Replace with a simpler O(probeCount) delegation to JaCoCo's Analyzer that is correct by construction, and add 33 unit tests covering branches, switches, loops, try-catch, nested conditions, and the specific bug scenarios. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../ClassProbeMappingBuilder.java | 390 ++------- .../codecoverage/CodeCoverageCollector.java | 9 +- .../codecoverage/CodeCoverageSystem.java | 3 +- .../codecoverage/CodeCoverageTransformer.java | 27 +- .../codecoverage/CoverageBinaryEncoder.java | 7 +- .../trace/codecoverage/ProbeMappingCache.java | 9 +- .../ClassProbeMappingBuilderTest.java | 803 ++++++++++++++++++ .../CoverageBinaryEncoderTest.java | 90 +- 8 files changed, 960 insertions(+), 378 deletions(-) create mode 100644 dd-java-agent/agent-code-coverage/src/test/java/datadog/trace/codecoverage/ClassProbeMappingBuilderTest.java diff --git a/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/ClassProbeMappingBuilder.java b/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/ClassProbeMappingBuilder.java index 0ba0c734f99..de7400ab4da 100644 --- a/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/ClassProbeMappingBuilder.java +++ b/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/ClassProbeMappingBuilder.java @@ -1,358 +1,90 @@ package datadog.trace.codecoverage; -import java.util.ArrayList; import java.util.BitSet; -import java.util.HashMap; -import java.util.List; -import java.util.Map; -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.jacoco.core.internal.instr.InstrSupport; -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; +import org.jacoco.core.analysis.Analyzer; +import org.jacoco.core.analysis.CoverageBuilder; +import org.jacoco.core.analysis.IClassCoverage; +import org.jacoco.core.analysis.ISourceNode; +import org.jacoco.core.data.ExecutionData; +import org.jacoco.core.data.ExecutionDataStore; /** - * Builds a {@link ClassProbeMapping} by parsing the class bytecode once using JaCoCo's {@link - * ClassProbesAdapter}, building a simplified instruction graph, and walking predecessor chains to - * determine which lines each probe covers. + * Builds a {@link ClassProbeMapping} by delegating to JaCoCo's {@link Analyzer}. For each probe, + * the class is analyzed with only that probe marked as executed; the covered lines are then + * collected into the mapping. * - *

This replaces the previous N+1 pass approach (one {@code Analyzer} pass per probe plus one for - * executable lines) with a single-pass design that is significantly faster for classes with many - * probes. + *

This approach is O(probeCount) in the number of JaCoCo analysis passes. It is slower than a + * single-pass approach but guaranteed to be correct, as it reuses JaCoCo's own analysis logic + * including all filters and edge-case handling. */ final class ClassProbeMappingBuilder { static ClassProbeMapping build( long classId, String className, int probeCount, byte[] classBytes) { - ClassReader reader = InstrSupport.classReaderFor(classBytes); - ProbeMappingVisitor visitor = new ProbeMappingVisitor(); - ClassProbesAdapter adapter = new ClassProbesAdapter(visitor, false); - reader.accept(adapter, 0); - return visitor.toMapping(classId, className, probeCount); - } - - /** Simplified instruction node with a line number and a single predecessor link. */ - static final class ProbeNode { - final int line; - ProbeNode predecessor; - - ProbeNode(int line) { - this.line = line; - } - } - - /** A deferred jump from a source instruction to a target label. */ - static final class Jump { - final ProbeNode source; - final Label target; - final int branch; - - Jump(ProbeNode source, Label target, int branch) { - this.source = source; - this.target = target; - this.branch = branch; - } - } - - /** - * Class-level visitor that collects source file info and delegates method visiting to {@link - * MethodMapper}. - */ - private static final class ProbeMappingVisitor extends ClassProbesVisitor { - private String sourceFile; - private final BitSet executableLines = new BitSet(); - private final Map probeToLines = new HashMap<>(); - - @Override - public void visitSource(String source, String debug) { - sourceFile = source; + // Analyze with no probes to determine executable lines and source file + IClassCoverage baseline = analyzeClass(classId, className, classBytes, null); + if (baseline == null) { + return new ClassProbeMapping(classId, className, null, new BitSet(), new int[probeCount][]); } - @Override - public MethodProbesVisitor visitMethod( - int access, String name, String desc, String signature, String[] exceptions) { - return new MethodMapper(executableLines, probeToLines); - } + String sourceFile = baseline.getSourceFileName(); + BitSet executableLines = collectLines(baseline, false); - @Override - public void visitTotalProbeCount(int count) { - // no-op; we get probeCount from the caller + // For each probe, analyze with only that probe set to determine covered lines + int[][] probeToLines = new int[probeCount][]; + for (int p = 0; p < probeCount; p++) { + boolean[] probes = new boolean[probeCount]; + probes[p] = true; + IClassCoverage cc = analyzeClass(classId, className, classBytes, probes); + probeToLines[p] = (cc != null) ? bitSetToArray(collectLines(cc, true)) : new int[0]; } - ClassProbeMapping toMapping(long classId, String className, int probeCount) { - int[][] probeToLinesArray = new int[probeCount][]; - for (int p = 0; p < probeCount; p++) { - BitSet lines = probeToLines.get(p); - probeToLinesArray[p] = (lines != null) ? bitSetToArray(lines) : new int[0]; - } - return new ClassProbeMapping(classId, className, sourceFile, executableLines, probeToLinesArray); - } - - private static int[] bitSetToArray(BitSet bs) { - int[] result = new int[bs.cardinality()]; - int idx = 0; - for (int bit = bs.nextSetBit(0); bit >= 0; bit = bs.nextSetBit(bit + 1)) { - result[idx++] = bit; - } - return result; - } + return new ClassProbeMapping(classId, className, sourceFile, executableLines, probeToLines); } - /** - * Method-level visitor that builds a simplified instruction graph (with predecessor links) and - * records probe-to-instruction associations. After all instructions are replayed, jump targets are - * wired and predecessor chains are walked to collect covered lines per probe. - * - *

This replicates the logic of JaCoCo's {@code InstructionsBuilder} and {@code - * MethodAnalyzer}, which are package-private and cannot be used directly from this package. - */ - private static final class MethodMapper extends MethodProbesVisitor { - private static final int UNKNOWN_LINE = -1; - - private final BitSet executableLines; - private final Map probeToLines; - - // Per-method state - private int currentLine = UNKNOWN_LINE; - private ProbeNode currentInsn; - private final List

On the first collection cycle (or when new classes appear), a classpath scan builds the - * cache. Subsequent cycles simply iterate boolean probe arrays and set bits -- no JaCoCo {@code - * Analyzer} pass is needed. + *

On the first collection cycle (or when new classes appear), a classpath scan builds the cache. + * Subsequent cycles simply iterate boolean probe arrays and set bits -- no JaCoCo {@code Analyzer} + * pass is needed. */ public final class CodeCoverageCollector { @@ -58,8 +58,7 @@ public CodeCoverageCollector( public void start() { scheduler.scheduleAtFixedRate( this::collect, intervalSeconds, intervalSeconds, TimeUnit.SECONDS); - log.debug( - "Code coverage collector started with interval of {} seconds", intervalSeconds); + log.debug("Code coverage collector started with interval of {} seconds", intervalSeconds); } /** Stops the periodic collection scheduler. */ diff --git a/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageSystem.java b/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageSystem.java index 59707c105a0..7affc7ec00e 100644 --- a/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageSystem.java +++ b/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageSystem.java @@ -77,8 +77,7 @@ public static void startCollector(Object transformerObj, Object scoObj) { } // Create BackendApi for coverage uploads - BackendApiFactory factory = - new BackendApiFactory(config, (SharedCommunicationObjects) scoObj); + BackendApiFactory factory = new BackendApiFactory(config, (SharedCommunicationObjects) scoObj); BackendApi backendApi = factory.createBackendApi(Intake.CI_INTAKE); if (backendApi == null) { log.warn( diff --git a/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageTransformer.java b/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageTransformer.java index 933319baf1f..95199e47fdf 100644 --- a/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageTransformer.java +++ b/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageTransformer.java @@ -66,8 +66,7 @@ public CodeCoverageTransformer(Instrumentation inst, Predicate filter) t ClassLoader isolatedLoader = new ClassLoader(agentLoader) { @Override - protected Class loadClass(String name, boolean resolve) - throws ClassNotFoundException { + protected Class loadClass(String name, boolean resolve) throws ClassNotFoundException { if (!scope.contains(name)) { return super.loadClass(name, resolve); } @@ -92,12 +91,11 @@ protected Class loadClass(String name, boolean resolve) // Load InjectedClassRuntime in the isolated module @SuppressWarnings("unchecked") Class rtClass = - (Class) isolatedLoader.loadClass(InjectedClassRuntime.class.getName()); + (Class) + isolatedLoader.loadClass(InjectedClassRuntime.class.getName()); IRuntime runtime = - rtClass - .getConstructor(Class.class, String.class) - .newInstance(Object.class, "$DDCov"); + rtClass.getConstructor(Class.class, String.class).newInstance(Object.class, "$DDCov"); runtime.startup(runtimeData); this.instrumenter = new Instrumenter(runtime); @@ -141,13 +139,7 @@ private static void openPackage( // Instrumentation.redefineModule(Module, Set, Map, Map>, Set, Map) Instrumentation.class .getMethod( - "redefineModule", - moduleClass, - Set.class, - Map.class, - Map.class, - Set.class, - Map.class) + "redefineModule", moduleClass, Set.class, Map.class, Map.class, Set.class, Map.class) .invoke( inst, module, // module to modify @@ -189,10 +181,10 @@ public byte[] transform( * *

Uses a serialize/deserialize round-trip to capture probe values before reset. This is * necessary because {@code RuntimeData.collect()} passes references to the live {@code boolean[]} - * probe arrays to the visitor. If we passed an {@code ExecutionDataStore} directly, it would store - * references to the same arrays that {@code reset()} then zeroes out — destroying the collected - * data. The byte-stream approach (same as JaCoCo's own {@code Agent.getExecutionData()}) captures - * probe values into the stream before the reset runs. + * probe arrays to the visitor. If we passed an {@code ExecutionDataStore} directly, it would + * store references to the same arrays that {@code reset()} then zeroes out — destroying the + * collected data. The byte-stream approach (same as JaCoCo's own {@code + * Agent.getExecutionData()}) captures probe values into the stream before the reset runs. * * @param target store to receive the execution data * @param sessionTarget store to receive session info @@ -214,5 +206,4 @@ public void collectAndReset(ExecutionDataStore target, SessionInfoStore sessionT throw new RuntimeException("Failed to collect coverage data", e); } } - } diff --git a/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CoverageBinaryEncoder.java b/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CoverageBinaryEncoder.java index 3a7dff733c3..6995dbf515f 100644 --- a/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CoverageBinaryEncoder.java +++ b/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CoverageBinaryEncoder.java @@ -8,9 +8,7 @@ import java.util.BitSet; import java.util.Map; -/** - * Encodes coverage data into the Coverage Binary Protocol v1 - */ +/** Encodes coverage data into the Coverage Binary Protocol v1 */ public final class CoverageBinaryEncoder { private static final int VERSION = 1; @@ -31,8 +29,7 @@ private static void writeRecord(CoverageKey key, LinesCoverage lines, OutputStre writeString(key.sourceFile, out); writeString(key.className, out); - int maxLine = - Math.max(lines.executableLines.length(), lines.coveredLines.length()) - 1; + int maxLine = Math.max(lines.executableLines.length(), lines.coveredLines.length()) - 1; if (maxLine < 0) { writeUvarint(0, out); return; diff --git a/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/ProbeMappingCache.java b/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/ProbeMappingCache.java index 62169796b3c..79ad70a4a1e 100644 --- a/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/ProbeMappingCache.java +++ b/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/ProbeMappingCache.java @@ -60,8 +60,7 @@ void buildMissing(Collection missingClasses, List classpath // Mark them with a sentinel so we don't retry on subsequent cycles. for (Map.Entry e : needed.entrySet()) { cache.put( - e.getKey(), - new ClassProbeMapping(e.getKey(), null, null, new BitSet(), new int[0][])); + e.getKey(), new ClassProbeMapping(e.getKey(), null, null, new BitSet(), new int[0][])); log.debug( "Class {} (id {}) not found on classpath; skipping", e.getValue().getName(), @@ -102,8 +101,7 @@ private void resolveViaClassloader(Map needed) { continue; } ClassProbeMapping mapping = - ClassProbeMappingBuilder.build( - ed.getId(), ed.getName(), ed.getProbes().length, bytes); + ClassProbeMappingBuilder.build(ed.getId(), ed.getName(), ed.getProbes().length, bytes); cache.put(ed.getId(), mapping); needed.remove(ed.getId()); } catch (Exception e) { @@ -116,8 +114,7 @@ private void resolveViaClassloader(Map needed) { * Tries to find a class resource using the given classloaders, returning the first non-null * InputStream. Returns null if no classloader can find the resource. */ - private static InputStream findResource( - String resource, ClassLoader... classLoaders) { + private static InputStream findResource(String resource, ClassLoader... classLoaders) { for (ClassLoader cl : classLoaders) { if (cl == null) { continue; diff --git a/dd-java-agent/agent-code-coverage/src/test/java/datadog/trace/codecoverage/ClassProbeMappingBuilderTest.java b/dd-java-agent/agent-code-coverage/src/test/java/datadog/trace/codecoverage/ClassProbeMappingBuilderTest.java new file mode 100644 index 00000000000..1ac9a1f0acf --- /dev/null +++ b/dd-java-agent/agent-code-coverage/src/test/java/datadog/trace/codecoverage/ClassProbeMappingBuilderTest.java @@ -0,0 +1,803 @@ +package datadog.trace.codecoverage; + +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.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.InputStream; +import java.util.Arrays; +import java.util.BitSet; +import org.jacoco.core.analysis.Analyzer; +import org.jacoco.core.analysis.CoverageBuilder; +import org.jacoco.core.analysis.IClassCoverage; +import org.jacoco.core.analysis.ISourceNode; +import org.jacoco.core.data.ExecutionData; +import org.jacoco.core.data.ExecutionDataStore; +import org.jacoco.core.internal.data.CRC64; +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.MethodProbesVisitor; +import org.jacoco.core.internal.instr.InstrSupport; +import org.junit.jupiter.api.Test; +import org.objectweb.asm.Label; + +class ClassProbeMappingBuilderTest { + + // ===== Sample classes exercising different bytecode patterns ===== + + /** Linear code with no branches. */ + @SuppressWarnings("unused") + static class SimpleLinear { + int compute(int x) { + int a = x + 1; + int b = a * 2; + return b; + } + } + + /** If/else branch. */ + @SuppressWarnings("unused") + static class IfElseBranch { + int abs(int x) { + if (x < 0) { + return -x; + } else { + return x; + } + } + } + + /** Multiple sequential conditions — tests fall-through after probed conditional jumps. */ + @SuppressWarnings("unused") + static class MultipleConditions { + int classify(int x) { + int result = 0; + if (x > 0) { + result = 1; + } + if (x > 10) { + result = 2; + } + return result; + } + } + + /** Conditional with fall-through: exercises the visitJumpInsnWithProbe bug. */ + @SuppressWarnings("unused") + static class ConditionalFallThrough { + int compute(int x) { + int a = 1; + if (x > 0) { + a = 2; + } + int b = a + 1; + return b; + } + } + + /** Dense switch statement (compiled as tableswitch). */ + @SuppressWarnings("unused") + static class TableSwitch { + int compute(int x) { + switch (x) { + case 0: + return 10; + case 1: + return 20; + case 2: + return 30; + case 3: + return 40; + default: + return -1; + } + } + } + + /** Sparse switch statement (compiled as lookupswitch). */ + @SuppressWarnings("unused") + static class LookupSwitch { + int compute(int x) { + switch (x) { + case 100: + return 1; + case 200: + return 2; + case 300: + return 3; + default: + return 0; + } + } + } + + /** Switch with shared targets — exercises visitSwitchInsnWithProbes. */ + @SuppressWarnings("unused") + static class SwitchWithSharedTargets { + String describe(int x) { + switch (x) { + case 1: + case 2: + case 3: + return "small"; + case 4: + case 5: + case 6: + return "medium"; + default: + return "other"; + } + } + } + + /** Try-catch block. */ + @SuppressWarnings("unused") + static class TryCatch { + int safeDivide(int a, int b) { + try { + return a / b; + } catch (ArithmeticException e) { + return 0; + } + } + } + + /** Try-catch-finally block. */ + @SuppressWarnings("unused") + static class TryCatchFinally { + int compute(int x) { + int result = 0; + try { + result = 100 / x; + } catch (ArithmeticException e) { + result = -1; + } finally { + result += 1; + } + return result; + } + } + + /** For loop. */ + @SuppressWarnings("unused") + static class ForLoop { + int sum(int n) { + int total = 0; + for (int i = 0; i < n; i++) { + total += i; + } + return total; + } + } + + /** While loop. */ + @SuppressWarnings("unused") + static class WhileLoop { + int countDown(int n) { + int count = 0; + while (n > 0) { + count++; + n--; + } + return count; + } + } + + /** Nested conditions. */ + @SuppressWarnings("unused") + static class NestedConditions { + String classify(int x, int y) { + if (x > 0) { + if (y > 0) { + return "both positive"; + } else { + return "x positive, y not"; + } + } else { + return "x not positive"; + } + } + } + + /** Class with multiple methods. */ + @SuppressWarnings("unused") + static class MultipleMethods { + int add(int a, int b) { + return a + b; + } + + int multiply(int a, int b) { + return a * b; + } + + int negate(int a) { + return -a; + } + } + + /** Empty void method. */ + @SuppressWarnings("unused") + static class EmptyMethod { + void doNothing() {} + } + + /** Ternary expression (inline conditional). */ + @SuppressWarnings("unused") + static class TernaryExpression { + int max(int a, int b) { + return a > b ? a : b; + } + } + + /** Boolean short-circuit expressions. */ + @SuppressWarnings("unused") + static class ShortCircuit { + boolean check(int x, int y) { + return x > 0 && y > 0; + } + } + + /** Method with early return. */ + @SuppressWarnings("unused") + static class EarlyReturn { + int compute(int x) { + if (x == 0) { + return -1; + } + int a = x * 2; + int b = a + 1; + return b; + } + } + + /** Interface with no methods (zero probes). */ + @SuppressWarnings("unused") + interface EmptyInterface {} + + // ===== Tests: builder output matches direct JaCoCo analysis ===== + + @Test + void simpleLinear() throws Exception { + assertMatchesJaCoCo(SimpleLinear.class); + } + + @Test + void ifElseBranch() throws Exception { + assertMatchesJaCoCo(IfElseBranch.class); + } + + @Test + void multipleConditions() throws Exception { + assertMatchesJaCoCo(MultipleConditions.class); + } + + @Test + void conditionalFallThrough() throws Exception { + assertMatchesJaCoCo(ConditionalFallThrough.class); + } + + @Test + void tableSwitch() throws Exception { + assertMatchesJaCoCo(TableSwitch.class); + } + + @Test + void lookupSwitch() throws Exception { + assertMatchesJaCoCo(LookupSwitch.class); + } + + @Test + void switchWithSharedTargets() throws Exception { + assertMatchesJaCoCo(SwitchWithSharedTargets.class); + } + + @Test + void tryCatch() throws Exception { + assertMatchesJaCoCo(TryCatch.class); + } + + @Test + void tryCatchFinally() throws Exception { + assertMatchesJaCoCo(TryCatchFinally.class); + } + + @Test + void forLoop() throws Exception { + assertMatchesJaCoCo(ForLoop.class); + } + + @Test + void whileLoop() throws Exception { + assertMatchesJaCoCo(WhileLoop.class); + } + + @Test + void nestedConditions() throws Exception { + assertMatchesJaCoCo(NestedConditions.class); + } + + @Test + void multipleMethods() throws Exception { + assertMatchesJaCoCo(MultipleMethods.class); + } + + @Test + void emptyMethod() throws Exception { + assertMatchesJaCoCo(EmptyMethod.class); + } + + @Test + void ternaryExpression() throws Exception { + assertMatchesJaCoCo(TernaryExpression.class); + } + + @Test + void shortCircuit() throws Exception { + assertMatchesJaCoCo(ShortCircuit.class); + } + + @Test + void earlyReturn() throws Exception { + assertMatchesJaCoCo(EarlyReturn.class); + } + + // ===== Tests: structural properties across all sample classes ===== + + @Test + void probeLines_areSubsetOfExecutableLines() throws Exception { + for (Class clazz : allSampleClasses()) { + ClassProbeMapping mapping = buildMapping(clazz); + int probeCount = mapping.probeToLines.length; + for (int p = 0; p < probeCount; p++) { + for (int line : mapping.probeToLines[p]) { + assertTrue( + mapping.executableLines.get(line), + clazz.getSimpleName() + + ": probe " + + p + + " covers line " + + line + + " which is not executable"); + } + } + } + } + + @Test + void allExecutableLines_areCoveredBySomeProbe() throws Exception { + for (Class clazz : allSampleClasses()) { + ClassProbeMapping mapping = buildMapping(clazz); + BitSet covered = new BitSet(); + for (int[] lines : mapping.probeToLines) { + for (int line : lines) { + covered.set(line); + } + } + assertEquals( + mapping.executableLines, + covered, + clazz.getSimpleName() + ": union of probe lines doesn't match executable lines"); + } + } + + @Test + void probeToLines_areSorted() throws Exception { + for (Class clazz : allSampleClasses()) { + ClassProbeMapping mapping = buildMapping(clazz); + for (int p = 0; p < mapping.probeToLines.length; p++) { + int[] lines = mapping.probeToLines[p]; + for (int i = 1; i < lines.length; i++) { + assertTrue( + lines[i] > lines[i - 1], + clazz.getSimpleName() + + ": probe " + + p + + " lines not sorted: " + + Arrays.toString(lines)); + } + } + } + } + + @Test + void probeToLines_lengthMatchesProbeCount() throws Exception { + for (Class clazz : allSampleClasses()) { + byte[] bytes = bytecodeFor(clazz); + int probeCount = countProbes(bytes); + ClassProbeMapping mapping = buildMapping(clazz); + assertEquals( + probeCount, + mapping.probeToLines.length, + clazz.getSimpleName() + ": probeToLines length mismatch"); + } + } + + @Test + void sourceFile_isPopulated() throws Exception { + for (Class clazz : allSampleClasses()) { + ClassProbeMapping mapping = buildMapping(clazz); + assertNotNull(mapping.sourceFile, clazz.getSimpleName() + ": sourceFile should not be null"); + } + } + + // ===== Tests: specific behaviours and edge cases ===== + + @Test + void classIdAndClassName_arePreserved() throws Exception { + byte[] bytes = bytecodeFor(SimpleLinear.class); + long classId = CRC64.classId(bytes); + String className = SimpleLinear.class.getName().replace('.', '/'); + int probeCount = countProbes(bytes); + + ClassProbeMapping mapping = + ClassProbeMappingBuilder.build(classId, className, probeCount, bytes); + + assertEquals(classId, mapping.classId); + assertEquals(className, mapping.className); + } + + @Test + void emptyMethod_hasExecutableLines() throws Exception { + ClassProbeMapping mapping = buildMapping(EmptyMethod.class); + assertFalse( + mapping.executableLines.isEmpty(), "Empty method should still have executable lines"); + assertTrue(mapping.probeToLines.length > 0, "Empty method class should have probes"); + } + + @Test + void emptyInterface_hasNoExecutableLinesAndZeroProbes() throws Exception { + byte[] bytes = bytecodeFor(EmptyInterface.class); + long classId = CRC64.classId(bytes); + String className = EmptyInterface.class.getName().replace('.', '/'); + int probeCount = countProbes(bytes); + + assertEquals(0, probeCount, "Empty interface should have zero probes"); + + ClassProbeMapping mapping = + ClassProbeMappingBuilder.build(classId, className, probeCount, bytes); + + assertEquals(0, mapping.probeToLines.length); + assertTrue(mapping.executableLines.isEmpty()); + } + + @Test + void conditionalFallThrough_probeCoversLinesAcrossBranch() throws Exception { + ClassProbeMapping mapping = buildMapping(ConditionalFallThrough.class); + + // The if-body probe covers: int a = 1, if(x>0), a = 2 — three lines. The merge point + // after the if breaks the predecessor chain, so the return probe only covers lines after + // the merge. With the old buggy implementation the fall-through edge was broken after the + // probed jump, so the if-body probe would only cover 1 line (a = 2), giving max=2. + int maxProbeLines = 0; + for (int[] lines : mapping.probeToLines) { + maxProbeLines = Math.max(maxProbeLines, lines.length); + } + assertTrue( + maxProbeLines >= 3, + "Expected a probe covering 3+ lines (if-body path including lines before the branch), " + + "max was " + + maxProbeLines); + } + + @Test + void multipleConditions_probeCoversAllPrecedingLines() throws Exception { + ClassProbeMapping mapping = buildMapping(MultipleConditions.class); + + // The first if-body probe covers: int result=0, if(x>0), result=1 — three lines. + // Merge points between conditionals break the predecessor chain, so no single probe + // spans both ifs. With the old buggy implementation, the if-body probes lose their + // predecessor link to the branch instruction, covering only the if-body line (max=2). + int maxProbeLines = 0; + for (int[] lines : mapping.probeToLines) { + maxProbeLines = Math.max(maxProbeLines, lines.length); + } + assertTrue( + maxProbeLines >= 3, + "Expected a probe covering 3+ lines (if-body path including lines before the branch), " + + "max was " + + maxProbeLines); + } + + @Test + void tableSwitch_allCasesHaveProbes() throws Exception { + ClassProbeMapping mapping = buildMapping(TableSwitch.class); + + // The table switch has 5 outcomes (case 0-3 + default), each returning a value. + // Each case should have its own probe. With the old buggy implementation, + // later switch target probes were silently dropped. + int nonEmptyProbes = 0; + for (int[] lines : mapping.probeToLines) { + if (lines.length > 0) { + nonEmptyProbes++; + } + } + + // At least 5 non-empty probes (one per case + default), plus constructor probe + assertTrue( + nonEmptyProbes >= 6, + "Expected at least 6 non-empty probes (5 switch branches + constructor), got " + + nonEmptyProbes); + } + + @Test + void lookupSwitch_allCasesHaveProbes() throws Exception { + ClassProbeMapping mapping = buildMapping(LookupSwitch.class); + + int nonEmptyProbes = 0; + for (int[] lines : mapping.probeToLines) { + if (lines.length > 0) { + nonEmptyProbes++; + } + } + + // At least 4 non-empty probes (case 100, 200, 300, default) + constructor + assertTrue( + nonEmptyProbes >= 5, + "Expected at least 5 non-empty probes (4 switch branches + constructor), got " + + nonEmptyProbes); + } + + @Test + void switchWithSharedTargets_allTargetsHaveProbes() throws Exception { + ClassProbeMapping mapping = buildMapping(SwitchWithSharedTargets.class); + + int nonEmptyProbes = 0; + for (int[] lines : mapping.probeToLines) { + if (lines.length > 0) { + nonEmptyProbes++; + } + } + + // At least 3 non-empty probes (small, medium, other) + constructor + assertTrue( + nonEmptyProbes >= 4, + "Expected at least 4 non-empty probes (3 shared switch targets + constructor), got " + + nonEmptyProbes); + } + + @Test + void multipleMethods_haveIndependentProbes() throws Exception { + ClassProbeMapping mapping = buildMapping(MultipleMethods.class); + + // Each of the 3 methods + the constructor should have its own probe(s). + // probeCount should be at least 4. + assertTrue( + mapping.probeToLines.length >= 4, + "Expected at least 4 probes for 3 methods + constructor, got " + + mapping.probeToLines.length); + + // No two probes should cover the exact same set of lines + // (each method is independent and has distinct lines) + for (int i = 0; i < mapping.probeToLines.length; i++) { + for (int j = i + 1; j < mapping.probeToLines.length; j++) { + if (mapping.probeToLines[i].length > 0 && mapping.probeToLines[j].length > 0) { + assertFalse( + Arrays.equals(mapping.probeToLines[i], mapping.probeToLines[j]), + "Probes " + i + " and " + j + " should not cover identical line sets"); + } + } + } + } + + @Test + void tryCatch_exceptionPathHasProbe() throws Exception { + ClassProbeMapping mapping = buildMapping(TryCatch.class); + + // The try block and catch block should be on different probes. + // Both should have non-empty line sets. + int nonEmptyProbes = 0; + for (int[] lines : mapping.probeToLines) { + if (lines.length > 0) { + nonEmptyProbes++; + } + } + + // At least: try-path return, catch-path return, constructor + assertTrue( + nonEmptyProbes >= 3, + "Expected at least 3 non-empty probes (try + catch + constructor), got " + nonEmptyProbes); + } + + @Test + void earlyReturn_laterCodeIsStillReachable() throws Exception { + ClassProbeMapping mapping = buildMapping(EarlyReturn.class); + + // The code after the early return guard should be covered by a probe + // that also includes the initial guard check line. + int firstLine = mapping.executableLines.nextSetBit(0); + int lastLine = mapping.executableLines.previousSetBit(mapping.executableLines.length()); + + // Find maximum line count covered by any single probe + int maxProbeLines = 0; + for (int[] lines : mapping.probeToLines) { + maxProbeLines = Math.max(maxProbeLines, lines.length); + } + + // The non-early-return path should cover multiple lines + assertTrue( + maxProbeLines >= 3, + "Expected at least one probe covering 3+ lines for the non-early-return path, got " + + maxProbeLines); + } + + // ===== Helpers ===== + + private static final Class[] ALL_SAMPLE_CLASSES = { + SimpleLinear.class, + IfElseBranch.class, + MultipleConditions.class, + ConditionalFallThrough.class, + TableSwitch.class, + LookupSwitch.class, + SwitchWithSharedTargets.class, + TryCatch.class, + TryCatchFinally.class, + ForLoop.class, + WhileLoop.class, + NestedConditions.class, + MultipleMethods.class, + EmptyMethod.class, + TernaryExpression.class, + ShortCircuit.class, + EarlyReturn.class, + }; + + private static Class[] allSampleClasses() { + return ALL_SAMPLE_CLASSES; + } + + private static ClassProbeMapping buildMapping(Class clazz) throws Exception { + byte[] bytes = bytecodeFor(clazz); + long classId = CRC64.classId(bytes); + String className = clazz.getName().replace('.', '/'); + int probeCount = countProbes(bytes); + return ClassProbeMappingBuilder.build(classId, className, probeCount, bytes); + } + + /** + * Verifies that ClassProbeMappingBuilder produces results consistent with a direct JaCoCo + * Analyzer run. The builder and the reference use the same JaCoCo APIs but exercise the + * translation layer (line iteration, BitSet/array conversion) independently. + */ + private void assertMatchesJaCoCo(Class clazz) throws Exception { + byte[] bytes = bytecodeFor(clazz); + long classId = CRC64.classId(bytes); + String className = clazz.getName().replace('.', '/'); + int probeCount = countProbes(bytes); + + ClassProbeMapping actual = + ClassProbeMappingBuilder.build(classId, className, probeCount, bytes); + + // Reference: executable lines + BitSet expectedExec = referenceExecutableLines(classId, className, bytes); + assertEquals( + expectedExec, actual.executableLines, clazz.getSimpleName() + ": executable lines"); + + // Reference: probe-to-lines + for (int p = 0; p < probeCount; p++) { + int[] expectedLines = referenceProbeLines(classId, className, probeCount, bytes, p); + assertArrayEquals( + expectedLines, actual.probeToLines[p], clazz.getSimpleName() + ": probe " + p + " lines"); + } + } + + // --- Reference implementation using JaCoCo's Analyzer directly --- + + private static BitSet referenceExecutableLines(long classId, String className, byte[] bytes) + throws Exception { + IClassCoverage cc = runJaCoCoAnalysis(classId, className, bytes, null); + BitSet lines = new BitSet(); + if (cc != null && cc.getFirstLine() != ISourceNode.UNKNOWN_LINE) { + for (int line = cc.getFirstLine(); line <= cc.getLastLine(); line++) { + if (cc.getLine(line).getInstructionCounter().getTotalCount() > 0) { + lines.set(line); + } + } + } + return lines; + } + + private static int[] referenceProbeLines( + long classId, String className, int probeCount, byte[] bytes, int probeId) throws Exception { + boolean[] probes = new boolean[probeCount]; + probes[probeId] = true; + IClassCoverage cc = runJaCoCoAnalysis(classId, className, bytes, probes); + BitSet lines = new BitSet(); + if (cc != null && cc.getFirstLine() != ISourceNode.UNKNOWN_LINE) { + for (int line = cc.getFirstLine(); line <= cc.getLastLine(); line++) { + if (cc.getLine(line).getInstructionCounter().getCoveredCount() > 0) { + lines.set(line); + } + } + } + return bitSetToArray(lines); + } + + private static IClassCoverage runJaCoCoAnalysis( + long classId, String className, byte[] bytes, boolean[] probes) throws Exception { + ExecutionDataStore store = new ExecutionDataStore(); + if (probes != null) { + store.put(new ExecutionData(classId, className, probes)); + } + CoverageBuilder builder = new CoverageBuilder(); + Analyzer analyzer = new Analyzer(store, builder); + analyzer.analyzeClass(bytes, className); + for (IClassCoverage cc : builder.getClasses()) { + return cc; + } + return null; + } + + // --- Utility methods --- + + private static byte[] bytecodeFor(Class clazz) throws IOException { + String resource = clazz.getName().replace('.', '/') + ".class"; + try (InputStream is = clazz.getClassLoader().getResourceAsStream(resource)) { + assertNotNull(is, "Could not find class file for " + clazz.getName()); + return readAllBytes(is); + } + } + + private static int countProbes(byte[] classBytes) { + int[] count = {0}; + ClassProbesAdapter adapter = + new ClassProbesAdapter( + new ClassProbesVisitor() { + @Override + public MethodProbesVisitor visitMethod( + int access, String name, String desc, String signature, String[] exceptions) { + return new MethodProbesVisitor() { + @Override + public void visitProbe(int probeId) {} + + @Override + public void visitJumpInsnWithProbe( + int opcode, Label label, int probeId, IFrame frame) {} + + @Override + public void visitInsnWithProbe(int opcode, int probeId) {} + + @Override + public void visitTableSwitchInsnWithProbes( + int min, int max, Label dflt, Label[] labels, IFrame frame) {} + + @Override + public void visitLookupSwitchInsnWithProbes( + Label dflt, int[] keys, Label[] labels, IFrame frame) {} + }; + } + + @Override + public void visitTotalProbeCount(int c) { + count[0] = c; + } + }, + false); + InstrSupport.classReaderFor(classBytes).accept(adapter, 0); + return count[0]; + } + + private static int[] bitSetToArray(BitSet bs) { + int[] result = new int[bs.cardinality()]; + int idx = 0; + for (int bit = bs.nextSetBit(0); bit >= 0; bit = bs.nextSetBit(bit + 1)) { + result[idx++] = bit; + } + return result; + } + + private static byte[] readAllBytes(InputStream is) throws IOException { + ByteArrayOutputStream out = new ByteArrayOutputStream(); + byte[] buf = new byte[4096]; + int r; + while ((r = is.read(buf)) != -1) { + out.write(buf, 0, r); + } + return out.toByteArray(); + } +} diff --git a/dd-java-agent/agent-code-coverage/src/test/java/datadog/trace/codecoverage/CoverageBinaryEncoderTest.java b/dd-java-agent/agent-code-coverage/src/test/java/datadog/trace/codecoverage/CoverageBinaryEncoderTest.java index 1b6a26f8a35..2d87374f534 100644 --- a/dd-java-agent/agent-code-coverage/src/test/java/datadog/trace/codecoverage/CoverageBinaryEncoderTest.java +++ b/dd-java-agent/agent-code-coverage/src/test/java/datadog/trace/codecoverage/CoverageBinaryEncoderTest.java @@ -293,24 +293,88 @@ void specExampleTwoRecords() throws IOException { // Expected byte sequence from the spec byte[] expected = { - 0x01, 0x01, 0x02, // header: version=1, extra_fields=1, records=2 + 0x01, + 0x01, + 0x02, // header: version=1, extra_fields=1, records=2 0x14, // file_name length = 20 - 0x63, 0x6F, 0x6D, 0x2F, 0x65, 0x78, 0x61, 0x6D, // "com/exam" - 0x70, 0x6C, 0x65, 0x2F, 0x46, 0x6F, 0x6F, 0x2E, // "ple/Foo." - 0x6A, 0x61, 0x76, 0x61, // "java" + 0x63, + 0x6F, + 0x6D, + 0x2F, + 0x65, + 0x78, + 0x61, + 0x6D, // "com/exam" + 0x70, + 0x6C, + 0x65, + 0x2F, + 0x46, + 0x6F, + 0x6F, + 0x2E, // "ple/Foo." + 0x6A, + 0x61, + 0x76, + 0x61, // "java" 0x0F, // extra_fields[0] length = 15 - 0x63, 0x6F, 0x6D, 0x2E, 0x65, 0x78, 0x61, 0x6D, // "com.exam" - 0x70, 0x6C, 0x65, 0x2E, 0x46, 0x6F, 0x6F, // "ple.Foo" + 0x63, + 0x6F, + 0x6D, + 0x2E, + 0x65, + 0x78, + 0x61, + 0x6D, // "com.exam" + 0x70, + 0x6C, + 0x65, + 0x2E, + 0x46, + 0x6F, + 0x6F, // "ple.Foo" 0x02, // bitvec_byte_count = 2 - 0x2E, 0x01, // executable_lines - 0x2A, 0x00, // covered_lines + 0x2E, + 0x01, // executable_lines + 0x2A, + 0x00, // covered_lines 0x14, // file_name length = 20 - 0x63, 0x6F, 0x6D, 0x2F, 0x65, 0x78, 0x61, 0x6D, // "com/exam" - 0x70, 0x6C, 0x65, 0x2F, 0x42, 0x61, 0x72, 0x2E, // "ple/Bar." - 0x6A, 0x61, 0x76, 0x61, // "java" + 0x63, + 0x6F, + 0x6D, + 0x2F, + 0x65, + 0x78, + 0x61, + 0x6D, // "com/exam" + 0x70, + 0x6C, + 0x65, + 0x2F, + 0x42, + 0x61, + 0x72, + 0x2E, // "ple/Bar." + 0x6A, + 0x61, + 0x76, + 0x61, // "java" 0x0F, // extra_fields[0] length = 15 - 0x63, 0x6F, 0x6D, 0x2E, 0x65, 0x78, 0x61, 0x6D, // "com.exam" - 0x70, 0x6C, 0x65, 0x2E, 0x42, 0x61, 0x72, // "ple.Bar" + 0x63, + 0x6F, + 0x6D, + 0x2E, + 0x65, + 0x78, + 0x61, + 0x6D, // "com.exam" + 0x70, + 0x6C, + 0x65, + 0x2E, + 0x42, + 0x61, + 0x72, // "ple.Bar" 0x01, // bitvec_byte_count = 1 0x54, // executable_lines 0x10 // covered_lines From 1a38ffe2e89c96281599398e99d48b95f06eaddc Mon Sep 17 00:00:00 2001 From: Nikita Tkachenko Date: Thu, 26 Mar 2026 18:00:06 +0100 Subject: [PATCH 15/19] Fix single-pass ClassProbeMappingBuilder probe semantics Three fixes to match JaCoCo's MethodAnalyzer behavior: 1. addProbe() no longer unconditionally nulls currentInsn. This was breaking fall-through edges after probed conditional jumps and silently dropping later probed switch targets. 2. Only visitProbe() (standalone probes) sets currentInsn = null, matching JaCoCo where only visitProbe calls noSuccessor(). visitJumpInsnWithProbe, visitInsnWithProbe, and switch target probes correctly leave the chain intact. 3. Added one JaCoCo Analyzer pass (no probes) to obtain filtered executable lines (FinallyFilter, etc.). Probe-to-lines are intersected against these filtered lines so compiler-generated duplicates (e.g. finally block copies) are excluded. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../ClassProbeMappingBuilder.java | 429 +++++++++++++++--- 1 file changed, 374 insertions(+), 55 deletions(-) diff --git a/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/ClassProbeMappingBuilder.java b/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/ClassProbeMappingBuilder.java index de7400ab4da..1498bcc7173 100644 --- a/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/ClassProbeMappingBuilder.java +++ b/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/ClassProbeMappingBuilder.java @@ -1,90 +1,409 @@ package datadog.trace.codecoverage; +import java.util.ArrayList; import java.util.BitSet; +import java.util.HashMap; +import java.util.List; +import java.util.Map; import org.jacoco.core.analysis.Analyzer; import org.jacoco.core.analysis.CoverageBuilder; import org.jacoco.core.analysis.IClassCoverage; import org.jacoco.core.analysis.ISourceNode; -import org.jacoco.core.data.ExecutionData; import org.jacoco.core.data.ExecutionDataStore; +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.jacoco.core.internal.instr.InstrSupport; +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; /** - * Builds a {@link ClassProbeMapping} by delegating to JaCoCo's {@link Analyzer}. For each probe, - * the class is analyzed with only that probe marked as executed; the covered lines are then - * collected into the mapping. + * Builds a {@link ClassProbeMapping} by parsing the class bytecode using JaCoCo's {@link + * ClassProbesAdapter}, building a simplified instruction graph, and walking predecessor chains to + * determine which lines each probe covers. * - *

This approach is O(probeCount) in the number of JaCoCo analysis passes. It is slower than a - * single-pass approach but guaranteed to be correct, as it reuses JaCoCo's own analysis logic - * including all filters and edge-case handling. + *

Uses two passes over the bytecode: one via {@link ClassProbesAdapter} to build probe-to-line + * mappings (single-pass, O(1) regardless of probe count), and one via JaCoCo's {@link Analyzer} to + * obtain the filtered set of executable lines (applying FinallyFilter, etc.). This is significantly + * faster than the N+1 pass approach while remaining correct. */ final class ClassProbeMappingBuilder { static ClassProbeMapping build( long classId, String className, int probeCount, byte[] classBytes) { - // Analyze with no probes to determine executable lines and source file - IClassCoverage baseline = analyzeClass(classId, className, classBytes, null); - if (baseline == null) { - return new ClassProbeMapping(classId, className, null, new BitSet(), new int[probeCount][]); + // Single-pass: build probe-to-lines mapping via predecessor chains + ClassReader reader = InstrSupport.classReaderFor(classBytes); + ProbeMappingVisitor visitor = new ProbeMappingVisitor(); + ClassProbesAdapter adapter = new ClassProbesAdapter(visitor, false); + reader.accept(adapter, 0); + + // One Analyzer pass: get filtered executable lines (handles FinallyFilter, etc.) + BitSet executableLines = filteredExecutableLines(classBytes); + if (executableLines == null) { + executableLines = visitor.executableLines; } + return visitor.toMapping(classId, className, probeCount, executableLines); + } - String sourceFile = baseline.getSourceFileName(); - BitSet executableLines = collectLines(baseline, false); + /** + * Runs JaCoCo's Analyzer (with no probes) to get the set of executable lines after all filters + * (FinallyFilter, bridge methods, etc.) have been applied. Returns null on failure. + */ + private static BitSet filteredExecutableLines(byte[] classBytes) { + try { + CoverageBuilder builder = new CoverageBuilder(); + Analyzer analyzer = new Analyzer(new ExecutionDataStore(), builder); + analyzer.analyzeClass(classBytes, ""); + for (IClassCoverage cc : builder.getClasses()) { + BitSet lines = new BitSet(); + int first = cc.getFirstLine(); + int last = cc.getLastLine(); + if (first != ISourceNode.UNKNOWN_LINE) { + for (int line = first; line <= last; line++) { + if (cc.getLine(line).getInstructionCounter().getTotalCount() > 0) { + lines.set(line); + } + } + } + return lines; + } + } catch (Exception e) { + // fall through + } + return null; + } + + /** Simplified instruction node with a line number and a single predecessor link. */ + static final class ProbeNode { + final int line; + ProbeNode predecessor; - // For each probe, analyze with only that probe set to determine covered lines - int[][] probeToLines = new int[probeCount][]; - for (int p = 0; p < probeCount; p++) { - boolean[] probes = new boolean[probeCount]; - probes[p] = true; - IClassCoverage cc = analyzeClass(classId, className, classBytes, probes); - probeToLines[p] = (cc != null) ? bitSetToArray(collectLines(cc, true)) : new int[0]; + ProbeNode(int line) { + this.line = line; } + } + + /** A deferred jump from a source instruction to a target label. */ + static final class Jump { + final ProbeNode source; + final Label target; + final int branch; - return new ClassProbeMapping(classId, className, sourceFile, executableLines, probeToLines); + Jump(ProbeNode source, Label target, int branch) { + this.source = source; + this.target = target; + this.branch = branch; + } } - private static IClassCoverage analyzeClass( - long classId, String className, byte[] classBytes, boolean[] probes) { - ExecutionDataStore store = new ExecutionDataStore(); - if (probes != null) { - store.put(new ExecutionData(classId, className, probes)); + /** + * Class-level visitor that collects source file info and delegates method visiting to {@link + * MethodMapper}. + */ + private static final class ProbeMappingVisitor extends ClassProbesVisitor { + private String sourceFile; + private final BitSet executableLines = new BitSet(); + private final Map probeToLines = new HashMap<>(); + + @Override + public void visitSource(String source, String debug) { + sourceFile = source; } - CoverageBuilder builder = new CoverageBuilder(); - Analyzer analyzer = new Analyzer(store, builder); - try { - analyzer.analyzeClass(classBytes, className); - } catch (Exception e) { - return null; + + @Override + public MethodProbesVisitor visitMethod( + int access, String name, String desc, String signature, String[] exceptions) { + return new MethodMapper(executableLines, probeToLines); } - for (IClassCoverage cc : builder.getClasses()) { - return cc; + + @Override + public void visitTotalProbeCount(int count) { + // no-op; we get probeCount from the caller } - return null; - } - private static BitSet collectLines(IClassCoverage cc, boolean coveredOnly) { - BitSet lines = new BitSet(); - int first = cc.getFirstLine(); - int last = cc.getLastLine(); - if (first != ISourceNode.UNKNOWN_LINE) { - for (int line = first; line <= last; line++) { - int count = - coveredOnly - ? cc.getLine(line).getInstructionCounter().getCoveredCount() - : cc.getLine(line).getInstructionCounter().getTotalCount(); - if (count > 0) { - lines.set(line); + ClassProbeMapping toMapping( + long classId, String className, int probeCount, BitSet filteredExecLines) { + int[][] probeToLinesArray = new int[probeCount][]; + for (int p = 0; p < probeCount; p++) { + BitSet lines = probeToLines.get(p); + if (lines != null) { + // Intersect with filtered executable lines so probe lines never include + // lines that JaCoCo's filters removed (e.g. duplicated finally blocks). + BitSet filtered = (BitSet) lines.clone(); + filtered.and(filteredExecLines); + probeToLinesArray[p] = bitSetToArray(filtered); + } else { + probeToLinesArray[p] = new int[0]; } } + return new ClassProbeMapping( + classId, className, sourceFile, filteredExecLines, probeToLinesArray); + } + + private static int[] bitSetToArray(BitSet bs) { + int[] result = new int[bs.cardinality()]; + int idx = 0; + for (int bit = bs.nextSetBit(0); bit >= 0; bit = bs.nextSetBit(bit + 1)) { + result[idx++] = bit; + } + return result; } - return lines; } - private static int[] bitSetToArray(BitSet bs) { - int[] result = new int[bs.cardinality()]; - int idx = 0; - for (int bit = bs.nextSetBit(0); bit >= 0; bit = bs.nextSetBit(bit + 1)) { - result[idx++] = bit; + /** + * Method-level visitor that builds a simplified instruction graph (with predecessor links) and + * records probe-to-instruction associations. After all instructions are replayed, jump targets + * are wired and predecessor chains are walked to collect covered lines per probe. + * + *

This replicates the logic of JaCoCo's {@code InstructionsBuilder} and {@code + * MethodAnalyzer}, which are package-private and cannot be used directly from this package. + */ + private static final class MethodMapper extends MethodProbesVisitor { + private static final int UNKNOWN_LINE = -1; + + private final BitSet executableLines; + private final Map probeToLines; + + // Per-method state + private int currentLine = UNKNOWN_LINE; + private ProbeNode currentInsn; + private final List

On the first collection cycle (or when new classes appear), a classpath scan builds the cache. * Subsequent cycles simply iterate boolean probe arrays and set bits -- no JaCoCo {@code Analyzer} * pass is needed. + * + *

Newly instrumented classes that have not yet received any probe hits are also reported (with + * executable lines but empty covered lines) so the backend can compute accurate total coverage. */ public final class CodeCoverageCollector { private static final Logger log = LoggerFactory.getLogger(CodeCoverageCollector.class); - private final CodeCoverageTransformer transformer; - private final CodeCoverageSender sender; + private final BiConsumer collectAndResetFn; + private final Supplier> drainNewClassesFn; + private final Consumer> uploadFn; private final int intervalSeconds; private final String explicitClasspath; private final ProbeMappingCache probeCache = new ProbeMappingCache(); @@ -48,8 +61,24 @@ public CodeCoverageCollector( CodeCoverageSender sender, int intervalSeconds, String explicitClasspath) { - this.transformer = transformer; - this.sender = sender; + this( + transformer::collectAndReset, + transformer::drainNewClasses, + sender::upload, + intervalSeconds, + explicitClasspath); + } + + /** Package-private constructor for testing. */ + CodeCoverageCollector( + BiConsumer collectAndResetFn, + Supplier> drainNewClassesFn, + Consumer> uploadFn, + int intervalSeconds, + String explicitClasspath) { + this.collectAndResetFn = collectAndResetFn; + this.drainNewClassesFn = drainNewClassesFn; + this.uploadFn = uploadFn; this.intervalSeconds = intervalSeconds; this.explicitClasspath = explicitClasspath; } @@ -72,7 +101,7 @@ void collect() { // 1. Collect and reset probes ExecutionDataStore execStore = new ExecutionDataStore(); SessionInfoStore sessionStore = new SessionInfoStore(); - transformer.collectAndReset(execStore, sessionStore); + collectAndResetFn.accept(execStore, sessionStore); // 2. Separate cache hits from misses Collection allEntries = execStore.getContents(); @@ -90,9 +119,11 @@ void collect() { log.debug("Built cache entries for {} new classes", cacheMisses.size()); } - // 4. Build coverage from cache + // 4. Build coverage from hit data Map coverage = new HashMap<>(); + Set hitClassNames = new HashSet<>(); for (ExecutionData ed : allEntries) { + hitClassNames.add(ed.getName()); ClassProbeMapping mapping = probeCache.get(ed.getId()); if (mapping == null || mapping.className == null || mapping.sourceFile == null) { continue; // no mapping available @@ -112,9 +143,33 @@ void collect() { } } - // 5. Send if there is data + // 5. Report newly instrumented classes that had no hits this cycle + List newClasses = drainNewClassesFn.get(); + for (String className : newClasses) { + if (hitClassNames.contains(className)) { + continue; // already covered by hit data above + } + byte[] classBytes = resolveClassBytes(className); + if (classBytes == null) { + continue; + } + long classId = CRC64.classId(classBytes); + ClassProbeMapping mapping = + ClassProbeMappingBuilder.buildBaseline(classId, className, classBytes); + if (mapping == null || mapping.sourceFile == null) { + continue; + } + CoverageKey key = new CoverageKey(mapping.sourceFile, mapping.className); + if (!coverage.containsKey(key)) { + LinesCoverage lc = new LinesCoverage(); + lc.executableLines.or(mapping.executableLines); + coverage.put(key, lc); + } + } + + // 6. Send if there is data if (!coverage.isEmpty()) { - sender.upload(coverage); + uploadFn.accept(coverage); } } catch (Exception e) { log.debug("Error during code coverage collection", e); @@ -143,4 +198,36 @@ private List resolveClasspath() { } return entries; } + + /** Resolves class bytes via system and context classloaders. Returns null if not found. */ + private static byte[] resolveClassBytes(String className) { + String resource = className + ".class"; + ClassLoader systemCl = ClassLoader.getSystemClassLoader(); + ClassLoader contextCl = Thread.currentThread().getContextClassLoader(); + for (ClassLoader cl : new ClassLoader[] {systemCl, contextCl}) { + if (cl == null) { + continue; + } + InputStream is = cl.getResourceAsStream(resource); + if (is == null) { + continue; + } + try (InputStream stream = is) { + return readAllBytes(stream); + } catch (IOException e) { + // try next classloader + } + } + return null; + } + + private static byte[] readAllBytes(InputStream is) throws IOException { + ByteArrayOutputStream out = new ByteArrayOutputStream(); + byte[] buf = new byte[4096]; + int r; + while ((r = is.read(buf)) != -1) { + out.write(buf, 0, r); + } + return out.toByteArray(); + } } diff --git a/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageTransformer.java b/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageTransformer.java index 95199e47fdf..f240464d6cd 100644 --- a/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageTransformer.java +++ b/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageTransformer.java @@ -6,10 +6,13 @@ import java.lang.instrument.ClassFileTransformer; import java.lang.instrument.Instrumentation; import java.security.ProtectionDomain; +import java.util.ArrayList; import java.util.Collections; import java.util.HashSet; +import java.util.List; import java.util.Map; import java.util.Set; +import java.util.concurrent.ConcurrentLinkedQueue; import java.util.function.Predicate; import org.jacoco.core.data.ExecutionDataReader; import org.jacoco.core.data.ExecutionDataStore; @@ -36,6 +39,7 @@ public final class CodeCoverageTransformer implements ClassFileTransformer { private final RuntimeData runtimeData; private final Instrumenter instrumenter; private final Predicate filter; + private final ConcurrentLinkedQueue newlyInstrumented = new ConcurrentLinkedQueue<>(); /** * Initializes the JaCoCo runtime and instrumenter. @@ -169,13 +173,28 @@ public byte[] transform( return null; } try { - return instrumenter.instrument(classfileBuffer, className); + byte[] instrumented = instrumenter.instrument(classfileBuffer, className); + newlyInstrumented.add(className); + return instrumented; } catch (Exception e) { log.debug("Failed to instrument class {}", className, e); return null; } } + /** + * Drains and returns the list of class names that have been instrumented since the last call. + * Each class name is in VM internal format (e.g. {@code com/example/MyClass}). + */ + public List drainNewClasses() { + List result = new ArrayList<>(); + String name; + while ((name = newlyInstrumented.poll()) != null) { + result.add(name); + } + return result; + } + /** * Collects current probe data and resets all probes to {@code false}. * diff --git a/dd-java-agent/agent-code-coverage/src/test/java/datadog/trace/codecoverage/CodeCoverageCollectorTest.java b/dd-java-agent/agent-code-coverage/src/test/java/datadog/trace/codecoverage/CodeCoverageCollectorTest.java new file mode 100644 index 00000000000..f087ae508e7 --- /dev/null +++ b/dd-java-agent/agent-code-coverage/src/test/java/datadog/trace/codecoverage/CodeCoverageCollectorTest.java @@ -0,0 +1,305 @@ +package datadog.trace.codecoverage; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import datadog.trace.coverage.CoverageKey; +import datadog.trace.coverage.LinesCoverage; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.InputStream; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import java.util.concurrent.atomic.AtomicInteger; +import org.jacoco.core.data.ExecutionData; +import org.jacoco.core.internal.data.CRC64; +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.MethodProbesVisitor; +import org.jacoco.core.internal.instr.InstrSupport; +import org.junit.jupiter.api.Test; +import org.objectweb.asm.Label; + +class CodeCoverageCollectorTest { + + // ===== Sample classes used as test subjects ===== + + @SuppressWarnings("unused") + static class SampleClassA { + int compute(int x) { + int a = x + 1; + int b = a * 2; + return b; + } + } + + @SuppressWarnings("unused") + static class SampleClassB { + int compute(int x) { + if (x > 0) { + return x; + } + return -x; + } + } + + // ===== Tests ===== + + @Test + void classWithHits_producesNormalCoverageRecord() { + byte[] classBytes = bytecodeFor(SampleClassA.class); + long classId = CRC64.classId(classBytes); + String className = SampleClassA.class.getName().replace('.', '/'); + int probeCount = countProbes(classBytes); + + boolean[] probes = new boolean[probeCount]; + Arrays.fill(probes, true); + + List> uploads = new ArrayList<>(); + + CodeCoverageCollector collector = + new CodeCoverageCollector( + (store, session) -> store.put(new ExecutionData(classId, className, probes)), + Collections::emptyList, + uploads::add, + 60, + null); + + collector.collect(); + + assertEquals(1, uploads.size()); + Map coverage = uploads.get(0); + assertEquals(1, coverage.size()); + + LinesCoverage lc = coverage.values().iterator().next(); + assertFalse(lc.executableLines.isEmpty(), "should have executable lines"); + assertFalse(lc.coveredLines.isEmpty(), "should have covered lines"); + } + + @Test + void newClassWithoutHits_reportsExecutableLinesOnly() { + String className = SampleClassB.class.getName().replace('.', '/'); + + List> uploads = new ArrayList<>(); + + CodeCoverageCollector collector = + new CodeCoverageCollector( + (store, session) -> {}, + () -> Collections.singletonList(className), + uploads::add, + 60, + null); + + collector.collect(); + + assertEquals(1, uploads.size()); + Map coverage = uploads.get(0); + assertEquals(1, coverage.size()); + + LinesCoverage lc = coverage.values().iterator().next(); + assertFalse(lc.executableLines.isEmpty(), "should have executable lines"); + assertTrue(lc.coveredLines.isEmpty(), "should have no covered lines"); + } + + @Test + void newClassWithHits_notDuplicated() { + byte[] classBytes = bytecodeFor(SampleClassA.class); + long classId = CRC64.classId(classBytes); + String className = SampleClassA.class.getName().replace('.', '/'); + int probeCount = countProbes(classBytes); + + boolean[] probes = new boolean[probeCount]; + probes[0] = true; + + List> uploads = new ArrayList<>(); + + CodeCoverageCollector collector = + new CodeCoverageCollector( + (store, session) -> store.put(new ExecutionData(classId, className, probes)), + () -> Collections.singletonList(className), + uploads::add, + 60, + null); + + collector.collect(); + + assertEquals(1, uploads.size()); + Map coverage = uploads.get(0); + assertEquals(1, coverage.size(), "class with hits and in drain should produce one entry"); + } + + @Test + void noHitsNoDrain_nothingUploaded() { + List> uploads = new ArrayList<>(); + + CodeCoverageCollector collector = + new CodeCoverageCollector( + (store, session) -> {}, Collections::emptyList, uploads::add, 60, null); + + collector.collect(); + + assertTrue(uploads.isEmpty(), "nothing should be uploaded"); + } + + @Test + void drainedClassNotReReported() { + String className = SampleClassB.class.getName().replace('.', '/'); + AtomicInteger drainCallCount = new AtomicInteger(0); + + List> uploads = new ArrayList<>(); + + CodeCoverageCollector collector = + new CodeCoverageCollector( + (store, session) -> {}, + () -> { + if (drainCallCount.getAndIncrement() == 0) { + return Collections.singletonList(className); + } + return Collections.emptyList(); + }, + uploads::add, + 60, + null); + + // First cycle: should report the class + collector.collect(); + assertEquals(1, uploads.size()); + + // Second cycle: drain returns empty, no hits -> nothing to upload + collector.collect(); + assertEquals(1, uploads.size(), "should not upload again when no new data"); + } + + @Test + void mixedHitAndNoHitClasses() { + byte[] hitClassBytes = bytecodeFor(SampleClassA.class); + long hitClassId = CRC64.classId(hitClassBytes); + String hitClassName = SampleClassA.class.getName().replace('.', '/'); + int hitProbeCount = countProbes(hitClassBytes); + + String noHitClassName = SampleClassB.class.getName().replace('.', '/'); + + boolean[] probes = new boolean[hitProbeCount]; + Arrays.fill(probes, true); + + List> uploads = new ArrayList<>(); + + CodeCoverageCollector collector = + new CodeCoverageCollector( + (store, session) -> store.put(new ExecutionData(hitClassId, hitClassName, probes)), + () -> Arrays.asList(hitClassName, noHitClassName), + uploads::add, + 60, + null); + + collector.collect(); + + assertEquals(1, uploads.size()); + Map coverage = uploads.get(0); + assertEquals(2, coverage.size(), "should have entries for both classes"); + + LinesCoverage hitLc = null; + LinesCoverage noHitLc = null; + for (Map.Entry entry : coverage.entrySet()) { + if (entry.getKey().className.equals(hitClassName)) { + hitLc = entry.getValue(); + } else if (entry.getKey().className.equals(noHitClassName)) { + noHitLc = entry.getValue(); + } + } + + assertNotNull(hitLc, "hit class should be in coverage"); + assertFalse(hitLc.coveredLines.isEmpty(), "hit class should have covered lines"); + assertFalse(hitLc.executableLines.isEmpty(), "hit class should have executable lines"); + + assertNotNull(noHitLc, "no-hit class should be in coverage"); + assertTrue(noHitLc.coveredLines.isEmpty(), "no-hit class should have no covered lines"); + assertFalse(noHitLc.executableLines.isEmpty(), "no-hit class should have executable lines"); + } + + @Test + void newClassExecutableLines_matchFullBuild() { + // Verify that buildBaseline produces the same executable lines as the full build + byte[] classBytes = bytecodeFor(SampleClassB.class); + long classId = CRC64.classId(classBytes); + String className = SampleClassB.class.getName().replace('.', '/'); + int probeCount = countProbes(classBytes); + + ClassProbeMapping full = + ClassProbeMappingBuilder.build(classId, className, probeCount, classBytes); + ClassProbeMapping baseline = + ClassProbeMappingBuilder.buildBaseline(classId, className, classBytes); + + assertNotNull(baseline); + assertEquals(full.executableLines, baseline.executableLines); + assertEquals(full.sourceFile, baseline.sourceFile); + assertEquals(0, baseline.probeToLines.length, "baseline should have no probe-to-lines mapping"); + } + + // ===== Helpers ===== + + private static byte[] bytecodeFor(Class clazz) { + String resource = clazz.getName().replace('.', '/') + ".class"; + try (InputStream is = clazz.getClassLoader().getResourceAsStream(resource)) { + assertNotNull(is, "Could not find class file for " + clazz.getName()); + return readAllBytes(is); + } catch (IOException e) { + throw new RuntimeException(e); + } + } + + private static int countProbes(byte[] classBytes) { + int[] count = {0}; + ClassProbesAdapter adapter = + new ClassProbesAdapter( + new ClassProbesVisitor() { + @Override + public MethodProbesVisitor visitMethod( + int access, String name, String desc, String signature, String[] exceptions) { + return new MethodProbesVisitor() { + @Override + public void visitProbe(int probeId) {} + + @Override + public void visitJumpInsnWithProbe( + int opcode, Label label, int probeId, IFrame frame) {} + + @Override + public void visitInsnWithProbe(int opcode, int probeId) {} + + @Override + public void visitTableSwitchInsnWithProbes( + int min, int max, Label dflt, Label[] labels, IFrame frame) {} + + @Override + public void visitLookupSwitchInsnWithProbes( + Label dflt, int[] keys, Label[] labels, IFrame frame) {} + }; + } + + @Override + public void visitTotalProbeCount(int c) { + count[0] = c; + } + }, + false); + InstrSupport.classReaderFor(classBytes).accept(adapter, 0); + return count[0]; + } + + private static byte[] readAllBytes(InputStream is) throws IOException { + ByteArrayOutputStream out = new ByteArrayOutputStream(); + byte[] buf = new byte[4096]; + int r; + while ((r = is.read(buf)) != -1) { + out.write(buf, 0, r); + } + return out.toByteArray(); + } +} From bb39c2b857eccffc12ea521ad52e16345414723d Mon Sep 17 00:00:00 2001 From: Nikita Tkachenko Date: Thu, 26 Mar 2026 18:34:06 +0100 Subject: [PATCH 17/19] Resolve class bytes via defining ClassLoader recorded at transform time MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The transformer now records CRC64 → WeakReference at transform time, when the JVM provides the exact defining classloader. ProbeMappingCache and the baseline reporting path use this recorded origin as the primary resolution strategy, falling back to system and context classloaders only if the defining loader is unavailable. This fixes resolution for classes loaded by custom classloaders (Spring Boot nested jars, OSGi, app-server modules, etc.) that are invisible to the system classloader and java.class.path. Other changes: - Removed recursive classpath scanning (scanDirectory, scanJar) and the explicitClasspath configuration — no longer needed - Removed permanent sentinel misses — unresolved classes are retried on subsequent collection cycles instead of being skipped forever - Transformer queues (classId, className) pairs instead of just names, so baseline resolution also uses origin-based lookup with CRC64 verification, eliminating the separate guess-based resolveClassBytes Co-Authored-By: Claude Opus 4.6 (1M context) --- .../codecoverage/CodeCoverageCollector.java | 109 ++------ .../codecoverage/CodeCoverageSystem.java | 5 +- .../codecoverage/CodeCoverageTransformer.java | 50 +++- .../trace/codecoverage/ProbeMappingCache.java | 232 ++++++------------ .../CodeCoverageCollectorTest.java | 44 ++-- 5 files changed, 165 insertions(+), 275 deletions(-) diff --git a/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageCollector.java b/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageCollector.java index 4306b330bf4..52a78f4c427 100644 --- a/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageCollector.java +++ b/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageCollector.java @@ -5,10 +5,6 @@ import datadog.trace.coverage.CoverageKey; import datadog.trace.coverage.LinesCoverage; import datadog.trace.util.AgentTaskScheduler; -import java.io.ByteArrayOutputStream; -import java.io.File; -import java.io.IOException; -import java.io.InputStream; import java.util.ArrayList; import java.util.Collection; import java.util.HashMap; @@ -19,11 +15,11 @@ import java.util.concurrent.TimeUnit; import java.util.function.BiConsumer; import java.util.function.Consumer; +import java.util.function.LongFunction; import java.util.function.Supplier; import org.jacoco.core.data.ExecutionData; import org.jacoco.core.data.ExecutionDataStore; import org.jacoco.core.data.SessionInfoStore; -import org.jacoco.core.internal.data.CRC64; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -31,9 +27,10 @@ * Periodically collects code coverage probe data, resolves it to covered source lines using a * cached probe-to-line mapping, and sends the results via a {@link CodeCoverageSender}. * - *

On the first collection cycle (or when new classes appear), a classpath scan builds the cache. - * Subsequent cycles simply iterate boolean probe arrays and set bits -- no JaCoCo {@code Analyzer} - * pass is needed. + *

On the first collection cycle (or when new classes appear), cache entries are built by + * resolving class bytes through the defining ClassLoader recorded at transform time. Subsequent + * cycles simply iterate boolean probe arrays and set bits -- no JaCoCo {@code Analyzer} pass is + * needed. * *

Newly instrumented classes that have not yet received any probe hits are also reported (with * executable lines but empty covered lines) so the backend can compute accurate total coverage. @@ -43,44 +40,40 @@ public final class CodeCoverageCollector { private static final Logger log = LoggerFactory.getLogger(CodeCoverageCollector.class); private final BiConsumer collectAndResetFn; - private final Supplier> drainNewClassesFn; + private final Supplier> drainNewClassesFn; + private final LongFunction classLoaderLookup; private final Consumer> uploadFn; private final int intervalSeconds; - private final String explicitClasspath; private final ProbeMappingCache probeCache = new ProbeMappingCache(); private final AgentTaskScheduler scheduler = new AgentTaskScheduler(CODE_COVERAGE); /** - * @param transformer the transformer that holds runtime probe data + * @param transformer the transformer that holds runtime probe data and classloader origins * @param sender the sender to deliver coverage results to * @param intervalSeconds interval between collection cycles - * @param explicitClasspath explicit classpath override (nullable; if null, auto-detected) */ public CodeCoverageCollector( - CodeCoverageTransformer transformer, - CodeCoverageSender sender, - int intervalSeconds, - String explicitClasspath) { + CodeCoverageTransformer transformer, CodeCoverageSender sender, int intervalSeconds) { this( transformer::collectAndReset, transformer::drainNewClasses, + transformer::getDefiningClassLoader, sender::upload, - intervalSeconds, - explicitClasspath); + intervalSeconds); } /** Package-private constructor for testing. */ CodeCoverageCollector( BiConsumer collectAndResetFn, - Supplier> drainNewClassesFn, + Supplier> drainNewClassesFn, + LongFunction classLoaderLookup, Consumer> uploadFn, - int intervalSeconds, - String explicitClasspath) { + int intervalSeconds) { this.collectAndResetFn = collectAndResetFn; this.drainNewClassesFn = drainNewClassesFn; + this.classLoaderLookup = classLoaderLookup; this.uploadFn = uploadFn; this.intervalSeconds = intervalSeconds; - this.explicitClasspath = explicitClasspath; } /** Starts the periodic collection scheduler. */ @@ -112,10 +105,9 @@ void collect() { } } - // 3. Build cache entries for misses (scans classpath) + // 3. Build cache entries for misses via recorded classloader if (!cacheMisses.isEmpty()) { - List classpathEntries = resolveClasspath(); - probeCache.buildMissing(cacheMisses, classpathEntries); + probeCache.buildMissing(cacheMisses, classLoaderLookup); log.debug("Built cache entries for {} new classes", cacheMisses.size()); } @@ -144,18 +136,18 @@ void collect() { } // 5. Report newly instrumented classes that had no hits this cycle - List newClasses = drainNewClassesFn.get(); - for (String className : newClasses) { - if (hitClassNames.contains(className)) { + List newClasses = drainNewClassesFn.get(); + for (CodeCoverageTransformer.NewClass nc : newClasses) { + if (hitClassNames.contains(nc.className)) { continue; // already covered by hit data above } - byte[] classBytes = resolveClassBytes(className); + byte[] classBytes = + ProbeMappingCache.resolveClassBytes(nc.className, nc.classId, classLoaderLookup); if (classBytes == null) { continue; } - long classId = CRC64.classId(classBytes); ClassProbeMapping mapping = - ClassProbeMappingBuilder.buildBaseline(classId, className, classBytes); + ClassProbeMappingBuilder.buildBaseline(nc.classId, nc.className, classBytes); if (mapping == null || mapping.sourceFile == null) { continue; } @@ -175,59 +167,4 @@ void collect() { log.debug("Error during code coverage collection", e); } } - - /** - * Resolves classpath entries to analyze. If an explicit classpath is configured, it takes - * precedence. Otherwise, falls back to {@code java.class.path} system property. - */ - private List resolveClasspath() { - String cp; - if (explicitClasspath != null && !explicitClasspath.isEmpty()) { - cp = explicitClasspath; - } else { - cp = System.getProperty("java.class.path"); - } - List entries = new ArrayList<>(); - if (cp != null && !cp.isEmpty()) { - for (String path : cp.split(File.pathSeparator)) { - String trimmed = path.trim(); - if (!trimmed.isEmpty()) { - entries.add(new File(trimmed)); - } - } - } - return entries; - } - - /** Resolves class bytes via system and context classloaders. Returns null if not found. */ - private static byte[] resolveClassBytes(String className) { - String resource = className + ".class"; - ClassLoader systemCl = ClassLoader.getSystemClassLoader(); - ClassLoader contextCl = Thread.currentThread().getContextClassLoader(); - for (ClassLoader cl : new ClassLoader[] {systemCl, contextCl}) { - if (cl == null) { - continue; - } - InputStream is = cl.getResourceAsStream(resource); - if (is == null) { - continue; - } - try (InputStream stream = is) { - return readAllBytes(stream); - } catch (IOException e) { - // try next classloader - } - } - return null; - } - - private static byte[] readAllBytes(InputStream is) throws IOException { - ByteArrayOutputStream out = new ByteArrayOutputStream(); - byte[] buf = new byte[4096]; - int r; - while ((r = is.read(buf)) != -1) { - out.write(buf, 0, r); - } - return out.toByteArray(); - } } diff --git a/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageSystem.java b/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageSystem.java index 7affc7ec00e..870c1651385 100644 --- a/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageSystem.java +++ b/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageSystem.java @@ -101,10 +101,7 @@ public static void startCollector(Object transformerObj, Object scoObj) { CodeCoverageCollector collector = new CodeCoverageCollector( - transformer, - sender, - config.getCodeCoverageReportIntervalSeconds(), - config.getCodeCoverageClasspath()); + transformer, sender, config.getCodeCoverageReportIntervalSeconds()); collector.start(); } diff --git a/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageTransformer.java b/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageTransformer.java index f240464d6cd..1236202fc2f 100644 --- a/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageTransformer.java +++ b/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageTransformer.java @@ -5,6 +5,7 @@ import java.io.InputStream; import java.lang.instrument.ClassFileTransformer; import java.lang.instrument.Instrumentation; +import java.lang.ref.WeakReference; import java.security.ProtectionDomain; import java.util.ArrayList; import java.util.Collections; @@ -12,6 +13,7 @@ import java.util.List; import java.util.Map; import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentLinkedQueue; import java.util.function.Predicate; import org.jacoco.core.data.ExecutionDataReader; @@ -39,7 +41,25 @@ public final class CodeCoverageTransformer implements ClassFileTransformer { private final RuntimeData runtimeData; private final Instrumenter instrumenter; private final Predicate filter; - private final ConcurrentLinkedQueue newlyInstrumented = new ConcurrentLinkedQueue<>(); + private final ConcurrentLinkedQueue newlyInstrumented = new ConcurrentLinkedQueue<>(); + + /** Lightweight pair of CRC64 class ID and VM class name, queued at transform time. */ + static final class NewClass { + final long classId; + final String className; + + NewClass(long classId, String className) { + this.classId = classId; + this.className = className; + } + } + + /** + * Maps CRC64 class ID to the defining ClassLoader recorded at transform time. Uses weak + * references so classloaders can be garbage-collected when their classes are unloaded. + */ + private final ConcurrentHashMap> classLoadersByClassId = + new ConcurrentHashMap<>(); /** * Initializes the JaCoCo runtime and instrumenter. @@ -174,7 +194,9 @@ public byte[] transform( } try { byte[] instrumented = instrumenter.instrument(classfileBuffer, className); - newlyInstrumented.add(className); + long classId = org.jacoco.core.internal.data.CRC64.classId(classfileBuffer); + classLoadersByClassId.put(classId, new WeakReference<>(loader)); + newlyInstrumented.add(new NewClass(classId, className)); return instrumented; } catch (Exception e) { log.debug("Failed to instrument class {}", className, e); @@ -183,14 +205,24 @@ public byte[] transform( } /** - * Drains and returns the list of class names that have been instrumented since the last call. - * Each class name is in VM internal format (e.g. {@code com/example/MyClass}). + * Returns the defining ClassLoader recorded at transform time for the given CRC64 class ID, or + * null if the class was not instrumented by this transformer or if the classloader has been + * garbage-collected. + */ + public ClassLoader getDefiningClassLoader(long classId) { + WeakReference ref = classLoadersByClassId.get(classId); + return (ref != null) ? ref.get() : null; + } + + /** + * Drains and returns the list of classes that have been instrumented since the last call. Each + * entry contains the CRC64 class ID and the VM-format class name. */ - public List drainNewClasses() { - List result = new ArrayList<>(); - String name; - while ((name = newlyInstrumented.poll()) != null) { - result.add(name); + public List drainNewClasses() { + List result = new ArrayList<>(); + NewClass entry; + while ((entry = newlyInstrumented.poll()) != null) { + result.add(entry); } return result; } diff --git a/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/ProbeMappingCache.java b/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/ProbeMappingCache.java index 79ad70a4a1e..11a92c9b269 100644 --- a/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/ProbeMappingCache.java +++ b/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/ProbeMappingCache.java @@ -1,18 +1,12 @@ package datadog.trace.codecoverage; import java.io.ByteArrayOutputStream; -import java.io.File; -import java.io.FileInputStream; import java.io.IOException; import java.io.InputStream; -import java.util.ArrayList; -import java.util.BitSet; import java.util.Collection; import java.util.HashMap; -import java.util.List; import java.util.Map; -import java.util.zip.ZipEntry; -import java.util.zip.ZipInputStream; +import java.util.function.LongFunction; import org.jacoco.core.data.ExecutionData; import org.jacoco.core.internal.data.CRC64; import org.slf4j.Logger; @@ -20,7 +14,8 @@ /** * Maintains a cache of {@link ClassProbeMapping} entries, keyed by CRC64 class ID. Builds entries - * lazily from classpath analysis when cache misses occur. + * lazily when cache misses occur by resolving class bytes through the defining ClassLoader recorded + * at transform time. */ final class ProbeMappingCache { @@ -34,181 +29,106 @@ ClassProbeMapping get(long classId) { } /** - * Populates cache entries for all classes in {@code missingClasses}. First attempts targeted - * lookup via the context classloader's {@code getResourceAsStream} (O(1) per class). Any classes - * that can't be resolved this way fall back to a full classpath scan. + * Populates cache entries for all classes in {@code missingClasses}. Resolves class bytes + * primarily via the defining ClassLoader recorded at transform time, falling back to the system + * and context classloaders. + * + *

Classes that cannot be resolved are not cached — they will be retried on subsequent + * collection cycles (the defining classloader may become reachable later, e.g. after lazy module + * initialization). * * @param missingClasses execution data entries that have no cached mapping - * @param classpathEntries jars/directories to scan as fallback + * @param classLoaderLookup function that returns the recorded defining ClassLoader for a classId */ - void buildMissing(Collection missingClasses, List classpathEntries) { - // Build a lookup: classId -> ExecutionData for the missing entries - Map needed = new HashMap<>(); + void buildMissing( + Collection missingClasses, LongFunction classLoaderLookup) { for (ExecutionData ed : missingClasses) { - needed.put(ed.getId(), ed); - } - - // Phase 1: targeted classloader lookup (fast path) - resolveViaClassloader(needed); - - // Phase 2: fall back to classpath scan for anything still unresolved - if (!needed.isEmpty()) { - resolveViaClasspathScan(needed, classpathEntries); - } - - // Any remaining entries couldn't be found anywhere. - // Mark them with a sentinel so we don't retry on subsequent cycles. - for (Map.Entry e : needed.entrySet()) { - cache.put( - e.getKey(), new ClassProbeMapping(e.getKey(), null, null, new BitSet(), new int[0][])); - log.debug( - "Class {} (id {}) not found on classpath; skipping", - e.getValue().getName(), - Long.toHexString(e.getKey())); - } - } - - /** - * Attempts to resolve missing classes via the context classloader's resource lookup. This is O(1) - * per class — the classloader already knows where each class file lives. CRC64 is verified after - * reading to ensure the bytes match what JaCoCo instrumented. - */ - private void resolveViaClassloader(Map needed) { - // Try the system classloader (application classpath) first, then the context classloader. - // The dd-code-coverage thread inherits the agent's context classloader, which typically - // can't find application classes. The system classloader is the standard app classloader. - ClassLoader systemCl = ClassLoader.getSystemClassLoader(); - ClassLoader contextCl = Thread.currentThread().getContextClassLoader(); - - // Iterate over a copy since we modify 'needed' during iteration - for (ExecutionData ed : new ArrayList<>(needed.values())) { - String resource = ed.getName() + ".class"; - InputStream is = findResource(resource, systemCl, contextCl); - if (is == null) { - continue; // not found via any classloader — will try classpath scan + byte[] bytes = resolveClassBytes(ed.getName(), ed.getId(), classLoaderLookup); + if (bytes == null) { + log.debug( + "Class {} (id {}) could not be resolved; will retry next cycle", + ed.getName(), + Long.toHexString(ed.getId())); + continue; } - try (InputStream stream = is) { - byte[] bytes = readAllBytes(stream); - long crc = CRC64.classId(bytes); - if (crc != ed.getId()) { - // CRC64 mismatch — classloader returned different bytes than what was instrumented. - // Fall through to classpath scan. - log.debug( - "CRC64 mismatch for {} via classloader (expected {}, got {}); will try classpath scan", - ed.getName(), - Long.toHexString(ed.getId()), - Long.toHexString(crc)); - continue; - } + try { ClassProbeMapping mapping = ClassProbeMappingBuilder.build(ed.getId(), ed.getName(), ed.getProbes().length, bytes); cache.put(ed.getId(), mapping); - needed.remove(ed.getId()); } catch (Exception e) { - log.debug("Failed to resolve class {} via classloader", ed.getName(), e); + log.debug("Failed to build probe mapping for class {}", ed.getName(), e); } } } /** - * Tries to find a class resource using the given classloaders, returning the first non-null - * InputStream. Returns null if no classloader can find the resource. + * Resolves the original class bytes for a given class. Tries the following sources in order: + * + *

    + *
  1. The defining ClassLoader recorded at transform time (most reliable — works for custom + * classloaders, Spring Boot nested jars, OSGi, etc.) + *
  2. The system classloader (standard application classpath) + *
  3. The context classloader of the current thread + *
+ * + *

CRC64 is verified to ensure the returned bytes match the version that was instrumented. + * + * @return the class bytes, or null if the class could not be resolved from any source */ - private static InputStream findResource(String resource, ClassLoader... classLoaders) { - for (ClassLoader cl : classLoaders) { - if (cl == null) { - continue; - } - InputStream is = cl.getResourceAsStream(resource); - if (is != null) { - return is; - } + static byte[] resolveClassBytes( + String className, long expectedClassId, LongFunction classLoaderLookup) { + String resource = className + ".class"; + + // 1. Try the defining classloader recorded at transform time + ClassLoader definingLoader = classLoaderLookup.apply(expectedClassId); + byte[] bytes = tryLoadResource(resource, expectedClassId, definingLoader); + if (bytes != null) { + return bytes; } - return null; - } - /** - * Falls back to scanning classpath jars/directories for classes that couldn't be resolved via the - * classloader. - */ - private void resolveViaClasspathScan( - Map needed, List classpathEntries) { - for (File entry : classpathEntries) { - if (needed.isEmpty()) { - break; - } - if (!entry.exists()) { - continue; - } - - try { - if (entry.isDirectory()) { - scanDirectory(entry, needed); - } else if (entry.getName().endsWith(".jar") || entry.getName().endsWith(".zip")) { - scanJar(entry, needed); - } - } catch (IOException e) { - log.debug("Failed to scan classpath entry for cache building: {}", entry, e); - } + // 2. Try system classloader + bytes = tryLoadResource(resource, expectedClassId, ClassLoader.getSystemClassLoader()); + if (bytes != null) { + return bytes; } - } - private void scanDirectory(File dir, Map needed) throws IOException { - File[] files = dir.listFiles(); - if (files == null) { - return; - } - for (File f : files) { - if (needed.isEmpty()) { - return; - } - if (f.isDirectory()) { - scanDirectory(f, needed); - } else if (f.getName().endsWith(".class")) { - // Use try-with-resources to avoid leaking the FileInputStream - try (FileInputStream fis = new FileInputStream(f)) { - byte[] bytes = readAllBytes(fis); - tryBuildMapping(bytes, needed); - } - } - } + // 3. Try context classloader + bytes = + tryLoadResource(resource, expectedClassId, Thread.currentThread().getContextClassLoader()); + return bytes; } - private void scanJar(File jarFile, Map needed) throws IOException { - try (ZipInputStream zis = new ZipInputStream(new FileInputStream(jarFile))) { - ZipEntry entry; - while ((entry = zis.getNextEntry()) != null && !needed.isEmpty()) { - if (entry.getName().endsWith(".class") && !entry.isDirectory()) { - // Do NOT close the ZipInputStream here -- readAllBytes reads without closing - byte[] bytes = readAllBytes(zis); - tryBuildMapping(bytes, needed); - } - } + /** + * Attempts to load class bytes from the given classloader and verifies the CRC64 matches. Returns + * null if the classloader is null, the resource is not found, or the CRC64 doesn't match. + */ + private static byte[] tryLoadResource(String resource, long expectedClassId, ClassLoader loader) { + if (loader == null) { + return null; } - } - - private void tryBuildMapping(byte[] classBytes, Map needed) { - long crc = CRC64.classId(classBytes); - ExecutionData ed = needed.get(crc); - if (ed == null) { - return; // this class isn't one we're looking for + InputStream is = loader.getResourceAsStream(resource); + if (is == null) { + return null; } - - try { - ClassProbeMapping mapping = - ClassProbeMappingBuilder.build( - ed.getId(), ed.getName(), ed.getProbes().length, classBytes); - cache.put(ed.getId(), mapping); - needed.remove(crc); + try (InputStream stream = is) { + byte[] bytes = readAllBytes(stream); + long crc = CRC64.classId(bytes); + if (crc != expectedClassId) { + log.debug( + "CRC64 mismatch for {} via {} (expected {}, got {})", + resource, + loader.getClass().getName(), + Long.toHexString(expectedClassId), + Long.toHexString(crc)); + return null; + } + return bytes; } catch (Exception e) { - log.debug("Failed to build probe mapping for class {}", ed.getName(), e); + log.debug("Failed to read {} from {}", resource, loader.getClass().getName(), e); + return null; } } - /** - * Reads all bytes from an input stream WITHOUT closing it (important for ZipInputStream where - * closing the stream would close the zip). - */ private static byte[] readAllBytes(InputStream is) throws IOException { ByteArrayOutputStream out = new ByteArrayOutputStream(); byte[] buf = new byte[4096]; diff --git a/dd-java-agent/agent-code-coverage/src/test/java/datadog/trace/codecoverage/CodeCoverageCollectorTest.java b/dd-java-agent/agent-code-coverage/src/test/java/datadog/trace/codecoverage/CodeCoverageCollectorTest.java index f087ae508e7..c6281513ec7 100644 --- a/dd-java-agent/agent-code-coverage/src/test/java/datadog/trace/codecoverage/CodeCoverageCollectorTest.java +++ b/dd-java-agent/agent-code-coverage/src/test/java/datadog/trace/codecoverage/CodeCoverageCollectorTest.java @@ -67,9 +67,9 @@ void classWithHits_producesNormalCoverageRecord() { new CodeCoverageCollector( (store, session) -> store.put(new ExecutionData(classId, className, probes)), Collections::emptyList, + id -> null, uploads::add, - 60, - null); + 60); collector.collect(); @@ -84,17 +84,15 @@ void classWithHits_producesNormalCoverageRecord() { @Test void newClassWithoutHits_reportsExecutableLinesOnly() { - String className = SampleClassB.class.getName().replace('.', '/'); - List> uploads = new ArrayList<>(); CodeCoverageCollector collector = new CodeCoverageCollector( (store, session) -> {}, - () -> Collections.singletonList(className), + () -> Collections.singletonList(newClass(SampleClassB.class)), + id -> null, uploads::add, - 60, - null); + 60); collector.collect(); @@ -122,10 +120,10 @@ void newClassWithHits_notDuplicated() { CodeCoverageCollector collector = new CodeCoverageCollector( (store, session) -> store.put(new ExecutionData(classId, className, probes)), - () -> Collections.singletonList(className), + () -> Collections.singletonList(newClass(SampleClassA.class)), + id -> null, uploads::add, - 60, - null); + 60); collector.collect(); @@ -140,7 +138,7 @@ void noHitsNoDrain_nothingUploaded() { CodeCoverageCollector collector = new CodeCoverageCollector( - (store, session) -> {}, Collections::emptyList, uploads::add, 60, null); + (store, session) -> {}, Collections::emptyList, id -> null, uploads::add, 60); collector.collect(); @@ -149,8 +147,8 @@ void noHitsNoDrain_nothingUploaded() { @Test void drainedClassNotReReported() { - String className = SampleClassB.class.getName().replace('.', '/'); AtomicInteger drainCallCount = new AtomicInteger(0); + CodeCoverageTransformer.NewClass nc = newClass(SampleClassB.class); List> uploads = new ArrayList<>(); @@ -159,13 +157,13 @@ void drainedClassNotReReported() { (store, session) -> {}, () -> { if (drainCallCount.getAndIncrement() == 0) { - return Collections.singletonList(className); + return Collections.singletonList(nc); } return Collections.emptyList(); }, + id -> null, uploads::add, - 60, - null); + 60); // First cycle: should report the class collector.collect(); @@ -183,7 +181,7 @@ void mixedHitAndNoHitClasses() { String hitClassName = SampleClassA.class.getName().replace('.', '/'); int hitProbeCount = countProbes(hitClassBytes); - String noHitClassName = SampleClassB.class.getName().replace('.', '/'); + CodeCoverageTransformer.NewClass noHitNc = newClass(SampleClassB.class); boolean[] probes = new boolean[hitProbeCount]; Arrays.fill(probes, true); @@ -193,10 +191,10 @@ void mixedHitAndNoHitClasses() { CodeCoverageCollector collector = new CodeCoverageCollector( (store, session) -> store.put(new ExecutionData(hitClassId, hitClassName, probes)), - () -> Arrays.asList(hitClassName, noHitClassName), + () -> Arrays.asList(newClass(SampleClassA.class), noHitNc), + id -> null, uploads::add, - 60, - null); + 60); collector.collect(); @@ -209,7 +207,7 @@ void mixedHitAndNoHitClasses() { for (Map.Entry entry : coverage.entrySet()) { if (entry.getKey().className.equals(hitClassName)) { hitLc = entry.getValue(); - } else if (entry.getKey().className.equals(noHitClassName)) { + } else if (entry.getKey().className.equals(noHitNc.className)) { noHitLc = entry.getValue(); } } @@ -244,6 +242,12 @@ void newClassExecutableLines_matchFullBuild() { // ===== Helpers ===== + private static CodeCoverageTransformer.NewClass newClass(Class clazz) { + byte[] bytes = bytecodeFor(clazz); + return new CodeCoverageTransformer.NewClass( + CRC64.classId(bytes), clazz.getName().replace('.', '/')); + } + private static byte[] bytecodeFor(Class clazz) { String resource = clazz.getName().replace('.', '/') + ".class"; try (InputStream is = clazz.getClassLoader().getResourceAsStream(resource)) { From 88fb7afa35e0468b56ba064d5def955ac80498a3 Mon Sep 17 00:00:00 2001 From: Nikita Tkachenko Date: Thu, 26 Mar 2026 19:00:43 +0100 Subject: [PATCH 18/19] Remove dead dd.code.coverage.classpath configuration The config key was parsed and exposed via getter but never read by the production coverage module, so the intended classpath fallback did not exist. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../java/datadog/trace/api/config/CodeCoverageConfig.java | 1 - internal-api/src/main/java/datadog/trace/api/Config.java | 7 ------- 2 files changed, 8 deletions(-) diff --git a/dd-trace-api/src/main/java/datadog/trace/api/config/CodeCoverageConfig.java b/dd-trace-api/src/main/java/datadog/trace/api/config/CodeCoverageConfig.java index cee50521510..0b0be402e3c 100644 --- a/dd-trace-api/src/main/java/datadog/trace/api/config/CodeCoverageConfig.java +++ b/dd-trace-api/src/main/java/datadog/trace/api/config/CodeCoverageConfig.java @@ -8,7 +8,6 @@ public final class CodeCoverageConfig { public static final String CODE_COVERAGE_EXCLUDES = "code.coverage.excludes"; public static final String CODE_COVERAGE_REPORT_INTERVAL_SECONDS = "code.coverage.report.interval.seconds"; - public static final String CODE_COVERAGE_CLASSPATH = "code.coverage.classpath"; private CodeCoverageConfig() {} } diff --git a/internal-api/src/main/java/datadog/trace/api/Config.java b/internal-api/src/main/java/datadog/trace/api/Config.java index 6375241f53f..4b552469a72 100644 --- a/internal-api/src/main/java/datadog/trace/api/Config.java +++ b/internal-api/src/main/java/datadog/trace/api/Config.java @@ -294,7 +294,6 @@ import static datadog.trace.api.config.CiVisibilityConfig.TEST_MANAGEMENT_ATTEMPT_TO_FIX_RETRIES; import static datadog.trace.api.config.CiVisibilityConfig.TEST_MANAGEMENT_ENABLED; import static datadog.trace.api.config.CiVisibilityConfig.TEST_SESSION_NAME; -import static datadog.trace.api.config.CodeCoverageConfig.CODE_COVERAGE_CLASSPATH; import static datadog.trace.api.config.CodeCoverageConfig.CODE_COVERAGE_EXCLUDES; import static datadog.trace.api.config.CodeCoverageConfig.CODE_COVERAGE_INCLUDES; import static datadog.trace.api.config.CodeCoverageConfig.CODE_COVERAGE_REPORT_INTERVAL_SECONDS; @@ -1255,7 +1254,6 @@ public static String getHostName() { private final String[] codeCoverageIncludes; private final String[] codeCoverageExcludes; private final int codeCoverageReportIntervalSeconds; - private final String codeCoverageClasspath; private final boolean dataJobsOpenLineageEnabled; private final boolean dataJobsOpenLineageTimeoutEnabled; @@ -2837,7 +2835,6 @@ PROFILING_DATADOG_PROFILER_ENABLED, isDatadogProfilerSafeInCurrentEnvironment()) } codeCoverageReportIntervalSeconds = configProvider.getInteger(CODE_COVERAGE_REPORT_INTERVAL_SECONDS, 900); - codeCoverageClasspath = configProvider.getString(CODE_COVERAGE_CLASSPATH); dataJobsOpenLineageEnabled = configProvider.getBoolean( @@ -4712,10 +4709,6 @@ public int getCodeCoverageReportIntervalSeconds() { return codeCoverageReportIntervalSeconds; } - public String getCodeCoverageClasspath() { - return codeCoverageClasspath; - } - public int getCwsTlsRefresh() { return cwsTlsRefresh; } From 3b91f886212531092ab7c8186f3e5945d03b34bb Mon Sep 17 00:00:00 2001 From: Nikita Tkachenko Date: Thu, 26 Mar 2026 19:29:33 +0100 Subject: [PATCH 19/19] Avoid round-trip when collecting code coverage --- .../codecoverage/CodeCoverageTransformer.java | 36 +++++++--------- .../CodeCoverageTransformerTest.java | 41 +++++++++++++++++++ 2 files changed, 55 insertions(+), 22 deletions(-) create mode 100644 dd-java-agent/agent-code-coverage/src/test/java/datadog/trace/codecoverage/CodeCoverageTransformerTest.java diff --git a/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageTransformer.java b/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageTransformer.java index 1236202fc2f..9bf4d6a7aab 100644 --- a/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageTransformer.java +++ b/dd-java-agent/agent-code-coverage/src/main/java/datadog/trace/codecoverage/CodeCoverageTransformer.java @@ -16,9 +16,9 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentLinkedQueue; import java.util.function.Predicate; -import org.jacoco.core.data.ExecutionDataReader; +import org.jacoco.core.data.ExecutionData; import org.jacoco.core.data.ExecutionDataStore; -import org.jacoco.core.data.ExecutionDataWriter; +import org.jacoco.core.data.IExecutionDataVisitor; import org.jacoco.core.data.SessionInfoStore; import org.jacoco.core.instr.Instrumenter; import org.jacoco.core.runtime.IRuntime; @@ -230,31 +230,23 @@ public List drainNewClasses() { /** * Collects current probe data and resets all probes to {@code false}. * - *

Uses a serialize/deserialize round-trip to capture probe values before reset. This is - * necessary because {@code RuntimeData.collect()} passes references to the live {@code boolean[]} - * probe arrays to the visitor. If we passed an {@code ExecutionDataStore} directly, it would - * store references to the same arrays that {@code reset()} then zeroes out — destroying the - * collected data. The byte-stream approach (same as JaCoCo's own {@code - * Agent.getExecutionData()}) captures probe values into the stream before the reset runs. + *

{@code RuntimeData.collect()} passes references to live {@code boolean[]} probe arrays to + * the visitor and then resets them when requested. To keep the snapshot intact, clone the arrays + * before forwarding them to the target store. This preserves the existing JaCoCo + * serialize/deserialize semantics without the extra byte buffer round-trip. * * @param target store to receive the execution data * @param sessionTarget store to receive session info */ public void collectAndReset(ExecutionDataStore target, SessionInfoStore sessionTarget) { - try { - // Serialize probe data to bytes (captures values before reset) - ByteArrayOutputStream buffer = new ByteArrayOutputStream(); - ExecutionDataWriter writer = new ExecutionDataWriter(buffer); - runtimeData.collect(writer, writer, true); + runtimeData.collect(snapshotExecutionData(target), sessionTarget, true); + } - // Deserialize into the target stores - ExecutionDataReader reader = - new ExecutionDataReader(new java.io.ByteArrayInputStream(buffer.toByteArray())); - reader.setExecutionDataVisitor(target); - reader.setSessionInfoVisitor(sessionTarget); - reader.read(); - } catch (IOException e) { - throw new RuntimeException("Failed to collect coverage data", e); - } + static IExecutionDataVisitor snapshotExecutionData(ExecutionDataStore target) { + return data -> { + if (data.hasHits()) { + target.put(new ExecutionData(data.getId(), data.getName(), data.getProbes().clone())); + } + }; } } diff --git a/dd-java-agent/agent-code-coverage/src/test/java/datadog/trace/codecoverage/CodeCoverageTransformerTest.java b/dd-java-agent/agent-code-coverage/src/test/java/datadog/trace/codecoverage/CodeCoverageTransformerTest.java new file mode 100644 index 00000000000..2e84def82c5 --- /dev/null +++ b/dd-java-agent/agent-code-coverage/src/test/java/datadog/trace/codecoverage/CodeCoverageTransformerTest.java @@ -0,0 +1,41 @@ +package datadog.trace.codecoverage; + +import static org.junit.jupiter.api.Assertions.assertArrayEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNotSame; +import static org.junit.jupiter.api.Assertions.assertNull; + +import org.jacoco.core.data.ExecutionData; +import org.jacoco.core.data.ExecutionDataStore; +import org.junit.jupiter.api.Test; + +class CodeCoverageTransformerTest { + + @Test + void snapshotExecutionData_clonesHitProbeArrays() { + ExecutionDataStore target = new ExecutionDataStore(); + boolean[] probes = new boolean[] {true, false, true}; + + CodeCoverageTransformer.snapshotExecutionData(target) + .visitClassExecution(new ExecutionData(123L, "example/Foo", probes)); + + // Emulate the reset that RuntimeData.collect(..., true) performs on the live array. + probes[0] = false; + probes[2] = false; + + ExecutionData snapshot = target.get(123L); + assertNotNull(snapshot); + assertNotSame(probes, snapshot.getProbes()); + assertArrayEquals(new boolean[] {true, false, true}, snapshot.getProbes()); + } + + @Test + void snapshotExecutionData_skipsClassesWithoutHits() { + ExecutionDataStore target = new ExecutionDataStore(); + + CodeCoverageTransformer.snapshotExecutionData(target) + .visitClassExecution(new ExecutionData(123L, "example/Foo", new boolean[3])); + + assertNull(target.get(123L)); + } +}