diff --git a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/agent/DebuggerAgent.java b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/agent/DebuggerAgent.java index bc59cc47438..dbeb9738565 100644 --- a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/agent/DebuggerAgent.java +++ b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/agent/DebuggerAgent.java @@ -212,7 +212,8 @@ public static void startExceptionReplay() { configurationUpdater, classNameFilter, Duration.ofSeconds(config.getDebuggerExceptionCaptureInterval()), - config.getDebuggerMaxExceptionPerSecond()); + config.getDebuggerMaxExceptionPerSecond(), + config.getDebuggerExceptionMaxCapturedFrames()); DebuggerContext.initExceptionDebugger(exceptionDebugger); LOGGER.info("Started Exception Replay"); } diff --git a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/exception/DefaultExceptionDebugger.java b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/exception/DefaultExceptionDebugger.java index 2d4b2dd5fe5..981f52208b8 100644 --- a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/exception/DefaultExceptionDebugger.java +++ b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/exception/DefaultExceptionDebugger.java @@ -38,28 +38,33 @@ public class DefaultExceptionDebugger implements DebuggerContext.ExceptionDebugg private final ConfigurationUpdater configurationUpdater; private final ClassNameFilter classNameFiltering; private final CircuitBreaker circuitBreaker; + private final int maxCapturedFrames; public DefaultExceptionDebugger( ConfigurationUpdater configurationUpdater, ClassNameFilter classNameFiltering, Duration captureInterval, - int maxExceptionPerSecond) { + int maxExceptionPerSecond, + int maxCapturedFrames) { this( new ExceptionProbeManager(classNameFiltering, captureInterval), configurationUpdater, classNameFiltering, - maxExceptionPerSecond); + maxExceptionPerSecond, + maxCapturedFrames); } DefaultExceptionDebugger( ExceptionProbeManager exceptionProbeManager, ConfigurationUpdater configurationUpdater, ClassNameFilter classNameFiltering, - int maxExceptionPerSecond) { + int maxExceptionPerSecond, + int maxCapturedFrames) { this.exceptionProbeManager = exceptionProbeManager; this.configurationUpdater = configurationUpdater; this.classNameFiltering = classNameFiltering; this.circuitBreaker = new CircuitBreaker(maxExceptionPerSecond, Duration.ofSeconds(1)); + this.maxCapturedFrames = maxCapturedFrames; } @Override @@ -91,7 +96,8 @@ public void handleException(Throwable t, AgentSpan span) { LOGGER.debug("Unable to find state for throwable: {}", innerMostException.toString()); return; } - processSnapshotsAndSetTags(t, span, state, chainedExceptionsList, fingerprint); + processSnapshotsAndSetTags( + t, span, state, chainedExceptionsList, fingerprint, maxCapturedFrames); exceptionProbeManager.updateLastCapture(fingerprint); } else { // climb up the exception chain to find the first exception that has instrumented frames @@ -128,7 +134,8 @@ private static void processSnapshotsAndSetTags( AgentSpan span, ThrowableState state, List chainedExceptions, - String fingerprint) { + String fingerprint, + int maxCapturedFrames) { if (span.getTag(DD_DEBUG_ERROR_EXCEPTION_ID) != null) { LOGGER.debug("Clear previous frame tags"); // already set for this span, clear the frame tags @@ -142,7 +149,8 @@ private static void processSnapshotsAndSetTags( } boolean snapshotAssigned = false; List snapshots = state.getSnapshots(); - for (int i = 0; i < snapshots.size(); i++) { + int maxSnapshotSize = Math.min(snapshots.size(), maxCapturedFrames); + for (int i = 0; i < maxSnapshotSize; i++) { Snapshot snapshot = snapshots.get(i); Throwable currentEx = chainedExceptions.get(snapshot.getChainedExceptionIdx()); int[] mapping = createThrowableMapping(currentEx, t); diff --git a/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/exception/DefaultExceptionDebuggerTest.java b/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/exception/DefaultExceptionDebuggerTest.java index aaabb300cc0..a399fee533b 100644 --- a/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/exception/DefaultExceptionDebuggerTest.java +++ b/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/exception/DefaultExceptionDebuggerTest.java @@ -67,7 +67,7 @@ public void setUp() { new HashSet<>(singletonList("com.datadog.debugger.exception.ThirdPartyCode"))); exceptionDebugger = new DefaultExceptionDebugger( - configurationUpdater, classNameFiltering, Duration.ofHours(1), 100); + configurationUpdater, classNameFiltering, Duration.ofHours(1), 100, 3); listener = new TestSnapshotListener(createConfig(), mock(ProbeStatusSink.class)); DebuggerAgentHelper.injectSink(listener); } @@ -275,7 +275,7 @@ public void nestedExceptionFullThirdParty() { public void filteringOutErrors() { ExceptionProbeManager manager = mock(ExceptionProbeManager.class); exceptionDebugger = - new DefaultExceptionDebugger(manager, configurationUpdater, classNameFiltering, 100); + new DefaultExceptionDebugger(manager, configurationUpdater, classNameFiltering, 100, 3); exceptionDebugger.handleException(new AssertionError("test"), mock(AgentSpan.class)); verify(manager, times(0)).isAlreadyInstrumented(any()); } 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 060b7b370c0..ed7500e0819 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 @@ -17,7 +17,6 @@ import com.datadog.debugger.agent.ClassesToRetransformFinder; import com.datadog.debugger.agent.Configuration; import com.datadog.debugger.agent.ConfigurationUpdater; -import com.datadog.debugger.agent.DebuggerAgent; import com.datadog.debugger.agent.DebuggerAgentHelper; import com.datadog.debugger.agent.DebuggerTransformer; import com.datadog.debugger.agent.JsonSnapshotSerializer; @@ -99,7 +98,10 @@ public void before() { ProbeRateLimiter.setSamplerSupplier(rate -> rate < 101 ? probeSampler : globalSampler); ProbeRateLimiter.setGlobalSnapshotRate(1000); // to activate the call to DebuggerContext.handleException - DebuggerAgent.startExceptionReplay(); + DebuggerContext.ProductConfigUpdater mockProductConfigUpdater = + mock(DebuggerContext.ProductConfigUpdater.class); + when(mockProductConfigUpdater.isExceptionReplayEnabled()).thenReturn(true); + DebuggerContext.initProductConfigUpdater(mockProductConfigUpdater); setFieldInConfig(Config.get(), "debuggerExceptionEnabled", true); setFieldInConfig(Config.get(), "dynamicInstrumentationClassFileDumpEnabled", true); } @@ -224,7 +226,8 @@ public void recursive() throws Exception { callMethodFiboException(testClass); // generate snapshots Map> probeIdsByMethodName = extractProbeIdsByMethodName(exceptionProbeManager); - assertEquals(10, listener.snapshots.size()); + // limited by Config::getDebuggerExceptionMaxCapturedFrames + assertEquals(3, listener.snapshots.size()); Snapshot snapshot0 = listener.snapshots.get(0); assertProbeId(probeIdsByMethodName, "fiboException", snapshot0.getProbe().getId()); assertEquals( @@ -234,8 +237,6 @@ public void recursive() throws Exception { assertEquals("2", getValue(snapshot1.getCaptures().getReturn().getArguments().get("n"))); Snapshot snapshot2 = listener.snapshots.get(2); assertEquals("3", getValue(snapshot2.getCaptures().getReturn().getArguments().get("n"))); - Snapshot snapshot9 = listener.snapshots.get(9); - assertEquals("10", getValue(snapshot9.getCaptures().getReturn().getArguments().get("n"))); // sampling happens only once ont he first snapshot then forced for coordinated sampling assertEquals(1, probeSampler.getCallCount()); assertEquals(1, globalSampler.getCallCount()); @@ -382,7 +383,7 @@ private TestSnapshotListener setupExceptionDebugging( DebuggerContext.initValueSerializer(new JsonSnapshotSerializer()); DefaultExceptionDebugger exceptionDebugger = new DefaultExceptionDebugger( - exceptionProbeManager, configurationUpdater, classNameFiltering, 100); + exceptionProbeManager, configurationUpdater, classNameFiltering, 100, 3); DebuggerContext.initExceptionDebugger(exceptionDebugger); configurationUpdater.accept(REMOTE_CONFIG, definitions); return listener; diff --git a/dd-smoke-tests/debugger-integration-tests/src/main/java/datadog/smoketest/debugger/ServerDebuggerTestApplication.java b/dd-smoke-tests/debugger-integration-tests/src/main/java/datadog/smoketest/debugger/ServerDebuggerTestApplication.java index 75ee4ee5ef3..6462849d025 100644 --- a/dd-smoke-tests/debugger-integration-tests/src/main/java/datadog/smoketest/debugger/ServerDebuggerTestApplication.java +++ b/dd-smoke-tests/debugger-integration-tests/src/main/java/datadog/smoketest/debugger/ServerDebuggerTestApplication.java @@ -153,6 +153,8 @@ private static void runTracedMethod(String arg) { tracedMethodWithDeepException1(42, "foobar", 3.42, map, "var1", "var2", "var3"); } else if ("lambdaOops".equals(arg)) { tracedMethodWithLambdaException(42, "foobar", 3.42, map, "var1", "var2", "var3"); + } else if ("recursiveOops".equals(arg)) { + tracedMethodWithRecursiveException(42, "foobar", 3.42, map, "var1", "var2", "var3"); } else { tracedMethod(42, "foobar", 3.42, map, "var1", "var2", "var3"); } @@ -239,6 +241,15 @@ private static void tracedMethodWithLambdaException( throw toRuntimeException("lambdaOops"); } + private static void tracedMethodWithRecursiveException( + int argInt, String argStr, double argDouble, Map argMap, String... argVar) { + if (argInt > 0) { + tracedMethodWithRecursiveException(argInt - 8, argStr, argDouble, argMap, argVar); + } else { + throw new RuntimeException("recursiveOops"); + } + } + private static RuntimeException toRuntimeException(String msg) { return toException(RuntimeException::new, msg); } diff --git a/dd-smoke-tests/debugger-integration-tests/src/test/java/datadog/smoketest/ExceptionDebuggerIntegrationTest.java b/dd-smoke-tests/debugger-integration-tests/src/test/java/datadog/smoketest/ExceptionDebuggerIntegrationTest.java index 3bd483c648a..f73ad478fcf 100644 --- a/dd-smoke-tests/debugger-integration-tests/src/test/java/datadog/smoketest/ExceptionDebuggerIntegrationTest.java +++ b/dd-smoke-tests/debugger-integration-tests/src/test/java/datadog/smoketest/ExceptionDebuggerIntegrationTest.java @@ -237,6 +237,51 @@ void test5CapturedFrames() throws Exception { }); } + @Test + @DisplayName("test3CapturedRecursiveFrames") + @DisabledIf( + value = "datadog.trace.api.Platform#isJ9", + disabledReason = "we cannot get local variable debug info") + void test3CapturedRecursiveFrames() throws Exception { + appUrl = startAppAndAndGetUrl(); + execute(appUrl, TRACED_METHOD_NAME, "recursiveOops"); // instrumenting first exception + waitForInstrumentation(appUrl); + execute(appUrl, TRACED_METHOD_NAME, "recursiveOops"); // collecting snapshots and sending them + registerTraceListener(this::receiveExceptionReplayTrace); + registerSnapshotListener(this::receiveSnapshot); + processRequests( + () -> { + if (snapshotIdTags.isEmpty()) { + return false; + } + if (traceReceived + && snapshotReceived + && snapshots.containsKey(snapshotIdTags.get(0)) + && snapshots.containsKey(snapshotIdTags.get(1)) + && snapshots.containsKey(snapshotIdTags.get(2))) { + assertEquals(3, snapshotIdTags.size()); + assertEquals(3, snapshots.size()); + // snapshot 0 + assertRecursiveSnapshot(snapshots.get(snapshotIdTags.get(0))); + // snapshot 1 + assertRecursiveSnapshot(snapshots.get(snapshotIdTags.get(1))); + // snapshot 2 + assertRecursiveSnapshot(snapshots.get(snapshotIdTags.get(2))); + return true; + } + return false; + }); + } + + private static void assertRecursiveSnapshot(Snapshot snapshot) { + assertNotNull(snapshot); + assertEquals( + "recursiveOops", snapshot.getCaptures().getReturn().getCapturedThrowable().getMessage()); + assertEquals( + "datadog.smoketest.debugger.ServerDebuggerTestApplication.tracedMethodWithRecursiveException", + snapshot.getStack().get(0).getFunction()); + } + @Test @DisplayName("testLambdaHiddenFrames") @DisabledIf(value = "datadog.trace.api.Platform#isJ9", disabledReason = "HotSpot specific test") diff --git a/dd-smoke-tests/debugger-integration-tests/src/test/java/datadog/smoketest/InProductEnablementIntegrationTest.java b/dd-smoke-tests/debugger-integration-tests/src/test/java/datadog/smoketest/InProductEnablementIntegrationTest.java index 1964fcaa7ce..d844492ea82 100644 --- a/dd-smoke-tests/debugger-integration-tests/src/test/java/datadog/smoketest/InProductEnablementIntegrationTest.java +++ b/dd-smoke-tests/debugger-integration-tests/src/test/java/datadog/smoketest/InProductEnablementIntegrationTest.java @@ -45,7 +45,7 @@ void testDynamicInstrumentationEnablementWithLineProbe() throws Exception { LogProbe probe = LogProbe.builder() .probeId(LINE_PROBE_ID1) - .where("ServerDebuggerTestApplication.java", 301) + .where("ServerDebuggerTestApplication.java", 312) .build(); setCurrentConfiguration(createConfig(probe)); waitForFeatureStarted(appUrl, "Dynamic Instrumentation");