Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions dd-trace-core/src/main/java/datadog/trace/core/DDSpan.java
Original file line number Diff line number Diff line change
Expand Up @@ -354,10 +354,10 @@ public DDSpan addThrowable(final Throwable error) {
@Override
public DDSpan addThrowable(Throwable error, byte errorPriority) {
if (null != error) {
String message = error.getMessage();
String message = StackTraces.safeGetMessage(error);
if (!"broken pipe".equalsIgnoreCase(message)
&& (error.getCause() == null
|| !"broken pipe".equalsIgnoreCase(error.getCause().getMessage()))) {
|| !"broken pipe".equalsIgnoreCase(StackTraces.safeGetMessage(error.getCause())))) {
// broken pipes happen when clients abort connections,
// which might happen because the application is overloaded
// or warming up - capturing the stack trace and keeping
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import java.io.StringReader;
import java.io.StringWriter;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
Expand All @@ -13,10 +14,66 @@
public final class StackTraces {
private StackTraces() {}

/**
* Safely retrieves the message from a throwable.
*
* <p>Third-party exception classes occasionally use formatting utilities (e.g. {@code
* java.text.MessageFormat}) inside {@code getMessage()}, which can throw when the pattern
* contains non-integer placeholders.
*
* @param t the throwable to retrieve the message from
* @return {@code null} if {@code t} is {@code null}; the result of {@link Throwable#getMessage()}
* on success; or a diagnostic string of the form {@code "(Exception message unavailable for
* ClassName: getMessage() threw ExceptionType)"} if {@code getMessage()} throws
*/
public static String safeGetMessage(Throwable t) {
if (t == null) {
return null;
}
try {
return t.getMessage();
} catch (Exception e) {
return "(Exception message unavailable for "
+ t.getClass().getSimpleName()
+ ": getMessage() threw "
+ e.getClass().getSimpleName()
+ ")";
}
}

/**
* Returns the stack trace of {@code t} as a string, truncated to {@code maxChars} characters.
*
* <p>Uses {@link Throwable#printStackTrace(java.io.PrintWriter)} to produce the full trace
* including {@code Caused by} and {@code Suppressed} chains. If {@code printStackTrace} itself
* throws (e.g. because {@link Throwable#getMessage()} throws inside {@code toString()}), falls
* back to reconstructing the trace from {@link Throwable#getStackTrace()} so the call site
* remains locatable.
*
* @param t the throwable to format
* @param maxChars maximum length of the returned string
* @return the stack trace string, truncated if necessary
*/
public static String getStackTrace(Throwable t, int maxChars) {
StringWriter sw = new StringWriter();
t.printStackTrace(new PrintWriter(sw));
String trace = sw.toString();
String trace;
try {
StringWriter sw = new StringWriter();
t.printStackTrace(new PrintWriter(sw));
trace = sw.toString();
} catch (Exception ignored) {
// printStackTrace() failed (e.g. getMessage() throws inside toString()).
// Reconstruct from getStackTrace() so the call site is still locatable.
try {
trace =
t.getClass().getName()
+ System.lineSeparator()
+ Arrays.stream(t.getStackTrace())
.map(f -> "\tat " + f)
.collect(Collectors.joining(System.lineSeparator()));
} catch (Exception ignored2) {
trace = t.getClass().getName();
}
}
try {
return truncate(trace, maxChars);
} catch (Exception e) {
Expand All @@ -43,6 +100,9 @@ static String truncate(String trace, int maxChars) {
/* last-ditch centre cut to guarantee the limit */
String cutMessage = "\t... trace centre-cut to " + maxChars + " chars ...";
int retainedLength = maxChars - cutMessage.length() - 2; // 2 for the newlines
if (retainedLength <= 0) {
return cutMessage + System.lineSeparator();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks added so will modify also the existing implementation

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the fix for edge case: when maxChars is smaller than cutMessage itself, retainedLength goes negative and substring() throws. Caught it while working in this area

}
int half = retainedLength / 2;
return trace.substring(0, half)
+ System.lineSeparator()
Expand Down
35 changes: 35 additions & 0 deletions dd-trace-core/src/test/java/datadog/trace/core/DDSpanTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import static datadog.trace.core.DDSpanContext.SPAN_SAMPLING_RULE_RATE_TAG;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.params.provider.Arguments.arguments;
Expand All @@ -33,6 +34,7 @@
import datadog.trace.common.writer.ListWriter;
import datadog.trace.core.propagation.ExtractedContext;
import datadog.trace.core.propagation.PropagationTags;
import datadog.trace.core.util.TestThrowables;
import java.io.Closeable;
import java.io.IOException;
import java.lang.reflect.Field;
Expand Down Expand Up @@ -447,6 +449,39 @@ void nullExceptionSafeToAdd() {
assertNull(span.getTag(DDTags.ERROR_STACK));
}

@Test
void addThrowableDoesNotFailWhenGetMessageThrows() {
DDSpan span = (DDSpan) tracer.buildSpan("datadog", "root").start();

span.addThrowable(TestThrowables.throwingGetMessage());

assertTrue(span.isError());
assertNotNull(span.getTag(DDTags.ERROR_TYPE));
assertNotNull(
span.getTag(DDTags.ERROR_STACK),
"stack trace must be captured even when getMessage() throws");
assertTrue(((String) span.getTag(DDTags.ERROR_MSG)).contains("Exception message unavailable"));
}

@Test
void addThrowableDoesNotFailWhenGetCauseMessageThrows() {
// Outer exception has a normal message, so the broken-pipe check reaches
// getCause().getMessage()
RuntimeException error =
new RuntimeException("outer message", TestThrowables.throwingGetMessage());
DDSpan span = (DDSpan) tracer.buildSpan("datadog", "root").start();

span.addThrowable(error);

assertTrue(span.isError());
assertNotNull(span.getTag(DDTags.ERROR_TYPE));
assertNotNull(
span.getTag(DDTags.ERROR_STACK),
"stack trace must be captured even when cause's getMessage() throws");
// Outer getMessage() works normally — message must be recorded
assertEquals("outer message", span.getTag(DDTags.ERROR_MSG));
}

@TableTest({
"scenario | rate | limit ",
"rate=1.0 lim=10 | 1.0 | 10 ",
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
package datadog.trace.core.util;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.params.provider.Arguments.arguments;

import java.util.stream.Stream;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;
Expand Down Expand Up @@ -64,6 +67,24 @@ class StackTracesTest {
+ " at com.example.util.ResourceManager.close(ResourceManager.java:21)\n"
+ " ... 3 more\n";

// --- safeGetMessage ---

@Test
void safeGetMessageReturnsFallbackWhenGetMessageThrows() {
String message = StackTraces.safeGetMessage(TestThrowables.throwingGetMessage());
assertTrue(
message.contains("Exception message unavailable"), "must indicate message is unavailable");
assertTrue(
message.contains("IllegalArgumentException"), "must include secondary exception type");
}

@Test
void safeGetMessageReturnsNullForNullInput() {
assertNull(StackTraces.safeGetMessage(null));
}

// --- getStackTrace with broken getMessage ---

@ParameterizedTest(name = "truncation limit {0}")
@MethodSource("testTruncateArguments")
void testTruncate(int limit, String expected) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
package datadog.trace.core.util;

import java.text.MessageFormat;

/** Test helpers for throwables with non-standard {@code getMessage()} behaviour. */
public final class TestThrowables {
private TestThrowables() {}

/**
* Returns a {@link RuntimeException} whose {@link Throwable#getMessage()} throws {@link
* IllegalArgumentException} via {@link MessageFormat} with non-integer placeholders — simulating
* the third-party exception that triggered the production bug in {@code DDSpan.addThrowable}.
*/
public static RuntimeException throwingGetMessage() {
return new RuntimeException() {
@Override
public String getMessage() {
return MessageFormat.format(
"Timeout after {TotalMilliseconds}ms matching pattern {Pattern}", "arg0", "arg1");
}
};
}
}