From 60088ac98ffabee5b9407e80bd92e6e13389bea4 Mon Sep 17 00:00:00 2001 From: Gus Wynn Date: Thu, 9 Jun 2022 15:54:58 -0700 Subject: [PATCH] implement reload for the layer/subscriber --- tracing-subscriber/src/reload.rs | 119 +++++++++++++++++++++++------ tracing-subscriber/tests/reload.rs | 59 ++++++++++++++ 2 files changed, 155 insertions(+), 23 deletions(-) diff --git a/tracing-subscriber/src/reload.rs b/tracing-subscriber/src/reload.rs index f2c577b32f..9baad3dd44 100644 --- a/tracing-subscriber/src/reload.rs +++ b/tracing-subscriber/src/reload.rs @@ -1,15 +1,25 @@ //! Wrapper for a `Collect` or `Subscribe` to allow it to be dynamically reloaded. //! -//! This module provides a type implementing [`Subscribe`] which wraps another type implementing -//! the [`Subscribe`] trait, allowing the wrapped type to be replaced with another +//! This module provides a type implementing [`Subscribe`] and [`Filter`] +//! which wraps another type implementing the corresponding trait. This +//! allows the wrapped type to be replaced with another //! instance of that type at runtime. //! -//! This can be used in cases where a subset of `Collect` functionality +//! This can be used in cases where a subset of `Collect` or `Filter` functionality //! should be dynamically reconfigured, such as when filtering directives may //! change at runtime. Note that this subscriber introduces a (relatively small) //! amount of overhead, and should thus only be used as needed. //! +//! ## Note +//! +//! //! The [`Subscribe`] implementation is unable to implement downcasting functionality, +//! so certain `Subscribers` will fail to reload if wrapped in a `reload::Subscriber`. +//! +//! If you only want to be able to dynamically change the +//! `Filter` on your layer, prefer wrapping that `Filter` in the `reload::Subscriber`. +//! //! [`Subscribe`]: crate::Subscribe +//! [`Filter`]: crate::subscribe::Filter use crate::subscribe; use crate::sync::RwLock; @@ -23,7 +33,10 @@ use tracing_core::{ span, Event, Metadata, }; -/// Wraps a `Collect` or `Subscribe`, allowing it to be reloaded dynamically at runtime. +/// Wraps a `Filter` or `Subscribe`, allowing it to be reloaded dynamically at runtime. +/// +/// [`Filter`]: crate::subscribe::Filter +/// [`Subscribe`]: crate::Subscribe #[derive(Debug)] pub struct Subscriber { // TODO(eliza): this once used a `crossbeam_util::ShardedRwLock`. We may @@ -119,10 +132,67 @@ where } } -impl Subscriber { - /// Wraps the given `Subscribe`, returning a subscriber and a `Handle` that allows +#[cfg(all(feature = "registry", feature = "std"))] +#[cfg_attr(docsrs, doc(cfg(all(feature = "registry", feature = "std"))))] +impl crate::subscribe::Filter for Subscriber +where + S: crate::subscribe::Filter + 'static, + C: Collect, +{ + #[inline] + fn callsite_enabled(&self, metadata: &'static Metadata<'static>) -> Interest { + try_lock!(self.inner.read(), else return Interest::sometimes()).callsite_enabled(metadata) + } + + #[inline] + fn enabled(&self, metadata: &Metadata<'_>, ctx: &subscribe::Context<'_, C>) -> bool { + try_lock!(self.inner.read(), else return false).enabled(metadata, ctx) + } + + #[inline] + fn on_new_span( + &self, + attrs: &span::Attributes<'_>, + id: &span::Id, + ctx: subscribe::Context<'_, C>, + ) { + try_lock!(self.inner.read()).on_new_span(attrs, id, ctx) + } + + #[inline] + fn on_record( + &self, + span: &span::Id, + values: &span::Record<'_>, + ctx: subscribe::Context<'_, C>, + ) { + try_lock!(self.inner.read()).on_record(span, values, ctx) + } + + #[inline] + fn on_enter(&self, id: &span::Id, ctx: subscribe::Context<'_, C>) { + try_lock!(self.inner.read()).on_enter(id, ctx) + } + + #[inline] + fn on_exit(&self, id: &span::Id, ctx: subscribe::Context<'_, C>) { + try_lock!(self.inner.read()).on_exit(id, ctx) + } + + #[inline] + fn on_close(&self, id: span::Id, ctx: subscribe::Context<'_, C>) { + try_lock!(self.inner.read()).on_close(id, ctx) + } +} + +impl Subscriber { + /// Wraps the given `Subscribe` or `Filter`, + /// returning a subscriber or filter and a `Handle` that allows /// the inner type to be modified at runtime. - pub fn new(inner: S) -> (Self, Handle) { + /// + /// [`Filter`]: crate::subscribe::Filter + /// [`Subscribe`]: crate::Subscribe + pub fn new(inner: T) -> (Self, Handle) { let this = Self { inner: Arc::new(RwLock::new(inner)), }; @@ -131,7 +201,7 @@ impl Subscriber { } /// Returns a `Handle` that can be used to reload the wrapped `Subscribe`. - pub fn handle(&self) -> Handle { + pub fn handle(&self) -> Handle { Handle { inner: Arc::downgrade(&self.inner), } @@ -140,22 +210,25 @@ impl Subscriber { // ===== impl Handle ===== -impl Handle { - /// Replace the current subscriber with the provided `new_subscriber`. +impl Handle { + /// Replace the current subscriber or filter with the provided `new_value`. + /// + /// **Warning:** [`Handle::reload`] cannot be used with the [`Filtered`](crate::filter::Filtered) + /// subscriber; use [`Handle::modify`] instead (this [this issue] for additional details). + /// + /// However, if the _only_ the [`Filter`] needs to be modified, use `reload::Subscriber` to + /// to wrap the `Filter` directly. /// - /// **Warning:** The [`Filtered`](crate::filter::Filtered) type currently can't be changed - /// at runtime via the [`Handle::reload`] method. - /// Use the [`Handle::modify`] method to change the filter instead. - /// (see ) - pub fn reload(&self, new_subscriber: impl Into) -> Result<(), Error> { - self.modify(|subscriber| { - *subscriber = new_subscriber.into(); + /// [this issue]: https://github.com/tokio-rs/tracing/issues/1629 + pub fn reload(&self, new_value: impl Into) -> Result<(), Error> { + self.modify(|object| { + *object = new_value.into(); }) } /// Invokes a closure with a mutable reference to the current subscriber, /// allowing it to be modified in place. - pub fn modify(&self, f: impl FnOnce(&mut S)) -> Result<(), Error> { + pub fn modify(&self, f: impl FnOnce(&mut T)) -> Result<(), Error> { let inner = self.inner.upgrade().ok_or(Error { kind: ErrorKind::CollectorGone, })?; @@ -182,16 +255,16 @@ impl Handle { /// Returns a clone of the subscriber's current value if it still exists. /// Otherwise, if the collector has been dropped, returns `None`. - pub fn clone_current(&self) -> Option + pub fn clone_current(&self) -> Option where - S: Clone, + T: Clone, { - self.with_current(S::clone).ok() + self.with_current(T::clone).ok() } /// Invokes a closure with a borrowed reference to the current subscriber, /// returning the result (or an error if the collector no longer exists). - pub fn with_current(&self, f: impl FnOnce(&S) -> T) -> Result { + pub fn with_current(&self, f: impl FnOnce(&T) -> T2) -> Result { let inner = self.inner.upgrade().ok_or(Error { kind: ErrorKind::CollectorGone, })?; @@ -200,7 +273,7 @@ impl Handle { } } -impl Clone for Handle { +impl Clone for Handle { fn clone(&self) -> Self { Handle { inner: self.inner.clone(), diff --git a/tracing-subscriber/tests/reload.rs b/tracing-subscriber/tests/reload.rs index ee133366dd..f979087afd 100644 --- a/tracing-subscriber/tests/reload.rs +++ b/tracing-subscriber/tests/reload.rs @@ -32,6 +32,17 @@ impl Collect for NopCollector { } } +pub struct NopSubscriber; +impl tracing_subscriber::Subscribe for NopSubscriber { + fn register_callsite(&self, _m: &Metadata<'_>) -> Interest { + Interest::sometimes() + } + + fn enabled(&self, _m: &Metadata<'_>, _: subscribe::Context<'_, S>) -> bool { + true + } +} + #[test] fn reload_handle() { static FILTER1_CALLS: AtomicUsize = AtomicUsize::new(0); @@ -82,3 +93,51 @@ fn reload_handle() { assert_eq!(FILTER2_CALLS.load(Ordering::SeqCst), 1); }) } + +#[test] +fn reload_filter() { + static FILTER1_CALLS: AtomicUsize = AtomicUsize::new(0); + static FILTER2_CALLS: AtomicUsize = AtomicUsize::new(0); + + enum Filter { + One, + Two, + } + + impl tracing_subscriber::subscribe::Filter for Filter { + fn enabled(&self, m: &Metadata<'_>, _: &subscribe::Context<'_, S>) -> bool { + println!("ENABLED: {:?}", m); + match self { + Filter::One => FILTER1_CALLS.fetch_add(1, Ordering::SeqCst), + Filter::Two => FILTER2_CALLS.fetch_add(1, Ordering::SeqCst), + }; + true + } + } + fn event() { + tracing::trace!("my event"); + } + + let (filter, handle) = Subscriber::new(Filter::One); + + let dispatcher = tracing_core::dispatch::Dispatch::new( + tracing_subscriber::registry().with(NopSubscriber.with_filter(filter)), + ); + + tracing_core::dispatch::with_default(&dispatcher, || { + assert_eq!(FILTER1_CALLS.load(Ordering::SeqCst), 0); + assert_eq!(FILTER2_CALLS.load(Ordering::SeqCst), 0); + + event(); + + assert_eq!(FILTER1_CALLS.load(Ordering::SeqCst), 1); + assert_eq!(FILTER2_CALLS.load(Ordering::SeqCst), 0); + + handle.reload(Filter::Two).expect("should reload"); + + event(); + + assert_eq!(FILTER1_CALLS.load(Ordering::SeqCst), 1); + assert_eq!(FILTER2_CALLS.load(Ordering::SeqCst), 1); + }) +}