Skip to content

Commit 590a5ed

Browse files
committed
refactor unit tests, strengthen integration test
1 parent 41eeee6 commit 590a5ed

File tree

3 files changed

+149
-60
lines changed
  • dev-packages/browser-integration-tests/suites/tracing/metrics/pageload-resource-spans
  • packages/browser-utils

3 files changed

+149
-60
lines changed
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import type { Route } from '@playwright/test';
22
import { expect } from '@playwright/test';
3-
import type { Event } from '@sentry/core';
3+
import { SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, type Event } from '@sentry/core';
44

55
import { sentryTest } from '../../../../utils/fixtures';
66
import { getFirstSentryEnvelopeRequest, shouldSkipTracingTest } from '../../../../utils/helpers';
@@ -12,13 +12,31 @@ sentryTest('should add resource spans to pageload transaction', async ({ getLoca
1212

1313
// Intercepting asset requests to avoid network-related flakiness and random retries (on Firefox).
1414
await page.route('https://example.com/path/to/image.svg', (route: Route) =>
15-
route.fulfill({ path: `${__dirname}/assets/image.svg` }),
15+
route.fulfill({
16+
path: `${__dirname}/assets/image.svg`,
17+
headers: {
18+
'Timing-Allow-Origin': '*',
19+
'Content-Type': 'image/svg+xml',
20+
},
21+
}),
1622
);
1723
await page.route('https://example.com/path/to/script.js', (route: Route) =>
18-
route.fulfill({ path: `${__dirname}/assets/script.js` }),
24+
route.fulfill({
25+
path: `${__dirname}/assets/script.js`,
26+
headers: {
27+
'Timing-Allow-Origin': '*',
28+
'Content-Type': 'application/javascript',
29+
},
30+
}),
1931
);
2032
await page.route('https://example.com/path/to/style.css', (route: Route) =>
21-
route.fulfill({ path: `${__dirname}/assets/style.css` }),
33+
route.fulfill({
34+
path: `${__dirname}/assets/style.css`,
35+
headers: {
36+
'Timing-Allow-Origin': '*',
37+
'Content-Type': 'text/css',
38+
},
39+
}),
2240
);
2341

2442
const url = await getLocalTestUrl({ testDir: __dirname });
@@ -27,11 +45,14 @@ sentryTest('should add resource spans to pageload transaction', async ({ getLoca
2745
const resourceSpans = eventData.spans?.filter(({ op }) => op?.startsWith('resource'));
2846

2947
const scriptSpans = resourceSpans?.filter(({ op }) => op === 'resource.script');
30-
const linkSpans = resourceSpans?.filter(({ op }) => op === 'resource.link');
31-
const imgSpans = resourceSpans?.filter(({ op }) => op === 'resource.img');
48+
const linkSpan = resourceSpans?.filter(({ op }) => op === 'resource.link')[0];
49+
const imgSpan = resourceSpans?.filter(({ op }) => op === 'resource.img')[0];
3250

33-
expect(imgSpans).toHaveLength(1);
34-
expect(linkSpans).toHaveLength(1);
51+
const spanId = eventData.contexts?.trace?.span_id;
52+
const traceId = eventData.contexts?.trace?.trace_id;
53+
54+
expect(spanId).toBeDefined();
55+
expect(traceId).toBeDefined();
3556

3657
const hasCdnBundle = (process.env.PW_BUNDLE || '').startsWith('bundle');
3758

@@ -41,11 +62,84 @@ sentryTest('should add resource spans to pageload transaction', async ({ getLoca
4162
}
4263

4364
expect(scriptSpans?.map(({ description }) => description).sort()).toEqual(expectedScripts);
65+
expect(scriptSpans?.map(({ parent_span_id }) => parent_span_id)).toEqual(expectedScripts.map(() => spanId));
4466

45-
const spanId = eventData.contexts?.trace?.span_id;
67+
const customScriptSpan = scriptSpans?.find(
68+
({ description }) => description === 'https://example.com/path/to/script.js',
69+
);
4670

47-
expect(spanId).toBeDefined();
48-
expect(imgSpans?.[0].parent_span_id).toBe(spanId);
49-
expect(linkSpans?.[0].parent_span_id).toBe(spanId);
50-
expect(scriptSpans?.map(({ parent_span_id }) => parent_span_id)).toEqual(expectedScripts.map(() => spanId));
71+
expect(imgSpan).toEqual({
72+
data: {
73+
'http.decoded_response_content_length': expect.any(Number),
74+
'http.response_content_length': expect.any(Number),
75+
'http.response_delivery_type': '',
76+
'http.response_transfer_size': expect.any(Number),
77+
'network.protocol.name': 'unknown',
78+
'network.protocol.version': 'unknown',
79+
'resource.render_blocking_status': 'non-blocking',
80+
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'resource.img',
81+
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.resource.browser.metrics',
82+
'server.address': 'example.com',
83+
'url.same_origin': false,
84+
'url.scheme': 'https',
85+
},
86+
description: 'https://example.com/path/to/image.svg',
87+
op: 'resource.img',
88+
origin: 'auto.resource.browser.metrics',
89+
parent_span_id: spanId,
90+
span_id: expect.stringMatching(/^[a-f0-9]{16}$/),
91+
start_timestamp: expect.any(Number),
92+
timestamp: expect.any(Number),
93+
trace_id: traceId,
94+
});
95+
96+
expect(linkSpan).toEqual({
97+
data: {
98+
'http.decoded_response_content_length': expect.any(Number),
99+
'http.response_content_length': expect.any(Number),
100+
'http.response_delivery_type': '',
101+
'http.response_transfer_size': expect.any(Number),
102+
'network.protocol.name': 'unknown',
103+
'network.protocol.version': 'unknown',
104+
'resource.render_blocking_status': 'non-blocking',
105+
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'resource.link',
106+
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.resource.browser.metrics',
107+
'server.address': 'example.com',
108+
'url.same_origin': false,
109+
'url.scheme': 'https',
110+
},
111+
description: 'https://example.com/path/to/style.css',
112+
op: 'resource.link',
113+
origin: 'auto.resource.browser.metrics',
114+
parent_span_id: spanId,
115+
span_id: expect.stringMatching(/^[a-f0-9]{16}$/),
116+
start_timestamp: expect.any(Number),
117+
timestamp: expect.any(Number),
118+
trace_id: traceId,
119+
});
120+
121+
expect(customScriptSpan).toEqual({
122+
data: {
123+
'http.decoded_response_content_length': expect.any(Number),
124+
'http.response_content_length': expect.any(Number),
125+
'http.response_delivery_type': '',
126+
'http.response_transfer_size': expect.any(Number),
127+
'network.protocol.name': 'unknown',
128+
'network.protocol.version': 'unknown',
129+
'resource.render_blocking_status': 'non-blocking',
130+
'sentry.op': 'resource.script',
131+
'sentry.origin': 'auto.resource.browser.metrics',
132+
'server.address': 'example.com',
133+
'url.same_origin': false,
134+
'url.scheme': 'https',
135+
},
136+
description: 'https://example.com/path/to/script.js',
137+
op: 'resource.script',
138+
origin: 'auto.resource.browser.metrics',
139+
parent_span_id: spanId,
140+
span_id: expect.stringMatching(/^[a-f0-9]{16}$/),
141+
start_timestamp: expect.any(Number),
142+
timestamp: expect.any(Number),
143+
trace_id: traceId,
144+
});
51145
});

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

+5
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,11 @@ export function msToSec(time: number): number {
147147
export function extractNetworkProtocol(nextHopProtocol: string): { name: string; version: string } {
148148
let name = 'unknown';
149149
let version = 'unknown';
150+
151+
if (!nextHopProtocol) {
152+
return { name, version };
153+
}
154+
150155
let _name = '';
151156
for (const char of nextHopProtocol) {
152157
// http/1.1 etc.

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

+37-47
Original file line numberDiff line numberDiff line change
@@ -55,53 +55,43 @@ describe('startAndEndSpan()', () => {
5555
});
5656
});
5757

58-
interface ProtocolInfo {
59-
name: string;
60-
version: string;
61-
}
62-
6358
describe('HTTPTimings', () => {
64-
test('Extracting version from ALPN protocol', () => {
65-
const nextHopToNetworkVersion: Record<string, ProtocolInfo> = {
66-
'http/0.9': { name: 'http', version: '0.9' },
67-
'http/1.0': { name: 'http', version: '1.0' },
68-
'http/1.1': { name: 'http', version: '1.1' },
69-
'spdy/1': { name: 'spdy', version: '1' },
70-
'spdy/2': { name: 'spdy', version: '2' },
71-
'spdy/3': { name: 'spdy', version: '3' },
72-
'stun.turn': { name: 'stun.turn', version: 'unknown' },
73-
'stun.nat-discovery': { name: 'stun.nat-discovery', version: 'unknown' },
74-
h2: { name: 'http', version: '2' },
75-
h2c: { name: 'http', version: '2c' },
76-
webrtc: { name: 'webrtc', version: 'unknown' },
77-
'c-webrtc': { name: 'c-webrtc', version: 'unknown' },
78-
ftp: { name: 'ftp', version: 'unknown' },
79-
imap: { name: 'imap', version: 'unknown' },
80-
pop3: { name: 'pop', version: '3' },
81-
managesieve: { name: 'managesieve', version: 'unknown' },
82-
coap: { name: 'coap', version: 'unknown' },
83-
'xmpp-client': { name: 'xmpp-client', version: 'unknown' },
84-
'xmpp-server': { name: 'xmpp-server', version: 'unknown' },
85-
'acme-tls/1': { name: 'acme-tls', version: '1' },
86-
mqtt: { name: 'mqtt', version: 'unknown' },
87-
dot: { name: 'dot', version: 'unknown' },
88-
'ntske/1': { name: 'ntske', version: '1' },
89-
sunrpc: { name: 'sunrpc', version: 'unknown' },
90-
h3: { name: 'http', version: '3' },
91-
smb: { name: 'smb', version: 'unknown' },
92-
irc: { name: 'irc', version: 'unknown' },
93-
nntp: { name: 'nntp', version: 'unknown' },
94-
nnsp: { name: 'nnsp', version: 'unknown' },
95-
doq: { name: 'doq', version: 'unknown' },
96-
'sip/2': { name: 'sip', version: '2' },
97-
'tds/8.0': { name: 'tds', version: '8.0' },
98-
dicom: { name: 'dicom', version: 'unknown' },
99-
};
100-
101-
const protocols = Object.keys(nextHopToNetworkVersion);
102-
for (const protocol of protocols) {
103-
const expected = nextHopToNetworkVersion[protocol]!;
104-
expect(extractNetworkProtocol(protocol)).toMatchObject(expected);
105-
}
59+
test.each([
60+
['http/0.9', { name: 'http', version: '0.9' }],
61+
['http/1.0', { name: 'http', version: '1.0' }],
62+
['http/1.1', { name: 'http', version: '1.1' }],
63+
['spdy/1', { name: 'spdy', version: '1' }],
64+
['spdy/2', { name: 'spdy', version: '2' }],
65+
['spdy/3', { name: 'spdy', version: '3' }],
66+
['stun.turn', { name: 'stun.turn', version: 'unknown' }],
67+
['stun.nat-discovery', { name: 'stun.nat-discovery', version: 'unknown' }],
68+
['h2', { name: 'http', version: '2' }],
69+
['h2c', { name: 'http', version: '2c' }],
70+
['webrtc', { name: 'webrtc', version: 'unknown' }],
71+
['c-webrtc', { name: 'c-webrtc', version: 'unknown' }],
72+
['ftp', { name: 'ftp', version: 'unknown' }],
73+
['imap', { name: 'imap', version: 'unknown' }],
74+
['pop3', { name: 'pop', version: '3' }],
75+
['managesieve', { name: 'managesieve', version: 'unknown' }],
76+
['coap', { name: 'coap', version: 'unknown' }],
77+
['xmpp-client', { name: 'xmpp-client', version: 'unknown' }],
78+
['xmpp-server', { name: 'xmpp-server', version: 'unknown' }],
79+
['acme-tls/1', { name: 'acme-tls', version: '1' }],
80+
['mqtt', { name: 'mqtt', version: 'unknown' }],
81+
['dot', { name: 'dot', version: 'unknown' }],
82+
['ntske/1', { name: 'ntske', version: '1' }],
83+
['sunrpc', { name: 'sunrpc', version: 'unknown' }],
84+
['h3', { name: 'http', version: '3' }],
85+
['smb', { name: 'smb', version: 'unknown' }],
86+
['irc', { name: 'irc', version: 'unknown' }],
87+
['nntp', { name: 'nntp', version: 'unknown' }],
88+
['nnsp', { name: 'nnsp', version: 'unknown' }],
89+
['doq', { name: 'doq', version: 'unknown' }],
90+
['sip/2', { name: 'sip', version: '2' }],
91+
['tds/8.0', { name: 'tds', version: '8.0' }],
92+
['dicom', { name: 'dicom', version: 'unknown' }],
93+
['', { name: 'unknown', version: 'unknown' }],
94+
])('Extracting version from ALPN protocol %s', (protocol, expected) => {
95+
expect(extractNetworkProtocol(protocol)).toMatchObject(expected);
10696
});
10797
});

0 commit comments

Comments
 (0)