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

Add Subscribe::event_enabled to conditionally dis/enable events based on fields #2008

Merged
merged 12 commits into from Jun 21, 2022
26 changes: 26 additions & 0 deletions tracing-core/src/collect.rs
Expand Up @@ -57,6 +57,9 @@ use core::ptr::NonNull;
/// See also the [documentation on the callsite registry][cs-reg] for details
/// on [`register_callsite`].
///
/// - [`event_enabled`] is called once before every [`event`] is recorded. This
/// can be used to implement filtering on events once their field values are
/// known but before any processing is done in `event`.
CAD97 marked this conversation as resolved.
Show resolved Hide resolved
/// - [`clone_span`] is called every time a span ID is cloned, and [`try_close`]
/// is called when a span ID is dropped. By default, these functions do
/// nothing. However, they can be used to implement reference counting for
Expand All @@ -72,6 +75,8 @@ use core::ptr::NonNull;
/// [`clone_span`]: Collect::clone_span
/// [`try_close`]: Collect::try_close
/// [cs-reg]: crate::callsite#registering-callsites
/// [`event`]: Collect::event
/// [`event_enabled`]: Collect::event_enabled
pub trait Collect: 'static {
// === Span registry methods ==============================================

Expand Down Expand Up @@ -288,6 +293,17 @@ pub trait Collect: 'static {
/// follow from _b_), it may silently do nothing.
fn record_follows_from(&self, span: &span::Id, follows: &span::Id);

/// Determine if an [`Event`] should be recorded.
///
/// By default, this returns `true` and collectors can filter events in
/// [`event`][Self::event] without any penalty. However, when `event` is
/// more complicated, this can be used to determine if `event` should be
/// called at all, separating out the decision from the processing.
fn event_enabled(&self, event: &Event<'_>) -> bool {
let _ = event;
true
}
hawkw marked this conversation as resolved.
Show resolved Hide resolved

/// Records that an [`Event`] has occurred.
///
/// This method will be invoked when an Event is constructed by
Expand Down Expand Up @@ -685,6 +701,11 @@ impl Collect for alloc::boxed::Box<dyn Collect + Send + Sync + 'static> {
self.as_ref().record_follows_from(span, follows)
}

#[inline]
fn event_enabled(&self, event: &Event<'_>) -> bool {
self.as_ref().event_enabled(event)
}

#[inline]
fn event(&self, event: &Event<'_>) {
self.as_ref().event(event)
Expand Down Expand Up @@ -756,6 +777,11 @@ impl Collect for alloc::sync::Arc<dyn Collect + Send + Sync + 'static> {
self.as_ref().record_follows_from(span, follows)
}

#[inline]
fn event_enabled(&self, event: &Event<'_>) -> bool {
self.as_ref().event_enabled(event)
}

#[inline]
fn event(&self, event: &Event<'_>) {
self.as_ref().event(event)
Expand Down
5 changes: 4 additions & 1 deletion tracing-core/src/dispatch.rs
Expand Up @@ -682,7 +682,10 @@ impl Dispatch {
/// [`event`]: super::collect::Collect::event
#[inline]
pub fn event(&self, event: &Event<'_>) {
self.collector().event(event)
let collector = self.collector();
if collector.event_enabled(event) {
CAD97 marked this conversation as resolved.
Show resolved Hide resolved
collector.event(event);
}
}

/// Records that a span has been can_enter.
Expand Down
5 changes: 5 additions & 0 deletions tracing-subscriber/src/fmt/mod.rs
Expand Up @@ -393,6 +393,11 @@ where
self.inner.record_follows_from(span, follows)
}

#[inline]
fn event_enabled(&self, event: &Event<'_>) -> bool {
self.inner.event_enabled(event)
}

#[inline]
fn event(&self, event: &Event<'_>) {
self.inner.event(event);
Expand Down
5 changes: 5 additions & 0 deletions tracing-subscriber/src/reload.rs
Expand Up @@ -93,6 +93,11 @@ where
try_lock!(self.inner.read()).on_follows_from(span, follows, ctx)
}

#[inline]
fn event_enabled(&self, event: &Event<'_>, ctx: subscribe::Context<'_, C>) -> bool {
try_lock!(self.inner.read(), else return false).event_enabled(event, ctx)
}

#[inline]
fn on_event(&self, event: &Event<'_>, ctx: subscribe::Context<'_, C>) {
try_lock!(self.inner.read()).on_event(event, ctx)
Expand Down
21 changes: 21 additions & 0 deletions tracing-subscriber/src/subscribe/layered.rs
Expand Up @@ -138,6 +138,16 @@ where
self.subscriber.on_follows_from(span, follows, self.ctx());
}

fn event_enabled(&self, event: &Event<'_>) -> bool {
if self.subscriber.event_enabled(event, self.ctx()) {
// if the outer subscriber enables the event, ask the inner collector.
self.inner.event_enabled(event)
} else {
// otherwise, the event is disabled by this subscriber
false
}
}

fn event(&self, event: &Event<'_>) {
self.inner.event(event);
self.subscriber.on_event(event, self.ctx());
Expand Down Expand Up @@ -280,6 +290,17 @@ where
self.subscriber.on_follows_from(span, follows, ctx);
}

#[inline]
fn event_enabled(&self, event: &Event<'_>, ctx: Context<'_, C>) -> bool {
if self.subscriber.event_enabled(event, ctx.clone()) {
// if the outer subscriber enables the event, ask the inner collector.
self.inner.event_enabled(event, ctx)
} else {
// otherwise, the event is disabled by this subscriber
false
}
}

#[inline]
fn on_event(&self, event: &Event<'_>, ctx: Context<'_, C>) {
self.inner.on_event(event, ctx.clone());
Expand Down
62 changes: 62 additions & 0 deletions tracing-subscriber/src/subscribe/mod.rs
Expand Up @@ -686,7 +686,31 @@ pub(crate) mod tests;
/// be composed together with other subscribers to build a [collector]. See the
/// [module-level documentation](crate::subscribe) for details.
///
/// # Enabling interest
///
/// Whenever an tracing event (or span) is emitted, it goes through a number of
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

might be nice if "event" and "span" linked to the tracing docs, but it's not a big deal --- people reading this documentation probably know what those are by now.

/// steps to determine how and how much it should be processed. The earlier an
/// event is disabled, the less work has to be done to process the event, so
/// subscribers should attempt to provide accurate information as early as
CAD97 marked this conversation as resolved.
Show resolved Hide resolved
/// possible. Note that this determines **global** interest for the entire stack
/// of subscribers; a subscriber that wants to ignore certain events/spans
/// should just ignore them in the notifications. In order, each event checks:
///
/// - [`register_callsite`], once per callsite (roughly: once per time that
/// `event!` or `span!` is written in the source code; this is cached at the
/// callsite). See [`Collect::register_callsite`] for how this behaves.
CAD97 marked this conversation as resolved.
Show resolved Hide resolved
/// - [`enabled`], once per emitted event (or span). This is the main point to
CAD97 marked this conversation as resolved.
Show resolved Hide resolved
/// (globally) filter events based on their [`Metadata`]. If an event can be
/// disabled by just its structure, it should be, as this allows construction
CAD97 marked this conversation as resolved.
Show resolved Hide resolved
/// of the actual `Event`/`Span` to be skipped.
/// - For events only (and not spans), [`event_enabled`] is called just before
/// processing the event. This gives subscribers one last chance to say that
/// an event should be filtered out, now that the event's fields are known.
CAD97 marked this conversation as resolved.
Show resolved Hide resolved
///
/// [collector]: tracing_core::Collect
/// [`enabled`]: Self::enabled
/// [`event_enabled`]: Self::event_enabled
/// [`register_callsite`]: Self::register_callsite
#[cfg_attr(docsrs, doc(notable_trait))]
pub trait Subscribe<C>
where
Expand Down Expand Up @@ -827,6 +851,31 @@ where
// seems like a good future-proofing measure as it may grow other methods later...
fn on_follows_from(&self, _span: &span::Id, _follows: &span::Id, _ctx: Context<'_, C>) {}

/// Called before [`on_event`], to determine if `on_event` should be called.
///
CAD97 marked this conversation as resolved.
Show resolved Hide resolved
/// <div class="example-wrap" style="display:inline-block">
/// <pre class="ignore" style="white-space:normal;font:inherit;">
///
/// **Note**: This method determines whether an event is globally enabled,
/// *not* whether the individual subscriber will be notified about the
/// event. This is intended to be used by layers that implement filtering
/// for the entire stack. Layers which do not wish to be notified about
CAD97 marked this conversation as resolved.
Show resolved Hide resolved
/// certain events but do not wish to globally disable them should ignore
/// those events in their [on_event][Self::on_event].
///
/// </pre></div>
///
/// See [the trait-level documentation] for more information on filtering
/// with `Subscriber`s.
///
/// [`on_event`]: Self::on_event
/// [`Interest`]: tracing_core::Interest
/// [the trait-level documentation]: #filtering-with-subscribers
#[inline] // collapse this to a constant please mrs optimizer
fn event_enabled(&self, _event: &Event<'_>, _ctx: Context<'_, C>) -> bool {
true
}

/// Notifies this subscriber that an event has occurred.
fn on_event(&self, _event: &Event<'_>, _ctx: Context<'_, C>) {}

Expand Down Expand Up @@ -1455,6 +1504,14 @@ where
}
}

#[inline]
fn event_enabled(&self, event: &Event<'_>, ctx: Context<'_, C>) -> bool {
match self {
Some(ref inner) => inner.event_enabled(event, ctx),
None => false,
}
}

#[inline]
fn on_event(&self, event: &Event<'_>, ctx: Context<'_, C>) {
if let Some(ref inner) = self {
Expand Down Expand Up @@ -1534,6 +1591,11 @@ macro_rules! subscriber_impl_body {
self.deref().on_follows_from(span, follows, ctx)
}

#[inline]
fn event_enabled(&self, event: &Event<'_>, ctx: Context<'_, C>) -> bool {
self.deref().event_enabled(event, ctx)
}

#[inline]
fn on_event(&self, event: &Event<'_>, ctx: Context<'_, C>) {
self.deref().on_event(event, ctx)
Expand Down