Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import * as Sentry from '@sentry/node';
import { loggingTransport } from '@sentry-internal/node-integration-tests';

Sentry.init({
dsn: 'https://public@dsn.ingest.sentry.io/1337',
release: '1.0',
tracesSampleRate: 1.0,
transport: loggingTransport,
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import Fastify from 'fastify';
import http2 from 'node:http2';

async function run() {
const app = Fastify({ http2: true });

app.get('/users/:id', async () => {
return { ok: true };
});

await app.listen({ port: 0 });

const client = http2.connect(`http://localhost:${app.server.address().port}`);
await new Promise((resolve, reject) => {
const stream = client.request({ ':path': '/users/42' });
stream.on('error', reject);
stream.on('data', () => {});
stream.on('end', resolve);
stream.end();
});

client.close();
await app.close();
}

run();
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
import * as Sentry from '@sentry/node';
import http2 from 'node:http2';

const server = http2.createServer((request, response) => {
if (request.url === '/error') {
Sentry.captureException(new Error('error in http2 handler'));
response.statusCode = 500;
response.end();
return;
}

Sentry.startSpan({ name: 'handler-work' }, () => {});
response.end('ok');
});

function request(client, headers) {
return new Promise((resolve, reject) => {
const stream = client.request(headers);
stream.on('error', reject);
stream.on('data', () => {});
stream.on('end', resolve);
stream.end();
});
}

server.listen(0, async () => {
const client = http2.connect(`http://localhost:${server.address().port}`);

await request(client, {
':path': '/users/42?q=1',
'sentry-trace': '12312012123120121231201212312012-1121201211212012-1',
baggage: 'sentry-release=2.1.0,sentry-environment=myEnv',
});
await request(client, {
':path': '/error',
'sentry-trace': '45645645645645645645645645645645-4564564564564564-1',
});

client.close();
server.close();
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
import {
HTTP_RESPONSE_STATUS_CODE,
NETWORK_PROTOCOL_VERSION,
SENTRY_OP,
SENTRY_ORIGIN,
SERVER_ADDRESS,
URL_PATH,
URL_QUERY,
} from '@sentry/conventions/attributes';
import { HTTP_SERVER } from '@sentry/conventions/op';
import { afterAll, describe, expect } from 'vitest';
import { supports } from '../../../utils';
import { cleanupChildProcesses, createEsmAndCjsTests } from '../../../utils/runner';

describe.runIf(supports({ min: 22, runtimes: ['node'] }))('http2 server', () => {
afterAll(() => {
cleanupChildProcesses();
});

createEsmAndCjsTests(__dirname, 'scenario.mjs', 'instrument.mjs', (createRunner, test) => {
test('isolates compat API requests and continues their traces', async () => {
await createRunner()
.unordered()
.expect({
span: container => {
const segment = container.items.find(item => item.is_segment);
expect(segment?.attributes[URL_PATH]?.value).toBe('/users/42');

expect(segment).toMatchObject({
name: 'GET',
trace_id: '12312012123120121231201212312012',
parent_span_id: '1121201211212012',
status: 'ok',
});
expect(segment?.attributes).toMatchObject({
[SENTRY_OP]: { value: HTTP_SERVER },
[SENTRY_ORIGIN]: { value: 'auto.http.http_server' },
[URL_QUERY]: { value: 'q=1' },
[SERVER_ADDRESS]: { value: 'localhost' },
[NETWORK_PROTOCOL_VERSION]: { value: '2.0' },
[HTTP_RESPONSE_STATUS_CODE]: { value: 200 },
});
expect(Object.keys(segment?.attributes ?? {}).filter(key => key.includes(':'))).toEqual([]);

const child = container.items.find(item => item.name === 'handler-work');
expect(child?.parent_span_id).toBe(segment?.span_id);
},
})
.expect({
event: event => {
expect(event.exception?.values?.[0]?.value).toBe('error in http2 handler');
expect(event.request?.url).toMatch(/^http:\/\/localhost:\d+\/error$/);
expect(event.request?.method).toBe('GET');
expect(Object.keys(event.request?.headers ?? {}).filter(key => key.startsWith(':'))).toEqual([]);
expect(event.transaction).toBe('GET /error');
expect(event.contexts?.trace?.trace_id).toBe('45645645645645645645645645645645');
},
})
.expect({
span: container => {
const segment = container.items.find(item => item.is_segment);
expect(segment?.attributes[URL_PATH]?.value).toBe('/error');
expect(segment?.status).toBe('error');
expect(segment?.attributes[HTTP_RESPONSE_STATUS_CODE]?.value).toBe(500);
expect(segment?.trace_id).toBe('45645645645645645645645645645645');
},
Comment thread
cursor[bot] marked this conversation as resolved.
Resolved
})
.start()
.completed();
});

test('does not trigger HTTP/2 compat API warnings', async () => {
await createRunner().ensureNoErrorOutput().start().completed();
});
});

createEsmAndCjsTests(__dirname, 'scenario-fastify.mjs', 'instrument.mjs', (createRunner, test) => {
test('nests fastify spans under the HTTP/2 server span', async () => {
await createRunner()
.expect({
span: container => {
const segment = container.items.find(item => item.is_segment);
expect(segment?.name).toBe('GET /users/:id');
expect(segment?.attributes).toMatchObject({
[SENTRY_OP]: { value: HTTP_SERVER },
[NETWORK_PROTOCOL_VERSION]: { value: '2.0' },
[HTTP_RESPONSE_STATUS_CODE]: { value: 200 },
});

const fastifySpans = container.items.filter(
item => item.attributes[SENTRY_ORIGIN]?.value === 'auto.http.fastify',
);
Comment thread
cursor[bot] marked this conversation as resolved.
Resolved
expect(fastifySpans.length).toBeGreaterThan(0);
for (const span of fastifySpans) {
expect(span.trace_id).toBe(segment?.trace_id);
}
},
})
.start()
.completed();
});
});
});
1 change: 1 addition & 0 deletions packages/core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ export {
httpRequestToRequestData,
extractQueryParamsFromUrl,
headersToDict,
getHostFromHeaders,
httpHeadersToSpanAttributes,
getContentLengthFromHeaders,
getMaxBodyByteLength,
Expand Down
3 changes: 2 additions & 1 deletion packages/core/src/integrations/http/constants.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
export const LOG_PREFIX = '@sentry/instrumentation-http';
export const HTTP_ON_CLIENT_REQUEST = 'http.client.request.created';
export const HTTP_ON_SERVER_REQUEST = 'http.server.request.start';
export const HTTP2_ON_SERVER_STREAM = 'http2.server.stream.start';
export type ClientSubscriptionName = typeof HTTP_ON_CLIENT_REQUEST;
export type ServerSubscriptionName = typeof HTTP_ON_SERVER_REQUEST;
export type ServerSubscriptionName = typeof HTTP_ON_SERVER_REQUEST | typeof HTTP2_ON_SERVER_STREAM;
21 changes: 17 additions & 4 deletions packages/core/src/integrations/http/server-subscription.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@

import type { ServerSubscriptionName } from './constants';
type ChannelListener = (message: unknown, name: string | symbol) => void;
import { HTTP_ON_SERVER_REQUEST } from './constants';
import { HTTP2_ON_SERVER_STREAM, HTTP_ON_SERVER_REQUEST } from './constants';
import type { HttpIncomingMessage, HttpInstrumentationOptions, HttpServer, HttpServerResponse } from './types';
import { DEBUG_BUILD } from '../../debug-build';
import { debug } from '../../utils/debug-logger';
Expand All @@ -30,6 +30,7 @@ import { getClient, getCurrentScope, getIsolationScope, withIsolationScope } fro
import { hasSpansEnabled } from '../../utils/hasSpansEnabled';
import {
getContentLengthFromHeaders,
getHostFromHeaders,
headersToDict,
httpHeadersToSpanAttributes,
httpRequestToRequestData,
Expand Down Expand Up @@ -280,7 +281,17 @@ export function getHttpServerSubscriptions(options: HttpInstrumentationOptions):
instrumentServer(effectiveOptions, server);
};

return { [HTTP_ON_SERVER_REQUEST]: onHttpServerRequest };
// HTTP/2 servers emit the compat API's `'request'` event from inside their
// `'stream'` listener, so the same emit wrapper covers them. Servers that
// use only the raw `'stream'` API (e.g. gRPC) are not instrumented.
const onHttp2ServerStream: ChannelListener = (data: unknown): void => {
const server = (data as { stream?: { session?: { server?: HttpServer } } }).stream?.session?.server;
if (server) {
instrumentServer(effectiveOptions, server);
}
};

return { [HTTP_ON_SERVER_REQUEST]: onHttpServerRequest, [HTTP2_ON_SERVER_STREAM]: onHttp2ServerStream };
}

function buildServerSpanWrap(
Expand Down Expand Up @@ -335,7 +346,7 @@ function buildServerSpanWrap(
const headers = request.headers;
const userAgent = headers['user-agent'];
const httpVersion = request.httpVersion;
const host = headers.host as undefined | string;
const host = getHostFromHeaders(headers);
const hostname = host?.replace(/^(.*)(:[0-9]{1,5})/, '$1') || 'localhost';
const scheme = fullUrl.startsWith('https') ? 'https' : 'http';
const { socket } = request;
Expand Down Expand Up @@ -398,7 +409,9 @@ function buildServerSpanWrap(
isEnded = true;
// set attributes that come from the response
span.setAttributes({
[HTTP_RESPONSE_STATUS_TEXT]: response.statusMessage?.toUpperCase(),
// HTTP/2 has no status text, and reading it emits a process warning.
[HTTP_RESPONSE_STATUS_TEXT]:
request.httpVersion === '2.0' ? undefined : response.statusMessage?.toUpperCase(),
[HTTP_RESPONSE_STATUS_CODE]: response.statusCode,
...httpHeadersToSpanAttributes(headersToDict(response.headers), dataCollectionOptions, 'response'),
});
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ export {
getRequestUrlFromClientRequest,
getRequestOptions,
} from './integrations/http/get-request-url';
export { HTTP_ON_CLIENT_REQUEST, HTTP_ON_SERVER_REQUEST } from './integrations/http/constants';
export { HTTP_ON_CLIENT_REQUEST, HTTP_ON_SERVER_REQUEST, HTTP2_ON_SERVER_STREAM } from './integrations/http/constants';
export type {
HttpInstrumentationOptions,
HttpClientRequest,
Expand Down
14 changes: 13 additions & 1 deletion packages/core/src/utils/request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,10 @@ export function headersToDict(

try {
Object.entries(reqHeaders).forEach(([key, value]) => {
// HTTP/2 pseudo-headers (`:method`, `:path`, ...) are not headers.
if (key.startsWith(':')) {
return;
}
if (typeof value === 'string') {
headers[key] = value;
} else if (typeof value === 'number') {
Expand All @@ -87,6 +91,14 @@ export function headersToDict(
return headers;
}

/**
* The `host` header, or the HTTP/2 `:authority` pseudo-header that replaces it.
*/
export function getHostFromHeaders(headers: Record<string, string | string[] | undefined>): string | undefined {
const host = headers.host ?? headers[':authority'];
return typeof host === 'string' ? host : undefined;
}

/**
* Converts a `Request` object that implements the `Web Fetch API` (https://developer.mozilla.org/en-US/docs/Web/API/Headers) into the format that the `RequestData` integration understands.
*/
Expand Down Expand Up @@ -213,7 +225,7 @@ export function httpRequestToRequestData(request: {

// Check for x-forwarded-host first, then fall back to host header
const forwardedHost = typeof headers['x-forwarded-host'] === 'string' ? headers['x-forwarded-host'] : undefined;
const host = forwardedHost || (typeof headers.host === 'string' ? headers.host : undefined);
const host = forwardedHost || getHostFromHeaders(headers);

// Check for x-forwarded-proto first, then fall back to existing protocol detection
const forwardedProto = typeof headers['x-forwarded-proto'] === 'string' ? headers['x-forwarded-proto'] : undefined;
Expand Down
5 changes: 5 additions & 0 deletions packages/core/test/lib/integrations/http/constants.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { describe, expect, it } from 'vitest';
import {
HTTP2_ON_SERVER_STREAM,
HTTP_ON_CLIENT_REQUEST,
HTTP_ON_SERVER_REQUEST,
LOG_PREFIX,
Expand All @@ -17,4 +18,8 @@ describe('http constants', () => {
it('HTTP_ON_SERVER_REQUEST is the diagnostics-channel name', () => {
expect(HTTP_ON_SERVER_REQUEST).toBe('http.server.request.start');
});

it('HTTP2_ON_SERVER_STREAM is the diagnostics-channel name', () => {
expect(HTTP2_ON_SERVER_STREAM).toBe('http2.server.stream.start');
});
});
Original file line number Diff line number Diff line change
@@ -1,28 +1,33 @@
import {
CLIENT_ADDRESS,
CLIENT_PORT,
HTTP_RESPONSE_STATUS_CODE,
HTTP_RESPONSE_STATUS_TEXT,
NETWORK_LOCAL_ADDRESS,
NETWORK_LOCAL_PORT,
NETWORK_PEER_ADDRESS,
NETWORK_PEER_PORT,
NETWORK_PROTOCOL_NAME,
NETWORK_PROTOCOL_VERSION,
NETWORK_TRANSPORT,
SENTRY_OP,
SERVER_ADDRESS,
SERVER_PORT,
URL_FULL,
URL_PATH,
URL_QUERY,
} from '@sentry/conventions/attributes';
import { HTTP_SERVER } from '@sentry/conventions/op';
import * as http from 'node:http';
import * as http2 from 'node:http2';
import type { AddressInfo } from 'node:net';
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';

import { getIsolationScope } from '../../../../src/currentScopes';
import { Scope } from '../../../../src/scope';
import { spanToJSON } from '../../../../src/utils/spanUtils';
import { setCurrentClient } from '../../../../src/sdk';
import { HTTP_ON_SERVER_REQUEST } from '../../../../src/integrations/http/constants';
import { HTTP2_ON_SERVER_STREAM, HTTP_ON_SERVER_REQUEST } from '../../../../src/integrations/http/constants';
import { getHttpServerSubscriptions } from '../../../../src/integrations/http/server-subscription';
import type { HttpInstrumentationOptions } from '../../../../src/integrations/http/types';
import type { Event } from '../../../../src/types/event';
Expand Down Expand Up @@ -147,6 +152,42 @@ describe('getHttpServerSubscriptions', () => {
);
});

it('creates a root http.server span for an HTTP/2 compat API request', async () => {
const http2Server = http2.createServer((_req, res) => res.end('ok'));
server = http2Server as unknown as http.Server;
await new Promise<void>(resolve => http2Server.listen(0, '127.0.0.1', () => resolve()));

const { [HTTP2_ON_SERVER_STREAM]: onServerStream } = getHttpServerSubscriptions({ spans: true });
// The listener must run before the compat API's own 'stream' listener, like the channel does.
http2Server.prependListener('stream', stream => onServerStream({ stream }, HTTP2_ON_SERVER_STREAM));

const { port } = http2Server.address() as AddressInfo;
const session = http2.connect(`http://127.0.0.1:${port}`);
await new Promise<void>((resolve, reject) => {
const stream = session.request({ ':path': '/users/42?foo=bar' });
stream.on('error', reject);
stream.resume();
stream.on('end', resolve);
stream.end();
});
session.close();

const transaction = await waitForTransaction();
const data = transaction.contexts?.trace?.data;
expect(transaction.transaction).toBe('GET /users/42');
expect(data).toEqual(
expect.objectContaining({
[SENTRY_OP]: HTTP_SERVER,
[HTTP_RESPONSE_STATUS_CODE]: 200,
[URL_FULL]: `http://127.0.0.1:${port}/users/42?foo=bar`,
[SERVER_ADDRESS]: '127.0.0.1',
[NETWORK_PROTOCOL_VERSION]: '2.0',
}),
);
expect(data).not.toHaveProperty(HTTP_RESPONSE_STATUS_TEXT);
expect(Object.keys(data ?? {}).filter(key => key.includes(':'))).toEqual([]);
});

it('prefers the forwarded client over the socket for `client.address`', async () => {
server = http.createServer((_req, res) => res.end('ok'));
await new Promise<void>(resolve => server.listen(0, '127.0.0.1', () => resolve()));
Expand Down
Loading
Loading