Skip to content

Commit

Permalink
fix(tracing): Improve network.protocol.version (#8502)
Browse files Browse the repository at this point in the history
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 <aprasad@sentry.io>
  • Loading branch information
k-fish and AbhiPrasad committed Jul 17, 2023
1 parent e6cc124 commit 6b009c0
Show file tree
Hide file tree
Showing 3 changed files with 98 additions and 6 deletions.
7 changes: 6 additions & 1 deletion packages/remix/test/integration/test/client/pageload.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,12 @@ import { getFirstSentryEnvelopeRequest } from './utils/helpers';
import { test, expect } from '@playwright/test';
import { Event } from '@sentry/types';

test('should add `pageload` transaction on load.', async ({ page }) => {
test('should add `pageload` transaction on load.', async ({ page, browserName }) => {
// This test is flaky on firefox
if (browserName === 'firefox') {
test.skip();
}

const envelope = await getFirstSentryEnvelopeRequest<Event>(page, '/');

expect(envelope.contexts?.trace.op).toBe('pageload');
Expand Down
38 changes: 34 additions & 4 deletions packages/tracing-internal/src/browser/request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -182,17 +182,47 @@ function addHTTPTimings(span: Span): void {
});
}

/**
* Converts ALPN protocol ids to name and version.
*
* (https://www.iana.org/assignments/tls-extensiontype-values/tls-extensiontype-values.xhtml#alpn-protocol-ids)
* @param nextHopProtocol PerformanceResourceTiming.nextHopProtocol
*/
export function extractNetworkProtocol(nextHopProtocol: string): { name: string; version: string } {
let name = 'unknown';
let version = 'unknown';
let _name = '';
for (const char of nextHopProtocol) {
// http/1.1 etc.
if (char === '/') {
[name, version] = nextHopProtocol.split('/');
break;
}
// h2, h3 etc.
if (!isNaN(Number(char))) {
name = _name === 'h' ? 'http' : _name;
version = nextHopProtocol.split(_name)[1];
break;
}
_name += char;
}
if (_name === nextHopProtocol) {
// webrtc, ftp, etc.
name = _name;
}
return { name, version };
}

function getAbsoluteTime(time: number): number {
return ((browserPerformanceTimeOrigin || performance.timeOrigin) + time) / 1000;
}

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

const timingSpanData: [string, string | number][] = [];
if (version) {
timingSpanData.push(['network.protocol.version', version]);
}

timingSpanData.push(['network.protocol.version', version], ['network.protocol.name', name]);

if (!browserPerformanceTimeOrigin) {
return timingSpanData;
Expand Down
59 changes: 58 additions & 1 deletion packages/tracing-internal/test/browser/request.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,13 @@ import type { Transaction } from '../../../tracing/src';
import { addExtensionMethods, Span, spanStatusfromHttpCode } from '../../../tracing/src';
import { getDefaultBrowserClientOptions } from '../../../tracing/test/testutils';
import type { FetchData, XHRData } from '../../src/browser/request';
import { fetchCallback, instrumentOutgoingRequests, shouldAttachHeaders, xhrCallback } from '../../src/browser/request';
import {
extractNetworkProtocol,
fetchCallback,
instrumentOutgoingRequests,
shouldAttachHeaders,
xhrCallback,
} from '../../src/browser/request';
import { TestClient } from '../utils/TestClient';

beforeAll(() => {
Expand Down Expand Up @@ -388,6 +394,57 @@ describe('callbacks', () => {
});
});

interface ProtocolInfo {
name: string;
version: string;
}

describe('HTTPTimings', () => {
describe('Extracting version from ALPN protocol', () => {
const nextHopToNetworkVersion: Record<string, ProtocolInfo> = {
'http/0.9': { name: 'http', version: '0.9' },
'http/1.0': { name: 'http', version: '1.0' },
'http/1.1': { name: 'http', version: '1.1' },
'spdy/1': { name: 'spdy', version: '1' },
'spdy/2': { name: 'spdy', version: '2' },
'spdy/3': { name: 'spdy', version: '3' },
'stun.turn': { name: 'stun.turn', version: 'unknown' },
'stun.nat-discovery': { name: 'stun.nat-discovery', version: 'unknown' },
h2: { name: 'http', version: '2' },
h2c: { name: 'http', version: '2c' },
webrtc: { name: 'webrtc', version: 'unknown' },
'c-webrtc': { name: 'c-webrtc', version: 'unknown' },
ftp: { name: 'ftp', version: 'unknown' },
imap: { name: 'imap', version: 'unknown' },
pop3: { name: 'pop', version: '3' },
managesieve: { name: 'managesieve', version: 'unknown' },
coap: { name: 'coap', version: 'unknown' },
'xmpp-client': { name: 'xmpp-client', version: 'unknown' },
'xmpp-server': { name: 'xmpp-server', version: 'unknown' },
'acme-tls/1': { name: 'acme-tls', version: '1' },
mqtt: { name: 'mqtt', version: 'unknown' },
dot: { name: 'dot', version: 'unknown' },
'ntske/1': { name: 'ntske', version: '1' },
sunrpc: { name: 'sunrpc', version: 'unknown' },
h3: { name: 'http', version: '3' },
smb: { name: 'smb', version: 'unknown' },
irc: { name: 'irc', version: 'unknown' },
nntp: { name: 'nntp', version: 'unknown' },
nnsp: { name: 'nnsp', version: 'unknown' },
doq: { name: 'doq', version: 'unknown' },
'sip/2': { name: 'sip', version: '2' },
'tds/8.0': { name: 'tds', version: '8.0' },
dicom: { name: 'dicom', version: 'unknown' },
};

const protocols = Object.keys(nextHopToNetworkVersion);
for (const protocol of protocols) {
const expected: ProtocolInfo = nextHopToNetworkVersion[protocol];
expect(extractNetworkProtocol(protocol)).toMatchObject(expected);
}
});
});

describe('shouldAttachHeaders', () => {
describe('should prefer `tracePropagationTargets` over defaults', () => {
it('should return `true` if the url matches the new tracePropagationTargets', () => {
Expand Down

0 comments on commit 6b009c0

Please sign in to comment.