From d79dc7c9c403aa90a801ac06d71e4bbe3f1cc546 Mon Sep 17 00:00:00 2001 From: ValentinZakharov Date: Fri, 20 Sep 2024 15:47:31 +0200 Subject: [PATCH] Improved stack trace reporting via telemetry (#7632) * Improved stacktrace report via telemetry * Additional tests --- .../telemetry/log/LogPeriodicAction.java | 100 ++++++++++---- .../log/LogPeriodicActionTest.groovy | 125 +++++++++++++++++- 2 files changed, 195 insertions(+), 30 deletions(-) diff --git a/telemetry/src/main/java/datadog/telemetry/log/LogPeriodicAction.java b/telemetry/src/main/java/datadog/telemetry/log/LogPeriodicAction.java index 279ddd5bc3a..094e9aaae59 100644 --- a/telemetry/src/main/java/datadog/telemetry/log/LogPeriodicAction.java +++ b/telemetry/src/main/java/datadog/telemetry/log/LogPeriodicAction.java @@ -15,7 +15,15 @@ public class LogPeriodicAction implements TelemetryRunnable.TelemetryPeriodicAct * as a filter) */ static final String[] PACKAGE_ALLOW_LIST = { - "datadog.", "com.datadog.", "java.", "javax.", "jakarta.", "jdk.", "sun.", "com.sun." + "datadog.", + "com.datadog.", + "java.", + "javax.", + "jakarta.", + "jdk.", + "sun.", + "com.sun.", + "io.sqreen.powerwaf." }; private static final String UNKNOWN = ""; @@ -45,43 +53,79 @@ public void doIteration(TelemetryService service) { private static String renderStackTrace(Throwable t) { StringBuilder result = new StringBuilder(); - String name = t.getClass().getCanonicalName(); - if (name == null || name.isEmpty()) { - result.append(UNKNOWN); - } else { - result.append(name); - } + StackTraceElement[] previousStackTrace = null; - if (isDataDogCode(t)) { - String msg = t.getMessage(); - result.append(": "); - if (msg == null || msg.isEmpty()) { + while (t != null) { + String name = t.getClass().getCanonicalName(); + if (name == null || name.isEmpty()) { result.append(UNKNOWN); } else { - result.append(msg); + result.append(name); } - } - result.append('\n'); - - final StackTraceElement[] stacktrace = t.getStackTrace(); - int pendingRedacted = 0; - if (stacktrace != null) { - for (final StackTraceElement frame : t.getStackTrace()) { - final String className = frame.getClassName(); - if (shouldRedactClass(className)) { - pendingRedacted++; + + if (isDataDogCode(t)) { + String msg = t.getMessage(); + result.append(": "); + if (msg == null || msg.isEmpty()) { + result.append(UNKNOWN); } else { - writePendingRedacted(result, pendingRedacted); - pendingRedacted = 0; - result.append(" at ").append(frame).append('\n'); + result.append(msg); } } + result.append('\n'); + + final StackTraceElement[] stacktrace = t.getStackTrace(); + int pendingRedacted = 0; + if (stacktrace != null) { + int commonFrames = 0; + if (previousStackTrace != null) { + commonFrames = countCommonFrames(previousStackTrace, stacktrace); + } + int maxIndex = stacktrace.length - commonFrames; + + for (int i = 0; i < maxIndex; i++) { + final StackTraceElement frame = stacktrace[i]; + final String className = frame.getClassName(); + if (shouldRedactClass(className)) { + pendingRedacted++; + } else { + writePendingRedacted(result, pendingRedacted); + pendingRedacted = 0; + result.append(" at ").append(frame).append('\n'); + } + } + writePendingRedacted(result, pendingRedacted); + + if (commonFrames > 0) { + result.append(" ... ").append(commonFrames).append(" more\n"); + } + } + + previousStackTrace = stacktrace; + t = t.getCause(); + if (t != null) { + result.append("Caused by: "); + } } - writePendingRedacted(result, pendingRedacted); return result.toString(); } + private static int countCommonFrames( + StackTraceElement[] previousStackTrace, StackTraceElement[] currentStackTrace) { + int previousIndex = previousStackTrace.length - 1; + int currentIndex = currentStackTrace.length - 1; + int count = 0; + while (previousIndex >= 0 + && currentIndex >= 0 + && previousStackTrace[previousIndex].equals(currentStackTrace[currentIndex])) { + count++; + previousIndex--; + currentIndex--; + } + return count; + } + private static boolean isDataDogCode(Throwable t) { StackTraceElement[] stackTrace = t.getStackTrace(); if (stackTrace == null || stackTrace.length == 0) { @@ -91,7 +135,9 @@ private static boolean isDataDogCode(Throwable t) { if (cn.isEmpty()) { return false; } - return cn.startsWith("datadog.") || cn.startsWith("com.datadog."); + return cn.startsWith("datadog.") + || cn.startsWith("com.datadog.") + || cn.startsWith("io.sqreen.powerwaf."); } private static boolean shouldRedactClass(final String className) { diff --git a/telemetry/src/test/groovy/datadog/telemetry/log/LogPeriodicActionTest.groovy b/telemetry/src/test/groovy/datadog/telemetry/log/LogPeriodicActionTest.groovy index 94c10ac4a37..aa29acdd6a3 100644 --- a/telemetry/src/test/groovy/datadog/telemetry/log/LogPeriodicActionTest.groovy +++ b/telemetry/src/test/groovy/datadog/telemetry/log/LogPeriodicActionTest.groovy @@ -138,14 +138,133 @@ class LogPeriodicActionTest extends DDSpecification { " at (redacted: 2 frames)\n" } + void 'stacktrace with multiple frames and common frames'() { + LogMessage logMessage + + given: + final t = throwable("exception", stacktrace( + frame(""), + frame("datadog.MyClass"), + frame("mycorp.MyClass"), + frame("mycorp.MyClass"), + ), throwable("exception 2", stacktrace( + frame("java.MyClass"), + frame("mycorp.MyClass"), + frame("datadog.MyClass"), + frame("mycorp.MyClass"), + frame("mycorp.MyClass"), + ))) + + when: + LogCollector.get().addLogMessage(LogMessageLevel.ERROR.toString(), "test", t) + periodicAction.doIteration(telemetryService) + + then: + 1 * telemetryService.addLogMessage(_) >> { args -> logMessage = args[0] } + 0 * _ + logMessage.getMessage() == 'test' + logMessage.getStackTrace() == "${MutableException.canonicalName}\n" + + " at (redacted)\n" + + " at datadog.MyClass.method(file:42)\n" + + " at (redacted: 2 frames)\n" + + "Caused by: ${MutableException.canonicalName}\n" + + " at java.MyClass.method(file:42)\n" + + " at (redacted)\n" + + " ... 3 more\n" + } + + void 'stacktrace with common frames only'() { + LogMessage logMessage + + given: + final t = throwable("exception", stacktrace( + frame("java.MyClass"), + frame("mycorp.MyClass"), + frame("datadog.MyClass"), + frame("mycorp.MyClass"), + frame("mycorp.MyClass"), + ), throwable("exception 2", stacktrace( + frame("java.MyClass"), + frame("mycorp.MyClass"), + frame("datadog.MyClass"), + frame("mycorp.MyClass"), + frame("mycorp.MyClass"), + ), throwable("exception 3", stacktrace( + frame("java.MyClass"), + frame("mycorp.MyClass"), + frame("datadog.MyClass"), + frame("mycorp.MyClass"), + frame("mycorp.MyClass"), + )))) + + when: + LogCollector.get().addLogMessage(LogMessageLevel.ERROR.toString(), "test", t) + periodicAction.doIteration(telemetryService) + + then: + 1 * telemetryService.addLogMessage(_) >> { args -> logMessage = args[0] } + 0 * _ + logMessage.getMessage() == 'test' + logMessage.getStackTrace() == "${MutableException.canonicalName}\n" + + " at java.MyClass.method(file:42)\n" + + " at (redacted)\n" + + " at datadog.MyClass.method(file:42)\n" + + " at (redacted: 2 frames)\n" + + "Caused by: ${MutableException.canonicalName}\n" + + " ... 5 more\n" + + "Caused by: ${MutableException.canonicalName}\n" + + " ... 5 more\n" + } + + void 'stacktrace without common frames'() { + LogMessage logMessage + + given: + final t = throwable("exception", stacktrace( + frame("java.MyClass"), + frame("mycorp.MyClass"), + frame("datadog.MyClass"), + frame("mycorp.MyClass"), + frame("mycorp.MyClass"), + ), throwable("exception 2", stacktrace( + frame("java.MyClass"), + frame("org.datadog.Test"), + frame("io.DataTest"), + frame("dd.MainClass"), + ))) + + when: + LogCollector.get().addLogMessage(LogMessageLevel.ERROR.toString(), "test", t) + periodicAction.doIteration(telemetryService) + + then: + 1 * telemetryService.addLogMessage(_) >> { args -> logMessage = args[0] } + 0 * _ + logMessage.getMessage() == 'test' + logMessage.getStackTrace() == "${MutableException.canonicalName}\n" + + " at java.MyClass.method(file:42)\n" + + " at (redacted)\n" + + " at datadog.MyClass.method(file:42)\n" + + " at (redacted: 2 frames)\n" + + "Caused by: ${MutableException.canonicalName}\n" + + " at java.MyClass.method(file:42)\n" + + " at (redacted: 3 frames)\n" + } + static class MutableException extends Exception { - MutableException(String message) { - super(message, null, true, true) + MutableException(String message, Throwable cause) { + super(message, cause, true, true) } } static Throwable throwable(String message, StackTraceElement[] stacktrace) { - final MutableException t = new MutableException(message) + final MutableException t = new MutableException(message, null) + t.setStackTrace(stacktrace) + return t + } + + static Throwable throwable(String message, StackTraceElement[] stacktrace, Throwable cause) { + final MutableException t = new MutableException(message, cause) t.setStackTrace(stacktrace) return t }