Skip to content

Commit

Permalink
πŸ› skip worker timing when no worker is used (#3147)
Browse files Browse the repository at this point in the history
* βœ… 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.
  • Loading branch information
BenoitZugmeyer authored Nov 20, 2024
1 parent 66ceb30 commit 82d898a
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 13 deletions.
24 changes: 12 additions & 12 deletions packages/rum-core/src/domain/resource/resourceUtils.spec.ts
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -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,
Expand All @@ -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', () => {
Expand Down Expand Up @@ -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', () => {
Expand Down
2 changes: 1 addition & 1 deletion packages/rum-core/src/domain/resource/resourceUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down

0 comments on commit 82d898a

Please sign in to comment.