Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

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)

AltStyle によって変換されたページ (->オリジナル) /