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

✨ Implement addTiming #668

Merged
merged 29 commits into from
Jan 11, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
b4b8a78
✨ Implement addTiming
webNeat Dec 24, 2020
740ffb1
Fix closure bug
webNeat Jan 5, 2021
5eb9c52
👌 Rename setCustomTiming to addTiming for consistency
webNeat Jan 5, 2021
a43de62
👌 Use mocked clock on tests
webNeat Jan 5, 2021
7d2c641
👌 Convert timings to nanoseconds
webNeat Jan 5, 2021
5016c63
Add feature flag for addTiming
webNeat Jan 6, 2021
5067d43
👌 Remove time parameter
webNeat Jan 6, 2021
a6d6bbe
Remove useless parameter
webNeat Jan 7, 2021
1f1ae99
👌 Convert timings to nanoseconds on viewCollection
webNeat Jan 7, 2021
226afdb
👌 Remove uneeded line
webNeat Jan 7, 2021
6430fc1
👌 Update viewCollection test to include custom timings
webNeat Jan 7, 2021
c3ccd1c
👌 Remove useless parameter
webNeat Jan 7, 2021
e738352
Merge last master changes
webNeat Jan 7, 2021
ed06dc5
Add addTiming to public RUM API
webNeat Jan 7, 2021
a83d75c
Fix ESLint errors
webNeat Jan 7, 2021
b71fbe9
Use more specific rule
webNeat Jan 8, 2021
7a0b0b8
Use better typing for mock configuration
webNeat Jan 8, 2021
cc74668
👌 Use better typing for mock data
webNeat Jan 8, 2021
1380c25
👌 Do not include customTimings when empty
webNeat Jan 8, 2021
003f548
👌 Rename mapObject to mapValues
webNeat Jan 8, 2021
ab222cf
👌 Add custom timings to rumEvent.types
webNeat Jan 8, 2021
2ee6e03
Fix Linting errors
webNeat Jan 8, 2021
555e463
Merge branch 'master' into amine/add-timing
webNeat Jan 8, 2021
adfcfe8
Fix tests and Typescript error
webNeat Jan 8, 2021
636e0c1
Merge branch 'master' into amine/add-timing
webNeat Jan 11, 2021
a369c17
👌 Add custom timings to rumEvent.types
webNeat Jan 11, 2021
9cd9261
👌 Use better typing
webNeat Jan 11, 2021
0f19e60
👌 Remove useless attribute
webNeat Jan 11, 2021
d671d92
Fix linter errors
webNeat Jan 11, 2021
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
8 changes: 8 additions & 0 deletions packages/core/src/tools/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,14 @@ export function isEmptyObject(object: object) {
return Object.keys(object).length === 0
}

export function mapValues<A, B>(object: { [key: string]: A }, fn: (arg: A) => B) {
const newObject: { [key: string]: B } = {}
for (const key of Object.keys(object)) {
newObject[key] = fn(object[key])
}
return newObject
}

/**
* inspired by https://mathiasbynens.be/notes/globalthis
*/
Expand Down
8 changes: 6 additions & 2 deletions packages/rum-core/src/boot/rum.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ export function startRum(userConfiguration: RumUserConfiguration, getCommonConte
)
})

const { parentContexts, addError, addAction } = startRumEventCollection(
const { parentContexts, addError, addAction, addTiming } = startRumEventCollection(
userConfiguration.applicationId,
location,
lifeCycle,
Expand All @@ -52,6 +52,8 @@ export function startRum(userConfiguration: RumUserConfiguration, getCommonConte
return {
addAction,
addError,
addTiming,
configuration,
getInternalContext: internalContext.get,
}
}
Expand All @@ -69,7 +71,7 @@ export function startRumEventCollection(
startRumAssembly(applicationId, configuration, lifeCycle, session, parentContexts, getCommonContext)
startLongTaskCollection(lifeCycle, configuration)
startResourceCollection(lifeCycle, configuration, session)
startViewCollection(lifeCycle, configuration, location)
const { addTiming } = startViewCollection(lifeCycle, configuration, location)
const { addError } = startErrorCollection(lifeCycle, configuration)
const { addAction } = startActionCollection(lifeCycle, configuration)

Expand All @@ -78,6 +80,8 @@ export function startRumEventCollection(
addError,
parentContexts,

addTiming,

stop() {
// prevent batch from previous tests to keep running and send unwanted requests
// could be replaced by stopping all the component when they will all have a stop method
Expand Down
40 changes: 39 additions & 1 deletion packages/rum-core/src/boot/rumPublicApi.spec.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,16 @@
import { ErrorSource, ONE_SECOND } from '@datadog/browser-core'
import { Configuration, ErrorSource, ONE_SECOND } from '@datadog/browser-core'
import { setup, TestSetupBuilder } from '../../test/specHelper'
import { ActionType } from '../domain/rumEventsCollection/action/trackActions'
import { makeRumPublicApi, RumPublicApi, RumUserConfiguration, StartRum } from './rumPublicApi'

const configuration: Partial<Configuration> = {
isEnabled: () => false,
}
const noopStartRum = () => ({
addAction: () => undefined,
addError: () => undefined,
addTiming: () => undefined,
configuration: configuration as Configuration,
getInternalContext: () => undefined,
})
const DEFAULT_INIT_CONFIGURATION = { applicationId: 'xxx', clientToken: 'xxx' }
Expand Down Expand Up @@ -385,4 +390,37 @@ describe('rum entry', () => {
expect(errorSpy).toHaveBeenCalledTimes(3)
})
})

describe('addTiming', () => {
let addTimingSpy: jasmine.Spy<ReturnType<StartRum>['addTiming']>
let errorSpy: jasmine.Spy<() => void>
let rumGlobal: RumPublicApi
let setupBuilder: TestSetupBuilder

beforeEach(() => {
addTimingSpy = jasmine.createSpy()
errorSpy = spyOn(console, 'error')
const otherConfiguration: Partial<Configuration> = {
isEnabled: () => true,
}
rumGlobal = makeRumPublicApi(() => ({
...noopStartRum(),
addTiming: addTimingSpy,
configuration: otherConfiguration as Configuration,
}))
setupBuilder = setup()
})

afterEach(() => {
setupBuilder.cleanup()
})

it('should add custom timings', () => {
rumGlobal.init(DEFAULT_INIT_CONFIGURATION)
// tslint:disable-next-line: no-unsafe-any
;(rumGlobal as any).addTiming('foo')
expect(addTimingSpy.calls.argsFor(0)[0]).toEqual('foo')
expect(errorSpy).not.toHaveBeenCalled()
})
})
})
9 changes: 9 additions & 0 deletions packages/rum-core/src/boot/rumPublicApi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {
buildCookieOptions,
checkCookiesAuthorized,
checkIsNotLocalFile,
Configuration,
Context,
createContextManager,
deepClone,
Expand Down Expand Up @@ -67,7 +68,11 @@ export function makeRumPublicApi(startRumImpl: StartRum) {
userConfiguration.clientToken = userConfiguration.publicApiKey
}

let configuration: Configuration
let addTiming: ReturnType<StartRum>['addTiming']
;({
addTiming,
configuration,
addAction: addActionStrategy,
addError: addErrorStrategy,
getInternalContext: getInternalContextStrategy,
Expand All @@ -78,6 +83,10 @@ export function makeRumPublicApi(startRumImpl: StartRum) {
beforeInitAddAction.drain(([action, commonContext]) => addActionStrategy(action, commonContext))
beforeInitAddError.drain(([error, commonContext]) => addErrorStrategy(error, commonContext))

if (configuration.isEnabled('custom-timings')) {
;(rumGlobal as any).addTiming = addTiming
}

isAlreadyInitialized = true
}),

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -829,3 +829,91 @@ describe('rum view measures', () => {
})
})
})

describe('rum track custom timings', () => {
let setupBuilder: TestSetupBuilder
let handler: jasmine.Spy
let getViewEvent: (index: number) => View
let addTiming: (name: string, inInitialView?: boolean, time?: number) => void

beforeEach(() => {
;({ handler, getViewEvent } = spyOnViews())

setupBuilder = setup()
.withFakeLocation('/foo')
.withFakeClock()
.beforeBuild(({ location, lifeCycle }) => {
lifeCycle.subscribe(LifeCycleEventType.VIEW_UPDATED, handler)
;({ addTiming } = trackViews(location, lifeCycle))
})
})

afterEach(() => {
setupBuilder.cleanup()
})

it('should add custom timing to current view', () => {
const { clock } = setupBuilder.build()
history.pushState({}, '', '/bar')
const currentViewId = getViewEvent(2).id
clock.tick(20)
addTiming('foo', false)

const event = getViewEvent(3)
expect(event.id).toEqual(currentViewId)
expect(event.customTimings).toEqual({ foo: 20 })
})

it('should add custom timing to initial view', () => {
const { clock } = setupBuilder.build()
clock.tick(20)
history.pushState({}, '', '/bar')
const initialViewId = getViewEvent(0).id

clock.tick(20)
addTiming('foo', true)

const event = getViewEvent(3)
expect(event.id).toEqual(initialViewId)
expect(event.customTimings).toEqual({ foo: 40 })
})

it('should add multiple custom timings', () => {
const { clock } = setupBuilder.build()
clock.tick(20)
addTiming('foo', false)

clock.tick(10)
addTiming('bar', false)

const event = getViewEvent(2)
expect(event.customTimings).toEqual({
bar: 30,
foo: 20,
})
})

it('should update custom timing', () => {
const { clock } = setupBuilder.build()
clock.tick(20)
addTiming('foo', false)

clock.tick(10)
addTiming('bar', false)

let event = getViewEvent(2)
expect(event.customTimings).toEqual({
bar: 30,
foo: 20,
})

clock.tick(20)
addTiming('foo', false)

event = getViewEvent(3)
expect(event.customTimings).toEqual({
bar: 30,
foo: 50,
})
})
})
Original file line number Diff line number Diff line change
@@ -1,4 +1,13 @@
import { addEventListener, DOM_EVENT, generateUUID, monitor, noop, ONE_MINUTE, throttle } from '@datadog/browser-core'
import {
addEventListener,
DOM_EVENT,
generateUUID,
monitor,
msToNs,
noop,
ONE_MINUTE,
throttle,
} from '@datadog/browser-core'

import { supportPerformanceTimingEvent } from '../../../browser/performanceCollection'
import { LifeCycle, LifeCycleEventType } from '../../lifeCycle'
Expand All @@ -11,6 +20,7 @@ export interface View {
location: Location
referrer: string
timings: Timings
customTimings: ViewCustomTimings
eventCounts: EventCounts
documentVersion: number
startTime: number
Expand All @@ -33,6 +43,10 @@ export enum ViewLoadingType {
ROUTE_CHANGE = 'route_change',
}

export interface ViewCustomTimings {
[key: string]: number
}

export const THROTTLE_VIEW_UPDATE_PERIOD = 3000
export const SESSION_KEEP_ALIVE_INTERVAL = 5 * ONE_MINUTE

Expand Down Expand Up @@ -83,6 +97,11 @@ export function trackViews(location: Location, lifeCycle: LifeCycle) {
)

return {
addTiming(name: string, inInitialView = false) {
const view = inInitialView ? initialView : currentView
view.addTiming(name)
view.triggerUpdate()
},
stop() {
stopTimingsTracking()
currentView.end()
Expand All @@ -107,6 +126,7 @@ function newView(
userActionCount: 0,
}
let timings: Timings = {}
const customTimings: ViewCustomTimings = {}
let documentVersion = 0
let cumulativeLayoutShift: number | undefined
let loadingTime: number | undefined
Expand Down Expand Up @@ -154,6 +174,7 @@ function newView(
documentVersion += 1
lifeCycle.notify(LifeCycleEventType.VIEW_UPDATED, {
cumulativeLayoutShift,
customTimings,
documentVersion,
eventCounts,
id,
Expand Down Expand Up @@ -193,6 +214,9 @@ function newView(
setLoadEvent(newTimings.loadEvent)
}
},
addTiming(name: string) {
customTimings[name] = performance.now() - startTime
},
updateLocation(newLocation: Location) {
location = { ...newLocation }
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,13 @@ describe('viewCollection', () => {

it('should create view from view update', () => {
const { lifeCycle, rawRumEvents } = setupBuilder.build()
const location: Partial<Location> = {}
const view = {
cumulativeLayoutShift: 1,
customTimings: {
bar: 20,
foo: 10,
},
documentVersion: 3,
duration: 100,
eventCounts: {
Expand All @@ -37,7 +42,7 @@ describe('viewCollection', () => {
isActive: false,
loadingTime: 20,
loadingType: ViewLoadingType.INITIAL_LOAD,
location: {},
location: location as Location,
referrer: '',
startTime: 1234,
timings: {
Expand All @@ -51,7 +56,7 @@ describe('viewCollection', () => {
loadEvent: 10,
},
}
lifeCycle.notify(LifeCycleEventType.VIEW_UPDATED, view as View)
lifeCycle.notify(LifeCycleEventType.VIEW_UPDATED, view)

expect(rawRumEvents[rawRumEvents.length - 1].startTime).toBe(1234)
expect(rawRumEvents[rawRumEvents.length - 1].rawRumEvent).toEqual({
Expand All @@ -65,6 +70,10 @@ describe('viewCollection', () => {
count: 10,
},
cumulativeLayoutShift: 1,
customTimings: {
bar: 20 * 1e6,
foo: 10 * 1e6,
},
domComplete: 10 * 1e6,
domContentLoaded: 10 * 1e6,
domInteractive: 10 * 1e6,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Configuration, getTimestamp, msToNs } from '@datadog/browser-core'
import { Configuration, getTimestamp, isEmptyObject, mapValues, msToNs } from '@datadog/browser-core'
import { RawRumViewEvent, RumEventType } from '../../../rawRumEvent.types'
import { LifeCycle, LifeCycleEventType } from '../../lifeCycle'
import { trackViews, View } from './trackViews'
Expand Down Expand Up @@ -46,6 +46,9 @@ function processViewUpdate(view: View) {
timeSpent: msToNs(view.duration),
},
}
if (!isEmptyObject(view.customTimings)) {
viewEvent.view.customTimings = mapValues(view.customTimings, msToNs)
}
return {
rawRumEvent: viewEvent,
startTime: view.startTime,
Expand Down
3 changes: 2 additions & 1 deletion packages/rum-core/src/rawRumEvent.types.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { Context, ErrorSource, ResourceType } from '@datadog/browser-core'
import { ActionType } from './domain/rumEventsCollection/action/trackActions'
import { PerformanceResourceDetailsElement } from './domain/rumEventsCollection/resource/resourceUtils'
import { ViewLoadingType } from './domain/rumEventsCollection/view/trackViews'
import { ViewCustomTimings, ViewLoadingType } from './domain/rumEventsCollection/view/trackViews'

export enum RumEventType {
ACTION = 'action',
Expand Down Expand Up @@ -60,6 +60,7 @@ export interface RawRumViewEvent {
firstInputDelay?: number
firstInputTime?: number
cumulativeLayoutShift?: number
customTimings?: ViewCustomTimings
largestContentfulPaint?: number
domInteractive?: number
domContentLoaded?: number
Expand Down
6 changes: 6 additions & 0 deletions packages/rum-core/src/rumEvent.types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -459,6 +459,12 @@ export type RumViewEvent = CommonProperties & {
* Duration in ns to the end of the load event handler execution
*/
readonly load_event?: number
/**
* User custom timings of the view
*/
readonly custom_timings?: {
[k: string]: number
}
/**
* Whether the View corresponding to this event is considered active
*/
Expand Down
2 changes: 1 addition & 1 deletion rum-events-format