From e8a8fe1da9eb3ac21b63847358eee66fd2b7cda8 Mon Sep 17 00:00:00 2001 From: jean-philippe bempel Date: Wed, 15 Jul 2026 17:55:10 +0200 Subject: [PATCH] Hoist local vars for exception probes Exception probe instrumentation was not performing hoisting of local vars which is in fact required if we want to capture those locals because the wrapping with try/catch of the whole method body create sub scope preventing locals to be captured in the catch clause. --- .../com/datadog/debugger/agent/ConfigurationUpdater.java | 8 +++----- .../instrumentation/CapturedContextInstrumenter.java | 4 ++-- .../debugger/instrumentation/ExceptionInstrumenter.java | 4 ++++ .../exception/ExceptionProbeInstrumentationTest.java | 5 +++++ 4 files changed, 14 insertions(+), 7 deletions(-) diff --git a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/agent/ConfigurationUpdater.java b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/agent/ConfigurationUpdater.java index 5a68838f3b8..25fc968a560 100644 --- a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/agent/ConfigurationUpdater.java +++ b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/agent/ConfigurationUpdater.java @@ -95,6 +95,7 @@ DebuggerTransformer supply( private volatile Configuration currentConfiguration; private DebuggerTransformer currentTransformer; private final ProbeMetadata probeMetadata = new ProbeMetadata(); + private final Config config; private final DebuggerSink sink; private final ClassesToRetransformFinder finder; private final String serviceName; @@ -112,6 +113,7 @@ public ConfigurationUpdater( this.instrumentation = instrumentation; this.transformerSupplier = transformerSupplier; this.serviceName = TagsHelper.sanitize(config.getServiceName()); + this.config = config; this.sink = sink; this.finder = finder; } @@ -255,11 +257,7 @@ private void installNewDefinitions(Configuration newConfiguration) { // install new probe definitions DebuggerTransformer newTransformer = transformerSupplier.supply( - Config.get(), - newConfiguration, - this::recordInstrumentationProgress, - probeMetadata, - sink); + config, newConfiguration, this::recordInstrumentationProgress, probeMetadata, sink); instrumentation.addTransformer(newTransformer, true); currentTransformer = newTransformer; LOGGER.debug("New transformer installed with probes: {}", newConfiguration.getDefinitions()); diff --git a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/instrumentation/CapturedContextInstrumenter.java b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/instrumentation/CapturedContextInstrumenter.java index 6a6f4077984..5cbbfedb083 100644 --- a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/instrumentation/CapturedContextInstrumenter.java +++ b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/instrumentation/CapturedContextInstrumenter.java @@ -74,7 +74,7 @@ public class CapturedContextInstrumenter extends Instrumenter { private int exitContextVar = -1; private int timestampStartVar = -1; private int throwableListVar = -1; - private Collection hoistedLocalVars = Collections.emptyList(); + protected Collection hoistedLocalVars = Collections.emptyList(); public CapturedContextInstrumenter( ProbeDefinition definition, @@ -470,7 +470,7 @@ protected void addIsReadyToCaptureCall(InsnList insnList) { // Initialize and hoist local variables to the top of the method // if there is name/slot conflict, do nothing for the conflicting local variable - private Collection initAndHoistLocalVars(MethodNode methodNode) { + protected Collection initAndHoistLocalVars(MethodNode methodNode) { int hoistingLevel = Config.get().getDynamicInstrumentationLocalVarHoistingLevel(); if (hoistingLevel == 0 || language != JvmLanguage.JAVA) { // for now, only hoist local vars for Java diff --git a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/instrumentation/ExceptionInstrumenter.java b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/instrumentation/ExceptionInstrumenter.java index 245d95e124d..34c9e0cd1ca 100644 --- a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/instrumentation/ExceptionInstrumenter.java +++ b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/instrumentation/ExceptionInstrumenter.java @@ -21,6 +21,10 @@ public ExceptionInstrumenter( @Override public InstrumentationResult.Status instrument() { + // hoisting is required because exception instrumentation is wrapping the whole method body in + // a try/catch that create a subscobe and even level method local vars are not accessible + // in the catch clause for capture + hoistedLocalVars = initAndHoistLocalVars(methodNode); Map> frames = computeFrames(classNode.name, methodNode); processInstructions(frames); // fill returnHandlerLabel addFinallyHandler(methodEnterLabel, returnHandlerLabel); diff --git a/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/exception/ExceptionProbeInstrumentationTest.java b/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/exception/ExceptionProbeInstrumentationTest.java index e72e14c4201..64709cc54ee 100644 --- a/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/exception/ExceptionProbeInstrumentationTest.java +++ b/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/exception/ExceptionProbeInstrumentationTest.java @@ -135,6 +135,9 @@ public void onlyInstrument() throws Exception { } @Test + @DisabledIf( + value = "datadog.environment.JavaVirtualMachine#isJ9", + disabledReason = "Bug in J9: no LocalVariableTable for ClassFileTransformer") public void instrumentAndCaptureSnapshots() throws Exception { Config config = createConfig(); ExceptionProbeManager exceptionProbeManager = new ExceptionProbeManager(classNameFiltering); @@ -162,6 +165,8 @@ public void instrumentAndCaptureSnapshots() throws Exception { assertEquals(fingerprint, span.getTags().get(DD_DEBUG_ERROR_EXCEPTION_HASH)); assertEquals(Boolean.TRUE, span.getTags().get(Tags.ERROR_DEBUG_INFO_CAPTURED)); assertEquals(snapshot0.getId(), span.getTags().get(String.format(SNAPSHOT_ID_TAG_FMT, 0))); + assertTrue(snapshot0.getCaptures().getReturn().getArguments().containsKey("arg")); + assertTrue(snapshot0.getCaptures().getReturn().getLocals().containsKey("intLocal")); assertEquals(1, probeSampler.getCallCount()); assertEquals(1, globalSampler.getCallCount()); }