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

sentry-tracing: SentryLayer ignores event_filter if event_mapper is set #630

Open
jinohkang-theori opened this issue Dec 4, 2023 · 3 comments

Comments

@jinohkang-theori
Copy link

Environment

Linux x86_64

Steps to Reproduce

  1. Build SentryLayer with both event_filter and event_mapper set.
    • event_filter ignores tracing events below Level::ERROR.
    • event_mapper always returns EventMapping::Event(..).
  2. Emit a tracing event with Level::WARN (warn!()).

Expected Result

The warn event is discarded by the filter; the Sentry instance does not receive any events.

Actual Result

The warn event is not discarded by the filter; the Sentry instance receives the mapped event.

@Swatinem
Copy link
Member

Swatinem commented Dec 4, 2023

Yes, this is pretty much intentional. The event_mapper takes precedence over the filter, as you can also filter things in it, you just return an EventMapping::Ignore.

let item = match &self.event_mapper {
Some(mapper) => mapper(event, ctx),
None => match (self.event_filter)(event.metadata()) {

Though I admit the docs might not fully reflect this.

@jinohkang-theori
Copy link
Author

Yes, this is pretty much intentional. The event_mapper takes precedence over the filter, as you can also filter things in it, you just return an EventMapping::Ignore.

let item = match &self.event_mapper {
Some(mapper) => mapper(event, ctx),
None => match (self.event_filter)(event.metadata()) {

Though I admit the docs might not fully reflect this.

Thank you for your swift response! I appreciate it, really.

I believe this behavior is part of stable API and guaranteed to stay the same in the foreseeable future. Is this correct? If this is the case, I could submit a PR to document this explicitly.

@Swatinem
Copy link
Member

Swatinem commented Dec 4, 2023

I believe this behavior is part of stable API and guaranteed to stay the same in the foreseeable future. Is this correct? If this is the case, I could submit a PR to document this explicitly.

Yes, we wouldn’t want to change this behavior suddenly. PRs to improve docs are always appreciated ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants