From 2ca4cddaa677e5c070796fa545b2ce935ce53397 Mon Sep 17 00:00:00 2001 From: jean-philippe bempel Date: Thu, 21 Mar 2024 18:00:18 +0100 Subject: [PATCH 1/3] Use stack depth to do snapshot assignment Instead of relying solely on matching class and method name for snapshot assignment on exception debugging we are using the stack depth of the snapshot of the current frame. It allows to support recursive calls --- .../exception/DefaultExceptionDebugger.java | 23 +++- .../debugger/util/ExceptionHelper.java | 18 ++- .../debugger/agent/CapturedSnapshotTest.java | 78 ++--------- ...panDecorationProbeInstrumentationTest.java | 6 +- .../DefaultExceptionDebuggerTest.java | 125 +++++++++++++++--- .../ExceptionProbeInstrumentationTest.java | 49 ++++++- .../debugger/util/ExceptionHelperTest.java | 20 +++ .../util/MoshiSnapshotTestHelper.java | 55 ++++++++ .../datadog/debugger/CapturedSnapshot20.java | 14 ++ 9 files changed, 288 insertions(+), 100 deletions(-) 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 01bf7cdb9fe..9756d7be5a6 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 @@ -96,14 +96,11 @@ private static void processSnapshotsAndSetTags( List snapshots = state.getSnapshots(); for (int i = 0; i < snapshots.size(); i++) { Snapshot snapshot = snapshots.get(i); - String className = snapshot.getProbe().getLocation().getType(); - String methodName = snapshot.getProbe().getLocation().getMethod(); - while (currentIdx < innerTrace.length - && !innerTrace[currentIdx].getClassName().equals(className) - && !innerTrace[currentIdx].getMethodName().equals(methodName)) { - currentIdx++; + currentIdx = innerTrace.length - snapshot.getStack().size(); + if (!sanityCheckSnapshotAssignment(snapshot, innerTrace, currentIdx)) { + continue; } - int frameIndex = mapping[currentIdx++]; + int frameIndex = mapping[currentIdx]; if (frameIndex == -1) { continue; } @@ -124,6 +121,18 @@ private static void processSnapshotsAndSetTags( } } + private static boolean sanityCheckSnapshotAssignment( + Snapshot snapshot, StackTraceElement[] innerTrace, int currentIdx) { + String className = snapshot.getProbe().getLocation().getType(); + String methodName = snapshot.getProbe().getLocation().getMethod(); + if (!className.equals(innerTrace[currentIdx].getClassName()) + || !methodName.equals(innerTrace[currentIdx].getMethodName())) { + LOGGER.warn("issue when assigning snapshot to frame: {} {}", className, methodName); + return false; + } + return true; + } + ExceptionProbeManager getExceptionProbeManager() { return exceptionProbeManager; } diff --git a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/util/ExceptionHelper.java b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/util/ExceptionHelper.java index 4fff8558bb3..9c5ea74e119 100644 --- a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/util/ExceptionHelper.java +++ b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/util/ExceptionHelper.java @@ -98,16 +98,22 @@ private static void internalFlattenStackTrace( } public static int[] createThrowableMapping(Throwable innerMost, Throwable current) { - StackTraceElement[] trace = innerMost.getStackTrace(); + StackTraceElement[] innerTrace = innerMost.getStackTrace(); StackTraceElement[] currentTrace = flattenStackTrace(current); - int[] mapping = new int[trace.length]; - for (int i = 0; i < trace.length; i++) { + int[] mapping = new int[innerTrace.length]; + int currentIdx = 0; + for (int i = 0; i < innerTrace.length; i++) { mapping[i] = -1; - for (int j = 0; j < currentTrace.length; j++) { - if (trace[i].equals(currentTrace[j])) { - mapping[i] = j; + int count = currentTrace.length; + int idx = currentIdx; + while (count > 0) { + if (innerTrace[i].equals(currentTrace[idx])) { + mapping[i] = idx; + currentIdx = (idx + 1) % currentTrace.length; break; } + idx = (idx + 1) % currentTrace.length; + count--; } } return mapping; 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 a6dcd05dd92..17544dca957 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 @@ -6,6 +6,7 @@ import static com.datadog.debugger.util.MoshiSnapshotHelper.NOT_CAPTURED_REASON; 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; @@ -104,8 +105,6 @@ public class CapturedSnapshotTest { private static final ProbeId PROBE_ID2 = new ProbeId("beae1807-f3b0-4ea8-a74f-826790c5e6f7", 0); private static final ProbeId PROBE_ID3 = new ProbeId("beae1807-f3b0-4ea8-a74f-826790c5e6f8", 0); private static final String SERVICE_NAME = "service-name"; - private static final JsonAdapter VALUE_ADAPTER = - new MoshiSnapshotTestHelper.CapturedValueAdapter(); private static final JsonAdapter> GENERIC_ADAPTER = MoshiHelper.createGenericAdapter(); @@ -1033,9 +1032,10 @@ public void staticFieldCondition() throws IOException, URISyntaxException { Map staticFields = snapshot.getCaptures().getReturn().getStaticFields(); assertEquals(4, staticFields.size()); - assertEquals("foo", getValue(staticFields.get("strField"))); - assertEquals("1001", getValue(staticFields.get("intField"))); - assertEquals(String.valueOf(Math.PI), getValue(staticFields.get("doubleField"))); + assertEquals("foo", MoshiSnapshotTestHelper.getValue(staticFields.get("strField"))); + assertEquals("1001", MoshiSnapshotTestHelper.getValue(staticFields.get("intField"))); + assertEquals( + String.valueOf(Math.PI), MoshiSnapshotTestHelper.getValue(staticFields.get("doubleField"))); assertTrue(staticFields.containsKey("intArrayField")); } @@ -1366,11 +1366,11 @@ public void staticInheritedFields() throws IOException, URISyntaxException { Map staticFields = snapshot.getCaptures().getReturn().getStaticFields(); assertEquals(7, staticFields.size()); - assertEquals("barfoo", getValue(staticFields.get("strValue"))); - assertEquals("48", getValue(staticFields.get("intValue"))); - assertEquals("6.28", getValue(staticFields.get("doubleValue"))); - assertEquals("[1, 2, 3, 4]", getValue(staticFields.get("longValues"))); - assertEquals("[foo, bar]", getValue(staticFields.get("strValues"))); + assertEquals("barfoo", MoshiSnapshotTestHelper.getValue(staticFields.get("strValue"))); + assertEquals("48", MoshiSnapshotTestHelper.getValue(staticFields.get("intValue"))); + assertEquals("6.28", MoshiSnapshotTestHelper.getValue(staticFields.get("doubleValue"))); + assertEquals("[1, 2, 3, 4]", MoshiSnapshotTestHelper.getValue(staticFields.get("longValues"))); + assertEquals("[foo, bar]", MoshiSnapshotTestHelper.getValue(staticFields.get("strValues"))); } @Test @@ -2264,14 +2264,14 @@ private void assertCaptureArgs( CapturedContext context, String name, String typeName, String value) { CapturedContext.CapturedValue capturedValue = context.getArguments().get(name); assertEquals(typeName, capturedValue.getType()); - assertEquals(value, getValue(capturedValue)); + assertEquals(value, MoshiSnapshotTestHelper.getValue(capturedValue)); } private void assertCaptureLocals( CapturedContext context, String name, String typeName, String value) { CapturedContext.CapturedValue localVar = context.getLocals().get(name); assertEquals(typeName, localVar.getType()); - assertEquals(value, getValue(localVar)); + assertEquals(value, MoshiSnapshotTestHelper.getValue(localVar)); } private void assertCaptureLocals( @@ -2294,7 +2294,7 @@ private void assertCaptureFields( CapturedContext context, String name, String typeName, String value) { CapturedContext.CapturedValue field = context.getFields().get(name); assertEquals(typeName, field.getType()); - assertEquals(value, getValue(field)); + assertEquals(value, MoshiSnapshotTestHelper.getValue(field)); } private void assertCaptureFields( @@ -2330,7 +2330,7 @@ private void assertCaptureFieldCount(CapturedContext context, int expectedFieldC private void assertCaptureReturnValue(CapturedContext context, String typeName, String value) { CapturedContext.CapturedValue returnValue = context.getLocals().get("@return"); assertEquals(typeName, returnValue.getType()); - assertEquals(value, getValue(returnValue)); + assertEquals(value, MoshiSnapshotTestHelper.getValue(returnValue)); } private void assertCaptureReturnValue( @@ -2370,56 +2370,6 @@ private void assertCaptureThrowable( assertEquals(lineNumber, throwable.getStacktrace().get(0).getLineNumber()); } - private static String getValue(CapturedContext.CapturedValue capturedValue) { - CapturedContext.CapturedValue valued = null; - try { - valued = VALUE_ADAPTER.fromJson(capturedValue.getStrValue()); - if (valued.getNotCapturedReason() != null) { - Assertions.fail("NotCapturedReason: " + valued.getNotCapturedReason()); - } - Object obj = valued.getValue(); - if (obj != null && obj.getClass().isArray()) { - if (obj.getClass().getComponentType().isPrimitive()) { - return primitiveArrayToString(obj); - } - return Arrays.toString((Object[]) obj); - } - return obj != null ? String.valueOf(obj) : null; - } catch (IOException e) { - e.printStackTrace(); - return null; - } - } - - private static String primitiveArrayToString(Object obj) { - Class componentType = obj.getClass().getComponentType(); - if (componentType == long.class) { - return Arrays.toString((long[]) obj); - } - if (componentType == int.class) { - return Arrays.toString((int[]) obj); - } - if (componentType == short.class) { - return Arrays.toString((short[]) obj); - } - if (componentType == char.class) { - return Arrays.toString((char[]) obj); - } - if (componentType == byte.class) { - return Arrays.toString((byte[]) obj); - } - if (componentType == boolean.class) { - return Arrays.toString((boolean[]) obj); - } - if (componentType == float.class) { - return Arrays.toString((float[]) obj); - } - if (componentType == double.class) { - return Arrays.toString((double[]) obj); - } - return null; - } - public static Map getFields( CapturedContext.CapturedValue capturedValue) { try { diff --git a/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/SpanDecorationProbeInstrumentationTest.java b/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/SpanDecorationProbeInstrumentationTest.java index ac75591ef3f..ea91a721f41 100644 --- a/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/SpanDecorationProbeInstrumentationTest.java +++ b/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/agent/SpanDecorationProbeInstrumentationTest.java @@ -238,7 +238,7 @@ public void methodActiveSpanSynthException() throws IOException, URISyntaxExcept public void lineActiveSpanSimpleTag() throws IOException, URISyntaxException { final String CLASS_NAME = "com.datadog.debugger.CapturedSnapshot20"; SpanDecorationProbe.Decoration decoration = createDecoration("tag1", "{arg}"); - installSingleSpanDecoration(CLASS_NAME, ACTIVE, decoration, "CapturedSnapshot20.java", 44); + installSingleSpanDecoration(CLASS_NAME, ACTIVE, decoration, "CapturedSnapshot20.java", 47); Class testClass = compileAndLoadClass(CLASS_NAME); int result = Reflect.on(testClass).call("main", "1").get(); assertEquals(84, result); @@ -271,7 +271,7 @@ public void lineActiveSpanCondition() throws IOException, URISyntaxException { final String CLASS_NAME = "com.datadog.debugger.CapturedSnapshot20"; SpanDecorationProbe.Decoration decoration = createDecoration(eq(ref("arg"), value("5")), "arg == '5'", "tag1", "{arg}"); - installSingleSpanDecoration(CLASS_NAME, ACTIVE, decoration, "CapturedSnapshot20.java", 44); + installSingleSpanDecoration(CLASS_NAME, ACTIVE, decoration, "CapturedSnapshot20.java", 47); Class testClass = compileAndLoadClass(CLASS_NAME); for (int i = 0; i < 10; i++) { int result = Reflect.on(testClass).call("main", String.valueOf(i)).get(); @@ -287,7 +287,7 @@ public void lineActiveSpanInvalidCondition() throws IOException, URISyntaxExcept final String CLASS_NAME = "com.datadog.debugger.CapturedSnapshot20"; SpanDecorationProbe.Decoration decoration = createDecoration(eq(ref("noarg"), value("5")), "arg == '5'", "tag1", "{arg}"); - installSingleSpanDecoration(CLASS_NAME, ACTIVE, decoration, "CapturedSnapshot20.java", 44); + installSingleSpanDecoration(CLASS_NAME, ACTIVE, decoration, "CapturedSnapshot20.java", 47); Class testClass = compileAndLoadClass(CLASS_NAME); int result = Reflect.on(testClass).call("main", "5").get(); assertEquals(84, result); 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 671c7619036..ceb8b295c63 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 @@ -3,6 +3,7 @@ import static com.datadog.debugger.exception.DefaultExceptionDebugger.SNAPSHOT_ID_TAG_FMT; import static com.datadog.debugger.util.TestHelper.assertWithTimeout; import static java.util.Collections.emptyList; +import static java.util.stream.Collectors.toList; import static java.util.stream.Collectors.toMap; import static org.junit.jupiter.api.Assertions.*; import static org.mockito.ArgumentMatchers.any; @@ -24,6 +25,7 @@ import com.datadog.debugger.util.TestSnapshotListener; import datadog.trace.api.Config; import datadog.trace.bootstrap.debugger.CapturedContext; +import datadog.trace.bootstrap.debugger.CapturedStackFrame; import datadog.trace.bootstrap.debugger.MethodLocation; import datadog.trace.bootstrap.instrumentation.api.AgentSpan; import java.io.BufferedReader; @@ -33,6 +35,7 @@ import java.io.StringWriter; import java.time.Duration; import java.util.ArrayList; +import java.util.Arrays; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -40,6 +43,7 @@ import java.util.function.Function; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.mockito.invocation.InvocationOnMock; class DefaultExceptionDebuggerTest { @@ -47,6 +51,7 @@ class DefaultExceptionDebuggerTest { private ConfigurationUpdater configurationUpdater; private DefaultExceptionDebugger exceptionDebugger; private TestSnapshotListener listener; + private Map spanTags = new HashMap<>(); @BeforeEach public void setUp() { @@ -70,21 +75,24 @@ public void simpleException() { verify(configurationUpdater).accept(eq(ConfigurationAcceptor.Source.EXCEPTION), any()); } + @Test + public void doubleHandleException() { + RuntimeException exception = new RuntimeException("test"); + String fingerprint = Fingerprinter.fingerprint(exception, classNameFiltering); + AgentSpan span = mock(AgentSpan.class); + exceptionDebugger.handleException(exception, span); + assertWithTimeout( + () -> exceptionDebugger.getExceptionProbeManager().isAlreadyInstrumented(fingerprint), + Duration.ofSeconds(30)); + exceptionDebugger.handleException(exception, span); + verify(configurationUpdater).accept(eq(ConfigurationAcceptor.Source.EXCEPTION), any()); + } + @Test public void nestedException() { RuntimeException exception = createNestException(); AgentSpan span = mock(AgentSpan.class); - Map tags = new HashMap<>(); - doAnswer( - invocationOnMock -> { - Object[] args = invocationOnMock.getArguments(); - String key = (String) args[0]; - String value = (String) args[1]; - tags.put(key, value); - return null; - }) - .when(span) - .setTag(anyString(), anyString()); + doAnswer(this::recordTags).when(span).setTag(anyString(), anyString()); exceptionDebugger.handleException(exception, span); generateSnapshots(exception); exception.printStackTrace(); @@ -94,7 +102,7 @@ public void nestedException() { .getExceptionProbeManager() .getSateByThrowable(ExceptionHelper.getInnerMostThrowable(exception)); assertEquals( - state.getExceptionId(), tags.get(DefaultExceptionDebugger.DD_DEBUG_ERROR_EXCEPTION_ID)); + state.getExceptionId(), spanTags.get(DefaultExceptionDebugger.DD_DEBUG_ERROR_EXCEPTION_ID)); Map snapshotMap = listener.snapshots.stream().collect(toMap(Snapshot::getId, Function.identity())); List lines = parseStackTrace(exception); @@ -103,7 +111,7 @@ public void nestedException() { lines, "com.datadog.debugger.exception.DefaultExceptionDebuggerTest.createNestException"); assertSnapshot( - tags, + spanTags, snapshotMap, expectedFrameIndex, "com.datadog.debugger.exception.DefaultExceptionDebuggerTest", @@ -112,7 +120,7 @@ public void nestedException() { findFrameIndex( lines, "com.datadog.debugger.exception.DefaultExceptionDebuggerTest.nestedException"); assertSnapshot( - tags, + spanTags, snapshotMap, expectedFrameIndex, "com.datadog.debugger.exception.DefaultExceptionDebuggerTest", @@ -122,13 +130,80 @@ public void nestedException() { lines, "com.datadog.debugger.exception.DefaultExceptionDebuggerTest.createTest1Exception"); assertSnapshot( - tags, + spanTags, snapshotMap, expectedFrameIndex, "com.datadog.debugger.exception.DefaultExceptionDebuggerTest", "createTest1Exception"); } + @Test + public void doubleNestedException() { + RuntimeException nestedException = createNestException(); + RuntimeException simpleException = new RuntimeException("test"); + AgentSpan span = mock(AgentSpan.class); + doAnswer(this::recordTags).when(span).setTag(anyString(), anyString()); + when(span.getTag(anyString())) + .thenAnswer(invocationOnMock -> spanTags.get(invocationOnMock.getArgument(0))); + when(span.getTags()).thenReturn(spanTags); + // instrument first nested Exception + exceptionDebugger.handleException(nestedException, span); + // instrument first simple Exception + exceptionDebugger.handleException(simpleException, span); + generateSnapshots(nestedException); + generateSnapshots(simpleException); + exceptionDebugger.handleException(simpleException, span); + nestedException.printStackTrace(); + exceptionDebugger.handleException(nestedException, span); + ExceptionProbeManager.ThrowableState state = + exceptionDebugger + .getExceptionProbeManager() + .getSateByThrowable(ExceptionHelper.getInnerMostThrowable(nestedException)); + assertEquals( + state.getExceptionId(), spanTags.get(DefaultExceptionDebugger.DD_DEBUG_ERROR_EXCEPTION_ID)); + Map snapshotMap = + listener.snapshots.stream().collect(toMap(Snapshot::getId, Function.identity())); + List lines = parseStackTrace(nestedException); + int expectedFrameIndex = + findFrameIndex( + lines, + "com.datadog.debugger.exception.DefaultExceptionDebuggerTest.createNestException"); + assertSnapshot( + spanTags, + snapshotMap, + expectedFrameIndex, + "com.datadog.debugger.exception.DefaultExceptionDebuggerTest", + "createNestException"); + expectedFrameIndex = + findFrameIndex( + lines, + "com.datadog.debugger.exception.DefaultExceptionDebuggerTest.doubleNestedException"); + assertSnapshot( + spanTags, + snapshotMap, + expectedFrameIndex, + "com.datadog.debugger.exception.DefaultExceptionDebuggerTest", + "doubleNestedException"); + expectedFrameIndex = + findFrameIndex( + lines, + "com.datadog.debugger.exception.DefaultExceptionDebuggerTest.createTest1Exception"); + assertSnapshot( + spanTags, + snapshotMap, + expectedFrameIndex, + "com.datadog.debugger.exception.DefaultExceptionDebuggerTest", + "createTest1Exception"); + } + + private Object recordTags(InvocationOnMock invocationOnMock) { + Object[] args = invocationOnMock.getArguments(); + String key = (String) args[0]; + String value = (String) args[1]; + spanTags.put(key, value); + return null; + } + private static int findFrameIndex(List lines, String str) { for (int i = 0; i < lines.size(); i++) { if (lines.get(i).contains(str)) { @@ -158,12 +233,12 @@ private static List parseStackTrace(RuntimeException exception) { } private static void assertSnapshot( - Map tags, + Map tags, Map snapshotMap, int frameIndex, String className, String methodName) { - String snapshotId = tags.get(String.format(SNAPSHOT_ID_TAG_FMT, frameIndex)); + String snapshotId = (String) tags.get(String.format(SNAPSHOT_ID_TAG_FMT, frameIndex)); Snapshot snapshot = snapshotMap.get(snapshotId); assertEquals(className, snapshot.getProbe().getLocation().getType()); assertEquals(methodName, snapshot.getProbe().getLocation().getMethod()); @@ -184,7 +259,9 @@ private void generateSnapshots(RuntimeException exception) { Function.identity(), dropMerger)); Throwable innerMost = ExceptionHelper.getInnerMostThrowable(exception); + int framesToRemove = -1; for (StackTraceElement element : innerMost.getStackTrace()) { + framesToRemove++; ExceptionProbe exceptionProbe = probesByLocation.get( element.getClassName() @@ -205,6 +282,20 @@ private void generateSnapshots(RuntimeException exception) { System.currentTimeMillis(), MethodLocation.EXIT); exceptionProbe.commit(CapturedContext.EMPTY_CAPTURING_CONTEXT, capturedContext, emptyList()); + // rewrite snapshot stacktrace + ExceptionProbeManager.ThrowableState state = + exceptionDebugger + .getExceptionProbeManager() + .getSateByThrowable(ExceptionHelper.getInnerMostThrowable(exception)); + Snapshot lastSnapshot = state.getSnapshots().get(state.getSnapshots().size() - 1); + lastSnapshot.getStack().clear(); + lastSnapshot + .getStack() + .addAll( + Arrays.stream(innerMost.getStackTrace()) + .skip(framesToRemove) + .map(CapturedStackFrame::from) + .collect(toList())); } } 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 5932268c404..085ac4cb0c3 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 @@ -4,6 +4,7 @@ import static com.datadog.debugger.exception.DefaultExceptionDebugger.DD_DEBUG_ERROR_EXCEPTION_ID; import static com.datadog.debugger.exception.DefaultExceptionDebugger.ERROR_DEBUG_INFO_CAPTURED; import static com.datadog.debugger.exception.DefaultExceptionDebugger.SNAPSHOT_ID_TAG_FMT; +import static com.datadog.debugger.util.MoshiSnapshotTestHelper.getValue; import static com.datadog.debugger.util.TestHelper.assertWithTimeout; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -161,6 +162,37 @@ public void differentExceptionsSameStack() throws Exception { assertEquals(snapshot1.getId(), span1.getTags().get(String.format(SNAPSHOT_ID_TAG_FMT, 0))); } + @Test + public void recursive() throws Exception { + Config config = createConfig(); + ExceptionProbeManager exceptionProbeManager = new ExceptionProbeManager(classNameFiltering); + TestSnapshotListener listener = + setupExceptionDebugging(config, exceptionProbeManager, classNameFiltering); + final String CLASS_NAME = "com.datadog.debugger.CapturedSnapshot20"; + Class testClass = compileAndLoadClass(CLASS_NAME); + // instrument RuntimeException stacktrace + String fingerprint = callMethodFiboException(testClass); + assertWithTimeout( + () -> exceptionProbeManager.isAlreadyInstrumented(fingerprint), Duration.ofSeconds(30)); + assertEquals(11, exceptionProbeManager.getProbes().size()); + callMethodFiboException(testClass); // generate snapshots + Map> probeIdsByMethodName = + extractProbeIdsByMethodName(exceptionProbeManager); + assertEquals(10, listener.snapshots.size()); + Snapshot snapshot0 = listener.snapshots.get(0); + assertProbeId(probeIdsByMethodName, "fiboException", snapshot0.getProbe().getId()); + assertEquals( + "oops fibo", snapshot0.getCaptures().getReturn().getCapturedThrowable().getMessage()); + assertTrue(snapshot0.getCaptures().getReturn().getLocals().containsKey("@exception")); + assertEquals("1", getValue(snapshot0.getCaptures().getReturn().getArguments().get("n"))); + Snapshot snapshot1 = listener.snapshots.get(1); + 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"))); + } + private static void assertExceptionMsg(String expectedMsg, Snapshot snapshot) { assertEquals( expectedMsg, snapshot.getCaptures().getReturn().getCapturedThrowable().getMessage()); @@ -174,7 +206,7 @@ private static void assertProbeId( private String callMethodThrowingRuntimeException(Class testClass) { try { - Reflect.on(testClass).call("main", "exception").get(); + Reflect.onClass(testClass).call("main", "exception").get(); Assertions.fail("should not reach this code"); } catch (RuntimeException ex) { assertEquals("oops", ex.getCause().getCause().getMessage()); @@ -185,7 +217,7 @@ private String callMethodThrowingRuntimeException(Class testClass) { private String callMethodThrowingIllegalArgException(Class testClass) { try { - Reflect.on(testClass).call("main", "illegal").get(); + Reflect.onClass(testClass).call("main", "illegal").get(); Assertions.fail("should not reach this code"); } catch (RuntimeException ex) { assertEquals("illegal argument", ex.getCause().getCause().getMessage()); @@ -195,10 +227,21 @@ private String callMethodThrowingIllegalArgException(Class testClass) { } private static void callMethodNoException(Class testClass) { - int result = Reflect.on(testClass).call("main", "1").get(); + int result = Reflect.onClass(testClass).call("main", "1").get(); assertEquals(84, result); } + private String callMethodFiboException(Class testClass) { + try { + Reflect.onClass(testClass).call("main", "recursive").get(); + Assertions.fail("should not reach this code"); + } catch (RuntimeException ex) { + assertEquals("oops fibo", ex.getCause().getCause().getMessage()); + return Fingerprinter.fingerprint(ex, classNameFiltering); + } + return null; + } + private static Map> extractProbeIdsByMethodName( ExceptionProbeManager exceptionProbeManager) { return exceptionProbeManager.getProbes().stream() diff --git a/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/util/ExceptionHelperTest.java b/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/util/ExceptionHelperTest.java index ad3825896f9..0a3c4e60725 100644 --- a/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/util/ExceptionHelperTest.java +++ b/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/util/ExceptionHelperTest.java @@ -122,6 +122,26 @@ public void createThrowableMapping() { } } + @Test + public void createThrowableMappingRecursive() { + Throwable nestedException = createNestExceptionRecursive(4); + Throwable innerMostThrowable = ExceptionHelper.getInnerMostThrowable(nestedException); + int[] mapping = ExceptionHelper.createThrowableMapping(innerMostThrowable, nestedException); + StackTraceElement[] flattenedTrace = ExceptionHelper.flattenStackTrace(nestedException); + for (int i = 0; i < mapping.length; i++) { + assertEquals( + flattenedTrace[mapping[i]].getClassName(), + innerMostThrowable.getStackTrace()[i].getClassName()); + } + } + + private RuntimeException createNestExceptionRecursive(int depth) { + if (depth == 0) { + return createNestException(); + } + return createNestExceptionRecursive(depth - 1); + } + private RuntimeException createNestException() { return new RuntimeException("test3", createTest2Exception(createTest1Exception())); } diff --git a/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/util/MoshiSnapshotTestHelper.java b/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/util/MoshiSnapshotTestHelper.java index 4623ec9c17a..9805504abe6 100644 --- a/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/util/MoshiSnapshotTestHelper.java +++ b/dd-java-agent/agent-debugger/src/test/java/com/datadog/debugger/util/MoshiSnapshotTestHelper.java @@ -36,14 +36,40 @@ import java.lang.annotation.Annotation; import java.lang.reflect.Type; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Set; +import org.junit.jupiter.api.Assertions; public class MoshiSnapshotTestHelper { + public static final JsonAdapter VALUE_ADAPTER = + new MoshiSnapshotTestHelper.CapturedValueAdapter(); + + public static String getValue(CapturedContext.CapturedValue capturedValue) { + CapturedContext.CapturedValue valued = null; + try { + valued = VALUE_ADAPTER.fromJson(capturedValue.getStrValue()); + if (valued.getNotCapturedReason() != null) { + Assertions.fail("NotCapturedReason: " + valued.getNotCapturedReason()); + } + Object obj = valued.getValue(); + if (obj != null && obj.getClass().isArray()) { + if (obj.getClass().getComponentType().isPrimitive()) { + return primitiveArrayToString(obj); + } + return Arrays.toString((Object[]) obj); + } + return obj != null ? String.valueOf(obj) : null; + } catch (IOException e) { + e.printStackTrace(); + return null; + } + } + public static class SnapshotJsonFactory implements JsonAdapter.Factory { @Override public JsonAdapter create(Type type, Set set, Moshi moshi) { @@ -74,6 +100,35 @@ public static Moshi createMoshiSnapshot() { .build(); } + private static String primitiveArrayToString(Object obj) { + Class componentType = obj.getClass().getComponentType(); + if (componentType == long.class) { + return Arrays.toString((long[]) obj); + } + if (componentType == int.class) { + return Arrays.toString((int[]) obj); + } + if (componentType == short.class) { + return Arrays.toString((short[]) obj); + } + if (componentType == char.class) { + return Arrays.toString((char[]) obj); + } + if (componentType == byte.class) { + return Arrays.toString((byte[]) obj); + } + if (componentType == boolean.class) { + return Arrays.toString((boolean[]) obj); + } + if (componentType == float.class) { + return Arrays.toString((float[]) obj); + } + if (componentType == double.class) { + return Arrays.toString((double[]) obj); + } + return null; + } + private static class CapturesAdapter extends MoshiSnapshotHelper.CapturesAdapter { public CapturesAdapter(Moshi moshi, JsonAdapter capturedContextAdapter) { diff --git a/dd-java-agent/agent-debugger/src/test/resources/com/datadog/debugger/CapturedSnapshot20.java b/dd-java-agent/agent-debugger/src/test/resources/com/datadog/debugger/CapturedSnapshot20.java index 6fea461d5fe..fce2851290d 100644 --- a/dd-java-agent/agent-debugger/src/test/resources/com/datadog/debugger/CapturedSnapshot20.java +++ b/dd-java-agent/agent-debugger/src/test/resources/com/datadog/debugger/CapturedSnapshot20.java @@ -30,6 +30,9 @@ public static int main(String arg) { if (arg.equals("exception") || arg.equals("illegal")) { return new CapturedSnapshot20().processWithException(arg); } + if (arg.equals("recursive")) { + return new CapturedSnapshot20().fiboException(10); + } return new CapturedSnapshot20().process(arg); } catch (Exception ex) { span.addThrowable(ex); @@ -51,4 +54,15 @@ private int processWithException(String arg) { } throw new RuntimeException("oops"); } + + private int fiboException(int n) { + if (n <= 1) { + throw new RuntimeException("oops fibo"); + } + try { + return fiboException(n - 1) + fiboException(n - 2); + } catch (Exception ex) { + throw new RuntimeException(ex.getMessage(), ex); + } + } } From d04e6ee42bfb85268057d9e91111c7bd65f8a6ca Mon Sep 17 00:00:00 2001 From: jean-philippe bempel Date: Wed, 3 Apr 2024 08:05:29 +0200 Subject: [PATCH 2/3] Added some comments --- .../datadog/debugger/exception/DefaultExceptionDebugger.java | 3 +-- .../main/java/com/datadog/debugger/util/ExceptionHelper.java | 2 ++ 2 files changed, 3 insertions(+), 2 deletions(-) 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 9756d7be5a6..a7a15d0f340 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 @@ -91,12 +91,11 @@ private static void processSnapshotsAndSetTags( } int[] mapping = createThrowableMapping(innerMostException, t); StackTraceElement[] innerTrace = innerMostException.getStackTrace(); - int currentIdx = 0; boolean snapshotAssigned = false; List snapshots = state.getSnapshots(); for (int i = 0; i < snapshots.size(); i++) { Snapshot snapshot = snapshots.get(i); - currentIdx = innerTrace.length - snapshot.getStack().size(); + int currentIdx = innerTrace.length - snapshot.getStack().size(); if (!sanityCheckSnapshotAssignment(snapshot, innerTrace, currentIdx)) { continue; } diff --git a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/util/ExceptionHelper.java b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/util/ExceptionHelper.java index 9c5ea74e119..f827bf28428 100644 --- a/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/util/ExceptionHelper.java +++ b/dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/util/ExceptionHelper.java @@ -97,6 +97,8 @@ private static void internalFlattenStackTrace( } } + // Because flattened stack traces are organized with first frames at the bottom I need to follow + // the order of the first frame and wrap around the array with a modulo to continue the matching public static int[] createThrowableMapping(Throwable innerMost, Throwable current) { StackTraceElement[] innerTrace = innerMost.getStackTrace(); StackTraceElement[] currentTrace = flattenStackTrace(current); From 34c20c00906f4ffd32e523c18672947730e6d3d3 Mon Sep 17 00:00:00 2001 From: jean-philippe bempel Date: Thu, 4 Apr 2024 10:58:25 +0200 Subject: [PATCH 3/3] disabled test for j9 --- .../debugger/exception/ExceptionProbeInstrumentationTest.java | 4 ++++ 1 file changed, 4 insertions(+) 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 085ac4cb0c3..76bb3944573 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 @@ -44,6 +44,7 @@ import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.condition.DisabledIf; public class ExceptionProbeInstrumentationTest { private final Instrumentation instr = ByteBuddyAgent.install(); @@ -163,6 +164,9 @@ public void differentExceptionsSameStack() throws Exception { } @Test + @DisabledIf( + value = "datadog.trace.api.Platform#isJ9", + disabledReason = "Bug in J9: no LocalVariableTable for ClassFileTransformer") public void recursive() throws Exception { Config config = createConfig(); ExceptionProbeManager exceptionProbeManager = new ExceptionProbeManager(classNameFiltering);