From 372bb5978e190ef86740d942cd05f55e5f99fe9c Mon Sep 17 00:00:00 2001 From: DJ Gregor Date: Sat, 15 Nov 2025 16:37:15 -0800 Subject: [PATCH 01/14] ExceptionHandlers: Fix code in bytecode comment --- .../tooling/bytebuddy/ExceptionHandlers.java | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-) diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/ExceptionHandlers.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/ExceptionHandlers.java index 20099d76a37..ddf9187fd8a 100644 --- a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/ExceptionHandlers.java +++ b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/ExceptionHandlers.java @@ -39,24 +39,14 @@ public Size apply(final MethodVisitor mv, final Implementation.Context context) final boolean exitOnFailure = InstrumenterConfig.get().isInternalExitOnFailure(); final String logMethod = exitOnFailure ? "error" : "debug"; - // Writes the following bytecode if exitOnFailure is false: + // Writes the following bytecode (note that two statements are conditionally written): // - // BlockingExceptionHandler.rethrowIfBlockingException(t); - // try { - // InstrumentationErrors.incrementErrorCount(); - // org.slf4j.LoggerFactory.getLogger((Class)ExceptionLogger.class) - // .debug("Failed to handle exception in instrumentation for ...", t); - // } catch (Throwable t2) { - // } - // - // And the following bytecode if exitOnFailure is true: - // - // BlockingExceptionHandler.rethrowIfBlockingException(t); + // BlockingExceptionHandler.rethrowIfBlockingException(t); // when appSecEnabled=true // try { // InstrumentationErrors.incrementErrorCount(); // org.slf4j.LoggerFactory.getLogger((Class)ExceptionLogger.class) // .error("Failed to handle exception in instrumentation for ...", t); - // System.exit(1); + // System.exit(1); // when exitOnFailure=true // } catch (Throwable t2) { // } // From 7dfea0755fdc99356801ac2fee2957d8e4ad1686 Mon Sep 17 00:00:00 2001 From: DJ Gregor Date: Sat, 15 Nov 2025 16:50:38 -0800 Subject: [PATCH 02/14] InstrumentationContext: be more explicit about the reasons for failure --- .../trace/bootstrap/InstrumentationContext.java | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/InstrumentationContext.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/InstrumentationContext.java index 5a7dda111d2..5768cff4d3f 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/InstrumentationContext.java +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/InstrumentationContext.java @@ -24,7 +24,10 @@ private InstrumentationContext() {} public static ContextStore get( final Class keyClass, final Class contextClass) { throw new RuntimeException( - "Calls to this method will be rewritten by Instrumentation Context Provider (e.g. FieldBackedProvider)"); + "Calls to this method will be rewritten by Instrumentation Context Provider (e.g. FieldBackedProvider)." + + " If you get this exception, this method has not been rewritten." + + " Ensure instrumentation class has a contextStore method and the call to InstrumentationContext.get happens directly in an instrumentation Advice class." + + " See how_instrumentations_work.md for details."); } /** @@ -35,6 +38,9 @@ public static ContextStore get( */ public static ContextStore get(final String keyClass, final String contextClass) { throw new RuntimeException( - "Calls to this method will be rewritten by Instrumentation Context Provider (e.g. FieldBackedProvider)"); + "Calls to this method will be rewritten by Instrumentation Context Provider (e.g. FieldBackedProvider)." + + " If you get this exception, this method has not been rewritten." + + " Ensure instrumentation class has a contextStore method and the call to InstrumentationContext.get happens directly in an instrumentation Advice class." + + " See how_instrumentations_work.md for details."); } } From 2ab845d242668f0046fc2f2e98d76fe5e7d03b20 Mon Sep 17 00:00:00 2001 From: DJ Gregor Date: Sat, 15 Nov 2025 17:00:11 -0800 Subject: [PATCH 03/14] Add instrumentation error details on test failures This puts the details front and center in the test summary and is much easier than hunting back through log messages. Note: In InstrumentationErrors, errors are only recorded when enableRecordingAndReset has been called. This is only done from the InstrumentationSpecification test specification, so we don't need to worry about the ArrayList growing without bounds in production. I considered adding a limit to the ArrayList, but opted not to for simplicity. I also wanted to avoid silently discarding some errors. --- .../bootstrap/InstrumentationErrors.java | 33 ++++++++++++++----- .../tooling/bytebuddy/ExceptionHandlers.java | 10 +++--- .../test/InstrumentationSpecification.groovy | 17 ++++++---- 3 files changed, 40 insertions(+), 20 deletions(-) diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/InstrumentationErrors.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/InstrumentationErrors.java index 580b1557f3c..382af51c62d 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/InstrumentationErrors.java +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/InstrumentationErrors.java @@ -1,21 +1,36 @@ package datadog.trace.bootstrap; -import java.util.concurrent.atomic.AtomicLong; +import java.io.PrintWriter; +import java.io.StringWriter; +import java.util.Collections; +import java.util.List; +import java.util.concurrent.CopyOnWriteArrayList; public class InstrumentationErrors { - private static final AtomicLong COUNTER = new AtomicLong(); + private static final List ERRORS = new CopyOnWriteArrayList<>(); + private static volatile boolean recordErrors = false; - public static long getErrorCount() { - return COUNTER.get(); + /** + * Record an error from {@link datadog.trace.agent.tooling.bytebuddy.ExceptionHandlers} for test + * visibility. + */ + @SuppressWarnings("unused") + public static void recordError(final Throwable throwable) { + if (recordErrors) { + StringWriter stackTrace = new StringWriter(); + throwable.printStackTrace(new PrintWriter(stackTrace)); + ERRORS.add(stackTrace.toString()); + } } - @SuppressWarnings("unused") - public static void incrementErrorCount() { - COUNTER.incrementAndGet(); + // Visible for testing + public static void enableRecordingAndReset() { + recordErrors = true; + ERRORS.clear(); } // Visible for testing - public static void resetErrorCount() { - COUNTER.set(0); + public static List getErrors() { + return Collections.unmodifiableList(ERRORS); } } diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/ExceptionHandlers.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/ExceptionHandlers.java index ddf9187fd8a..263136898fa 100644 --- a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/ExceptionHandlers.java +++ b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/ExceptionHandlers.java @@ -43,7 +43,7 @@ public Size apply(final MethodVisitor mv, final Implementation.Context context) // // BlockingExceptionHandler.rethrowIfBlockingException(t); // when appSecEnabled=true // try { - // InstrumentationErrors.incrementErrorCount(); + // InstrumentationErrors.recordError(t); // org.slf4j.LoggerFactory.getLogger((Class)ExceptionLogger.class) // .error("Failed to handle exception in instrumentation for ...", t); // System.exit(1); // when exitOnFailure=true @@ -71,12 +71,14 @@ public Size apply(final MethodVisitor mv, final Implementation.Context context) mv.visitTryCatchBlock(logStart, logEnd, eatException, "java/lang/Throwable"); mv.visitLabel(logStart); - // invoke incrementAndGet on our exception counter + mv.visitInsn(Opcodes.DUP); // stack: (top) throwable,throwable + // invoke recordError on our exception tracker mv.visitMethodInsn( Opcodes.INVOKESTATIC, "datadog/trace/bootstrap/InstrumentationErrors", - "incrementErrorCount", - "()V"); + "recordError", + "(Ljava/lang/Throwable;)V"); + // stack: (top) throwable mv.visitLdcInsn(Type.getType("L" + HANDLER_NAME + ";")); mv.visitMethodInsn( diff --git a/dd-java-agent/instrumentation-testing/src/main/groovy/datadog/trace/agent/test/InstrumentationSpecification.groovy b/dd-java-agent/instrumentation-testing/src/main/groovy/datadog/trace/agent/test/InstrumentationSpecification.groovy index 9b376a55be8..332b133f273 100644 --- a/dd-java-agent/instrumentation-testing/src/main/groovy/datadog/trace/agent/test/InstrumentationSpecification.groovy +++ b/dd-java-agent/instrumentation-testing/src/main/groovy/datadog/trace/agent/test/InstrumentationSpecification.groovy @@ -10,7 +10,6 @@ import static datadog.trace.api.config.TraceInstrumentationConfig.CODE_ORIGIN_FO import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.closePrevious import static datadog.trace.util.AgentThreadFactory.AgentThread.TASK_SCHEDULER - import ch.qos.logback.classic.Level import ch.qos.logback.classic.util.ContextInitializer import com.datadog.debugger.agent.ClassesToRetransformFinder @@ -76,9 +75,9 @@ import java.lang.instrument.ClassFileTransformer import java.lang.instrument.Instrumentation import java.nio.ByteBuffer import java.util.concurrent.ConcurrentHashMap +import java.util.concurrent.CopyOnWriteArrayList import java.util.concurrent.TimeUnit import java.util.concurrent.TimeoutException -import java.util.concurrent.atomic.AtomicInteger import net.bytebuddy.agent.ByteBuddyAgent import net.bytebuddy.agent.builder.AgentBuilder import net.bytebuddy.description.type.TypeDescription @@ -170,7 +169,7 @@ abstract class InstrumentationSpecification extends DDSpecification implements A @SuppressWarnings('PropertyName') @Shared - AtomicInteger INSTRUMENTATION_ERROR_COUNT = new AtomicInteger(0) + List INSTRUMENTATION_ERRORS = new CopyOnWriteArrayList() // don't use mocks because it will break too many exhaustive interaction-verifying tests @SuppressWarnings('PropertyName') @@ -478,7 +477,7 @@ abstract class InstrumentationSpecification extends DDSpecification implements A if (forceAppSecActive) { ActiveSubsystems.APPSEC_ACTIVE = true } - InstrumentationErrors.resetErrorCount() + InstrumentationErrors.enableRecordingAndReset() ProcessTags.reset() } @@ -520,7 +519,8 @@ abstract class InstrumentationSpecification extends DDSpecification implements A spanFinishLocations.clear() originalToTrackingSpan.clear() } - assert InstrumentationErrors.errorCount == 0 + def instrumentationErrorCount = InstrumentationErrors.getErrors().size() + assert instrumentationErrorCount == 0, "${instrumentationErrorCount} instrumentation errors were seen:\n${InstrumentationErrors.getErrors().join("\n---\n")}" } private void doCheckRepeatedFinish() { @@ -562,7 +562,8 @@ abstract class InstrumentationSpecification extends DDSpecification implements A cleanupAfterAgent() // All cleanup should happen before these assertion. If not, a failing assertion may prevent cleanup - assert INSTRUMENTATION_ERROR_COUNT.get() == 0: INSTRUMENTATION_ERROR_COUNT.get() + " Instrumentation errors during test" + def instrumentationErrors = INSTRUMENTATION_ERRORS.size() + assert instrumentationErrors == 0: "${instrumentationErrors} Instrumentation errors during test:\n${INSTRUMENTATION_ERRORS.join("\n---\n")}" assert TRANSFORMED_CLASSES_TYPES.findAll { GlobalIgnores.isAdditionallyIgnored(it.getActualName()) @@ -656,7 +657,9 @@ abstract class InstrumentationSpecification extends DDSpecification implements A println "Unexpected instrumentation error when instrumenting ${typeName} on ${classLoader}" throwable.printStackTrace() - INSTRUMENTATION_ERROR_COUNT.incrementAndGet() + def stackTrace = new StringWriter() + throwable.printStackTrace(new PrintWriter(stackTrace)) + INSTRUMENTATION_ERRORS.add(stackTrace.toString()) } @Override From 4f536b7592d8cd94546a345d2928f2060d0023a8 Mon Sep 17 00:00:00 2001 From: DJ Gregor Date: Sat, 15 Nov 2025 17:30:30 -0800 Subject: [PATCH 04/14] Check for blocked instrumentation in tests If there is a blocked instrumentation, the test will fail and the failure message will include the same information that is logged from MuzzleCheck. --- .../agent/tooling/muzzle/MuzzleCheck.java | 45 ++++++++++++++++++- .../test/InstrumentationSpecification.groovy | 4 ++ 2 files changed, 48 insertions(+), 1 deletion(-) diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/muzzle/MuzzleCheck.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/muzzle/MuzzleCheck.java index 2544e2732c8..fa408768870 100644 --- a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/muzzle/MuzzleCheck.java +++ b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/muzzle/MuzzleCheck.java @@ -3,7 +3,9 @@ import datadog.trace.agent.tooling.InstrumenterModule; import datadog.trace.agent.tooling.InstrumenterState; import datadog.trace.agent.tooling.Utils; +import java.util.Collections; import java.util.List; +import java.util.concurrent.CopyOnWriteArrayList; import net.bytebuddy.matcher.ElementMatcher; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -11,6 +13,9 @@ public class MuzzleCheck implements ElementMatcher { private static final Logger log = LoggerFactory.getLogger(MuzzleCheck.class); + private static final List ERRORS = new CopyOnWriteArrayList<>(); + private static volatile boolean recordErrors = false; + private final int instrumentationId; private final String instrumentationClass; private final ReferenceProvider runtimeMuzzleReferences; @@ -33,9 +38,13 @@ public boolean matches(ClassLoader classLoader) { InstrumenterState.applyInstrumentation(classLoader, instrumentationId); } else { InstrumenterState.blockInstrumentation(classLoader, instrumentationId); - if (log.isDebugEnabled()) { + if (recordErrors || log.isDebugEnabled()) { final List mismatches = muzzle.getMismatchedReferenceSources(classLoader); + if (recordErrors) { + recordMuzzleMismatch( + InstrumenterState.describe(instrumentationId), classLoader, mismatches); + } log.debug( "Muzzled - {} instrumentation.target.classloader={}", InstrumenterState.describe(instrumentationId), @@ -61,4 +70,38 @@ private ReferenceMatcher muzzle() { } return muzzle; } + + /** + * Record a muzzle mismatch error for test visibility. + * + * @param instrumentationDescription the description of the instrumentation that was blocked + * @param classLoader the classloader where the instrumentation was blocked + * @param mismatches the list of mismatch details + */ + private static void recordMuzzleMismatch( + String instrumentationDescription, + ClassLoader classLoader, + List mismatches) { + StringBuilder sb = new StringBuilder(); + sb.append("Muzzled - ") + .append(instrumentationDescription) + .append(" instrumentation.target.classloader=") + .append(classLoader) + .append("\n"); + for (Reference.Mismatch mismatch : mismatches) { + sb.append(" Mismatch: ").append(mismatch).append("\n"); + } + ERRORS.add(sb.toString()); + } + + // Visible for testing + public static void enableRecordingAndReset() { + recordErrors = true; + ERRORS.clear(); + } + + // Visible for testing + public static List getErrors() { + return Collections.unmodifiableList(ERRORS); + } } diff --git a/dd-java-agent/instrumentation-testing/src/main/groovy/datadog/trace/agent/test/InstrumentationSpecification.groovy b/dd-java-agent/instrumentation-testing/src/main/groovy/datadog/trace/agent/test/InstrumentationSpecification.groovy index 332b133f273..5120fa27355 100644 --- a/dd-java-agent/instrumentation-testing/src/main/groovy/datadog/trace/agent/test/InstrumentationSpecification.groovy +++ b/dd-java-agent/instrumentation-testing/src/main/groovy/datadog/trace/agent/test/InstrumentationSpecification.groovy @@ -39,6 +39,7 @@ import datadog.trace.agent.tooling.InstrumenterModule import datadog.trace.agent.tooling.TracerInstaller import datadog.trace.agent.tooling.bytebuddy.matcher.ClassLoaderMatchers import datadog.trace.agent.tooling.bytebuddy.matcher.GlobalIgnores +import datadog.trace.agent.tooling.muzzle.MuzzleCheck import datadog.trace.api.Config import datadog.trace.api.IdGenerationStrategy import datadog.trace.api.Pair @@ -478,6 +479,7 @@ abstract class InstrumentationSpecification extends DDSpecification implements A ActiveSubsystems.APPSEC_ACTIVE = true } InstrumentationErrors.enableRecordingAndReset() + MuzzleCheck.enableRecordingAndReset() ProcessTags.reset() } @@ -521,6 +523,8 @@ abstract class InstrumentationSpecification extends DDSpecification implements A } def instrumentationErrorCount = InstrumentationErrors.getErrors().size() assert instrumentationErrorCount == 0, "${instrumentationErrorCount} instrumentation errors were seen:\n${InstrumentationErrors.getErrors().join("\n---\n")}" + def muzzleErrorCount = MuzzleCheck.getErrors().size() + assert muzzleErrorCount == 0, "${muzzleErrorCount} muzzle errors were seen:\n${MuzzleCheck.getErrors().join("\n---\n")}" } private void doCheckRepeatedFinish() { From ca324efa5a66a81d6c3eda99c82f9e63e5da3b08 Mon Sep 17 00:00:00 2001 From: DJ Gregor Date: Sat, 15 Nov 2025 17:54:12 -0800 Subject: [PATCH 05/14] Fail fast on muzzle or instrumentation errors --- .../agent/test/asserts/ListWriterAssert.groovy | 18 +++++++++++++++++- .../trace/common/writer/ListWriter.java | 9 ++++++++- 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/asserts/ListWriterAssert.groovy b/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/asserts/ListWriterAssert.groovy index 166eb92f6b6..18b336be32e 100644 --- a/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/asserts/ListWriterAssert.groovy +++ b/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/asserts/ListWriterAssert.groovy @@ -2,6 +2,8 @@ package datadog.trace.agent.test.asserts import static TraceAssert.assertTrace +import datadog.trace.agent.tooling.muzzle.MuzzleCheck +import datadog.trace.bootstrap.InstrumentationErrors import datadog.trace.common.writer.ListWriter import datadog.trace.core.DDSpan import groovy.transform.stc.ClosureParams @@ -47,7 +49,15 @@ class ListWriterAssert { @ClosureParams(value = SimpleType, options = ['datadog.trace.agent.test.asserts.ListWriterAssert']) @DelegatesTo(value = ListWriterAssert, strategy = Closure.DELEGATE_FIRST) Closure spec) { try { - writer.waitForTraces(expectedSize) + // Fail fast if we see any muzzle errors or instrumentation errors + for (int timeoutRemaining = ListWriter.WAIT_FOR_TRACES_TIMEOUT_SECONDS; timeoutRemaining > 0; timeoutRemaining--) { + if (writer.waitForTracesMax(expectedSize, 1)) { + break + } + assertNoErrors() + } + assertNoErrors() // even if we break immediately above, make sure we check for errors at least once + writer.waitForTraces(expectedSize, 0) def array = writer.toArray() assert array.length == expectedSize def traces = (Arrays.asList(array) as List>) @@ -91,6 +101,12 @@ class ListWriterAssert { } } + static void assertNoErrors() { + assert InstrumentationErrors.getErrors().size() == 0 : "Instrumentation errors were seen, failing fast" + assert MuzzleCheck.getErrors().size() == 0 : "Muzzle errors were seen, failing fast" + // InstrumentationSpecification.{cleanup,cleanupAfterAgent} will provide details later + } + void sortSpansByStart() { traces = traces.collect { return new ArrayList(it).sort { a, b -> diff --git a/dd-trace-core/src/main/java/datadog/trace/common/writer/ListWriter.java b/dd-trace-core/src/main/java/datadog/trace/common/writer/ListWriter.java index dec3a8eb9e5..7b2c9e72a94 100644 --- a/dd-trace-core/src/main/java/datadog/trace/common/writer/ListWriter.java +++ b/dd-trace-core/src/main/java/datadog/trace/common/writer/ListWriter.java @@ -18,6 +18,8 @@ /** List writer used by tests mostly */ public class ListWriter extends CopyOnWriteArrayList> implements Writer { + public static final int WAIT_FOR_TRACES_TIMEOUT_SECONDS = 20; + private static final Logger log = LoggerFactory.getLogger(ListWriter.class); private static final Filter ACCEPT_ALL = trace -> true; @@ -85,7 +87,12 @@ public boolean waitForTracesMax(final int number, int seconds) throws Interrupte } public void waitForTraces(final int number) throws InterruptedException, TimeoutException { - if (!waitForTracesMax(number, 20)) { + waitForTraces(number, WAIT_FOR_TRACES_TIMEOUT_SECONDS); + } + + public void waitForTraces(final int number, final int seconds) + throws InterruptedException, TimeoutException { + if (!waitForTracesMax(number, seconds)) { String msg = "Timeout waiting for " + number From 18644fc79f9884601e816a16f0302144fd8bd5ba Mon Sep 17 00:00:00 2001 From: Andrea Marziali Date: Fri, 24 Apr 2026 09:52:22 +0200 Subject: [PATCH 06/14] revert asserting muzzle failures in instrumentation tests --- .../agent/tooling/muzzle/MuzzleCheck.java | 45 +------------------ .../test/InstrumentationSpecification.groovy | 4 -- .../test/asserts/ListWriterAssert.groovy | 2 - 3 files changed, 1 insertion(+), 50 deletions(-) diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/muzzle/MuzzleCheck.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/muzzle/MuzzleCheck.java index fa408768870..2544e2732c8 100644 --- a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/muzzle/MuzzleCheck.java +++ b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/muzzle/MuzzleCheck.java @@ -3,9 +3,7 @@ import datadog.trace.agent.tooling.InstrumenterModule; import datadog.trace.agent.tooling.InstrumenterState; import datadog.trace.agent.tooling.Utils; -import java.util.Collections; import java.util.List; -import java.util.concurrent.CopyOnWriteArrayList; import net.bytebuddy.matcher.ElementMatcher; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -13,9 +11,6 @@ public class MuzzleCheck implements ElementMatcher { private static final Logger log = LoggerFactory.getLogger(MuzzleCheck.class); - private static final List ERRORS = new CopyOnWriteArrayList<>(); - private static volatile boolean recordErrors = false; - private final int instrumentationId; private final String instrumentationClass; private final ReferenceProvider runtimeMuzzleReferences; @@ -38,13 +33,9 @@ public boolean matches(ClassLoader classLoader) { InstrumenterState.applyInstrumentation(classLoader, instrumentationId); } else { InstrumenterState.blockInstrumentation(classLoader, instrumentationId); - if (recordErrors || log.isDebugEnabled()) { + if (log.isDebugEnabled()) { final List mismatches = muzzle.getMismatchedReferenceSources(classLoader); - if (recordErrors) { - recordMuzzleMismatch( - InstrumenterState.describe(instrumentationId), classLoader, mismatches); - } log.debug( "Muzzled - {} instrumentation.target.classloader={}", InstrumenterState.describe(instrumentationId), @@ -70,38 +61,4 @@ private ReferenceMatcher muzzle() { } return muzzle; } - - /** - * Record a muzzle mismatch error for test visibility. - * - * @param instrumentationDescription the description of the instrumentation that was blocked - * @param classLoader the classloader where the instrumentation was blocked - * @param mismatches the list of mismatch details - */ - private static void recordMuzzleMismatch( - String instrumentationDescription, - ClassLoader classLoader, - List mismatches) { - StringBuilder sb = new StringBuilder(); - sb.append("Muzzled - ") - .append(instrumentationDescription) - .append(" instrumentation.target.classloader=") - .append(classLoader) - .append("\n"); - for (Reference.Mismatch mismatch : mismatches) { - sb.append(" Mismatch: ").append(mismatch).append("\n"); - } - ERRORS.add(sb.toString()); - } - - // Visible for testing - public static void enableRecordingAndReset() { - recordErrors = true; - ERRORS.clear(); - } - - // Visible for testing - public static List getErrors() { - return Collections.unmodifiableList(ERRORS); - } } diff --git a/dd-java-agent/instrumentation-testing/src/main/groovy/datadog/trace/agent/test/InstrumentationSpecification.groovy b/dd-java-agent/instrumentation-testing/src/main/groovy/datadog/trace/agent/test/InstrumentationSpecification.groovy index 5120fa27355..332b133f273 100644 --- a/dd-java-agent/instrumentation-testing/src/main/groovy/datadog/trace/agent/test/InstrumentationSpecification.groovy +++ b/dd-java-agent/instrumentation-testing/src/main/groovy/datadog/trace/agent/test/InstrumentationSpecification.groovy @@ -39,7 +39,6 @@ import datadog.trace.agent.tooling.InstrumenterModule import datadog.trace.agent.tooling.TracerInstaller import datadog.trace.agent.tooling.bytebuddy.matcher.ClassLoaderMatchers import datadog.trace.agent.tooling.bytebuddy.matcher.GlobalIgnores -import datadog.trace.agent.tooling.muzzle.MuzzleCheck import datadog.trace.api.Config import datadog.trace.api.IdGenerationStrategy import datadog.trace.api.Pair @@ -479,7 +478,6 @@ abstract class InstrumentationSpecification extends DDSpecification implements A ActiveSubsystems.APPSEC_ACTIVE = true } InstrumentationErrors.enableRecordingAndReset() - MuzzleCheck.enableRecordingAndReset() ProcessTags.reset() } @@ -523,8 +521,6 @@ abstract class InstrumentationSpecification extends DDSpecification implements A } def instrumentationErrorCount = InstrumentationErrors.getErrors().size() assert instrumentationErrorCount == 0, "${instrumentationErrorCount} instrumentation errors were seen:\n${InstrumentationErrors.getErrors().join("\n---\n")}" - def muzzleErrorCount = MuzzleCheck.getErrors().size() - assert muzzleErrorCount == 0, "${muzzleErrorCount} muzzle errors were seen:\n${MuzzleCheck.getErrors().join("\n---\n")}" } private void doCheckRepeatedFinish() { diff --git a/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/asserts/ListWriterAssert.groovy b/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/asserts/ListWriterAssert.groovy index 18b336be32e..e67228d9d8e 100644 --- a/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/asserts/ListWriterAssert.groovy +++ b/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/asserts/ListWriterAssert.groovy @@ -2,7 +2,6 @@ package datadog.trace.agent.test.asserts import static TraceAssert.assertTrace -import datadog.trace.agent.tooling.muzzle.MuzzleCheck import datadog.trace.bootstrap.InstrumentationErrors import datadog.trace.common.writer.ListWriter import datadog.trace.core.DDSpan @@ -103,7 +102,6 @@ class ListWriterAssert { static void assertNoErrors() { assert InstrumentationErrors.getErrors().size() == 0 : "Instrumentation errors were seen, failing fast" - assert MuzzleCheck.getErrors().size() == 0 : "Muzzle errors were seen, failing fast" // InstrumentationSpecification.{cleanup,cleanupAfterAgent} will provide details later } From b406b8c6cd2deab9ec48fe153f5b777c006451ec Mon Sep 17 00:00:00 2001 From: Andrea Marziali Date: Wed, 29 Apr 2026 10:54:43 +0200 Subject: [PATCH 07/14] Avoid allocating a stack argument if errors are not recorded --- .../bootstrap/InstrumentationErrors.java | 13 ++++++----- .../tooling/bytebuddy/ExceptionHandlers.java | 22 +++++++++++++++---- 2 files changed, 26 insertions(+), 9 deletions(-) diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/InstrumentationErrors.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/InstrumentationErrors.java index 382af51c62d..5cf322ca970 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/InstrumentationErrors.java +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/InstrumentationErrors.java @@ -14,13 +14,16 @@ public class InstrumentationErrors { * Record an error from {@link datadog.trace.agent.tooling.bytebuddy.ExceptionHandlers} for test * visibility. */ + @SuppressWarnings("unused") + public static boolean isEnabled() { + return recordErrors; + } + @SuppressWarnings("unused") public static void recordError(final Throwable throwable) { - if (recordErrors) { - StringWriter stackTrace = new StringWriter(); - throwable.printStackTrace(new PrintWriter(stackTrace)); - ERRORS.add(stackTrace.toString()); - } + StringWriter stackTrace = new StringWriter(); + throwable.printStackTrace(new PrintWriter(stackTrace)); + ERRORS.add(stackTrace.toString()); } // Visible for testing diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/ExceptionHandlers.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/ExceptionHandlers.java index 263136898fa..e58c5247854 100644 --- a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/ExceptionHandlers.java +++ b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/ExceptionHandlers.java @@ -39,11 +39,12 @@ public Size apply(final MethodVisitor mv, final Implementation.Context context) final boolean exitOnFailure = InstrumenterConfig.get().isInternalExitOnFailure(); final String logMethod = exitOnFailure ? "error" : "debug"; - // Writes the following bytecode (note that two statements are conditionally written): + // Writes the following bytecode (note that some statements are conditionally + // written): // // BlockingExceptionHandler.rethrowIfBlockingException(t); // when appSecEnabled=true // try { - // InstrumentationErrors.recordError(t); + // if (InstrumentationErrors.isEnabled()) InstrumentationErrors.recordError(t); // org.slf4j.LoggerFactory.getLogger((Class)ExceptionLogger.class) // .error("Failed to handle exception in instrumentation for ...", t); // System.exit(1); // when exitOnFailure=true @@ -69,15 +70,28 @@ public Size apply(final MethodVisitor mv, final Implementation.Context context) "(Ljava/lang/Throwable;)Ljava/lang/Throwable;"); } + final Label skipRecord = new Label(); + mv.visitTryCatchBlock(logStart, logEnd, eatException, "java/lang/Throwable"); mv.visitLabel(logStart); + mv.visitMethodInsn( + Opcodes.INVOKESTATIC, + "datadog/trace/bootstrap/InstrumentationErrors", + "isEnabled", + "()Z", + false); + mv.visitJumpInsn(Opcodes.IFEQ, skipRecord); mv.visitInsn(Opcodes.DUP); // stack: (top) throwable,throwable - // invoke recordError on our exception tracker mv.visitMethodInsn( Opcodes.INVOKESTATIC, "datadog/trace/bootstrap/InstrumentationErrors", "recordError", - "(Ljava/lang/Throwable;)V"); + "(Ljava/lang/Throwable;)V", + false); + mv.visitLabel(skipRecord); + if (frames) { + mv.visitFrame(Opcodes.F_SAME1, 0, null, 1, new Object[] {"java/lang/Throwable"}); + } // stack: (top) throwable mv.visitLdcInsn(Type.getType("L" + HANDLER_NAME + ";")); From 32bb9fa191cae047f8d370d06d677e23f7de7996 Mon Sep 17 00:00:00 2001 From: Stuart McCulloch Date: Thu, 21 May 2026 11:04:35 +0100 Subject: [PATCH 08/14] Rework recording of detailed instrumentation errors * Can be toggled on/off with feature-flag (on for InstrumentationSpecification) * Zero-impact when the detailed instrumentation error feature is off * Allow recording of errors indirectly via InstrumentationSpecification * Reverted change to trace asserts (we shouldn't mix concerns in asserts) --- .../bootstrap/InstrumentationErrors.java | 64 +++++++++++++------ .../tooling/bytebuddy/ExceptionHandlers.java | 59 +++++++++-------- .../test/InstrumentationSpecification.groovy | 20 ++---- .../test/asserts/ListWriterAssert.groovy | 16 +---- .../config/TraceInstrumentationConfig.java | 1 + .../trace/common/writer/ListWriter.java | 9 +-- .../datadog/trace/api/InstrumenterConfig.java | 8 +++ metadata/supported-configurations.json | 8 +++ 8 files changed, 102 insertions(+), 83 deletions(-) diff --git a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/InstrumentationErrors.java b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/InstrumentationErrors.java index 5cf322ca970..c18534a73e9 100644 --- a/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/InstrumentationErrors.java +++ b/dd-java-agent/agent-bootstrap/src/main/java/datadog/trace/bootstrap/InstrumentationErrors.java @@ -2,38 +2,60 @@ import java.io.PrintWriter; import java.io.StringWriter; -import java.util.Collections; import java.util.List; import java.util.concurrent.CopyOnWriteArrayList; +import java.util.concurrent.atomic.AtomicLong; public class InstrumentationErrors { - private static final List ERRORS = new CopyOnWriteArrayList<>(); - private static volatile boolean recordErrors = false; + private static final AtomicLong COUNTER = new AtomicLong(); - /** - * Record an error from {@link datadog.trace.agent.tooling.bytebuddy.ExceptionHandlers} for test - * visibility. - */ - @SuppressWarnings("unused") - public static boolean isEnabled() { - return recordErrors; + private static final class Detailed { + static final List ERRORS = new CopyOnWriteArrayList<>(); + } + + private static volatile boolean detailed; + + /** Record an error occurred without any detail about it. */ + public static void recordError() { + COUNTER.incrementAndGet(); } - @SuppressWarnings("unused") - public static void recordError(final Throwable throwable) { - StringWriter stackTrace = new StringWriter(); - throwable.printStackTrace(new PrintWriter(stackTrace)); - ERRORS.add(stackTrace.toString()); + /** Record an error occurred, including its stack trace. */ + public static Throwable recordError(Throwable error) { + COUNTER.incrementAndGet(); + StringWriter detail = new StringWriter(); + error.printStackTrace(new PrintWriter(detail)); + Detailed.ERRORS.add(detail.toString()); + detailed = true; + return error; // keep throwable at top of the stack } // Visible for testing - public static void enableRecordingAndReset() { - recordErrors = true; - ERRORS.clear(); + public static void resetErrors() { + COUNTER.set(0); + if (detailed) { + Detailed.ERRORS.clear(); + detailed = false; + } } - // Visible for testing - public static List getErrors() { - return Collections.unmodifiableList(ERRORS); + /** + * @return {@code true} if no errors were recorded; otherwise {@code false} + */ + public static boolean noErrors() { + return COUNTER.get() == 0; + } + + /** + * @return a human-readable description of the errors recorded so far + */ + public static String describeErrors() { + StringBuilder buf = new StringBuilder().append(COUNTER.get()).append(" instrumentation errors"); + if (detailed) { + for (String error : Detailed.ERRORS) { + buf.append("\n---\n").append(error); + } + } + return buf.toString(); } } diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/ExceptionHandlers.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/ExceptionHandlers.java index e58c5247854..5b4f58ade00 100644 --- a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/ExceptionHandlers.java +++ b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/ExceptionHandlers.java @@ -27,6 +27,8 @@ public class ExceptionHandlers { private final Size size = new StackManipulation.Size(-1, 3); private final boolean appSecEnabled = InstrumenterConfig.get().getAppSecActivation() != ProductActivation.FULLY_DISABLED; + private final boolean detailedErrors = + InstrumenterConfig.get().isDetailedInstrumentationErrors(); @Override public boolean isValid() { @@ -39,15 +41,24 @@ public Size apply(final MethodVisitor mv, final Implementation.Context context) final boolean exitOnFailure = InstrumenterConfig.get().isInternalExitOnFailure(); final String logMethod = exitOnFailure ? "error" : "debug"; - // Writes the following bytecode (note that some statements are conditionally - // written): + // Writes the following bytecode if exitOnFailure is false: // - // BlockingExceptionHandler.rethrowIfBlockingException(t); // when appSecEnabled=true + // BlockingExceptionHandler.rethrowIfBlockingException(t); // try { - // if (InstrumentationErrors.isEnabled()) InstrumentationErrors.recordError(t); + // InstrumentationErrors.incrementErrorCount(); + // org.slf4j.LoggerFactory.getLogger((Class)ExceptionLogger.class) + // .debug("Failed to handle exception in instrumentation for ...", t); + // } catch (Throwable t2) { + // } + // + // And the following bytecode if exitOnFailure is true: + // + // BlockingExceptionHandler.rethrowIfBlockingException(t); + // try { + // InstrumentationErrors.incrementErrorCount(); // org.slf4j.LoggerFactory.getLogger((Class)ExceptionLogger.class) // .error("Failed to handle exception in instrumentation for ...", t); - // System.exit(1); // when exitOnFailure=true + // System.exit(1); // } catch (Throwable t2) { // } // @@ -67,32 +78,28 @@ public Size apply(final MethodVisitor mv, final Implementation.Context context) Opcodes.INVOKESTATIC, "datadog/trace/bootstrap/blocking/BlockingExceptionHandler", "rethrowIfBlockingException", - "(Ljava/lang/Throwable;)Ljava/lang/Throwable;"); + "(Ljava/lang/Throwable;)Ljava/lang/Throwable;", + false); } - final Label skipRecord = new Label(); - mv.visitTryCatchBlock(logStart, logEnd, eatException, "java/lang/Throwable"); mv.visitLabel(logStart); - mv.visitMethodInsn( - Opcodes.INVOKESTATIC, - "datadog/trace/bootstrap/InstrumentationErrors", - "isEnabled", - "()Z", - false); - mv.visitJumpInsn(Opcodes.IFEQ, skipRecord); - mv.visitInsn(Opcodes.DUP); // stack: (top) throwable,throwable - mv.visitMethodInsn( - Opcodes.INVOKESTATIC, - "datadog/trace/bootstrap/InstrumentationErrors", - "recordError", - "(Ljava/lang/Throwable;)V", - false); - mv.visitLabel(skipRecord); - if (frames) { - mv.visitFrame(Opcodes.F_SAME1, 0, null, 1, new Object[] {"java/lang/Throwable"}); + // record instrumentation error + if (detailedErrors) { + mv.visitMethodInsn( + Opcodes.INVOKESTATIC, + "datadog/trace/bootstrap/InstrumentationErrors", + "recordError", + "(Ljava/lang/Throwable;)Ljava/lang/Throwable;", + false); + } else { + mv.visitMethodInsn( + Opcodes.INVOKESTATIC, + "datadog/trace/bootstrap/InstrumentationErrors", + "recordError", + "()V", + false); } - // stack: (top) throwable mv.visitLdcInsn(Type.getType("L" + HANDLER_NAME + ";")); mv.visitMethodInsn( diff --git a/dd-java-agent/instrumentation-testing/src/main/groovy/datadog/trace/agent/test/InstrumentationSpecification.groovy b/dd-java-agent/instrumentation-testing/src/main/groovy/datadog/trace/agent/test/InstrumentationSpecification.groovy index 45ac4286334..ac42023f1fb 100644 --- a/dd-java-agent/instrumentation-testing/src/main/groovy/datadog/trace/agent/test/InstrumentationSpecification.groovy +++ b/dd-java-agent/instrumentation-testing/src/main/groovy/datadog/trace/agent/test/InstrumentationSpecification.groovy @@ -10,6 +10,7 @@ import static datadog.trace.api.config.TraceInstrumentationConfig.CODE_ORIGIN_FO import static datadog.trace.bootstrap.instrumentation.api.AgentTracer.closePrevious import static datadog.trace.util.AgentThreadFactory.AgentThread.TASK_SCHEDULER + import ch.qos.logback.classic.Level import ch.qos.logback.classic.util.ContextInitializer import com.datadog.debugger.agent.ClassesToRetransformFinder @@ -46,6 +47,7 @@ import datadog.trace.api.Pair import datadog.trace.api.ProcessTags import datadog.trace.api.TraceConfig import datadog.trace.api.config.GeneralConfig +import datadog.trace.api.config.TraceInstrumentationConfig import datadog.trace.api.config.TracerConfig import datadog.trace.api.datastreams.AgentDataStreamsMonitoring import datadog.trace.api.datastreams.DataStreamsTransactionExtractor @@ -76,7 +78,6 @@ import java.lang.instrument.ClassFileTransformer import java.lang.instrument.Instrumentation import java.nio.ByteBuffer import java.util.concurrent.ConcurrentHashMap -import java.util.concurrent.CopyOnWriteArrayList import java.util.concurrent.TimeUnit import java.util.concurrent.TimeoutException import net.bytebuddy.agent.ByteBuddyAgent @@ -168,10 +169,6 @@ abstract class InstrumentationSpecification extends DDSpecification implements A @Shared Set TRANSFORMED_CLASSES_TYPES = Sets.newConcurrentHashSet() - @SuppressWarnings('PropertyName') - @Shared - List INSTRUMENTATION_ERRORS = new CopyOnWriteArrayList() - // don't use mocks because it will break too many exhaustive interaction-verifying tests @SuppressWarnings('PropertyName') @Shared @@ -430,6 +427,7 @@ abstract class InstrumentationSpecification extends DDSpecification implements A /** Override to set config before the agent is installed */ protected void configurePreAgent() { + injectSysConfig(TraceInstrumentationConfig.DETAILED_INSTRUMENTATION_ERRORS, "true") injectSysConfig(TracerConfig.SCOPE_ITERATION_KEEP_ALIVE, "1") // don't let iteration spans linger injectSysConfig(GeneralConfig.DATA_STREAMS_ENABLED, String.valueOf(isDataStreamsEnabled())) injectSysConfig(GeneralConfig.DATA_JOBS_ENABLED, String.valueOf(isDataJobsEnabled())) @@ -471,7 +469,7 @@ abstract class InstrumentationSpecification extends DDSpecification implements A if (forceAppSecActive) { ActiveSubsystems.APPSEC_ACTIVE = true } - InstrumentationErrors.enableRecordingAndReset() + InstrumentationErrors.resetErrors() ProcessTags.reset() } @@ -513,8 +511,7 @@ abstract class InstrumentationSpecification extends DDSpecification implements A spanFinishLocations.clear() originalToTrackingSpan.clear() } - def instrumentationErrorCount = InstrumentationErrors.getErrors().size() - assert instrumentationErrorCount == 0, "${instrumentationErrorCount} instrumentation errors were seen:\n${InstrumentationErrors.getErrors().join("\n---\n")}" + assert InstrumentationErrors.noErrors(): InstrumentationErrors.describeErrors() } private void doCheckRepeatedFinish() { @@ -556,8 +553,7 @@ abstract class InstrumentationSpecification extends DDSpecification implements A cleanupAfterAgent() // All cleanup should happen before these assertion. If not, a failing assertion may prevent cleanup - def instrumentationErrors = INSTRUMENTATION_ERRORS.size() - assert instrumentationErrors == 0: "${instrumentationErrors} Instrumentation errors during test:\n${INSTRUMENTATION_ERRORS.join("\n---\n")}" + assert InstrumentationErrors.noErrors(): InstrumentationErrors.describeErrors() assert TRANSFORMED_CLASSES_TYPES.findAll { GlobalIgnores.isAdditionallyIgnored(it.getActualName()) @@ -659,11 +655,9 @@ abstract class InstrumentationSpecification extends DDSpecification implements A return } + InstrumentationErrors.recordError(throwable) println "Unexpected instrumentation error when instrumenting ${typeName} on ${classLoader}" throwable.printStackTrace() - def stackTrace = new StringWriter() - throwable.printStackTrace(new PrintWriter(stackTrace)) - INSTRUMENTATION_ERRORS.add(stackTrace.toString()) } @Override diff --git a/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/asserts/ListWriterAssert.groovy b/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/asserts/ListWriterAssert.groovy index e67228d9d8e..166eb92f6b6 100644 --- a/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/asserts/ListWriterAssert.groovy +++ b/dd-java-agent/testing/src/main/groovy/datadog/trace/agent/test/asserts/ListWriterAssert.groovy @@ -2,7 +2,6 @@ package datadog.trace.agent.test.asserts import static TraceAssert.assertTrace -import datadog.trace.bootstrap.InstrumentationErrors import datadog.trace.common.writer.ListWriter import datadog.trace.core.DDSpan import groovy.transform.stc.ClosureParams @@ -48,15 +47,7 @@ class ListWriterAssert { @ClosureParams(value = SimpleType, options = ['datadog.trace.agent.test.asserts.ListWriterAssert']) @DelegatesTo(value = ListWriterAssert, strategy = Closure.DELEGATE_FIRST) Closure spec) { try { - // Fail fast if we see any muzzle errors or instrumentation errors - for (int timeoutRemaining = ListWriter.WAIT_FOR_TRACES_TIMEOUT_SECONDS; timeoutRemaining > 0; timeoutRemaining--) { - if (writer.waitForTracesMax(expectedSize, 1)) { - break - } - assertNoErrors() - } - assertNoErrors() // even if we break immediately above, make sure we check for errors at least once - writer.waitForTraces(expectedSize, 0) + writer.waitForTraces(expectedSize) def array = writer.toArray() assert array.length == expectedSize def traces = (Arrays.asList(array) as List>) @@ -100,11 +91,6 @@ class ListWriterAssert { } } - static void assertNoErrors() { - assert InstrumentationErrors.getErrors().size() == 0 : "Instrumentation errors were seen, failing fast" - // InstrumentationSpecification.{cleanup,cleanupAfterAgent} will provide details later - } - void sortSpansByStart() { traces = traces.collect { return new ArrayList(it).sort { a, b -> diff --git a/dd-trace-api/src/main/java/datadog/trace/api/config/TraceInstrumentationConfig.java b/dd-trace-api/src/main/java/datadog/trace/api/config/TraceInstrumentationConfig.java index 0d21a7e371d..6d44cf8b249 100644 --- a/dd-trace-api/src/main/java/datadog/trace/api/config/TraceInstrumentationConfig.java +++ b/dd-trace-api/src/main/java/datadog/trace/api/config/TraceInstrumentationConfig.java @@ -15,6 +15,7 @@ public final class TraceInstrumentationConfig { public static final String CODE_ORIGIN_MAX_USER_FRAMES = "code.origin.max.user.frames"; public static final String TRACE_ENABLED = "trace.enabled"; public static final String INTEGRATIONS_ENABLED = "integrations.enabled"; + public static final String DETAILED_INSTRUMENTATION_ERRORS = "detailed.instrumentation.errors"; public static final String TRACE_EXTENSIONS_PATH = "trace.extensions.path"; diff --git a/dd-trace-core/src/main/java/datadog/trace/common/writer/ListWriter.java b/dd-trace-core/src/main/java/datadog/trace/common/writer/ListWriter.java index 324e1c562d8..7c151dab5c4 100644 --- a/dd-trace-core/src/main/java/datadog/trace/common/writer/ListWriter.java +++ b/dd-trace-core/src/main/java/datadog/trace/common/writer/ListWriter.java @@ -18,8 +18,6 @@ /** List writer used by tests mostly */ public class ListWriter extends CopyOnWriteArrayList> implements Writer { - public static final int WAIT_FOR_TRACES_TIMEOUT_SECONDS = 20; - private static final Logger log = LoggerFactory.getLogger(ListWriter.class); private static final Filter ACCEPT_ALL = trace -> true; @@ -91,12 +89,7 @@ public boolean waitForTracesMax(final int number, int seconds) throws Interrupte } public void waitForTraces(final int number) throws InterruptedException, TimeoutException { - waitForTraces(number, WAIT_FOR_TRACES_TIMEOUT_SECONDS); - } - - public void waitForTraces(final int number, final int seconds) - throws InterruptedException, TimeoutException { - if (!waitForTracesMax(number, seconds)) { + if (!waitForTracesMax(number, 20)) { String msg = "Timeout waiting for " + number 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 e8627a5a852..74bff640024 100644 --- a/internal-api/src/main/java/datadog/trace/api/InstrumenterConfig.java +++ b/internal-api/src/main/java/datadog/trace/api/InstrumenterConfig.java @@ -56,6 +56,7 @@ import static datadog.trace.api.config.TraceInstrumentationConfig.AXIS_TRANSPORT_CLASS_NAME; import static datadog.trace.api.config.TraceInstrumentationConfig.CODE_ORIGIN_FOR_SPANS_ENABLED; import static datadog.trace.api.config.TraceInstrumentationConfig.CODE_ORIGIN_FOR_SPANS_INTERFACE_SUPPORT; +import static datadog.trace.api.config.TraceInstrumentationConfig.DETAILED_INSTRUMENTATION_ERRORS; import static datadog.trace.api.config.TraceInstrumentationConfig.EXPERIMENTAL_DEFER_INTEGRATIONS_UNTIL; import static datadog.trace.api.config.TraceInstrumentationConfig.HTTP_URL_CONNECTION_CLASS_NAME; import static datadog.trace.api.config.TraceInstrumentationConfig.INSTRUMENTATION_CONFIG_ID; @@ -145,6 +146,7 @@ public class InstrumenterConfig { private final boolean triageEnabled; private final boolean integrationsEnabled; + private final boolean detailedInstrumentationErrors; private final boolean codeOriginEnabled; private final boolean codeOriginInterfaceSupport; @@ -249,6 +251,8 @@ private InstrumenterConfig() { integrationsEnabled = configProvider.getBoolean(INTEGRATIONS_ENABLED, DEFAULT_INTEGRATIONS_ENABLED); + detailedInstrumentationErrors = + configProvider.getBoolean(DETAILED_INSTRUMENTATION_ERRORS, false); codeOriginEnabled = configProvider.getBoolean( @@ -408,6 +412,10 @@ public boolean isIntegrationsEnabled() { return integrationsEnabled; } + public boolean isDetailedInstrumentationErrors() { + return detailedInstrumentationErrors; + } + /** * isIntegrationEnabled determines whether an integration under the specified name(s) is enabled * according to the following list of configurations, from highest to lowest precedence: diff --git a/metadata/supported-configurations.json b/metadata/supported-configurations.json index 01087b11aa4..82699576845 100644 --- a/metadata/supported-configurations.json +++ b/metadata/supported-configurations.json @@ -11792,6 +11792,14 @@ "default": "true", "aliases": ["DD_TRACE_INTEGRATION_JAVA_MODULE_ENABLED", "DD_INTEGRATION_JAVA_MODULE_ENABLED"] } + ], + "DD_DETAILED_INSTRUMENTATION_ERRORS": [ + { + "version": "A", + "type": "boolean", + "default": "false", + "aliases": [] + } ] }, "deprecations": {} From 25f0c47e858e40a509894bd63b6292b4ed48eb8f Mon Sep 17 00:00:00 2001 From: Stuart McCulloch Date: Thu, 21 May 2026 17:58:07 +0100 Subject: [PATCH 09/14] Update comment --- .../trace/agent/tooling/bytebuddy/ExceptionHandlers.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/ExceptionHandlers.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/ExceptionHandlers.java index 5b4f58ade00..2fd96e2edf1 100644 --- a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/ExceptionHandlers.java +++ b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/ExceptionHandlers.java @@ -45,7 +45,7 @@ public Size apply(final MethodVisitor mv, final Implementation.Context context) // // BlockingExceptionHandler.rethrowIfBlockingException(t); // try { - // InstrumentationErrors.incrementErrorCount(); + // InstrumentationErrors.recordError(); // org.slf4j.LoggerFactory.getLogger((Class)ExceptionLogger.class) // .debug("Failed to handle exception in instrumentation for ...", t); // } catch (Throwable t2) { @@ -55,7 +55,7 @@ public Size apply(final MethodVisitor mv, final Implementation.Context context) // // BlockingExceptionHandler.rethrowIfBlockingException(t); // try { - // InstrumentationErrors.incrementErrorCount(); + // InstrumentationErrors.recordError(); // org.slf4j.LoggerFactory.getLogger((Class)ExceptionLogger.class) // .error("Failed to handle exception in instrumentation for ...", t); // System.exit(1); From 0239cffcf78c638f84eb195333ba387c8a065c73 Mon Sep 17 00:00:00 2001 From: Stuart McCulloch Date: Thu, 21 May 2026 18:02:29 +0100 Subject: [PATCH 10/14] Update ClassFileTransformerListener to use InstrumentationErrors --- .../trace/agent/test/ClassFileTransformerListener.java | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/dd-java-agent/instrumentation-testing/src/main/java/datadog/trace/agent/test/ClassFileTransformerListener.java b/dd-java-agent/instrumentation-testing/src/main/java/datadog/trace/agent/test/ClassFileTransformerListener.java index 16ba4954efc..de0169a0af8 100644 --- a/dd-java-agent/instrumentation-testing/src/main/java/datadog/trace/agent/test/ClassFileTransformerListener.java +++ b/dd-java-agent/instrumentation-testing/src/main/java/datadog/trace/agent/test/ClassFileTransformerListener.java @@ -1,13 +1,12 @@ package datadog.trace.agent.test; -import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertTrue; import com.google.common.collect.Sets; import datadog.trace.agent.tooling.bytebuddy.matcher.GlobalIgnores; +import datadog.trace.bootstrap.InstrumentationErrors; import de.thetaphi.forbiddenapis.SuppressForbidden; import java.util.Set; -import java.util.concurrent.atomic.AtomicInteger; import net.bytebuddy.agent.builder.AgentBuilder; import net.bytebuddy.description.type.TypeDescription; import net.bytebuddy.dynamic.DynamicType; @@ -18,7 +17,6 @@ public class ClassFileTransformerListener implements AgentBuilder.Listener { final Set transformedClassesNames = Sets.newConcurrentHashSet(); final Set transformedClassesTypes = Sets.newConcurrentHashSet(); - final AtomicInteger instrumentationErrorCount = new AtomicInteger(0); @Override public void onTransformation( @@ -45,10 +43,10 @@ public void onError( return; } + InstrumentationErrors.recordError(throwable); System.out.println( "Unexpected instrumentation error when instrumenting " + typeName + " on " + classLoader); throwable.printStackTrace(); - instrumentationErrorCount.incrementAndGet(); } @Override @@ -71,8 +69,7 @@ public void onComplete( public void verify() { // Check instrumentation errors - int errorCount = this.instrumentationErrorCount.get(); - assertEquals(0, errorCount, errorCount + " instrumentation errors during test"); + assertTrue(InstrumentationErrors.noErrors(), InstrumentationErrors::describeErrors); // Check effectively transformed classes that should have been ignored assertTrue( this.transformedClassesTypes.stream() From 40eae5d32d10ee1ae8e8e349351733fbe3710496 Mon Sep 17 00:00:00 2001 From: Stuart McCulloch Date: Thu, 21 May 2026 18:26:59 +0100 Subject: [PATCH 11/14] Clean-up reporting of instrumentation errors (installation vs per-test) --- .../agent/test/InstrumentationSpecification.groovy | 10 +++++++--- .../trace/agent/test/AbstractInstrumentationTest.java | 8 ++++++++ .../trace/agent/test/ClassFileTransformerListener.java | 2 -- 3 files changed, 15 insertions(+), 5 deletions(-) diff --git a/dd-java-agent/instrumentation-testing/src/main/groovy/datadog/trace/agent/test/InstrumentationSpecification.groovy b/dd-java-agent/instrumentation-testing/src/main/groovy/datadog/trace/agent/test/InstrumentationSpecification.groovy index ac42023f1fb..cefd3351250 100644 --- a/dd-java-agent/instrumentation-testing/src/main/groovy/datadog/trace/agent/test/InstrumentationSpecification.groovy +++ b/dd-java-agent/instrumentation-testing/src/main/groovy/datadog/trace/agent/test/InstrumentationSpecification.groovy @@ -419,6 +419,9 @@ abstract class InstrumentationSpecification extends DDSpecification implements A .hasNext(): "No instrumentation found" activeTransformer = AgentInstaller.installBytebuddyAgent( INSTRUMENTATION, true, AgentInstaller.getEnabledSystems(), this) + + // check for instrumentation issues during installation + assert InstrumentationErrors.noErrors(): InstrumentationErrors.describeErrors() } protected String idGenerationStrategyName() { @@ -434,6 +437,8 @@ abstract class InstrumentationSpecification extends DDSpecification implements A } void setup() { + InstrumentationErrors.resetErrors() // reset for each test + configureLoggingLevels() assertThreadsEachCleanup = false @@ -469,7 +474,6 @@ abstract class InstrumentationSpecification extends DDSpecification implements A if (forceAppSecActive) { ActiveSubsystems.APPSEC_ACTIVE = true } - InstrumentationErrors.resetErrors() ProcessTags.reset() } @@ -511,6 +515,8 @@ abstract class InstrumentationSpecification extends DDSpecification implements A spanFinishLocations.clear() originalToTrackingSpan.clear() } + + // check for instrumentation issues while running each test assert InstrumentationErrors.noErrors(): InstrumentationErrors.describeErrors() } @@ -553,8 +559,6 @@ abstract class InstrumentationSpecification extends DDSpecification implements A cleanupAfterAgent() // All cleanup should happen before these assertion. If not, a failing assertion may prevent cleanup - assert InstrumentationErrors.noErrors(): InstrumentationErrors.describeErrors() - assert TRANSFORMED_CLASSES_TYPES.findAll { GlobalIgnores.isAdditionallyIgnored(it.getActualName()) }.isEmpty(): "Transformed classes match global libraries ignore matcher" diff --git a/dd-java-agent/instrumentation-testing/src/main/java/datadog/trace/agent/test/AbstractInstrumentationTest.java b/dd-java-agent/instrumentation-testing/src/main/java/datadog/trace/agent/test/AbstractInstrumentationTest.java index 58f1831c0f3..3ac7d9371e1 100644 --- a/dd-java-agent/instrumentation-testing/src/main/java/datadog/trace/agent/test/AbstractInstrumentationTest.java +++ b/dd-java-agent/instrumentation-testing/src/main/java/datadog/trace/agent/test/AbstractInstrumentationTest.java @@ -13,6 +13,7 @@ import datadog.trace.agent.tooling.bytebuddy.matcher.ClassLoaderMatchers; import datadog.trace.api.Config; import datadog.trace.api.IdGenerationStrategy; +import datadog.trace.bootstrap.InstrumentationErrors; import datadog.trace.bootstrap.instrumentation.api.AgentSpan; import datadog.trace.bootstrap.instrumentation.api.AgentTracer.TracerAPI; import datadog.trace.common.writer.ListWriter; @@ -97,10 +98,14 @@ static void initAll() { activeTransformer = AgentInstaller.installBytebuddyAgent( INSTRUMENTATION, true, AgentInstaller.getEnabledSystems(), transformerListener); + + // check for instrumentation issues during installation + assertTrue(InstrumentationErrors.noErrors(), InstrumentationErrors::describeErrors); } @BeforeEach public void init() { + InstrumentationErrors.resetErrors(); // reset for each test tracer.flush(); writer.start(); } @@ -108,6 +113,9 @@ public void init() { @AfterEach public void tearDown() { tracer.flush(); + + // check for instrumentation issues while running each test + assertTrue(InstrumentationErrors.noErrors(), InstrumentationErrors::describeErrors); } @AfterAll diff --git a/dd-java-agent/instrumentation-testing/src/main/java/datadog/trace/agent/test/ClassFileTransformerListener.java b/dd-java-agent/instrumentation-testing/src/main/java/datadog/trace/agent/test/ClassFileTransformerListener.java index de0169a0af8..a865bf374d1 100644 --- a/dd-java-agent/instrumentation-testing/src/main/java/datadog/trace/agent/test/ClassFileTransformerListener.java +++ b/dd-java-agent/instrumentation-testing/src/main/java/datadog/trace/agent/test/ClassFileTransformerListener.java @@ -68,8 +68,6 @@ public void onComplete( } public void verify() { - // Check instrumentation errors - assertTrue(InstrumentationErrors.noErrors(), InstrumentationErrors::describeErrors); // Check effectively transformed classes that should have been ignored assertTrue( this.transformedClassesTypes.stream() From 83b1043e6ffac7f153eecdb16ff59a55e5aba7bb Mon Sep 17 00:00:00 2001 From: Stuart McCulloch Date: Thu, 21 May 2026 18:39:42 +0100 Subject: [PATCH 12/14] Enable detailed instrumentation errors for AbstractInstrumentationTest --- .../datadog/trace/agent/test/AbstractInstrumentationTest.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/dd-java-agent/instrumentation-testing/src/main/java/datadog/trace/agent/test/AbstractInstrumentationTest.java b/dd-java-agent/instrumentation-testing/src/main/java/datadog/trace/agent/test/AbstractInstrumentationTest.java index 3ac7d9371e1..84f8cda2359 100644 --- a/dd-java-agent/instrumentation-testing/src/main/java/datadog/trace/agent/test/AbstractInstrumentationTest.java +++ b/dd-java-agent/instrumentation-testing/src/main/java/datadog/trace/agent/test/AbstractInstrumentationTest.java @@ -21,6 +21,7 @@ import datadog.trace.core.DDSpan; import datadog.trace.core.PendingTrace; import datadog.trace.core.TraceCollector; +import datadog.trace.junit.utils.config.WithConfig; import datadog.trace.junit.utils.context.AllowContextTestingExtension; import java.lang.instrument.ClassFileTransformer; import java.lang.instrument.Instrumentation; @@ -52,6 +53,7 @@ *
  • {@code @AfterAll}: Closes the tracer and removes the agent transformer * */ +@WithConfig(key = "detailed.instrumentation.errors", value = "true") @ExtendWith({TestClassShadowingExtension.class, AllowContextTestingExtension.class}) public abstract class AbstractInstrumentationTest { static final Instrumentation INSTRUMENTATION = ByteBuddyAgent.getInstrumentation(); From 8666b1a064079100a37f0ae3e97fb54c20272a3b Mon Sep 17 00:00:00 2001 From: Stuart McCulloch Date: Thu, 21 May 2026 18:52:28 +0100 Subject: [PATCH 13/14] Fix metadata/supported-configurations.json --- metadata/supported-configurations.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/metadata/supported-configurations.json b/metadata/supported-configurations.json index 82699576845..32e40412662 100644 --- a/metadata/supported-configurations.json +++ b/metadata/supported-configurations.json @@ -11797,7 +11797,7 @@ { "version": "A", "type": "boolean", - "default": "false", + "default": null, "aliases": [] } ] From 8089ad538dd77794690ec35f9a34ba023c821d41 Mon Sep 17 00:00:00 2001 From: Stuart McCulloch Date: Thu, 21 May 2026 21:24:58 +0100 Subject: [PATCH 14/14] Reset InstrumentationErrors before setupSpec --- .../trace/agent/test/InstrumentationSpecification.groovy | 2 ++ .../datadog/trace/agent/test/AbstractInstrumentationTest.java | 2 ++ 2 files changed, 4 insertions(+) diff --git a/dd-java-agent/instrumentation-testing/src/main/groovy/datadog/trace/agent/test/InstrumentationSpecification.groovy b/dd-java-agent/instrumentation-testing/src/main/groovy/datadog/trace/agent/test/InstrumentationSpecification.groovy index cefd3351250..be0b321216e 100644 --- a/dd-java-agent/instrumentation-testing/src/main/groovy/datadog/trace/agent/test/InstrumentationSpecification.groovy +++ b/dd-java-agent/instrumentation-testing/src/main/groovy/datadog/trace/agent/test/InstrumentationSpecification.groovy @@ -349,6 +349,8 @@ abstract class InstrumentationSpecification extends DDSpecification implements A @SuppressForbidden void setupSpec() { + InstrumentationErrors.resetErrors() + AgentMeter.registerIfAbsent( STATS_D_CLIENT, new MonitoringImpl(STATS_D_CLIENT, 10, TimeUnit.SECONDS), diff --git a/dd-java-agent/instrumentation-testing/src/main/java/datadog/trace/agent/test/AbstractInstrumentationTest.java b/dd-java-agent/instrumentation-testing/src/main/java/datadog/trace/agent/test/AbstractInstrumentationTest.java index 84f8cda2359..a82c8319abd 100644 --- a/dd-java-agent/instrumentation-testing/src/main/java/datadog/trace/agent/test/AbstractInstrumentationTest.java +++ b/dd-java-agent/instrumentation-testing/src/main/java/datadog/trace/agent/test/AbstractInstrumentationTest.java @@ -69,6 +69,8 @@ public abstract class AbstractInstrumentationTest { @BeforeAll static void initAll() { + InstrumentationErrors.resetErrors(); + // If this fails, it's likely the result of another test loading Config before it can be // injected into the bootstrap classpath. assertNull(Config.class.getClassLoader(), "Config must load on the bootstrap classpath.");