From bd67b655ff11c7c749ef8e73a24d5cf7e6045af6 Mon Sep 17 00:00:00 2001 From: Geoffroy Lorieux Date: Wed, 15 Apr 2020 09:18:01 +0200 Subject: [PATCH 01/23] [RUMF-447]: Only collect first-contentful-paint if page is visible --- packages/rum/src/viewTracker.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/rum/src/viewTracker.ts b/packages/rum/src/viewTracker.ts index 4ab7a08ac3..6535cd2a28 100644 --- a/packages/rum/src/viewTracker.ts +++ b/packages/rum/src/viewTracker.ts @@ -128,7 +128,11 @@ function trackMeasures(lifeCycle: LifeCycle, scheduleViewUpdate: () => void) { loadEventEnd: msToNs(navigationEntry.loadEventEnd), } scheduleViewUpdate() - } else if (entry.entryType === 'paint' && entry.name === 'first-contentful-paint') { + } else if ( + entry.entryType === 'paint' && + entry.name === 'first-contentful-paint' && + document.visibilityState === 'visible' + ) { const paintEntry = entry as PerformancePaintTiming viewMeasures = { ...viewMeasures, From c453cbb585669854ef44655f6d469bae07113564 Mon Sep 17 00:00:00 2001 From: Geoffroy Lorieux Date: Wed, 15 Apr 2020 09:45:22 +0200 Subject: [PATCH 02/23] Adds test --- packages/core/src/specHelper.ts | 13 ++++++++++ packages/core/test/sessionManagement.spec.ts | 15 +---------- packages/rum/test/viewTracker.spec.ts | 27 ++++++++++++++++++++ 3 files changed, 41 insertions(+), 14 deletions(-) diff --git a/packages/core/src/specHelper.ts b/packages/core/src/specHelper.ts index 192c1c1116..ee97193523 100644 --- a/packages/core/src/specHelper.ts +++ b/packages/core/src/specHelper.ts @@ -125,3 +125,16 @@ export class PerformanceObserverStubBuilder { } as unknown) as PerformanceObserver } } + +export function setPageVisibility(visibility: 'visible' | 'hidden') { + Object.defineProperty(document, 'visibilityState', { + get() { + return visibility + }, + configurable: true, + }) +} + +export function restorePageVisibility() { + delete (document as any).visibilityState +} diff --git a/packages/core/test/sessionManagement.spec.ts b/packages/core/test/sessionManagement.spec.ts index e0fe3dc06f..530bb25d01 100644 --- a/packages/core/test/sessionManagement.spec.ts +++ b/packages/core/test/sessionManagement.spec.ts @@ -9,7 +9,7 @@ import { stopSessionManagement, VISIBILITY_CHECK_DELAY, } from '../src/sessionManagement' -import { isIE } from '../src/specHelper' +import { isIE, setPageVisibility, restorePageVisibility } from '../src/specHelper' describe('cacheCookieAccess', () => { const TEST_COOKIE = 'test' @@ -310,19 +310,6 @@ describe('startSessionManagement', () => { }) describe('session expiration', () => { - function setPageVisibility(visibility: 'visible' | 'hidden') { - Object.defineProperty(document, 'visibilityState', { - get() { - return visibility - }, - configurable: true, - }) - } - - function restorePageVisibility() { - delete (document as any).visibilityState - } - beforeEach(() => { setPageVisibility('hidden') }) diff --git a/packages/rum/test/viewTracker.spec.ts b/packages/rum/test/viewTracker.spec.ts index 0ac9991362..74f715aecf 100644 --- a/packages/rum/test/viewTracker.spec.ts +++ b/packages/rum/test/viewTracker.spec.ts @@ -4,6 +4,7 @@ import { LifeCycle, LifeCycleEventType } from '../src/lifeCycle' import { PerformanceLongTaskTiming, PerformancePaintTiming, RumViewEvent, UserAction, UserActionType } from '../src/rum' import { RumSession } from '../src/rumSession' import { trackView, viewContext } from '../src/viewTracker' +import { restorePageVisibility, setPageVisibility } from '../../core/src/specHelper' function setup({ addRumEvent, @@ -178,6 +179,7 @@ describe('rum view measures', () => { }) it('should track performance timings', () => { + setPageVisibility('visible') const lifeCycle = new LifeCycle() setup({ addRumEvent, lifeCycle }) @@ -214,5 +216,30 @@ describe('rum view measures', () => { resourceCount: 0, userActionCount: 0, }) + restorePageVisibility() + }) + it('should not collect firstContentfulPaint if page is not visible', () => { + setPageVisibility('hidden') + const lifeCycle = new LifeCycle() + setup({ addRumEvent, lifeCycle }) + lifeCycle.notify(LifeCycleEventType.PERFORMANCE_ENTRY_COLLECTED, FAKE_PAINT_ENTRY as PerformancePaintTiming) + lifeCycle.notify( + LifeCycleEventType.PERFORMANCE_ENTRY_COLLECTED, + FAKE_NAVIGATION_ENTRY as PerformanceNavigationTiming + ) + history.pushState({}, '', '/bar') + + expect(getEventCount()).toEqual(3) + expect(getViewEvent(1).view.measures).toEqual({ + domComplete: 456e6, + domContentLoaded: 345e6, + domInteractive: 234e6, + errorCount: 0, + loadEventEnd: 567e6, + longTaskCount: 0, + resourceCount: 0, + userActionCount: 0, + }) + restorePageVisibility() }) }) From 79fb88b4ab2ca912df75b6ea865e2322e95e5052 Mon Sep 17 00:00:00 2001 From: Geoffroy Lorieux Date: Wed, 15 Apr 2020 09:49:02 +0200 Subject: [PATCH 03/23] Fixes imports --- packages/core/test/sessionManagement.spec.ts | 2 +- packages/rum/test/viewTracker.spec.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/core/test/sessionManagement.spec.ts b/packages/core/test/sessionManagement.spec.ts index 530bb25d01..87d15e24dd 100644 --- a/packages/core/test/sessionManagement.spec.ts +++ b/packages/core/test/sessionManagement.spec.ts @@ -9,7 +9,7 @@ import { stopSessionManagement, VISIBILITY_CHECK_DELAY, } from '../src/sessionManagement' -import { isIE, setPageVisibility, restorePageVisibility } from '../src/specHelper' +import { isIE, restorePageVisibility, setPageVisibility } from '../src/specHelper' describe('cacheCookieAccess', () => { const TEST_COOKIE = 'test' diff --git a/packages/rum/test/viewTracker.spec.ts b/packages/rum/test/viewTracker.spec.ts index 74f715aecf..33fb20ee20 100644 --- a/packages/rum/test/viewTracker.spec.ts +++ b/packages/rum/test/viewTracker.spec.ts @@ -1,10 +1,10 @@ import { getHash, getPathName, getSearch } from '@datadog/browser-core' +import { restorePageVisibility, setPageVisibility } from '../../core/src/specHelper' import { LifeCycle, LifeCycleEventType } from '../src/lifeCycle' import { PerformanceLongTaskTiming, PerformancePaintTiming, RumViewEvent, UserAction, UserActionType } from '../src/rum' import { RumSession } from '../src/rumSession' import { trackView, viewContext } from '../src/viewTracker' -import { restorePageVisibility, setPageVisibility } from '../../core/src/specHelper' function setup({ addRumEvent, From 4d8338330dcc8219caf61970f06ef87970cd6941 Mon Sep 17 00:00:00 2001 From: Geoffroy Lorieux Date: Wed, 15 Apr 2020 16:51:54 +0200 Subject: [PATCH 04/23] Post review fixes --- packages/rum/test/viewTracker.spec.ts | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/packages/rum/test/viewTracker.spec.ts b/packages/rum/test/viewTracker.spec.ts index 33fb20ee20..109a789d53 100644 --- a/packages/rum/test/viewTracker.spec.ts +++ b/packages/rum/test/viewTracker.spec.ts @@ -218,11 +218,11 @@ describe('rum view measures', () => { }) restorePageVisibility() }) + it('should not collect firstContentfulPaint if page is not visible', () => { setPageVisibility('hidden') const lifeCycle = new LifeCycle() setup({ addRumEvent, lifeCycle }) - lifeCycle.notify(LifeCycleEventType.PERFORMANCE_ENTRY_COLLECTED, FAKE_PAINT_ENTRY as PerformancePaintTiming) lifeCycle.notify( LifeCycleEventType.PERFORMANCE_ENTRY_COLLECTED, FAKE_NAVIGATION_ENTRY as PerformanceNavigationTiming @@ -230,16 +230,7 @@ describe('rum view measures', () => { history.pushState({}, '', '/bar') expect(getEventCount()).toEqual(3) - expect(getViewEvent(1).view.measures).toEqual({ - domComplete: 456e6, - domContentLoaded: 345e6, - domInteractive: 234e6, - errorCount: 0, - loadEventEnd: 567e6, - longTaskCount: 0, - resourceCount: 0, - userActionCount: 0, - }) + expect(getViewEvent(1).view.measures.firstContentfulPaint).toBeUndefined() restorePageVisibility() }) }) From 34cabc4d4ee8261941d0d61777209be800a5e8b5 Mon Sep 17 00:00:00 2001 From: Geoffroy Lorieux Date: Thu, 16 Apr 2020 12:03:50 +0200 Subject: [PATCH 05/23] Move conditional collection of paint event one level up --- packages/rum/src/performanceCollection.ts | 7 ++++++- packages/rum/src/viewTracker.ts | 6 +----- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/packages/rum/src/performanceCollection.ts b/packages/rum/src/performanceCollection.ts index b300cc57d2..0aad1eeffb 100644 --- a/packages/rum/src/performanceCollection.ts +++ b/packages/rum/src/performanceCollection.ts @@ -31,7 +31,12 @@ export function startPerformanceCollection(lifeCycle: LifeCycle, session: RumSes const observer = new PerformanceObserver( monitor((entries) => handlePerformanceEntries(session, lifeCycle, entries.getEntries())) ) - observer.observe({ entryTypes: ['resource', 'navigation', 'paint', 'longtask'] }) + const entryTypes = ['resource', 'navigation', 'longtask'] + + if (document.visibilityState === 'visible') { + entryTypes.push('paint') + } + observer.observe({ entryTypes }) if (supportPerformanceObject() && 'addEventListener' in performance) { // https://bugzilla.mozilla.org/show_bug.cgi?id=1559377 diff --git a/packages/rum/src/viewTracker.ts b/packages/rum/src/viewTracker.ts index f27e0bb6c8..9aa4543f93 100644 --- a/packages/rum/src/viewTracker.ts +++ b/packages/rum/src/viewTracker.ts @@ -131,11 +131,7 @@ function trackMeasures(lifeCycle: LifeCycle, scheduleViewUpdate: () => void) { loadEventEnd: msToNs(navigationEntry.loadEventEnd), } scheduleViewUpdate() - } else if ( - entry.entryType === 'paint' && - entry.name === 'first-contentful-paint' && - document.visibilityState === 'visible' - ) { + } else if (entry.entryType === 'paint' && entry.name === 'first-contentful-paint') { const paintEntry = entry as PerformancePaintTiming viewMeasures = { ...viewMeasures, From f9e3d93a2b8d00d4c58e1edb9e19ac9cd25a6f37 Mon Sep 17 00:00:00 2001 From: Geoffroy Lorieux Date: Thu, 16 Apr 2020 15:21:06 +0200 Subject: [PATCH 06/23] Update packages/rum/src/performanceCollection.ts Co-Authored-By: Bastien Caudan --- packages/rum/src/performanceCollection.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/rum/src/performanceCollection.ts b/packages/rum/src/performanceCollection.ts index 0aad1eeffb..aab70ccb7e 100644 --- a/packages/rum/src/performanceCollection.ts +++ b/packages/rum/src/performanceCollection.ts @@ -33,6 +33,7 @@ export function startPerformanceCollection(lifeCycle: LifeCycle, session: RumSes ) const entryTypes = ['resource', 'navigation', 'longtask'] + // cf https://github.com/w3c/paint-timing/issues/40 if (document.visibilityState === 'visible') { entryTypes.push('paint') } From 5d4bf8a8f80a52382010459dc7196cfc063b1ede Mon Sep 17 00:00:00 2001 From: Geoffroy Lorieux Date: Tue, 21 Apr 2020 10:38:04 +0200 Subject: [PATCH 07/23] Rework test --- packages/rum/test/rum.spec.ts | 60 ++++++++++++++++++++++++++- packages/rum/test/viewTracker.spec.ts | 18 -------- 2 files changed, 59 insertions(+), 19 deletions(-) diff --git a/packages/rum/test/rum.spec.ts b/packages/rum/test/rum.spec.ts index 2cc9973143..de187bd6b3 100644 --- a/packages/rum/test/rum.spec.ts +++ b/packages/rum/test/rum.spec.ts @@ -11,9 +11,19 @@ import { } from '@datadog/browser-core' import sinon from 'sinon' +import { restorePageVisibility, setPageVisibility } from '../../core/src/specHelper' import { LifeCycle, LifeCycleEventType } from '../src/lifeCycle' import { startPerformanceCollection } from '../src/performanceCollection' -import { handleResourceEntry, RumEvent, RumResourceEvent, startRum, UserAction, UserActionType } from '../src/rum' +import { + handleResourceEntry, + RumEvent, + RumResourceEvent, + startRum, + UserAction, + UserActionType, + PerformancePaintTiming, + RumViewEvent, +} from '../src/rum' import { RumGlobal } from '../src/rum.entry' interface BrowserWindow extends Window { @@ -509,3 +519,51 @@ describe('rum user action', () => { expect((getRumMessage(server, 0) as any).fooBar).toEqual('foo') }) }) + +describe('RUM hidden page', () => { + let lifeCycle: LifeCycle + let RUM: RumApi + let server: sinon.SinonFakeServer + const session = { + getId: () => undefined, + isTracked: () => true, + isTrackedWithResource: () => true, + } + + beforeEach(() => { + setPageVisibility('hidden') + server = sinon.fakeServer.create() + lifeCycle = new LifeCycle() + server.requests = [] + RUM = startRum('appId', lifeCycle, configuration as Configuration, session, internalMonitoring) as RumApi + startPerformanceCollection(lifeCycle, session) + history.pushState({}, '', '/bar') + }) + + afterEach(() => { + restorePageVisibility() + server.restore() + }) + + it('should not collect first_contentful_paint if page is not visible', () => { + interface ExpectedRequestBody { + evt: { + category: string + } + view: { + measures: { + first_contentful_paint: number + } + } + } + + const initialRequests = getServerRequestBodies(server) + expect(initialRequests.length).toBeGreaterThan(0) + + initialRequests.map((request) => { + if (request.evt.category === 'view') { + expect(request.view.measures.first_contentful_paint).toBeUndefined() + } + }) + }) +}) diff --git a/packages/rum/test/viewTracker.spec.ts b/packages/rum/test/viewTracker.spec.ts index 109a789d53..0ac9991362 100644 --- a/packages/rum/test/viewTracker.spec.ts +++ b/packages/rum/test/viewTracker.spec.ts @@ -1,6 +1,5 @@ import { getHash, getPathName, getSearch } from '@datadog/browser-core' -import { restorePageVisibility, setPageVisibility } from '../../core/src/specHelper' import { LifeCycle, LifeCycleEventType } from '../src/lifeCycle' import { PerformanceLongTaskTiming, PerformancePaintTiming, RumViewEvent, UserAction, UserActionType } from '../src/rum' import { RumSession } from '../src/rumSession' @@ -179,7 +178,6 @@ describe('rum view measures', () => { }) it('should track performance timings', () => { - setPageVisibility('visible') const lifeCycle = new LifeCycle() setup({ addRumEvent, lifeCycle }) @@ -216,21 +214,5 @@ describe('rum view measures', () => { resourceCount: 0, userActionCount: 0, }) - restorePageVisibility() - }) - - it('should not collect firstContentfulPaint if page is not visible', () => { - setPageVisibility('hidden') - const lifeCycle = new LifeCycle() - setup({ addRumEvent, lifeCycle }) - lifeCycle.notify( - LifeCycleEventType.PERFORMANCE_ENTRY_COLLECTED, - FAKE_NAVIGATION_ENTRY as PerformanceNavigationTiming - ) - history.pushState({}, '', '/bar') - - expect(getEventCount()).toEqual(3) - expect(getViewEvent(1).view.measures.firstContentfulPaint).toBeUndefined() - restorePageVisibility() }) }) From 8f3c7a99ff77efbc919dc8dd0278a9b1e97f1818 Mon Sep 17 00:00:00 2001 From: Geoffroy Lorieux Date: Mon, 27 Apr 2020 13:48:43 +0200 Subject: [PATCH 08/23] Adds visiblilty test --- packages/rum/test/rum.spec.ts | 73 +++++++++++++++++++++++++++-------- 1 file changed, 57 insertions(+), 16 deletions(-) diff --git a/packages/rum/test/rum.spec.ts b/packages/rum/test/rum.spec.ts index a97ea9add8..db31f06d96 100644 --- a/packages/rum/test/rum.spec.ts +++ b/packages/rum/test/rum.spec.ts @@ -526,46 +526,87 @@ describe('RUM hidden page', () => { let lifeCycle: LifeCycle let RUM: RumApi let server: sinon.SinonFakeServer + const browserWindow = window as BrowserWindow + let stubBuilder: PerformanceObserverStubBuilder + let original: PerformanceObserver | undefined const session = { - getId: () => undefined, + getId: () => '42', isTracked: () => true, isTrackedWithResource: () => true, } + interface ExpectedRequestBody { + evt: { + category: string + } + view: { + measures: { + first_contentful_paint: number + } + } + } + + const FAKE_RESOURCE: Partial = { name: 'http://foo.com', entryType: 'resource' } + const performanceEntry = { + duration: 1234567, + entryType: 'view', + name: 'first-contentful-paint', + startTime: 0, + } + beforeEach(() => { - setPageVisibility('hidden') server = sinon.fakeServer.create() + original = browserWindow.PerformanceObserver + stubBuilder = new PerformanceObserverStubBuilder() + browserWindow.PerformanceObserver = stubBuilder.getStub() lifeCycle = new LifeCycle() - server.requests = [] - RUM = startRum('appId', lifeCycle, configuration as Configuration, session, internalMonitoring) as RumApi - startPerformanceCollection(lifeCycle, session) - history.pushState({}, '', '/bar') }) afterEach(() => { + browserWindow.PerformanceObserver = original restorePageVisibility() server.restore() }) it('should not collect first_contentful_paint if page is not visible', () => { - interface ExpectedRequestBody { - evt: { - category: string - } - view: { - measures: { - first_contentful_paint: number - } - } - } + setPageVisibility('hidden') + RUM = startRum('appId', lifeCycle, configuration as Configuration, session, internalMonitoring) as RumApi + startPerformanceCollection(lifeCycle, session) + startViewCollection(location, lifeCycle, session) + server.requests = [] + + stubBuilder.fakeEntry(FAKE_RESOURCE as PerformanceEntry, 'resource') + lifeCycle.notify(LifeCycleEventType.PERFORMANCE_ENTRY_COLLECTED, performanceEntry as PerformanceEntry) const initialRequests = getServerRequestBodies(server) expect(initialRequests.length).toBeGreaterThan(0) initialRequests.map((request) => { if (request.evt.category === 'view') { + console.log(request) expect(request.view.measures.first_contentful_paint).toBeUndefined() } }) }) + + it('should collect first_contentful_paint if page is visible', () => { + setPageVisibility('visible') + RUM = startRum('appId', lifeCycle, configuration as Configuration, session, internalMonitoring) as RumApi + startPerformanceCollection(lifeCycle, session) + startViewCollection(location, lifeCycle, session) + server.requests = [] + + stubBuilder.fakeEntry(FAKE_RESOURCE as PerformanceEntry, 'resource') + lifeCycle.notify(LifeCycleEventType.PERFORMANCE_ENTRY_COLLECTED, performanceEntry as PerformanceEntry) + + const initialRequests = getServerRequestBodies(server) + expect(initialRequests.length).toBeGreaterThan(0) + + initialRequests.map((request) => { + if (request.evt.category === 'view') { + console.log(request) + expect(request.view.measures.first_contentful_paint).toBeDefined() + } + }) + }) }) From d014276fd3402c26a86ad0d96a7bd94112b0c07a Mon Sep 17 00:00:00 2001 From: Geoffroy Lorieux Date: Mon, 27 Apr 2020 13:57:03 +0200 Subject: [PATCH 09/23] Removes console and session id --- packages/rum/test/rum.spec.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/rum/test/rum.spec.ts b/packages/rum/test/rum.spec.ts index db31f06d96..5c6de55966 100644 --- a/packages/rum/test/rum.spec.ts +++ b/packages/rum/test/rum.spec.ts @@ -530,7 +530,7 @@ describe('RUM hidden page', () => { let stubBuilder: PerformanceObserverStubBuilder let original: PerformanceObserver | undefined const session = { - getId: () => '42', + getId: () => undefined, isTracked: () => true, isTrackedWithResource: () => true, } @@ -583,7 +583,6 @@ describe('RUM hidden page', () => { initialRequests.map((request) => { if (request.evt.category === 'view') { - console.log(request) expect(request.view.measures.first_contentful_paint).toBeUndefined() } }) From 18e3f64803664583724e5fb2921e0d60f8a3ed24 Mon Sep 17 00:00:00 2001 From: Geoffroy Lorieux Date: Mon, 27 Apr 2020 14:03:59 +0200 Subject: [PATCH 10/23] Fixes duration issue --- packages/rum/test/rum.spec.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/rum/test/rum.spec.ts b/packages/rum/test/rum.spec.ts index 5c6de55966..57f6223493 100644 --- a/packages/rum/test/rum.spec.ts +++ b/packages/rum/test/rum.spec.ts @@ -548,10 +548,9 @@ describe('RUM hidden page', () => { const FAKE_RESOURCE: Partial = { name: 'http://foo.com', entryType: 'resource' } const performanceEntry = { - duration: 1234567, entryType: 'view', name: 'first-contentful-paint', - startTime: 0, + startTime: 123, } beforeEach(() => { From 14b6fbdb5bbc88d1f2de22b6894155fb78594fb5 Mon Sep 17 00:00:00 2001 From: Geoffroy Lorieux Date: Mon, 27 Apr 2020 14:50:48 +0200 Subject: [PATCH 11/23] Move session definitions --- packages/rum/test/rum.spec.ts | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/packages/rum/test/rum.spec.ts b/packages/rum/test/rum.spec.ts index 57f6223493..0246da492a 100644 --- a/packages/rum/test/rum.spec.ts +++ b/packages/rum/test/rum.spec.ts @@ -529,11 +529,6 @@ describe('RUM hidden page', () => { const browserWindow = window as BrowserWindow let stubBuilder: PerformanceObserverStubBuilder let original: PerformanceObserver | undefined - const session = { - getId: () => undefined, - isTracked: () => true, - isTrackedWithResource: () => true, - } interface ExpectedRequestBody { evt: { @@ -568,6 +563,12 @@ describe('RUM hidden page', () => { }) it('should not collect first_contentful_paint if page is not visible', () => { + const session = { + getId: () => undefined, + isTracked: () => true, + isTrackedWithResource: () => true, + } + setPageVisibility('hidden') RUM = startRum('appId', lifeCycle, configuration as Configuration, session, internalMonitoring) as RumApi startPerformanceCollection(lifeCycle, session) @@ -588,6 +589,12 @@ describe('RUM hidden page', () => { }) it('should collect first_contentful_paint if page is visible', () => { + const session = { + getId: () => undefined, + isTracked: () => true, + isTrackedWithResource: () => true, + } + setPageVisibility('visible') RUM = startRum('appId', lifeCycle, configuration as Configuration, session, internalMonitoring) as RumApi startPerformanceCollection(lifeCycle, session) From b9d4ef8be76223324d0293ae3ca9602c1f030702 Mon Sep 17 00:00:00 2001 From: Geoffroy Lorieux Date: Mon, 27 Apr 2020 15:01:17 +0200 Subject: [PATCH 12/23] Fixes performanceEntry issues --- packages/rum/test/rum.spec.ts | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/packages/rum/test/rum.spec.ts b/packages/rum/test/rum.spec.ts index 0246da492a..6c5604d48c 100644 --- a/packages/rum/test/rum.spec.ts +++ b/packages/rum/test/rum.spec.ts @@ -541,13 +541,6 @@ describe('RUM hidden page', () => { } } - const FAKE_RESOURCE: Partial = { name: 'http://foo.com', entryType: 'resource' } - const performanceEntry = { - entryType: 'view', - name: 'first-contentful-paint', - startTime: 123, - } - beforeEach(() => { server = sinon.fakeServer.create() original = browserWindow.PerformanceObserver @@ -568,6 +561,12 @@ describe('RUM hidden page', () => { isTracked: () => true, isTrackedWithResource: () => true, } + const FAKE_RESOURCE: Partial = { name: 'http://foo.com', entryType: 'resource' } + const performanceEntry = { + entryType: 'view', + name: 'first-contentful-paint', + startTime: 123, + } setPageVisibility('hidden') RUM = startRum('appId', lifeCycle, configuration as Configuration, session, internalMonitoring) as RumApi @@ -594,6 +593,12 @@ describe('RUM hidden page', () => { isTracked: () => true, isTrackedWithResource: () => true, } + const FAKE_RESOURCE: Partial = { name: 'http://foo.com', entryType: 'resource' } + const performanceEntry = { + entryType: 'view', + name: 'first-contentful-paint', + startTime: 123, + } setPageVisibility('visible') RUM = startRum('appId', lifeCycle, configuration as Configuration, session, internalMonitoring) as RumApi From 335de6af7c8abb291e2ab78a6685fe7c367ba647 Mon Sep 17 00:00:00 2001 From: Geoffroy Lorieux Date: Mon, 27 Apr 2020 15:27:03 +0200 Subject: [PATCH 13/23] Fixes --- packages/rum/test/rum.spec.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/rum/test/rum.spec.ts b/packages/rum/test/rum.spec.ts index 6c5604d48c..8939e8b41f 100644 --- a/packages/rum/test/rum.spec.ts +++ b/packages/rum/test/rum.spec.ts @@ -570,8 +570,8 @@ describe('RUM hidden page', () => { setPageVisibility('hidden') RUM = startRum('appId', lifeCycle, configuration as Configuration, session, internalMonitoring) as RumApi - startPerformanceCollection(lifeCycle, session) startViewCollection(location, lifeCycle, session) + startPerformanceCollection(lifeCycle, session) server.requests = [] stubBuilder.fakeEntry(FAKE_RESOURCE as PerformanceEntry, 'resource') @@ -602,8 +602,8 @@ describe('RUM hidden page', () => { setPageVisibility('visible') RUM = startRum('appId', lifeCycle, configuration as Configuration, session, internalMonitoring) as RumApi - startPerformanceCollection(lifeCycle, session) startViewCollection(location, lifeCycle, session) + startPerformanceCollection(lifeCycle, session) server.requests = [] stubBuilder.fakeEntry(FAKE_RESOURCE as PerformanceEntry, 'resource') @@ -614,7 +614,6 @@ describe('RUM hidden page', () => { initialRequests.map((request) => { if (request.evt.category === 'view') { - console.log(request) expect(request.view.measures.first_contentful_paint).toBeDefined() } }) From d357c9a001170e9cbd08b662f2c9787c8dfe2ca0 Mon Sep 17 00:00:00 2001 From: Geoffroy Lorieux Date: Mon, 27 Apr 2020 16:40:28 +0200 Subject: [PATCH 14/23] Removes startTime --- packages/rum/test/rum.spec.ts | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/packages/rum/test/rum.spec.ts b/packages/rum/test/rum.spec.ts index 8939e8b41f..a4d1f7f705 100644 --- a/packages/rum/test/rum.spec.ts +++ b/packages/rum/test/rum.spec.ts @@ -562,11 +562,7 @@ describe('RUM hidden page', () => { isTrackedWithResource: () => true, } const FAKE_RESOURCE: Partial = { name: 'http://foo.com', entryType: 'resource' } - const performanceEntry = { - entryType: 'view', - name: 'first-contentful-paint', - startTime: 123, - } + const performanceEntry = { entryType: 'view', name: 'first-contentful-paint' } setPageVisibility('hidden') RUM = startRum('appId', lifeCycle, configuration as Configuration, session, internalMonitoring) as RumApi @@ -594,11 +590,7 @@ describe('RUM hidden page', () => { isTrackedWithResource: () => true, } const FAKE_RESOURCE: Partial = { name: 'http://foo.com', entryType: 'resource' } - const performanceEntry = { - entryType: 'view', - name: 'first-contentful-paint', - startTime: 123, - } + const performanceEntry = { entryType: 'view', name: 'first-contentful-paint' } setPageVisibility('visible') RUM = startRum('appId', lifeCycle, configuration as Configuration, session, internalMonitoring) as RumApi From b4aee4d66a77de03678173bd688da412891d4d6c Mon Sep 17 00:00:00 2001 From: Geoffroy Lorieux Date: Mon, 27 Apr 2020 16:47:56 +0200 Subject: [PATCH 15/23] Ignore IE test --- packages/rum/test/rum.spec.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/rum/test/rum.spec.ts b/packages/rum/test/rum.spec.ts index a4d1f7f705..26b48f6ad3 100644 --- a/packages/rum/test/rum.spec.ts +++ b/packages/rum/test/rum.spec.ts @@ -542,6 +542,9 @@ describe('RUM hidden page', () => { } beforeEach(() => { + if (isIE()) { + pending('no full rum support') + } server = sinon.fakeServer.create() original = browserWindow.PerformanceObserver stubBuilder = new PerformanceObserverStubBuilder() From 96074b2ba71d8909881b02790833a946d9d3b604 Mon Sep 17 00:00:00 2001 From: Geoffroy Lorieux Date: Tue, 28 Apr 2020 08:16:29 +0200 Subject: [PATCH 16/23] Update packages/rum/test/rum.spec.ts Co-Authored-By: Bastien Caudan --- packages/rum/test/rum.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/rum/test/rum.spec.ts b/packages/rum/test/rum.spec.ts index 26b48f6ad3..c6b0ec98e7 100644 --- a/packages/rum/test/rum.spec.ts +++ b/packages/rum/test/rum.spec.ts @@ -522,7 +522,7 @@ describe('rum user action', () => { }) }) -describe('RUM hidden page', () => { +describe('rum first_contentful_paint', () => { let lifeCycle: LifeCycle let RUM: RumApi let server: sinon.SinonFakeServer From cd995a4bd0f40ff86d9125b18cd2047c08156021 Mon Sep 17 00:00:00 2001 From: Geoffroy Lorieux Date: Tue, 28 Apr 2020 08:16:41 +0200 Subject: [PATCH 17/23] Update packages/rum/test/rum.spec.ts Co-Authored-By: Bastien Caudan --- packages/rum/test/rum.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/rum/test/rum.spec.ts b/packages/rum/test/rum.spec.ts index c6b0ec98e7..041fd974c9 100644 --- a/packages/rum/test/rum.spec.ts +++ b/packages/rum/test/rum.spec.ts @@ -558,7 +558,7 @@ describe('rum first_contentful_paint', () => { server.restore() }) - it('should not collect first_contentful_paint if page is not visible', () => { + it('should not be collected when page starts not visible', () => { const session = { getId: () => undefined, isTracked: () => true, From c72c2461edbd7693e516f6c1d68936c2e4b347d5 Mon Sep 17 00:00:00 2001 From: Geoffroy Lorieux Date: Tue, 28 Apr 2020 08:16:49 +0200 Subject: [PATCH 18/23] Update packages/rum/test/rum.spec.ts Co-Authored-By: Bastien Caudan --- packages/rum/test/rum.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/rum/test/rum.spec.ts b/packages/rum/test/rum.spec.ts index 041fd974c9..f2853c2e80 100644 --- a/packages/rum/test/rum.spec.ts +++ b/packages/rum/test/rum.spec.ts @@ -586,7 +586,7 @@ describe('rum first_contentful_paint', () => { }) }) - it('should collect first_contentful_paint if page is visible', () => { + it('should be collected when page starts visible', () => { const session = { getId: () => undefined, isTracked: () => true, From ec6c3580a0a338890584edc6e60658b6f7b795fa Mon Sep 17 00:00:00 2001 From: Geoffroy Lorieux Date: Tue, 28 Apr 2020 11:37:49 +0200 Subject: [PATCH 19/23] Test simplification --- packages/core/src/specHelper.ts | 10 +++++- packages/rum/test/rum.spec.ts | 62 ++++----------------------------- 2 files changed, 16 insertions(+), 56 deletions(-) diff --git a/packages/core/src/specHelper.ts b/packages/core/src/specHelper.ts index ee97193523..5b6aa37599 100644 --- a/packages/core/src/specHelper.ts +++ b/packages/core/src/specHelper.ts @@ -96,6 +96,11 @@ export interface FetchStubPromise extends Promise { export class PerformanceObserverStubBuilder { public instance: any + getEntryTypes() { + // tslint:disable-next-line: no-unsafe-any + return this.instance.entryTypes + } + fakeEntry(entry: PerformanceEntry, entryType: string) { const asEntryList = () => [entry] // tslint:disable-next-line: no-unsafe-any @@ -120,7 +125,10 @@ export class PerformanceObserverStubBuilder { builder.instance = this } observe(options?: PerformanceObserverInit) { - // do nothing + if (options) { + // tslint:disable-next-line: no-unsafe-any + builder.instance.entryTypes = options.entryTypes + } } } as unknown) as PerformanceObserver } diff --git a/packages/rum/test/rum.spec.ts b/packages/rum/test/rum.spec.ts index f2853c2e80..650f0531fd 100644 --- a/packages/rum/test/rum.spec.ts +++ b/packages/rum/test/rum.spec.ts @@ -525,27 +525,20 @@ describe('rum user action', () => { describe('rum first_contentful_paint', () => { let lifeCycle: LifeCycle let RUM: RumApi - let server: sinon.SinonFakeServer - const browserWindow = window as BrowserWindow let stubBuilder: PerformanceObserverStubBuilder let original: PerformanceObserver | undefined + const browserWindow = window as BrowserWindow - interface ExpectedRequestBody { - evt: { - category: string - } - view: { - measures: { - first_contentful_paint: number - } - } + const session = { + getId: () => '42', + isTracked: () => true, + isTrackedWithResource: () => true, } beforeEach(() => { if (isIE()) { pending('no full rum support') } - server = sinon.fakeServer.create() original = browserWindow.PerformanceObserver stubBuilder = new PerformanceObserverStubBuilder() browserWindow.PerformanceObserver = stubBuilder.getStub() @@ -555,62 +548,21 @@ describe('rum first_contentful_paint', () => { afterEach(() => { browserWindow.PerformanceObserver = original restorePageVisibility() - server.restore() }) it('should not be collected when page starts not visible', () => { - const session = { - getId: () => undefined, - isTracked: () => true, - isTrackedWithResource: () => true, - } - const FAKE_RESOURCE: Partial = { name: 'http://foo.com', entryType: 'resource' } - const performanceEntry = { entryType: 'view', name: 'first-contentful-paint' } - setPageVisibility('hidden') RUM = startRum('appId', lifeCycle, configuration as Configuration, session, internalMonitoring) as RumApi - startViewCollection(location, lifeCycle, session) startPerformanceCollection(lifeCycle, session) - server.requests = [] - - stubBuilder.fakeEntry(FAKE_RESOURCE as PerformanceEntry, 'resource') - lifeCycle.notify(LifeCycleEventType.PERFORMANCE_ENTRY_COLLECTED, performanceEntry as PerformanceEntry) - const initialRequests = getServerRequestBodies(server) - expect(initialRequests.length).toBeGreaterThan(0) - - initialRequests.map((request) => { - if (request.evt.category === 'view') { - expect(request.view.measures.first_contentful_paint).toBeUndefined() - } - }) + expect(stubBuilder.getEntryTypes()).not.toContain('paint') }) it('should be collected when page starts visible', () => { - const session = { - getId: () => undefined, - isTracked: () => true, - isTrackedWithResource: () => true, - } - const FAKE_RESOURCE: Partial = { name: 'http://foo.com', entryType: 'resource' } - const performanceEntry = { entryType: 'view', name: 'first-contentful-paint' } - setPageVisibility('visible') RUM = startRum('appId', lifeCycle, configuration as Configuration, session, internalMonitoring) as RumApi - startViewCollection(location, lifeCycle, session) startPerformanceCollection(lifeCycle, session) - server.requests = [] - - stubBuilder.fakeEntry(FAKE_RESOURCE as PerformanceEntry, 'resource') - lifeCycle.notify(LifeCycleEventType.PERFORMANCE_ENTRY_COLLECTED, performanceEntry as PerformanceEntry) - const initialRequests = getServerRequestBodies(server) - expect(initialRequests.length).toBeGreaterThan(0) - - initialRequests.map((request) => { - if (request.evt.category === 'view') { - expect(request.view.measures.first_contentful_paint).toBeDefined() - } - }) + expect(stubBuilder.getEntryTypes()).toContain('paint') }) }) From d76a834bf44af4b448114fd60d5f492ed11b061b Mon Sep 17 00:00:00 2001 From: Geoffroy Lorieux Date: Tue, 28 Apr 2020 11:58:58 +0200 Subject: [PATCH 20/23] Fixes sessions issues --- packages/rum/test/rum.spec.ts | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/packages/rum/test/rum.spec.ts b/packages/rum/test/rum.spec.ts index 650f0531fd..2798d9acf8 100644 --- a/packages/rum/test/rum.spec.ts +++ b/packages/rum/test/rum.spec.ts @@ -523,18 +523,11 @@ describe('rum user action', () => { }) describe('rum first_contentful_paint', () => { - let lifeCycle: LifeCycle let RUM: RumApi let stubBuilder: PerformanceObserverStubBuilder let original: PerformanceObserver | undefined const browserWindow = window as BrowserWindow - const session = { - getId: () => '42', - isTracked: () => true, - isTrackedWithResource: () => true, - } - beforeEach(() => { if (isIE()) { pending('no full rum support') @@ -542,7 +535,6 @@ describe('rum first_contentful_paint', () => { original = browserWindow.PerformanceObserver stubBuilder = new PerformanceObserverStubBuilder() browserWindow.PerformanceObserver = stubBuilder.getStub() - lifeCycle = new LifeCycle() }) afterEach(() => { @@ -551,6 +543,13 @@ describe('rum first_contentful_paint', () => { }) it('should not be collected when page starts not visible', () => { + const session = { + getId: () => undefined, + isTracked: () => true, + isTrackedWithResource: () => true, + } + const lifeCycle = new LifeCycle() + setPageVisibility('hidden') RUM = startRum('appId', lifeCycle, configuration as Configuration, session, internalMonitoring) as RumApi startPerformanceCollection(lifeCycle, session) @@ -559,6 +558,13 @@ describe('rum first_contentful_paint', () => { }) it('should be collected when page starts visible', () => { + const session = { + getId: () => undefined, + isTracked: () => true, + isTrackedWithResource: () => true, + } + const lifeCycle = new LifeCycle() + setPageVisibility('visible') RUM = startRum('appId', lifeCycle, configuration as Configuration, session, internalMonitoring) as RumApi startPerformanceCollection(lifeCycle, session) From 640f83f89774c261cc44b4c10bb16141006b1e60 Mon Sep 17 00:00:00 2001 From: Geoffroy Lorieux Date: Tue, 28 Apr 2020 12:06:23 +0200 Subject: [PATCH 21/23] Fixes sessions id --- packages/rum/test/rum.spec.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/rum/test/rum.spec.ts b/packages/rum/test/rum.spec.ts index 2798d9acf8..005f2773a1 100644 --- a/packages/rum/test/rum.spec.ts +++ b/packages/rum/test/rum.spec.ts @@ -544,7 +544,7 @@ describe('rum first_contentful_paint', () => { it('should not be collected when page starts not visible', () => { const session = { - getId: () => undefined, + getId: () => '42', isTracked: () => true, isTrackedWithResource: () => true, } @@ -559,7 +559,7 @@ describe('rum first_contentful_paint', () => { it('should be collected when page starts visible', () => { const session = { - getId: () => undefined, + getId: () => '42', isTracked: () => true, isTrackedWithResource: () => true, } From 7ede9ddb11b318b1f570a8b760c18f9c2e408a99 Mon Sep 17 00:00:00 2001 From: Geoffroy Lorieux Date: Tue, 28 Apr 2020 12:11:43 +0200 Subject: [PATCH 22/23] Fixes startViewCollection --- packages/rum/test/rum.spec.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/rum/test/rum.spec.ts b/packages/rum/test/rum.spec.ts index 005f2773a1..ae8d1718e3 100644 --- a/packages/rum/test/rum.spec.ts +++ b/packages/rum/test/rum.spec.ts @@ -552,6 +552,7 @@ describe('rum first_contentful_paint', () => { setPageVisibility('hidden') RUM = startRum('appId', lifeCycle, configuration as Configuration, session, internalMonitoring) as RumApi + startViewCollection(location, lifeCycle, session) startPerformanceCollection(lifeCycle, session) expect(stubBuilder.getEntryTypes()).not.toContain('paint') @@ -567,6 +568,7 @@ describe('rum first_contentful_paint', () => { setPageVisibility('visible') RUM = startRum('appId', lifeCycle, configuration as Configuration, session, internalMonitoring) as RumApi + startViewCollection(location, lifeCycle, session) startPerformanceCollection(lifeCycle, session) expect(stubBuilder.getEntryTypes()).toContain('paint') From aa9dadba2d5e29001c7b3171e7c24b838d5b6f47 Mon Sep 17 00:00:00 2001 From: Geoffroy Lorieux Date: Tue, 28 Apr 2020 13:09:29 +0200 Subject: [PATCH 23/23] Moves test into performanceCollection.spec.ts and remove startRum section --- .../rum/test/performanceCollection.spec.ts | 52 ++++++++++++++++++ packages/rum/test/rum.spec.ts | 54 ------------------- 2 files changed, 52 insertions(+), 54 deletions(-) create mode 100644 packages/rum/test/performanceCollection.spec.ts diff --git a/packages/rum/test/performanceCollection.spec.ts b/packages/rum/test/performanceCollection.spec.ts new file mode 100644 index 0000000000..815e4ad137 --- /dev/null +++ b/packages/rum/test/performanceCollection.spec.ts @@ -0,0 +1,52 @@ +import { isIE, PerformanceObserverStubBuilder } from '@datadog/browser-core' + +import { restorePageVisibility, setPageVisibility } from '../../core/src/specHelper' +import { LifeCycle } from '../src/lifeCycle' +import { startPerformanceCollection } from '../src/performanceCollection' + +interface BrowserWindow extends Window { + PerformanceObserver?: PerformanceObserver +} + +describe('rum first_contentful_paint', () => { + let stubBuilder: PerformanceObserverStubBuilder + let original: PerformanceObserver | undefined + let lifeCycle: LifeCycle + + const browserWindow = window as BrowserWindow + const session = { + getId: () => undefined, + isTracked: () => true, + isTrackedWithResource: () => true, + } + + beforeEach(() => { + if (isIE()) { + pending('no full rum support') + } + + lifeCycle = new LifeCycle() + original = browserWindow.PerformanceObserver + stubBuilder = new PerformanceObserverStubBuilder() + browserWindow.PerformanceObserver = stubBuilder.getStub() + }) + + afterEach(() => { + browserWindow.PerformanceObserver = original + restorePageVisibility() + }) + + it('should not be collected when page starts not visible', () => { + setPageVisibility('hidden') + startPerformanceCollection(lifeCycle, session) + + expect(stubBuilder.getEntryTypes()).not.toContain('paint') + }) + + it('should be collected when page starts visible', () => { + setPageVisibility('visible') + startPerformanceCollection(lifeCycle, session) + + expect(stubBuilder.getEntryTypes()).toContain('paint') + }) +}) diff --git a/packages/rum/test/rum.spec.ts b/packages/rum/test/rum.spec.ts index ae8d1718e3..e2304ded19 100644 --- a/packages/rum/test/rum.spec.ts +++ b/packages/rum/test/rum.spec.ts @@ -11,7 +11,6 @@ import { } from '@datadog/browser-core' import sinon from 'sinon' -import { restorePageVisibility, setPageVisibility } from '../../core/src/specHelper' import { LifeCycle, LifeCycleEventType } from '../src/lifeCycle' import { startPerformanceCollection } from '../src/performanceCollection' import { handleResourceEntry, RumEvent, RumResourceEvent, startRum } from '../src/rum' @@ -521,56 +520,3 @@ describe('rum user action', () => { expect((getRumMessage(server, 0) as any).fooBar).toEqual('foo') }) }) - -describe('rum first_contentful_paint', () => { - let RUM: RumApi - let stubBuilder: PerformanceObserverStubBuilder - let original: PerformanceObserver | undefined - const browserWindow = window as BrowserWindow - - beforeEach(() => { - if (isIE()) { - pending('no full rum support') - } - original = browserWindow.PerformanceObserver - stubBuilder = new PerformanceObserverStubBuilder() - browserWindow.PerformanceObserver = stubBuilder.getStub() - }) - - afterEach(() => { - browserWindow.PerformanceObserver = original - restorePageVisibility() - }) - - it('should not be collected when page starts not visible', () => { - const session = { - getId: () => '42', - isTracked: () => true, - isTrackedWithResource: () => true, - } - const lifeCycle = new LifeCycle() - - setPageVisibility('hidden') - RUM = startRum('appId', lifeCycle, configuration as Configuration, session, internalMonitoring) as RumApi - startViewCollection(location, lifeCycle, session) - startPerformanceCollection(lifeCycle, session) - - expect(stubBuilder.getEntryTypes()).not.toContain('paint') - }) - - it('should be collected when page starts visible', () => { - const session = { - getId: () => '42', - isTracked: () => true, - isTrackedWithResource: () => true, - } - const lifeCycle = new LifeCycle() - - setPageVisibility('visible') - RUM = startRum('appId', lifeCycle, configuration as Configuration, session, internalMonitoring) as RumApi - startViewCollection(location, lifeCycle, session) - startPerformanceCollection(lifeCycle, session) - - expect(stubBuilder.getEntryTypes()).toContain('paint') - }) -})