Skip to content

Commit

Permalink
Merge branch 'najib.boutaib/RUM-3837-force-replay-recording-on-sample…
Browse files Browse the repository at this point in the history
…d-out-sessions' (59d7883) into staging-14

 pm_trace_id: 31365005
 feature_branch_pipeline_id: 31365005
 source: to-staging

* commit '59d7883785c4b1f6be30c79e82c0856c1c394918':
  [RUM-3837] Format code
  [RUM-3837] Add unit and E2E tests
  [RUM-3837] Force replay recording on sampled out sessions using public API
  • Loading branch information
N-Boutaib committed Apr 3, 2024
2 parents cfbad71 + 59d7883 commit dabd85f
Show file tree
Hide file tree
Showing 5 changed files with 50 additions and 8 deletions.
8 changes: 5 additions & 3 deletions packages/rum-core/src/boot/rumPublicApi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,11 @@ import { createPreStartStrategy } from './preStartRum'
import type { StartRum, StartRumResult } from './startRum'

export type RumPublicApi = ReturnType<typeof makeRumPublicApi>

export interface StartStrategyOptions {
forceStart: boolean
}
export interface RecorderApi {
start: () => void
start: (options?: StartStrategyOptions) => void
stop: () => void
onRumStart: (
lifeCycle: LifeCycle,
Expand Down Expand Up @@ -288,7 +290,7 @@ export function makeRumPublicApi(startRumImpl: StartRum, recorderApi: RecorderAp
}),

getSessionReplayLink: monitor(() => recorderApi.getSessionReplayLink()),
startSessionReplayRecording: monitor(() => recorderApi.start()),
startSessionReplayRecording: monitor((options?: StartStrategyOptions) => recorderApi.start(options)),
stopSessionReplayRecording: monitor(() => recorderApi.stop()),
})

Expand Down
2 changes: 1 addition & 1 deletion packages/rum-core/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
export { RumPublicApi, makeRumPublicApi, RecorderApi } from './boot/rumPublicApi'
export { RumPublicApi, makeRumPublicApi, RecorderApi, StartStrategyOptions } from './boot/rumPublicApi'
export { StartRum } from './boot/startRum'
export {
RumEvent,
Expand Down
7 changes: 7 additions & 0 deletions packages/rum/src/boot/recorderApi.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,13 @@ describe('makeRecorderApi', () => {
expect(startRecordingSpy).not.toHaveBeenCalled()
})

it('should start recording if session is tracked without session replay when forced', () => {
setupBuilder.withSessionManager(createRumSessionManagerMock().setTrackedWithoutSessionReplay()).build()
rumInit()
recorderApi.start({ forceStart: true })
expect(startRecordingSpy).toHaveBeenCalledTimes(1)
})

it('uses the previously created worker if available', () => {
setupBuilder.build()
rumInit({ worker: mockWorker })
Expand Down
13 changes: 9 additions & 4 deletions packages/rum/src/boot/recorderApi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import type {
RumSessionManager,
RecorderApi,
RumConfiguration,
StartStrategyOptions,
} from '@datadog/browser-rum-core'
import { LifeCycleEventType } from '@datadog/browser-rum-core'
import { getReplayStats as getReplayStatsImpl } from '../domain/replayStats'
Expand Down Expand Up @@ -57,6 +58,8 @@ type RecorderState =
stopRecording: () => void
}

type StartStrategyFn = (options?: StartStrategyOptions) => void

export function makeRecorderApi(
startRecordingImpl: StartRecording,
createDeflateWorkerImpl?: CreateDeflateWorker
Expand All @@ -76,7 +79,7 @@ export function makeRecorderApi(
status: RecorderStatus.IntentToStart,
}

let startStrategy = () => {
let startStrategy: StartStrategyFn = () => {
state = { status: RecorderStatus.IntentToStart }
}
let stopStrategy = () => {
Expand All @@ -85,7 +88,7 @@ export function makeRecorderApi(
let getSessionReplayLinkStrategy = noop as () => string | undefined

return {
start: () => startStrategy(),
start: (options?: StartStrategyOptions) => startStrategy(options),
stop: () => stopStrategy(),
getSessionReplayLink: () => getSessionReplayLinkStrategy(),
onRumStart: (
Expand Down Expand Up @@ -139,9 +142,11 @@ export function makeRecorderApi(
return cachedDeflateEncoder
}

startStrategy = () => {
startStrategy = (options?: StartStrategyOptions) => {
const session = sessionManager.findTrackedSession()
if (!session || !session.sessionReplayAllowed) {
// If session is undefined (untracked), recording should never start
// If session is not allowed for replay and not being forced, recording should not start
if (!session || (!session.sessionReplayAllowed && !options?.forceStart)) {
state = { status: RecorderStatus.IntentToStart }
return
}
Expand Down
28 changes: 28 additions & 0 deletions test/e2e/scenario/recorder/recorder.scenario.ts
Original file line number Diff line number Diff line change
Expand Up @@ -807,6 +807,34 @@ describe('recorder', () => {
})
})

describe('recording of sampled out sessions', () => {
createTest('should not start recording when session is sampled out')
.withRum({ sessionReplaySampleRate: 0 })
.withSetup(bundleSetup)
.run(async ({ intakeRegistry }) => {
await browserExecute(() => {
window.DD_RUM!.startSessionReplayRecording()
})

await flushEvents()

expect(intakeRegistry.replaySegments.length).toBe(0)
})

createTest('should start recording if forced when session is sampled out')
.withRum({ sessionReplaySampleRate: 0 })
.withSetup(bundleSetup)
.run(async ({ intakeRegistry }) => {
await browserExecute(() => {
window.DD_RUM!.startSessionReplayRecording({ forceStart: true })
})

await flushEvents()

expect(intakeRegistry.replaySegments.length).toBe(1)
})
})

createTest('restarting recording should send a new full snapshot')
.withRum()
.withSetup(bundleSetup)
Expand Down

0 comments on commit dabd85f

Please sign in to comment.