From 9f2ecdebb720d31613da42b70bb27118411eeaa8 Mon Sep 17 00:00:00 2001 From: Jack Berg Date: Thu, 6 Mar 2025 12:37:55 -0600 Subject: [PATCH 1/8] Add ExtendedLogRecordBuilder#setException, make exception.* resolution configurable --- .../incubator/logs/ExtendedDefaultLogger.java | 5 + .../logs/ExtendedLogRecordBuilder.java | 3 + .../kotlin/otel.java-conventions.gradle.kts | 4 +- .../DefaultExceptionAttributeResolver.java | 49 +++++++ .../internal/ExceptionAttributeResolver.java | 44 +++++++ .../sdk/logs/ExtendedSdkLogRecordBuilder.java | 6 + .../sdk/logs/LoggerSharedState.java | 10 +- .../sdk/logs/SdkLogRecordBuilder.java | 27 ++++ .../sdk/logs/SdkLoggerProvider.java | 7 +- .../sdk/logs/SdkLoggerProviderBuilder.java | 26 +++- .../logs/internal/SdkLoggerProviderUtil.java | 17 +++ .../sdk/logs/LoggerSharedStateTest.java | 7 +- .../sdk/logs/ExtendedLoggerBuilderTest.java | 86 +++++++++++++ .../io/opentelemetry/sdk/trace/SdkSpan.java | 43 +++---- .../sdk/trace/SdkSpanBuilder.java | 1 + .../sdk/trace/SdkTracerProvider.java | 12 +- .../sdk/trace/SdkTracerProviderBuilder.java | 42 ++++-- .../sdk/trace/TracerSharedState.java | 13 +- .../trace/internal/SdkTracerProviderUtil.java | 17 +++ .../opentelemetry/sdk/trace/SdkSpanTest.java | 120 +++++++++++++----- .../sdk/trace/SdkTracerProviderTest.java | 74 +++++++---- 21 files changed, 511 insertions(+), 102 deletions(-) create mode 100644 sdk/common/src/main/java/io/opentelemetry/sdk/internal/DefaultExceptionAttributeResolver.java create mode 100644 sdk/common/src/main/java/io/opentelemetry/sdk/internal/ExceptionAttributeResolver.java create mode 100644 sdk/logs/src/testIncubating/java/io/opentelemetry/sdk/logs/ExtendedLoggerBuilderTest.java diff --git a/api/incubator/src/main/java/io/opentelemetry/api/incubator/logs/ExtendedDefaultLogger.java b/api/incubator/src/main/java/io/opentelemetry/api/incubator/logs/ExtendedDefaultLogger.java index 378af1c6b00..d9a95a6d2d4 100644 --- a/api/incubator/src/main/java/io/opentelemetry/api/incubator/logs/ExtendedDefaultLogger.java +++ b/api/incubator/src/main/java/io/opentelemetry/api/incubator/logs/ExtendedDefaultLogger.java @@ -40,6 +40,11 @@ public ExtendedLogRecordBuilder setEventName(String eventName) { return this; } + @Override + public ExtendedLogRecordBuilder setException(Throwable throwable) { + return this; + } + @Override public LogRecordBuilder setTimestamp(long timestamp, TimeUnit unit) { return this; diff --git a/api/incubator/src/main/java/io/opentelemetry/api/incubator/logs/ExtendedLogRecordBuilder.java b/api/incubator/src/main/java/io/opentelemetry/api/incubator/logs/ExtendedLogRecordBuilder.java index 4a18baa07ed..d6fde3699b1 100644 --- a/api/incubator/src/main/java/io/opentelemetry/api/incubator/logs/ExtendedLogRecordBuilder.java +++ b/api/incubator/src/main/java/io/opentelemetry/api/incubator/logs/ExtendedLogRecordBuilder.java @@ -19,4 +19,7 @@ public interface ExtendedLogRecordBuilder extends LogRecordBuilder { * record with a non-empty event name is an Event. */ ExtendedLogRecordBuilder setEventName(String eventName); + + /** Set standard {@code exception.*} attributes based on the {@code throwable}. */ + ExtendedLogRecordBuilder setException(Throwable throwable); } diff --git a/buildSrc/src/main/kotlin/otel.java-conventions.gradle.kts b/buildSrc/src/main/kotlin/otel.java-conventions.gradle.kts index ea6230fda36..95ca6c9026c 100644 --- a/buildSrc/src/main/kotlin/otel.java-conventions.gradle.kts +++ b/buildSrc/src/main/kotlin/otel.java-conventions.gradle.kts @@ -87,9 +87,7 @@ tasks { // https://groups.google.com/forum/#!topic/bazel-discuss/_R3A9TJSoPM "-Xlint:-processing", // We suppress the "options" warning because it prevents compilation on modern JDKs - "-Xlint:-options", - // Fail build on any warning - "-Werror", + "-Xlint:-options" ), ) } 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 new file mode 100644 index 00000000000..af5fb8f77f8 --- /dev/null +++ b/sdk/common/src/main/java/io/opentelemetry/sdk/internal/DefaultExceptionAttributeResolver.java @@ -0,0 +1,49 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.sdk.internal; + +import java.io.PrintWriter; +import java.io.StringWriter; +import javax.annotation.Nullable; + +/** + * 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. + */ +public final class DefaultExceptionAttributeResolver implements ExceptionAttributeResolver { + + private static final DefaultExceptionAttributeResolver INSTANCE = + new DefaultExceptionAttributeResolver(); + + private DefaultExceptionAttributeResolver() {} + + public static ExceptionAttributeResolver getInstance() { + return INSTANCE; + } + + @Override + @Nullable + public String getExceptionType(Throwable throwable) { + return throwable.getClass().getCanonicalName(); + } + + @Override + @Nullable + public String getExceptionMessage(Throwable throwable) { + return throwable.getMessage(); + } + + @Override + @Nullable + public String getExceptionStacktrace(Throwable throwable) { + StringWriter stringWriter = new StringWriter(); + try (PrintWriter printWriter = new PrintWriter(stringWriter)) { + throwable.printStackTrace(printWriter); + } + return stringWriter.toString(); + } +} 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 new file mode 100644 index 00000000000..b5c26fc56a6 --- /dev/null +++ b/sdk/common/src/main/java/io/opentelemetry/sdk/internal/ExceptionAttributeResolver.java @@ -0,0 +1,44 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.sdk.internal; + +import io.opentelemetry.api.common.AttributeKey; +import javax.annotation.Nullable; + +/** + * Implementations resolve {@code exception.*} attributes attached to span events, logs, etc. + * + *

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. + */ +public interface ExceptionAttributeResolver { + + AttributeKey EXCEPTION_TYPE = AttributeKey.stringKey("exception.type"); + AttributeKey EXCEPTION_MESSAGE = AttributeKey.stringKey("exception.message"); + AttributeKey EXCEPTION_STACKTRACE = AttributeKey.stringKey("exception.stacktrace"); + + /** + * Resolve the {@link #EXCEPTION_TYPE} attribute from the {@code throwable}, or {@code null} if no + * value should be set. + */ + @Nullable + String getExceptionType(Throwable throwable); + + /** + * Resolve the {@link #EXCEPTION_MESSAGE} attribute from the {@code throwable}, or {@code null} if + * no value should be set. + */ + @Nullable + String getExceptionMessage(Throwable throwable); + + /** + * Resolve the {@link #EXCEPTION_STACKTRACE} attribute from the {@code throwable}, or {@code null} + * if no value should be set. + */ + @Nullable + String getExceptionStacktrace(Throwable throwable); +} 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 9f874d1f4b3..97f337cc194 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 @@ -29,6 +29,12 @@ public ExtendedSdkLogRecordBuilder setEventName(String eventName) { return this; } + @Override + public ExtendedSdkLogRecordBuilder setException(Throwable throwable) { + super.setException(throwable); + return this; + } + @Override public ExtendedSdkLogRecordBuilder setTimestamp(long timestamp, TimeUnit unit) { super.setTimestamp(timestamp, unit); diff --git a/sdk/logs/src/main/java/io/opentelemetry/sdk/logs/LoggerSharedState.java b/sdk/logs/src/main/java/io/opentelemetry/sdk/logs/LoggerSharedState.java index 768871e1e57..5b40f897a32 100644 --- a/sdk/logs/src/main/java/io/opentelemetry/sdk/logs/LoggerSharedState.java +++ b/sdk/logs/src/main/java/io/opentelemetry/sdk/logs/LoggerSharedState.java @@ -7,6 +7,7 @@ import io.opentelemetry.sdk.common.Clock; import io.opentelemetry.sdk.common.CompletableResultCode; +import io.opentelemetry.sdk.internal.ExceptionAttributeResolver; import io.opentelemetry.sdk.resources.Resource; import java.util.function.Supplier; import javax.annotation.Nullable; @@ -21,17 +22,20 @@ final class LoggerSharedState { private final Supplier logLimitsSupplier; private final LogRecordProcessor logRecordProcessor; private final Clock clock; + private final ExceptionAttributeResolver exceptionAttributeResolver; @Nullable private volatile CompletableResultCode shutdownResult = null; LoggerSharedState( Resource resource, Supplier logLimitsSupplier, LogRecordProcessor logRecordProcessor, - Clock clock) { + Clock clock, + ExceptionAttributeResolver exceptionAttributeResolver) { this.resource = resource; this.logLimitsSupplier = logLimitsSupplier; this.logRecordProcessor = logRecordProcessor; this.clock = clock; + this.exceptionAttributeResolver = exceptionAttributeResolver; } Resource getResource() { @@ -50,6 +54,10 @@ Clock getClock() { return clock; } + ExceptionAttributeResolver getExceptionAttributeResolver() { + return exceptionAttributeResolver; + } + boolean hasBeenShutdown() { return shutdownResult != null; } diff --git a/sdk/logs/src/main/java/io/opentelemetry/sdk/logs/SdkLogRecordBuilder.java b/sdk/logs/src/main/java/io/opentelemetry/sdk/logs/SdkLogRecordBuilder.java index da7e1f45e7e..48af664b4f2 100644 --- a/sdk/logs/src/main/java/io/opentelemetry/sdk/logs/SdkLogRecordBuilder.java +++ b/sdk/logs/src/main/java/io/opentelemetry/sdk/logs/SdkLogRecordBuilder.java @@ -5,6 +5,10 @@ package io.opentelemetry.sdk.logs; +import static io.opentelemetry.sdk.internal.ExceptionAttributeResolver.EXCEPTION_MESSAGE; +import static io.opentelemetry.sdk.internal.ExceptionAttributeResolver.EXCEPTION_STACKTRACE; +import static io.opentelemetry.sdk.internal.ExceptionAttributeResolver.EXCEPTION_TYPE; + import io.opentelemetry.api.common.AttributeKey; import io.opentelemetry.api.common.Value; import io.opentelemetry.api.logs.LogRecordBuilder; @@ -13,6 +17,7 @@ import io.opentelemetry.context.Context; import io.opentelemetry.sdk.common.InstrumentationScopeInfo; import io.opentelemetry.sdk.internal.AttributesMap; +import io.opentelemetry.sdk.internal.ExceptionAttributeResolver; import java.time.Instant; import java.util.concurrent.TimeUnit; import javax.annotation.Nullable; @@ -46,6 +51,28 @@ SdkLogRecordBuilder setEventName(String eventName) { return this; } + // accessible via ExtendedSdkLogRecordBuilder + SdkLogRecordBuilder setException(Throwable throwable) { + if (throwable == null) { + return this; + } + ExceptionAttributeResolver exceptionAttributeResolver = + loggerSharedState.getExceptionAttributeResolver(); + String type = exceptionAttributeResolver.getExceptionType(throwable); + if (type != null) { + setAttribute(EXCEPTION_TYPE, type); + } + String message = exceptionAttributeResolver.getExceptionMessage(throwable); + if (message != null) { + setAttribute(EXCEPTION_MESSAGE, message); + } + String stacktrace = exceptionAttributeResolver.getExceptionStacktrace(throwable); + if (stacktrace != null) { + setAttribute(EXCEPTION_STACKTRACE, stacktrace); + } + return this; + } + @Override public SdkLogRecordBuilder setTimestamp(long timestamp, TimeUnit unit) { this.timestampEpochNanos = unit.toNanos(timestamp); diff --git a/sdk/logs/src/main/java/io/opentelemetry/sdk/logs/SdkLoggerProvider.java b/sdk/logs/src/main/java/io/opentelemetry/sdk/logs/SdkLoggerProvider.java index ea68a6c2a5c..b698318cf16 100644 --- a/sdk/logs/src/main/java/io/opentelemetry/sdk/logs/SdkLoggerProvider.java +++ b/sdk/logs/src/main/java/io/opentelemetry/sdk/logs/SdkLoggerProvider.java @@ -13,6 +13,7 @@ import io.opentelemetry.sdk.common.CompletableResultCode; import io.opentelemetry.sdk.common.InstrumentationScopeInfo; import io.opentelemetry.sdk.internal.ComponentRegistry; +import io.opentelemetry.sdk.internal.ExceptionAttributeResolver; import io.opentelemetry.sdk.internal.ScopeConfigurator; import io.opentelemetry.sdk.logs.internal.LoggerConfig; import io.opentelemetry.sdk.resources.Resource; @@ -53,10 +54,12 @@ public static SdkLoggerProviderBuilder builder() { Supplier logLimitsSupplier, List processors, Clock clock, - ScopeConfigurator loggerConfigurator) { + ScopeConfigurator loggerConfigurator, + ExceptionAttributeResolver exceptionAttributeResolver) { LogRecordProcessor logRecordProcessor = LogRecordProcessor.composite(processors); this.sharedState = - new LoggerSharedState(resource, logLimitsSupplier, logRecordProcessor, clock); + new LoggerSharedState( + resource, logLimitsSupplier, logRecordProcessor, clock, exceptionAttributeResolver); this.loggerComponentRegistry = new ComponentRegistry<>( instrumentationScopeInfo -> 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 94d990c8a4b..298a1511f37 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,6 +12,8 @@ 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; import io.opentelemetry.sdk.logs.data.LogRecordData; @@ -37,6 +39,8 @@ public final class SdkLoggerProviderBuilder { private Clock clock = Clock.getDefault(); private ScopeConfiguratorBuilder loggerConfiguratorBuilder = LoggerConfig.configuratorBuilder(); + private ExceptionAttributeResolver exceptionAttributeResolver = + DefaultExceptionAttributeResolver.getInstance(); SdkLoggerProviderBuilder() {} @@ -149,6 +153,21 @@ SdkLoggerProviderBuilder addLoggerConfiguratorCondition( return this; } + /** + * Set the exception attribute resolver, which resolves {@code exception.*} attributes an + * exception is set on a log. + * + *

This method is experimental so not public. You may reflectively call it using {@link + * SdkLoggerProviderUtil#setExceptionAttributeResolver(SdkLoggerProviderBuilder, + * ExceptionAttributeResolver)}. + */ + SdkLoggerProviderBuilder setExceptionAttributeResolver( + ExceptionAttributeResolver exceptionAttributeResolver) { + requireNonNull(exceptionAttributeResolver, "exceptionAttributeResolver"); + this.exceptionAttributeResolver = exceptionAttributeResolver; + return this; + } + /** * Create a {@link SdkLoggerProvider} instance. * @@ -156,6 +175,11 @@ SdkLoggerProviderBuilder addLoggerConfiguratorCondition( */ public SdkLoggerProvider build() { return new SdkLoggerProvider( - resource, logLimitsSupplier, logRecordProcessors, clock, loggerConfiguratorBuilder.build()); + resource, + logLimitsSupplier, + logRecordProcessors, + clock, + loggerConfiguratorBuilder.build(), + exceptionAttributeResolver); } } diff --git a/sdk/logs/src/main/java/io/opentelemetry/sdk/logs/internal/SdkLoggerProviderUtil.java b/sdk/logs/src/main/java/io/opentelemetry/sdk/logs/internal/SdkLoggerProviderUtil.java index eb4fbb4ec29..979e62c5c06 100644 --- a/sdk/logs/src/main/java/io/opentelemetry/sdk/logs/internal/SdkLoggerProviderUtil.java +++ b/sdk/logs/src/main/java/io/opentelemetry/sdk/logs/internal/SdkLoggerProviderUtil.java @@ -6,6 +6,7 @@ package io.opentelemetry.sdk.logs.internal; import io.opentelemetry.sdk.common.InstrumentationScopeInfo; +import io.opentelemetry.sdk.internal.ExceptionAttributeResolver; import io.opentelemetry.sdk.internal.ScopeConfigurator; import io.opentelemetry.sdk.logs.SdkLoggerProviderBuilder; import java.lang.reflect.InvocationTargetException; @@ -55,4 +56,20 @@ public static void addLoggerConfiguratorCondition( "Error calling addLoggerConfiguratorCondition on SdkLoggerProviderBuilder", e); } } + + /** Reflectively set exception attribute resolver to the {@link SdkLoggerProviderBuilder}. */ + public static void setExceptionAttributeResolver( + SdkLoggerProviderBuilder sdkLoggerProviderBuilder, + ExceptionAttributeResolver exceptionAttributeResolver) { + try { + Method method = + SdkLoggerProviderBuilder.class.getDeclaredMethod( + "setExceptionAttributeResolver", ExceptionAttributeResolver.class); + method.setAccessible(true); + method.invoke(sdkLoggerProviderBuilder, exceptionAttributeResolver); + } catch (NoSuchMethodException | IllegalAccessException | InvocationTargetException e) { + throw new IllegalStateException( + "Error calling setExceptionAttributeResolver on SdkLoggerProviderBuilder", e); + } + } } 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 29a3a846a56..31b46c65206 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,6 +12,7 @@ import io.opentelemetry.sdk.common.Clock; import io.opentelemetry.sdk.common.CompletableResultCode; +import io.opentelemetry.sdk.internal.DefaultExceptionAttributeResolver; import io.opentelemetry.sdk.resources.Resource; import org.junit.jupiter.api.Test; @@ -24,7 +25,11 @@ void shutdown() { when(logRecordProcessor.shutdown()).thenReturn(code); LoggerSharedState state = new LoggerSharedState( - Resource.empty(), LogLimits::getDefault, logRecordProcessor, Clock.getDefault()); + Resource.empty(), + LogLimits::getDefault, + logRecordProcessor, + Clock.getDefault(), + DefaultExceptionAttributeResolver.getInstance()); state.shutdown(); state.shutdown(); verify(logRecordProcessor, times(1)).shutdown(); diff --git a/sdk/logs/src/testIncubating/java/io/opentelemetry/sdk/logs/ExtendedLoggerBuilderTest.java b/sdk/logs/src/testIncubating/java/io/opentelemetry/sdk/logs/ExtendedLoggerBuilderTest.java new file mode 100644 index 00000000000..92c36c27f06 --- /dev/null +++ b/sdk/logs/src/testIncubating/java/io/opentelemetry/sdk/logs/ExtendedLoggerBuilderTest.java @@ -0,0 +1,86 @@ +/* + * Copyright The OpenTelemetry Authors + * SPDX-License-Identifier: Apache-2.0 + */ + +package io.opentelemetry.sdk.logs; + +import static io.opentelemetry.sdk.internal.ExceptionAttributeResolver.EXCEPTION_MESSAGE; +import static io.opentelemetry.sdk.internal.ExceptionAttributeResolver.EXCEPTION_STACKTRACE; +import static io.opentelemetry.sdk.internal.ExceptionAttributeResolver.EXCEPTION_TYPE; +import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.assertThat; +import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.equalTo; +import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.satisfies; + +import io.opentelemetry.api.incubator.logs.ExtendedLogRecordBuilder; +import io.opentelemetry.api.logs.Logger; +import io.opentelemetry.sdk.internal.ExceptionAttributeResolver; +import io.opentelemetry.sdk.logs.export.SimpleLogRecordProcessor; +import io.opentelemetry.sdk.logs.internal.SdkLoggerProviderUtil; +import io.opentelemetry.sdk.testing.exporter.InMemoryLogRecordExporter; +import javax.annotation.Nullable; +import org.junit.jupiter.api.Test; + +class ExtendedLoggerBuilderTest { + + private final InMemoryLogRecordExporter exporter = InMemoryLogRecordExporter.create(); + private final SdkLoggerProviderBuilder loggerProviderBuilder = + SdkLoggerProvider.builder().addLogRecordProcessor(SimpleLogRecordProcessor.create(exporter)); + + @Test + void setException_DefaultResolver() { + Logger logger = loggerProviderBuilder.build().get("logger"); + + ((ExtendedLogRecordBuilder) logger.logRecordBuilder()) + .setException(new Exception("error")) + .emit(); + + assertThat(exporter.getFinishedLogRecordItems()) + .satisfiesExactly( + logRecord -> + assertThat(logRecord) + .hasAttributesSatisfyingExactly( + equalTo(EXCEPTION_TYPE, "java.lang.Exception"), + equalTo(EXCEPTION_MESSAGE, "error"), + satisfies( + EXCEPTION_STACKTRACE, + stacktrace -> stacktrace.startsWith("java.lang.Exception: error\n")))); + } + + @Test + void setException_CustomResolver() { + SdkLoggerProviderUtil.setExceptionAttributeResolver( + loggerProviderBuilder, + new ExceptionAttributeResolver() { + @Override + public String getExceptionType(Throwable throwable) { + return "type"; + } + + @Nullable + @Override + public String getExceptionMessage(Throwable throwable) { + return null; + } + + @Override + public String getExceptionStacktrace(Throwable throwable) { + return "stacktrace"; + } + }); + + Logger logger = loggerProviderBuilder.build().get("logger"); + + ((ExtendedLogRecordBuilder) logger.logRecordBuilder()) + .setException(new Exception("error")) + .emit(); + + assertThat(exporter.getFinishedLogRecordItems()) + .satisfiesExactly( + logRecord -> + assertThat(logRecord) + .hasAttributesSatisfyingExactly( + equalTo(EXCEPTION_TYPE, "type"), + equalTo(EXCEPTION_STACKTRACE, "stacktrace"))); + } +} 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..cf140d1fc96 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 @@ -5,6 +5,10 @@ package io.opentelemetry.sdk.trace; +import static io.opentelemetry.sdk.internal.ExceptionAttributeResolver.EXCEPTION_MESSAGE; +import static io.opentelemetry.sdk.internal.ExceptionAttributeResolver.EXCEPTION_STACKTRACE; +import static io.opentelemetry.sdk.internal.ExceptionAttributeResolver.EXCEPTION_TYPE; + import io.opentelemetry.api.common.AttributeKey; import io.opentelemetry.api.common.Attributes; import io.opentelemetry.api.internal.GuardedBy; @@ -17,6 +21,7 @@ import io.opentelemetry.sdk.common.InstrumentationScopeInfo; import io.opentelemetry.sdk.internal.AttributeUtil; import io.opentelemetry.sdk.internal.AttributesMap; +import io.opentelemetry.sdk.internal.ExceptionAttributeResolver; import io.opentelemetry.sdk.internal.InstrumentationScopeUtil; import io.opentelemetry.sdk.resources.Resource; import io.opentelemetry.sdk.trace.data.EventData; @@ -25,8 +30,6 @@ 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 java.util.ArrayList; import java.util.Collections; import java.util.List; @@ -50,6 +53,8 @@ final class SdkSpan implements ReadWriteSpan { private final SpanContext parentSpanContext; // Handler called when the span starts and ends. private final SpanProcessor spanProcessor; + // Resolves exception.* when an recordException is called + private final ExceptionAttributeResolver exceptionAttributeResolver; // The kind of the span. private final SpanKind kind; // The clock used to get the time. @@ -117,13 +122,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, @@ -132,6 +130,7 @@ private SdkSpan( SpanContext parentSpanContext, SpanLimits spanLimits, SpanProcessor spanProcessor, + ExceptionAttributeResolver exceptionAttributeResolver, AnchoredClock clock, Resource resource, @Nullable AttributesMap attributes, @@ -146,6 +145,7 @@ private SdkSpan( this.name = name; this.kind = kind; this.spanProcessor = spanProcessor; + this.exceptionAttributeResolver = exceptionAttributeResolver; this.resource = resource; this.hasEnded = EndState.NOT_ENDED; this.clock = clock; @@ -178,6 +178,7 @@ static SdkSpan startSpan( Context parentContext, SpanLimits spanLimits, SpanProcessor spanProcessor, + ExceptionAttributeResolver exceptionAttributeResolver, Clock tracerClock, Resource resource, @Nullable AttributesMap attributes, @@ -216,6 +217,7 @@ static SdkSpan startSpan( parentSpan.getSpanContext(), spanLimits, spanProcessor, + exceptionAttributeResolver, clock, resource, attributes, @@ -466,7 +468,6 @@ public ReadWriteSpan recordException(Throwable exception) { } @Override - @SuppressWarnings("unchecked") public ReadWriteSpan recordException(Throwable exception, Attributes additionalAttributes) { if (exception == null) { return this; @@ -478,22 +479,18 @@ public ReadWriteSpan recordException(Throwable exception, Attributes additionalA 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); + String type = exceptionAttributeResolver.getExceptionType(exception); + if (type != null) { + attributes.put(EXCEPTION_TYPE, type); } - if (exceptionMessage != null) { - attributes.put(EXCEPTION_MESSAGE, exceptionMessage); + String message = exceptionAttributeResolver.getExceptionMessage(exception); + if (message != null) { + attributes.put(EXCEPTION_MESSAGE, message); } - if (stackTrace != null) { - attributes.put(EXCEPTION_STACKTRACE, stackTrace); + String stacktrace = exceptionAttributeResolver.getExceptionStacktrace(exception); + if (stacktrace != null) { + attributes.put(EXCEPTION_STACKTRACE, stacktrace); } additionalAttributes.forEach(attributes::put); diff --git a/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/SdkSpanBuilder.java b/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/SdkSpanBuilder.java index b12107c7d43..c0f872265ec 100644 --- a/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/SdkSpanBuilder.java +++ b/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/SdkSpanBuilder.java @@ -226,6 +226,7 @@ public Span startSpan() { parentContext, spanLimits, tracerSharedState.getActiveSpanProcessor(), + tracerSharedState.getExceptionAttributesResolver(), tracerSharedState.getClock(), tracerSharedState.getResource(), recordedAttributes, diff --git a/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/SdkTracerProvider.java b/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/SdkTracerProvider.java index 7bd69a24236..deee9024151 100644 --- a/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/SdkTracerProvider.java +++ b/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/SdkTracerProvider.java @@ -12,6 +12,7 @@ import io.opentelemetry.sdk.common.CompletableResultCode; import io.opentelemetry.sdk.common.InstrumentationScopeInfo; import io.opentelemetry.sdk.internal.ComponentRegistry; +import io.opentelemetry.sdk.internal.ExceptionAttributeResolver; import io.opentelemetry.sdk.internal.ScopeConfigurator; import io.opentelemetry.sdk.resources.Resource; import io.opentelemetry.sdk.trace.internal.SdkTracerProviderUtil; @@ -52,10 +53,17 @@ public static SdkTracerProviderBuilder builder() { Supplier spanLimitsSupplier, Sampler sampler, List spanProcessors, - ScopeConfigurator tracerConfigurator) { + ScopeConfigurator tracerConfigurator, + ExceptionAttributeResolver exceptionAttributeResolver) { this.sharedState = new TracerSharedState( - clock, idsGenerator, resource, spanLimitsSupplier, sampler, spanProcessors); + clock, + idsGenerator, + resource, + spanLimitsSupplier, + sampler, + spanProcessors, + exceptionAttributeResolver); this.tracerSdkComponentRegistry = new ComponentRegistry<>( instrumentationScopeInfo -> 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 531cd1a6384..f83b21fcbbf 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 @@ -7,8 +7,11 @@ import static java.util.Objects.requireNonNull; +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; import io.opentelemetry.sdk.resources.Resource; @@ -34,10 +37,12 @@ public final class SdkTracerProviderBuilder { private Sampler sampler = DEFAULT_SAMPLER; private ScopeConfiguratorBuilder tracerConfiguratorBuilder = TracerConfig.configuratorBuilder(); + private ExceptionAttributeResolver exceptionAttributeResolver = + DefaultExceptionAttributeResolver.getInstance(); /** - * Assign a {@link Clock}. {@link Clock} will be used each time a {@link - * io.opentelemetry.api.trace.Span} is started, ended or any event is recorded. + * Assign a {@link Clock}. {@link Clock} will be used each time a {@link Span} is started, ended + * or any event is recorded. * *

The {@code clock} must be thread-safe and return immediately (no remote calls, as contention * free as possible). @@ -52,8 +57,8 @@ public SdkTracerProviderBuilder setClock(Clock clock) { } /** - * Assign an {@link IdGenerator}. {@link IdGenerator} will be used each time a {@link - * io.opentelemetry.api.trace.Span} is started. + * Assign an {@link IdGenerator}. {@link IdGenerator} will be used each time a {@link Span} is + * started. * *

The {@code idGenerator} must be thread-safe and return immediately (no remote calls, as * contention free as possible). @@ -97,8 +102,7 @@ public SdkTracerProviderBuilder addResource(Resource resource) { *

This method is equivalent to calling {@link #setSpanLimits(Supplier)} like this {@code * #setSpanLimits(() -> spanLimits)}. * - * @param spanLimits the limits that will be used for every {@link - * io.opentelemetry.api.trace.Span}. + * @param spanLimits the limits that will be used for every {@link Span}. * @return this */ public SdkTracerProviderBuilder setSpanLimits(SpanLimits spanLimits) { @@ -109,13 +113,13 @@ public SdkTracerProviderBuilder setSpanLimits(SpanLimits spanLimits) { /** * Assign a {@link Supplier} of {@link SpanLimits}. {@link SpanLimits} will be retrieved each time - * a {@link io.opentelemetry.api.trace.Span} is started. + * a {@link Span} is started. * *

The {@code spanLimitsSupplier} must be thread-safe and return immediately (no remote calls, * as contention free as possible). * * @param spanLimitsSupplier the supplier that will be used to retrieve the {@link SpanLimits} for - * every {@link io.opentelemetry.api.trace.Span}. + * every {@link Span}. * @return this */ public SdkTracerProviderBuilder setSpanLimits(Supplier spanLimitsSupplier) { @@ -126,7 +130,7 @@ public SdkTracerProviderBuilder setSpanLimits(Supplier spanLimitsSup /** * Assign a {@link Sampler} to use for sampling traces. {@link Sampler} will be called each time a - * {@link io.opentelemetry.api.trace.Span} is started. + * {@link Span} is started. * *

The {@code sampler} must be thread-safe and return immediately (no remote calls, as * contention free as possible). @@ -142,7 +146,7 @@ public SdkTracerProviderBuilder setSampler(Sampler sampler) { /** * Add a SpanProcessor to the span pipeline that will be built. {@link SpanProcessor} will be - * called each time a {@link io.opentelemetry.api.trace.Span} is started or ended. + * called each time a {@link Span} is started or ended. * *

The {@code spanProcessor} must be thread-safe and return immediately (no remote calls, as * contention free as possible). @@ -196,6 +200,21 @@ SdkTracerProviderBuilder addTracerConfiguratorCondition( return this; } + /** + * Set the exception attribute resolver, which resolves {@code exception.*} attributes when {@link + * Span#recordException(Throwable)} + * + *

This method is experimental so not public. You may reflectively call it using {@link + * SdkTracerProviderUtil#setExceptionAttributeResolver(SdkTracerProviderBuilder, + * ExceptionAttributeResolver)}. + */ + SdkTracerProviderBuilder setExceptionAttributeResolver( + ExceptionAttributeResolver exceptionAttributeResolver) { + requireNonNull(exceptionAttributeResolver, "exceptionAttributeResolver"); + this.exceptionAttributeResolver = exceptionAttributeResolver; + return this; + } + /** * Create a new {@link SdkTracerProvider} instance with the configuration. * @@ -209,7 +228,8 @@ public SdkTracerProvider build() { spanLimitsSupplier, sampler, spanProcessors, - tracerConfiguratorBuilder.build()); + tracerConfiguratorBuilder.build(), + exceptionAttributeResolver); } SdkTracerProviderBuilder() {} diff --git a/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/TracerSharedState.java b/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/TracerSharedState.java index 3d07a2853f8..74d43076b97 100644 --- a/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/TracerSharedState.java +++ b/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/TracerSharedState.java @@ -7,6 +7,7 @@ import io.opentelemetry.sdk.common.Clock; import io.opentelemetry.sdk.common.CompletableResultCode; +import io.opentelemetry.sdk.internal.ExceptionAttributeResolver; import io.opentelemetry.sdk.resources.Resource; import io.opentelemetry.sdk.trace.samplers.Sampler; import java.util.List; @@ -26,6 +27,7 @@ final class TracerSharedState { private final Supplier spanLimitsSupplier; private final Sampler sampler; private final SpanProcessor activeSpanProcessor; + private final ExceptionAttributeResolver exceptionAttributeResolver; @Nullable private volatile CompletableResultCode shutdownResult = null; @@ -35,14 +37,16 @@ final class TracerSharedState { Resource resource, Supplier spanLimitsSupplier, Sampler sampler, - List spanProcessors) { + List spanProcessors, + ExceptionAttributeResolver exceptionAttributeResolver) { this.clock = clock; this.idGenerator = idGenerator; this.idGeneratorSafeToSkipIdValidation = idGenerator instanceof RandomIdGenerator; this.resource = resource; this.spanLimitsSupplier = spanLimitsSupplier; this.sampler = sampler; - activeSpanProcessor = SpanProcessor.composite(spanProcessors); + this.activeSpanProcessor = SpanProcessor.composite(spanProcessors); + this.exceptionAttributeResolver = exceptionAttributeResolver; } Clock getClock() { @@ -89,6 +93,11 @@ boolean hasBeenShutdown() { return shutdownResult != null; } + /** Return the {@link ExceptionAttributeResolver}. */ + ExceptionAttributeResolver getExceptionAttributesResolver() { + return exceptionAttributeResolver; + } + /** * Stops tracing, including shutting down processors and set to {@code true} {@link * #hasBeenShutdown()}. diff --git a/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/internal/SdkTracerProviderUtil.java b/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/internal/SdkTracerProviderUtil.java index 753a312fa1f..f9ca8fafc56 100644 --- a/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/internal/SdkTracerProviderUtil.java +++ b/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/internal/SdkTracerProviderUtil.java @@ -6,6 +6,7 @@ package io.opentelemetry.sdk.trace.internal; import io.opentelemetry.sdk.common.InstrumentationScopeInfo; +import io.opentelemetry.sdk.internal.ExceptionAttributeResolver; import io.opentelemetry.sdk.internal.ScopeConfigurator; import io.opentelemetry.sdk.trace.SdkTracerProvider; import io.opentelemetry.sdk.trace.SdkTracerProviderBuilder; @@ -72,4 +73,20 @@ public static void addTracerConfiguratorCondition( "Error calling addTracerConfiguratorCondition on SdkTracerProviderBuilder", e); } } + + /** Reflectively set exception attribute resolver to the {@link SdkTracerProviderBuilder}. */ + public static void setExceptionAttributeResolver( + SdkTracerProviderBuilder sdkTracerProviderBuilder, + ExceptionAttributeResolver exceptionAttributeResolver) { + try { + Method method = + SdkTracerProviderBuilder.class.getDeclaredMethod( + "setExceptionAttributeResolver", ExceptionAttributeResolver.class); + method.setAccessible(true); + method.invoke(sdkTracerProviderBuilder, exceptionAttributeResolver); + } catch (NoSuchMethodException | IllegalAccessException | InvocationTargetException e) { + throw new IllegalStateException( + "Error calling setExceptionAttributeResolver on SdkTracerProviderBuilder", e); + } + } } 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 fb387cf2a18..bd7fc11ec06 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 @@ -13,6 +13,8 @@ import static io.opentelemetry.api.common.AttributeKey.longKey; import static io.opentelemetry.api.common.AttributeKey.stringArrayKey; import static io.opentelemetry.api.common.AttributeKey.stringKey; +import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.assertThat; +import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.equalTo; import static java.util.stream.Collectors.joining; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatCode; @@ -37,6 +39,8 @@ 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; import io.opentelemetry.sdk.testing.time.TestClock; @@ -67,6 +71,7 @@ import java.util.stream.IntStream; import java.util.stream.Stream; import javax.annotation.Nullable; +import org.assertj.core.api.Assertions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -441,7 +446,7 @@ void getAttribute() { void getAttributes() { SdkSpan span = createTestSpanWithAttributes(attributes); try { - assertThat(span.getAttributes()) + Assertions.assertThat(span.getAttributes()) .isEqualTo( Attributes.builder() .put("MyBooleanAttributeKey", false) @@ -457,7 +462,7 @@ void getAttributes() { void getAttributes_Empty() { SdkSpan span = createTestSpan(SpanKind.INTERNAL); try { - assertThat(span.getAttributes()).isEqualTo(Attributes.empty()); + Assertions.assertThat(span.getAttributes()).isEqualTo(Attributes.empty()); } finally { span.end(); } @@ -791,48 +796,48 @@ void addEvent() { } List events = span.toSpanData().getEvents(); assertThat(events).hasSize(6); - assertThat(events.get(0)) + Assertions.assertThat(events.get(0)) .satisfies( event -> { assertThat(event.getName()).isEqualTo("event1"); - assertThat(event.getAttributes()).isEqualTo(Attributes.empty()); + Assertions.assertThat(event.getAttributes()).isEqualTo(Attributes.empty()); assertThat(event.getEpochNanos()).isEqualTo(START_EPOCH_NANOS); }); - assertThat(events.get(1)) + Assertions.assertThat(events.get(1)) .satisfies( event -> { assertThat(event.getName()).isEqualTo("event2"); - assertThat(event.getAttributes()) + Assertions.assertThat(event.getAttributes()) .isEqualTo(Attributes.of(stringKey("e1key"), "e1Value")); assertThat(event.getEpochNanos()).isEqualTo(START_EPOCH_NANOS); }); - assertThat(events.get(2)) + Assertions.assertThat(events.get(2)) .satisfies( event -> { assertThat(event.getName()).isEqualTo("event3"); - assertThat(event.getAttributes()).isEqualTo(Attributes.empty()); + Assertions.assertThat(event.getAttributes()).isEqualTo(Attributes.empty()); assertThat(event.getEpochNanos()).isEqualTo(TimeUnit.SECONDS.toNanos(10)); }); - assertThat(events.get(3)) + Assertions.assertThat(events.get(3)) .satisfies( event -> { assertThat(event.getName()).isEqualTo("event4"); - assertThat(event.getAttributes()).isEqualTo(Attributes.empty()); + Assertions.assertThat(event.getAttributes()).isEqualTo(Attributes.empty()); assertThat(event.getEpochNanos()).isEqualTo(TimeUnit.SECONDS.toNanos(20)); }); - assertThat(events.get(4)) + Assertions.assertThat(events.get(4)) .satisfies( event -> { assertThat(event.getName()).isEqualTo("event5"); - assertThat(event.getAttributes()) + Assertions.assertThat(event.getAttributes()) .isEqualTo(Attributes.builder().put("foo", "bar").build()); assertThat(event.getEpochNanos()).isEqualTo(TimeUnit.MILLISECONDS.toNanos(30)); }); - assertThat(events.get(5)) + Assertions.assertThat(events.get(5)) .satisfies( event -> { assertThat(event.getName()).isEqualTo("event6"); - assertThat(event.getAttributes()) + Assertions.assertThat(event.getAttributes()) .isEqualTo(Attributes.builder().put("foo", "bar").build()); assertThat(event.getEpochNanos()).isEqualTo(TimeUnit.MILLISECONDS.toNanos(1000)); }); @@ -942,7 +947,8 @@ void addLink() { .build(), parentSpanId, null, - null); + null, + DefaultExceptionAttributeResolver.getInstance()); try { Span span1 = createTestSpan(SpanKind.INTERNAL); Span span2 = createTestSpan(SpanKind.INTERNAL); @@ -981,11 +987,11 @@ void addLink() { .satisfiesExactly( link -> { assertThat(link.getSpanContext()).isEqualTo(span1.getSpanContext()); - assertThat(link.getAttributes()).isEqualTo(Attributes.empty()); + Assertions.assertThat(link.getAttributes()).isEqualTo(Attributes.empty()); }, link -> { assertThat(link.getSpanContext()).isEqualTo(span2.getSpanContext()); - assertThat(link.getAttributes()) + Assertions.assertThat(link.getAttributes()) .isEqualTo( Attributes.builder() .put("key1", true) @@ -1032,6 +1038,7 @@ void addLink_FaultIn() { Context.root(), SpanLimits.getDefault(), spanProcessor, + DefaultExceptionAttributeResolver.getInstance(), testClock, resource, null, @@ -1137,7 +1144,7 @@ void droppingEvents() { EventData expectedEvent = EventData.create( START_EPOCH_NANOS + i * NANOS_PER_SECOND, "event2", Attributes.empty(), 0); - assertThat(spanData.getEvents().get(i)).isEqualTo(expectedEvent); + Assertions.assertThat(spanData.getEvents().get(i)).isEqualTo(expectedEvent); assertThat(spanData.getTotalRecordedEvents()).isEqualTo(2 * maxNumberOfEvents); } } finally { @@ -1149,7 +1156,7 @@ void droppingEvents() { EventData expectedEvent = EventData.create( START_EPOCH_NANOS + i * NANOS_PER_SECOND, "event2", Attributes.empty(), 0); - assertThat(spanData.getEvents().get(i)).isEqualTo(expectedEvent); + Assertions.assertThat(spanData.getEvents().get(i)).isEqualTo(expectedEvent); } } @@ -1181,7 +1188,7 @@ void recordException() { .isEqualTo(exception.getClass().getName()); assertThat(event.getAttributes().get(stringKey("exception.stacktrace"))).isEqualTo(stacktrace); assertThat(event.getAttributes().size()).isEqualTo(3); - assertThat(event) + Assertions.assertThat(event) .isInstanceOfSatisfying( ExceptionEventData.class, exceptionEvent -> { @@ -1269,7 +1276,7 @@ void recordException_additionalAttributes() { assertThat(event.getAttributes().get(stringKey("exception.stacktrace"))).isEqualTo(stacktrace); assertThat(event.getAttributes().size()).isEqualTo(4); - assertThat(event) + Assertions.assertThat(event) .isInstanceOfSatisfying( ExceptionEventData.class, exceptionEvent -> { @@ -1292,6 +1299,47 @@ void recordException_SpanLimits() { assertThat(event.getTotalAttributeCount() - event.getAttributes().size()).isPositive(); } + @Test + void recordException_CustomResolver() { + ExceptionAttributeResolver exceptionAttributeResolver = + new ExceptionAttributeResolver() { + @Override + public String getExceptionType(Throwable throwable) { + return "type"; + } + + @Nullable + @Override + public String getExceptionMessage(Throwable throwable) { + return null; + } + + @Override + public String getExceptionStacktrace(Throwable throwable) { + return "stacktrace"; + } + }; + + SdkSpan span = + createTestSpan( + SpanKind.INTERNAL, + SpanLimits.getDefault(), + parentSpanId, + null, + Collections.singletonList(link), + exceptionAttributeResolver); + + span.recordException(new IllegalStateException("error")); + + List events = span.toSpanData().getEvents(); + assertThat(events.size()).isEqualTo(1); + EventData event = events.get(0); + assertThat(event) + .hasAttributesSatisfyingExactly( + equalTo(ExceptionAttributeResolver.EXCEPTION_TYPE, "type"), + equalTo(ExceptionAttributeResolver.EXCEPTION_STACKTRACE, "stacktrace")); + } + @Test void badArgsIgnored() { SdkSpan span = createTestRootSpan(); @@ -1343,6 +1391,7 @@ void onStartOnEndNotRequired() { Context.root(), spanLimits, spanProcessor, + DefaultExceptionAttributeResolver.getInstance(), testClock, resource, AttributesMap.create( @@ -1424,7 +1473,8 @@ private SdkSpan createTestSpanWithAttributes(Map attribute SpanLimits.getDefault(), null, attributesMap, - Collections.singletonList(link)); + Collections.singletonList(link), + DefaultExceptionAttributeResolver.getInstance()); } private SdkSpan createTestRootSpan() { @@ -1433,17 +1483,28 @@ private SdkSpan createTestRootSpan() { SpanLimits.getDefault(), SpanId.getInvalid(), null, - Collections.singletonList(link)); + Collections.singletonList(link), + DefaultExceptionAttributeResolver.getInstance()); } private SdkSpan createTestSpan(SpanKind kind) { return createTestSpan( - kind, SpanLimits.getDefault(), parentSpanId, null, Collections.singletonList(link)); + kind, + SpanLimits.getDefault(), + parentSpanId, + null, + Collections.singletonList(link), + DefaultExceptionAttributeResolver.getInstance()); } private SdkSpan createTestSpan(SpanLimits config) { return createTestSpan( - SpanKind.INTERNAL, config, parentSpanId, null, Collections.singletonList(link)); + SpanKind.INTERNAL, + config, + parentSpanId, + null, + Collections.singletonList(link), + DefaultExceptionAttributeResolver.getInstance()); } private SdkSpan createTestSpan( @@ -1451,7 +1512,8 @@ private SdkSpan createTestSpan( SpanLimits config, @Nullable String parentSpanId, @Nullable AttributesMap attributes, - @Nullable List links) { + @Nullable List links, + ExceptionAttributeResolver exceptionAttributeResolver) { List linksCopy = links == null ? new ArrayList<>() : new ArrayList<>(links); SdkSpan span = @@ -1468,6 +1530,7 @@ private SdkSpan createTestSpan( Context.root(), config, spanProcessor, + exceptionAttributeResolver, testClock, resource, attributes, @@ -1534,9 +1597,7 @@ void testAsSpanData() { Resource resource = this.resource; Attributes attributes = TestUtils.generateRandomAttributes(); AttributesMap attributesWithCapacity = AttributesMap.create(32, Integer.MAX_VALUE); - attributes.forEach( - (attributeKey, object) -> - attributesWithCapacity.put((AttributeKey) attributeKey, object)); + attributes.forEach(attributesWithCapacity::put); Attributes event1Attributes = TestUtils.generateRandomAttributes(); Attributes event2Attributes = TestUtils.generateRandomAttributes(); SpanContext context = @@ -1557,6 +1618,7 @@ void testAsSpanData() { Context.root(), spanLimits, spanProcessor, + DefaultExceptionAttributeResolver.getInstance(), 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 61b64b4ebdf..191b6ad6fec 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 @@ -9,6 +9,8 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import io.opentelemetry.api.common.Attributes; @@ -18,6 +20,8 @@ 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; import io.opentelemetry.sdk.trace.internal.SdkTracerProviderUtil; @@ -38,11 +42,11 @@ @MockitoSettings(strictness = Strictness.LENIENT) class SdkTracerProviderTest { @Mock private SpanProcessor spanProcessor; - private SdkTracerProvider tracerFactory; + private SdkTracerProvider tracerProvider; @BeforeEach void setUp() { - tracerFactory = SdkTracerProvider.builder().addSpanProcessor(spanProcessor).build(); + tracerProvider = SdkTracerProvider.builder().addSpanProcessor(spanProcessor).build(); when(spanProcessor.forceFlush()).thenReturn(CompletableResultCode.ofSuccess()); when(spanProcessor.shutdown()).thenReturn(CompletableResultCode.ofSuccess()); } @@ -137,34 +141,35 @@ void builder_NullIdsGenerator() { @Test void defaultGet() { - assertThat(tracerFactory.get("test")).isInstanceOf(SdkTracer.class); + assertThat(tracerProvider.get("test")).isInstanceOf(SdkTracer.class); } @Test void getSameInstanceForSameName_WithoutVersion() { - assertThat(tracerFactory.get("test")).isSameAs(tracerFactory.get("test")); - assertThat(tracerFactory.get("test")) - .isSameAs(tracerFactory.get("test", null)) - .isSameAs(tracerFactory.tracerBuilder("test").build()); + assertThat(tracerProvider.get("test")).isSameAs(tracerProvider.get("test")); + assertThat(tracerProvider.get("test")) + .isSameAs(tracerProvider.get("test", null)) + .isSameAs(tracerProvider.tracerBuilder("test").build()); } @Test void getSameInstanceForSameName_WithVersion() { - assertThat(tracerFactory.get("test", "version")) - .isSameAs(tracerFactory.get("test", "version")) - .isSameAs(tracerFactory.tracerBuilder("test").setInstrumentationVersion("version").build()); + assertThat(tracerProvider.get("test", "version")) + .isSameAs(tracerProvider.get("test", "version")) + .isSameAs( + tracerProvider.tracerBuilder("test").setInstrumentationVersion("version").build()); } @Test void getSameInstanceForSameName_WithVersionAndSchema() { assertThat( - tracerFactory + tracerProvider .tracerBuilder("test") .setInstrumentationVersion("version") .setSchemaUrl("http://url") .build()) .isSameAs( - tracerFactory + tracerProvider .tracerBuilder("test") .setInstrumentationVersion("version") .setSchemaUrl("http://url") @@ -179,7 +184,7 @@ void propagatesInstrumentationScopeInfoToTracer() { .setSchemaUrl("http://url") .build(); Tracer tracer = - tracerFactory + tracerProvider .tracerBuilder(expected.getName()) .setInstrumentationVersion(expected.getVersion()) .setSchemaUrl(expected.getSchemaUrl()) @@ -198,7 +203,7 @@ void propagatesEnablementToTracerByUtil() { } void propagatesEnablementToTracer(boolean directly) { - SdkTracer tracer = (SdkTracer) tracerFactory.get("test"); + SdkTracer tracer = (SdkTracer) tracerProvider.get("test"); boolean isEnabled = tracer.isEnabled(); ScopeConfigurator flipConfigurator = new ScopeConfigurator() { @@ -209,9 +214,9 @@ public TracerConfig apply(InstrumentationScopeInfo scopeInfo) { }; // all in the same thread, so should see enablement change immediately if (directly) { - tracerFactory.setTracerConfigurator(flipConfigurator); + tracerProvider.setTracerConfigurator(flipConfigurator); } else { - SdkTracerProviderUtil.setTracerConfigurator(tracerFactory, flipConfigurator); + SdkTracerProviderUtil.setTracerConfigurator(tracerProvider, flipConfigurator); } assertThat(tracer.isEnabled()).isEqualTo(!isEnabled); } @@ -227,58 +232,73 @@ void build_SpanLimits() { @Test void shutdown() { - tracerFactory.shutdown(); + tracerProvider.shutdown(); Mockito.verify(spanProcessor, Mockito.times(1)).shutdown(); } @Test void close() { - tracerFactory.close(); + tracerProvider.close(); Mockito.verify(spanProcessor, Mockito.times(1)).shutdown(); } @Test void forceFlush() { - tracerFactory.forceFlush(); + tracerProvider.forceFlush(); Mockito.verify(spanProcessor, Mockito.times(1)).forceFlush(); } @Test @SuppressLogger(SdkTracerProvider.class) void shutdownTwice_OnlyFlushSpanProcessorOnce() { - tracerFactory.shutdown(); + tracerProvider.shutdown(); Mockito.verify(spanProcessor, Mockito.times(1)).shutdown(); - tracerFactory.shutdown(); // the second call will be ignored + tracerProvider.shutdown(); // the second call will be ignored Mockito.verify(spanProcessor, Mockito.times(1)).shutdown(); } @Test void returnNoopSpanAfterShutdown() { - tracerFactory.shutdown(); - Span span = tracerFactory.get("noop").spanBuilder("span").startSpan(); + tracerProvider.shutdown(); + Span span = tracerProvider.get("noop").spanBuilder("span").startSpan(); assertThat(span.getSpanContext().isValid()).isFalse(); span.end(); } @Test void suppliesDefaultTracerForNullName() { - SdkTracer tracer = (SdkTracer) tracerFactory.get(null); + SdkTracer tracer = (SdkTracer) tracerProvider.get(null); assertThat(tracer.getInstrumentationScopeInfo().getName()) .isEqualTo(SdkTracerProvider.DEFAULT_TRACER_NAME); - tracer = (SdkTracer) tracerFactory.get(null, null); + tracer = (SdkTracer) tracerProvider.get(null, null); assertThat(tracer.getInstrumentationScopeInfo().getName()) .isEqualTo(SdkTracerProvider.DEFAULT_TRACER_NAME); } @Test void suppliesDefaultTracerForEmptyName() { - SdkTracer tracer = (SdkTracer) tracerFactory.get(""); + SdkTracer tracer = (SdkTracer) tracerProvider.get(""); assertThat(tracer.getInstrumentationScopeInfo().getName()) .isEqualTo(SdkTracerProvider.DEFAULT_TRACER_NAME); - tracer = (SdkTracer) tracerFactory.get("", ""); + tracer = (SdkTracer) tracerProvider.get("", ""); assertThat(tracer.getInstrumentationScopeInfo().getName()) .isEqualTo(SdkTracerProvider.DEFAULT_TRACER_NAME); } + + @Test + void exceptionAttributeResolver() { + SdkTracerProviderBuilder builder = SdkTracerProvider.builder().addSpanProcessor(spanProcessor); + ExceptionAttributeResolver exceptionAttributeResolver = + spy(DefaultExceptionAttributeResolver.getInstance()); + SdkTracerProviderUtil.setExceptionAttributeResolver(builder, exceptionAttributeResolver); + + Exception exception = new Exception("error"); + builder.build().get("tracer").spanBuilder("span").startSpan().recordException(exception).end(); + + verify(exceptionAttributeResolver).getExceptionType(exception); + verify(exceptionAttributeResolver).getExceptionMessage(exception); + verify(exceptionAttributeResolver).getExceptionStacktrace(exception); + } } From 1b0a28e8403fa60b827b234b57c66f3e2d789fde Mon Sep 17 00:00:00 2001 From: Jack Berg Date: Thu, 6 Mar 2025 13:32:39 -0600 Subject: [PATCH 2/8] Use System.lineSeparator() instead of \n to fix build --- .../io/opentelemetry/sdk/logs/ExtendedLoggerBuilderTest.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/sdk/logs/src/testIncubating/java/io/opentelemetry/sdk/logs/ExtendedLoggerBuilderTest.java b/sdk/logs/src/testIncubating/java/io/opentelemetry/sdk/logs/ExtendedLoggerBuilderTest.java index 92c36c27f06..698da1fc8df 100644 --- a/sdk/logs/src/testIncubating/java/io/opentelemetry/sdk/logs/ExtendedLoggerBuilderTest.java +++ b/sdk/logs/src/testIncubating/java/io/opentelemetry/sdk/logs/ExtendedLoggerBuilderTest.java @@ -44,7 +44,9 @@ void setException_DefaultResolver() { equalTo(EXCEPTION_MESSAGE, "error"), satisfies( EXCEPTION_STACKTRACE, - stacktrace -> stacktrace.startsWith("java.lang.Exception: error\n")))); + stacktrace -> + stacktrace.startsWith( + "java.lang.Exception: error" + System.lineSeparator())))); } @Test From daa8e384d0b9f880877d146b5c692dd8008c5b7e Mon Sep 17 00:00:00 2001 From: Jack Berg Date: Thu, 10 Apr 2025 15:40:26 -0500 Subject: [PATCH 3/8] Remove ExtendedLogRecordBuilder#setException --- .../incubator/logs/ExtendedDefaultLogger.java | 5 -- .../logs/ExtendedLogRecordBuilder.java | 3 - .../sdk/logs/ExtendedSdkLogRecordBuilder.java | 6 -- .../sdk/logs/SdkLogRecordBuilder.java | 27 ------ .../sdk/logs/ExtendedLoggerBuilderTest.java | 88 ------------------- 5 files changed, 129 deletions(-) delete mode 100644 sdk/logs/src/testIncubating/java/io/opentelemetry/sdk/logs/ExtendedLoggerBuilderTest.java diff --git a/api/incubator/src/main/java/io/opentelemetry/api/incubator/logs/ExtendedDefaultLogger.java b/api/incubator/src/main/java/io/opentelemetry/api/incubator/logs/ExtendedDefaultLogger.java index d9a95a6d2d4..378af1c6b00 100644 --- a/api/incubator/src/main/java/io/opentelemetry/api/incubator/logs/ExtendedDefaultLogger.java +++ b/api/incubator/src/main/java/io/opentelemetry/api/incubator/logs/ExtendedDefaultLogger.java @@ -40,11 +40,6 @@ public ExtendedLogRecordBuilder setEventName(String eventName) { return this; } - @Override - public ExtendedLogRecordBuilder setException(Throwable throwable) { - return this; - } - @Override public LogRecordBuilder setTimestamp(long timestamp, TimeUnit unit) { return this; diff --git a/api/incubator/src/main/java/io/opentelemetry/api/incubator/logs/ExtendedLogRecordBuilder.java b/api/incubator/src/main/java/io/opentelemetry/api/incubator/logs/ExtendedLogRecordBuilder.java index d6fde3699b1..4a18baa07ed 100644 --- a/api/incubator/src/main/java/io/opentelemetry/api/incubator/logs/ExtendedLogRecordBuilder.java +++ b/api/incubator/src/main/java/io/opentelemetry/api/incubator/logs/ExtendedLogRecordBuilder.java @@ -19,7 +19,4 @@ public interface ExtendedLogRecordBuilder extends LogRecordBuilder { * record with a non-empty event name is an Event. */ ExtendedLogRecordBuilder setEventName(String eventName); - - /** Set standard {@code exception.*} attributes based on the {@code throwable}. */ - ExtendedLogRecordBuilder setException(Throwable throwable); } 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 97f337cc194..9f874d1f4b3 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 @@ -29,12 +29,6 @@ public ExtendedSdkLogRecordBuilder setEventName(String eventName) { return this; } - @Override - public ExtendedSdkLogRecordBuilder setException(Throwable throwable) { - super.setException(throwable); - return this; - } - @Override public ExtendedSdkLogRecordBuilder setTimestamp(long timestamp, TimeUnit unit) { super.setTimestamp(timestamp, unit); diff --git a/sdk/logs/src/main/java/io/opentelemetry/sdk/logs/SdkLogRecordBuilder.java b/sdk/logs/src/main/java/io/opentelemetry/sdk/logs/SdkLogRecordBuilder.java index 48af664b4f2..da7e1f45e7e 100644 --- a/sdk/logs/src/main/java/io/opentelemetry/sdk/logs/SdkLogRecordBuilder.java +++ b/sdk/logs/src/main/java/io/opentelemetry/sdk/logs/SdkLogRecordBuilder.java @@ -5,10 +5,6 @@ package io.opentelemetry.sdk.logs; -import static io.opentelemetry.sdk.internal.ExceptionAttributeResolver.EXCEPTION_MESSAGE; -import static io.opentelemetry.sdk.internal.ExceptionAttributeResolver.EXCEPTION_STACKTRACE; -import static io.opentelemetry.sdk.internal.ExceptionAttributeResolver.EXCEPTION_TYPE; - import io.opentelemetry.api.common.AttributeKey; import io.opentelemetry.api.common.Value; import io.opentelemetry.api.logs.LogRecordBuilder; @@ -17,7 +13,6 @@ import io.opentelemetry.context.Context; import io.opentelemetry.sdk.common.InstrumentationScopeInfo; import io.opentelemetry.sdk.internal.AttributesMap; -import io.opentelemetry.sdk.internal.ExceptionAttributeResolver; import java.time.Instant; import java.util.concurrent.TimeUnit; import javax.annotation.Nullable; @@ -51,28 +46,6 @@ SdkLogRecordBuilder setEventName(String eventName) { return this; } - // accessible via ExtendedSdkLogRecordBuilder - SdkLogRecordBuilder setException(Throwable throwable) { - if (throwable == null) { - return this; - } - ExceptionAttributeResolver exceptionAttributeResolver = - loggerSharedState.getExceptionAttributeResolver(); - String type = exceptionAttributeResolver.getExceptionType(throwable); - if (type != null) { - setAttribute(EXCEPTION_TYPE, type); - } - String message = exceptionAttributeResolver.getExceptionMessage(throwable); - if (message != null) { - setAttribute(EXCEPTION_MESSAGE, message); - } - String stacktrace = exceptionAttributeResolver.getExceptionStacktrace(throwable); - if (stacktrace != null) { - setAttribute(EXCEPTION_STACKTRACE, stacktrace); - } - return this; - } - @Override public SdkLogRecordBuilder setTimestamp(long timestamp, TimeUnit unit) { this.timestampEpochNanos = unit.toNanos(timestamp); diff --git a/sdk/logs/src/testIncubating/java/io/opentelemetry/sdk/logs/ExtendedLoggerBuilderTest.java b/sdk/logs/src/testIncubating/java/io/opentelemetry/sdk/logs/ExtendedLoggerBuilderTest.java deleted file mode 100644 index 698da1fc8df..00000000000 --- a/sdk/logs/src/testIncubating/java/io/opentelemetry/sdk/logs/ExtendedLoggerBuilderTest.java +++ /dev/null @@ -1,88 +0,0 @@ -/* - * Copyright The OpenTelemetry Authors - * SPDX-License-Identifier: Apache-2.0 - */ - -package io.opentelemetry.sdk.logs; - -import static io.opentelemetry.sdk.internal.ExceptionAttributeResolver.EXCEPTION_MESSAGE; -import static io.opentelemetry.sdk.internal.ExceptionAttributeResolver.EXCEPTION_STACKTRACE; -import static io.opentelemetry.sdk.internal.ExceptionAttributeResolver.EXCEPTION_TYPE; -import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.assertThat; -import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.equalTo; -import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.satisfies; - -import io.opentelemetry.api.incubator.logs.ExtendedLogRecordBuilder; -import io.opentelemetry.api.logs.Logger; -import io.opentelemetry.sdk.internal.ExceptionAttributeResolver; -import io.opentelemetry.sdk.logs.export.SimpleLogRecordProcessor; -import io.opentelemetry.sdk.logs.internal.SdkLoggerProviderUtil; -import io.opentelemetry.sdk.testing.exporter.InMemoryLogRecordExporter; -import javax.annotation.Nullable; -import org.junit.jupiter.api.Test; - -class ExtendedLoggerBuilderTest { - - private final InMemoryLogRecordExporter exporter = InMemoryLogRecordExporter.create(); - private final SdkLoggerProviderBuilder loggerProviderBuilder = - SdkLoggerProvider.builder().addLogRecordProcessor(SimpleLogRecordProcessor.create(exporter)); - - @Test - void setException_DefaultResolver() { - Logger logger = loggerProviderBuilder.build().get("logger"); - - ((ExtendedLogRecordBuilder) logger.logRecordBuilder()) - .setException(new Exception("error")) - .emit(); - - assertThat(exporter.getFinishedLogRecordItems()) - .satisfiesExactly( - logRecord -> - assertThat(logRecord) - .hasAttributesSatisfyingExactly( - equalTo(EXCEPTION_TYPE, "java.lang.Exception"), - equalTo(EXCEPTION_MESSAGE, "error"), - satisfies( - EXCEPTION_STACKTRACE, - stacktrace -> - stacktrace.startsWith( - "java.lang.Exception: error" + System.lineSeparator())))); - } - - @Test - void setException_CustomResolver() { - SdkLoggerProviderUtil.setExceptionAttributeResolver( - loggerProviderBuilder, - new ExceptionAttributeResolver() { - @Override - public String getExceptionType(Throwable throwable) { - return "type"; - } - - @Nullable - @Override - public String getExceptionMessage(Throwable throwable) { - return null; - } - - @Override - public String getExceptionStacktrace(Throwable throwable) { - return "stacktrace"; - } - }); - - Logger logger = loggerProviderBuilder.build().get("logger"); - - ((ExtendedLogRecordBuilder) logger.logRecordBuilder()) - .setException(new Exception("error")) - .emit(); - - assertThat(exporter.getFinishedLogRecordItems()) - .satisfiesExactly( - logRecord -> - assertThat(logRecord) - .hasAttributesSatisfyingExactly( - equalTo(EXCEPTION_TYPE, "type"), - equalTo(EXCEPTION_STACKTRACE, "stacktrace"))); - } -} From 64ea76d72f742273d77a5a4205083b0586ed8f96 Mon Sep 17 00:00:00 2001 From: Jack Berg Date: Thu, 10 Apr 2025 15:41:08 -0500 Subject: [PATCH 4/8] Restore werror --- buildSrc/src/main/kotlin/otel.java-conventions.gradle.kts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/buildSrc/src/main/kotlin/otel.java-conventions.gradle.kts b/buildSrc/src/main/kotlin/otel.java-conventions.gradle.kts index b5eb4b8ab03..d1957a45ebd 100644 --- a/buildSrc/src/main/kotlin/otel.java-conventions.gradle.kts +++ b/buildSrc/src/main/kotlin/otel.java-conventions.gradle.kts @@ -87,7 +87,9 @@ tasks { // https://groups.google.com/forum/#!topic/bazel-discuss/_R3A9TJSoPM "-Xlint:-processing", // We suppress the "options" warning because it prevents compilation on modern JDKs - "-Xlint:-options" + "-Xlint:-options", + // Fail build on any warning + "-Werror", ), ) } From 8c5e2f67fd359ba643a2bd5dde50bc79f285688f Mon Sep 17 00:00:00 2001 From: Jack Berg Date: Fri, 30 May 2025 16:42:19 -0500 Subject: [PATCH 5/8] Consolidate to single ExceptionAttributeResolver method --- .../sdk/internal/AttributeUtil.java | 29 ------------- .../sdk/internal/AttributesMap.java | 14 +++++-- .../DefaultExceptionAttributeResolver.java | 26 +++++------- .../internal/ExceptionAttributeResolver.java | 42 +++++-------------- .../sdk/internal/ExtendedAttributesMap.java | 17 ++++++-- .../sdk/internal/AttributesMapTest.java | 4 +- .../sdk/logs/ExtendedSdkLogRecordBuilder.java | 14 +++---- .../logs/ExtendedSdkReadWriteLogRecord.java | 2 +- .../sdk/logs/SdkLogRecordBuilder.java | 2 +- .../sdk/logs/SdkReadWriteLogRecord.java | 2 +- .../sdk/logs/ReadWriteLogRecordTest.java | 4 +- .../sdk/logs/ExtendedLoggerBuilderTest.java | 19 +++------ .../io/opentelemetry/sdk/trace/SdkSpan.java | 7 ++-- .../sdk/trace/SdkSpanBuilder.java | 5 ++- .../opentelemetry/sdk/trace/SdkSpanTest.java | 20 +++------ .../sdk/trace/SdkTracerProviderTest.java | 6 +-- 16 files changed, 79 insertions(+), 134 deletions(-) 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 b77a39f08b0..de7fac88eba 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 @@ -5,17 +5,12 @@ package io.opentelemetry.sdk.internal; -import static io.opentelemetry.sdk.internal.ExceptionAttributeResolver.EXCEPTION_MESSAGE; -import static io.opentelemetry.sdk.internal.ExceptionAttributeResolver.EXCEPTION_STACKTRACE; -import static io.opentelemetry.sdk.internal.ExceptionAttributeResolver.EXCEPTION_TYPE; - import io.opentelemetry.api.common.AttributeKey; import io.opentelemetry.api.common.Attributes; import io.opentelemetry.api.common.AttributesBuilder; import java.util.ArrayList; import java.util.List; import java.util.Map; -import java.util.function.BiConsumer; import java.util.function.Predicate; /** @@ -100,28 +95,4 @@ public static Object applyAttributeLengthLimit(Object value, int lengthLimit) { } return value; } - - public static void addExceptionAttributes( - ExceptionAttributeResolver exceptionAttributeResolver, - int maxAttributeLength, - Throwable exception, - BiConsumer, String> attributeConsumer) { - String exceptionType = - exceptionAttributeResolver.getExceptionType(exception, maxAttributeLength); - if (exceptionType != null) { - attributeConsumer.accept(EXCEPTION_TYPE, exceptionType); - } - - String exceptionMessage = - exceptionAttributeResolver.getExceptionMessage(exception, maxAttributeLength); - if (exceptionMessage != null) { - attributeConsumer.accept(EXCEPTION_MESSAGE, exceptionMessage); - } - - String stackTrace = - exceptionAttributeResolver.getExceptionStacktrace(exception, maxAttributeLength); - if (stackTrace != null) { - attributeConsumer.accept(EXCEPTION_STACKTRACE, stackTrace); - } - } } diff --git a/sdk/common/src/main/java/io/opentelemetry/sdk/internal/AttributesMap.java b/sdk/common/src/main/java/io/opentelemetry/sdk/internal/AttributesMap.java index 7a09093a315..2e481723c9c 100644 --- a/sdk/common/src/main/java/io/opentelemetry/sdk/internal/AttributesMap.java +++ b/sdk/common/src/main/java/io/opentelemetry/sdk/internal/AttributesMap.java @@ -21,8 +21,8 @@ *

WARNING: In order to reduce memory allocation, this class extends {@link HashMap} when it * would be more appropriate to delegate. The problem with extending is that we don't enforce that * all {@link HashMap} methods for reading / writing data conform to the configured attribute - * limits. Therefore, it's easy to accidentally call something like {@link Map#putAll(Map)} or - * {@link Map#put(Object, Object)} and bypass the restrictions (see #7135). Callers MUST * take care to only call methods from {@link AttributesMap}, and not {@link HashMap}. * @@ -58,7 +58,10 @@ public static AttributesMap create(long capacity, int lengthLimit) { */ @Override @Nullable - public Object put(AttributeKey key, Object value) { + public Object put(AttributeKey key, @Nullable Object value) { + if (value == null) { + return null; + } totalAddedValues++; if (size() >= capacity && !containsKey(key)) { return null; @@ -66,6 +69,11 @@ public Object put(AttributeKey key, Object value) { return super.put(key, AttributeUtil.applyAttributeLengthLimit(value, lengthLimit)); } + /** Generic overload of {@link #put(AttributeKey, Object)}. */ + public void putIfCapacity(AttributeKey key, @Nullable T value) { + put(key, value); + } + /** Get the total number of attributes added, including those dropped for capacity limits. */ public int getTotalAddedValues() { return totalAddedValues; 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 6001e986a0d..2c78c4f1d86 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 @@ -7,7 +7,6 @@ import java.io.PrintWriter; import java.io.StringWriter; -import javax.annotation.Nullable; /** * This class is internal and experimental. Its APIs are unstable and can change at any time. Its @@ -26,24 +25,19 @@ public static ExceptionAttributeResolver getInstance() { } @Override - @Nullable - public String getExceptionType(Throwable throwable, int maxAttributeLength) { - return throwable.getClass().getCanonicalName(); - } - - @Override - @Nullable - public String getExceptionMessage(Throwable throwable, int maxAttributeLength) { - return throwable.getMessage(); - } - - @Override - @Nullable - public String getExceptionStacktrace(Throwable throwable, int maxAttributeLength) { + public void setExceptionAttributes( + AttributeSetter attributeSetter, Throwable throwable, int maxAttributeLength) { + attributeSetter.setAttribute( + ExceptionAttributeResolver.EXCEPTION_TYPE, throwable.getClass().getCanonicalName()); + String message = throwable.getMessage(); + if (message != null) { + attributeSetter.setAttribute(ExceptionAttributeResolver.EXCEPTION_MESSAGE, message); + } StringWriter stringWriter = new StringWriter(); try (PrintWriter printWriter = new PrintWriter(stringWriter)) { throwable.printStackTrace(printWriter); } - return stringWriter.toString(); + attributeSetter.setAttribute( + ExceptionAttributeResolver.EXCEPTION_STACKTRACE, stringWriter.toString()); } } 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 4293a2506a1..4b9d7a8dc9d 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 @@ -21,39 +21,17 @@ public interface ExceptionAttributeResolver { AttributeKey EXCEPTION_MESSAGE = AttributeKey.stringKey("exception.message"); AttributeKey EXCEPTION_STACKTRACE = AttributeKey.stringKey("exception.stacktrace"); - /** - * Resolve the {@link #EXCEPTION_TYPE} attribute from the {@code throwable}, or {@code null} if no - * value should be set. - * - * @param throwable the throwable - * @param maxAttributeLength the max attribute length that will be retained by the SDK. Responses - * are not required to conform to this limit, but implementations may incorporate this limit - * to avoid unnecessary compute. - */ - @Nullable - String getExceptionType(Throwable throwable, int maxAttributeLength); - - /** - * Resolve the {@link #EXCEPTION_MESSAGE} attribute from the {@code throwable}, or {@code null} if - * no value should be set. - * - * @param throwable the throwable - * @param maxAttributeLength the max attribute length that will be retained by the SDK. Responses - * are not required to conform to this limit, but implementations may incorporate this limit - * to avoid unnecessary compute. - */ - @Nullable - String getExceptionMessage(Throwable throwable, int maxAttributeLength); + void setExceptionAttributes( + AttributeSetter attributeSetter, Throwable throwable, int maxAttributeLength); /** - * Resolve the {@link #EXCEPTION_STACKTRACE} attribute from the {@code throwable}, or {@code null} - * if no value should be set. - * - * @param throwable the throwable - * @param maxAttributeLength the max attribute length that will be retained by the SDK. Responses - * are not required to conform to this limit, but implementations may incorporate this limit - * to avoid unnecessary compute. + * 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. */ - @Nullable - String getExceptionStacktrace(Throwable throwable, int maxAttributeLength); + // TODO(jack-berg): Consider promoting to opentelemetry and extending with Span, LogRecordBuilder, + // AttributeBuilder, AttributesMap etc. + interface AttributeSetter { + void setAttribute(AttributeKey key, @Nullable T value); + } } diff --git a/sdk/common/src/main/java/io/opentelemetry/sdk/internal/ExtendedAttributesMap.java b/sdk/common/src/main/java/io/opentelemetry/sdk/internal/ExtendedAttributesMap.java index 440e472666e..52959b1b617 100644 --- a/sdk/common/src/main/java/io/opentelemetry/sdk/internal/ExtendedAttributesMap.java +++ b/sdk/common/src/main/java/io/opentelemetry/sdk/internal/ExtendedAttributesMap.java @@ -49,14 +49,23 @@ public static ExtendedAttributesMap create(long capacity, int lengthLimit) { } /** Add the attribute key value pair, applying capacity and length limits. */ - public void put(ExtendedAttributeKey key, T value) { + @Override + @Nullable + public Object put(ExtendedAttributeKey key, @Nullable Object value) { + if (value == null) { + return null; + } totalAddedValues++; - // TODO(jack-berg): apply capcity to nested entries + // TODO(jack-berg): apply capacity to nested entries if (size() >= capacity && !containsKey(key)) { - return; + return null; } // TODO(jack-berg): apply limits to nested entries - super.put(key, AttributeUtil.applyAttributeLengthLimit(value, lengthLimit)); + return super.put(key, AttributeUtil.applyAttributeLengthLimit(value, lengthLimit)); + } + + public void putIfCapacity(ExtendedAttributeKey key, @Nullable T value) { + put(key, value); } /** diff --git a/sdk/common/src/test/java/io/opentelemetry/sdk/internal/AttributesMapTest.java b/sdk/common/src/test/java/io/opentelemetry/sdk/internal/AttributesMapTest.java index 7dfdf6c808c..2ea6c328a62 100644 --- a/sdk/common/src/test/java/io/opentelemetry/sdk/internal/AttributesMapTest.java +++ b/sdk/common/src/test/java/io/opentelemetry/sdk/internal/AttributesMapTest.java @@ -16,8 +16,8 @@ class AttributesMapTest { @Test void asMap() { AttributesMap attributesMap = AttributesMap.create(2, Integer.MAX_VALUE); - attributesMap.put(longKey("one"), 1L); - attributesMap.put(longKey("two"), 2L); + attributesMap.putIfCapacity(longKey("one"), 1L); + attributesMap.putIfCapacity(longKey("two"), 2L); assertThat(attributesMap.asMap()) .containsOnly(entry(longKey("one"), 1L), entry(longKey("two"), 2L)); 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 e6abb0ffa43..a245f4a1203 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 @@ -13,7 +13,6 @@ import io.opentelemetry.api.trace.Span; import io.opentelemetry.context.Context; import io.opentelemetry.sdk.common.InstrumentationScopeInfo; -import io.opentelemetry.sdk.internal.AttributeUtil; import io.opentelemetry.sdk.internal.ExtendedAttributesMap; import java.time.Instant; import java.util.concurrent.TimeUnit; @@ -42,11 +41,12 @@ public ExtendedSdkLogRecordBuilder setException(Throwable throwable) { return this; } - AttributeUtil.addExceptionAttributes( - loggerSharedState.getExceptionAttributeResolver(), - loggerSharedState.getLogLimits().getMaxAttributeValueLength(), - throwable, - this::setAttribute); + loggerSharedState + .getExceptionAttributeResolver() + .setExceptionAttributes( + this::setAttribute, + throwable, + loggerSharedState.getLogLimits().getMaxAttributeValueLength()); return this; } @@ -115,7 +115,7 @@ public ExtendedSdkLogRecordBuilder setAttribute(ExtendedAttributeKey key, ExtendedAttributesMap.create( logLimits.getMaxNumberOfAttributes(), logLimits.getMaxAttributeValueLength()); } - this.extendedAttributes.put(key, value); + this.extendedAttributes.putIfCapacity(key, value); return this; } diff --git a/sdk/logs/src/main/java/io/opentelemetry/sdk/logs/ExtendedSdkReadWriteLogRecord.java b/sdk/logs/src/main/java/io/opentelemetry/sdk/logs/ExtendedSdkReadWriteLogRecord.java index 0cd5c78566e..d7c26a16864 100644 --- a/sdk/logs/src/main/java/io/opentelemetry/sdk/logs/ExtendedSdkReadWriteLogRecord.java +++ b/sdk/logs/src/main/java/io/opentelemetry/sdk/logs/ExtendedSdkReadWriteLogRecord.java @@ -105,7 +105,7 @@ public ExtendedSdkReadWriteLogRecord setAttribute(ExtendedAttributeKey ke ExtendedAttributesMap.create( logLimits.getMaxNumberOfAttributes(), logLimits.getMaxAttributeValueLength()); } - extendedAttributes.put(key, value); + extendedAttributes.putIfCapacity(key, value); } return this; } diff --git a/sdk/logs/src/main/java/io/opentelemetry/sdk/logs/SdkLogRecordBuilder.java b/sdk/logs/src/main/java/io/opentelemetry/sdk/logs/SdkLogRecordBuilder.java index 1fa78926396..7da8d41b1df 100644 --- a/sdk/logs/src/main/java/io/opentelemetry/sdk/logs/SdkLogRecordBuilder.java +++ b/sdk/logs/src/main/java/io/opentelemetry/sdk/logs/SdkLogRecordBuilder.java @@ -111,7 +111,7 @@ public SdkLogRecordBuilder setAttribute(AttributeKey key, @Nullable T val AttributesMap.create( logLimits.getMaxNumberOfAttributes(), logLimits.getMaxAttributeValueLength()); } - this.attributes.put(key, value); + this.attributes.putIfCapacity(key, value); return this; } diff --git a/sdk/logs/src/main/java/io/opentelemetry/sdk/logs/SdkReadWriteLogRecord.java b/sdk/logs/src/main/java/io/opentelemetry/sdk/logs/SdkReadWriteLogRecord.java index ee3bc5b385b..32e77beb6ce 100644 --- a/sdk/logs/src/main/java/io/opentelemetry/sdk/logs/SdkReadWriteLogRecord.java +++ b/sdk/logs/src/main/java/io/opentelemetry/sdk/logs/SdkReadWriteLogRecord.java @@ -100,7 +100,7 @@ public ReadWriteLogRecord setAttribute(AttributeKey key, T value) { AttributesMap.create( logLimits.getMaxNumberOfAttributes(), logLimits.getMaxAttributeValueLength()); } - attributes.put(key, value); + attributes.putIfCapacity(key, value); } return this; } diff --git a/sdk/logs/src/test/java/io/opentelemetry/sdk/logs/ReadWriteLogRecordTest.java b/sdk/logs/src/test/java/io/opentelemetry/sdk/logs/ReadWriteLogRecordTest.java index 3dfca08f04b..c229c8e9c0f 100644 --- a/sdk/logs/src/test/java/io/opentelemetry/sdk/logs/ReadWriteLogRecordTest.java +++ b/sdk/logs/src/test/java/io/opentelemetry/sdk/logs/ReadWriteLogRecordTest.java @@ -51,8 +51,8 @@ void allHandlesEmpty() { SdkReadWriteLogRecord buildLogRecord() { Value body = Value.of("bod"); AttributesMap initialAttributes = AttributesMap.create(100, 200); - initialAttributes.put(stringKey("foo"), "aaiosjfjioasdiojfjioasojifja"); - initialAttributes.put(stringKey("untouched"), "yes"); + initialAttributes.putIfCapacity(stringKey("foo"), "aaiosjfjioasdiojfjioasojifja"); + initialAttributes.putIfCapacity(stringKey("untouched"), "yes"); LogLimits limits = LogLimits.getDefault(); Resource resource = Resource.empty(); InstrumentationScopeInfo scope = InstrumentationScopeInfo.create("test"); diff --git a/sdk/logs/src/testIncubating/java/io/opentelemetry/sdk/logs/ExtendedLoggerBuilderTest.java b/sdk/logs/src/testIncubating/java/io/opentelemetry/sdk/logs/ExtendedLoggerBuilderTest.java index ca2a4942a0b..17e1eb0ddff 100644 --- a/sdk/logs/src/testIncubating/java/io/opentelemetry/sdk/logs/ExtendedLoggerBuilderTest.java +++ b/sdk/logs/src/testIncubating/java/io/opentelemetry/sdk/logs/ExtendedLoggerBuilderTest.java @@ -18,7 +18,6 @@ import io.opentelemetry.sdk.logs.export.SimpleLogRecordProcessor; import io.opentelemetry.sdk.logs.internal.SdkLoggerProviderUtil; import io.opentelemetry.sdk.testing.exporter.InMemoryLogRecordExporter; -import javax.annotation.Nullable; import org.junit.jupiter.api.Test; class ExtendedLoggerBuilderTest { @@ -55,19 +54,11 @@ void setException_CustomResolver() { loggerProviderBuilder, new ExceptionAttributeResolver() { @Override - public String getExceptionType(Throwable throwable, int maxAttributeLength) { - return "type"; - } - - @Nullable - @Override - public String getExceptionMessage(Throwable throwable, int maxAttributeLength) { - return null; - } - - @Override - public String getExceptionStacktrace(Throwable throwable, int maxAttributeLength) { - return "stacktrace"; + public void setExceptionAttributes( + AttributeSetter attributeSetter, Throwable throwable, int maxAttributeLength) { + attributeSetter.setAttribute(ExceptionAttributeResolver.EXCEPTION_TYPE, "type"); + attributeSetter.setAttribute( + ExceptionAttributeResolver.EXCEPTION_STACKTRACE, "stacktrace"); } }); 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 141f1805f0a..73f83019d48 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 @@ -336,7 +336,7 @@ public ReadWriteSpan setAttribute(AttributeKey key, @Nullable T value) { spanLimits.getMaxNumberOfAttributes(), spanLimits.getMaxAttributeValueLength()); } - attributes.put(key, value); + attributes.putIfCapacity(key, value); } return this; } @@ -464,6 +464,7 @@ public ReadWriteSpan recordException(Throwable exception) { } @Override + @SuppressWarnings("unchecked") public ReadWriteSpan recordException(Throwable exception, Attributes additionalAttributes) { if (exception == null) { return this; @@ -477,8 +478,8 @@ public ReadWriteSpan recordException(Throwable exception, Attributes additionalA AttributesMap.create( spanLimits.getMaxNumberOfAttributes(), spanLimits.getMaxAttributeValueLength()); - AttributeUtil.addExceptionAttributes( - exceptionAttributeResolver, maxAttributeLength, exception, attributes::put); + exceptionAttributeResolver.setExceptionAttributes( + attributes::putIfCapacity, exception, maxAttributeLength); additionalAttributes.forEach(attributes::put); diff --git a/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/SdkSpanBuilder.java b/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/SdkSpanBuilder.java index c0f872265ec..e3da934f0a4 100644 --- a/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/SdkSpanBuilder.java +++ b/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/SdkSpanBuilder.java @@ -150,7 +150,7 @@ public SpanBuilder setAttribute(AttributeKey key, T value) { if (key == null || key.getKey().isEmpty() || value == null) { return this; } - attributes().put(key, value); + attributes().putIfCapacity(key, value); return this; } @@ -209,7 +209,8 @@ public Span startSpan() { } Attributes samplingAttributes = samplingResult.getAttributes(); if (!samplingAttributes.isEmpty()) { - samplingAttributes.forEach((key, value) -> attributes().put((AttributeKey) key, value)); + samplingAttributes.forEach( + (key, value) -> attributes().putIfCapacity((AttributeKey) key, value)); } // Avoid any possibility to modify the attributes by adding attributes to the Builder after the 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 beaa480061f..17334c2c893 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 @@ -1303,19 +1303,11 @@ void recordException_CustomResolver() { ExceptionAttributeResolver exceptionAttributeResolver = new ExceptionAttributeResolver() { @Override - public String getExceptionType(Throwable throwable, int maxAttributeLength) { - return "type"; - } - - @Nullable - @Override - public String getExceptionMessage(Throwable throwable, int maxAttributeLength) { - return null; - } - - @Override - public String getExceptionStacktrace(Throwable throwable, int maxAttributeLength) { - return "stacktrace"; + public void setExceptionAttributes( + AttributeSetter attributeSetter, Throwable throwable, int maxAttributeLength) { + attributeSetter.setAttribute(ExceptionAttributeResolver.EXCEPTION_TYPE, "type"); + attributeSetter.setAttribute( + ExceptionAttributeResolver.EXCEPTION_STACKTRACE, "stacktrace"); } }; @@ -1466,7 +1458,7 @@ private SdkSpan createTestSpanWithAttributes(Map attribute AttributesMap attributesMap = AttributesMap.create( spanLimits.getMaxNumberOfAttributes(), spanLimits.getMaxAttributeValueLength()); - attributes.forEach(attributesMap::put); + attributes.forEach(attributesMap::putIfCapacity); return createTestSpan( SpanKind.INTERNAL, SpanLimits.getDefault(), 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 5198a9b3cb7..f0b2d64f203 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 @@ -8,6 +8,8 @@ import static io.opentelemetry.api.common.AttributeKey.stringKey; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.verify; @@ -302,8 +304,6 @@ void exceptionAttributeResolver() { Exception exception = new Exception("error"); builder.build().get("tracer").spanBuilder("span").startSpan().recordException(exception).end(); - verify(exceptionAttributeResolver).getExceptionType(exception, maxAttributeLength); - verify(exceptionAttributeResolver).getExceptionMessage(exception, maxAttributeLength); - verify(exceptionAttributeResolver).getExceptionStacktrace(exception, maxAttributeLength); + verify(exceptionAttributeResolver).setExceptionAttributes(any(), any(), anyInt()); } } From 84d78c7bcac75f3db935ff01b837e658c5cc22bf Mon Sep 17 00:00:00 2001 From: Jack Berg Date: Fri, 30 May 2025 16:47:23 -0500 Subject: [PATCH 6/8] cleanup --- .../sdk/internal/AttributesMapTest.java | 4 +- .../sdk/logs/ExtendedSdkLogRecordBuilder.java | 2 +- .../sdk/logs/SdkLogRecordBuilder.java | 2 +- .../sdk/logs/SdkReadWriteLogRecord.java | 2 +- .../sdk/logs/ReadWriteLogRecordTest.java | 4 +- .../io/opentelemetry/sdk/trace/SdkSpan.java | 3 +- .../opentelemetry/sdk/trace/SdkSpanTest.java | 42 +++++++++---------- 7 files changed, 29 insertions(+), 30 deletions(-) diff --git a/sdk/common/src/test/java/io/opentelemetry/sdk/internal/AttributesMapTest.java b/sdk/common/src/test/java/io/opentelemetry/sdk/internal/AttributesMapTest.java index 2ea6c328a62..7dfdf6c808c 100644 --- a/sdk/common/src/test/java/io/opentelemetry/sdk/internal/AttributesMapTest.java +++ b/sdk/common/src/test/java/io/opentelemetry/sdk/internal/AttributesMapTest.java @@ -16,8 +16,8 @@ class AttributesMapTest { @Test void asMap() { AttributesMap attributesMap = AttributesMap.create(2, Integer.MAX_VALUE); - attributesMap.putIfCapacity(longKey("one"), 1L); - attributesMap.putIfCapacity(longKey("two"), 2L); + attributesMap.put(longKey("one"), 1L); + attributesMap.put(longKey("two"), 2L); assertThat(attributesMap.asMap()) .containsOnly(entry(longKey("one"), 1L), entry(longKey("two"), 2L)); 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 a245f4a1203..41596afa3da 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 @@ -115,7 +115,7 @@ public ExtendedSdkLogRecordBuilder setAttribute(ExtendedAttributeKey key, ExtendedAttributesMap.create( logLimits.getMaxNumberOfAttributes(), logLimits.getMaxAttributeValueLength()); } - this.extendedAttributes.putIfCapacity(key, value); + this.extendedAttributes.put(key, value); return this; } diff --git a/sdk/logs/src/main/java/io/opentelemetry/sdk/logs/SdkLogRecordBuilder.java b/sdk/logs/src/main/java/io/opentelemetry/sdk/logs/SdkLogRecordBuilder.java index 7da8d41b1df..1fa78926396 100644 --- a/sdk/logs/src/main/java/io/opentelemetry/sdk/logs/SdkLogRecordBuilder.java +++ b/sdk/logs/src/main/java/io/opentelemetry/sdk/logs/SdkLogRecordBuilder.java @@ -111,7 +111,7 @@ public SdkLogRecordBuilder setAttribute(AttributeKey key, @Nullable T val AttributesMap.create( logLimits.getMaxNumberOfAttributes(), logLimits.getMaxAttributeValueLength()); } - this.attributes.putIfCapacity(key, value); + this.attributes.put(key, value); return this; } diff --git a/sdk/logs/src/main/java/io/opentelemetry/sdk/logs/SdkReadWriteLogRecord.java b/sdk/logs/src/main/java/io/opentelemetry/sdk/logs/SdkReadWriteLogRecord.java index 32e77beb6ce..ee3bc5b385b 100644 --- a/sdk/logs/src/main/java/io/opentelemetry/sdk/logs/SdkReadWriteLogRecord.java +++ b/sdk/logs/src/main/java/io/opentelemetry/sdk/logs/SdkReadWriteLogRecord.java @@ -100,7 +100,7 @@ public ReadWriteLogRecord setAttribute(AttributeKey key, T value) { AttributesMap.create( logLimits.getMaxNumberOfAttributes(), logLimits.getMaxAttributeValueLength()); } - attributes.putIfCapacity(key, value); + attributes.put(key, value); } return this; } diff --git a/sdk/logs/src/test/java/io/opentelemetry/sdk/logs/ReadWriteLogRecordTest.java b/sdk/logs/src/test/java/io/opentelemetry/sdk/logs/ReadWriteLogRecordTest.java index c229c8e9c0f..3dfca08f04b 100644 --- a/sdk/logs/src/test/java/io/opentelemetry/sdk/logs/ReadWriteLogRecordTest.java +++ b/sdk/logs/src/test/java/io/opentelemetry/sdk/logs/ReadWriteLogRecordTest.java @@ -51,8 +51,8 @@ void allHandlesEmpty() { SdkReadWriteLogRecord buildLogRecord() { Value body = Value.of("bod"); AttributesMap initialAttributes = AttributesMap.create(100, 200); - initialAttributes.putIfCapacity(stringKey("foo"), "aaiosjfjioasdiojfjioasojifja"); - initialAttributes.putIfCapacity(stringKey("untouched"), "yes"); + initialAttributes.put(stringKey("foo"), "aaiosjfjioasdiojfjioasojifja"); + initialAttributes.put(stringKey("untouched"), "yes"); LogLimits limits = LogLimits.getDefault(); Resource resource = Resource.empty(); InstrumentationScopeInfo scope = InstrumentationScopeInfo.create("test"); 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 73f83019d48..37deab7ffc8 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 @@ -336,7 +336,7 @@ public ReadWriteSpan setAttribute(AttributeKey key, @Nullable T value) { spanLimits.getMaxNumberOfAttributes(), spanLimits.getMaxAttributeValueLength()); } - attributes.putIfCapacity(key, value); + attributes.put(key, value); } return this; } @@ -464,7 +464,6 @@ public ReadWriteSpan recordException(Throwable exception) { } @Override - @SuppressWarnings("unchecked") public ReadWriteSpan recordException(Throwable exception, Attributes additionalAttributes) { if (exception == null) { return this; 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 17334c2c893..d14eda2ef61 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 @@ -445,7 +445,7 @@ void getAttribute() { void getAttributes() { SdkSpan span = createTestSpanWithAttributes(attributes); try { - Assertions.assertThat(span.getAttributes()) + assertThat(span.getAttributes()) .isEqualTo( Attributes.builder() .put("MyBooleanAttributeKey", false) @@ -461,7 +461,7 @@ void getAttributes() { void getAttributes_Empty() { SdkSpan span = createTestSpan(SpanKind.INTERNAL); try { - Assertions.assertThat(span.getAttributes()).isEqualTo(Attributes.empty()); + assertThat(span.getAttributes()).isEqualTo(Attributes.empty()); } finally { span.end(); } @@ -795,48 +795,48 @@ void addEvent() { } List events = span.toSpanData().getEvents(); assertThat(events).hasSize(6); - Assertions.assertThat(events.get(0)) + assertThat(events.get(0)) .satisfies( event -> { assertThat(event.getName()).isEqualTo("event1"); - Assertions.assertThat(event.getAttributes()).isEqualTo(Attributes.empty()); + assertThat(event.getAttributes()).isEqualTo(Attributes.empty()); assertThat(event.getEpochNanos()).isEqualTo(START_EPOCH_NANOS); }); - Assertions.assertThat(events.get(1)) + assertThat(events.get(1)) .satisfies( event -> { assertThat(event.getName()).isEqualTo("event2"); - Assertions.assertThat(event.getAttributes()) + assertThat(event.getAttributes()) .isEqualTo(Attributes.of(stringKey("e1key"), "e1Value")); assertThat(event.getEpochNanos()).isEqualTo(START_EPOCH_NANOS); }); - Assertions.assertThat(events.get(2)) + assertThat(events.get(2)) .satisfies( event -> { assertThat(event.getName()).isEqualTo("event3"); - Assertions.assertThat(event.getAttributes()).isEqualTo(Attributes.empty()); + assertThat(event.getAttributes()).isEqualTo(Attributes.empty()); assertThat(event.getEpochNanos()).isEqualTo(TimeUnit.SECONDS.toNanos(10)); }); - Assertions.assertThat(events.get(3)) + assertThat(events.get(3)) .satisfies( event -> { assertThat(event.getName()).isEqualTo("event4"); - Assertions.assertThat(event.getAttributes()).isEqualTo(Attributes.empty()); + assertThat(event.getAttributes()).isEqualTo(Attributes.empty()); assertThat(event.getEpochNanos()).isEqualTo(TimeUnit.SECONDS.toNanos(20)); }); - Assertions.assertThat(events.get(4)) + assertThat(events.get(4)) .satisfies( event -> { assertThat(event.getName()).isEqualTo("event5"); - Assertions.assertThat(event.getAttributes()) + assertThat(event.getAttributes()) .isEqualTo(Attributes.builder().put("foo", "bar").build()); assertThat(event.getEpochNanos()).isEqualTo(TimeUnit.MILLISECONDS.toNanos(30)); }); - Assertions.assertThat(events.get(5)) + assertThat(events.get(5)) .satisfies( event -> { assertThat(event.getName()).isEqualTo("event6"); - Assertions.assertThat(event.getAttributes()) + assertThat(event.getAttributes()) .isEqualTo(Attributes.builder().put("foo", "bar").build()); assertThat(event.getEpochNanos()).isEqualTo(TimeUnit.MILLISECONDS.toNanos(1000)); }); @@ -986,11 +986,11 @@ void addLink() { .satisfiesExactly( link -> { assertThat(link.getSpanContext()).isEqualTo(span1.getSpanContext()); - Assertions.assertThat(link.getAttributes()).isEqualTo(Attributes.empty()); + assertThat(link.getAttributes()).isEqualTo(Attributes.empty()); }, link -> { assertThat(link.getSpanContext()).isEqualTo(span2.getSpanContext()); - Assertions.assertThat(link.getAttributes()) + assertThat(link.getAttributes()) .isEqualTo( Attributes.builder() .put("key1", true) @@ -1143,7 +1143,7 @@ void droppingEvents() { EventData expectedEvent = EventData.create( START_EPOCH_NANOS + i * NANOS_PER_SECOND, "event2", Attributes.empty(), 0); - Assertions.assertThat(spanData.getEvents().get(i)).isEqualTo(expectedEvent); + assertThat(spanData.getEvents().get(i)).isEqualTo(expectedEvent); assertThat(spanData.getTotalRecordedEvents()).isEqualTo(2 * maxNumberOfEvents); } } finally { @@ -1155,7 +1155,7 @@ void droppingEvents() { EventData expectedEvent = EventData.create( START_EPOCH_NANOS + i * NANOS_PER_SECOND, "event2", Attributes.empty(), 0); - Assertions.assertThat(spanData.getEvents().get(i)).isEqualTo(expectedEvent); + assertThat(spanData.getEvents().get(i)).isEqualTo(expectedEvent); } } @@ -1187,7 +1187,7 @@ void recordException() { .isEqualTo(exception.getClass().getName()); assertThat(event.getAttributes().get(stringKey("exception.stacktrace"))).isEqualTo(stacktrace); assertThat(event.getAttributes().size()).isEqualTo(3); - Assertions.assertThat(event) + assertThat(event) .isInstanceOfSatisfying( ExceptionEventData.class, exceptionEvent -> { @@ -1275,7 +1275,7 @@ void recordException_additionalAttributes() { assertThat(event.getAttributes().get(stringKey("exception.stacktrace"))).isEqualTo(stacktrace); assertThat(event.getAttributes().size()).isEqualTo(4); - Assertions.assertThat(event) + assertThat(event) .isInstanceOfSatisfying( ExceptionEventData.class, exceptionEvent -> { @@ -1458,7 +1458,7 @@ private SdkSpan createTestSpanWithAttributes(Map attribute AttributesMap attributesMap = AttributesMap.create( spanLimits.getMaxNumberOfAttributes(), spanLimits.getMaxAttributeValueLength()); - attributes.forEach(attributesMap::putIfCapacity); + attributes.forEach(attributesMap::put); return createTestSpan( SpanKind.INTERNAL, SpanLimits.getDefault(), From 06f974defe7a8850b13c2c76e72e03c56d6c9ee2 Mon Sep 17 00:00:00 2001 From: Jack Berg Date: Fri, 30 May 2025 16:54:42 -0500 Subject: [PATCH 7/8] More cleanup --- .../DefaultExceptionAttributeResolver.java | 21 ++++++++++++------- .../logs/ExtendedSdkReadWriteLogRecord.java | 2 +- .../sdk/trace/SdkSpanBuilder.java | 5 ++--- .../opentelemetry/sdk/trace/SdkSpanTest.java | 1 - 4 files changed, 17 insertions(+), 12 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 2c78c4f1d86..44061c1644f 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 @@ -27,17 +27,24 @@ public static ExceptionAttributeResolver getInstance() { @Override public void setExceptionAttributes( AttributeSetter attributeSetter, Throwable throwable, int maxAttributeLength) { - attributeSetter.setAttribute( - ExceptionAttributeResolver.EXCEPTION_TYPE, throwable.getClass().getCanonicalName()); - String message = throwable.getMessage(); - if (message != null) { - attributeSetter.setAttribute(ExceptionAttributeResolver.EXCEPTION_MESSAGE, message); + String exceptionType = throwable.getClass().getCanonicalName(); + if (exceptionType != null) { + attributeSetter.setAttribute(ExceptionAttributeResolver.EXCEPTION_TYPE, exceptionType); } + + String exceptionMessage = throwable.getMessage(); + if (exceptionMessage != null) { + attributeSetter.setAttribute(ExceptionAttributeResolver.EXCEPTION_MESSAGE, exceptionMessage); + } + StringWriter stringWriter = new StringWriter(); try (PrintWriter printWriter = new PrintWriter(stringWriter)) { throwable.printStackTrace(printWriter); } - attributeSetter.setAttribute( - ExceptionAttributeResolver.EXCEPTION_STACKTRACE, stringWriter.toString()); + String exceptionStacktrace = stringWriter.toString(); + if (exceptionStacktrace != null) { + attributeSetter.setAttribute( + ExceptionAttributeResolver.EXCEPTION_STACKTRACE, exceptionStacktrace); + } } } diff --git a/sdk/logs/src/main/java/io/opentelemetry/sdk/logs/ExtendedSdkReadWriteLogRecord.java b/sdk/logs/src/main/java/io/opentelemetry/sdk/logs/ExtendedSdkReadWriteLogRecord.java index d7c26a16864..0cd5c78566e 100644 --- a/sdk/logs/src/main/java/io/opentelemetry/sdk/logs/ExtendedSdkReadWriteLogRecord.java +++ b/sdk/logs/src/main/java/io/opentelemetry/sdk/logs/ExtendedSdkReadWriteLogRecord.java @@ -105,7 +105,7 @@ public ExtendedSdkReadWriteLogRecord setAttribute(ExtendedAttributeKey ke ExtendedAttributesMap.create( logLimits.getMaxNumberOfAttributes(), logLimits.getMaxAttributeValueLength()); } - extendedAttributes.putIfCapacity(key, value); + extendedAttributes.put(key, value); } return this; } diff --git a/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/SdkSpanBuilder.java b/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/SdkSpanBuilder.java index e3da934f0a4..c0f872265ec 100644 --- a/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/SdkSpanBuilder.java +++ b/sdk/trace/src/main/java/io/opentelemetry/sdk/trace/SdkSpanBuilder.java @@ -150,7 +150,7 @@ public SpanBuilder setAttribute(AttributeKey key, T value) { if (key == null || key.getKey().isEmpty() || value == null) { return this; } - attributes().putIfCapacity(key, value); + attributes().put(key, value); return this; } @@ -209,8 +209,7 @@ public Span startSpan() { } Attributes samplingAttributes = samplingResult.getAttributes(); if (!samplingAttributes.isEmpty()) { - samplingAttributes.forEach( - (key, value) -> attributes().putIfCapacity((AttributeKey) key, value)); + samplingAttributes.forEach((key, value) -> attributes().put((AttributeKey) key, value)); } // Avoid any possibility to modify the attributes by adding attributes to the Builder after the 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 d14eda2ef61..fcfec65d47f 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 @@ -70,7 +70,6 @@ import java.util.stream.IntStream; import java.util.stream.Stream; import javax.annotation.Nullable; -import org.assertj.core.api.Assertions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; From 09b99a5649e9c2ae56c99c58c360b305b01628fe Mon Sep 17 00:00:00 2001 From: Jack Berg Date: Wed, 4 Jun 2025 10:03:51 -0500 Subject: [PATCH 8/8] feedback --- .../io/opentelemetry/sdk/trace/SdkTracerProviderTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 f0b2d64f203..345f97d52c0 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 @@ -9,7 +9,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.anyInt; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.verify; @@ -304,6 +304,6 @@ void exceptionAttributeResolver() { Exception exception = new Exception("error"); builder.build().get("tracer").spanBuilder("span").startSpan().recordException(exception).end(); - verify(exceptionAttributeResolver).setExceptionAttributes(any(), any(), anyInt()); + verify(exceptionAttributeResolver).setExceptionAttributes(any(), any(), eq(maxAttributeLength)); } }