Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

subscriber: fix reload ergonomics #1035

Merged
merged 2 commits into from
Oct 13, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 7 additions & 16 deletions examples/examples/tower-load.rs
Original file line number Diff line number Diff line change
Expand Up @@ -185,23 +185,20 @@ impl<T> Service<T> for MakeSvc {
}
}

struct AdminSvc<S> {
handle: Handle<EnvFilter, S>,
struct AdminSvc {
handle: Handle<EnvFilter>,
}

impl<S> Clone for AdminSvc<S> {
impl Clone for AdminSvc {
fn clone(&self) -> Self {
Self {
handle: self.handle.clone(),
}
}
}

impl<'a, S> Service<&'a AddrStream> for AdminSvc<S>
where
S: tracing::Subscriber,
{
type Response = AdminSvc<S>;
impl<'a> Service<&'a AddrStream> for AdminSvc {
type Response = AdminSvc;
type Error = hyper::Error;
type Future = Ready<Result<Self::Response, Self::Error>>;

Expand All @@ -214,10 +211,7 @@ where
}
}

impl<S> Service<Request<Body>> for AdminSvc<S>
where
S: tracing::Subscriber + 'static,
{
impl Service<Request<Body>> for AdminSvc {
type Response = Response<Body>;
type Error = Err;
type Future = Pin<Box<dyn Future<Output = Result<Response<Body>, Err>> + std::marker::Send>>;
Expand Down Expand Up @@ -252,10 +246,7 @@ where
}
}

impl<S> AdminSvc<S>
where
S: tracing::Subscriber + 'static,
{
impl AdminSvc {
fn set_from(&self, bytes: Bytes) -> Result<(), String> {
use std::str;
let body = str::from_utf8(&bytes.as_ref()).map_err(|e| format!("{}", e))?;
Expand Down
72 changes: 48 additions & 24 deletions tracing-subscriber/src/fmt/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ use crate::{
filter::LevelFilter,
layer,
registry::{LookupSpan, Registry},
reload,
};

#[doc(inline)]
Expand Down Expand Up @@ -646,35 +647,13 @@ impl<T, F, W> SubscriberBuilder<format::JsonFields, format::Format<format::Json,
}
}

#[cfg(feature = "env-filter")]
#[cfg_attr(docsrs, doc(cfg(feature = "env-filter")))]
impl<N, E, W> SubscriberBuilder<N, E, crate::EnvFilter, W>
where
Formatter<N, E, W>: tracing_core::Subscriber + 'static,
{
/// Configures the subscriber being built to allow filter reloading at
/// runtime.
pub fn with_filter_reloading(
self,
) -> SubscriberBuilder<N, E, crate::reload::Layer<crate::EnvFilter, Formatter<N, E, W>>, W>
{
let (filter, _) = crate::reload::Layer::new(self.filter);
SubscriberBuilder {
filter,
inner: self.inner,
}
}
}

#[cfg(feature = "env-filter")]
#[cfg_attr(docsrs, doc(cfg(feature = "env-filter")))]
impl<N, E, W> SubscriberBuilder<N, E, crate::reload::Layer<crate::EnvFilter, Formatter<N, E, W>>, W>
impl<N, E, F, W> SubscriberBuilder<N, E, reload::Layer<F>, W>
where
Formatter<N, E, W>: tracing_core::Subscriber + 'static,
{
/// Returns a `Handle` that may be used to reload the constructed subscriber's
/// filter.
pub fn reload_handle(&self) -> crate::reload::Handle<crate::EnvFilter, Formatter<N, E, W>> {
pub fn reload_handle(&self) -> reload::Handle<F> {
self.filter.handle()
}
}
Expand Down Expand Up @@ -814,6 +793,51 @@ impl<N, E, F, W> SubscriberBuilder<N, E, F, W> {
}
}

/// Configures the subscriber being built to allow filter reloading at
/// runtime.
///
/// The returned builder will have a [`reload_handle`] method, which returns
/// a [`reload::Handle`] that may be used to set a new filter value.
///
/// For example:
///
/// ```
/// use tracing::Level;
/// use tracing_subscriber::prelude::*;
///
/// let builder = tracing_subscriber::fmt()
/// // Set a max level filter on the subscriber
/// .with_max_level(Level::INFO)
/// .with_filter_reloading();
///
/// // Get a handle for modifying the subscriber's max level filter.
/// let handle = builder.reload_handle();
///
/// // Finish building the subscriber, and set it as the default.
/// builder.finish().init();
///
/// // Currently, the max level is INFO, so this event will be disabled.
/// tracing::debug!("this is not recorded!");
///
/// // Use the handle to set a new max level filter.
/// // (this returns an error if the subscriber has been dropped, which shouldn't
/// // happen in this example.)
/// handle.reload(Level::DEBUG).expect("the subscriber should still exist");
///
/// // Now, the max level is INFO, so this event will be recorded.
/// tracing::debug!("this is recorded!");
/// ```
///
/// [`reload_handle`]: SubscriberBuilder::reload_handle
/// [`reload::Handle`]: crate::reload::Handle
pub fn with_filter_reloading(self) -> SubscriberBuilder<N, E, reload::Layer<F>, W> {
let (filter, _) = reload::Layer::new(self.filter);
SubscriberBuilder {
filter,
inner: self.inner,
}
}

/// Sets the function that the subscriber being built should use to format
/// events that occur.
pub fn event_format<E2>(self, fmt_event: E2) -> SubscriberBuilder<N, E2, F, W>
Expand Down
30 changes: 8 additions & 22 deletions tracing-subscriber/src/reload.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ use crate::sync::RwLock;

use std::{
error, fmt,
marker::PhantomData,
sync::{Arc, Weak},
};
use tracing_core::{
Expand All @@ -27,20 +26,18 @@ use tracing_core::{

/// Wraps a `Layer`, allowing it to be reloaded dynamically at runtime.
#[derive(Debug)]
pub struct Layer<L, S> {
pub struct Layer<L> {
// TODO(eliza): this once used a `crossbeam_util::ShardedRwLock`. We may
// eventually wish to replace it with a sharded lock implementation on top
// of our internal `RwLock` wrapper type. If possible, we should profile
// this first to determine if it's necessary.
inner: Arc<RwLock<L>>,
_s: PhantomData<fn(S)>,
}

/// Allows reloading the state of an associated `Layer`.
#[derive(Debug)]
pub struct Handle<L, S> {
pub struct Handle<L> {
inner: Weak<RwLock<L>>,
_s: PhantomData<fn(S)>,
}

/// Indicates that an error occurred when reloading a layer.
Expand All @@ -57,7 +54,7 @@ enum ErrorKind {

// ===== impl Layer =====

impl<L, S> crate::Layer<S> for Layer<L, S>
impl<L, S> crate::Layer<S> for Layer<L>
where
L: crate::Layer<S> + 'static,
S: Subscriber,
Expand Down Expand Up @@ -113,38 +110,28 @@ where
}
}

impl<L, S> Layer<L, S>
where
L: crate::Layer<S> + 'static,
S: Subscriber,
{
impl<L> Layer<L> {
/// Wraps the given `Layer`, returning a `Layer` and a `Handle` that allows
/// the inner type to be modified at runtime.
pub fn new(inner: L) -> (Self, Handle<L, S>) {
pub fn new(inner: L) -> (Self, Handle<L>) {
let this = Self {
inner: Arc::new(RwLock::new(inner)),
_s: PhantomData,
};
let handle = this.handle();
(this, handle)
}

/// Returns a `Handle` that can be used to reload the wrapped `Layer`.
pub fn handle(&self) -> Handle<L, S> {
pub fn handle(&self) -> Handle<L> {
Handle {
inner: Arc::downgrade(&self.inner),
_s: PhantomData,
}
}
}

// ===== impl Handle =====

impl<L, S> Handle<L, S>
where
L: crate::Layer<S> + 'static,
S: Subscriber,
{
impl<L> Handle<L> {
/// Replace the current layer with the provided `new_layer`.
pub fn reload(&self, new_layer: impl Into<L>) -> Result<(), Error> {
self.modify(|layer| {
Expand Down Expand Up @@ -189,11 +176,10 @@ where
}
}

impl<L, S> Clone for Handle<L, S> {
impl<L> Clone for Handle<L> {
fn clone(&self) -> Self {
Handle {
inner: self.inner.clone(),
_s: PhantomData,
}
}
}
Expand Down