Skip to content

Commit 6b009c0

Browse files
k-fishAbhiPrasad
andauthored
fix(tracing): Improve network.protocol.version (#8502)
Protocols are from https://www.iana.org/assignments/tls-extensiontype-values/tls-extensiontype-values.xhtml#alpn-protocol-ids which don't exactly match the OSI. We can consider adding a lookup table later if people are finding this insufficient. Co-authored-by: Abhijeet Prasad <[email protected]>
1 parent e6cc124 commit 6b009c0

File tree

3 files changed

+98
-6
lines changed

3 files changed

+98
-6
lines changed

packages/remix/test/integration/test/client/pageload.test.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,12 @@ import { getFirstSentryEnvelopeRequest } from './utils/helpers';
44
import { test, expect } from '@playwright/test';
55
import { Event } from '@sentry/types';
66

7-
test('should add `pageload` transaction on load.', async ({ page }) => {
7+
test('should add `pageload` transaction on load.', async ({ page, browserName }) => {
8+
// This test is flaky on firefox
9+
if (browserName === 'firefox') {
10+
test.skip();
11+
}
12+
813
const envelope = await getFirstSentryEnvelopeRequest<Event>(page, '/');
914

1015
expect(envelope.contexts?.trace.op).toBe('pageload');

packages/tracing-internal/src/browser/request.ts

Lines changed: 34 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -182,17 +182,47 @@ function addHTTPTimings(span: Span): void {
182182
});
183183
}
184184

185+
/**
186+
* Converts ALPN protocol ids to name and version.
187+
*
188+
* (https://www.iana.org/assignments/tls-extensiontype-values/tls-extensiontype-values.xhtml#alpn-protocol-ids)
189+
* @param nextHopProtocol PerformanceResourceTiming.nextHopProtocol
190+
*/
191+
export function extractNetworkProtocol(nextHopProtocol: string): { name: string; version: string } {
192+
let name = 'unknown';
193+
let version = 'unknown';
194+
let _name = '';
195+
for (const char of nextHopProtocol) {
196+
// http/1.1 etc.
197+
if (char === '/') {
198+
[name, version] = nextHopProtocol.split('/');
199+
break;
200+
}
201+
// h2, h3 etc.
202+
if (!isNaN(Number(char))) {
203+
name = _name === 'h' ? 'http' : _name;
204+
version = nextHopProtocol.split(_name)[1];
205+
break;
206+
}
207+
_name += char;
208+
}
209+
if (_name === nextHopProtocol) {
210+
// webrtc, ftp, etc.
211+
name = _name;
212+
}
213+
return { name, version };
214+
}
215+
185216
function getAbsoluteTime(time: number): number {
186217
return ((browserPerformanceTimeOrigin || performance.timeOrigin) + time) / 1000;
187218
}
188219

189220
function resourceTimingEntryToSpanData(resourceTiming: PerformanceResourceTiming): [string, string | number][] {
190-
const version = resourceTiming.nextHopProtocol.split('/')[1] || 'none';
221+
const { name, version } = extractNetworkProtocol(resourceTiming.nextHopProtocol);
191222

192223
const timingSpanData: [string, string | number][] = [];
193-
if (version) {
194-
timingSpanData.push(['network.protocol.version', version]);
195-
}
224+
225+
timingSpanData.push(['network.protocol.version', version], ['network.protocol.name', name]);
196226

197227
if (!browserPerformanceTimeOrigin) {
198228
return timingSpanData;

packages/tracing-internal/test/browser/request.test.ts

Lines changed: 58 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,13 @@ import type { Transaction } from '../../../tracing/src';
77
import { addExtensionMethods, Span, spanStatusfromHttpCode } from '../../../tracing/src';
88
import { getDefaultBrowserClientOptions } from '../../../tracing/test/testutils';
99
import type { FetchData, XHRData } from '../../src/browser/request';
10-
import { fetchCallback, instrumentOutgoingRequests, shouldAttachHeaders, xhrCallback } from '../../src/browser/request';
10+
import {
11+
extractNetworkProtocol,
12+
fetchCallback,
13+
instrumentOutgoingRequests,
14+
shouldAttachHeaders,
15+
xhrCallback,
16+
} from '../../src/browser/request';
1117
import { TestClient } from '../utils/TestClient';
1218

1319
beforeAll(() => {
@@ -388,6 +394,57 @@ describe('callbacks', () => {
388394
});
389395
});
390396

397+
interface ProtocolInfo {
398+
name: string;
399+
version: string;
400+
}
401+
402+
describe('HTTPTimings', () => {
403+
describe('Extracting version from ALPN protocol', () => {
404+
const nextHopToNetworkVersion: Record<string, ProtocolInfo> = {
405+
'http/0.9': { name: 'http', version: '0.9' },
406+
'http/1.0': { name: 'http', version: '1.0' },
407+
'http/1.1': { name: 'http', version: '1.1' },
408+
'spdy/1': { name: 'spdy', version: '1' },
409+
'spdy/2': { name: 'spdy', version: '2' },
410+
'spdy/3': { name: 'spdy', version: '3' },
411+
'stun.turn': { name: 'stun.turn', version: 'unknown' },
412+
'stun.nat-discovery': { name: 'stun.nat-discovery', version: 'unknown' },
413+
h2: { name: 'http', version: '2' },
414+
h2c: { name: 'http', version: '2c' },
415+
webrtc: { name: 'webrtc', version: 'unknown' },
416+
'c-webrtc': { name: 'c-webrtc', version: 'unknown' },
417+
ftp: { name: 'ftp', version: 'unknown' },
418+
imap: { name: 'imap', version: 'unknown' },
419+
pop3: { name: 'pop', version: '3' },
420+
managesieve: { name: 'managesieve', version: 'unknown' },
421+
coap: { name: 'coap', version: 'unknown' },
422+
'xmpp-client': { name: 'xmpp-client', version: 'unknown' },
423+
'xmpp-server': { name: 'xmpp-server', version: 'unknown' },
424+
'acme-tls/1': { name: 'acme-tls', version: '1' },
425+
mqtt: { name: 'mqtt', version: 'unknown' },
426+
dot: { name: 'dot', version: 'unknown' },
427+
'ntske/1': { name: 'ntske', version: '1' },
428+
sunrpc: { name: 'sunrpc', version: 'unknown' },
429+
h3: { name: 'http', version: '3' },
430+
smb: { name: 'smb', version: 'unknown' },
431+
irc: { name: 'irc', version: 'unknown' },
432+
nntp: { name: 'nntp', version: 'unknown' },
433+
nnsp: { name: 'nnsp', version: 'unknown' },
434+
doq: { name: 'doq', version: 'unknown' },
435+
'sip/2': { name: 'sip', version: '2' },
436+
'tds/8.0': { name: 'tds', version: '8.0' },
437+
dicom: { name: 'dicom', version: 'unknown' },
438+
};
439+
440+
const protocols = Object.keys(nextHopToNetworkVersion);
441+
for (const protocol of protocols) {
442+
const expected: ProtocolInfo = nextHopToNetworkVersion[protocol];
443+
expect(extractNetworkProtocol(protocol)).toMatchObject(expected);
444+
}
445+
});
446+
});
447+
391448
describe('shouldAttachHeaders', () => {
392449
describe('should prefer `tracePropagationTargets` over defaults', () => {
393450
it('should return `true` if the url matches the new tracePropagationTargets', () => {

0 commit comments

Comments
 (0)