From d5dff8ce77c25715a76b80adebd2ae0a422972fb Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Tue, 24 Sep 2024 10:41:49 +0200 Subject: [PATCH 01/14] feat(node): Implement Sentry-specific http instrumentation --- .../node-otel-without-tracing/package.json | 1 + .../node-otel-without-tracing/src/app.ts | 9 +- .../src/instrument.ts | 7 +- .../tests/errors.test.ts | 21 ++ .../tests/transactions.test.ts | 9 +- packages/node/src/integrations/http.ts | 312 ------------------ .../http/SentryHttpInstrumentation.ts | 285 ++++++++++++++++ packages/node/src/integrations/http/index.ts | 222 +++++++++++++ packages/opentelemetry/src/index.ts | 2 +- 9 files changed, 543 insertions(+), 325 deletions(-) delete mode 100644 packages/node/src/integrations/http.ts create mode 100644 packages/node/src/integrations/http/SentryHttpInstrumentation.ts create mode 100644 packages/node/src/integrations/http/index.ts diff --git a/dev-packages/e2e-tests/test-applications/node-otel-without-tracing/package.json b/dev-packages/e2e-tests/test-applications/node-otel-without-tracing/package.json index afe666c2a8f1..ed01eff7dce2 100644 --- a/dev-packages/e2e-tests/test-applications/node-otel-without-tracing/package.json +++ b/dev-packages/e2e-tests/test-applications/node-otel-without-tracing/package.json @@ -14,6 +14,7 @@ "@opentelemetry/sdk-trace-node": "1.26.0", "@opentelemetry/exporter-trace-otlp-http": "0.53.0", "@opentelemetry/instrumentation-undici": "0.6.0", + "@opentelemetry/instrumentation-http": "0.53.0", "@opentelemetry/instrumentation": "0.53.0", "@sentry/core": "latest || *", "@sentry/node": "latest || *", diff --git a/dev-packages/e2e-tests/test-applications/node-otel-without-tracing/src/app.ts b/dev-packages/e2e-tests/test-applications/node-otel-without-tracing/src/app.ts index c3fdfb9134a5..383eaf1b4484 100644 --- a/dev-packages/e2e-tests/test-applications/node-otel-without-tracing/src/app.ts +++ b/dev-packages/e2e-tests/test-applications/node-otel-without-tracing/src/app.ts @@ -7,6 +7,8 @@ import express from 'express'; const app = express(); const port = 3030; +Sentry.setTag('root-level-tag', 'yes'); + app.get('/test-success', function (req, res) { res.send({ version: 'v1' }); }); @@ -23,8 +25,6 @@ app.get('/test-transaction', function (req, res) { await fetch('http://localhost:3030/test-success'); - await Sentry.flush(); - res.send({}); }); }); @@ -38,7 +38,10 @@ app.get('/test-error', async function (req, res) { }); app.get('/test-exception/:id', function (req, _res) { - throw new Error(`This is an exception with id ${req.params.id}`); + const id = req.params.id; + Sentry.setTag(`param-${id}`, id); + + throw new Error(`This is an exception with id ${id}`); }); Sentry.setupExpressErrorHandler(app); diff --git a/dev-packages/e2e-tests/test-applications/node-otel-without-tracing/src/instrument.ts b/dev-packages/e2e-tests/test-applications/node-otel-without-tracing/src/instrument.ts index d887696b1e73..52c4403b2164 100644 --- a/dev-packages/e2e-tests/test-applications/node-otel-without-tracing/src/instrument.ts +++ b/dev-packages/e2e-tests/test-applications/node-otel-without-tracing/src/instrument.ts @@ -1,7 +1,8 @@ +const { HttpInstrumentation } = require('@opentelemetry/instrumentation-http'); const { NodeTracerProvider, BatchSpanProcessor } = require('@opentelemetry/sdk-trace-node'); const { OTLPTraceExporter } = require('@opentelemetry/exporter-trace-otlp-http'); const Sentry = require('@sentry/node'); -const { SentrySpanProcessor, SentryPropagator } = require('@sentry/opentelemetry'); +const { SentryPropagator } = require('@sentry/opentelemetry'); const { UndiciInstrumentation } = require('@opentelemetry/instrumentation-undici'); const { registerInstrumentations } = require('@opentelemetry/instrumentation'); @@ -15,6 +16,8 @@ Sentry.init({ tunnel: `http://localhost:3031/`, // proxy server // Tracing is completely disabled + integrations: [Sentry.httpIntegration({ spans: true })], + // Custom OTEL setup skipOpenTelemetrySetup: true, }); @@ -37,5 +40,5 @@ provider.register({ }); registerInstrumentations({ - instrumentations: [new UndiciInstrumentation()], + instrumentations: [new UndiciInstrumentation(), new HttpInstrumentation()], }); diff --git a/dev-packages/e2e-tests/test-applications/node-otel-without-tracing/tests/errors.test.ts b/dev-packages/e2e-tests/test-applications/node-otel-without-tracing/tests/errors.test.ts index 28e63f02090c..1e4526a891a3 100644 --- a/dev-packages/e2e-tests/test-applications/node-otel-without-tracing/tests/errors.test.ts +++ b/dev-packages/e2e-tests/test-applications/node-otel-without-tracing/tests/errors.test.ts @@ -28,3 +28,24 @@ test('Sends correct error event', async ({ baseURL }) => { span_id: expect.any(String), }); }); + +test('Isolates requests correctly', async ({ baseURL }) => { + const errorEventPromise1 = waitForError('node-otel-without-tracing', event => { + return !event.type && event.exception?.values?.[0]?.value === 'This is an exception with id 555-a'; + }); + const errorEventPromise2 = waitForError('node-otel-without-tracing', event => { + return !event.type && event.exception?.values?.[0]?.value === 'This is an exception with id 555-b'; + }); + + fetch(`${baseURL}/test-exception/555-a`); + fetch(`${baseURL}/test-exception/555-b`); + + const errorEvent1 = await errorEventPromise1; + const errorEvent2 = await errorEventPromise2; + + expect(errorEvent1.transaction).toEqual('GET /test-exception/555-a'); + expect(errorEvent1.tags).toEqual({ 'root-level-tag': 'yes', 'param-555-a': '555-a' }); + + expect(errorEvent2.transaction).toEqual('GET /test-exception/555-b'); + expect(errorEvent2.tags).toEqual({ 'root-level-tag': 'yes', 'param-555-b': '555-b' }); +}); diff --git a/dev-packages/e2e-tests/test-applications/node-otel-without-tracing/tests/transactions.test.ts b/dev-packages/e2e-tests/test-applications/node-otel-without-tracing/tests/transactions.test.ts index 9c91a0ed9531..bb069b7e3e11 100644 --- a/dev-packages/e2e-tests/test-applications/node-otel-without-tracing/tests/transactions.test.ts +++ b/dev-packages/e2e-tests/test-applications/node-otel-without-tracing/tests/transactions.test.ts @@ -12,9 +12,7 @@ test('Sends an API route transaction to OTLP', async ({ baseURL }) => { const scopeSpans = json.resourceSpans?.[0]?.scopeSpans; - const httpScope = scopeSpans?.find( - scopeSpan => scopeSpan.scope.name === '@opentelemetry_sentry-patched/instrumentation-http', - ); + const httpScope = scopeSpans?.find(scopeSpan => scopeSpan.scope.name === '@opentelemetry/instrumentation-http'); return ( httpScope && @@ -40,9 +38,7 @@ test('Sends an API route transaction to OTLP', async ({ baseURL }) => { // But our default node-fetch spans are not emitted expect(scopeSpans.length).toEqual(2); - const httpScopes = scopeSpans?.filter( - scopeSpan => scopeSpan.scope.name === '@opentelemetry_sentry-patched/instrumentation-http', - ); + const httpScopes = scopeSpans?.filter(scopeSpan => scopeSpan.scope.name === '@opentelemetry/instrumentation-http'); const undiciScopes = scopeSpans?.filter( scopeSpan => scopeSpan.scope.name === '@opentelemetry/instrumentation-undici', ); @@ -114,7 +110,6 @@ test('Sends an API route transaction to OTLP', async ({ baseURL }) => { { key: 'net.peer.port', value: { intValue: expect.any(Number) } }, { key: 'http.status_code', value: { intValue: 200 } }, { key: 'http.status_text', value: { stringValue: 'OK' } }, - { key: 'sentry.origin', value: { stringValue: 'auto.http.otel.http' } }, ]), droppedAttributesCount: 0, events: [], diff --git a/packages/node/src/integrations/http.ts b/packages/node/src/integrations/http.ts deleted file mode 100644 index d6796aa866e5..000000000000 --- a/packages/node/src/integrations/http.ts +++ /dev/null @@ -1,312 +0,0 @@ -import type { ClientRequest, IncomingMessage, RequestOptions, ServerResponse } from 'node:http'; -import type { Span } from '@opentelemetry/api'; -import { diag } from '@opentelemetry/api'; -import { HttpInstrumentation } from '@opentelemetry/instrumentation-http'; -import { addOpenTelemetryInstrumentation } from '@sentry/opentelemetry'; - -import { - addBreadcrumb, - defineIntegration, - getCapturedScopesOnSpan, - getCurrentScope, - getIsolationScope, - setCapturedScopesOnSpan, -} from '@sentry/core'; -import { getClient } from '@sentry/opentelemetry'; -import type { IntegrationFn, SanitizedRequestData } from '@sentry/types'; - -import { - getBreadcrumbLogLevelFromHttpStatusCode, - getSanitizedUrlString, - parseUrl, - stripUrlQueryAndFragment, -} from '@sentry/utils'; -import type { NodeClient } from '../sdk/client'; -import { setIsolationScope } from '../sdk/scope'; -import type { HTTPModuleRequestIncomingMessage } from '../transports/http-module'; -import { addOriginToSpan } from '../utils/addOriginToSpan'; -import { getRequestUrl } from '../utils/getRequestUrl'; - -const INTEGRATION_NAME = 'Http'; - -const INSTRUMENTATION_NAME = '@opentelemetry_sentry-patched/instrumentation-http'; - -interface HttpOptions { - /** - * Whether breadcrumbs should be recorded for requests. - * Defaults to true - */ - breadcrumbs?: boolean; - - /** - * Do not capture spans or breadcrumbs for outgoing HTTP requests to URLs where the given callback returns `true`. - * This controls both span & breadcrumb creation - spans will be non recording if tracing is disabled. - * - * The `url` param contains the entire URL, including query string (if any), protocol, host, etc. of the outgoing request. - * For example: `'https://someService.com/users/details?id=123'` - * - * The `request` param contains the original {@type RequestOptions} object used to make the outgoing request. - * You can use it to filter on additional properties like method, headers, etc. - */ - ignoreOutgoingRequests?: (url: string, request: RequestOptions) => boolean; - - /** - * Do not capture spans or breadcrumbs for incoming HTTP requests to URLs where the given callback returns `true`. - * This controls both span & breadcrumb creation - spans will be non recording if tracing is disabled. - * - * The `urlPath` param consists of the URL path and query string (if any) of the incoming request. - * For example: `'/users/details?id=123'` - * - * The `request` param contains the original {@type IncomingMessage} object of the incoming request. - * You can use it to filter on additional properties like method, headers, etc. - */ - ignoreIncomingRequests?: (urlPath: string, request: IncomingMessage) => boolean; - - /** - * Additional instrumentation options that are passed to the underlying HttpInstrumentation. - */ - instrumentation?: { - requestHook?: (span: Span, req: ClientRequest | HTTPModuleRequestIncomingMessage) => void; - responseHook?: (span: Span, response: HTTPModuleRequestIncomingMessage | ServerResponse) => void; - applyCustomAttributesOnSpan?: ( - span: Span, - request: ClientRequest | HTTPModuleRequestIncomingMessage, - response: HTTPModuleRequestIncomingMessage | ServerResponse, - ) => void; - - /** - * You can pass any configuration through to the underlying instrumention. - * Note that there are no semver guarantees for this! - */ - _experimentalConfig?: ConstructorParameters[0]; - }; - - /** Allows to pass a custom version of HttpInstrumentation. We use this for Next.js. */ - _instrumentation?: typeof HttpInstrumentation; -} - -let _httpOptions: HttpOptions = {}; -let _httpInstrumentation: HttpInstrumentation | undefined; - -/** - * Instrument the HTTP module. - * This can only be instrumented once! If this called again later, we just update the options. - */ -export const instrumentHttp = Object.assign( - function (): void { - if (_httpInstrumentation) { - return; - } - - const _InstrumentationClass = _httpOptions._instrumentation || HttpInstrumentation; - - _httpInstrumentation = new _InstrumentationClass({ - ..._httpOptions.instrumentation?._experimentalConfig, - ignoreOutgoingRequestHook: request => { - const url = getRequestUrl(request); - - if (!url) { - return false; - } - - const _ignoreOutgoingRequests = _httpOptions.ignoreOutgoingRequests; - if (_ignoreOutgoingRequests && _ignoreOutgoingRequests(url, request)) { - return true; - } - - return false; - }, - - ignoreIncomingRequestHook: request => { - // request.url is the only property that holds any information about the url - // it only consists of the URL path and query string (if any) - const urlPath = request.url; - - const method = request.method?.toUpperCase(); - // We do not capture OPTIONS/HEAD requests as transactions - if (method === 'OPTIONS' || method === 'HEAD') { - return true; - } - - const _ignoreIncomingRequests = _httpOptions.ignoreIncomingRequests; - if (urlPath && _ignoreIncomingRequests && _ignoreIncomingRequests(urlPath, request)) { - return true; - } - - return false; - }, - - requireParentforOutgoingSpans: false, - requireParentforIncomingSpans: false, - requestHook: (span, req) => { - addOriginToSpan(span, 'auto.http.otel.http'); - - // both, incoming requests and "client" requests made within the app trigger the requestHook - // we only want to isolate and further annotate incoming requests (IncomingMessage) - if (_isClientRequest(req)) { - _httpOptions.instrumentation?.requestHook?.(span, req); - return; - } - - const scopes = getCapturedScopesOnSpan(span); - - const isolationScope = (scopes.isolationScope || getIsolationScope()).clone(); - const scope = scopes.scope || getCurrentScope(); - - // Update the isolation scope, isolate this request - isolationScope.setSDKProcessingMetadata({ request: req }); - - const client = getClient(); - if (client && client.getOptions().autoSessionTracking) { - isolationScope.setRequestSession({ status: 'ok' }); - } - setIsolationScope(isolationScope); - setCapturedScopesOnSpan(span, scope, isolationScope); - - // attempt to update the scope's `transactionName` based on the request URL - // Ideally, framework instrumentations coming after the HttpInstrumentation - // update the transactionName once we get a parameterized route. - const httpMethod = (req.method || 'GET').toUpperCase(); - const httpTarget = stripUrlQueryAndFragment(req.url || '/'); - - const bestEffortTransactionName = `${httpMethod} ${httpTarget}`; - - isolationScope.setTransactionName(bestEffortTransactionName); - - if (isKnownPrefetchRequest(req)) { - span.setAttribute('sentry.http.prefetch', true); - } - - _httpOptions.instrumentation?.requestHook?.(span, req); - }, - responseHook: (span, res) => { - const client = getClient(); - if (client && client.getOptions().autoSessionTracking) { - setImmediate(() => { - client['_captureRequestSession'](); - }); - } - - _httpOptions.instrumentation?.responseHook?.(span, res); - }, - applyCustomAttributesOnSpan: ( - span: Span, - request: ClientRequest | HTTPModuleRequestIncomingMessage, - response: HTTPModuleRequestIncomingMessage | ServerResponse, - ) => { - const _breadcrumbs = typeof _httpOptions.breadcrumbs === 'undefined' ? true : _httpOptions.breadcrumbs; - if (_breadcrumbs) { - _addRequestBreadcrumb(request, response); - } - - _httpOptions.instrumentation?.applyCustomAttributesOnSpan?.(span, request, response); - }, - }); - - // We want to update the logger namespace so we can better identify what is happening here - try { - _httpInstrumentation['_diag'] = diag.createComponentLogger({ - namespace: INSTRUMENTATION_NAME, - }); - - // @ts-expect-error This is marked as read-only, but we overwrite it anyhow - _httpInstrumentation.instrumentationName = INSTRUMENTATION_NAME; - } catch { - // ignore errors here... - } - addOpenTelemetryInstrumentation(_httpInstrumentation); - }, - { - id: INTEGRATION_NAME, - }, -); - -const _httpIntegration = ((options: HttpOptions = {}) => { - return { - name: INTEGRATION_NAME, - setupOnce() { - _httpOptions = options; - instrumentHttp(); - }, - }; -}) satisfies IntegrationFn; - -/** - * The http integration instruments Node's internal http and https modules. - * It creates breadcrumbs and spans for outgoing HTTP requests which will be attached to the currently active span. - */ -export const httpIntegration = defineIntegration(_httpIntegration); - -/** Add a breadcrumb for outgoing requests. */ -function _addRequestBreadcrumb( - request: ClientRequest | HTTPModuleRequestIncomingMessage, - response: HTTPModuleRequestIncomingMessage | ServerResponse, -): void { - // Only generate breadcrumbs for outgoing requests - if (!_isClientRequest(request)) { - return; - } - - const data = getBreadcrumbData(request); - const statusCode = response.statusCode; - const level = getBreadcrumbLogLevelFromHttpStatusCode(statusCode); - - addBreadcrumb( - { - category: 'http', - data: { - status_code: statusCode, - ...data, - }, - type: 'http', - level, - }, - { - event: 'response', - request, - response, - }, - ); -} - -function getBreadcrumbData(request: ClientRequest): Partial { - try { - // `request.host` does not contain the port, but the host header does - const host = request.getHeader('host') || request.host; - const url = new URL(request.path, `${request.protocol}//${host}`); - const parsedUrl = parseUrl(url.toString()); - - const data: Partial = { - url: getSanitizedUrlString(parsedUrl), - 'http.method': request.method || 'GET', - }; - - if (parsedUrl.search) { - data['http.query'] = parsedUrl.search; - } - if (parsedUrl.hash) { - data['http.fragment'] = parsedUrl.hash; - } - - return data; - } catch { - return {}; - } -} - -/** - * Determines if @param req is a ClientRequest, meaning the request was created within the express app - * and it's an outgoing request. - * Checking for properties instead of using `instanceOf` to avoid importing the request classes. - */ -function _isClientRequest(req: ClientRequest | HTTPModuleRequestIncomingMessage): req is ClientRequest { - return 'outputData' in req && 'outputSize' in req && !('client' in req) && !('statusCode' in req); -} - -/** - * Detects if an incoming request is a prefetch request. - */ -function isKnownPrefetchRequest(req: HTTPModuleRequestIncomingMessage): boolean { - // Currently only handles Next.js prefetch requests but may check other frameworks in the future. - return req.headers['next-router-prefetch'] === '1'; -} diff --git a/packages/node/src/integrations/http/SentryHttpInstrumentation.ts b/packages/node/src/integrations/http/SentryHttpInstrumentation.ts new file mode 100644 index 000000000000..1d256f8df8d5 --- /dev/null +++ b/packages/node/src/integrations/http/SentryHttpInstrumentation.ts @@ -0,0 +1,285 @@ +import type * as http from 'node:http'; +import type * as https from 'node:https'; +import { context } from '@opentelemetry/api'; +import { VERSION } from '@opentelemetry/core'; +import type { InstrumentationConfig } from '@opentelemetry/instrumentation'; +import { InstrumentationBase, InstrumentationNodeModuleDefinition } from '@opentelemetry/instrumentation'; +import { addBreadcrumb, getClient, getCurrentScope, getIsolationScope } from '@sentry/core'; +import { setScopesOnContext } from '@sentry/opentelemetry'; +import type { SanitizedRequestData } from '@sentry/types'; +import { + getBreadcrumbLogLevelFromHttpStatusCode, + getSanitizedUrlString, + parseUrl, + stripUrlQueryAndFragment, +} from '@sentry/utils'; +import type { NodeClient } from '../../sdk/client'; + +type Http = typeof http; +type Https = typeof https; + +type SentryHttpInstrumentationOptions = InstrumentationConfig & { + breadcrumbs?: boolean; +}; + +/** + * This custom HTTP instrumentation is used to isolate incoming requests and annotate them with additional information. + * It does not emit any spans. + * + * The reason this is isolated from the OpenTelemetry instrumentation is that users may overwrite this, + * which would lead to Sentry not working as expected. + * + * Important note: Contrary to other OTEL instrumentation, this one cannot be unwrapped. + * It only does minimal things though and does not emit any spans. + * + * This is heavily inspired & adapted from: + * https://github.com/open-telemetry/opentelemetry-js/blob/f8ab5592ddea5cba0a3b33bf8d74f27872c0367f/experimental/packages/opentelemetry-instrumentation-http/src/http.ts + */ +export class SentryHttpInstrumentation extends InstrumentationBase { + public constructor(config: SentryHttpInstrumentationOptions = {}) { + super('@sentry/instrumentation-http', VERSION, config); + } + + /** @inheritdoc */ + public init(): [InstrumentationNodeModuleDefinition, InstrumentationNodeModuleDefinition] { + return [this._getHttpsInstrumentation(), this._getHttpInstrumentation()]; + } + + /** Get the instrumentation for the http module. */ + private _getHttpInstrumentation(): InstrumentationNodeModuleDefinition { + return new InstrumentationNodeModuleDefinition( + 'http', + ['*'], + (moduleExports: Http): Http => { + // Patch incoming requests for request isolation + stealthWrap(moduleExports.Server.prototype, 'emit', this._getPatchIncomingRequestFunction()); + + // Patch outgoing requests for breadcrumbs + const patchedRequest = stealthWrap(moduleExports, 'request', this._getPatchOutgoingRequestFunction()); + stealthWrap(moduleExports, 'get', this._getPatchOutgoingGetFunction(patchedRequest)); + + return moduleExports; + }, + () => { + // no unwrap here + }, + ); + } + + /** Get the instrumentation for the https module. */ + private _getHttpsInstrumentation(): InstrumentationNodeModuleDefinition { + return new InstrumentationNodeModuleDefinition( + 'https', + ['*'], + (moduleExports: Https): Https => { + // Patch incoming requests for request isolation + stealthWrap(moduleExports.Server.prototype, 'emit', this._getPatchIncomingRequestFunction()); + + // Patch outgoing requests for breadcrumbs + const patchedRequest = stealthWrap(moduleExports, 'request', this._getPatchOutgoingRequestFunction()); + stealthWrap(moduleExports, 'get', this._getPatchOutgoingGetFunction(patchedRequest)); + + return moduleExports; + }, + () => { + // no unwrap here + }, + ); + } + + /** + * Patch the incoming request function for request isolation. + */ + private _getPatchIncomingRequestFunction(): ( + original: (event: string, ...args: unknown[]) => boolean, + ) => (this: unknown, event: string, ...args: unknown[]) => boolean { + // eslint-disable-next-line @typescript-eslint/no-this-alias + const instrumentation = this; + + return ( + original: (event: string, ...args: unknown[]) => boolean, + ): ((this: unknown, event: string, ...args: unknown[]) => boolean) => { + return function incomingRequest(this: unknown, event: string, ...args: unknown[]): boolean { + // Only traces request events + if (event !== 'request') { + return original.apply(this, [event, ...args]); + } + + instrumentation._diag.debug('http instrumentation for incoming request'); + + const request = args[0] as http.IncomingMessage; + + const isolationScope = getIsolationScope().clone(); + + // Update the isolation scope, isolate this request + isolationScope.setSDKProcessingMetadata({ request }); + + const client = getClient(); + if (client && client.getOptions().autoSessionTracking) { + isolationScope.setRequestSession({ status: 'ok' }); + } + + // attempt to update the scope's `transactionName` based on the request URL + // Ideally, framework instrumentations coming after the HttpInstrumentation + // update the transactionName once we get a parameterized route. + const httpMethod = (request.method || 'GET').toUpperCase(); + const httpTarget = stripUrlQueryAndFragment(request.url || '/'); + + const bestEffortTransactionName = `${httpMethod} ${httpTarget}`; + + isolationScope.setTransactionName(bestEffortTransactionName); + + const parentContext = context.active(); + const requestContext = setScopesOnContext(parentContext, { + scope: getCurrentScope(), + isolationScope, + }); + + return context.with(requestContext, () => { + return original.apply(this, [event, ...args]); + }); + }; + }; + } + + /** + * Patch the outgoing request function for breadcrumbs. + */ + private _getPatchOutgoingRequestFunction(): ( + // eslint-disable-next-line @typescript-eslint/no-explicit-any + original: (...args: any[]) => http.ClientRequest, + ) => (...args: unknown[]) => http.ClientRequest { + // eslint-disable-next-line @typescript-eslint/no-this-alias + const instrumentation = this; + + return (original: (...args: unknown[]) => http.ClientRequest): ((...args: unknown[]) => http.ClientRequest) => { + return function outgoingRequest(this: unknown, ...args: unknown[]): http.ClientRequest { + instrumentation._diag.debug('http instrumentation for outgoing requests'); + + const request = original.apply(this, args) as ReturnType; + + request.prependListener('response', (response: http.IncomingMessage) => { + const breadcrumbs = instrumentation.getConfig().breadcrumbs; + const _breadcrumbs = typeof breadcrumbs === 'undefined' ? true : breadcrumbs; + if (_breadcrumbs) { + addRequestBreadcrumb(request, response); + } + }); + + return request; + }; + }; + } + + /** Path the outgoing get function for breadcrumbs. */ + // eslint-disable-next-line @typescript-eslint/no-explicit-any + private _getPatchOutgoingGetFunction(clientRequest: (...args: any[]) => http.ClientRequest) { + return (_original: unknown): ((...args: unknown[]) => http.ClientRequest) => { + // Re-implement http.get. This needs to be done (instead of using + // getPatchOutgoingRequestFunction to patch it) because we need to + // set the trace context header before the returned http.ClientRequest is + // ended. The Node.js docs state that the only differences between + // request and get are that (1) get defaults to the HTTP GET method and + // (2) the returned request object is ended immediately. The former is + // already true (at least in supported Node versions up to v10), so we + // simply follow the latter. Ref: + // https://nodejs.org/dist/latest/docs/api/http.html#http_http_get_options_callback + // https://github.com/googleapis/cloud-trace-nodejs/blob/master/src/instrumentations/instrumentation-http.ts#L198 + return function outgoingGetRequest(...args: unknown[]): http.ClientRequest { + const req = clientRequest(...args); + req.end(); + return req; + }; + }; + } +} + +/** + * This is a minimal version of `wrap` from shimmer: + * https://github.com/othiym23/shimmer/blob/master/index.js + * + * In contrast to the original implementation, this version does not allow to unwrap, + * and does not make it clear that the method is wrapped. + * This is necessary because we want to wrap the http module with our own code, + * while still allowing to use the HttpInstrumentation from OTEL. + * + * Without this, if we'd just use `wrap` from shimmer, the OTEL instrumentation would remove our wrapping, + * because it only allows any module to be wrapped a single time. + */ +function stealthWrap( + nodule: Nodule, + name: FieldName, + wrapper: (original: Nodule[FieldName]) => Nodule[FieldName], +): Nodule[FieldName] { + const original = nodule[name]; + const wrapped = wrapper(original); + + defineProperty(nodule, name, wrapped); + return wrapped; +} + +// Sets a property on an object, preserving its enumerability. +function defineProperty( + obj: Nodule, + name: FieldName, + value: Nodule[FieldName], +): void { + const enumerable = !!obj[name] && Object.prototype.propertyIsEnumerable.call(obj, name); + + Object.defineProperty(obj, name, { + configurable: true, + enumerable: enumerable, + writable: true, + value: value, + }); +} + +/** Add a breadcrumb for outgoing requests. */ +function addRequestBreadcrumb(request: http.ClientRequest, response: http.IncomingMessage): void { + const data = getBreadcrumbData(request); + + const statusCode = response.statusCode; + const level = getBreadcrumbLogLevelFromHttpStatusCode(statusCode); + + addBreadcrumb( + { + category: 'http', + data: { + status_code: statusCode, + ...data, + }, + type: 'http', + level, + }, + { + event: 'response', + request, + response, + }, + ); +} + +function getBreadcrumbData(request: http.ClientRequest): Partial { + try { + // `request.host` does not contain the port, but the host header does + const host = request.getHeader('host') || request.host; + const url = new URL(request.path, `${request.protocol}//${host}`); + const parsedUrl = parseUrl(url.toString()); + + const data: Partial = { + url: getSanitizedUrlString(parsedUrl), + 'http.method': request.method || 'GET', + }; + + if (parsedUrl.search) { + data['http.query'] = parsedUrl.search; + } + if (parsedUrl.hash) { + data['http.fragment'] = parsedUrl.hash; + } + + return data; + } catch { + return {}; + } +} diff --git a/packages/node/src/integrations/http/index.ts b/packages/node/src/integrations/http/index.ts new file mode 100644 index 000000000000..414c1029333c --- /dev/null +++ b/packages/node/src/integrations/http/index.ts @@ -0,0 +1,222 @@ +import type { ClientRequest, IncomingMessage, RequestOptions, ServerResponse } from 'node:http'; +import { diag } from '@opentelemetry/api'; +import { HttpInstrumentation } from '@opentelemetry/instrumentation-http'; +import { addOpenTelemetryInstrumentation } from '@sentry/opentelemetry'; + +import { defineIntegration } from '@sentry/core'; +import { getClient } from '@sentry/opentelemetry'; +import type { IntegrationFn, Span } from '@sentry/types'; + +import type { NodeClient } from '../../sdk/client'; +import type { HTTPModuleRequestIncomingMessage } from '../../transports/http-module'; +import { addOriginToSpan } from '../../utils/addOriginToSpan'; +import { getRequestUrl } from '../../utils/getRequestUrl'; +import { SentryHttpInstrumentation } from './SentryHttpInstrumentation'; + +const INTEGRATION_NAME = 'Http'; + +const INSTRUMENTATION_NAME = '@opentelemetry_sentry-patched/instrumentation-http'; + +interface HttpOptions { + /** + * Whether breadcrumbs should be recorded for requests. + * Defaults to true + */ + breadcrumbs?: boolean; + + /** + * If set to false, do not emit any spans. + * This will ensure that the default HttpInstrumentation from OpenTelemetry is not setup, + * only the Sentry-specific instrumentation for request isolation is applied. + */ + spans?: boolean; + + /** + * Do not capture spans or breadcrumbs for outgoing HTTP requests to URLs where the given callback returns `true`. + * This controls both span & breadcrumb creation - spans will be non recording if tracing is disabled. + * + * The `url` param contains the entire URL, including query string (if any), protocol, host, etc. of the outgoing request. + * For example: `'https://someService.com/users/details?id=123'` + * + * The `request` param contains the original {@type RequestOptions} object used to make the outgoing request. + * You can use it to filter on additional properties like method, headers, etc. + */ + ignoreOutgoingRequests?: (url: string, request: RequestOptions) => boolean; + + /** + * Do not capture spans or breadcrumbs for incoming HTTP requests to URLs where the given callback returns `true`. + * This controls both span & breadcrumb creation - spans will be non recording if tracing is disabled. + * + * The `urlPath` param consists of the URL path and query string (if any) of the incoming request. + * For example: `'/users/details?id=123'` + * + * The `request` param contains the original {@type IncomingMessage} object of the incoming request. + * You can use it to filter on additional properties like method, headers, etc. + */ + ignoreIncomingRequests?: (urlPath: string, request: IncomingMessage) => boolean; + + /** + * Additional instrumentation options that are passed to the underlying HttpInstrumentation. + */ + instrumentation?: { + requestHook?: (span: Span, req: ClientRequest | HTTPModuleRequestIncomingMessage) => void; + responseHook?: (span: Span, response: HTTPModuleRequestIncomingMessage | ServerResponse) => void; + applyCustomAttributesOnSpan?: ( + span: Span, + request: ClientRequest | HTTPModuleRequestIncomingMessage, + response: HTTPModuleRequestIncomingMessage | ServerResponse, + ) => void; + + /** + * You can pass any configuration through to the underlying instrumention. + * Note that there are no semver guarantees for this! + */ + _experimentalConfig?: ConstructorParameters[0]; + }; + + /** Allows to pass a custom version of HttpInstrumentation. We use this for Next.js. */ + _instrumentation?: typeof HttpInstrumentation; +} + +let _httpOptions: HttpOptions = {}; +let _sentryHttpInstrumentation: SentryHttpInstrumentation | undefined; +let _httpInstrumentation: HttpInstrumentation | undefined; + +/** + * Instrument the HTTP module. + * This can only be instrumented once! If this called again later, we just update the options. + */ +export const instrumentHttp = Object.assign( + function (): void { + // This is the "regular" OTEL instrumentation that emits spans + if (_httpOptions.spans !== false && !_httpInstrumentation) { + const _InstrumentationClass = _httpOptions._instrumentation || HttpInstrumentation; + + _httpInstrumentation = new _InstrumentationClass({ + ..._httpOptions.instrumentation?._experimentalConfig, + ignoreOutgoingRequestHook: request => { + const url = getRequestUrl(request); + + if (!url) { + return false; + } + + const _ignoreOutgoingRequests = _httpOptions.ignoreOutgoingRequests; + if (_ignoreOutgoingRequests && _ignoreOutgoingRequests(url, request)) { + return true; + } + + return false; + }, + + ignoreIncomingRequestHook: request => { + // request.url is the only property that holds any information about the url + // it only consists of the URL path and query string (if any) + const urlPath = request.url; + + const method = request.method?.toUpperCase(); + // We do not capture OPTIONS/HEAD requests as transactions + if (method === 'OPTIONS' || method === 'HEAD') { + return true; + } + + const _ignoreIncomingRequests = _httpOptions.ignoreIncomingRequests; + if (urlPath && _ignoreIncomingRequests && _ignoreIncomingRequests(urlPath, request)) { + return true; + } + + return false; + }, + + requireParentforOutgoingSpans: false, + requireParentforIncomingSpans: false, + requestHook: (span, req) => { + addOriginToSpan(span, 'auto.http.otel.http'); + if (!_isClientRequest(req) && isKnownPrefetchRequest(req)) { + span.setAttribute('sentry.http.prefetch', true); + } + + _httpOptions.instrumentation?.requestHook?.(span, req); + }, + responseHook: (span, res) => { + const client = getClient(); + if (client && client.getOptions().autoSessionTracking) { + setImmediate(() => { + client['_captureRequestSession'](); + }); + } + + _httpOptions.instrumentation?.responseHook?.(span, res); + }, + applyCustomAttributesOnSpan: ( + span: Span, + request: ClientRequest | HTTPModuleRequestIncomingMessage, + response: HTTPModuleRequestIncomingMessage | ServerResponse, + ) => { + _httpOptions.instrumentation?.applyCustomAttributesOnSpan?.(span, request, response); + }, + }); + + // We want to update the logger namespace so we can better identify what is happening here + try { + _httpInstrumentation['_diag'] = diag.createComponentLogger({ + namespace: INSTRUMENTATION_NAME, + }); + // @ts-expect-error We are writing a read-only property here... + _httpInstrumentation.instrumentationName = INSTRUMENTATION_NAME; + } catch { + // ignore errors here... + } + + addOpenTelemetryInstrumentation(_httpInstrumentation); + } else if (_httpOptions.spans === false && _httpInstrumentation) { + _httpInstrumentation.disable(); + } + + // This is our custom instrumentation that is responsible for request isolation etc. + // We have to add it after the OTEL instrumentation to ensure that we wrap the already wrapped http module + // Otherwise, the isolation scope does not encompass the OTEL spans + if (!_sentryHttpInstrumentation) { + _sentryHttpInstrumentation = new SentryHttpInstrumentation({ breadcrumbs: _httpOptions.breadcrumbs }); + addOpenTelemetryInstrumentation(_sentryHttpInstrumentation); + } else { + _sentryHttpInstrumentation.setConfig({ breadcrumbs: _httpOptions.breadcrumbs }); + } + }, + { + id: INTEGRATION_NAME, + }, +); + +const _httpIntegration = ((options: HttpOptions = {}) => { + return { + name: INTEGRATION_NAME, + setupOnce() { + _httpOptions = options; + instrumentHttp(); + }, + }; +}) satisfies IntegrationFn; + +/** + * The http integration instruments Node's internal http and https modules. + * It creates breadcrumbs and spans for outgoing HTTP requests which will be attached to the currently active span. + */ +export const httpIntegration = defineIntegration(_httpIntegration); + +/** + * Determines if @param req is a ClientRequest, meaning the request was created within the express app + * and it's an outgoing request. + * Checking for properties instead of using `instanceOf` to avoid importing the request classes. + */ +function _isClientRequest(req: ClientRequest | HTTPModuleRequestIncomingMessage): req is ClientRequest { + return 'outputData' in req && 'outputSize' in req && !('client' in req) && !('statusCode' in req); +} + +/** + * Detects if an incoming request is a prefetch request. + */ +function isKnownPrefetchRequest(req: HTTPModuleRequestIncomingMessage): boolean { + // Currently only handles Next.js prefetch requests but may check other frameworks in the future. + return req.headers['next-router-prefetch'] === '1'; +} diff --git a/packages/opentelemetry/src/index.ts b/packages/opentelemetry/src/index.ts index 98460b575c8d..6fcf40e197d6 100644 --- a/packages/opentelemetry/src/index.ts +++ b/packages/opentelemetry/src/index.ts @@ -7,7 +7,7 @@ export { wrapClientClass } from './custom/client'; export { getSpanKind } from './utils/getSpanKind'; -export { getScopesFromContext } from './utils/contextData'; +export { getScopesFromContext, setScopesOnContext } from './utils/contextData'; export { spanHasAttributes, From 927d16a9919db51b2270f354c27986de380a7f9a Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Tue, 24 Sep 2024 11:00:25 +0200 Subject: [PATCH 02/14] Update dev-packages/e2e-tests/test-applications/node-otel-without-tracing/src/instrument.ts --- .../node-otel-without-tracing/src/instrument.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev-packages/e2e-tests/test-applications/node-otel-without-tracing/src/instrument.ts b/dev-packages/e2e-tests/test-applications/node-otel-without-tracing/src/instrument.ts index 52c4403b2164..47078a504e18 100644 --- a/dev-packages/e2e-tests/test-applications/node-otel-without-tracing/src/instrument.ts +++ b/dev-packages/e2e-tests/test-applications/node-otel-without-tracing/src/instrument.ts @@ -16,7 +16,7 @@ Sentry.init({ tunnel: `http://localhost:3031/`, // proxy server // Tracing is completely disabled - integrations: [Sentry.httpIntegration({ spans: true })], + integrations: [Sentry.httpIntegration({ spans: false })], // Custom OTEL setup skipOpenTelemetrySetup: true, From 75309b742c5d53597ba638481a9216bc29af50ee Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Tue, 24 Sep 2024 14:19:46 +0200 Subject: [PATCH 03/14] fix preload --- .../integrations/http/SentryHttpInstrumentation.ts | 13 +++---------- packages/node/src/integrations/tracing/index.ts | 2 -- packages/opentelemetry/src/index.ts | 2 +- 3 files changed, 4 insertions(+), 13 deletions(-) diff --git a/packages/node/src/integrations/http/SentryHttpInstrumentation.ts b/packages/node/src/integrations/http/SentryHttpInstrumentation.ts index 1d256f8df8d5..bd6ab7447bda 100644 --- a/packages/node/src/integrations/http/SentryHttpInstrumentation.ts +++ b/packages/node/src/integrations/http/SentryHttpInstrumentation.ts @@ -1,11 +1,9 @@ import type * as http from 'node:http'; import type * as https from 'node:https'; -import { context } from '@opentelemetry/api'; import { VERSION } from '@opentelemetry/core'; import type { InstrumentationConfig } from '@opentelemetry/instrumentation'; import { InstrumentationBase, InstrumentationNodeModuleDefinition } from '@opentelemetry/instrumentation'; -import { addBreadcrumb, getClient, getCurrentScope, getIsolationScope } from '@sentry/core'; -import { setScopesOnContext } from '@sentry/opentelemetry'; +import { addBreadcrumb, getClient, getIsolationScope } from '@sentry/core'; import type { SanitizedRequestData } from '@sentry/types'; import { getBreadcrumbLogLevelFromHttpStatusCode, @@ -13,6 +11,7 @@ import { parseUrl, stripUrlQueryAndFragment, } from '@sentry/utils'; +import { withIsolationScope } from '../..'; import type { NodeClient } from '../../sdk/client'; type Http = typeof http; @@ -129,13 +128,7 @@ export class SentryHttpInstrumentation extends InstrumentationBase { + return withIsolationScope(isolationScope, () => { return original.apply(this, [event, ...args]); }); }; diff --git a/packages/node/src/integrations/tracing/index.ts b/packages/node/src/integrations/tracing/index.ts index 1181179a57d3..ea967e017882 100644 --- a/packages/node/src/integrations/tracing/index.ts +++ b/packages/node/src/integrations/tracing/index.ts @@ -1,5 +1,4 @@ import type { Integration } from '@sentry/types'; -import { instrumentHttp } from '../http'; import { amqplibIntegration, instrumentAmqplib } from './amqplib'; import { connectIntegration, instrumentConnect } from './connect'; @@ -54,7 +53,6 @@ export function getAutoPerformanceIntegrations(): Integration[] { // eslint-disable-next-line @typescript-eslint/no-explicit-any export function getOpenTelemetryInstrumentationToPreload(): (((options?: any) => void) & { id: string })[] { return [ - instrumentHttp, instrumentExpress, instrumentConnect, instrumentFastify, diff --git a/packages/opentelemetry/src/index.ts b/packages/opentelemetry/src/index.ts index 6fcf40e197d6..98460b575c8d 100644 --- a/packages/opentelemetry/src/index.ts +++ b/packages/opentelemetry/src/index.ts @@ -7,7 +7,7 @@ export { wrapClientClass } from './custom/client'; export { getSpanKind } from './utils/getSpanKind'; -export { getScopesFromContext, setScopesOnContext } from './utils/contextData'; +export { getScopesFromContext } from './utils/contextData'; export { spanHasAttributes, From 27c22f1d098d07898b8506d9f0491c4319845f20 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Tue, 24 Sep 2024 14:35:37 +0200 Subject: [PATCH 04/14] fix madge --- .../node/src/integrations/http/SentryHttpInstrumentation.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/node/src/integrations/http/SentryHttpInstrumentation.ts b/packages/node/src/integrations/http/SentryHttpInstrumentation.ts index bd6ab7447bda..62922b1b3921 100644 --- a/packages/node/src/integrations/http/SentryHttpInstrumentation.ts +++ b/packages/node/src/integrations/http/SentryHttpInstrumentation.ts @@ -3,7 +3,7 @@ import type * as https from 'node:https'; import { VERSION } from '@opentelemetry/core'; import type { InstrumentationConfig } from '@opentelemetry/instrumentation'; import { InstrumentationBase, InstrumentationNodeModuleDefinition } from '@opentelemetry/instrumentation'; -import { addBreadcrumb, getClient, getIsolationScope } from '@sentry/core'; +import { addBreadcrumb, getClient, getIsolationScope, withIsolationScope } from '@sentry/core'; import type { SanitizedRequestData } from '@sentry/types'; import { getBreadcrumbLogLevelFromHttpStatusCode, @@ -11,7 +11,6 @@ import { parseUrl, stripUrlQueryAndFragment, } from '@sentry/utils'; -import { withIsolationScope } from '../..'; import type { NodeClient } from '../../sdk/client'; type Http = typeof http; From e64e393c79859a0317c9e7442b8731d79054c9ad Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Tue, 24 Sep 2024 14:36:06 +0200 Subject: [PATCH 05/14] fix tests --- packages/node/test/sdk/preload.test.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/node/test/sdk/preload.test.ts b/packages/node/test/sdk/preload.test.ts index fedba139b0f6..565931bc7982 100644 --- a/packages/node/test/sdk/preload.test.ts +++ b/packages/node/test/sdk/preload.test.ts @@ -28,7 +28,6 @@ describe('preload', () => { await import('../../src/preload'); - expect(logSpy).toHaveBeenCalledWith('Sentry Logger [log]:', '[Sentry] Preloaded Http instrumentation'); expect(logSpy).toHaveBeenCalledWith('Sentry Logger [log]:', '[Sentry] Preloaded Express instrumentation'); expect(logSpy).toHaveBeenCalledWith('Sentry Logger [log]:', '[Sentry] Preloaded Graphql instrumentation'); }); @@ -43,7 +42,6 @@ describe('preload', () => { await import('../../src/preload'); - expect(logSpy).toHaveBeenCalledWith('Sentry Logger [log]:', '[Sentry] Preloaded Http instrumentation'); expect(logSpy).toHaveBeenCalledWith('Sentry Logger [log]:', '[Sentry] Preloaded Express instrumentation'); expect(logSpy).not.toHaveBeenCalledWith('Sentry Logger [log]:', '[Sentry] Preloaded Graphql instrumentation'); }); From 93a871d4905a16fa9e8fa736cbc4dfba148d16e3 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Tue, 24 Sep 2024 15:27:44 +0200 Subject: [PATCH 06/14] Revert "fix tests" This reverts commit f5ab591dbfa86104de34f2518b4cd7d1c944a63a. --- packages/node/test/sdk/preload.test.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/node/test/sdk/preload.test.ts b/packages/node/test/sdk/preload.test.ts index 565931bc7982..fedba139b0f6 100644 --- a/packages/node/test/sdk/preload.test.ts +++ b/packages/node/test/sdk/preload.test.ts @@ -28,6 +28,7 @@ describe('preload', () => { await import('../../src/preload'); + expect(logSpy).toHaveBeenCalledWith('Sentry Logger [log]:', '[Sentry] Preloaded Http instrumentation'); expect(logSpy).toHaveBeenCalledWith('Sentry Logger [log]:', '[Sentry] Preloaded Express instrumentation'); expect(logSpy).toHaveBeenCalledWith('Sentry Logger [log]:', '[Sentry] Preloaded Graphql instrumentation'); }); @@ -42,6 +43,7 @@ describe('preload', () => { await import('../../src/preload'); + expect(logSpy).toHaveBeenCalledWith('Sentry Logger [log]:', '[Sentry] Preloaded Http instrumentation'); expect(logSpy).toHaveBeenCalledWith('Sentry Logger [log]:', '[Sentry] Preloaded Express instrumentation'); expect(logSpy).not.toHaveBeenCalledWith('Sentry Logger [log]:', '[Sentry] Preloaded Graphql instrumentation'); }); From 0dc11097dd939e27e41e5fdcead06186d0669e11 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Tue, 24 Sep 2024 16:25:40 +0200 Subject: [PATCH 07/14] WIP improve things... --- packages/node/src/integrations/http/index.ts | 92 ++++++++++--------- .../node/src/integrations/tracing/index.ts | 2 + packages/remix/package.json | 1 - packages/remix/src/utils/integrations/http.ts | 18 +--- 4 files changed, 51 insertions(+), 62 deletions(-) diff --git a/packages/node/src/integrations/http/index.ts b/packages/node/src/integrations/http/index.ts index 414c1029333c..d8af19a10b88 100644 --- a/packages/node/src/integrations/http/index.ts +++ b/packages/node/src/integrations/http/index.ts @@ -1,12 +1,12 @@ import type { ClientRequest, IncomingMessage, RequestOptions, ServerResponse } from 'node:http'; import { diag } from '@opentelemetry/api'; -import { HttpInstrumentation } from '@opentelemetry/instrumentation-http'; -import { addOpenTelemetryInstrumentation } from '@sentry/opentelemetry'; +import { HttpInstrumentation, HttpInstrumentationConfig } from '@opentelemetry/instrumentation-http'; import { defineIntegration } from '@sentry/core'; import { getClient } from '@sentry/opentelemetry'; import type { IntegrationFn, Span } from '@sentry/types'; +import { generateInstrumentOnce } from '../../otel/instrument'; import type { NodeClient } from '../../sdk/client'; import type { HTTPModuleRequestIncomingMessage } from '../../transports/http-module'; import { addOriginToSpan } from '../../utils/addOriginToSpan'; @@ -55,6 +55,12 @@ interface HttpOptions { */ ignoreIncomingRequests?: (urlPath: string, request: IncomingMessage) => boolean; + /** + * If true, do not generate spans for incoming requests at all. + * This is used by Remix to avoid generating spans for incoming requests, as it generates its own spans. + */ + disableIncomingRequestSpans?: boolean; + /** * Additional instrumentation options that are passed to the underlying HttpInstrumentation. */ @@ -73,27 +79,45 @@ interface HttpOptions { */ _experimentalConfig?: ConstructorParameters[0]; }; - - /** Allows to pass a custom version of HttpInstrumentation. We use this for Next.js. */ - _instrumentation?: typeof HttpInstrumentation; } -let _httpOptions: HttpOptions = {}; -let _sentryHttpInstrumentation: SentryHttpInstrumentation | undefined; -let _httpInstrumentation: HttpInstrumentation | undefined; +const instrumentSentryHttp = generateInstrumentOnce<{ breadcrumbs?: boolean }>( + `${INTEGRATION_NAME}.sentry`, + options => { + return new SentryHttpInstrumentation({ breadcrumbs: options?.breadcrumbs }); + }, +); + +const instrumentOtelHttp = generateInstrumentOnce(`${INTEGRATION_NAME}.otel`, config => { + const instrumentation = new HttpInstrumentation(config); + + // We want to update the logger namespace so we can better identify what is happening here + try { + instrumentation['_diag'] = diag.createComponentLogger({ + namespace: INSTRUMENTATION_NAME, + }); + // @ts-expect-error We are writing a read-only property here... + instrumentation.instrumentationName = INSTRUMENTATION_NAME; + } catch { + // ignore errors here... + } + + return instrumentation; +}); /** * Instrument the HTTP module. * This can only be instrumented once! If this called again later, we just update the options. */ export const instrumentHttp = Object.assign( - function (): void { + function (options: HttpOptions = {}) { // This is the "regular" OTEL instrumentation that emits spans - if (_httpOptions.spans !== false && !_httpInstrumentation) { - const _InstrumentationClass = _httpOptions._instrumentation || HttpInstrumentation; + if (options.spans !== false) { + const instrumentationConfig = { + ...options.instrumentation?._experimentalConfig, + + disableIncomingRequestInstrumentation: options.disableIncomingRequestSpans, - _httpInstrumentation = new _InstrumentationClass({ - ..._httpOptions.instrumentation?._experimentalConfig, ignoreOutgoingRequestHook: request => { const url = getRequestUrl(request); @@ -101,7 +125,7 @@ export const instrumentHttp = Object.assign( return false; } - const _ignoreOutgoingRequests = _httpOptions.ignoreOutgoingRequests; + const _ignoreOutgoingRequests = options.ignoreOutgoingRequests; if (_ignoreOutgoingRequests && _ignoreOutgoingRequests(url, request)) { return true; } @@ -120,7 +144,7 @@ export const instrumentHttp = Object.assign( return true; } - const _ignoreIncomingRequests = _httpOptions.ignoreIncomingRequests; + const _ignoreIncomingRequests = options.ignoreIncomingRequests; if (urlPath && _ignoreIncomingRequests && _ignoreIncomingRequests(urlPath, request)) { return true; } @@ -136,7 +160,7 @@ export const instrumentHttp = Object.assign( span.setAttribute('sentry.http.prefetch', true); } - _httpOptions.instrumentation?.requestHook?.(span, req); + options.instrumentation?.requestHook?.(span, req); }, responseHook: (span, res) => { const client = getClient(); @@ -146,42 +170,21 @@ export const instrumentHttp = Object.assign( }); } - _httpOptions.instrumentation?.responseHook?.(span, res); + options.instrumentation?.responseHook?.(span, res); }, applyCustomAttributesOnSpan: ( span: Span, request: ClientRequest | HTTPModuleRequestIncomingMessage, response: HTTPModuleRequestIncomingMessage | ServerResponse, ) => { - _httpOptions.instrumentation?.applyCustomAttributesOnSpan?.(span, request, response); + options.instrumentation?.applyCustomAttributesOnSpan?.(span, request, response); }, - }); - - // We want to update the logger namespace so we can better identify what is happening here - try { - _httpInstrumentation['_diag'] = diag.createComponentLogger({ - namespace: INSTRUMENTATION_NAME, - }); - // @ts-expect-error We are writing a read-only property here... - _httpInstrumentation.instrumentationName = INSTRUMENTATION_NAME; - } catch { - // ignore errors here... - } - - addOpenTelemetryInstrumentation(_httpInstrumentation); - } else if (_httpOptions.spans === false && _httpInstrumentation) { - _httpInstrumentation.disable(); - } + } satisfies HttpInstrumentationConfig; - // This is our custom instrumentation that is responsible for request isolation etc. - // We have to add it after the OTEL instrumentation to ensure that we wrap the already wrapped http module - // Otherwise, the isolation scope does not encompass the OTEL spans - if (!_sentryHttpInstrumentation) { - _sentryHttpInstrumentation = new SentryHttpInstrumentation({ breadcrumbs: _httpOptions.breadcrumbs }); - addOpenTelemetryInstrumentation(_sentryHttpInstrumentation); - } else { - _sentryHttpInstrumentation.setConfig({ breadcrumbs: _httpOptions.breadcrumbs }); + instrumentOtelHttp(instrumentationConfig); } + + instrumentSentryHttp(options); }, { id: INTEGRATION_NAME, @@ -192,8 +195,7 @@ const _httpIntegration = ((options: HttpOptions = {}) => { return { name: INTEGRATION_NAME, setupOnce() { - _httpOptions = options; - instrumentHttp(); + instrumentHttp(options); }, }; }) satisfies IntegrationFn; diff --git a/packages/node/src/integrations/tracing/index.ts b/packages/node/src/integrations/tracing/index.ts index ea967e017882..1181179a57d3 100644 --- a/packages/node/src/integrations/tracing/index.ts +++ b/packages/node/src/integrations/tracing/index.ts @@ -1,4 +1,5 @@ import type { Integration } from '@sentry/types'; +import { instrumentHttp } from '../http'; import { amqplibIntegration, instrumentAmqplib } from './amqplib'; import { connectIntegration, instrumentConnect } from './connect'; @@ -53,6 +54,7 @@ export function getAutoPerformanceIntegrations(): Integration[] { // eslint-disable-next-line @typescript-eslint/no-explicit-any export function getOpenTelemetryInstrumentationToPreload(): (((options?: any) => void) & { id: string })[] { return [ + instrumentHttp, instrumentExpress, instrumentConnect, instrumentFastify, diff --git a/packages/remix/package.json b/packages/remix/package.json index 042da8843032..4ed3b71f626c 100644 --- a/packages/remix/package.json +++ b/packages/remix/package.json @@ -52,7 +52,6 @@ "access": "public" }, "dependencies": { - "@opentelemetry/instrumentation-http": "0.53.0", "@remix-run/router": "1.x", "@sentry/cli": "^2.35.0", "@sentry/core": "8.34.0", diff --git a/packages/remix/src/utils/integrations/http.ts b/packages/remix/src/utils/integrations/http.ts index d3a7ae03e351..be519a36806a 100644 --- a/packages/remix/src/utils/integrations/http.ts +++ b/packages/remix/src/utils/integrations/http.ts @@ -1,21 +1,6 @@ -// This integration is ported from the Next.JS SDK. -import { HttpInstrumentation } from '@opentelemetry/instrumentation-http'; import { httpIntegration as originalHttpIntegration } from '@sentry/node'; import type { IntegrationFn } from '@sentry/types'; -class RemixHttpIntegration extends HttpInstrumentation { - // Instead of the default behavior, we just don't do any wrapping for incoming requests - protected _getPatchIncomingRequestFunction(_component: 'http' | 'https') { - return ( - original: (event: string, ...args: unknown[]) => boolean, - ): ((this: unknown, event: string, ...args: unknown[]) => boolean) => { - return function incomingRequest(this: unknown, event: string, ...args: unknown[]): boolean { - return original.apply(this, [event, ...args]); - }; - }; - } -} - type HttpOptions = Parameters[0]; /** @@ -25,6 +10,7 @@ type HttpOptions = Parameters[0]; export const httpIntegration = ((options: HttpOptions = {}) => { return originalHttpIntegration({ ...options, - _instrumentation: RemixHttpIntegration, + // We disable incoming request spans here, because otherwise we'd end up with duplicate spans. + disableIncomingRequestSpans: true, }); }) satisfies IntegrationFn; From 9d998676c0541ae41491a57626073bd4f759a54d Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Tue, 24 Sep 2024 16:55:54 +0200 Subject: [PATCH 08/14] only preload once Do not preload our custom sentry stuff --- packages/node/src/integrations/http/index.ts | 42 +++++++++++-------- .../node/src/integrations/tracing/index.ts | 4 +- 2 files changed, 26 insertions(+), 20 deletions(-) diff --git a/packages/node/src/integrations/http/index.ts b/packages/node/src/integrations/http/index.ts index d8af19a10b88..77f89734fcf6 100644 --- a/packages/node/src/integrations/http/index.ts +++ b/packages/node/src/integrations/http/index.ts @@ -88,28 +88,34 @@ const instrumentSentryHttp = generateInstrumentOnce<{ breadcrumbs?: boolean }>( }, ); -const instrumentOtelHttp = generateInstrumentOnce(`${INTEGRATION_NAME}.otel`, config => { - const instrumentation = new HttpInstrumentation(config); - - // We want to update the logger namespace so we can better identify what is happening here - try { - instrumentation['_diag'] = diag.createComponentLogger({ - namespace: INSTRUMENTATION_NAME, - }); - // @ts-expect-error We are writing a read-only property here... - instrumentation.instrumentationName = INSTRUMENTATION_NAME; - } catch { - // ignore errors here... - } - - return instrumentation; -}); +/** + * We only preload this one. + * If we preload both this and `instrumentSentryHttp`, it leads to weird issues with instrumentation. + */ +export const instrumentOtelHttp = generateInstrumentOnce( + `${INTEGRATION_NAME}.otel`, + config => { + const instrumentation = new HttpInstrumentation(config); + + // We want to update the logger namespace so we can better identify what is happening here + try { + instrumentation['_diag'] = diag.createComponentLogger({ + namespace: INSTRUMENTATION_NAME, + }); + // @ts-expect-error We are writing a read-only property here... + instrumentation.instrumentationName = INSTRUMENTATION_NAME; + } catch { + // ignore errors here... + } + + return instrumentation; + }, +); /** * Instrument the HTTP module. - * This can only be instrumented once! If this called again later, we just update the options. */ -export const instrumentHttp = Object.assign( +const instrumentHttp = Object.assign( function (options: HttpOptions = {}) { // This is the "regular" OTEL instrumentation that emits spans if (options.spans !== false) { diff --git a/packages/node/src/integrations/tracing/index.ts b/packages/node/src/integrations/tracing/index.ts index 1181179a57d3..328767c403be 100644 --- a/packages/node/src/integrations/tracing/index.ts +++ b/packages/node/src/integrations/tracing/index.ts @@ -1,5 +1,5 @@ import type { Integration } from '@sentry/types'; -import { instrumentHttp } from '../http'; +import { instrumentOtelHttp } from '../http'; import { amqplibIntegration, instrumentAmqplib } from './amqplib'; import { connectIntegration, instrumentConnect } from './connect'; @@ -54,7 +54,7 @@ export function getAutoPerformanceIntegrations(): Integration[] { // eslint-disable-next-line @typescript-eslint/no-explicit-any export function getOpenTelemetryInstrumentationToPreload(): (((options?: any) => void) & { id: string })[] { return [ - instrumentHttp, + instrumentOtelHttp, instrumentExpress, instrumentConnect, instrumentFastify, From 023622c7305d15ff757f107d0c9427a77868bc6f Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Tue, 24 Sep 2024 17:18:57 +0200 Subject: [PATCH 09/14] fix name for preload --- packages/node/src/integrations/http/index.ts | 187 +++++++++---------- 1 file changed, 90 insertions(+), 97 deletions(-) diff --git a/packages/node/src/integrations/http/index.ts b/packages/node/src/integrations/http/index.ts index 77f89734fcf6..aa741a5d2336 100644 --- a/packages/node/src/integrations/http/index.ts +++ b/packages/node/src/integrations/http/index.ts @@ -1,6 +1,7 @@ import type { ClientRequest, IncomingMessage, RequestOptions, ServerResponse } from 'node:http'; import { diag } from '@opentelemetry/api'; -import { HttpInstrumentation, HttpInstrumentationConfig } from '@opentelemetry/instrumentation-http'; +import type { HttpInstrumentationConfig } from '@opentelemetry/instrumentation-http'; +import { HttpInstrumentation } from '@opentelemetry/instrumentation-http'; import { defineIntegration } from '@sentry/core'; import { getClient } from '@sentry/opentelemetry'; @@ -92,110 +93,102 @@ const instrumentSentryHttp = generateInstrumentOnce<{ breadcrumbs?: boolean }>( * We only preload this one. * If we preload both this and `instrumentSentryHttp`, it leads to weird issues with instrumentation. */ -export const instrumentOtelHttp = generateInstrumentOnce( - `${INTEGRATION_NAME}.otel`, - config => { - const instrumentation = new HttpInstrumentation(config); - - // We want to update the logger namespace so we can better identify what is happening here - try { - instrumentation['_diag'] = diag.createComponentLogger({ - namespace: INSTRUMENTATION_NAME, - }); - // @ts-expect-error We are writing a read-only property here... - instrumentation.instrumentationName = INSTRUMENTATION_NAME; - } catch { - // ignore errors here... - } - - return instrumentation; - }, -); +export const instrumentOtelHttp = generateInstrumentOnce(INTEGRATION_NAME, config => { + const instrumentation = new HttpInstrumentation(config); + + // We want to update the logger namespace so we can better identify what is happening here + try { + instrumentation['_diag'] = diag.createComponentLogger({ + namespace: INSTRUMENTATION_NAME, + }); + // @ts-expect-error We are writing a read-only property here... + instrumentation.instrumentationName = INSTRUMENTATION_NAME; + } catch { + // ignore errors here... + } + + return instrumentation; +}); /** * Instrument the HTTP module. */ -const instrumentHttp = Object.assign( - function (options: HttpOptions = {}) { - // This is the "regular" OTEL instrumentation that emits spans - if (options.spans !== false) { - const instrumentationConfig = { - ...options.instrumentation?._experimentalConfig, - - disableIncomingRequestInstrumentation: options.disableIncomingRequestSpans, - - ignoreOutgoingRequestHook: request => { - const url = getRequestUrl(request); - - if (!url) { - return false; - } - - const _ignoreOutgoingRequests = options.ignoreOutgoingRequests; - if (_ignoreOutgoingRequests && _ignoreOutgoingRequests(url, request)) { - return true; - } +const instrumentHttp = (options: HttpOptions = {}): void => { + // This is the "regular" OTEL instrumentation that emits spans + if (options.spans !== false) { + const instrumentationConfig = { + ...options.instrumentation?._experimentalConfig, - return false; - }, - - ignoreIncomingRequestHook: request => { - // request.url is the only property that holds any information about the url - // it only consists of the URL path and query string (if any) - const urlPath = request.url; - - const method = request.method?.toUpperCase(); - // We do not capture OPTIONS/HEAD requests as transactions - if (method === 'OPTIONS' || method === 'HEAD') { - return true; - } + disableIncomingRequestInstrumentation: options.disableIncomingRequestSpans, - const _ignoreIncomingRequests = options.ignoreIncomingRequests; - if (urlPath && _ignoreIncomingRequests && _ignoreIncomingRequests(urlPath, request)) { - return true; - } + ignoreOutgoingRequestHook: request => { + const url = getRequestUrl(request); + if (!url) { return false; - }, - - requireParentforOutgoingSpans: false, - requireParentforIncomingSpans: false, - requestHook: (span, req) => { - addOriginToSpan(span, 'auto.http.otel.http'); - if (!_isClientRequest(req) && isKnownPrefetchRequest(req)) { - span.setAttribute('sentry.http.prefetch', true); - } - - options.instrumentation?.requestHook?.(span, req); - }, - responseHook: (span, res) => { - const client = getClient(); - if (client && client.getOptions().autoSessionTracking) { - setImmediate(() => { - client['_captureRequestSession'](); - }); - } - - options.instrumentation?.responseHook?.(span, res); - }, - applyCustomAttributesOnSpan: ( - span: Span, - request: ClientRequest | HTTPModuleRequestIncomingMessage, - response: HTTPModuleRequestIncomingMessage | ServerResponse, - ) => { - options.instrumentation?.applyCustomAttributesOnSpan?.(span, request, response); - }, - } satisfies HttpInstrumentationConfig; - - instrumentOtelHttp(instrumentationConfig); - } - - instrumentSentryHttp(options); - }, - { - id: INTEGRATION_NAME, - }, -); + } + + const _ignoreOutgoingRequests = options.ignoreOutgoingRequests; + if (_ignoreOutgoingRequests && _ignoreOutgoingRequests(url, request)) { + return true; + } + + return false; + }, + + ignoreIncomingRequestHook: request => { + // request.url is the only property that holds any information about the url + // it only consists of the URL path and query string (if any) + const urlPath = request.url; + + const method = request.method?.toUpperCase(); + // We do not capture OPTIONS/HEAD requests as transactions + if (method === 'OPTIONS' || method === 'HEAD') { + return true; + } + + const _ignoreIncomingRequests = options.ignoreIncomingRequests; + if (urlPath && _ignoreIncomingRequests && _ignoreIncomingRequests(urlPath, request)) { + return true; + } + + return false; + }, + + requireParentforOutgoingSpans: false, + requireParentforIncomingSpans: false, + requestHook: (span, req) => { + addOriginToSpan(span, 'auto.http.otel.http'); + if (!_isClientRequest(req) && isKnownPrefetchRequest(req)) { + span.setAttribute('sentry.http.prefetch', true); + } + + options.instrumentation?.requestHook?.(span, req); + }, + responseHook: (span, res) => { + const client = getClient(); + if (client && client.getOptions().autoSessionTracking) { + setImmediate(() => { + client['_captureRequestSession'](); + }); + } + + options.instrumentation?.responseHook?.(span, res); + }, + applyCustomAttributesOnSpan: ( + span: Span, + request: ClientRequest | HTTPModuleRequestIncomingMessage, + response: HTTPModuleRequestIncomingMessage | ServerResponse, + ) => { + options.instrumentation?.applyCustomAttributesOnSpan?.(span, request, response); + }, + } satisfies HttpInstrumentationConfig; + + instrumentOtelHttp(instrumentationConfig); + } + + instrumentSentryHttp(options); +}; const _httpIntegration = ((options: HttpOptions = {}) => { return { From d462caf95fd4a498bb40870683b82091934691e2 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Wed, 25 Sep 2024 10:41:55 +0200 Subject: [PATCH 10/14] test adjustments --- .../test-applications/aws-lambda-layer-cjs/package.json | 3 +-- .../test-applications/aws-serverless-esm/package.json | 3 +-- .../test-applications/aws-serverless-esm/tests/basic.test.ts | 2 +- 3 files changed, 3 insertions(+), 5 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/aws-lambda-layer-cjs/package.json b/dev-packages/e2e-tests/test-applications/aws-lambda-layer-cjs/package.json index e16d49c799f4..01375fe0c346 100644 --- a/dev-packages/e2e-tests/test-applications/aws-lambda-layer-cjs/package.json +++ b/dev-packages/e2e-tests/test-applications/aws-lambda-layer-cjs/package.json @@ -14,8 +14,7 @@ }, "devDependencies": { "@sentry-internal/test-utils": "link:../../../test-utils", - "@playwright/test": "^1.44.1", - "wait-port": "1.0.4" + "@playwright/test": "^1.44.1" }, "volta": { "extends": "../../package.json" diff --git a/dev-packages/e2e-tests/test-applications/aws-serverless-esm/package.json b/dev-packages/e2e-tests/test-applications/aws-serverless-esm/package.json index ebd28b380d68..67aa6bc247d5 100644 --- a/dev-packages/e2e-tests/test-applications/aws-serverless-esm/package.json +++ b/dev-packages/e2e-tests/test-applications/aws-serverless-esm/package.json @@ -14,8 +14,7 @@ }, "devDependencies": { "@sentry-internal/test-utils": "link:../../../test-utils", - "@playwright/test": "^1.41.1", - "wait-port": "1.0.4" + "@playwright/test": "^1.41.1" }, "volta": { "extends": "../../package.json" diff --git a/dev-packages/e2e-tests/test-applications/aws-serverless-esm/tests/basic.test.ts b/dev-packages/e2e-tests/test-applications/aws-serverless-esm/tests/basic.test.ts index 83fab96ee117..f6e57655ad08 100644 --- a/dev-packages/e2e-tests/test-applications/aws-serverless-esm/tests/basic.test.ts +++ b/dev-packages/e2e-tests/test-applications/aws-serverless-esm/tests/basic.test.ts @@ -18,7 +18,7 @@ test('AWS Serverless SDK sends events in ESM mode', async ({ request }) => { ); child_process.execSync('pnpm start', { - stdio: 'ignore', + stdio: 'inherit', }); const transactionEvent = await transactionEventPromise; From 6b4ad587effe449ceda466a03bc5a1bf47c7c660 Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Tue, 8 Oct 2024 13:53:54 +0200 Subject: [PATCH 11/14] Update import-in-the-middle to 1.11.2 --- packages/node/package.json | 2 +- yarn.lock | 70 ++++++-------------------------------- 2 files changed, 11 insertions(+), 61 deletions(-) diff --git a/packages/node/package.json b/packages/node/package.json index d9ca6f86074e..8145cf18a7d6 100644 --- a/packages/node/package.json +++ b/packages/node/package.json @@ -99,7 +99,7 @@ "@sentry/opentelemetry": "8.34.0", "@sentry/types": "8.34.0", "@sentry/utils": "8.34.0", - "import-in-the-middle": "^1.11.0" + "import-in-the-middle": "^1.11.2" }, "devDependencies": { "@types/node": "^14.18.0" diff --git a/yarn.lock b/yarn.lock index 63ee9ec0b8bd..5ab8ce9b9952 100644 --- a/yarn.lock +++ b/yarn.lock @@ -9826,17 +9826,7 @@ dependencies: "@types/unist" "*" -"@types/history-4@npm:@types/history@4.7.8": - version "4.7.8" - resolved "https://registry.yarnpkg.com/@types/history/-/history-4.7.8.tgz#49348387983075705fe8f4e02fb67f7daaec4934" - integrity sha512-S78QIYirQcUoo6UJZx9CSP0O2ix9IaeAXwQi26Rhr/+mg7qqPy8TzaxHSUut7eGjL8WmLccT7/MXf304WjqHcA== - -"@types/history-5@npm:@types/history@4.7.8": - version "4.7.8" - resolved "https://registry.yarnpkg.com/@types/history/-/history-4.7.8.tgz#49348387983075705fe8f4e02fb67f7daaec4934" - integrity sha512-S78QIYirQcUoo6UJZx9CSP0O2ix9IaeAXwQi26Rhr/+mg7qqPy8TzaxHSUut7eGjL8WmLccT7/MXf304WjqHcA== - -"@types/history@*": +"@types/history-4@npm:@types/history@4.7.8", "@types/history-5@npm:@types/history@4.7.8", "@types/history@*": version "4.7.8" resolved "https://registry.yarnpkg.com/@types/history/-/history-4.7.8.tgz#49348387983075705fe8f4e02fb67f7daaec4934" integrity sha512-S78QIYirQcUoo6UJZx9CSP0O2ix9IaeAXwQi26Rhr/+mg7qqPy8TzaxHSUut7eGjL8WmLccT7/MXf304WjqHcA== @@ -10164,15 +10154,7 @@ "@types/history" "^3" "@types/react" "*" -"@types/react-router-4@npm:@types/react-router@5.1.14": - version "5.1.14" - resolved "https://registry.yarnpkg.com/@types/react-router/-/react-router-5.1.14.tgz#e0442f4eb4c446541ad7435d44a97f8fe6df40da" - integrity sha512-LAJpqYUaCTMT2anZheoidiIymt8MuX286zoVFPM3DVb23aQBH0mAkFvzpd4LKqiolV8bBtZWT5Qp7hClCNDENw== - dependencies: - "@types/history" "*" - "@types/react" "*" - -"@types/react-router-5@npm:@types/react-router@5.1.14": +"@types/react-router-4@npm:@types/react-router@5.1.14", "@types/react-router-5@npm:@types/react-router@5.1.14": version "5.1.14" resolved "https://registry.yarnpkg.com/@types/react-router/-/react-router-5.1.14.tgz#e0442f4eb4c446541ad7435d44a97f8fe6df40da" integrity sha512-LAJpqYUaCTMT2anZheoidiIymt8MuX286zoVFPM3DVb23aQBH0mAkFvzpd4LKqiolV8bBtZWT5Qp7hClCNDENw== @@ -20791,10 +20773,10 @@ import-in-the-middle@1.4.2: cjs-module-lexer "^1.2.2" module-details-from-path "^1.0.3" -import-in-the-middle@^1.11.0, import-in-the-middle@^1.8.1: - version "1.11.0" - resolved "https://registry.yarnpkg.com/import-in-the-middle/-/import-in-the-middle-1.11.0.tgz#a94c4925b8da18256cde3b3b7b38253e6ca5e708" - integrity sha512-5DimNQGoe0pLUHbR9qK84iWaWjjbsxiqXnw6Qz64+azRgleqv9k2kTt5fw7QsOpmaGYtuxxursnPPsnTKEx10Q== +import-in-the-middle@^1.11.2, import-in-the-middle@^1.8.1: + version "1.11.2" + resolved "https://registry.yarnpkg.com/import-in-the-middle/-/import-in-the-middle-1.11.2.tgz#dd848e72b63ca6cd7c34df8b8d97fc9baee6174f" + integrity sha512-gK6Rr6EykBcc6cVWRSBR5TWf8nn6hZMYSRYqCcHa0l0d1fPK7JSYo6+Mlmck76jIX9aL/IZ71c06U2VpFwl1zA== dependencies: acorn "^8.8.2" acorn-import-attributes "^1.9.5" @@ -28644,7 +28626,7 @@ react-is@^18.0.0: dependencies: "@remix-run/router" "1.0.2" -"react-router-6@npm:react-router@6.3.0": +"react-router-6@npm:react-router@6.3.0", react-router@6.3.0: version "6.3.0" resolved "https://registry.yarnpkg.com/react-router/-/react-router-6.3.0.tgz#3970cc64b4cb4eae0c1ea5203a80334fdd175557" integrity sha512-7Wh1DzVQ+tlFjkeo+ujvjSqSJmkt1+8JO+T5xklPlgrh70y7ogx75ODRW0ThWhY7S+6yEDks8TYrtQe/aoboBQ== @@ -28659,13 +28641,6 @@ react-router-dom@^6.2.2: history "^5.2.0" react-router "6.3.0" -react-router@6.3.0: - version "6.3.0" - resolved "https://registry.yarnpkg.com/react-router/-/react-router-6.3.0.tgz#3970cc64b4cb4eae0c1ea5203a80334fdd175557" - integrity sha512-7Wh1DzVQ+tlFjkeo+ujvjSqSJmkt1+8JO+T5xklPlgrh70y7ogx75ODRW0ThWhY7S+6yEDks8TYrtQe/aoboBQ== - dependencies: - history "^5.2.0" - react@^18.0.0: version "18.0.0" resolved "https://registry.yarnpkg.com/react/-/react-18.0.0.tgz#b468736d1f4a5891f38585ba8e8fb29f91c3cb96" @@ -31114,16 +31089,7 @@ string-template@~0.2.1: resolved "https://registry.yarnpkg.com/string-template/-/string-template-0.2.1.tgz#42932e598a352d01fc22ec3367d9d84eec6c9add" integrity sha1-QpMuWYo1LQH8IuwzZ9nYTuxsmt0= -"string-width-cjs@npm:string-width@^4.2.0": - version "4.2.3" - resolved "https://registry.yarnpkg.com/string-width/-/string-width-4.2.3.tgz#269c7117d27b05ad2e536830a8ec895ef9c6d010" - integrity sha512-wKyQRQpjJ0sIp62ErSZdGsjMJWsap5oRNihHhu6G7JVO/9jIB6UyevL+tXuOqrng8j/cxKTWyWUwvSTriiZz/g== - dependencies: - emoji-regex "^8.0.0" - is-fullwidth-code-point "^3.0.0" - strip-ansi "^6.0.1" - -string-width@4.2.3, string-width@^4.2.0, string-width@^4.2.2, string-width@^4.2.3: +"string-width-cjs@npm:string-width@^4.2.0", string-width@4.2.3, string-width@^4.2.0, string-width@^4.2.2, string-width@^4.2.3: version "4.2.3" resolved "https://registry.yarnpkg.com/string-width/-/string-width-4.2.3.tgz#269c7117d27b05ad2e536830a8ec895ef9c6d010" integrity sha512-wKyQRQpjJ0sIp62ErSZdGsjMJWsap5oRNihHhu6G7JVO/9jIB6UyevL+tXuOqrng8j/cxKTWyWUwvSTriiZz/g== @@ -31235,14 +31201,7 @@ stringify-object@^3.2.1: is-obj "^1.0.1" is-regexp "^1.0.0" -"strip-ansi-cjs@npm:strip-ansi@^6.0.1": - version "6.0.1" - resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-6.0.1.tgz#9e26c63d30f53443e9489495b2105d37b67a85d9" - integrity sha512-Y38VPSHcqkFrCpFnQ9vuSXmquuv5oXOKpGeT6aGrr3o3Gc9AlVa6JBfUSOCnbxGGZF+/0ooI7KrPuUSztUdU5A== - dependencies: - ansi-regex "^5.0.1" - -strip-ansi@6.0.1, strip-ansi@^6.0.0, strip-ansi@^6.0.1: +"strip-ansi-cjs@npm:strip-ansi@^6.0.1", strip-ansi@6.0.1, strip-ansi@^6.0.0, strip-ansi@^6.0.1: version "6.0.1" resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-6.0.1.tgz#9e26c63d30f53443e9489495b2105d37b67a85d9" integrity sha512-Y38VPSHcqkFrCpFnQ9vuSXmquuv5oXOKpGeT6aGrr3o3Gc9AlVa6JBfUSOCnbxGGZF+/0ooI7KrPuUSztUdU5A== @@ -34259,16 +34218,7 @@ wrangler@^3.67.1: optionalDependencies: fsevents "~2.3.2" -"wrap-ansi-cjs@npm:wrap-ansi@^7.0.0": - version "7.0.0" - resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-7.0.0.tgz#67e145cff510a6a6984bdf1152911d69d2eb9e43" - integrity sha512-YVGIj2kamLSTxw6NsZjoBxfSwsn0ycdesmc4p+Q21c5zPuZ1pl+NfxVdxPtdHvmNVOQ6XSYG4AUtyt/Fi7D16Q== - dependencies: - ansi-styles "^4.0.0" - string-width "^4.1.0" - strip-ansi "^6.0.0" - -wrap-ansi@7.0.0, wrap-ansi@^7.0.0: +"wrap-ansi-cjs@npm:wrap-ansi@^7.0.0", wrap-ansi@7.0.0, wrap-ansi@^7.0.0: version "7.0.0" resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-7.0.0.tgz#67e145cff510a6a6984bdf1152911d69d2eb9e43" integrity sha512-YVGIj2kamLSTxw6NsZjoBxfSwsn0ycdesmc4p+Q21c5zPuZ1pl+NfxVdxPtdHvmNVOQ6XSYG4AUtyt/Fi7D16Q== From 9528b55b5ea89dc61c12c9f0fe8923f2dbb39b5e Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Tue, 8 Oct 2024 14:17:36 +0200 Subject: [PATCH 12/14] small refs --- packages/node/src/integrations/http/index.ts | 152 +++++++++--------- .../node/src/integrations/tracing/index.ts | 3 +- 2 files changed, 80 insertions(+), 75 deletions(-) diff --git a/packages/node/src/integrations/http/index.ts b/packages/node/src/integrations/http/index.ts index aa741a5d2336..9c8d13b58127 100644 --- a/packages/node/src/integrations/http/index.ts +++ b/packages/node/src/integrations/http/index.ts @@ -82,17 +82,13 @@ interface HttpOptions { }; } -const instrumentSentryHttp = generateInstrumentOnce<{ breadcrumbs?: boolean }>( +export const instrumentSentryHttp = generateInstrumentOnce<{ breadcrumbs?: boolean }>( `${INTEGRATION_NAME}.sentry`, options => { return new SentryHttpInstrumentation({ breadcrumbs: options?.breadcrumbs }); }, ); -/** - * We only preload this one. - * If we preload both this and `instrumentSentryHttp`, it leads to weird issues with instrumentation. - */ export const instrumentOtelHttp = generateInstrumentOnce(INTEGRATION_NAME, config => { const instrumentation = new HttpInstrumentation(config); @@ -111,82 +107,18 @@ export const instrumentOtelHttp = generateInstrumentOnce { // This is the "regular" OTEL instrumentation that emits spans if (options.spans !== false) { - const instrumentationConfig = { - ...options.instrumentation?._experimentalConfig, - - disableIncomingRequestInstrumentation: options.disableIncomingRequestSpans, - - ignoreOutgoingRequestHook: request => { - const url = getRequestUrl(request); - - if (!url) { - return false; - } - - const _ignoreOutgoingRequests = options.ignoreOutgoingRequests; - if (_ignoreOutgoingRequests && _ignoreOutgoingRequests(url, request)) { - return true; - } - - return false; - }, - - ignoreIncomingRequestHook: request => { - // request.url is the only property that holds any information about the url - // it only consists of the URL path and query string (if any) - const urlPath = request.url; - - const method = request.method?.toUpperCase(); - // We do not capture OPTIONS/HEAD requests as transactions - if (method === 'OPTIONS' || method === 'HEAD') { - return true; - } - - const _ignoreIncomingRequests = options.ignoreIncomingRequests; - if (urlPath && _ignoreIncomingRequests && _ignoreIncomingRequests(urlPath, request)) { - return true; - } - - return false; - }, - - requireParentforOutgoingSpans: false, - requireParentforIncomingSpans: false, - requestHook: (span, req) => { - addOriginToSpan(span, 'auto.http.otel.http'); - if (!_isClientRequest(req) && isKnownPrefetchRequest(req)) { - span.setAttribute('sentry.http.prefetch', true); - } - - options.instrumentation?.requestHook?.(span, req); - }, - responseHook: (span, res) => { - const client = getClient(); - if (client && client.getOptions().autoSessionTracking) { - setImmediate(() => { - client['_captureRequestSession'](); - }); - } - - options.instrumentation?.responseHook?.(span, res); - }, - applyCustomAttributesOnSpan: ( - span: Span, - request: ClientRequest | HTTPModuleRequestIncomingMessage, - response: HTTPModuleRequestIncomingMessage | ServerResponse, - ) => { - options.instrumentation?.applyCustomAttributesOnSpan?.(span, request, response); - }, - } satisfies HttpInstrumentationConfig; - + const instrumentationConfig = getConfigWithDefaults(options); instrumentOtelHttp(instrumentationConfig); } + // This is the Sentry-specific instrumentation that isolates requests & creates breadcrumbs + // Note that this _has_ to be wrapped after the OTEL instrumentation, + // otherwise the isolation will not work correctly instrumentSentryHttp(options); }; @@ -221,3 +153,75 @@ function isKnownPrefetchRequest(req: HTTPModuleRequestIncomingMessage): boolean // Currently only handles Next.js prefetch requests but may check other frameworks in the future. return req.headers['next-router-prefetch'] === '1'; } + +function getConfigWithDefaults(options: Partial = {}): HttpInstrumentationConfig { + const instrumentationConfig = { + ...options.instrumentation?._experimentalConfig, + + disableIncomingRequestInstrumentation: options.disableIncomingRequestSpans, + + ignoreOutgoingRequestHook: request => { + const url = getRequestUrl(request); + + if (!url) { + return false; + } + + const _ignoreOutgoingRequests = options.ignoreOutgoingRequests; + if (_ignoreOutgoingRequests && _ignoreOutgoingRequests(url, request)) { + return true; + } + + return false; + }, + + ignoreIncomingRequestHook: request => { + // request.url is the only property that holds any information about the url + // it only consists of the URL path and query string (if any) + const urlPath = request.url; + + const method = request.method?.toUpperCase(); + // We do not capture OPTIONS/HEAD requests as transactions + if (method === 'OPTIONS' || method === 'HEAD') { + return true; + } + + const _ignoreIncomingRequests = options.ignoreIncomingRequests; + if (urlPath && _ignoreIncomingRequests && _ignoreIncomingRequests(urlPath, request)) { + return true; + } + + return false; + }, + + requireParentforOutgoingSpans: false, + requireParentforIncomingSpans: false, + requestHook: (span, req) => { + addOriginToSpan(span, 'auto.http.otel.http'); + if (!_isClientRequest(req) && isKnownPrefetchRequest(req)) { + span.setAttribute('sentry.http.prefetch', true); + } + + options.instrumentation?.requestHook?.(span, req); + }, + responseHook: (span, res) => { + const client = getClient(); + if (client && client.getOptions().autoSessionTracking) { + setImmediate(() => { + client['_captureRequestSession'](); + }); + } + + options.instrumentation?.responseHook?.(span, res); + }, + applyCustomAttributesOnSpan: ( + span: Span, + request: ClientRequest | HTTPModuleRequestIncomingMessage, + response: HTTPModuleRequestIncomingMessage | ServerResponse, + ) => { + options.instrumentation?.applyCustomAttributesOnSpan?.(span, request, response); + }, + } satisfies HttpInstrumentationConfig; + + return instrumentationConfig; +} diff --git a/packages/node/src/integrations/tracing/index.ts b/packages/node/src/integrations/tracing/index.ts index 328767c403be..1cc08a973c1a 100644 --- a/packages/node/src/integrations/tracing/index.ts +++ b/packages/node/src/integrations/tracing/index.ts @@ -1,5 +1,5 @@ import type { Integration } from '@sentry/types'; -import { instrumentOtelHttp } from '../http'; +import { instrumentOtelHttp, instrumentSentryHttp } from '../http'; import { amqplibIntegration, instrumentAmqplib } from './amqplib'; import { connectIntegration, instrumentConnect } from './connect'; @@ -55,6 +55,7 @@ export function getAutoPerformanceIntegrations(): Integration[] { export function getOpenTelemetryInstrumentationToPreload(): (((options?: any) => void) & { id: string })[] { return [ instrumentOtelHttp, + instrumentSentryHttp, instrumentExpress, instrumentConnect, instrumentFastify, From e5685aad7d804ef4ff7518e97543497f54c35acd Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Tue, 8 Oct 2024 14:59:55 +0200 Subject: [PATCH 13/14] WIP only preload otel http? --- packages/node/src/integrations/tracing/index.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/node/src/integrations/tracing/index.ts b/packages/node/src/integrations/tracing/index.ts index 1cc08a973c1a..bacc8e397b92 100644 --- a/packages/node/src/integrations/tracing/index.ts +++ b/packages/node/src/integrations/tracing/index.ts @@ -55,7 +55,6 @@ export function getAutoPerformanceIntegrations(): Integration[] { export function getOpenTelemetryInstrumentationToPreload(): (((options?: any) => void) & { id: string })[] { return [ instrumentOtelHttp, - instrumentSentryHttp, instrumentExpress, instrumentConnect, instrumentFastify, From d0aac35664283cc3a9cc31727e73ec0c1c97401d Mon Sep 17 00:00:00 2001 From: Francesco Novy Date: Tue, 8 Oct 2024 16:46:11 +0200 Subject: [PATCH 14/14] fix linting --- packages/node/src/integrations/tracing/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/node/src/integrations/tracing/index.ts b/packages/node/src/integrations/tracing/index.ts index bacc8e397b92..328767c403be 100644 --- a/packages/node/src/integrations/tracing/index.ts +++ b/packages/node/src/integrations/tracing/index.ts @@ -1,5 +1,5 @@ import type { Integration } from '@sentry/types'; -import { instrumentOtelHttp, instrumentSentryHttp } from '../http'; +import { instrumentOtelHttp } from '../http'; import { amqplibIntegration, instrumentAmqplib } from './amqplib'; import { connectIntegration, instrumentConnect } from './connect';