From bbaa9e50dd54a0975673f42fb7dcdb005258aa99 Mon Sep 17 00:00:00 2001 From: Jack Berg Date: Tue, 4 Mar 2025 16:38:24 -0600 Subject: [PATCH 1/4] Lazily evaluate ExceptionEventData exception.* attributes --- .../io/opentelemetry/sdk/trace/SdkSpan.java | 38 +------ .../internal/data/LazyExceptionEventData.java | 100 ++++++++++++++++++ 2 files changed, 103 insertions(+), 35 deletions(-) create mode 100644 sdk/trace/src/main/java/io/opentelemetry/sdk/trace/internal/data/LazyExceptionEventData.java 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 c1c788bf931..2531fe3e0fb 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 @@ -20,13 +20,11 @@ import io.opentelemetry.sdk.internal.InstrumentationScopeUtil; import io.opentelemetry.sdk.resources.Resource; import io.opentelemetry.sdk.trace.data.EventData; -import io.opentelemetry.sdk.trace.data.ExceptionEventData; import io.opentelemetry.sdk.trace.data.LinkData; import io.opentelemetry.sdk.trace.data.SpanData; import io.opentelemetry.sdk.trace.data.StatusData; import io.opentelemetry.sdk.trace.internal.ExtendedSpanProcessor; -import java.io.PrintWriter; -import java.io.StringWriter; +import io.opentelemetry.sdk.trace.internal.data.LazyExceptionEventData; import java.util.ArrayList; import java.util.Collections; import java.util.List; @@ -117,13 +115,6 @@ private enum EndState { @Nullable private Thread spanEndingThread; - private static final AttributeKey EXCEPTION_TYPE = - AttributeKey.stringKey("exception.type"); - private static final AttributeKey EXCEPTION_MESSAGE = - AttributeKey.stringKey("exception.message"); - private static final AttributeKey EXCEPTION_STACKTRACE = - AttributeKey.stringKey("exception.stacktrace"); - private SdkSpan( SpanContext context, String name, @@ -475,32 +466,9 @@ public ReadWriteSpan recordException(Throwable exception, Attributes additionalA additionalAttributes = Attributes.empty(); } - AttributesMap attributes = - AttributesMap.create( - spanLimits.getMaxNumberOfAttributes(), spanLimits.getMaxAttributeValueLength()); - String exceptionName = exception.getClass().getCanonicalName(); - String exceptionMessage = exception.getMessage(); - StringWriter stringWriter = new StringWriter(); - try (PrintWriter printWriter = new PrintWriter(stringWriter)) { - exception.printStackTrace(printWriter); - } - String stackTrace = stringWriter.toString(); - - if (exceptionName != null) { - attributes.put(EXCEPTION_TYPE, exceptionName); - } - if (exceptionMessage != null) { - attributes.put(EXCEPTION_MESSAGE, exceptionMessage); - } - if (stackTrace != null) { - attributes.put(EXCEPTION_STACKTRACE, stackTrace); - } - - additionalAttributes.forEach(attributes::put); - addTimedEvent( - ExceptionEventData.create( - clock.now(), exception, attributes, attributes.getTotalAddedValues())); + LazyExceptionEventData.create(clock.now(), exception, additionalAttributes, spanLimits)); + return this; } diff --git a/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/internal/data/LazyExceptionEventData.java b/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/internal/data/LazyExceptionEventData.java new file mode 100644 index 00000000000..2bd08f68bed --- /dev/null +++ b/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/internal/data/LazyExceptionEventData.java @@ -0,0 +1,100 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.sdk.trace.internal.data; + +import com.google.auto.value.AutoValue; +import com.google.auto.value.extension.memoized.Memoized; +import io.opentelemetry.api.common.AttributeKey; +import io.opentelemetry.api.common.Attributes; +import io.opentelemetry.sdk.internal.AttributesMap; +import io.opentelemetry.sdk.trace.SpanLimits; +import io.opentelemetry.sdk.trace.data.ExceptionEventData; +import java.io.PrintWriter; +import java.io.StringWriter; +import javax.annotation.concurrent.Immutable; + +/** + * An {@link ExceptionEventData} implementation with {@link #getAttributes()} lazily evaluated, + * allowing the (relatively) expensive exception attribute rendering to take place off the hot path. + * + *

This class is internal and is hence not for public use. Its APIs are unstable and can change + * at any time. + */ +@AutoValue +@Immutable +public abstract class LazyExceptionEventData implements ExceptionEventData { + + private static final AttributeKey EXCEPTION_TYPE = + AttributeKey.stringKey("exception.type"); + private static final AttributeKey EXCEPTION_MESSAGE = + AttributeKey.stringKey("exception.message"); + private static final AttributeKey EXCEPTION_STACKTRACE = + AttributeKey.stringKey("exception.stacktrace"); + private static final String EXCEPTION_EVENT_NAME = "exception"; + + @Override + public final String getName() { + return EXCEPTION_EVENT_NAME; + } + + /** TODO. */ + public static ExceptionEventData create( + long epochNanos, + Throwable exception, + Attributes additionalAttributes, + SpanLimits spanLimits) { + return new AutoValue_LazyExceptionEventData( + epochNanos, exception, spanLimits, additionalAttributes); + } + + abstract SpanLimits getSpanLimits(); + + public abstract Attributes getAdditionalAttributes(); + + @Override + @Memoized + public Attributes getAttributes() { + Throwable exception = getException(); + SpanLimits spanLimits = getSpanLimits(); + Attributes additionalAttributes = getAdditionalAttributes(); + + AttributesMap attributes = + AttributesMap.create( + spanLimits.getMaxNumberOfAttributes(), spanLimits.getMaxAttributeValueLength()); + String exceptionName = exception.getClass().getCanonicalName(); + String exceptionMessage = exception.getMessage(); + StringWriter stringWriter = new StringWriter(); + try (PrintWriter printWriter = new PrintWriter(stringWriter)) { + exception.printStackTrace(printWriter); + } + String stackTrace = stringWriter.toString(); + + if (exceptionName != null) { + attributes.put(EXCEPTION_TYPE, exceptionName); + } + if (exceptionMessage != null) { + attributes.put(EXCEPTION_MESSAGE, exceptionMessage); + } + if (stackTrace != null) { + attributes.put(EXCEPTION_STACKTRACE, stackTrace); + } + + additionalAttributes.forEach(attributes::put); + + return attributes.immutableCopy(); + } + + @Override + public int getTotalAttributeCount() { + // getAttributes() lazily adds 3 attributes to getAdditionalAttributes(): + // - exception.type + // - exception.message + // - exception.stacktrace + return getAdditionalAttributes().size() + 3; + } + + LazyExceptionEventData() {} +} From 37e47a259c2ab45364a3cb797b31817f5d9d5cc5 Mon Sep 17 00:00:00 2001 From: Jack Berg Date: Thu, 10 Apr 2025 16:16:38 -0500 Subject: [PATCH 2/4] Compute exception.message on initialization --- .../internal/data/LazyExceptionEventData.java | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/internal/data/LazyExceptionEventData.java b/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/internal/data/LazyExceptionEventData.java index 2bd08f68bed..117edf69ae7 100644 --- a/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/internal/data/LazyExceptionEventData.java +++ b/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/internal/data/LazyExceptionEventData.java @@ -14,6 +14,7 @@ import io.opentelemetry.sdk.trace.data.ExceptionEventData; import java.io.PrintWriter; import java.io.StringWriter; +import javax.annotation.Nullable; import javax.annotation.concurrent.Immutable; /** @@ -40,18 +41,27 @@ public final String getName() { return EXCEPTION_EVENT_NAME; } - /** TODO. */ + // Autowire generates AutoValue_LazyExceptionEventData and $AutoValue_LazyExceptionEventData to + // account to memoize getAttributes. Unfortunately, $AutoValue_LazyExceptionEventData's + // exceptionMessage constructor param is properly annotated with @Nullable, but + // AutoValue_LazyExceptionEventData's is not. So we suppress the NullAway false positive. + @SuppressWarnings("NullAway") public static ExceptionEventData create( long epochNanos, Throwable exception, Attributes additionalAttributes, SpanLimits spanLimits) { + // Compute exception message at initialization time to be conservative about possibility of + // Exception#geMessage() not being thread safe return new AutoValue_LazyExceptionEventData( - epochNanos, exception, spanLimits, additionalAttributes); + epochNanos, exception, spanLimits, exception.getMessage(), additionalAttributes); } abstract SpanLimits getSpanLimits(); + @Nullable + abstract String getExceptionMessage(); + public abstract Attributes getAdditionalAttributes(); @Override @@ -65,7 +75,7 @@ public Attributes getAttributes() { AttributesMap.create( spanLimits.getMaxNumberOfAttributes(), spanLimits.getMaxAttributeValueLength()); String exceptionName = exception.getClass().getCanonicalName(); - String exceptionMessage = exception.getMessage(); + String exceptionMessage = getExceptionMessage(); StringWriter stringWriter = new StringWriter(); try (PrintWriter printWriter = new PrintWriter(stringWriter)) { exception.printStackTrace(printWriter); From 2fe42337c0ab4568fe5e9df23285fa0f211cbdf8 Mon Sep 17 00:00:00 2001 From: Jack Berg Date: Fri, 11 Apr 2025 10:49:02 -0500 Subject: [PATCH 3/4] Remove extra code from merge --- .../internal/data/LazyExceptionEventData.java | 22 ------------------- 1 file changed, 22 deletions(-) diff --git a/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/internal/data/LazyExceptionEventData.java b/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/internal/data/LazyExceptionEventData.java index d0f82b49a52..34d0bc475b7 100644 --- a/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/internal/data/LazyExceptionEventData.java +++ b/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/internal/data/LazyExceptionEventData.java @@ -13,8 +13,6 @@ import io.opentelemetry.sdk.internal.AttributesMap; import io.opentelemetry.sdk.trace.SpanLimits; import io.opentelemetry.sdk.trace.data.ExceptionEventData; -import java.io.PrintWriter; -import java.io.StringWriter; import javax.annotation.Nullable; import javax.annotation.concurrent.Immutable; @@ -80,26 +78,6 @@ public Attributes getAttributes() { additionalAttributes.forEach(attributes::put); - String exceptionName = exception.getClass().getCanonicalName(); - String exceptionMessage = getExceptionMessage(); - StringWriter stringWriter = new StringWriter(); - try (PrintWriter printWriter = new PrintWriter(stringWriter)) { - exception.printStackTrace(printWriter); - } - String stackTrace = stringWriter.toString(); - - if (exceptionName != null) { - attributes.put(EXCEPTION_TYPE, exceptionName); - } - if (exceptionMessage != null) { - attributes.put(EXCEPTION_MESSAGE, exceptionMessage); - } - if (stackTrace != null) { - attributes.put(EXCEPTION_STACKTRACE, stackTrace); - } - - additionalAttributes.forEach(attributes::put); - return attributes.immutableCopy(); } From a7931ca986564fc50b33218f7082ec5d93be8ddc Mon Sep 17 00:00:00 2001 From: Jack Berg Date: Fri, 11 Apr 2025 10:49:43 -0500 Subject: [PATCH 4/4] Use precomputed exception message --- .../sdk/trace/internal/data/LazyExceptionEventData.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/internal/data/LazyExceptionEventData.java b/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/internal/data/LazyExceptionEventData.java index 34d0bc475b7..aba715d0daf 100644 --- a/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/internal/data/LazyExceptionEventData.java +++ b/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/internal/data/LazyExceptionEventData.java @@ -74,7 +74,7 @@ public Attributes getAttributes() { AttributesMap.create( spanLimits.getMaxNumberOfAttributes(), spanLimits.getMaxAttributeValueLength()); - AttributeUtil.addExceptionAttributes(attributes::put, exception); + AttributeUtil.addExceptionAttributes(attributes::put, exception, getExceptionMessage()); additionalAttributes.forEach(attributes::put);