From 3da59293095bcdc8dfb6962d104133dad752a52a Mon Sep 17 00:00:00 2001 From: Jack Berg Date: Wed, 16 Apr 2025 09:56:14 -0500 Subject: [PATCH 1/6] Add custom stacktrace renderer which is length limit aware --- sdk/common/build.gradle.kts | 1 + .../internal/StacktraceRenderBenchmark.java | 106 +++++++++++++ .../sdk/internal/AttributeUtil.java | 12 +- .../sdk/internal/StackTraceRenderer.java | 139 ++++++++++++++++++ .../sdk/internal/StackTraceRendererTest.java | 87 +++++++++++ .../sdk/logs/ExtendedSdkLogRecordBuilder.java | 3 +- .../io/opentelemetry/sdk/trace/SdkSpan.java | 3 +- 7 files changed, 341 insertions(+), 10 deletions(-) create mode 100644 sdk/common/src/jmh/java/io/opentelemetry/sdk/internal/StacktraceRenderBenchmark.java create mode 100644 sdk/common/src/main/java/io/opentelemetry/sdk/internal/StackTraceRenderer.java create mode 100644 sdk/common/src/test/java/io/opentelemetry/sdk/internal/StackTraceRendererTest.java diff --git a/sdk/common/build.gradle.kts b/sdk/common/build.gradle.kts index f369ae1e84f..89a76361b24 100644 --- a/sdk/common/build.gradle.kts +++ b/sdk/common/build.gradle.kts @@ -4,6 +4,7 @@ plugins { id("otel.java-conventions") id("otel.publish-conventions") id("otel.animalsniffer-conventions") + id("otel.jmh-conventions") } apply() diff --git a/sdk/common/src/jmh/java/io/opentelemetry/sdk/internal/StacktraceRenderBenchmark.java b/sdk/common/src/jmh/java/io/opentelemetry/sdk/internal/StacktraceRenderBenchmark.java new file mode 100644 index 00000000000..0b236f3dd0d --- /dev/null +++ b/sdk/common/src/jmh/java/io/opentelemetry/sdk/internal/StacktraceRenderBenchmark.java @@ -0,0 +1,106 @@ +/* + * 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) +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)) { + 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 renderer; + + Renderer(BiFunction renderer) { + this.renderer = renderer; + } + + BiFunction 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 renderer = benchmarkState.renderer.renderer(); + Throwable throwable = benchmarkState.exceptionParam.throwable(); + int limit = benchmarkState.lengthLimit; + + renderer.apply(throwable, limit); + } +} diff --git a/sdk/common/src/main/java/io/opentelemetry/sdk/internal/AttributeUtil.java b/sdk/common/src/main/java/io/opentelemetry/sdk/internal/AttributeUtil.java index e0fc9b56b90..fcd57e62b93 100644 --- a/sdk/common/src/main/java/io/opentelemetry/sdk/internal/AttributeUtil.java +++ b/sdk/common/src/main/java/io/opentelemetry/sdk/internal/AttributeUtil.java @@ -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; @@ -107,7 +105,9 @@ public static Object applyAttributeLengthLimit(Object value, int lengthLimit) { } public static void addExceptionAttributes( - Throwable exception, BiConsumer, String> attributeConsumer) { + Throwable exception, + BiConsumer, String> attributeConsumer, + int lengthLimit) { String exceptionType = exception.getClass().getCanonicalName(); if (exceptionType != null) { attributeConsumer.accept(EXCEPTION_TYPE, exceptionType); @@ -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); } diff --git a/sdk/common/src/main/java/io/opentelemetry/sdk/internal/StackTraceRenderer.java b/sdk/common/src/main/java/io/opentelemetry/sdk/internal/StackTraceRenderer.java new file mode 100644 index 00000000000..b975078af22 --- /dev/null +++ b/sdk/common/src/main/java/io/opentelemetry/sdk/internal/StackTraceRenderer.java @@ -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 lenght limit has been exceeded to avoid unnecessary computation. + * + *

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)) { + return; + } + } + + Set 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 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; + } +} diff --git a/sdk/common/src/test/java/io/opentelemetry/sdk/internal/StackTraceRendererTest.java b/sdk/common/src/test/java/io/opentelemetry/sdk/internal/StackTraceRendererTest.java new file mode 100644 index 00000000000..c32485df953 --- /dev/null +++ b/sdk/common/src/test/java/io/opentelemetry/sdk/internal/StackTraceRendererTest.java @@ -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)); + } + + private static Stream 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)); + } + + 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)); + } +} diff --git a/sdk/logs/src/main/java/io/opentelemetry/sdk/logs/ExtendedSdkLogRecordBuilder.java b/sdk/logs/src/main/java/io/opentelemetry/sdk/logs/ExtendedSdkLogRecordBuilder.java index 2e4b4b5e40d..06dad088981 100644 --- a/sdk/logs/src/main/java/io/opentelemetry/sdk/logs/ExtendedSdkLogRecordBuilder.java +++ b/sdk/logs/src/main/java/io/opentelemetry/sdk/logs/ExtendedSdkLogRecordBuilder.java @@ -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; } diff --git a/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/SdkSpan.java b/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/SdkSpan.java index 30ee4bbdebf..215b5e941a1 100644 --- a/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/SdkSpan.java +++ b/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/SdkSpan.java @@ -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); From b2d3b5850ea0d65f16ad55cc8d00a242723837cb Mon Sep 17 00:00:00 2001 From: Jack Berg Date: Wed, 16 Apr 2025 10:41:32 -0500 Subject: [PATCH 2/6] suppress warnings --- .../io/opentelemetry/sdk/internal/StacktraceRenderBenchmark.java | 1 + 1 file changed, 1 insertion(+) diff --git a/sdk/common/src/jmh/java/io/opentelemetry/sdk/internal/StacktraceRenderBenchmark.java b/sdk/common/src/jmh/java/io/opentelemetry/sdk/internal/StacktraceRenderBenchmark.java index 0b236f3dd0d..04121e9ae82 100644 --- a/sdk/common/src/jmh/java/io/opentelemetry/sdk/internal/StacktraceRenderBenchmark.java +++ b/sdk/common/src/jmh/java/io/opentelemetry/sdk/internal/StacktraceRenderBenchmark.java @@ -32,6 +32,7 @@ @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"); From a098e83ba4835b293e208ad8e7159bbc7e6319d5 Mon Sep 17 00:00:00 2001 From: Jack Berg Date: Wed, 16 Apr 2025 11:09:19 -0500 Subject: [PATCH 3/6] Fix typo --- .../java/io/opentelemetry/sdk/internal/StackTraceRenderer.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/common/src/main/java/io/opentelemetry/sdk/internal/StackTraceRenderer.java b/sdk/common/src/main/java/io/opentelemetry/sdk/internal/StackTraceRenderer.java index b975078af22..c36e978e841 100644 --- a/sdk/common/src/main/java/io/opentelemetry/sdk/internal/StackTraceRenderer.java +++ b/sdk/common/src/main/java/io/opentelemetry/sdk/internal/StackTraceRenderer.java @@ -13,7 +13,7 @@ /** * 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 lenght limit has been exceeded to avoid unnecessary computation. + * and exits early when the length limit has been exceeded to avoid unnecessary computation. * *

Instances should only be used once. */ From 2159b0e7bdaf8d8607f203131373879de71ba498 Mon Sep 17 00:00:00 2001 From: Jack Berg Date: Mon, 21 Apr 2025 17:07:09 -0500 Subject: [PATCH 4/6] more perf --- .../sdk/internal/StackTraceRenderer.java | 39 +++++++++++++------ .../sdk/internal/StackTraceRendererTest.java | 16 +++++++- 2 files changed, 42 insertions(+), 13 deletions(-) diff --git a/sdk/common/src/main/java/io/opentelemetry/sdk/internal/StackTraceRenderer.java b/sdk/common/src/main/java/io/opentelemetry/sdk/internal/StackTraceRenderer.java index c36e978e841..fe89e5481be 100644 --- a/sdk/common/src/main/java/io/opentelemetry/sdk/internal/StackTraceRenderer.java +++ b/sdk/common/src/main/java/io/opentelemetry/sdk/internal/StackTraceRenderer.java @@ -40,13 +40,15 @@ String render() { } private void appendStackTrace() { - if (appendLine(throwable.toString())) { + builder.append(throwable).append(System.lineSeparator()); + if (isOverLimit()) { return; } StackTraceElement[] stackTraceElements = throwable.getStackTrace(); for (StackTraceElement stackTraceElement : stackTraceElements) { - if (appendLine("\tat " + stackTraceElement)) { + builder.append("\tat ").append(stackTraceElement).append(System.lineSeparator()); + if (isOverLimit()) { return; } } @@ -75,7 +77,13 @@ private boolean appendInnerStacktrace( String caption, Set seen) { if (seen.contains(innerThrowable)) { - appendLine(prefix + caption + "[CIRCULAR REFERENCE: " + innerThrowable + "]"); + builder + .append(prefix) + .append(caption) + .append("[CIRCULAR REFERENCE: ") + .append(innerThrowable) + .append("]") + .append(System.lineSeparator()); return true; } seen.add(innerThrowable); @@ -96,20 +104,32 @@ private boolean appendInnerStacktrace( lastSharedFrameIndex--; } - if (appendLine(prefix + caption + innerThrowable)) { + builder.append(prefix).append(caption).append(innerThrowable).append(System.lineSeparator()); + if (isOverLimit()) { return true; } for (int i = 0; i <= lastSharedFrameIndex; i++) { StackTraceElement stackTraceElement = currentElements[i]; - if (appendLine(prefix + "\tat " + stackTraceElement)) { + builder + .append(prefix) + .append("\tat ") + .append(stackTraceElement) + .append(System.lineSeparator()); + if (isOverLimit()) { return true; } } int duplicateFrames = currentElements.length - 1 - lastSharedFrameIndex; if (duplicateFrames != 0) { - if (appendLine(prefix + "\t... " + duplicateFrames + " more")) { + builder + .append(prefix) + .append("\t... ") + .append(duplicateFrames) + .append(" more") + .append(System.lineSeparator()); + if (isOverLimit()) { return true; } } @@ -128,12 +148,7 @@ private boolean appendInnerStacktrace( 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()); + private boolean isOverLimit() { return builder.length() >= lengthLimit; } } diff --git a/sdk/common/src/test/java/io/opentelemetry/sdk/internal/StackTraceRendererTest.java b/sdk/common/src/test/java/io/opentelemetry/sdk/internal/StackTraceRendererTest.java index c32485df953..a4a4605eaec 100644 --- a/sdk/common/src/test/java/io/opentelemetry/sdk/internal/StackTraceRendererTest.java +++ b/sdk/common/src/test/java/io/opentelemetry/sdk/internal/StackTraceRendererTest.java @@ -9,6 +9,7 @@ import java.io.PrintWriter; import java.io.StringWriter; +import java.util.Random; import java.util.stream.Stream; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; @@ -16,9 +17,22 @@ class StackTraceRendererTest { + private static final Random random = new Random(); + + @ParameterizedTest + @MethodSource("renderStacktraceArgs") + void renderStacktrace_randomLength(Throwable throwable) { + // Test equal stacktrace for random lengths to test edges + for (int i = 0; i < 100; i++) { + int length = random.nextInt(10_000); + assertThat(new StackTraceRenderer(throwable, length).render()) + .isEqualTo(jdkStackTrace(throwable, length)); + } + } + @ParameterizedTest @MethodSource("renderStacktraceArgs") - void renderStacktrace(Throwable throwable) { + void renderStacktrace_fixedLengths(Throwable throwable) { assertThat(new StackTraceRenderer(throwable, 10).render()) .isEqualTo(jdkStackTrace(throwable, 10)); assertThat(new StackTraceRenderer(throwable, 100).render()) From 7408c0855c55d245481b09ebf531accee5ac3a8b Mon Sep 17 00:00:00 2001 From: Jack Berg Date: Thu, 5 Jun 2025 18:02:44 -0500 Subject: [PATCH 5/6] Add property to render stacktrace using jvm default --- .../DefaultExceptionAttributeResolver.java | 33 ++++++-- ...DefaultExceptionAttributeResolverTest.java | 76 +++++++++++++++++++ 2 files changed, 102 insertions(+), 7 deletions(-) create mode 100644 sdk/common/src/test/java/io/opentelemetry/sdk/internal/DefaultExceptionAttributeResolverTest.java diff --git a/sdk/common/src/main/java/io/opentelemetry/sdk/internal/DefaultExceptionAttributeResolver.java b/sdk/common/src/main/java/io/opentelemetry/sdk/internal/DefaultExceptionAttributeResolver.java index 44061c1644f..d05633624f4 100644 --- a/sdk/common/src/main/java/io/opentelemetry/sdk/internal/DefaultExceptionAttributeResolver.java +++ b/sdk/common/src/main/java/io/opentelemetry/sdk/internal/DefaultExceptionAttributeResolver.java @@ -5,6 +5,7 @@ package io.opentelemetry.sdk.internal; +import io.opentelemetry.api.internal.ConfigUtil; import java.io.PrintWriter; import java.io.StringWriter; @@ -15,10 +16,19 @@ */ public final class DefaultExceptionAttributeResolver implements ExceptionAttributeResolver { + private static final String ENABLE_JVM_STACKTRACE_PROPERTY = + "otel.experimental.sdk.jvm_stacktrace"; + private static final DefaultExceptionAttributeResolver INSTANCE = - new DefaultExceptionAttributeResolver(); + new DefaultExceptionAttributeResolver( + Boolean.parseBoolean(ConfigUtil.getString(ENABLE_JVM_STACKTRACE_PROPERTY, "false"))); + + private final boolean jvmStacktraceEnabled; - private DefaultExceptionAttributeResolver() {} + // Visible for testing + DefaultExceptionAttributeResolver(boolean jvmStacktraceEnabled) { + this.jvmStacktraceEnabled = jvmStacktraceEnabled; + } public static ExceptionAttributeResolver getInstance() { return INSTANCE; @@ -37,14 +47,23 @@ public void setExceptionAttributes( attributeSetter.setAttribute(ExceptionAttributeResolver.EXCEPTION_MESSAGE, exceptionMessage); } + String exceptionStacktrace = + jvmStacktraceEnabled + ? jvmStacktrace(throwable) + : limitsAwareStacktrace(throwable, maxAttributeLength); + attributeSetter.setAttribute( + ExceptionAttributeResolver.EXCEPTION_STACKTRACE, exceptionStacktrace); + } + + private static String jvmStacktrace(Throwable throwable) { StringWriter stringWriter = new StringWriter(); try (PrintWriter printWriter = new PrintWriter(stringWriter)) { throwable.printStackTrace(printWriter); } - String exceptionStacktrace = stringWriter.toString(); - if (exceptionStacktrace != null) { - attributeSetter.setAttribute( - ExceptionAttributeResolver.EXCEPTION_STACKTRACE, exceptionStacktrace); - } + return stringWriter.toString(); + } + + private static String limitsAwareStacktrace(Throwable throwable, int maxAttributeLength) { + return new StackTraceRenderer(throwable, maxAttributeLength).render(); } } diff --git a/sdk/common/src/test/java/io/opentelemetry/sdk/internal/DefaultExceptionAttributeResolverTest.java b/sdk/common/src/test/java/io/opentelemetry/sdk/internal/DefaultExceptionAttributeResolverTest.java new file mode 100644 index 00000000000..de93d4d9dc9 --- /dev/null +++ b/sdk/common/src/test/java/io/opentelemetry/sdk/internal/DefaultExceptionAttributeResolverTest.java @@ -0,0 +1,76 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.sdk.internal; + +import static org.assertj.core.api.Assertions.assertThat; + +import io.opentelemetry.api.common.AttributeKey; +import java.util.HashMap; +import java.util.Map; +import java.util.function.Predicate; +import java.util.stream.Stream; +import javax.annotation.Nullable; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; + +class DefaultExceptionAttributeResolverTest { + + @ParameterizedTest + @MethodSource("setExceptionAttributesArgs") + void setExceptionAttributes( + boolean jvmStacktraceEnabled, + Throwable throwable, + int maxAttributeLength, + String expectedMessage, + String expectedType, + Predicate validStacktrace) { + DefaultExceptionAttributeResolver resolver = + new DefaultExceptionAttributeResolver(jvmStacktraceEnabled); + Map attributes = new HashMap<>(); + + resolver.setExceptionAttributes( + new ExceptionAttributeResolver.AttributeSetter() { + @Override + public void setAttribute(AttributeKey key, @Nullable T value) { + attributes.put(key.toString(), value); + } + }, + throwable, + maxAttributeLength); + + assertThat(attributes.get(ExceptionAttributeResolver.EXCEPTION_MESSAGE.getKey())) + .isEqualTo(expectedMessage); + assertThat(attributes.get(ExceptionAttributeResolver.EXCEPTION_TYPE.getKey())) + .isEqualTo(expectedType); + assertThat((String) attributes.get(ExceptionAttributeResolver.EXCEPTION_STACKTRACE.getKey())) + .matches(validStacktrace); + } + + private static Stream setExceptionAttributesArgs() { + return Stream.of( + // When jvmStacktraceEnabled=true, limit is ignored + Arguments.of( + true, + new Exception("error"), + 10, + "error", + "java.lang.Exception", + predicate(stacktrace -> stacktrace.length() > 10)), + // When jvmStacktraceEnabled=false, limit is adhered + Arguments.of( + false, + new Exception("error"), + 10, + "error", + "java.lang.Exception", + predicate(stacktrace -> stacktrace.length() == 10))); + } + + private static Predicate predicate(Predicate predicate) { + return predicate; + } +} From a0ded5ef6f6be01ea050524ea4d8e894000afe4c Mon Sep 17 00:00:00 2001 From: Jack Berg Date: Fri, 6 Jun 2025 14:40:35 -0500 Subject: [PATCH 6/6] Improve API ergonomics --- .../DefaultExceptionAttributeResolver.java | 23 ++++++++----------- .../internal/ExceptionAttributeResolver.java | 23 +++++++++++++++++++ ...DefaultExceptionAttributeResolverTest.java | 4 ++-- .../sdk/logs/SdkLoggerProviderBuilder.java | 3 +-- .../sdk/logs/LoggerSharedStateTest.java | 4 ++-- .../sdk/trace/SdkTracerProviderBuilder.java | 3 +-- .../opentelemetry/sdk/trace/SdkSpanTest.java | 17 +++++++------- .../sdk/trace/SdkTracerProviderTest.java | 3 +-- 8 files changed, 47 insertions(+), 33 deletions(-) diff --git a/sdk/common/src/main/java/io/opentelemetry/sdk/internal/DefaultExceptionAttributeResolver.java b/sdk/common/src/main/java/io/opentelemetry/sdk/internal/DefaultExceptionAttributeResolver.java index d05633624f4..f3b75731c72 100644 --- a/sdk/common/src/main/java/io/opentelemetry/sdk/internal/DefaultExceptionAttributeResolver.java +++ b/sdk/common/src/main/java/io/opentelemetry/sdk/internal/DefaultExceptionAttributeResolver.java @@ -5,35 +5,30 @@ package io.opentelemetry.sdk.internal; -import io.opentelemetry.api.internal.ConfigUtil; import java.io.PrintWriter; import java.io.StringWriter; /** - * This class is internal and experimental. Its APIs are unstable and can change at any time. Its + * The default {@link ExceptionAttributeResolver}, populating standard {@code exception.*} as + * defined by the semantic conventions. + * + *

This class is internal and experimental. Its APIs are unstable and can change at any time. Its * APIs (or a version of them) may be promoted to the public stable API in the future, but no * guarantees are made. + * + * @see ExceptionAttributeResolver#getDefault() + * @see ExceptionAttributeResolver#getDefault(boolean) () */ -public final class DefaultExceptionAttributeResolver implements ExceptionAttributeResolver { +final class DefaultExceptionAttributeResolver implements ExceptionAttributeResolver { - private static final String ENABLE_JVM_STACKTRACE_PROPERTY = - "otel.experimental.sdk.jvm_stacktrace"; - - private static final DefaultExceptionAttributeResolver INSTANCE = - new DefaultExceptionAttributeResolver( - Boolean.parseBoolean(ConfigUtil.getString(ENABLE_JVM_STACKTRACE_PROPERTY, "false"))); + static final String ENABLE_JVM_STACKTRACE_PROPERTY = "otel.experimental.sdk.jvm_stacktrace"; private final boolean jvmStacktraceEnabled; - // Visible for testing DefaultExceptionAttributeResolver(boolean jvmStacktraceEnabled) { this.jvmStacktraceEnabled = jvmStacktraceEnabled; } - public static ExceptionAttributeResolver getInstance() { - return INSTANCE; - } - @Override public void setExceptionAttributes( AttributeSetter attributeSetter, Throwable throwable, int maxAttributeLength) { diff --git a/sdk/common/src/main/java/io/opentelemetry/sdk/internal/ExceptionAttributeResolver.java b/sdk/common/src/main/java/io/opentelemetry/sdk/internal/ExceptionAttributeResolver.java index 4b9d7a8dc9d..9e6378e880e 100644 --- a/sdk/common/src/main/java/io/opentelemetry/sdk/internal/ExceptionAttributeResolver.java +++ b/sdk/common/src/main/java/io/opentelemetry/sdk/internal/ExceptionAttributeResolver.java @@ -5,7 +5,10 @@ package io.opentelemetry.sdk.internal; +import static io.opentelemetry.sdk.internal.DefaultExceptionAttributeResolver.ENABLE_JVM_STACKTRACE_PROPERTY; + import io.opentelemetry.api.common.AttributeKey; +import io.opentelemetry.api.internal.ConfigUtil; import javax.annotation.Nullable; /** @@ -24,6 +27,26 @@ public interface ExceptionAttributeResolver { void setExceptionAttributes( AttributeSetter attributeSetter, Throwable throwable, int maxAttributeLength); + /** + * Return the default exception attribute resolver, setting {@code jvmStacktraceEnabled} based on + * {@link DefaultExceptionAttributeResolver#ENABLE_JVM_STACKTRACE_PROPERTY}. + */ + static ExceptionAttributeResolver getDefault() { + return getDefault( + Boolean.parseBoolean(ConfigUtil.getString(ENABLE_JVM_STACKTRACE_PROPERTY, "false"))); + } + + /** + * Return the default exception attribute resolver. + * + * @param jvmStacktraceEnabled if true, resolve stacktrace using the stacktrace renderer built + * into the JVM. This built in JVM renderer is not attribute limits aware, and may utilize + * more CPU / memory than is needed. Most users will prefer to set this to {@code false}. + */ + static ExceptionAttributeResolver getDefault(boolean jvmStacktraceEnabled) { + return new DefaultExceptionAttributeResolver(jvmStacktraceEnabled); + } + /** * This class is internal and experimental. Its APIs are unstable and can change at any time. Its * APIs (or a version of them) may be promoted to the public stable API in the future, but no diff --git a/sdk/common/src/test/java/io/opentelemetry/sdk/internal/DefaultExceptionAttributeResolverTest.java b/sdk/common/src/test/java/io/opentelemetry/sdk/internal/DefaultExceptionAttributeResolverTest.java index de93d4d9dc9..5c4446f0cbc 100644 --- a/sdk/common/src/test/java/io/opentelemetry/sdk/internal/DefaultExceptionAttributeResolverTest.java +++ b/sdk/common/src/test/java/io/opentelemetry/sdk/internal/DefaultExceptionAttributeResolverTest.java @@ -28,8 +28,8 @@ void setExceptionAttributes( String expectedMessage, String expectedType, Predicate validStacktrace) { - DefaultExceptionAttributeResolver resolver = - new DefaultExceptionAttributeResolver(jvmStacktraceEnabled); + ExceptionAttributeResolver resolver = + ExceptionAttributeResolver.getDefault(jvmStacktraceEnabled); Map attributes = new HashMap<>(); resolver.setExceptionAttributes( diff --git a/sdk/logs/src/main/java/io/opentelemetry/sdk/logs/SdkLoggerProviderBuilder.java b/sdk/logs/src/main/java/io/opentelemetry/sdk/logs/SdkLoggerProviderBuilder.java index 25fdeaa4454..41ce7d44980 100644 --- a/sdk/logs/src/main/java/io/opentelemetry/sdk/logs/SdkLoggerProviderBuilder.java +++ b/sdk/logs/src/main/java/io/opentelemetry/sdk/logs/SdkLoggerProviderBuilder.java @@ -12,7 +12,6 @@ import io.opentelemetry.context.Context; import io.opentelemetry.sdk.common.Clock; import io.opentelemetry.sdk.common.InstrumentationScopeInfo; -import io.opentelemetry.sdk.internal.DefaultExceptionAttributeResolver; import io.opentelemetry.sdk.internal.ExceptionAttributeResolver; import io.opentelemetry.sdk.internal.ScopeConfigurator; import io.opentelemetry.sdk.internal.ScopeConfiguratorBuilder; @@ -40,7 +39,7 @@ public final class SdkLoggerProviderBuilder { private ScopeConfiguratorBuilder loggerConfiguratorBuilder = LoggerConfig.configuratorBuilder(); private ExceptionAttributeResolver exceptionAttributeResolver = - DefaultExceptionAttributeResolver.getInstance(); + ExceptionAttributeResolver.getDefault(); SdkLoggerProviderBuilder() {} diff --git a/sdk/logs/src/test/java/io/opentelemetry/sdk/logs/LoggerSharedStateTest.java b/sdk/logs/src/test/java/io/opentelemetry/sdk/logs/LoggerSharedStateTest.java index 31b46c65206..7aa4f02aa0a 100644 --- a/sdk/logs/src/test/java/io/opentelemetry/sdk/logs/LoggerSharedStateTest.java +++ b/sdk/logs/src/test/java/io/opentelemetry/sdk/logs/LoggerSharedStateTest.java @@ -12,7 +12,7 @@ import io.opentelemetry.sdk.common.Clock; import io.opentelemetry.sdk.common.CompletableResultCode; -import io.opentelemetry.sdk.internal.DefaultExceptionAttributeResolver; +import io.opentelemetry.sdk.internal.ExceptionAttributeResolver; import io.opentelemetry.sdk.resources.Resource; import org.junit.jupiter.api.Test; @@ -29,7 +29,7 @@ void shutdown() { LogLimits::getDefault, logRecordProcessor, Clock.getDefault(), - DefaultExceptionAttributeResolver.getInstance()); + ExceptionAttributeResolver.getDefault()); state.shutdown(); state.shutdown(); verify(logRecordProcessor, times(1)).shutdown(); diff --git a/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/SdkTracerProviderBuilder.java b/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/SdkTracerProviderBuilder.java index 194aa67bc17..71f589d15b6 100644 --- a/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/SdkTracerProviderBuilder.java +++ b/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/SdkTracerProviderBuilder.java @@ -10,7 +10,6 @@ import io.opentelemetry.api.trace.Span; import io.opentelemetry.sdk.common.Clock; import io.opentelemetry.sdk.common.InstrumentationScopeInfo; -import io.opentelemetry.sdk.internal.DefaultExceptionAttributeResolver; import io.opentelemetry.sdk.internal.ExceptionAttributeResolver; import io.opentelemetry.sdk.internal.ScopeConfigurator; import io.opentelemetry.sdk.internal.ScopeConfiguratorBuilder; @@ -38,7 +37,7 @@ public final class SdkTracerProviderBuilder { private ScopeConfiguratorBuilder tracerConfiguratorBuilder = TracerConfig.configuratorBuilder(); private ExceptionAttributeResolver exceptionAttributeResolver = - DefaultExceptionAttributeResolver.getInstance(); + ExceptionAttributeResolver.getDefault(); /** * Assign a {@link Clock}. {@link Clock} will be used each time a {@link Span} is started, ended diff --git a/sdk/trace/src/test/java/io/opentelemetry/sdk/trace/SdkSpanTest.java b/sdk/trace/src/test/java/io/opentelemetry/sdk/trace/SdkSpanTest.java index fcfec65d47f..d5c40fcccbf 100644 --- a/sdk/trace/src/test/java/io/opentelemetry/sdk/trace/SdkSpanTest.java +++ b/sdk/trace/src/test/java/io/opentelemetry/sdk/trace/SdkSpanTest.java @@ -38,7 +38,6 @@ import io.opentelemetry.context.Context; import io.opentelemetry.sdk.common.InstrumentationScopeInfo; import io.opentelemetry.sdk.internal.AttributesMap; -import io.opentelemetry.sdk.internal.DefaultExceptionAttributeResolver; import io.opentelemetry.sdk.internal.ExceptionAttributeResolver; import io.opentelemetry.sdk.internal.InstrumentationScopeUtil; import io.opentelemetry.sdk.resources.Resource; @@ -946,7 +945,7 @@ void addLink() { parentSpanId, null, null, - DefaultExceptionAttributeResolver.getInstance()); + ExceptionAttributeResolver.getDefault()); try { Span span1 = createTestSpan(SpanKind.INTERNAL); Span span2 = createTestSpan(SpanKind.INTERNAL); @@ -1036,7 +1035,7 @@ void addLink_FaultIn() { Context.root(), SpanLimits.getDefault(), spanProcessor, - DefaultExceptionAttributeResolver.getInstance(), + ExceptionAttributeResolver.getDefault(), testClock, resource, null, @@ -1381,7 +1380,7 @@ void onStartOnEndNotRequired() { Context.root(), spanLimits, spanProcessor, - DefaultExceptionAttributeResolver.getInstance(), + ExceptionAttributeResolver.getDefault(), testClock, resource, AttributesMap.create( @@ -1464,7 +1463,7 @@ private SdkSpan createTestSpanWithAttributes(Map attribute null, attributesMap, Collections.singletonList(link), - DefaultExceptionAttributeResolver.getInstance()); + ExceptionAttributeResolver.getDefault()); } private SdkSpan createTestRootSpan() { @@ -1474,7 +1473,7 @@ private SdkSpan createTestRootSpan() { SpanId.getInvalid(), null, Collections.singletonList(link), - DefaultExceptionAttributeResolver.getInstance()); + ExceptionAttributeResolver.getDefault()); } private SdkSpan createTestSpan(SpanKind kind) { @@ -1484,7 +1483,7 @@ private SdkSpan createTestSpan(SpanKind kind) { parentSpanId, null, Collections.singletonList(link), - DefaultExceptionAttributeResolver.getInstance()); + ExceptionAttributeResolver.getDefault()); } private SdkSpan createTestSpan(SpanLimits config) { @@ -1494,7 +1493,7 @@ private SdkSpan createTestSpan(SpanLimits config) { parentSpanId, null, Collections.singletonList(link), - DefaultExceptionAttributeResolver.getInstance()); + ExceptionAttributeResolver.getDefault()); } private SdkSpan createTestSpan( @@ -1608,7 +1607,7 @@ void testAsSpanData() { Context.root(), spanLimits, spanProcessor, - DefaultExceptionAttributeResolver.getInstance(), + ExceptionAttributeResolver.getDefault(), clock, resource, attributesWithCapacity, diff --git a/sdk/trace/src/test/java/io/opentelemetry/sdk/trace/SdkTracerProviderTest.java b/sdk/trace/src/test/java/io/opentelemetry/sdk/trace/SdkTracerProviderTest.java index 345f97d52c0..9fb7724524a 100644 --- a/sdk/trace/src/test/java/io/opentelemetry/sdk/trace/SdkTracerProviderTest.java +++ b/sdk/trace/src/test/java/io/opentelemetry/sdk/trace/SdkTracerProviderTest.java @@ -22,7 +22,6 @@ import io.opentelemetry.sdk.common.Clock; import io.opentelemetry.sdk.common.CompletableResultCode; import io.opentelemetry.sdk.common.InstrumentationScopeInfo; -import io.opentelemetry.sdk.internal.DefaultExceptionAttributeResolver; import io.opentelemetry.sdk.internal.ExceptionAttributeResolver; import io.opentelemetry.sdk.internal.ScopeConfigurator; import io.opentelemetry.sdk.resources.Resource; @@ -298,7 +297,7 @@ void exceptionAttributeResolver() { .setSpanLimits( SpanLimits.builder().setMaxAttributeValueLength(maxAttributeLength).build()); ExceptionAttributeResolver exceptionAttributeResolver = - spy(DefaultExceptionAttributeResolver.getInstance()); + spy(ExceptionAttributeResolver.getDefault()); SdkTracerProviderUtil.setExceptionAttributeResolver(builder, exceptionAttributeResolver); Exception exception = new Exception("error");