🐛 [RUM-94] ignore performance resource timings with negative duration

packages/rum-core/src/browser/performanceObservable.spec.ts

@@ -1,4 +1,4 @@
-import type { Subscription } from '@datadog/browser-core'
+import type { Duration, Subscription } from '@datadog/browser-core'
 import type { Clock } from '@datadog/browser-core/test'
 import { mockClock } from '@datadog/browser-core/test'
 import type { RumConfiguration } from '../domain/configuration'
@@ -39,7 +39,7 @@ describe('performanceObservable', () => {
       expect(observableCallback).toHaveBeenCalledWith([jasmine.objectContaining({ name: allowedUrl })])
     })
 
-    it('should not notify forbidden performance resources', () => {
+    it('should not notify performance resources with forbidden url', () => {
       const { notifyPerformanceEntries } = mockPerformanceObserver()
       const performanceResourceObservable = createPerformanceObservable(configuration, {
         type: RumPerformanceEntryType.RESOURCE,
@@ -50,6 +50,17 @@ describe('performanceObservable', () => {
       expect(observableCallback).not.toHaveBeenCalled()
     })
 
+    it('should not notify performance resources with invalid duration', () => {
+      const { notifyPerformanceEntries } = mockPerformanceObserver()
+      const performanceResourceObservable = createPerformanceObservable(configuration, {
+        type: RumPerformanceEntryType.RESOURCE,
+      })
+      performanceSubscription = performanceResourceObservable.subscribe(observableCallback)
+
+      notifyPerformanceEntries([createPerformanceEntry(RumPerformanceEntryType.RESOURCE, { duration: -1 as Duration })])
+      expect(observableCallback).not.toHaveBeenCalled()
+    })
+
     it('should notify buffered performance resources asynchronously', () => {
       const { notifyPerformanceEntries } = mockPerformanceObserver()
       // add the performance entry to the buffer

packages/rum-core/src/browser/performanceObservable.ts

@@ -1,7 +1,7 @@
 import type { Duration, RelativeTime, TimeoutId } 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'
+import { hasValidResourceEntryDuration, isAllowedRequestUrl } from '../domain/resource/resourceUtils'
 
 type RumPerformanceObserverConstructor = new (callback: PerformanceObserverCallback) => RumPerformanceObserver
 
@@ -283,5 +283,8 @@ function filterRumPerformanceEntries<T extends RumPerformanceEntryType>(
 }
 
 function isForbiddenResource(configuration: RumConfiguration, entry: RumPerformanceEntry) {
-  return entry.entryType === RumPerformanceEntryType.RESOURCE && !isAllowedRequestUrl(configuration, entry.name)
+  return (
+    entry.entryType === RumPerformanceEntryType.RESOURCE &&
+    (!isAllowedRequestUrl(configuration, entry.name) || !hasValidResourceEntryDuration(entry))
+  )
 }

packages/rum-core/src/domain/resource/matchRequestTiming.spec.ts → packages/rum-core/src/domain/resource/matchRequestResourceEntry.spec.ts

@@ -6,9 +6,9 @@ import type { RumPerformanceResourceTiming } from '../../browser/performanceObse
 import { RumPerformanceEntryType } from '../../browser/performanceObservable'
 import type { RequestCompleteEvent } from '../requestCollection'
 
-import { matchRequestTiming } from './matchRequestTiming'
+import { matchRequestResourceEntry } from './matchRequestResourceEntry'
 
-describe('matchRequestTiming', () => {
+describe('matchRequestResourceEntry', () => {
   const FAKE_REQUEST: Partial<RequestCompleteEvent> = {
     startClocks: relativeToClocks(100 as RelativeTime),
     duration: 500 as Duration,
@@ -23,65 +23,65 @@ describe('matchRequestTiming', () => {
     spyOn(performance, 'getEntriesByName').and.returnValue(entries)
   })
 
-  it('should match single timing nested in the request ', () => {
+  it('should match single entry nested in the request ', () => {
     const entry = createPerformanceEntry(RumPerformanceEntryType.RESOURCE, {
       startTime: 200 as RelativeTime,
       duration: 300 as Duration,
     })
     entries.push(entry)
 
-    const matchingTiming = matchRequestTiming(FAKE_REQUEST as RequestCompleteEvent)
+    const matchingEntry = matchRequestResourceEntry(FAKE_REQUEST as RequestCompleteEvent)
 
-    expect(matchingTiming).toEqual(entry.toJSON() as RumPerformanceResourceTiming)
+    expect(matchingEntry).toEqual(entry.toJSON() as RumPerformanceResourceTiming)
   })
 
-  it('should match single timing nested in the request with error margin', () => {
+  it('should match single entry nested in the request with error margin', () => {
     const entry = createPerformanceEntry(RumPerformanceEntryType.RESOURCE, {
       startTime: 99 as RelativeTime,
       duration: 502 as Duration,
     })
     entries.push(entry)
 
-    const matchingTiming = matchRequestTiming(FAKE_REQUEST as RequestCompleteEvent)
+    const matchingEntry = matchRequestResourceEntry(FAKE_REQUEST as RequestCompleteEvent)
 
-    expect(matchingTiming).toEqual(entry.toJSON() as RumPerformanceResourceTiming)
+    expect(matchingEntry).toEqual(entry.toJSON() as RumPerformanceResourceTiming)
   })
 
-  it('should not match single timing outside the request ', () => {
+  it('should not match single entry outside the request ', () => {
     const entry = createPerformanceEntry(RumPerformanceEntryType.RESOURCE, {
       startTime: 0 as RelativeTime,
       duration: 300 as Duration,
     })
     entries.push(entry)
 
-    const matchingTiming = matchRequestTiming(FAKE_REQUEST as RequestCompleteEvent)
+    const matchingEntry = matchRequestResourceEntry(FAKE_REQUEST as RequestCompleteEvent)
 
-    expect(matchingTiming).toEqual(undefined)
+    expect(matchingEntry).toEqual(undefined)
   })
 
-  it('should discard already matched timings when multiple identical requests are done conurently', () => {
+  it('should discard already matched entries when multiple identical requests are done conurently', () => {
     const entry1 = createPerformanceEntry(RumPerformanceEntryType.RESOURCE, {
       startTime: 200 as RelativeTime,
       duration: 300 as Duration,
     })
     entries.push(entry1)
 
-    const matchingTiming1 = matchRequestTiming(FAKE_REQUEST as RequestCompleteEvent)
+    const matchingEntry1 = matchRequestResourceEntry(FAKE_REQUEST as RequestCompleteEvent)
 
-    expect(matchingTiming1).toEqual(entry1.toJSON() as RumPerformanceResourceTiming)
+    expect(matchingEntry1).toEqual(entry1.toJSON() as RumPerformanceResourceTiming)
 
     const entry2 = createPerformanceEntry(RumPerformanceEntryType.RESOURCE, {
       startTime: 99 as RelativeTime,
       duration: 502 as Duration,
     })
     entries.push(entry2)
 
-    const matchingTiming2 = matchRequestTiming(FAKE_REQUEST as RequestCompleteEvent)
+    const matchingEntry2 = matchRequestResourceEntry(FAKE_REQUEST as RequestCompleteEvent)
 
-    expect(matchingTiming2).toEqual(entry2.toJSON() as RumPerformanceResourceTiming)
+    expect(matchingEntry2).toEqual(entry2.toJSON() as RumPerformanceResourceTiming)
   })
 
-  it('should not match two not following timings nested in the request ', () => {
+  it('should not match two not following entries nested in the request ', () => {
     const entry1 = createPerformanceEntry(RumPerformanceEntryType.RESOURCE, {
       startTime: 150 as RelativeTime,
       duration: 100 as Duration,
@@ -92,12 +92,12 @@ describe('matchRequestTiming', () => {
     })
     entries.push(entry1, entry2)
 
-    const matchingTiming = matchRequestTiming(FAKE_REQUEST as RequestCompleteEvent)
+    const matchingEntry = matchRequestResourceEntry(FAKE_REQUEST as RequestCompleteEvent)
 
-    expect(matchingTiming).toEqual(undefined)
+    expect(matchingEntry).toEqual(undefined)
   })
 
-  it('should not match multiple timings nested in the request', () => {
+  it('should not match multiple entries nested in the request', () => {
     const entry1 = createPerformanceEntry(RumPerformanceEntryType.RESOURCE, {
       startTime: 100 as RelativeTime,
       duration: 50 as Duration,
@@ -112,12 +112,24 @@ describe('matchRequestTiming', () => {
     })
     entries.push(entry1, entry2, entry3)
 
-    const matchingTiming = matchRequestTiming(FAKE_REQUEST as RequestCompleteEvent)
+    const matchingEntry = matchRequestResourceEntry(FAKE_REQUEST as RequestCompleteEvent)
 
-    expect(matchingTiming).toEqual(undefined)
+    expect(matchingEntry).toEqual(undefined)
   })
 
-  it('[without tolerant_resource_timings] should not match invalid timing nested in the request ', () => {
+  it('should not match entry with invalid duration', () => {
+    const entry = createPerformanceEntry(RumPerformanceEntryType.RESOURCE, {
+      duration: -1 as Duration,
+    })
+
+    entries.push(entry)
+
+    const matchingEntry = matchRequestResourceEntry(FAKE_REQUEST as RequestCompleteEvent)
+
+    expect(matchingEntry).toEqual(undefined)
+  })
+
+  it('[without tolerant_resource_timings] should not match invalid entry nested in the request ', () => {
     const entry = createPerformanceEntry(RumPerformanceEntryType.RESOURCE, {
       // fetchStart < startTime is invalid
       fetchStart: 0 as RelativeTime,
@@ -126,12 +138,12 @@ describe('matchRequestTiming', () => {
 
     entries.push(entry)
 
-    const matchingTiming = matchRequestTiming(FAKE_REQUEST as RequestCompleteEvent)
+    const matchingEntry = matchRequestResourceEntry(FAKE_REQUEST as RequestCompleteEvent)
 
-    expect(matchingTiming).toEqual(undefined)
+    expect(matchingEntry).toEqual(undefined)
   })
 
-  it('[with tolerant_resource_timings] should match invalid timing nested in the request ', () => {
+  it('[with tolerant_resource_timings] should match invalid entry nested in the request ', () => {
     mockExperimentalFeatures([ExperimentalFeature.TOLERANT_RESOURCE_TIMINGS])
     const entry = createPerformanceEntry(RumPerformanceEntryType.RESOURCE, {
       // fetchStart < startTime is invalid
@@ -141,8 +153,8 @@ describe('matchRequestTiming', () => {
 
     entries.push(entry)
 
-    const matchingTiming = matchRequestTiming(FAKE_REQUEST as RequestCompleteEvent)
+    const matchingEntry = matchRequestResourceEntry(FAKE_REQUEST as RequestCompleteEvent)
 
-    expect(matchingTiming).toBeDefined()
+    expect(matchingEntry).toBeDefined()
   })
 })

packages/rum-core/src/domain/resource/matchRequestTiming.ts → packages/rum-core/src/domain/resource/matchRequestResourceEntry.ts

@@ -3,7 +3,7 @@ import { addDuration } from '@datadog/browser-core'
 import type { RumPerformanceResourceTiming } from '../../browser/performanceObservable'
 import type { RequestCompleteEvent } from '../requestCollection'
 import { WeakSet } from '../../browser/polyfills'
-import { isValidEntry } from './resourceUtils'
+import { hasValidResourceEntryDuration, hasValidResourceEntryTimings } from './resourceUtils'
 
 interface Timing {
   startTime: RelativeTime
@@ -25,7 +25,7 @@ const alreadyMatchedEntries = new WeakSet<PerformanceEntry>()
  * - then, if a single timing match, return the timing
  * - otherwise we can't decide, return undefined
  */
-export function matchRequestTiming(request: RequestCompleteEvent) {
+export function matchRequestResourceEntry(request: RequestCompleteEvent) {
   if (!performance || !('getEntriesByName' in performance)) {
     return
   }
@@ -37,7 +37,7 @@ export function matchRequestTiming(request: RequestCompleteEvent) {
 
   const candidates = sameNameEntries
     .filter((entry) => !alreadyMatchedEntries.has(entry))
-    .filter((entry) => isValidEntry(entry))
+    .filter((entry) => hasValidResourceEntryDuration(entry) && hasValidResourceEntryTimings(entry))
     .filter((entry) =>
       isBetween(
         entry,

packages/rum-core/src/domain/resource/resourceCollection.ts

@@ -24,13 +24,13 @@ import type { RequestCompleteEvent } from '../requestCollection'
 import type { PageStateHistory } from '../contexts/pageStateHistory'
 import { PageState } from '../contexts/pageStateHistory'
 import { createTraceIdentifier } from '../tracing/tracer'
-import { matchRequestTiming } from './matchRequestTiming'
+import { matchRequestResourceEntry } from './matchRequestResourceEntry'
 import {
-  computePerformanceResourceDetails,
-  computePerformanceResourceDuration,
-  computeResourceKind,
-  computeSize,
-  isRequestKind,
+  computeResourceEntryDetails,
+  computeResourceEntryDuration,
+  computeResourceEntryType,
+  computeResourceEntrySize,
+  isResourceEntryRequestType,
   isLongDataUrl,
   sanitizeDataUrl,
 } from './resourceUtils'
@@ -54,7 +54,7 @@ export function startResourceCollection(
     buffered: true,
   }).subscribe((entries) => {
     for (const entry of entries) {
-      if (!isRequestKind(entry)) {
+      if (!isResourceEntryRequestType(entry)) {
         const rawEvent = processResourceEntry(entry, configuration)
         if (rawEvent) {
           lifeCycle.notify(LifeCycleEventType.RAW_RUM_EVENT_COLLECTED, rawEvent)
@@ -82,7 +82,7 @@ function processRequest(
   configuration: RumConfiguration,
   pageStateHistory: PageStateHistory
 ): RawRumEventCollectedData<RawRumResourceEvent> | undefined {
-  const matchingTiming = matchRequestTiming(request)
+  const matchingTiming = matchRequestResourceEntry(request)
   const startClocks = matchingTiming ? relativeToClocks(matchingTiming.startTime) : request.startClocks
   const tracingInfo = computeRequestTracingInfo(request, configuration)
   if (!configuration.trackResources && !tracingInfo) {
@@ -91,7 +91,7 @@ function processRequest(
 
   const type = request.type === RequestType.XHR ? ResourceType.XHR : ResourceType.FETCH
 
-  const correspondingTimingOverrides = matchingTiming ? computePerformanceEntryMetrics(matchingTiming) : undefined
+  const correspondingTimingOverrides = matchingTiming ? computeResourceEntryMetrics(matchingTiming) : undefined
 
   const duration = computeRequestDuration(pageStateHistory, startClocks, request.duration)
 
@@ -136,13 +136,13 @@ function processResourceEntry(
   configuration: RumConfiguration
 ): RawRumEventCollectedData<RawRumResourceEvent> | undefined {
   const startClocks = relativeToClocks(entry.startTime)
-  const tracingInfo = computeEntryTracingInfo(entry, configuration)
+  const tracingInfo = computeResourceEntryTracingInfo(entry, configuration)
   if (!configuration.trackResources && !tracingInfo) {
     return
   }
 
-  const type = computeResourceKind(entry)
-  const entryMetrics = computePerformanceEntryMetrics(entry)
+  const type = computeResourceEntryType(entry)
+  const entryMetrics = computeResourceEntryMetrics(entry)
 
   const resourceEvent = combine(
     {
@@ -170,16 +170,16 @@ function processResourceEntry(
   }
 }
 
-function computePerformanceEntryMetrics(timing: RumPerformanceResourceTiming) {
-  const { renderBlockingStatus } = timing
+function computeResourceEntryMetrics(entry: RumPerformanceResourceTiming) {
+  const { renderBlockingStatus } = entry
   return {
     resource: assign(
       {
-        duration: computePerformanceResourceDuration(timing),
+        duration: computeResourceEntryDuration(entry),
         render_blocking_status: renderBlockingStatus,
       },
-      computeSize(timing),
-      computePerformanceResourceDetails(timing)
+      computeResourceEntrySize(entry),
+      computeResourceEntryDetails(entry)
     ),
   }
 }
@@ -198,7 +198,7 @@ function computeRequestTracingInfo(request: RequestCompleteEvent, configuration:
   }
 }
 
-function computeEntryTracingInfo(entry: RumPerformanceResourceTiming, configuration: RumConfiguration) {
+function computeResourceEntryTracingInfo(entry: RumPerformanceResourceTiming, configuration: RumConfiguration) {
   const hasBeenTraced = entry.traceId
   if (!hasBeenTraced) {
     return undefined