From e73747feeb958f2b2902ca539390dcec8658a8a7 Mon Sep 17 00:00:00 2001
From: Gus Wynn <guswynn@gmail.com>
Date: Fri, 1 Jul 2022 12:39:36 -0700
Subject: [PATCH] subscriber: pass through `max_level_hint` in `reload` (#2204)

## 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`
---
 tracing-subscriber/src/reload.rs   | 12 ++++++++++-
 tracing-subscriber/tests/reload.rs | 33 +++++++++++++++++++++---------
 2 files changed, 34 insertions(+), 11 deletions(-)

diff --git a/tracing-subscriber/src/reload.rs b/tracing-subscriber/src/reload.rs
index 3d1743440e..0c6c1c45c4 100644
--- a/tracing-subscriber/src/reload.rs
+++ b/tracing-subscriber/src/reload.rs
@@ -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.
@@ -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 =====
@@ -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> {
diff --git a/tracing-subscriber/tests/reload.rs b/tracing-subscriber/tests/reload.rs
index b8b6c2b461..7fc39aa51e 100644
--- a/tracing-subscriber/tests/reload.rs
+++ b/tracing-subscriber/tests/reload.rs
@@ -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 {
@@ -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);
@@ -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();
 
@@ -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 {
@@ -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);
@@ -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();