Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🐛 fix current action context reset on custom action #444

Merged
merged 1 commit into from
Jul 10, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
mquentin marked this conversation as resolved.
Show resolved Hide resolved

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