🐛 [RUM-2782] validate resource timings more granularly

Before this commit, we dropped any `PerformanceResourceEntry` that had
inconsistent timings. With this commit, when the experimental feature
"tolerant_resource_timings" is enabled, the various timings are
validated independently.

This should improve the situation on Chrome when a Service Worker is
involved. In this case, most of the time, connection timings are
unexpectedly set after the `requestStart` timing (see Chromium
issue[1]). Before this change, the whole `PerformanceResourceEntry`
would be ignored, and no timings would be included. With this change
(and the flag enabled), relevant timings will be correctly defined.

Related issue: #2566

[1]: https://issues.chromium.org/issues/323703325

packages/core/src/tools/experimentalFeatures.ts

@@ -19,6 +19,7 @@ export enum ExperimentalFeature {
   DISABLE_REPLAY_INLINE_CSS = 'disable_replay_inline_css',
   WRITABLE_RESOURCE_GRAPHQL = 'writable_resource_graphql',
   CUSTOM_VITALS = 'custom_vitals',
+  TOLERANT_RESOURCE_TIMINGS = 'tolerant_resource_timings',
 }
 
 const enabledExperimentalFeatures: Set<ExperimentalFeature> = new Set()

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

@@ -1,5 +1,6 @@
 import type { Duration, RelativeTime } from '@datadog/browser-core'
-import { isIE, relativeToClocks } from '@datadog/browser-core'
+import { ExperimentalFeature, isIE, relativeToClocks } from '@datadog/browser-core'
+import { mockExperimentalFeatures } from '@datadog/browser-core/test'
 import { createPerformanceEntry } from '../../../test'
 import type { RumPerformanceResourceTiming } from '../../browser/performanceCollection'
 import { RumPerformanceEntryType } from '../../browser/performanceCollection'
@@ -94,7 +95,7 @@ describe('matchRequestTiming', () => {
     expect(matchingTiming).toEqual(undefined)
   })
 
-  it('should not match invalid timing nested in the request ', () => {
+  it('[without tolerant_resource_timings] should not match invalid timing nested in the request ', () => {
     const entry = createPerformanceEntry(RumPerformanceEntryType.RESOURCE, {
       // fetchStart < startTime is invalid
       fetchStart: 0 as RelativeTime,
@@ -107,4 +108,19 @@ describe('matchRequestTiming', () => {
 
     expect(matchingTiming).toEqual(undefined)
   })
+
+  it('[with tolerant_resource_timings] should match invalid timing nested in the request ', () => {
+    mockExperimentalFeatures([ExperimentalFeature.TOLERANT_RESOURCE_TIMINGS])
+    const entry = createPerformanceEntry(RumPerformanceEntryType.RESOURCE, {
+      // fetchStart < startTime is invalid
+      fetchStart: 0 as RelativeTime,
+      startTime: 200 as RelativeTime,
+    })
+
+    entries.push(entry)
+
+    const matchingTiming = matchRequestTiming(FAKE_REQUEST as RequestCompleteEvent)
+
+    expect(matchingTiming).toBeDefined()
+  })
 })

packages/rum-core/src/domain/resource/resourceUtils.spec.ts

@@ -1,5 +1,5 @@
-import type { Duration, RelativeTime, ServerDuration } from '@datadog/browser-core'
-import { SPEC_ENDPOINTS } from '@datadog/browser-core/test'
+import { ExperimentalFeature, type Duration, type RelativeTime, type ServerDuration } from '@datadog/browser-core'
+import { SPEC_ENDPOINTS, mockExperimentalFeatures } from '@datadog/browser-core/test'
 import { RumPerformanceEntryType, type RumPerformanceResourceTiming } from '../../browser/performanceCollection'
 import type { RumConfiguration } from '../configuration'
 import { validateAndBuildRumConfiguration } from '../configuration'
@@ -196,31 +196,37 @@ describe('computePerformanceResourceDetails', () => {
   })
   ;[
     {
+      timing: 'connect' as const,
+      reason: 'connectStart > connectEnd',
       connectEnd: 10 as RelativeTime,
       connectStart: 20 as RelativeTime,
-      reason: 'connectStart > connectEnd',
     },
     {
+      timing: 'dns' as const,
+      reason: 'domainLookupStart > domainLookupEnd',
       domainLookupEnd: 10 as RelativeTime,
       domainLookupStart: 20 as RelativeTime,
-      reason: 'domainLookupStart > domainLookupEnd',
     },
     {
+      timing: 'download' as const,
       reason: 'responseStart > responseEnd',
       responseEnd: 10 as RelativeTime,
       responseStart: 20 as RelativeTime,
     },
     {
+      timing: 'first_byte' as const,
       reason: 'requestStart > responseStart',
       requestStart: 20 as RelativeTime,
       responseStart: 10 as RelativeTime,
     },
     {
+      timing: 'redirect' as const,
       reason: 'redirectStart > redirectEnd',
       redirectEnd: 15 as RelativeTime,
       redirectStart: 20 as RelativeTime,
     },
     {
+      timing: 'ssl' as const,
       connectEnd: 10 as RelativeTime,
       reason: 'secureConnectionStart > connectEnd',
       secureConnectionStart: 20 as RelativeTime,
@@ -231,10 +237,17 @@ describe('computePerformanceResourceDetails', () => {
       fetchStart: 10 as RelativeTime,
       reason: 'negative timing start',
     },
-  ].forEach(({ reason, ...overrides }) => {
-    it(`should not compute entry when ${reason}`, () => {
+  ].forEach(({ reason, timing, ...overrides }) => {
+    it(`[without tolerant-resource-timings] should not compute entry when ${reason}`, () => {
       expect(computePerformanceResourceDetails(generateResourceWith(overrides))).toBeUndefined()
     })
+
+    if (timing) {
+      it(`[with tolerant-resource-timings] should not include the '${timing}' timing when ${reason}`, () => {
+        mockExperimentalFeatures([ExperimentalFeature.TOLERANT_RESOURCE_TIMINGS])
+        expect(computePerformanceResourceDetails(generateResourceWith(overrides))![timing]).toBeUndefined()
+      })
+    }
   })
 
   it('should allow really fast document resource', () => {

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

@@ -2,8 +2,10 @@ import type { RelativeTime, ServerDuration } from '@datadog/browser-core'
 import {
   addTelemetryDebug,
   elapsed,
+  ExperimentalFeature,
   getPathName,
   includes,
+  isExperimentalFeatureEnabled,
   isValidUrl,
   ResourceType,
   toServerDuration,
@@ -19,8 +21,8 @@ export interface PerformanceResourceDetails {
   dns?: PerformanceResourceDetailsElement
   connect?: PerformanceResourceDetailsElement
   ssl?: PerformanceResourceDetailsElement
-  first_byte: PerformanceResourceDetailsElement
-  download: PerformanceResourceDetailsElement
+  first_byte?: PerformanceResourceDetailsElement
+  download?: PerformanceResourceDetailsElement
 }
 
 export const FAKE_INITIAL_DOCUMENT = 'initial_document'
@@ -113,28 +115,33 @@ export function computePerformanceResourceDetails(
   }
 
   // Make sure a connection occurred
-  if (connectEnd !== fetchStart) {
+  if (fetchStart < connectEnd) {
     details.connect = formatTiming(startTime, connectStart, connectEnd)
 
     // Make sure a secure connection occurred
-    if (areInOrder(connectStart, secureConnectionStart, connectEnd)) {
+    if (connectStart <= secureConnectionStart && secureConnectionStart <= connectEnd) {
       details.ssl = formatTiming(startTime, secureConnectionStart, connectEnd)
     }
   }
 
   // Make sure a domain lookup occurred
-  if (domainLookupEnd !== fetchStart) {
+  if (fetchStart < domainLookupEnd) {
     details.dns = formatTiming(startTime, domainLookupStart, domainLookupEnd)
   }
 
-  if (hasRedirection(entry)) {
+  // Make sure a redirection occurred
+  if (startTime < redirectEnd) {
     details.redirect = formatTiming(startTime, redirectStart, redirectEnd)
   }
 
   return details
 }
 
 export function toValidEntry(entry: RumPerformanceResourceTiming) {
+  if (isExperimentalFeatureEnabled(ExperimentalFeature.TOLERANT_RESOURCE_TIMINGS)) {
+    return entry
+  }
+
   // Ensure timings are in the right order. On top of filtering out potential invalid
   // RumPerformanceResourceTiming, it will ignore entries from requests where timings cannot be
   // collected, for example cross origin requests without a "Timing-Allow-Origin" header allowing
@@ -163,11 +170,12 @@ export function toValidEntry(entry: RumPerformanceResourceTiming) {
 function hasRedirection(entry: RumPerformanceResourceTiming) {
   return entry.redirectEnd > entry.startTime
 }
-
 function formatTiming(origin: RelativeTime, start: RelativeTime, end: RelativeTime) {
-  return {
-    duration: toServerDuration(elapsed(start, end)),
-    start: toServerDuration(elapsed(origin, start)),
+  if (origin <= start && start <= end) {
+    return {
+      duration: toServerDuration(elapsed(start, end)),
+      start: toServerDuration(elapsed(origin, start)),
+    }
   }
 }