diff --git a/CHANGELOG.md b/CHANGELOG.md index 5c63ede6e4..651eef2749 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,19 @@ --- +## Unreleased + +- ✨ Two new init options keep only the sessions that report an error, for customers who want every + error investigated without storing and paying for every session. `sessionOnError` keeps the + events of a session the plain `sessionSampleRate` draw missed: it records from the start, uploads + nothing, and is never stored unless it reports an error — on the first error the withheld history, + up to the last minute of it, is uploaded and collection continues. `sessionReplayOnError` does the + same for the Session Replay of a session the plain `sessionReplaySampleRate` draw missed. Both are + switches, default off, and apply only to what the plain rate did not already draw, so a session is + never counted twice. Both can also be set from the console when `remoteConfigurationEnabled` is on. + View events of such a session carry `sampled_for_error` / `sampled_for_error_replay` so a stored + error session can be told apart from an ordinary one. + ## v0.2.2 - 🐛 The settings cache no longer grows by one entry per release of your site. Entries are keyed by diff --git a/packages/core/src/transport/flushController.ts b/packages/core/src/transport/flushController.ts index d13c672bdc..2d203db939 100644 --- a/packages/core/src/transport/flushController.ts +++ b/packages/core/src/transport/flushController.ts @@ -78,6 +78,7 @@ export function createFlushController({ } return { + flush, flushObservable, get messagesCount() { return currentMessagesCount diff --git a/packages/core/src/transport/startBatchWithReplica.ts b/packages/core/src/transport/startBatchWithReplica.ts index 1009b2dbea..1b0a3ce855 100644 --- a/packages/core/src/transport/startBatchWithReplica.ts +++ b/packages/core/src/transport/startBatchWithReplica.ts @@ -6,6 +6,7 @@ import type { RawError } from '../domain/error/error.types' import type { Encoder } from '../tools/encoder' import { createBatch } from './batch' import { createHttpRequest } from './httpRequest' +import type { FlushReason } from './flushController' import { createFlushController } from './flushController' export interface BatchConfiguration { @@ -45,6 +46,10 @@ export function startBatchWithReplica( } return { + flush: (reason: FlushReason) => { + primaryBatch.flushController.flush(reason) + replicaBatch?.flushController.flush(reason) + }, flushObservable: primaryBatch.flushController.flushObservable, add(message: T, replicated = true) { diff --git a/packages/core/test/emulate/mockFlushController.ts b/packages/core/test/emulate/mockFlushController.ts index c894908d47..60beb27d9e 100644 --- a/packages/core/test/emulate/mockFlushController.ts +++ b/packages/core/test/emulate/mockFlushController.ts @@ -8,7 +8,7 @@ export function createMockFlushController() { let currentMessagesCount = 0 let currentBytesCount = 0 - return { + const controller = { notifyBeforeAddMessage: jasmine .createSpy() .and.callFake((messageBytesCount) => { @@ -33,6 +33,11 @@ export function createMockFlushController() { return currentBytesCount }, flushObservable, + flush(reason: FlushReason) { + if (currentMessagesCount > 0) { + controller.notifyFlush(reason) + } + }, notifyFlush(reason: FlushReason = 'bytes_limit') { if (currentMessagesCount === 0) { throw new Error( @@ -53,4 +58,5 @@ export function createMockFlushController() { }) }, } satisfies Record & FlushController + return controller } diff --git a/packages/rum-core/src/boot/startRum.ts b/packages/rum-core/src/boot/startRum.ts index f874605b70..11cb19a730 100644 --- a/packages/rum-core/src/boot/startRum.ts +++ b/packages/rum-core/src/boot/startRum.ts @@ -104,6 +104,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) }) @@ -122,6 +127,9 @@ export function startRum( : startRumSessionManager(configuration, lifeCycle, trackingConsentState) cleanupTasks.push(session.stop) + // 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()) @@ -139,7 +147,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 20f89eb359..8dc6853653 100644 --- a/packages/rum-core/src/domain/configuration/configuration.spec.ts +++ b/packages/rum-core/src/domain/configuration/configuration.spec.ts @@ -140,6 +140,109 @@ describe('validateAndBuildRumConfiguration', () => { }) }) + describe('sessionOnError', () => { + it('is carried into the built configuration', () => { + expect( + validateAndBuildRumConfiguration({ ...DEFAULT_INIT_CONFIGURATION, sessionOnError: true })!.sessionOnError + ).toBeTrue() + }) + + it('defaults to collecting no error-only session at all', () => { + expect(validateAndBuildRumConfiguration(DEFAULT_INIT_CONFIGURATION)!.sessionOnError).toBeFalse() + }) + + it('is read as a switch, whatever it was given', () => { + expect( + validateAndBuildRumConfiguration({ + ...DEFAULT_INIT_CONFIGURATION, + sessionOnError: 'yes' as unknown as boolean, + })!.sessionOnError + ).toBeTrue() + }) + + it('warns when the replay it would withhold is never recorded', () => { + validateAndBuildRumConfiguration({ + ...DEFAULT_INIT_CONFIGURATION, + sessionSampleRate: 20, + sessionOnError: true, + sessionReplaySampleRate: 50, + startSessionReplayRecordingManually: true, + }) + + expect(displayWarnSpy).toHaveBeenCalledTimes(1) + expect(displayWarnSpy.calls.argsFor(0)[0]).toContain('startSessionReplayRecordingManually') + }) + + it('says nothing about a replay it could never withhold anyway', () => { + validateAndBuildRumConfiguration({ + ...DEFAULT_INIT_CONFIGURATION, + sessionOnError: true, + sessionReplaySampleRate: 30, + startSessionReplayRecordingManually: true, + }) + + // the switch cannot apply 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('does not warn about manual recording when replay is disabled for the on-error session', () => { + // there is nothing to withhold on the replay side, so the manual-start warning does not apply - + // even though the plain session rate leaves room for the switch and recording is manual + validateAndBuildRumConfiguration({ + ...DEFAULT_INIT_CONFIGURATION, + sessionSampleRate: 20, + sessionOnError: true, + sessionReplaySampleRate: 0, + startSessionReplayRecordingManually: true, + }) + + expect(displayWarnSpy).not.toHaveBeenCalled() + }) + + it('warns when the default session rate leaves it nothing to apply to', () => { + validateAndBuildRumConfiguration({ + ...DEFAULT_INIT_CONFIGURATION, + sessionOnError: true, + }) + + expect(displayWarnSpy).toHaveBeenCalledTimes(1) + }) + + it('stays silent under remote configuration, where the console owns the session rate', () => { + // the documented remote-config setup: the site omits the rate and lets the console deliver it, + // so the init default of 100 is a fallback, not the rate the switch will face + validateAndBuildRumConfiguration({ + ...DEFAULT_INIT_CONFIGURATION, + sessionOnError: true, + remoteConfigurationEnabled: true, + }) + + expect(displayWarnSpy).not.toHaveBeenCalled() + }) + + it('says nothing once the plain session rate leaves room for it', () => { + validateAndBuildRumConfiguration({ + ...DEFAULT_INIT_CONFIGURATION, + sessionSampleRate: 20, + sessionOnError: true, + }) + + expect(displayWarnSpy).not.toHaveBeenCalled() + }) + + it('makes the replay-on-error switch meaningful even with no plainly sampled session', () => { + validateAndBuildRumConfiguration({ + ...DEFAULT_INIT_CONFIGURATION, + sessionSampleRate: 0, + sessionOnError: true, + sessionReplayOnError: true, + }) + + expect(displayWarnSpy).not.toHaveBeenCalled() + }) + }) + describe('traceSampleRate', () => { it('defaults to 100 if the option is not provided', () => { expect(validateAndBuildRumConfiguration(DEFAULT_INIT_CONFIGURATION)!.traceSampleRate).toBe(100) @@ -650,6 +753,7 @@ describe('serializeRumConfiguration', () => { subdomain: 'foo', sessionReplaySampleRate: 60, sessionReplayOnError: true, + sessionOnError: true, startSessionReplayRecordingManually: true, sessionReplayDirectUpload: true, trackUserInteractions: true, @@ -685,6 +789,7 @@ describe('serializeRumConfiguration', () => { | 'beforeSampling' // not reported yet: needs a rum-events-format schema change first | 'sessionReplayOnError' + | 'sessionOnError' ? 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 c003eec4ac..c1caad3070 100644 --- a/packages/rum-core/src/domain/configuration/configuration.ts +++ b/packages/rum-core/src/domain/configuration/configuration.ts @@ -209,6 +209,19 @@ export interface RumInitConfiguration extends InitConfiguration { * the withheld minute is uploaded and recording continues normally for the rest of the session. */ sessionReplayOnError?: boolean | undefined + /** + * Whether the sessions that `sessionSampleRate` did not draw still collect events, uploaded only + * if the session reports an error. Default: false. It only applies to what the plain rate missed, + * so with the default `sessionSampleRate` of 100 there is nothing left for it to apply to. + * + * 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 kept 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. + */ + sessionOnError?: boolean | 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. * @@ -317,6 +330,7 @@ export interface RumConfiguration extends Configuration { enablePrivacyForActionName: boolean sessionReplaySampleRate: number sessionReplayOnError: boolean + sessionOnError: boolean startSessionReplayRecordingManually: boolean sessionReplayDirectUpload: boolean trackUserInteractions: boolean @@ -402,32 +416,57 @@ export function validateAndBuildRumConfiguration( const sessionReplaySampleRate = initConfiguration.sessionReplaySampleRate ?? 0 const sessionReplayOnError = !!initConfiguration.sessionReplayOnError + const sessionOnError = !!initConfiguration.sessionOnError - // Each of these is a combination the customer can set that cannot apply to a single session. It - // is valid, so validation lets it through - but silence would leave them waiting for data that is - // never coming. - if (sessionReplayOnError) { - if (sessionReplaySampleRate === 100) { + // Each of the cases below is a combination the customer can set that cannot apply to a single + // session. It is valid, so validation lets it through - but silence would leave someone waiting + // for data that is never coming. + // + // Only judged against the init rates when the console cannot change them: under remote + // configuration these values are a fallback until the first fetch lands, so the console may + // deliver the very rate that leaves the switch room to apply. Warning on the init values there + // would fire on the documented remote-config setup - a site that omits the rate and lets the + // console own it - which is exactly not a misconfiguration. + if (!initConfiguration.remoteConfigurationEnabled) { + if (sessionOnError && (initConfiguration.sessionSampleRate ?? 100) === 100) { display.warn( - 'sessionReplayOnError only applies to sessions sessionReplaySampleRate did not draw, and that rate is 100: it will never apply.' + 'sessionOnError only applies to sessions sessionSampleRate did not draw, and that rate is 100: it will never apply.' ) } - if ((initConfiguration.sessionSampleRate ?? 100) === 0) { - display.warn('sessionReplayOnError has no effect while sessionSampleRate is 0: no session is tracked.') - } - if (initConfiguration.startSessionReplayRecordingManually) { - display.warn( - 'sessionReplayOnError 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.' - ) + if (sessionReplayOnError) { + if (sessionReplaySampleRate === 100) { + display.warn( + 'sessionReplayOnError only applies to sessions sessionReplaySampleRate did not draw, and that rate is 100: it will never apply.' + ) + } + if ((initConfiguration.sessionSampleRate ?? 100) === 0 && !sessionOnError) { + display.warn( + 'sessionReplayOnError has no effect while sessionSampleRate is 0 and sessionOnError is off: no session is tracked.' + ) + } } } + // A session kept 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 && + (sessionReplayOnError || + (sessionOnError && 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.' + ) + } + return { applicationId: initConfiguration.applicationId, version: initConfiguration.version || undefined, actionNameAttribute: initConfiguration.actionNameAttribute, sessionReplaySampleRate, sessionReplayOnError, + sessionOnError, startSessionReplayRecordingManually: initConfiguration.startSessionReplayRecordingManually !== undefined ? !!initConfiguration.startSessionReplayRecordingManually @@ -525,7 +564,7 @@ export function serializeRumConfiguration(configuration: RumInitConfiguration) { return { session_replay_sample_rate: configuration.sessionReplaySampleRate, - // `session_replay_on_error` is deliberately not reported yet: the telemetry + // `session_replay_on_error` and `session_on_error` are deliberately not reported yet: the telemetry // configuration type is generated from the rum-events-format schema, so adding it needs a schema // change first, and that is a separate repository. start_session_replay_recording_manually: configuration.startSessionReplayRecordingManually, diff --git a/packages/rum-core/src/domain/configuration/remoteConfiguration.spec.ts b/packages/rum-core/src/domain/configuration/remoteConfiguration.spec.ts index 1bc0b3fd24..df490e069a 100644 --- a/packages/rum-core/src/domain/configuration/remoteConfiguration.spec.ts +++ b/packages/rum-core/src/domain/configuration/remoteConfiguration.spec.ts @@ -121,13 +121,17 @@ describe('remoteConfiguration', () => { start(configurationWith()) }) - it('keeps the replay-on-error switch the server reports, either way it is set', (done) => { + it('keeps the on-error switches the server reports, either way they are set', (done) => { interceptor.withMockXhr((xhr) => { - xhr.complete(200, body({ rum: { sessionReplaySampleRate: 10, sessionReplayOnError: false } })) + xhr.complete( + 200, + body({ rum: { sessionReplaySampleRate: 10, sessionReplayOnError: false, sessionOnError: true } }) + ) expect(readRemoteConfig(setup)).toEqual({ sessionReplaySampleRate: 10, sessionReplayOnError: false, + sessionOnError: true, version: 3, }) done() diff --git a/packages/rum-core/src/domain/configuration/remoteConfiguration.ts b/packages/rum-core/src/domain/configuration/remoteConfiguration.ts index 24be706f18..84b3cc34a1 100644 --- a/packages/rum-core/src/domain/configuration/remoteConfiguration.ts +++ b/packages/rum-core/src/domain/configuration/remoteConfiguration.ts @@ -108,6 +108,11 @@ export interface RemoteConfigValues { * either withholds its replay from the start or never does. */ sessionReplayOnError?: boolean + /** + * Whether the sessions `sessionSampleRate` did not draw still collect, uploaded only if the + * session errors. Same footing as the replay switch above. + */ + sessionOnError?: boolean /** * Which version of the settings these rates came from. Reported back on the next request so the * console can say how far a change has actually reached — a question the events cannot answer, @@ -270,6 +275,9 @@ function readStoredValues(parsed: unknown): RemoteConfigValues { if (isSwitch(stored.sessionReplayOnError)) { values.sessionReplayOnError = stored.sessionReplayOnError } + if (isSwitch(stored.sessionOnError)) { + values.sessionOnError = stored.sessionOnError + } if (isBag(stored.custom)) { values.custom = stored.custom } @@ -498,6 +506,9 @@ function store(setup: RemoteConfigSetup, response: RemoteConfigurationResponse) if (isSwitch(response.rum.sessionReplayOnError)) { values.sessionReplayOnError = response.rum.sessionReplayOnError } + if (isSwitch(response.rum.sessionOnError)) { + values.sessionOnError = response.rum.sessionOnError + } } // The custom bag rides along untouched — the platform's job is delivery, its meaning belongs to // the host application. Gone from the response (or the kill switch off) means gone from storage. diff --git a/packages/rum-core/src/domain/contexts/sessionContext.spec.ts b/packages/rum-core/src/domain/contexts/sessionContext.spec.ts index 2d73f0d58f..1a2dc35263 100644 --- a/packages/rum-core/src/domain/contexts/sessionContext.spec.ts +++ b/packages/rum-core/src/domain/contexts/sessionContext.spec.ts @@ -112,6 +112,20 @@ describe('session context', () => { expect(plainEvent.session!.sampled_for_error_replay).toBeUndefined() }) + it('does not report sampled_for_replay for an error-replay session that has not errored', () => { + // a type-3 session withholds only its replay, not its events; its events ship on their own, so + // reporting sampled_for_replay before the error would claim a replay for a recording that may + // never be sent + sessionManager.setTrackedWithErrorSessionReplay() + + const event = hooks.triggerHook(HookNames.Assemble, { + eventType: 'view', + startTime: 0 as RelativeTime, + }) as DefaultRumEventAttributes + + expect(event.session!.sampled_for_replay).toBe(false) + }) + it('should not set hasReplay when a dropped buffer left the view with nothing', () => { // a withheld buffer that was dropped rolls back what it held, and a view left with an empty // stats entry has no replay to offer @@ -170,6 +184,70 @@ 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.setTrackedOnErrorWithSessionReplay() + + const event = hooks.triggerHook(HookNames.Assemble, { + eventType: 'view', + startTime: 0 as RelativeTime, + }) as DefaultRumEventAttributes + + expect(event.session!.sampled_for_replay).toBe(true) + }) + + 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 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(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', () => { + 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 report the configuration the session was drawn under', () => { sessionManager.setDrawnConfiguration({ version: 12, diff --git a/packages/rum-core/src/domain/contexts/sessionContext.ts b/packages/rum-core/src/domain/contexts/sessionContext.ts index b928ac3acd..30c63198f9 100644 --- a/packages/rum-core/src/domain/contexts/sessionContext.ts +++ b/packages/rum-core/src/domain/contexts/sessionContext.ts @@ -39,11 +39,16 @@ 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. 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 let hasReplay let sampledForReplay + let sampledForError let sampledForErrorReplay let isActive if (eventType === RumEventType.VIEW) { @@ -53,7 +58,14 @@ export function startSessionContext( // because a host bridge takes the records itself and no segment is ever built for them. const replayStats = recorderApi.getReplayStats(view.id) hasReplay = !isReplayWithheld && replayStats && replayStats.records_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 || (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 // 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 @@ -62,14 +74,27 @@ export function startSessionContext( hasReplay = !isReplayWithheld && recorderApi.isRecording() ? true : undefined } + // These three are fork additions the generated event schema does not declare, so on the session + // object below they would only be checked against its `[k: string]: unknown` index signature - a + // typo in a name would compile and silently emit a field the backend never reads. Typing them + // here makes an excess or misspelled key fail the build instead. + const forkMarkers: { + sampled_for_replay: boolean | undefined + sampled_for_error: boolean | undefined + sampled_for_error_replay: boolean | undefined + } = { + sampled_for_replay: sampledForReplay, + sampled_for_error: sampledForError, + sampled_for_error_replay: sampledForErrorReplay, + } + return { type: eventType, session: { id: session.id, type: SessionType.USER, has_replay: hasReplay, - sampled_for_replay: sampledForReplay, - sampled_for_error_replay: sampledForErrorReplay, + ...forkMarkers, is_active: isActive, }, // FLASHCAT FORK - overrides the init values reported by the default context with the rates diff --git a/packages/rum-core/src/domain/rumSessionManager.spec.ts b/packages/rum-core/src/domain/rumSessionManager.spec.ts index 0fd2bf83f0..e5547e98a7 100644 --- a/packages/rum-core/src/domain/rumSessionManager.spec.ts +++ b/packages/rum-core/src/domain/rumSessionManager.spec.ts @@ -230,6 +230,7 @@ describe('rum session manager', () => { traceSampleRate?: number defaultPrivacyLevel?: string sessionReplayOnError?: boolean + sessionOnError?: boolean }) { localStorage.setItem(STORE_KEY, JSON.stringify(values)) registerCleanupTask(() => localStorage.removeItem(STORE_KEY)) @@ -275,6 +276,24 @@ describe('rum session manager', () => { ) }) + it('keeps a session on error when the console says so, over what init said', () => { + storeRemoteConfigValues({ sessionSampleRate: 0, sessionOnError: true }) + + startRumSessionManagerWithDefaults({ + configuration: { + sessionSampleRate: 100, + sessionReplaySampleRate: 0, + sessionOnError: false, + remoteConfig: REMOTE_SAMPLING_SETUP, + }, + }) + document.dispatchEvent(createNewEvent(DOM_EVENT.CLICK)) + + expect(getSessionState(SESSION_STORE_KEY)[RUM_SESSION_KEY]).toBe( + RumTrackingType.TRACKED_ON_ERROR_WITHOUT_SESSION_REPLAY + ) + }) + it('turns the replay-on-error switch off when the console says so', () => { storeRemoteConfigValues({ sessionReplayOnError: false }) @@ -291,6 +310,22 @@ describe('rum session manager', () => { expect(getSessionState(SESSION_STORE_KEY)[RUM_SESSION_KEY]).toBe(RumTrackingType.TRACKED_WITHOUT_SESSION_REPLAY) }) + it('turns the session-on-error switch off when the console says so', () => { + storeRemoteConfigValues({ sessionOnError: false }) + + startRumSessionManagerWithDefaults({ + configuration: { + sessionSampleRate: 0, + sessionOnError: true, + remoteConfig: REMOTE_SAMPLING_SETUP, + }, + }) + document.dispatchEvent(createNewEvent(DOM_EVENT.CLICK)) + + // a delivered false must win over init's true, so nothing is collected - not fall back to it + expect(getSessionState(SESSION_STORE_KEY)[RUM_SESSION_KEY]).toBe(RumTrackingType.NOT_TRACKED) + }) + it('falls back to the rate passed to init for a knob the console did not set', () => { storeRemoteConfigValues({ sessionReplaySampleRate: 100 }) @@ -521,6 +556,29 @@ describe('rum session manager', () => { }) }) + it('reports a zero session sample rate for a session kept only because it errors', () => { + // 99 is above any rate below 100, so the plain draw misses and the switch keeps the session + spyOn(Math, 'random').and.returnValue(0.99) + storeRemote({ version: 7, sessionSampleRate: 50, sessionReplaySampleRate: 0 }) + + const rumSessionManager = startRumSessionManagerWithDefaults({ + configuration: { + sessionSampleRate: 50, + sessionOnError: true, + remoteConfig: REMOTE_SAMPLING_SETUP, + drawStoreKey: DRAW_KEY, + }, + }) + document.dispatchEvent(createNewEvent(DOM_EVENT.CLICK)) + + expect(getSessionState(SESSION_STORE_KEY)[RUM_SESSION_KEY]).toBe( + RumTrackingType.TRACKED_ON_ERROR_WITHOUT_SESSION_REPLAY + ) + // It was kept by the switch, not by the 50% draw it missed, so it stands for one session, not + // 100/50. Reporting the plain rate would have the adoption panel count it as two. + expect(rumSessionManager.findTrackedSession()!.drawnConfiguration!.sessionSampleRate).toBe(0) + }) + it('reports the rate beforeSampling decided, not the delivered one', () => { storeRemote({ version: 3, sessionSampleRate: 0, sessionReplaySampleRate: 0 }) @@ -971,6 +1029,32 @@ describe('rum session manager', () => { }) }) + describe('a session the on-error switch keeps', () => { + it('does not end an on-error session when the rate is zero, because the switch still collects it', () => { + // The switch's own documented shape: the plain rate misses every session, `sessionOnError` + // keeps the ones that error. A zero rate here is that setting, not a stop - ending the + // session would discard exactly what the switch exists to keep, and blind the page from this + // fetch (which lands on every fresh profile and after every deploy) until the first click. + startWith({ sessionSampleRate: 0, sessionOnError: true }) + expect(getSessionState(SESSION_STORE_KEY)[RUM_SESSION_KEY]).not.toBe(RumTrackingType.NOT_TRACKED) + + deliver({ version: 1, sessionSampleRate: 0 }) + + expect(isSessionEnded()).toBeFalse() + }) + + it('still ends the session when the console turns the switch off at a zero rate', () => { + // The emergency stop is preserved: a rate of zero with the switch explicitly off collects + // nothing, so the running session is decided against and ended. + startWith({ sessionSampleRate: 0, sessionOnError: true }) + expect(getSessionState(SESSION_STORE_KEY)[RUM_SESSION_KEY]).not.toBe(RumTrackingType.NOT_TRACKED) + + deliver({ version: 1, sessionSampleRate: 0, sessionOnError: false }) + + expect(isSessionEnded()).toBeTrue() + }) + }) + describe('everything else waits for the next session', () => { it('leaves the session alone when the rate moves to a value it cannot decide on', () => { storeRemote({ version: 1, sessionSampleRate: 100 }) @@ -1419,6 +1503,132 @@ describe('rum session manager', () => { }) }) + describe('session on error', () => { + const ON_ERROR_ONLY = { + sessionSampleRate: 0, + sessionOnError: true, + sessionReplaySampleRate: 0, + sessionReplayOnError: false, + } + + it('applies 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(sessionManager.findTrackedSession()!.id) + + 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(sessionManager.findTrackedSession()!.id) + + const session = sessionManager.findTrackedSession()! + expect(session.eventsWithheld).toBeFalse() + 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 }, + }) + + sessionManager.setForcedReplay() + + const session = sessionManager.findTrackedSession()! + expect(session.eventsWithheld).toBeFalse() + expect(session.sessionReplay).toBe(SessionReplayState.FORCED) + }) + + it('draws the type that withholds the replay too when only the on-error replay switch is on', () => { + const sessionManager = startRumSessionManagerWithDefaults({ + configuration: { ...ON_ERROR_ONLY, sessionReplayOnError: true }, + }) + + 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&created=${Date.now()}&expire=${Date.now() + DURATION}`, 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&created=${Date.now()}&expire=${Date.now() + DURATION}`, + 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) + + expect(sessionManager.findTrackedSession()!.sampledOnError).toBeTrue() + }) + }) + function startRumSessionManagerWithDefaults({ configuration, trackingConsentState = createTrackingConsentState(TrackingConsent.GRANTED), diff --git a/packages/rum-core/src/domain/rumSessionManager.ts b/packages/rum-core/src/domain/rumSessionManager.ts index 898b4ac6c7..46a3756eb9 100644 --- a/packages/rum-core/src/domain/rumSessionManager.ts +++ b/packages/rum-core/src/domain/rumSessionManager.ts @@ -37,9 +37,10 @@ export interface RumSessionManager { setForcedReplay: () => void setForcedSession: () => void /** - * Marks the given session as having reported an error. For a session sampled by - * `sessionReplayOnError`, this is what releases the withheld replay. The id is required - * because the store write can be deferred by the lock, and it must not land on a later session. + * Marks the given session as having reported an error. This is what releases what an on-error + * session withheld: the replay for a `sessionReplayOnError` session, and the withheld events for a + * `sessionOnError` one. The id is required because the store write can be deferred by the lock, and + * it must not land on a later session. */ setSessionHasError: (sessionId: string) => void } @@ -87,9 +88,19 @@ export type RumSession = { id: string sessionReplay: SessionReplayState /** - * Whether the replay of this session is only kept if it reports an error. Unlike - * {@link sessionReplay} this stays true once the error has been reported, so a replay collected - * that way can be told apart from one collected unconditionally. + * 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 is only kept because of `sessionOnError`. 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 + /** + * Whether the replay of this session is only kept if it reports an error. Same idea as + * {@link sampledOnError}, for the replay rather than the events. */ sampledOnErrorReplay: boolean anonymousId?: string @@ -104,6 +115,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 { @@ -302,8 +315,15 @@ export function startRumSessionManager( return } - const { sessionSampleRate } = resolveSampleRates(configuration, remote) - if (sessionSampleRate === 0) { + // FLASHCAT FORK - a rate of zero ends a running session only when nothing else would keep it. + // `sessionOnError` collects exactly the sessions the plain rate misses, so a zero rate next to + // it is the switch's ordinary setting, not a stop: at a zero rate a session is tracked if and + // only if the switch is on (a replay-on-error switch cannot keep one on its own, since the + // session draw fails first). Ending it here would discard the very session the switch exists to + // keep, and leave the page blind from this fetch until the visitor's first interaction - which + // is what a fresh profile and every deploy would hit on their first configuration fetch. + const { sessionSampleRate, sessionOnError } = resolveSampleRates(configuration, remote) + if (sessionSampleRate === 0 && !sessionOnError) { sessionManager.expire() } } @@ -363,6 +383,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), sampledOnErrorReplay: withholdsReplay(session.trackingType), anonymousId: session.anonymousId, // FLASHCAT FORK - looked up at the same time as the session itself, so an event that @@ -384,8 +406,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() @@ -393,7 +416,8 @@ export function startRumSessionManager( sessionManager.expire() } else if ( session.trackingType === RumTrackingType.TRACKED_WITHOUT_SESSION_REPLAY || - withholdsReplay(session.trackingType) + withholdsReplay(session.trackingType) || + withholdsEvents(session.trackingType) ) { forceReplay() } @@ -417,7 +441,17 @@ export function startRumSessionManager( } export function withholdsReplay(trackingType: RumTrackingType) { - return trackingType === RumTrackingType.TRACKED_WITH_ERROR_SESSION_REPLAY + 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( @@ -442,6 +476,19 @@ export function computeSessionReplayState( 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) +} + /** * Session id used when the host application does not answer for one, because it was built against * an SDK that predates `getSessionId()`. Such a host is expected to override the session id of the @@ -534,6 +581,8 @@ export function startRumSessionManagerStub( id: sessionId ?? STUB_SESSION_ID, sessionReplay, // The host records for us, or this page uploads what the plain rate drew: neither withholds. + eventsWithheld: false, + sampledOnError: false, sampledOnErrorReplay: false, anonymousId: bridge?.getAnonymousId(), } @@ -573,23 +622,41 @@ function computeSessionState( // the decision it was created with: settings arriving mid-session never start or stop // collecting for a visitor already on the site. const remote = readRemoteConfig(configuration.remoteConfig) - const { sessionSampleRate, sessionReplaySampleRate, sessionReplayOnError } = resolveSampleRates( + const { sessionSampleRate, sessionReplaySampleRate, sessionOnError, sessionReplayOnError } = resolveSampleRates( configuration, remote ) - reportDraw(configuration, remote, sessionSampleRate, sessionReplaySampleRate, onDraw) - - if (!performDraw(sessionSampleRate)) { - trackingType = RumTrackingType.NOT_TRACKED - } else if (performDraw(sessionReplaySampleRate)) { - trackingType = RumTrackingType.TRACKED_WITH_SESSION_REPLAY - } else if (sessionReplayOnError) { - // Only for sessions the plain replay draw missed, so a session is never counted by both. - trackingType = RumTrackingType.TRACKED_WITH_ERROR_SESSION_REPLAY + if (performDraw(sessionSampleRate)) { + if (performDraw(sessionReplaySampleRate)) { + trackingType = RumTrackingType.TRACKED_WITH_SESSION_REPLAY + } else if (sessionReplayOnError) { + // Only for sessions the plain replay draw missed, so a session is never counted by both. + trackingType = RumTrackingType.TRACKED_WITH_ERROR_SESSION_REPLAY + } else { + trackingType = RumTrackingType.TRACKED_WITHOUT_SESSION_REPLAY + } + } else if (sessionOnError) { + // Only for sessions the plain session draw missed, so a session is never counted by both. + // Such a session never uploads its replay ahead of its events: whichever replay 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(sessionReplaySampleRate) || sessionReplayOnError + ? 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 } + + // Reported after the ladder, not before, so an on-error session can report the rate the backend + // should extrapolate from. Such a session was kept despite the plain draw missing it, so it + // stands for itself, not for `100 / rate` like a plainly sampled one - reporting the plain rate + // would have the console's adoption panel count each error session as `100 / rate` sessions. A + // rate of 0 there is read as "one session, do not scale". A session merely withholding its + // replay (type '3') was still drawn by the plain rate and reports it unchanged. + const reportedSampleRate = withholdsEvents(trackingType) ? 0 : sessionSampleRate + reportDraw(configuration, remote, reportedSampleRate, sessionReplaySampleRate, onDraw) } return { trackingType, @@ -598,7 +665,7 @@ function computeSessionState( } /** - * FLASHCAT FORK - the rates a draw would use right now, and the on-error switch beside them: what + * FLASHCAT FORK - the rates a draw would use right now, and the on-error switches beside them: what * the console delivered, falling back to what the site passed to init, with the application's * `beforeSampling` given the last word on the rates (the switch is not offered to it: it is a * yes or a no the console already answered). This @@ -638,6 +705,7 @@ function resolveSampleRates(configuration: RumConfiguration, remote: RemoteConfi return { sessionSampleRate, sessionReplaySampleRate, + sessionOnError: remote.sessionOnError ?? configuration.sessionOnError, sessionReplayOnError: remote.sessionReplayOnError ?? configuration.sessionReplayOnError, } } @@ -784,7 +852,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 ) } @@ -792,6 +862,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/domain/trackSessionError.spec.ts b/packages/rum-core/src/domain/trackSessionError.spec.ts index a0057d01f7..3b72e12d84 100644 --- a/packages/rum-core/src/domain/trackSessionError.spec.ts +++ b/packages/rum-core/src/domain/trackSessionError.spec.ts @@ -59,6 +59,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 4f8669d7b9..c431eb9a1d 100644 --- a/packages/rum-core/src/domain/trackSessionError.ts +++ b/packages/rum-core/src/domain/trackSessionError.ts @@ -5,8 +5,8 @@ import { LifeCycleEventType } from './lifeCycle' import type { RumSessionManager } from './rumSessionManager' /** - * Marks the session as having reported an error, which is what releases a replay withheld by - * `sessionReplayOnError`. + * Marks the session as having reported an error, which is what releases what an on-error session + * withheld: a replay withheld by `sessionReplayOnError`, and the events withheld by `sessionOnError`. * * It listens after assembly rather than on the raw error, so an error discarded by `beforeSend` or * by a rate limiter does not release anything: a session billed for an error that cannot be found @@ -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 || event.session?.id !== session.id) { + if (!session || event.session?.id !== session.id || (!session.sampledOnError && !session.sampledOnErrorReplay)) { return } hasReportedError = true diff --git a/packages/rum-core/src/index.ts b/packages/rum-core/src/index.ts index 5aeb6bb7a0..3d23ab451c 100644 --- a/packages/rum-core/src/index.ts +++ b/packages/rum-core/src/index.ts @@ -53,3 +53,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/startRumBatch.spec.ts b/packages/rum-core/src/transport/startRumBatch.spec.ts new file mode 100644 index 0000000000..33c671f783 --- /dev/null +++ b/packages/rum-core/src/transport/startRumBatch.spec.ts @@ -0,0 +1,111 @@ +import { + SESSION_STORE_KEY, + STORAGE_POLL_DELAY, + setCookie, + createTrackingConsentState, + TrackingConsent, + stopSessionManager, + Observable, + createIdentityEncoder, + noop, +} from '@flashcatcloud/browser-core' +import { getSessionState, interceptRequests, mockClock, registerCleanupTask } from '@flashcatcloud/browser-core/test' +import { createRumSessionManagerMock, mockRumConfiguration } from '../../test' +import { LifeCycle, LifeCycleEventType } from '../domain/lifeCycle' +import { startSessionErrorTracking } from '../domain/trackSessionError' +import { startRumSessionManager } from '../domain/rumSessionManager' +import type { RumEvent } from '../rumEvent.types' +import { startRumBatch } from './startRumBatch' + +describe('withheld events through the real batch', () => { + for (const released of [true, false]) { + it(`observes a shared cookie release without a new RUM event (released=${released})`, () => { + const clock = mockClock() + const lifeCycle = new LifeCycle() + const configuration = mockRumConfiguration({ sessionSampleRate: 0, sessionOnError: true }) + const session = startRumSessionManager( + configuration, + lifeCycle, + createTrackingConsentState(TrackingConsent.GRANTED) + ) + const requests = interceptRequests() + const batch = startRumBatch( + configuration, + lifeCycle, + new Observable(), + noop, + new Observable(), + session, + createIdentityEncoder + ) + registerCleanupTask(() => { + batch.stop() + session.stop() + stopSessionManager() + clock.cleanup() + }) + lifeCycle.notify(LifeCycleEventType.RUM_EVENT_COLLECTED, { + type: 'view', + date: 1, + session: { id: session.findTrackedSession()!.id }, + view: { id: 'view-id' }, + } as any) + if (released) { + setCookie( + SESSION_STORE_KEY, + Object.entries({ ...getSessionState(SESSION_STORE_KEY), hasError: '1' }) + .map(([key, value]) => `${key}=${value}`) + .join('&'), + 60000 + ) + } + clock.tick(STORAGE_POLL_DELAY + 3001) + batch.flush('session_expire') + const events = requests.requests.flatMap((request) => + request.body.split('\n').map((line) => JSON.parse(line) as RumEvent) + ) + expect(events.map((event) => event.type)).toEqual(released ? ['view'] : []) + }) + } + + for (const error of [true, false]) { + it(`drains only released events when stopping (error=${error})`, () => { + const clock = mockClock() + const lifeCycle = new LifeCycle() + const session = createRumSessionManagerMock().setTrackedOnError() + const requests = interceptRequests() + const tracker = startSessionErrorTracking(lifeCycle, session) + const batch = startRumBatch( + mockRumConfiguration(), + lifeCycle, + new Observable(), + noop, + new Observable(), + session, + createIdentityEncoder + ) + registerCleanupTask(() => { + tracker.stop() + batch.stop() + clock.cleanup() + }) + const emit = (type: string) => + lifeCycle.notify(LifeCycleEventType.RUM_EVENT_COLLECTED, { + type, + date: 1, + session: { id: 'session-id' }, + view: { id: 'view-id' }, + error: { source: 'custom' }, + } as any) + emit('view') + if (error) { + emit('error') + } + batch.stop() + const events = requests.requests.flatMap((request) => + request.body.split('\n').map((line) => JSON.parse(line) as RumEvent) + ) + expect(events.map((event) => event.type).sort()).toEqual(error ? ['error', 'view'] : []) + }) + } +}) diff --git a/packages/rum-core/src/transport/startRumBatch.ts b/packages/rum-core/src/transport/startRumBatch.ts index 34e62f83d0..c2ab233233 100644 --- a/packages/rum-core/src/transport/startRumBatch.ts +++ b/packages/rum-core/src/transport/startRumBatch.ts @@ -14,9 +14,9 @@ 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, @@ -24,7 +24,7 @@ export function startRumBatch( telemetryEventObservable: Observable, reportError: (error: RawError) => void, pageMayExitObservable: Observable, - sessionExpireObservable: Observable, + sessionManager: RumSessionManager, createEncoder: (streamId: DeflateEncoderStreamId) => Encoder ) { const replica = configuration.replica @@ -42,10 +42,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. + const withheldEventBuffer = startWithheldEventBuffer(lifeCycle, sessionManager, (serverRumEvent) => { if (serverRumEvent.type === RumEventType.VIEW) { batch.upsert(serverRumEvent, serverRumEvent.view.id) } else { @@ -55,5 +57,13 @@ export function startRumBatch( telemetryEventObservable.subscribe((event) => batch.add(event, isTelemetryReplicationAllowed(configuration))) - return batch + return { + ...batch, + stop: () => { + // Drain released history while the batch is still listening, then flush its final messages. + withheldEventBuffer.stop() + batch.flush('session_expire') + batch.stop() + }, + } } 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..d64cf4dd16 --- /dev/null +++ b/packages/rum-core/src/transport/withheldEventBuffer.spec.ts @@ -0,0 +1,656 @@ +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_BYTES_LIMIT, + WITHHELD_BUFFER_DURATION, + WITHHELD_BUFFER_EVENTS_LIMIT, + WITHHELD_BUFFER_VIEWS_LIMIT, + WITHHELD_BUFFER_RELEASE_MAX_DELAY, + computeReleaseDelay, + startWithheldEventBuffer, +} from './withheldEventBuffer' + +describe('startWithheldEventBuffer', () => { + let clock: Clock + let lifeCycle: LifeCycle + let sessionManager: ReturnType + let forwarded: Array + let stopBuffer: () => void + + 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)) + stopBuffer = stop + registerCleanupTask(() => { + stop() + clock.cleanup() + }) + }) + + it('releases immediately when the current session is forced', () => { + collect(RumEventType.VIEW) + collect(RumEventType.RESOURCE) + sessionManager.setForcedReplay() + lifeCycle.notify(LifeCycleEventType.SESSION_RELEASED, { sessionId: 'session-id', reason: 'force' }) + expect(forwarded.length).toBe(2) + }) + + it('schedules a release learned from another tab without requiring a new event', () => { + collect(RumEventType.VIEW) + sessionManager.setSessionHasError() + lifeCycle.notify(LifeCycleEventType.SESSION_RELEASED, { sessionId: 'session-id', reason: 'error' }) + expect(forwarded.length).toBe(0) + expect(releasedAfterJitter().length).toBe(1) + }) + + it('ignores a release notification for another session', () => { + collect(RumEventType.VIEW) + lifeCycle.notify(LifeCycleEventType.SESSION_RELEASED, { sessionId: 'other-session', reason: 'force' }) + expect(releasedAfterJitter().length).toBe(0) + }) + + it('settles an errored buffer before stopping and does not forward again', () => { + collect(RumEventType.VIEW) + sessionManager.setSessionHasError() + collect(RumEventType.ERROR) + stopBuffer() + expect(forwarded.length).toBe(2) + stopBuffer() + expect(releasedAfterJitter().length).toBe(2) + }) + + it('forwards immediately when the session is not withholding', () => { + sessionManager.setTrackedWithSessionReplay() + + collect(RumEventType.VIEW) + collect(RumEventType.RESOURCE) + + expect(forwarded.length).toBe(2) + }) + + it('forwards an event collected after the release instead of holding it again', () => { + collect(RumEventType.VIEW) + collect(RumEventType.RESOURCE) + sessionManager.setSessionHasError() + collect(RumEventType.ERROR) + releasedAfterJitter() + const forwardedAfterRelease = forwarded.length + + // The buffer released and cleared; a later event of the same, now-released session must reach + // the batch straight away rather than be held into a fresh hold-then-release cycle. + collect(RumEventType.RESOURCE, { date: 5678 }) + + expect(forwarded.length).toBe(forwardedAfterRelease + 1) + }) + + 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('preserves the history and a releasing error larger than the buffer budget', () => { + const view = collect(RumEventType.VIEW) + const resource = collect(RumEventType.RESOURCE) + sessionManager.setSessionHasError() + const error = collect(RumEventType.ERROR, { + error: { source: 'custom', message: 'x'.repeat(WITHHELD_BUFFER_BYTES_LIMIT + 1) }, + }) + + // The oversized error cannot be held, so it goes out first, ahead of the history it precedes; + // the backend orders by client time, so the wire order does not matter. + expect(releasedAfterJitter()).toEqual([error, view, resource]) + }) + + it('still spreads the history behind the jitter when the releasing error is oversized', () => { + const view = collect(RumEventType.VIEW) + const resource = collect(RumEventType.RESOURCE) + sessionManager.setSessionHasError() + const error = collect(RumEventType.ERROR, { + error: { source: 'custom', message: 'x'.repeat(WITHHELD_BUFFER_BYTES_LIMIT + 1) }, + }) + + // Only the oversized error has left so far; releasing the history in this same tick would defeat + // the jitter for exactly the correlated outage it protects against. + expect(forwarded).toEqual([error]) + expect(releasedAfterJitter()).toEqual([error, view, resource]) + }) + + it('does not release a large error while the session is still withholding', () => { + collect(RumEventType.VIEW) + collect(RumEventType.ERROR, { + error: { source: 'agent', message: 'x'.repeat(WITHHELD_BUFFER_BYTES_LIMIT + 1) }, + }) + expect(releasedAfterJitter()).toEqual([]) + }) + + 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, 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().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', () => { + // 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('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('drops the buffer when the session ends without ever having errored', () => { + collect(RumEventType.VIEW) + collect(RumEventType.RESOURCE) + + lifeCycle.notify(LifeCycleEventType.SESSION_EXPIRED) + + 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('drops newer long tasks before an older action, by tier rather than by age', () => { + collect(RumEventType.VIEW) + // the action is the oldest detail, so eviction by age would take it first; its tier is above a + // long task's, so tiered eviction must keep it and give up the newer long tasks instead + collect(RumEventType.ACTION, { date: 1 }) + for (let i = 0; i < WITHHELD_BUFFER_EVENTS_LIMIT; i++) { + collect(RumEventType.LONG_TASK, { date: 2 }) + } + + sessionManager.setSessionHasError() + collect(RumEventType.ERROR) + + // the eviction gives up a long task, not the older action - collapsing the action into the long + // task's tier would take the oldest detail, the action, instead + 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('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('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, { view: { id: 'view-59' } }) + + const released = releasedAfterJitter() + 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') + }) + + 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 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('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('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('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('discards an unreleased buffer when stopping', () => { + collect(RumEventType.VIEW) + collect(RumEventType.RESOURCE) + + stopBuffer() + clock.tick(WITHHELD_BUFFER_RELEASE_MAX_DELAY) + + 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' } }) + // 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', () => { + 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 new file mode 100644 index 0000000000..e6ece26508 --- /dev/null +++ b/packages/rum-core/src/transport/withheldEventBuffer.ts @@ -0,0 +1,410 @@ +import type { Context, RelativeTime, TimeoutId } from '@flashcatcloud/browser-core' +import { + ONE_KIBI_BYTE, + ONE_SECOND, + addTelemetryDebug, + clearTimeout, + computeBytesCount, + 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, 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 + +/** 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 + +/** + * 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 + +/** 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, 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 { + 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 currentViewId: string | undefined + let currentViewDate = -Infinity + let withheldForSessionId: 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) => { + 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 && 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 + } + + 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)) { + return + } + } + + if (session?.eventsWithheld && isFrom(session.id)) { + withheldForSessionId = session.id + hold(event) + return + } + + if (withheldForSessionId !== undefined && isFrom(withheldForSessionId)) { + if ( + event.type === RumEventType.ERROR && + computeBytesCount(jsonStringify(event) ?? '') > WITHHELD_BUFFER_BYTES_LIMIT + ) { + // The session has already earned its release. A single error larger than the history + // budget must reach the normal batch, without evicting itself or the history preceding it - + // so it is forwarded straight away rather than held. The history it precedes still leaves + // behind the jitter: releasing it here in the same tick would defeat the anti-thundering-herd + // spread for exactly the correlated outage the jitter exists for. `scheduleRelease` is a + // no-op if the release the mark already scheduled is still pending. + forward(event) + scheduleRelease() + return + } + // 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() + return + } + + forward(event) + }) + + /** + * 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. + * + * 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) { + 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 if (discardIfUnreleased) { + discardBuffer() + } + } + + // 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 sessionReleaseSubscription = lifeCycle.subscribe( + LifeCycleEventType.SESSION_RELEASED, + ({ sessionId, reason }) => { + if (withheldForSessionId !== sessionId) { + return + } + if (reason === 'force') { + release() + } else { + // The local triggering error is collected later in the same synchronous notification. + scheduleRelease() + } + } + ) + 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) { + // 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. 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) + // 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) { + 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() + return + } + + const held: WithheldEvent = { + event, + viewId: event.view.id, + time: relativeNow(), + bytes: computeBytesCount(jsonStringify(event) ?? ''), + tier: getEvictionTier(event), + } + details.push(held) + bytes += held.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() { + // 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 + droppedCount += 1 + cutoff += 1 + } + 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. */ + function evictOne() { + for (const tier of [EvictionTier.FIRST, EvictionTier.LAST]) { + const index = details.findIndex((held) => held.tier === tier) + if (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 + } + releaseScheduledAt = relativeNow() + releaseTimeoutId = setTimeout(release, computeReleaseDelay(withheldForSessionId!)) + } + + function release() { + 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)) + + // 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) + + orderedViews.forEach(forward) + 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, + }) + + clearBuffer() + } + + /** Throws the buffer away, and remembers whose it was so its stragglers go the same way. */ + function discardBuffer() { + if (withheldForSessionId !== undefined) { + discardedSessionIds.push(withheldForSessionId) + if (discardedSessionIds.length > DISCARDED_SESSIONS_REMEMBERED) { + discardedSessionIds.shift() + } + } + clearBuffer() + } + + function clearBuffer() { + clearTimeout(releaseTimeoutId) + releaseTimeoutId = undefined + releaseScheduledAt = undefined + views = new Map() + details = [] + bytes = 0 + droppedCount = 0 + currentViewId = undefined + currentViewDate = -Infinity + withheldForSessionId = undefined + } + + return { + stop: () => { + settleBuffer(true) + sessionReleaseSubscription.unsubscribe() + eventSubscription.unsubscribe() + pageMayExitSubscription.unsubscribe() + sessionExpireSubscription.unsubscribe() + }, + } +} + +function getEvictionTier(event: RumEvent): EvictionTier { + switch (event.type) { + case RumEventType.ERROR: + return EvictionTier.LAST_RESORT + 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. + // -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 + } +} + +/** + * 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 = Math.imul(hash, 31) + sessionId.charCodeAt(i) + } + return Math.abs(hash) % WITHHELD_BUFFER_RELEASE_MAX_DELAY +} diff --git a/packages/rum-core/test/mockRumSessionManager.ts b/packages/rum-core/test/mockRumSessionManager.ts index 80c9d90002..402abb2349 100644 --- a/packages/rum-core/test/mockRumSessionManager.ts +++ b/packages/rum-core/test/mockRumSessionManager.ts @@ -1,7 +1,9 @@ import { Observable } from '@flashcatcloud/browser-core' import { RumTrackingType, + computeEventsWithheld, computeSessionReplayState, + withholdsEvents, withholdsReplay, type DrawnConfiguration, type RumSessionManager, @@ -13,6 +15,8 @@ export interface RumSessionManagerMock extends RumSessionManager { setTrackedWithoutSessionReplay(): RumSessionManagerMock setTrackedWithSessionReplay(): RumSessionManagerMock setTrackedWithErrorSessionReplay(): RumSessionManagerMock + setTrackedOnError(): RumSessionManagerMock + setTrackedOnErrorWithSessionReplay(): RumSessionManagerMock setForcedReplay(): RumSessionManagerMock setSessionHasError(): RumSessionManagerMock setDrawnConfiguration(drawn: DrawnConfiguration): RumSessionManagerMock @@ -23,6 +27,8 @@ const enum SessionStatus { TRACKED_WITH_SESSION_REPLAY, TRACKED_WITHOUT_SESSION_REPLAY, TRACKED_WITH_ERROR_SESSION_REPLAY, + TRACKED_ON_ERROR, + TRACKED_ON_ERROR_WITH_SESSION_REPLAY, NOT_TRACKED, EXPIRED, } @@ -31,6 +37,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_WITHOUT_SESSION_REPLAY, + [SessionStatus.TRACKED_ON_ERROR_WITH_SESSION_REPLAY]: RumTrackingType.TRACKED_ON_ERROR_WITH_SESSION_REPLAY, } export function createRumSessionManagerMock(): RumSessionManagerMock { @@ -49,6 +57,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), sampledOnErrorReplay: withholdsReplay(trackingType), anonymousId: 'device-123', drawnConfiguration, @@ -79,6 +89,14 @@ export function createRumSessionManagerMock(): RumSessionManagerMock { sessionStatus = SessionStatus.TRACKED_WITH_ERROR_SESSION_REPLAY return this }, + setTrackedOnError() { + sessionStatus = SessionStatus.TRACKED_ON_ERROR + return this + }, + setTrackedOnErrorWithSessionReplay() { + sessionStatus = SessionStatus.TRACKED_ON_ERROR_WITH_SESSION_REPLAY + return this + }, setForcedReplay() { forcedReplay = true return this diff --git a/packages/rum/src/domain/segmentCollection/segmentCollection.spec.ts b/packages/rum/src/domain/segmentCollection/segmentCollection.spec.ts index 39f738d65e..a49666cc58 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, @@ -386,7 +385,7 @@ describe('startSegmentCollection withholding (error session replay)', () => { it('releases a checkout still being encoded without reusing its segment index', async () => { addRecord({ ...RECORD, type: RecordType.FullSnapshot, data: {} } as BrowserRecord) worker.processAllMessages() - clock.tick(BUFFER_CHECKOUT_TIME) + clock.tick(WITHHELD_BUFFER_DURATION) reportError() lifeCycle.notify(LifeCycleEventType.RUM_EVENT_COLLECTED, { type: 'error' } as any) addRecord(RECORD) @@ -404,7 +403,7 @@ describe('startSegmentCollection withholding (error session replay)', () => { it('remembers a release if recording ends before the worker answers', () => { addRecord(RECORD) worker.processAllMessages() - clock.tick(BUFFER_CHECKOUT_TIME) + clock.tick(WITHHELD_BUFFER_DURATION) reportError() lifeCycle.notify(LifeCycleEventType.RUM_EVENT_COLLECTED, { type: 'error' } as any) stopCollection() @@ -416,7 +415,7 @@ describe('startSegmentCollection withholding (error session replay)', () => { it('drains records and a stop queued behind a released flush', async () => { addRecord(RECORD) worker.processAllMessages() - clock.tick(BUFFER_CHECKOUT_TIME) + clock.tick(WITHHELD_BUFFER_DURATION) addRecord(RECORD) reportError() stopCollection() @@ -441,7 +440,7 @@ describe('startSegmentCollection withholding (error session replay)', () => { restartFromFullSnapshot: noop, }) first.addRecord(RECORD) - clock.tick(BUFFER_CHECKOUT_TIME) + clock.tick(WITHHELD_BUFFER_DURATION) first.addRecord(RECORD) released = true first.stop() @@ -480,7 +479,7 @@ describe('startSegmentCollection withholding (error session replay)', () => { it('never releases an unfinished flush for a different session', () => { addRecord(RECORD) - clock.tick(BUFFER_CHECKOUT_TIME) + clock.tick(WITHHELD_BUFFER_DURATION) releasedSessionId = 'different-session' stopCollection() worker.processAllMessages() @@ -564,7 +563,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() @@ -713,7 +712,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) @@ -731,7 +730,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() @@ -745,7 +744,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() @@ -758,7 +757,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 7c7ed72d98..582b438fb0 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 { RecordType } from '../../types' import { discardSegmentData, removeSegment } from '../replayStats' @@ -18,12 +18,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 = /** * These two are internal and never reach the intake, so they are mapped back to a schema value where * the next segment records why it was created. `buffer_checkout` drops a withheld buffer that has - * grown past {@link BUFFER_CHECKOUT_TIME}; `page_reactivated` cuts a segment when the page is + * grown past {@link WITHHELD_BUFFER_DURATION}; `page_reactivated` cuts a segment when the page is * switched back to, so the next one starts from the fresh full snapshot taken on the same event. */ type InternalFlushReason = FlushReason | 'buffer_checkout' | 'page_reactivated' @@ -382,7 +376,7 @@ export function doStartSegmentCollection( withheldForSessionId !== undefined ? setTimeout(() => { requestFlush('buffer_checkout') - }, BUFFER_CHECKOUT_TIME) + }, WITHHELD_BUFFER_DURATION) : undefined, withheldForSessionId, }