🐛 [RUM-2782] remove buggy redirect timing estimation based on fetchSt…

…art (#2683) * 🐛 [RUM-2782] remove redirect estimation based on fetchStart * 👌 clarify the condition a bit * 👌 clearer condition
DataDog · Apr 4, 2024 · c47a9dd · c47a9dd
1 parent 61258c1
commit c47a9dd
Show file tree

Hide file tree

Showing 2 changed files with 19 additions and 59 deletions.
diff --git a/packages/rum-core/src/domain/resource/resourceUtils.spec.ts b/packages/rum-core/src/domain/resource/resourceUtils.spec.ts
@@ -217,7 +217,7 @@ describe('computePerformanceResourceDetails', () => {
     },
     {
       reason: 'redirectStart > redirectEnd',
-      redirectEnd: 10 as RelativeTime,
+      redirectEnd: 15 as RelativeTime,
       redirectStart: 20 as RelativeTime,
     },
     {
@@ -259,24 +259,6 @@ describe('computePerformanceResourceDetails', () => {
       first_byte: { start: 0 as ServerDuration, duration: 30e6 as ServerDuration },
     })
   })
-
-  it('should use startTime and fetchStart as fallback for redirectStart and redirectEnd', () => {
-    expect(
-      computePerformanceResourceDetails(
-        generateResourceWith({
-          redirectEnd: 0 as RelativeTime,
-          redirectStart: 0 as RelativeTime,
-        })
-      )
-    ).toEqual({
-      connect: { start: 5e6 as ServerDuration, duration: 2e6 as ServerDuration },
-      dns: { start: 3e6 as ServerDuration, duration: 1e6 as ServerDuration },
-      download: { start: 40e6 as ServerDuration, duration: 10e6 as ServerDuration },
-      first_byte: { start: 10e6 as ServerDuration, duration: 30e6 as ServerDuration },
-      redirect: { start: 0 as ServerDuration, duration: 2e6 as ServerDuration },
-      ssl: { start: 6e6 as ServerDuration, duration: 1e6 as ServerDuration },
-    })
-  })
 })
 
 describe('computePerformanceResourceDuration', () => {

diff --git a/packages/rum-core/src/domain/resource/resourceUtils.ts b/packages/rum-core/src/domain/resource/resourceUtils.ts
@@ -1,6 +1,5 @@
 import type { RelativeTime, ServerDuration } from '@datadog/browser-core'
 import {
-  assign,
   addTelemetryDebug,
   elapsed,
   getPathName,
@@ -140,50 +139,29 @@ export function toValidEntry(entry: RumPerformanceResourceTiming) {
   // RumPerformanceResourceTiming, it will ignore entries from requests where timings cannot be
   // collected, for example cross origin requests without a "Timing-Allow-Origin" header allowing
   // it.
-  if (
-    !areInOrder(
-      entry.startTime,
-      entry.fetchStart,
-      entry.domainLookupStart,
-      entry.domainLookupEnd,
-      entry.connectStart,
-      entry.connectEnd,
-      entry.requestStart,
-      entry.responseStart,
-      entry.responseEnd
-    )
-  ) {
-    return undefined
-  }
-
-  if (!hasRedirection(entry)) {
+  const areCommonTimingsInOrder = areInOrder(
+    entry.startTime,
+    entry.fetchStart,
+    entry.domainLookupStart,
+    entry.domainLookupEnd,
+    entry.connectStart,
+    entry.connectEnd,
+    entry.requestStart,
+    entry.responseStart,
+    entry.responseEnd
+  )
+
+  const areRedirectionTimingsInOrder = hasRedirection(entry)
+    ? areInOrder(entry.startTime, entry.redirectStart, entry.redirectEnd, entry.fetchStart)
+    : true
+
+  if (areCommonTimingsInOrder && areRedirectionTimingsInOrder) {
     return entry
   }
-
-  let { redirectStart, redirectEnd } = entry
-  // Firefox doesn't provide redirect timings on cross origin requests.
-  // Provide a default for those.
-  if (redirectStart < entry.startTime) {
-    redirectStart = entry.startTime
-  }
-  if (redirectEnd < entry.startTime) {
-    redirectEnd = entry.fetchStart
-  }
-
-  // Make sure redirect timings are in order
-  if (!areInOrder(entry.startTime, redirectStart, redirectEnd, entry.fetchStart)) {
-    return undefined
-  }
-
-  return assign({}, entry, {
-    redirectEnd,
-    redirectStart,
-  })
 }
 
 function hasRedirection(entry: RumPerformanceResourceTiming) {
-  // The only time fetchStart is different than startTime is if a redirection occurred.
-  return entry.fetchStart !== entry.startTime
+  return entry.redirectEnd > entry.startTime
 }
 
 function formatTiming(origin: RelativeTime, start: RelativeTime, end: RelativeTime) {