Skip to content

Commit

Permalink
[RUMF-867] implement stop recording (#771)
Browse files Browse the repository at this point in the history
* [RUMF-867] implement stopSessionReplayRecording

* [RUMF-867] flush the pending segment when stop recording

* [RUMF-867] special has_replay processing on views
  • Loading branch information
BenoitZugmeyer authored Mar 31, 2021
1 parent b3a24e5 commit f7e1ef6
Show file tree
Hide file tree
Showing 14 changed files with 259 additions and 45 deletions.
12 changes: 11 additions & 1 deletion packages/rum-core/src/domain/assembly.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
})
})
5 changes: 4 additions & 1 deletion packages/rum-core/src/domain/assembly.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand All @@ -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
Expand Down
8 changes: 7 additions & 1 deletion packages/rum-core/src/domain/lifeCycle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ export enum LifeCycleEventType {
BEFORE_UNLOAD,
RAW_RUM_EVENT_COLLECTED,
RUM_EVENT_COLLECTED,
RECORD_STARTED,
RECORD_STOPPED,
}

export interface Subscription {
Expand All @@ -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,
Expand Down Expand Up @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1061,3 +1061,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)
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ export interface View {
loadingTime?: Duration
loadingType: ViewLoadingType
cumulativeLayoutShift?: number
hasReplay: boolean
}

export interface ViewCreatedEvent {
Expand All @@ -55,9 +56,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,
Expand All @@ -79,7 +82,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)
Expand All @@ -90,7 +101,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
Expand All @@ -99,6 +110,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(() => {
Expand All @@ -125,6 +145,7 @@ export function trackViews(
function newView(
lifeCycle: LifeCycle,
initialLocation: Location,
initialHasReplay: boolean,
loadingType: ViewLoadingType,
referrer: string,
startTime = relativeNow(),
Expand All @@ -145,6 +166,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 })

Expand Down Expand Up @@ -195,6 +217,7 @@ function newView(
loadingTime,
loadingType,
location,
hasReplay,
referrer,
startTime,
timings,
Expand Down Expand Up @@ -238,6 +261,9 @@ function newView(
updateLocation(newLocation: Location) {
location = { ...newLocation }
},
updateHasReplay(newHasReplay: boolean) {
hasReplay = newHasReplay
},
get url() {
return location.href
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand All @@ -24,7 +25,7 @@ describe('viewCollection', () => {
it('should create view from view update', () => {
const { lifeCycle, rawRumEvents } = setupBuilder.build()
const location: Partial<Location> = {}
const view = {
const view: View = {
cumulativeLayoutShift: 1,
customTimings: {
bar: 20 as Duration,
Expand All @@ -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,
Expand Down Expand Up @@ -98,6 +100,9 @@ describe('viewCollection', () => {
},
time_spent: (100 * 1e6) as ServerDuration,
},
session: {
has_replay: undefined,
},
})
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
3 changes: 3 additions & 0 deletions packages/rum-core/src/rawRumEvent.types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,9 @@ export interface RawRumViewEvent {
long_task: Count
resource: Count
}
session: {
has_replay: true | undefined
}
_dd: {
document_version: number
}
Expand Down
3 changes: 3 additions & 0 deletions packages/rum-core/test/fixtures.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
)
Expand Down
2 changes: 1 addition & 1 deletion packages/rum-recorder/src/boot/recorder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ export function startRecording(
trackViewEndRecord(lifeCycle, (record) => addRawRecord(record))

return {
stop() {
stop: () => {
stopRecording()
stopSegmentCollection()
stopTrackingFocusRecords()
Expand Down
6 changes: 4 additions & 2 deletions packages/rum-recorder/src/boot/rumRecorderPublicApi.spec.ts
Original file line number Diff line number Diff line change
@@ -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'

Expand All @@ -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<Configuration> = {
isEnabled(flag: string) {
Expand Down
Loading

0 comments on commit f7e1ef6

Please sign in to comment.