Skip to content

Commit

Permalink
✨ [RUMF-867] stabilize start/stop recording API
Browse files Browse the repository at this point in the history
  • Loading branch information
@BenoitZugmeyer
BenoitZugmeyer committed Apr 6, 2021
1 parent e704def commit 8c84b5f
Show file tree
Hide file tree
Showing 2 changed files with 78 additions and 81 deletions.
114 changes: 47 additions & 67 deletions packages/rum-recorder/src/boot/rumRecorderPublicApi.spec.ts
View file
Original file line number Diff line number Diff line change
@@ -1,32 +1,23 @@
/* eslint-disable @typescript-eslint/unbound-method */
import { Configuration, noop } from '@datadog/browser-core'
import { RumPublicApi, RumUserConfiguration, StartRum } from '@datadog/browser-rum-core'
import { makeRumRecorderPublicApi, StartRecording } from './rumRecorderPublicApi'
import { StartRum } from '@datadog/browser-rum-core'
import { makeRumRecorderPublicApi, RumRecorderPublicApi, StartRecording } from './rumRecorderPublicApi'

const DEFAULT_INIT_CONFIGURATION = { applicationId: 'xxx', clientToken: 'xxx' }

describe('makeRumRecorderPublicApi', () => {
let rumGlobal: RumPublicApi & {
// TODO postpone_start_recording: those types will be included in rum-recorder public API when
// postpone_start_recording is stabilized.
startSessionReplayRecording?(): void
init(options: RumUserConfiguration & { manualSessionReplayRecordingStart?: boolean }): void
}
let rumGlobal: RumRecorderPublicApi
let startRecordingSpy: jasmine.Spy<StartRecording>
let stopRecordingSpy: jasmine.Spy<() => void>
let startRumSpy: jasmine.Spy<StartRum>
let enabledFlags: string[] = []

beforeEach(() => {
enabledFlags = []
stopRecordingSpy = jasmine.createSpy()
startRecordingSpy = jasmine.createSpy().and.callFake(() => ({
stop: noop,
stop: stopRecordingSpy,
}))
startRumSpy = jasmine.createSpy().and.callFake(() => {
const configuration: Partial<Configuration> = {
isEnabled(flag: string) {
return enabledFlags.indexOf(flag) >= 0
},
}
const configuration: Partial<Configuration> = {}
return ({ configuration } as unknown) as ReturnType<StartRum>
})
rumGlobal = makeRumRecorderPublicApi(startRumSpy, startRecordingSpy)
Expand All @@ -37,75 +28,64 @@ describe('makeRumRecorderPublicApi', () => {
}

describe('init', () => {
it('should start RUM when init is called', () => {
it('starts RUM when init is called', () => {
expect(startRumSpy).not.toHaveBeenCalled()
rumGlobal.init(DEFAULT_INIT_CONFIGURATION)
expect(startRumSpy).toHaveBeenCalled()
})

it('should start recording when init is called', () => {
it('starts recording when init() is called', () => {
expect(startRecordingSpy).not.toHaveBeenCalled()
rumGlobal.init(DEFAULT_INIT_CONFIGURATION)
expect(startRecordingSpy).toHaveBeenCalled()
})

it('should set commonContext.hasReplay to true', () => {
rumGlobal.init(DEFAULT_INIT_CONFIGURATION)
expect(getCommonContext().hasReplay).toBe(true)
it('does not start recording when calling init() with manualSessionReplayRecordingStart: true', () => {
rumGlobal.init({ ...DEFAULT_INIT_CONFIGURATION, manualSessionReplayRecordingStart: true })
expect(startRecordingSpy).not.toHaveBeenCalled()
})
})

describe('experimental flag postpone_start_recording', () => {
describe('if disabled', () => {
it('startSessionReplayRecording should not be defined', () => {
expect(rumGlobal.startSessionReplayRecording).toBeUndefined()
rumGlobal.init(DEFAULT_INIT_CONFIGURATION)
expect(rumGlobal.startSessionReplayRecording).toBeUndefined()
})

it('option manualSessionReplayRecordingStart should not be taken into account', () => {
rumGlobal.init({ ...DEFAULT_INIT_CONFIGURATION, manualSessionReplayRecordingStart: true })
expect(startRecordingSpy).toHaveBeenCalled()
})
describe('startSessionReplayRecording()', () => {
it('ignores calls while recording is already started', () => {
rumGlobal.init(DEFAULT_INIT_CONFIGURATION)
rumGlobal.startSessionReplayRecording()
rumGlobal.startSessionReplayRecording()
rumGlobal.startSessionReplayRecording()
expect(startRecordingSpy).toHaveBeenCalledTimes(1)
})

describe('if enabled', () => {
beforeEach(() => {
enabledFlags = ['postpone_start_recording']
})

it('recording should start when calling init() with default options', () => {
rumGlobal.init(DEFAULT_INIT_CONFIGURATION)
expect(startRecordingSpy).toHaveBeenCalled()
})

it('startSessionReplayRecording should be defined', () => {
expect(rumGlobal.startSessionReplayRecording).toBeUndefined()
rumGlobal.init(DEFAULT_INIT_CONFIGURATION)
expect(rumGlobal.startSessionReplayRecording).toEqual(jasmine.any(Function))
})
it('starts recording if called before init()', () => {
rumGlobal.startSessionReplayRecording()
rumGlobal.init({ ...DEFAULT_INIT_CONFIGURATION, manualSessionReplayRecordingStart: true })
expect(startRecordingSpy).toHaveBeenCalled()
})
})

it('calling startSessionReplayRecording while recording is already start should be ignored', () => {
rumGlobal.init(DEFAULT_INIT_CONFIGURATION)
rumGlobal.startSessionReplayRecording!()
rumGlobal.startSessionReplayRecording!()
rumGlobal.startSessionReplayRecording!()
expect(startRecordingSpy).toHaveBeenCalledTimes(1)
})
describe('stopSessionReplayRecording()', () => {
it('ignores calls while recording is already stopped', () => {
rumGlobal.init(DEFAULT_INIT_CONFIGURATION)
rumGlobal.stopSessionReplayRecording()
rumGlobal.stopSessionReplayRecording()
rumGlobal.stopSessionReplayRecording()
expect(stopRecordingSpy).toHaveBeenCalledTimes(1)
})

describe('with manualSessionReplayRecordingStart: true option', () => {
it('recording should not start when calling init() with option manualSessionReplayRecordingStart', () => {
rumGlobal.init({ ...DEFAULT_INIT_CONFIGURATION, manualSessionReplayRecordingStart: true })
expect(startRecordingSpy).not.toHaveBeenCalled()
})
it('does not start recording if called before init()', () => {
rumGlobal.stopSessionReplayRecording()
rumGlobal.init({ ...DEFAULT_INIT_CONFIGURATION })
expect(startRecordingSpy).not.toHaveBeenCalled()
})
})

it('commonContext.hasReplay should be true only if startSessionReplayRecording is called', () => {
rumGlobal.init({ ...DEFAULT_INIT_CONFIGURATION, manualSessionReplayRecordingStart: true })
expect(getCommonContext().hasReplay).toBeUndefined()
rumGlobal.startSessionReplayRecording!()
expect(getCommonContext().hasReplay).toBe(true)
})
})
describe('commonContext hasReplay', () => {
it('is true only if recording', () => {
rumGlobal.init({ ...DEFAULT_INIT_CONFIGURATION, manualSessionReplayRecordingStart: true })
expect(getCommonContext().hasReplay).toBeUndefined()
rumGlobal.startSessionReplayRecording()
expect(getCommonContext().hasReplay).toBe(true)
rumGlobal.stopSessionReplayRecording()
expect(getCommonContext().hasReplay).toBeUndefined()
})
})
})
45 changes: 31 additions & 14 deletions packages/rum-recorder/src/boot/rumRecorderPublicApi.ts
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { monitor } from '@datadog/browser-core'
import { monitor, noop } from '@datadog/browser-core'
import { LifeCycleEventType, makeRumPublicApi, RumUserConfiguration, StartRum } from '@datadog/browser-rum-core'

import { startRecording } from './recorder'
Expand All @@ -24,7 +24,7 @@ type RecorderState =
}

export function makeRumRecorderPublicApi(startRumImpl: StartRum, startRecordingImpl: StartRecording) {
const rumRecorderGlobal = makeRumPublicApi<RumRecorderUserConfiguration>((userConfiguration, getCommonContext) => {
const rumPublicApi = makeRumPublicApi<RumRecorderUserConfiguration>((userConfiguration, getCommonContext) => {
let state: RecorderState = {
status: RecorderStatus.Stopped,
}
Expand All @@ -36,16 +36,7 @@ export function makeRumRecorderPublicApi(startRumImpl: StartRum, startRecordingI

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.manualSessionReplayRecordingStart) {
startSessionReplayRecording()
}

function startSessionReplayRecording() {
startSessionReplayRecordingImpl = () => {
if (state.status === RecorderStatus.Started) {
return
}
Expand All @@ -64,7 +55,7 @@ export function makeRumRecorderPublicApi(startRumImpl: StartRum, startRecordingI
lifeCycle.notify(LifeCycleEventType.RECORD_STARTED)
}

function stopSessionReplayRecording() {
stopSessionReplayRecordingImpl = () => {
if (state.status !== RecorderStatus.Started) {
return
}
Expand All @@ -76,7 +67,33 @@ export function makeRumRecorderPublicApi(startRumImpl: StartRum, startRecordingI
lifeCycle.notify(LifeCycleEventType.RECORD_STOPPED)
}

onInit(userConfiguration)

return startRumResult
})
return rumRecorderGlobal

let onInit = (userConfiguration: RumRecorderUserConfiguration) => {
if (!userConfiguration.manualSessionReplayRecordingStart) {
startSessionReplayRecordingImpl()
}
}

let startSessionReplayRecordingImpl = () => {
onInit = () => startSessionReplayRecordingImpl()
}

let stopSessionReplayRecordingImpl = () => {
onInit = noop
}

const rumRecorderPublicApi = {
...rumPublicApi,
startSessionReplayRecording: monitor(() => {
startSessionReplayRecordingImpl()
}),
stopSessionReplayRecording: monitor(() => {
stopSessionReplayRecordingImpl()
}),
}
return rumRecorderPublicApi
}

0 comments on commit 8c84b5f

Please sign in to comment.