From 5486c5dd383eb83f7ccdcc3b03356642f07adea1 Mon Sep 17 00:00:00 2001 From: Fiona Date: Thu, 20 Aug 2026 03:24:48 -0700 Subject: [PATCH 01/27] feat(rum): add sessionOnErrorSampleRate 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. --- packages/rum-core/src/boot/startRum.ts | 2 +- .../configuration/configuration.spec.ts | 2 + .../src/domain/configuration/configuration.ts | 17 ++ .../src/domain/contexts/sessionContext.ts | 5 + .../src/domain/rumSessionManager.spec.ts | 73 +++++ .../rum-core/src/domain/rumSessionManager.ts | 83 +++++- .../rum-core/src/transport/startRumBatch.ts | 20 +- .../src/transport/withheldEventBuffer.spec.ts | 199 ++++++++++++++ .../src/transport/withheldEventBuffer.ts | 253 ++++++++++++++++++ .../rum-core/test/mockRumSessionManager.ts | 17 +- 10 files changed, 652 insertions(+), 19 deletions(-) create mode 100644 packages/rum-core/src/transport/withheldEventBuffer.spec.ts create mode 100644 packages/rum-core/src/transport/withheldEventBuffer.ts diff --git a/packages/rum-core/src/boot/startRum.ts b/packages/rum-core/src/boot/startRum.ts index 0c1af632b9..322bcb2d52 100644 --- a/packages/rum-core/src/boot/startRum.ts +++ b/packages/rum-core/src/boot/startRum.ts @@ -121,7 +121,7 @@ export function startRum( telemetry.observable, reportError, pageMayExitObservable, - session.expireObservable, + session, createEncoder ) cleanupTasks.push(() => batch.stop()) diff --git a/packages/rum-core/src/domain/configuration/configuration.spec.ts b/packages/rum-core/src/domain/configuration/configuration.spec.ts index a2c4c48875..558b208d23 100644 --- a/packages/rum-core/src/domain/configuration/configuration.spec.ts +++ b/packages/rum-core/src/domain/configuration/configuration.spec.ts @@ -533,6 +533,7 @@ describe('serializeRumConfiguration', () => { subdomain: 'foo', sessionReplaySampleRate: 60, sessionReplayOnErrorSampleRate: 40, + sessionOnErrorSampleRate: 30, startSessionReplayRecordingManually: true, trackUserInteractions: true, actionNameAttribute: 'test-id', @@ -560,6 +561,7 @@ describe('serializeRumConfiguration', () => { | 'propagateTraceBaggage' // not reported yet: needs a rum-events-format schema change first | 'sessionReplayOnErrorSampleRate' + | 'sessionOnErrorSampleRate' ? never : CamelToSnakeCase // By specifying the type here, we can ensure that serializeConfiguration is returning an diff --git a/packages/rum-core/src/domain/configuration/configuration.ts b/packages/rum-core/src/domain/configuration/configuration.ts index 33743b5d9e..85476d86b7 100644 --- a/packages/rum-core/src/domain/configuration/configuration.ts +++ b/packages/rum-core/src/domain/configuration/configuration.ts @@ -110,6 +110,19 @@ export interface RumInitConfiguration extends InitConfiguration { * the withheld minute is uploaded and recording continues normally for the rest of the session. */ sessionReplayOnErrorSampleRate?: number | undefined + /** + * The percentage of tracked sessions that collect events but only upload them if the session + * reports an error: 100 for all, 0 for none. Drawn only for sessions that the plain + * `sessionSampleRate` draw missed, so a session is never counted by both rates. + * + * Such a session collects from the start and keeps at most the last minute of it in memory. If it + * never reports an error, nothing is uploaded and the session is not stored. On the first error, + * the withheld minute is uploaded and collection continues normally. + * + * A session sampled this way never uploads its replay ahead of its events: until the events are + * released the session does not exist yet, and a replay sent then would have nothing to attach to. + */ + sessionOnErrorSampleRate?: number | undefined /** * If the session is sampled for Session Replay, only start the recording when `startSessionReplayRecording()` is called, instead of at the beginning of the session. Default: if startSessionReplayRecording is 0, true; otherwise, false. * See [Session Replay Usage](https://docs.datadoghq.com/real_user_monitoring/session_replay/browser/#usage) for further information. @@ -186,6 +199,7 @@ export interface RumConfiguration extends Configuration { enablePrivacyForActionName: boolean sessionReplaySampleRate: number sessionReplayOnErrorSampleRate: number + sessionOnErrorSampleRate: number startSessionReplayRecordingManually: boolean trackUserInteractions: boolean trackViewsManually: boolean @@ -219,6 +233,7 @@ export function validateAndBuildRumConfiguration( if ( !isSampleRate(initConfiguration.sessionReplaySampleRate, 'Session Replay') || !isSampleRate(initConfiguration.sessionReplayOnErrorSampleRate, 'Session Replay on Error') || + !isSampleRate(initConfiguration.sessionOnErrorSampleRate, 'Session on Error') || !isSampleRate(initConfiguration.traceSampleRate, 'Trace') ) { return @@ -243,6 +258,7 @@ export function validateAndBuildRumConfiguration( const sessionReplaySampleRate = initConfiguration.sessionReplaySampleRate ?? 0 const sessionReplayOnErrorSampleRate = initConfiguration.sessionReplayOnErrorSampleRate ?? 0 + const sessionOnErrorSampleRate = initConfiguration.sessionOnErrorSampleRate ?? 0 return { applicationId: initConfiguration.applicationId, @@ -250,6 +266,7 @@ export function validateAndBuildRumConfiguration( actionNameAttribute: initConfiguration.actionNameAttribute, sessionReplaySampleRate, sessionReplayOnErrorSampleRate, + sessionOnErrorSampleRate, startSessionReplayRecordingManually: initConfiguration.startSessionReplayRecordingManually !== undefined ? !!initConfiguration.startSessionReplayRecordingManually diff --git a/packages/rum-core/src/domain/contexts/sessionContext.ts b/packages/rum-core/src/domain/contexts/sessionContext.ts index c8d893c110..b3676907db 100644 --- a/packages/rum-core/src/domain/contexts/sessionContext.ts +++ b/packages/rum-core/src/domain/contexts/sessionContext.ts @@ -26,10 +26,14 @@ export function startSessionContext( let hasReplay let sampledForReplay + let sampledForError let isActive if (eventType === RumEventType.VIEW) { hasReplay = !isReplayWithheld && recorderApi.getReplayStats(view.id) ? true : undefined sampledForReplay = session.sessionReplay === SessionReplayState.SAMPLED + // Tells the backend that this session's detail only starts where the buffer reached, so the + // gap before it reads as "not collected" rather than as missing data. + sampledForError = session.sampledOnError || undefined isActive = view.sessionIsActive ? undefined : false } else { hasReplay = !isReplayWithheld && recorderApi.isRecording() ? true : undefined @@ -42,6 +46,7 @@ export function startSessionContext( type: SessionType.USER, has_replay: hasReplay, sampled_for_replay: sampledForReplay, + sampled_for_error: sampledForError, is_active: isActive, }, } diff --git a/packages/rum-core/src/domain/rumSessionManager.spec.ts b/packages/rum-core/src/domain/rumSessionManager.spec.ts index 0c286da966..3432300232 100644 --- a/packages/rum-core/src/domain/rumSessionManager.spec.ts +++ b/packages/rum-core/src/domain/rumSessionManager.spec.ts @@ -269,6 +269,79 @@ describe('rum session manager', () => { }) }) + describe('on-error session sampling', () => { + const ON_ERROR_ONLY = { + sessionSampleRate: 0, + sessionOnErrorSampleRate: 100, + sessionReplaySampleRate: 0, + sessionReplayOnErrorSampleRate: 0, + } + + it('draws the on-error type only when the plain session draw missed', () => { + startRumSessionManagerWithDefaults({ + configuration: { ...ON_ERROR_ONLY, sessionSampleRate: 100 }, + }) + + expect(getSessionState(SESSION_STORE_KEY)[RUM_SESSION_KEY]).toBe(RumTrackingType.TRACKED_WITHOUT_SESSION_REPLAY) + }) + + it('withholds the events of a session drawn on error', () => { + const sessionManager = startRumSessionManagerWithDefaults({ configuration: ON_ERROR_ONLY }) + + expect(getSessionState(SESSION_STORE_KEY)[RUM_SESSION_KEY]).toBe( + RumTrackingType.TRACKED_ON_ERROR_WITHOUT_SESSION_REPLAY + ) + expect(sessionManager.findTrackedSession()!.eventsWithheld).toBeTrue() + + sessionManager.setSessionHasError() + + expect(sessionManager.findTrackedSession()!.eventsWithheld).toBeFalse() + }) + + it('withholds the replay alongside the events, even when the plain replay rate was drawn', () => { + // a replay uploaded while the events are withheld would have no session to attach to + const sessionManager = startRumSessionManagerWithDefaults({ + configuration: { ...ON_ERROR_ONLY, sessionReplaySampleRate: 100 }, + }) + + expect(getSessionState(SESSION_STORE_KEY)[RUM_SESSION_KEY]).toBe( + RumTrackingType.TRACKED_ON_ERROR_WITH_SESSION_REPLAY + ) + expect(sessionManager.findTrackedSession()!.sessionReplay).toBe(SessionReplayState.BUFFERED_ON_ERROR) + }) + + it('releases events and replay together on the first error', () => { + const sessionManager = startRumSessionManagerWithDefaults({ + configuration: { ...ON_ERROR_ONLY, sessionReplaySampleRate: 100 }, + }) + + sessionManager.setSessionHasError() + + const session = sessionManager.findTrackedSession()! + expect(session.eventsWithheld).toBeFalse() + expect(session.sessionReplay).toBe(SessionReplayState.SAMPLED) + }) + + it('releases the events when capture is forced, so the forced replay is not left orphaned', () => { + const sessionManager = startRumSessionManagerWithDefaults({ + configuration: { ...ON_ERROR_ONLY, sessionReplaySampleRate: 100 }, + }) + + sessionManager.setForcedReplay() + + const session = sessionManager.findTrackedSession()! + expect(session.eventsWithheld).toBeFalse() + expect(session.sessionReplay).toBe(SessionReplayState.FORCED) + }) + + it('keeps marking the session as on-error once its events have been released', () => { + const sessionManager = startRumSessionManagerWithDefaults({ configuration: ON_ERROR_ONLY }) + sessionManager.setSessionHasError() + + expect(sessionManager.findTrackedSession()!.sampledOnError).toBeTrue() + }) + }) + function startRumSessionManagerWithDefaults({ configuration }: { configuration?: Partial } = {}) { return startRumSessionManager( mockRumConfiguration({ diff --git a/packages/rum-core/src/domain/rumSessionManager.ts b/packages/rum-core/src/domain/rumSessionManager.ts index e58383718a..5a18afc626 100644 --- a/packages/rum-core/src/domain/rumSessionManager.ts +++ b/packages/rum-core/src/domain/rumSessionManager.ts @@ -34,6 +34,17 @@ export interface RumSessionManager { export type RumSession = { id: string sessionReplay: SessionReplayState + /** + * Whether the session collects events but withholds them until it reports an error. Nothing is + * uploaded while this is true, and if the session never errors nothing ever is. + */ + eventsWithheld: boolean + /** + * Whether the session was drawn by `sessionOnErrorSampleRate`. Unlike {@link eventsWithheld} this + * stays true once the error has been reported, so what is stored can be told apart from a plainly + * sampled session - its detail only starts where the buffer reached. + */ + sampledOnError: boolean anonymousId?: string } @@ -42,6 +53,8 @@ export const enum RumTrackingType { TRACKED_WITH_SESSION_REPLAY = '1', TRACKED_WITHOUT_SESSION_REPLAY = '2', TRACKED_WITH_ERROR_SESSION_REPLAY = '3', + TRACKED_ON_ERROR_WITHOUT_SESSION_REPLAY = '4', + TRACKED_ON_ERROR_WITH_SESSION_REPLAY = '5', } export const enum SessionReplayState { @@ -99,6 +112,8 @@ export function startRumSessionManager( return { id: session.id, sessionReplay: computeSessionReplayState(session.trackingType, session.hasError, session.isReplayForced), + eventsWithheld: computeEventsWithheld(session.trackingType, session.hasError, session.isReplayForced), + sampledOnError: withholdsEvents(session.trackingType), anonymousId: session.anonymousId, } }, @@ -109,6 +124,20 @@ export function startRumSessionManager( } } +function withholdsReplay(trackingType: RumTrackingType) { + return ( + trackingType === RumTrackingType.TRACKED_WITH_ERROR_SESSION_REPLAY || + trackingType === RumTrackingType.TRACKED_ON_ERROR_WITH_SESSION_REPLAY + ) +} + +export function withholdsEvents(trackingType: RumTrackingType) { + return ( + trackingType === RumTrackingType.TRACKED_ON_ERROR_WITHOUT_SESSION_REPLAY || + trackingType === RumTrackingType.TRACKED_ON_ERROR_WITH_SESSION_REPLAY + ) +} + export function computeSessionReplayState( trackingType: RumTrackingType, hasError: boolean, @@ -117,7 +146,7 @@ export function computeSessionReplayState( if (trackingType === RumTrackingType.TRACKED_WITH_SESSION_REPLAY) { return SessionReplayState.SAMPLED } - if (trackingType === RumTrackingType.TRACKED_WITH_ERROR_SESSION_REPLAY && hasError) { + if (withholdsReplay(trackingType) && hasError) { return SessionReplayState.SAMPLED } // A forced replay wins over withholding: the host explicitly asked for this user's replay, so it @@ -125,12 +154,25 @@ export function computeSessionReplayState( if (isReplayForced) { return SessionReplayState.FORCED } - if (trackingType === RumTrackingType.TRACKED_WITH_ERROR_SESSION_REPLAY) { + if (withholdsReplay(trackingType)) { return SessionReplayState.BUFFERED_ON_ERROR } return SessionReplayState.OFF } +export function computeEventsWithheld( + trackingType: RumTrackingType, + hasError: boolean, + isReplayForced: boolean +): boolean { + // Forcing capture asks for this user's whole session, so it releases the events too - otherwise + // the forced replay would be uploaded for a session that does not exist yet. + if (hasError || isReplayForced) { + return false + } + return withholdsEvents(trackingType) +} + /** * Start a tracked replay session stub */ @@ -138,6 +180,8 @@ export function startRumSessionManagerStub(): RumSessionManager { const session: RumSession = { id: '00000000-aaaa-0000-aaaa-000000000000', sessionReplay: bridgeSupports(BridgeCapability.RECORDS) ? SessionReplayState.SAMPLED : SessionReplayState.OFF, + eventsWithheld: false, + sampledOnError: false, } return { findTrackedSession: () => session, @@ -152,15 +196,26 @@ function computeSessionState(configuration: RumConfiguration, rawTrackingType?: let trackingType: RumTrackingType if (hasValidRumSession(rawTrackingType)) { trackingType = rawTrackingType - } else if (!performDraw(configuration.sessionSampleRate)) { - trackingType = RumTrackingType.NOT_TRACKED - } else if (performDraw(configuration.sessionReplaySampleRate)) { - trackingType = RumTrackingType.TRACKED_WITH_SESSION_REPLAY - } else if (performDraw(configuration.sessionReplayOnErrorSampleRate)) { - // Drawn only when the plain replay draw missed, so a session is never counted by both rates. - trackingType = RumTrackingType.TRACKED_WITH_ERROR_SESSION_REPLAY + } else if (performDraw(configuration.sessionSampleRate)) { + if (performDraw(configuration.sessionReplaySampleRate)) { + trackingType = RumTrackingType.TRACKED_WITH_SESSION_REPLAY + } else if (performDraw(configuration.sessionReplayOnErrorSampleRate)) { + // Drawn only when the plain replay draw missed, so a session is never counted by both rates. + trackingType = RumTrackingType.TRACKED_WITH_ERROR_SESSION_REPLAY + } else { + trackingType = RumTrackingType.TRACKED_WITHOUT_SESSION_REPLAY + } + } else if (performDraw(configuration.sessionOnErrorSampleRate)) { + // Drawn only when the plain session draw missed, so a session is never counted by both rates. + // Such a session never uploads its replay ahead of its events: whichever replay rate it draws, + // the replay is withheld alongside them, because until they are released the session does not + // exist yet and a replay sent then would have nothing to attach to. + trackingType = + performDraw(configuration.sessionReplaySampleRate) || performDraw(configuration.sessionReplayOnErrorSampleRate) + ? RumTrackingType.TRACKED_ON_ERROR_WITH_SESSION_REPLAY + : RumTrackingType.TRACKED_ON_ERROR_WITHOUT_SESSION_REPLAY } else { - trackingType = RumTrackingType.TRACKED_WITHOUT_SESSION_REPLAY + trackingType = RumTrackingType.NOT_TRACKED } return { trackingType, @@ -173,7 +228,9 @@ function hasValidRumSession(trackingType?: string): trackingType is RumTrackingT trackingType === RumTrackingType.NOT_TRACKED || trackingType === RumTrackingType.TRACKED_WITH_SESSION_REPLAY || trackingType === RumTrackingType.TRACKED_WITHOUT_SESSION_REPLAY || - trackingType === RumTrackingType.TRACKED_WITH_ERROR_SESSION_REPLAY + trackingType === RumTrackingType.TRACKED_WITH_ERROR_SESSION_REPLAY || + trackingType === RumTrackingType.TRACKED_ON_ERROR_WITHOUT_SESSION_REPLAY || + trackingType === RumTrackingType.TRACKED_ON_ERROR_WITH_SESSION_REPLAY ) } @@ -181,6 +238,8 @@ function isTypeTracked(rumSessionType: RumTrackingType | undefined) { return ( rumSessionType === RumTrackingType.TRACKED_WITHOUT_SESSION_REPLAY || rumSessionType === RumTrackingType.TRACKED_WITH_SESSION_REPLAY || - rumSessionType === RumTrackingType.TRACKED_WITH_ERROR_SESSION_REPLAY + rumSessionType === RumTrackingType.TRACKED_WITH_ERROR_SESSION_REPLAY || + rumSessionType === RumTrackingType.TRACKED_ON_ERROR_WITHOUT_SESSION_REPLAY || + rumSessionType === RumTrackingType.TRACKED_ON_ERROR_WITH_SESSION_REPLAY ) } diff --git a/packages/rum-core/src/transport/startRumBatch.ts b/packages/rum-core/src/transport/startRumBatch.ts index 3f7238ca65..42456b295f 100644 --- a/packages/rum-core/src/transport/startRumBatch.ts +++ b/packages/rum-core/src/transport/startRumBatch.ts @@ -1,4 +1,11 @@ -import type { Context, TelemetryEvent, Observable, RawError, PageMayExitEvent, Encoder } from '@flashcatcloud/browser-core' +import type { + Context, + TelemetryEvent, + Observable, + RawError, + PageMayExitEvent, + Encoder, +} from '@flashcatcloud/browser-core' import { DeflateEncoderStreamId, combine, @@ -7,9 +14,10 @@ import { } from '@flashcatcloud/browser-core' import type { RumConfiguration } from '../domain/configuration' import type { LifeCycle } from '../domain/lifeCycle' -import { LifeCycleEventType } from '../domain/lifeCycle' +import type { RumSessionManager } from '../domain/rumSessionManager' import { RumEventType } from '../rawRumEvent.types' import type { RumEvent } from '../rumEvent.types' +import { startWithheldEventBuffer } from './withheldEventBuffer' export function startRumBatch( configuration: RumConfiguration, @@ -17,7 +25,7 @@ export function startRumBatch( telemetryEventObservable: Observable, reportError: (error: RawError) => void, pageMayExitObservable: Observable, - sessionExpireObservable: Observable, + sessionManager: RumSessionManager, createEncoder: (streamId: DeflateEncoderStreamId) => Encoder ) { const replica = configuration.replica @@ -35,10 +43,12 @@ export function startRumBatch( }, reportError, pageMayExitObservable, - sessionExpireObservable + sessionManager.expireObservable ) - lifeCycle.subscribe(LifeCycleEventType.RUM_EVENT_COLLECTED, (serverRumEvent: RumEvent & Context) => { + // Events reach the batch through the buffer, which either forwards them straight away or withholds + // them until the session reports an error. A session that never errors uploads nothing at all. + startWithheldEventBuffer(lifeCycle, sessionManager, (serverRumEvent: RumEvent & Context) => { if (serverRumEvent.type === RumEventType.VIEW) { batch.upsert(serverRumEvent, serverRumEvent.view.id) } else { diff --git a/packages/rum-core/src/transport/withheldEventBuffer.spec.ts b/packages/rum-core/src/transport/withheldEventBuffer.spec.ts new file mode 100644 index 0000000000..c8c9ff169a --- /dev/null +++ b/packages/rum-core/src/transport/withheldEventBuffer.spec.ts @@ -0,0 +1,199 @@ +import type { Context } from '@flashcatcloud/browser-core' +import { ONE_SECOND, PageExitReason } from '@flashcatcloud/browser-core' +import type { Clock } from '@flashcatcloud/browser-core/test' +import { mockClock, registerCleanupTask } from '@flashcatcloud/browser-core/test' +import { createRumSessionManagerMock } from '../../test' +import { RumEventType } from '../rawRumEvent.types' +import type { RumEvent } from '../rumEvent.types' +import { LifeCycle, LifeCycleEventType } from '../domain/lifeCycle' +import { + WITHHELD_BUFFER_DURATION, + WITHHELD_BUFFER_EVENTS_LIMIT, + WITHHELD_BUFFER_RELEASE_MAX_DELAY, + startWithheldEventBuffer, +} from './withheldEventBuffer' + +describe('startWithheldEventBuffer', () => { + let clock: Clock + let lifeCycle: LifeCycle + let sessionManager: ReturnType + let forwarded: Array + + function collect(type: RumEventType, overrides: Context = {}) { + const event = { + type, + date: 1234, + view: { id: 'view-1' }, + session: {}, + ...(type === RumEventType.RESOURCE ? { resource: { status_code: 200 } } : {}), + ...overrides, + } as unknown as RumEvent & Context + lifeCycle.notify(LifeCycleEventType.RUM_EVENT_COLLECTED, event) + return event + } + + /** Everything the buffer released, once the release jitter has elapsed. */ + function releasedAfterJitter() { + clock.tick(WITHHELD_BUFFER_RELEASE_MAX_DELAY) + return forwarded + } + + beforeEach(() => { + clock = mockClock() + lifeCycle = new LifeCycle() + forwarded = [] + sessionManager = createRumSessionManagerMock().setTrackedOnError() + const { stop } = startWithheldEventBuffer(lifeCycle, sessionManager, (event) => forwarded.push(event)) + registerCleanupTask(() => { + stop() + clock.cleanup() + }) + }) + + it('forwards immediately when the session is not withholding', () => { + sessionManager.setTrackedWithSessionReplay() + + collect(RumEventType.VIEW) + collect(RumEventType.RESOURCE) + + expect(forwarded.length).toBe(2) + }) + + it('uploads nothing while the session has not reported an error', () => { + collect(RumEventType.VIEW) + collect(RumEventType.RESOURCE) + collect(RumEventType.ACTION) + clock.tick(30 * ONE_SECOND) + + expect(forwarded.length).toBe(0) + }) + + it('releases the buffer once the session reports an error', () => { + collect(RumEventType.VIEW) + collect(RumEventType.RESOURCE) + collect(RumEventType.ACTION) + + sessionManager.setSessionHasError() + collect(RumEventType.ERROR) + + const released = releasedAfterJitter() + expect(released.map((event) => event.type)).toEqual([ + RumEventType.VIEW, + RumEventType.RESOURCE, + RumEventType.ACTION, + RumEventType.ERROR, + ]) + }) + + it('marks how far back the released detail reaches', () => { + collect(RumEventType.VIEW) + collect(RumEventType.RESOURCE, { date: 4321 }) + + sessionManager.setSessionHasError() + collect(RumEventType.ERROR) + + const view = releasedAfterJitter().find((event) => event.type === RumEventType.VIEW)! + expect((view.session as Context).detail_sampled_from).toBe(4321) + }) + + it('keeps only the latest event of a view, since a view event supersedes the ones before it', () => { + collect(RumEventType.VIEW, { documentVersion: 1 }) + collect(RumEventType.VIEW, { documentVersion: 2 }) + collect(RumEventType.VIEW, { documentVersion: 3 }) + + sessionManager.setSessionHasError() + collect(RumEventType.ERROR) + + const views = releasedAfterJitter().filter((event) => event.type === RumEventType.VIEW) + expect(views.length).toBe(1) + expect((views[0] as unknown as Context).documentVersion).toBe(3) + }) + + it('drops detail that has aged out of the window', () => { + collect(RumEventType.VIEW) + collect(RumEventType.RESOURCE) + clock.tick(WITHHELD_BUFFER_DURATION + ONE_SECOND) + collect(RumEventType.ACTION) + + sessionManager.setSessionHasError() + collect(RumEventType.ERROR) + + const types = releasedAfterJitter().map((event) => event.type) + expect(types).not.toContain(RumEventType.RESOURCE) + expect(types).toContain(RumEventType.ACTION) + }) + + it('drops the buffer when the session expires without ever reporting an error', () => { + collect(RumEventType.VIEW) + collect(RumEventType.RESOURCE) + + sessionManager.setNotTracked() + collect(RumEventType.RESOURCE) + + expect(releasedAfterJitter().filter((event) => event.type === RumEventType.RESOURCE).length).toBe(1) + }) + + it('drops the buffer on page exit rather than uploading a session that never errored', () => { + collect(RumEventType.VIEW) + collect(RumEventType.RESOURCE) + + lifeCycle.notify(LifeCycleEventType.PAGE_MAY_EXIT, { reason: PageExitReason.UNLOADING }) + + expect(releasedAfterJitter().length).toBe(0) + }) + + it('sends a release that is still waiting on jitter when the page is about to go', () => { + collect(RumEventType.VIEW) + collect(RumEventType.RESOURCE) + sessionManager.setSessionHasError() + collect(RumEventType.ERROR) + // still inside the jitter window: the error rides along with the buffer, so nothing left yet + expect(forwarded.length).toBe(0) + + lifeCycle.notify(LifeCycleEventType.PAGE_MAY_EXIT, { reason: PageExitReason.UNLOADING }) + + expect(forwarded.map((event) => event.type)).toEqual([RumEventType.VIEW, RumEventType.RESOURCE, RumEventType.ERROR]) + }) + + it('drops long tasks before actions when it runs out of room', () => { + collect(RumEventType.VIEW) + for (let i = 0; i < WITHHELD_BUFFER_EVENTS_LIMIT; i++) { + collect(RumEventType.LONG_TASK) + } + collect(RumEventType.ACTION) + + sessionManager.setSessionHasError() + collect(RumEventType.ERROR) + + expect(releasedAfterJitter().map((event) => event.type)).toContain(RumEventType.ACTION) + }) + + it('never drops errors, however full the buffer gets', () => { + collect(RumEventType.VIEW) + collect(RumEventType.ERROR, { date: 1 }) + for (let i = 0; i < WITHHELD_BUFFER_EVENTS_LIMIT * 2; i++) { + collect(RumEventType.LONG_TASK) + } + + sessionManager.setSessionHasError() + collect(RumEventType.ERROR, { date: 2 }) + + const errors = releasedAfterJitter().filter((event) => event.type === RumEventType.ERROR) + expect(errors.some((event) => event.date === 1)).toBeTrue() + }) + + it('does not release detail whose view is no longer buffered', () => { + collect(RumEventType.VIEW, { view: { id: 'old-view' } }) + collect(RumEventType.RESOURCE, { view: { id: 'old-view' } }) + // push the old view out of the view map + for (let i = 0; i < 60; i++) { + collect(RumEventType.VIEW, { view: { id: `view-${i}` } }) + } + + sessionManager.setSessionHasError() + collect(RumEventType.ERROR) + + const released = releasedAfterJitter() + expect(released.some((event) => event.type === RumEventType.RESOURCE)).toBeFalse() + }) +}) diff --git a/packages/rum-core/src/transport/withheldEventBuffer.ts b/packages/rum-core/src/transport/withheldEventBuffer.ts new file mode 100644 index 0000000000..edefdda24f --- /dev/null +++ b/packages/rum-core/src/transport/withheldEventBuffer.ts @@ -0,0 +1,253 @@ +import type { Context, RelativeTime, TimeoutId } from '@flashcatcloud/browser-core' +import { + ONE_KIBI_BYTE, + ONE_SECOND, + addTelemetryDebug, + clearTimeout, + jsonStringify, + relativeNow, + setTimeout, +} from '@flashcatcloud/browser-core' +import type { LifeCycle } from '../domain/lifeCycle' +import { LifeCycleEventType } from '../domain/lifeCycle' +import type { RumSessionManager } from '../domain/rumSessionManager' +import { RumEventType } from '../rawRumEvent.types' +import type { RumEvent } from '../rumEvent.types' + +/** + * How much history a withheld buffer may span. Same number as the replay side, because it is the + * same promise to the customer: an error session shows the minute leading up to the error. + */ +export const WITHHELD_BUFFER_DURATION = 60 * ONE_SECOND + +/** Memory bound. Above it the least valuable events are dropped first, see {@link EvictionTier}. */ +export const WITHHELD_BUFFER_BYTES_LIMIT = 64 * ONE_KIBI_BYTE +export const WITHHELD_BUFFER_EVENTS_LIMIT = 200 + +/** + * A view is the container its events hang from: the backend builds the session row out of view + * events, so a detail released without its view would be unreachable. Views are kept out of the + * eviction budget for that reason, and this only bounds pathological single-page navigation counts. + */ +export const WITHHELD_BUFFER_VIEWS_LIMIT = 50 + +/** + * Correlated errors make every client release at the same instant, right when whatever caused them + * is already under strain. Releases are spread over this window instead. + */ +export const WITHHELD_BUFFER_RELEASE_MAX_DELAY = 3 * ONE_SECOND + +/** What gets dropped first when the buffer is over budget. Lower goes first. */ +const enum EvictionTier { + /** Long tasks, and requests that succeeded without complaint. */ + FIRST, + /** Actions and vitals: they explain what the user was doing. */ + LAST, + /** Errors are the reason the session is kept at all. */ + NEVER, +} + +interface WithheldEvent { + event: RumEvent & Context + viewId: string + time: RelativeTime + bytes: number + tier: EvictionTier +} + +export function startWithheldEventBuffer( + lifeCycle: LifeCycle, + sessionManager: RumSessionManager, + forward: (event: RumEvent & Context) => void +) { + /** Latest event per view, in insertion order. */ + let views = new Map() + let details: WithheldEvent[] = [] + let bytes = 0 + let withheldForSessionId: string | undefined + let releaseTimeoutId: TimeoutId | undefined + let droppedCount = 0 + + const eventSubscription = lifeCycle.subscribe(LifeCycleEventType.RUM_EVENT_COLLECTED, (event) => { + const session = sessionManager.findTrackedSession() + + if (session?.eventsWithheld) { + if (withheldForSessionId !== undefined && withheldForSessionId !== session.id) { + // A renewed session is a different session: it draws its own sampling and starts without an + // error, so what the previous one collected must not ride along. + discard() + } + withheldForSessionId = session.id + hold(event) + return + } + + if (withheldForSessionId !== undefined) { + if (session && session.id === withheldForSessionId) { + // The session just reported its error. This event - typically the error itself - joins what + // is held so that the whole history leaves in order, and behind the same jitter. + hold(event) + scheduleRelease() + return + } + // The session that was withholding is gone without ever reporting an error. + discard() + } + + forward(event) + }) + + // Whatever is still held when the page goes away belongs to a session that never reported an + // error, so it is dropped rather than sent. A release already scheduled is sent immediately + // instead of losing it to the jitter window. + const pageMayExitSubscription = lifeCycle.subscribe(LifeCycleEventType.PAGE_MAY_EXIT, () => { + if (releaseTimeoutId !== undefined) { + release() + } else { + discard() + } + }) + + function hold(event: RumEvent & Context) { + if (event.type === RumEventType.VIEW) { + // Upsert: a view event is cumulative, so the latest one supersedes the ones before it. This + // mirrors what the batch already does with view events. + views.delete(event.view.id) + views.set(event.view.id, event) + while (views.size > WITHHELD_BUFFER_VIEWS_LIMIT) { + views.delete(views.keys().next().value!) + } + return + } + + const serialized = jsonStringify(event) + details.push({ + event, + viewId: event.view.id, + time: relativeNow(), + bytes: serialized ? serialized.length : 0, + tier: getEvictionTier(event), + }) + bytes += details[details.length - 1].bytes + + prune() + while (details.length > WITHHELD_BUFFER_EVENTS_LIMIT || bytes > WITHHELD_BUFFER_BYTES_LIMIT) { + if (!evictOne()) { + break + } + } + } + + /** Drops what has aged out of the window, so the span kept is the one we promise. */ + function prune() { + const oldestAllowed = (relativeNow() - WITHHELD_BUFFER_DURATION) as RelativeTime + let cutoff = 0 + while (cutoff < details.length && details[cutoff].time < oldestAllowed) { + bytes -= details[cutoff].bytes + droppedCount += 1 + cutoff += 1 + } + if (cutoff > 0) { + details = details.slice(cutoff) + } + } + + /** Removes the oldest event of the least valuable tier present. Returns false when empty. */ + function evictOne() { + for (const tier of [EvictionTier.FIRST, EvictionTier.LAST, EvictionTier.NEVER]) { + const index = details.findIndex((held) => held.tier === tier) + if (index !== -1) { + bytes -= details[index].bytes + droppedCount += 1 + details.splice(index, 1) + return true + } + } + return false + } + + function scheduleRelease() { + if (releaseTimeoutId !== undefined) { + return + } + releaseTimeoutId = setTimeout(release, computeReleaseDelay(withheldForSessionId!)) + } + + function release() { + clearTimeout(releaseTimeoutId) + releaseTimeoutId = undefined + prune() + + // A detail whose view is gone has no container to hang from, so it would be unreachable. + const releasable = details.filter((held) => views.has(held.viewId)) + const detailSampledFrom = releasable.length > 0 ? releasable[0].event.date : undefined + + views.forEach((view) => { + // `sampled_for_error` is stamped at assembly for every view of the session; only the point the + // detail actually reaches back to is known here. + if (detailSampledFrom !== undefined) { + view.session.detail_sampled_from = detailSampledFrom + } + forward(view) + }) + releasable.forEach((held) => forward(held.event)) + + addTelemetryDebug('Error session event buffer released', { + 'buffer.views_count': views.size, + 'buffer.events_count': releasable.length, + 'buffer.dropped_count': droppedCount, + 'buffer.bytes': bytes, + }) + + reset() + } + + function discard() { + clearTimeout(releaseTimeoutId) + releaseTimeoutId = undefined + reset() + } + + function reset() { + views = new Map() + details = [] + bytes = 0 + droppedCount = 0 + withheldForSessionId = undefined + } + + return { + stop: () => { + discard() + eventSubscription.unsubscribe() + pageMayExitSubscription.unsubscribe() + }, + } +} + +function getEvictionTier(event: RumEvent): EvictionTier { + switch (event.type) { + case RumEventType.ERROR: + return EvictionTier.NEVER + case RumEventType.LONG_TASK: + return EvictionTier.FIRST + case RumEventType.RESOURCE: { + // A request that failed is part of how the error happened; one that succeeded rarely is. + const statusCode = event.resource?.status_code + return statusCode === 0 || (statusCode !== undefined && statusCode >= 400) + ? EvictionTier.LAST + : EvictionTier.FIRST + } + default: + return EvictionTier.LAST + } +} + +/** Deterministic per session, so a client always spreads to the same offset. */ +export function computeReleaseDelay(sessionId: string) { + let hash = 0 + for (let i = 0; i < sessionId.length; i += 1) { + hash = (hash + sessionId.charCodeAt(i)) % WITHHELD_BUFFER_RELEASE_MAX_DELAY + } + return hash +} diff --git a/packages/rum-core/test/mockRumSessionManager.ts b/packages/rum-core/test/mockRumSessionManager.ts index 9314b732a9..4a1ebfe986 100644 --- a/packages/rum-core/test/mockRumSessionManager.ts +++ b/packages/rum-core/test/mockRumSessionManager.ts @@ -1,5 +1,11 @@ import { Observable } from '@flashcatcloud/browser-core' -import { RumTrackingType, computeSessionReplayState, type RumSessionManager } from '../src/domain/rumSessionManager' +import { + RumTrackingType, + computeEventsWithheld, + computeSessionReplayState, + withholdsEvents, + type RumSessionManager, +} from '../src/domain/rumSessionManager' export interface RumSessionManagerMock extends RumSessionManager { setId(id: string): RumSessionManagerMock @@ -7,6 +13,7 @@ export interface RumSessionManagerMock extends RumSessionManager { setTrackedWithoutSessionReplay(): RumSessionManagerMock setTrackedWithSessionReplay(): RumSessionManagerMock setTrackedWithErrorSessionReplay(): RumSessionManagerMock + setTrackedOnError(): RumSessionManagerMock setForcedReplay(): RumSessionManagerMock setSessionHasError(): RumSessionManagerMock } @@ -16,6 +23,7 @@ const enum SessionStatus { TRACKED_WITH_SESSION_REPLAY, TRACKED_WITHOUT_SESSION_REPLAY, TRACKED_WITH_ERROR_SESSION_REPLAY, + TRACKED_ON_ERROR, NOT_TRACKED, EXPIRED, } @@ -24,6 +32,7 @@ const TRACKING_TYPES: { [key in SessionStatus]?: RumTrackingType } = { [SessionStatus.TRACKED_WITH_SESSION_REPLAY]: RumTrackingType.TRACKED_WITH_SESSION_REPLAY, [SessionStatus.TRACKED_WITHOUT_SESSION_REPLAY]: RumTrackingType.TRACKED_WITHOUT_SESSION_REPLAY, [SessionStatus.TRACKED_WITH_ERROR_SESSION_REPLAY]: RumTrackingType.TRACKED_WITH_ERROR_SESSION_REPLAY, + [SessionStatus.TRACKED_ON_ERROR]: RumTrackingType.TRACKED_ON_ERROR_WITH_SESSION_REPLAY, } export function createRumSessionManagerMock(): RumSessionManagerMock { @@ -41,6 +50,8 @@ export function createRumSessionManagerMock(): RumSessionManagerMock { id, // Derived the same way as in production, so the mock cannot drift from the real state machine sessionReplay: computeSessionReplayState(trackingType, hasError, forcedReplay), + eventsWithheld: computeEventsWithheld(trackingType, hasError, forcedReplay), + sampledOnError: withholdsEvents(trackingType), anonymousId: 'device-123', } }, @@ -69,6 +80,10 @@ export function createRumSessionManagerMock(): RumSessionManagerMock { sessionStatus = SessionStatus.TRACKED_WITH_ERROR_SESSION_REPLAY return this }, + setTrackedOnError() { + sessionStatus = SessionStatus.TRACKED_ON_ERROR + return this + }, setForcedReplay() { forcedReplay = true return this From 9cd24fa26247c9ec238697ee8823fed635e4684f Mon Sep 17 00:00:00 2001 From: Fiona Date: Thu, 20 Aug 2026 04:48:07 -0700 Subject: [PATCH 02/27] refactor(rum): trim the withheld event buffer 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. --- .../src/transport/withheldEventBuffer.ts | 46 +++++++++---------- 1 file changed, 21 insertions(+), 25 deletions(-) diff --git a/packages/rum-core/src/transport/withheldEventBuffer.ts b/packages/rum-core/src/transport/withheldEventBuffer.ts index edefdda24f..40cdfa14eb 100644 --- a/packages/rum-core/src/transport/withheldEventBuffer.ts +++ b/packages/rum-core/src/transport/withheldEventBuffer.ts @@ -21,7 +21,7 @@ import type { RumEvent } from '../rumEvent.types' export const WITHHELD_BUFFER_DURATION = 60 * ONE_SECOND /** Memory bound. Above it the least valuable events are dropped first, see {@link EvictionTier}. */ -export const WITHHELD_BUFFER_BYTES_LIMIT = 64 * ONE_KIBI_BYTE +const WITHHELD_BUFFER_BYTES_LIMIT = 64 * ONE_KIBI_BYTE export const WITHHELD_BUFFER_EVENTS_LIMIT = 200 /** @@ -29,7 +29,7 @@ export const WITHHELD_BUFFER_EVENTS_LIMIT = 200 * events, so a detail released without its view would be unreachable. Views are kept out of the * eviction budget for that reason, and this only bounds pathological single-page navigation counts. */ -export const WITHHELD_BUFFER_VIEWS_LIMIT = 50 +const WITHHELD_BUFFER_VIEWS_LIMIT = 50 /** * Correlated errors make every client release at the same instant, right when whatever caused them @@ -75,7 +75,7 @@ export function startWithheldEventBuffer( if (withheldForSessionId !== undefined && withheldForSessionId !== session.id) { // A renewed session is a different session: it draws its own sampling and starts without an // error, so what the previous one collected must not ride along. - discard() + clearBuffer() } withheldForSessionId = session.id hold(event) @@ -91,7 +91,7 @@ export function startWithheldEventBuffer( return } // The session that was withholding is gone without ever reporting an error. - discard() + clearBuffer() } forward(event) @@ -104,14 +104,16 @@ export function startWithheldEventBuffer( if (releaseTimeoutId !== undefined) { release() } else { - discard() + clearBuffer() } }) function hold(event: RumEvent & Context) { if (event.type === RumEventType.VIEW) { // Upsert: a view event is cumulative, so the latest one supersedes the ones before it. This - // mirrors what the batch already does with view events. + // mirrors what the batch already does with view events. The delete is deliberate - setting an + // existing key leaves its insertion order untouched, so without it the oldest entry would be + // the first view seen rather than the least recently updated one. views.delete(event.view.id) views.set(event.view.id, event) while (views.size > WITHHELD_BUFFER_VIEWS_LIMIT) { @@ -120,15 +122,15 @@ export function startWithheldEventBuffer( return } - const serialized = jsonStringify(event) - details.push({ + const held: WithheldEvent = { event, viewId: event.view.id, time: relativeNow(), - bytes: serialized ? serialized.length : 0, + bytes: jsonStringify(event)?.length ?? 0, tier: getEvictionTier(event), - }) - bytes += details[details.length - 1].bytes + } + details.push(held) + bytes += held.bytes prune() while (details.length > WITHHELD_BUFFER_EVENTS_LIMIT || bytes > WITHHELD_BUFFER_BYTES_LIMIT) { @@ -174,8 +176,6 @@ export function startWithheldEventBuffer( } function release() { - clearTimeout(releaseTimeoutId) - releaseTimeoutId = undefined prune() // A detail whose view is gone has no container to hang from, so it would be unreachable. @@ -199,16 +199,13 @@ export function startWithheldEventBuffer( 'buffer.bytes': bytes, }) - reset() + clearBuffer() } - function discard() { + /** Empties the buffer, whether it was just released or is being thrown away. */ + function clearBuffer() { clearTimeout(releaseTimeoutId) releaseTimeoutId = undefined - reset() - } - - function reset() { views = new Map() details = [] bytes = 0 @@ -218,7 +215,7 @@ export function startWithheldEventBuffer( return { stop: () => { - discard() + clearBuffer() eventSubscription.unsubscribe() pageMayExitSubscription.unsubscribe() }, @@ -233,10 +230,9 @@ function getEvictionTier(event: RumEvent): EvictionTier { return EvictionTier.FIRST case RumEventType.RESOURCE: { // A request that failed is part of how the error happened; one that succeeded rarely is. - const statusCode = event.resource?.status_code - return statusCode === 0 || (statusCode !== undefined && statusCode >= 400) - ? EvictionTier.LAST - : EvictionTier.FIRST + // -1 stands for an unknown status code, which is treated like an ordinary success + const statusCode = event.resource?.status_code ?? -1 + return statusCode === 0 || statusCode >= 400 ? EvictionTier.LAST : EvictionTier.FIRST } default: return EvictionTier.LAST @@ -244,7 +240,7 @@ function getEvictionTier(event: RumEvent): EvictionTier { } /** Deterministic per session, so a client always spreads to the same offset. */ -export function computeReleaseDelay(sessionId: string) { +function computeReleaseDelay(sessionId: string) { let hash = 0 for (let i = 0; i < sessionId.length; i += 1) { hash = (hash + sessionId.charCodeAt(i)) % WITHHELD_BUFFER_RELEASE_MAX_DELAY From db44a198508e754a8c21e6963ed22faabfd48975 Mon Sep 17 00:00:00 2001 From: Fiona Date: Thu, 20 Aug 2026 04:54:30 -0700 Subject: [PATCH 03/27] fix(rum): spread releases properly, and release on exit when the error 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. --- .../src/transport/withheldEventBuffer.spec.ts | 60 +++++++++++++++++++ .../src/transport/withheldEventBuffer.ts | 33 +++++++--- 2 files changed, 85 insertions(+), 8 deletions(-) diff --git a/packages/rum-core/src/transport/withheldEventBuffer.spec.ts b/packages/rum-core/src/transport/withheldEventBuffer.spec.ts index c8c9ff169a..5f9b6c83db 100644 --- a/packages/rum-core/src/transport/withheldEventBuffer.spec.ts +++ b/packages/rum-core/src/transport/withheldEventBuffer.spec.ts @@ -10,6 +10,7 @@ import { WITHHELD_BUFFER_DURATION, WITHHELD_BUFFER_EVENTS_LIMIT, WITHHELD_BUFFER_RELEASE_MAX_DELAY, + computeReleaseDelay, startWithheldEventBuffer, } from './withheldEventBuffer' @@ -133,6 +134,19 @@ describe('startWithheldEventBuffer', () => { expect(releasedAfterJitter().filter((event) => event.type === RumEventType.RESOURCE).length).toBe(1) }) + it('releases on page exit when the session errored without the buffer having noticed yet', () => { + // the event arrives synchronously, but the session state behind it is written through a lock + // that can defer the write - so the buffer can still read the session as withholding + collect(RumEventType.VIEW) + collect(RumEventType.RESOURCE) + sessionManager.setSessionHasError() + // no further event, so nothing re-reads the session before the page goes + + lifeCycle.notify(LifeCycleEventType.PAGE_MAY_EXIT, { reason: PageExitReason.UNLOADING }) + + expect(forwarded.map((event) => event.type)).toEqual([RumEventType.VIEW, RumEventType.RESOURCE]) + }) + it('drops the buffer on page exit rather than uploading a session that never errored', () => { collect(RumEventType.VIEW) collect(RumEventType.RESOURCE) @@ -197,3 +211,49 @@ describe('startWithheldEventBuffer', () => { expect(released.some((event) => event.type === RumEventType.RESOURCE)).toBeFalse() }) }) + +describe('computeReleaseDelay', () => { + function randomSessionId() { + const hex = '0123456789abcdef' + let id = '' + for (let i = 0; i < 36; i++) { + id += i === 8 || i === 13 || i === 18 || i === 23 ? '-' : hex[Math.floor(Math.random() * 16)] + } + return id + } + + it('is stable for a given session', () => { + const id = randomSessionId() + + expect(computeReleaseDelay(id)).toBe(computeReleaseDelay(id)) + }) + + it('stays within the release window', () => { + for (let i = 0; i < 1000; i++) { + const delay = computeReleaseDelay(randomSessionId()) + expect(delay).toBeGreaterThanOrEqual(0) + expect(delay).toBeLessThan(WITHHELD_BUFFER_RELEASE_MAX_DELAY) + } + }) + + it('spreads sessions across the window rather than bunching them up', () => { + // session ids are same-length strings over one small alphabet, so a running sum of their + // character codes lands nearly all of them within a few hundred ms of each other - which delays + // the herd instead of spreading it + const bucketCount = 10 + const buckets = new Array(bucketCount).fill(0) + const samples = 10000 + for (let i = 0; i < samples; i++) { + const bucket = Math.floor( + (computeReleaseDelay(randomSessionId()) / WITHHELD_BUFFER_RELEASE_MAX_DELAY) * bucketCount + ) + buckets[bucket] += 1 + } + + buckets.forEach((count) => { + // a flat spread puts 10% in each; allow a wide margin and still catch bunching + expect(count / samples).toBeGreaterThan(0.05) + expect(count / samples).toBeLessThan(0.2) + }) + }) +}) diff --git a/packages/rum-core/src/transport/withheldEventBuffer.ts b/packages/rum-core/src/transport/withheldEventBuffer.ts index 40cdfa14eb..8a798e1c40 100644 --- a/packages/rum-core/src/transport/withheldEventBuffer.ts +++ b/packages/rum-core/src/transport/withheldEventBuffer.ts @@ -97,13 +97,21 @@ export function startWithheldEventBuffer( forward(event) }) - // Whatever is still held when the page goes away belongs to a session that never reported an - // error, so it is dropped rather than sent. A release already scheduled is sent immediately - // instead of losing it to the jitter window. const pageMayExitSubscription = lifeCycle.subscribe(LifeCycleEventType.PAGE_MAY_EXIT, () => { - if (releaseTimeoutId !== undefined) { + if (withheldForSessionId === undefined) { + return + } + // A release already scheduled goes out now rather than being lost to the jitter window. The + // session is also re-read, because it may have reported 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, and "an error, then the user leaves" is exactly what this feature is for. + const session = sessionManager.findTrackedSession() + const hasSinceErrored = !!session && session.id === withheldForSessionId && !session.eventsWithheld + + if (releaseTimeoutId !== undefined || hasSinceErrored) { release() } else { + // Nothing was ever released for this session, so what is held goes no further. clearBuffer() } }) @@ -239,11 +247,20 @@ function getEvictionTier(event: RumEvent): EvictionTier { } } -/** Deterministic per session, so a client always spreads to the same offset. */ -function computeReleaseDelay(sessionId: string) { +/** Keeps the running hash inside the range `Math.imul` is exact over. */ +const LARGEST_INT32_PRIME = 2147483647 + +/** + * Deterministic per session, so a client always spreads to the same offset. + * + * Multiplicative rather than a running sum: session ids are same-length strings drawn from the same + * small alphabet, so summing their character codes lands almost every session within a few hundred + * milliseconds of the same value - which delays the herd instead of spreading it. + */ +export function computeReleaseDelay(sessionId: string) { let hash = 0 for (let i = 0; i < sessionId.length; i += 1) { - hash = (hash + sessionId.charCodeAt(i)) % WITHHELD_BUFFER_RELEASE_MAX_DELAY + hash = (Math.imul(hash, 31) + sessionId.charCodeAt(i)) % LARGEST_INT32_PRIME } - return hash + return Math.abs(hash) % WITHHELD_BUFFER_RELEASE_MAX_DELAY } From 6698ae6999b70b698c012c1826d99ca25062a185 Mon Sep 17 00:00:00 2001 From: Fiona Date: Thu, 20 Aug 2026 04:56:48 -0700 Subject: [PATCH 04/27] fix(rum): count buffered bytes as bytes, and stop calling a tier that 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. --- .../src/transport/withheldEventBuffer.spec.ts | 16 ++++++++ .../src/transport/withheldEventBuffer.ts | 37 ++++++++++++++----- 2 files changed, 44 insertions(+), 9 deletions(-) diff --git a/packages/rum-core/src/transport/withheldEventBuffer.spec.ts b/packages/rum-core/src/transport/withheldEventBuffer.spec.ts index 5f9b6c83db..68dcfd9503 100644 --- a/packages/rum-core/src/transport/withheldEventBuffer.spec.ts +++ b/packages/rum-core/src/transport/withheldEventBuffer.spec.ts @@ -196,6 +196,22 @@ describe('startWithheldEventBuffer', () => { expect(errors.some((event) => event.date === 1)).toBeTrue() }) + it('gives up the newest error rather than the first one when only errors are left', () => { + collect(RumEventType.VIEW) + for (let i = 0; i < WITHHELD_BUFFER_EVENTS_LIMIT + 20; i++) { + collect(RumEventType.ERROR, { date: i }) + } + + sessionManager.setSessionHasError() + collect(RumEventType.ERROR, { date: 9999 }) + + const dates = releasedAfterJitter() + .filter((event) => event.type === RumEventType.ERROR) + .map((event) => event.date) + // the first error - the one the session is about - survives + expect(dates).toContain(0) + }) + it('does not release detail whose view is no longer buffered', () => { collect(RumEventType.VIEW, { view: { id: 'old-view' } }) collect(RumEventType.RESOURCE, { view: { id: 'old-view' } }) diff --git a/packages/rum-core/src/transport/withheldEventBuffer.ts b/packages/rum-core/src/transport/withheldEventBuffer.ts index 8a798e1c40..803342232d 100644 --- a/packages/rum-core/src/transport/withheldEventBuffer.ts +++ b/packages/rum-core/src/transport/withheldEventBuffer.ts @@ -4,6 +4,7 @@ import { ONE_SECOND, addTelemetryDebug, clearTimeout, + computeBytesCount, jsonStringify, relativeNow, setTimeout, @@ -43,8 +44,12 @@ const enum EvictionTier { FIRST, /** Actions and vitals: they explain what the user was doing. */ LAST, - /** Errors are the reason the session is kept at all. */ - NEVER, + /** + * Errors are the reason the session is kept at all, so they go only once nothing else is left - + * and even then the newest goes first, because the earliest error is the one that releases the + * buffer and the one the session is about. + */ + LAST_RESORT, } interface WithheldEvent { @@ -134,7 +139,7 @@ export function startWithheldEventBuffer( event, viewId: event.view.id, time: relativeNow(), - bytes: jsonStringify(event)?.length ?? 0, + bytes: computeBytesCount(jsonStringify(event) ?? ''), tier: getEvictionTier(event), } details.push(held) @@ -162,20 +167,34 @@ export function startWithheldEventBuffer( } } - /** Removes the oldest event of the least valuable tier present. Returns false when empty. */ + /** Removes one event of the least valuable tier present. Returns false when there is none left. */ function evictOne() { - for (const tier of [EvictionTier.FIRST, EvictionTier.LAST, EvictionTier.NEVER]) { + for (const tier of [EvictionTier.FIRST, EvictionTier.LAST]) { const index = details.findIndex((held) => held.tier === tier) if (index !== -1) { - bytes -= details[index].bytes - droppedCount += 1 - details.splice(index, 1) + evictAt(index) + return true + } + } + + // Only errors are left. One still has to go to stay within budget, and it is the newest: an + // error storm would otherwise push out the first error, which is the one that released the + // buffer and the one the session is really about. + for (let index = details.length - 1; index >= 0; index -= 1) { + if (details[index].tier === EvictionTier.LAST_RESORT) { + evictAt(index) return true } } return false } + function evictAt(index: number) { + bytes -= details[index].bytes + droppedCount += 1 + details.splice(index, 1) + } + function scheduleRelease() { if (releaseTimeoutId !== undefined) { return @@ -233,7 +252,7 @@ export function startWithheldEventBuffer( function getEvictionTier(event: RumEvent): EvictionTier { switch (event.type) { case RumEventType.ERROR: - return EvictionTier.NEVER + return EvictionTier.LAST_RESORT case RumEventType.LONG_TASK: return EvictionTier.FIRST case RumEventType.RESOURCE: { From 30dcbe0e4054d17c6c444422d3dde2a751789bb1 Mon Sep 17 00:00:00 2001 From: Fiona Date: Thu, 20 Aug 2026 06:48:24 -0700 Subject: [PATCH 05/27] fix(rum): settle the buffer when the session ends, and let stale views 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. --- .../rum-core/src/transport/startRumBatch.ts | 12 +++++-- .../src/transport/withheldEventBuffer.spec.ts | 33 +++++++++++++++---- .../src/transport/withheldEventBuffer.ts | 30 +++++++++++++++-- 3 files changed, 64 insertions(+), 11 deletions(-) diff --git a/packages/rum-core/src/transport/startRumBatch.ts b/packages/rum-core/src/transport/startRumBatch.ts index 42456b295f..f8a1b8bf30 100644 --- a/packages/rum-core/src/transport/startRumBatch.ts +++ b/packages/rum-core/src/transport/startRumBatch.ts @@ -48,7 +48,7 @@ export function startRumBatch( // Events reach the batch through the buffer, which either forwards them straight away or withholds // them until the session reports an error. A session that never errors uploads nothing at all. - startWithheldEventBuffer(lifeCycle, sessionManager, (serverRumEvent: RumEvent & Context) => { + const withheldEventBuffer = startWithheldEventBuffer(lifeCycle, sessionManager, (serverRumEvent) => { if (serverRumEvent.type === RumEventType.VIEW) { batch.upsert(serverRumEvent, serverRumEvent.view.id) } else { @@ -58,5 +58,13 @@ export function startRumBatch( telemetryEventObservable.subscribe((event) => batch.add(event, isTelemetryReplicationAllowed(configuration))) - return batch + return { + ...batch, + stop: () => { + // Stops the buffer too, so a release waiting on its jitter cannot fire into a batch that is + // no longer flushing. + withheldEventBuffer.stop() + batch.stop() + }, + } } diff --git a/packages/rum-core/src/transport/withheldEventBuffer.spec.ts b/packages/rum-core/src/transport/withheldEventBuffer.spec.ts index 68dcfd9503..158d3453d0 100644 --- a/packages/rum-core/src/transport/withheldEventBuffer.spec.ts +++ b/packages/rum-core/src/transport/withheldEventBuffer.spec.ts @@ -212,19 +212,40 @@ describe('startWithheldEventBuffer', () => { expect(dates).toContain(0) }) - it('does not release detail whose view is no longer buffered', () => { - collect(RumEventType.VIEW, { view: { id: 'old-view' } }) - collect(RumEventType.RESOURCE, { view: { id: 'old-view' } }) - // push the old view out of the view map + it('releases every detail alongside the view it hangs from', () => { + // the backend builds the session row out of view events, so a detail without its view would be + // unreachable however the view came to be missing for (let i = 0; i < 60; i++) { collect(RumEventType.VIEW, { view: { id: `view-${i}` } }) + collect(RumEventType.RESOURCE, { view: { id: `view-${i}` } }) } sessionManager.setSessionHasError() - collect(RumEventType.ERROR) + collect(RumEventType.ERROR, { view: { id: 'view-59' } }) const released = releasedAfterJitter() - expect(released.some((event) => event.type === RumEventType.RESOURCE)).toBeFalse() + const releasedViewIds = new Set( + released.filter((event) => event.type === RumEventType.VIEW).map((event) => event.view.id) + ) + released + .filter((event) => event.type !== RumEventType.VIEW) + .forEach((event) => expect(releasedViewIds.has(event.view.id)).toBeTrue()) + }) + + it('lets a view go once none of its detail is left inside the window', () => { + collect(RumEventType.VIEW, { view: { id: 'old-view' } }) + collect(RumEventType.RESOURCE, { view: { id: 'old-view' } }) + clock.tick(WITHHELD_BUFFER_DURATION + ONE_SECOND) + collect(RumEventType.VIEW, { view: { id: 'current-view' } }) + + sessionManager.setSessionHasError() + collect(RumEventType.ERROR, { view: { id: 'current-view' } }) + + const releasedViewIds = releasedAfterJitter() + .filter((event) => event.type === RumEventType.VIEW) + .map((event) => event.view.id) + expect(releasedViewIds).not.toContain('old-view') + expect(releasedViewIds).toContain('current-view') }) }) diff --git a/packages/rum-core/src/transport/withheldEventBuffer.ts b/packages/rum-core/src/transport/withheldEventBuffer.ts index 803342232d..2dee4c2b68 100644 --- a/packages/rum-core/src/transport/withheldEventBuffer.ts +++ b/packages/rum-core/src/transport/withheldEventBuffer.ts @@ -69,6 +69,7 @@ export function startWithheldEventBuffer( let views = new Map() let details: WithheldEvent[] = [] let bytes = 0 + let currentViewId: string | undefined let withheldForSessionId: string | undefined let releaseTimeoutId: TimeoutId | undefined let droppedCount = 0 @@ -102,7 +103,11 @@ export function startWithheldEventBuffer( forward(event) }) - const pageMayExitSubscription = lifeCycle.subscribe(LifeCycleEventType.PAGE_MAY_EXIT, () => { + /** + * Called when the session the buffer belongs to may be about to end - the page is going away, or + * the session expired (which is also how a withdrawn tracking consent arrives here). + */ + function settleBuffer() { if (withheldForSessionId === undefined) { return } @@ -116,10 +121,14 @@ export function startWithheldEventBuffer( if (releaseTimeoutId !== undefined || hasSinceErrored) { release() } else { - // Nothing was ever released for this session, so what is held goes no further. + // Nothing was ever released for this session, so what is held goes no further - and is not + // kept in memory either, which matters when the session ended because consent was withdrawn. clearBuffer() } - }) + } + + const pageMayExitSubscription = lifeCycle.subscribe(LifeCycleEventType.PAGE_MAY_EXIT, settleBuffer) + const sessionExpireSubscription = lifeCycle.subscribe(LifeCycleEventType.SESSION_EXPIRED, settleBuffer) function hold(event: RumEvent & Context) { if (event.type === RumEventType.VIEW) { @@ -129,9 +138,11 @@ export function startWithheldEventBuffer( // the first view seen rather than the least recently updated one. views.delete(event.view.id) views.set(event.view.id, event) + currentViewId = event.view.id while (views.size > WITHHELD_BUFFER_VIEWS_LIMIT) { views.delete(views.keys().next().value!) } + prune() return } @@ -165,6 +176,17 @@ export function startWithheldEventBuffer( if (cutoff > 0) { details = details.slice(cutoff) } + + // A view is kept as the container of the detail hanging from it, so once none of its detail is + // left inside the window it has nothing left to contain. Without this the map would grow with + // every route change for as long as the page lives, holding more than the detail budget itself. + // The view in progress always stays: it is the container the error will hang from. + const viewsWithDetail = new Set(details.map((held) => held.viewId)) + views.forEach((_, viewId) => { + if (viewId !== currentViewId && !viewsWithDetail.has(viewId)) { + views.delete(viewId) + } + }) } /** Removes one event of the least valuable tier present. Returns false when there is none left. */ @@ -237,6 +259,7 @@ export function startWithheldEventBuffer( details = [] bytes = 0 droppedCount = 0 + currentViewId = undefined withheldForSessionId = undefined } @@ -245,6 +268,7 @@ export function startWithheldEventBuffer( clearBuffer() eventSubscription.unsubscribe() pageMayExitSubscription.unsubscribe() + sessionExpireSubscription.unsubscribe() }, } } From f39523991f7ec2d10d565e7892097034d8a9106e Mon Sep 17 00:00:00 2001 From: Fiona Date: Thu, 20 Aug 2026 06:49:16 -0700 Subject: [PATCH 06/27] style: drop an import left unused by the buffer wiring --- packages/rum-core/src/transport/startRumBatch.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/rum-core/src/transport/startRumBatch.ts b/packages/rum-core/src/transport/startRumBatch.ts index f8a1b8bf30..aa03cf23d2 100644 --- a/packages/rum-core/src/transport/startRumBatch.ts +++ b/packages/rum-core/src/transport/startRumBatch.ts @@ -16,7 +16,6 @@ import type { RumConfiguration } from '../domain/configuration' import type { LifeCycle } from '../domain/lifeCycle' import type { RumSessionManager } from '../domain/rumSessionManager' import { RumEventType } from '../rawRumEvent.types' -import type { RumEvent } from '../rumEvent.types' import { startWithheldEventBuffer } from './withheldEventBuffer' export function startRumBatch( From 078a46ed5b1921225584540e9bb99108dc14d8d8 Mon Sep 17 00:00:00 2001 From: Fiona Date: Thu, 20 Aug 2026 20:47:05 -0700 Subject: [PATCH 07/27] fix(rum): keep the event buffer across a tab switch, and make the detail 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. --- .../core/src/domain/session/sessionManager.ts | 3 ++ .../src/domain/contexts/sessionContext.ts | 3 ++ .../rum-core/src/domain/rumSessionManager.ts | 19 +++++++++++ .../src/transport/withheldEventBuffer.spec.ts | 32 +++++++++++++++++-- .../src/transport/withheldEventBuffer.ts | 31 ++++++++++++------ .../rum-core/test/mockRumSessionManager.ts | 7 ++++ 6 files changed, 83 insertions(+), 12 deletions(-) diff --git a/packages/core/src/domain/session/sessionManager.ts b/packages/core/src/domain/session/sessionManager.ts index 789d0d5487..68c4d9d7a4 100644 --- a/packages/core/src/domain/session/sessionManager.ts +++ b/packages/core/src/domain/session/sessionManager.ts @@ -33,6 +33,8 @@ export interface SessionContext extends Context { * just because the user moved to another page. */ hasError: boolean + /** Where the detail stored for this session starts, when its events were withheld for a while. */ + detailSampledFrom: number | undefined anonymousId: string | undefined } @@ -99,6 +101,7 @@ export function startSessionManager( trackingType: sessionStore.getSession()[productKey] as TrackingType, isReplayForced: !!sessionStore.getSession().forcedReplay, hasError: !!sessionStore.getSession().hasError, + detailSampledFrom: Number(sessionStore.getSession().detailFrom) || undefined, anonymousId: sessionStore.getSession().anonymousId, } } diff --git a/packages/rum-core/src/domain/contexts/sessionContext.ts b/packages/rum-core/src/domain/contexts/sessionContext.ts index b3676907db..bf2bd34537 100644 --- a/packages/rum-core/src/domain/contexts/sessionContext.ts +++ b/packages/rum-core/src/domain/contexts/sessionContext.ts @@ -27,6 +27,7 @@ export function startSessionContext( let hasReplay let sampledForReplay let sampledForError + let detailSampledFrom let isActive if (eventType === RumEventType.VIEW) { hasReplay = !isReplayWithheld && recorderApi.getReplayStats(view.id) ? true : undefined @@ -34,6 +35,7 @@ export function startSessionContext( // Tells the backend that this session's detail only starts where the buffer reached, so the // gap before it reads as "not collected" rather than as missing data. sampledForError = session.sampledOnError || undefined + detailSampledFrom = session.detailSampledFrom isActive = view.sessionIsActive ? undefined : false } else { hasReplay = !isReplayWithheld && recorderApi.isRecording() ? true : undefined @@ -47,6 +49,7 @@ export function startSessionContext( has_replay: hasReplay, sampled_for_replay: sampledForReplay, sampled_for_error: sampledForError, + detail_sampled_from: detailSampledFrom, is_active: isActive, }, } diff --git a/packages/rum-core/src/domain/rumSessionManager.ts b/packages/rum-core/src/domain/rumSessionManager.ts index 5a18afc626..0b0ad13328 100644 --- a/packages/rum-core/src/domain/rumSessionManager.ts +++ b/packages/rum-core/src/domain/rumSessionManager.ts @@ -29,6 +29,8 @@ export interface RumSessionManager { * `sessionReplayOnErrorSampleRate`, this is what releases the withheld replay. */ setSessionHasError: () => void + /** Records how far back the detail released for this session actually reaches. */ + setSessionDetailSampledFrom: (timestamp: number) => void } export type RumSession = { @@ -45,6 +47,11 @@ export type RumSession = { * sampled session - its detail only starts where the buffer reached. */ sampledOnError: boolean + /** + * Where the detail stored for this session starts, for a session whose events were withheld. The + * gap before it is data that was never collected rather than data that went missing. + */ + detailSampledFrom?: number anonymousId?: string } @@ -102,6 +109,12 @@ export function startRumSessionManager( sessionEntity.hasError = true } } + if (!previousState.detailFrom && newState.detailFrom) { + const sessionEntity = sessionManager.findSession() + if (sessionEntity) { + sessionEntity.detailSampledFrom = Number(newState.detailFrom) || undefined + } + } }) return { findTrackedSession: (startTime) => { @@ -114,6 +127,7 @@ export function startRumSessionManager( sessionReplay: computeSessionReplayState(session.trackingType, session.hasError, session.isReplayForced), eventsWithheld: computeEventsWithheld(session.trackingType, session.hasError, session.isReplayForced), sampledOnError: withholdsEvents(session.trackingType), + detailSampledFrom: session.detailSampledFrom, anonymousId: session.anonymousId, } }, @@ -121,6 +135,10 @@ export function startRumSessionManager( expireObservable: sessionManager.expireObservable, setForcedReplay: () => sessionManager.updateSessionState({ forcedReplay: '1' }), setSessionHasError: () => sessionManager.updateSessionState({ hasError: '1' }), + // Kept on the session rather than stamped on the released view events: the batch upserts views + // by id, so the next ordinary view update - which arrives within seconds - would replace the + // stamped one before the batch is ever sent. + setSessionDetailSampledFrom: (timestamp) => sessionManager.updateSessionState({ detailFrom: String(timestamp) }), } } @@ -189,6 +207,7 @@ export function startRumSessionManagerStub(): RumSessionManager { expireObservable: new Observable(), setForcedReplay: noop, setSessionHasError: noop, + setSessionDetailSampledFrom: noop, } } diff --git a/packages/rum-core/src/transport/withheldEventBuffer.spec.ts b/packages/rum-core/src/transport/withheldEventBuffer.spec.ts index 158d3453d0..51ddc90a58 100644 --- a/packages/rum-core/src/transport/withheldEventBuffer.spec.ts +++ b/packages/rum-core/src/transport/withheldEventBuffer.spec.ts @@ -147,11 +147,39 @@ describe('startWithheldEventBuffer', () => { expect(forwarded.map((event) => event.type)).toEqual([RumEventType.VIEW, RumEventType.RESOURCE]) }) - it('drops the buffer on page exit rather than uploading a session that never errored', () => { + it('keeps the buffer when the page is only hidden, since it comes back', () => { + collect(RumEventType.VIEW) + collect(RumEventType.RESOURCE, { date: 111 }) + + lifeCycle.notify(LifeCycleEventType.PAGE_MAY_EXIT, { reason: PageExitReason.HIDDEN }) + expect(forwarded.length).toBe(0) + + sessionManager.setSessionHasError() + collect(RumEventType.ERROR) + + const dates = releasedAfterJitter().map((event) => event.date) + expect(dates).toContain(111) + }) + + it('records on the session how far back the released detail reaches', () => { + const spy = spyOn(sessionManager, 'setSessionDetailSampledFrom').and.callThrough() + collect(RumEventType.VIEW) + collect(RumEventType.RESOURCE, { date: 4321 }) + + sessionManager.setSessionHasError() + collect(RumEventType.ERROR) + releasedAfterJitter() + + // kept on the session, because the batch upserts views by id and the next ordinary view update + // would otherwise replace the stamped one before anything is sent + expect(spy).toHaveBeenCalledWith(4321) + }) + + it('drops the buffer when the session ends without ever having errored', () => { collect(RumEventType.VIEW) collect(RumEventType.RESOURCE) - lifeCycle.notify(LifeCycleEventType.PAGE_MAY_EXIT, { reason: PageExitReason.UNLOADING }) + lifeCycle.notify(LifeCycleEventType.SESSION_EXPIRED) expect(releasedAfterJitter().length).toBe(0) }) diff --git a/packages/rum-core/src/transport/withheldEventBuffer.ts b/packages/rum-core/src/transport/withheldEventBuffer.ts index 2dee4c2b68..d06e314ca0 100644 --- a/packages/rum-core/src/transport/withheldEventBuffer.ts +++ b/packages/rum-core/src/transport/withheldEventBuffer.ts @@ -104,10 +104,14 @@ export function startWithheldEventBuffer( }) /** - * Called when the session the buffer belongs to may be about to end - the page is going away, or - * the session expired (which is also how a withdrawn tracking consent arrives here). + * Called when what is held may not get another chance to leave: the page is going away, or the + * session ended (which is also how a withdrawn tracking consent arrives here). + * + * `discardIfUnreleased` says whether the buffer has anything left to wait for. A session that + * ended is over, so what it never released goes no further. A page being hidden is not: it comes + * back, and dropping the minute it had collected would leave the error that follows with nothing. */ - function settleBuffer() { + function settleBuffer(discardIfUnreleased: boolean) { if (withheldForSessionId === undefined) { return } @@ -120,15 +124,16 @@ export function startWithheldEventBuffer( if (releaseTimeoutId !== undefined || hasSinceErrored) { release() - } else { - // Nothing was ever released for this session, so what is held goes no further - and is not - // kept in memory either, which matters when the session ended because consent was withdrawn. + } else if (discardIfUnreleased) { clearBuffer() } } - const pageMayExitSubscription = lifeCycle.subscribe(LifeCycleEventType.PAGE_MAY_EXIT, settleBuffer) - const sessionExpireSubscription = lifeCycle.subscribe(LifeCycleEventType.SESSION_EXPIRED, settleBuffer) + // Kept on a page exit: switching tabs raises one and the page comes straight back, while a page + // that is really unloading takes the buffer with it either way - so there is nothing to gain by + // dropping it, and a minute of history to lose. The replay side reasons the same way. + const pageMayExitSubscription = lifeCycle.subscribe(LifeCycleEventType.PAGE_MAY_EXIT, () => settleBuffer(false)) + const sessionExpireSubscription = lifeCycle.subscribe(LifeCycleEventType.SESSION_EXPIRED, () => settleBuffer(true)) function hold(event: RumEvent & Context) { if (event.type === RumEventType.VIEW) { @@ -231,9 +236,15 @@ export function startWithheldEventBuffer( const releasable = details.filter((held) => views.has(held.viewId)) const detailSampledFrom = releasable.length > 0 ? releasable[0].event.date : undefined + if (detailSampledFrom !== undefined) { + // Recorded on the session so that every view update from here on carries it - the batch + // upserts views by id, so the next ordinary update would otherwise replace these ones before + // the batch is ever sent. These were assembled too early to pick it up, so they are given the + // same value directly, which is what the backend sees if the page goes before the next update. + sessionManager.setSessionDetailSampledFrom(detailSampledFrom) + } + views.forEach((view) => { - // `sampled_for_error` is stamped at assembly for every view of the session; only the point the - // detail actually reaches back to is known here. if (detailSampledFrom !== undefined) { view.session.detail_sampled_from = detailSampledFrom } diff --git a/packages/rum-core/test/mockRumSessionManager.ts b/packages/rum-core/test/mockRumSessionManager.ts index 4a1ebfe986..b32c15c1f7 100644 --- a/packages/rum-core/test/mockRumSessionManager.ts +++ b/packages/rum-core/test/mockRumSessionManager.ts @@ -16,6 +16,7 @@ export interface RumSessionManagerMock extends RumSessionManager { setTrackedOnError(): RumSessionManagerMock setForcedReplay(): RumSessionManagerMock setSessionHasError(): RumSessionManagerMock + setSessionDetailSampledFrom(timestamp: number): RumSessionManagerMock } const DEFAULT_ID = 'session-id' @@ -40,6 +41,7 @@ export function createRumSessionManagerMock(): RumSessionManagerMock { let sessionStatus: SessionStatus = SessionStatus.TRACKED_WITH_SESSION_REPLAY let forcedReplay: boolean = false let hasError: boolean = false + let detailSampledFrom: number | undefined return { findTrackedSession() { const trackingType = TRACKING_TYPES[sessionStatus] @@ -52,6 +54,7 @@ export function createRumSessionManagerMock(): RumSessionManagerMock { sessionReplay: computeSessionReplayState(trackingType, hasError, forcedReplay), eventsWithheld: computeEventsWithheld(trackingType, hasError, forcedReplay), sampledOnError: withholdsEvents(trackingType), + detailSampledFrom, anonymousId: 'device-123', } }, @@ -92,5 +95,9 @@ export function createRumSessionManagerMock(): RumSessionManagerMock { hasError = true return this }, + setSessionDetailSampledFrom(timestamp) { + detailSampledFrom = timestamp + return this + }, } } From 734001f170836567111a326752350c6a5d87c525 Mon Sep 17 00:00:00 2001 From: Fiona Date: Thu, 20 Aug 2026 20:47:45 -0700 Subject: [PATCH 08/27] docs(rum): record why error tracking subscribes before the batch --- packages/rum-core/src/boot/startRum.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/rum-core/src/boot/startRum.ts b/packages/rum-core/src/boot/startRum.ts index 322bcb2d52..b1872e08a2 100644 --- a/packages/rum-core/src/boot/startRum.ts +++ b/packages/rum-core/src/boot/startRum.ts @@ -111,6 +111,9 @@ export function startRum( ? startRumSessionManager(configuration, lifeCycle, trackingConsentState) : startRumSessionManagerStub() + // Subscribed before the batch below, and it has to stay that way: the withheld event buffer runs + // on the same event, and only sees a session as released if this has already marked it. Reorder + // them and the release waits for whatever event happens to come next. const sessionErrorTracking = startSessionErrorTracking(lifeCycle, session) cleanupTasks.push(() => sessionErrorTracking.stop()) From d0309f43f485fcd6572d3e1117a100d7fca4609f Mon Sep 17 00:00:00 2001 From: Fiona Date: Tue, 1 Sep 2026 08:52:53 -0700 Subject: [PATCH 09/27] fix(rum): decide what to withhold by the event's own session, not the 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. --- .../src/transport/withheldEventBuffer.spec.ts | 47 +++++++++++++++++++ .../src/transport/withheldEventBuffer.ts | 44 ++++++++++++++--- 2 files changed, 84 insertions(+), 7 deletions(-) diff --git a/packages/rum-core/src/transport/withheldEventBuffer.spec.ts b/packages/rum-core/src/transport/withheldEventBuffer.spec.ts index 51ddc90a58..aad4a10339 100644 --- a/packages/rum-core/src/transport/withheldEventBuffer.spec.ts +++ b/packages/rum-core/src/transport/withheldEventBuffer.spec.ts @@ -275,6 +275,53 @@ describe('startWithheldEventBuffer', () => { expect(releasedViewIds).not.toContain('old-view') expect(releasedViewIds).toContain('current-view') }) + + it('drops a straggler of a session whose buffer was already thrown away', () => { + collect(RumEventType.VIEW, { session: { id: 'session-id' } }) + collect(RumEventType.RESOURCE, { session: { id: 'session-id' } }) + + lifeCycle.notify(LifeCycleEventType.SESSION_EXPIRED) + sessionManager.setNotTracked() + + // a request that started before the session ended completes after it, still carrying its id - + // uploading it would store the very session the withholding was there to avoid + collect(RumEventType.RESOURCE, { session: { id: 'session-id' } }) + + expect(releasedAfterJitter().length).toBe(0) + }) + + it('does not let a straggler of the previous session ride the new one buffer', () => { + sessionManager.setId('session-1') + collect(RumEventType.VIEW, { session: { id: 'session-1' } }) + collect(RumEventType.RESOURCE, { session: { id: 'session-1' } }) + + lifeCycle.notify(LifeCycleEventType.SESSION_EXPIRED) + sessionManager.setId('session-2') + + collect(RumEventType.RESOURCE, { session: { id: 'session-1' } }) + collect(RumEventType.VIEW, { session: { id: 'session-2' } }) + + sessionManager.setSessionHasError() + collect(RumEventType.ERROR, { session: { id: 'session-2' } }) + + const releasedSessionIds = releasedAfterJitter().map((event) => (event.session as Context).id) + expect(releasedSessionIds).toEqual(['session-2', 'session-2']) + }) + + it('keeps the view an error hangs from when a view that already ended is updated late', () => { + collect(RumEventType.VIEW, { date: 1000, view: { id: 'first-view' } }) + collect(RumEventType.VIEW, { date: 2000, view: { id: 'second-view' } }) + // nothing happens in the second view for longer than the window + clock.tick(WITHHELD_BUFFER_DURATION + ONE_SECOND) + // a late update of the view that already ended: it carries that view's start date, so it must + // not become current again - otherwise the view the error hangs from is the one pruned away + collect(RumEventType.VIEW, { date: 1000, view: { id: 'first-view' } }) + + sessionManager.setSessionHasError() + collect(RumEventType.ERROR, { view: { id: 'second-view' } }) + + expect(releasedAfterJitter().map((event) => event.type)).toContain(RumEventType.ERROR) + }) }) describe('computeReleaseDelay', () => { diff --git a/packages/rum-core/src/transport/withheldEventBuffer.ts b/packages/rum-core/src/transport/withheldEventBuffer.ts index d06e314ca0..543f991f58 100644 --- a/packages/rum-core/src/transport/withheldEventBuffer.ts +++ b/packages/rum-core/src/transport/withheldEventBuffer.ts @@ -70,25 +70,40 @@ export function startWithheldEventBuffer( let details: WithheldEvent[] = [] let bytes = 0 let currentViewId: string | undefined + let currentViewDate = -Infinity let withheldForSessionId: string | undefined + /** The last session whose buffer was thrown away, so its stragglers are thrown away too. */ + let discardedSessionId: string | undefined let releaseTimeoutId: TimeoutId | undefined let droppedCount = 0 const eventSubscription = lifeCycle.subscribe(LifeCycleEventType.RUM_EVENT_COLLECTED, (event) => { const session = sessionManager.findTrackedSession() + // Which session an event belongs to is what the event says, not whichever session is current: + // assembly resolves the 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. An event that does not + // say is treated as the current one's, which is how it was handled before there was a buffer. + const eventSessionId = event.session?.id + const isFrom = (sessionId: string | undefined) => eventSessionId === undefined || eventSessionId === sessionId + + if (eventSessionId !== undefined && eventSessionId === discardedSessionId) { + // Its session ended without ever reporting an error and everything held for it was thrown + // away. Letting a straggler through would store the very session the withholding avoided. + return + } - if (session?.eventsWithheld) { + if (session?.eventsWithheld && isFrom(session.id)) { if (withheldForSessionId !== undefined && withheldForSessionId !== session.id) { // A renewed session is a different session: it draws its own sampling and starts without an // error, so what the previous one collected must not ride along. - clearBuffer() + discardBuffer() } withheldForSessionId = session.id hold(event) return } - if (withheldForSessionId !== undefined) { + if (withheldForSessionId !== undefined && isFrom(withheldForSessionId)) { if (session && session.id === withheldForSessionId) { // The session just reported its error. This event - typically the error itself - joins what // is held so that the whole history leaves in order, and behind the same jitter. @@ -96,8 +111,10 @@ export function startWithheldEventBuffer( scheduleRelease() return } - // The session that was withholding is gone without ever reporting an error. - clearBuffer() + // The session that was withholding is gone without ever reporting an error, and this event is + // one of its own, so it goes the same way as everything held for it. + discardBuffer() + return } forward(event) @@ -125,7 +142,7 @@ export function startWithheldEventBuffer( if (releaseTimeoutId !== undefined || hasSinceErrored) { release() } else if (discardIfUnreleased) { - clearBuffer() + discardBuffer() } } @@ -143,7 +160,13 @@ export function startWithheldEventBuffer( // the first view seen rather than the least recently updated one. views.delete(event.view.id) views.set(event.view.id, event) - currentViewId = event.view.id + // A view event carries its view's start date, so a late update of a view that already ended + // does not make it current again. Letting it would have `prune` drop the view the next error + // hangs from, and the release would then filter that error out of its own buffer. + if (event.date >= currentViewDate) { + currentViewDate = event.date + currentViewId = event.view.id + } while (views.size > WITHHELD_BUFFER_VIEWS_LIMIT) { views.delete(views.keys().next().value!) } @@ -262,6 +285,12 @@ export function startWithheldEventBuffer( clearBuffer() } + /** Throws the buffer away, and remembers whose it was so its stragglers go the same way. */ + function discardBuffer() { + discardedSessionId = withheldForSessionId + clearBuffer() + } + /** Empties the buffer, whether it was just released or is being thrown away. */ function clearBuffer() { clearTimeout(releaseTimeoutId) @@ -271,6 +300,7 @@ export function startWithheldEventBuffer( bytes = 0 droppedCount = 0 currentViewId = undefined + currentViewDate = -Infinity withheldForSessionId = undefined } From 1a52a703261cd9d316c9b5f1869329adc1bde800 Mon Sep 17 00:00:00 2001 From: Fiona Date: Tue, 1 Sep 2026 08:59:03 -0700 Subject: [PATCH 10/27] fix(rum): drop a withheld buffer as soon as its own session is gone 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. --- .../src/domain/trackSessionError.spec.ts | 8 +++++ .../rum-core/src/domain/trackSessionError.ts | 2 +- .../src/transport/withheldEventBuffer.spec.ts | 14 +++++++-- .../src/transport/withheldEventBuffer.ts | 30 +++++++++---------- 4 files changed, 36 insertions(+), 18 deletions(-) diff --git a/packages/rum-core/src/domain/trackSessionError.spec.ts b/packages/rum-core/src/domain/trackSessionError.spec.ts index 05b254496c..84b297ac8e 100644 --- a/packages/rum-core/src/domain/trackSessionError.spec.ts +++ b/packages/rum-core/src/domain/trackSessionError.spec.ts @@ -37,6 +37,14 @@ describe('startSessionErrorTracking', () => { expect(setSessionHasErrorSpy).not.toHaveBeenCalled() }) + it('marks a session that withholds only its events, which has no replay to release', () => { + sessionManager.setTrackedOnError() + + collect('error') + + expect(setSessionHasErrorSpy).toHaveBeenCalledTimes(1) + }) + it('leaves an untracked session alone', () => { sessionManager.setNotTracked() diff --git a/packages/rum-core/src/domain/trackSessionError.ts b/packages/rum-core/src/domain/trackSessionError.ts index 8946faf2af..3eeb389a8c 100644 --- a/packages/rum-core/src/domain/trackSessionError.ts +++ b/packages/rum-core/src/domain/trackSessionError.ts @@ -30,7 +30,7 @@ export function startSessionErrorTracking(lifeCycle: LifeCycle, sessionManager: // write also pushes the session's expiry out (`processSessionStoreOperations` expands every // state it persists), which would move where their sessions end. const session = sessionManager.findTrackedSession() - if (!session?.sampledOnErrorReplay) { + if (!session || (!session.sampledOnError && !session.sampledOnErrorReplay)) { return } hasReportedError = true diff --git a/packages/rum-core/src/transport/withheldEventBuffer.spec.ts b/packages/rum-core/src/transport/withheldEventBuffer.spec.ts index aad4a10339..69378570e9 100644 --- a/packages/rum-core/src/transport/withheldEventBuffer.spec.ts +++ b/packages/rum-core/src/transport/withheldEventBuffer.spec.ts @@ -124,14 +124,24 @@ describe('startWithheldEventBuffer', () => { expect(types).toContain(RumEventType.ACTION) }) - it('drops the buffer when the session expires without ever reporting an error', () => { + it('drops the buffer, and what is still arriving for it, when the session ends without an error', () => { collect(RumEventType.VIEW) collect(RumEventType.RESOURCE) sessionManager.setNotTracked() collect(RumEventType.RESOURCE) - expect(releasedAfterJitter().filter((event) => event.type === RumEventType.RESOURCE).length).toBe(1) + expect(releasedAfterJitter().length).toBe(0) + }) + + it('forwards the events of a new session that withholds nothing', () => { + sessionManager.setId('session-1') + collect(RumEventType.VIEW, { session: { id: 'session-1' } }) + + sessionManager.setId('session-2').setTrackedWithSessionReplay() + collect(RumEventType.RESOURCE, { session: { id: 'session-2' } }) + + expect(releasedAfterJitter().map((event) => (event.session as Context).id)).toEqual(['session-2']) }) it('releases on page exit when the session errored without the buffer having noticed yet', () => { diff --git a/packages/rum-core/src/transport/withheldEventBuffer.ts b/packages/rum-core/src/transport/withheldEventBuffer.ts index 543f991f58..f4aaeb736d 100644 --- a/packages/rum-core/src/transport/withheldEventBuffer.ts +++ b/packages/rum-core/src/transport/withheldEventBuffer.ts @@ -92,28 +92,28 @@ export function startWithheldEventBuffer( return } - if (session?.eventsWithheld && isFrom(session.id)) { - if (withheldForSessionId !== undefined && withheldForSessionId !== session.id) { - // A renewed session is a different session: it draws its own sampling and starts without an - // error, so what the previous one collected must not ride along. - discardBuffer() + if (withheldForSessionId !== undefined && session?.id !== withheldForSessionId) { + // The session that was withholding is gone - expired, or renewed into another one - without + // ever reporting an error, so what it collected never earned its way out. A session that did + // report one keeps its id and is left alone here. + const wasWithheldFor = withheldForSessionId + discardBuffer() + if (isFrom(wasWithheldFor)) { + return } + } + + if (session?.eventsWithheld && isFrom(session.id)) { withheldForSessionId = session.id hold(event) return } if (withheldForSessionId !== undefined && isFrom(withheldForSessionId)) { - if (session && session.id === withheldForSessionId) { - // The session just reported its error. This event - typically the error itself - joins what - // is held so that the whole history leaves in order, and behind the same jitter. - hold(event) - scheduleRelease() - return - } - // The session that was withholding is gone without ever reporting an error, and this event is - // one of its own, so it goes the same way as everything held for it. - discardBuffer() + // The session just reported its error. This event - typically the error itself - joins what is + // held so that the whole history leaves in order, and behind the same jitter. + hold(event) + scheduleRelease() return } From 2d54f00cb9b2301e1fa6042b488f82eb5d989349 Mon Sep 17 00:00:00 2001 From: Fiona Date: Tue, 1 Sep 2026 09:21:13 -0700 Subject: [PATCH 11/27] fix(rum): hold the released window at the error, and remember more than 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. --- packages/rum-core/src/boot/startRum.ts | 5 +++ .../src/domain/rumSessionManager.spec.ts | 22 +++++++++++ .../rum-core/src/domain/rumSessionManager.ts | 23 ++++++++--- .../src/transport/withheldEventBuffer.spec.ts | 39 ++++++++++++++++++- .../src/transport/withheldEventBuffer.ts | 32 ++++++++++++--- 5 files changed, 109 insertions(+), 12 deletions(-) diff --git a/packages/rum-core/src/boot/startRum.ts b/packages/rum-core/src/boot/startRum.ts index b1872e08a2..d2469a72d2 100644 --- a/packages/rum-core/src/boot/startRum.ts +++ b/packages/rum-core/src/boot/startRum.ts @@ -102,6 +102,11 @@ export function startRum( } const pageMayExitObservable = createPageMayExitObservable(configuration) + // Subscribed before the batch below, and it has to stay that way. The batch flushes on this same + // observable, and observers run in the order they subscribed - so the withheld event buffer, which + // releases on the lifecycle notification raised here, has to get its events into the batch before + // the flush that is the page's last chance to send them. The same holds for the session expiry + // relay in `startRumSessionManager`, which the session manager registers just below. const pageMayExitSubscription = pageMayExitObservable.subscribe((event) => { lifeCycle.notify(LifeCycleEventType.PAGE_MAY_EXIT, event) }) diff --git a/packages/rum-core/src/domain/rumSessionManager.spec.ts b/packages/rum-core/src/domain/rumSessionManager.spec.ts index 79173b3d54..4c0cd5e8c0 100644 --- a/packages/rum-core/src/domain/rumSessionManager.spec.ts +++ b/packages/rum-core/src/domain/rumSessionManager.spec.ts @@ -395,6 +395,28 @@ describe('rum session manager', () => { expect(sessionManager.findTrackedSession()!.sampledOnError).toBeTrue() }) + + it('keeps the earliest point any tab reached as where the stored detail starts', () => { + const sessionManager = startRumSessionManagerWithDefaults({ configuration: ON_ERROR_ONLY }) + const sessionId = sessionManager.findTrackedSession()!.id + + // two tabs of the same session release their own buffers, each reaching back a different way + sessionManager.setSessionDetailSampledFrom(2000, sessionId) + sessionManager.setSessionDetailSampledFrom(1000, sessionId) + sessionManager.setSessionDetailSampledFrom(3000, sessionId) + + expect(getSessionState(SESSION_STORE_KEY).detailFrom).toBe('1000') + expect(sessionManager.findTrackedSession()!.detailSampledFrom).toBe(1000) + }) + + it('does not record where the detail starts on a session that has since been replaced', () => { + const sessionManager = startRumSessionManagerWithDefaults({ configuration: ON_ERROR_ONLY }) + + setCookie(SESSION_STORE_KEY, 'id=other-session&rum=4', DURATION) + sessionManager.setSessionDetailSampledFrom(1000, 'a-session-that-is-gone') + + expect(getSessionState(SESSION_STORE_KEY).detailFrom).toBeUndefined() + }) }) function startRumSessionManagerWithDefaults({ configuration }: { configuration?: Partial } = {}) { diff --git a/packages/rum-core/src/domain/rumSessionManager.ts b/packages/rum-core/src/domain/rumSessionManager.ts index aa807c3f78..0f49604da5 100644 --- a/packages/rum-core/src/domain/rumSessionManager.ts +++ b/packages/rum-core/src/domain/rumSessionManager.ts @@ -30,8 +30,11 @@ export interface RumSessionManager { * because the store write can be deferred by the lock, and it must not land on a later session. */ setSessionHasError: (sessionId: string) => void - /** Records how far back the detail released for this session actually reaches. */ - setSessionDetailSampledFrom: (timestamp: number) => void + /** + * Records how far back the detail released for this session actually reaches. The earliest point + * any tab reached wins, since that is where the session's stored detail really starts. + */ + setSessionDetailSampledFrom: (timestamp: number, sessionId: string) => void } export type RumSession = { @@ -115,7 +118,9 @@ export function startRumSessionManager( sessionEntity.hasError = true } } - if (!previousState.detailFrom && newState.detailFrom) { + // Followed rather than latched on the first value seen: the store keeps the earliest point any + // tab reached, so a later, earlier write is a correction and not a second opinion. + if (previousState.detailFrom !== newState.detailFrom) { const sessionEntity = sessionManager.findSession() if (sessionEntity) { sessionEntity.detailSampledFrom = Number(newState.detailFrom) || undefined @@ -155,8 +160,16 @@ export function startRumSessionManager( // Kept on the session rather than stamped on the released view events: the batch upserts views // by id, so the next ordinary view update - which arrives within seconds - would replace the // stamped one before the batch is ever sent. - setSessionDetailSampledFrom: (timestamp) => - sessionManager.updateSessionState(() => ({ detailFrom: String(timestamp) })), + setSessionDetailSampledFrom: (timestamp, sessionId) => + sessionManager.updateSessionState((state) => { + if (state.id !== sessionId) { + return undefined + } + // Both tabs of a session release their own buffer on the same error, and the session's + // detail starts wherever the earliest of them reached. + const stored = Number(state.detailFrom) + return stored && stored <= timestamp ? undefined : { detailFrom: String(timestamp) } + }), } } diff --git a/packages/rum-core/src/transport/withheldEventBuffer.spec.ts b/packages/rum-core/src/transport/withheldEventBuffer.spec.ts index 69378570e9..7eeb7a8fbb 100644 --- a/packages/rum-core/src/transport/withheldEventBuffer.spec.ts +++ b/packages/rum-core/src/transport/withheldEventBuffer.spec.ts @@ -182,7 +182,7 @@ describe('startWithheldEventBuffer', () => { // kept on the session, because the batch upserts views by id and the next ordinary view update // would otherwise replace the stamped one before anything is sent - expect(spy).toHaveBeenCalledWith(4321) + expect(spy).toHaveBeenCalledWith(4321, 'session-id') }) it('drops the buffer when the session ends without ever having errored', () => { @@ -318,6 +318,43 @@ describe('startWithheldEventBuffer', () => { expect(releasedSessionIds).toEqual(['session-2', 'session-2']) }) + it('keeps the minute before the error when the release timer is held back', () => { + collect(RumEventType.VIEW) + collect(RumEventType.RESOURCE, { date: 111 }) + + sessionManager.setSessionHasError() + collect(RumEventType.ERROR) + + // a backgrounded tab clamps timers to about once a minute, so the release runs long after it + // was scheduled - the window it releases has to be the one around the error, not around now + clock.setDate(new Date(Date.now() + WITHHELD_BUFFER_DURATION + ONE_SECOND)) + clock.tick(WITHHELD_BUFFER_RELEASE_MAX_DELAY) + + expect(forwarded.map((event) => event.date)).toContain(111) + }) + + it('still drops a straggler of a session discarded several renewals ago', () => { + sessionManager.setId('session-1') + collect(RumEventType.VIEW, { session: { id: 'session-1' } }) + lifeCycle.notify(LifeCycleEventType.SESSION_EXPIRED) + + sessionManager.setId('session-2') + collect(RumEventType.VIEW, { session: { id: 'session-2' } }) + lifeCycle.notify(LifeCycleEventType.SESSION_EXPIRED) + + sessionManager.setId('session-3') + collect(RumEventType.VIEW, { session: { id: 'session-3' } }) + + // a request that outlived two withheld sessions finally completes + collect(RumEventType.RESOURCE, { session: { id: 'session-1' } }) + + sessionManager.setSessionHasError() + collect(RumEventType.ERROR, { session: { id: 'session-3' } }) + + const releasedSessionIds = releasedAfterJitter().map((event) => (event.session as Context).id) + expect(releasedSessionIds).toEqual(['session-3', 'session-3']) + }) + it('keeps the view an error hangs from when a view that already ended is updated late', () => { collect(RumEventType.VIEW, { date: 1000, view: { id: 'first-view' } }) collect(RumEventType.VIEW, { date: 2000, view: { id: 'second-view' } }) diff --git a/packages/rum-core/src/transport/withheldEventBuffer.ts b/packages/rum-core/src/transport/withheldEventBuffer.ts index f4aaeb736d..6be2979fdf 100644 --- a/packages/rum-core/src/transport/withheldEventBuffer.ts +++ b/packages/rum-core/src/transport/withheldEventBuffer.ts @@ -38,6 +38,13 @@ const WITHHELD_BUFFER_VIEWS_LIMIT = 50 */ export const WITHHELD_BUFFER_RELEASE_MAX_DELAY = 3 * ONE_SECOND +/** + * How many thrown-away sessions to remember, so their stragglers are thrown away too. A late event + * can only still be assembled for a session while the session context history holds it, which is far + * shorter than the life of one session - a handful covers every straggler that can still arrive. + */ +const DISCARDED_SESSIONS_REMEMBERED = 4 + /** What gets dropped first when the buffer is over budget. Lower goes first. */ const enum EvictionTier { /** Long tasks, and requests that succeeded without complaint. */ @@ -72,9 +79,11 @@ export function startWithheldEventBuffer( let currentViewId: string | undefined let currentViewDate = -Infinity let withheldForSessionId: string | undefined - /** The last session whose buffer was thrown away, so its stragglers are thrown away too. */ - let discardedSessionId: string | undefined + /** The sessions whose buffers were thrown away, so their stragglers are thrown away too. */ + const discardedSessionIds: string[] = [] let releaseTimeoutId: TimeoutId | undefined + /** When the release was scheduled, which is what freezes the window - see {@link prune}. */ + let releaseScheduledAt: RelativeTime | undefined let droppedCount = 0 const eventSubscription = lifeCycle.subscribe(LifeCycleEventType.RUM_EVENT_COLLECTED, (event) => { @@ -86,7 +95,7 @@ export function startWithheldEventBuffer( const eventSessionId = event.session?.id const isFrom = (sessionId: string | undefined) => eventSessionId === undefined || eventSessionId === sessionId - if (eventSessionId !== undefined && eventSessionId === discardedSessionId) { + if (eventSessionId !== undefined && discardedSessionIds.indexOf(eventSessionId) !== -1) { // Its session ended without ever reporting an error and everything held for it was thrown // away. Letting a straggler through would store the very session the withholding avoided. return @@ -194,7 +203,11 @@ export function startWithheldEventBuffer( /** Drops what has aged out of the window, so the span kept is the one we promise. */ function prune() { - const oldestAllowed = (relativeNow() - WITHHELD_BUFFER_DURATION) as RelativeTime + // Once a release is scheduled the window stops moving. The timer carrying that release is + // clamped to about once a minute in a background tab, and pruning against a later `now` would + // throw away exactly the minute before the error that the release exists to deliver. + const now = releaseScheduledAt ?? relativeNow() + const oldestAllowed = (now - WITHHELD_BUFFER_DURATION) as RelativeTime let cutoff = 0 while (cutoff < details.length && details[cutoff].time < oldestAllowed) { bytes -= details[cutoff].bytes @@ -249,6 +262,7 @@ export function startWithheldEventBuffer( if (releaseTimeoutId !== undefined) { return } + releaseScheduledAt = relativeNow() releaseTimeoutId = setTimeout(release, computeReleaseDelay(withheldForSessionId!)) } @@ -264,7 +278,7 @@ export function startWithheldEventBuffer( // upserts views by id, so the next ordinary update would otherwise replace these ones before // the batch is ever sent. These were assembled too early to pick it up, so they are given the // same value directly, which is what the backend sees if the page goes before the next update. - sessionManager.setSessionDetailSampledFrom(detailSampledFrom) + sessionManager.setSessionDetailSampledFrom(detailSampledFrom, withheldForSessionId!) } views.forEach((view) => { @@ -287,7 +301,12 @@ export function startWithheldEventBuffer( /** Throws the buffer away, and remembers whose it was so its stragglers go the same way. */ function discardBuffer() { - discardedSessionId = withheldForSessionId + if (withheldForSessionId !== undefined) { + discardedSessionIds.push(withheldForSessionId) + if (discardedSessionIds.length > DISCARDED_SESSIONS_REMEMBERED) { + discardedSessionIds.shift() + } + } clearBuffer() } @@ -295,6 +314,7 @@ export function startWithheldEventBuffer( function clearBuffer() { clearTimeout(releaseTimeoutId) releaseTimeoutId = undefined + releaseScheduledAt = undefined views = new Map() details = [] bytes = 0 From 248f275b0fd5a89059eea1bb10e680e2d51610d3 Mon Sep 17 00:00:00 2001 From: Fiona Date: Tue, 1 Sep 2026 09:23:47 -0700 Subject: [PATCH 12/27] test(rum): follow the session id through the session manager mock --- packages/rum-core/test/mockRumSessionManager.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/rum-core/test/mockRumSessionManager.ts b/packages/rum-core/test/mockRumSessionManager.ts index 0ce90f5175..6bf45734ac 100644 --- a/packages/rum-core/test/mockRumSessionManager.ts +++ b/packages/rum-core/test/mockRumSessionManager.ts @@ -17,7 +17,7 @@ export interface RumSessionManagerMock extends RumSessionManager { setTrackedOnError(): RumSessionManagerMock setForcedReplay(): RumSessionManagerMock setSessionHasError(): RumSessionManagerMock - setSessionDetailSampledFrom(timestamp: number): RumSessionManagerMock + setSessionDetailSampledFrom(timestamp: number, sessionId: string): RumSessionManagerMock } const DEFAULT_ID = 'session-id' From 1f0129614d373e5252be9cb0ee069cf16da3d370 Mon Sep 17 00:00:00 2001 From: Fiona Date: Tue, 1 Sep 2026 09:35:21 -0700 Subject: [PATCH 13/27] fix(rum): release withheld events only when their own session earned it 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. --- .../src/transport/withheldEventBuffer.spec.ts | 12 ++++++++++++ .../rum-core/src/transport/withheldEventBuffer.ts | 13 ++++++++----- 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/packages/rum-core/src/transport/withheldEventBuffer.spec.ts b/packages/rum-core/src/transport/withheldEventBuffer.spec.ts index 7eeb7a8fbb..4c4183e325 100644 --- a/packages/rum-core/src/transport/withheldEventBuffer.spec.ts +++ b/packages/rum-core/src/transport/withheldEventBuffer.spec.ts @@ -355,6 +355,18 @@ describe('startWithheldEventBuffer', () => { expect(releasedSessionIds).toEqual(['session-3', 'session-3']) }) + it('drops the buffer when the session stops withholding without having errored', () => { + collect(RumEventType.VIEW) + collect(RumEventType.RESOURCE) + + // an older SDK sharing the same session store does not know this tracking type and redraws it: + // the session stops withholding, but it never reported an error + sessionManager.setTrackedWithoutSessionReplay() + collect(RumEventType.RESOURCE) + + expect(releasedAfterJitter().length).toBe(0) + }) + it('keeps the view an error hangs from when a view that already ended is updated late', () => { collect(RumEventType.VIEW, { date: 1000, view: { id: 'first-view' } }) collect(RumEventType.VIEW, { date: 2000, view: { id: 'second-view' } }) diff --git a/packages/rum-core/src/transport/withheldEventBuffer.ts b/packages/rum-core/src/transport/withheldEventBuffer.ts index 6be2979fdf..55def2e2e6 100644 --- a/packages/rum-core/src/transport/withheldEventBuffer.ts +++ b/packages/rum-core/src/transport/withheldEventBuffer.ts @@ -101,10 +101,12 @@ export function startWithheldEventBuffer( return } - if (withheldForSessionId !== undefined && session?.id !== withheldForSessionId) { - // The session that was withholding is gone - expired, or renewed into another one - without - // ever reporting an error, so what it collected never earned its way out. A session that did - // report one keeps its id and is left alone here. + if (withheldForSessionId !== undefined && !(session?.id === withheldForSessionId && session.sampledOnError)) { + // The session that was withholding is gone without ever reporting an error, so what it + // collected never earned its way out. Gone covers more than expiry and renewal: the session + // store is shared with every other SDK bundle on the domain, and one that predates these + // tracking types does not recognise them, so it redraws the session and rewrites the type. + // A session that did report an error keeps both its id and its type, and is left alone here. const wasWithheldFor = withheldForSessionId discardBuffer() if (isFrom(wasWithheldFor)) { @@ -119,7 +121,8 @@ export function startWithheldEventBuffer( } if (withheldForSessionId !== undefined && isFrom(withheldForSessionId)) { - // The session just reported its error. This event - typically the error itself - joins what is + // Whatever is still withheld here belongs to a session that has just reported its error: the + // guard above ended every other case. This event, typically the error itself, joins what is // held so that the whole history leaves in order, and behind the same jitter. hold(event) scheduleRelease() From c56015c0cb02db46aa1744347f3137eb5254f854 Mon Sep 17 00:00:00 2001 From: Fiona Date: Tue, 1 Sep 2026 09:55:27 -0700 Subject: [PATCH 14/27] fix(rum): release a buffer as a session that reads back the way it happened 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. --- .../src/transport/withheldEventBuffer.spec.ts | 47 ++++++++++++++++++- .../src/transport/withheldEventBuffer.ts | 28 ++++++++++- 2 files changed, 71 insertions(+), 4 deletions(-) diff --git a/packages/rum-core/src/transport/withheldEventBuffer.spec.ts b/packages/rum-core/src/transport/withheldEventBuffer.spec.ts index 4c4183e325..29f68b7327 100644 --- a/packages/rum-core/src/transport/withheldEventBuffer.spec.ts +++ b/packages/rum-core/src/transport/withheldEventBuffer.spec.ts @@ -91,7 +91,7 @@ describe('startWithheldEventBuffer', () => { collect(RumEventType.RESOURCE, { date: 4321 }) sessionManager.setSessionHasError() - collect(RumEventType.ERROR) + collect(RumEventType.ERROR, { date: 9999 }) const view = releasedAfterJitter().find((event) => event.type === RumEventType.VIEW)! expect((view.session as Context).detail_sampled_from).toBe(4321) @@ -177,7 +177,7 @@ describe('startWithheldEventBuffer', () => { collect(RumEventType.RESOURCE, { date: 4321 }) sessionManager.setSessionHasError() - collect(RumEventType.ERROR) + collect(RumEventType.ERROR, { date: 9999 }) releasedAfterJitter() // kept on the session, because the batch upserts views by id and the next ordinary view update @@ -367,6 +367,49 @@ describe('startWithheldEventBuffer', () => { expect(releasedAfterJitter().length).toBe(0) }) + it('releases the views oldest first, since a session is built out of the first one to arrive', () => { + collect(RumEventType.VIEW, { date: 1000, view: { id: 'view-1' } }) + collect(RumEventType.RESOURCE, { view: { id: 'view-1' } }) + collect(RumEventType.VIEW, { date: 2000, view: { id: 'view-2' } }) + collect(RumEventType.RESOURCE, { view: { id: 'view-2' } }) + collect(RumEventType.VIEW, { date: 3000, view: { id: 'view-3' } }) + collect(RumEventType.RESOURCE, { view: { id: 'view-3' } }) + // a late update of the first view, which puts the oldest view last in the buffer + collect(RumEventType.VIEW, { date: 1000, view: { id: 'view-1' } }) + + sessionManager.setSessionHasError() + collect(RumEventType.ERROR, { view: { id: 'view-3' } }) + + const releasedViewDates = releasedAfterJitter() + .filter((event) => event.type === RumEventType.VIEW) + .map((event) => event.date) + expect(releasedViewDates).toEqual([1000, 2000, 3000]) + }) + + it('marks the detail as starting at the earliest event, not at the first one held', () => { + collect(RumEventType.VIEW) + // a request that took minutes is only held once it finishes, but it started well before that + collect(RumEventType.RESOURCE, { date: 5000 }) + collect(RumEventType.RESOURCE, { date: 1000 }) + + sessionManager.setSessionHasError() + collect(RumEventType.ERROR, { date: 9000 }) + + const view = releasedAfterJitter().find((event) => event.type === RumEventType.VIEW)! + expect((view.session as Context).detail_sampled_from).toBe(1000) + }) + + it('marks the released views as sampled for replay, since the replay leaves with them', () => { + collect(RumEventType.VIEW) + collect(RumEventType.RESOURCE) + + sessionManager.setSessionHasError() + collect(RumEventType.ERROR) + + const view = releasedAfterJitter().find((event) => event.type === RumEventType.VIEW)! + expect((view.session as Context).sampled_for_replay).toBeTrue() + }) + it('keeps the view an error hangs from when a view that already ended is updated late', () => { collect(RumEventType.VIEW, { date: 1000, view: { id: 'first-view' } }) collect(RumEventType.VIEW, { date: 2000, view: { id: 'second-view' } }) diff --git a/packages/rum-core/src/transport/withheldEventBuffer.ts b/packages/rum-core/src/transport/withheldEventBuffer.ts index 55def2e2e6..cada1f69b0 100644 --- a/packages/rum-core/src/transport/withheldEventBuffer.ts +++ b/packages/rum-core/src/transport/withheldEventBuffer.ts @@ -12,6 +12,7 @@ import { import type { LifeCycle } from '../domain/lifeCycle' import { LifeCycleEventType } from '../domain/lifeCycle' import type { RumSessionManager } from '../domain/rumSessionManager' +import { SessionReplayState } from '../domain/rumSessionManager' import { RumEventType } from '../rawRumEvent.types' import type { RumEvent } from '../rumEvent.types' @@ -274,7 +275,16 @@ export function startWithheldEventBuffer( // A detail whose view is gone has no container to hang from, so it would be unreachable. const releasable = details.filter((held) => views.has(held.viewId)) - const detailSampledFrom = releasable.length > 0 ? releasable[0].event.date : undefined + + // The earliest date among them, not the first one held: an event is dated when it started, and a + // request that took minutes is held only once it finishes - so the first held is not the first + // to have happened, and the marker has to be a point no released detail precedes. + let detailSampledFrom: number | undefined + releasable.forEach((held) => { + if (detailSampledFrom === undefined || held.event.date < detailSampledFrom) { + detailSampledFrom = held.event.date + } + }) if (detailSampledFrom !== undefined) { // Recorded on the session so that every view update from here on carries it - the batch @@ -284,10 +294,24 @@ export function startWithheldEventBuffer( sessionManager.setSessionDetailSampledFrom(detailSampledFrom, withheldForSessionId!) } - views.forEach((view) => { + // Oldest first. A Map holds its entries in the order they were last updated, which for a burst + // released all at once is not the order the views happened - and a session is built out of + // whichever of its views arrives first, so that one has to be the earliest. + const orderedViews: Array = [] + views.forEach((view) => orderedViews.push(view)) + orderedViews.sort((left, right) => left.date - right.date) + + // Assembled while the replay was still withheld, so they carry the state of a session that had + // no replay yet. By the time they leave, the replay they belong to is on its way with them. + const isReplaySampled = sessionManager.findTrackedSession()?.sessionReplay === SessionReplayState.SAMPLED + + orderedViews.forEach((view) => { if (detailSampledFrom !== undefined) { view.session.detail_sampled_from = detailSampledFrom } + if (isReplaySampled) { + view.session.sampled_for_replay = true + } forward(view) }) releasable.forEach((held) => forward(held.event)) From 19b0367dbc1dfd3d5315501679007001f0c70e66 Mon Sep 17 00:00:00 2001 From: Fiona Date: Tue, 1 Sep 2026 10:06:37 -0700 Subject: [PATCH 15/27] fix(rum): let forcing a replay reach a session that is withholding one 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. --- packages/rum/src/boot/postStartStrategy.ts | 20 +++++++++++++++----- packages/rum/src/boot/recorderApi.spec.ts | 21 +++++++++++++++++++++ 2 files changed, 36 insertions(+), 5 deletions(-) diff --git a/packages/rum/src/boot/postStartStrategy.ts b/packages/rum/src/boot/postStartStrategy.ts index c9678c6f21..1f37d4db0e 100644 --- a/packages/rum/src/boot/postStartStrategy.ts +++ b/packages/rum/src/boot/postStartStrategy.ts @@ -87,6 +87,13 @@ export function createPostStartStrategy( return } + if (shouldForceReplay(session!, options)) { + // Applied before the guard below, not after starting: a session that withholds its replay is + // already recording, so the guard would return without ever releasing it - and releasing what + // is held is the whole of what forcing means for such a session. + sessionManager.setForcedReplay() + } + if (isRecordingInProgress(status)) { return } @@ -95,10 +102,6 @@ export function createPostStartStrategy( // Intentionally not awaiting doStart() to keep it asynchronous doStart().catch(monitorError) - - if (shouldForceReplay(session!, options)) { - sessionManager.setForcedReplay() - } } function stop() { @@ -128,5 +131,12 @@ function isRecordingInProgress(status: RecorderStatus) { } function shouldForceReplay(session: RumSession, options?: StartRecordingOptions) { - return options && options.force && session.sessionReplay === SessionReplayState.OFF + return ( + options && + options.force && + // A withheld replay is as much in need of forcing as one that was never sampled: the host asked + // for this user's replay, so it must not go on waiting for an error that may never come. + (session.sessionReplay === SessionReplayState.OFF || + session.sessionReplay === SessionReplayState.BUFFERED_ON_ERROR) + ) } diff --git a/packages/rum/src/boot/recorderApi.spec.ts b/packages/rum/src/boot/recorderApi.spec.ts index b61ae8ea00..3c839095ff 100644 --- a/packages/rum/src/boot/recorderApi.spec.ts +++ b/packages/rum/src/boot/recorderApi.spec.ts @@ -178,6 +178,27 @@ describe('makeRecorderApi', () => { expect(setForcedReplaySpy).toHaveBeenCalledTimes(1) }) + it('releases a withheld replay when forced, although it is already recording', async () => { + const setForcedReplaySpy = jasmine.createSpy() + + setupRecorderApi({ + sessionManager: { + ...createRumSessionManagerMock().setTrackedWithErrorSessionReplay(), + setForcedReplay: setForcedReplaySpy, + }, + startSessionReplayRecordingManually: false, + }) + + rumInit() + await collectAsyncCalls(startRecordingSpy, 1) + + // the recording is already running - what forcing asks for here is that what it holds stops + // waiting for an error + recorderApi.start({ force: true }) + + expect(setForcedReplaySpy).toHaveBeenCalledTimes(1) + }) + it('uses the previously created worker if available', async () => { setupRecorderApi({ startSessionReplayRecordingManually: true }) rumInit({ worker: mockWorker }) From 436f34773a3b2cbd40f0cb43b06624d8a51306cb Mon Sep 17 00:00:00 2001 From: Fiona Date: Tue, 1 Sep 2026 10:06:37 -0700 Subject: [PATCH 16/27] feat(rum): say so when a sampling rate cannot draw a single session 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. --- .../configuration/configuration.spec.ts | 42 +++++++++++++++++++ .../src/domain/configuration/configuration.ts | 26 ++++++++++-- 2 files changed, 65 insertions(+), 3 deletions(-) diff --git a/packages/rum-core/src/domain/configuration/configuration.spec.ts b/packages/rum-core/src/domain/configuration/configuration.spec.ts index a2c4c48875..6c39bb7cfa 100644 --- a/packages/rum-core/src/domain/configuration/configuration.spec.ts +++ b/packages/rum-core/src/domain/configuration/configuration.spec.ts @@ -65,6 +65,48 @@ describe('validateAndBuildRumConfiguration', () => { }) }) + describe('sessionReplayOnErrorSampleRate', () => { + it('warns when the plain replay rate leaves it nothing to draw from', () => { + validateAndBuildRumConfiguration({ + ...DEFAULT_INIT_CONFIGURATION, + sessionReplaySampleRate: 100, + sessionReplayOnErrorSampleRate: 50, + }) + + expect(displayWarnSpy).toHaveBeenCalledTimes(1) + }) + + it('warns when no session is tracked at all', () => { + validateAndBuildRumConfiguration({ + ...DEFAULT_INIT_CONFIGURATION, + sessionSampleRate: 0, + sessionReplayOnErrorSampleRate: 50, + }) + + expect(displayWarnSpy).toHaveBeenCalledTimes(1) + }) + + it('warns when the recording is left for the customer to start, since nothing would be held', () => { + validateAndBuildRumConfiguration({ + ...DEFAULT_INIT_CONFIGURATION, + sessionReplayOnErrorSampleRate: 50, + startSessionReplayRecordingManually: true, + }) + + expect(displayWarnSpy).toHaveBeenCalledTimes(1) + }) + + it('says nothing about a rate that can draw', () => { + validateAndBuildRumConfiguration({ + ...DEFAULT_INIT_CONFIGURATION, + sessionReplaySampleRate: 20, + sessionReplayOnErrorSampleRate: 50, + }) + + expect(displayWarnSpy).not.toHaveBeenCalled() + }) + }) + describe('traceSampleRate', () => { it('defaults to 100 if the option is not provided', () => { expect(validateAndBuildRumConfiguration(DEFAULT_INIT_CONFIGURATION)!.traceSampleRate).toBe(100) diff --git a/packages/rum-core/src/domain/configuration/configuration.ts b/packages/rum-core/src/domain/configuration/configuration.ts index 33743b5d9e..657adabfd2 100644 --- a/packages/rum-core/src/domain/configuration/configuration.ts +++ b/packages/rum-core/src/domain/configuration/configuration.ts @@ -101,9 +101,10 @@ export interface RumInitConfiguration extends InitConfiguration { */ sessionReplaySampleRate?: number | undefined /** - * The percentage of tracked sessions that record a replay but only upload it if the session - * reports an error: 100 for all, 0 for none. Drawn only for sessions that the plain - * `sessionReplaySampleRate` draw missed, so a session is never counted by both rates. + * Of the tracked sessions that `sessionReplaySampleRate` did not draw, the percentage that record + * a replay but only upload it if the session reports an error: 100 for all of them, 0 for none. + * The base is what the plain rate missed, so a session is never counted by both, and the share of + * all tracked sessions this covers is `(100 - sessionReplaySampleRate) * this / 100`. * * Such a session records from the start and keeps at most the last minute of it in memory. If it * never reports an error, nothing is uploaded and the session is not billed. On the first error, @@ -244,6 +245,25 @@ export function validateAndBuildRumConfiguration( const sessionReplaySampleRate = initConfiguration.sessionReplaySampleRate ?? 0 const sessionReplayOnErrorSampleRate = initConfiguration.sessionReplayOnErrorSampleRate ?? 0 + // Each of these is a rate the customer set that cannot draw a single session. They are valid + // numbers, so validation lets them through - but silence would leave them waiting for data that + // is never coming. + if (sessionReplayOnErrorSampleRate > 0) { + if (sessionReplaySampleRate === 100) { + display.warn( + 'sessionReplayOnErrorSampleRate is drawn only for sessions sessionReplaySampleRate did not draw, and that rate is 100: it will never apply.' + ) + } + if ((initConfiguration.sessionSampleRate ?? 100) === 0) { + display.warn('sessionReplayOnErrorSampleRate has no effect while sessionSampleRate is 0: no session is tracked.') + } + if (initConfiguration.startSessionReplayRecordingManually) { + display.warn( + 'sessionReplayOnErrorSampleRate needs the recording to already be running when the error happens, and startSessionReplayRecordingManually keeps it stopped until you start it: there would be nothing to release.' + ) + } + } + return { applicationId: initConfiguration.applicationId, version: initConfiguration.version || undefined, From 7b1c0834d4f46a2974a87c0ef7f0f78e8fcb014e Mon Sep 17 00:00:00 2001 From: Fiona Date: Tue, 1 Sep 2026 10:08:04 -0700 Subject: [PATCH 17/27] feat(rum): say so when sessionOnErrorSampleRate cannot draw a session 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. --- .../configuration/configuration.spec.ts | 32 +++++++++++++++++++ .../src/domain/configuration/configuration.ts | 25 ++++++++++----- 2 files changed, 49 insertions(+), 8 deletions(-) diff --git a/packages/rum-core/src/domain/configuration/configuration.spec.ts b/packages/rum-core/src/domain/configuration/configuration.spec.ts index 850b8691bf..266ec5efc5 100644 --- a/packages/rum-core/src/domain/configuration/configuration.spec.ts +++ b/packages/rum-core/src/domain/configuration/configuration.spec.ts @@ -107,6 +107,38 @@ describe('validateAndBuildRumConfiguration', () => { }) }) + describe('sessionOnErrorSampleRate', () => { + it('warns when the default session rate leaves it nothing to draw from', () => { + validateAndBuildRumConfiguration({ + ...DEFAULT_INIT_CONFIGURATION, + sessionOnErrorSampleRate: 50, + }) + + expect(displayWarnSpy).toHaveBeenCalledTimes(1) + }) + + it('says nothing once the plain session rate leaves room for it', () => { + validateAndBuildRumConfiguration({ + ...DEFAULT_INIT_CONFIGURATION, + sessionSampleRate: 20, + sessionOnErrorSampleRate: 50, + }) + + expect(displayWarnSpy).not.toHaveBeenCalled() + }) + + it('makes a replay-on-error rate meaningful even with no plainly sampled session', () => { + validateAndBuildRumConfiguration({ + ...DEFAULT_INIT_CONFIGURATION, + sessionSampleRate: 0, + sessionOnErrorSampleRate: 100, + sessionReplayOnErrorSampleRate: 50, + }) + + expect(displayWarnSpy).not.toHaveBeenCalled() + }) + }) + describe('traceSampleRate', () => { it('defaults to 100 if the option is not provided', () => { expect(validateAndBuildRumConfiguration(DEFAULT_INIT_CONFIGURATION)!.traceSampleRate).toBe(100) diff --git a/packages/rum-core/src/domain/configuration/configuration.ts b/packages/rum-core/src/domain/configuration/configuration.ts index a49afde5ad..7a31be18f4 100644 --- a/packages/rum-core/src/domain/configuration/configuration.ts +++ b/packages/rum-core/src/domain/configuration/configuration.ts @@ -112,9 +112,11 @@ export interface RumInitConfiguration extends InitConfiguration { */ sessionReplayOnErrorSampleRate?: number | undefined /** - * The percentage of tracked sessions that collect events but only upload them if the session - * reports an error: 100 for all, 0 for none. Drawn only for sessions that the plain - * `sessionSampleRate` draw missed, so a session is never counted by both rates. + * Of the sessions that `sessionSampleRate` did not draw, the percentage that collect events but + * only upload them if the session reports an error: 100 for all of them, 0 for none. The base is + * what the plain rate missed - so with the default `sessionSampleRate` of 100 there is nothing + * left to draw from and this does nothing - and the share of all sessions it covers is + * `(100 - sessionSampleRate) * this / 100`. * * Such a session collects from the start and keeps at most the last minute of it in memory. If it * never reports an error, nothing is uploaded and the session is not stored. On the first error, @@ -261,17 +263,24 @@ export function validateAndBuildRumConfiguration( const sessionReplayOnErrorSampleRate = initConfiguration.sessionReplayOnErrorSampleRate ?? 0 const sessionOnErrorSampleRate = initConfiguration.sessionOnErrorSampleRate ?? 0 - // Each of these is a rate the customer set that cannot draw a single session. They are valid - // numbers, so validation lets them through - but silence would leave them waiting for data that - // is never coming. + // Each of the cases below is a rate the customer set that cannot draw a single session. They are + // valid numbers, so validation lets them through - but silence would leave someone waiting for + // data that is never coming. + if (sessionOnErrorSampleRate > 0 && (initConfiguration.sessionSampleRate ?? 100) === 100) { + display.warn( + 'sessionOnErrorSampleRate is drawn only for sessions sessionSampleRate did not draw, and that rate is 100: it will never apply.' + ) + } if (sessionReplayOnErrorSampleRate > 0) { if (sessionReplaySampleRate === 100) { display.warn( 'sessionReplayOnErrorSampleRate is drawn only for sessions sessionReplaySampleRate did not draw, and that rate is 100: it will never apply.' ) } - if ((initConfiguration.sessionSampleRate ?? 100) === 0) { - display.warn('sessionReplayOnErrorSampleRate has no effect while sessionSampleRate is 0: no session is tracked.') + if ((initConfiguration.sessionSampleRate ?? 100) === 0 && sessionOnErrorSampleRate === 0) { + display.warn( + 'sessionReplayOnErrorSampleRate has no effect while sessionSampleRate is 0 and sessionOnErrorSampleRate is unset: no session is tracked.' + ) } if (initConfiguration.startSessionReplayRecordingManually) { display.warn( From b8089986532de4f27ae36cc8093269690758ab78 Mon Sep 17 00:00:00 2001 From: Fiona Date: Tue, 1 Sep 2026 10:11:24 -0700 Subject: [PATCH 18/27] fix(rum): tell a withheld session's events that their replay is coming 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. --- .../src/domain/contexts/sessionContext.spec.ts | 13 +++++++++++++ .../rum-core/src/domain/contexts/sessionContext.ts | 6 +++++- .../src/transport/withheldEventBuffer.spec.ts | 11 ----------- .../rum-core/src/transport/withheldEventBuffer.ts | 8 -------- 4 files changed, 18 insertions(+), 20 deletions(-) diff --git a/packages/rum-core/src/domain/contexts/sessionContext.spec.ts b/packages/rum-core/src/domain/contexts/sessionContext.spec.ts index 3d72ad5199..c0ae0fe668 100644 --- a/packages/rum-core/src/domain/contexts/sessionContext.spec.ts +++ b/packages/rum-core/src/domain/contexts/sessionContext.spec.ts @@ -140,6 +140,19 @@ describe('session context', () => { expect(eventSampledOutForReplay.session!.sampled_for_replay).toBe(false) }) + it('should set sampled_for_replay on a session whose events are withheld alongside its replay', () => { + // these events only ever leave together with that replay, so reporting the state as it stands + // while they are held would mark the whole released burst as having none + sessionManager.setTrackedOnError() + + const event = hooks.triggerHook(HookNames.Assemble, { + eventType: 'view', + startTime: 0 as RelativeTime, + }) as DefaultRumEventAttributes + + expect(event.session!.sampled_for_replay).toBe(true) + }) + it('should discard the event if no session', () => { sessionManager.setNotTracked() const defaultRumEventAttributes = hooks.triggerHook(HookNames.Assemble, { diff --git a/packages/rum-core/src/domain/contexts/sessionContext.ts b/packages/rum-core/src/domain/contexts/sessionContext.ts index dd9bd5f877..4747efe7d9 100644 --- a/packages/rum-core/src/domain/contexts/sessionContext.ts +++ b/packages/rum-core/src/domain/contexts/sessionContext.ts @@ -36,7 +36,11 @@ export function startSessionContext( // that was never uploaded is worse than not offering one. const replayStats = recorderApi.getReplayStats(view.id) hasReplay = !isReplayWithheld && replayStats && replayStats.segments_count > 0 ? true : undefined - sampledForReplay = session.sessionReplay === SessionReplayState.SAMPLED + // A session that withholds its events withholds its replay alongside them, so if these events + // are ever uploaded that replay is on its way with them. Reporting the state as it stands at + // assembly time would mark the whole released burst as a session that has no replay. + sampledForReplay = + session.sessionReplay === SessionReplayState.SAMPLED || (session.eventsWithheld && isReplayWithheld) // Tells the backend that this session's detail only starts where the buffer reached, so the // gap before it reads as "not collected" rather than as missing data. sampledForError = session.sampledOnError || undefined diff --git a/packages/rum-core/src/transport/withheldEventBuffer.spec.ts b/packages/rum-core/src/transport/withheldEventBuffer.spec.ts index 29f68b7327..98713a1563 100644 --- a/packages/rum-core/src/transport/withheldEventBuffer.spec.ts +++ b/packages/rum-core/src/transport/withheldEventBuffer.spec.ts @@ -399,17 +399,6 @@ describe('startWithheldEventBuffer', () => { expect((view.session as Context).detail_sampled_from).toBe(1000) }) - it('marks the released views as sampled for replay, since the replay leaves with them', () => { - collect(RumEventType.VIEW) - collect(RumEventType.RESOURCE) - - sessionManager.setSessionHasError() - collect(RumEventType.ERROR) - - const view = releasedAfterJitter().find((event) => event.type === RumEventType.VIEW)! - expect((view.session as Context).sampled_for_replay).toBeTrue() - }) - it('keeps the view an error hangs from when a view that already ended is updated late', () => { collect(RumEventType.VIEW, { date: 1000, view: { id: 'first-view' } }) collect(RumEventType.VIEW, { date: 2000, view: { id: 'second-view' } }) diff --git a/packages/rum-core/src/transport/withheldEventBuffer.ts b/packages/rum-core/src/transport/withheldEventBuffer.ts index cada1f69b0..c4e6e58cd4 100644 --- a/packages/rum-core/src/transport/withheldEventBuffer.ts +++ b/packages/rum-core/src/transport/withheldEventBuffer.ts @@ -12,7 +12,6 @@ import { import type { LifeCycle } from '../domain/lifeCycle' import { LifeCycleEventType } from '../domain/lifeCycle' import type { RumSessionManager } from '../domain/rumSessionManager' -import { SessionReplayState } from '../domain/rumSessionManager' import { RumEventType } from '../rawRumEvent.types' import type { RumEvent } from '../rumEvent.types' @@ -301,17 +300,10 @@ export function startWithheldEventBuffer( views.forEach((view) => orderedViews.push(view)) orderedViews.sort((left, right) => left.date - right.date) - // Assembled while the replay was still withheld, so they carry the state of a session that had - // no replay yet. By the time they leave, the replay they belong to is on its way with them. - const isReplaySampled = sessionManager.findTrackedSession()?.sessionReplay === SessionReplayState.SAMPLED - orderedViews.forEach((view) => { if (detailSampledFrom !== undefined) { view.session.detail_sampled_from = detailSampledFrom } - if (isReplaySampled) { - view.session.sampled_for_replay = true - } forward(view) }) releasable.forEach((held) => forward(held.event)) From 53e3c642675272a669c68a6b8f08b17a17fe1989 Mon Sep 17 00:00:00 2001 From: Fiona Date: Tue, 1 Sep 2026 10:13:42 -0700 Subject: [PATCH 19/27] fix(rum): drop a duplicate copy of the sampling warnings left by a merge --- .../src/domain/configuration/configuration.ts | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/packages/rum-core/src/domain/configuration/configuration.ts b/packages/rum-core/src/domain/configuration/configuration.ts index 1a8ed1db25..7a31be18f4 100644 --- a/packages/rum-core/src/domain/configuration/configuration.ts +++ b/packages/rum-core/src/domain/configuration/configuration.ts @@ -289,25 +289,6 @@ export function validateAndBuildRumConfiguration( } } - // Each of these is a rate the customer set that cannot draw a single session. They are valid - // numbers, so validation lets them through - but silence would leave them waiting for data that - // is never coming. - if (sessionReplayOnErrorSampleRate > 0) { - if (sessionReplaySampleRate === 100) { - display.warn( - 'sessionReplayOnErrorSampleRate is drawn only for sessions sessionReplaySampleRate did not draw, and that rate is 100: it will never apply.' - ) - } - if ((initConfiguration.sessionSampleRate ?? 100) === 0) { - display.warn('sessionReplayOnErrorSampleRate has no effect while sessionSampleRate is 0: no session is tracked.') - } - if (initConfiguration.startSessionReplayRecordingManually) { - display.warn( - 'sessionReplayOnErrorSampleRate needs the recording to already be running when the error happens, and startSessionReplayRecordingManually keeps it stopped until you start it: there would be nothing to release.' - ) - } - } - return { applicationId: initConfiguration.applicationId, version: initConfiguration.version || undefined, From aac59e1f3cb7baa5740ab5708ee3a07ea2cc26a4 Mon Sep 17 00:00:00 2001 From: Fiona Date: Tue, 1 Sep 2026 10:29:08 -0700 Subject: [PATCH 20/27] test(rum): hold the on-error session sampling to the promises it makes 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. --- .../configuration/configuration.spec.ts | 21 ++++ .../domain/contexts/sessionContext.spec.ts | 42 ++++++- .../src/domain/rumSessionManager.spec.ts | 35 ++++++ .../src/transport/withheldEventBuffer.spec.ts | 114 ++++++++++++++++++ .../src/transport/withheldEventBuffer.ts | 4 +- .../rum-core/test/mockRumSessionManager.ts | 9 +- 6 files changed, 221 insertions(+), 4 deletions(-) diff --git a/packages/rum-core/src/domain/configuration/configuration.spec.ts b/packages/rum-core/src/domain/configuration/configuration.spec.ts index 47659ca130..0f8dc16e98 100644 --- a/packages/rum-core/src/domain/configuration/configuration.spec.ts +++ b/packages/rum-core/src/domain/configuration/configuration.spec.ts @@ -142,6 +142,27 @@ describe('validateAndBuildRumConfiguration', () => { }) describe('sessionOnErrorSampleRate', () => { + it('is carried into the built configuration', () => { + expect( + validateAndBuildRumConfiguration({ ...DEFAULT_INIT_CONFIGURATION, sessionOnErrorSampleRate: 40 })! + .sessionOnErrorSampleRate + ).toBe(40) + }) + + it('defaults to collecting no error-only session at all', () => { + expect(validateAndBuildRumConfiguration(DEFAULT_INIT_CONFIGURATION)!.sessionOnErrorSampleRate).toBe(0) + }) + + it('is rejected when it is not a sample rate', () => { + expect( + validateAndBuildRumConfiguration({ + ...DEFAULT_INIT_CONFIGURATION, + sessionOnErrorSampleRate: 'foo' as unknown as number, + }) + ).toBeUndefined() + expect(displayErrorSpy).toHaveBeenCalledTimes(1) + }) + it('warns when the default session rate leaves it nothing to draw from', () => { validateAndBuildRumConfiguration({ ...DEFAULT_INIT_CONFIGURATION, diff --git a/packages/rum-core/src/domain/contexts/sessionContext.spec.ts b/packages/rum-core/src/domain/contexts/sessionContext.spec.ts index 9ab0ca3755..74dc341caf 100644 --- a/packages/rum-core/src/domain/contexts/sessionContext.spec.ts +++ b/packages/rum-core/src/domain/contexts/sessionContext.spec.ts @@ -161,7 +161,7 @@ describe('session context', () => { it('should set sampled_for_replay on a session whose events are withheld alongside its replay', () => { // these events only ever leave together with that replay, so reporting the state as it stands // while they are held would mark the whole released burst as having none - sessionManager.setTrackedOnError() + sessionManager.setTrackedOnErrorWithSessionReplay() const event = hooks.triggerHook(HookNames.Assemble, { eventType: 'view', @@ -171,6 +171,46 @@ describe('session context', () => { expect(event.session!.sampled_for_replay).toBe(true) }) + it('should not claim a replay for a session that withholds its events and has none', () => { + sessionManager.setTrackedOnError() + + const event = hooks.triggerHook(HookNames.Assemble, { + eventType: 'view', + startTime: 0 as RelativeTime, + }) as DefaultRumEventAttributes + + expect(event.session!.sampled_for_replay).toBe(false) + }) + + it('should tell the backend a session was stored only because it errored', () => { + sessionManager.setTrackedOnError() + const onErrorEvent = hooks.triggerHook(HookNames.Assemble, { + eventType: 'view', + startTime: 0 as RelativeTime, + }) as DefaultRumEventAttributes + + sessionManager.setTrackedWithSessionReplay() + const plainEvent = hooks.triggerHook(HookNames.Assemble, { + eventType: 'view', + startTime: 0 as RelativeTime, + }) as DefaultRumEventAttributes + + expect(onErrorEvent.session!.sampled_for_error).toBeTrue() + // absent rather than false, so it costs nothing on every ordinary session + expect(plainEvent.session!.sampled_for_error).toBeUndefined() + }) + + it('should say where the stored detail of a released session starts', () => { + sessionManager.setTrackedOnError().setSessionDetailSampledFrom(1234, 'session-id') + + const event = hooks.triggerHook(HookNames.Assemble, { + eventType: 'view', + startTime: 0 as RelativeTime, + }) as DefaultRumEventAttributes + + expect(event.session!.detail_sampled_from).toBe(1234) + }) + it('should discard the event if no session', () => { sessionManager.setNotTracked() const defaultRumEventAttributes = hooks.triggerHook(HookNames.Assemble, { diff --git a/packages/rum-core/src/domain/rumSessionManager.spec.ts b/packages/rum-core/src/domain/rumSessionManager.spec.ts index 4c0cd5e8c0..2a79d11ca5 100644 --- a/packages/rum-core/src/domain/rumSessionManager.spec.ts +++ b/packages/rum-core/src/domain/rumSessionManager.spec.ts @@ -389,6 +389,41 @@ describe('rum session manager', () => { expect(session.sessionReplay).toBe(SessionReplayState.FORCED) }) + it('draws the type that withholds the replay too when only the on-error replay rate is set', () => { + const sessionManager = startRumSessionManagerWithDefaults({ + configuration: { ...ON_ERROR_ONLY, sessionReplayOnErrorSampleRate: 100 }, + }) + + expect(getSessionState(SESSION_STORE_KEY)[RUM_SESSION_KEY]).toBe( + RumTrackingType.TRACKED_ON_ERROR_WITH_SESSION_REPLAY + ) + expect(sessionManager.findTrackedSession()!.sessionReplay).toBe(SessionReplayState.BUFFERED_ON_ERROR) + }) + + it('keeps a stored on-error type across a page load rather than drawing again', () => { + setCookie(SESSION_STORE_KEY, 'id=abcdef&rum=4', DURATION) + + // a rate that would draw a plainly tracked session, so honouring the stored type is the only + // way this can still be an on-error one + const sessionManager = startRumSessionManagerWithDefaults({ configuration: { sessionSampleRate: 100 } }) + + expect(getSessionState(SESSION_STORE_KEY)[RUM_SESSION_KEY]).toBe( + RumTrackingType.TRACKED_ON_ERROR_WITHOUT_SESSION_REPLAY + ) + expect(sessionManager.findTrackedSession()!.eventsWithheld).toBeTrue() + }) + + it('keeps a released on-error session released across a page load', () => { + setCookie(SESSION_STORE_KEY, 'id=abcdef&rum=5&hasError=1', DURATION) + + const sessionManager = startRumSessionManagerWithDefaults({ configuration: { sessionSampleRate: 100 } }) + + const session = sessionManager.findTrackedSession()! + expect(session.eventsWithheld).toBeFalse() + expect(session.sampledOnError).toBeTrue() + expect(session.sessionReplay).toBe(SessionReplayState.SAMPLED) + }) + it('keeps marking the session as on-error once its events have been released', () => { const sessionManager = startRumSessionManagerWithDefaults({ configuration: ON_ERROR_ONLY }) sessionManager.setSessionHasError(sessionManager.findTrackedSession()!.id) diff --git a/packages/rum-core/src/transport/withheldEventBuffer.spec.ts b/packages/rum-core/src/transport/withheldEventBuffer.spec.ts index 98713a1563..b4f2425306 100644 --- a/packages/rum-core/src/transport/withheldEventBuffer.spec.ts +++ b/packages/rum-core/src/transport/withheldEventBuffer.spec.ts @@ -7,8 +7,10 @@ import { RumEventType } from '../rawRumEvent.types' import type { RumEvent } from '../rumEvent.types' import { LifeCycle, LifeCycleEventType } from '../domain/lifeCycle' import { + WITHHELD_BUFFER_BYTES_LIMIT, WITHHELD_BUFFER_DURATION, WITHHELD_BUFFER_EVENTS_LIMIT, + WITHHELD_BUFFER_VIEWS_LIMIT, WITHHELD_BUFFER_RELEASE_MAX_DELAY, computeReleaseDelay, startWithheldEventBuffer, @@ -19,6 +21,7 @@ describe('startWithheldEventBuffer', () => { let lifeCycle: LifeCycle let sessionManager: ReturnType let forwarded: Array + let stopBuffer: () => void function collect(type: RumEventType, overrides: Context = {}) { const event = { @@ -45,6 +48,7 @@ describe('startWithheldEventBuffer', () => { forwarded = [] sessionManager = createRumSessionManagerMock().setTrackedOnError() const { stop } = startWithheldEventBuffer(lifeCycle, sessionManager, (event) => forwarded.push(event)) + stopBuffer = stop registerCleanupTask(() => { stop() clock.cleanup() @@ -399,6 +403,116 @@ describe('startWithheldEventBuffer', () => { expect((view.session as Context).detail_sampled_from).toBe(1000) }) + it('spreads the release over the window it computed for this session', () => { + const delay = computeReleaseDelay('session-id') + // the fixture itself has to have something to spread, or this proves nothing + expect(delay).toBeGreaterThan(0) + + collect(RumEventType.VIEW) + collect(RumEventType.RESOURCE) + sessionManager.setSessionHasError() + collect(RumEventType.ERROR) + + clock.tick(delay - 1) + expect(forwarded.length).toBe(0) + + clock.tick(1) + expect(forwarded.length).toBeGreaterThan(0) + }) + + it('gives up detail once the bytes budget is spent, not only once the count is', () => { + const bulk = 'x'.repeat(8000) + collect(RumEventType.VIEW) + const heldCount = Math.ceil(WITHHELD_BUFFER_BYTES_LIMIT / 8000) + 2 + for (let i = 0; i < heldCount; i++) { + collect(RumEventType.LONG_TASK, { date: i + 1, context: { bulk } }) + } + + sessionManager.setSessionHasError() + collect(RumEventType.ERROR) + + const releasedLongTasks = releasedAfterJitter().filter((event) => event.type === RumEventType.LONG_TASK) + expect(releasedLongTasks.length).toBeLessThan(heldCount) + }) + + it('gives up requests that succeeded before those that failed', () => { + collect(RumEventType.VIEW) + collect(RumEventType.RESOURCE, { resource: { status_code: 500 }, date: 500 }) + collect(RumEventType.RESOURCE, { resource: { status_code: 0 }, date: 1 }) + for (let i = 0; i < WITHHELD_BUFFER_EVENTS_LIMIT; i++) { + collect(RumEventType.RESOURCE, { date: 200 }) + } + + sessionManager.setSessionHasError() + collect(RumEventType.ERROR) + + const releasedDates = releasedAfterJitter().map((event) => event.date) + expect(releasedDates).toContain(500) + expect(releasedDates).toContain(1) + expect(releasedDates.filter((date) => date === 200).length).toBeLessThan(WITHHELD_BUFFER_EVENTS_LIMIT) + }) + + it('keeps no more views than its limit, however many the page goes through', () => { + const viewCount = WITHHELD_BUFFER_VIEWS_LIMIT + 10 + for (let i = 0; i < viewCount; i++) { + collect(RumEventType.VIEW, { date: i + 1, view: { id: `view-${i}` } }) + collect(RumEventType.RESOURCE, { view: { id: `view-${i}` } }) + } + + sessionManager.setSessionHasError() + collect(RumEventType.ERROR, { view: { id: `view-${viewCount - 1}` } }) + + const releasedViews = releasedAfterJitter().filter((event) => event.type === RumEventType.VIEW) + expect(releasedViews.length).toBe(WITHHELD_BUFFER_VIEWS_LIMIT) + }) + + it('drops what has aged out even when the release comes from the page going', () => { + collect(RumEventType.VIEW) + collect(RumEventType.RESOURCE, { date: 111 }) + // another tab marked the session; this one collects nothing further before the page goes + clock.tick(WITHHELD_BUFFER_DURATION + ONE_SECOND) + sessionManager.setSessionHasError() + + lifeCycle.notify(LifeCycleEventType.PAGE_MAY_EXIT, { reason: PageExitReason.UNLOADING }) + + expect(forwarded.map((event) => event.date)).not.toContain(111) + }) + + it('forwards a straggler of a session that was never withholding', () => { + sessionManager.setId('session-2') + collect(RumEventType.VIEW, { session: { id: 'session-2' } }) + + // a request of an earlier, plainly sampled session completes now: it was never withheld from + // anyone, and dropping it would lose an event of a session that is already stored + collect(RumEventType.RESOURCE, { session: { id: 'session-1' } }) + + expect(forwarded.map((event) => (event.session as Context).id)).toEqual(['session-1']) + }) + + it('sends a release that is still waiting on jitter when the session ends', () => { + collect(RumEventType.VIEW) + collect(RumEventType.RESOURCE) + sessionManager.setSessionHasError() + collect(RumEventType.ERROR) + expect(forwarded.length).toBe(0) + + lifeCycle.notify(LifeCycleEventType.SESSION_EXPIRED) + + expect(forwarded.map((event) => event.type)).toEqual([RumEventType.VIEW, RumEventType.RESOURCE, RumEventType.ERROR]) + }) + + it('forwards nothing into a batch that has been stopped', () => { + collect(RumEventType.VIEW) + collect(RumEventType.RESOURCE) + sessionManager.setSessionHasError() + collect(RumEventType.ERROR) + + stopBuffer() + clock.tick(WITHHELD_BUFFER_RELEASE_MAX_DELAY) + + expect(forwarded.length).toBe(0) + }) + it('keeps the view an error hangs from when a view that already ended is updated late', () => { collect(RumEventType.VIEW, { date: 1000, view: { id: 'first-view' } }) collect(RumEventType.VIEW, { date: 2000, view: { id: 'second-view' } }) diff --git a/packages/rum-core/src/transport/withheldEventBuffer.ts b/packages/rum-core/src/transport/withheldEventBuffer.ts index c4e6e58cd4..68c5c89a3a 100644 --- a/packages/rum-core/src/transport/withheldEventBuffer.ts +++ b/packages/rum-core/src/transport/withheldEventBuffer.ts @@ -22,7 +22,7 @@ import type { RumEvent } from '../rumEvent.types' export const WITHHELD_BUFFER_DURATION = 60 * ONE_SECOND /** Memory bound. Above it the least valuable events are dropped first, see {@link EvictionTier}. */ -const WITHHELD_BUFFER_BYTES_LIMIT = 64 * ONE_KIBI_BYTE +export const WITHHELD_BUFFER_BYTES_LIMIT = 64 * ONE_KIBI_BYTE export const WITHHELD_BUFFER_EVENTS_LIMIT = 200 /** @@ -30,7 +30,7 @@ export const WITHHELD_BUFFER_EVENTS_LIMIT = 200 * events, so a detail released without its view would be unreachable. Views are kept out of the * eviction budget for that reason, and this only bounds pathological single-page navigation counts. */ -const WITHHELD_BUFFER_VIEWS_LIMIT = 50 +export const WITHHELD_BUFFER_VIEWS_LIMIT = 50 /** * Correlated errors make every client release at the same instant, right when whatever caused them diff --git a/packages/rum-core/test/mockRumSessionManager.ts b/packages/rum-core/test/mockRumSessionManager.ts index 6bf45734ac..832677fd46 100644 --- a/packages/rum-core/test/mockRumSessionManager.ts +++ b/packages/rum-core/test/mockRumSessionManager.ts @@ -15,6 +15,7 @@ export interface RumSessionManagerMock extends RumSessionManager { setTrackedWithSessionReplay(): RumSessionManagerMock setTrackedWithErrorSessionReplay(): RumSessionManagerMock setTrackedOnError(): RumSessionManagerMock + setTrackedOnErrorWithSessionReplay(): RumSessionManagerMock setForcedReplay(): RumSessionManagerMock setSessionHasError(): RumSessionManagerMock setSessionDetailSampledFrom(timestamp: number, sessionId: string): RumSessionManagerMock @@ -26,6 +27,7 @@ const enum SessionStatus { TRACKED_WITHOUT_SESSION_REPLAY, TRACKED_WITH_ERROR_SESSION_REPLAY, TRACKED_ON_ERROR, + TRACKED_ON_ERROR_WITH_SESSION_REPLAY, NOT_TRACKED, EXPIRED, } @@ -34,7 +36,8 @@ const TRACKING_TYPES: { [key in SessionStatus]?: RumTrackingType } = { [SessionStatus.TRACKED_WITH_SESSION_REPLAY]: RumTrackingType.TRACKED_WITH_SESSION_REPLAY, [SessionStatus.TRACKED_WITHOUT_SESSION_REPLAY]: RumTrackingType.TRACKED_WITHOUT_SESSION_REPLAY, [SessionStatus.TRACKED_WITH_ERROR_SESSION_REPLAY]: RumTrackingType.TRACKED_WITH_ERROR_SESSION_REPLAY, - [SessionStatus.TRACKED_ON_ERROR]: RumTrackingType.TRACKED_ON_ERROR_WITH_SESSION_REPLAY, + [SessionStatus.TRACKED_ON_ERROR]: RumTrackingType.TRACKED_ON_ERROR_WITHOUT_SESSION_REPLAY, + [SessionStatus.TRACKED_ON_ERROR_WITH_SESSION_REPLAY]: RumTrackingType.TRACKED_ON_ERROR_WITH_SESSION_REPLAY, } export function createRumSessionManagerMock(): RumSessionManagerMock { @@ -89,6 +92,10 @@ export function createRumSessionManagerMock(): RumSessionManagerMock { sessionStatus = SessionStatus.TRACKED_ON_ERROR return this }, + setTrackedOnErrorWithSessionReplay() { + sessionStatus = SessionStatus.TRACKED_ON_ERROR_WITH_SESSION_REPLAY + return this + }, setForcedReplay() { forcedReplay = true return this From 6e1903cec881849f100d80ef63579cf80f1e29ec Mon Sep 17 00:00:00 2001 From: Fiona Date: Tue, 1 Sep 2026 10:48:40 -0700 Subject: [PATCH 21/27] refactor(rum): keep the withheld window as one number, and stop guarding 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. --- packages/rum-core/src/index.ts | 1 + .../rum-core/src/transport/withheldEventBuffer.ts | 12 +++++------- .../segmentCollection/segmentCollection.spec.ts | 13 ++++++------- .../domain/segmentCollection/segmentCollection.ts | 12 +++--------- 4 files changed, 15 insertions(+), 23 deletions(-) diff --git a/packages/rum-core/src/index.ts b/packages/rum-core/src/index.ts index c586e8522f..9c790d7f09 100644 --- a/packages/rum-core/src/index.ts +++ b/packages/rum-core/src/index.ts @@ -52,3 +52,4 @@ export type { RumPlugin } from './domain/plugins' export type { MouseEventOnElement } from './domain/action/listenActionEvents' export { supportPerformanceTimingEvent } from './browser/performanceObservable' export { RumPerformanceEntryType } from './browser/performanceObservable' +export { WITHHELD_BUFFER_DURATION } from './transport/withheldEventBuffer' diff --git a/packages/rum-core/src/transport/withheldEventBuffer.ts b/packages/rum-core/src/transport/withheldEventBuffer.ts index 68c5c89a3a..8729f1574e 100644 --- a/packages/rum-core/src/transport/withheldEventBuffer.ts +++ b/packages/rum-core/src/transport/withheldEventBuffer.ts @@ -16,8 +16,10 @@ import { RumEventType } from '../rawRumEvent.types' import type { RumEvent } from '../rumEvent.types' /** - * How much history a withheld buffer may span. Same number as the replay side, because it is the - * same promise to the customer: an error session shows the minute leading up to the error. + * How much history a withheld buffer may span, on the event side and on the replay side alike: it is + * one promise to the customer, that an error session shows the minute leading up to the error. The + * replay side also drops and restarts its buffer on it, which bounds what a session that never + * errors holds on to. */ export const WITHHELD_BUFFER_DURATION = 60 * ONE_SECOND @@ -329,7 +331,6 @@ export function startWithheldEventBuffer( clearBuffer() } - /** Empties the buffer, whether it was just released or is being thrown away. */ function clearBuffer() { clearTimeout(releaseTimeoutId) releaseTimeoutId = undefined @@ -370,9 +371,6 @@ function getEvictionTier(event: RumEvent): EvictionTier { } } -/** Keeps the running hash inside the range `Math.imul` is exact over. */ -const LARGEST_INT32_PRIME = 2147483647 - /** * Deterministic per session, so a client always spreads to the same offset. * @@ -383,7 +381,7 @@ const LARGEST_INT32_PRIME = 2147483647 export function computeReleaseDelay(sessionId: string) { let hash = 0 for (let i = 0; i < sessionId.length; i += 1) { - hash = (Math.imul(hash, 31) + sessionId.charCodeAt(i)) % LARGEST_INT32_PRIME + hash = Math.imul(hash, 31) + sessionId.charCodeAt(i) } return Math.abs(hash) % WITHHELD_BUFFER_RELEASE_MAX_DELAY } diff --git a/packages/rum/src/domain/segmentCollection/segmentCollection.spec.ts b/packages/rum/src/domain/segmentCollection/segmentCollection.spec.ts index c312a2f2fb..75c0340cdf 100644 --- a/packages/rum/src/domain/segmentCollection/segmentCollection.spec.ts +++ b/packages/rum/src/domain/segmentCollection/segmentCollection.spec.ts @@ -1,7 +1,7 @@ import type { ClocksState, HttpRequest, TimeStamp } from '@flashcatcloud/browser-core' import { DeflateEncoderStreamId, noop, PageExitReason } from '@flashcatcloud/browser-core' import type { ViewHistory, ViewHistoryEntry, RumConfiguration } from '@flashcatcloud/browser-rum-core' -import { LifeCycle, LifeCycleEventType } from '@flashcatcloud/browser-rum-core' +import { LifeCycle, LifeCycleEventType, WITHHELD_BUFFER_DURATION } from '@flashcatcloud/browser-rum-core' import type { Clock } from '@flashcatcloud/browser-core/test' import { mockClock, registerCleanupTask, restorePageVisibility } from '@flashcatcloud/browser-core/test' import { createRumSessionManagerMock } from '../../../../rum-core/test' @@ -11,7 +11,6 @@ import { MockWorker, readMetadataFromReplayPayload } from '../../../test' import { createDeflateEncoder } from '../deflate' import * as replayStats from '../replayStats' import { - BUFFER_CHECKOUT_TIME, computeSegmentContext, doStartSegmentCollection, SEGMENT_BYTES_LIMIT, @@ -404,7 +403,7 @@ describe('startSegmentCollection withholding (error session replay)', () => { it('drops the buffer and restarts from a full snapshot once it spans the checkout time', () => { addRecord(RECORD) - clock.tick(BUFFER_CHECKOUT_TIME) + clock.tick(WITHHELD_BUFFER_DURATION) worker.processAllMessages() expect(httpRequestSpy.send).not.toHaveBeenCalled() @@ -467,7 +466,7 @@ describe('startSegmentCollection withholding (error session replay)', () => { restartFromFullSnapshotSpy.and.callFake(() => addRecord(RECORD)) addRecord(RECORD) - clock.tick(BUFFER_CHECKOUT_TIME) + clock.tick(WITHHELD_BUFFER_DURATION) worker.processAllMessages() expect(restartFromFullSnapshotSpy).toHaveBeenCalledTimes(1) @@ -485,7 +484,7 @@ describe('startSegmentCollection withholding (error session replay)', () => { it('does not restart the buffer when collection was stopped while the flush was in flight', () => { addRecord(RECORD) // the checkout flush is posted to the worker, and recording is stopped before it answers - clock.tick(BUFFER_CHECKOUT_TIME) + clock.tick(WITHHELD_BUFFER_DURATION) stopCollection() worker.processAllMessages() @@ -499,7 +498,7 @@ describe('startSegmentCollection withholding (error session replay)', () => { // The flush is posted to the worker but not answered yet - in production that round trip always // happens, because flushing writes the trailer before finishing. A record arriving now creates // the next segment, which reads its index while the dropped one is still counted. - clock.tick(BUFFER_CHECKOUT_TIME) + clock.tick(WITHHELD_BUFFER_DURATION) addRecord(RECORD) worker.processAllMessages() @@ -512,7 +511,7 @@ describe('startSegmentCollection withholding (error session replay)', () => { it('leaves no trace of a dropped buffer in the replay stats', () => { addRecord(RECORD) - clock.tick(BUFFER_CHECKOUT_TIME) + clock.tick(WITHHELD_BUFFER_DURATION) worker.processAllMessages() const stats = replayStats.getReplayStats(CONTEXT.view.id) diff --git a/packages/rum/src/domain/segmentCollection/segmentCollection.ts b/packages/rum/src/domain/segmentCollection/segmentCollection.ts index 3e7346d935..972d62af98 100644 --- a/packages/rum/src/domain/segmentCollection/segmentCollection.ts +++ b/packages/rum/src/domain/segmentCollection/segmentCollection.ts @@ -8,7 +8,7 @@ import { setTimeout, } from '@flashcatcloud/browser-core' import type { LifeCycle, ViewHistory, RumSessionManager, RumConfiguration } from '@flashcatcloud/browser-rum-core' -import { LifeCycleEventType } from '@flashcatcloud/browser-rum-core' +import { LifeCycleEventType, WITHHELD_BUFFER_DURATION } from '@flashcatcloud/browser-rum-core' import type { BrowserRecord, CreationReason, SegmentContext } from '../../types' import { discardSegmentData, removeSegment } from '../replayStats' import { buildReplayPayload } from './buildReplayPayload' @@ -17,12 +17,6 @@ import { createSegment } from './segment' export const SEGMENT_DURATION_LIMIT = 5 * ONE_SECOND -/** - * How much history a withheld buffer may span before it is dropped and restarted from a fresh full - * snapshot. This bounds two things at once: the memory a session that never errors holds on to, and - * how far back an error session can show once its buffer is released. - */ -export const BUFFER_CHECKOUT_TIME = 60 * ONE_SECOND /** * beacon payload max queue size implementation is 64kb * ensure that we leave room for logs, rum and potential other users @@ -121,7 +115,7 @@ type SegmentCollectionState = /** * `buffer_checkout` is internal: it drops a withheld buffer that has grown past - * {@link BUFFER_CHECKOUT_TIME}. It never reaches the intake, so it is mapped back to a schema value + * {@link WITHHELD_BUFFER_DURATION}. It never reaches the intake, so it is mapped back to a schema value * where the next segment records why it was created. */ type InternalFlushReason = FlushReason | 'buffer_checkout' @@ -281,7 +275,7 @@ export function doStartSegmentCollection( withheldForSessionId !== undefined ? setTimeout(() => { flushSegment('buffer_checkout') - }, BUFFER_CHECKOUT_TIME) + }, WITHHELD_BUFFER_DURATION) : undefined, withheldForSessionId, viewId: context.view.id, From a140bca26e9db68c5a8dc298d23a2db82dfd88d5 Mon Sep 17 00:00:00 2001 From: Fiona Date: Tue, 1 Sep 2026 11:23:44 -0700 Subject: [PATCH 22/27] fix(rum): finish the half-handled cases the earlier fixes left behind 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. --- .../configuration/configuration.spec.ts | 13 ++++++++++++ .../src/domain/configuration/configuration.ts | 17 ++++++++++----- .../domain/contexts/sessionContext.spec.ts | 14 +++++++++++++ .../src/domain/contexts/sessionContext.ts | 11 ++++++---- .../src/transport/withheldEventBuffer.spec.ts | 21 +++++++++++++++++++ .../src/transport/withheldEventBuffer.ts | 17 ++++++++++++--- 6 files changed, 81 insertions(+), 12 deletions(-) diff --git a/packages/rum-core/src/domain/configuration/configuration.spec.ts b/packages/rum-core/src/domain/configuration/configuration.spec.ts index 0f8dc16e98..b3d1845ca7 100644 --- a/packages/rum-core/src/domain/configuration/configuration.spec.ts +++ b/packages/rum-core/src/domain/configuration/configuration.spec.ts @@ -163,6 +163,19 @@ describe('validateAndBuildRumConfiguration', () => { expect(displayErrorSpy).toHaveBeenCalledTimes(1) }) + it('warns when the replay it would withhold is never recorded', () => { + validateAndBuildRumConfiguration({ + ...DEFAULT_INIT_CONFIGURATION, + sessionSampleRate: 20, + sessionOnErrorSampleRate: 50, + sessionReplaySampleRate: 50, + startSessionReplayRecordingManually: true, + }) + + expect(displayWarnSpy).toHaveBeenCalledTimes(1) + expect(displayWarnSpy.calls.argsFor(0)[0]).toContain('startSessionReplayRecordingManually') + }) + it('warns when the default session rate leaves it nothing to draw from', () => { validateAndBuildRumConfiguration({ ...DEFAULT_INIT_CONFIGURATION, diff --git a/packages/rum-core/src/domain/configuration/configuration.ts b/packages/rum-core/src/domain/configuration/configuration.ts index 7a31be18f4..b57bfbdadb 100644 --- a/packages/rum-core/src/domain/configuration/configuration.ts +++ b/packages/rum-core/src/domain/configuration/configuration.ts @@ -282,11 +282,18 @@ export function validateAndBuildRumConfiguration( 'sessionReplayOnErrorSampleRate has no effect while sessionSampleRate is 0 and sessionOnErrorSampleRate is unset: no session is tracked.' ) } - if (initConfiguration.startSessionReplayRecordingManually) { - display.warn( - 'sessionReplayOnErrorSampleRate needs the recording to already be running when the error happens, and startSessionReplayRecordingManually keeps it stopped until you start it: there would be nothing to release.' - ) - } + } + + // A session drawn on error withholds whichever replay it draws, so the same trap is reachable + // through the plain replay rate as well - and there it is worse than silence, since the released + // views would report a replay for a recording that never ran. + if ( + initConfiguration.startSessionReplayRecordingManually && + (sessionReplayOnErrorSampleRate > 0 || (sessionOnErrorSampleRate > 0 && sessionReplaySampleRate > 0)) + ) { + display.warn( + 'A replay kept until the session errors has to be recording before that error, and startSessionReplayRecordingManually keeps it stopped until you start it: there would be nothing to release.' + ) } return { diff --git a/packages/rum-core/src/domain/contexts/sessionContext.spec.ts b/packages/rum-core/src/domain/contexts/sessionContext.spec.ts index 74dc341caf..56792d440c 100644 --- a/packages/rum-core/src/domain/contexts/sessionContext.spec.ts +++ b/packages/rum-core/src/domain/contexts/sessionContext.spec.ts @@ -171,6 +171,20 @@ describe('session context', () => { expect(event.session!.sampled_for_replay).toBe(true) }) + it('should set hasReplay on the events of a session that withholds them alongside its replay', () => { + // they only ever leave together with that replay, so the error that releases them must not say + // there is no replay to watch + sessionManager.setTrackedOnErrorWithSessionReplay() + isRecordingSpy.and.returnValue(true) + + const event = hooks.triggerHook(HookNames.Assemble, { + eventType: 'error', + startTime: 0 as RelativeTime, + }) as DefaultRumEventAttributes + + expect(event.session!.has_replay).toBe(true) + }) + it('should not claim a replay for a session that withholds its events and has none', () => { sessionManager.setTrackedOnError() diff --git a/packages/rum-core/src/domain/contexts/sessionContext.ts b/packages/rum-core/src/domain/contexts/sessionContext.ts index 4747efe7d9..d9ecad0f34 100644 --- a/packages/rum-core/src/domain/contexts/sessionContext.ts +++ b/packages/rum-core/src/domain/contexts/sessionContext.ts @@ -21,8 +21,11 @@ export function startSessionContext( } // A session withholding its replay is recording, but nothing has been uploaded and nothing may - // ever be. Reporting `has_replay` here would offer a replay that does not exist. + // ever be, so reporting `has_replay` would offer a replay that does not exist. Unless the events + // are withheld alongside it: those only ever leave together with that replay, so by the time + // they are read the replay is there. const isReplayWithheld = session.sessionReplay === SessionReplayState.BUFFERED_ON_ERROR + const shipsWithoutItsReplay = isReplayWithheld && !session.eventsWithheld let hasReplay let sampledForReplay @@ -35,12 +38,12 @@ export function startSessionContext( // back, which leaves a view with replay stats and no replay at all - and offering a replay // that was never uploaded is worse than not offering one. const replayStats = recorderApi.getReplayStats(view.id) - hasReplay = !isReplayWithheld && replayStats && replayStats.segments_count > 0 ? true : undefined + hasReplay = !shipsWithoutItsReplay && replayStats && replayStats.segments_count > 0 ? true : undefined // A session that withholds its events withholds its replay alongside them, so if these events // are ever uploaded that replay is on its way with them. Reporting the state as it stands at // assembly time would mark the whole released burst as a session that has no replay. sampledForReplay = - session.sessionReplay === SessionReplayState.SAMPLED || (session.eventsWithheld && isReplayWithheld) + session.sessionReplay === SessionReplayState.SAMPLED || (isReplayWithheld && session.eventsWithheld) // Tells the backend that this session's detail only starts where the buffer reached, so the // gap before it reads as "not collected" rather than as missing data. sampledForError = session.sampledOnError || undefined @@ -50,7 +53,7 @@ export function startSessionContext( sampledForErrorReplay = session.sampledOnErrorReplay || undefined isActive = view.sessionIsActive ? undefined : false } else { - hasReplay = !isReplayWithheld && recorderApi.isRecording() ? true : undefined + hasReplay = !shipsWithoutItsReplay && recorderApi.isRecording() ? true : undefined } return { diff --git a/packages/rum-core/src/transport/withheldEventBuffer.spec.ts b/packages/rum-core/src/transport/withheldEventBuffer.spec.ts index b4f2425306..168453dc0d 100644 --- a/packages/rum-core/src/transport/withheldEventBuffer.spec.ts +++ b/packages/rum-core/src/transport/withheldEventBuffer.spec.ts @@ -513,6 +513,27 @@ describe('startWithheldEventBuffer', () => { expect(forwarded.length).toBe(0) }) + it('keeps the view an error hangs from even when the view cap has to evict one', () => { + const last = WITHHELD_BUFFER_VIEWS_LIMIT - 1 + // a page that has been through exactly as many views as the buffer will hold + for (let i = 0; i <= last; i++) { + collect(RumEventType.VIEW, { date: i + 1, view: { id: `view-${i}` } }) + collect(RumEventType.RESOURCE, { view: { id: `view-${i}` } }) + } + // every earlier view is updated late, which moves each of them behind the current one - so the + // view in progress ends up the oldest entry, and the cap takes from the oldest + for (let i = 0; i < last; i++) { + collect(RumEventType.VIEW, { date: i + 1, view: { id: `view-${i}` } }) + } + // one more late update, for a view old enough to have been dropped already, tips it over the cap + collect(RumEventType.VIEW, { date: 1, view: { id: 'long-gone-view' } }) + + sessionManager.setSessionHasError() + collect(RumEventType.ERROR, { view: { id: `view-${last}` } }) + + expect(releasedAfterJitter().map((event) => event.type)).toContain(RumEventType.ERROR) + }) + it('keeps the view an error hangs from when a view that already ended is updated late', () => { collect(RumEventType.VIEW, { date: 1000, view: { id: 'first-view' } }) collect(RumEventType.VIEW, { date: 2000, view: { id: 'second-view' } }) diff --git a/packages/rum-core/src/transport/withheldEventBuffer.ts b/packages/rum-core/src/transport/withheldEventBuffer.ts index 8729f1574e..0dd391008e 100644 --- a/packages/rum-core/src/transport/withheldEventBuffer.ts +++ b/packages/rum-core/src/transport/withheldEventBuffer.ts @@ -41,9 +41,11 @@ export const WITHHELD_BUFFER_VIEWS_LIMIT = 50 export const WITHHELD_BUFFER_RELEASE_MAX_DELAY = 3 * ONE_SECOND /** - * How many thrown-away sessions to remember, so their stragglers are thrown away too. A late event - * can only still be assembled for a session while the session context history holds it, which is far - * shorter than the life of one session - a handful covers every straggler that can still arrive. + * How many thrown-away sessions to remember, so their stragglers are thrown away too. The session + * context history holds a session for up to its maximum length, so a request that outlives this many + * discarded sessions - hours of them - is forwarded after all. What escapes is a lone detail event + * with no view of its own, which has nothing to attach to at the other end; paying for a longer + * memory to catch it would cost more than it saves. */ const DISCARDED_SESSIONS_REMEMBERED = 4 @@ -182,6 +184,15 @@ export function startWithheldEventBuffer( currentViewId = event.view.id } while (views.size > WITHHELD_BUFFER_VIEWS_LIMIT) { + const oldestViewId: string = views.keys().next().value! + if (oldestViewId === currentViewId) { + // The view in progress is the container the error will hang from, which is why `prune` + // spares it too. Late updates of ended views can push it to the front of the map, so it is + // moved to the back here rather than dropped - which, as above, takes a delete. + const currentView = views.get(oldestViewId)! + views.delete(oldestViewId) + views.set(oldestViewId, currentView) + } views.delete(views.keys().next().value!) } prune() From 659f0e01caef36da64aebe66317c89e8c58edcdb Mon Sep 17 00:00:00 2001 From: Fiona Date: Tue, 1 Sep 2026 11:35:17 -0700 Subject: [PATCH 23/27] fix(rum): do not claim a replay whose fate is not decided yet 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. --- .../domain/configuration/configuration.spec.ts | 13 +++++++++++++ .../src/domain/configuration/configuration.ts | 5 ++++- .../src/domain/contexts/sessionContext.spec.ts | 18 +++++++++++++----- .../src/domain/contexts/sessionContext.ts | 13 +++++++------ 4 files changed, 37 insertions(+), 12 deletions(-) diff --git a/packages/rum-core/src/domain/configuration/configuration.spec.ts b/packages/rum-core/src/domain/configuration/configuration.spec.ts index b3d1845ca7..88de319050 100644 --- a/packages/rum-core/src/domain/configuration/configuration.spec.ts +++ b/packages/rum-core/src/domain/configuration/configuration.spec.ts @@ -176,6 +176,19 @@ describe('validateAndBuildRumConfiguration', () => { expect(displayWarnSpy.calls.argsFor(0)[0]).toContain('startSessionReplayRecordingManually') }) + it('says nothing about a replay it could never withhold anyway', () => { + validateAndBuildRumConfiguration({ + ...DEFAULT_INIT_CONFIGURATION, + sessionOnErrorSampleRate: 50, + sessionReplaySampleRate: 30, + startSessionReplayRecordingManually: true, + }) + + // the on-error rate cannot draw at all here, which is the one thing worth saying + expect(displayWarnSpy).toHaveBeenCalledTimes(1) + expect(displayWarnSpy.calls.argsFor(0)[0]).toContain('sessionSampleRate did not draw') + }) + it('warns when the default session rate leaves it nothing to draw from', () => { validateAndBuildRumConfiguration({ ...DEFAULT_INIT_CONFIGURATION, diff --git a/packages/rum-core/src/domain/configuration/configuration.ts b/packages/rum-core/src/domain/configuration/configuration.ts index b57bfbdadb..863d3bb540 100644 --- a/packages/rum-core/src/domain/configuration/configuration.ts +++ b/packages/rum-core/src/domain/configuration/configuration.ts @@ -289,7 +289,10 @@ export function validateAndBuildRumConfiguration( // views would report a replay for a recording that never ran. if ( initConfiguration.startSessionReplayRecordingManually && - (sessionReplayOnErrorSampleRate > 0 || (sessionOnErrorSampleRate > 0 && sessionReplaySampleRate > 0)) + (sessionReplayOnErrorSampleRate > 0 || + (sessionOnErrorSampleRate > 0 && + sessionReplaySampleRate > 0 && + (initConfiguration.sessionSampleRate ?? 100) < 100)) ) { display.warn( 'A replay kept until the session errors has to be recording before that error, and startSessionReplayRecordingManually keeps it stopped until you start it: there would be nothing to release.' diff --git a/packages/rum-core/src/domain/contexts/sessionContext.spec.ts b/packages/rum-core/src/domain/contexts/sessionContext.spec.ts index 56792d440c..eef27da7e7 100644 --- a/packages/rum-core/src/domain/contexts/sessionContext.spec.ts +++ b/packages/rum-core/src/domain/contexts/sessionContext.spec.ts @@ -171,18 +171,26 @@ describe('session context', () => { expect(event.session!.sampled_for_replay).toBe(true) }) - it('should set hasReplay on the events of a session that withholds them alongside its replay', () => { - // they only ever leave together with that replay, so the error that releases them must not say - // there is no replay to watch + it('should not claim a replay while one is withheld, whichever way it turns out', () => { + // the segment covering this event is dropped on the next view change and sent only if the error + // comes first; the event is assembled before either, so it claims nothing sessionManager.setTrackedOnErrorWithSessionReplay() isRecordingSpy.and.returnValue(true) + getReplayStatsSpy.and.returnValue(fakeStats) - const event = hooks.triggerHook(HookNames.Assemble, { + const errorEvent = hooks.triggerHook(HookNames.Assemble, { eventType: 'error', startTime: 0 as RelativeTime, }) as DefaultRumEventAttributes + const viewEvent = hooks.triggerHook(HookNames.Assemble, { + eventType: 'view', + startTime: 0 as RelativeTime, + }) as DefaultRumEventAttributes - expect(event.session!.has_replay).toBe(true) + expect(errorEvent.session!.has_replay).toBeUndefined() + expect(viewEvent.session!.has_replay).toBeUndefined() + // but the session was sampled for one, and that is answerable without knowing any segment's fate + expect(viewEvent.session!.sampled_for_replay).toBe(true) }) it('should not claim a replay for a session that withholds its events and has none', () => { diff --git a/packages/rum-core/src/domain/contexts/sessionContext.ts b/packages/rum-core/src/domain/contexts/sessionContext.ts index d9ecad0f34..ce8f7ed1f5 100644 --- a/packages/rum-core/src/domain/contexts/sessionContext.ts +++ b/packages/rum-core/src/domain/contexts/sessionContext.ts @@ -21,11 +21,12 @@ export function startSessionContext( } // A session withholding its replay is recording, but nothing has been uploaded and nothing may - // ever be, so reporting `has_replay` would offer a replay that does not exist. Unless the events - // are withheld alongside it: those only ever leave together with that replay, so by the time - // they are read the replay is there. + // ever be. An event assembled now cannot know which of the two it will turn out to be: the + // segment covering it is dropped on the next view change and sent only if the error comes first, + // and it is assembled before either happens - the final update of a view is emitted before the + // view change that drops that view's segment. So it does not claim a replay. Whether the session + // was *sampled* for one is a different question, answerable here, and answered below. const isReplayWithheld = session.sessionReplay === SessionReplayState.BUFFERED_ON_ERROR - const shipsWithoutItsReplay = isReplayWithheld && !session.eventsWithheld let hasReplay let sampledForReplay @@ -38,7 +39,7 @@ export function startSessionContext( // back, which leaves a view with replay stats and no replay at all - and offering a replay // that was never uploaded is worse than not offering one. const replayStats = recorderApi.getReplayStats(view.id) - hasReplay = !shipsWithoutItsReplay && replayStats && replayStats.segments_count > 0 ? true : undefined + hasReplay = !isReplayWithheld && replayStats && replayStats.segments_count > 0 ? true : undefined // A session that withholds its events withholds its replay alongside them, so if these events // are ever uploaded that replay is on its way with them. Reporting the state as it stands at // assembly time would mark the whole released burst as a session that has no replay. @@ -53,7 +54,7 @@ export function startSessionContext( sampledForErrorReplay = session.sampledOnErrorReplay || undefined isActive = view.sessionIsActive ? undefined : false } else { - hasReplay = !shipsWithoutItsReplay && recorderApi.isRecording() ? true : undefined + hasReplay = !isReplayWithheld && recorderApi.isRecording() ? true : undefined } return { From fc3ed2f499d61a06684405aeaecf10551b641ec7 Mon Sep 17 00:00:00 2001 From: Fiona Date: Tue, 1 Sep 2026 23:19:31 -0700 Subject: [PATCH 24/27] refactor(rum): stop shipping a number the stored data already answers `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. --- .../core/src/domain/session/sessionManager.ts | 3 -- .../domain/contexts/sessionContext.spec.ts | 11 ------ .../src/domain/contexts/sessionContext.ts | 3 -- .../src/domain/rumSessionManager.spec.ts | 22 ----------- .../rum-core/src/domain/rumSessionManager.ts | 33 ---------------- .../src/transport/withheldEventBuffer.spec.ts | 38 ------------------- .../src/transport/withheldEventBuffer.ts | 25 +----------- .../rum-core/test/mockRumSessionManager.ts | 7 ---- 8 files changed, 1 insertion(+), 141 deletions(-) diff --git a/packages/core/src/domain/session/sessionManager.ts b/packages/core/src/domain/session/sessionManager.ts index 7ada9bd2d3..fae90339a5 100644 --- a/packages/core/src/domain/session/sessionManager.ts +++ b/packages/core/src/domain/session/sessionManager.ts @@ -33,8 +33,6 @@ export interface SessionContext extends Context { * just because the user moved to another page. */ hasError: boolean - /** Where the detail stored for this session starts, when its events were withheld for a while. */ - detailSampledFrom: number | undefined anonymousId: string | undefined } @@ -101,7 +99,6 @@ export function startSessionManager( trackingType: sessionStore.getSession()[productKey] as TrackingType, isReplayForced: !!sessionStore.getSession().forcedReplay, hasError: !!sessionStore.getSession().hasError, - detailSampledFrom: Number(sessionStore.getSession().detailFrom) || undefined, anonymousId: sessionStore.getSession().anonymousId, } } diff --git a/packages/rum-core/src/domain/contexts/sessionContext.spec.ts b/packages/rum-core/src/domain/contexts/sessionContext.spec.ts index 70e9917366..affad1c3a2 100644 --- a/packages/rum-core/src/domain/contexts/sessionContext.spec.ts +++ b/packages/rum-core/src/domain/contexts/sessionContext.spec.ts @@ -234,17 +234,6 @@ describe('session context', () => { expect(plainEvent.session!.sampled_for_error).toBeUndefined() }) - it('should say where the stored detail of a released session starts', () => { - sessionManager.setTrackedOnError().setSessionDetailSampledFrom(1234, 'session-id') - - const event = hooks.triggerHook(HookNames.Assemble, { - eventType: 'view', - startTime: 0 as RelativeTime, - }) as DefaultRumEventAttributes - - expect(event.session!.detail_sampled_from).toBe(1234) - }) - it('should discard the event if no session', () => { sessionManager.setNotTracked() const defaultRumEventAttributes = hooks.triggerHook(HookNames.Assemble, { diff --git a/packages/rum-core/src/domain/contexts/sessionContext.ts b/packages/rum-core/src/domain/contexts/sessionContext.ts index 3f99618aa6..cf9b3b3e29 100644 --- a/packages/rum-core/src/domain/contexts/sessionContext.ts +++ b/packages/rum-core/src/domain/contexts/sessionContext.ts @@ -31,7 +31,6 @@ export function startSessionContext( let hasReplay let sampledForReplay let sampledForError - let detailSampledFrom let sampledForErrorReplay let isActive if (eventType === RumEventType.VIEW) { @@ -49,7 +48,6 @@ export function startSessionContext( // Tells the backend that this session's detail only starts where the buffer reached, so the // gap before it reads as "not collected" rather than as missing data. sampledForError = session.sampledOnError || undefined - detailSampledFrom = session.detailSampledFrom // Tells a replay collected only because the session errored apart from one collected // unconditionally - the two cost differently and are answered by different questions. sampledForErrorReplay = session.sampledOnErrorReplay || undefined @@ -66,7 +64,6 @@ export function startSessionContext( has_replay: hasReplay, sampled_for_replay: sampledForReplay, sampled_for_error: sampledForError, - detail_sampled_from: detailSampledFrom, sampled_for_error_replay: sampledForErrorReplay, is_active: isActive, }, diff --git a/packages/rum-core/src/domain/rumSessionManager.spec.ts b/packages/rum-core/src/domain/rumSessionManager.spec.ts index 2a79d11ca5..48e5d6071c 100644 --- a/packages/rum-core/src/domain/rumSessionManager.spec.ts +++ b/packages/rum-core/src/domain/rumSessionManager.spec.ts @@ -430,28 +430,6 @@ describe('rum session manager', () => { expect(sessionManager.findTrackedSession()!.sampledOnError).toBeTrue() }) - - it('keeps the earliest point any tab reached as where the stored detail starts', () => { - const sessionManager = startRumSessionManagerWithDefaults({ configuration: ON_ERROR_ONLY }) - const sessionId = sessionManager.findTrackedSession()!.id - - // two tabs of the same session release their own buffers, each reaching back a different way - sessionManager.setSessionDetailSampledFrom(2000, sessionId) - sessionManager.setSessionDetailSampledFrom(1000, sessionId) - sessionManager.setSessionDetailSampledFrom(3000, sessionId) - - expect(getSessionState(SESSION_STORE_KEY).detailFrom).toBe('1000') - expect(sessionManager.findTrackedSession()!.detailSampledFrom).toBe(1000) - }) - - it('does not record where the detail starts on a session that has since been replaced', () => { - const sessionManager = startRumSessionManagerWithDefaults({ configuration: ON_ERROR_ONLY }) - - setCookie(SESSION_STORE_KEY, 'id=other-session&rum=4', DURATION) - sessionManager.setSessionDetailSampledFrom(1000, 'a-session-that-is-gone') - - expect(getSessionState(SESSION_STORE_KEY).detailFrom).toBeUndefined() - }) }) function startRumSessionManagerWithDefaults({ configuration }: { configuration?: Partial } = {}) { diff --git a/packages/rum-core/src/domain/rumSessionManager.ts b/packages/rum-core/src/domain/rumSessionManager.ts index 0f49604da5..1be9441871 100644 --- a/packages/rum-core/src/domain/rumSessionManager.ts +++ b/packages/rum-core/src/domain/rumSessionManager.ts @@ -30,11 +30,6 @@ export interface RumSessionManager { * because the store write can be deferred by the lock, and it must not land on a later session. */ setSessionHasError: (sessionId: string) => void - /** - * Records how far back the detail released for this session actually reaches. The earliest point - * any tab reached wins, since that is where the session's stored detail really starts. - */ - setSessionDetailSampledFrom: (timestamp: number, sessionId: string) => void } export type RumSession = { @@ -56,11 +51,6 @@ export type RumSession = { * {@link sampledOnError}, for the replay rather than the events. */ sampledOnErrorReplay: boolean - /** - * Where the detail stored for this session starts, for a session whose events were withheld. The - * gap before it is data that was never collected rather than data that went missing. - */ - detailSampledFrom?: number anonymousId?: string } @@ -118,14 +108,6 @@ export function startRumSessionManager( sessionEntity.hasError = true } } - // Followed rather than latched on the first value seen: the store keeps the earliest point any - // tab reached, so a later, earlier write is a correction and not a second opinion. - if (previousState.detailFrom !== newState.detailFrom) { - const sessionEntity = sessionManager.findSession() - if (sessionEntity) { - sessionEntity.detailSampledFrom = Number(newState.detailFrom) || undefined - } - } }) return { findTrackedSession: (startTime) => { @@ -139,7 +121,6 @@ export function startRumSessionManager( eventsWithheld: computeEventsWithheld(session.trackingType, session.hasError, session.isReplayForced), sampledOnError: withholdsEvents(session.trackingType), sampledOnErrorReplay: withholdsReplay(session.trackingType), - detailSampledFrom: session.detailSampledFrom, anonymousId: session.anonymousId, } }, @@ -157,19 +138,6 @@ export function startRumSessionManager( } sessionManager.updateSessionState((state) => (state.id === sessionId ? { hasError: '1' } : undefined)) }, - // Kept on the session rather than stamped on the released view events: the batch upserts views - // by id, so the next ordinary view update - which arrives within seconds - would replace the - // stamped one before the batch is ever sent. - setSessionDetailSampledFrom: (timestamp, sessionId) => - sessionManager.updateSessionState((state) => { - if (state.id !== sessionId) { - return undefined - } - // Both tabs of a session release their own buffer on the same error, and the session's - // detail starts wherever the earliest of them reached. - const stored = Number(state.detailFrom) - return stored && stored <= timestamp ? undefined : { detailFrom: String(timestamp) } - }), } } @@ -239,7 +207,6 @@ export function startRumSessionManagerStub(): RumSessionManager { expireObservable: new Observable(), setForcedReplay: noop, setSessionHasError: noop, - setSessionDetailSampledFrom: noop, } } diff --git a/packages/rum-core/src/transport/withheldEventBuffer.spec.ts b/packages/rum-core/src/transport/withheldEventBuffer.spec.ts index 168453dc0d..6d260a1035 100644 --- a/packages/rum-core/src/transport/withheldEventBuffer.spec.ts +++ b/packages/rum-core/src/transport/withheldEventBuffer.spec.ts @@ -90,17 +90,6 @@ describe('startWithheldEventBuffer', () => { ]) }) - it('marks how far back the released detail reaches', () => { - collect(RumEventType.VIEW) - collect(RumEventType.RESOURCE, { date: 4321 }) - - sessionManager.setSessionHasError() - collect(RumEventType.ERROR, { date: 9999 }) - - const view = releasedAfterJitter().find((event) => event.type === RumEventType.VIEW)! - expect((view.session as Context).detail_sampled_from).toBe(4321) - }) - it('keeps only the latest event of a view, since a view event supersedes the ones before it', () => { collect(RumEventType.VIEW, { documentVersion: 1 }) collect(RumEventType.VIEW, { documentVersion: 2 }) @@ -175,20 +164,6 @@ describe('startWithheldEventBuffer', () => { expect(dates).toContain(111) }) - it('records on the session how far back the released detail reaches', () => { - const spy = spyOn(sessionManager, 'setSessionDetailSampledFrom').and.callThrough() - collect(RumEventType.VIEW) - collect(RumEventType.RESOURCE, { date: 4321 }) - - sessionManager.setSessionHasError() - collect(RumEventType.ERROR, { date: 9999 }) - releasedAfterJitter() - - // kept on the session, because the batch upserts views by id and the next ordinary view update - // would otherwise replace the stamped one before anything is sent - expect(spy).toHaveBeenCalledWith(4321, 'session-id') - }) - it('drops the buffer when the session ends without ever having errored', () => { collect(RumEventType.VIEW) collect(RumEventType.RESOURCE) @@ -390,19 +365,6 @@ describe('startWithheldEventBuffer', () => { expect(releasedViewDates).toEqual([1000, 2000, 3000]) }) - it('marks the detail as starting at the earliest event, not at the first one held', () => { - collect(RumEventType.VIEW) - // a request that took minutes is only held once it finishes, but it started well before that - collect(RumEventType.RESOURCE, { date: 5000 }) - collect(RumEventType.RESOURCE, { date: 1000 }) - - sessionManager.setSessionHasError() - collect(RumEventType.ERROR, { date: 9000 }) - - const view = releasedAfterJitter().find((event) => event.type === RumEventType.VIEW)! - expect((view.session as Context).detail_sampled_from).toBe(1000) - }) - it('spreads the release over the window it computed for this session', () => { const delay = computeReleaseDelay('session-id') // the fixture itself has to have something to spread, or this proves nothing diff --git a/packages/rum-core/src/transport/withheldEventBuffer.ts b/packages/rum-core/src/transport/withheldEventBuffer.ts index 0dd391008e..c803a42bd5 100644 --- a/packages/rum-core/src/transport/withheldEventBuffer.ts +++ b/packages/rum-core/src/transport/withheldEventBuffer.ts @@ -288,24 +288,6 @@ export function startWithheldEventBuffer( // A detail whose view is gone has no container to hang from, so it would be unreachable. const releasable = details.filter((held) => views.has(held.viewId)) - // The earliest date among them, not the first one held: an event is dated when it started, and a - // request that took minutes is held only once it finishes - so the first held is not the first - // to have happened, and the marker has to be a point no released detail precedes. - let detailSampledFrom: number | undefined - releasable.forEach((held) => { - if (detailSampledFrom === undefined || held.event.date < detailSampledFrom) { - detailSampledFrom = held.event.date - } - }) - - if (detailSampledFrom !== undefined) { - // Recorded on the session so that every view update from here on carries it - the batch - // upserts views by id, so the next ordinary update would otherwise replace these ones before - // the batch is ever sent. These were assembled too early to pick it up, so they are given the - // same value directly, which is what the backend sees if the page goes before the next update. - sessionManager.setSessionDetailSampledFrom(detailSampledFrom, withheldForSessionId!) - } - // Oldest first. A Map holds its entries in the order they were last updated, which for a burst // released all at once is not the order the views happened - and a session is built out of // whichever of its views arrives first, so that one has to be the earliest. @@ -313,12 +295,7 @@ export function startWithheldEventBuffer( views.forEach((view) => orderedViews.push(view)) orderedViews.sort((left, right) => left.date - right.date) - orderedViews.forEach((view) => { - if (detailSampledFrom !== undefined) { - view.session.detail_sampled_from = detailSampledFrom - } - forward(view) - }) + orderedViews.forEach(forward) releasable.forEach((held) => forward(held.event)) addTelemetryDebug('Error session event buffer released', { diff --git a/packages/rum-core/test/mockRumSessionManager.ts b/packages/rum-core/test/mockRumSessionManager.ts index 832677fd46..98940471b0 100644 --- a/packages/rum-core/test/mockRumSessionManager.ts +++ b/packages/rum-core/test/mockRumSessionManager.ts @@ -18,7 +18,6 @@ export interface RumSessionManagerMock extends RumSessionManager { setTrackedOnErrorWithSessionReplay(): RumSessionManagerMock setForcedReplay(): RumSessionManagerMock setSessionHasError(): RumSessionManagerMock - setSessionDetailSampledFrom(timestamp: number, sessionId: string): RumSessionManagerMock } const DEFAULT_ID = 'session-id' @@ -45,7 +44,6 @@ export function createRumSessionManagerMock(): RumSessionManagerMock { let sessionStatus: SessionStatus = SessionStatus.TRACKED_WITH_SESSION_REPLAY let forcedReplay: boolean = false let hasError: boolean = false - let detailSampledFrom: number | undefined return { findTrackedSession() { const trackingType = TRACKING_TYPES[sessionStatus] @@ -58,7 +56,6 @@ export function createRumSessionManagerMock(): RumSessionManagerMock { sessionReplay: computeSessionReplayState(trackingType, hasError, forcedReplay), eventsWithheld: computeEventsWithheld(trackingType, hasError, forcedReplay), sampledOnError: withholdsEvents(trackingType), - detailSampledFrom, sampledOnErrorReplay: withholdsReplay(trackingType), anonymousId: 'device-123', } @@ -104,9 +101,5 @@ export function createRumSessionManagerMock(): RumSessionManagerMock { hasError = true return this }, - setSessionDetailSampledFrom(timestamp) { - detailSampledFrom = timestamp - return this - }, } } From 9acdb7a51e48898a0267c42636a7854b747b3aa8 Mon Sep 17 00:00:00 2001 From: Fiona Date: Wed, 2 Sep 2026 05:23:15 -0700 Subject: [PATCH 25/27] docs(rum): record that a withdrawn consent releases what it already earned 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. --- packages/rum-core/src/transport/withheldEventBuffer.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/rum-core/src/transport/withheldEventBuffer.ts b/packages/rum-core/src/transport/withheldEventBuffer.ts index c803a42bd5..8643e59a13 100644 --- a/packages/rum-core/src/transport/withheldEventBuffer.ts +++ b/packages/rum-core/src/transport/withheldEventBuffer.ts @@ -143,6 +143,12 @@ export function startWithheldEventBuffer( * `discardIfUnreleased` says whether the buffer has anything left to wait for. A session that * ended is over, so what it never released goes no further. A page being hidden is not: it comes * back, and dropping the minute it had collected would leave the error that follows with nothing. + * + * A session that had already reported its error is released here rather than dropped, and that + * holds when the session ended because consent was withdrawn: everything held was collected while + * consent stood, and the batch has always flushed what it was holding when a session ends. The + * difference this feature makes is the size of that last flush, up to a minute rather than up to + * a batch. Deliberate, and settled - do not turn it into a discard without saying so out loud. */ function settleBuffer(discardIfUnreleased: boolean) { if (withheldForSessionId === undefined) { From 55e9fece9abe1a48eb67d84e69f285910246d854 Mon Sep 17 00:00:00 2001 From: Fiona Date: Sat, 5 Sep 2026 08:17:21 -0700 Subject: [PATCH 26/27] test(rum): name the session on error specs after the switch --- packages/rum-core/src/domain/rumSessionManager.spec.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/rum-core/src/domain/rumSessionManager.spec.ts b/packages/rum-core/src/domain/rumSessionManager.spec.ts index 1f22b30fe7..30e4047545 100644 --- a/packages/rum-core/src/domain/rumSessionManager.spec.ts +++ b/packages/rum-core/src/domain/rumSessionManager.spec.ts @@ -324,7 +324,7 @@ describe('rum session manager', () => { }) }) - describe('on-error session sampling', () => { + describe('session on error', () => { const ON_ERROR_ONLY = { sessionSampleRate: 0, sessionOnError: true, @@ -332,7 +332,7 @@ describe('rum session manager', () => { sessionReplayOnError: false, } - it('draws the on-error type only when the plain session draw missed', () => { + it('applies the on-error type only when the plain session draw missed', () => { startRumSessionManagerWithDefaults({ configuration: { ...ON_ERROR_ONLY, sessionSampleRate: 100 }, }) From 1ef917a5c3272d46e066700d36704c14df98d2ec Mon Sep 17 00:00:00 2001 From: Fiona Date: Sun, 6 Sep 2026 19:29:58 -0700 Subject: [PATCH 27/27] fix(rum): let a forced session release events it withholds without a replay --- .../rum-core/src/domain/rumSessionManager.spec.ts | 14 ++++++++++++++ packages/rum-core/src/domain/rumSessionManager.ts | 8 +++++--- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/packages/rum-core/src/domain/rumSessionManager.spec.ts b/packages/rum-core/src/domain/rumSessionManager.spec.ts index 858ee8a9f5..a718035de6 100644 --- a/packages/rum-core/src/domain/rumSessionManager.spec.ts +++ b/packages/rum-core/src/domain/rumSessionManager.spec.ts @@ -1421,6 +1421,20 @@ describe('rum session manager', () => { expect(session.sessionReplay).toBe(SessionReplayState.SAMPLED) }) + it('releases a session withholding only its events when the host forces it', () => { + const sessionManager = startRumSessionManagerWithDefaults({ configuration: ON_ERROR_ONLY }) + const sessionId = sessionManager.findTrackedSession()!.id + expect(sessionManager.findTrackedSession()!.eventsWithheld).toBeTrue() + + sessionManager.setForcedSession() + + const session = sessionManager.findTrackedSession()! + // the same session, released, with the replay the host asked for + expect(session.id).toBe(sessionId) + expect(session.eventsWithheld).toBeFalse() + expect(session.sessionReplay).toBe(SessionReplayState.FORCED) + }) + it('releases the events when capture is forced, so the forced replay is not left orphaned', () => { const sessionManager = startRumSessionManagerWithDefaults({ configuration: { ...ON_ERROR_ONLY, sessionReplaySampleRate: 100 }, diff --git a/packages/rum-core/src/domain/rumSessionManager.ts b/packages/rum-core/src/domain/rumSessionManager.ts index 229f623e71..3fe44a9f47 100644 --- a/packages/rum-core/src/domain/rumSessionManager.ts +++ b/packages/rum-core/src/domain/rumSessionManager.ts @@ -370,8 +370,9 @@ export function startRumSessionManager( // A session keeps the decision it was drawn with, so forcing a visitor that was not being // collected means ending their current (empty) session; the next activity draws again with // `forcedSession` set and starts a collected session with replay. A session already collected - // only needs replay forced on, which is the existing forced-replay path - and a session whose - // replay is withheld until it errors is released the same way, since the host asked for it now. + // only needs replay forced on, which is the existing forced-replay path - and a session that + // withholds its events or its replay until it errors is released the same way, since the host + // asked for it now: forcing the replay is what releases the events too. setForcedSession: () => { forcedSession = true const session = sessionManager.findSession() @@ -379,7 +380,8 @@ export function startRumSessionManager( sessionManager.expire() } else if ( session.trackingType === RumTrackingType.TRACKED_WITHOUT_SESSION_REPLAY || - withholdsReplay(session.trackingType) + withholdsReplay(session.trackingType) || + withholdsEvents(session.trackingType) ) { sessionManager.updateSessionState(() => ({ forcedReplay: '1' })) }