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

🐛 [RUM-958] Fix performance observable compatibility with old browser version #2850

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
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
93 changes: 66 additions & 27 deletions packages/rum-core/src/browser/performanceObservable.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { RumPerformanceEntryType, createPerformanceObservable } from './performa

describe('performanceObservable', () => {
let configuration: RumConfiguration
let performanceSubscription: Subscription
let performanceSubscription: Subscription | undefined
const forbiddenUrl = 'https://forbidden.url'
const allowedUrl = 'https://allowed.url'
let notifyPerformanceEntries: (entries: RumPerformanceEntry[]) => void
Expand All @@ -21,46 +21,85 @@ describe('performanceObservable', () => {
}
observableCallback = jasmine.createSpy()
configuration = { isIntakeUrl: (url: string) => url === forbiddenUrl } as unknown as RumConfiguration
;({ notifyPerformanceEntries } = mockPerformanceObserver())
clock = mockClock()
})

afterEach(() => {
performanceSubscription.unsubscribe()
clock.cleanup()
performanceSubscription?.unsubscribe()
clock?.cleanup()
})

it('should notify performance resources', () => {
const performanceResourceObservable = createPerformanceObservable(configuration, {
type: RumPerformanceEntryType.RESOURCE,
describe('primary strategy when type supported', () => {
beforeEach(() => {
;({ notifyPerformanceEntries } = mockPerformanceObserver())
})
performanceSubscription = performanceResourceObservable.subscribe(observableCallback)

notifyPerformanceEntries([createPerformanceEntry(RumPerformanceEntryType.RESOURCE, { name: allowedUrl })])
expect(observableCallback).toHaveBeenCalledWith([jasmine.objectContaining({ name: allowedUrl })])
})
it('should notify performance resources', () => {
const performanceResourceObservable = createPerformanceObservable(configuration, {
type: RumPerformanceEntryType.RESOURCE,
})
performanceSubscription = performanceResourceObservable.subscribe(observableCallback)

notifyPerformanceEntries([createPerformanceEntry(RumPerformanceEntryType.RESOURCE, { name: allowedUrl })])
expect(observableCallback).toHaveBeenCalledWith([jasmine.objectContaining({ name: allowedUrl })])
})

it('should not notify forbidden performance resources', () => {
const performanceResourceObservable = createPerformanceObservable(configuration, {
type: RumPerformanceEntryType.RESOURCE,
it('should not notify forbidden performance resources', () => {
const performanceResourceObservable = createPerformanceObservable(configuration, {
type: RumPerformanceEntryType.RESOURCE,
})
performanceSubscription = performanceResourceObservable.subscribe(observableCallback)

notifyPerformanceEntries([createPerformanceEntry(RumPerformanceEntryType.RESOURCE, { name: forbiddenUrl })])
expect(observableCallback).not.toHaveBeenCalled()
})
performanceSubscription = performanceResourceObservable.subscribe(observableCallback)

notifyPerformanceEntries([createPerformanceEntry(RumPerformanceEntryType.RESOURCE, { name: forbiddenUrl })])
expect(observableCallback).not.toHaveBeenCalled()
it('should notify buffered performance resources asynchronously', () => {
// add the performance entry to the buffer
notifyPerformanceEntries([createPerformanceEntry(RumPerformanceEntryType.RESOURCE, { name: allowedUrl })])

const performanceResourceObservable = createPerformanceObservable(configuration, {
type: RumPerformanceEntryType.RESOURCE,
buffered: true,
})
performanceSubscription = performanceResourceObservable.subscribe(observableCallback)
expect(observableCallback).not.toHaveBeenCalled()
clock.tick(0)
expect(observableCallback).toHaveBeenCalledWith([jasmine.objectContaining({ name: allowedUrl })])
})
})

it('should notify buffered performance resources asynchronously', () => {
// add the performance entry to the buffer
notifyPerformanceEntries([createPerformanceEntry(RumPerformanceEntryType.RESOURCE, { name: allowedUrl })])
describe('fallback strategy when type not supported', () => {
let bufferedEntries: PerformanceEntryList

beforeEach(() => {
bufferedEntries = []
spyOn(performance, 'getEntriesByType').and.callFake(() => bufferedEntries)
;({ notifyPerformanceEntries } = mockPerformanceObserver({ typeSupported: false }))
})

it('should notify performance resources when type not supported', () => {
const performanceResourceObservable = createPerformanceObservable(configuration, {
type: RumPerformanceEntryType.RESOURCE,
})
performanceSubscription = performanceResourceObservable.subscribe(observableCallback)

notifyPerformanceEntries([createPerformanceEntry(RumPerformanceEntryType.RESOURCE, { name: allowedUrl })])
expect(observableCallback).toHaveBeenCalledWith([jasmine.objectContaining({ name: allowedUrl })])
})

it('should notify buffered performance resources when type not supported', () => {
// add the performance entry to the buffer
bufferedEntries = [createPerformanceEntry(RumPerformanceEntryType.RESOURCE, { name: allowedUrl })]

const performanceResourceObservable = createPerformanceObservable(configuration, {
type: RumPerformanceEntryType.RESOURCE,
buffered: true,
const performanceResourceObservable = createPerformanceObservable(configuration, {
type: RumPerformanceEntryType.RESOURCE,
buffered: true,
})
performanceSubscription = performanceResourceObservable.subscribe(observableCallback)
expect(observableCallback).not.toHaveBeenCalled()
clock.tick(0)
expect(observableCallback).toHaveBeenCalledWith([jasmine.objectContaining({ name: allowedUrl })])
})
performanceSubscription = performanceResourceObservable.subscribe(observableCallback)
expect(observableCallback).not.toHaveBeenCalled()
clock.tick(0)
expect(observableCallback).toHaveBeenCalledWith([jasmine.objectContaining({ name: allowedUrl })])
})
})
44 changes: 32 additions & 12 deletions packages/rum-core/src/browser/performanceObservable.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type { Duration, RelativeTime, TimeoutId } from '@datadog/browser-core'
import { addEventListener, Observable, setTimeout, clearTimeout } from '@datadog/browser-core'
import { addEventListener, Observable, setTimeout, clearTimeout, monitor, includes } from '@datadog/browser-core'
import type { RumConfiguration } from '../domain/configuration'
import { isAllowedRequestUrl } from '../domain/resource/resourceUtils'

Expand Down Expand Up @@ -148,25 +148,45 @@ export function createPerformanceObservable<T extends RumPerformanceEntryType>(

if (window.PerformanceObserver) {
let isObserverInitializing = true
const handlePerformanceEntries = (entries: PerformanceObserverEntryList) => {
const handlePerformanceEntries = (entries: PerformanceEntryList) => {
const rumPerformanceEntries = filterRumPerformanceEntries(
configuration,
entries.getEntries() as Array<EntryTypeToReturnType[T]>
entries as Array<EntryTypeToReturnType[T]>
)
if (rumPerformanceEntries.length > 0) {
observable.notify(rumPerformanceEntries)
}
}
observer = new PerformanceObserver((entries) => {
// In Safari the performance observer callback is synchronous.
// Because the buffered performance entry list can be quite large we delay the computation to prevent the SDK from blocking the main thread on init
if (isObserverInitializing) {
timeoutId = setTimeout(() => handlePerformanceEntries(entries))
} else {
handlePerformanceEntries(entries)
observer = new PerformanceObserver(
monitor((entries) => {
// In Safari the performance observer callback is synchronous.
// Because the buffered performance entry list can be quite large we delay the computation to prevent the SDK from blocking the main thread on init
if (isObserverInitializing) {
timeoutId = setTimeout(() => handlePerformanceEntries(entries.getEntries()))
} else {
handlePerformanceEntries(entries.getEntries())
}
})
)
try {
observer.observe(options)
} catch {
// Some old browser versions (<= chrome 74 ) don't support the PerformanceObserver type and buffered options
// In these cases, fallback to getEntriesByType and PerformanceObserver with entryTypes
// TODO: remove this fallback in the next major version
const fallbackSupportedEntryTypes = [
RumPerformanceEntryType.RESOURCE,
RumPerformanceEntryType.NAVIGATION,
RumPerformanceEntryType.LONG_TASK,
RumPerformanceEntryType.PAINT,
]
if (includes(fallbackSupportedEntryTypes, options.type)) {
if (options.buffered) {
timeoutId = setTimeout(() => handlePerformanceEntries(performance.getEntriesByType(options.type)))
}
observer.observe({ entryTypes: [options.type] })
}
})
observer.observe(options)
}
isObserverInitializing = false
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ describe('trackCumulativeLayoutShift', () => {

beforeEach(() => {
if (
!('PerformanceObserver' in window) ||
!('supportedEntryTypes' in PerformanceObserver) ||
!window.PerformanceObserver ||
!PerformanceObserver.supportedEntryTypes ||
!PerformanceObserver.supportedEntryTypes.includes('layout-shift')
) {
pending('No LayoutShift API support')
Expand Down
5 changes: 4 additions & 1 deletion packages/rum-core/test/emulate/mockPerformanceObserver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ type PerformanceObserverInstance = {
entryTypes: string[]
}

export function mockPerformanceObserver() {
export function mockPerformanceObserver({ typeSupported } = { typeSupported: true }) {
const originalPerformanceObserver = window.PerformanceObserver
const instances = new Set<PerformanceObserverInstance>()
let performanceObserver: PerformanceObserver
Expand All @@ -21,6 +21,9 @@ export function mockPerformanceObserver() {
instances.delete(instance)
},
observe({ entryTypes, type, buffered }: PerformanceObserverInit) {
if (!typeSupported && type) {
throw new Error("Uncaught TypeError: Failed to execute 'observe' on 'PerformanceObserver")
}
instance.entryTypes = entryTypes || (type ? [type] : [])
instances.add(instance)
if (buffered) {
Expand Down