Skip to content

feat(rum): end the session when new settings decide its fate - #31

Closed
Fiona2016 wants to merge 3 commits into
feat/remote-sampling-configurationfrom
feat/remote-config-session-restart
Closed

feat(rum): end the session when new settings decide its fate#31
Fiona2016 wants to merge 3 commits into
feat/remote-sampling-configurationfrom
feat/remote-config-session-restart

Conversation

@Fiona2016

Copy link
Copy Markdown
Collaborator

Stacked on feat/remote-sampling-configuration, and targets that branch. Review only the two
commits on top of it.

Why

Settings published from the console apply to sessions created after they arrive, and to nothing
else. A session ends after fifteen minutes without activity, or four hours outright, so for a
visitor who never goes idle — a support console, a trading screen, a wallboard — the change can be
hours away. Those are exactly the visitors who produce the most data, so a change made to stop a
flood reaches the flood last.

What it does

Three changes no longer wait, and they are exactly the three whose effect on the running session
can be told without drawing again:

Change Current session Action
session sample rate → 0 being collected end it
session sample rate → 100 not being collected end it
defaultPrivacyLevel gets stricter either end it
any other rate either leave it
only the custom bag, the trace rate or the replay rate changed either leave it

Ending is the whole point, rather than flipping the running session: the old session is collected
to its end as it was begun, so no replay is masked in one half and plain in the other, and no
session is invented that begins in the middle of a visit. The visitor's next action starts a new
session, which draws on the settings that just landed. The recorder already stops on session
expiry and starts again on renewal, so a tightened privacy level stops uploading plaintext at the
moment it arrives.

0 and 100 are the only rates that can decide anything, because they are the only ones whose
outcome needs no lottery. For any rate in between there is no answer to "should THIS session have
been kept" — only a second draw could produce one, and drawing twice turns a rate p into
with nothing to show for it. A privacy level that gets looser deliberately waits too: the
delay is what leaves room to undo a mistake, and what it costs meanwhile is more of the data
already being collected. The judgement is on the direction, not the volume.

How it is wired

store() now reports whether the response actually reached storage, and the fetcher emits
REMOTE_CONFIGURATION_STORED only when it did — a stale response and a failed write both stay
silent, since a subscriber must never act on settings the next draw will not find.

The comparison lives in the session manager, which already holds the record of what each session
was drawn under and the ability to end one. Remote configuration needs no reference to the session
manager.

What it compares is what the running session was drawn under against what a draw would use
now. That is not the previously stored settings: settings are stored while a session runs, and the
session was drawn on whatever was stored before that. Ending the session is precisely what makes
that difference disappear, which is why this needs no "only interrupt once per version"
bookkeeping to be idempotent — the same response arriving again, in another tab, on a retry or
after a reload, finds nothing left to act on.

Two things are deliberately left alone:

  • A session the host application forced is not ended by a rate. Every draw it makes is
    collected regardless, so ending it would only replace it with an identical forced session. A
    tightening privacy level still ends it — forcing decides whether the visitor is collected, not
    how much of their page may be uploaded in the clear.
  • The replay sample rate is not one of the three. It decides a draw nested inside the session
    draw, and a rule for it would first have to say what happens to a replay that was forced on.

Known limitation

Settings are still fetched only at start-up and on session renewal. A visitor who never goes idle
therefore never fetches again, so an immediate change cannot reach the very sessions it was most
likely meant for. Closing that gap needs a signal on a channel those visitors do use.

Notes for integrators

beforeSampling is now also called away from a draw, to resolve which rate newly delivered
settings would actually apply. The documentation now asks for a callback that is free of side
effects and answers the same way for the same input. Both were already implied by it running
inside session creation; neither was written down.

Testing

typecheck, lint and format are clean and the unit suite is green (2947).

Twenty-four new unit tests cover each rule and each non-rule — twenty on the decision itself, four
on the announcement that triggers it. Every one of them was checked by
reverting the implementation and confirming it fails: neutralising the decision fails the eight
tests that expect a session to end, widening it to fire on any change fails the ten that expect a
session to be left alone, and removing either guard fails the test written for it.

The rate a session is drawn on is the console's value falling back to
init, with the application's beforeSampling given the last word. That
resolution was written inline in the only branch that draws, which is
fine as long as a draw is the only thing that needs to know the answer.

Move it into a function that resolves and never draws, so the same
question can be asked without spending a lottery ticket to find out.
No behaviour changes.
Settings published from the console applied to sessions created after
they arrived, and to nothing else. For a visitor who never goes idle
that is hours: a session ends after fifteen minutes without activity or
four hours outright, so the change everyone is waiting on reaches the
people generating the most data last.

Three changes cannot wait, and they are exactly the three whose effect
on the running session can be told without drawing again:

  - a session sample rate of 0 while the visitor is being collected;
  - a rate of 100 while they are not;
  - a stricter defaultPrivacyLevel, where every further second recorded
    is a second of plaintext uploaded that masking cannot reach back for.

Each of them ends the current session; the visitor's next action starts
a new one under the new settings. Ending rather than flipping is the
point: the old session is collected to its end as it was begun, so no
replay is masked in one half and plain in the other, and no session is
invented that starts in the middle of a visit.

No other rate says anything about whether THIS session should have been
kept. Only a second draw could, and drawing twice quietly turns a rate p
into p², so every other change waits for the next session — a loosening
privacy level included, where being slow is what leaves room to undo a
mistake.

It needs no bookkeeping to stay idempotent: what it compares is what the
session was drawn under against what a draw would use now, and ending
the session is exactly what makes that difference disappear. The same
response arriving again, in another tab or after a reload, finds nothing
left to act on.

beforeSampling is now called outside a draw as well, to resolve the rate
that would actually apply, so the documentation asks for a callback free
of side effects and stable for the same input.
…ering

A const enum's values are inlined at build time and every entry after an
insertion shifts, so an entry wedged into the middle of a list that is
otherwise upstream's is both a renumbering and a conflict on the next
upstream merge. Move it to the end.

Also drop a guard that restated its caller's precondition: the event is
only ever emitted by the fetcher, which does not exist unless the site
opted in, and reading the settings already answers with nothing when it
did not.
@Fiona2016

Copy link
Copy Markdown
Collaborator Author

CI does not run automatically on this PR: the workflow only triggers for pull requests targeting main or publish, and this one targets a feature branch. Triggered manually on the branch instead — https://github.com/flashcatcloud/browser-sdk/actions/runs/33352709371

@Fiona2016

Copy link
Copy Markdown
Collaborator Author

Superseded by #40, which replays these commits on publish now that feat/remote-sampling-configuration has landed there, and carries the review fixes on top. Merged there.

@Fiona2016 Fiona2016 closed this Sep 1, 2026
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

Successfully merging this pull request may close these issues.

1 participant