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 Jul 1, 2022
1 parent 1eaa128 commit 2aa0cb0
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 9 deletions.
12 changes: 11 additions & 1 deletion tracing-subscriber/src/reload.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ use std::{
use tracing_core::{
callsite,
collect::{Collect, Interest},
span, Event, Metadata,
span, Event, LevelFilter, Metadata,
};

/// Wraps a `Filter` or `Subscribe`, allowing it to be reloaded dynamically at runtime.
Expand Down Expand Up @@ -135,6 +135,11 @@ where
fn on_id_change(&self, old: &span::Id, new: &span::Id, ctx: subscribe::Context<'_, C>) {
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()
}
}

#[cfg(all(feature = "registry", feature = "std"))]
Expand Down Expand Up @@ -188,6 +193,11 @@ where
fn on_close(&self, id: span::Id, ctx: subscribe::Context<'_, C>) {
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<T> Subscriber<T> {
Expand Down
32 changes: 24 additions & 8 deletions tracing-subscriber/tests/reload.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,16 @@
#![cfg(feature = "reload")]
#![cfg(feature = "registry")]
use std::sync::atomic::{AtomicUsize, Ordering};
use tracing_core::{
collect::Interest,
span::{Attributes, Id, Record},
Collect, Event, Metadata,
Collect, Event, LevelFilter, Metadata,
};
use tracing_subscriber::{prelude::*, reload::*, subscribe};

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

pub struct NopCollector;

impl Collect for NopCollector {
Expand Down Expand Up @@ -67,9 +71,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 (subscriber, handle) = Subscriber::new(Filter::One);
Expand All @@ -85,7 +93,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 Down Expand Up @@ -113,9 +123,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) = Subscriber::new(Filter::One);
Expand All @@ -133,7 +147,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 2aa0cb0

Please sign in to comment.