Skip to content

Commit 6316fba

Browse files
committed
fix(subscriber): panic when reloading Filtered
This commit adds support for reloading filtered layers without breaking the existing API. The `Reload` handle now provides a `reload_filtered` function, which invokes a newly introduced `on_reload_layer` function on the `Layer`. This function is responsible for registering the filter with the subscriber via `register_filter_immut`. Unlike `register_filter`, `register_filter_immut` takes a shared reference to the subscriber, allowing the filter registration to occur after the subscriber has been fully initialized.
1 parent e63ef57 commit 6316fba

File tree

7 files changed

+250
-10
lines changed

7 files changed

+250
-10
lines changed

tracing-subscriber/src/filter/layer_filters/mod.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -731,6 +731,11 @@ where
731731
self.layer.on_layer(subscriber);
732732
}
733733

734+
fn on_reload_layer(&mut self, subscriber: &S) {
735+
self.id = MagicPlfDowncastMarker(subscriber.register_filter_immut());
736+
self.layer.on_reload_layer(subscriber);
737+
}
738+
734739
// TODO(eliza): can we figure out a nice way to make the `Filtered` layer
735740
// not call `is_enabled_for` in hooks that the inner layer doesn't actually
736741
// have real implementations of? probably not...

tracing-subscriber/src/layer/layered.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -254,6 +254,11 @@ where
254254
self.inner.on_layer(subscriber);
255255
}
256256

257+
fn on_reload_layer(&mut self, subscriber: &S) {
258+
self.layer.on_reload_layer(subscriber);
259+
self.inner.on_reload_layer(subscriber);
260+
}
261+
257262
fn register_callsite(&self, metadata: &'static Metadata<'static>) -> Interest {
258263
self.pick_interest(self.layer.register_callsite(metadata), || {
259264
self.inner.register_callsite(metadata)
@@ -396,6 +401,11 @@ where
396401
fn register_filter(&mut self) -> FilterId {
397402
self.inner.register_filter()
398403
}
404+
405+
#[cfg(all(feature = "registry", feature = "std"))]
406+
fn register_filter_immut(&self) -> FilterId {
407+
self.inner.register_filter_immut()
408+
}
399409
}
400410

401411
impl<L, S> Layered<L, S>

tracing-subscriber/src/layer/mod.rs

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -786,6 +786,30 @@ where
786786
let _ = subscriber;
787787
}
788788

789+
/// Performs late initialization when a `Layer` is reloaded into a [`Subscriber`].
790+
///
791+
/// This is similar to [`Layer::on_layer`], but is intended for use when a
792+
/// `Layer` is dynamically reloaded via [`reload`](crate::reload), rather than
793+
/// attached to a subscriber during initial construction.
794+
///
795+
/// Unlike `on_layer`, this method receives a shared (`&`) reference to the
796+
/// [`Subscriber`], allowing it to be called in contexts where the subscriber
797+
/// is already in use and cannot be mutated. This is useful for registering
798+
/// internal state (e.g., with [`register_filter_immut`]) that would otherwise
799+
/// require a mutable subscriber.
800+
///
801+
/// Like `on_layer`, `Layer` implementations that wrap other layers (such as
802+
/// [`Filtered`] or [`Layered`]) MUST ensure that their inner layers'
803+
/// `on_reload_layer` methods are called during reload, if applicable.
804+
///
805+
/// [`Subscriber`]: tracing::Subscriber
806+
/// [`Filtered`]: crate::filter::Filtered
807+
/// [`Layered`]: crate::layer::Layered
808+
/// [`register_filter_immut`]: crate::registry::LookupSpan::register_filter_immut
809+
fn on_reload_layer(&mut self, subscriber: &S) {
810+
let _ = subscriber;
811+
}
812+
789813
/// Registers a new callsite with this layer, returning whether or not
790814
/// the layer is interested in being notified about the callsite, similarly
791815
/// to [`Subscriber::register_callsite`].
@@ -1568,6 +1592,12 @@ where
15681592
}
15691593
}
15701594

1595+
fn on_reload_layer(&mut self, subscriber: &S) {
1596+
if let Some(ref mut layer) = self {
1597+
layer.on_reload_layer(subscriber);
1598+
}
1599+
}
1600+
15711601
#[inline]
15721602
fn on_new_span(&self, attrs: &span::Attributes<'_>, id: &span::Id, ctx: Context<'_, S>) {
15731603
if let Some(ref inner) = self {
@@ -1690,6 +1720,11 @@ feature! {
16901720
self.deref_mut().on_layer(subscriber);
16911721
}
16921722

1723+
#[inline]
1724+
fn on_reload_layer(&mut self, subscriber: &S) {
1725+
self.deref_mut().on_reload_layer(subscriber);
1726+
}
1727+
16931728
#[inline]
16941729
fn on_new_span(&self, attrs: &span::Attributes<'_>, id: &span::Id, ctx: Context<'_, S>) {
16951730
self.deref().on_new_span(attrs, id, ctx)
@@ -1787,6 +1822,12 @@ feature! {
17871822
}
17881823
}
17891824

1825+
fn on_reload_layer(&mut self, subscriber: &S) {
1826+
for l in self {
1827+
l.on_reload_layer(subscriber);
1828+
}
1829+
}
1830+
17901831
fn register_callsite(&self, metadata: &'static Metadata<'static>) -> Interest {
17911832
// Return highest level of interest.
17921833
let mut interest = Interest::never();

tracing-subscriber/src/registry/mod.rs

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,32 @@ pub trait LookupSpan<'a> {
151151
std::any::type_name::<Self>()
152152
)
153153
}
154+
155+
/// Registers a [`Filter`] for [per-layer filtering] with this
156+
/// [`Subscriber`], using a shared reference.
157+
///
158+
/// This method is similar to [`register_filter`], but takes `&self`
159+
/// instead of `&mut self`. It is intended for use in cases where
160+
/// filters must be registered after the subscriber has already been
161+
/// constructed, such as when reloading a [`Layer`] dynamically.
162+
///
163+
/// # Panics
164+
///
165+
/// If this `Subscriber` does not support [per-layer filtering].
166+
///
167+
/// [`Filter`]: crate::layer::Filter
168+
/// [per-layer filtering]: crate::layer::Layer#per-layer-filtering
169+
/// [`Subscriber`]: tracing_core::Subscriber
170+
/// [`Layer`]: crate::layer::Layer
171+
/// [`register_filter`]: Self::register_filter
172+
#[cfg(feature = "registry")]
173+
#[cfg_attr(docsrs, doc(cfg(feature = "registry")))]
174+
fn register_filter_immut(&self) -> FilterId {
175+
panic!(
176+
"{} does not currently support filters",
177+
std::any::type_name::<Self>()
178+
)
179+
}
154180
}
155181

156182
/// A stored representation of data associated with a span.

tracing-subscriber/src/registry/sharded.rs

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ use crate::{
1212
};
1313
use std::{
1414
cell::{self, Cell, RefCell},
15-
sync::atomic::{fence, AtomicUsize, Ordering},
15+
sync::atomic::{fence, AtomicU8, AtomicUsize, Ordering},
1616
};
1717
use tracing_core::{
1818
dispatcher::{self, Dispatch},
@@ -92,7 +92,7 @@ use tracing_core::{
9292
pub struct Registry {
9393
spans: Pool<DataInner>,
9494
current_spans: ThreadLocal<RefCell<SpanStack>>,
95-
next_filter_id: u8,
95+
next_filter_id: AtomicU8,
9696
}
9797

9898
/// Span data stored in a [`Registry`].
@@ -137,7 +137,7 @@ impl Default for Registry {
137137
Self {
138138
spans: Pool::new(),
139139
current_spans: ThreadLocal::new(),
140-
next_filter_id: 0,
140+
next_filter_id: AtomicU8::new(0),
141141
}
142142
}
143143
}
@@ -201,7 +201,7 @@ impl Registry {
201201
}
202202

203203
pub(crate) fn has_per_layer_filters(&self) -> bool {
204-
self.next_filter_id > 0
204+
self.next_filter_id.load(Ordering::SeqCst) > 0
205205
}
206206

207207
pub(crate) fn span_stack(&self) -> cell::Ref<'_, SpanStack> {
@@ -375,9 +375,12 @@ impl<'a> LookupSpan<'a> for Registry {
375375
}
376376

377377
fn register_filter(&mut self) -> FilterId {
378-
let id = FilterId::new(self.next_filter_id);
379-
self.next_filter_id += 1;
380-
id
378+
self.register_filter_immut()
379+
}
380+
381+
fn register_filter_immut(&self) -> FilterId {
382+
let filter_id = self.next_filter_id.fetch_add(1, Ordering::SeqCst);
383+
FilterId::new(filter_id)
381384
}
382385
}
383386

tracing-subscriber/src/reload.rs

Lines changed: 86 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@
3232
//! info!("This will be logged");
3333
//! ```
3434
//!
35-
//! Reloading a [`Filtered`](crate::filter::Filtered) layer:
35+
//! Reloading the [`Filter`] of a [`Filtered`](crate::filter::Filtered) layer:
3636
//!
3737
//! ```rust
3838
//! # use tracing::info;
@@ -53,6 +53,53 @@
5353
//! info!("This will be logged");
5454
//! ```
5555
//!
56+
//! Reloading a [`Filtered`](crate::filter::Filtered) layer using [`Handle::reload_filtered`]:
57+
//!
58+
//! ```rust
59+
//! use tracing::info;
60+
//! use tracing_subscriber::{filter, fmt, reload, prelude::*};
61+
//! let filtered_layer = fmt::Layer::default().with_filter(filter::LevelFilter::WARN);
62+
//! let (filtered_layer, reload_handle) = reload::Layer::new(filtered_layer);
63+
//! let dispatcher =
64+
//! tracing_core::Dispatch::new(tracing_subscriber::registry().with(filtered_layer));
65+
//!
66+
//! tracing_core::dispatcher::with_default(&dispatcher, || {
67+
//! info!("This will be ignored");
68+
//! let new_layer = fmt::Layer::default().with_filter(filter::LevelFilter::INFO);
69+
//! let subscriber = dispatcher
70+
//! .downcast_ref::<tracing_subscriber::Registry>()
71+
//! .unwrap();
72+
//! reload_handle
73+
//! .reload_filtered(new_layer, subscriber)
74+
//! .unwrap();
75+
//! info!("This will be logged");
76+
//! });
77+
//! ```
78+
//!
79+
//! Reloading a [`Filtered`](crate::filter::Filtered) layer using [`Handle::modify`]:
80+
//!
81+
//! ```rust
82+
//! use tracing::info;
83+
//! use tracing_subscriber::{filter, fmt, reload, prelude::*};
84+
//! let filtered_layer = fmt::Layer::default().with_filter(filter::LevelFilter::WARN);
85+
//! let (filtered_layer, reload_handle) = reload::Layer::new(filtered_layer);
86+
//! let dispatcher =
87+
//! tracing_core::Dispatch::new(tracing_subscriber::registry().with(filtered_layer));
88+
//!
89+
//! tracing_core::dispatcher::with_default(&dispatcher, || {
90+
//! info!("This will be ignored");
91+
//! let new_layer = fmt::Layer::default().with_filter(filter::LevelFilter::INFO);
92+
//! let subscriber = dispatcher
93+
//! .downcast_ref::<tracing_subscriber::Registry>()
94+
//! .unwrap();
95+
//! reload_handle.modify(|layer| {
96+
//! *layer = new_layer;
97+
//! layer.on_reload_layer(subscriber);
98+
//! }).unwrap();
99+
//! info!("This will be logged");
100+
//! });
101+
//! ```
102+
//!
56103
//! ## Note
57104
//!
58105
//! The [`Layer`] implementation is unable to implement downcasting functionality,
@@ -62,6 +109,7 @@
62109
//! `Filter` on a layer, prefer wrapping that `Filter` in the `reload::Layer`.
63110
//!
64111
//! [`Filter` trait]: crate::layer::Filter
112+
//! [`Filter`]: crate::layer::Filter
65113
//! [`Layer` type]: Layer
66114
//! [`Layer` trait]: super::layer::Layer
67115
use crate::layer;
@@ -124,6 +172,10 @@ where
124172
try_lock!(self.inner.write(), else return).on_layer(subscriber);
125173
}
126174

175+
fn on_reload_layer(&mut self, subscriber: &S) {
176+
try_lock!(self.inner.write(), else return).on_reload_layer(subscriber);
177+
}
178+
127179
#[inline]
128180
fn register_callsite(&self, metadata: &'static Metadata<'static>) -> Interest {
129181
try_lock!(self.inner.read(), else return Interest::sometimes()).register_callsite(metadata)
@@ -287,7 +339,9 @@ impl<L, S> Handle<L, S> {
287339
/// Replace the current [`Layer`] or [`Filter`] with the provided `new_value`.
288340
///
289341
/// [`Handle::reload`] cannot be used with the [`Filtered`] layer; use
290-
/// [`Handle::modify`] instead (see [this issue] for additional details).
342+
/// [`Handle::reload_filtered`] instead, or [`Handle::modify`] to replace
343+
/// the [`Filter`] of a [`Filtered`] layer (see [this issue] for additional
344+
/// details).
291345
///
292346
/// However, if the _only_ the [`Filter`] needs to be modified, use
293347
/// `reload::Layer` to wrap the `Filter` directly.
@@ -303,6 +357,36 @@ impl<L, S> Handle<L, S> {
303357
})
304358
}
305359

360+
/// Replaces the current [`Filtered`] layer with a new one and performs
361+
/// late initialization using [`Layer::on_reload_layer`].
362+
///
363+
/// Unlike [`Handle::reload`], this method is specifically designed to
364+
/// support [per-layer filtering] by ensuring that the [`Filter`] within
365+
/// a [`Filtered`] layer is correctly registered with the [`Subscriber`].
366+
/// It achieves this by invoking the [`Layer::on_reload_layer`] callback
367+
/// after replacing the layer, allowing the new filter to be initialized
368+
/// using only a shared reference to the `Subscriber`.
369+
///
370+
/// This method should be used instead of [`reload`] when reloading a
371+
/// [`Filtered`] layer.
372+
///
373+
/// [`Filter`]: crate::layer::Filter
374+
/// [`Filtered`]: crate::filter::Filtered
375+
/// [`reload`]: Self::reload
376+
/// [`Subscriber`]: tracing::Subscriber
377+
/// [`Layer::on_reload_layer`]: crate::layer::Layer::on_reload_layer
378+
/// [per-layer filtering]: crate::layer::Layer#per-layer-filtering
379+
pub fn reload_filtered(&self, new_value: impl Into<L>, subscriber: &S) -> Result<(), Error>
380+
where
381+
L: crate::layer::Layer<S> + 'static,
382+
S: Subscriber,
383+
{
384+
self.modify(|layer| {
385+
*layer = new_value.into();
386+
layer.on_reload_layer(subscriber);
387+
})
388+
}
389+
306390
/// Invokes a closure with a mutable reference to the current layer or filter,
307391
/// allowing it to be modified in place.
308392
pub fn modify(&self, f: impl FnOnce(&mut L)) -> Result<(), Error> {

0 commit comments

Comments
 (0)