Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 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
1 change: 1 addition & 0 deletions sdk/common/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ plugins {
id("otel.java-conventions")
id("otel.publish-conventions")
id("otel.animalsniffer-conventions")
id("otel.jmh-conventions")
}
apply<OtelVersionClassPlugin>()

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.sdk.internal;

import java.io.PrintStream;
import java.io.PrintWriter;
import java.io.StringWriter;
import java.util.concurrent.TimeUnit;
import java.util.function.BiFunction;
import org.openjdk.jmh.annotations.Benchmark;
import org.openjdk.jmh.annotations.BenchmarkMode;
import org.openjdk.jmh.annotations.Fork;
import org.openjdk.jmh.annotations.Measurement;
import org.openjdk.jmh.annotations.Mode;
import org.openjdk.jmh.annotations.OutputTimeUnit;
import org.openjdk.jmh.annotations.Param;
import org.openjdk.jmh.annotations.Scope;
import org.openjdk.jmh.annotations.State;
import org.openjdk.jmh.annotations.Threads;
import org.openjdk.jmh.annotations.Warmup;

/**
* This benchmark compares the performance of {@link StackTraceRenderer}, the custom length limit
* aware exception render, to the built-in JDK stacktrace renderer {@link
* Throwable#printStackTrace(PrintStream)}.
*/
@BenchmarkMode({Mode.AverageTime})
@OutputTimeUnit(TimeUnit.NANOSECONDS)
@Warmup(iterations = 5, time = 100, timeUnit = TimeUnit.MILLISECONDS)
@Measurement(iterations = 5, time = 100, timeUnit = TimeUnit.MILLISECONDS)
@Fork(1)
@SuppressWarnings("StaticAssignmentOfThrowable")
public class StacktraceRenderBenchmark {

private static final Exception simple = new Exception("error");
private static final Exception complex =
new Exception("error", new Exception("cause1", new Exception("cause2")));

static {
complex.addSuppressed(new Exception("suppressed1"));
complex.addSuppressed(new Exception("suppressed2", new Exception("cause")));
}

@State(Scope.Benchmark)
public static class BenchmarkState {

@Param Renderer renderer;
@Param ExceptionParam exceptionParam;

@Param({"10", "1000", "100000"})
int lengthLimit;
}

@SuppressWarnings("ImmutableEnumChecker")
public enum Renderer {
JDK(
(throwable, limit) -> {
StringWriter stringWriter = new StringWriter();
try (PrintWriter printWriter = new PrintWriter(stringWriter)) {
Comment thread
jack-berg marked this conversation as resolved.
throwable.printStackTrace(printWriter);
}
String stacktrace = stringWriter.toString();
return stacktrace.substring(0, Math.min(stacktrace.length(), limit));
}),
CUSTOM((throwable, limit) -> new StackTraceRenderer(throwable, limit).render());

private final BiFunction<Throwable, Integer, String> renderer;

Renderer(BiFunction<Throwable, Integer, String> renderer) {
this.renderer = renderer;
}

BiFunction<Throwable, Integer, String> renderer() {
return renderer;
}
}

@SuppressWarnings("ImmutableEnumChecker")
public enum ExceptionParam {
SIMPLE(simple),
COMPLEX(complex);

private final Throwable throwable;

ExceptionParam(Throwable throwable) {
this.throwable = throwable;
}

Throwable throwable() {
return throwable;
}
}

@Benchmark
@Threads(1)
@SuppressWarnings("ReturnValueIgnored")
public void render(BenchmarkState benchmarkState) {
BiFunction<Throwable, Integer, String> renderer = benchmarkState.renderer.renderer();
Throwable throwable = benchmarkState.exceptionParam.throwable();
int limit = benchmarkState.lengthLimit;

renderer.apply(throwable, limit);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@
import io.opentelemetry.api.common.AttributeKey;
import io.opentelemetry.api.common.Attributes;
import io.opentelemetry.api.common.AttributesBuilder;
import java.io.PrintWriter;
import java.io.StringWriter;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -107,7 +105,9 @@ public static Object applyAttributeLengthLimit(Object value, int lengthLimit) {
}

public static void addExceptionAttributes(

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

One thing I've considered is retaining some sort of system property / env var to fallback to the built-in JDK exception rendering. Would offer a nice escape hatch to revert to the built-in jdk rendering in case there's any bugs in the code, but downside is it would be hard to know when it would be safe to finally remove.

Throwable exception, BiConsumer<AttributeKey<String>, String> attributeConsumer) {
Throwable exception,
BiConsumer<AttributeKey<String>, String> attributeConsumer,
int lengthLimit) {
String exceptionType = exception.getClass().getCanonicalName();
if (exceptionType != null) {
attributeConsumer.accept(EXCEPTION_TYPE, exceptionType);
Expand All @@ -118,11 +118,7 @@ public static void addExceptionAttributes(
attributeConsumer.accept(EXCEPTION_MESSAGE, exceptionMessage);
}

StringWriter stringWriter = new StringWriter();
try (PrintWriter printWriter = new PrintWriter(stringWriter)) {
exception.printStackTrace(printWriter);
}
String stackTrace = stringWriter.toString();
String stackTrace = new StackTraceRenderer(exception, lengthLimit).render();
if (stackTrace != null) {
attributeConsumer.accept(EXCEPTION_STACKTRACE, stackTrace);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,139 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.sdk.internal;

import java.io.PrintStream;
import java.util.Collections;
import java.util.IdentityHashMap;
import java.util.Set;

/**
* An alternative to exception stacktrace renderer that replicates the behavior of {@link
* Throwable#printStackTrace(PrintStream)}, but which is aware of a maximum stacktrace length limit,
* and exits early when the length limit has been exceeded to avoid unnecessary computation.
*
* <p>Instances should only be used once.
*/
class StackTraceRenderer {

private static final String CAUSED_BY = "Caused by: ";
private static final String SUPPRESSED = "Suppressed: ";

private final Throwable throwable;
private final int lengthLimit;
private final StringBuilder builder = new StringBuilder();

StackTraceRenderer(Throwable throwable, int lengthLimit) {
this.throwable = throwable;
this.lengthLimit = lengthLimit;
}

String render() {
if (builder.length() == 0) {
appendStackTrace();
}

return builder.substring(0, Math.min(builder.length(), lengthLimit));
}

private void appendStackTrace() {
if (appendLine(throwable.toString())) {
return;
}

StackTraceElement[] stackTraceElements = throwable.getStackTrace();
for (StackTraceElement stackTraceElement : stackTraceElements) {
if (appendLine("\tat " + stackTraceElement)) {
Comment thread
jack-berg marked this conversation as resolved.
Outdated
return;
}
}

Set<Throwable> seen = Collections.newSetFromMap(new IdentityHashMap<>());
seen.add(throwable);

for (Throwable suppressed : throwable.getSuppressed()) {
appendInnerStacktrace(stackTraceElements, suppressed, "\t", SUPPRESSED, seen);
}

Throwable cause = throwable.getCause();
if (cause != null) {
appendInnerStacktrace(stackTraceElements, cause, "", CAUSED_BY, seen);
}
}

/**
* Append the {@code innerThrowable} to the {@link #builder}, returning {@code true} if the
* builder now exceeds the length limit.
*/
private boolean appendInnerStacktrace(
StackTraceElement[] parentElements,
Throwable innerThrowable,
String prefix,
String caption,
Set<Throwable> seen) {
if (seen.contains(innerThrowable)) {
appendLine(prefix + caption + "[CIRCULAR REFERENCE: " + innerThrowable + "]");
return true;
}
seen.add(innerThrowable);

// Iterating back to front, compute the lastSharedFrameIndex, which tracks the point at which
// this exception's stacktrace elements start repeating the parent's elements
StackTraceElement[] currentElements = innerThrowable.getStackTrace();
int parentIndex = parentElements.length - 1;
int lastSharedFrameIndex = currentElements.length - 1;
while (true) {
if (parentIndex < 0 || lastSharedFrameIndex < 0) {
break;
}
if (!parentElements[parentIndex].equals(currentElements[lastSharedFrameIndex])) {
break;
}
parentIndex--;
lastSharedFrameIndex--;
}

if (appendLine(prefix + caption + innerThrowable)) {
return true;
}

for (int i = 0; i <= lastSharedFrameIndex; i++) {
StackTraceElement stackTraceElement = currentElements[i];
if (appendLine(prefix + "\tat " + stackTraceElement)) {
return true;
}
}

int duplicateFrames = currentElements.length - 1 - lastSharedFrameIndex;
if (duplicateFrames != 0) {
if (appendLine(prefix + "\t... " + duplicateFrames + " more")) {
return true;
}
}

for (Throwable suppressed : innerThrowable.getSuppressed()) {
if (appendInnerStacktrace(currentElements, suppressed, prefix + "\t", SUPPRESSED, seen)) {
return true;
}
}

Throwable cause = innerThrowable.getCause();
if (cause != null) {
return appendInnerStacktrace(currentElements, cause, prefix, CAUSED_BY, seen);
}

return false;
}

/**
* Append the string as a new line on {@link #builder}, returning {@code true} if the builder now
* exceeds the length limit.
*/
private boolean appendLine(String s) {
builder.append(s).append(System.lineSeparator());
return builder.length() >= lengthLimit;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.sdk.internal;

import static org.assertj.core.api.Assertions.assertThat;

import java.io.PrintWriter;
import java.io.StringWriter;
import java.util.stream.Stream;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;

class StackTraceRendererTest {

@ParameterizedTest
@MethodSource("renderStacktraceArgs")
void renderStacktrace(Throwable throwable) {
assertThat(new StackTraceRenderer(throwable, 10).render())
.isEqualTo(jdkStackTrace(throwable, 10));
assertThat(new StackTraceRenderer(throwable, 100).render())
.isEqualTo(jdkStackTrace(throwable, 100));
assertThat(new StackTraceRenderer(throwable, 1000).render())
.isEqualTo(jdkStackTrace(throwable, 1000));
assertThat(new StackTraceRenderer(throwable, Integer.MAX_VALUE).render())
.isEqualTo(jdkStackTrace(throwable, Integer.MAX_VALUE));

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I love this test: for a list of exceptions render each at a variety of length limits, and compare the result to the built-in JDK stack trace renderer.

}

private static Stream<Arguments> renderStacktraceArgs() {
Exception withCycle = new Exception("error1");
Exception withCycleInner = new Exception("error2", withCycle);
withCycle.initCause(withCycleInner);

Exception withSuppressed = new Exception("error");
withSuppressed.addSuppressed(new Exception("suppressed"));

Exception withMultipleSuppressed = new Exception("error");
withMultipleSuppressed.addSuppressed(new Exception("suppressed1"));
withMultipleSuppressed.addSuppressed(new Exception("suppressed2"));

Exception withNestedSuppressed = new Exception("error");
withNestedSuppressed.addSuppressed(withSuppressed);
withNestedSuppressed.addSuppressed(withMultipleSuppressed);

Exception withKitchenSink = new Exception("kitchenSink", withCycle);
withKitchenSink.addSuppressed(withMultipleSuppressed);
withKitchenSink.addSuppressed(withNestedSuppressed);

return Stream.of(
// simple
Arguments.of(new Exception()),
Arguments.of(new Exception("error")),
// with cause
Arguments.of(new Exception(new Exception("cause"))),
Arguments.of(new Exception("error", new Exception("cause"))),
// with nested causes
Arguments.of(
new Exception(
"error",
new Exception(
"cause1",
new Exception(
"cause2",
new Exception(
"cause3", new Exception("cause4", new Exception("cause5"))))))),
// with cause with circular reference
Arguments.of(withCycle),
// with suppressed
Arguments.of(withSuppressed),
Arguments.of(withMultipleSuppressed),
Arguments.of(withNestedSuppressed),
// with cause, cycle, and suppressed!
Arguments.of(withKitchenSink));
Comment thread
jack-berg marked this conversation as resolved.
}

private static String jdkStackTrace(Throwable exception, int limit) {
StringWriter stringWriter = new StringWriter();
try (PrintWriter printWriter = new PrintWriter(stringWriter)) {
exception.printStackTrace(printWriter);
}
String stacktrace = stringWriter.toString();
return stacktrace.substring(0, Math.min(stacktrace.length(), limit));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,8 @@ public ExtendedSdkLogRecordBuilder setException(Throwable throwable) {
return this;
}

AttributeUtil.addExceptionAttributes(throwable, this::setAttribute);
AttributeUtil.addExceptionAttributes(
throwable, this::setAttribute, logLimits.getMaxAttributeValueLength());

return this;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -469,7 +469,8 @@ public ReadWriteSpan recordException(Throwable exception, Attributes additionalA
AttributesMap.create(
spanLimits.getMaxNumberOfAttributes(), spanLimits.getMaxAttributeValueLength());

AttributeUtil.addExceptionAttributes(exception, attributes::put);
AttributeUtil.addExceptionAttributes(
exception, attributes::put, spanLimits.getMaxAttributeValueLength());

additionalAttributes.forEach(attributes::put);

Expand Down