diff --git a/.circleci/config.continue.yml.j2 b/.circleci/config.continue.yml.j2 index 94dcdfe4bd0..6caba320728 100644 --- a/.circleci/config.continue.yml.j2 +++ b/.circleci/config.continue.yml.j2 @@ -36,7 +36,7 @@ instrumentation_modules: &instrumentation_modules "dd-java-agent/instrumentation debugger_modules: &debugger_modules "dd-java-agent/agent-debugger|dd-java-agent/agent-bootstrap|dd-java-agent/agent-builder|internal-api|communication|dd-trace-core" profiling_modules: &profiling_modules "dd-java-agent/agent-profiling" -default_system_tests_commit: &default_system_tests_commit 752456136fab0642446d124ead05aa7d8f4fa5c4 +default_system_tests_commit: &default_system_tests_commit c6e54d143cfdf97b2f0a815f22f53247c119f635 parameters: nightly: diff --git a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/instrumentation/CapturedContextInstrumentor.java b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/instrumentation/CapturedContextInstrumentor.java index f2c17783763..2f63264dfae 100644 --- a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/instrumentation/CapturedContextInstrumentor.java +++ b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/instrumentation/CapturedContextInstrumentor.java @@ -902,7 +902,8 @@ private void collectLocalVariables(AbstractInsnNode location, InsnList insnList) return; } Collection localVarNodes; - if (definition.isLineProbe()) { + boolean isLocalVarHoistingEnabled = Config.get().isDebuggerHoistLocalVarsEnabled(); + if (definition.isLineProbe() || !isLocalVarHoistingEnabled) { localVarNodes = methodNode.localVariables; } else { localVarNodes = unscopedLocalVars; @@ -913,7 +914,7 @@ private void collectLocalVariables(AbstractInsnNode location, InsnList insnList) int idx = variableNode.index - localVarBaseOffset; if (idx >= argOffset) { // var is local not arg - if (isLineProbe) { + if (isLineProbe || !isLocalVarHoistingEnabled) { if (ASMHelper.isInScope(methodNode, variableNode, location)) { applicableVars.add(variableNode); } diff --git a/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/CapturedSnapshotTest.java b/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/CapturedSnapshotTest.java index 006dce057fa..05e81a0c1a0 100644 --- a/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/CapturedSnapshotTest.java +++ b/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/CapturedSnapshotTest.java @@ -7,6 +7,7 @@ import static com.datadog.debugger.util.MoshiSnapshotHelper.REDACTED_IDENT_REASON; import static com.datadog.debugger.util.MoshiSnapshotHelper.REDACTED_TYPE_REASON; import static com.datadog.debugger.util.MoshiSnapshotTestHelper.VALUE_ADAPTER; +import static com.datadog.debugger.util.TestHelper.setFieldInConfig; import static datadog.trace.bootstrap.debugger.util.Redaction.REDACTED_VALUE; import static org.junit.jupiter.api.Assertions.assertArrayEquals; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -238,14 +239,20 @@ public void oldClass1_1() throws Exception { @Test public void oldJavacBug() throws Exception { - final String CLASS_NAME = "com.datadog.debugger.classfiles.JavacBug"; // compiled with jdk 1.6 - TestSnapshotListener listener = installSingleProbe(CLASS_NAME, "main", null); - Class testClass = Class.forName(CLASS_NAME); - assertNotNull(testClass); - int result = Reflect.onClass(testClass).call("main", "").get(); - assertEquals(45, result); - // with local var hoisting and initialization at the beginning of the method, issue is resolved - assertEquals(1, listener.snapshots.size()); + setFieldInConfig(Config.get(), "debuggerHoistLocalVarsEnabled", true); + try { + final String CLASS_NAME = "com.datadog.debugger.classfiles.JavacBug"; // compiled with jdk 1.6 + TestSnapshotListener listener = installSingleProbe(CLASS_NAME, "main", null); + Class testClass = Class.forName(CLASS_NAME); + assertNotNull(testClass); + int result = Reflect.onClass(testClass).call("main", "").get(); + assertEquals(45, result); + // with local var hoisting and initialization at the beginning of the method, issue is + // resolved + assertEquals(1, listener.snapshots.size()); + } finally { + setFieldInConfig(Config.get(), "debuggerHoistLocalVarsEnabled", false); + } } @Test @@ -1801,32 +1808,39 @@ public void evaluateAtExitFalse() throws IOException, URISyntaxException { value = "datadog.trace.api.Platform#isJ9", disabledReason = "we cannot get local variable debug info") public void uncaughtExceptionConditionLocalVar() throws IOException, URISyntaxException { - final String CLASS_NAME = "CapturedSnapshot05"; - LogProbe probe = - createProbeBuilder(PROBE_ID, CLASS_NAME, "main", "(String)") - .when( - new ProbeCondition(DSL.when(DSL.ge(DSL.ref("after"), DSL.value(0))), "after >= 0")) - .evaluateAt(MethodLocation.EXIT) - .build(); - TestSnapshotListener listener = installProbes(probe); - Class testClass = compileAndLoadClass(CLASS_NAME); + setFieldInConfig(Config.get(), "debuggerHoistLocalVarsEnabled", true); try { - Reflect.onClass(testClass).call("main", "triggerUncaughtException").get(); - Assertions.fail("should not reach this code"); - } catch (ReflectException ex) { - assertEquals("oops", ex.getCause().getCause().getMessage()); + + final String CLASS_NAME = "CapturedSnapshot05"; + LogProbe probe = + createProbeBuilder(PROBE_ID, CLASS_NAME, "main", "(String)") + .when( + new ProbeCondition( + DSL.when(DSL.ge(DSL.ref("after"), DSL.value(0))), "after >= 0")) + .evaluateAt(MethodLocation.EXIT) + .build(); + TestSnapshotListener listener = installProbes(probe); + Class testClass = compileAndLoadClass(CLASS_NAME); + try { + Reflect.onClass(testClass).call("main", "triggerUncaughtException").get(); + Assertions.fail("should not reach this code"); + } catch (ReflectException ex) { + assertEquals("oops", ex.getCause().getCause().getMessage()); + } + Snapshot snapshot = assertOneSnapshot(listener); + assertCaptureThrowable( + snapshot.getCaptures().getReturn(), + "CapturedSnapshot05$CustomException", + "oops", + "CapturedSnapshot05.triggerUncaughtException", + 8); + assertNull(snapshot.getEvaluationErrors()); + // after is 0 because the exception is thrown before the assignment and local var initialized + // at the beginning of the method by instrumentation + assertCaptureLocals(snapshot.getCaptures().getReturn(), "after", "long", "0"); + } finally { + setFieldInConfig(Config.get(), "debuggerHoistLocalVarsEnabled", false); } - Snapshot snapshot = assertOneSnapshot(listener); - assertCaptureThrowable( - snapshot.getCaptures().getReturn(), - "CapturedSnapshot05$CustomException", - "oops", - "CapturedSnapshot05.triggerUncaughtException", - 8); - assertNull(snapshot.getEvaluationErrors()); - // after is 0 because the exception is thrown before the assignment and local var initialized - // at the beginning of the method by instrumentation - assertCaptureLocals(snapshot.getCaptures().getReturn(), "after", "long", "0"); } @Test @@ -1858,15 +1872,20 @@ public void uncaughtExceptionCaptureLocalVars() throws IOException, URISyntaxExc value = "datadog.trace.api.Platform#isJ9", disabledReason = "we cannot get local variable debug info") public void methodProbeLocalVarsLocalScopes() throws IOException, URISyntaxException { - final String CLASS_NAME = "com.datadog.debugger.CapturedSnapshot31"; - LogProbe probe = createProbeAtExit(PROBE_ID, CLASS_NAME, "localScopes", "(String)"); - TestSnapshotListener listener = installProbes(probe); - Class testClass = compileAndLoadClass(CLASS_NAME); - int result = Reflect.onClass(testClass).call("main", "localScopes").get(); - assertEquals(42, result); - Snapshot snapshot = assertOneSnapshot(listener); - assertEquals(1, snapshot.getCaptures().getReturn().getLocals().size()); - assertCaptureLocals(snapshot.getCaptures().getReturn(), "@return", "int", "42"); + setFieldInConfig(Config.get(), "debuggerHoistLocalVarsEnabled", true); + try { + final String CLASS_NAME = "com.datadog.debugger.CapturedSnapshot31"; + LogProbe probe = createProbeAtExit(PROBE_ID, CLASS_NAME, "localScopes", "(String)"); + TestSnapshotListener listener = installProbes(probe); + Class testClass = compileAndLoadClass(CLASS_NAME); + int result = Reflect.onClass(testClass).call("main", "localScopes").get(); + assertEquals(42, result); + Snapshot snapshot = assertOneSnapshot(listener); + assertEquals(1, snapshot.getCaptures().getReturn().getLocals().size()); + assertCaptureLocals(snapshot.getCaptures().getReturn(), "@return", "int", "42"); + } finally { + setFieldInConfig(Config.get(), "debuggerHoistLocalVarsEnabled", false); + } } @Test @@ -1957,16 +1976,21 @@ public void overlappingLocalVarSlot() throws IOException, URISyntaxException { value = "datadog.trace.api.Platform#isJ9", disabledReason = "we cannot get local variable debug info") public void duplicateLocalDifferentScope() throws IOException, URISyntaxException { - final String CLASS_NAME = "com.datadog.debugger.CapturedSnapshot31"; - LogProbe probe = - createProbeAtExit(PROBE_ID, CLASS_NAME, "duplicateLocalDifferentScope", "(String)"); - TestSnapshotListener listener = installProbes(probe); - Class testClass = compileAndLoadClass(CLASS_NAME); - int result = Reflect.onClass(testClass).call("main", "duplicateLocalDifferentScope").get(); - assertEquals(28, result); - Snapshot snapshot = assertOneSnapshot(listener); - assertCaptureLocals( - snapshot.getCaptures().getReturn(), "ch", Character.TYPE.getTypeName(), "e"); + setFieldInConfig(Config.get(), "debuggerHoistLocalVarsEnabled", true); + try { + final String CLASS_NAME = "com.datadog.debugger.CapturedSnapshot31"; + LogProbe probe = + createProbeAtExit(PROBE_ID, CLASS_NAME, "duplicateLocalDifferentScope", "(String)"); + TestSnapshotListener listener = installProbes(probe); + Class testClass = compileAndLoadClass(CLASS_NAME); + int result = Reflect.onClass(testClass).call("main", "duplicateLocalDifferentScope").get(); + assertEquals(28, result); + Snapshot snapshot = assertOneSnapshot(listener); + assertCaptureLocals( + snapshot.getCaptures().getReturn(), "ch", Character.TYPE.getTypeName(), "e"); + } finally { + setFieldInConfig(Config.get(), "debuggerHoistLocalVarsEnabled", false); + } } @Test diff --git a/dd-trace-api/src/main/java/datadog/trace/api/ConfigDefaults.java b/dd-trace-api/src/main/java/datadog/trace/api/ConfigDefaults.java index 4b3df14e981..ceb390acfe9 100644 --- a/dd-trace-api/src/main/java/datadog/trace/api/ConfigDefaults.java +++ b/dd-trace-api/src/main/java/datadog/trace/api/ConfigDefaults.java @@ -181,7 +181,7 @@ public final class ConfigDefaults { static final boolean DEFAULT_DEBUGGER_VERIFY_BYTECODE = true; static final boolean DEFAULT_DEBUGGER_INSTRUMENT_THE_WORLD = false; static final int DEFAULT_DEBUGGER_CAPTURE_TIMEOUT = 100; // milliseconds - static final boolean DEFAULT_DEBUGGER_HOIST_LOCALVARS_ENABLED = true; + static final boolean DEFAULT_DEBUGGER_HOIST_LOCALVARS_ENABLED = false; static final boolean DEFAULT_DEBUGGER_SYMBOL_ENABLED = true; static final boolean DEFAULT_DEBUGGER_SYMBOL_FORCE_UPLOAD = false; static final int DEFAULT_DEBUGGER_SYMBOL_FLUSH_THRESHOLD = 100; // nb of classes