Skip to content

Commit

Permalink
subscriber: fix missing on_layer for Box and Arc layer impls
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
hawkw committed Sep 17, 2021
1 parent 251ecfa commit 211b9b0
Showing 1 changed file with 31 additions and 1 deletion.
32 changes: 31 additions & 1 deletion tracing-subscriber/src/layer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -414,7 +414,11 @@
//! [`LevelFilter`]: crate::filter::LevelFilter
//! [feat]: crate#feature-flags
use crate::filter;
use std::{any::TypeId, ops::Deref, sync::Arc};
use std::{
any::TypeId,
ops::{Deref, DerefMut},
sync::Arc,
};
use tracing_core::{
metadata::Metadata,
span,
Expand Down Expand Up @@ -1160,13 +1164,31 @@ where
L: Layer<S>,
S: Subscriber,
{
fn on_layer(&mut self, subscriber: &mut S) {
if let Some(inner) = Arc::get_mut(self) {
// XXX(eliza): this may behave weird if another `Arc` clone of this
// layer is layered onto a _different_ subscriber...but there's no
// good solution for that...
inner.on_layer(subscriber);
}
}

layer_impl_body! {}
}

impl<S> Layer<S> for Arc<dyn Layer<S> + Send + Sync>
where
S: Subscriber,
{
fn on_layer(&mut self, subscriber: &mut S) {
if let Some(inner) = Arc::get_mut(self) {
// XXX(eliza): this may behave weird if another `Arc` clone of this
// layer is layered onto a _different_ subscriber...but there's no
// good solution for that...
inner.on_layer(subscriber);
}
}

layer_impl_body! {}
}

Expand All @@ -1175,13 +1197,21 @@ where
L: Layer<S>,
S: Subscriber,
{
fn on_layer(&mut self, subscriber: &mut S) {
self.deref_mut().on_layer(subscriber);
}

layer_impl_body! {}
}

impl<S> Layer<S> for Box<dyn Layer<S> + Send + Sync>
where
S: Subscriber,
{
fn on_layer(&mut self, subscriber: &mut S) {
self.deref_mut().on_layer(subscriber);
}

layer_impl_body! {}
}

Expand Down

0 comments on commit 211b9b0

Please sign in to comment.