Skip to content

Commit 71a5a7b

Browse files
mydeamydea
authored
ref(browser): Refactor type casts of browser metrics (#14542)
Get rid of some type casts around browser timing events/metrics capture.
1 parent be9edf1 commit 71a5a7b

File tree

2 files changed

+98
-61
lines changed

2 files changed

+98
-61
lines changed

‎packages/browser-utils/src/metrics/browserMetrics.ts

+69-41
Original file line numberDiff line numberDiff line change
@@ -300,7 +300,7 @@ interface AddPerformanceEntriesOptions {
300300
/** Add performance related spans to a transaction */
301301
export function addPerformanceEntries(span: Span, options: AddPerformanceEntriesOptions): void {
302302
const performance = getBrowserPerformanceAPI();
303-
if (!performance || !WINDOW.performance.getEntries || !browserPerformanceTimeOrigin) {
303+
if (!performance || !performance.getEntries || !browserPerformanceTimeOrigin) {
304304
// Gatekeeper if performance API not available
305305
return;
306306
}
@@ -311,8 +311,7 @@ export function addPerformanceEntries(span: Span, options: AddPerformanceEntries
311311

312312
const { op, start_timestamp: transactionStartTime } = spanToJSON(span);
313313

314-
// eslint-disable-next-line @typescript-eslint/no-explicit-any
315-
performanceEntries.slice(_performanceCursor).forEach((entry: Record<string, any>) => {
314+
performanceEntries.slice(_performanceCursor).forEach(entry => {
316315
const startTime = msToSec(entry.startTime);
317316
const duration = msToSec(
318317
// Inexplicably, Chrome sometimes emits a negative duration. We need to work around this.
@@ -328,7 +327,7 @@ export function addPerformanceEntries(span: Span, options: AddPerformanceEntries
328327

329328
switch (entry.entryType) {
330329
case 'navigation': {
331-
_addNavigationSpans(span, entry, timeOrigin);
330+
_addNavigationSpans(span, entry as PerformanceNavigationTiming, timeOrigin);
332331
break;
333332
}
334333
case 'mark':
@@ -350,10 +349,9 @@ export function addPerformanceEntries(span: Span, options: AddPerformanceEntries
350349
break;
351350
}
352351
case 'resource': {
353-
_addResourceSpans(span, entry, entry.name as string, startTime, duration, timeOrigin);
352+
_addResourceSpans(span, entry as PerformanceResourceTiming, entry.name, startTime, duration, timeOrigin);
354353
break;
355354
}
356-
default:
357355
// Ignore other entry types.
358356
}
359357
});
@@ -411,11 +409,13 @@ export function addPerformanceEntries(span: Span, options: AddPerformanceEntries
411409
_measurements = {};
412410
}
413411

414-
/** Create measure related spans */
412+
/**
413+
* Create measure related spans.
414+
* Exported only for tests.
415+
*/
415416
export function _addMeasureSpans(
416417
span: Span,
417-
// eslint-disable-next-line @typescript-eslint/no-explicit-any
418-
entry: Record<string, any>,
418+
entry: PerformanceEntry,
419419
startTime: number,
420420
duration: number,
421421
timeOrigin: number,
@@ -454,44 +454,72 @@ export function _addMeasureSpans(
454454
}
455455

456456
/** Instrument navigation entries */
457-
// eslint-disable-next-line @typescript-eslint/no-explicit-any
458-
function _addNavigationSpans(span: Span, entry: Record<string, any>, timeOrigin: number): void {
459-
['unloadEvent', 'redirect', 'domContentLoadedEvent', 'loadEvent', 'connect'].forEach(event => {
457+
function _addNavigationSpans(span: Span, entry: PerformanceNavigationTiming, timeOrigin: number): void {
458+
(['unloadEvent', 'redirect', 'domContentLoadedEvent', 'loadEvent', 'connect'] as const).forEach(event => {
460459
_addPerformanceNavigationTiming(span, entry, event, timeOrigin);
461460
});
462-
_addPerformanceNavigationTiming(span, entry, 'secureConnection', timeOrigin, 'TLS/SSL', 'connectEnd');
463-
_addPerformanceNavigationTiming(span, entry, 'fetch', timeOrigin, 'cache', 'domainLookupStart');
461+
_addPerformanceNavigationTiming(span, entry, 'secureConnection', timeOrigin, 'TLS/SSL');
462+
_addPerformanceNavigationTiming(span, entry, 'fetch', timeOrigin, 'cache');
464463
_addPerformanceNavigationTiming(span, entry, 'domainLookup', timeOrigin, 'DNS');
464+
465465
_addRequest(span, entry, timeOrigin);
466466
}
467467

468+
type StartEventName =
469+
| 'secureConnection'
470+
| 'fetch'
471+
| 'domainLookup'
472+
| 'unloadEvent'
473+
| 'redirect'
474+
| 'connect'
475+
| 'domContentLoadedEvent'
476+
| 'loadEvent';
477+
478+
type EndEventName =
479+
| 'connectEnd'
480+
| 'domainLookupStart'
481+
| 'domainLookupEnd'
482+
| 'unloadEventEnd'
483+
| 'redirectEnd'
484+
| 'connectEnd'
485+
| 'domContentLoadedEventEnd'
486+
| 'loadEventEnd';
487+
468488
/** Create performance navigation related spans */
469489
function _addPerformanceNavigationTiming(
470490
span: Span,
471-
// eslint-disable-next-line @typescript-eslint/no-explicit-any
472-
entry: Record<string, any>,
473-
event: string,
491+
entry: PerformanceNavigationTiming,
492+
event: StartEventName,
474493
timeOrigin: number,
475-
name?: string,
476-
eventEnd?: string,
494+
name: string = event,
477495
): void {
478-
const end = eventEnd ? (entry[eventEnd] as number | undefined) : (entry[`${event}End`] as number | undefined);
479-
const start = entry[`${event}Start`] as number | undefined;
496+
const eventEnd = _getEndPropertyNameForNavigationTiming(event) satisfies keyof PerformanceNavigationTiming;
497+
const end = entry[eventEnd];
498+
const start = entry[`${event}Start`];
480499
if (!start || !end) {
481500
return;
482501
}
483502
startAndEndSpan(span, timeOrigin + msToSec(start), timeOrigin + msToSec(end), {
484-
op: `browser.${name || event}`,
503+
op: `browser.${name}`,
485504
name: entry.name,
486505
attributes: {
487506
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.ui.browser.metrics',
488507
},
489508
});
490509
}
491510

511+
function _getEndPropertyNameForNavigationTiming(event: StartEventName): EndEventName {
512+
if (event === 'secureConnection') {
513+
return 'connectEnd';
514+
}
515+
if (event === 'fetch') {
516+
return 'domainLookupStart';
517+
}
518+
return `${event}End`;
519+
}
520+
492521
/** Create request and response related spans */
493-
// eslint-disable-next-line @typescript-eslint/no-explicit-any
494-
function _addRequest(span: Span, entry: Record<string, any>, timeOrigin: number): void {
522+
function _addRequest(span: Span, entry: PerformanceNavigationTiming, timeOrigin: number): void {
495523
const requestStartTimestamp = timeOrigin + msToSec(entry.requestStart as number);
496524
const responseEndTimestamp = timeOrigin + msToSec(entry.responseEnd as number);
497525
const responseStartTimestamp = timeOrigin + msToSec(entry.responseStart as number);
@@ -518,19 +546,13 @@ function _addRequest(span: Span, entry: Record<string, any>, timeOrigin: number)
518546
}
519547
}
520548

521-
export interface ResourceEntry extends Record<string, unknown> {
522-
initiatorType?: string;
523-
transferSize?: number;
524-
encodedBodySize?: number;
525-
decodedBodySize?: number;
526-
renderBlockingStatus?: string;
527-
deliveryType?: string;
528-
}
529-
530-
/** Create resource-related spans */
549+
/**
550+
* Create resource-related spans.
551+
* Exported only for tests.
552+
*/
531553
export function _addResourceSpans(
532554
span: Span,
533-
entry: ResourceEntry,
555+
entry: PerformanceResourceTiming,
534556
resourceUrl: string,
535557
startTime: number,
536558
duration: number,
@@ -551,13 +573,19 @@ export function _addResourceSpans(
551573
setResourceEntrySizeData(attributes, entry, 'encodedBodySize', 'http.response_content_length');
552574
setResourceEntrySizeData(attributes, entry, 'decodedBodySize', 'http.decoded_response_content_length');
553575

554-
if (entry.deliveryType != null) {
555-
attributes['http.response_delivery_type'] = entry.deliveryType;
576+
// `deliveryType` is experimental and does not exist everywhere
577+
const deliveryType = (entry as { deliveryType?: 'cache' | 'navigational-prefetch' | '' }).deliveryType;
578+
if (deliveryType != null) {
579+
attributes['http.response_delivery_type'] = deliveryType;
556580
}
557581

558-
if ('renderBlockingStatus' in entry) {
559-
attributes['resource.render_blocking_status'] = entry.renderBlockingStatus;
582+
// Types do not reflect this property yet
583+
const renderBlockingStatus = (entry as { renderBlockingStatus?: 'render-blocking' | 'non-render-blocking' })
584+
.renderBlockingStatus;
585+
if (renderBlockingStatus) {
586+
attributes['resource.render_blocking_status'] = renderBlockingStatus;
560587
}
588+
561589
if (parsedUrl.protocol) {
562590
attributes['url.scheme'] = parsedUrl.protocol.split(':').pop(); // the protocol returned by parseUrl includes a :, but OTEL spec does not, so we remove it.
563591
}
@@ -655,8 +683,8 @@ function _setWebVitalAttributes(span: Span): void {
655683

656684
function setResourceEntrySizeData(
657685
attributes: SpanAttributes,
658-
entry: ResourceEntry,
659-
key: keyof Pick<ResourceEntry, 'transferSize' | 'encodedBodySize' | 'decodedBodySize'>,
686+
entry: PerformanceResourceTiming,
687+
key: keyof Pick<PerformanceResourceTiming, 'transferSize' | 'encodedBodySize' | 'decodedBodySize'>,
660688
dataKey: 'http.response_transfer_size' | 'http.response_content_length' | 'http.decoded_response_content_length',
661689
): void {
662690
const entryVal = entry[key];

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

+29-20
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import {
99
spanToJSON,
1010
} from '@sentry/core';
1111
import type { Span } from '@sentry/core';
12-
import type { ResourceEntry } from '../../src/metrics/browserMetrics';
1312
import { _addMeasureSpans, _addResourceSpans } from '../../src/metrics/browserMetrics';
1413
import { WINDOW } from '../../src/types';
1514
import { TestClient, getDefaultClientOptions } from '../utils/TestClient';
@@ -31,6 +30,17 @@ const originalLocation = WINDOW.location;
3130

3231
const resourceEntryName = 'https://example.com/assets/to/css';
3332

33+
interface AdditionalPerformanceResourceTiming {
34+
renderBlockingStatus?: 'non-blocking' | 'blocking' | '';
35+
deliveryType?: 'cache' | 'navigational-prefetch' | '';
36+
}
37+
38+
function mockPerformanceResourceTiming(
39+
data: Partial<PerformanceResourceTiming> & AdditionalPerformanceResourceTiming,
40+
): PerformanceResourceTiming & AdditionalPerformanceResourceTiming {
41+
return data as PerformanceResourceTiming & AdditionalPerformanceResourceTiming;
42+
}
43+
3444
describe('_addMeasureSpans', () => {
3545
const span = new SentrySpan({ op: 'pageload', name: '/', sampled: true });
3646

@@ -54,13 +64,12 @@ describe('_addMeasureSpans', () => {
5464
spans.push(span);
5565
});
5666

57-
const entry: Omit<PerformanceMeasure, 'toJSON'> = {
67+
const entry = {
5868
entryType: 'measure',
5969
name: 'measure-1',
6070
duration: 10,
6171
startTime: 12,
62-
detail: undefined,
63-
};
72+
} as PerformanceEntry;
6473

6574
const timeOrigin = 100;
6675
const startTime = 23;
@@ -116,13 +125,13 @@ describe('_addResourceSpans', () => {
116125
spans.push(span);
117126
});
118127

119-
const entry: ResourceEntry = {
128+
const entry = mockPerformanceResourceTiming({
120129
initiatorType: 'xmlhttprequest',
121130
transferSize: 256,
122131
encodedBodySize: 256,
123132
decodedBodySize: 256,
124133
renderBlockingStatus: 'non-blocking',
125-
};
134+
});
126135
_addResourceSpans(span, entry, resourceEntryName, 123, 456, 100);
127136

128137
expect(spans).toHaveLength(0);
@@ -135,13 +144,13 @@ describe('_addResourceSpans', () => {
135144
spans.push(span);
136145
});
137146

138-
const entry: ResourceEntry = {
147+
const entry = mockPerformanceResourceTiming({
139148
initiatorType: 'fetch',
140149
transferSize: 256,
141150
encodedBodySize: 256,
142151
decodedBodySize: 256,
143152
renderBlockingStatus: 'non-blocking',
144-
};
153+
});
145154
_addResourceSpans(span, entry, 'https://example.com/assets/to/me', 123, 456, 100);
146155

147156
expect(spans).toHaveLength(0);
@@ -154,13 +163,13 @@ describe('_addResourceSpans', () => {
154163
spans.push(span);
155164
});
156165

157-
const entry: ResourceEntry = {
166+
const entry = mockPerformanceResourceTiming({
158167
initiatorType: 'css',
159168
transferSize: 256,
160169
encodedBodySize: 456,
161170
decodedBodySize: 593,
162171
renderBlockingStatus: 'non-blocking',
163-
};
172+
});
164173

165174
const timeOrigin = 100;
166175
const startTime = 23;
@@ -222,9 +231,9 @@ describe('_addResourceSpans', () => {
222231
];
223232
for (let i = 0; i < table.length; i++) {
224233
const { initiatorType, op } = table[i]!;
225-
const entry: ResourceEntry = {
234+
const entry = mockPerformanceResourceTiming({
226235
initiatorType,
227-
};
236+
});
228237
_addResourceSpans(span, entry, 'https://example.com/assets/to/me', 123, 234, 465);
229238

230239
expect(spans).toHaveLength(i + 1);
@@ -239,13 +248,13 @@ describe('_addResourceSpans', () => {
239248
spans.push(span);
240249
});
241250

242-
const entry: ResourceEntry = {
251+
const entry = mockPerformanceResourceTiming({
243252
initiatorType: 'css',
244253
transferSize: 0,
245254
encodedBodySize: 0,
246255
decodedBodySize: 0,
247256
renderBlockingStatus: 'non-blocking',
248-
};
257+
});
249258

250259
_addResourceSpans(span, entry, resourceEntryName, 100, 23, 345);
251260

@@ -274,12 +283,12 @@ describe('_addResourceSpans', () => {
274283
spans.push(span);
275284
});
276285

277-
const entry: ResourceEntry = {
286+
const entry = mockPerformanceResourceTiming({
278287
initiatorType: 'css',
279288
transferSize: 2147483647,
280289
encodedBodySize: 2147483647,
281290
decodedBodySize: 2147483647,
282-
};
291+
});
283292

284293
_addResourceSpans(span, entry, resourceEntryName, 100, 23, 345);
285294

@@ -316,7 +325,7 @@ describe('_addResourceSpans', () => {
316325
transferSize: null,
317326
encodedBodySize: null,
318327
decodedBodySize: null,
319-
} as unknown as ResourceEntry;
328+
} as unknown as PerformanceResourceTiming;
320329

321330
_addResourceSpans(span, entry, resourceEntryName, 100, 23, 345);
322331

@@ -341,7 +350,7 @@ describe('_addResourceSpans', () => {
341350

342351
// resource delivery types: https://developer.mozilla.org/en-US/docs/Web/API/PerformanceResourceTiming/deliveryType
343352
// i.e. better but not yet widely supported way to check for browser cache hit
344-
it.each(['cache', 'navigational-prefetch', ''])(
353+
it.each(['cache', 'navigational-prefetch', ''] as const)(
345354
'attaches delivery type ("%s") to resource spans if available',
346355
deliveryType => {
347356
const spans: Span[] = [];
@@ -350,13 +359,13 @@ describe('_addResourceSpans', () => {
350359
spans.push(span);
351360
});
352361

353-
const entry: ResourceEntry = {
362+
const entry = mockPerformanceResourceTiming({
354363
initiatorType: 'css',
355364
transferSize: 0,
356365
encodedBodySize: 0,
357366
decodedBodySize: 0,
358367
deliveryType,
359-
};
368+
});
360369

361370
_addResourceSpans(span, entry, resourceEntryName, 100, 23, 345);
362371

0 commit comments

Comments
 (0)