Skip to content

fix(browser): Improve handling of 0 and undefined resource timing values #17751

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Sep 24, 2025
Merged

fix(browser): Improve handling of 0 and undefined resource timing values #17751

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
12a5cdd
fix(browser): Improve handling of `0` and `undefined` resource timing…
Lms24 Sep 24, 2025
5ff0ca3
Update packages/browser-utils/src/metrics/resourceTiming.ts
Lms24 Sep 24, 2025
58c24b7
partial
Lms24 Sep 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 23 additions & 5 deletions packages/browser-utils/src/metrics/resourceTiming.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

/**
Expand All @@ -30,7 +32,7 @@ export function resourceTimingToSpanAttributes(resourceTiming: PerformanceResour
return timingSpanData;
}

return {
return dropUndefinedKeysFromObject({
...timingSpanData,

'http.request.redirect_start': getAbsoluteTime(resourceTiming.redirectStart),
Expand All @@ -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<T extends object>(attrs: T): T {
Copy link
Member Author

@Lms24 Lms24 Sep 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is probably useful to be extracted into browser or core at some point. I know we had our reservations against dropUndefinedKeys but IIRC this was mainly due to over-usage and it recursively going through the object. I think a light-weight implementation like this one should be fine but happy to hear other opinions.

Main motivation for using it in browser: it's more size-efficient than checking every value for definedness before adding the attribute.

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;
}
28 changes: 26 additions & 2 deletions packages/browser-utils/test/browser/browserMetrics.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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),
},
}),
Expand Down Expand Up @@ -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);
Expand All @@ -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',
Expand Down
62 changes: 30 additions & 32 deletions packages/browser-utils/test/metrics/resourceTiming.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ describe('resourceTimingToSpanAttributes', () => {
duration: 200,
initiatorType: 'fetch',
nextHopProtocol: 'h2',
workerStart: 0,
workerStart: 1,
redirectStart: 10,
redirectEnd: 20,
fetchStart: 25,
Expand Down Expand Up @@ -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',
Expand All @@ -284,34 +291,36 @@ 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);

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,
});
});
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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({
Expand All @@ -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,
Expand All @@ -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,
});
});

Expand All @@ -534,6 +531,7 @@ describe('resourceTimingToSpanAttributes', () => {
requestStart: 999999,
responseStart: 999999,
responseEnd: 999999,
workerStart: 999999,
});

const result = resourceTimingToSpanAttributes(mockResourceTiming);
Expand All @@ -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,
Expand Down
Loading