-
Notifications
You must be signed in to change notification settings - Fork 254
Bug 2013658 - Register for multiple tracing events #7202
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
base: main
Are you sure you want to change the base?
Conversation
| warn!(target: "third_target", extra=-4, "event message3"); | ||
| info!(target: "third_target", extra=-5, "event message (should be filtered)"); | ||
| // Matches both for first_target and for the min-level, it should only be received once by | ||
| // the sink |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the main reason for this change. On Desktop, when I went to wire up the error-reporting code, I had issues because both the event's target matched and the min-level matched, so we were getting duplicate events and would be reporting each error twice.
e16c768 to
5eb10fd
Compare
mhammond
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this looks great! I think you should get a patch up for the breaking changes on desktop before landing this though given how it might end up treading on Ted's toes with the work he's doing around the vendoring.
6dc67bb to
be52154
Compare
Added a new `register_event_sink` method that registers event sinks using the new `EventSinkSpecification` struct. This is general enough that we can use it to implement the older registration methods. This is a breaking change, but Desktop is the only platform that's will be affected. We can resolve the breakage when we do the next vendor.
That makes sense to me, I just put up https://phabricator.services.mozilla.com/D282661 for that. |
Added a new
register_event_sinkmethod that registers event sinks using the newEventSinkSpecificationstruct. This is general enough that we can use it to implement the older registration methods.This is a breaking change, but Desktop is the only platform that's will be affected. We can resolve the breakage when we do the next vendor.
Pull Request checklist
[ci full]to the PR title.