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.

Sign up for GitHub

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

Jump to bottom

It is impossible to dis-/en- able event processing based on event field contents in tracing_subsriber::Subsribe #2007

Closed
CAD97 opened this issue Mar 19, 2022 · 0 comments · Fixed by #2008
Closed

It is impossible to dis-/en- able event processing based on event field contents in tracing_subsriber::Subsribe #2007

CAD97 opened this issue Mar 19, 2022 · 0 comments · Fixed by #2008

Comments

@CAD97
Copy link
Contributor

CAD97 commented Mar 19, 2022

This means it is impossible to implement features like e.g. env_logger's regex log message filtering in a separate Subscribe layer; it must be implemented in each and every layer which processes Subscribe::on_event.

Feature Request

Allow Subscribe layers in a Layered Collector to determine if lower layers should receive the on_event callback.

Crates

This effects exclusively tracing_subscriber. The most straightforward implementation is a breaking change.

Motivation

I'm working on an initiative (tracing-filter) to provide an alternative to EnvFilter which offers a more principled/structured way of doing tracing event filtering. Filtering on event field contents is a natural ability to have, and is required for feature parity with env_logger (transparent compatibility with which is a goal for tracing-filter, and not provided by EnvFilter, as it simply (at best) errors on /filter directives).

Proposal

Change the signature of Subscribe::on_event to

fn Subscribe::<C: Collect>::on_event(&self, event: &Event<'_>, ctx: Context<'_, C>) -> std::ops::ControlFlow<()>;

Call on_event starting from the top layer. Similar to how Subscribe::enabled is handled, if on_event returns

  • ControlFlow::Continue, continue and call the next layer's on_event.
  • ControlFlow::Break, stop calling on_event.

Drawbacks

  • A slight complexity increase.
  • Breaking change.
    • Mitigable with making this a new function defaulted to call the old one, for a complexity increase.
    • Mitigable by bundling with tracing_core 0.2.
  • MSRV 1.55.0.
  • Unknown unknowns.

Alternatives

  • Status quo; just don't allow layered filtering based on the contents of an event, and only filter on events' Metadata.
  • Add a separate Subscribe::event_enabled rather than adding this functionality to on_event.
    • Drawback: potential duplication of logic between event_enabled and on_event.
  • Go even further: Subscribe::on_new_span, on_record, on_follows_from, on_event, on_enter, on_exit, on_close, and on_id_change could all theoretically return ControlFlow and control whether lower layers see these callbacks. The author of this issue has not considered the ramifications of doing so, beyond that of on_event.
    • Of these, on_new_span and on_record seem the most promising; on_enter/exit/close seem potentially problematic; on_follows_from and on_id_change very much so.
@CAD97 CAD97 mentioned this issue Mar 21, 2022
Merged
hawkw pushed a commit that referenced this issue Jun 21, 2022
@CAD97
core, subscriber: add event_enabled to filter events on fields (#2008)
acb078f 
## Motivation

Allow filter layers to filter on the contents of events (see #2007).

## Solution

This branch adds a new `Collect::event_enabled` method, taking an
`Event` and returning `bool`. This is called before the `Collect::event`
method, and if it returns `false`, `Collect::event` is not called.

For simple collectors (e.g. not using `Subscribe` layers), the
`event_enabled` method is not particulary necessary, as the collector
can just skip the `event` call. However, this branch also adds a new
`Subscribe::event_enabled` method, with the signature:
```rust
fn event_enabled(&self, event: &Event<'_>, ctx: Context<'_, C>) -> bool;
```

This is called before `Subscribe::on_event`, and if `event_enabled`
returns `false`, `on_event` is not called (nor is `Collect::event`).
This allows filter subscribers to filter out an `Event` based on its
fields.

Closes #2007
hawkw pushed a commit that referenced this issue Jun 22, 2022
@CAD97 @hawkw
core, subscriber: add event_enabled to filter events on fields (#2008)
8dd8151 
## Motivation

Allow filter layers to filter on the contents of events (see #2007).

## Solution

This branch adds a new `Subscriber::event_enabled` method, taking an
`Event` and returning `bool`. This is called before the
`Subscriber::event` method, and if it returns `false`,
`Subscriber::event` is not called.

For simple subscriber (e.g. not using `Layer`s), the `event_enabled`
method is not particulary necessary, as the subscriber can just skip the
`event` call. However, this branch also adds a new
`Layer::event_enabled` method, with the signature:
```rust
fn event_enabled(&self, event: &Event<'_>, ctx: Context<'_, S>) -> bool;
```

This is called before `Layer::on_event`, and if `event_enabled`
returns `false`, `on_event` is not called (nor is `Subscriber::event`).
This allows filter `Layer`s to filter out an `Event` based on its
fields.

Closes #2007
hawkw pushed a commit that referenced this issue Jun 22, 2022
@CAD97 @hawkw
core, subscriber: add event_enabled to filter events on fields (#2008)
a06c85f 
## Motivation

Allow filter layers to filter on the contents of events (see #2007).

## Solution

This branch adds a new `Subscriber::event_enabled` method, taking an
`Event` and returning `bool`. This is called before the
`Subscriber::event` method, and if it returns `false`,
`Subscriber::event` is not called.

For simple subscriber (e.g. not using `Layer`s), the `event_enabled`
method is not particulary necessary, as the subscriber can just skip the
`event` call. However, this branch also adds a new
`Layer::event_enabled` method, with the signature:
```rust
fn event_enabled(&self, event: &Event<'_>, ctx: Context<'_, S>) -> bool;
```

This is called before `Layer::on_event`, and if `event_enabled`
returns `false`, `on_event` is not called (nor is `Subscriber::event`).
This allows filter `Layer`s to filter out an `Event` based on its
fields.

Closes #2007
hawkw pushed a commit that referenced this issue Jun 22, 2022
@CAD97 @hawkw
core, subscriber: add event_enabled to filter events on fields (#2008)
907604c 
## Motivation

Allow filter layers to filter on the contents of events (see #2007).

## Solution

This branch adds a new `Subscriber::event_enabled` method, taking an
`Event` and returning `bool`. This is called before the
`Subscriber::event` method, and if it returns `false`,
`Subscriber::event` is not called.

For simple subscriber (e.g. not using `Layer`s), the `event_enabled`
method is not particulary necessary, as the subscriber can just skip the
`event` call. However, this branch also adds a new
`Layer::event_enabled` method, with the signature:
```rust
fn event_enabled(&self, event: &Event<'_>, ctx: Context<'_, S>) -> bool;
```

This is called before `Layer::on_event`, and if `event_enabled`
returns `false`, `on_event` is not called (nor is `Subscriber::event`).
This allows filter `Layer`s to filter out an `Event` based on its
fields.

Closes #2007
hawkw pushed a commit that referenced this issue Jun 22, 2022
@CAD97 @hawkw
core, subscriber: add event_enabled to filter events on fields (#2008)
f42e7e6 
## Motivation

Allow filter layers to filter on the contents of events (see #2007).

## Solution

This branch adds a new `Subscriber::event_enabled` method, taking an
`Event` and returning `bool`. This is called before the
`Subscriber::event` method, and if it returns `false`,
`Subscriber::event` is not called.

For simple subscriber (e.g. not using `Layer`s), the `event_enabled`
method is not particulary necessary, as the subscriber can just skip the
`event` call. However, this branch also adds a new
`Layer::event_enabled` method, with the signature:
```rust
fn event_enabled(&self, event: &Event<'_>, ctx: Context<'_, S>) -> bool;
```

This is called before `Layer::on_event`, and if `event_enabled`
returns `false`, `on_event` is not called (nor is `Subscriber::event`).
This allows filter `Layer`s to filter out an `Event` based on its
fields.

Closes #2007
kaffarell pushed a commit to kaffarell/tracing that referenced this issue May 22, 2024
@CAD97 @kaffarell
core, subscriber: add event_enabled to filter events on fields (tok…
8e207de 
…io-rs#2008)

## Motivation

Allow filter layers to filter on the contents of events (see tokio-rs#2007).

## Solution

This branch adds a new `Subscriber::event_enabled` method, taking an
`Event` and returning `bool`. This is called before the
`Subscriber::event` method, and if it returns `false`,
`Subscriber::event` is not called.

For simple subscriber (e.g. not using `Layer`s), the `event_enabled`
method is not particulary necessary, as the subscriber can just skip the
`event` call. However, this branch also adds a new
`Layer::event_enabled` method, with the signature:
```rust
fn event_enabled(&self, event: &Event<'_>, ctx: Context<'_, S>) -> bool;
```

This is called before `Layer::on_event`, and if `event_enabled`
returns `false`, `on_event` is not called (nor is `Subscriber::event`).
This allows filter `Layer`s to filter out an `Event` based on its
fields.

Closes tokio-rs#2007
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add Subscribe::event_enabled to conditionally dis/enable events based on fields CAD97/tracing
1 participant
@CAD97