From 2d27332953f60048421e7f33324d9b478e35df0c Mon Sep 17 00:00:00 2001 From: tfreiberg-fastly <98025906+tfreiberg-fastly@users.noreply.github.com> Date: Tue, 8 Mar 2022 21:58:57 +0100 Subject: [PATCH] subscriber: add `Filter::on_{new_span, enter, exit, close}` (#1973) Closes #1955 Call those methods in the `Layer` methods for `Filtered` and delegate them in the filter combinators ## Motivation Currently, if a `Filter` is going to implement dynamic filtering, it must do this by querying the wrapped Subscriber for the current span context. Since `Filter` only has callsite_enabled and enabled methods, a `Filter` implementation cannot track the span context internally, the way a Layer can. Unfortunately, this means that the current implementation of `EnvFilter` can only implement `Layer` (and provide global filtering). It cannot currently implement `Filter`, because it stores span context data internally. See https://github.com/tokio-rs/tracing/issues/1868 for details. ## Proposal We should add `on_new_span`, `on_enter`, `on_exit`, and `on_close` methods to `Filter`. This would allow implementing `Filter`s (such as `EnvFilter`) that internally track span states. --- .../src/filter/layer_filters/combinator.rs | 74 ++++++++++++++++++- .../src/filter/layer_filters/mod.rs | 13 +++- tracing-subscriber/src/layer/mod.rs | 36 ++++++++- 3 files changed, 117 insertions(+), 6 deletions(-) diff --git a/tracing-subscriber/src/filter/layer_filters/combinator.rs b/tracing-subscriber/src/filter/layer_filters/combinator.rs index f9d089ec..cf93c8c0 100644 --- a/tracing-subscriber/src/filter/layer_filters/combinator.rs +++ b/tracing-subscriber/src/filter/layer_filters/combinator.rs @@ -1,7 +1,11 @@ //! Filter combinators use crate::layer::{Context, Filter}; use std::{cmp, fmt, marker::PhantomData}; -use tracing_core::{subscriber::Interest, LevelFilter, Metadata}; +use tracing_core::{ + span::{Attributes, Id}, + subscriber::Interest, + LevelFilter, Metadata, +}; /// Combines two [`Filter`]s so that spans and events are enabled if and only if /// *both* filters return `true`. @@ -132,6 +136,30 @@ where // If either hint is `None`, return `None`. Otherwise, return the most restrictive. cmp::min(self.a.max_level_hint(), self.b.max_level_hint()) } + + #[inline] + fn on_new_span(&self, attrs: &Attributes<'_>, id: &Id, ctx: Context<'_, S>) { + self.a.on_new_span(attrs, id, ctx.clone()); + self.b.on_new_span(attrs, id, ctx) + } + + #[inline] + fn on_enter(&self, id: &Id, ctx: Context<'_, S>) { + self.a.on_enter(id, ctx.clone()); + self.b.on_enter(id, ctx); + } + + #[inline] + fn on_exit(&self, id: &Id, ctx: Context<'_, S>) { + self.a.on_exit(id, ctx.clone()); + self.b.on_exit(id, ctx); + } + + #[inline] + fn on_close(&self, id: Id, ctx: Context<'_, S>) { + self.a.on_close(id.clone(), ctx.clone()); + self.b.on_close(id, ctx); + } } impl Clone for And @@ -289,6 +317,30 @@ where // If either hint is `None`, return `None`. Otherwise, return the less restrictive. Some(cmp::max(self.a.max_level_hint()?, self.b.max_level_hint()?)) } + + #[inline] + fn on_new_span(&self, attrs: &Attributes<'_>, id: &Id, ctx: Context<'_, S>) { + self.a.on_new_span(attrs, id, ctx.clone()); + self.b.on_new_span(attrs, id, ctx) + } + + #[inline] + fn on_enter(&self, id: &Id, ctx: Context<'_, S>) { + self.a.on_enter(id, ctx.clone()); + self.b.on_enter(id, ctx); + } + + #[inline] + fn on_exit(&self, id: &Id, ctx: Context<'_, S>) { + self.a.on_exit(id, ctx.clone()); + self.b.on_exit(id, ctx); + } + + #[inline] + fn on_close(&self, id: Id, ctx: Context<'_, S>) { + self.a.on_close(id.clone(), ctx.clone()); + self.b.on_close(id, ctx); + } } impl Clone for Or @@ -356,6 +408,26 @@ where // TODO(eliza): figure this out??? None } + + #[inline] + fn on_new_span(&self, attrs: &Attributes<'_>, id: &Id, ctx: Context<'_, S>) { + self.a.on_new_span(attrs, id, ctx); + } + + #[inline] + fn on_enter(&self, id: &Id, ctx: Context<'_, S>) { + self.a.on_enter(id, ctx); + } + + #[inline] + fn on_exit(&self, id: &Id, ctx: Context<'_, S>) { + self.a.on_exit(id, ctx); + } + + #[inline] + fn on_close(&self, id: Id, ctx: Context<'_, S>) { + self.a.on_close(id, ctx); + } } impl Clone for Not diff --git a/tracing-subscriber/src/filter/layer_filters/mod.rs b/tracing-subscriber/src/filter/layer_filters/mod.rs index 313fef39..c77ad165 100644 --- a/tracing-subscriber/src/filter/layer_filters/mod.rs +++ b/tracing-subscriber/src/filter/layer_filters/mod.rs @@ -579,7 +579,9 @@ where fn on_new_span(&self, attrs: &span::Attributes<'_>, id: &span::Id, cx: Context<'_, S>) { self.did_enable(|| { - self.layer.on_new_span(attrs, id, cx.with_filter(self.id())); + let cx = cx.with_filter(self.id()); + self.filter.on_new_span(attrs, id, cx.clone()); + self.layer.on_new_span(attrs, id, cx); }) } @@ -610,19 +612,22 @@ where fn on_enter(&self, id: &span::Id, cx: Context<'_, S>) { if let Some(cx) = cx.if_enabled_for(id, self.id()) { - self.layer.on_enter(id, cx) + self.filter.on_enter(id, cx.clone()); + self.layer.on_enter(id, cx); } } fn on_exit(&self, id: &span::Id, cx: Context<'_, S>) { if let Some(cx) = cx.if_enabled_for(id, self.id()) { - self.layer.on_exit(id, cx) + self.filter.on_enter(id, cx.clone()); + self.layer.on_exit(id, cx); } } fn on_close(&self, id: span::Id, cx: Context<'_, S>) { if let Some(cx) = cx.if_enabled_for(&id, self.id()) { - self.layer.on_close(id, cx) + self.filter.on_close(id.clone(), cx.clone()); + self.layer.on_close(id, cx); } } diff --git a/tracing-subscriber/src/layer/mod.rs b/tracing-subscriber/src/layer/mod.rs index d041d7f7..8255aef4 100644 --- a/tracing-subscriber/src/layer/mod.rs +++ b/tracing-subscriber/src/layer/mod.rs @@ -457,7 +457,7 @@ //! .json() //! .with_writer(Arc::new(file)); //! Some(json_log) -//! } else { +//! } else { //! None //! }; //! @@ -1054,6 +1054,40 @@ feature! { fn max_level_hint(&self) -> Option { None } + + /// Notifies this filter that a new span was constructed with the given + /// `Attributes` and `Id`. + /// + /// By default, this method does nothing. `Filter` implementations that + /// need to be notified when new spans are created can override this + /// method. + fn on_new_span(&self, attrs: &span::Attributes<'_>, id: &span::Id, ctx: Context<'_, S>) { + let _ = (attrs, id, ctx); + } + + /// Notifies this filter that a span with the given ID was entered. + /// + /// By default, this method does nothing. `Filter` implementations that + /// need to be notified when a span is entered can override this method. + fn on_enter(&self, id: &span::Id, ctx: Context<'_, S>) { + let _ = (id, ctx); + } + + /// Notifies this filter that a span with the given ID was exited. + /// + /// By default, this method does nothing. `Filter` implementations that + /// need to be notified when a span is exited can override this method. + fn on_exit(&self, id: &span::Id, ctx: Context<'_, S>) { + let _ = (id, ctx); + } + + /// Notifies this filter that a span with the given ID has been closed. + /// + /// By default, this method does nothing. `Filter` implementations that + /// need to be notified when a span is closed can override this method. + fn on_close(&self, id: span::Id, ctx: Context<'_, S>) { + let _ = (id, ctx); + } } }