From 12a5cdd5399bc6a3cad43e77aa3d4682e9ef7451 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Wed, 24 Sep 2025 12:21:53 +0200 Subject: [PATCH 1/3] fix(browser): Improve handling of `0` and `undefined` resource timing values --- .../src/metrics/resourceTiming.ts | 28 +++++++-- .../test/browser/browserMetrics.test.ts | 28 ++++++++- .../test/metrics/resourceTiming.test.ts | 62 +++++++++---------- 3 files changed, 79 insertions(+), 39 deletions(-) diff --git a/packages/browser-utils/src/metrics/resourceTiming.ts b/packages/browser-utils/src/metrics/resourceTiming.ts index fe613355c55d..becafd69bba0 100644 --- a/packages/browser-utils/src/metrics/resourceTiming.ts +++ b/packages/browser-utils/src/metrics/resourceTiming.ts @@ -2,8 +2,10 @@ import type { SpanAttributes } from '@sentry/core'; import { browserPerformanceTimeOrigin } from '@sentry/core'; import { extractNetworkProtocol, getBrowserPerformanceAPI } from './utils'; -function getAbsoluteTime(time = 0): number { - return ((browserPerformanceTimeOrigin() || performance.timeOrigin) + time) / 1000; +function getAbsoluteTime(time: number | undefined): number | undefined { + // falsy values should be preserved so that we can later on drop undefined values and + // preserve 0 vals for cross-origin resources without proper `Timing-Allow-Origin` header. + return time ? ((browserPerformanceTimeOrigin() || performance.timeOrigin) + time) / 1000 : time; } /** @@ -30,7 +32,7 @@ export function resourceTimingToSpanAttributes(resourceTiming: PerformanceResour return timingSpanData; } - return { + return dropUndefinedKeysFromObject({ ...timingSpanData, 'http.request.redirect_start': getAbsoluteTime(resourceTiming.redirectStart), @@ -55,6 +57,22 @@ export function resourceTimingToSpanAttributes(resourceTiming: PerformanceResour // For TTFB we actually want the relative time from timeOrigin to responseStart // This way, TTFB always measures the "first page load" experience. // see: https://web.dev/articles/ttfb#measure-resource-requests - 'http.request.time_to_first_byte': (resourceTiming.responseStart ?? 0) / 1000, - }; + 'http.request.time_to_first_byte': + resourceTiming.responseStart != null ? resourceTiming.responseStart / 1000 : undefined, + }); +} + +/** + * Remove properties with `undefined` as value from an object. + * In contrast to `dropUndefinedKeys` in core this funciton only works on first-level + * key-value objects and does not recursively go into object properties or arrays. + */ +function dropUndefinedKeysFromObject(attrs: T): T { + const cleaned = {} as T; + Object.keys(attrs).forEach(key => { + if (attrs[key as keyof T] != null) { + cleaned[key as keyof T] = attrs[key as keyof T]; + } + }); + return cleaned; } diff --git a/packages/browser-utils/test/browser/browserMetrics.test.ts b/packages/browser-utils/test/browser/browserMetrics.test.ts index c717bb81ca0b..c734ec326b47 100644 --- a/packages/browser-utils/test/browser/browserMetrics.test.ts +++ b/packages/browser-utils/test/browser/browserMetrics.test.ts @@ -266,6 +266,18 @@ describe('_addResourceSpans', () => { decodedBodySize: 593, renderBlockingStatus: 'non-blocking', nextHopProtocol: 'http/1.1', + connectStart: 1000, + connectEnd: 1001, + redirectStart: 1002, + redirectEnd: 1003, + fetchStart: 1004, + domainLookupStart: 1005, + domainLookupEnd: 1006, + requestStart: 1007, + responseStart: 1008, + responseEnd: 1009, + secureConnectionStart: 1005, + workerStart: 1006, }); const timeOrigin = 100; @@ -305,7 +317,7 @@ describe('_addResourceSpans', () => { 'http.request.response_end': expect.any(Number), 'http.request.response_start': expect.any(Number), 'http.request.secure_connection_start': expect.any(Number), - 'http.request.time_to_first_byte': 0, + 'http.request.time_to_first_byte': 1.008, 'http.request.worker_start': expect.any(Number), }, }), @@ -492,6 +504,18 @@ describe('_addResourceSpans', () => { encodedBodySize: null, decodedBodySize: null, nextHopProtocol: 'h3', + connectStart: 1000, + connectEnd: 1001, + redirectStart: 1002, + redirectEnd: 1003, + fetchStart: 1004, + domainLookupStart: 1005, + domainLookupEnd: 1006, + requestStart: 1007, + responseStart: 1008, + responseEnd: 1009, + secureConnectionStart: 1005, + workerStart: 1006, } as unknown as PerformanceResourceTiming; _addResourceSpans(span, entry, resourceEntryName, 100, 23, 345); @@ -518,7 +542,7 @@ describe('_addResourceSpans', () => { 'http.request.response_end': expect.any(Number), 'http.request.response_start': expect.any(Number), 'http.request.secure_connection_start': expect.any(Number), - 'http.request.time_to_first_byte': 0, + 'http.request.time_to_first_byte': 1.008, 'http.request.worker_start': expect.any(Number), }, description: '/assets/to/css', diff --git a/packages/browser-utils/test/metrics/resourceTiming.test.ts b/packages/browser-utils/test/metrics/resourceTiming.test.ts index 881a7075441e..5e35097423d1 100644 --- a/packages/browser-utils/test/metrics/resourceTiming.test.ts +++ b/packages/browser-utils/test/metrics/resourceTiming.test.ts @@ -26,7 +26,7 @@ describe('resourceTimingToSpanAttributes', () => { duration: 200, initiatorType: 'fetch', nextHopProtocol: 'h2', - workerStart: 0, + workerStart: 1, redirectStart: 10, redirectEnd: 20, fetchStart: 25, @@ -276,6 +276,13 @@ describe('resourceTimingToSpanAttributes', () => { }); it('handles zero timing values', () => { + /** + * Most resource timing entries have a 0 value if the resource was requested from + * a cross-origin source which does not return a matching `Timing-Allow-Origin` header. + * + * see: https://developer.mozilla.org/en-US/docs/Web/API/Performance_API/Resource_timing#cross-origin_timing_information + */ + extractNetworkProtocolSpy.mockReturnValue({ name: '', version: 'unknown', @@ -284,15 +291,17 @@ describe('resourceTimingToSpanAttributes', () => { const mockResourceTiming = createMockResourceTiming({ nextHopProtocol: '', redirectStart: 0, - fetchStart: 0, + redirectEnd: 0, + workerStart: 0, + fetchStart: 1000100, // fetchStart is not restricted by `Timing-Allow-Origin` header domainLookupStart: 0, domainLookupEnd: 0, connectStart: 0, - secureConnectionStart: 0, connectEnd: 0, + secureConnectionStart: 0, requestStart: 0, responseStart: 0, - responseEnd: 0, + responseEnd: 1000200, // responseEnd is not restricted by `Timing-Allow-Origin` header }); const result = resourceTimingToSpanAttributes(mockResourceTiming); @@ -300,18 +309,18 @@ describe('resourceTimingToSpanAttributes', () => { expect(result).toEqual({ 'network.protocol.version': 'unknown', 'network.protocol.name': '', - 'http.request.redirect_start': 1000, // (1000000 + 0) / 1000 - 'http.request.redirect_end': 1000.02, - 'http.request.worker_start': 1000, - 'http.request.fetch_start': 1000, - 'http.request.domain_lookup_start': 1000, - 'http.request.domain_lookup_end': 1000, - 'http.request.connect_start': 1000, - 'http.request.secure_connection_start': 1000, - 'http.request.connection_end': 1000, - 'http.request.request_start': 1000, - 'http.request.response_start': 1000, - 'http.request.response_end': 1000, + 'http.request.redirect_start': 0, + 'http.request.redirect_end': 0, + 'http.request.worker_start': 0, + 'http.request.fetch_start': 2000.1, + 'http.request.domain_lookup_start': 0, + 'http.request.domain_lookup_end': 0, + 'http.request.connect_start': 0, + 'http.request.secure_connection_start': 0, + 'http.request.connection_end': 0, + 'http.request.request_start': 0, + 'http.request.response_start': 0, + 'http.request.response_end': 2000.2, 'http.request.time_to_first_byte': 0, }); }); @@ -343,7 +352,7 @@ describe('resourceTimingToSpanAttributes', () => { 'network.protocol.name': 'http', 'http.request.redirect_start': 1000.005, 'http.request.redirect_end': 1000.02, - 'http.request.worker_start': 1000, + 'http.request.worker_start': 1000.001, 'http.request.fetch_start': 1000.01, 'http.request.domain_lookup_start': 1000.015, 'http.request.domain_lookup_end': 1000.02, @@ -470,7 +479,7 @@ describe('resourceTimingToSpanAttributes', () => { }); describe('edge cases', () => { - it('handles undefined timing values', () => { + it("doesn't include undefined timing values", () => { browserPerformanceTimeOriginSpy.mockReturnValue(1000000); extractNetworkProtocolSpy.mockReturnValue({ @@ -481,6 +490,7 @@ describe('resourceTimingToSpanAttributes', () => { const mockResourceTiming = createMockResourceTiming({ nextHopProtocol: '', redirectStart: undefined as any, + redirectEnd: undefined as any, fetchStart: undefined as any, workerStart: undefined as any, domainLookupStart: undefined as any, @@ -498,19 +508,6 @@ describe('resourceTimingToSpanAttributes', () => { expect(result).toEqual({ 'network.protocol.version': 'unknown', 'network.protocol.name': '', - 'http.request.redirect_start': 1000, // (1000000 + 0) / 1000 - 'http.request.redirect_end': 1000.02, - 'http.request.worker_start': 1000, - 'http.request.fetch_start': 1000, - 'http.request.domain_lookup_start': 1000, - 'http.request.domain_lookup_end': 1000, - 'http.request.connect_start': 1000, - 'http.request.secure_connection_start': 1000, - 'http.request.connection_end': 1000, - 'http.request.request_start': 1000, - 'http.request.response_start': 1000, - 'http.request.response_end': 1000, - 'http.request.time_to_first_byte': 0, }); }); @@ -534,6 +531,7 @@ describe('resourceTimingToSpanAttributes', () => { requestStart: 999999, responseStart: 999999, responseEnd: 999999, + workerStart: 999999, }); const result = resourceTimingToSpanAttributes(mockResourceTiming); @@ -543,7 +541,7 @@ describe('resourceTimingToSpanAttributes', () => { 'network.protocol.name': '', 'http.request.redirect_start': 1999.999, // (1000000 + 999999) / 1000 'http.request.redirect_end': 1000.02, - 'http.request.worker_start': 1000, + 'http.request.worker_start': 1999.999, 'http.request.fetch_start': 1999.999, 'http.request.domain_lookup_start': 1999.999, 'http.request.domain_lookup_end': 1999.999, From 5ff0ca3f988e434cb3c4929e6e837164cc3fb06e Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Wed, 24 Sep 2025 15:34:01 +0200 Subject: [PATCH 2/3] Update packages/browser-utils/src/metrics/resourceTiming.ts Co-authored-by: Abhijeet Prasad --- packages/browser-utils/src/metrics/resourceTiming.ts | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/packages/browser-utils/src/metrics/resourceTiming.ts b/packages/browser-utils/src/metrics/resourceTiming.ts index becafd69bba0..d003c3621596 100644 --- a/packages/browser-utils/src/metrics/resourceTiming.ts +++ b/packages/browser-utils/src/metrics/resourceTiming.ts @@ -68,11 +68,5 @@ export function resourceTimingToSpanAttributes(resourceTiming: PerformanceResour * key-value objects and does not recursively go into object properties or arrays. */ function dropUndefinedKeysFromObject(attrs: T): T { - const cleaned = {} as T; - Object.keys(attrs).forEach(key => { - if (attrs[key as keyof T] != null) { - cleaned[key as keyof T] = attrs[key as keyof T]; - } - }); - return cleaned; + return Object.fromEntries(Object.entries(attrs).filter(([, value]) => value != null)) as Partial; } From 58c24b74ed129c7ff09fc3b5a7a9e15df54f3555 Mon Sep 17 00:00:00 2001 From: Lukas Stracke Date: Wed, 24 Sep 2025 15:35:20 +0200 Subject: [PATCH 3/3] partial --- packages/browser-utils/src/metrics/resourceTiming.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/browser-utils/src/metrics/resourceTiming.ts b/packages/browser-utils/src/metrics/resourceTiming.ts index d003c3621596..5a711d307cf3 100644 --- a/packages/browser-utils/src/metrics/resourceTiming.ts +++ b/packages/browser-utils/src/metrics/resourceTiming.ts @@ -67,6 +67,6 @@ export function resourceTimingToSpanAttributes(resourceTiming: PerformanceResour * In contrast to `dropUndefinedKeys` in core this funciton only works on first-level * key-value objects and does not recursively go into object properties or arrays. */ -function dropUndefinedKeysFromObject(attrs: T): T { +function dropUndefinedKeysFromObject(attrs: T): Partial { return Object.fromEntries(Object.entries(attrs).filter(([, value]) => value != null)) as Partial; }