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

Remove experimental parts of tracing-error #547

Merged
merged 12 commits into from
Jan 29, 2020

Conversation

yaahc
Copy link
Collaborator

@yaahc yaahc commented Jan 27, 2020

Motivation

Much of the way that tracing Contexts should be integrated with errors is currently up for debate but the usage of Backtrace like object for printing spans in errors has been uncontested for a while. This change removes the error integration temporarily in favor of merging a std::backtrace::Backtrace-like API for capturing contexts from spans.

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.

this is a great start. i left some notes. hopefully they're helpful!

tracing-error/src/lib.rs Outdated Show resolved Hide resolved
tracing-error/src/backtrace.rs Outdated Show resolved Hide resolved
tracing-error/src/backtrace.rs Show resolved Hide resolved
tracing-error/src/backtrace.rs Show resolved Hide resolved
tracing-error/src/backtrace.rs Show resolved Hide resolved
tracing-error/src/backtrace.rs Show resolved Hide resolved
tracing-error/src/backtrace.rs Show resolved Hide resolved
tracing-error/src/lib.rs Show resolved Hide resolved
khionu and others added 9 commits January 27, 2020 17:32
Fixes a small typo that inverts the intended meaning.
…-rs#549)

Currently, a `Layered` struct's implementations of `Subscriber` and
`Layer` will successfully downcast to either the the `Layer` type or the
inner type, but *not* to `Layered<Layer, Inner>`. This means that when a
`Layer` tries to downcast the wrapped subscriber to a known type, and it
is a `Layered` (so, any time more than one layer wraps a subscriber and
a layer other than the first one tries to downcast to the inner type it
wraps), the downcast will fail incorrectly.

This commit fixes the issue by checking if the downcast target `TypeId`
equals the `Layered` type's `TypeId`, _before_ trying to downcast to the
layer itself or to the inner type.

Signed-off-by: Eliza Weisman <[email protected]>
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.

This looks great! I commented on a couple minor nits.

I think once this merges, we just need to add some docs (& maybe a few tests), and we can probably release an 0.0.1 (or even 0.1) of a tracing-error crate with just the SpanTrace type. I'd like to eventually include a strategy for attaching SpanTraces to std::error::Errors, but if you'd rather experiment with that out of tree, I can deal with that :)

examples/examples/error.rs Outdated Show resolved Hide resolved
examples/examples/error.rs Show resolved Hide resolved
tracing-error/src/backtrace.rs Show resolved Hide resolved
tracing-error/src/backtrace.rs Outdated Show resolved Hide resolved
tracing-error/src/layer.rs Show resolved Hide resolved
tracing-subscriber/src/fmt/fmt_layer.rs Show resolved Hide resolved
@hawkw
Copy link
Member

hawkw commented Jan 28, 2020

@yaahc it looks like clippy is upset about an unused import: https://github.com/tokio-rs/tracing/pull/547/checks?check_run_id=413925052#step:4:566 would you mind fixing this?

@hawkw hawkw merged commit 44fae33 into tokio-rs:eliza/error-sketch Jan 29, 2020
@hawkw
Copy link
Member

hawkw commented Jan 29, 2020

cool, merged it

hawkw added a commit that referenced this pull request Feb 1, 2020
* readme: quick typo fix (#546)

Fixes a small typo that inverts the intended meaning.

* Remove experimental parts of tracing-error

* Rename span backtrace to SpanTrace

* Swap bodies of Display and Debug

* Make try-like macro internal only

* Break eliza's code some more

* Broken presents for eliza

* subscriber: fix `Layered` layers not downcasting to themselves (#549)

Currently, a `Layered` struct's implementations of `Subscriber` and
`Layer` will successfully downcast to either the the `Layer` type or the
inner type, but *not* to `Layered<Layer, Inner>`. This means that when a
`Layer` tries to downcast the wrapped subscriber to a known type, and it
is a `Layered` (so, any time more than one layer wraps a subscriber and
a layer other than the first one tries to downcast to the inner type it
wraps), the downcast will fail incorrectly.

This commit fixes the issue by checking if the downcast target `TypeId`
equals the `Layered` type's `TypeId`, _before_ trying to downcast to the
layer itself or to the inner type.

Signed-off-by: Eliza Weisman <[email protected]>

* Fix unused result

* Fix last clippy warn

* Minor tweeks based on eliza's suggestions

* all hail our lord and savior clippy

Co-authored-by: Khionu Sybiern <[email protected]>
Co-authored-by: Eliza Weisman <[email protected]>
hawkw added a commit that referenced this pull request Feb 2, 2020
* readme: quick typo fix (#546)

Fixes a small typo that inverts the intended meaning.

* Remove experimental parts of tracing-error

* Rename span backtrace to SpanTrace

* Swap bodies of Display and Debug

* Make try-like macro internal only

* Break eliza's code some more

* Broken presents for eliza

* subscriber: fix `Layered` layers not downcasting to themselves (#549)

Currently, a `Layered` struct's implementations of `Subscriber` and
`Layer` will successfully downcast to either the the `Layer` type or the
inner type, but *not* to `Layered<Layer, Inner>`. This means that when a
`Layer` tries to downcast the wrapped subscriber to a known type, and it
is a `Layered` (so, any time more than one layer wraps a subscriber and
a layer other than the first one tries to downcast to the inner type it
wraps), the downcast will fail incorrectly.

This commit fixes the issue by checking if the downcast target `TypeId`
equals the `Layered` type's `TypeId`, _before_ trying to downcast to the
layer itself or to the inner type.

Signed-off-by: Eliza Weisman <[email protected]>

* Fix unused result

* Fix last clippy warn

* Minor tweeks based on eliza's suggestions

* all hail our lord and savior clippy

Co-authored-by: Khionu Sybiern <[email protected]>
Co-authored-by: Eliza Weisman <[email protected]>
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.

3 participants