Skip to content

Commit

Permalink
subscriber: add Filter::on_{new_span, enter, exit, close} (#1973)
Browse files Browse the repository at this point in the history
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 #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.
  • Loading branch information
tfreiberg-fastly authored and hawkw committed Mar 24, 2022
1 parent a572fe4 commit 4831948
Show file tree
Hide file tree
Showing 3 changed files with 116 additions and 6 deletions.
74 changes: 73 additions & 1 deletion tracing-subscriber/src/filter/subscriber_filters/combinator.rs
Original file line number Diff line number Diff line change
@@ -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`.
Expand Down Expand Up @@ -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<A, B, S> Clone for And<A, B, S>
Expand Down Expand Up @@ -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<A, B, S> Clone for Or<A, B, S>
Expand Down Expand Up @@ -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<A, S> Clone for Not<A, S>
Expand Down
14 changes: 9 additions & 5 deletions tracing-subscriber/src/filter/subscriber_filters/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
})
}

Expand Down Expand Up @@ -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);
}
}

Expand Down
34 changes: 34 additions & 0 deletions tracing-subscriber/src/subscribe/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -965,6 +965,40 @@ pub trait Filter<S> {
fn max_level_hint(&self) -> Option<LevelFilter> {
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
Expand Down

0 comments on commit 4831948

Please sign in to comment.