feat(rum): end the session when new settings decide its fate - #31
Closed
Fiona2016 wants to merge 3 commits into
Closed
feat(rum): end the session when new settings decide its fate#31Fiona2016 wants to merge 3 commits into
Fiona2016 wants to merge 3 commits into
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.
Collaborator
Author
|
CI does not run automatically on this PR: the workflow only triggers for pull requests targeting |
Collaborator
Author
|
Superseded by #40, which replays these commits on |
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
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.
Stacked on
feat/remote-sampling-configuration, and targets that branch. Review only the twocommits 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:
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. 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.
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 judgement is on the direction, not the volume.
How it is wired
store()now reports whether the response actually reached storage, and the fetcher emitsREMOTE_CONFIGURATION_STOREDonly when it did — a stale response and a failed write both staysilent, 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:
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.
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
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.
Testing
typecheck,lintandformatare 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.