From 64ef3900929d75c544e9b8f055b5f3bc9e28f86f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Zugmeyer?= Date: Thu, 28 Mar 2024 14:48:20 +0100 Subject: [PATCH 1/3] =?UTF-8?q?=F0=9F=90=9B=20[RUM-2782]=20remove=20redire?= =?UTF-8?q?ct=20estimation=20based=20on=20fetchStart?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../src/domain/resource/resourceUtils.spec.ts | 20 +---------- .../src/domain/resource/resourceUtils.ts | 33 +++---------------- 2 files changed, 5 insertions(+), 48 deletions(-) diff --git a/packages/rum-core/src/domain/resource/resourceUtils.spec.ts b/packages/rum-core/src/domain/resource/resourceUtils.spec.ts index 4254431759..af31099f28 100644 --- 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 index 69d8a81499..41a8eaa180 100644 --- 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, @@ -141,7 +140,7 @@ export function toValidEntry(entry: RumPerformanceResourceTiming) { // collected, for example cross origin requests without a "Timing-Allow-Origin" header allowing // it. if ( - !areInOrder( + areInOrder( entry.startTime, entry.fetchStart, entry.domainLookupStart, @@ -151,39 +150,15 @@ export function toValidEntry(entry: RumPerformanceResourceTiming) { entry.requestStart, entry.responseStart, entry.responseEnd - ) + ) && + (!hasRedirection(entry) || areInOrder(entry.startTime, entry.redirectStart, entry.redirectEnd, entry.fetchStart)) ) { - return undefined - } - - if (!hasRedirection(entry)) { 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) { From 62efcfa2e1276bf8005657efd9e85cd3d765dc34 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Zugmeyer?= Date: Wed, 3 Apr 2024 18:37:36 +0200 Subject: [PATCH 2/3] =?UTF-8?q?=F0=9F=91=8C=20clarify=20the=20condition=20?= =?UTF-8?q?a=20bit?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../src/domain/resource/resourceUtils.ts | 30 ++++++++++--------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/packages/rum-core/src/domain/resource/resourceUtils.ts b/packages/rum-core/src/domain/resource/resourceUtils.ts index 41a8eaa180..18274547bd 100644 --- a/packages/rum-core/src/domain/resource/resourceUtils.ts +++ b/packages/rum-core/src/domain/resource/resourceUtils.ts @@ -139,20 +139,22 @@ 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 - ) && - (!hasRedirection(entry) || areInOrder(entry.startTime, entry.redirectStart, entry.redirectEnd, entry.fetchStart)) - ) { + 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) + + if (areCommonTimingsInOrder && areRedirectionTimingsInOrder) { return entry } } From 76ac2e2a62d7c91be9bbd9e459c0b35a987a23a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Zugmeyer?= Date: Thu, 4 Apr 2024 10:20:23 +0200 Subject: [PATCH 3/3] =?UTF-8?q?=F0=9F=91=8C=20clearer=20condition?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/rum-core/src/domain/resource/resourceUtils.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/rum-core/src/domain/resource/resourceUtils.ts b/packages/rum-core/src/domain/resource/resourceUtils.ts index 18274547bd..6421aa7e8b 100644 --- a/packages/rum-core/src/domain/resource/resourceUtils.ts +++ b/packages/rum-core/src/domain/resource/resourceUtils.ts @@ -151,8 +151,9 @@ export function toValidEntry(entry: RumPerformanceResourceTiming) { entry.responseEnd ) - const areRedirectionTimingsInOrder = - !hasRedirection(entry) || areInOrder(entry.startTime, entry.redirectStart, entry.redirectEnd, entry.fetchStart) + const areRedirectionTimingsInOrder = hasRedirection(entry) + ? areInOrder(entry.startTime, entry.redirectStart, entry.redirectEnd, entry.fetchStart) + : true if (areCommonTimingsInOrder && areRedirectionTimingsInOrder) { return entry