Skip to content

Commit

Permalink
subscriber: pass through max_level_hint in reload (#2204)
Browse files Browse the repository at this point in the history
## Motivation

When using a `reload` layer, the fast-path current level check doesn't
work, as the `max_level_hint` is just `None`, which `rebuild_interest`
interprets as `TRACE`

## Solution

Pass through to the underlying layer/filter. On poisons, when already
panicking, just return `None`
  • Loading branch information
guswynn authored and hawkw committed Jul 20, 2022
1 parent 9c4bd43 commit e73747f
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 11 deletions.
12 changes: 11 additions & 1 deletion tracing-subscriber/src/reload.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ use std::{
use tracing_core::{
callsite, span,
subscriber::{Interest, Subscriber},
Event, Metadata,
Event, LevelFilter, Metadata,
};

/// Wraps a `Layer` or `Filter`, allowing it to be reloaded dynamically at runtime.
Expand Down Expand Up @@ -173,6 +173,11 @@ where
fn on_id_change(&self, old: &span::Id, new: &span::Id, ctx: layer::Context<'_, S>) {
try_lock!(self.inner.read()).on_id_change(old, new, ctx)
}

#[inline]
fn max_level_hint(&self) -> Option<LevelFilter> {
try_lock!(self.inner.read(), else return None).max_level_hint()
}
}

// ===== impl Filter =====
Expand Down Expand Up @@ -218,6 +223,11 @@ where
fn on_close(&self, id: span::Id, ctx: layer::Context<'_, S>) {
try_lock!(self.inner.read()).on_close(id, ctx)
}

#[inline]
fn max_level_hint(&self) -> Option<LevelFilter> {
try_lock!(self.inner.read(), else return None).max_level_hint()
}
}

impl<L, S> Layer<L, S> {
Expand Down
33 changes: 23 additions & 10 deletions tracing-subscriber/tests/reload.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
#![cfg(feature = "std")]
#![cfg(feature = "registry")]
use std::sync::atomic::{AtomicUsize, Ordering};
use tracing_core::{
span::{Attributes, Id, Record},
subscriber::Interest,
Event, Metadata, Subscriber,
Event, LevelFilter, Metadata, Subscriber, subscriber::Interest,
};
use tracing_subscriber::{layer, prelude::*, reload::*};

pub struct NopSubscriber;
fn event() {
tracing::info!("my event");
}

impl Subscriber for NopSubscriber {
fn register_callsite(&self, _: &'static Metadata<'static>) -> Interest {
Expand Down Expand Up @@ -53,9 +55,13 @@ fn reload_handle() {
};
true
}
}
fn event() {
tracing::trace!("my event");

fn max_level_hint(&self) -> Option<LevelFilter> {
match self {
Filter::One => Some(LevelFilter::INFO),
Filter::Two => Some(LevelFilter::DEBUG),
}
}
}

let (layer, handle) = Layer::new(Filter::One);
Expand All @@ -71,7 +77,9 @@ fn reload_handle() {
assert_eq!(FILTER1_CALLS.load(Ordering::SeqCst), 1);
assert_eq!(FILTER2_CALLS.load(Ordering::SeqCst), 0);

assert_eq!(LevelFilter::current(), LevelFilter::INFO);
handle.reload(Filter::Two).expect("should reload");
assert_eq!(LevelFilter::current(), LevelFilter::DEBUG);

event();

Expand All @@ -81,7 +89,6 @@ fn reload_handle() {
}

#[test]
#[cfg(feature = "registry")]
fn reload_filter() {
struct NopLayer;
impl<S: Subscriber> tracing_subscriber::Layer<S> for NopLayer {
Expand Down Expand Up @@ -111,9 +118,13 @@ fn reload_filter() {
};
true
}
}
fn event() {
tracing::trace!("my event");

fn max_level_hint(&self) -> Option<LevelFilter> {
match self {
Filter::One => Some(LevelFilter::INFO),
Filter::Two => Some(LevelFilter::DEBUG),
}
}
}

let (filter, handle) = Layer::new(Filter::One);
Expand All @@ -131,7 +142,9 @@ fn reload_filter() {
assert_eq!(FILTER1_CALLS.load(Ordering::SeqCst), 1);
assert_eq!(FILTER2_CALLS.load(Ordering::SeqCst), 0);

assert_eq!(LevelFilter::current(), LevelFilter::INFO);
handle.reload(Filter::Two).expect("should reload");
assert_eq!(LevelFilter::current(), LevelFilter::DEBUG);

event();

Expand Down

0 comments on commit e73747f

Please sign in to comment.