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-447]: Only collect first-contentful-paint if page is visible #361

Merged
merged 30 commits into from
Apr 28, 2020
Merged
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
bd67b65
[RUMF-447]: Only collect first-contentful-paint if page is visible
glorieux Apr 15, 2020
61c7ba4
Merge branch 'master' into glorieux/fcp-visible
glorieux Apr 15, 2020
c453cbb
Adds test
glorieux Apr 15, 2020
79fb88b
Fixes imports
glorieux Apr 15, 2020
4d83383
Post review fixes
glorieux Apr 15, 2020
34cabc4
Move conditional collection of paint event one level up
glorieux Apr 16, 2020
7ec113f
Merge branch 'master' into glorieux/fcp-visible
glorieux Apr 16, 2020
f9e3d93
Update packages/rum/src/performanceCollection.ts
glorieux Apr 16, 2020
bb36a48
Merge branch 'master' into glorieux/fcp-visible
glorieux Apr 16, 2020
5d4bf8a
Rework test
glorieux Apr 21, 2020
fe7e42e
Merge branch 'master' into glorieux/fcp-visible
glorieux Apr 21, 2020
536e210
Merge branch 'master' into glorieux/fcp-visible
glorieux Apr 22, 2020
dc9cfc1
Merge branch 'master' into glorieux/fcp-visible
glorieux Apr 27, 2020
8f3c7a9
Adds visiblilty test
glorieux Apr 27, 2020
c14af62
Merge branch 'master' into glorieux/fcp-visible
glorieux Apr 27, 2020
d014276
Removes console and session id
glorieux Apr 27, 2020
18e3f64
Fixes duration issue
glorieux Apr 27, 2020
14b6fbd
Move session definitions
glorieux Apr 27, 2020
b9d4ef8
Fixes performanceEntry issues
glorieux Apr 27, 2020
335de6a
Fixes
glorieux Apr 27, 2020
d357c9a
Removes startTime
glorieux Apr 27, 2020
b4aee4d
Ignore IE test
glorieux Apr 27, 2020
96074b2
Update packages/rum/test/rum.spec.ts
glorieux Apr 28, 2020
cd995a4
Update packages/rum/test/rum.spec.ts
glorieux Apr 28, 2020
c72c246
Update packages/rum/test/rum.spec.ts
glorieux Apr 28, 2020
ec6c358
Test simplification
glorieux Apr 28, 2020
d76a834
Fixes sessions issues
glorieux Apr 28, 2020
640f83f
Fixes sessions id
glorieux Apr 28, 2020
7ede9dd
Fixes startViewCollection
glorieux Apr 28, 2020
aa9dadb
Moves test into performanceCollection.spec.ts and remove startRum
glorieux Apr 28, 2020
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
13 changes: 13 additions & 0 deletions packages/core/src/specHelper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
15 changes: 1 addition & 14 deletions packages/core/test/sessionManagement.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import {
stopSessionManagement,
VISIBILITY_CHECK_DELAY,
} from '../src/sessionManagement'
import { isIE } from '../src/specHelper'
import { isIE, restorePageVisibility, setPageVisibility } from '../src/specHelper'

describe('cacheCookieAccess', () => {
const TEST_COOKIE = 'test'
Expand Down Expand Up @@ -298,19 +298,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')
})
Expand Down
8 changes: 7 additions & 1 deletion packages/rum/src/performanceCollection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,13 @@ 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']

// cf https://github.com/w3c/paint-timing/issues/40
if (document.visibilityState === 'visible') {
entryTypes.push('paint')
}
glorieux marked this conversation as resolved.
Show resolved Hide resolved
observer.observe({ entryTypes })

if (supportPerformanceObject() && 'addEventListener' in performance) {
// https://bugzilla.mozilla.org/show_bug.cgi?id=1559377
Expand Down
94 changes: 94 additions & 0 deletions packages/rum/test/rum.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ 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'
Expand Down Expand Up @@ -520,3 +521,96 @@ describe('rum user action', () => {
expect((getRumMessage(server, 0) as any).fooBar).toEqual('foo')
})
})

describe('RUM hidden page', () => {
glorieux marked this conversation as resolved.
Show resolved Hide resolved
let lifeCycle: LifeCycle
let RUM: RumApi
let server: sinon.SinonFakeServer
const browserWindow = window as BrowserWindow
let stubBuilder: PerformanceObserverStubBuilder
let original: PerformanceObserver | undefined

interface ExpectedRequestBody {
evt: {
category: string
}
view: {
measures: {
first_contentful_paint: number
}
}
}

beforeEach(() => {
if (isIE()) {
pending('no full rum support')
}
server = sinon.fakeServer.create()
original = browserWindow.PerformanceObserver
stubBuilder = new PerformanceObserverStubBuilder()
browserWindow.PerformanceObserver = stubBuilder.getStub()
lifeCycle = new LifeCycle()
})

afterEach(() => {
browserWindow.PerformanceObserver = original
restorePageVisibility()
server.restore()
})

it('should not collect first_contentful_paint if page is not visible', () => {
glorieux marked this conversation as resolved.
Show resolved Hide resolved
const session = {
getId: () => undefined,
isTracked: () => true,
isTrackedWithResource: () => true,
}
const FAKE_RESOURCE: Partial<PerformanceEntry> = { name: 'http://foo.com', entryType: 'resource' }
const performanceEntry = { entryType: 'view', name: 'first-contentful-paint' }
Copy link
Contributor

@bcaudan bcaudan Apr 27, 2020

Choose a reason for hiding this comment

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

it could be moved to the constant section at the beginning of the describe in order to not repeat it between the two tests


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<ExpectedRequestBody>(server)
expect(initialRequests.length).toBeGreaterThan(0)

initialRequests.map((request) => {
if (request.evt.category === 'view') {
expect(request.view.measures.first_contentful_paint).toBeUndefined()
}
})
glorieux marked this conversation as resolved.
Show resolved Hide resolved
})

it('should collect first_contentful_paint if page is visible', () => {
glorieux marked this conversation as resolved.
Show resolved Hide resolved
const session = {
getId: () => undefined,
isTracked: () => true,
isTrackedWithResource: () => true,
}
const FAKE_RESOURCE: Partial<PerformanceEntry> = { 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<ExpectedRequestBody>(server)
glorieux marked this conversation as resolved.
Show resolved Hide resolved
expect(initialRequests.length).toBeGreaterThan(0)

initialRequests.map((request) => {
if (request.evt.category === 'view') {
expect(request.view.measures.first_contentful_paint).toBeDefined()
}
})
})
})