Merge error-session capture onto publish - #45
Open
Fiona2016 wants to merge 89 commits into
Open
Conversation
…export from mockProfiler imported registerCleanupTask and getGlobalObject from the rum package rather than core, and profiler.spec.ts imported from package names this repository does not publish. Since mockProfiler is re-exported from the rum test barrel, the broken imports took every spec that touches that barrel down with them - around 220 tests never ran.
A session drawn by this rate records from the start but uploads nothing until it reports an error. If none ever happens, nothing is sent and the session is never stored. On the first error the withheld buffer is released and recording continues normally, so the replay covers what led up to the error rather than starting at it. The buffer is bounded on both axes. Time: a buffer that spans more than a minute is dropped and restarted from a fresh full snapshot, so what is released stays a minute at most. Size: the existing segment byte limit still applies while withheld, and restarts are spaced out so that a document whose full snapshot alone exceeds that limit degrades instead of restarting in a loop. A withheld buffer belongs to the session that produced it. It is released only when that same session reports the error - if the session expires or is renewed first, the records are dropped, so an expiry can never turn into an upload for a session that never errored. Buffers that are dropped roll back their replay stats, and has_replay is not reported while a replay is being withheld, so neither the counters nor the link offer a replay that does not exist. Errors raised by the SDK about its own transport do not release anything: those are our failures, not the application's, and counting them would make every session an error session wherever our endpoint is unreachable.
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.
The flush path derived the same thing twice under two names, and the mapping of the internal checkout reason onto a schema value only ever had one caller.
…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.
A page-exit rotation used to throw the buffer away, and with it the full snapshot a released replay has to start from - everything recorded afterwards is incremental and cannot be played on its own. Switching tabs raises this exit, and the page comes straight back, so an error reported after that would have released a replay that renders as good as nothing until the next view. Nothing can be sent while withheld, so there was never anything to gain from the rotation. A page that is really unloading takes the buffer with it either way.
…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.
…ased buffer Only the rotation notices that the withheld replay has been released, so a session that expires within one rotation of its own error still loses what the error had earned. Closing it would mean asking the session manager on every record.
…sion-event-sampling
The rollback that gives a dropped buffer's index_in_view back only lands when the encoder finishes, which is always a turn later. Restarting from a fresh full snapshot emitted records right away, so the next segment took its index before the rollback arrived - and once that session errored, two uploaded segments claimed the same index within one view while nothing claimed the first. Any error session that spends a minute on one view before erroring hit it. The restart now happens where the rollback lands. Also corrects a comment: a session expiring right after its own error does not lose the buffer. The history entry is still open when the recorder is stopped, so the stop flush sees the session as released and sends.
…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.
Without it, a replay collected under this rate is indistinguishable from one collected unconditionally once it has been uploaded - the two cost differently and answer different questions, and nothing downstream could tell them apart.
…sion-event-sampling
Marking a session as having reported an error is only useful to a session that is withholding its replay. Doing it for every session wrote the session store for customers who enabled no error sampling at all, and that write also pushes the session's expiry out, which moves where their sessions end.
…take it Flushing a segment always waits for a round trip to the deflate worker, because the trailer is written just before finishing. The collection state is reset synchronously, so a record arriving during that round trip created the next segment while the dropped one was still counted: two uploaded segments then claimed the same index_in_view, and index 0 was never uploaded at all. Each counter is now given back in the phase it was taken in - the segment count synchronously, the record and byte counts in the flush callback.
… 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.
…d on A store write goes through a lock and can be retried for up to a second, and other tabs write the same store meanwhile - so the state a write lands on is not necessarily the one it was decided against. Updates are now expressed as a function of that state, and returning nothing makes the write a no-op, which is what a caller needs to say "only if this is still the session I meant".
Marking a session as having errored merged into whatever session the store held at the moment the write went through. A session that rolled over while the write waited for the lock - or that another tab renewed - was marked instead, and then uploaded a whole session that never reported anything. The mark now names the session it belongs to and is dropped if that session is gone. The same mark is also applied to the in-memory session straight away rather than only once the write lands, because until then the withheld buffer still reads the session as withholding: an error followed closely by the page or the session ending threw away the very buffer the error was meant to release.
Dropping a withheld buffer restarts it from a fresh full snapshot, and that runs in a flush callback which only arrives after a round trip to the deflate worker. Recording stopped in between still got a full re-serialization of the document, and its records counted into the replay stats with no segment to hold them.
…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.
A replay kept only because the session errored answers "do I want to see this error's session" - and that is a yes or a no, not a share. Keeping a random half of the error replays would just leave half the reports uninvestigable, and the cost this could guard against is already bounded by sessionReplaySampleRate and by turning the option off. The rate also hid an arithmetic trap: it applied to whatever the plain rate missed, so the real share was (100 - sessionReplaySampleRate) * rate / 100, and a rate set next to a plain rate of 100 silently did nothing. A switch has nothing to multiply. `sessionReplayOnErrorSampleRate: number` becomes `sessionReplayOnError: boolean`, default false. The tracking types and the session cookie are unchanged: what was drawn is now simply applied.
Merge branch 'feat/error-session-replay-sampling', which turned sessionReplayOnErrorSampleRate into the sessionReplayOnError switch, and give sessionOnErrorSampleRate the same treatment for the same reasons. Whether a session that errored should be kept is a yes or a no: each such session is wanted for its own sake, so keeping a random share of them only leaves the rest of the error reports uninvestigable. The volume this adds is already bounded by sessionSampleRate and by switching it off. The rate also hid an arithmetic trap - it applied to what the plain rate missed, so next to the default sessionSampleRate of 100 it silently did nothing. A switch has nothing to multiply; the warning for that combination stays. `sessionOnErrorSampleRate: number` becomes `sessionOnError: boolean`, default false. The tracking types and the session cookie are unchanged.
…sion-event-sampling
The error-session replay work was cut from main, but the fork integrates on publish, which has since gained remote configuration, forced sessions, direct replay upload and page reactivation. Resolved so that: - the on-error replay switch sits inside the draw branch, after the remote and beforeSampling-resolved replay rate has missed, so a session is still never counted by both; - forcing a session releases a withheld replay the same way it forces one onto a session without replay, since the host asked for it now; - a withheld buffer survives page reactivation: the fresh full snapshot taken on that event lands inside the buffer, which stays replayable from it, where cutting would only throw away what came before the switch; - the bridge stub never withholds, whether the host records or this page uploads directly.
…sion-event-sampling Brings the publish line in underneath the on-error session work. Resolved so that the on-error session switch sits in the draw branch after the remote and beforeSampling-resolved rates, the bridge stub never withholds, and the stored sessions the specs plant carry the expiry a session now has to prove.
…sion-event-sampling
The console can now deliver the switch beside the rates, so an operator turns error replays on or off without shipping a release. It is read at the draw like the rates and latched the same way: a session either withholds its replay from the start or never does. `beforeSampling` is not offered it - a switch is a yes or a no the console already answered. A delivered value that is not a boolean is dropped, so it reads as "not delivered" rather than as either position.
Merge branch 'feat/error-session-replay-sampling', which reads the replay switch from remote configuration, and read the session switch the same way: delivered beside the rates, latched at the draw, a non-boolean dropped as "not delivered". `beforeSampling` is offered neither switch.
…on-event-sampling
…on-event-sampling
endSessionIfSettingsAreDecisive ends a running session when the settings just delivered resolve the session sample rate to zero. With sessionOnError a zero rate is the switch's ordinary setting rather than a stop: at a zero rate a session is tracked exactly when the switch is on (a replay-on-error switch cannot keep one on its own, the session draw fails first). Ending it there discarded the very session the switch exists to keep and left the page blind from the first configuration fetch - which lands on every fresh profile and after every deploy - until the visitor's first interaction, so page-load errors were never captured. Gate the expire on the switch being off: a zero rate still stops plain sessions, and one turned off from the console still stops on-error ones, but a zero rate beside an on switch no longer ends the session. Add specs for both directions.
…d branches - startSessionReplayRecordingManually's doc still stated the pre-switch default rule; describe the derived default that also accounts for sessionReplayOnError and remoteConfigurationEnabled. - The mark guard comment said "neither rate"; both are switches now. - Add specs for two withheld-buffer branches that no test reached: a page reactivation must not cut the withheld buffer, and the SESSION_RELEASED subscription must wake the deferred snapshot restore with no rum event.
…ng' into feat/error-session-event-sampling # Conflicts: # packages/rum-core/src/domain/configuration/configuration.ts
…ive spec gaps - Add a changelog entry for sessionOnError / sessionReplayOnError. - The setSessionHasError doc and the error-tracking doc said the mark only releases a withheld replay; it now also releases withheld events. - Type the three fork-added session marker fields at the assembly site so a misspelled key fails the build instead of the schema's index signature quietly accepting it. - Cover five branches no spec reached: the console turning sessionOnError off, tiered eviction keeping an older action over newer long tasks, an event collected after the release flowing through instead of being held again, sampled_for_replay staying off for an error-replay session that has not errored, and the manual-start warning staying silent when replay is off.
…oversized-error release - An on-error session (kept by sessionOnError despite the plain draw missing it) now reports session_sample_rate 0 instead of the plain rate. The console extrapolates stored sessions by 100/session_sample_rate; reporting the rate that missed the session had each error session counted as 100/rate sessions. Reported after the draw ladder so the tracking type is known; a type that only withholds its replay still reports the plain rate it was drawn under. - The never-applies / no-session-tracked init warnings no longer fire under remoteConfigurationEnabled, where the init rates are a fallback the console can override - they were false-positiving on the documented remote-config setup that omits the rate. - A releasing error larger than the buffer budget now forwards on its own and schedules the history's release behind the jitter, instead of releasing the whole history in the same tick and defeating the anti-thundering-herd spread for the correlated outage the jitter exists for.
…nd two buffer edges - The zero-rate emergency stop now spares only the session that is itself an on-error one (withholdsEvents its type), not every session while the switch is merely on: a plainly drawn session is still ended by a rate-0 publish and redraws as on-error next action, instead of continuing to upload in full. - beforeSampling returning 0 for a rate now also clears the matching on-error switch, so the documented "0 never collects" is not quietly turned into "collect on error" for an excluded visitor. - A session whose type an older shared-cookie bundle rewrote under the same id is no longer blacklisted: its withheld buffer is dropped, but its events go on uploading as the plain session it now is. - A single non-error event larger than the whole buffer budget is dropped rather than held, so it cannot evict the minute of history to make room it could never fit into.
Record three known limitations of sessionOnError / sessionReplayOnError in the changelog, and note the replay's view-boundary in the sessionReplayOnError doc: consent-gated recording must set startSessionReplayRecordingManually explicitly; the released replay reaches back only to the error's view while events reach back the full minute; and under the opt-in compressIntakeRequests a tab close within seconds of the first error can lose that release.
Resolves the two conflicts with the "a rate that leaves zero" work already on publish: the zero-rate emergency stop keeps its on-error guard, so a rate of 0 spares only a session that is itself on-error and still ends a plainly drawn one; the not-tracked branch also ends a session when the switch turns on at rate 0; and the CHANGELOG Unreleased block sits above v0.2.3.
…der the same id The same-id guard added in 9eef19d only speaks on the event path, but the store poll always speaks first: it expires the session on the type change, and the expiry discards the buffer with the blacklist on. The session is then renewed under the SAME id by the store, and every event it collects is dropped for the rest of its life - a session the backend goes on storing and billing. Enforce the blacklist only against a session that is not the current one: a blacklisted id that is nonetheless live can only come from such a foreign same-id redraw, while a session that truly ended comes back with a new id, so real stragglers are still dropped. Cover the actual flow (poll expiry, then same-id renewal) with a spec.
The lock field fell into the carry-unknown-fields path: every access rewrote the cookie with the lock kept and the expiry renewed for a year. The modern bundle has no stale-lock recovery, so a lock whose owner was gone (a crashed tab, or one whose write we raced) stayed alive for as long as a legacy page kept the cookie warm - wedging the modern session store: every write retried and dropped, the in-memory session never expiring, and every new page's init failing on an empty cache. Dropping the field at parse time turns the legacy rewrite into a stale-lock cleaner, and a legacy write that lands inside the modern lock window now fails the modern corruption check (a retry) instead of passing it with the session silently rolled back.
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.
What this is
The release integration of the error-session capture feature (
sessionOnError/sessionReplayOnError) ontopublish. It merges the two stacked review branches (feat/error-session-replay-sampling#24 andfeat/error-session-event-sampling#25) and resolves the conflict with the "a rate that leaves zero" work already onpublish.Opened to run CI on the exact tree that would ship. Not for merge yet — pending release go-ahead.
Conflict resolution
rumSessionManager.ts: the zero-rate emergency stop keeps its on-error guard — a rate of 0 spares only a session that is itself on-error (withholdsEvents), and still ends a plainly drawn one. The not-tracked branch also ends a session when the switch turns on at rate 0, matching the existing rate-above-0 behaviour.CHANGELOG.md: theUnreleasedblock sits abovev0.2.3.Verification
yarn test:unit(3170 passed, 0 failed),yarn typecheck,yarn lint,yarn formatall clean on the merged tree.🤖 Generated with Claude Code