From db869b95e18d75df4a252a3d666cba23783a4ac6 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 `Subscribe` 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 subscriber can. Unfortunately, this means that the current implementation of `EnvFilter` can only implement `Subscribe` (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. --- .../filter/subscriber_filters/combinator.rs | 74 ++++++++++++++++++- .../src/filter/subscriber_filters/mod.rs | 14 ++-- tracing-subscriber/src/subscribe/mod.rs | 34 +++++++++ 3 files changed, 116 insertions(+), 6 deletions(-) diff --git a/tracing-subscriber/src/filter/subscriber_filters/combinator.rs b/tracing-subscriber/src/filter/subscriber_filters/combinator.rs index 55bd250a42..730f1abb2a 100644 --- a/tracing-subscriber/src/filter/subscriber_filters/combinator.rs +++ b/tracing-subscriber/src/filter/subscriber_filters/combinator.rs @@ -1,7 +1,11 @@ //! Filter combinators use crate::subscribe::{Context, Filter}; use std::{cmp, fmt, marker::PhantomData}; -use tracing_core::{collect::Interest, LevelFilter, Metadata}; +use tracing_core::{ + collect::Interest, + span::{Attributes, Id}, + 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/subscriber_filters/mod.rs b/tracing-subscriber/src/filter/subscriber_filters/mod.rs index dfcb2abe60..cb90397861 100644 --- a/tracing-subscriber/src/filter/subscriber_filters/mod.rs +++ b/tracing-subscriber/src/filter/subscriber_filters/mod.rs @@ -612,8 +612,9 @@ where fn on_new_span(&self, attrs: &span::Attributes<'_>, id: &span::Id, cx: Context<'_, C>) { self.did_enable(|| { - self.subscriber - .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.subscriber.on_new_span(attrs, id, cx); }) } @@ -644,19 +645,22 @@ where fn on_enter(&self, id: &span::Id, cx: Context<'_, C>) { if let Some(cx) = cx.if_enabled_for(id, self.id()) { - self.subscriber.on_enter(id, cx) + self.filter.on_enter(id, cx.clone()); + self.subscriber.on_enter(id, cx); } } fn on_exit(&self, id: &span::Id, cx: Context<'_, C>) { if let Some(cx) = cx.if_enabled_for(id, self.id()) { - self.subscriber.on_exit(id, cx) + self.filter.on_enter(id, cx.clone()); + self.subscriber.on_exit(id, cx); } } fn on_close(&self, id: span::Id, cx: Context<'_, C>) { if let Some(cx) = cx.if_enabled_for(&id, self.id()) { - self.subscriber.on_close(id, cx) + self.filter.on_close(id.clone(), cx.clone()); + self.subscriber.on_close(id, cx); } } diff --git a/tracing-subscriber/src/subscribe/mod.rs b/tracing-subscriber/src/subscribe/mod.rs index 7459c6854e..72684456de 100644 --- a/tracing-subscriber/src/subscribe/mod.rs +++ b/tracing-subscriber/src/subscribe/mod.rs @@ -965,6 +965,40 @@ pub trait Filter { 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); + } } /// Extension trait adding a `with(Subscribe)` combinator to types implementing