-
Notifications
You must be signed in to change notification settings - Fork 920
Lazily evaluate ExceptionEventData exception.* attributes #7172
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
bbaa9e5
c77f5ab
37e47a2
081960d
2fe4233
a7931ca
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,94 @@ | ||
| /* | ||
| * Copyright The OpenTelemetry Authors | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| */ | ||
|
|
||
| package io.opentelemetry.sdk.trace.internal.data; | ||
|
|
||
| import com.google.auto.value.AutoValue; | ||
| import com.google.auto.value.extension.memoized.Memoized; | ||
| import io.opentelemetry.api.common.AttributeKey; | ||
| import io.opentelemetry.api.common.Attributes; | ||
| import io.opentelemetry.sdk.internal.AttributeUtil; | ||
| import io.opentelemetry.sdk.internal.AttributesMap; | ||
| import io.opentelemetry.sdk.trace.SpanLimits; | ||
| import io.opentelemetry.sdk.trace.data.ExceptionEventData; | ||
| import javax.annotation.Nullable; | ||
| import javax.annotation.concurrent.Immutable; | ||
|
|
||
| /** | ||
| * An {@link ExceptionEventData} implementation with {@link #getAttributes()} lazily evaluated, | ||
| * allowing the (relatively) expensive exception attribute rendering to take place off the hot path. | ||
| * | ||
| * <p>This class is internal and is hence not for public use. Its APIs are unstable and can change | ||
| * at any time. | ||
| */ | ||
| @AutoValue | ||
| @Immutable | ||
| public abstract class LazyExceptionEventData implements ExceptionEventData { | ||
|
|
||
| private static final AttributeKey<String> EXCEPTION_TYPE = | ||
| AttributeKey.stringKey("exception.type"); | ||
| private static final AttributeKey<String> EXCEPTION_MESSAGE = | ||
| AttributeKey.stringKey("exception.message"); | ||
| private static final AttributeKey<String> EXCEPTION_STACKTRACE = | ||
| AttributeKey.stringKey("exception.stacktrace"); | ||
| private static final String EXCEPTION_EVENT_NAME = "exception"; | ||
|
|
||
| @Override | ||
| public final String getName() { | ||
| return EXCEPTION_EVENT_NAME; | ||
| } | ||
|
|
||
| // Autowire generates AutoValue_LazyExceptionEventData and $AutoValue_LazyExceptionEventData to | ||
| // account to memoize getAttributes. Unfortunately, $AutoValue_LazyExceptionEventData's | ||
| // exceptionMessage constructor param is properly annotated with @Nullable, but | ||
| // AutoValue_LazyExceptionEventData's is not. So we suppress the NullAway false positive. | ||
| @SuppressWarnings("NullAway") | ||
| public static ExceptionEventData create( | ||
| long epochNanos, | ||
| Throwable exception, | ||
| Attributes additionalAttributes, | ||
| SpanLimits spanLimits) { | ||
| // Compute exception message at initialization time to be conservative about possibility of | ||
| // Exception#geMessage() not being thread safe | ||
| return new AutoValue_LazyExceptionEventData( | ||
| epochNanos, exception, spanLimits, exception.getMessage(), additionalAttributes); | ||
| } | ||
|
|
||
| abstract SpanLimits getSpanLimits(); | ||
|
|
||
| @Nullable | ||
| abstract String getExceptionMessage(); | ||
|
|
||
| public abstract Attributes getAdditionalAttributes(); | ||
|
|
||
| @Override | ||
| @Memoized | ||
| public Attributes getAttributes() { | ||
| Throwable exception = getException(); | ||
| SpanLimits spanLimits = getSpanLimits(); | ||
| Attributes additionalAttributes = getAdditionalAttributes(); | ||
|
|
||
| AttributesMap attributes = | ||
| AttributesMap.create( | ||
| spanLimits.getMaxNumberOfAttributes(), spanLimits.getMaxAttributeValueLength()); | ||
|
|
||
| AttributeUtil.addExceptionAttributes(attributes::put, exception, getExceptionMessage()); | ||
|
|
||
| additionalAttributes.forEach(attributes::put); | ||
|
|
||
| return attributes.immutableCopy(); | ||
| } | ||
|
|
||
| @Override | ||
| public int getTotalAttributeCount() { | ||
| // getAttributes() lazily adds 3 attributes to getAdditionalAttributes(): | ||
| // - exception.type | ||
| // - exception.message | ||
| // - exception.stacktrace | ||
| return getAdditionalAttributes().size() + 3; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. for users of this method does it matter that there could be overlap and so this calculation could overcount (e.g. a user overriding
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh great point. Need to think this through a little more
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should we move this to a Draft until you're ready to get this figured out? |
||
| } | ||
|
|
||
| LazyExceptionEventData() {} | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cc @HaloFour I believe this should be a (temporary) workaround to the changes in #6795. To restore the behavior you were relying on, you should be able to:
eventData instanceOf LazyExceptionEventDataLazyExceptionEventData#getAdditionalAttributes()without rendering theexception.*I'd like to followup with this PRs change with:
LogRecordBuilder#setException