From cc25db8045b9268b2f47344e6265fb60ca2f1378 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Zugmeyer?= Date: Mon, 22 Mar 2021 17:41:50 +0100 Subject: [PATCH 1/6] [RUMF-867] implement stopSessionReplayRecording --- packages/rum-recorder/src/boot/recorder.ts | 2 +- .../src/boot/rumRecorderPublicApi.spec.ts | 6 +++-- .../src/boot/rumRecorderPublicApi.ts | 24 +++++++++++++++---- 3 files changed, 24 insertions(+), 8 deletions(-) diff --git a/packages/rum-recorder/src/boot/recorder.ts b/packages/rum-recorder/src/boot/recorder.ts index 4696e94524..fc38041eea 100644 --- a/packages/rum-recorder/src/boot/recorder.ts +++ b/packages/rum-recorder/src/boot/recorder.ts @@ -34,7 +34,7 @@ export function startRecording( trackViewEndRecord(lifeCycle, (record) => addRawRecord(record)) return { - stop() { + stop: () => { stopRecording() stopSegmentCollection() stopTrackingFocusRecords() diff --git a/packages/rum-recorder/src/boot/rumRecorderPublicApi.spec.ts b/packages/rum-recorder/src/boot/rumRecorderPublicApi.spec.ts index d966696dde..87e4e867b2 100644 --- a/packages/rum-recorder/src/boot/rumRecorderPublicApi.spec.ts +++ b/packages/rum-recorder/src/boot/rumRecorderPublicApi.spec.ts @@ -1,5 +1,5 @@ /* eslint-disable @typescript-eslint/unbound-method */ -import { Configuration } from '@datadog/browser-core' +import { Configuration, noop } from '@datadog/browser-core' import { RumPublicApi, RumUserConfiguration, StartRum } from '@datadog/browser-rum-core' import { makeRumRecorderPublicApi, StartRecording } from './rumRecorderPublicApi' @@ -18,7 +18,9 @@ describe('makeRumRecorderPublicApi', () => { beforeEach(() => { enabledFlags = [] - startRecordingSpy = jasmine.createSpy() + startRecordingSpy = jasmine.createSpy().and.callFake(() => ({ + stop: noop, + })) startRumSpy = jasmine.createSpy().and.callFake(() => { const configuration: Partial = { isEnabled(flag: string) { diff --git a/packages/rum-recorder/src/boot/rumRecorderPublicApi.ts b/packages/rum-recorder/src/boot/rumRecorderPublicApi.ts index acfb7b15e4..519630f978 100644 --- a/packages/rum-recorder/src/boot/rumRecorderPublicApi.ts +++ b/packages/rum-recorder/src/boot/rumRecorderPublicApi.ts @@ -7,17 +7,18 @@ export type StartRecording = typeof startRecording export function makeRumRecorderPublicApi(startRumImpl: StartRum, startRecordingImpl: StartRecording) { const rumRecorderGlobal = makeRumPublicApi((userConfiguration, getCommonContext) => { - let isRecording: true | undefined + let stopRecording: (() => void) | undefined const startRumResult = startRumImpl(userConfiguration, () => ({ ...getCommonContext(), - hasReplay: isRecording, + hasReplay: stopRecording ? true : undefined, })) const { lifeCycle, parentContexts, configuration, session } = startRumResult if (configuration.isEnabled('postpone_start_recording')) { ;(rumRecorderGlobal as any).startSessionReplayRecording = monitor(startSessionReplayRecording) + ;(rumRecorderGlobal as any).stopSessionReplayRecording = monitor(stopSessionReplayRecording) if (!(userConfiguration as any).manualSessionReplayRecordingStart) { startSessionReplayRecording() } @@ -26,12 +27,25 @@ export function makeRumRecorderPublicApi(startRumImpl: StartRum, startRecordingI } function startSessionReplayRecording() { - if (isRecording) { + if (stopRecording) { + return + } + ;({ stop: stopRecording } = startRecordingImpl( + lifeCycle, + userConfiguration.applicationId, + configuration, + session, + parentContexts + )) + } + + function stopSessionReplayRecording() { + if (!stopRecording) { return } - isRecording = true - startRecordingImpl(lifeCycle, userConfiguration.applicationId, configuration, session, parentContexts) + stopRecording() + stopRecording = undefined } return startRumResult From 3376ace04cade7c3b2215af6ac690d650b289c5c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Zugmeyer?= Date: Tue, 23 Mar 2021 14:14:45 +0100 Subject: [PATCH 2/6] [RUMF-867] flush the pending segment when stop recording --- .../src/domain/segmentCollection.spec.ts | 13 ++++++++++--- .../src/domain/segmentCollection.ts | 17 ++++++++++++----- 2 files changed, 22 insertions(+), 8 deletions(-) diff --git a/packages/rum-recorder/src/domain/segmentCollection.spec.ts b/packages/rum-recorder/src/domain/segmentCollection.spec.ts index 3c059d4f35..d87b0e915e 100644 --- a/packages/rum-recorder/src/domain/segmentCollection.spec.ts +++ b/packages/rum-recorder/src/domain/segmentCollection.spec.ts @@ -12,7 +12,7 @@ const RECORD: Record = { type: RecordType.ViewEnd, timestamp: 10 } const BEFORE_MAX_SEGMENT_DURATION = MAX_SEGMENT_DURATION * 0.9 describe('startSegmentCollection', () => { - let stopErrorCollection: () => void + let stopSegmentCollection: () => void function startSegmentCollection(context: SegmentContext | undefined) { const lifeCycle = new LifeCycle() @@ -21,7 +21,7 @@ describe('startSegmentCollection', () => { const sendSpy = jasmine.createSpy<(data: Uint8Array, meta: SegmentMeta) => void>() const { stop, addRecord } = doStartSegmentCollection(lifeCycle, () => context, sendSpy, worker, eventEmitter) - stopErrorCollection = stop + stopSegmentCollection = stop const segmentFlushSpy = spyOn(Segment.prototype, 'flush').and.callThrough() return { addRecord, @@ -42,7 +42,7 @@ describe('startSegmentCollection', () => { afterEach(() => { jasmine.clock().uninstall() - stopErrorCollection() + stopSegmentCollection() }) it('immediately starts a new segment', () => { @@ -133,6 +133,13 @@ describe('startSegmentCollection', () => { expect(segmentFlushSpy).toHaveBeenCalledTimes(1) expect(sendCurrentSegment().creation_reason).not.toBe('max_duration') }) + + it('flushes a segment when calling stop()', () => { + const { segmentFlushSpy, addRecord } = startSegmentCollection(CONTEXT) + addRecord(RECORD) + stopSegmentCollection() + expect(segmentFlushSpy).toHaveBeenCalled() + }) }) }) diff --git a/packages/rum-recorder/src/domain/segmentCollection.ts b/packages/rum-recorder/src/domain/segmentCollection.ts index b9cc63d932..f2c290ce49 100644 --- a/packages/rum-recorder/src/domain/segmentCollection.ts +++ b/packages/rum-recorder/src/domain/segmentCollection.ts @@ -33,6 +33,8 @@ export const MAX_SEGMENT_DURATION = 30_000 // To help investigate session replays issues, each segment is created with a "creation reason", // indicating why the session has been created. +let workerSingleton: DeflateWorker + export function startSegmentCollection( lifeCycle: LifeCycle, applicationId: string, @@ -40,12 +42,14 @@ export function startSegmentCollection( parentContexts: ParentContexts, send: (data: Uint8Array, meta: SegmentMeta) => void ) { - const worker = createDeflateWorker() + if (!workerSingleton) { + workerSingleton = createDeflateWorker() + } return doStartSegmentCollection( lifeCycle, () => computeSegmentContext(applicationId, session, parentContexts), send, - worker + workerSingleton ) } @@ -58,7 +62,7 @@ export function doStartSegmentCollection( ) { let currentSegment: Segment | undefined let currentSegmentExpirationTimeoutId: number - let nextSegmentCreationReason: CreationReason = 'init' + let nextSegmentCreationReason: CreationReason | undefined = 'init' const writer = new DeflateSegmentWriter( worker, @@ -91,7 +95,7 @@ export function doStartSegmentCollection( { capture: true } ) - function flushSegment(creationReason: CreationReason) { + function flushSegment(creationReason?: CreationReason) { if (currentSegment) { currentSegment.flush() currentSegment = undefined @@ -104,6 +108,9 @@ export function doStartSegmentCollection( return { addRecord: (record: Record) => { if (!currentSegment) { + if (!nextSegmentCreationReason) { + return + } const context = getSegmentContext() if (!context) { return @@ -121,10 +128,10 @@ export function doStartSegmentCollection( } }, stop: () => { + flushSegment() unsubscribeViewCreated() unsubscribeBeforeUnload() unsubscribeVisibilityChange() - worker.terminate() }, } } From b20e09dcf9e542b0c3cd7ae051e03b84043d4948 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Zugmeyer?= Date: Mon, 22 Mar 2021 17:44:06 +0100 Subject: [PATCH 3/6] [RUMF-867] sepcial has_replay processing on views --- packages/rum-core/src/domain/assembly.spec.ts | 12 +++- packages/rum-core/src/domain/assembly.ts | 5 +- packages/rum-core/src/domain/lifeCycle.ts | 8 ++- .../view/trackViews.spec.ts | 69 +++++++++++++++++++ .../rumEventsCollection/view/trackViews.ts | 30 +++++++- .../view/viewCollection.spec.ts | 7 +- .../view/viewCollection.ts | 3 + packages/rum-core/src/rawRumEvent.types.ts | 3 + packages/rum-core/test/fixtures.ts | 3 + .../src/boot/rumRecorderPublicApi.ts | 4 +- 10 files changed, 137 insertions(+), 7 deletions(-) diff --git a/packages/rum-core/src/domain/assembly.spec.ts b/packages/rum-core/src/domain/assembly.spec.ts index 4870e45cb6..00bab64a41 100644 --- a/packages/rum-core/src/domain/assembly.spec.ts +++ b/packages/rum-core/src/domain/assembly.spec.ts @@ -367,10 +367,20 @@ describe('rum assembly', () => { commonContext.hasReplay = true lifeCycle.notify(LifeCycleEventType.RAW_RUM_EVENT_COLLECTED, { - rawRumEvent: createRawRumEvent(RumEventType.VIEW), + rawRumEvent: createRawRumEvent(RumEventType.ERROR), startTime: 0 as RelativeTime, }) expect(serverRumEvents[0].session.has_replay).toBe(true) }) + + it('should not use commonContext.hasReplay on view events', () => { + commonContext.hasReplay = true + + lifeCycle.notify(LifeCycleEventType.RAW_RUM_EVENT_COLLECTED, { + rawRumEvent: createRawRumEvent(RumEventType.VIEW), + startTime: 0 as RelativeTime, + }) + expect(serverRumEvents[0].session.has_replay).toBe(undefined) + }) }) }) diff --git a/packages/rum-core/src/domain/assembly.ts b/packages/rum-core/src/domain/assembly.ts index 9f8c075fb4..8be81c142e 100644 --- a/packages/rum-core/src/domain/assembly.ts +++ b/packages/rum-core/src/domain/assembly.ts @@ -58,7 +58,6 @@ export function startRumAssembly( date: timeStampNow(), service: configuration.service, session: { - has_replay: commonContext.hasReplay, // must be computed on each event because synthetics instrumentation can be done after sdk execution // cf https://github.com/puppeteer/puppeteer/issues/3667 type: getSessionType(), @@ -73,6 +72,10 @@ export function startRumAssembly( serverRumEvent.context = context } + if (!('has_replay' in serverRumEvent.session)) { + ;(serverRumEvent.session as { has_replay?: boolean }).has_replay = commonContext.hasReplay + } + if (!isEmptyObject(commonContext.user)) { // eslint-disable-next-line @typescript-eslint/no-unnecessary-type-assertion ;(serverRumEvent.usr as RumEvent['usr']) = commonContext.user as User & Context diff --git a/packages/rum-core/src/domain/lifeCycle.ts b/packages/rum-core/src/domain/lifeCycle.ts index 9a865dd88b..651768750e 100644 --- a/packages/rum-core/src/domain/lifeCycle.ts +++ b/packages/rum-core/src/domain/lifeCycle.ts @@ -21,6 +21,8 @@ export enum LifeCycleEventType { BEFORE_UNLOAD, RAW_RUM_EVENT_COLLECTED, RUM_EVENT_COLLECTED, + RECORD_STARTED, + RECORD_STOPPED, } export interface Subscription { @@ -44,6 +46,8 @@ export class LifeCycle { | LifeCycleEventType.BEFORE_UNLOAD | LifeCycleEventType.AUTO_ACTION_DISCARDED | LifeCycleEventType.VIEW_ENDED + | LifeCycleEventType.RECORD_STARTED + | LifeCycleEventType.RECORD_STOPPED ): void notify( eventType: LifeCycleEventType.RAW_RUM_EVENT_COLLECTED, @@ -84,7 +88,9 @@ export class LifeCycle { | LifeCycleEventType.DOM_MUTATED | LifeCycleEventType.BEFORE_UNLOAD | LifeCycleEventType.AUTO_ACTION_DISCARDED - | LifeCycleEventType.VIEW_ENDED, + | LifeCycleEventType.VIEW_ENDED + | LifeCycleEventType.RECORD_STARTED + | LifeCycleEventType.RECORD_STOPPED, callback: () => void ): Subscription subscribe( diff --git a/packages/rum-core/src/domain/rumEventsCollection/view/trackViews.spec.ts b/packages/rum-core/src/domain/rumEventsCollection/view/trackViews.spec.ts index 6b82c67213..4c49b1eda9 100644 --- a/packages/rum-core/src/domain/rumEventsCollection/view/trackViews.spec.ts +++ b/packages/rum-core/src/domain/rumEventsCollection/view/trackViews.spec.ts @@ -1040,3 +1040,72 @@ describe('rum track custom timings', () => { expect(warnSpy).toHaveBeenCalled() }) }) + +describe('track hasReplay', () => { + let setupBuilder: TestSetupBuilder + let handler: jasmine.Spy + let getViewEvent: (index: number) => View + + beforeEach(() => { + ;({ handler, getViewEvent } = spyOnViews()) + + setupBuilder = setup() + .withFakeLocation('/foo') + .withFakeClock() + .beforeBuild(({ location, lifeCycle }) => { + lifeCycle.subscribe(LifeCycleEventType.VIEW_UPDATED, handler) + return trackViews(location, lifeCycle) + }) + }) + + afterEach(() => { + setupBuilder.cleanup() + }) + + it('sets hasReplay to false by default', () => { + setupBuilder.build() + expect(getViewEvent(0).hasReplay).toBe(false) + }) + + it('sets hasReplay to true when the recording starts', () => { + const { lifeCycle } = setupBuilder.build() + + lifeCycle.notify(LifeCycleEventType.RECORD_STARTED) + + history.pushState({}, '', '/bar') + + expect(getViewEvent(1).hasReplay).toBe(true) + }) + + it('keeps hasReplay to true when the recording stops', () => { + const { lifeCycle } = setupBuilder.build() + + lifeCycle.notify(LifeCycleEventType.RECORD_STARTED) + lifeCycle.notify(LifeCycleEventType.RECORD_STOPPED) + + history.pushState({}, '', '/bar') + + expect(getViewEvent(1).hasReplay).toBe(true) + }) + + it('sets hasReplay to true when a new view is created after the recording starts', () => { + const { lifeCycle } = setupBuilder.build() + + lifeCycle.notify(LifeCycleEventType.RECORD_STARTED) + + history.pushState({}, '', '/bar') + + expect(getViewEvent(2).hasReplay).toBe(true) + }) + + it('sets hasReplay to false when a new view is created after the recording stops', () => { + const { lifeCycle } = setupBuilder.build() + + lifeCycle.notify(LifeCycleEventType.RECORD_STARTED) + lifeCycle.notify(LifeCycleEventType.RECORD_STOPPED) + + history.pushState({}, '', '/bar') + + expect(getViewEvent(2).hasReplay).toBe(false) + }) +}) diff --git a/packages/rum-core/src/domain/rumEventsCollection/view/trackViews.ts b/packages/rum-core/src/domain/rumEventsCollection/view/trackViews.ts index f6ca6ae051..e99860d4f1 100644 --- a/packages/rum-core/src/domain/rumEventsCollection/view/trackViews.ts +++ b/packages/rum-core/src/domain/rumEventsCollection/view/trackViews.ts @@ -35,6 +35,7 @@ export interface View { loadingTime?: Duration loadingType: ViewLoadingType cumulativeLayoutShift?: number + hasReplay: boolean } export interface ViewCreatedEvent { @@ -54,9 +55,11 @@ export function trackViews( onNewLocation: NewLocationListener = () => undefined ) { const startOrigin = 0 as RelativeTime + let hasReplay = false const initialView = newView( lifeCycle, location, + hasReplay, ViewLoadingType.INITIAL_LOAD, document.referrer, startOrigin, @@ -78,7 +81,15 @@ export function trackViews( // Renew view on location changes currentView.end() currentView.triggerUpdate() - currentView = newView(lifeCycle, location, ViewLoadingType.ROUTE_CHANGE, currentView.url, undefined, viewName) + currentView = newView( + lifeCycle, + location, + hasReplay, + ViewLoadingType.ROUTE_CHANGE, + currentView.url, + undefined, + viewName + ) return } currentView.updateLocation(location) @@ -89,7 +100,7 @@ export function trackViews( lifeCycle.subscribe(LifeCycleEventType.SESSION_RENEWED, () => { // do not trigger view update to avoid wrong data currentView.end() - currentView = newView(lifeCycle, location, ViewLoadingType.ROUTE_CHANGE, currentView.url) + currentView = newView(lifeCycle, location, hasReplay, ViewLoadingType.ROUTE_CHANGE, currentView.url) }) // End the current view on page unload @@ -98,6 +109,15 @@ export function trackViews( currentView.triggerUpdate() }) + lifeCycle.subscribe(LifeCycleEventType.RECORD_STARTED, () => { + hasReplay = true + currentView.updateHasReplay(true) + }) + + lifeCycle.subscribe(LifeCycleEventType.RECORD_STOPPED, () => { + hasReplay = false + }) + // Session keep alive const keepAliveInterval = window.setInterval( monitor(() => { @@ -124,6 +144,7 @@ export function trackViews( function newView( lifeCycle: LifeCycle, initialLocation: Location, + initialHasReplay: boolean, loadingType: ViewLoadingType, referrer: string, startTime = relativeNow(), @@ -144,6 +165,7 @@ function newView( let loadingTime: Duration | undefined let endTime: RelativeTime | undefined let location: Location = { ...initialLocation } + let hasReplay = initialHasReplay lifeCycle.notify(LifeCycleEventType.VIEW_CREATED, { id, startTime, location, referrer }) @@ -194,6 +216,7 @@ function newView( loadingTime, loadingType, location, + hasReplay, referrer, startTime, timings, @@ -237,6 +260,9 @@ function newView( updateLocation(newLocation: Location) { location = { ...newLocation } }, + updateHasReplay(newHasReplay: boolean) { + hasReplay = newHasReplay + }, get url() { return location.href }, diff --git a/packages/rum-core/src/domain/rumEventsCollection/view/viewCollection.spec.ts b/packages/rum-core/src/domain/rumEventsCollection/view/viewCollection.spec.ts index 04416e0078..8b10ff5126 100644 --- a/packages/rum-core/src/domain/rumEventsCollection/view/viewCollection.spec.ts +++ b/packages/rum-core/src/domain/rumEventsCollection/view/viewCollection.spec.ts @@ -2,6 +2,7 @@ import { Duration, RelativeTime, ServerDuration } from '@datadog/browser-core' import { setup, TestSetupBuilder } from '../../../../test/specHelper' import { RumEventType, ViewLoadingType } from '../../../rawRumEvent.types' import { LifeCycleEventType } from '../../lifeCycle' +import { View } from './trackViews' import { startViewCollection } from './viewCollection' describe('viewCollection', () => { @@ -24,7 +25,7 @@ describe('viewCollection', () => { it('should create view from view update', () => { const { lifeCycle, rawRumEvents } = setupBuilder.build() const location: Partial = {} - const view = { + const view: View = { cumulativeLayoutShift: 1, customTimings: { bar: 20 as Duration, @@ -41,6 +42,7 @@ describe('viewCollection', () => { id: 'xxx', name: undefined, isActive: false, + hasReplay: false, loadingTime: 20 as Duration, loadingType: ViewLoadingType.INITIAL_LOAD, location: location as Location, @@ -98,6 +100,9 @@ describe('viewCollection', () => { }, time_spent: (100 * 1e6) as ServerDuration, }, + session: { + has_replay: undefined, + }, }) }) }) diff --git a/packages/rum-core/src/domain/rumEventsCollection/view/viewCollection.ts b/packages/rum-core/src/domain/rumEventsCollection/view/viewCollection.ts index 1a4b0596bf..7f5881a29c 100644 --- a/packages/rum-core/src/domain/rumEventsCollection/view/viewCollection.ts +++ b/packages/rum-core/src/domain/rumEventsCollection/view/viewCollection.ts @@ -54,6 +54,9 @@ function processViewUpdate(view: View) { }, time_spent: toServerDuration(view.duration), }, + session: { + has_replay: view.hasReplay || undefined, + }, } if (!isEmptyObject(view.customTimings)) { viewEvent.view.custom_timings = mapValues( diff --git a/packages/rum-core/src/rawRumEvent.types.ts b/packages/rum-core/src/rawRumEvent.types.ts index b2669b5c6a..df306d5c19 100644 --- a/packages/rum-core/src/rawRumEvent.types.ts +++ b/packages/rum-core/src/rawRumEvent.types.ts @@ -79,6 +79,9 @@ export interface RawRumViewEvent { long_task: Count resource: Count } + session: { + has_replay: true | undefined + } _dd: { document_version: number } diff --git a/packages/rum-core/test/fixtures.ts b/packages/rum-core/test/fixtures.ts index 2d4451e5f5..fc14631216 100644 --- a/packages/rum-core/test/fixtures.ts +++ b/packages/rum-core/test/fixtures.ts @@ -80,6 +80,9 @@ export function createRawRumEvent(type: RumEventType, overrides?: Context): RawR resource: { count: 0 }, time_spent: 0 as ServerDuration, }, + session: { + has_replay: undefined, + }, }, overrides ) diff --git a/packages/rum-recorder/src/boot/rumRecorderPublicApi.ts b/packages/rum-recorder/src/boot/rumRecorderPublicApi.ts index 519630f978..8e35317285 100644 --- a/packages/rum-recorder/src/boot/rumRecorderPublicApi.ts +++ b/packages/rum-recorder/src/boot/rumRecorderPublicApi.ts @@ -1,5 +1,5 @@ import { monitor } from '@datadog/browser-core' -import { makeRumPublicApi, StartRum } from '@datadog/browser-rum-core' +import { LifeCycleEventType, makeRumPublicApi, StartRum } from '@datadog/browser-rum-core' import { startRecording } from './recorder' @@ -37,6 +37,7 @@ export function makeRumRecorderPublicApi(startRumImpl: StartRum, startRecordingI session, parentContexts )) + lifeCycle.notify(LifeCycleEventType.RECORD_STARTED) } function stopSessionReplayRecording() { @@ -46,6 +47,7 @@ export function makeRumRecorderPublicApi(startRumImpl: StartRum, startRecordingI stopRecording() stopRecording = undefined + lifeCycle.notify(LifeCycleEventType.RECORD_STOPPED) } return startRumResult From a24aa8a54fb6f3e2568972b21fcc89d488dc479f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Zugmeyer?= Date: Mon, 29 Mar 2021 18:20:23 +0200 Subject: [PATCH 4/6] =?UTF-8?q?=F0=9F=91=8C=20[RUMF-867]=20make=20segment?= =?UTF-8?q?=20collection=20state=20more=20explicit?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Having multiple loose variables to reflect a state does not scale. Instead of adding a new `isStopped` variable, make things more explicit by using a finite number of states, acting like a single source of truth for the whole segment collection. This helps to understand the various state transitions. --- .../src/domain/segmentCollection.ts | 86 +++++++++++++------ 1 file changed, 59 insertions(+), 27 deletions(-) diff --git a/packages/rum-recorder/src/domain/segmentCollection.ts b/packages/rum-recorder/src/domain/segmentCollection.ts index f2c290ce49..02be35010a 100644 --- a/packages/rum-recorder/src/domain/segmentCollection.ts +++ b/packages/rum-recorder/src/domain/segmentCollection.ts @@ -53,6 +53,25 @@ export function startSegmentCollection( ) } +const enum StateType { + WaitingForInitialRecord, + SegmentPending, + Stopped, +} +type State = + | { + type: StateType.WaitingForInitialRecord + nextSegmentCreationReason: CreationReason + } + | { + type: StateType.SegmentPending + segment: Segment + expirationTimeoutId: number + } + | { + type: StateType.Stopped + } + export function doStartSegmentCollection( lifeCycle: LifeCycle, getSegmentContext: () => SegmentContext | undefined, @@ -60,9 +79,10 @@ export function doStartSegmentCollection( worker: DeflateWorker, emitter: EventEmitter = window ) { - let currentSegment: Segment | undefined - let currentSegmentExpirationTimeoutId: number - let nextSegmentCreationReason: CreationReason | undefined = 'init' + let state: State = { + type: StateType.WaitingForInitialRecord, + nextSegmentCreationReason: 'init', + } const writer = new DeflateSegmentWriter( worker, @@ -95,38 +115,50 @@ export function doStartSegmentCollection( { capture: true } ) - function flushSegment(creationReason?: CreationReason) { - if (currentSegment) { - currentSegment.flush() - currentSegment = undefined - clearTimeout(currentSegmentExpirationTimeoutId) + function flushSegment(nextSegmentCreationReason?: CreationReason) { + if (state.type === StateType.SegmentPending) { + state.segment.flush() + clearTimeout(state.expirationTimeoutId) } - nextSegmentCreationReason = creationReason + if (nextSegmentCreationReason) { + state = { + type: StateType.WaitingForInitialRecord, + nextSegmentCreationReason, + } + } else { + state = { + type: StateType.Stopped, + } + } } return { addRecord: (record: Record) => { - if (!currentSegment) { - if (!nextSegmentCreationReason) { - return - } - const context = getSegmentContext() - if (!context) { - return - } - - currentSegment = new Segment(writer, context, nextSegmentCreationReason, record) - currentSegmentExpirationTimeoutId = setTimeout( - monitor(() => { - flushSegment('max_duration') - }), - MAX_SEGMENT_DURATION - ) - } else { - currentSegment.addRecord(record) + switch (state.type) { + case StateType.WaitingForInitialRecord: + const context = getSegmentContext() + if (!context) { + return + } + state = { + type: StateType.SegmentPending, + segment: new Segment(writer, context, state.nextSegmentCreationReason, record), + expirationTimeoutId: setTimeout( + monitor(() => { + flushSegment('max_duration') + }), + MAX_SEGMENT_DURATION + ), + } + break + + case StateType.SegmentPending: + state.segment.addRecord(record) + break } }, + stop: () => { flushSegment() unsubscribeViewCreated() From b8d1383517f2609ba67fddf90581a0a0391fafae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Zugmeyer?= Date: Mon, 29 Mar 2021 18:37:06 +0200 Subject: [PATCH 5/6] =?UTF-8?q?=F0=9F=91=8C=20[RUMF-867]=20make=20rum=20re?= =?UTF-8?q?corder=20state=20more=20explicit?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../src/boot/rumRecorderPublicApi.ts | 31 ++++++++++++++----- 1 file changed, 23 insertions(+), 8 deletions(-) diff --git a/packages/rum-recorder/src/boot/rumRecorderPublicApi.ts b/packages/rum-recorder/src/boot/rumRecorderPublicApi.ts index 8e35317285..0bfb7ad7ec 100644 --- a/packages/rum-recorder/src/boot/rumRecorderPublicApi.ts +++ b/packages/rum-recorder/src/boot/rumRecorderPublicApi.ts @@ -5,13 +5,21 @@ import { startRecording } from './recorder' export type StartRecording = typeof startRecording +const enum StateType { + Init, + Recording, +} +type State = { type: StateType.Init } | { type: StateType.Recording; stopRecording: () => void } + export function makeRumRecorderPublicApi(startRumImpl: StartRum, startRecordingImpl: StartRecording) { const rumRecorderGlobal = makeRumPublicApi((userConfiguration, getCommonContext) => { - let stopRecording: (() => void) | undefined + let state: State = { + type: StateType.Init, + } const startRumResult = startRumImpl(userConfiguration, () => ({ ...getCommonContext(), - hasReplay: stopRecording ? true : undefined, + hasReplay: state.type === StateType.Recording ? true : undefined, })) const { lifeCycle, parentContexts, configuration, session } = startRumResult @@ -27,26 +35,33 @@ export function makeRumRecorderPublicApi(startRumImpl: StartRum, startRecordingI } function startSessionReplayRecording() { - if (stopRecording) { + if (state.type === StateType.Recording) { return } - ;({ stop: stopRecording } = startRecordingImpl( + + const { stop: stopRecording } = startRecordingImpl( lifeCycle, userConfiguration.applicationId, configuration, session, parentContexts - )) + ) + state = { + type: StateType.Recording, + stopRecording, + } lifeCycle.notify(LifeCycleEventType.RECORD_STARTED) } function stopSessionReplayRecording() { - if (!stopRecording) { + if (state.type !== StateType.Recording) { return } - stopRecording() - stopRecording = undefined + state.stopRecording() + state = { + type: StateType.Init, + } lifeCycle.notify(LifeCycleEventType.RECORD_STOPPED) } From 6b0af7bbaa439943ee9897013885c8d214bc644d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Zugmeyer?= Date: Tue, 30 Mar 2021 10:58:29 +0200 Subject: [PATCH 6/6] =?UTF-8?q?=F0=9F=91=8C=20[RUMF-867]=20rename=20state?= =?UTF-8?q?=20structures?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../src/boot/rumRecorderPublicApi.ts | 29 ++++++++++++------- .../src/domain/segmentCollection.ts | 28 +++++++++--------- 2 files changed, 32 insertions(+), 25 deletions(-) diff --git a/packages/rum-recorder/src/boot/rumRecorderPublicApi.ts b/packages/rum-recorder/src/boot/rumRecorderPublicApi.ts index 0bfb7ad7ec..a42edc10d6 100644 --- a/packages/rum-recorder/src/boot/rumRecorderPublicApi.ts +++ b/packages/rum-recorder/src/boot/rumRecorderPublicApi.ts @@ -5,21 +5,28 @@ import { startRecording } from './recorder' export type StartRecording = typeof startRecording -const enum StateType { - Init, - Recording, +const enum RecorderStatus { + Stopped, + Started, } -type State = { type: StateType.Init } | { type: StateType.Recording; stopRecording: () => void } +type RecorderState = + | { + status: RecorderStatus.Stopped + } + | { + status: RecorderStatus.Started + stopRecording: () => void + } export function makeRumRecorderPublicApi(startRumImpl: StartRum, startRecordingImpl: StartRecording) { const rumRecorderGlobal = makeRumPublicApi((userConfiguration, getCommonContext) => { - let state: State = { - type: StateType.Init, + let state: RecorderState = { + status: RecorderStatus.Stopped, } const startRumResult = startRumImpl(userConfiguration, () => ({ ...getCommonContext(), - hasReplay: state.type === StateType.Recording ? true : undefined, + hasReplay: state.status === RecorderStatus.Started ? true : undefined, })) const { lifeCycle, parentContexts, configuration, session } = startRumResult @@ -35,7 +42,7 @@ export function makeRumRecorderPublicApi(startRumImpl: StartRum, startRecordingI } function startSessionReplayRecording() { - if (state.type === StateType.Recording) { + if (state.status === RecorderStatus.Started) { return } @@ -47,20 +54,20 @@ export function makeRumRecorderPublicApi(startRumImpl: StartRum, startRecordingI parentContexts ) state = { - type: StateType.Recording, + status: RecorderStatus.Started, stopRecording, } lifeCycle.notify(LifeCycleEventType.RECORD_STARTED) } function stopSessionReplayRecording() { - if (state.type !== StateType.Recording) { + if (state.status !== RecorderStatus.Started) { return } state.stopRecording() state = { - type: StateType.Init, + status: RecorderStatus.Stopped, } lifeCycle.notify(LifeCycleEventType.RECORD_STOPPED) } diff --git a/packages/rum-recorder/src/domain/segmentCollection.ts b/packages/rum-recorder/src/domain/segmentCollection.ts index 02be35010a..0f313ea031 100644 --- a/packages/rum-recorder/src/domain/segmentCollection.ts +++ b/packages/rum-recorder/src/domain/segmentCollection.ts @@ -53,23 +53,23 @@ export function startSegmentCollection( ) } -const enum StateType { +const enum SegmentCollectionStatus { WaitingForInitialRecord, SegmentPending, Stopped, } -type State = +type SegmentCollectionState = | { - type: StateType.WaitingForInitialRecord + status: SegmentCollectionStatus.WaitingForInitialRecord nextSegmentCreationReason: CreationReason } | { - type: StateType.SegmentPending + status: SegmentCollectionStatus.SegmentPending segment: Segment expirationTimeoutId: number } | { - type: StateType.Stopped + status: SegmentCollectionStatus.Stopped } export function doStartSegmentCollection( @@ -79,8 +79,8 @@ export function doStartSegmentCollection( worker: DeflateWorker, emitter: EventEmitter = window ) { - let state: State = { - type: StateType.WaitingForInitialRecord, + let state: SegmentCollectionState = { + status: SegmentCollectionStatus.WaitingForInitialRecord, nextSegmentCreationReason: 'init', } @@ -116,33 +116,33 @@ export function doStartSegmentCollection( ) function flushSegment(nextSegmentCreationReason?: CreationReason) { - if (state.type === StateType.SegmentPending) { + if (state.status === SegmentCollectionStatus.SegmentPending) { state.segment.flush() clearTimeout(state.expirationTimeoutId) } if (nextSegmentCreationReason) { state = { - type: StateType.WaitingForInitialRecord, + status: SegmentCollectionStatus.WaitingForInitialRecord, nextSegmentCreationReason, } } else { state = { - type: StateType.Stopped, + status: SegmentCollectionStatus.Stopped, } } } return { addRecord: (record: Record) => { - switch (state.type) { - case StateType.WaitingForInitialRecord: + switch (state.status) { + case SegmentCollectionStatus.WaitingForInitialRecord: const context = getSegmentContext() if (!context) { return } state = { - type: StateType.SegmentPending, + status: SegmentCollectionStatus.SegmentPending, segment: new Segment(writer, context, state.nextSegmentCreationReason, record), expirationTimeoutId: setTimeout( monitor(() => { @@ -153,7 +153,7 @@ export function doStartSegmentCollection( } break - case StateType.SegmentPending: + case SegmentCollectionStatus.SegmentPending: state.segment.addRecord(record) break }