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.

Sign up for GitHub

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

Jump to bottom

0.1: add Layer implementations for various containers #1536

Merged
merged 2 commits into from
Sep 4, 2021
Merged

0.1: add Layer implementations for various containers #1536

merged 2 commits into from
Sep 4, 2021

Conversation

jsgf
Copy link
Contributor

@jsgf jsgf commented Sep 3, 2021 •
edited
Loading

Add Layer<S> impls for:

  • Box<L>
  • Box<dyn Layer<S>>
  • Arc<L>
  • Arc<dyn Layer<S>>

where L: Layer<S>

@jsgf
0.1: add Layer implementations for various containers
9862254 
Add Layer<S> impls for:
- Box<L>
- Box<dyn Layer<S>>
- Arc<L>
- Arc<dyn Layer<S>>

where L: Layer<S>
@jsgf jsgf requested review from hawkw and a team as code owners September 3, 2021 19:38
@jsgf
Copy link
Contributor Author

jsgf commented Sep 3, 2021 •
edited
Loading

Obv this could be mechanised with something like auto_impl but I didn't want to add a dependency. And I also considered a macro, since the only difference for each type is in the impl<S> Layer<S> for ... line.

... Hm, and the type params and where constraints.

davidbarsky
davidbarsky approved these changes Sep 3, 2021
View reviewed changes
Copy link
Member

@davidbarsky davidbarsky left a comment

Choose a reason for hiding this comment

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

I think this looks good to me. I'm personally (slightly) against adding auto_impl as a dependency for two reasons:

  • some consumers of tracing-subscriber are sensitive to macro dependencies, and
  • this isn't that much boilerplate anyways.

@davidbarsky
Copy link
Member

(i'm not gonna merge, I still want Eliza to review this.)

@jsgf
Copy link
Contributor Author

jsgf commented Sep 3, 2021

I think a plain macro is a good tradeoff.

hawkw
hawkw approved these changes Sep 4, 2021
View reviewed changes
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.

looks good to me --- the macro seems like the right approach!

@hawkw hawkw merged commit f21028f into tokio-rs:v0.1.x Sep 4, 2021
@jsgf jsgf mentioned this pull request Sep 10, 2021
Merged
hawkw pushed a commit that referenced this pull request Sep 10, 2021
@jsgf
subscriber: make Layer impls for Box<dyn Layer<..>> Send+Sync (#…
6ac5215 
…1547)

And for Arc

This is a fixup for #1536.
hawkw added a commit that referenced this pull request Sep 12, 2021
@hawkw
subscriber: prepare to release 0.2.21
c87fbe5 
# 0.2.21 (September 12, 2021)

This release introduces the [`Filter`] trait, a new API for [per-layer
filtering][plf]. This allows controlling which spans and events are
recorded by various layers individually, rather than globally.

In addition, it adds a new [`Targets`] filter, which provides a
lighter-weight version of the filtering provided by [`EnvFilter`], as
well as other smaller API improvements and fixes.

### Deprecated

- **registry**: `SpanRef::parent_id`, which cannot properly support
  per-layer filtering. Use `.parent().map(SpanRef::id)` instead.
  ([#1523])

### Fixed

- **layer** `Context` methods that are provided when the `Subscriber`
  implements `LookupSpan` no longer require the "registry" feature flag
  ([#1525])
- **layer** `fmt::Debug` implementation for `Layered` no longer requires
  the `S` type parameter to implement `Debug` ([#1528])

### Added

- **registry**: `Filter` trait, `Filtered` type, `Layer::with_filter`
  method, and other APIs for per-layer filtering ([#1523])
- **filter**: `FilterFn` and `DynFilterFn` types that implement global
  (`Layer`) and per-layer (`Filter`) filtering for closures and function
  pointers ([#1523])
- **filter**: `Targets` filter, which implements a lighter-weight form
  of `EnvFilter`-like filtering ([#1550])
- **env-filter**: Added support for filtering on floating-point values
  ([#1507])
- **layer**: `Layer::on_layer` callback, called when layering the
  `Layer` onto a `Subscriber` ([#1523])
- **layer**: `Layer` implementations for `Box<L>` and `Arc<L>` where `L:
  Layer` ([#1536])
- **layer**: `Layer` implementations for `Box<dyn Layer<S> + Send + Sync
  + 'static>` and `Arc<dyn Layer<S> + Send + Sync + 'static>` ([#1536])
- A number of small documentation fixes and improvements ([#1553],
  [#1544], [#1539], [#1524])

Special thanks to new contributors @jsgf and @maxburke for contributing
to this release!

[`Filter`]: https://docs.rs/tracing-subscriber/0.2.21/tracing_subscriber/layer/trait.Filter.html
[`plf`]: https://docs.rs/tracing-subscriber/0.2.21/tracing_subscriber/layer/index.html#per-layer-filtering
[`Targets`]: https://docs.rs/tracing-subscriber/0.2.21/tracing_subscriber/filter/struct.Targets.html
[`EnvFilter`]: https://docs.rs/tracing-subscriber/0.2.21/tracing_subscriber/filter/struct.EnvFilter.html
[#1507]: #1507
[#1523]: #1523
[#1524]: #1524
[#1525]: #1525
[#1528]: #1528
[#1539]: #1539
[#1544]: #1544
[#1550]: #1550
[#1553]: #1553
@hawkw hawkw mentioned this pull request Sep 12, 2021
Merged
hawkw added a commit that referenced this pull request Sep 12, 2021
@hawkw
subscriber: prepare to release 0.2.21
e8dc5e5 
# 0.2.21 (September 12, 2021)

This release introduces the [`Filter`] trait, a new API for [per-layer
filtering][plf]. This allows controlling which spans and events are
recorded by various layers individually, rather than globally.

In addition, it adds a new [`Targets`] filter, which provides a
lighter-weight version of the filtering provided by [`EnvFilter`], as
well as other smaller API improvements and fixes.

### Deprecated

- **registry**: `SpanRef::parent_id`, which cannot properly support
  per-layer filtering. Use `.parent().map(SpanRef::id)` instead.
  ([#1523])

### Fixed

- **layer** `Context` methods that are provided when the `Subscriber`
  implements `LookupSpan` no longer require the "registry" feature flag
  ([#1525])
- **layer** `fmt::Debug` implementation for `Layered` no longer requires
  the `S` type parameter to implement `Debug` ([#1528])

### Added

- **registry**: `Filter` trait, `Filtered` type, `Layer::with_filter`
  method, and other APIs for per-layer filtering ([#1523])
- **filter**: `FilterFn` and `DynFilterFn` types that implement global
  (`Layer`) and per-layer (`Filter`) filtering for closures and function
  pointers ([#1523])
- **filter**: `Targets` filter, which implements a lighter-weight form
  of `EnvFilter`-like filtering ([#1550])
- **env-filter**: Added support for filtering on floating-point values
  ([#1507])
- **layer**: `Layer::on_layer` callback, called when layering the
  `Layer` onto a `Subscriber` ([#1523])
- **layer**: `Layer` implementations for `Box<L>` and `Arc<L>` where `L:
  Layer` ([#1536])
- **layer**: `Layer` implementations for `Box<dyn Layer<S> + Send + Sync
  + 'static>` and `Arc<dyn Layer<S> + Send + Sync + 'static>` ([#1536])
- A number of small documentation fixes and improvements ([#1553],
  [#1544], [#1539], [#1524])

Special thanks to new contributors @jsgf and @maxburke for contributing
to this release!

[`Filter`]: https://docs.rs/tracing-subscriber/0.2.21/tracing_subscriber/layer/trait.Filter.html
[`plf`]: https://docs.rs/tracing-subscriber/0.2.21/tracing_subscriber/layer/index.html#per-layer-filtering
[`Targets`]: https://docs.rs/tracing-subscriber/0.2.21/tracing_subscriber/filter/struct.Targets.html
[`EnvFilter`]: https://docs.rs/tracing-subscriber/0.2.21/tracing_subscriber/filter/struct.EnvFilter.html
[#1507]: #1507
[#1523]: #1523
[#1524]: #1524
[#1525]: #1525
[#1528]: #1528
[#1539]: #1539
[#1544]: #1544
[#1550]: #1550
[#1553]: #1553
hawkw added a commit that referenced this pull request Sep 12, 2021
@hawkw
subscriber: prepare to release v0.2.21 (#1555)
97b1486 
# 0.2.21 (September 12, 2021)

This release introduces the [`Filter`] trait, a new API for [per-layer
filtering][plf]. This allows controlling which spans and events are
recorded by various layers individually, rather than globally.

In addition, it adds a new [`Targets`] filter, which provides a
lighter-weight version of the filtering provided by [`EnvFilter`], as
well as other smaller API improvements and fixes.

### Deprecated

- **registry**: `SpanRef::parent_id`, which cannot properly support
  per-layer filtering. Use `.parent().map(SpanRef::id)` instead.
  ([#1523])

### Fixed

- **layer** `Context` methods that are provided when the `Subscriber`
  implements `LookupSpan` no longer require the "registry" feature flag
  ([#1525])
- **layer** `fmt::Debug` implementation for `Layered` no longer requires
  the `S` type parameter to implement `Debug` ([#1528])

### Added

- **registry**: `Filter` trait, `Filtered` type, `Layer::with_filter`
  method, and other APIs for per-layer filtering ([#1523])
- **filter**: `FilterFn` and `DynFilterFn` types that implement global
  (`Layer`) and per-layer (`Filter`) filtering for closures and function
  pointers ([#1523])
- **filter**: `Targets` filter, which implements a lighter-weight form
  of `EnvFilter`-like filtering ([#1550])
- **env-filter**: Added support for filtering on floating-point values
  ([#1507])
- **layer**: `Layer::on_layer` callback, called when layering the
  `Layer` onto a `Subscriber` ([#1523])
- **layer**: `Layer` implementations for `Box<L>` and `Arc<L>` where `L:
  Layer` ([#1536])
- **layer**: `Layer` implementations for  `Box<dyn Layer>` and 
  `Arc<dyn Layer>` ([#1536])
- A number of small documentation fixes and improvements ([#1553],
  [#1544], [#1539], [#1524])

Special thanks to new contributors @jsgf and @maxburke for contributing
to this release!

[`Filter`]: https://docs.rs/tracing-subscriber/0.2.21/tracing_subscriber/layer/trait.Filter.html
[plf]: https://docs.rs/tracing-subscriber/0.2.21/tracing_subscriber/layer/index.html#per-layer-filtering
[`Targets`]: https://docs.rs/tracing-subscriber/0.2.21/tracing_subscriber/filter/struct.Targets.html
[`EnvFilter`]: https://docs.rs/tracing-subscriber/0.2.21/tracing_subscriber/filter/struct.EnvFilter.html
[#1507]: #1507
[#1523]: #1523
[#1524]: #1524
[#1525]: #1525
[#1528]: #1528
[#1539]: #1539
[#1544]: #1544
[#1550]: #1550
[#1553]: #1553
hawkw added a commit that referenced this pull request Sep 12, 2021
@hawkw
subscriber: prepare to release v0.2.21 (#1555)
47f1cd2 
# 0.2.21 (September 12, 2021)

This release introduces the [`Filter`] trait, a new API for [per-layer
filtering][plf]. This allows controlling which spans and events are
recorded by various layers individually, rather than globally.

In addition, it adds a new [`Targets`] filter, which provides a
lighter-weight version of the filtering provided by [`EnvFilter`], as
well as other smaller API improvements and fixes.

### Deprecated

- **registry**: `SpanRef::parent_id`, which cannot properly support
  per-layer filtering. Use `.parent().map(SpanRef::id)` instead.
  ([#1523])

### Fixed

- **layer** `Context` methods that are provided when the `Subscriber`
  implements `LookupSpan` no longer require the "registry" feature flag
  ([#1525])
- **layer** `fmt::Debug` implementation for `Layered` no longer requires
  the `S` type parameter to implement `Debug` ([#1528])

### Added

- **registry**: `Filter` trait, `Filtered` type, `Layer::with_filter`
  method, and other APIs for per-layer filtering ([#1523])
- **filter**: `FilterFn` and `DynFilterFn` types that implement global
  (`Layer`) and per-layer (`Filter`) filtering for closures and function
  pointers ([#1523])
- **filter**: `Targets` filter, which implements a lighter-weight form
  of `EnvFilter`-like filtering ([#1550])
- **env-filter**: Added support for filtering on floating-point values
  ([#1507])
- **layer**: `Layer::on_layer` callback, called when layering the
  `Layer` onto a `Subscriber` ([#1523])
- **layer**: `Layer` implementations for `Box<L>` and `Arc<L>` where `L:
  Layer` ([#1536])
- **layer**: `Layer` implementations for  `Box<dyn Layer>` and
  `Arc<dyn Layer>` ([#1536])
- A number of small documentation fixes and improvements ([#1553],
  [#1544], [#1539], [#1524])

Special thanks to new contributors @jsgf and @maxburke for contributing
to this release!

[`Filter`]: https://docs.rs/tracing-subscriber/0.2.21/tracing_subscriber/layer/trait.Filter.html
[plf]: https://docs.rs/tracing-subscriber/0.2.21/tracing_subscriber/layer/index.html#per-layer-filtering
[`Targets`]: https://docs.rs/tracing-subscriber/0.2.21/tracing_subscriber/filter/struct.Targets.html
[`EnvFilter`]: https://docs.rs/tracing-subscriber/0.2.21/tracing_subscriber/filter/struct.EnvFilter.html
[#1507]: #1507
[#1523]: #1523
[#1524]: #1524
[#1525]: #1525
[#1528]: #1528
[#1539]: #1539
[#1544]: #1544
[#1550]: #1550
[#1553]: #1553
davidbarsky added a commit that referenced this pull request Sep 13, 2021
@davidbarsky
subscriber: add Layer implementations for various containers (#1536)
f6ec3d4 
Add `Layer<S>` impls for:
- `Box<L>`
- `Box<dyn Layer<S>>`
- `Arc<L>`
- `Arc<dyn Layer<S>>`
 where `L: Layer<S>`
davidbarsky added a commit that referenced this pull request Sep 13, 2021
@davidbarsky
subscriber: make Layer impls for Box<dyn Layer<..>> Send+Sync (#…
92ca4a1 
…1547)

And for Arc

 This is a fixup for #1536.
@davidbarsky davidbarsky mentioned this pull request Sep 13, 2021
Merged
hawkw pushed a commit that referenced this pull request Sep 15, 2021
@davidbarsky @hawkw
subscriber: add Layer implementations for various containers (#1536)
c2f9cee 
Add `Layer<S>` impls for:
- `Box<L>`
- `Box<dyn Layer<S>>`
- `Arc<L>`
- `Arc<dyn Layer<S>>`
 where `L: Layer<S>`
hawkw pushed a commit that referenced this pull request Sep 15, 2021
@davidbarsky @hawkw
subscriber: make Layer impls for Box<dyn Layer<..>> Send+Sync (#…
50a3340 
…1547)

And for Arc

 This is a fixup for #1536.
facebook-github-bot pushed a commit to facebook/fb303 that referenced this pull request Sep 15, 2021
@jsgf @facebook-github-bot
third-party/rust: bump all the tracing packages
f652015 
Summary:
This includes recent changes:
- `skip_all` in #[instrument] tokio-rs/tracing#1548
- NoSubscriber no-op Subscriber tokio-rs/tracing#1549
- Box/Arc Layer (tokio-rs/tracing#1536 +
  tokio-rs/tracing#1547)
- log filtering fixes tokio-rs/tracing#1543

Local: force `log` and `log-always` features off. This removes the
`enable-trace-logging` flag from smartplatform/reconciler.

Reviewed By: guswynn

Differential Revision: D30906032

fbshipit-source-id: fdc3e4e29a39e9e181d9f4ff017e6b3e16acdcd8
facebook-github-bot pushed a commit to facebook/fbthrift that referenced this pull request Sep 15, 2021
@jsgf @facebook-github-bot
third-party/rust: bump all the tracing packages
4a5d933 
Summary:
This includes recent changes:
- `skip_all` in #[instrument] tokio-rs/tracing#1548
- NoSubscriber no-op Subscriber tokio-rs/tracing#1549
- Box/Arc Layer (tokio-rs/tracing#1536 +
  tokio-rs/tracing#1547)
- log filtering fixes tokio-rs/tracing#1543

Local: force `log` and `log-always` features off. This removes the
`enable-trace-logging` flag from smartplatform/reconciler.

Reviewed By: guswynn

Differential Revision: D30906032

fbshipit-source-id: fdc3e4e29a39e9e181d9f4ff017e6b3e16acdcd8
facebook-github-bot pushed a commit to facebookexperimental/rust-shed that referenced this pull request Sep 15, 2021
@jsgf @facebook-github-bot
third-party/rust: bump all the tracing packages
2526d47 
Summary:
This includes recent changes:
- `skip_all` in #[instrument] tokio-rs/tracing#1548
- NoSubscriber no-op Subscriber tokio-rs/tracing#1549
- Box/Arc Layer (tokio-rs/tracing#1536 +
  tokio-rs/tracing#1547)
- log filtering fixes tokio-rs/tracing#1543

Local: force `log` and `log-always` features off. This removes the
`enable-trace-logging` flag from smartplatform/reconciler.

Reviewed By: guswynn

Differential Revision: D30906032

fbshipit-source-id: fdc3e4e29a39e9e181d9f4ff017e6b3e16acdcd8
facebook-github-bot pushed a commit to facebookincubator/antlir that referenced this pull request Sep 15, 2021
@jsgf @facebook-github-bot
third-party/rust: bump all the tracing packages
23221b2 
Summary:
This includes recent changes:
- `skip_all` in #[instrument] tokio-rs/tracing#1548
- NoSubscriber no-op Subscriber tokio-rs/tracing#1549
- Box/Arc Layer (tokio-rs/tracing#1536 +
  tokio-rs/tracing#1547)
- log filtering fixes tokio-rs/tracing#1543

Local: force `log` and `log-always` features off. This removes the
`enable-trace-logging` flag from smartplatform/reconciler.

Reviewed By: guswynn

Differential Revision: D30906032

fbshipit-source-id: fdc3e4e29a39e9e181d9f4ff017e6b3e16acdcd8
facebook-github-bot pushed a commit to facebook/sapling that referenced this pull request Sep 15, 2021
@jsgf @facebook-github-bot
third-party/rust: bump all the tracing packages
113060b 
Summary:
This includes recent changes:
- `skip_all` in #[instrument] tokio-rs/tracing#1548
- NoSubscriber no-op Subscriber tokio-rs/tracing#1549
- Box/Arc Layer (tokio-rs/tracing#1536 +
  tokio-rs/tracing#1547)
- log filtering fixes tokio-rs/tracing#1543

Local: force `log` and `log-always` features off. This removes the
`enable-trace-logging` flag from smartplatform/reconciler.

Reviewed By: guswynn

Differential Revision: D30906032

fbshipit-source-id: fdc3e4e29a39e9e181d9f4ff017e6b3e16acdcd8
hawkw added a commit that referenced this pull request Sep 17, 2021
@hawkw
subscriber: fix missing on_layer for Box and Arc layer impls
211b9b0 
The `Layer::on_layer` method on `Layer` was added in PR #1523. PR #1536,
which added `Layer` implementations to `Box<dyn Layer<...> + ...>` and
`Arc<dyn Layer<...> + ...>`, merged prior to #1523. However, when
merging #1523, I didn't think to update the `Layer` impl for `Box`ed and
`Arc`ed layers to forward `on_layer` to the inner `Layer`. This means
that when a `Layer` is wrapped in an `Arc` or a `Box`, the `on_layer`
method never gets called.

When per-layer filters are in use, the `on_layer` method is necessary to
ensure the filter is registered with the inner subscriber and has a
valid ID. This bug means that when per-layer filters are wrapped in a
`Box` or `Arc`, they won't be registered, and per-layer filtering
breaks.

This PR fixes the bug by adding `on_layer` implementations to the
`Layer` impls for `Arc`ed and `Box`ed layers. I also added some tests
--- thanks to @Noah-Kennedy for the original repro that these were based
on (#1563 (comment)).

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
@hawkw hawkw mentioned this pull request Sep 17, 2021
Merged
hawkw added a commit that referenced this pull request Sep 17, 2021
@hawkw
subscriber: fix missing on_layer for Box/Arc layer impls (#1576)
e597953 
The `Layer::on_layer` method on `Layer` was added in PR #1523. PR #1536,
which added `Layer` implementations to `Box<dyn Layer<...> + ...>` and
`Arc<dyn Layer<...> + ...>`, merged prior to #1523. However, when
merging #1523, I didn't think to update the `Layer` impl for `Box`ed and
`Arc`ed layers to forward `on_layer` to the inner `Layer`. This means
that when a `Layer` is wrapped in an `Arc` or a `Box`, the `on_layer`
method never gets called.

When per-layer filters are in use, the `on_layer` method is necessary to
ensure the filter is registered with the inner subscriber and has a
valid ID. This bug means that when per-layer filters are wrapped in a
`Box` or `Arc`, they won't be registered, and per-layer filtering
breaks.

This PR fixes the bug by adding `on_layer` implementations to the
`Layer` impls for `Arc`ed and `Box`ed layers. I also added some tests
--- thanks to @Noah-Kennedy for the original repro that these were based
on (#1563 (comment)).

I also added a nicer debug assertion to `Filtered` for cases where
`Layer` impls don't call `on_layer`, so that this fails less confusingly
in the future.

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
vkill pushed a commit to bk-rs/fbthrift-git-rs that referenced this pull request Jan 12, 2023
@jsgf @facebook-github-bot
third-party/rust: bump all the tracing packages
45a39bc 
Summary:
This includes recent changes:
- `skip_all` in #[instrument] tokio-rs/tracing#1548
- NoSubscriber no-op Subscriber tokio-rs/tracing#1549
- Box/Arc Layer (tokio-rs/tracing#1536 +
  tokio-rs/tracing#1547)
- log filtering fixes tokio-rs/tracing#1543

Local: force `log` and `log-always` features off. This removes the
`enable-trace-logging` flag from smartplatform/reconciler.

Reviewed By: guswynn

Differential Revision: D30906032

fbshipit-source-id: fdc3e4e29a39e9e181d9f4ff017e6b3e16acdcd8
@davidbarsky davidbarsky mentioned this pull request Sep 26, 2023
Merged
kaffarell pushed a commit to kaffarell/tracing that referenced this pull request May 22, 2024
@jsgf @kaffarell
subscriber: add Layer implementations for various containers (tokio-r…
84379a0 
…s#1536)

Add `Layer<S>` impls for:
- `Box<L>`
- `Box<dyn Layer<S>>`
- `Arc<L>`
- `Arc<dyn Layer<S>>`

where `L: Layer<S>`
kaffarell pushed a commit to kaffarell/tracing that referenced this pull request May 22, 2024
@jsgf @kaffarell
subscriber: make Layer impls for Box<dyn Layer<..>> Send+Sync (t…
35a2772 
…okio-rs#1547)

And for Arc

This is a fixup for tokio-rs#1536.
kaffarell pushed a commit to kaffarell/tracing that referenced this pull request May 22, 2024
@hawkw @kaffarell
subscriber: prepare to release v0.2.21 (tokio-rs#1555)
ce74266 
# 0.2.21 (September 12, 2021)

This release introduces the [`Filter`] trait, a new API for [per-layer
filtering][plf]. This allows controlling which spans and events are
recorded by various layers individually, rather than globally.

In addition, it adds a new [`Targets`] filter, which provides a
lighter-weight version of the filtering provided by [`EnvFilter`], as
well as other smaller API improvements and fixes.

### Deprecated

- **registry**: `SpanRef::parent_id`, which cannot properly support
  per-layer filtering. Use `.parent().map(SpanRef::id)` instead.
  ([tokio-rs#1523])

### Fixed

- **layer** `Context` methods that are provided when the `Subscriber`
  implements `LookupSpan` no longer require the "registry" feature flag
  ([tokio-rs#1525])
- **layer** `fmt::Debug` implementation for `Layered` no longer requires
  the `S` type parameter to implement `Debug` ([tokio-rs#1528])

### Added

- **registry**: `Filter` trait, `Filtered` type, `Layer::with_filter`
  method, and other APIs for per-layer filtering ([tokio-rs#1523])
- **filter**: `FilterFn` and `DynFilterFn` types that implement global
  (`Layer`) and per-layer (`Filter`) filtering for closures and function
  pointers ([tokio-rs#1523])
- **filter**: `Targets` filter, which implements a lighter-weight form
  of `EnvFilter`-like filtering ([tokio-rs#1550])
- **env-filter**: Added support for filtering on floating-point values
  ([tokio-rs#1507])
- **layer**: `Layer::on_layer` callback, called when layering the
  `Layer` onto a `Subscriber` ([tokio-rs#1523])
- **layer**: `Layer` implementations for `Box<L>` and `Arc<L>` where `L:
  Layer` ([tokio-rs#1536])
- **layer**: `Layer` implementations for  `Box<dyn Layer>` and
  `Arc<dyn Layer>` ([tokio-rs#1536])
- A number of small documentation fixes and improvements ([tokio-rs#1553],
  [tokio-rs#1544], [tokio-rs#1539], [tokio-rs#1524])

Special thanks to new contributors @jsgf and @maxburke for contributing
to this release!

[`Filter`]: https://docs.rs/tracing-subscriber/0.2.21/tracing_subscriber/layer/trait.Filter.html
[plf]: https://docs.rs/tracing-subscriber/0.2.21/tracing_subscriber/layer/index.html#per-layer-filtering
[`Targets`]: https://docs.rs/tracing-subscriber/0.2.21/tracing_subscriber/filter/struct.Targets.html
[`EnvFilter`]: https://docs.rs/tracing-subscriber/0.2.21/tracing_subscriber/filter/struct.EnvFilter.html
[tokio-rs#1507]: tokio-rs#1507
[tokio-rs#1523]: tokio-rs#1523
[tokio-rs#1524]: tokio-rs#1524
[tokio-rs#1525]: tokio-rs#1525
[tokio-rs#1528]: tokio-rs#1528
[tokio-rs#1539]: tokio-rs#1539
[tokio-rs#1544]: tokio-rs#1544
[tokio-rs#1550]: tokio-rs#1550
[tokio-rs#1553]: tokio-rs#1553
kaffarell pushed a commit to kaffarell/tracing that referenced this pull request May 22, 2024
@hawkw @kaffarell
subscriber: fix missing on_layer for Box/Arc layer impls (tokio…
ebb1a0e 
…-rs#1576)

The `Layer::on_layer` method on `Layer` was added in PR tokio-rs#1523. PR tokio-rs#1536,
which added `Layer` implementations to `Box<dyn Layer<...> + ...>` and
`Arc<dyn Layer<...> + ...>`, merged prior to tokio-rs#1523. However, when
merging tokio-rs#1523, I didn't think to update the `Layer` impl for `Box`ed and
`Arc`ed layers to forward `on_layer` to the inner `Layer`. This means
that when a `Layer` is wrapped in an `Arc` or a `Box`, the `on_layer`
method never gets called.

When per-layer filters are in use, the `on_layer` method is necessary to
ensure the filter is registered with the inner subscriber and has a
valid ID. This bug means that when per-layer filters are wrapped in a
`Box` or `Arc`, they won't be registered, and per-layer filtering
breaks.

This PR fixes the bug by adding `on_layer` implementations to the
`Layer` impls for `Arc`ed and `Box`ed layers. I also added some tests
--- thanks to @Noah-Kennedy for the original repro that these were based
on (tokio-rs#1563 (comment)).

I also added a nicer debug assertion to `Filtered` for cases where
`Layer` impls don't call `on_layer`, so that this fails less confusingly
in the future.

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hawkw hawkw hawkw approved these changes

@davidbarsky davidbarsky davidbarsky approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jsgf @davidbarsky @hawkw