Update recording errors doc - document when to use error, exception (or both) attributes#2296
Update recording errors doc - document when to use error, exception (or both) attributes#2296lmolkova wants to merge 6 commits intoopen-telemetry:mainfrom
error, exception (or both) attributes#2296Conversation
2ec5940 to
1dc9015
Compare
Co-authored-by: Armin Ruech <[email protected]>
|
@smaddock could you please leave specific suggestions on the content modified by this PR? WRT
There is no active discussion on this - I'd suggest to start with an issue in the https://github.com/open-telemetry/opentelemetry-specification. You also probably want to read open-telemetry/opentelemetry-specification#4333 |
| - Semantic conventions intended for cross-language applicability SHOULD use | ||
| `error.*` attributes. | ||
|
|
||
| - `exception` attributes SHOULD be documented for conventions that target |
There was a problem hiding this comment.
this maybe a breaking(?) change for go/rust/no-exception languages
There was a problem hiding this comment.
So for RUST at least - I'd tie the notion of "exception" to the presence of a backtrace on the error type.
Specifically, we want:
- In Rust, if you are signalling any type
Twhich implementsstd::error::Errorwith a backtrace and are logging it, you fill outexception.stacktracewith theBacktracefrom Rust. - Following this guidance for accessing members by type on Error with a convention of attaching
Backtraceas a field in Error. - Ensuring this guidance works with
anyhowcrate.
If I read this guidance correctly, this means Rust would do the following:
- Always write
error.type(the rust error type) - Always write
error.message(usingdescriptionof the rust error) - Optionally write
exception.stacktrace(by accessingBacktracefield onErrorwhen available. - Optional write
exception.typeto matcherror.typeif a Backtrace exists?
Can you confirm?
Also would be good for @open-telemetry/rust-maintainers to confirm my assumptions on error handling here.
There was a problem hiding this comment.
https://github.com/open-telemetry/opentelemetry-rust/blob/main/opentelemetry-appender-tracing/src/layer.rs#L83-L93
This is what we do today (only error.message)
|
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
There was a problem hiding this comment.
Pull Request Overview
This PR enhances the error guidance by clarifying when to use error versus exception attributes, adds logging/event recommendations, and updates examples to ensure proper error reporting.
- Defines “Error vs exception” and when both attribute sets apply
- Adds “Recording errors on logs and events” section
- Updates example snippets and cross-links to the general error guidance
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| model/error/registry.yaml | Added link to the general recording-errors documentation |
| docs/registry/attributes/error.md | Linked to the new recording-errors guide |
| docs/general/recording-errors.md | Introduced Error vs exception section, log/event guidance, warnings, and updated examples |
| .chloggen/2296.yaml | Updated changelog metadata |
Comments suppressed due to low confidence (3)
docs/general/recording-errors.md:57
- Consider capitalizing the first word of this bullet to match the style of other list items (e.g., change “when” to “When”).
- when the error and exception details are identical, and both sets
docs/general/recording-errors.md:170
- The blank
>line after the warning header may break the admonition rendering. Removing this blank line will ensure the warning block displays correctly.
>
docs/general/recording-errors.md:210
- Align the
finallybrace indentation with thecatchblock for consistent formatting (e.g., match the indentation level of the closing}of thecatch).
} finally {
|
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
|
@lmolkova Is this PR still active? |
| It's NOT RECOMMENDED to duplicate status code or `error.type` in span status description. | ||
| It's NOT RECOMMENDED to duplicate status code, `error.type`, `error.message`, | ||
| or `exception.message` in the span status description. |
There was a problem hiding this comment.
The note on the error.message attribute says it is NOT RECOMMENDED for spans. This text about duplicating error.message in the span status description may give the false impression that the attribute is preferred.
| Exceptions and how they are recorded in telemetry are inherently | ||
| language-specific. Some languages, such as Rust or Go, do not use exceptions | ||
| at all. |
There was a problem hiding this comment.
I think that a panic in Go be considered as an exception.
Then a deffered span.End can handle "the exception" (recover the panic and record it).
See: https://github.com/open-telemetry/opentelemetry-go/blob/b5b6989c7a08b5e4773236fc8d9fa0e5fe9788e1/sdk/trace/span.go#L476-L493
I think it would be good to clarify it.
| In the scope of this document, the terms error and exception are defined as follows: | ||
|
|
||
| - *Error* refers to a general concept describing any non-success condition. | ||
| This may include a non-success status code, or an invalid response. | ||
|
|
||
| - *Exception* specifically refers to runtime exceptions and their | ||
| associated stack traces. |
There was a problem hiding this comment.
@open-telemetry/go-maintainers,
While a personally like this distinction (and I find it correct), I am not sure if/how we are going to make it in Go in a non-breaking way given https://pkg.go.dev/go.opentelemetry.io/otel/trace#Span:
// RecordError will record err as an exception span event for this span. An
// additional call to SetStatus is required if the Status of the Span should
// be set to Error, as this method does not change the Span status. If this
// span is not being recorded or err is nil then this method does nothing.
RecordError(err error), options ...EventOption)Maybe deprecate this function and create a new one that would record errors according to the new semantics?
Related issues:
|
Drafting this one and blocking on open-telemetry/opentelemetry-specification#4333 |
|
This PR has been labeled as stale due to lack of activity. It will be automatically closed if there is no further activity over the next 7 days. |
Fixes #2001
Updates errors guidance to cover:
error.messageattribute