Skip to content

Commit e8fa689

Browse files
Lms24Lms24
committed
rework implementation to use resourceTimingToSpanAttributes
1 parent be78017 commit e8fa689

File tree

9 files changed

+126
-120
lines changed

9 files changed

+126
-120
lines changed

‎dev-packages/browser-integration-tests/suites/tracing/metrics/pageload-resource-spans/test.ts‎

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -74,8 +74,8 @@ sentryTest('adds resource spans to pageload transaction', async ({ getLocalTestU
7474
'http.decoded_response_content_length': expect.any(Number),
7575
'http.response_content_length': expect.any(Number),
7676
'http.response_transfer_size': expect.any(Number),
77-
'http.request.connect_end': expect.any(Number),
7877
'http.request.connect_start': expect.any(Number),
78+
'http.request.connection_end': expect.any(Number),
7979
'http.request.domain_lookup_end': expect.any(Number),
8080
'http.request.domain_lookup_start': expect.any(Number),
8181
'http.request.fetch_start': expect.any(Number),
@@ -84,8 +84,9 @@ sentryTest('adds resource spans to pageload transaction', async ({ getLocalTestU
8484
'http.request.request_start': expect.any(Number),
8585
'http.request.secure_connection_start': expect.any(Number),
8686
'http.request.worker_start': expect.any(Number),
87-
'http.response.end': expect.any(Number),
88-
'http.response.start': expect.any(Number),
87+
'http.request.response_end': expect.any(Number),
88+
'http.request.response_start': expect.any(Number),
89+
'http.request.time_to_first_byte': expect.any(Number),
8990
'network.protocol.name': '',
9091
'network.protocol.version': 'unknown',
9192
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'resource.img',
@@ -95,8 +96,6 @@ sentryTest('adds resource spans to pageload transaction', async ({ getLocalTestU
9596
'url.scheme': 'https',
9697
...(!isWebkitRun && {
9798
'http.response.status_code': expect.any(Number),
98-
'http.response.final_response_headers_start': expect.any(Number),
99-
'http.response.first_interim_response_start': expect.any(Number),
10099
'resource.render_blocking_status': 'non-blocking',
101100
'http.response_delivery_type': '',
102101
}),
@@ -116,8 +115,8 @@ sentryTest('adds resource spans to pageload transaction', async ({ getLocalTestU
116115
'http.decoded_response_content_length': expect.any(Number),
117116
'http.response_content_length': expect.any(Number),
118117
'http.response_transfer_size': expect.any(Number),
119-
'http.request.connect_end': expect.any(Number),
120118
'http.request.connect_start': expect.any(Number),
119+
'http.request.connection_end': expect.any(Number),
121120
'http.request.domain_lookup_end': expect.any(Number),
122121
'http.request.domain_lookup_start': expect.any(Number),
123122
'http.request.fetch_start': expect.any(Number),
@@ -126,8 +125,9 @@ sentryTest('adds resource spans to pageload transaction', async ({ getLocalTestU
126125
'http.request.request_start': expect.any(Number),
127126
'http.request.secure_connection_start': expect.any(Number),
128127
'http.request.worker_start': expect.any(Number),
129-
'http.response.end': expect.any(Number),
130-
'http.response.start': expect.any(Number),
128+
'http.request.response_end': expect.any(Number),
129+
'http.request.response_start': expect.any(Number),
130+
'http.request.time_to_first_byte': expect.any(Number),
131131
'network.protocol.name': '',
132132
'network.protocol.version': 'unknown',
133133
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'resource.link',
@@ -137,8 +137,6 @@ sentryTest('adds resource spans to pageload transaction', async ({ getLocalTestU
137137
'url.scheme': 'https',
138138
...(!isWebkitRun && {
139139
'http.response.status_code': expect.any(Number),
140-
'http.response.final_response_headers_start': expect.any(Number),
141-
'http.response.first_interim_response_start': expect.any(Number),
142140
'resource.render_blocking_status': 'non-blocking',
143141
'http.response_delivery_type': '',
144142
}),
@@ -158,7 +156,7 @@ sentryTest('adds resource spans to pageload transaction', async ({ getLocalTestU
158156
'http.decoded_response_content_length': expect.any(Number),
159157
'http.response_content_length': expect.any(Number),
160158
'http.response_transfer_size': expect.any(Number),
161-
'http.request.connect_end': expect.any(Number),
159+
'http.request.connection_end': expect.any(Number),
162160
'http.request.connect_start': expect.any(Number),
163161
'http.request.domain_lookup_end': expect.any(Number),
164162
'http.request.domain_lookup_start': expect.any(Number),
@@ -168,8 +166,9 @@ sentryTest('adds resource spans to pageload transaction', async ({ getLocalTestU
168166
'http.request.request_start': expect.any(Number),
169167
'http.request.secure_connection_start': expect.any(Number),
170168
'http.request.worker_start': expect.any(Number),
171-
'http.response.end': expect.any(Number),
172-
'http.response.start': expect.any(Number),
169+
'http.request.response_end': expect.any(Number),
170+
'http.request.response_start': expect.any(Number),
171+
'http.request.time_to_first_byte': expect.any(Number),
173172
'network.protocol.name': '',
174173
'network.protocol.version': 'unknown',
175174
'sentry.op': 'resource.script',
@@ -179,8 +178,6 @@ sentryTest('adds resource spans to pageload transaction', async ({ getLocalTestU
179178
'url.scheme': 'https',
180179
...(!isWebkitRun && {
181180
'http.response.status_code': expect.any(Number),
182-
'http.response.final_response_headers_start': expect.any(Number),
183-
'http.response.first_interim_response_start': expect.any(Number),
184181
'resource.render_blocking_status': 'non-blocking',
185182
'http.response_delivery_type': '',
186183
}),

‎packages/browser-utils/src/index.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,4 +30,6 @@ export { addXhrInstrumentationHandler, SENTRY_XHR_DATA_KEY } from './instrument/
3030

3131
export { getBodyString, getFetchRequestArgBody, serializeFormData } from './networkUtils';
3232

33+
export { resourceTimingToSpanAttributes } from './metrics/resourceTiming';
34+
3335
export type { FetchHint, NetworkMetaWarning, XhrHint } from './types';

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

Lines changed: 4 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import {
2222
addTtfbInstrumentationHandler,
2323
} from './instrument';
2424
import { trackLcpAsStandaloneSpan } from './lcp';
25+
import { resourceTimingToSpanAttributes } from './resourceTiming';
2526
import {
2627
extractNetworkProtocol,
2728
getBrowserPerformanceAPI,
@@ -666,47 +667,10 @@ export function _addResourceSpans(
666667

667668
attributes['url.same_origin'] = resourceUrl.includes(WINDOW.location.origin);
668669

669-
// Checking for only `undefined` and `null` is intentional because it's
670-
// valid for `nextHopProtocol` to be an empty string.
671-
if (entry.nextHopProtocol != null) {
672-
const { name, version } = extractNetworkProtocol(entry.nextHopProtocol);
673-
attributes['network.protocol.name'] = name;
674-
attributes['network.protocol.version'] = version;
675-
}
676-
677670
_setResourceRequestAttributes(entry, attributes, [
678671
// https://developer.mozilla.org/en-US/docs/Web/API/PerformanceResourceTiming/responseStatus
679672
['responseStatus', 'http.response.status_code'],
680673

681-
// Timing attributes (request/response lifecycle)
682-
// https://developer.mozilla.org/en-US/docs/Web/API/PerformanceResourceTiming#timestamps
683-
['redirectStart', 'http.request.redirect_start'],
684-
['redirectEnd', 'http.request.redirect_end'],
685-
686-
['workerStart', 'http.request.worker_start'],
687-
688-
['fetchStart', 'http.request.fetch_start'],
689-
690-
['domainLookupStart', 'http.request.domain_lookup_start'],
691-
['domainLookupEnd', 'http.request.domain_lookup_end'],
692-
693-
['connectStart', 'http.request.connect_start'],
694-
['secureConnectionStart', 'http.request.secure_connection_start'],
695-
['connectEnd', 'http.request.connect_end'],
696-
697-
['requestStart', 'http.request.request_start'],
698-
699-
['firstInterimResponseStart', 'http.response.first_interim_response_start'],
700-
['finalResponseHeadersStart', 'http.response.final_response_headers_start'],
701-
702-
// ResponseStart can also be interpreted as TTFB for resource requests: https://web.dev/articles/ttfb
703-
['responseStart', 'http.response.start'],
704-
['responseEnd', 'http.response.end'],
705-
706-
// Size attributes:
707-
// https://developer.mozilla.org/en-US/docs/Web/API/PerformanceResourceTiming/transferSize
708-
// https://developer.mozilla.org/en-US/docs/Web/API/PerformanceResourceTiming/encodedBodySize
709-
// https://developer.mozilla.org/en-US/docs/Web/API/PerformanceResourceTiming/decodedBodySize
710674
['transferSize', 'http.response_transfer_size'],
711675
['encodedBodySize', 'http.response_content_length'],
712676
['decodedBodySize', 'http.decoded_response_content_length'],
@@ -718,13 +682,15 @@ export function _addResourceSpans(
718682
['deliveryType', 'http.response_delivery_type'],
719683
]);
720684

685+
const attributesWithResourceTiming: SpanAttributes = { ...attributes, ...resourceTimingToSpanAttributes(entry) };
686+
721687
const startTimestamp = timeOrigin + startTime;
722688
const endTimestamp = startTimestamp + duration;
723689

724690
startAndEndSpan(span, startTimestamp, endTimestamp, {
725691
name: resourceUrl.replace(WINDOW.location.origin, ''),
726692
op,
727-
attributes,
693+
attributes: attributesWithResourceTiming,
728694
});
729695
}
730696

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

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
import type { SpanAttributes } from '@sentry/core';
2+
import { browserPerformanceTimeOrigin } from '@sentry/core';
3+
import { extractNetworkProtocol, getBrowserPerformanceAPI } from './utils';
4+
5+
function getAbsoluteTime(time = 0): number {
6+
return ((browserPerformanceTimeOrigin() || performance.timeOrigin) + time) / 1000;
7+
}
8+
9+
/**
10+
* Converts a PerformanceResourceTiming entry to span data for the resource span. Most importantly,
11+
* it converts the timing values from timestamps relative to the `performance.timeOrigin` to absolute timestamps
12+
* in seconds.
13+
*
14+
* @see https://developer.mozilla.org/en-US/docs/Web/API/PerformanceResourceTiming#timestamps
15+
*
16+
* @param resourceTiming
17+
* @returns An array where the first element is the attribute name and the second element is the attribute value.
18+
*/
19+
export function resourceTimingToSpanAttributes(resourceTiming: PerformanceResourceTiming): SpanAttributes {
20+
const timingSpanData: SpanAttributes = {};
21+
// Checking for only `undefined` and `null` is intentional because it's
22+
// valid for `nextHopProtocol` to be an empty string.
23+
if (resourceTiming.nextHopProtocol != undefined) {
24+
const { name, version } = extractNetworkProtocol(resourceTiming.nextHopProtocol);
25+
timingSpanData['network.protocol.version'] = version;
26+
timingSpanData['network.protocol.name'] = name;
27+
}
28+
29+
if (!(browserPerformanceTimeOrigin() || getBrowserPerformanceAPI()?.timeOrigin)) {
30+
return timingSpanData;
31+
}
32+
33+
return {
34+
...timingSpanData,
35+
36+
'http.request.redirect_start': getAbsoluteTime(resourceTiming.redirectStart),
37+
'http.request.redirect_end': getAbsoluteTime(resourceTiming.redirectEnd),
38+
39+
'http.request.worker_start': getAbsoluteTime(resourceTiming.workerStart),
40+
41+
'http.request.fetch_start': getAbsoluteTime(resourceTiming.fetchStart),
42+
43+
'http.request.domain_lookup_start': getAbsoluteTime(resourceTiming.domainLookupStart),
44+
'http.request.domain_lookup_end': getAbsoluteTime(resourceTiming.domainLookupEnd),
45+
46+
'http.request.connect_start': getAbsoluteTime(resourceTiming.connectStart),
47+
'http.request.secure_connection_start': getAbsoluteTime(resourceTiming.secureConnectionStart),
48+
'http.request.connection_end': getAbsoluteTime(resourceTiming.connectEnd),
49+
50+
'http.request.request_start': getAbsoluteTime(resourceTiming.requestStart),
51+
52+
'http.request.response_start': getAbsoluteTime(resourceTiming.responseStart),
53+
'http.request.response_end': getAbsoluteTime(resourceTiming.responseEnd),
54+
55+
// For TTFB we actually want the relative time from timeOrigin to responseStart
56+
// This way, TTFB always measures the "first page load" experience.
57+
// see: https://web.dev/articles/ttfb#measure-resource-requests
58+
'http.request.time_to_first_byte': (resourceTiming.responseStart ?? 0) / 1000,
59+
};
60+
}

‎packages/browser-utils/test/instrument/metrics/elementTiming.test.ts‎ renamed to ‎packages/browser-utils/test/metrics/elementTiming.test.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
import * as sentryCore from '@sentry/core';
22
import { beforeEach, describe, expect, it, vi } from 'vitest';
3-
import { _onElementTiming, startTrackingElementTiming } from '../../../src/metrics/elementTiming';
4-
import * as browserMetricsInstrumentation from '../../../src/metrics/instrument';
5-
import * as browserMetricsUtils from '../../../src/metrics/utils';
3+
import { _onElementTiming, startTrackingElementTiming } from '../../src/metrics/elementTiming';
4+
import * as browserMetricsInstrumentation from '../../src/metrics/instrument';
5+
import * as browserMetricsUtils from '../../src/metrics/utils';
66

77
describe('_onElementTiming', () => {
88
const spanEndSpy = vi.fn();

‎packages/browser-utils/test/instrument/metrics/inpt.test.ts‎ renamed to ‎packages/browser-utils/test/metrics/inpt.test.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
import { afterEach } from 'node:test';
22
import { describe, expect, it, vi } from 'vitest';
3-
import { _onInp, _trackINP } from '../../../src/metrics/inp';
4-
import * as instrument from '../../../src/metrics/instrument';
5-
import * as utils from '../../../src/metrics/utils';
3+
import { _onInp, _trackINP } from '../../src/metrics/inp';
4+
import * as instrument from '../../src/metrics/instrument';
5+
import * as utils from '../../src/metrics/utils';
66

77
describe('_trackINP', () => {
88
const addInpInstrumentationHandler = vi.spyOn(instrument, 'addInpInstrumentationHandler');

0 commit comments

Comments
 (0)