Skip to content

Conversation

@robertlaurin
Copy link
Contributor

@robertlaurin robertlaurin commented Aug 29, 2025

Non breaking change, default behaviour is preserved. Allows for the in_span convenience method to be configured to not capture exception events. Maintains recording of the span status as is.

Related context: open-telemetry/opentelemetry-ruby-contrib#1653

rescue Exception => e # rubocop:disable Lint/RescueException
span&.record_exception(e)
span&.record_exception(e) if record_exception
span&.status = Status.error("Unhandled exception of type: #{e.class}")
Copy link

@kirs kirs Aug 29, 2025

Choose a reason for hiding this comment

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

I find that Unhandled might be misleading here. Maybe just Exception of type:?

Copy link
Contributor

Choose a reason for hiding this comment

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

Unhandled was included in the message to indicate that the exception was unhandled at the time the span was finished, based on the language in the spec:

An exception SHOULD be recorded as an Event on the span during which it occurred if and only if it remains unhandled when the span ends and causes the span status to be set to ERROR.

It may be handled later, but it was not handled within the lifetime of the span.

The spec recommends:

When the status is set to Error by Instrumentation Libraries, the Description SHOULD be documented and predictable.

Sadly, we're violating this other recommendation, but I think changing the description would be more problematic at this point:

It's NOT RECOMMENDED to duplicate status code or error.type in span status description.

@robertlaurin robertlaurin merged commit a6ce85b into main Sep 5, 2025
70 checks passed
@robertlaurin robertlaurin deleted the in_span_record_exception branch September 5, 2025 15:54
@github-actions github-actions bot mentioned this pull request Sep 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants