Skip to content

Commit

Permalink
🐛 fix current action context reset on custom action (#444)
Browse files Browse the repository at this point in the history
  • Loading branch information
bcaudan authored Jul 10, 2020
1 parent dae5a13 commit b0c4ca2
Show file tree
Hide file tree
Showing 10 changed files with 92 additions and 76 deletions.
26 changes: 16 additions & 10 deletions packages/rum/src/lifeCycle.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
import { ErrorMessage, RequestCompleteEvent, RequestStartEvent } from '@datadog/browser-core'
import { UserAction } from './userActionCollection'
import { AutoUserAction, CustomUserAction } from './userActionCollection'
import { View } from './viewCollection'

export enum LifeCycleEventType {
ERROR_COLLECTED,
PERFORMANCE_ENTRY_COLLECTED,
ACTION_CREATED,
ACTION_COMPLETED,
ACTION_DISCARDED,
CUSTOM_ACTION_COLLECTED,
AUTO_ACTION_CREATED,
AUTO_ACTION_COMPLETED,
AUTO_ACTION_DISCARDED,
VIEW_CREATED,
VIEW_UPDATED,
REQUEST_STARTED,
Expand All @@ -29,9 +30,10 @@ export class LifeCycle {
notify(eventType: LifeCycleEventType.PERFORMANCE_ENTRY_COLLECTED, data: PerformanceEntry): void
notify(eventType: LifeCycleEventType.REQUEST_STARTED, data: RequestStartEvent): void
notify(eventType: LifeCycleEventType.REQUEST_COMPLETED, data: RequestCompleteEvent): void
notify(eventType: LifeCycleEventType.ACTION_COMPLETED, data: UserAction): void
notify(eventType: LifeCycleEventType.AUTO_ACTION_COMPLETED, data: AutoUserAction): void
notify(eventType: LifeCycleEventType.CUSTOM_ACTION_COLLECTED, data: CustomUserAction): void
notify(
eventType: LifeCycleEventType.ACTION_CREATED | LifeCycleEventType.VIEW_CREATED,
eventType: LifeCycleEventType.AUTO_ACTION_CREATED | LifeCycleEventType.VIEW_CREATED,
{ id, startTime }: { id: string; startTime: number }
): void
notify(eventType: LifeCycleEventType.VIEW_UPDATED, data: View): void
Expand All @@ -41,7 +43,7 @@ export class LifeCycle {
| LifeCycleEventType.RESOURCE_ADDED_TO_BATCH
| LifeCycleEventType.DOM_MUTATED
| LifeCycleEventType.BEFORE_UNLOAD
| LifeCycleEventType.ACTION_DISCARDED
| LifeCycleEventType.AUTO_ACTION_DISCARDED
): void
notify(eventType: LifeCycleEventType, data?: any) {
const eventCallbacks = this.callbacks[eventType]
Expand All @@ -60,19 +62,23 @@ export class LifeCycle {
eventType: LifeCycleEventType.REQUEST_COMPLETED,
callback: (data: RequestCompleteEvent) => void
): Subscription
subscribe(eventType: LifeCycleEventType.ACTION_COMPLETED, callback: (data: UserAction) => void): Subscription
subscribe(eventType: LifeCycleEventType.AUTO_ACTION_COMPLETED, callback: (data: AutoUserAction) => void): Subscription
subscribe(
eventType: LifeCycleEventType.ACTION_CREATED | LifeCycleEventType.VIEW_CREATED,
eventType: LifeCycleEventType.AUTO_ACTION_CREATED | LifeCycleEventType.VIEW_CREATED,
callback: ({ id, startTime }: { id: string; startTime: number }) => void
): Subscription
subscribe(
eventType: LifeCycleEventType.CUSTOM_ACTION_COLLECTED,
callback: (data: CustomUserAction) => void
): Subscription
subscribe(eventType: LifeCycleEventType.VIEW_UPDATED, callback: (data: View) => void): Subscription
subscribe(
eventType:
| LifeCycleEventType.SESSION_RENEWED
| LifeCycleEventType.RESOURCE_ADDED_TO_BATCH
| LifeCycleEventType.DOM_MUTATED
| LifeCycleEventType.BEFORE_UNLOAD
| LifeCycleEventType.ACTION_DISCARDED,
| LifeCycleEventType.AUTO_ACTION_DISCARDED,
callback: () => void
): Subscription
subscribe(eventType: LifeCycleEventType, callback: (data?: any) => void) {
Expand Down
6 changes: 3 additions & 3 deletions packages/rum/src/parentContexts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,15 +36,15 @@ export function startParentContexts(location: Location, lifeCycle: LifeCycle, se
currentSessionId = session.getId()
})

lifeCycle.subscribe(LifeCycleEventType.ACTION_CREATED, (internalContext) => {
lifeCycle.subscribe(LifeCycleEventType.AUTO_ACTION_CREATED, (internalContext) => {
currentAction = internalContext
})

lifeCycle.subscribe(LifeCycleEventType.ACTION_COMPLETED, () => {
lifeCycle.subscribe(LifeCycleEventType.AUTO_ACTION_COMPLETED, () => {
currentAction = undefined
})

lifeCycle.subscribe(LifeCycleEventType.ACTION_DISCARDED, () => {
lifeCycle.subscribe(LifeCycleEventType.AUTO_ACTION_DISCARDED, () => {
currentAction = undefined
})

Expand Down
56 changes: 26 additions & 30 deletions packages/rum/src/rum.ts
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ export function startRum(
globalContext[key] = value
}),
addUserAction: monitor((name: string, context?: Context) => {
lifeCycle.notify(LifeCycleEventType.ACTION_COMPLETED, { context, name, type: UserActionType.CUSTOM })
lifeCycle.notify(LifeCycleEventType.CUSTOM_ACTION_COLLECTED, { context, name, type: UserActionType.CUSTOM })
}),
getInternalContext: monitor(
(startTime?: number): InternalContext => {
Expand Down Expand Up @@ -383,42 +383,38 @@ function trackCustomUserAction(
lifeCycle: LifeCycle,
handler: (startTime: number, event: RumUserActionEvent, customerContext?: Context) => void
) {
lifeCycle.subscribe(LifeCycleEventType.ACTION_COMPLETED, (userAction) => {
if (userAction.type === UserActionType.CUSTOM) {
handler(
performance.now(),
{
evt: {
category: RumEventCategory.USER_ACTION,
name: userAction.name,
},
userAction: {
type: userAction.type,
},
},
userAction.context
)
}
})
}

function trackAutoUserAction(lifeCycle: LifeCycle, handler: (startTime: number, event: RumUserActionEvent) => void) {
lifeCycle.subscribe(LifeCycleEventType.ACTION_COMPLETED, (userAction) => {
if (userAction.type !== UserActionType.CUSTOM) {
handler(userAction.startTime, {
date: getTimestamp(userAction.startTime),
duration: msToNs(userAction.duration),
lifeCycle.subscribe(LifeCycleEventType.CUSTOM_ACTION_COLLECTED, (userAction) => {
handler(
performance.now(),
{
evt: {
category: RumEventCategory.USER_ACTION,
name: userAction.name,
},
userAction: {
id: userAction.id,
measures: userAction.measures,
type: userAction.type,
},
})
}
},
userAction.context
)
})
}

function trackAutoUserAction(lifeCycle: LifeCycle, handler: (startTime: number, event: RumUserActionEvent) => void) {
lifeCycle.subscribe(LifeCycleEventType.AUTO_ACTION_COMPLETED, (userAction) => {
handler(userAction.startTime, {
date: getTimestamp(userAction.startTime),
duration: msToNs(userAction.duration),
evt: {
category: RumEventCategory.USER_ACTION,
name: userAction.name,
},
userAction: {
id: userAction.id,
measures: userAction.measures,
type: userAction.type,
},
})
})
}

Expand Down
8 changes: 7 additions & 1 deletion packages/rum/src/trackEventCounts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,13 @@ export function trackEventCounts(lifeCycle: LifeCycle, callback: (eventCounts: E
})
)
subscriptions.push(
lifeCycle.subscribe(LifeCycleEventType.ACTION_COMPLETED, () => {
lifeCycle.subscribe(LifeCycleEventType.AUTO_ACTION_COMPLETED, () => {
eventCounts.userActionCount += 1
callback(eventCounts)
})
)
subscriptions.push(
lifeCycle.subscribe(LifeCycleEventType.CUSTOM_ACTION_COLLECTED, () => {
eventCounts.userActionCount += 1
callback(eventCounts)
})
Expand Down
16 changes: 9 additions & 7 deletions packages/rum/src/userActionCollection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,20 +9,22 @@ export enum UserActionType {
CUSTOM = 'custom',
}

type AutoUserActionType = UserActionType.CLICK

export interface UserActionMeasures {
errorCount: number
longTaskCount: number
resourceCount: number
}

interface CustomUserAction {
export interface CustomUserAction {
type: UserActionType.CUSTOM
name: string
context?: Context
}

export interface AutoUserAction {
type: UserActionType.CLICK
type: AutoUserActionType
id: string
name: string
startTime: number
Expand Down Expand Up @@ -66,7 +68,7 @@ function startUserActionManagement(lifeCycle: LifeCycle) {
let currentIdlePageActivitySubscription: { stop: () => void }

return {
create: (type: UserActionType, name: string) => {
create: (type: AutoUserActionType, name: string) => {
if (currentUserAction) {
// Ignore any new user action if another one is already occurring.
return
Expand Down Expand Up @@ -98,16 +100,16 @@ class PendingAutoUserAction {
private startTime: number
private eventCountsSubscription: { eventCounts: EventCounts; stop(): void }

constructor(private lifeCycle: LifeCycle, private type: UserActionType, private name: string) {
constructor(private lifeCycle: LifeCycle, private type: AutoUserActionType, private name: string) {
this.id = generateUUID()
this.startTime = performance.now()
this.eventCountsSubscription = trackEventCounts(lifeCycle)
this.lifeCycle.notify(LifeCycleEventType.ACTION_CREATED, { id: this.id, startTime: this.startTime })
this.lifeCycle.notify(LifeCycleEventType.AUTO_ACTION_CREATED, { id: this.id, startTime: this.startTime })
}

complete(endTime: number) {
const eventCounts = this.eventCountsSubscription.eventCounts
this.lifeCycle.notify(LifeCycleEventType.ACTION_COMPLETED, {
this.lifeCycle.notify(LifeCycleEventType.AUTO_ACTION_COMPLETED, {
duration: endTime - this.startTime,
id: this.id,
measures: {
Expand All @@ -123,7 +125,7 @@ class PendingAutoUserAction {
}

discard() {
this.lifeCycle.notify(LifeCycleEventType.ACTION_DISCARDED)
this.lifeCycle.notify(LifeCycleEventType.AUTO_ACTION_DISCARDED)
this.eventCountsSubscription.stop()
}
}
16 changes: 8 additions & 8 deletions packages/rum/test/parentContexts.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ describe('parentContexts', () => {
it('should return the current action context', () => {
const { lifeCycle, parentContexts } = setupBuilder.build()

lifeCycle.notify(LifeCycleEventType.ACTION_CREATED, { startTime, id: FAKE_ID })
lifeCycle.notify(LifeCycleEventType.AUTO_ACTION_CREATED, { startTime, id: FAKE_ID })

expect(parentContexts.findAction()).toBeDefined()
expect(parentContexts.findAction()!.userAction.id).toBe(FAKE_ID)
Expand All @@ -97,25 +97,25 @@ describe('parentContexts', () => {
it('should return undefined if startTime is before the start of the current action', () => {
const { lifeCycle, parentContexts } = setupBuilder.build()

lifeCycle.notify(LifeCycleEventType.ACTION_CREATED, { startTime, id: FAKE_ID })
lifeCycle.notify(LifeCycleEventType.AUTO_ACTION_CREATED, { startTime, id: FAKE_ID })

expect(parentContexts.findAction(startTime - 1)).toBeUndefined()
})

it('should clear the current action on ACTION_DISCARDED', () => {
it('should clear the current action on AUTO_ACTION_DISCARDED', () => {
const { lifeCycle, parentContexts } = setupBuilder.build()

lifeCycle.notify(LifeCycleEventType.ACTION_CREATED, { startTime, id: FAKE_ID })
lifeCycle.notify(LifeCycleEventType.ACTION_DISCARDED)
lifeCycle.notify(LifeCycleEventType.AUTO_ACTION_CREATED, { startTime, id: FAKE_ID })
lifeCycle.notify(LifeCycleEventType.AUTO_ACTION_DISCARDED)

expect(parentContexts.findAction()).toBeUndefined()
})

it('should clear the current action on ACTION_COMPLETED', () => {
it('should clear the current action on AUTO_ACTION_COMPLETED', () => {
const { lifeCycle, parentContexts } = setupBuilder.build()

lifeCycle.notify(LifeCycleEventType.ACTION_CREATED, { startTime, id: FAKE_ID })
lifeCycle.notify(LifeCycleEventType.ACTION_COMPLETED, undefined as any)
lifeCycle.notify(LifeCycleEventType.AUTO_ACTION_CREATED, { startTime, id: FAKE_ID })
lifeCycle.notify(LifeCycleEventType.AUTO_ACTION_COMPLETED, undefined as any)

expect(parentContexts.findAction()).toBeUndefined()
})
Expand Down
10 changes: 5 additions & 5 deletions packages/rum/test/rum.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import sinon from 'sinon'

import { LifeCycle, LifeCycleEventType } from '../src/lifeCycle'
import { handleResourceEntry, RawRumEvent, RumEvent, RumResourceEvent } from '../src/rum'
import { UserAction, UserActionType } from '../src/userActionCollection'
import { CustomUserAction, UserActionType } from '../src/userActionCollection'
import { SESSION_KEEP_ALIVE_INTERVAL, THROTTLE_VIEW_UPDATE_PERIOD } from '../src/viewCollection'
import { setup, TestSetupBuilder } from './specHelper'

Expand Down Expand Up @@ -215,7 +215,7 @@ describe('rum session', () => {
const FAKE_ERROR: Partial<ErrorMessage> = { message: 'test' }
const FAKE_RESOURCE: Partial<PerformanceEntry> = { name: 'http://foo.com', entryType: 'resource' }
const FAKE_REQUEST: Partial<RequestCompleteEvent> = { url: 'http://foo.com' }
const FAKE_USER_ACTION: UserAction = {
const FAKE_CUSTOM_USER_ACTION: CustomUserAction = {
context: { foo: 'bar' },
name: 'action',
type: UserActionType.CUSTOM,
Expand Down Expand Up @@ -247,7 +247,7 @@ describe('rum session', () => {
stubBuilder.fakeEntry(FAKE_RESOURCE as PerformanceEntry, 'resource')
lifeCycle.notify(LifeCycleEventType.ERROR_COLLECTED, FAKE_ERROR as ErrorMessage)
lifeCycle.notify(LifeCycleEventType.REQUEST_COMPLETED, FAKE_REQUEST as RequestCompleteEvent)
lifeCycle.notify(LifeCycleEventType.ACTION_COMPLETED, FAKE_USER_ACTION)
lifeCycle.notify(LifeCycleEventType.CUSTOM_ACTION_COLLECTED, FAKE_CUSTOM_USER_ACTION)

expect(server.requests.length).toEqual(4)
})
Expand Down Expand Up @@ -289,7 +289,7 @@ describe('rum session', () => {
stubBuilder.fakeEntry(FAKE_RESOURCE as PerformanceEntry, 'resource')
lifeCycle.notify(LifeCycleEventType.REQUEST_COMPLETED, FAKE_REQUEST as RequestCompleteEvent)
lifeCycle.notify(LifeCycleEventType.ERROR_COLLECTED, FAKE_ERROR as ErrorMessage)
lifeCycle.notify(LifeCycleEventType.ACTION_COMPLETED, FAKE_USER_ACTION)
lifeCycle.notify(LifeCycleEventType.CUSTOM_ACTION_COLLECTED, FAKE_CUSTOM_USER_ACTION)

expect(server.requests.length).toEqual(0)
})
Expand Down Expand Up @@ -529,7 +529,7 @@ describe('rum user action', () => {
const { server, lifeCycle } = setupBuilder.build()
server.requests = []

lifeCycle.notify(LifeCycleEventType.ACTION_COMPLETED, {
lifeCycle.notify(LifeCycleEventType.CUSTOM_ACTION_COLLECTED, {
context: { fooBar: 'foo' },
name: 'hello',
type: UserActionType.CUSTOM,
Expand Down
7 changes: 4 additions & 3 deletions packages/rum/test/trackEventCounts.spec.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { ErrorMessage, objectValues } from '@datadog/browser-core'
import { LifeCycle, LifeCycleEventType } from '../src/lifeCycle'
import { EventCounts, trackEventCounts } from '../src/trackEventCounts'
import { UserAction } from '../src/userActionCollection'
import { AutoUserAction, CustomUserAction } from '../src/userActionCollection'

describe('trackEventCounts', () => {
let lifeCycle: LifeCycle
Expand Down Expand Up @@ -34,8 +34,9 @@ describe('trackEventCounts', () => {
it('tracks user actions', () => {
const { eventCounts } = trackEventCounts(lifeCycle)
const userAction = {}
lifeCycle.notify(LifeCycleEventType.ACTION_COMPLETED, userAction as UserAction)
expect(eventCounts.userActionCount).toBe(1)
lifeCycle.notify(LifeCycleEventType.AUTO_ACTION_COMPLETED, userAction as AutoUserAction)
lifeCycle.notify(LifeCycleEventType.CUSTOM_ACTION_COLLECTED, userAction as CustomUserAction)
expect(eventCounts.userActionCount).toBe(2)
})

it('tracks resources', () => {
Expand Down
10 changes: 5 additions & 5 deletions packages/rum/test/userActionCollection.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,9 @@ describe('startUserActionCollection', () => {
.withFakeClock()
.withUserActionCollection()
.beforeBuild((lifeCycle) => {
lifeCycle.subscribe(LifeCycleEventType.ACTION_CREATED, createSpy)
lifeCycle.subscribe(LifeCycleEventType.ACTION_COMPLETED, pushEvent)
lifeCycle.subscribe(LifeCycleEventType.ACTION_DISCARDED, discardSpy)
lifeCycle.subscribe(LifeCycleEventType.AUTO_ACTION_CREATED, createSpy)
lifeCycle.subscribe(LifeCycleEventType.AUTO_ACTION_COMPLETED, pushEvent)
lifeCycle.subscribe(LifeCycleEventType.AUTO_ACTION_DISCARDED, discardSpy)
})
})

Expand Down Expand Up @@ -152,7 +152,7 @@ describe('newUserAction', () => {

it('ignores any starting user action while another one is happening', () => {
const { lifeCycle, clock } = setupBuilder.build()
lifeCycle.subscribe(LifeCycleEventType.ACTION_COMPLETED, pushEvent)
lifeCycle.subscribe(LifeCycleEventType.AUTO_ACTION_COMPLETED, pushEvent)

newClick('test-1')
newClick('test-2')
Expand All @@ -168,7 +168,7 @@ describe('newUserAction', () => {
it('counts errors occurring during the user action', () => {
const { lifeCycle, clock } = setupBuilder.build()
const error = {}
lifeCycle.subscribe(LifeCycleEventType.ACTION_COMPLETED, pushEvent)
lifeCycle.subscribe(LifeCycleEventType.AUTO_ACTION_COMPLETED, pushEvent)

newClick('test-1')

Expand Down
Loading

0 comments on commit b0c4ca2

Please sign in to comment.