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 `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 #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 Mar 8, 2022
1 parent cc1f155 commit 2d27332
Show file tree
Hide file tree
Showing 3 changed files with 117 additions and 6 deletions.
74 changes: 73 additions & 1 deletion tracing-subscriber/src/filter/layer_filters/combinator.rs
Original file line number Diff line number Diff line change
@@ -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`.
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
13 changes: 9 additions & 4 deletions tracing-subscriber/src/filter/layer_filters/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
})
}

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

Expand Down
36 changes: 35 additions & 1 deletion tracing-subscriber/src/layer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -457,7 +457,7 @@
//! .json()
//! .with_writer(Arc::new(file));
//! Some(json_log)
//! } else {
//! } else {
//! None
//! };
//!
Expand Down Expand Up @@ -1054,6 +1054,40 @@ feature! {
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);
}
}
}

Expand Down

0 comments on commit 2d27332

Please sign in to comment.