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.

Sign up for GitHub

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

✨ Discard long FCP and LCP #1045

Merged
merged 3 commits into from
Sep 15, 2021
Merged

✨ Discard long FCP and LCP #1045

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f9ef3d1
✨ Discard long FCP and LCP
amortemousque Sep 13, 2021
183e597
👌 Highlight long timing delay
amortemousque Sep 14, 2021
3083272
Merge branch 'main' into aymeric/discard-long-FCP-and-LCP
amortemousque Sep 15, 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
43 changes: 33 additions & 10 deletions packages/rum-core/src/domain/rumEventsCollection/view/trackInitialViewTimings.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,12 @@ import { LifeCycleEventType } from '../../lifeCycle'
import { resetFirstHidden } from './trackFirstHidden'
import {
Timings,
trackFirstContentfulPaint,
trackFirstContentfulPaintTiming,
trackFirstInputTimings,
trackLargestContentfulPaint,
trackLargestContentfulPaintTiming,
trackNavigationTimings,
trackInitialViewTimings,
TIMING_MAXIMUM_DELAY,
} from './trackInitialViewTimings'

const FAKE_PAINT_ENTRY: RumPerformancePaintTiming = {
Expand Down Expand Up @@ -105,13 +106,13 @@ describe('trackNavigationTimings', () => {
})
})

describe('trackFirstContentfulPaint', () => {
describe('trackFirstContentfulPaintTiming', () => {
let setupBuilder: TestSetupBuilder
let fcpCallback: jasmine.Spy<(value: RelativeTime) => void>

beforeEach(() => {
fcpCallback = jasmine.createSpy()
setupBuilder = setup().beforeBuild(({ lifeCycle }) => trackFirstContentfulPaint(lifeCycle, fcpCallback))
setupBuilder = setup().beforeBuild(({ lifeCycle }) => trackFirstContentfulPaintTiming(lifeCycle, fcpCallback))
resetFirstHidden()
})

Expand All @@ -130,23 +131,35 @@ describe('trackFirstContentfulPaint', () => {
expect(fcpCallback).toHaveBeenCalledWith(123 as RelativeTime)
})

it('should not set the first contentful paint if the page is hidden', () => {
it('should be discarded if the page is hidden', () => {
setPageVisibility('hidden')
const { lifeCycle } = setupBuilder.build()
lifeCycle.notify(LifeCycleEventType.PERFORMANCE_ENTRY_COLLECTED, FAKE_PAINT_ENTRY)
expect(fcpCallback).not.toHaveBeenCalled()
})

it('should be discarded if it is reported after a long time', () => {
const { lifeCycle } = setupBuilder.build()

lifeCycle.notify(LifeCycleEventType.PERFORMANCE_ENTRY_COLLECTED, {
...FAKE_PAINT_ENTRY,
startTime: TIMING_MAXIMUM_DELAY as RelativeTime,
})
expect(fcpCallback).not.toHaveBeenCalled()
})
})

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

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

Expand All @@ -164,7 +177,7 @@ describe('largestContentfulPaint', () => {
expect(lcpCallback).toHaveBeenCalledWith(789 as RelativeTime)
})

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

emitter.dispatchEvent(createNewEvent(DOM_EVENT.KEY_DOWN, { timeStamp: 1 }))
Expand All @@ -173,14 +186,24 @@ describe('largestContentfulPaint', () => {
expect(lcpCallback).not.toHaveBeenCalled()
})

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

lifeCycle.notify(LifeCycleEventType.PERFORMANCE_ENTRY_COLLECTED, FAKE_LARGEST_CONTENTFUL_PAINT_ENTRY)

expect(lcpCallback).not.toHaveBeenCalled()
})

it('should be discarded if it is reported after a long time', () => {
const { lifeCycle } = setupBuilder.build()

lifeCycle.notify(LifeCycleEventType.PERFORMANCE_ENTRY_COLLECTED, {
...FAKE_LARGEST_CONTENTFUL_PAINT_ENTRY,
startTime: TIMING_MAXIMUM_DELAY as RelativeTime,
})
expect(lcpCallback).not.toHaveBeenCalled()
})
})

describe('firstInputTimings', () => {
Expand Down Expand Up @@ -209,7 +232,7 @@ describe('firstInputTimings', () => {
expect(fitCallback).toHaveBeenCalledWith({ firstInputDelay: 100, firstInputTime: 1000 })
})

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

Expand Down
28 changes: 15 additions & 13 deletions packages/rum-core/src/domain/rumEventsCollection/view/trackInitialViewTimings.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,15 @@ import {
elapsed,
EventEmitter,
RelativeTime,
timeStampNow,
TimeStamp,
ONE_MINUTE,
} from '@datadog/browser-core'
import { LifeCycle, LifeCycleEventType } from '../../lifeCycle'
import { trackFirstHidden } from './trackFirstHidden'

// Discard LCP and FCP timings above a certain delay to avoid incorrect data
// It happens in some cases like sleep mode or some browser implementations
export const TIMING_MAXIMUM_DELAY = 10 * ONE_MINUTE

export interface Timings {
firstContentfulPaint?: Duration
domInteractive?: Duration
Expand All @@ -30,10 +33,10 @@ export function trackInitialViewTimings(lifeCycle: LifeCycle, callback: (timings
}

const { stop: stopNavigationTracking } = trackNavigationTimings(lifeCycle, setTimings)
const { stop: stopFCPTracking } = trackFirstContentfulPaint(lifeCycle, (firstContentfulPaint) =>
const { stop: stopFCPTracking } = trackFirstContentfulPaintTiming(lifeCycle, (firstContentfulPaint) =>
setTimings({ firstContentfulPaint })
)
const { stop: stopLCPTracking } = trackLargestContentfulPaint(lifeCycle, window, (largestContentfulPaint) => {
const { stop: stopLCPTracking } = trackLargestContentfulPaintTiming(lifeCycle, window, (largestContentfulPaint) => {
setTimings({
largestContentfulPaint,
})
Expand All @@ -55,7 +58,7 @@ export function trackInitialViewTimings(lifeCycle: LifeCycle, callback: (timings
}
}

export function trackNavigationTimings(lifeCycle: LifeCycle, callback: (newTimings: Partial<Timings>) => void) {
export function trackNavigationTimings(lifeCycle: LifeCycle, callback: (timings: Partial<Timings>) => void) {
const { unsubscribe: stop } = lifeCycle.subscribe(LifeCycleEventType.PERFORMANCE_ENTRY_COLLECTED, (entry) => {
if (entry.entryType === 'navigation') {
callback({
Expand All @@ -70,13 +73,14 @@ export function trackNavigationTimings(lifeCycle: LifeCycle, callback: (newTimin
return { stop }
}

export function trackFirstContentfulPaint(lifeCycle: LifeCycle, callback: (fcp: RelativeTime) => void) {
export function trackFirstContentfulPaintTiming(lifeCycle: LifeCycle, callback: (fcpTiming: RelativeTime) => void) {
const firstHidden = trackFirstHidden()
const { unsubscribe: stop } = lifeCycle.subscribe(LifeCycleEventType.PERFORMANCE_ENTRY_COLLECTED, (entry) => {
if (
entry.entryType === 'paint' &&
entry.name === 'first-contentful-paint' &&
entry.startTime < firstHidden.timeStamp
entry.startTime < firstHidden.timeStamp &&
entry.startTime < TIMING_MAXIMUM_DELAY
) {
callback(entry.startTime)
}
Expand All @@ -90,10 +94,10 @@ export function trackFirstContentfulPaint(lifeCycle: LifeCycle, callback: (fcp:
* Documentation: https://web.dev/lcp/
* Reference implementation: https://github.com/GoogleChrome/web-vitals/blob/master/src/getLCP.ts
*/
export function trackLargestContentfulPaint(
export function trackLargestContentfulPaintTiming(
lifeCycle: LifeCycle,
emitter: EventEmitter,
callback: (value: RelativeTime) => void
callback: (lcpTiming: RelativeTime) => void
) {
const firstHidden = trackFirstHidden()

Expand All @@ -110,17 +114,15 @@ export function trackLargestContentfulPaint(
{ capture: true, once: true }
)

const lcpSizes: Array<{ timeStamp: TimeStamp; startTime: RelativeTime; size: number }> = []

const { unsubscribe: unsubscribeLifeCycle } = lifeCycle.subscribe(
LifeCycleEventType.PERFORMANCE_ENTRY_COLLECTED,
(entry) => {
if (
entry.entryType === 'largest-contentful-paint' &&
entry.startTime < firstInteractionTimestamp &&
entry.startTime < firstHidden.timeStamp
entry.startTime < firstHidden.timeStamp &&
entry.startTime < TIMING_MAXIMUM_DELAY
) {
lcpSizes.push({ timeStamp: timeStampNow(), startTime: entry.startTime, size: entry.size })
callback(entry.startTime)
}
}
Expand Down