Skip to content
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

Recording exception as standalone events #970

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

Conversation

scheler
Copy link
Contributor

@scheler scheler commented Apr 26, 2024

Changes

Added semantic convention for recording exception on standalone events.

NOTE on when to use what for recording exceptions:

  1. When exception occurs in the context of a span, it MAY be recorded as a Span Event with name exception and attributes exception.*.
  2. When Log Bridge API is used to map exceptions from logs to OpenTelemetry LogRecords, the exception MAY be recorded as a LogRecord using the exception.* attributes.
  3. In other cases, the exception MAY be recorded as an Event with name exception and attributes exception.*.

--

Note: if the PR is touching an area that is not listed in the [existing areas](https://github.com/open-telemetry/semantic-## Merge requirement checklist

[events](https://github.com/open-telemetry/opentelemetry-specification/tree/v1.31.0/specification/logs/event-api.md#emit-event)
emitted through the [EventLogger](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/logs/event-api.md#eventlogger) API.


Copy link
Contributor

Choose a reason for hiding this comment

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

nit: two blank lines, probably failing lint

@martinkuba
Copy link
Contributor

I feel like we may need to document somewhere the difference of capturing exceptions as events vs logs. IMO, it is clear that "event exceptions" are the same as span exceptions - both captured as events with name exception. Log exceptions, I think, are captured only from logging frameworks as ERROR logs? Should backends interpret event exceptions and log exceptions as the same thing, or is it important to note the difference?

@scheler scheler marked this pull request as ready for review May 6, 2024 22:48
@scheler scheler requested review from tigrannajaryan and a team as code owners May 6, 2024 22:48
* Spans and Events: Exception recording in Spans and Events follows a similar pattern. Both utilize the event name `exception`. Exceptions MAY be recorded within Span Events if they occur during the span's lifecycle.
* Logs: Exceptions SHOULD be recorded in logs when utilizing the log bridge API to map application logs to OpenTelemetry logs.

Note that in all cases the exception data is stored in attributes.
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we entertain the idea of recording stack trace in the payload and not in the attribute to avoid all the common problems (parsing, truncation, etc) ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lmolkova Can you elaborate a bit on how recording the stacktrace in the Event Payload (LogRecord Body) address those issues? Even within the any type field, the data would still have to be put into the standard primitive types.

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@trask thanks for the pointer. I guess we were taking the simplistic approach of keeping the exception event consistent with exception spans and exception logs. However, I can see that moving the exception data to Event body/payload would allow us to have richer data in the Event. For eg., in addition to the structured vs plaintext stacktrace representation, we could add context or additonal_data to the Event to capture arbitrary objects which is not possible with attributes which only allow primitive types currently.


When it comes to recording exceptions, it's crucial to distinguish between Spans, Logs, and Events:

* Spans and Events: Exception recording in Spans and Events follows a similar pattern. Both utilize the event name `exception`. Exceptions MAY be recorded within Span Events if they occur during the span's lifecycle.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* Spans and Events: Exception recording in Spans and Events follows a similar pattern. Both utilize the event name `exception`. Exceptions MAY be recorded within Span Events if they occur during the span's lifecycle.
* Spans and Events: Exception recording in Spans and Events follows a similar pattern. Both utilize the event name `exception`. Exceptions SHOULD be recorded within Span Events if they occur during the span's lifecycle.

Is there any reason why we'd diverge from the stable spec statement that makes this a SHOULD here in semconv?
https://opentelemetry.io/docs/specs/otel/trace/exceptions/

Copy link
Member

@trask trask May 24, 2024

Choose a reason for hiding this comment

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

Suggested change
* Spans and Events: Exception recording in Spans and Events follows a similar pattern. Both utilize the event name `exception`. Exceptions MAY be recorded within Span Events if they occur during the span's lifecycle.
* Spans and Events: Exception recording in Spans and Events follows a similar pattern. Both utilize the event name `exception`. Exceptions MAY be recorded as Log-based Events even if they occur during the span's lifecycle.

Copy link
Contributor

@trisch-me trisch-me left a comment

Choose a reason for hiding this comment

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

please add changelog entry correctly

## Unreleased

### Breaking

### Features

- Add semantic conventions for recording exceptions in events [#970](https://github.com/open-telemetry/semantic-conventions/pull/970)
Copy link
Contributor

Choose a reason for hiding this comment

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

We are not updating changelog directly. Please check requirements in the description of your PR

Copy link

github-actions bot commented Jun 9, 2024

This PR was marked stale due to lack of activity. It will be closed in 7 days.

@github-actions github-actions bot added the Stale label Jun 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants