Session immutability constraints are violated

[Lms24]

Is there an existing issue for this?

• I have checked for existing issues https://github.com/getsentry/sentry-javascript/issues
• I have reviewed the documentation https://docs.sentry.io/
• I am using the latest SDK release https://github.com/getsentry/sentry-javascript/releases

How do you use Sentry?

Sentry Saas (sentry.io)

Which SDK are you using?

@sentry/browser

SDK Version

latest

Framework Version

N/A

Link to Sentry event

No response

Reproduction Example/SDK Setup

Sentry.init({...});

setTimeout(() => {
 Sentry.setUser({...});
}, 2000)

Steps to Reproduce

see above

Expected Result

Sessions' attributes like did, release, environment or timestamp MUST NOT be mutated after a session (identified by sid) was sent for the first time. Doing so, causes our session metrics extraction to incorrectly categorize future session updates, which leads to double-counted sessions and thus incorrect release health metrics (e.g. healthy, crashed, crash-free sessions).

See develop spec.

Actual Result

Today, our SDK does not enforce immutability. On the contrary, a recent change (my mistake, see #19341) even explicitly sends a session update after a user was set on the isolation scope. Though worth noting, anyone could call updateSession before, causing indirect mutations for sessions when we send status updates (exited, crashed, etc).

Additional Context

Ideally, we could update a session safely but sadly this is not supported by our current (and possibly future) data storage layer.

Solution Brainstorm

There are several strategies on how we can long-term fix this, but all of them come with problems and tradeoffs.
Important: Every proposed strategy includes enforcing immutability (e.g. in updateSession), treating session attributes as "frozen" once the first envelope was sent for a specific sid.

1. Do nothing about missing data (besides enforced immutability). The cheapest and spec-correct option. Impacts "Crash free users" and related metrics relying on did. This was largely the behaviour prior to fix(browser): Ensure user id is consistently added to sessions #19341 but was raised by users as a bug (via support): User information no available in the release session #19317

2. Defer sending of first session by grace period (e.g. 5sec). Any data (like setUser) set in the meantime will be included in the first envelope. Still leaves a chance for too late data but increases chances. Largest con: Any kind of short page visits are not tracked at all anymore. Could decrease overall session count and leave some sessions completely untracked.

3. Restart session on user change. This is what the spec suggests but in reality, it leads to 1. double counted "sessions" (one very short one without did, one longer one with did where both actually resemble the same session)

4. Defer sending + Restart (combines the pros of 2 and 3) but still leaves room for untracked sessions as well as double-counted sessions. The latter to a smaller extent than 3 alone.

5. Telemetry-driven first session send: Only send the first session envelope once the first telemetry item (e.g. error, transaction, log, metric) was sent. Might lead to longer grace periods than 2, but might also do the opposite. Leaves a much higher chance for untracked sessions, heavily skewing healthy session metrics. Especially for errors-only users.

6. Provide API for users to explicitly force our SDK to wait for sending a session until users deem the session ready to be sent. This could be a Promise<User> if we want to make it explicit, or a () => Promise<void> callback. In either case we would await a promise and only afterwards send a session. I could see us implementing this down the line as a custom strategy but we need one of 1-5 to handle the default case.

Priority

_{React with 👍 to help prioritize this issue. Please use comments to provide useful context, avoiding +1 or me too, to help us triage it.}

[linear-code]

JS-1929

[Lms24]

So in an unexpected turn of events, @Dav1dde and I discovered that the current SDK behaviour most likely works as intended and the spec is wrong too restrictively formulated (😅).

Next steps:

• verify that the numbers look plausible with the current implementation
• adjust develop docs and loosen the constraint if this is the case