Skip to content

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.

Sign up for GitHub

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

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Lazily evaluate ExceptionEventData exception.* attributes #7172

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
bbaa9e5
Lazily evaluate ExceptionEventData exception.* attributes
jack-berg Mar 4, 2025
c77f5ab
Merge branch 'main' of https://github.com/open-telemetry/opentelemetr…
jack-berg Apr 10, 2025
37e47a2
Compute exception.message on initialization
jack-berg Apr 10, 2025
081960d
Merge branch 'main' of https://github.com/open-telemetry/opentelemetr…
jack-berg Apr 11, 2025
2fe4233
Remove extra code from merge
jack-berg Apr 11, 2025
a7931ca
Use precomputed exception message
jack-berg Apr 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 3 additions & 35 deletions sdk/trace/src/main/java/io/opentelemetry/sdk/trace/SdkSpan.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,11 @@
import io.opentelemetry.sdk.internal.InstrumentationScopeUtil;
import io.opentelemetry.sdk.resources.Resource;
import io.opentelemetry.sdk.trace.data.EventData;
import io.opentelemetry.sdk.trace.data.ExceptionEventData;
import io.opentelemetry.sdk.trace.data.LinkData;
import io.opentelemetry.sdk.trace.data.SpanData;
import io.opentelemetry.sdk.trace.data.StatusData;
import io.opentelemetry.sdk.trace.internal.ExtendedSpanProcessor;
import java.io.PrintWriter;
import java.io.StringWriter;
import io.opentelemetry.sdk.trace.internal.data.LazyExceptionEventData;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
Expand Down Expand Up @@ -117,13 +115,6 @@ private enum EndState {
@Nullable
private Thread spanEndingThread;

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 SdkSpan(
SpanContext context,
String name,
Expand Down Expand Up @@ -475,32 +466,9 @@ public ReadWriteSpan recordException(Throwable exception, Attributes additionalA
additionalAttributes = Attributes.empty();
}

AttributesMap attributes =
AttributesMap.create(
spanLimits.getMaxNumberOfAttributes(), spanLimits.getMaxAttributeValueLength());
String exceptionName = exception.getClass().getCanonicalName();
String exceptionMessage = exception.getMessage();
StringWriter stringWriter = new StringWriter();
try (PrintWriter printWriter = new PrintWriter(stringWriter)) {
exception.printStackTrace(printWriter);
}
String stackTrace = stringWriter.toString();

if (exceptionName != null) {
attributes.put(EXCEPTION_TYPE, exceptionName);
}
if (exceptionMessage != null) {
attributes.put(EXCEPTION_MESSAGE, exceptionMessage);
}
if (stackTrace != null) {
attributes.put(EXCEPTION_STACKTRACE, stackTrace);
}

additionalAttributes.forEach(attributes::put);

addTimedEvent(
ExceptionEventData.create(
clock.now(), exception, attributes, attributes.getTotalAddedValues()));
LazyExceptionEventData.create(clock.now(), exception, additionalAttributes, spanLimits));

return this;
}

Expand Down
100 changes: 100 additions & 0 deletions sdk/trace/src/main/java/io/opentelemetry/sdk/trace/internal/data/LazyExceptionEventData.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.sdk.trace.internal.data;

import com.google.auto.value.AutoValue;
import com.google.auto.value.extension.memoized.Memoized;
import io.opentelemetry.api.common.AttributeKey;
import io.opentelemetry.api.common.Attributes;
import io.opentelemetry.sdk.internal.AttributesMap;
import io.opentelemetry.sdk.trace.SpanLimits;
import io.opentelemetry.sdk.trace.data.ExceptionEventData;
import java.io.PrintWriter;
import java.io.StringWriter;
import javax.annotation.concurrent.Immutable;

/**
* An {@link ExceptionEventData} implementation with {@link #getAttributes()} lazily evaluated,
* allowing the (relatively) expensive exception attribute rendering to take place off the hot path.
*
* <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;
}

/** TODO. */
public static ExceptionEventData create(
long epochNanos,
Throwable exception,
Attributes additionalAttributes,
SpanLimits spanLimits) {
return new AutoValue_LazyExceptionEventData(
epochNanos, exception, spanLimits, additionalAttributes);
}

abstract SpanLimits getSpanLimits();

public abstract Attributes getAdditionalAttributes();
Copy link
Member Author

@jack-berg jack-berg Mar 4, 2025

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:

  • Check if eventData instanceOf LazyExceptionEventData
  • If so, access the attributes via LazyExceptionEventData#getAdditionalAttributes() without rendering the exception.*

I'd like to followup with this PRs change with:

  • Add SDK customization hook for defining your own exception rendering logic
  • Introduce API for setting exception on logs LogRecordBuilder#setException
  • Improve the default exception rendering logic to be attribute limits aware, so that if attributes can't be longer than 1000 characters we don't waste resources rendering the stacktrace past this limit


@Override
@Memoized
public Attributes getAttributes() {
Throwable exception = getException();
SpanLimits spanLimits = getSpanLimits();
Attributes additionalAttributes = getAdditionalAttributes();

AttributesMap attributes =
AttributesMap.create(
spanLimits.getMaxNumberOfAttributes(), spanLimits.getMaxAttributeValueLength());
String exceptionName = exception.getClass().getCanonicalName();
String exceptionMessage = exception.getMessage();
StringWriter stringWriter = new StringWriter();
try (PrintWriter printWriter = new PrintWriter(stringWriter)) {
exception.printStackTrace(printWriter);
}
String stackTrace = stringWriter.toString();

if (exceptionName != null) {
attributes.put(EXCEPTION_TYPE, exceptionName);
}
if (exceptionMessage != null) {
attributes.put(EXCEPTION_MESSAGE, exceptionMessage);
}
if (stackTrace != null) {
attributes.put(EXCEPTION_STACKTRACE, stackTrace);
}

additionalAttributes.forEach(attributes::put);

return attributes.immutableCopy();
}

@Override
public int getTotalAttributeCount() {
// getAttributes() lazily adds 3 attributes to getAdditionalAttributes():
// - exception.type
// - exception.message
// - exception.stacktrace
return getAdditionalAttributes().size() + 3;
Copy link
Member

@trask trask Apr 11, 2025

Choose a reason for hiding this comment

The 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 exception.message)?

Copy link
Member Author

@jack-berg jack-berg Apr 11, 2025

Choose a reason for hiding this comment

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

Oh great point. Need to think this through a little more

Copy link
Contributor

@jkwatson jkwatson Sep 11, 2025

Choose a reason for hiding this comment

The 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() {}
}
Loading