Skip to content

Commit

Permalink
[RUMF-867] enable start/stop recording API (#784)
Browse files Browse the repository at this point in the history
* ✨ [RUMF-867] stabilize the `manualSessionReplayRecordingStart` option

* ✨ [RUMF-867] stabilize start/stop recording API

* 🏷️ fix compatibility issue with TS3.0

* 🏷️ Export RumRecorder{PublicApi,UserConfiguration} types

* 👌 rename a few variable to rumPublicApi / rumRecorderPublicApi

For consistency.
  • Loading branch information
BenoitZugmeyer authored Apr 8, 2021
1 parent 1e0244e commit c5f8aa5
Show file tree
Hide file tree
Showing 8 changed files with 182 additions and 175 deletions.
144 changes: 72 additions & 72 deletions packages/rum-core/src/boot/rumPublicApi.spec.ts

Large diffs are not rendered by default.

25 changes: 15 additions & 10 deletions packages/rum-core/src/boot/rumPublicApi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,28 +27,33 @@ export interface RumUserConfiguration extends UserConfiguration {

export type RumPublicApi = ReturnType<typeof makeRumPublicApi>

export type StartRum = typeof startRum
export type StartRum<C extends RumUserConfiguration = RumUserConfiguration> = (
userConfiguration: C,
getCommonContext: () => CommonContext
) => StartRumResult

export function makeRumPublicApi(startRumImpl: StartRum) {
type StartRumResult = ReturnType<typeof startRum>

export function makeRumPublicApi<C extends RumUserConfiguration>(startRumImpl: StartRum<C>) {
let isAlreadyInitialized = false

const globalContextManager = createContextManager()
let user: User = {}

let getInternalContextStrategy: ReturnType<StartRum>['getInternalContext'] = () => undefined
let getInternalContextStrategy: StartRumResult['getInternalContext'] = () => undefined

const beforeInitAddTiming = new BoundedBuffer<[string, RelativeTime]>()
let addTimingStrategy: ReturnType<StartRum>['addTiming'] = (name) => {
let addTimingStrategy: StartRumResult['addTiming'] = (name) => {
beforeInitAddTiming.add([name, relativeNow()])
}

const beforeInitAddAction = new BoundedBuffer<[CustomAction, CommonContext]>()
let addActionStrategy: ReturnType<StartRum>['addAction'] = (action) => {
let addActionStrategy: StartRumResult['addAction'] = (action) => {
beforeInitAddAction.add([action, clonedCommonContext()])
}

const beforeInitAddError = new BoundedBuffer<[ProvidedError, CommonContext]>()
let addErrorStrategy: ReturnType<StartRum>['addError'] = (providedError) => {
let addErrorStrategy: StartRumResult['addError'] = (providedError) => {
beforeInitAddError.add([providedError, clonedCommonContext()])
}

Expand All @@ -59,8 +64,8 @@ export function makeRumPublicApi(startRumImpl: StartRum) {
})
}

const rumGlobal = makePublicApi({
init: monitor((userConfiguration: RumUserConfiguration) => {
const rumPublicApi = makePublicApi({
init: monitor((userConfiguration: C) => {
if (
!checkCookiesAuthorized(buildCookieOptions(userConfiguration)) ||
!checkIsNotLocalFile() ||
Expand Down Expand Up @@ -110,7 +115,7 @@ export function makeRumPublicApi(startRumImpl: StartRum) {
* @deprecated use addAction instead
*/
addUserAction: (name: string, context?: object) => {
rumGlobal.addAction(name, context as Context)
rumPublicApi.addAction(name, context as Context)
},

addError: monitor((error: unknown, context?: object, source: ProvidedSource = ErrorSource.CUSTOM) => {
Expand Down Expand Up @@ -142,7 +147,7 @@ export function makeRumPublicApi(startRumImpl: StartRum) {
}
}),
})
return rumGlobal
return rumPublicApi

function sanitizeUser(newUser: unknown) {
if (typeof newUser !== 'object' || !newUser) {
Expand Down
6 changes: 3 additions & 3 deletions packages/rum-recorder/src/boot/recorder.entry.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import { defineGlobal, getGlobalObject } from '@datadog/browser-core'
import { RumPublicApi, startRum } from '@datadog/browser-rum-core'
import { startRum } from '@datadog/browser-rum-core'

import { startRecording } from './recorder'
import { makeRumRecorderPublicApi } from './rumRecorderPublicApi'
import { RumRecorderPublicApi, makeRumRecorderPublicApi } from './rumRecorderPublicApi'

export const datadogRum = makeRumRecorderPublicApi(startRum, startRecording)

interface BrowserWindow extends Window {
DD_RUM?: RumPublicApi
DD_RUM?: RumRecorderPublicApi
}
defineGlobal(getGlobalObject<BrowserWindow>(), 'DD_RUM', datadogRum)
122 changes: 51 additions & 71 deletions packages/rum-recorder/src/boot/rumRecorderPublicApi.spec.ts
Original file line number Diff line number Diff line change
@@ -1,111 +1,91 @@
/* 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 { Configuration } from '@datadog/browser-core'
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 rumRecorderPublicApi: 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)
rumRecorderPublicApi = makeRumRecorderPublicApi(startRumSpy, startRecordingSpy)
})

function getCommonContext() {
return startRumSpy.calls.first().args[1]()
}

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)
rumRecorderPublicApi.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)
rumRecorderPublicApi.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', () => {
rumRecorderPublicApi.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', () => {
rumRecorderPublicApi.init(DEFAULT_INIT_CONFIGURATION)
rumRecorderPublicApi.startSessionReplayRecording()
rumRecorderPublicApi.startSessionReplayRecording()
rumRecorderPublicApi.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()', () => {
rumRecorderPublicApi.startSessionReplayRecording()
rumRecorderPublicApi.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', () => {
rumRecorderPublicApi.init(DEFAULT_INIT_CONFIGURATION)
rumRecorderPublicApi.stopSessionReplayRecording()
rumRecorderPublicApi.stopSessionReplayRecording()
rumRecorderPublicApi.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()', () => {
rumRecorderPublicApi.stopSessionReplayRecording()
rumRecorderPublicApi.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', () => {
rumRecorderPublicApi.init({ ...DEFAULT_INIT_CONFIGURATION, manualSessionReplayRecordingStart: true })
expect(getCommonContext().hasReplay).toBeUndefined()
rumRecorderPublicApi.startSessionReplayRecording()
expect(getCommonContext().hasReplay).toBe(true)
rumRecorderPublicApi.stopSessionReplayRecording()
expect(getCommonContext().hasReplay).toBeUndefined()
})
})
})
53 changes: 37 additions & 16 deletions packages/rum-recorder/src/boot/rumRecorderPublicApi.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,14 @@
import { monitor } from '@datadog/browser-core'
import { LifeCycleEventType, makeRumPublicApi, StartRum } from '@datadog/browser-rum-core'
import { monitor, noop } from '@datadog/browser-core'
import { LifeCycleEventType, makeRumPublicApi, RumUserConfiguration, StartRum } from '@datadog/browser-rum-core'

import { startRecording } from './recorder'

export type StartRecording = typeof startRecording
export type RumRecorderPublicApi = ReturnType<typeof makeRumRecorderPublicApi>

export interface RumRecorderUserConfiguration extends RumUserConfiguration {
manualSessionReplayRecordingStart?: boolean
}

const enum RecorderStatus {
Stopped,
Expand All @@ -19,7 +24,7 @@ type RecorderState =
}

export function makeRumRecorderPublicApi(startRumImpl: StartRum, startRecordingImpl: StartRecording) {
const rumRecorderGlobal = makeRumPublicApi((userConfiguration, getCommonContext) => {
const rumPublicApi = makeRumPublicApi<RumRecorderUserConfiguration>((userConfiguration, getCommonContext) => {
let state: RecorderState = {
status: RecorderStatus.Stopped,
}
Expand All @@ -31,17 +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 as any).manualSessionReplayRecordingStart) {
startSessionReplayRecording()
}
} else {
startSessionReplayRecording()
}

function startSessionReplayRecording() {
startSessionReplayRecordingImpl = () => {
if (state.status === RecorderStatus.Started) {
return
}
Expand All @@ -60,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 @@ -72,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
}
2 changes: 1 addition & 1 deletion packages/rum-recorder/src/domain/rrweb/types.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { IdNodeMap, INode, SerializedNodeWithId } from '../rrweb-snapshot/types'
import type { FocusRecord, RawRecord } from '../../types'
import { FocusRecord, RawRecord } from '../../types'
import { MutationController } from './mutation'

export enum IncrementalSource {
Expand Down
3 changes: 2 additions & 1 deletion packages/rum-recorder/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ export { datadogRum } from './boot/recorder.entry'
export {
CommonProperties,
ProvidedSource,
RumPublicApi as RumGlobal,
RumUserConfiguration,
// Events
RumEvent,
Expand All @@ -13,3 +12,5 @@ export {
RumResourceEvent,
RumViewEvent,
} from '@datadog/browser-rum-core'

export { RumRecorderPublicApi as RumGlobal, RumRecorderUserConfiguration } from './boot/rumRecorderPublicApi'
2 changes: 1 addition & 1 deletion packages/rum-recorder/src/types.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { SerializedNodeWithId } from './domain/rrweb-snapshot/types'
import type { IncrementalData } from './domain/rrweb/types'
import { IncrementalData } from './domain/rrweb/types'

export { IncrementalSource, MutationData } from './domain/rrweb/types'

Expand Down

0 comments on commit c5f8aa5

Please sign in to comment.