Skip to content

Commit 105f78e

Browse files
Lms24AbhiPrasad
andauthored
fix(browser): Improve handling of 0 and undefined resource timing values (#17751)
This patch - drops any `http.request.*` timing attributes where the original value was `undefined` - sends `0` for any `http.request.*` timing attribte values that were originally `0` (i.e. no longer converts them to the `timeOrigin` absolute timestamp) --------- Co-authored-by: Abhijeet Prasad <aprasad@sentry.io>
1 parent 678d6e8 commit 105f78e

File tree

3 files changed

+73
-39
lines changed

3 files changed

+73
-39
lines changed

packages/browser-utils/src/metrics/resourceTiming.ts

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,10 @@ import type { SpanAttributes } from '@sentry/core';
22
import { browserPerformanceTimeOrigin } from '@sentry/core';
33
import { extractNetworkProtocol, getBrowserPerformanceAPI } from './utils';
44

5-
function getAbsoluteTime(time = 0): number {
6-
return ((browserPerformanceTimeOrigin() || performance.timeOrigin) + time) / 1000;
5+
function getAbsoluteTime(time: number | undefined): number | undefined {
6+
// falsy values should be preserved so that we can later on drop undefined values and
7+
// preserve 0 vals for cross-origin resources without proper `Timing-Allow-Origin` header.
8+
return time ? ((browserPerformanceTimeOrigin() || performance.timeOrigin) + time) / 1000 : time;
79
}
810

911
/**
@@ -30,7 +32,7 @@ export function resourceTimingToSpanAttributes(resourceTiming: PerformanceResour
3032
return timingSpanData;
3133
}
3234

33-
return {
35+
return dropUndefinedKeysFromObject({
3436
...timingSpanData,
3537

3638
'http.request.redirect_start': getAbsoluteTime(resourceTiming.redirectStart),
@@ -55,6 +57,16 @@ export function resourceTimingToSpanAttributes(resourceTiming: PerformanceResour
5557
// For TTFB we actually want the relative time from timeOrigin to responseStart
5658
// This way, TTFB always measures the "first page load" experience.
5759
// see: https://web.dev/articles/ttfb#measure-resource-requests
58-
'http.request.time_to_first_byte': (resourceTiming.responseStart ?? 0) / 1000,
59-
};
60+
'http.request.time_to_first_byte':
61+
resourceTiming.responseStart != null ? resourceTiming.responseStart / 1000 : undefined,
62+
});
63+
}
64+
65+
/**
66+
* Remove properties with `undefined` as value from an object.
67+
* In contrast to `dropUndefinedKeys` in core this funciton only works on first-level
68+
* key-value objects and does not recursively go into object properties or arrays.
69+
*/
70+
function dropUndefinedKeysFromObject<T extends object>(attrs: T): Partial<T> {
71+
return Object.fromEntries(Object.entries(attrs).filter(([, value]) => value != null)) as Partial<T>;
6072
}

packages/browser-utils/test/browser/browserMetrics.test.ts

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -266,6 +266,18 @@ describe('_addResourceSpans', () => {
266266
decodedBodySize: 593,
267267
renderBlockingStatus: 'non-blocking',
268268
nextHopProtocol: 'http/1.1',
269+
connectStart: 1000,
270+
connectEnd: 1001,
271+
redirectStart: 1002,
272+
redirectEnd: 1003,
273+
fetchStart: 1004,
274+
domainLookupStart: 1005,
275+
domainLookupEnd: 1006,
276+
requestStart: 1007,
277+
responseStart: 1008,
278+
responseEnd: 1009,
279+
secureConnectionStart: 1005,
280+
workerStart: 1006,
269281
});
270282

271283
const timeOrigin = 100;
@@ -305,7 +317,7 @@ describe('_addResourceSpans', () => {
305317
'http.request.response_end': expect.any(Number),
306318
'http.request.response_start': expect.any(Number),
307319
'http.request.secure_connection_start': expect.any(Number),
308-
'http.request.time_to_first_byte': 0,
320+
'http.request.time_to_first_byte': 1.008,
309321
'http.request.worker_start': expect.any(Number),
310322
},
311323
}),
@@ -492,6 +504,18 @@ describe('_addResourceSpans', () => {
492504
encodedBodySize: null,
493505
decodedBodySize: null,
494506
nextHopProtocol: 'h3',
507+
connectStart: 1000,
508+
connectEnd: 1001,
509+
redirectStart: 1002,
510+
redirectEnd: 1003,
511+
fetchStart: 1004,
512+
domainLookupStart: 1005,
513+
domainLookupEnd: 1006,
514+
requestStart: 1007,
515+
responseStart: 1008,
516+
responseEnd: 1009,
517+
secureConnectionStart: 1005,
518+
workerStart: 1006,
495519
} as unknown as PerformanceResourceTiming;
496520

497521
_addResourceSpans(span, entry, resourceEntryName, 100, 23, 345);
@@ -518,7 +542,7 @@ describe('_addResourceSpans', () => {
518542
'http.request.response_end': expect.any(Number),
519543
'http.request.response_start': expect.any(Number),
520544
'http.request.secure_connection_start': expect.any(Number),
521-
'http.request.time_to_first_byte': 0,
545+
'http.request.time_to_first_byte': 1.008,
522546
'http.request.worker_start': expect.any(Number),
523547
},
524548
description: '/assets/to/css',

packages/browser-utils/test/metrics/resourceTiming.test.ts

Lines changed: 30 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ describe('resourceTimingToSpanAttributes', () => {
2626
duration: 200,
2727
initiatorType: 'fetch',
2828
nextHopProtocol: 'h2',
29-
workerStart: 0,
29+
workerStart: 1,
3030
redirectStart: 10,
3131
redirectEnd: 20,
3232
fetchStart: 25,
@@ -276,6 +276,13 @@ describe('resourceTimingToSpanAttributes', () => {
276276
});
277277

278278
it('handles zero timing values', () => {
279+
/**
280+
* Most resource timing entries have a 0 value if the resource was requested from
281+
* a cross-origin source which does not return a matching `Timing-Allow-Origin` header.
282+
*
283+
* see: https://developer.mozilla.org/en-US/docs/Web/API/Performance_API/Resource_timing#cross-origin_timing_information
284+
*/
285+
279286
extractNetworkProtocolSpy.mockReturnValue({
280287
name: '',
281288
version: 'unknown',
@@ -284,34 +291,36 @@ describe('resourceTimingToSpanAttributes', () => {
284291
const mockResourceTiming = createMockResourceTiming({
285292
nextHopProtocol: '',
286293
redirectStart: 0,
287-
fetchStart: 0,
294+
redirectEnd: 0,
295+
workerStart: 0,
296+
fetchStart: 1000100, // fetchStart is not restricted by `Timing-Allow-Origin` header
288297
domainLookupStart: 0,
289298
domainLookupEnd: 0,
290299
connectStart: 0,
291-
secureConnectionStart: 0,
292300
connectEnd: 0,
301+
secureConnectionStart: 0,
293302
requestStart: 0,
294303
responseStart: 0,
295-
responseEnd: 0,
304+
responseEnd: 1000200, // responseEnd is not restricted by `Timing-Allow-Origin` header
296305
});
297306

298307
const result = resourceTimingToSpanAttributes(mockResourceTiming);
299308

300309
expect(result).toEqual({
301310
'network.protocol.version': 'unknown',
302311
'network.protocol.name': '',
303-
'http.request.redirect_start': 1000, // (1000000 + 0) / 1000
304-
'http.request.redirect_end': 1000.02,
305-
'http.request.worker_start': 1000,
306-
'http.request.fetch_start': 1000,
307-
'http.request.domain_lookup_start': 1000,
308-
'http.request.domain_lookup_end': 1000,
309-
'http.request.connect_start': 1000,
310-
'http.request.secure_connection_start': 1000,
311-
'http.request.connection_end': 1000,
312-
'http.request.request_start': 1000,
313-
'http.request.response_start': 1000,
314-
'http.request.response_end': 1000,
312+
'http.request.redirect_start': 0,
313+
'http.request.redirect_end': 0,
314+
'http.request.worker_start': 0,
315+
'http.request.fetch_start': 2000.1,
316+
'http.request.domain_lookup_start': 0,
317+
'http.request.domain_lookup_end': 0,
318+
'http.request.connect_start': 0,
319+
'http.request.secure_connection_start': 0,
320+
'http.request.connection_end': 0,
321+
'http.request.request_start': 0,
322+
'http.request.response_start': 0,
323+
'http.request.response_end': 2000.2,
315324
'http.request.time_to_first_byte': 0,
316325
});
317326
});
@@ -343,7 +352,7 @@ describe('resourceTimingToSpanAttributes', () => {
343352
'network.protocol.name': 'http',
344353
'http.request.redirect_start': 1000.005,
345354
'http.request.redirect_end': 1000.02,
346-
'http.request.worker_start': 1000,
355+
'http.request.worker_start': 1000.001,
347356
'http.request.fetch_start': 1000.01,
348357
'http.request.domain_lookup_start': 1000.015,
349358
'http.request.domain_lookup_end': 1000.02,
@@ -470,7 +479,7 @@ describe('resourceTimingToSpanAttributes', () => {
470479
});
471480

472481
describe('edge cases', () => {
473-
it('handles undefined timing values', () => {
482+
it("doesn't include undefined timing values", () => {
474483
browserPerformanceTimeOriginSpy.mockReturnValue(1000000);
475484

476485
extractNetworkProtocolSpy.mockReturnValue({
@@ -481,6 +490,7 @@ describe('resourceTimingToSpanAttributes', () => {
481490
const mockResourceTiming = createMockResourceTiming({
482491
nextHopProtocol: '',
483492
redirectStart: undefined as any,
493+
redirectEnd: undefined as any,
484494
fetchStart: undefined as any,
485495
workerStart: undefined as any,
486496
domainLookupStart: undefined as any,
@@ -498,19 +508,6 @@ describe('resourceTimingToSpanAttributes', () => {
498508
expect(result).toEqual({
499509
'network.protocol.version': 'unknown',
500510
'network.protocol.name': '',
501-
'http.request.redirect_start': 1000, // (1000000 + 0) / 1000
502-
'http.request.redirect_end': 1000.02,
503-
'http.request.worker_start': 1000,
504-
'http.request.fetch_start': 1000,
505-
'http.request.domain_lookup_start': 1000,
506-
'http.request.domain_lookup_end': 1000,
507-
'http.request.connect_start': 1000,
508-
'http.request.secure_connection_start': 1000,
509-
'http.request.connection_end': 1000,
510-
'http.request.request_start': 1000,
511-
'http.request.response_start': 1000,
512-
'http.request.response_end': 1000,
513-
'http.request.time_to_first_byte': 0,
514511
});
515512
});
516513

@@ -534,6 +531,7 @@ describe('resourceTimingToSpanAttributes', () => {
534531
requestStart: 999999,
535532
responseStart: 999999,
536533
responseEnd: 999999,
534+
workerStart: 999999,
537535
});
538536

539537
const result = resourceTimingToSpanAttributes(mockResourceTiming);
@@ -543,7 +541,7 @@ describe('resourceTimingToSpanAttributes', () => {
543541
'network.protocol.name': '',
544542
'http.request.redirect_start': 1999.999, // (1000000 + 999999) / 1000
545543
'http.request.redirect_end': 1000.02,
546-
'http.request.worker_start': 1000,
544+
'http.request.worker_start': 1999.999,
547545
'http.request.fetch_start': 1999.999,
548546
'http.request.domain_lookup_start': 1999.999,
549547
'http.request.domain_lookup_end': 1999.999,

0 commit comments

Comments
 (0)