From 82d898a97e4e299f41e5a53f230056d7c170da29 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Beno=C3=AEt?= <benoit.zugmeyer@datadoghq.com>
Date: Wed, 20 Nov 2024 10:55:36 +0100
Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20skip=20worker=20timing=20when=20?=
 =?UTF-8?q?no=20worker=20is=20used=20(#3147)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* ✅ set workerStart to 0 in mocked resource entries

`workerStart` defaults to 0 in resource entries, not 'undefined'. This
commit sets the correct default. It also removes the type cast, so we
have proper typechecking and we don't miss future properties in the
mock. To do this, I had to define all other properties so the entry is
actually complete.

* ✅ simplify the test assertion

We don't need to re-check every single timings, just focus on the worker
one.

* 🐛 do not use undefined worker timings

`workerStart` defaults to 0 when there is no worker in use. Make sure it
is not 0 before collecting it.
---
 .../src/domain/resource/resourceUtils.spec.ts | 24 +++++++++----------
 .../src/domain/resource/resourceUtils.ts      |  2 +-
 2 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/packages/rum-core/src/domain/resource/resourceUtils.spec.ts b/packages/rum-core/src/domain/resource/resourceUtils.spec.ts
index 3679acf4dd..bcf34a6351 100644
--- a/packages/rum-core/src/domain/resource/resourceUtils.spec.ts
+++ b/packages/rum-core/src/domain/resource/resourceUtils.spec.ts
@@ -1,4 +1,4 @@
-import { type Duration, type RelativeTime, type ServerDuration } from '@datadog/browser-core'
+import { assign, type Duration, type RelativeTime, type ServerDuration } from '@datadog/browser-core'
 import { RumPerformanceEntryType, type RumPerformanceResourceTiming } from '../../browser/performanceObservable'
 import {
   MAX_ATTRIBUTE_VALUE_CHAR_LENGTH,
@@ -11,7 +11,7 @@ import {
 } from './resourceUtils'
 
 function generateResourceWith(overrides: Partial<RumPerformanceResourceTiming>) {
-  const completeTiming: Partial<RumPerformanceResourceTiming> = {
+  const completeTiming: RumPerformanceResourceTiming = {
     connectEnd: 17 as RelativeTime,
     connectStart: 15 as RelativeTime,
     domainLookupEnd: 14 as RelativeTime,
@@ -27,9 +27,17 @@ function generateResourceWith(overrides: Partial<RumPerformanceResourceTiming>)
     responseStart: 50 as RelativeTime,
     secureConnectionStart: 16 as RelativeTime,
     startTime: 10 as RelativeTime,
+    workerStart: 0 as RelativeTime,
+
+    initiatorType: 'script',
+    decodedBodySize: 0,
+    encodedBodySize: 0,
+    transferSize: 0,
+    toJSON: () => assign({}, completeTiming, { toJSON: undefined }),
+
     ...overrides,
   }
-  return completeTiming as RumPerformanceResourceTiming
+  return completeTiming
 }
 
 describe('computeResourceEntryType', () => {
@@ -111,15 +119,7 @@ describe('computeResourceEntryDetails', () => {
       fetchStart: 12 as RelativeTime,
     })
     const details = computeResourceEntryDetails(resourceTiming)
-    expect(details).toEqual({
-      worker: { start: 1e6 as ServerDuration, duration: 1e6 as ServerDuration },
-      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: 1e6 as ServerDuration },
-      ssl: { start: 6e6 as ServerDuration, duration: 1e6 as ServerDuration },
-    })
+    expect(details!.worker).toEqual({ start: 1e6 as ServerDuration, duration: 1e6 as ServerDuration })
   })
 
   it('should not compute redirect timing when no redirect', () => {
diff --git a/packages/rum-core/src/domain/resource/resourceUtils.ts b/packages/rum-core/src/domain/resource/resourceUtils.ts
index ac4a354dcd..67eaf85fcd 100644
--- a/packages/rum-core/src/domain/resource/resourceUtils.ts
+++ b/packages/rum-core/src/domain/resource/resourceUtils.ts
@@ -111,7 +111,7 @@ export function computeResourceEntryDetails(entry: RumPerformanceResourceTiming)
   }
 
   // Make sure a worker processing time is recorded
-  if (workerStart < fetchStart) {
+  if (0 < workerStart && workerStart < fetchStart) {
     details.worker = formatTiming(startTime, workerStart, fetchStart)
   }