Specify behaviour of session updates on dropped events

[adinauer]

The SDKs behave differently when it comes to session updates for dropped events.

There are four ways of dropping an event:

• sampleRate
• ignoredExceptionForTypes, also called ignore_errors
• eventProcessor (scoped and global)
• beforeSend

python

In python all of them drop the event before updating the session. The order of execution in python is:

• ignore_errors
• sampleRate
• eventProcessor (scoped) - called error_processors
• eventProcessor (global) - called event_processors
• beforeSend
• --- session update --- never reached for dropped events

Java

The behaviour and order in Java differs from the python implementation:

• eventProcessor (scoped) - early return on drop
• eventProcessor (global) - event set to null, creates and sends envelope if attachments are available, no session update counted or sent
• --- session update --- note: this update may not make it to the server immediately
• sampleRate - event set to null, sends attachments and session update
• ignoredExceptionForTypes - semi early return, session has already been updated but is not being sent immediately
• beforeSend - event set to null, sends attachments and session update

Link to issue in sentry-java: getsentry/sentry-java#1916

JS

The JS SDK does not update the session in any of the cases. The order is:

• sampleRate
• eventProcessor - I'm not sure on the order of global vs scoped here: https://github.com/getsentry/sentry-javascript/blob/249302aa8fb30648528928a3629a7ea2349de62d/packages/hub/src/scope.ts#L441 My guess would be: global happens before scoped.
• beforeSend
• --- session update ---

Note: ignoredExceptionForTypes doesn't seem to be available in JS.

Other SDKs

Cocoa (thanks @philipphofmann), native (thanks @Swatinem) and Rust (thanks @Swatinem) update the session

Desired behaviour / suggestions

• Current implementation in python as a basis (i.e. don't update session for dropped events)
• @bruno-garcia suggested, we update the session for sampleRate as the session should reflect reality not dropped data - If it wasn't an error, don't count it.
• @untitaker on whether a dropped event should update the session:
  ** beforeSend - We don't really know; usecases are overloaded
  ** sampleRate - SDK option: IMO yes
  ** ignoredExceptionForTypes - option: IMO no
  ** eventProcessor - option: IMO no, but it depends who can set an event processor. is that private API? do we know what it's used for?
• @untitaker suggested we check ignoredExceptionForTypes before sampleRate

Questions:

• Seems there is some aggreement on only updating the session for sampleRate drops and nothing else. Should we change the implementations to match this?
• Do we want to align filter order across SDKs?

[adinauer]

Another question: If we send a session update for every error dropped by sampleRate, we're generating a lot of traffic. @bruno-garcia suggested, we only send for errors (not crashes) if the count moves from 0 to 1 for errors since it's only important if a session is marked as errored but not how many errors there were. Should we change the implementation on the Java SDK (and others that have the same issue)?

[bruno-garcia]

Put Platform Java so it goes on our board (since you're driving this) but it also affects other platforms. So not just Java.

[philipphofmann]

Not updating a session when the SDK drops events creates different release health statistics depending on the sample rate or beforeSend. For example, a small sample rate would lead to healthier release numbers than not sampling any events unless we also sample sessions. Therefore, I think we must always update the session no matter if an event gets dropped or not. As @bruno-garcia pointed out, release health should reflect reality no matter if you drop data or not.

[adinauer]

@philipphofmann reality in the sense of "is this a problem i don't care about" vs "i care about the event but want to save quota" is what I understood when @bruno-garcia said it. There was a discussion about whether we know which of the reasons applies for each of the filtering options (see the @untitaker section in Desired behaviour / suggestions above).

sampleRate was the one where people most likely care about the dropped event so that's why it should update the session counts. Also ignoredExceptionForTypes should be checked before sampleRate so events someone doesn't care about don't update the session counts due to being dropped by sampleRate.

[philipphofmann]

Ah, now I get your point. So when the user wants to save quota, the SDK must update the session state if they filter because they don't care the SDK must not.

[adinauer]

@philipphofmann that would be the best case but we can't say for sure without providing a way for developers to tell us why they dropped it. Maybe we can come back to this idea once client reports proof useful enough to justify e.g. new beforeSend / eventProcessor interfaces that allow returning the drop reason?

[philipphofmann]

I'm not sure if it makes sense to add an interface to let developers return a reason why they dropped something. I doubt that this is worth the effort.