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

subscriber: fix Layered layers not downcasting to themselves #549

Merged
merged 1 commit into from
Jan 28, 2020

Conversation

hawkw
Copy link
Member

@hawkw hawkw commented Jan 28, 2020

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]

@hawkw hawkw added kind/bug Something isn't working crate/subscriber Related to the `tracing-subscriber` crate labels Jan 28, 2020
@hawkw hawkw requested a review from a team January 28, 2020 00:14
@hawkw hawkw self-assigned this Jan 28, 2020
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]>
@hawkw hawkw merged commit fa1f826 into master Jan 28, 2020
yaahc pushed a commit to yaahc/tracing that referenced this pull request Jan 28, 2020
…-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]>
hawkw added a commit that referenced this pull request Jan 29, 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 Jan 31, 2020
Added:

- **env_filter**: Documentation for filtering directives (#554)
- **registry**, **env_filter**: Updated `smallvec` dependency to 0.1
  (#543)

Fixed:

- **registry**: Fixed a memory leak in the slab used to store per-span
  data (3c35048)
- **Layer**: Fixed `Layered` subscribers failing to downcast to their
  own type (#549)
- **fmt**: Fixed a panic when multiple layers insert `FormattedFields`
  extensions from the same formatter type (1c3bb70)
- **fmt**: Fixed `fmt::Layer::on_record` inserting a new
  `FormattedFields` when  formatted fields for a span already exist
  (1c3bb70)

Signed-off-by: Eliza Weisman <[email protected]>
hawkw added a commit that referenced this pull request Jan 31, 2020
### Added:

- **env_filter**: Documentation for filtering directives (#554)
- **registry**, **env_filter**: Updated `smallvec` dependency to 0.1
  (#543)

### Fixed:

- **registry**: Fixed a memory leak in the slab used to store per-span
  data (3c35048)
- **Layer**: Fixed `Layered` subscribers failing to downcast to their
  own type (#549)
- **fmt**: Fixed a panic when multiple layers insert `FormattedFields`
  extensions from the same formatter type (1c3bb70)
- **fmt**: Fixed `fmt::Layer::on_record` inserting a new
  `FormattedFields` when  formatted fields for a span already exist
  (1c3bb70)

Signed-off-by: Eliza Weisman <[email protected]>
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]>
hawkw added a commit that referenced this pull request Feb 4, 2020
0.2.0 (February 4, 2020)

Breaking Changes:

- **fmt**: Renamed `Context` to `FmtContext` (#420, #425)
- **fmt**: Renamed `Builder` to `SubscriberBuilder` (#420)
- **filter**: Removed `Filter`. Use `EnvFilter` instead (#434)

Added:

- **registry**: `Registry`, a reusable span store that `Layer`s can use
  a high-performance, in-memory store. (#420, #425, #432, #433, #435)
- **registry**: Added `LookupSpan` trait, implemented by `Subscriber`s
  to expose stored span data to `Layer`s (#420)
- **fmt**: Added `fmt::Layer`, to allow composing log formatting with
  other `Layer`s (#420)
- **fmt**: Added support for JSON field and event formatting (#377,
  #415)
- **filter**: Documentation for filtering directives (#554)

Changed:

- **fmt**: Renamed `Context` to `FmtContext` (#420, #425) (BREAKING)
- **fmt**: Renamed `Builder` to `SubscriberBuilder` (#420) (BREAKING)
- **fmt**: Reimplemented `fmt::Subscriber` in terms of the `Registry`
  and `Layer`s (#420)

Removed:

- **filter**: Removed `Filter`. Use `EnvFilter` instead (#434) (BREAKING)

Fixed:

- **fmt**: Fixed memory leaks in the slab used to store per-span data
  (3c35048)
- **fmt**: `fmt::SubscriberBuilder::init` not setting up `log`
  compatibility (#489)
- **fmt**: Spans closed by a child span closing not also closing _their_
  parents (#514)
- **Layer**: Fixed `Layered` subscribers failing to downcast to their
  own type (#549)
- **Layer**: Fixed `Layer::downcast_ref` returning invalid references
  (#454)

Signed-off-by: Eliza Weisman <[email protected]>
hawkw added a commit that referenced this pull request Feb 4, 2020
# 0.2.0 (February 4, 2020)

### Breaking Changes

- **fmt**: Renamed `Context` to `FmtContext` (#420, #425)
- **fmt**: Renamed `Builder` to `SubscriberBuilder` (#420)
- **filter**: Removed `Filter`. Use `EnvFilter` instead (#434)

### Added

- **registry**: `Registry`, a `Subscriber` implementation that `Layer`s
  can use as a high-performance, in-memory span store. (#420, #425, 
  #432, #433, #435)
- **registry**: Added `LookupSpan` trait, implemented by `Subscriber`s
  to expose stored span data to `Layer`s (#420)
- **fmt**: Added `fmt::Layer`, to allow composing log formatting with
  other `Layer`s
- **fmt**: Added support for JSON field and event formatting (#377, 
  #415)
- **filter**: Documentation for filtering directives (#554)

### Changed

- **fmt**: Renamed `Context` to `FmtContext` (#420, #425) (BREAKING)
- **fmt**: Renamed `Builder` to `SubscriberBuilder` (#420) (BREAKING)
- **fmt**: Reimplemented `fmt::Subscriber` in terms of the `Registry`
  and `Layer`s (#420)

### Removed

- **filter**: Removed `Filter`. Use `EnvFilter` instead (#434) 
  (BREAKING)

### Fixed

- **fmt**: Fixed memory leaks in the slab used to store per-span data
  (3c35048)
- **fmt**: `fmt::SubscriberBuilder::init` not setting up `log` 
  compatibility (#489)
- **fmt**: Spans closed by a child span closing not also closing 
  _their_ parents (#514)
- **Layer**: Fixed `Layered` subscribers failing to downcast to their
  own type (#549)
- **Layer**: Fixed `Layer::downcast_ref` returning invalid references
  (#454)

Fixes #515
Fixes #458 

Signed-off-by: Eliza Weisman <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
crate/subscriber Related to the `tracing-subscriber` crate kind/bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants