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

✨ [RUMF-775] implement Largest Contentful Paint #624

Merged
merged 5 commits into from
Nov 20, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
1 change: 1 addition & 0 deletions packages/core/src/tools/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ export enum DOM_EVENT {
TOUCH_START = 'touchstart',
VISIBILITY_CHANGE = 'visibilitychange',
DOM_CONTENT_LOADED = 'DOMContentLoaded',
POINTER_DOWN = 'pointerdown',
HASH_CHANGE = 'hashchange',
PAGE_HIDE = 'pagehide',
}
Expand Down
11 changes: 9 additions & 2 deletions packages/rum/src/browser/performanceCollection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,17 @@ export interface RumPerformanceNavigationTiming {
loadEventEnd: number
}

export interface RumLargestContentfulPaintTiming {
entryType: 'largest-contentful-paint'
startTime: number
}

export type RumPerformanceEntry =
| RumPerformanceResourceTiming
| RumPerformanceLongTaskTiming
| RumPerformancePaintTiming
| RumPerformanceNavigationTiming
| RumLargestContentfulPaintTiming

function supportPerformanceObject() {
return window.performance !== undefined && 'getEntries' in performance
Expand All @@ -79,7 +85,7 @@ export function startPerformanceCollection(lifeCycle: LifeCycle, configuration:
const observer = new PerformanceObserver(
monitor((entries) => handlePerformanceEntries(lifeCycle, configuration, entries.getEntries()))
)
const entryTypes = ['resource', 'navigation', 'longtask', 'paint']
const entryTypes = ['resource', 'navigation', 'longtask', 'paint', 'largest-contentful-paint']

observer.observe({ entryTypes })

Expand Down Expand Up @@ -168,7 +174,8 @@ function handlePerformanceEntries(lifeCycle: LifeCycle, configuration: Configura
entry.entryType === 'resource' ||
entry.entryType === 'navigation' ||
entry.entryType === 'paint' ||
entry.entryType === 'longtask'
entry.entryType === 'longtask' ||
entry.entryType === 'largest-contentful-paint'
) {
handleRumPerformanceEntry(lifeCycle, configuration, entry as RumPerformanceEntry)
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,19 @@
import { restorePageVisibility, setPageVisibility } from '@datadog/browser-core'
import { createNewEvent, DOM_EVENT, restorePageVisibility, setPageVisibility } from '@datadog/browser-core'
import { setup, TestSetupBuilder } from '../../../../test/specHelper'
import { RumPerformanceNavigationTiming, RumPerformancePaintTiming } from '../../../browser/performanceCollection'
import {
RumLargestContentfulPaintTiming,
RumPerformanceNavigationTiming,
RumPerformancePaintTiming,
} from '../../../browser/performanceCollection'
import { LifeCycleEventType } from '../../lifeCycle'
import { resetFirstHidden } from './trackFirstHidden'
import { Timings, trackFirstContentfulPaint, trackNavigationTimings, trackTimings } from './trackTimings'
import {
Timings,
trackFirstContentfulPaint,
trackLargestContentfulPaint,
trackNavigationTimings,
trackTimings,
} from './trackTimings'

const FAKE_PAINT_ENTRY: RumPerformancePaintTiming = {
entryType: 'paint',
Expand All @@ -19,6 +29,11 @@ const FAKE_NAVIGATION_ENTRY: RumPerformanceNavigationTiming = {
loadEventEnd: 567,
}

const FAKE_LARGEST_CONTENTFUL_PAINT_ENTRY: RumLargestContentfulPaintTiming = {
entryType: 'largest-contentful-paint',
startTime: 789,
}

describe('trackTimings', () => {
let setupBuilder: TestSetupBuilder
let spy: jasmine.Spy<(value: Partial<Timings>) => void>
Expand Down Expand Up @@ -115,3 +130,56 @@ describe('trackFirstContentfulPaint', () => {
expect(spy).not.toHaveBeenCalled()
})
})

describe('largestContentfulPaint', () => {
let setupBuilder: TestSetupBuilder
let spy: jasmine.Spy<(value: number) => void>
let emitter: Element

beforeEach(() => {
spy = jasmine.createSpy()
emitter = document.createElement('div')
setupBuilder = setup().beforeBuild(({ lifeCycle }) => {
return trackLargestContentfulPaint(lifeCycle, emitter, spy)
})
resetFirstHidden()
})

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

it('should provide the largest contentful paint timing', () => {
const { lifeCycle } = setupBuilder.build()

lifeCycle.notify(LifeCycleEventType.PERFORMANCE_ENTRY_COLLECTED, FAKE_LARGEST_CONTENTFUL_PAINT_ENTRY)
expect(spy).toHaveBeenCalledTimes(1)
BenoitZugmeyer marked this conversation as resolved.
Show resolved Hide resolved
expect(spy).toHaveBeenCalledWith(789)
})

it('should not be present if it happens after a user interaction', () => {
const { lifeCycle } = setupBuilder.build()

const event = createNewEvent(DOM_EVENT.KEY_DOWN)
Object.defineProperty(event, 'timeStamp', {
get() {
return 1
},
})
emitter.dispatchEvent(event)
BenoitZugmeyer marked this conversation as resolved.
Show resolved Hide resolved

lifeCycle.notify(LifeCycleEventType.PERFORMANCE_ENTRY_COLLECTED, FAKE_LARGEST_CONTENTFUL_PAINT_ENTRY)
expect(spy).not.toHaveBeenCalled()
})

it('should not be present if the page is hidden', () => {
setPageVisibility('hidden')
const { lifeCycle } = setupBuilder.build()

lifeCycle.notify(LifeCycleEventType.PERFORMANCE_ENTRY_COLLECTED, FAKE_LARGEST_CONTENTFUL_PAINT_ENTRY)

expect(spy).not.toHaveBeenCalled()
})
})
47 changes: 47 additions & 0 deletions packages/rum/src/domain/rumEventsCollection/view/trackTimings.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { addEventListeners, DOM_EVENT, EventEmitter } from '@datadog/browser-core'
import { LifeCycle, LifeCycleEventType } from '../../lifeCycle'
import { trackFirstHidden } from './trackFirstHidden'

Expand All @@ -7,6 +8,7 @@ export interface Timings {
domContentLoaded?: number
domComplete?: number
loadEventEnd?: number
largestContentfulPaint?: number
}

export function trackTimings(lifeCycle: LifeCycle, callback: (timings: Timings) => void) {
Expand All @@ -20,11 +22,17 @@ export function trackTimings(lifeCycle: LifeCycle, callback: (timings: Timings)
const { stop: stopFCPTracking } = trackFirstContentfulPaint(lifeCycle, (firstContentfulPaint) =>
setTimings({ firstContentfulPaint })
)
const { stop: stopLCPTracking } = trackLargestContentfulPaint(lifeCycle, window, (largestContentfulPaint) => {
setTimings({
largestContentfulPaint,
})
})

return {
stop() {
stopNavigationTracking()
stopFCPTracking()
stopLCPTracking()
},
}
}
Expand Down Expand Up @@ -57,3 +65,42 @@ export function trackFirstContentfulPaint(lifeCycle: LifeCycle, callback: (fcp:
})
return { stop }
}

export function trackLargestContentfulPaint(
lifeCycle: LifeCycle,
emitter: EventEmitter,
callback: (value: number) => void
) {
const firstHidden = trackFirstHidden()

// Ignore entries that come after the first user interaction
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it could be helpful to add a link to the spec of these new timings or maybe describe the different rules in a function comment.

Copy link
Member Author

@BenoitZugmeyer BenoitZugmeyer Nov 19, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't find a link to show that it is needed. All the documentation (here, spec draft readme, spec draft ) seems to agree that the browser won't send those performance entries after the first user interaction. I think that's just a safeguard that web-vitals implemented because of some Chrome issues.

let firstInteractionTimestamp: number = Infinity
const { stop: stopEventListener } = addEventListeners(
emitter,
[DOM_EVENT.POINTER_DOWN, DOM_EVENT.KEY_DOWN, DOM_EVENT.SCROLL],
BenoitZugmeyer marked this conversation as resolved.
Show resolved Hide resolved
(event) => {
firstInteractionTimestamp = event.timeStamp
},
{ capture: true, once: true }
)

const { unsubscribe: unsubcribeLifeCycle } = lifeCycle.subscribe(
LifeCycleEventType.PERFORMANCE_ENTRY_COLLECTED,
(entry) => {
if (
entry.entryType === 'largest-contentful-paint' &&
entry.startTime < firstInteractionTimestamp &&
entry.startTime < firstHidden.timeStamp
) {
callback(entry.startTime)
}
}
)

return {
stop() {
stopEventListener()
unsubcribeLifeCycle()
},
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
import { createRawRumEvent } from '../../../../test/fixtures'
import { setup, TestSetupBuilder } from '../../../../test/specHelper'
import { RumPerformanceNavigationTiming, RumPerformancePaintTiming } from '../../../browser/performanceCollection'
import {
RumLargestContentfulPaintTiming,
RumPerformanceNavigationTiming,
RumPerformancePaintTiming,
} from '../../../browser/performanceCollection'
import { RawRumEvent, RumEventCategory } from '../../../types'
import { RumEventType } from '../../../typesV2'
import { LifeCycleEventType } from '../../lifeCycle'
Expand All @@ -20,6 +24,10 @@ const FAKE_PAINT_ENTRY: RumPerformancePaintTiming = {
name: 'first-contentful-paint',
startTime: 123,
}
const FAKE_LARGEST_CONTENTFUL_PAINT_ENTRY: RumLargestContentfulPaintTiming = {
entryType: 'largest-contentful-paint',
startTime: 789,
}
const FAKE_NAVIGATION_ENTRY: RumPerformanceNavigationTiming = {
domComplete: 456,
domContentLoadedEventEnd: 345,
Expand Down Expand Up @@ -491,6 +499,7 @@ describe('rum view measures', () => {
expect(getViewEvent(0).timings).toEqual({})

lifeCycle.notify(LifeCycleEventType.PERFORMANCE_ENTRY_COLLECTED, FAKE_PAINT_ENTRY)
lifeCycle.notify(LifeCycleEventType.PERFORMANCE_ENTRY_COLLECTED, FAKE_LARGEST_CONTENTFUL_PAINT_ENTRY)
lifeCycle.notify(LifeCycleEventType.PERFORMANCE_ENTRY_COLLECTED, FAKE_NAVIGATION_ENTRY)
expect(getHandledCount()).toEqual(1)

Expand All @@ -502,6 +511,7 @@ describe('rum view measures', () => {
domContentLoaded: 345,
domInteractive: 234,
firstContentfulPaint: 123,
largestContentfulPaint: 789,
loadEventEnd: 567,
})
expect(getViewEvent(2).timings).toEqual({})
Expand All @@ -525,6 +535,7 @@ describe('rum view measures', () => {
expect(getHandledCount()).toEqual(3)

lifeCycle.notify(LifeCycleEventType.PERFORMANCE_ENTRY_COLLECTED, FAKE_PAINT_ENTRY)
lifeCycle.notify(LifeCycleEventType.PERFORMANCE_ENTRY_COLLECTED, FAKE_LARGEST_CONTENTFUL_PAINT_ENTRY)
lifeCycle.notify(LifeCycleEventType.PERFORMANCE_ENTRY_COLLECTED, FAKE_NAVIGATION_ENTRY)

clock.tick(THROTTLE_VIEW_UPDATE_PERIOD)
Expand Down Expand Up @@ -552,6 +563,7 @@ describe('rum view measures', () => {
domContentLoaded: 345,
domInteractive: 234,
firstContentfulPaint: 123,
largestContentfulPaint: 789,
loadEventEnd: 567,
})
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ describe('viewCollection', () => {
domContentLoaded: 10,
domInteractive: 10,
firstContentfulPaint: 10,
largestContentfulPaint: 10,
loadEventEnd: 10,
},
}
Expand Down Expand Up @@ -117,6 +118,7 @@ describe('viewCollection V2', () => {
domContentLoaded: 10,
domInteractive: 10,
firstContentfulPaint: 10,
largestContentfulPaint: 10,
loadEventEnd: 10,
},
}
Expand All @@ -140,6 +142,7 @@ describe('viewCollection V2', () => {
count: 10,
},
firstContentfulPaint: 10 * 1e6,
largestContentfulPaint: 10 * 1e6,
loadEventEnd: 10 * 1e6,
loadingTime: 20 * 1e6,
loadingType: ViewLoadingType.INITIAL_LOAD,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ function processViewUpdateV2(view: View) {
count: view.eventCounts.errorCount,
},
firstContentfulPaint: msToNs(view.timings.firstContentfulPaint),
largestContentfulPaint: msToNs(view.timings.largestContentfulPaint),
loadEventEnd: msToNs(view.timings.loadEventEnd),
loadingTime: msToNs(view.loadingTime),
loadingType: view.loadingType,
Expand Down
1 change: 1 addition & 0 deletions packages/rum/src/typesV2.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ export interface RumViewEventV2 {
view: {
loadingType: ViewLoadingType
firstContentfulPaint?: number
largestContentfulPaint?: number
domInteractive?: number
domContentLoaded?: number
domComplete?: number
Expand Down