diff --git a/dev-packages/node-integration-tests/suites/tracing/http2-server/instrument.mjs b/dev-packages/node-integration-tests/suites/tracing/http2-server/instrument.mjs new file mode 100644 index 000000000000..46a27dd03b74 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/http2-server/instrument.mjs @@ -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, +}); diff --git a/dev-packages/node-integration-tests/suites/tracing/http2-server/scenario-fastify.mjs b/dev-packages/node-integration-tests/suites/tracing/http2-server/scenario-fastify.mjs new file mode 100644 index 000000000000..d26b38fba250 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/http2-server/scenario-fastify.mjs @@ -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(); diff --git a/dev-packages/node-integration-tests/suites/tracing/http2-server/scenario.mjs b/dev-packages/node-integration-tests/suites/tracing/http2-server/scenario.mjs new file mode 100644 index 000000000000..db90c22c2e5c --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/http2-server/scenario.mjs @@ -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(); +}); diff --git a/dev-packages/node-integration-tests/suites/tracing/http2-server/test.ts b/dev-packages/node-integration-tests/suites/tracing/http2-server/test.ts new file mode 100644 index 000000000000..3559b8378261 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/tracing/http2-server/test.ts @@ -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'); + }, + }) + .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', + ); + expect(fastifySpans.length).toBeGreaterThan(0); + for (const span of fastifySpans) { + expect(span.trace_id).toBe(segment?.trace_id); + } + }, + }) + .start() + .completed(); + }); + }); +}); diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 11fd3f6cad1f..1af72cd73655 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -147,6 +147,7 @@ export { httpRequestToRequestData, extractQueryParamsFromUrl, headersToDict, + getHostFromHeaders, httpHeadersToSpanAttributes, getContentLengthFromHeaders, getMaxBodyByteLength, diff --git a/packages/core/src/integrations/http/constants.ts b/packages/core/src/integrations/http/constants.ts index f2af12b00b62..0fad2962451e 100644 --- a/packages/core/src/integrations/http/constants.ts +++ b/packages/core/src/integrations/http/constants.ts @@ -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; diff --git a/packages/core/src/integrations/http/server-subscription.ts b/packages/core/src/integrations/http/server-subscription.ts index 40298235066b..7acf305b6a5c 100644 --- a/packages/core/src/integrations/http/server-subscription.ts +++ b/packages/core/src/integrations/http/server-subscription.ts @@ -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'; @@ -30,6 +30,7 @@ import { getClient, getCurrentScope, getIsolationScope, withIsolationScope } fro import { hasSpansEnabled } from '../../utils/hasSpansEnabled'; import { getContentLengthFromHeaders, + getHostFromHeaders, headersToDict, httpHeadersToSpanAttributes, httpRequestToRequestData, @@ -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( @@ -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; @@ -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'), }); diff --git a/packages/core/src/server.ts b/packages/core/src/server.ts index 50dfb6639b05..ad4d521f39bc 100644 --- a/packages/core/src/server.ts +++ b/packages/core/src/server.ts @@ -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, diff --git a/packages/core/src/utils/request.ts b/packages/core/src/utils/request.ts index 82c7135fb2ac..9527b97d534e 100644 --- a/packages/core/src/utils/request.ts +++ b/packages/core/src/utils/request.ts @@ -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') { @@ -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 | 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. */ @@ -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; diff --git a/packages/core/test/lib/integrations/http/constants.test.ts b/packages/core/test/lib/integrations/http/constants.test.ts index 6a63e74ba5c6..d666c6167456 100644 --- a/packages/core/test/lib/integrations/http/constants.test.ts +++ b/packages/core/test/lib/integrations/http/constants.test.ts @@ -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, @@ -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'); + }); }); diff --git a/packages/core/test/lib/integrations/http/server-subscription.test.ts b/packages/core/test/lib/integrations/http/server-subscription.test.ts index 04c50f221632..68bd2327e304 100644 --- a/packages/core/test/lib/integrations/http/server-subscription.test.ts +++ b/packages/core/test/lib/integrations/http/server-subscription.test.ts @@ -1,6 +1,8 @@ import { CLIENT_ADDRESS, CLIENT_PORT, + HTTP_RESPONSE_STATUS_CODE, + HTTP_RESPONSE_STATUS_TEXT, NETWORK_LOCAL_ADDRESS, NETWORK_LOCAL_PORT, NETWORK_PEER_ADDRESS, @@ -8,13 +10,16 @@ import { 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'; @@ -22,7 +27,7 @@ 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'; @@ -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(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((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(resolve => server.listen(0, '127.0.0.1', () => resolve())); diff --git a/packages/core/test/lib/utils/request.test.ts b/packages/core/test/lib/utils/request.test.ts index 640e7c118f66..30af4105ea51 100644 --- a/packages/core/test/lib/utils/request.test.ts +++ b/packages/core/test/lib/utils/request.test.ts @@ -64,6 +64,19 @@ describe('request utils', () => { 'x-string': 'hello', }); }); + + it('drops HTTP/2 pseudo-headers', () => { + expect( + headersToDict({ + ':method': 'GET', + ':path': '/users', + ':authority': 'example.com', + 'user-agent': 'test', + }), + ).toEqual({ + 'user-agent': 'test', + }); + }); }); describe('winterCGRequestToRequestData', () => { @@ -143,6 +156,27 @@ describe('request utils', () => { }); }); + it('uses the HTTP/2 :authority pseudo-header as the host', () => { + const actual = httpRequestToRequestData({ + method: 'GET', + url: '/blabla?xx=a', + headers: { + ':method': 'GET', + ':path': '/blabla?xx=a', + ':authority': 'example.com:8443', + ':scheme': 'https', + }, + socket: { encrypted: true }, + }); + + expect(actual).toEqual({ + method: 'GET', + url: 'https://example.com:8443/blabla?xx=a', + query_string: 'xx=a', + headers: {}, + }); + }); + it('works with relative URL request with host', () => { const actual = httpRequestToRequestData({ url: '/blabla', diff --git a/packages/node/src/integrations/http/httpServerIntegration.ts b/packages/node/src/integrations/http/httpServerIntegration.ts index b3c0c939d971..0000e54bc683 100644 --- a/packages/node/src/integrations/http/httpServerIntegration.ts +++ b/packages/node/src/integrations/http/httpServerIntegration.ts @@ -3,7 +3,12 @@ import { context, createContextKey, propagation } from '@opentelemetry/api'; import type { Integration, IntegrationFn } from '@sentry/core'; import { addNonEnumerableProperty, debug, getClient } from '@sentry/core'; import type { HttpIncomingMessage, HttpServerResponse } from '@sentry/core/server'; -import { getHttpServerSubscriptions, HTTP_ON_SERVER_REQUEST, recordRequestSession } from '@sentry/core/server'; +import { + getHttpServerSubscriptions, + HTTP2_ON_SERVER_STREAM, + HTTP_ON_SERVER_REQUEST, + recordRequestSession, +} from '@sentry/core/server'; import { subscribeDiagnosticsChannel } from '@sentry/server-utils'; import type { RequestEventData } from '@sentry/core'; import { DEBUG_BUILD } from '../../debug-build'; @@ -138,8 +143,10 @@ const _httpServerIntegration = ((options: HttpServerIntegrationOptions = {}) => return { name: INTEGRATION_NAME, setupOnce() { - const { [HTTP_ON_SERVER_REQUEST]: onHttpServerRequestStart } = getHttpServerSubscriptions(_options); + const { [HTTP_ON_SERVER_REQUEST]: onHttpServerRequestStart, [HTTP2_ON_SERVER_STREAM]: onHttp2ServerStreamStart } = + getHttpServerSubscriptions(_options); subscribeDiagnosticsChannel(HTTP_ON_SERVER_REQUEST, onHttpServerRequestStart); + subscribeDiagnosticsChannel(HTTP2_ON_SERVER_STREAM, onHttp2ServerStreamStart); }, afterAllSetup(client) { if (DEBUG_BUILD && client.getIntegrationByName('Http')) { @@ -153,7 +160,7 @@ const _httpServerIntegration = ((options: HttpServerIntegrationOptions = {}) => /** * This integration handles request isolation, trace continuation and other core Sentry functionality around incoming http requests - * handled via the node `http` module. + * handled via the node `http` module, or via the compat API of the `http2` module. * * This version uses OpenTelemetry for context propagation and span management. * diff --git a/packages/node/src/integrations/http/httpServerSpansIntegration.ts b/packages/node/src/integrations/http/httpServerSpansIntegration.ts index 4d896a62485d..e83205354efb 100644 --- a/packages/node/src/integrations/http/httpServerSpansIntegration.ts +++ b/packages/node/src/integrations/http/httpServerSpansIntegration.ts @@ -42,6 +42,7 @@ import { getSpanStatusFromHttpCode, httpHeadersToSpanAttributes, getContentLengthFromHeaders, + getHostFromHeaders, parseStringToURLObject, SPAN_STATUS_ERROR, stripUrlQueryAndFragment, @@ -153,7 +154,7 @@ const _httpServerSpansIntegration = ((options: HttpServerSpansIntegrationOptions const headers = request.headers; const userAgent = headers['user-agent']; const httpVersion = request.httpVersion; - const host = headers.host as string | undefined; + const host = getHostFromHeaders(headers); const hostname = host?.replace(/^(.*)(:[0-9]{1,5})/, '$1') || 'localhost'; const scheme = fullUrl.startsWith('https') ? 'https' : 'http'; @@ -360,11 +361,12 @@ function getIncomingRequestAttributesOnResponse( // take socket from the request, // since it may be detached from the response object in keep-alive mode const { socket } = request; - const { statusCode, statusMessage } = response; + const { statusCode } = response; const newAttributes: SpanAttributes = { [HTTP_RESPONSE_STATUS_CODE]: statusCode, - [HTTP_RESPONSE_STATUS_TEXT]: 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(), }; // `client.address` is the originating client, so a forwarding header wins over the socket, which