diff --git a/developer-extension/package.json b/developer-extension/package.json index 4dfd2d82d8..9b3b33c883 100644 --- a/developer-extension/package.json +++ b/developer-extension/package.json @@ -1,6 +1,6 @@ { "name": "@flashcatcloud/browser-sdk-developer-extension", - "version": "0.0.2", + "version": "0.1.0", "private": true, "scripts": { "build": "rm -rf dist && webpack --mode production", diff --git a/packages/core/package.json b/packages/core/package.json index 5f10594575..0f1dbb5578 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -1,6 +1,6 @@ { "name": "@flashcatcloud/browser-core", - "version": "0.0.2", + "version": "0.1.0", "license": "Apache-2.0", "main": "cjs/index.js", "module": "esm/index.js", diff --git a/packages/core/src/domain/session/sessionManager.spec.ts b/packages/core/src/domain/session/sessionManager.spec.ts index 345502f1d6..f3d188ab22 100644 --- a/packages/core/src/domain/session/sessionManager.spec.ts +++ b/packages/core/src/domain/session/sessionManager.spec.ts @@ -637,7 +637,7 @@ describe('startSessionManager', () => { const sessionManager = startSessionManagerWithDefaults() sessionManager.sessionStateUpdateObservable.subscribe(sessionStateUpdateSpy) - sessionManager.updateSessionState({ extra: 'extra' }) + sessionManager.updateSessionState(() => ({ extra: 'extra' })) expectSessionIdToBeDefined(sessionManager) expect(sessionStateUpdateSpy).toHaveBeenCalledTimes(1) diff --git a/packages/core/src/domain/session/sessionManager.ts b/packages/core/src/domain/session/sessionManager.ts index 03caf2f49c..fae90339a5 100644 --- a/packages/core/src/domain/session/sessionManager.ts +++ b/packages/core/src/domain/session/sessionManager.ts @@ -20,13 +20,19 @@ export interface SessionManager { expireObservable: Observable sessionStateUpdateObservable: Observable<{ previousState: SessionState; newState: SessionState }> expire: () => void - updateSessionState: (state: Partial) => void + updateSessionState: (update: (state: SessionState) => Partial | undefined) => void } export interface SessionContext extends Context { id: string trackingType: TrackingType isReplayForced: boolean + /** + * Whether an error has already been reported during this session. Persisted in the session store + * so it survives page navigation: an error session must not go back to withholding its replay + * just because the user moved to another page. + */ + hasError: boolean anonymousId: string | undefined } @@ -92,6 +98,7 @@ export function startSessionManager( id: sessionStore.getSession().id!, trackingType: sessionStore.getSession()[productKey] as TrackingType, isReplayForced: !!sessionStore.getSession().forcedReplay, + hasError: !!sessionStore.getSession().hasError, anonymousId: sessionStore.getSession().anonymousId, } } diff --git a/packages/core/src/domain/session/sessionStore.spec.ts b/packages/core/src/domain/session/sessionStore.spec.ts index 7d8105177c..2bcd9378f7 100644 --- a/packages/core/src/domain/session/sessionStore.spec.ts +++ b/packages/core/src/domain/session/sessionStore.spec.ts @@ -596,7 +596,7 @@ describe('session store', () => { sessionStoreManager = setupSessionStore(updateSpy) otherSessionStoreManager = setupSessionStore(otherUpdateSpy) - sessionStoreManager.updateSessionState({ extra: 'extra' }) + sessionStoreManager.updateSessionState(() => ({ extra: 'extra' })) expect(updateSpy).toHaveBeenCalledTimes(1) diff --git a/packages/core/src/domain/session/sessionStore.ts b/packages/core/src/domain/session/sessionStore.ts index 4c0a1a74e5..accc0b8184 100644 --- a/packages/core/src/domain/session/sessionStore.ts +++ b/packages/core/src/domain/session/sessionStore.ts @@ -28,7 +28,12 @@ export interface SessionStore { sessionStateUpdateObservable: Observable<{ previousState: SessionState; newState: SessionState }> expire: () => void stop: () => void - updateSessionState: (state: Partial) => void + /** + * Applies a change to the stored session under the store lock. The producer sees the state the + * change would land on and returns `undefined` to make it a no-op - which is how a write meant for + * one session avoids landing on the one that replaced it while the write was waiting for the lock. + */ + updateSessionState: (update: (state: SessionState) => Partial | undefined) => void } /** @@ -203,10 +208,13 @@ export function startSessionStore( renewObservable.notify() } - function updateSessionState(partialSessionState: Partial) { + function updateSessionState(update: (state: SessionState) => Partial | undefined) { processSessionStoreOperations( { - process: (sessionState) => ({ ...sessionState, ...partialSessionState }), + process: (sessionState) => { + const partialSessionState = update(sessionState) + return partialSessionState && { ...sessionState, ...partialSessionState } + }, after: synchronizeSession, }, sessionStoreStrategy diff --git a/packages/flagging/package.json b/packages/flagging/package.json index 2358defd56..51fa4899c6 100644 --- a/packages/flagging/package.json +++ b/packages/flagging/package.json @@ -1,6 +1,6 @@ { "name": "@flashcatcloud/browser-flagging", - "version": "0.0.2", + "version": "0.1.0", "license": "Apache-2.0", "private": true, "main": "cjs/entries/main.js", diff --git a/packages/logs/package.json b/packages/logs/package.json index 5de7c65469..fb74d47565 100644 --- a/packages/logs/package.json +++ b/packages/logs/package.json @@ -1,6 +1,6 @@ { "name": "@flashcatcloud/browser-logs", - "version": "0.0.2", + "version": "0.1.0", "license": "Apache-2.0", "main": "cjs/entries/main.js", "module": "esm/entries/main.js", @@ -14,7 +14,7 @@ "replace-build-env": "node ../../scripts/build/replace-build-env.js" }, "dependencies": { - "@flashcatcloud/browser-core": "0.0.2" + "@flashcatcloud/browser-core": "0.1.0" }, "peerDependencies": { "@flashcatcloud/browser-rum": "0.0.2" diff --git a/packages/rum-core/package.json b/packages/rum-core/package.json index ea90c36a0d..b9ef0da2c1 100644 --- a/packages/rum-core/package.json +++ b/packages/rum-core/package.json @@ -1,6 +1,6 @@ { "name": "@flashcatcloud/browser-rum-core", - "version": "0.0.2", + "version": "0.1.0", "license": "Apache-2.0", "main": "cjs/index.js", "module": "esm/index.js", diff --git a/packages/rum-core/src/boot/startRum.ts b/packages/rum-core/src/boot/startRum.ts index e60741983b..0c1af632b9 100644 --- a/packages/rum-core/src/boot/startRum.ts +++ b/packages/rum-core/src/boot/startRum.ts @@ -28,6 +28,7 @@ import { startErrorCollection } from '../domain/error/errorCollection' import { startResourceCollection } from '../domain/resource/resourceCollection' import { startViewCollection } from '../domain/view/viewCollection' import { startRumSessionManager, startRumSessionManagerStub } from '../domain/rumSessionManager' +import { startSessionErrorTracking } from '../domain/trackSessionError' import { startRumBatch } from '../transport/startRumBatch' import { startRumEventBridge } from '../transport/startRumEventBridge' import { startUrlContexts } from '../domain/contexts/urlContexts' @@ -110,6 +111,9 @@ export function startRum( ? startRumSessionManager(configuration, lifeCycle, trackingConsentState) : startRumSessionManagerStub() + const sessionErrorTracking = startSessionErrorTracking(lifeCycle, session) + cleanupTasks.push(() => sessionErrorTracking.stop()) + if (!canUseEventBridge()) { const batch = startRumBatch( configuration, diff --git a/packages/rum-core/src/domain/configuration/configuration.spec.ts b/packages/rum-core/src/domain/configuration/configuration.spec.ts index 0331d764bf..bb01bf41c5 100644 --- a/packages/rum-core/src/domain/configuration/configuration.spec.ts +++ b/packages/rum-core/src/domain/configuration/configuration.spec.ts @@ -1,6 +1,9 @@ import type { InitConfiguration } from '@flashcatcloud/browser-core' import { DefaultPrivacyLevel, display, TraceContextInjection } from '@flashcatcloud/browser-core' -import { EXHAUSTIVE_INIT_CONFIGURATION, SERIALIZED_EXHAUSTIVE_INIT_CONFIGURATION } from '@flashcatcloud/browser-core/test' +import { + EXHAUSTIVE_INIT_CONFIGURATION, + SERIALIZED_EXHAUSTIVE_INIT_CONFIGURATION, +} from '@flashcatcloud/browser-core/test' import type { ExtractTelemetryConfiguration, CamelToSnakeCase, @@ -62,6 +65,82 @@ describe('validateAndBuildRumConfiguration', () => { }) }) + describe('sessionReplayOnErrorSampleRate', () => { + it('is carried into the built configuration', () => { + expect( + validateAndBuildRumConfiguration({ ...DEFAULT_INIT_CONFIGURATION, sessionReplayOnErrorSampleRate: 50 })! + .sessionReplayOnErrorSampleRate + ).toBe(50) + }) + + it('defaults to collecting no error replay at all', () => { + expect(validateAndBuildRumConfiguration(DEFAULT_INIT_CONFIGURATION)!.sessionReplayOnErrorSampleRate).toBe(0) + }) + + it('is rejected when it is not a sample rate', () => { + expect( + validateAndBuildRumConfiguration({ + ...DEFAULT_INIT_CONFIGURATION, + sessionReplayOnErrorSampleRate: 'foo' as unknown as number, + }) + ).toBeUndefined() + expect(displayErrorSpy).toHaveBeenCalledTimes(1) + }) + + it('starts the recording on its own, since there is nothing to withhold otherwise', () => { + expect( + validateAndBuildRumConfiguration({ + ...DEFAULT_INIT_CONFIGURATION, + sessionReplaySampleRate: 0, + sessionReplayOnErrorSampleRate: 30, + })!.startSessionReplayRecordingManually + ).toBeFalse() + }) + + it('warns when the plain replay rate leaves it nothing to draw from', () => { + validateAndBuildRumConfiguration({ + ...DEFAULT_INIT_CONFIGURATION, + sessionReplaySampleRate: 100, + sessionReplayOnErrorSampleRate: 50, + }) + + expect(displayWarnSpy).toHaveBeenCalledTimes(1) + expect(displayWarnSpy.calls.argsFor(0)[0]).toContain('sessionReplaySampleRate did not draw') + }) + + it('warns when no session is tracked at all', () => { + validateAndBuildRumConfiguration({ + ...DEFAULT_INIT_CONFIGURATION, + sessionSampleRate: 0, + sessionReplayOnErrorSampleRate: 50, + }) + + expect(displayWarnSpy).toHaveBeenCalledTimes(1) + expect(displayWarnSpy.calls.argsFor(0)[0]).toContain('no session is tracked') + }) + + it('warns when the recording is left for the customer to start, since nothing would be held', () => { + validateAndBuildRumConfiguration({ + ...DEFAULT_INIT_CONFIGURATION, + sessionReplayOnErrorSampleRate: 50, + startSessionReplayRecordingManually: true, + }) + + expect(displayWarnSpy).toHaveBeenCalledTimes(1) + expect(displayWarnSpy.calls.argsFor(0)[0]).toContain('startSessionReplayRecordingManually') + }) + + it('says nothing about a rate that can draw', () => { + validateAndBuildRumConfiguration({ + ...DEFAULT_INIT_CONFIGURATION, + sessionReplaySampleRate: 20, + sessionReplayOnErrorSampleRate: 50, + }) + + expect(displayWarnSpy).not.toHaveBeenCalled() + }) + }) + describe('traceSampleRate', () => { it('defaults to 100 if the option is not provided', () => { expect(validateAndBuildRumConfiguration(DEFAULT_INIT_CONFIGURATION)!.traceSampleRate).toBe(100) @@ -529,6 +608,7 @@ describe('serializeRumConfiguration', () => { enablePrivacyForActionName: false, subdomain: 'foo', sessionReplaySampleRate: 60, + sessionReplayOnErrorSampleRate: 40, startSessionReplayRecordingManually: true, trackUserInteractions: true, actionNameAttribute: 'test-id', @@ -554,6 +634,8 @@ describe('serializeRumConfiguration', () => { | 'remoteConfigurationId' | 'profilingSampleRate' | 'propagateTraceBaggage' + // not reported yet: needs a rum-events-format schema change first + | 'sessionReplayOnErrorSampleRate' ? 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 8e25227ef5..657adabfd2 100644 --- a/packages/rum-core/src/domain/configuration/configuration.ts +++ b/packages/rum-core/src/domain/configuration/configuration.ts @@ -100,6 +100,17 @@ export interface RumInitConfiguration extends InitConfiguration { * See [Configure Your Setup For Browser RUM and Browser RUM & Session Replay Sampling](https://docs.datadoghq.com/real_user_monitoring/guide/sampling-browser-plans) for further information. */ sessionReplaySampleRate?: number | undefined + /** + * Of the tracked sessions that `sessionReplaySampleRate` did not draw, the percentage that record + * a replay but only upload it if the session reports an error: 100 for all of them, 0 for none. + * The base is what the plain rate missed, so a session is never counted by both, and the share of + * all tracked sessions this covers is `(100 - sessionReplaySampleRate) * this / 100`. + * + * Such a session records from the start and keeps at most the last minute of it in memory. If it + * never reports an error, nothing is uploaded and the session is not billed. On the first error, + * the withheld minute is uploaded and recording continues normally for the rest of the session. + */ + sessionReplayOnErrorSampleRate?: number | undefined /** * If the session is sampled for Session Replay, only start the recording when `startSessionReplayRecording()` is called, instead of at the beginning of the session. Default: if startSessionReplayRecording is 0, true; otherwise, false. * See [Session Replay Usage](https://docs.datadoghq.com/real_user_monitoring/session_replay/browser/#usage) for further information. @@ -175,6 +186,7 @@ export interface RumConfiguration extends Configuration { defaultPrivacyLevel: DefaultPrivacyLevel enablePrivacyForActionName: boolean sessionReplaySampleRate: number + sessionReplayOnErrorSampleRate: number startSessionReplayRecordingManually: boolean trackUserInteractions: boolean trackViewsManually: boolean @@ -207,6 +219,7 @@ export function validateAndBuildRumConfiguration( if ( !isSampleRate(initConfiguration.sessionReplaySampleRate, 'Session Replay') || + !isSampleRate(initConfiguration.sessionReplayOnErrorSampleRate, 'Session Replay on Error') || !isSampleRate(initConfiguration.traceSampleRate, 'Trace') ) { return @@ -230,16 +243,39 @@ export function validateAndBuildRumConfiguration( const profilingEnabled = isExperimentalFeatureEnabled(ExperimentalFeature.PROFILING) const sessionReplaySampleRate = initConfiguration.sessionReplaySampleRate ?? 0 + const sessionReplayOnErrorSampleRate = initConfiguration.sessionReplayOnErrorSampleRate ?? 0 + + // Each of these is a rate the customer set that cannot draw a single session. They are valid + // numbers, so validation lets them through - but silence would leave them waiting for data that + // is never coming. + if (sessionReplayOnErrorSampleRate > 0) { + if (sessionReplaySampleRate === 100) { + display.warn( + 'sessionReplayOnErrorSampleRate is drawn only for sessions sessionReplaySampleRate did not draw, and that rate is 100: it will never apply.' + ) + } + if ((initConfiguration.sessionSampleRate ?? 100) === 0) { + display.warn('sessionReplayOnErrorSampleRate has no effect while sessionSampleRate is 0: no session is tracked.') + } + if (initConfiguration.startSessionReplayRecordingManually) { + display.warn( + 'sessionReplayOnErrorSampleRate needs the recording to already be running when the error happens, and startSessionReplayRecordingManually keeps it stopped until you start it: there would be nothing to release.' + ) + } + } return { applicationId: initConfiguration.applicationId, version: initConfiguration.version || undefined, actionNameAttribute: initConfiguration.actionNameAttribute, sessionReplaySampleRate, + sessionReplayOnErrorSampleRate, startSessionReplayRecordingManually: initConfiguration.startSessionReplayRecordingManually !== undefined ? !!initConfiguration.startSessionReplayRecordingManually - : sessionReplaySampleRate === 0, + : // An error-sampled session has to be recording before the error happens, otherwise there is + // nothing to withhold and release. So it must auto-start just like a plain sampled one. + sessionReplaySampleRate === 0 && sessionReplayOnErrorSampleRate === 0, traceSampleRate: initConfiguration.traceSampleRate ?? 100, rulePsr: isNumber(initConfiguration.traceSampleRate) ? initConfiguration.traceSampleRate / 100 : undefined, allowedTracingUrls, @@ -325,6 +361,9 @@ export function serializeRumConfiguration(configuration: RumInitConfiguration) { return { session_replay_sample_rate: configuration.sessionReplaySampleRate, + // `session_replay_on_error_sample_rate` is 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, trace_sample_rate: configuration.traceSampleRate, trace_context_injection: configuration.traceContextInjection, diff --git a/packages/rum-core/src/domain/contexts/sessionContext.spec.ts b/packages/rum-core/src/domain/contexts/sessionContext.spec.ts index c8aec0c702..5868240c9b 100644 --- a/packages/rum-core/src/domain/contexts/sessionContext.spec.ts +++ b/packages/rum-core/src/domain/contexts/sessionContext.spec.ts @@ -94,6 +94,49 @@ describe('session context', () => { expect(eventWithoutHasReplay.session!.has_replay).toBeUndefined() }) + it('should tell a replay kept only because the session errored apart from an unconditional one', () => { + sessionManager.setTrackedWithErrorSessionReplay() + const errorReplayEvent = 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(errorReplayEvent.session!.sampled_for_error_replay).toBeTrue() + // absent rather than false, so it costs nothing on every ordinary session + expect(plainEvent.session!.sampled_for_error_replay).toBeUndefined() + }) + + 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 + getReplayStatsSpy.and.returnValue({ segments_count: 0, records_count: 0, segments_total_raw_size: 0 }) + + const event = hooks.triggerHook(HookNames.Assemble, { + eventType: 'view', + startTime: 0 as RelativeTime, + }) as DefaultRumEventAttributes + + expect(event.session!.has_replay).toBeUndefined() + }) + + it('should set hasReplay when a host bridge took the records and no segment was built', () => { + // records go straight to the bridge, so nothing ever counts a segment for them + getReplayStatsSpy.and.returnValue({ ...fakeStats, segments_count: 0, records_count: 10 }) + + const event = hooks.triggerHook(HookNames.Assemble, { + eventType: 'view', + startTime: 0 as RelativeTime, + }) as DefaultRumEventAttributes + + expect(event.session!.has_replay).toBe(true) + }) + it('should set session.is_active when the session is active', () => { findViewSpy.and.returnValue({ ...fakeView, sessionIsActive: true }) const eventWithActiveSession = hooks.triggerHook(HookNames.Assemble, { diff --git a/packages/rum-core/src/domain/contexts/sessionContext.ts b/packages/rum-core/src/domain/contexts/sessionContext.ts index a62a2da0ff..aa0f84b4bf 100644 --- a/packages/rum-core/src/domain/contexts/sessionContext.ts +++ b/packages/rum-core/src/domain/contexts/sessionContext.ts @@ -20,15 +20,28 @@ export function startSessionContext( return DISCARDED } + // 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. + const isReplayWithheld = session.sessionReplay === SessionReplayState.BUFFERED_ON_ERROR + let hasReplay let sampledForReplay + let sampledForErrorReplay let isActive if (eventType === RumEventType.VIEW) { - hasReplay = recorderApi.getReplayStats(view.id) ? true : undefined + // Records rather than merely a stats entry: a withheld buffer that was dropped rolls back what + // it held, which leaves a view with an empty stats entry and no replay at all - and offering a + // replay that was never uploaded is worse than not offering one. Records, not segments, + // 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 + // 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 isActive = view.sessionIsActive ? undefined : false } else { - hasReplay = recorderApi.isRecording() ? true : undefined + hasReplay = !isReplayWithheld && recorderApi.isRecording() ? true : undefined } return { @@ -38,6 +51,7 @@ export function startSessionContext( type: SessionType.USER, has_replay: hasReplay, sampled_for_replay: sampledForReplay, + sampled_for_error_replay: sampledForErrorReplay, is_active: isActive, }, } diff --git a/packages/rum-core/src/domain/rumSessionManager.spec.ts b/packages/rum-core/src/domain/rumSessionManager.spec.ts index cb08bd0d1c..94e76fb1e4 100644 --- a/packages/rum-core/src/domain/rumSessionManager.spec.ts +++ b/packages/rum-core/src/domain/rumSessionManager.spec.ts @@ -9,6 +9,7 @@ import { createTrackingConsentState, TrackingConsent, BridgeCapability, + isChromium, } from '@flashcatcloud/browser-core' import type { Clock } from '@flashcatcloud/browser-core/test' import { @@ -209,6 +210,120 @@ describe('rum session manager', () => { ) }) + describe('error session replay sampling', () => { + it('draws the error-replay type only when the plain replay draw missed', () => { + startRumSessionManagerWithDefaults({ + configuration: { sessionSampleRate: 100, sessionReplaySampleRate: 100, sessionReplayOnErrorSampleRate: 100 }, + }) + + expect(getSessionState(SESSION_STORE_KEY)[RUM_SESSION_KEY]).toBe(RumTrackingType.TRACKED_WITH_SESSION_REPLAY) + }) + + it('stores the error-replay type when only that rate is hit', () => { + startRumSessionManagerWithDefaults({ + configuration: { sessionSampleRate: 100, sessionReplaySampleRate: 0, sessionReplayOnErrorSampleRate: 100 }, + }) + + expect(getSessionState(SESSION_STORE_KEY)[RUM_SESSION_KEY]).toBe( + RumTrackingType.TRACKED_WITH_ERROR_SESSION_REPLAY + ) + }) + + it('withholds the replay until the session reports an error', () => { + const sessionManager = startRumSessionManagerWithDefaults({ + configuration: { sessionSampleRate: 100, sessionReplaySampleRate: 0, sessionReplayOnErrorSampleRate: 100 }, + }) + + expect(sessionManager.findTrackedSession()!.sessionReplay).toBe(SessionReplayState.BUFFERED_ON_ERROR) + + sessionManager.setSessionHasError(sessionManager.findTrackedSession()!.id) + + expect(sessionManager.findTrackedSession()!.sessionReplay).toBe(SessionReplayState.SAMPLED) + }) + + it('does not mark a session that has since been replaced by another one', () => { + const sessionManager = startRumSessionManagerWithDefaults({ + configuration: { sessionSampleRate: 100, sessionReplaySampleRate: 0, sessionReplayOnErrorSampleRate: 100 }, + }) + + // another tab renewed the session while the mark was on its way to the store + setCookie(SESSION_STORE_KEY, 'id=other-session&rum=3', DURATION) + + sessionManager.setSessionHasError('a-session-that-is-gone') + + expect(getSessionState(SESSION_STORE_KEY).hasError).toBeUndefined() + }) + + it('releases the replay before the store write lands, since that write can be deferred', () => { + if (!isChromium()) { + pending('the store lock, and so a deferred write, only exists on Chromium') + } + const sessionManager = startRumSessionManagerWithDefaults({ + configuration: { sessionSampleRate: 100, sessionReplaySampleRate: 0, sessionReplayOnErrorSampleRate: 100 }, + }) + const sessionId = sessionManager.findTrackedSession()!.id + + // another tab holds the store lock, so the write is deferred through retries + setCookie(SESSION_STORE_KEY, `lock=other-tab&id=${sessionId}&rum=3`, DURATION) + + sessionManager.setSessionHasError(sessionId) + + expect(getSessionState(SESSION_STORE_KEY).hasError).toBeUndefined() + // and yet the buffer must already see it as released: the page or the session may end before + // the write ever lands, and the buffer would otherwise be thrown away + expect(sessionManager.findTrackedSession()!.sessionReplay).toBe(SessionReplayState.SAMPLED) + }) + + it('keeps the released state across a page load, since it is persisted in the session store', () => { + setCookie(SESSION_STORE_KEY, 'id=abcdef&rum=3&hasError=1', DURATION) + + const sessionManager = startRumSessionManagerWithDefaults() + + expect(sessionManager.findTrackedSession()!.sessionReplay).toBe(SessionReplayState.SAMPLED) + }) + + it('marks the session so a replay kept only because it errored can be told apart', () => { + const sessionManager = startRumSessionManagerWithDefaults({ + configuration: { sessionSampleRate: 100, sessionReplaySampleRate: 0, sessionReplayOnErrorSampleRate: 100 }, + }) + + expect(sessionManager.findTrackedSession()!.sampledOnErrorReplay).toBeTrue() + + // still true once released, so what was stored can be told apart afterwards + sessionManager.setSessionHasError(sessionManager.findTrackedSession()!.id) + + expect(sessionManager.findTrackedSession()!.sampledOnErrorReplay).toBeTrue() + }) + + it('does not mark a session whose replay is collected unconditionally', () => { + const sessionManager = startRumSessionManagerWithDefaults({ + configuration: { sessionSampleRate: 100, sessionReplaySampleRate: 100 }, + }) + + expect(sessionManager.findTrackedSession()!.sampledOnErrorReplay).toBeFalse() + }) + + it('releases the replay when it is forced, rather than waiting for an error that may never come', () => { + const sessionManager = startRumSessionManagerWithDefaults({ + configuration: { sessionSampleRate: 100, sessionReplaySampleRate: 0, sessionReplayOnErrorSampleRate: 100 }, + }) + expect(sessionManager.findTrackedSession()!.sessionReplay).toBe(SessionReplayState.BUFFERED_ON_ERROR) + + sessionManager.setForcedReplay() + + expect(sessionManager.findTrackedSession()!.sessionReplay).toBe(SessionReplayState.FORCED) + }) + + it('tracks the session even when no replay rate is hit at all', () => { + const sessionManager = startRumSessionManagerWithDefaults({ + configuration: { sessionSampleRate: 100, sessionReplaySampleRate: 0, sessionReplayOnErrorSampleRate: 0 }, + }) + + expect(getSessionState(SESSION_STORE_KEY)[RUM_SESSION_KEY]).toBe(RumTrackingType.TRACKED_WITHOUT_SESSION_REPLAY) + expect(sessionManager.findTrackedSession()!.sessionReplay).toBe(SessionReplayState.OFF) + }) + }) + function startRumSessionManagerWithDefaults({ configuration }: { configuration?: Partial } = {}) { return startRumSessionManager( mockRumConfiguration({ diff --git a/packages/rum-core/src/domain/rumSessionManager.ts b/packages/rum-core/src/domain/rumSessionManager.ts index 4d2f7829e7..ef6bcc4018 100644 --- a/packages/rum-core/src/domain/rumSessionManager.ts +++ b/packages/rum-core/src/domain/rumSessionManager.ts @@ -24,11 +24,23 @@ export interface RumSessionManager { expire: () => void expireObservable: Observable setForcedReplay: () => void + /** + * Marks the given session as having reported an error. For a session sampled by + * `sessionReplayOnErrorSampleRate`, 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. + */ + setSessionHasError: (sessionId: string) => void } 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. + */ + sampledOnErrorReplay: boolean anonymousId?: string } @@ -36,12 +48,19 @@ export const enum RumTrackingType { NOT_TRACKED = '0', TRACKED_WITH_SESSION_REPLAY = '1', TRACKED_WITHOUT_SESSION_REPLAY = '2', + TRACKED_WITH_ERROR_SESSION_REPLAY = '3', } export const enum SessionReplayState { OFF, SAMPLED, FORCED, + /** + * The session records, but every segment is withheld until it reports its first error. If no error + * ever happens, nothing is uploaded and the session is never billed. Once an error is reported the + * session moves to `SAMPLED` and the withheld buffer is released. + */ + BUFFERED_ON_ERROR, } export function startRumSessionManager( @@ -71,6 +90,12 @@ export function startRumSessionManager( sessionEntity.isReplayForced = true } } + if (!previousState.hasError && newState.hasError) { + const sessionEntity = sessionManager.findSession() + if (sessionEntity) { + sessionEntity.hasError = true + } + } }) return { findTrackedSession: (startTime) => { @@ -80,21 +105,54 @@ export function startRumSessionManager( } return { id: session.id, - sessionReplay: - session.trackingType === RumTrackingType.TRACKED_WITH_SESSION_REPLAY - ? SessionReplayState.SAMPLED - : session.isReplayForced - ? SessionReplayState.FORCED - : SessionReplayState.OFF, + sessionReplay: computeSessionReplayState(session.trackingType, session.hasError, session.isReplayForced), + sampledOnErrorReplay: withholdsReplay(session.trackingType), anonymousId: session.anonymousId, } }, expire: sessionManager.expire, expireObservable: sessionManager.expireObservable, - setForcedReplay: () => sessionManager.updateSessionState({ forcedReplay: '1' }), + setForcedReplay: () => sessionManager.updateSessionState(() => ({ forcedReplay: '1' })), + setSessionHasError: (sessionId) => { + const sessionEntity = sessionManager.findSession() + if (sessionEntity?.id === sessionId) { + // Marked in memory straight away, and not only once the store write lands: that write goes + // through a lock that can defer it by several retries, and until then the withheld buffer + // would still read the session as withholding - so an error followed closely by the page or + // the session ending would throw away the very buffer the error was meant to release. + sessionEntity.hasError = true + } + sessionManager.updateSessionState((state) => (state.id === sessionId ? { hasError: '1' } : undefined)) + }, } } +export function withholdsReplay(trackingType: RumTrackingType) { + return trackingType === RumTrackingType.TRACKED_WITH_ERROR_SESSION_REPLAY +} + +export function computeSessionReplayState( + trackingType: RumTrackingType, + hasError: boolean, + isReplayForced: boolean +): SessionReplayState { + if (trackingType === RumTrackingType.TRACKED_WITH_SESSION_REPLAY) { + return SessionReplayState.SAMPLED + } + if (withholdsReplay(trackingType) && hasError) { + return SessionReplayState.SAMPLED + } + // A forced replay wins over withholding: the host explicitly asked for this user's replay, so it + // must not keep waiting for an error that may never come. + if (isReplayForced) { + return SessionReplayState.FORCED + } + if (withholdsReplay(trackingType)) { + return SessionReplayState.BUFFERED_ON_ERROR + } + return SessionReplayState.OFF +} + /** * Start a tracked replay session stub */ @@ -102,12 +160,14 @@ export function startRumSessionManagerStub(): RumSessionManager { const session: RumSession = { id: '00000000-aaaa-0000-aaaa-000000000000', sessionReplay: bridgeSupports(BridgeCapability.RECORDS) ? SessionReplayState.SAMPLED : SessionReplayState.OFF, + sampledOnErrorReplay: false, } return { findTrackedSession: () => session, expire: noop, expireObservable: new Observable(), setForcedReplay: noop, + setSessionHasError: noop, } } @@ -117,10 +177,13 @@ function computeSessionState(configuration: RumConfiguration, rawTrackingType?: trackingType = rawTrackingType } else if (!performDraw(configuration.sessionSampleRate)) { trackingType = RumTrackingType.NOT_TRACKED - } else if (!performDraw(configuration.sessionReplaySampleRate)) { - trackingType = RumTrackingType.TRACKED_WITHOUT_SESSION_REPLAY - } else { + } else if (performDraw(configuration.sessionReplaySampleRate)) { trackingType = RumTrackingType.TRACKED_WITH_SESSION_REPLAY + } else if (performDraw(configuration.sessionReplayOnErrorSampleRate)) { + // Drawn only when the plain replay draw missed, so a session is never counted by both rates. + trackingType = RumTrackingType.TRACKED_WITH_ERROR_SESSION_REPLAY + } else { + trackingType = RumTrackingType.TRACKED_WITHOUT_SESSION_REPLAY } return { trackingType, @@ -132,13 +195,15 @@ function hasValidRumSession(trackingType?: string): trackingType is RumTrackingT return ( trackingType === RumTrackingType.NOT_TRACKED || trackingType === RumTrackingType.TRACKED_WITH_SESSION_REPLAY || - trackingType === RumTrackingType.TRACKED_WITHOUT_SESSION_REPLAY + trackingType === RumTrackingType.TRACKED_WITHOUT_SESSION_REPLAY || + trackingType === RumTrackingType.TRACKED_WITH_ERROR_SESSION_REPLAY ) } function isTypeTracked(rumSessionType: RumTrackingType | undefined) { return ( rumSessionType === RumTrackingType.TRACKED_WITHOUT_SESSION_REPLAY || - rumSessionType === RumTrackingType.TRACKED_WITH_SESSION_REPLAY + rumSessionType === RumTrackingType.TRACKED_WITH_SESSION_REPLAY || + rumSessionType === RumTrackingType.TRACKED_WITH_ERROR_SESSION_REPLAY ) } diff --git a/packages/rum-core/src/domain/trackSessionError.spec.ts b/packages/rum-core/src/domain/trackSessionError.spec.ts new file mode 100644 index 0000000000..5cfe610038 --- /dev/null +++ b/packages/rum-core/src/domain/trackSessionError.spec.ts @@ -0,0 +1,96 @@ +import type { Context } from '@flashcatcloud/browser-core' +import { registerCleanupTask } from '@flashcatcloud/browser-core/test' +import type { RumEvent } from '../rumEvent.types' +import { createRumSessionManagerMock } from '../../test' +import { LifeCycle, LifeCycleEventType } from './lifeCycle' +import { startSessionErrorTracking } from './trackSessionError' + +describe('startSessionErrorTracking', () => { + let lifeCycle: LifeCycle + let sessionManager: ReturnType + let setSessionHasErrorSpy: jasmine.Spy + + function collect(type: string, source = 'source') { + // only error events carry an `error` object; anything else that did would hide a guard that + // reads it before checking the type + const event = type === 'error' ? { type, error: { source } } : { type } + lifeCycle.notify(LifeCycleEventType.RUM_EVENT_COLLECTED, event as unknown as RumEvent & Context) + } + + beforeEach(() => { + lifeCycle = new LifeCycle() + sessionManager = createRumSessionManagerMock().setTrackedWithErrorSessionReplay() + setSessionHasErrorSpy = spyOn(sessionManager, 'setSessionHasError').and.callThrough() + const { stop } = startSessionErrorTracking(lifeCycle, sessionManager) + registerCleanupTask(stop) + }) + + it('marks the session on the first collected error', () => { + collect('error') + + // named, not just counted: the mark is refused if it does not name the session it belongs to + expect(setSessionHasErrorSpy).toHaveBeenCalledOnceWith('session-id') + }) + + it('leaves a session that withholds nothing alone, so an ordinary session store is never written', () => { + sessionManager.setTrackedWithSessionReplay() + + collect('error') + + expect(setSessionHasErrorSpy).not.toHaveBeenCalled() + }) + + it('leaves an untracked session alone', () => { + sessionManager.setNotTracked() + + collect('error') + + expect(setSessionHasErrorSpy).not.toHaveBeenCalled() + }) + + it('does not mark the session on other event types', () => { + collect('view') + collect('resource') + collect('action') + + expect(setSessionHasErrorSpy).not.toHaveBeenCalled() + }) + + it('ignores the SDK own failures, which are not the application reporting an error', () => { + collect('error', 'agent') + + expect(setSessionHasErrorSpy).not.toHaveBeenCalled() + }) + + it('still marks the session on a network error, which is the application reporting one', () => { + collect('error', 'network') + + expect(setSessionHasErrorSpy).toHaveBeenCalledTimes(1) + }) + + it('marks the session only once, however many errors follow', () => { + collect('error') + collect('error') + collect('error') + + expect(setSessionHasErrorSpy).toHaveBeenCalledTimes(1) + }) + + it('marks a renewed session again, since it is a different session', () => { + collect('error') + lifeCycle.notify(LifeCycleEventType.SESSION_RENEWED) + collect('error') + + expect(setSessionHasErrorSpy).toHaveBeenCalledTimes(2) + }) + + it('stops marking once stopped', () => { + const { stop } = startSessionErrorTracking(lifeCycle, sessionManager) + stop() + setSessionHasErrorSpy.calls.reset() + // the suite's own tracker is still running, so exactly one call is expected, not two + collect('error') + + expect(setSessionHasErrorSpy).toHaveBeenCalledTimes(1) + }) +}) diff --git a/packages/rum-core/src/domain/trackSessionError.ts b/packages/rum-core/src/domain/trackSessionError.ts new file mode 100644 index 0000000000..531d7f6aa5 --- /dev/null +++ b/packages/rum-core/src/domain/trackSessionError.ts @@ -0,0 +1,52 @@ +import { ErrorSource } from '@flashcatcloud/browser-core' +import { RumEventType } from '../rawRumEvent.types' +import type { LifeCycle } from './lifeCycle' +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 + * `sessionReplayOnErrorSampleRate`. + * + * 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 + * afterwards would be worse than no replay at all. + */ +export function startSessionErrorTracking(lifeCycle: LifeCycle, sessionManager: RumSessionManager) { + let hasReportedError = false + + const eventSubscription = lifeCycle.subscribe(LifeCycleEventType.RUM_EVENT_COLLECTED, (event) => { + if (hasReportedError || event.type !== RumEventType.ERROR) { + return + } + // The SDK's own failures — an intake request that could not be sent, for instance — are ours, + // not the application's. Counting them would turn every session into an error session for any + // customer whose network blocks our endpoint, billing them for replays of nothing. + if (event.error.source === ErrorSource.AGENT) { + return + } + // Only a session that is withholding something has any use for this mark. Setting it on any + // other session would write the session store for customers who enabled neither rate - and that + // 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) { + return + } + hasReportedError = true + sessionManager.setSessionHasError(session.id) + }) + + // A renewed session is a different session: it draws its own sampling and starts out without an + // error, so anything withheld for it must stay withheld until it reports one of its own. + const renewSubscription = lifeCycle.subscribe(LifeCycleEventType.SESSION_RENEWED, () => { + hasReportedError = false + }) + + return { + stop: () => { + eventSubscription.unsubscribe() + renewSubscription.unsubscribe() + }, + } +} diff --git a/packages/rum-core/test/mockRumSessionManager.ts b/packages/rum-core/test/mockRumSessionManager.ts index 6c43f9daec..a97a96fba0 100644 --- a/packages/rum-core/test/mockRumSessionManager.ts +++ b/packages/rum-core/test/mockRumSessionManager.ts @@ -1,42 +1,52 @@ import { Observable } from '@flashcatcloud/browser-core' -import { SessionReplayState, type RumSessionManager } from '../src/domain/rumSessionManager' +import { + RumTrackingType, + computeSessionReplayState, + withholdsReplay, + type RumSessionManager, +} from '../src/domain/rumSessionManager' export interface RumSessionManagerMock extends RumSessionManager { setId(id: string): RumSessionManagerMock setNotTracked(): RumSessionManagerMock setTrackedWithoutSessionReplay(): RumSessionManagerMock setTrackedWithSessionReplay(): RumSessionManagerMock + setTrackedWithErrorSessionReplay(): RumSessionManagerMock setForcedReplay(): RumSessionManagerMock + setSessionHasError(): RumSessionManagerMock } const DEFAULT_ID = 'session-id' const enum SessionStatus { TRACKED_WITH_SESSION_REPLAY, TRACKED_WITHOUT_SESSION_REPLAY, + TRACKED_WITH_ERROR_SESSION_REPLAY, NOT_TRACKED, EXPIRED, } +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, +} + export function createRumSessionManagerMock(): RumSessionManagerMock { let id = DEFAULT_ID let sessionStatus: SessionStatus = SessionStatus.TRACKED_WITH_SESSION_REPLAY let forcedReplay: boolean = false + let hasError: boolean = false return { findTrackedSession() { - if ( - sessionStatus !== SessionStatus.TRACKED_WITH_SESSION_REPLAY && - sessionStatus !== SessionStatus.TRACKED_WITHOUT_SESSION_REPLAY - ) { + const trackingType = TRACKING_TYPES[sessionStatus] + if (!trackingType) { return undefined } return { id, - sessionReplay: - sessionStatus === SessionStatus.TRACKED_WITH_SESSION_REPLAY - ? SessionReplayState.SAMPLED - : forcedReplay - ? SessionReplayState.FORCED - : SessionReplayState.OFF, + // Derived the same way as in production, so the mock cannot drift from the real state machine + sessionReplay: computeSessionReplayState(trackingType, hasError, forcedReplay), + sampledOnErrorReplay: withholdsReplay(trackingType), anonymousId: 'device-123', } }, @@ -61,9 +71,17 @@ export function createRumSessionManagerMock(): RumSessionManagerMock { sessionStatus = SessionStatus.TRACKED_WITH_SESSION_REPLAY return this }, + setTrackedWithErrorSessionReplay() { + sessionStatus = SessionStatus.TRACKED_WITH_ERROR_SESSION_REPLAY + return this + }, setForcedReplay() { forcedReplay = true return this }, + setSessionHasError() { + hasError = true + return this + }, } } diff --git a/packages/rum-react/package.json b/packages/rum-react/package.json index 0ab2acaf73..55c4336a26 100644 --- a/packages/rum-react/package.json +++ b/packages/rum-react/package.json @@ -1,6 +1,6 @@ { "name": "@flashcatcloud/browser-rum-react", - "version": "0.0.2", + "version": "0.1.0", "license": "Apache-2.0", "main": "cjs/entries/main.js", "module": "esm/entries/main.js", diff --git a/packages/rum-slim/package.json b/packages/rum-slim/package.json index 01a559216e..a83bbb4873 100644 --- a/packages/rum-slim/package.json +++ b/packages/rum-slim/package.json @@ -1,6 +1,6 @@ { "name": "@flashcatcloud/browser-rum-slim", - "version": "0.0.2", + "version": "0.1.0", "license": "Apache-2.0", "main": "cjs/entries/main.js", "module": "esm/entries/main.js", diff --git a/packages/rum/package.json b/packages/rum/package.json index 2e6bc64d23..c2bad47a7e 100644 --- a/packages/rum/package.json +++ b/packages/rum/package.json @@ -1,6 +1,6 @@ { "name": "@flashcatcloud/browser-rum", - "version": "0.0.2", + "version": "0.1.0", "license": "Apache-2.0", "main": "cjs/entries/main.js", "module": "esm/entries/main.js", diff --git a/packages/rum/src/boot/postStartStrategy.ts b/packages/rum/src/boot/postStartStrategy.ts index c9678c6f21..58e1854666 100644 --- a/packages/rum/src/boot/postStartStrategy.ts +++ b/packages/rum/src/boot/postStartStrategy.ts @@ -87,6 +87,13 @@ export function createPostStartStrategy( return } + if (shouldForceReplay(session!, options)) { + // Applied before the guard below, not after starting: a session that withholds its replay is + // already recording, so the guard would return without ever releasing it - and releasing what + // is held is the whole of what forcing means for such a session. + sessionManager.setForcedReplay() + } + if (isRecordingInProgress(status)) { return } @@ -95,10 +102,6 @@ export function createPostStartStrategy( // Intentionally not awaiting doStart() to keep it asynchronous doStart().catch(monitorError) - - if (shouldForceReplay(session!, options)) { - sessionManager.setForcedReplay() - } } function stop() { @@ -128,5 +131,11 @@ function isRecordingInProgress(status: RecorderStatus) { } function shouldForceReplay(session: RumSession, options?: StartRecordingOptions) { - return options && options.force && session.sessionReplay === SessionReplayState.OFF + return ( + options && + options.force && + // A withheld replay is as much in need of forcing as one that was never sampled: the host asked + // for this user's replay, so it must not go on waiting for an error that may never come. + (session.sessionReplay === SessionReplayState.OFF || session.sessionReplay === SessionReplayState.BUFFERED_ON_ERROR) + ) } diff --git a/packages/rum/src/boot/recorderApi.spec.ts b/packages/rum/src/boot/recorderApi.spec.ts index b61ae8ea00..3c839095ff 100644 --- a/packages/rum/src/boot/recorderApi.spec.ts +++ b/packages/rum/src/boot/recorderApi.spec.ts @@ -178,6 +178,27 @@ describe('makeRecorderApi', () => { expect(setForcedReplaySpy).toHaveBeenCalledTimes(1) }) + it('releases a withheld replay when forced, although it is already recording', async () => { + const setForcedReplaySpy = jasmine.createSpy() + + setupRecorderApi({ + sessionManager: { + ...createRumSessionManagerMock().setTrackedWithErrorSessionReplay(), + setForcedReplay: setForcedReplaySpy, + }, + startSessionReplayRecordingManually: false, + }) + + rumInit() + await collectAsyncCalls(startRecordingSpy, 1) + + // the recording is already running - what forcing asks for here is that what it holds stops + // waiting for an error + recorderApi.start({ force: true }) + + expect(setForcedReplaySpy).toHaveBeenCalledTimes(1) + }) + it('uses the previously created worker if available', async () => { setupRecorderApi({ startSessionReplayRecordingManually: true }) rumInit({ worker: mockWorker }) diff --git a/packages/rum/src/boot/startRecording.spec.ts b/packages/rum/src/boot/startRecording.spec.ts index 31f2b2bc84..a7342f185c 100644 --- a/packages/rum/src/boot/startRecording.spec.ts +++ b/packages/rum/src/boot/startRecording.spec.ts @@ -116,6 +116,47 @@ describe('startRecording', () => { expect(requests[0].metadata.records_count).toBe(1 + recordsPerFullSnapshot()) }) + it('sends the withheld replay once its session reports an error', async () => { + sessionManager.setTrackedWithErrorSessionReplay() + setupStartRecording() + + document.body.dispatchEvent(createNewEvent('click', { clientX: 1, clientY: 2 })) + // a page exit while the session is still waiting for an error keeps the buffer rather than + // sending it, so what follows joins the same segment + flushSegment(lifeCycle) + document.body.dispatchEvent(createNewEvent('click', { clientX: 3, clientY: 4 })) + + sessionManager.setSessionHasError() + flushSegment(lifeCycle) + + const requests = await readSentRequests(1) + expect(requestSendSpy).toHaveBeenCalledTimes(1) + // one segment, held since the recording started, carrying everything from before the error + expect(requests[0].metadata.creation_reason).toBe('init') + expect(requests[0].metadata.records_count).toBe(2 + recordsPerFullSnapshot()) + }) + + it('drops a withheld replay when the session stops withholding without having errored', async () => { + sessionManager.setTrackedWithErrorSessionReplay() + setupStartRecording() + + document.body.dispatchEvent(createNewEvent('click', { clientX: 1, clientY: 2 })) + + // 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, so what it held is not owed a + // trip to the intake. + sessionManager.setTrackedWithSessionReplay() + changeView(lifeCycle) + + document.body.dispatchEvent(createNewEvent('click', { clientX: 3, clientY: 4 })) + flushSegment(lifeCycle) + + const requests = await readSentRequests(1) + // 'init' would be the withheld segment; the first one to reach the intake is the one created + // after the session stopped withholding + expect(requests[0].metadata.creation_reason).toBe('view_change') + }) + it('restarts sending segments when the session is renewed', async () => { sessionManager.setNotTracked() setupStartRecording() diff --git a/packages/rum/src/boot/startRecording.ts b/packages/rum/src/boot/startRecording.ts index b3bfff31e9..8fc5cf3278 100644 --- a/packages/rum/src/boot/startRecording.ts +++ b/packages/rum/src/boot/startRecording.ts @@ -1,7 +1,7 @@ import type { RawError, HttpRequest, DeflateEncoder } from '@flashcatcloud/browser-core' -import { createHttpRequest, addTelemetryDebug, canUseEventBridge } from '@flashcatcloud/browser-core' +import { createHttpRequest, addTelemetryDebug, canUseEventBridge, noop } from '@flashcatcloud/browser-core' import type { LifeCycle, ViewHistory, RumConfiguration, RumSessionManager } from '@flashcatcloud/browser-rum-core' -import { LifeCycleEventType } from '@flashcatcloud/browser-rum-core' +import { LifeCycleEventType, SessionReplayState } from '@flashcatcloud/browser-rum-core' import { record } from '../domain/record' import { startSegmentCollection, SEGMENT_BYTES_LIMIT } from '../domain/segmentCollection' @@ -28,6 +28,10 @@ export function startRecording( let addRecord: (record: BrowserRecord) => void + // Assigned once recording has started. Segment collection is created first because `record()` + // emits into it, so the buffer reaches for the snapshot through this holder rather than directly. + let takeSubsequentFullSnapshot: () => void = noop + if (!canUseEventBridge()) { const segmentCollection = startSegmentCollection( lifeCycle, @@ -35,7 +39,27 @@ export function startRecording( sessionManager, viewHistory, replayRequest, - encoder + encoder, + { + getWithholdingSessionId: () => { + const session = sessionManager.findTrackedSession() + return session?.sessionReplay === SessionReplayState.BUFFERED_ON_ERROR ? session.id : undefined + }, + isReleased: (sessionId) => { + const session = sessionManager.findTrackedSession() + // Still the same session, still one whose replay is kept only on an error, and no longer + // withholding. The middle condition matters: a session can stop withholding without ever + // erroring - an older SDK sharing the same store does not know these tracking types and + // redraws them - and that is a session ending, not a replay earning its way out. + return ( + !!session && + session.id === sessionId && + session.sampledOnErrorReplay && + session.sessionReplay !== SessionReplayState.BUFFERED_ON_ERROR + ) + }, + restartFromFullSnapshot: () => takeSubsequentFullSnapshot(), + } ) addRecord = segmentCollection.addRecord cleanupTasks.push(segmentCollection.stop) @@ -43,13 +67,14 @@ export function startRecording( ;({ addRecord } = startRecordBridge(viewHistory)) } - const { stop: stopRecording } = record({ + const recording = record({ emit: addRecord, configuration, lifeCycle, viewHistory, }) - cleanupTasks.push(stopRecording) + takeSubsequentFullSnapshot = recording.takeSubsequentFullSnapshot + cleanupTasks.push(recording.stop) return { stop: () => { diff --git a/packages/rum/src/domain/getSessionReplayLink.ts b/packages/rum/src/domain/getSessionReplayLink.ts index 1bb7c38ea5..e8df168276 100644 --- a/packages/rum/src/domain/getSessionReplayLink.ts +++ b/packages/rum/src/domain/getSessionReplayLink.ts @@ -34,6 +34,11 @@ function getErrorType(session: RumSession | undefined, isRecordingStarted: boole // - replay sampled out return 'incorrect-session-plan' } + if (session.sessionReplay === SessionReplayState.BUFFERED_ON_ERROR) { + // the session records, but nothing has been uploaded yet and nothing may ever be: there is no + // replay to link to until the session reports an error + return 'replay-not-started' + } if (!isRecordingStarted) { return 'replay-not-started' } diff --git a/packages/rum/src/domain/profiling/profiler.spec.ts b/packages/rum/src/domain/profiling/profiler.spec.ts index eb10a48052..9c6cf38da6 100644 --- a/packages/rum/src/domain/profiling/profiler.spec.ts +++ b/packages/rum/src/domain/profiling/profiler.spec.ts @@ -1,6 +1,6 @@ -import { LifeCycle } from '@datadog/browser-rum-core' -import { relativeNow, timeStampNow } from '@datadog/browser-core' -import { setPageVisibility, restorePageVisibility, createNewEvent } from '@datadog/browser-core/test' +import { LifeCycle } from '@flashcatcloud/browser-rum-core' +import { relativeNow, timeStampNow } from '@flashcatcloud/browser-core' +import { setPageVisibility, restorePageVisibility, createNewEvent } from '@flashcatcloud/browser-core/test' import { createRumSessionManagerMock, mockPerformanceObserver, mockRumConfiguration } from '../../../../rum-core/test' import { mockProfiler } from '../../../test' import { mockedTrace } from './test-utils/mockedTrace' diff --git a/packages/rum/src/domain/record/record.ts b/packages/rum/src/domain/record/record.ts index 82e41cb1d6..c7187f1a44 100644 --- a/packages/rum/src/domain/record/record.ts +++ b/packages/rum/src/domain/record/record.ts @@ -33,6 +33,11 @@ export interface RecordOptions { export interface RecordAPI { stop: () => void flushMutations: () => void + /** + * Re-serializes the document so that the records that follow are replayable on their own. Needed + * when a withheld replay buffer is dropped, since it takes its full snapshot with it. + */ + takeSubsequentFullSnapshot: () => void shadowRootsController: ShadowRootsController } @@ -54,7 +59,7 @@ export function record(options: RecordOptions): RecordAPI { const shadowRootsController = initShadowRootsController(configuration, emitAndComputeStats, elementsScrollPositions) - const { stop: stopFullSnapshots } = startFullSnapshots( + const { stop: stopFullSnapshots, takeSubsequentFullSnapshot } = startFullSnapshots( elementsScrollPositions, shadowRootsController, lifeCycle, @@ -95,6 +100,7 @@ export function record(options: RecordOptions): RecordAPI { stopFullSnapshots() }, flushMutations, + takeSubsequentFullSnapshot, shadowRootsController, } } diff --git a/packages/rum/src/domain/record/startFullSnapshots.ts b/packages/rum/src/domain/record/startFullSnapshots.ts index 885d4ce31e..2438cd03a8 100644 --- a/packages/rum/src/domain/record/startFullSnapshots.ts +++ b/packages/rum/src/domain/record/startFullSnapshots.ts @@ -80,5 +80,19 @@ export function startFullSnapshots( return { stop: unsubscribe, + /** + * Re-serializes the document so that what follows is replayable on its own. Used when a withheld + * replay buffer is dropped: the records kept afterwards need a full snapshot to start from. + */ + takeSubsequentFullSnapshot: () => { + flushMutations() + fullSnapshotCallback( + takeFullSnapshot(timeStampNow(), { + shadowRootsController, + status: SerializationContextStatus.SUBSEQUENT_FULL_SNAPSHOT, + elementsScrollPositions, + }) + ) + }, } } diff --git a/packages/rum/src/domain/replayStats.ts b/packages/rum/src/domain/replayStats.ts index 76c5273f6c..3a69ce57c1 100644 --- a/packages/rum/src/domain/replayStats.ts +++ b/packages/rum/src/domain/replayStats.ts @@ -19,6 +19,33 @@ export function addWroteData(viewId: string, additionalBytesCount: number) { getOrCreateReplayStats(viewId).segments_total_raw_size += additionalBytesCount } +/** + * Gives back the segment count {@link addSegment} took, and with it the `index_in_view` the segment + * was holding. Undone in the same phase it was taken - synchronously - because the index is read at + * creation: a segment created before this runs would hold an index the dropped one still occupies. + */ +export function removeSegment(viewId: string) { + const replayStats = statsPerView?.get(viewId) + if (!replayStats) { + return + } + replayStats.segments_count = Math.max(0, replayStats.segments_count - 1) +} + +/** + * Rolls back what a dropped segment's records contributed. These are the counters reported on view + * events, and a withheld segment that is dropped never reached the intake, so it must leave no + * trace in them. + */ +export function discardSegmentData(viewId: string, rawBytesCount: number, recordsCount: number) { + const replayStats = statsPerView?.get(viewId) + if (!replayStats) { + return + } + replayStats.records_count = Math.max(0, replayStats.records_count - recordsCount) + replayStats.segments_total_raw_size = Math.max(0, replayStats.segments_total_raw_size - rawBytesCount) +} + export function getReplayStats(viewId: string) { return statsPerView?.get(viewId) } diff --git a/packages/rum/src/domain/segmentCollection/segmentCollection.spec.ts b/packages/rum/src/domain/segmentCollection/segmentCollection.spec.ts index ad68e05fd8..c312a2f2fb 100644 --- a/packages/rum/src/domain/segmentCollection/segmentCollection.spec.ts +++ b/packages/rum/src/domain/segmentCollection/segmentCollection.spec.ts @@ -1,5 +1,5 @@ import type { ClocksState, HttpRequest, TimeStamp } from '@flashcatcloud/browser-core' -import { DeflateEncoderStreamId, PageExitReason } 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 type { Clock } from '@flashcatcloud/browser-core/test' @@ -9,7 +9,9 @@ import type { BrowserRecord, SegmentContext } from '../../types' import { RecordType } from '../../types' import { MockWorker, readMetadataFromReplayPayload } from '../../../test' import { createDeflateEncoder } from '../deflate' +import * as replayStats from '../replayStats' import { + BUFFER_CHECKOUT_TIME, computeSegmentContext, doStartSegmentCollection, SEGMENT_BYTES_LIMIT, @@ -70,7 +72,8 @@ describe('startSegmentCollection', () => { lifeCycle, () => context, httpRequestSpy, - createDeflateEncoder(configuration, worker, DeflateEncoderStreamId.REPLAY) + createDeflateEncoder(configuration, worker, DeflateEncoderStreamId.REPLAY), + { getWithholdingSessionId: () => undefined, isReleased: () => false, restartFromFullSnapshot: noop } )) registerCleanupTask(() => { @@ -312,3 +315,298 @@ describe('computeSegmentContext', () => { } as any } }) + +describe('startSegmentCollection withholding (error session replay)', () => { + let clock: Clock + let lifeCycle: LifeCycle + let worker: MockWorker + let httpRequestSpy: { + sendOnExit: jasmine.Spy + send: jasmine.Spy + } + let addRecord: (record: BrowserRecord) => void + let withholdingSessionId: string | undefined + let releasedSessionId: string | undefined + let restartFromFullSnapshotSpy: jasmine.Spy<() => void> + let stopCollection: () => void + + function reportError() { + releasedSessionId = withholdingSessionId + withholdingSessionId = undefined + } + + beforeEach(() => { + clock = mockClock() + lifeCycle = new LifeCycle() + worker = new MockWorker() + httpRequestSpy = { sendOnExit: jasmine.createSpy(), send: jasmine.createSpy() } + withholdingSessionId = CONTEXT.session.id + releasedSessionId = undefined + restartFromFullSnapshotSpy = jasmine.createSpy() + replayStats.resetReplayStats() + + const { stop, addRecord: add } = doStartSegmentCollection( + lifeCycle, + () => CONTEXT, + httpRequestSpy, + createDeflateEncoder({} as RumConfiguration, worker, DeflateEncoderStreamId.REPLAY), + { + getWithholdingSessionId: () => withholdingSessionId, + isReleased: (sessionId) => releasedSessionId === sessionId, + restartFromFullSnapshot: restartFromFullSnapshotSpy, + } + ) + addRecord = add + stopCollection = stop + + registerCleanupTask(() => { + stop() + clock.cleanup() + replayStats.resetReplayStats() + }) + }) + + it('does not send anything while the session has not reported an error', () => { + addRecord(RECORD) + clock.tick(SEGMENT_DURATION_LIMIT) + worker.processAllMessages() + + expect(httpRequestSpy.send).not.toHaveBeenCalled() + expect(httpRequestSpy.sendOnExit).not.toHaveBeenCalled() + }) + + it('keeps buffering across several duration limits instead of cutting the segment', () => { + addRecord(RECORD) + clock.tick(SEGMENT_DURATION_LIMIT * 3) + addRecord(RECORD) + worker.processAllMessages() + + expect(httpRequestSpy.send).not.toHaveBeenCalled() + // still the same buffer: dropping it would have asked for a fresh full snapshot + expect(restartFromFullSnapshotSpy).not.toHaveBeenCalled() + }) + + it('sends the withheld buffer once the session reports an error', async () => { + addRecord(RECORD) + addRecord(RECORD) + clock.tick(SEGMENT_DURATION_LIMIT) + worker.processAllMessages() + expect(httpRequestSpy.send).not.toHaveBeenCalled() + + reportError() + clock.tick(SEGMENT_DURATION_LIMIT) + worker.processAllMessages() + + expect(httpRequestSpy.send).toHaveBeenCalledTimes(1) + // the records collected before the error are part of what is sent + expect((await readMetadataFromReplayPayload(httpRequestSpy.send.calls.mostRecent().args[0])).records_count).toBe(2) + }) + + it('drops the buffer and restarts from a full snapshot once it spans the checkout time', () => { + addRecord(RECORD) + clock.tick(BUFFER_CHECKOUT_TIME) + worker.processAllMessages() + + expect(httpRequestSpy.send).not.toHaveBeenCalled() + expect(restartFromFullSnapshotSpy).toHaveBeenCalledTimes(1) + }) + + it('drops the buffer and restarts from a full snapshot when it grows past the bytes limit', () => { + addRecord(VERY_BIG_RECORD) + worker.processAllMessages() + + expect(httpRequestSpy.send).not.toHaveBeenCalled() + expect(restartFromFullSnapshotSpy).toHaveBeenCalledTimes(1) + }) + + it('does not restart in a hot loop when the full snapshot alone exceeds the bytes limit', () => { + // every restart would blow the limit again straight away on such a document + addRecord(VERY_BIG_RECORD) + worker.processAllMessages() + addRecord(VERY_BIG_RECORD) + worker.processAllMessages() + addRecord(VERY_BIG_RECORD) + worker.processAllMessages() + + expect(restartFromFullSnapshotSpy).toHaveBeenCalledTimes(1) + + clock.tick(SEGMENT_DURATION_LIMIT) + addRecord(VERY_BIG_RECORD) + worker.processAllMessages() + + expect(restartFromFullSnapshotSpy).toHaveBeenCalledTimes(2) + }) + + it('keeps the buffer when the page is only hidden, so the replay can still start from its snapshot', () => { + // switching tabs is ordinary; dropping here would take the only full snapshot with it + addRecord(RECORD) + addRecord(RECORD) + lifeCycle.notify(LifeCycleEventType.PAGE_MAY_EXIT, { reason: PageExitReason.HIDDEN }) + worker.processAllMessages() + + expect(httpRequestSpy.sendOnExit).not.toHaveBeenCalled() + expect(restartFromFullSnapshotSpy).not.toHaveBeenCalled() + + reportError() + clock.tick(SEGMENT_DURATION_LIMIT) + worker.processAllMessages() + + expect(httpRequestSpy.send).toHaveBeenCalledTimes(1) + }) + + it('sends nothing on page exit for a session that never errored', () => { + addRecord(RECORD) + lifeCycle.notify(LifeCycleEventType.PAGE_MAY_EXIT, { reason: PageExitReason.UNLOADING }) + worker.processAllMessages() + + expect(httpRequestSpy.sendOnExit).not.toHaveBeenCalled() + }) + + it('does not let a dropped buffer leave its index_in_view behind for the next one to collide with', async () => { + // the restart emits records, exactly as taking a fresh full snapshot does in production + restartFromFullSnapshotSpy.and.callFake(() => addRecord(RECORD)) + + addRecord(RECORD) + clock.tick(BUFFER_CHECKOUT_TIME) + worker.processAllMessages() + expect(restartFromFullSnapshotSpy).toHaveBeenCalledTimes(1) + + reportError() + clock.tick(SEGMENT_DURATION_LIMIT) + worker.processAllMessages() + + // the dropped buffer never reached the intake, so the first segment that does is index 0 + const metadata = await readMetadataFromReplayPayload(httpRequestSpy.send.calls.mostRecent().args[0]) + expect(metadata.index_in_view).toBe(0) + // and it carries a reason the segment schema knows, not the internal one that dropped the buffer + expect(metadata.creation_reason).toBe('segment_duration_limit') + }) + + 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) + stopCollection() + worker.processAllMessages() + + expect(restartFromFullSnapshotSpy).not.toHaveBeenCalled() + }) + + it('does not hand the next segment an index the dropped one still holds when a record lands mid-flush', async () => { + restartFromFullSnapshotSpy.and.callFake(() => addRecord(RECORD)) + + addRecord(RECORD) + // 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) + addRecord(RECORD) + worker.processAllMessages() + + reportError() + clock.tick(SEGMENT_DURATION_LIMIT) + worker.processAllMessages() + + expect((await readMetadataFromReplayPayload(httpRequestSpy.send.calls.mostRecent().args[0])).index_in_view).toBe(0) + }) + + it('leaves no trace of a dropped buffer in the replay stats', () => { + addRecord(RECORD) + clock.tick(BUFFER_CHECKOUT_TIME) + worker.processAllMessages() + + const stats = replayStats.getReplayStats(CONTEXT.view.id) + expect(stats?.segments_count ?? 0).toBe(0) + expect(stats?.segments_total_raw_size ?? 0).toBe(0) + }) + + it('sends normally once released, without withholding the following segments', () => { + reportError() + addRecord(RECORD) + clock.tick(SEGMENT_DURATION_LIMIT) + worker.processAllMessages() + addRecord(RECORD) + clock.tick(SEGMENT_DURATION_LIMIT) + worker.processAllMessages() + + expect(httpRequestSpy.send).toHaveBeenCalledTimes(2) + }) +}) + +describe('startSegmentCollection withholding, session lifecycle', () => { + let clock: Clock + let lifeCycle: LifeCycle + let worker: MockWorker + let httpRequestSpy: { + sendOnExit: jasmine.Spy + send: jasmine.Spy + } + let addRecord: (record: BrowserRecord) => void + let stopSegmentCollection: () => void + let withholdingSessionId: string | undefined + let releasedSessionId: string | undefined + + beforeEach(() => { + clock = mockClock() + lifeCycle = new LifeCycle() + worker = new MockWorker() + httpRequestSpy = { sendOnExit: jasmine.createSpy(), send: jasmine.createSpy() } + withholdingSessionId = CONTEXT.session.id + releasedSessionId = undefined + + const { stop, addRecord: add } = doStartSegmentCollection( + lifeCycle, + () => CONTEXT, + httpRequestSpy, + createDeflateEncoder({} as RumConfiguration, worker, DeflateEncoderStreamId.REPLAY), + { + getWithholdingSessionId: () => withholdingSessionId, + isReleased: (sessionId) => releasedSessionId === sessionId, + restartFromFullSnapshot: () => undefined, + } + ) + addRecord = add + stopSegmentCollection = stop + + registerCleanupTask(() => { + stopSegmentCollection() + clock.cleanup() + }) + }) + + it('drops the buffer when the session expires without ever reporting an error', () => { + addRecord(RECORD) + // the session is gone, so nothing answers for these records any more + withholdingSessionId = undefined + releasedSessionId = undefined + + stopSegmentCollection() + worker.processAllMessages() + + expect(httpRequestSpy.send).not.toHaveBeenCalled() + expect(httpRequestSpy.sendOnExit).not.toHaveBeenCalled() + }) + + it('drops the buffer when the session is renewed into a different one', () => { + addRecord(RECORD) + withholdingSessionId = undefined + releasedSessionId = 'a-different-session' + + stopSegmentCollection() + worker.processAllMessages() + + expect(httpRequestSpy.send).not.toHaveBeenCalled() + expect(httpRequestSpy.sendOnExit).not.toHaveBeenCalled() + }) + + it('sends the buffer when its own session reports the error', () => { + addRecord(RECORD) + releasedSessionId = withholdingSessionId + withholdingSessionId = undefined + + stopSegmentCollection() + worker.processAllMessages() + + expect(httpRequestSpy.send).toHaveBeenCalledTimes(1) + }) +}) diff --git a/packages/rum/src/domain/segmentCollection/segmentCollection.ts b/packages/rum/src/domain/segmentCollection/segmentCollection.ts index 259ca609d3..3e7346d935 100644 --- a/packages/rum/src/domain/segmentCollection/segmentCollection.ts +++ b/packages/rum/src/domain/segmentCollection/segmentCollection.ts @@ -1,13 +1,28 @@ -import type { DeflateEncoder, HttpRequest, TimeoutId } from '@flashcatcloud/browser-core' -import { isPageExitReason, ONE_SECOND, clearTimeout, setTimeout } from '@flashcatcloud/browser-core' +import type { DeflateEncoder, HttpRequest, RelativeTime, TimeoutId } from '@flashcatcloud/browser-core' +import { + addTelemetryDebug, + isPageExitReason, + ONE_SECOND, + clearTimeout, + relativeNow, + setTimeout, +} from '@flashcatcloud/browser-core' import type { LifeCycle, ViewHistory, RumSessionManager, RumConfiguration } from '@flashcatcloud/browser-rum-core' import { LifeCycleEventType } from '@flashcatcloud/browser-rum-core' import type { BrowserRecord, CreationReason, SegmentContext } from '../../types' +import { discardSegmentData, removeSegment } from '../replayStats' import { buildReplayPayload } from './buildReplayPayload' import type { FlushReason, Segment } from './segment' 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 @@ -39,19 +54,43 @@ export let SEGMENT_BYTES_LIMIT = 60_000 // To help investigate session replays issues, each segment is created with a "creation reason", // indicating why the session has been created. +/** + * Lets a session record without uploading anything until it reports an error. Sessions drawn by + * `sessionReplayOnErrorSampleRate` record from the start, but every segment is withheld: dropped on + * checkout while no error has happened, sent normally from the moment one has. + */ +export interface SegmentBuffering { + /** + * The id of the current session if it is withholding its replay, `undefined` otherwise. A segment + * remembers this at creation, so that what happens to it later is decided by the session that + * actually produced its records. + */ + getWithholdingSessionId: () => string | undefined + /** + * Whether that same session has since reported its error. Anything else — the session expired, or + * was renewed into a different one — means the records were never released and must be dropped: + * uploading them would bill a session for a replay nobody asked for and nobody can explain. + */ + isReleased: (sessionId: string) => boolean + /** Restarts the buffer from a fresh full snapshot, after the previous one was dropped. */ + restartFromFullSnapshot: () => void +} + export function startSegmentCollection( lifeCycle: LifeCycle, configuration: RumConfiguration, sessionManager: RumSessionManager, viewHistory: ViewHistory, httpRequest: HttpRequest, - encoder: DeflateEncoder + encoder: DeflateEncoder, + buffering: SegmentBuffering ) { return doStartSegmentCollection( lifeCycle, () => computeSegmentContext(configuration.applicationId, sessionManager, viewHistory), httpRequest, - encoder + encoder, + buffering ) } @@ -69,22 +108,41 @@ type SegmentCollectionState = status: SegmentCollectionStatus.SegmentPending segment: Segment expirationTimeoutId: TimeoutId + /** Only armed while the segment is withheld: bounds how much history the buffer may span. */ + bufferCheckoutTimeoutId: TimeoutId | undefined + /** Set when the segment was created while its session was withholding its replay. */ + withheldForSessionId: string | undefined + /** The view the segment belongs to, so its index can be given back without waiting on a flush. */ + viewId: string } | { status: SegmentCollectionStatus.Stopped } +/** + * `buffer_checkout` is internal: it drops a withheld buffer that has grown past + * {@link BUFFER_CHECKOUT_TIME}. It never reaches the intake, so it is mapped back to a schema value + * where the next segment records why it was created. + */ +type InternalFlushReason = FlushReason | 'buffer_checkout' + export function doStartSegmentCollection( lifeCycle: LifeCycle, getSegmentContext: () => SegmentContext | undefined, httpRequest: HttpRequest, - encoder: DeflateEncoder + encoder: DeflateEncoder, + buffering: SegmentBuffering ) { let state: SegmentCollectionState = { status: SegmentCollectionStatus.WaitingForInitialRecord, nextSegmentCreationReason: 'init', } + // How many buffers were dropped before one was finally released. Without this, "the replay goes + // back up to a minute" is a promise nobody can check. + let droppedBufferCount = 0 + let lastBufferRestartAt: RelativeTime | undefined + const { unsubscribe: unsubscribeViewCreated } = lifeCycle.subscribe(LifeCycleEventType.VIEW_CREATED, () => { flushSegment('view_change') }) @@ -96,9 +154,61 @@ export function doStartSegmentCollection( } ) - function flushSegment(flushReason: FlushReason) { + function flushSegment(flushReason: InternalFlushReason) { + // Decided once, and against the session that produced the records rather than whatever session + // is current now: a segment must be either dropped or sent as a whole. + const withheldForSessionId = + state.status === SegmentCollectionStatus.SegmentPending ? state.withheldForSessionId : undefined + const isWithheld = withheldForSessionId !== undefined && !buffering.isReleased(withheldForSessionId) + if (state.status === SegmentCollectionStatus.SegmentPending) { + if (isWithheld && (flushReason === 'segment_duration_limit' || isPageExitReason(flushReason))) { + // Nothing can be sent while withheld, so these rotations would only throw the buffer away - + // and with it the full snapshot a released replay has to start from, leaving the rest of the + // session as incremental records nothing can be played from. A page that is merely hidden or + // frozen comes back and goes on recording; one that is really unloading takes the buffer with + // it either way. Keeping it is never worse than dropping it. + if (flushReason === 'segment_duration_limit') { + // Re-armed, so the buffer is flushed normally within one rotation of the session erroring. + // An expiring session does not lose it: the session history entry is still open when the + // recorder is stopped (`sessionManager.ts` notifies before closing it), so the stop flush + // still sees the session as released and sends. Only losing the page outright loses it. + state.expirationTimeoutId = setTimeout(() => flushSegment('segment_duration_limit'), SEGMENT_DURATION_LIMIT) + } + return + } + + if (isWithheld) { + // Given back here, synchronously, rather than in the flush callback below: that callback only + // runs after a round trip to the deflate worker, and a record arriving in between creates a + // segment that reads its `index_in_view` from a count this one still occupies - leaving two + // uploaded segments claiming the same index, and index 0 never uploaded at all. + removeSegment(state.viewId) + } + state.segment.flush((metadata, encoderResult) => { + if (isWithheld) { + // No error was reported, so this buffer is dropped rather than sent. Rolling back what its + // records contributed keeps `has_replay` and the counters on view events honest. + discardSegmentData(metadata.view.id, encoderResult.rawBytesCount, metadata.records_count) + droppedBufferCount += 1 + // Restarted from here rather than synchronously below, so the fresh full snapshot lands in + // the segment that follows this one rather than in the one being thrown away. + restartBuffer(flushReason) + return + } + + if (withheldForSessionId !== undefined) { + // The first segment released by an error: report how much history it actually carried, so + // the window we promise can be compared against the one users get. + addTelemetryDebug('Error session replay buffer released', { + 'buffer.duration': metadata.end - metadata.start, + 'buffer.records_count': metadata.records_count, + 'buffer.dropped_count': droppedBufferCount, + }) + droppedBufferCount = 0 + } + const payload = buildReplayPayload(encoderResult.output, metadata, encoderResult.rawBytesCount) if (isPageExitReason(flushReason)) { @@ -108,12 +218,13 @@ export function doStartSegmentCollection( } }) clearTimeout(state.expirationTimeoutId) + clearTimeout(state.bufferCheckoutTimeoutId) } if (flushReason !== 'stop') { state = { status: SegmentCollectionStatus.WaitingForInitialRecord, - nextSegmentCreationReason: flushReason, + nextSegmentCreationReason: flushReason === 'buffer_checkout' ? 'segment_duration_limit' : flushReason, } } else { state = { @@ -122,6 +233,31 @@ export function doStartSegmentCollection( } } + /** + * A dropped buffer leaves no full snapshot behind, so the next one would not be replayable on its + * own. A view change does not need this: the new view emits its own full snapshot. + */ + function restartBuffer(flushReason: InternalFlushReason) { + if (flushReason !== 'buffer_checkout' && flushReason !== 'segment_bytes_limit') { + return + } + if (state.status === SegmentCollectionStatus.Stopped) { + // The flush that got here waited on the deflate worker, and recording was stopped in the + // meantime. Re-serializing the document now would cost a full snapshot on a page that asked + // to stop, and count records into the replay stats that no segment will ever hold. + return + } + // On a document whose full snapshot alone exceeds the segment limit, every restart would blow + // the limit again straight away and restart once more. Spacing restarts out avoids that hot + // loop, at the cost of a buffer that carries no full snapshot until the next restart is allowed + // - if the error lands in that window, what is released cannot be played from its start. + const now = relativeNow() + if (lastBufferRestartAt === undefined || now - lastBufferRestartAt >= SEGMENT_DURATION_LIMIT) { + lastBufferRestartAt = now + buffering.restartFromFullSnapshot() + } + } + return { addRecord: (record: BrowserRecord) => { if (state.status === SegmentCollectionStatus.Stopped) { @@ -134,12 +270,21 @@ export function doStartSegmentCollection( return } + const withheldForSessionId = buffering.getWithholdingSessionId() state = { status: SegmentCollectionStatus.SegmentPending, segment: createSegment({ encoder, context, creationReason: state.nextSegmentCreationReason }), expirationTimeoutId: setTimeout(() => { flushSegment('segment_duration_limit') }, SEGMENT_DURATION_LIMIT), + bufferCheckoutTimeoutId: + withheldForSessionId !== undefined + ? setTimeout(() => { + flushSegment('buffer_checkout') + }, BUFFER_CHECKOUT_TIME) + : undefined, + withheldForSessionId, + viewId: context.view.id, } } diff --git a/packages/rum/test/mockProfiler.ts b/packages/rum/test/mockProfiler.ts index fca2861ad3..d4db678e5b 100644 --- a/packages/rum/test/mockProfiler.ts +++ b/packages/rum/test/mockProfiler.ts @@ -1,5 +1,5 @@ -import { registerCleanupTask } from '@flashcatcloud/browser-rum/test' -import { getGlobalObject } from '@flashcatcloud/browser-rum' +import { registerCleanupTask } from '@flashcatcloud/browser-core/test' +import { getGlobalObject } from '@flashcatcloud/browser-core' import type { Profiler, ProfilerTrace, ProfilerInitOptions } from '../src/domain/profiling/types' export function mockProfiler(mockedTrace: ProfilerTrace) { diff --git a/packages/worker/package.json b/packages/worker/package.json index 266fe2e8f6..b20c207129 100644 --- a/packages/worker/package.json +++ b/packages/worker/package.json @@ -1,6 +1,6 @@ { "name": "@flashcatcloud/browser-worker", - "version": "0.0.2", + "version": "0.1.0", "license": "Apache-2.0", "main": "bundle/worker.js", "scripts": { diff --git a/performances/package.json b/performances/package.json index 02eca5dbe0..780dddd734 100644 --- a/performances/package.json +++ b/performances/package.json @@ -1,7 +1,7 @@ { "private": true, "name": "performances", - "version": "0.0.2", + "version": "0.1.0", "scripts": { "start": "ts-node ./src/main.ts" },