Skip to content

Commit

Permalink
Make Exception debugging instrumentation async
Browse files Browse the repository at this point in the history 
Rename config option for exception debugging from
DD_DYNAMIC_INSTRUMENTATION_EXCEPTION_ENABLED to
DD_EXCEPTION_DEBUGGING_ENABLED

Fix frame index following the change in Error Tracking UI
  • Loading branch information
@jpbempel
jpbempel committed Mar 21, 2024
1 parent 97283c4 commit 1679925
Show file tree
Hide file tree
Showing 9 changed files with 64 additions and 32 deletions.
9 changes: 7 additions & 2 deletions ...agent-debugger/src/main/java/com/datadog/debugger/exception/DefaultExceptionDebugger.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import com.datadog.debugger.util.ExceptionHelper;
import datadog.trace.bootstrap.debugger.DebuggerContext;
import datadog.trace.bootstrap.instrumentation.api.AgentSpan;
import datadog.trace.util.AgentTaskScheduler;
import java.util.List;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand Down Expand Up @@ -66,11 +67,15 @@ public void handleException(Throwable t, AgentSpan span) {
} else {
exceptionProbeManager.createProbesForException(
fingerprint, innerMostException.getStackTrace());
// TODO make it async
configurationUpdater.accept(EXCEPTION, exceptionProbeManager.getProbes());
AgentTaskScheduler.INSTANCE.execute(() -> applyExceptionConfiguration(fingerprint));
}
}

private void applyExceptionConfiguration(String fingerprint) {
configurationUpdater.accept(EXCEPTION, exceptionProbeManager.getProbes());
exceptionProbeManager.addFingerprint(fingerprint);
}

private static void processSnapshotsAndSetTags(
Throwable t, AgentSpan span, ThrowableState state, Throwable innerMostException) {
if (span.getTag(DD_DEBUG_ERROR_EXCEPTION_ID) != null) {
Expand Down
7 changes: 3 additions & 4 deletions ...nt/agent-debugger/src/main/java/com/datadog/debugger/exception/ExceptionProbeManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,9 @@ public void createProbesForException(String fingerprint, StackTraceElement[] sta
ExceptionProbe probe = createMethodProbe(this, where);
probes.putIfAbsent(probe.getId(), probe);
}
}

void addFingerprint(String fingerprint) {
fingerprints.add(fingerprint);
}

Expand All @@ -78,10 +81,6 @@ public boolean shouldCaptureException(String fingerprint) {
return fingerprints.contains(fingerprint);
}

boolean containsFingerprint(String fingerprint) {
return fingerprints.contains(fingerprint);
}

public void addSnapshot(Snapshot snapshot) {
Throwable throwable = snapshot.getCaptures().getReturn().getCapturedThrowable().getThrowable();
throwable = ExceptionHelper.getInnerMostThrowable(throwable);
Expand Down
6 changes: 1 addition & 5 deletions dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/util/ExceptionHelper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ public static Throwable getInnerMostThrowable(Throwable t) {

public static StackTraceElement[] flattenStackTrace(Throwable t) {
List<StackTraceElement> result = new ArrayList<>();
result.add(null); // add a stack frame representing the exception message
// result.add(null); // add a stack frame representing the exception message
result.addAll(Arrays.asList(t.getStackTrace()));
if (t.getCause() != null) {
internalFlattenStackTrace(t.getCause(), t.getStackTrace(), result);
Expand All @@ -90,13 +90,9 @@ private static void internalFlattenStackTrace(
m--;
n--;
}
elements.add(null); // add a stack frame representing the exception message
for (int i = 0; i <= m; i++) {
elements.add(trace[i]);
}
if (trace.length - 1 - m > 0) {
elements.add(null); // add a stack frame representing number of common frames ("... n more")
}
if (t.getCause() != null) {
internalFlattenStackTrace(t.getCause(), trace, elements);
}
Expand Down
16 changes: 10 additions & 6 deletions ...t-debugger/src/test/java/com/datadog/debugger/exception/DefaultExceptionDebuggerTest.java
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package com.datadog.debugger.exception;

import static com.datadog.debugger.exception.DefaultExceptionDebugger.SNAPSHOT_ID_TAG_FMT;
import static com.datadog.debugger.exception.ExceptionProbeManagerTest.waitForInstrumentation;
import static java.util.Collections.emptyList;
import static java.util.stream.Collectors.toMap;
import static org.junit.jupiter.api.Assertions.*;
Expand Down Expand Up @@ -36,6 +37,7 @@
import java.util.Map;
import java.util.function.BinaryOperator;
import java.util.function.Function;
import java.util.regex.Pattern;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

Expand All @@ -61,8 +63,8 @@ public void simpleException() {
String fingerprint = Fingerprinter.fingerprint(exception, classNameFiltering);
AgentSpan span = mock(AgentSpan.class);
exceptionDebugger.handleException(exception, span);
waitForInstrumentation(exceptionDebugger.getExceptionProbeManager(), fingerprint);
exceptionDebugger.handleException(exception, span);
assertTrue(exceptionDebugger.getExceptionProbeManager().isAlreadyInstrumented(fingerprint));
verify(configurationUpdater).accept(eq(ConfigurationAcceptor.Source.EXCEPTION), any());
}

Expand Down Expand Up @@ -95,7 +97,7 @@ public void nestedException() {
listener.snapshots.stream().collect(toMap(Snapshot::getId, Function.identity()));
List<String> lines = parseStackTrace(exception);
int expectedFrameIndex =
findLine(
findFrameIndex(
lines,
"com.datadog.debugger.exception.DefaultExceptionDebuggerTest.createNestException");
assertSnapshot(
Expand All @@ -105,7 +107,7 @@ public void nestedException() {
"com.datadog.debugger.exception.DefaultExceptionDebuggerTest",
"createNestException");
expectedFrameIndex =
findLine(
findFrameIndex(
lines, "com.datadog.debugger.exception.DefaultExceptionDebuggerTest.nestedException");
assertSnapshot(
tags,
Expand All @@ -114,7 +116,7 @@ public void nestedException() {
"com.datadog.debugger.exception.DefaultExceptionDebuggerTest",
"nestedException");
expectedFrameIndex =
findLine(
findFrameIndex(
lines,
"com.datadog.debugger.exception.DefaultExceptionDebuggerTest.createTest1Exception");
assertSnapshot(
Expand All @@ -125,7 +127,7 @@ public void nestedException() {
"createTest1Exception");
}

private static int findLine(List<String> lines, String str) {
private static int findFrameIndex(List<String> lines, String str) {
for (int i = 0; i < lines.size(); i++) {
if (lines.get(i).contains(str)) {
return i;
Expand All @@ -143,7 +145,9 @@ private static List<String> parseStackTrace(RuntimeException exception) {
try {
String line;
while ((line = reader.readLine()) != null) {
results.add(line);
if (line.startsWith("\tat ")) {
results.add(line);
}
}
} catch (IOException e) {
e.printStackTrace();
Expand Down
29 changes: 20 additions & 9 deletions ...ugger/src/test/java/com/datadog/debugger/exception/ExceptionProbeInstrumentationTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.exception.ExceptionProbeManagerTest.waitForInstrumentation;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.Mockito.mock;
Expand Down Expand Up @@ -77,7 +78,9 @@ public void onlyInstrument() throws Exception {
setupExceptionDebugging(config, exceptionProbeManager, classNameFiltering);
final String CLASS_NAME = "com.datadog.debugger.CapturedSnapshot20";
Class<?> testClass = compileAndLoadClass(CLASS_NAME);
callMethodThrowingRuntimeException(testClass); // instrument exception stacktrace
String fingerprint =
callMethodThrowingRuntimeException(testClass); // instrument exception stacktrace
waitForInstrumentation(exceptionProbeManager, fingerprint);
assertEquals(2, exceptionProbeManager.getProbes().size());
callMethodNoException(testClass);
assertEquals(0, listener.snapshots.size());
Expand All @@ -91,7 +94,9 @@ public void instrumentAndCaptureSnapshots() throws Exception {
setupExceptionDebugging(config, exceptionProbeManager, classNameFiltering);
final String CLASS_NAME = "com.datadog.debugger.CapturedSnapshot20";
Class<?> testClass = compileAndLoadClass(CLASS_NAME);
callMethodThrowingRuntimeException(testClass); // instrument exception stacktrace
String fingerprint =
callMethodThrowingRuntimeException(testClass); // instrument exception stacktrace
waitForInstrumentation(exceptionProbeManager, fingerprint);
assertEquals(2, exceptionProbeManager.getProbes().size());
callMethodThrowingRuntimeException(testClass); // generate snapshots
Map<String, Set<String>> probeIdsByMethodName =
Expand All @@ -107,7 +112,7 @@ public void instrumentAndCaptureSnapshots() throws Exception {
MutableSpan span = traceInterceptor.getFirstSpan();
assertEquals(snapshot0.getExceptionId(), span.getTags().get(DD_DEBUG_ERROR_EXCEPTION_ID));
assertEquals(Boolean.TRUE, span.getTags().get(ERROR_DEBUG_INFO_CAPTURED));
assertEquals(snapshot0.getId(), span.getTags().get(String.format(SNAPSHOT_ID_TAG_FMT, 1)));
assertEquals(snapshot0.getId(), span.getTags().get(String.format(SNAPSHOT_ID_TAG_FMT, 0)));
}

@Test
Expand All @@ -119,10 +124,12 @@ public void differentExceptionsSameStack() throws Exception {
final String CLASS_NAME = "com.datadog.debugger.CapturedSnapshot20";
Class<?> testClass = compileAndLoadClass(CLASS_NAME);
// instrument RuntimeException stacktrace
callMethodThrowingRuntimeException(testClass);
String fingerprint0 = callMethodThrowingRuntimeException(testClass);
waitForInstrumentation(exceptionProbeManager, fingerprint0);
assertEquals(2, exceptionProbeManager.getProbes().size());
// instrument IllegalArgumentException stacktrace
callMethodThrowingIllegalArgException(testClass);
String fingerprint1 = callMethodThrowingIllegalArgException(testClass);
waitForInstrumentation(exceptionProbeManager, fingerprint1);
assertEquals(4, exceptionProbeManager.getProbes().size());
Map<String, Set<String>> probeIdsByMethodName =
extractProbeIdsByMethodName(exceptionProbeManager);
Expand All @@ -142,11 +149,11 @@ public void differentExceptionsSameStack() throws Exception {
MutableSpan span0 = traceInterceptor.getAllTraces().get(0).get(0);
assertEquals(snapshot0.getExceptionId(), span0.getTags().get(DD_DEBUG_ERROR_EXCEPTION_ID));
assertEquals(Boolean.TRUE, span0.getTags().get(ERROR_DEBUG_INFO_CAPTURED));
assertEquals(snapshot0.getId(), span0.getTags().get(String.format(SNAPSHOT_ID_TAG_FMT, 1)));
assertEquals(snapshot0.getId(), span0.getTags().get(String.format(SNAPSHOT_ID_TAG_FMT, 0)));
MutableSpan span1 = traceInterceptor.getAllTraces().get(1).get(0);
assertEquals(snapshot1.getExceptionId(), span1.getTags().get(DD_DEBUG_ERROR_EXCEPTION_ID));
assertEquals(Boolean.TRUE, span1.getTags().get(ERROR_DEBUG_INFO_CAPTURED));
assertEquals(snapshot1.getId(), span1.getTags().get(String.format(SNAPSHOT_ID_TAG_FMT, 1)));
assertEquals(snapshot1.getId(), span1.getTags().get(String.format(SNAPSHOT_ID_TAG_FMT, 0)));
}

private static void assertExceptionMsg(String expectedMsg, Snapshot snapshot) {
Expand All @@ -160,22 +167,26 @@ private static void assertProbeId(
assertTrue(probeIdsByMethodName.get(methodName).contains(id));
}

private static void callMethodThrowingRuntimeException(Class<?> testClass) {
private String callMethodThrowingRuntimeException(Class<?> testClass) {
try {
Reflect.on(testClass).call("main", "exception").get();
Assertions.fail("should not reach this code");
} catch (RuntimeException ex) {
assertEquals("oops", ex.getCause().getCause().getMessage());
return Fingerprinter.fingerprint(ex, classNameFiltering);
}
return null;
}

private static void callMethodThrowingIllegalArgException(Class<?> testClass) {
private String callMethodThrowingIllegalArgException(Class<?> testClass) {
try {
Reflect.on(testClass).call("main", "illegal").get();
Assertions.fail("should not reach this code");
} catch (RuntimeException ex) {
assertEquals("illegal argument", ex.getCause().getCause().getMessage());
return Fingerprinter.fingerprint(ex, classNameFiltering);
}
return null;
}

private static void callMethodNoException(Class<?> testClass) {
Expand Down
20 changes: 18 additions & 2 deletions ...gent-debugger/src/test/java/com/datadog/debugger/exception/ExceptionProbeManagerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

import com.datadog.debugger.probe.ExceptionProbe;
import com.datadog.debugger.util.ClassNameFiltering;
import java.time.Duration;
import java.util.Arrays;
import java.util.Collections;
import org.junit.jupiter.api.Test;
Expand All @@ -18,7 +19,7 @@ public void instrumentStackTrace() {
RuntimeException exception = new RuntimeException("test");
String fingerprint = Fingerprinter.fingerprint(exception, classNameFiltering);
exceptionProbeManager.createProbesForException(fingerprint, exception.getStackTrace());
assertTrue(exceptionProbeManager.isAlreadyInstrumented(fingerprint));
assertFalse(exceptionProbeManager.getProbes().isEmpty());
}

@Test
Expand All @@ -28,7 +29,7 @@ void instrumentSingleFrame() {
ExceptionProbeManager exceptionProbeManager = new ExceptionProbeManager(classNameFiltering);

String fingerprint = Fingerprinter.fingerprint(exception, classNameFiltering);
assertEquals("aa4a4dd768f6ef0fcc2b39a3bdedcbe44baff2e9dd0a779228db7bd8bf58", fingerprint);
assertEquals("ca4d9f3a1033d7262a89855f4b5cbdc225ed63c592c6cdf83fc5a88589e5fb", fingerprint);
exceptionProbeManager.createProbesForException(fingerprint, exception.getStackTrace());
assertEquals(1, exceptionProbeManager.getProbes().size());
ExceptionProbe exceptionProbe = exceptionProbeManager.getProbes().iterator().next();
Expand All @@ -52,4 +53,19 @@ void filterAllFrames() {
exceptionProbeManager.createProbesForException("", exception.getStackTrace());
assertEquals(0, exceptionProbeManager.getProbes().size());
}

static void waitForInstrumentation(
ExceptionProbeManager exceptionProbeManager, String fingerprint) {
Duration timeout = Duration.ofSeconds(30);
Duration sleepTime = Duration.ofMillis(10);
long count = timeout.toMillis() / sleepTime.toMillis();
while (count-- > 0 && !exceptionProbeManager.isAlreadyInstrumented(fingerprint)) {
try {
Thread.sleep(sleepTime.toMillis());
} catch (InterruptedException e) {
throw new RuntimeException(e);
}
}
assertTrue(exceptionProbeManager.isAlreadyInstrumented(fingerprint));
}
}
4 changes: 2 additions & 2 deletions ...ava-agent/agent-debugger/src/test/java/com/datadog/debugger/util/ExceptionHelperTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,9 +104,9 @@ public void flattenStackTrace() {
},
simpleException);
StackTraceElement[] stack = ExceptionHelper.flattenStackTrace(simpleException);
assertEquals(2, stack.length); // message + frame
assertEquals(1, stack.length);
stack = ExceptionHelper.flattenStackTrace(nestedException);
assertEquals(4, stack.length); // message + frame + message + frame
assertEquals(2, stack.length);
}

@Test
Expand Down
3 changes: 1 addition & 2 deletions dd-trace-api/src/main/java/datadog/trace/api/config/DebuggerConfig.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,7 @@ public final class DebuggerConfig {
public static final String DEBUGGER_SYMBOL_FORCE_UPLOAD = "internal.force.symbol.database.upload";
public static final String DEBUGGER_SYMBOL_INCLUDES = "symbol.database.includes";
public static final String DEBUGGER_SYMBOL_FLUSH_THRESHOLD = "symbol.database.flush.threshold";
public static final String DEBUGGER_EXCEPTION_ENABLED =
"dynamic.instrumentation.exception.enabled";
public static final String DEBUGGER_EXCEPTION_ENABLED = "exception.debugging.enabled";

private DebuggerConfig() {}
}
2 changes: 2 additions & 0 deletions dd-trace-core/src/main/java/datadog/trace/core/StatusLogger.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,8 @@ public void toJson(JsonWriter writer, Config config) throws IOException {
writer.value(config.isRemoteConfigEnabled());
writer.name("debugger_enabled");
writer.value(config.isDebuggerEnabled());
writer.name("debugger_exception_enabled");
writer.value(config.isDebuggerExceptionEnabled());
writer.name("appsec_enabled");
writer.value(config.getAppSecActivation().toString());
writer.name("appsec_rules_file_path");
Expand Down

0 comments on commit 1679925

Please sign in to comment.