ref(session): align processing sequence in sentry__capture_event() with docs #729
Conversation
processing sequence of sentry__capture_event() to align with the docs/spec (https://develop.sentry.dev/sdk/sessions/#filter-order).
supervacuus
force-pushed
the
ref/session_sampling_capture_transport
branch
from
89e2201
to
2ed455f
Compare
|
As you suggested, we can make this behavior more clear in the fn-level docs, and even link to the docs that specify this behavior. We should also call this change out in the changelog. |
with the cross-SDK session filter order (https://develop.sentry.dev/sdk/sessions/#filter-order).
|
The failing test is unrelated to my latest commit (which was only documentation), but @Swatinem, do you have any immediate suggestions? Otherwise, I would try to figure out how to fix this. |
The new virtual-environment image https://github.com/actions/virtual-environments/releases/tag/macOS-11%2F20220627.1 defaults clang and clang++ in the PATH to 13.0.0 which currently fails in the CI because there is no "detect_leaks" support. Clang/LLVM 13.0.1 is also packaged in that image but only accessible via '$(brew --prefix llvm@13)/bin/clang'. It is unclear whether this fixes the issue, but in the previous image we used 13.0.1 successfully with ASAN.
|
That is most unfortunate :-( Plus, someone with permissions has to update the branch protection rules once this is ready to merge @ashwoods |
Fixes #707.
The specified items in the filter-order apply to sentry-native accordingly:
1. Check for ignored exception types (a.k.a ignore_errors)2. Apply scoped event_processor (a.k.a error_processor)sentry__prepare_event():3. Apply global event_processorsentry__prepare_event():4. Apply before_sendsentry__capture_event():5. Update the session if an event made it this farsentry__prepare_event():6. Apply sampling rateThis means, in the current implementation, the sole conflict with the proposed filter order is in the application of the sampling at the beginning of the event processing and thus before
before_sendand the session update. The remaining implemented filters are in the order suggested in the docs.This PR moves the sampling application right before the transport invocation. Since this is - essentially - a breaking change (e.g.
before_sendis no longer affected by sampling and thus called on every event capture) I think we should reflect this in the API docs too. Please advise on where a good place for this might be (beyond the changelog).