feat(rum): end the session when new settings decide its fate - #40
Merged
Conversation
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.
A session that is not being collected is given no id, so no record of its draw is kept and the privacy level it was drawn under cannot be read back. The comparison fell through to the init value on every announcement and kept answering "tighter", so once an operator tightened `defaultPrivacyLevel` from the console, every sampled-out visitor was put on a loop: end the session, renew on the next click, refetch, end it again. It bought no privacy either -- a visitor who is not collected records nothing, so a stricter level has no plaintext to catch there. The rule now carries its own precondition and applies only while the session is being collected, which is also the only state in which a recording exists. Its fuel was the announcement firing on settings that had not changed: `store()` answered "stored" for a response repeating the version already held, which is the ordinary answer, since every new session refetches and most find nothing new. It now answers whether the stored version actually advanced. Three tests, each checked against the unfixed source first: a sampled-out session is left alone when the level tightens, it is still left alone as further settings arrive, and a response repeating the stored version is not announced.
Three paths the implementation documents had no test standing on them, each found by mutating the source and watching the suite stay green: - A session drawn before any settings arrived. A draw that lands exactly on the init values records nothing, so the level such a session runs under can only be read back off init -- the fallback every existing privacy test stepped around by storing settings before starting. Deleting that fallback passed the whole suite. - A response that carries no rate at all, with `beforeSampling` turning the delivered custom values into the decision. This is the "called away from a draw" contract, and both resolving the rate without the callback and bailing out when the console sends no rate passed the whole suite. - The console's kill switch, which stores a version and nothing else and so puts the rates back to the ones init passed. That is a change like any other, and where init never collected it is the decisive one. Also renames the opt-out test to what it actually pins down. Its store key is one no implementation could derive, so it cannot witness the store being left alone; what it does witness is the decision surviving an undefined `remoteConfig`.
…cts nothing The exemption that keeps a rate from ending a forced session was written for the case where ending it changes nothing: the page collects this visitor whatever the console says, so the replacement session would be the same session again. That reasoning runs out when the session is not collected. A page can adopt one drawn by a tab that never forced anything, and there a rate of 100 has something to change -- it is exactly the draw the page asked for. The guard now carries the precondition its own reasoning rests on. Also corrects two claims in the changelog entry that the code does not make good on: custom values do not always wait for the next session, since `beforeSampling` can turn them into a decisive rate -- the flagship pattern for this feature, and something the suite already pins down -- and the session after a split carries a new recording only if its draw keeps one.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Replaces #31, which was stacked on
feat/remote-sampling-configurationand targeted that branch.That branch has since landed on
publish, so the same three commits are replayed here on top ofit, with the conflicts against what
publishgained in the meantime resolved and four furthercommits on top.
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:
defaultPrivacyLevelgets stricterEnding 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.
0and100are the only rates that can decide anything, because they are the only ones whoseoutcome 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
pintop²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 custom bag is not in the table because it has no meaning of its own here. It reaches the
decision through
beforeSampling: a callback that answers 0 or 100 for the values just publishedends the session exactly as a published rate would, which is the point of asking it away from a
draw.
Each rule carries its own precondition
Two of the three are about what is being recorded, so both are gated on the session actually being
collected, and getting that wrong is not a small mistake:
records nothing, so a stricter level has no plaintext to catch there — and no draw record either,
because a session that is not collected is given no id and so no record is kept of what it was
drawn under. The comparison would fall through to the init value on every announcement and keep
answering "tighter", ending one empty session after another for as long as the visitor stayed.
a rate would ordinarily produce the same session again, which is why the exemption exists. But a
forced page can adopt a session drawn by a tab that never forced anything, and there a rate of
100 has something to change — it is precisely the draw the page asked for.
How it is wired
store()reports whether the response advanced what is in storage, and the fetcher emitsREMOTE_CONFIGURATION_STOREDonly when it did. A stale response, a failed write and a responserepeating the version already held all stay silent — that last one is the ordinary answer, since
every new session refetches and most find nothing new, and a subscriber woken by those would end a
session per renewal.
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.
Known limitation
Settings are still fetched only at start-up and on session renewal. A visitor who never reloads
therefore hears of a publish at their next session boundary, at most four hours away under the
session cap — this halves the worst case rather than removing it. Any tab they open, or any reload,
fetches at once and ends the session every tab shares. The changelog states this plainly rather
than leaving an operator to discover it.
Notes for integrators
beforeSamplingis now also called away from a draw, to resolve which rate newly deliveredsettings 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.
Release notes
CHANGELOG.mdgains an## Unreleasedsection. It leads with the symptoms that will read like aregression on publish day — session counts up, average session length down, a replay ending
mid-visit, collected volume climbing the same day on a rate of 100 — because without that, the
first report will be that an SDK upgrade doubled someone's session count.
Testing
format,lint,typecheck,build,check-packages,test:compat:tscandtest:compat:ssrare all clean, and the unit suite is green at 2985.
Twenty-nine new unit tests cover each rule and each non-rule. Every one was checked by mutating the
implementation and confirming the test fails: neutralising the decision, widening it to fire on any
change, removing either guard, dropping the fallback to the init privacy level, resolving the rate
without
beforeSampling, bailing out when the response carries no rate, and announcing a responsethat repeats the stored version — each is caught by the test written for it.