feat(rum): add sessionOnErrorSampleRate - #25
Open
Fiona2016 wants to merge 39 commits into
Open
Conversation
A session drawn by this rate collects events but uploads nothing until it reports an error. If none ever happens the session is never stored, and on the first error the withheld history is released so the detail leading up to the error is there rather than starting at it. Events are held upstream of the batch, which cannot serve as the buffer itself: ordinary events go straight into a compression stream and cannot be evicted one by one. View events are kept one-per-view and out of the eviction budget, since the backend builds the session row from them and a detail released without its view would be unreachable - anything whose view is gone is dropped at release for the same reason. The buffer is bounded by time, count and size. When it runs out of room it drops long tasks and unremarkable requests first, then actions, and never errors. The release is spread over a few seconds keyed on the session id, because correlated errors would otherwise have every client release at the same instant, and it is flushed early if the page is about to go rather than lost to that window. The replay of such a session is withheld alongside its events, whichever replay rate it drew: until the events are released the session does not exist yet, so a replay sent then would have nothing to attach to and would be stranded for good if the error never came. Forcing capture releases both, for the same reason.
…sion-event-sampling
Drops exports nothing outside the module uses, names the entry being appended instead of reading it back off the end, folds the two ways of emptying the buffer into one, and records why a view is deleted before being set again.
…sion-event-sampling
…r was missed Two problems with releasing a withheld event buffer. The jitter meant to spread correlated releases did not spread them. Session ids are same-length strings over one small alphabet, so summing their character codes put over 97% of them within 600ms of each other: the herd was delayed by about two and a half seconds rather than broken up. A multiplicative hash spreads them evenly across the window, which a distribution test now pins down. The other is that a session can report its error without the buffer noticing. The event arrives synchronously, but the state behind it is written through a lock that can defer the write, so the buffer may still read the session as withholding, hold the error, and schedule nothing. If the user then leaves - which is exactly the case this feature exists for - the whole session was thrown away. The session is now re-read before the buffer is discarded on page exit.
… is evicted 'never' The size budget measured UTF-16 code units, which understates non-ASCII payloads by up to three times - a buffer meant to stay inside a beacon could be well past it before the cap noticed. The error tier was documented as never evicted, but the eviction loop included it and took the oldest first: under an error storm the buffer would give up the very first error, the one that released it and the one the session is about. Errors are now given up only once nothing else remains, newest first.
…s go Three lifecycle gaps in the withheld event buffer. Nothing reacted to the session ending. A release waiting on its jitter was lost if the session expired first, and a buffer belonging to a session that ended because tracking consent was withdrawn stayed in memory until some later event happened to arrive. The session ending is now settled the same way the page going away already was. Its stop was never wired into the SDK teardown, so a pending release could still fire into a batch that had stopped flushing. Views were kept for as long as the page lived, one per route, which grew past the detail budget itself and put fifty of them into a release. A view is kept as the container of the detail hanging from it, so it now goes once none of its detail is left inside the window - except the view in progress, which is the container the error will hang from.
…sion-event-sampling
…sion-event-sampling
…ail marker survive Two problems the replay side had already reasoned its way out of, which the event side had not. The buffer was cleared on any page exit, and a page being hidden raises one - switching tabs, or switching apps on mobile, wiped the withheld minute and left an error arriving just afterwards with almost nothing. A page that is really unloading takes the buffer with it anyway, so there was never anything to gain. The session ending is different, and still clears it. The marker saying how far back the stored detail reaches was stamped on the view events being released, but the batch upserts views by id: the next ordinary view update, seconds later and without the marker, replaced them before the batch was ever sent. For the view the error happened in - the one that matters - it never arrived. It is now recorded on the session, so every later view update carries it.
…sion-event-sampling
… current one Assembly resolves a session at the event's own start time, so a request or a view update that finishes after its session ended still carries that session's id. The buffer read whichever session was current instead, which let two things through: a straggler of a session that had ended without ever reporting an error was uploaded on its own - storing the very session the withholding was there to avoid - and one arriving after a renewal was held in the new session's buffer and released by an error that was not its own. A view that already ended no longer becomes the current view when it is updated late either. It carries its own start date, and treating it as current had the pruning drop the view the next error hangs from, so the release filtered that error out of its own buffer.
…sion-event-sampling
Two gaps left by withholding events as well as replays. The mark that releases a buffer was being skipped for a session that withholds only its events, since the check knew about the replay side alone. And a buffer whose session had been renewed into one that withholds nothing was left behind until the session expiry notification arrived, rather than being dropped as soon as the session it belonged to was no longer current.
…sion-event-sampling # Conflicts: # packages/rum-core/src/domain/rumSessionManager.ts
…an one discarded session Three things a withheld event buffer got wrong once time or tabs were involved. The window it releases was measured from the moment the release ran rather than the moment it was scheduled. The timer carrying a release is clamped to roughly once a minute in a backgrounded tab, so by the time it ran the whole minute before the error had aged out - the release delivered the error and nothing leading up to it. The window is now fixed when the release is scheduled. Only the last thrown-away session was remembered, so a request that outlived two withheld sessions was uploaded on its own when it finally completed. A handful are remembered now, which is more than can still be assembled to. Where the stored detail starts is now the earliest point any tab reached, decided under the store lock, instead of whichever tab wrote last; and it is only recorded on the session it was measured for. Also records what the ordering between the page-exit relay and the batch is for, since nothing but the order of two statements enforces it.
…sion-event-sampling
Same reasoning as the replay side: a session can stop withholding without ever reporting an error, because an SDK bundle that predates these tracking types shares the session store, does not recognise them, and redraws the session. The buffer read that as a release and uploaded a session's whole history. Release now requires the session to still be one whose events are kept on an error; anything else ends the buffer.
…sion-event-sampling
…ppened Three things a released burst got wrong about itself. The views left in the order they were last updated, which is not the order they happened - a late update of an ended view puts the oldest one last. A session is built out of whichever of its views arrives first, and everything after is addressed to the earliest view's time, so a burst that led with the wrong view left the rest of the session unreachable. They now leave oldest first. Where the stored detail starts was taken from the first event held rather than the earliest one. An event is dated when it started, so a request that took minutes is held long after it began, and the marker claimed a start that some of the released detail preceded. The released views were assembled while the replay was still withheld, so they said the session was not sampled for replay. By the time they leave, that replay is on its way with them.
Forcing a replay was only ever applied to a session whose replay was off, and only when the
recorder was not already running. A session withholding its replay fails both: it is
recording, and its replay is not off. So the session manager's rule that a forced replay
wins over withholding, and releases the events with it, could not be reached from the
public API at all - `startSessionReplayRecording({ force: true })` did nothing for exactly
the sessions where it has something to do.
sessionReplayOnErrorSampleRate is drawn from what the plain replay rate did not take, so some perfectly valid configurations can never draw anything: a plain rate of 100 leaves it nothing, a session rate of 0 leaves no session to draw from, and starting the recording manually leaves nothing recorded to withhold. Each of those now says so once at init. The option's own description also led with "the percentage of tracked sessions", which is not the base it is drawn from.
…sion-event-sampling
… either It is drawn from what sessionSampleRate did not take, and that rate defaults to 100 - so the first thing a customer is likely to write, the option on its own, does nothing at all. That now says so at init. It also changes what "no session is tracked" means for the replay rate: a session rate of 0 no longer leaves nothing behind once sessions can be drawn on error, so that warning is narrowed to the case where both are out.
…g with them The events of a session that withholds them are assembled while its replay is still withheld too, so they said the session was not sampled for replay - and they are precisely the events that only ever leave together with that replay. They now report what will be true of them by the time they are uploaded, rather than what was true while they waited.
…sion-event-sampling # Conflicts: # packages/rum-core/src/domain/configuration/configuration.spec.ts # packages/rum/src/boot/postStartStrategy.ts
…sion-event-sampling
The mock could not represent a session that withholds its events without a replay - the plainest thing this feature does - so the test named after that case was quietly testing the other one. It can now represent both, and the buffer's own suite runs on the type a customer setting only sessionOnErrorSampleRate actually gets. The rest closes gaps where a one-line change would have shipped a feature that silently does nothing or quietly costs more: the rate never reaching the built configuration or never being range-checked, the on-error type never drawn with a replay, a stored type redrawn on every page load, the release jitter reduced to nothing, the bytes budget going unenforced, failed requests evicted before successful ones, the view cap not applied, aged detail released on the page-exit path, a straggler of a plainly sampled session swallowed, a release lost to the session ending inside its jitter window, a stopped buffer still forwarding, and the session markers no longer emitted.
…sion-event-sampling
…ing an impossibility The event side and the replay side each had their own sixty seconds, with a comment on one saying it had to equal the other. It is one promise to the customer, so it is now one constant that both sides read. The release jitter's hash also carried a modulo, and a constant and a comment explaining that it kept the running value inside the range `Math.imul` is exact over. `Math.imul` is defined on int32 and re-coerces on every iteration, so there was nothing to keep it inside.
Three of them, all found by re-reading the fixes rather than the feature. Events withheld alongside their replay were told the session was sampled for replay, but not that it has one - so the error that releases a session said there was no replay to watch, next to the replay that shows it. Both now follow the same rule: an event only hides a withheld replay when it would ship without it. The view cap could evict the view in progress, which is the one view `prune` goes out of its way to keep, because it is the container the released error hangs from. Late updates of ended views are what push it to the front, and the cap takes from the front. The manual-start warning only knew about the replay-on-error rate, but a session drawn on error withholds whichever replay it draws - and there the trap is worse than silence, since the released views would report a replay for a recording that never ran. The note on how many discarded sessions are remembered also claimed the session history keeps them for far less than a session's life. It keeps them for as long as a session can last; what escapes is a lone detail event with nothing to attach to, which is the reason the bound is affordable rather than an accident.
The previous commit had events withheld alongside their replay report that they have one, on the reasoning that they only ever leave together. They do - but which segment leaves with them is decided later than they are assembled: a view emits its final update before the view change that drops that view's withheld segment, so every ended view was released claiming a replay that had already been rolled back. That is the over-claim the code sets out to avoid, traded for the under-claim it was meant to fix. An event assembled while a replay is withheld goes back to claiming nothing. Whether the session was sampled for a replay is a different question, decided by the draw rather than by any segment's fate, and it keeps its answer. The manual-start warning also no longer fires for an on-error rate that cannot draw a session in the first place - there is already a warning saying exactly that.
…sion-event-sampling # Conflicts: # packages/rum-core/src/domain/contexts/sessionContext.ts
`detail_sampled_from` was the earliest date among the detail a session released. Every one of those events is uploaded and carries its own date, so the same number is the minimum of the non-view rows the backend already holds, and the console has that list in hand while it draws the line. Computing it on the client bought nothing and cost a session-store key, a cross-tab reconciliation under the store lock, and a second write onto the released views to survive the batch's view upsert. What tells a compensation-sampled session apart from an ordinary one is `session.sampled_for_error`, and that stays. The console's divider is gated on it, and already renders without naming a moment when no timestamp is there.
…arned A session that has reported its error releases its buffer when it ends, and consent being withdrawn is one of the ways a session ends. Everything held was collected while consent stood, and a batch has always flushed what it was holding when a session ends; what this feature changes is the size of that last flush, up to a minute rather than up to a batch. Written down because it reads like an oversight and is not 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.
Motivation
Stacked on #24, and based on that branch so the diff here is only this change. It should merge after it; GitHub will retarget this at
mainonce #24 lands.#24 gives an error session a replay that reaches back a minute. Its resource, action and long task detail still starts at the error, which leaves out the failing request and the click that led to it — usually the first two things anyone looks at.
sessionOnErrorSampleRateapplies the same idea to the events themselves: a session that is recorded but only stored if it reports an error.Changes
sessionOnErrorSampleRate, drawn only for sessions the plainsessionSampleRatedraw missed. Two tracking types are added; existing ones keep their meaning, so sessions in flight are unaffected.sampled_for_error, anddetail_sampled_fromrecords where the stored detail begins, so the gap before it reads as data that was never collected rather than data that went missing. It is recorded on the session rather than stamped on the released view events, because the batch upserts views by id and the next ordinary update would replace them first.Test instructions
yarn test:unit. New specs cover withholding, release ordering, the window, tiered eviction, the container guarantee, hidden pages, session end, and the spread of the release delay.By hand, against
yarn dev:initwithsessionSampleRate: 0,sessionOnErrorSampleRate: 100.sampled_for_erroranddetail_sampled_from.Driven through Playwright against the same page, including a tab switch midway: a session uploaded nothing across 43 seconds, then released 23 requests reaching back 46 seconds, with the marker present on the last view event.
Checklist