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

opentelemetry: record error source chain #2122

Merged
merged 4 commits into from
May 20, 2022
Merged

Conversation

lilymara-onesignal
Copy link
Contributor

BREAKING CHANGE: previously error values were recorded using their Debug representation. They are now reported with their Display implementation. This is more in line with current best practices for error handling, but we may want this to be an opt-in behavior.

This is a change in how error values are recorded in the opentelemetry adapter.
For a given field x that contains an error type, record an additional dynamic
field x.chain that contains an array of all errors in the source chain. This
allows users to determine where a high-level error originated.

Motivation

Rust's Error type includes a source method which allows library authors to compose errors on top of one another in a way that indicates how errors originated down to the OS level. Surfacing this information to the users will allow them to determine why errors are occurring with more detail.

Solution

Walking the source chain until there are no errors left and storing this at a new field called {fieldname}.chain allows operators to determine the causal chain of complex errors.

This is a change in how error values are recorded in the opentelemetry adapter.
For a given field `x` that contains an error type, record an additional dynamic
field `x.chain` that contains an array of all errors in the source chain. This
allows users to determine where a high-level error originated.
Copy link
Member

@hawkw hawkw left a comment

Choose a reason for hiding this comment

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

overall, this looks good to me. i had a couple minor suggestions.

tracing-opentelemetry/src/layer.rs Outdated Show resolved Hide resolved
tracing-opentelemetry/src/layer.rs Outdated Show resolved Hide resolved
@hawkw
Copy link
Member

hawkw commented May 20, 2022

another thought: looking at the OpenTelemetry documentation, i noticed that opentelemetry also has semantic conventions for recording "exceptions". it seems like we could additionally implement these conventions (at least partially) when an Error is recorded, which might be worth doing in a follow-up PR...

@hawkw
Copy link
Member

hawkw commented May 20, 2022

BREAKING CHANGE: previously error values were recorded using their Debug representation. They are now reported with their Display implementation. This is more in line with current best practices for error handling, but we may want this to be an opt-in behavior.

IMO, this probably shouldn't be considered "breaking", we generally only consider changes that break compilation or cause incompatible changes in an output format to be breaking. Since I wouldn't expect any OpenTelemetry collector to be performing structured parsing of an error's Debug format, I would probably not consider this breaking, since the error field is still emitted and it's still a String. But, I might ask @jtescher for a second opinion...

lilymara-onesignal and others added 2 commits May 20, 2022 12:03
Co-authored-by: Eliza Weisman <eliza@buoyant.io>
Co-authored-by: Eliza Weisman <eliza@buoyant.io>
@lilymara-onesignal
Copy link
Contributor Author

I could add the top-level exception related fields if that's desired in this PR, I hesitated to do that initially bc I wasn't sure if we were okay adding fields in that way.

@hawkw
Copy link
Member

hawkw commented May 20, 2022

I could add the top-level exception related fields if that's desired in this PR, I hesitated to do that initially bc I wasn't sure if we were okay adding fields in that way.

let's go ahead and merge this PR now, and look into supporting the exception conventions in a follow-up.

@hawkw hawkw enabled auto-merge (squash) May 20, 2022 19:57
@hawkw hawkw merged commit c744b2f into tokio-rs:v0.1.x May 20, 2022
@lilymara-onesignal lilymara-onesignal deleted the error-source branch May 20, 2022 20:03
hawkw added a commit that referenced this pull request Jun 7, 2022
# 0.17.3 (June 7, 2022)

This release adds support for emitting thread names and IDs to
OpenTelemetry, as well as recording `std::error::Error` values in a
structured manner with their source chain included. Additionally, this
release fixes issues related to event and span source code locations.

### Added

- `Layer::with_threads` to enable recording thread names/IDs according
  to [OpenTelemetry semantic conventions][thread-semconv] ([#2134])
- `Error::source` chain when recording `std::error::Error` values
  ([#2122])
- `Layer::with_location` method (replaces `Layer::with_event_location`)
  ([#2124])

### Changed

- `std::error::Error` values are now recorded using `fmt::Display`
  rather than `fmt::Debug` ([#2122])

### Fixed

- Fixed event source code locations overwriting the parent span's source
  location ([#2099])
- Fixed `Layer::with_event_location` not controlling whether locations
  are emitted for spans as well as events ([#2124])

### Deprecated

- `Layer::with_event_location`: renamed to `Layer::with_location`, as it
  now controls both span and event locations ([#2124])

[thread-semconv]: https://opentelemetry.io/docs/reference/specification/trace/semantic_conventions/span-general/#source-code-attributes
[#2134]: #2134
[#2122]: #2122
[#2124]: #2124
[#2099]: #2099
hawkw added a commit that referenced this pull request Jun 7, 2022
# 0.17.3 (June 7, 2022)

This release adds support for emitting thread names and IDs to
OpenTelemetry, as well as recording `std::error::Error` values in a
structured manner with their source chain included. Additionally, this
release fixes issues related to event and span source code locations.

### Added

- `Layer::with_threads` to enable recording thread names/IDs according
  to [OpenTelemetry semantic conventions][thread-semconv] ([#2134])
- `Error::source` chain when recording `std::error::Error` values
  ([#2122])
- `Layer::with_location` method (replaces `Layer::with_event_location`)
  ([#2124])

### Changed

- `std::error::Error` values are now recorded using `fmt::Display`
  rather than `fmt::Debug` ([#2122])

### Fixed

- Fixed event source code locations overwriting the parent span's source
  location ([#2099])
- Fixed `Layer::with_event_location` not controlling whether locations
  are emitted for spans as well as events ([#2124])

### Deprecated

- `Layer::with_event_location`: renamed to `Layer::with_location`, as it
  now controls both span and event locations ([#2124])

Thanks to new contributors @lilymara-onesignal, @hubertbudzynski, 
and @DevinCarr for contributing to this release!

[thread-semconv]: https://opentelemetry.io/docs/reference/specification/trace/semantic_conventions/span-general/#source-code-attributes
[#2134]: #2134
[#2122]: #2122
[#2124]: #2124
[#2099]: #2099
davidbarsky pushed a commit to tokio-rs/tracing-opentelemetry that referenced this pull request Mar 21, 2023
# 0.17.3 (June 7, 2022)

This release adds support for emitting thread names and IDs to
OpenTelemetry, as well as recording `std::error::Error` values in a
structured manner with their source chain included. Additionally, this
release fixes issues related to event and span source code locations.

### Added

- `Layer::with_threads` to enable recording thread names/IDs according
  to [OpenTelemetry semantic conventions][thread-semconv] ([#2134])
- `Error::source` chain when recording `std::error::Error` values
  ([#2122])
- `Layer::with_location` method (replaces `Layer::with_event_location`)
  ([#2124])

### Changed

- `std::error::Error` values are now recorded using `fmt::Display`
  rather than `fmt::Debug` ([#2122])

### Fixed

- Fixed event source code locations overwriting the parent span's source
  location ([#2099])
- Fixed `Layer::with_event_location` not controlling whether locations
  are emitted for spans as well as events ([#2124])

### Deprecated

- `Layer::with_event_location`: renamed to `Layer::with_location`, as it
  now controls both span and event locations ([#2124])

Thanks to new contributors @lilymara-onesignal, @hubertbudzynski, 
and @DevinCarr for contributing to this release!

[thread-semconv]: https://opentelemetry.io/docs/reference/specification/trace/semantic_conventions/span-general/#source-code-attributes
[#2134]: tokio-rs/tracing#2134
[#2122]: tokio-rs/tracing#2122
[#2124]: tokio-rs/tracing#2124
[#2099]: tokio-rs/tracing#2099
kaffarell pushed a commit to kaffarell/tracing that referenced this pull request May 22, 2024
Previously error values were recorded using their `Debug`
representation. They are now reported with their `Display`
implementation. This is more in line with current best practices for
error handling.

This is a change in how error values are recorded in the opentelemetry
adapter. For a given field `x` that contains an error type, record an
additional dynamic field `x.chain` that contains an array of all errors
in the source chain. This allows users to determine where a high-level
error originated.

## Motivation

Rust's `Error` type includes a `source` method which allows library
authors to compose errors on top of one another in a way that indicates
how errors originated down to the OS level. Surfacing this information
to the users will allow them to determine why errors are occurring with
more detail.

## Solution

Walking the `source` chain until there are no errors left and storing
this at a new field called `{fieldname}.chain` allows operators to
determine the causal chain of complex errors.
kaffarell pushed a commit to kaffarell/tracing that referenced this pull request May 22, 2024
# 0.17.3 (June 7, 2022)

This release adds support for emitting thread names and IDs to
OpenTelemetry, as well as recording `std::error::Error` values in a
structured manner with their source chain included. Additionally, this
release fixes issues related to event and span source code locations.

### Added

- `Layer::with_threads` to enable recording thread names/IDs according
  to [OpenTelemetry semantic conventions][thread-semconv] ([tokio-rs#2134])
- `Error::source` chain when recording `std::error::Error` values
  ([tokio-rs#2122])
- `Layer::with_location` method (replaces `Layer::with_event_location`)
  ([tokio-rs#2124])

### Changed

- `std::error::Error` values are now recorded using `fmt::Display`
  rather than `fmt::Debug` ([tokio-rs#2122])

### Fixed

- Fixed event source code locations overwriting the parent span's source
  location ([tokio-rs#2099])
- Fixed `Layer::with_event_location` not controlling whether locations
  are emitted for spans as well as events ([tokio-rs#2124])

### Deprecated

- `Layer::with_event_location`: renamed to `Layer::with_location`, as it
  now controls both span and event locations ([tokio-rs#2124])

Thanks to new contributors @lilymara-onesignal, @hubertbudzynski, 
and @DevinCarr for contributing to this release!

[thread-semconv]: https://opentelemetry.io/docs/reference/specification/trace/semantic_conventions/span-general/#source-code-attributes
[tokio-rs#2134]: tokio-rs#2134
[tokio-rs#2122]: tokio-rs#2122
[tokio-rs#2124]: tokio-rs#2124
[tokio-rs#2099]: tokio-rs#2099
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.

2 participants