diff --git a/dev-packages/node-integration-tests/suites/tracing/connect/test.ts b/dev-packages/node-integration-tests/suites/tracing/connect/test.ts index 97d9d3375e2a..fa50ca028ab8 100644 --- a/dev-packages/node-integration-tests/suites/tracing/connect/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/connect/test.ts @@ -97,6 +97,12 @@ describe('connect auto-instrumentation', () => { origin: 'auto.http.otel.connect', op: 'middleware.connect', }), + + expect.objectContaining({ + description: '/error', + op: 'request_handler.connect', + status: 'internal_error', + }), ]), }, }) diff --git a/packages/node/src/integrations/tracing/connect/index.ts b/packages/node/src/integrations/tracing/connect/index.ts index b8f5322b087d..0c1ad31bcdfc 100644 --- a/packages/node/src/integrations/tracing/connect/index.ts +++ b/packages/node/src/integrations/tracing/connect/index.ts @@ -1,13 +1,6 @@ import { ConnectInstrumentation } from './vendored/instrumentation'; -import type { IntegrationFn, Span } from '@sentry/core'; -import { - captureException, - defineIntegration, - getClient, - SEMANTIC_ATTRIBUTE_SENTRY_OP, - SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, - spanToJSON, -} from '@sentry/core'; +import type { IntegrationFn } from '@sentry/core'; +import { captureException, defineIntegration } from '@sentry/core'; import { ensureIsWrapped, generateInstrumentOnce } from '@sentry/node-core'; type ConnectApp = { @@ -78,39 +71,5 @@ function connectErrorMiddleware(err: any, req: any, res: any, next: any): void { */ export const setupConnectErrorHandler = (app: ConnectApp): void => { app.use(connectErrorMiddleware); - - // Sadly, ConnectInstrumentation has no requestHook, so we need to add the attributes here - // We register this hook in this method, because if we register it in the integration `setup`, - // it would always run even for users that are not even using connect - const client = getClient(); - if (client) { - client.on('spanStart', span => { - addConnectSpanAttributes(span); - }); - } - ensureIsWrapped(app.use, 'connect'); }; - -function addConnectSpanAttributes(span: Span): void { - const attributes = spanToJSON(span).data; - - // this is one of: middleware, request_handler - const type = attributes['connect.type']; - - // If this is already set, or we have no connect span, no need to process again... - if (attributes[SEMANTIC_ATTRIBUTE_SENTRY_OP] || !type) { - return; - } - - span.setAttributes({ - [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.otel.connect', - [SEMANTIC_ATTRIBUTE_SENTRY_OP]: `${type}.connect`, - }); - - // Also update the name, we don't need the "middleware - " prefix - const name = attributes['connect.name']; - if (typeof name === 'string') { - span.updateName(name); - } -} diff --git a/packages/node/src/integrations/tracing/connect/vendored/enums/AttributeNames.ts b/packages/node/src/integrations/tracing/connect/vendored/enums/AttributeNames.ts index 7ad986df3608..81aebb26d227 100644 --- a/packages/node/src/integrations/tracing/connect/vendored/enums/AttributeNames.ts +++ b/packages/node/src/integrations/tracing/connect/vendored/enums/AttributeNames.ts @@ -27,8 +27,3 @@ export enum ConnectTypes { MIDDLEWARE = 'middleware', REQUEST_HANDLER = 'request_handler', } - -export enum ConnectNames { - MIDDLEWARE = 'middleware', - REQUEST_HANDLER = 'request handler', -} diff --git a/packages/node/src/integrations/tracing/connect/vendored/instrumentation.ts b/packages/node/src/integrations/tracing/connect/vendored/instrumentation.ts index 4949df131839..16d3cf4b7808 100644 --- a/packages/node/src/integrations/tracing/connect/vendored/instrumentation.ts +++ b/packages/node/src/integrations/tracing/connect/vendored/instrumentation.ts @@ -19,11 +19,11 @@ * - Minor TypeScript strictness adjustments for this repository's compiler settings */ -import type { Span, SpanOptions } from '@opentelemetry/api'; import type { ServerResponse } from 'http'; -import { AttributeNames, ConnectNames, ConnectTypes } from './enums/AttributeNames'; +import { AttributeNames, ConnectTypes } from './enums/AttributeNames'; import type { HandleFunction, NextFunction, PatchedRequest, Server, Use, UseArgs, UseArgs2 } from './internal-types'; -import { SDK_VERSION } from '@sentry/core'; +import type { Span } from '@sentry/core'; +import { SDK_VERSION, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SPAN_STATUS_ERROR, startInactiveSpan } from '@sentry/core'; import { setHttpServerSpanRouteAttribute } from '../../../../utils/setHttpServerSpanRouteAttribute'; import type { InstrumentationConfig } from '@opentelemetry/instrumentation'; import { InstrumentationBase, InstrumentationNodeModuleDefinition, isWrapped } from '@opentelemetry/instrumentation'; @@ -68,8 +68,11 @@ export class ConnectInstrumentation extends InstrumentationBase { }; } - public _patchNext(next: NextFunction, finishSpan: () => void): NextFunction { + public _patchNext(next: NextFunction, span: Span, finishSpan: () => void): NextFunction { return function nextFunction(this: NextFunction, err?: unknown): void { + if (err) { + span.setStatus({ code: SPAN_STATUS_ERROR, message: 'internal_error' }); + } const result = next.apply(this, [err]); finishSpan(); return result; @@ -77,28 +80,18 @@ export class ConnectInstrumentation extends InstrumentationBase { } public _startSpan(routeName: string, middleWare: HandleFunction): Span { - let connectType: ConnectTypes; - let connectName: string; - let connectTypeName: string; - if (routeName) { - connectType = ConnectTypes.REQUEST_HANDLER; - connectTypeName = ConnectNames.REQUEST_HANDLER; - connectName = routeName; - } else { - connectType = ConnectTypes.MIDDLEWARE; - connectTypeName = ConnectNames.MIDDLEWARE; - connectName = middleWare.name || ANONYMOUS_NAME; - } - const spanName = `${connectTypeName} - ${connectName}`; - const options: SpanOptions = { + const connectType = routeName ? ConnectTypes.REQUEST_HANDLER : ConnectTypes.MIDDLEWARE; + const connectName = routeName || middleWare.name || ANONYMOUS_NAME; + return startInactiveSpan({ + name: connectName, + op: `${connectType}.connect`, attributes: { + [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.otel.connect', [ATTR_HTTP_ROUTE]: routeName.length > 0 ? routeName : '/', [AttributeNames.CONNECT_TYPE]: connectType, [AttributeNames.CONNECT_NAME]: connectName, }, - }; - - return this.tracer.startSpan(spanName, options); + }); } public _patchMiddleware(routeName: string, middleWare: HandleFunction): HandleFunction { @@ -134,9 +127,15 @@ export class ConnectInstrumentation extends InstrumentationBase { } res.addListener('close', finishSpan); - arguments[nextArgIdx] = patchNext(next, finishSpan); + arguments[nextArgIdx] = patchNext(next, span, finishSpan); - return Reflect.apply(middleWare, this, arguments); + try { + return Reflect.apply(middleWare, this, arguments); + } catch (e) { + span.setStatus({ code: SPAN_STATUS_ERROR, message: 'internal_error' }); + finishSpan(); + throw e; + } } Object.defineProperty(patchedMiddleware, 'length', { diff --git a/packages/node/src/integrations/tracing/connect/vendored/utils.ts b/packages/node/src/integrations/tracing/connect/vendored/utils.ts index e4bb26025388..74393ea17275 100644 --- a/packages/node/src/integrations/tracing/connect/vendored/utils.ts +++ b/packages/node/src/integrations/tracing/connect/vendored/utils.ts @@ -18,9 +18,10 @@ * - Upstream version: @opentelemetry/instrumentation-connect@0.61.0 */ -import { diag } from '@opentelemetry/api'; +import { debug } from '@sentry/core'; import type { PatchedRequest } from './internal-types'; import { _LAYERS_STORE_PROPERTY } from './internal-types'; +import { DEBUG_BUILD } from '../../../../debug-build'; export const addNewStackLayer = (request: PatchedRequest) => { if (Array.isArray(request[_LAYERS_STORE_PROPERTY]) === false) { @@ -37,7 +38,7 @@ export const addNewStackLayer = (request: PatchedRequest) => { if (stackLength === request[_LAYERS_STORE_PROPERTY].length) { request[_LAYERS_STORE_PROPERTY].pop(); } else { - diag.warn('Connect: Trying to pop the stack multiple time'); + DEBUG_BUILD && debug.warn('Connect: Trying to pop the stack multiple time'); } }; };