From a11202f0126d3ed76fcff5ce2b21df6796775e0c Mon Sep 17 00:00:00 2001 From: Abdelrahman Awad Date: Thu, 18 Jun 2026 13:46:51 -0400 Subject: [PATCH] feat(tracing-channel): Adopt bindTracingChannelToSpan in Nitro and redis Migrate the Nitro HTTP/storage channels and the node/deno redis diagnostics_channel subscribers off the `@sentry/opentelemetry` `tracingChannel` helper onto `bindTracingChannelToSpan` from `@sentry/server-utils`, using the `auto` lifecycle. - Add `beforeSpanEnd(span, data)` to enrich the span on the canonical terminal event (sync `end` / async `asyncEnd`) right before it ends. - Add `captureError` (default `true`) to gate exception capture; the span error status is always set. Redis opts out (`captureError: false`) since command failures are captured at the boundary that owns them. - Unify the error mechanism to `auto.diagnostic_channels.bind_span`. - Create channel spans with `startInactiveSpan`; the binding activates the span as the async context, so parenting is preserved. - node and deno redis now pass the native `node:diagnostics_channel` `tracingChannel` directly; deno's bespoke factory is removed. BREAKING: remove the `@sentry/opentelemetry/tracing-channel` subpath export (shipped in 10.58.0); use `bindTracingChannelToSpan` instead. --- .../nitro-3/tests/errors.test.ts | 2 +- packages/deno/src/integrations/redis.ts | 47 +- packages/nitro/package.json | 2 +- .../src/runtime/hooks/captureStorageEvents.ts | 58 +-- .../src/runtime/hooks/captureTracingEvents.ts | 205 ++++---- .../src/integrations/tracing/redis/index.ts | 16 +- packages/opentelemetry/package.json | 10 - packages/opentelemetry/rollup.npm.config.mjs | 4 +- packages/opentelemetry/src/tracingChannel.ts | 93 ---- .../opentelemetry/test/tracingChannel.test.ts | 251 ---------- packages/server-utils/src/index.ts | 3 - .../src/redis/redis-dc-subscriber.ts | 194 +++----- .../test/redis/redis-dc-subscriber.test.ts | 456 ++++++++---------- 13 files changed, 405 insertions(+), 936 deletions(-) delete mode 100644 packages/opentelemetry/src/tracingChannel.ts delete mode 100644 packages/opentelemetry/test/tracingChannel.test.ts diff --git a/dev-packages/e2e-tests/test-applications/nitro-3/tests/errors.test.ts b/dev-packages/e2e-tests/test-applications/nitro-3/tests/errors.test.ts index 673a6e293fab..36a6edacd981 100644 --- a/dev-packages/e2e-tests/test-applications/nitro-3/tests/errors.test.ts +++ b/dev-packages/e2e-tests/test-applications/nitro-3/tests/errors.test.ts @@ -17,7 +17,7 @@ test('Sends an error event to Sentry', async ({ request }) => { expect(errorEvent.exception?.values?.[0]?.mechanism).toEqual( expect.objectContaining({ handled: false, - type: 'auto.http.nitro.onTraceError', + type: 'auto.diagnostic_channels.bind_span', }), ); }); diff --git a/packages/deno/src/integrations/redis.ts b/packages/deno/src/integrations/redis.ts index ec3f8a60b241..a21b940aa372 100644 --- a/packages/deno/src/integrations/redis.ts +++ b/packages/deno/src/integrations/redis.ts @@ -2,14 +2,9 @@ // lacking `tracingChannel` (added in Deno 1.44.3). // On older runtimes the integration becomes a no-op. import * as dc from 'node:diagnostics_channel'; -import type { - RedisDiagnosticChannelResponseHook, - RedisTracingChannel, - RedisTracingChannelFactory, - RedisTracingChannelSubscribers, -} from '@sentry/server-utils'; +import type { RedisDiagnosticChannelResponseHook, RedisTracingChannelFactory } from '@sentry/server-utils'; import { subscribeRedisDiagnosticChannels } from '@sentry/server-utils'; -import type { Integration, IntegrationFn, Span } from '@sentry/core'; +import type { Integration, IntegrationFn } from '@sentry/core'; import { defineIntegration } from '@sentry/core'; import { setAsyncLocalStorageAsyncContextStrategy } from '../async'; @@ -23,40 +18,6 @@ export interface DenoRedisIntegrationOptions { responseHook?: RedisDiagnosticChannelResponseHook; } -/** - * Portable tracing-channel factory: wraps `node:diagnostics_channel.tracingChannel` - * and stamps `data._sentrySpan` from `transformStart` in the `start` subscriber. - * - * Unlike `@sentry/opentelemetry/tracing-channel`, this does not call `bindStore` - */ -type DataWithSpan = T & { _sentrySpan?: Span }; -type SubscriberFn = (data: DataWithSpan) => void; - -const portableTracingChannel: RedisTracingChannelFactory = ( - name: string, - transformStart: (data: T) => Span, -): RedisTracingChannel => { - const channel = dc.tracingChannel>(name); - return { - subscribe(subs: Partial>): void { - const userStart = subs.start as SubscriberFn | undefined; - const composed: Record> = { - start(data) { - data._sentrySpan = transformStart(data); - userStart?.(data); - }, - }; - for (const event of ['asyncStart', 'asyncEnd', 'end', 'error'] as const) { - const fn = subs[event] as SubscriberFn | undefined; - if (fn) composed[event] = fn; - } - // Native subscribe is typed for the full subscriber set, but only the - // handlers actually present are invoked at runtime. - channel.subscribe(composed as unknown as Parameters[0]); - }, - }; -}; - const _denoRedisIntegration = ((options: DenoRedisIntegrationOptions = {}) => { return { name: INTEGRATION_NAME, @@ -64,8 +25,10 @@ const _denoRedisIntegration = ((options: DenoRedisIntegrationOptions = {}) => { if (!dc.tracingChannel) { return; } + // The span is bound into Deno's AsyncLocalStorage context via the async-context + // strategy's `getTracingChannelBinding`, so the native channel can be passed directly. setAsyncLocalStorageAsyncContextStrategy(); - subscribeRedisDiagnosticChannels(portableTracingChannel, options.responseHook); + subscribeRedisDiagnosticChannels(dc.tracingChannel as RedisTracingChannelFactory, options.responseHook); }, }; }) satisfies IntegrationFn; diff --git a/packages/nitro/package.json b/packages/nitro/package.json index 9f4ae19da01e..98318da7b01f 100644 --- a/packages/nitro/package.json +++ b/packages/nitro/package.json @@ -38,7 +38,7 @@ "@sentry/bundler-plugin-core": "^5.3.0", "@sentry/core": "10.59.0", "@sentry/node": "10.59.0", - "@sentry/opentelemetry": "10.59.0" + "@sentry/server-utils": "10.59.0" }, "devDependencies": { "nitro": "^3.0.260415-beta", diff --git a/packages/nitro/src/runtime/hooks/captureStorageEvents.ts b/packages/nitro/src/runtime/hooks/captureStorageEvents.ts index 054fe6873ce3..838c8dfc5c02 100644 --- a/packages/nitro/src/runtime/hooks/captureStorageEvents.ts +++ b/packages/nitro/src/runtime/hooks/captureStorageEvents.ts @@ -1,16 +1,15 @@ +import { tracingChannel } from 'node:diagnostics_channel'; import { - captureException, flushIfServerless, GLOBAL_OBJ, SEMANTIC_ATTRIBUTE_CACHE_HIT, SEMANTIC_ATTRIBUTE_CACHE_KEY, SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, - SPAN_STATUS_ERROR, SPAN_STATUS_OK, - startSpanManual, + startInactiveSpan, } from '@sentry/core'; -import { tracingChannel, type TracingChannelContextWithSpan } from '@sentry/opentelemetry/tracing-channel'; +import { bindTracingChannelToSpan } from '@sentry/server-utils'; import type { TraceContext } from 'unstorage/tracing'; const ORIGIN = 'auto.cache.nitro'; @@ -57,11 +56,12 @@ function setupStorageTracingChannel(operation: TracedOperation): void { const keys = (data: TraceContext): string[] => data.keys ?? []; const mountBase = (data: TraceContext): string => (data.base ?? '').replace(/:$/, ''); - const channel = tracingChannel(`unstorage.${operation}`, data => { - const cacheKeys = keys(data); + bindTracingChannelToSpan( + tracingChannel(`unstorage.${operation}`), + data => { + const cacheKeys = keys(data); - return startSpanManual( - { + return startInactiveSpan({ name: cacheKeys.join(', ') || operation, attributes: { [SEMANTIC_ATTRIBUTE_SENTRY_OP]: `cache.${normalizeMethodName(operation)}`, @@ -71,34 +71,24 @@ function setupStorageTracingChannel(operation: TracedOperation): void { 'db.collection.name': mountBase(data), 'db.system.name': data.driver?.name ?? 'unknown', }, - }, - span => span, - ); - }); - - channel.subscribe({ - asyncEnd(data: TracingChannelContextWithSpan) { - if (data._sentrySpan && CACHE_HIT_OPERATIONS.has(operation)) { - const hit = operation === 'hasItem' ? Boolean(data.result) : isCacheHit(data.keys?.[0], data.result); - data._sentrySpan.setAttribute(SEMANTIC_ATTRIBUTE_CACHE_HIT, hit); - } - - data._sentrySpan?.setStatus({ code: SPAN_STATUS_OK }); - data._sentrySpan?.end(); - - void flushIfServerless(); - }, - error(data: TracingChannelContextWithSpan) { - captureException(data.error, { - mechanism: { handled: false, type: ORIGIN }, }); - - data._sentrySpan?.setStatus({ code: SPAN_STATUS_ERROR, message: 'internal_error' }); - data._sentrySpan?.end(); - - void flushIfServerless(); }, - }); + { + beforeSpanEnd(span, data) { + // Auto-capture + error status is handled by the binding; only enrich the success path. + if (!('error' in data)) { + const result = (data as { result?: unknown }).result; + if (CACHE_HIT_OPERATIONS.has(operation)) { + const hit = operation === 'hasItem' ? Boolean(result) : isCacheHit(data.keys?.[0], result); + span.setAttribute(SEMANTIC_ATTRIBUTE_CACHE_HIT, hit); + } + span.setStatus({ code: SPAN_STATUS_OK }); + } + + void flushIfServerless(); + }, + }, + ); } function normalizeMethodName(methodName: string): string { diff --git a/packages/nitro/src/runtime/hooks/captureTracingEvents.ts b/packages/nitro/src/runtime/hooks/captureTracingEvents.ts index 2dcf2a97f6d4..8e052d5a4acd 100644 --- a/packages/nitro/src/runtime/hooks/captureTracingEvents.ts +++ b/packages/nitro/src/runtime/hooks/captureTracingEvents.ts @@ -1,5 +1,5 @@ +import { tracingChannel } from 'node:diagnostics_channel'; import { - captureException, getActiveSpan, getClient, getHttpSpanDetailsFromUrlObject, @@ -12,11 +12,10 @@ import { SEMANTIC_ATTRIBUTE_SENTRY_SOURCE, setHttpStatus, type Span, - SPAN_STATUS_ERROR, - startSpanManual, + startInactiveSpan, updateSpanName, } from '@sentry/core'; -import { tracingChannel, type TracingChannelContextWithSpan } from '@sentry/opentelemetry/tracing-channel'; +import { bindTracingChannelToSpan, type TracingChannelPayloadWithSpan } from '@sentry/server-utils'; import type { TracingRequestEvent as H3TracingRequestEvent } from 'h3/tracing'; import type { RequestEvent as SrvxRequestEvent } from 'srvx/tracing'; import { setServerTimingHeaders } from './setServerTimingHeaders'; @@ -53,19 +52,12 @@ function getResponseStatusCode(result: unknown): number | undefined { return undefined; } -function onTraceEnd(data: TracingChannelContextWithSpan<{ result?: unknown }>): void { +/** Applies the HTTP status from the traced handler's result to the span, when present. */ +function applyResponseStatus(span: Span, data: TracingChannelPayloadWithSpan<{ result?: unknown }>): void { const statusCode = getResponseStatusCode(data.result); - if (data._sentrySpan && statusCode !== undefined) { - setHttpStatus(data._sentrySpan, statusCode); + if (statusCode !== undefined) { + setHttpStatus(span, statusCode); } - - data._sentrySpan?.end(); -} - -function onTraceError(data: TracingChannelContextWithSpan<{ error: unknown }>): void { - captureException(data.error, { mechanism: { type: 'auto.http.nitro.onTraceError', handled: false } }); - data._sentrySpan?.setStatus({ code: SPAN_STATUS_ERROR, message: 'internal_error' }); - data._sentrySpan?.end(); } /** @@ -88,64 +80,61 @@ function getParameterizedRoute(event: H3TracingRequestEvent['event']): string | } function setupH3TracingChannels(): void { - const h3Channel = tracingChannel('h3.request', data => { - const parsedUrl = parseStringToURLObject(data.event.url.href); - const routePattern = getParameterizedRoute(data.event); - - const [spanName, urlAttributes] = getHttpSpanDetailsFromUrlObject( - parsedUrl, - 'server', - 'auto.http.nitro.h3', - { method: data.event.req.method }, - routePattern, - ); - - return startSpanManual( - { + const { channel: h3Channel } = bindTracingChannelToSpan( + tracingChannel('h3.request'), + data => { + const parsedUrl = parseStringToURLObject(data.event.url.href); + const routePattern = getParameterizedRoute(data.event); + + const [spanName, urlAttributes] = getHttpSpanDetailsFromUrlObject( + parsedUrl, + 'server', + 'auto.http.nitro.h3', + { method: data.event.req.method }, + routePattern, + ); + + const span = startInactiveSpan({ name: spanName, attributes: { ...urlAttributes, [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.nitro.h3', [SEMANTIC_ATTRIBUTE_SENTRY_OP]: data?.type === 'middleware' ? 'middleware.nitro' : 'http.server', }, - }, - span => { - setParameterizedRouteAttributes(span, data.event); + }); + + setParameterizedRouteAttributes(span, data.event); - return span; + return span; + }, + { + beforeSpanEnd(span, data) { + applyResponseStatus(span, data); + + // Update the root span (srvx transaction) with the parameterized route name. + // The srvx span is created before h3 resolves the route, so it initially has the raw URL. + // Note: data.type is always 'middleware' here regardless of handler type, so we rely on + // getParameterizedRoute() to filter out catch-all routes instead. + const rootSpan = getRootSpan(span); + if (rootSpan && rootSpan !== span) { + const routePattern = getParameterizedRoute(data.event); + if (routePattern) { + const method = data.event.req.method || 'GET'; + updateSpanName(rootSpan, `${method} ${routePattern}`); + rootSpan.setAttributes({ + [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', + 'http.route': routePattern, + }); + } + } }, - ); - }); + }, + ); h3Channel.subscribe({ start: (data: H3TracingRequestEvent) => { setServerTimingHeaders(data.event); }, - asyncEnd: (data: TracingChannelContextWithSpan) => { - onTraceEnd(data); - - if (!data._sentrySpan) { - return; - } - - // Update the root span (srvx transaction) with the parameterized route name. - // The srvx span is created before h3 resolves the route, so it initially has the raw URL. - // Note: data.type is always 'middleware' in asyncEnd regardless of handler type, - // so we rely on getParameterizedRoute() to filter out catch-all routes instead. - const rootSpan = getRootSpan(data._sentrySpan); - if (rootSpan && rootSpan !== data._sentrySpan) { - const routePattern = getParameterizedRoute(data.event); - if (routePattern) { - const method = data.event.req.method || 'GET'; - updateSpanName(rootSpan, `${method} ${routePattern}`); - rootSpan.setAttributes({ - [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'route', - 'http.route': routePattern, - }); - } - } - }, - error: onTraceError, }); } @@ -154,19 +143,20 @@ function setupSrvxTracingChannels(): void { // WeakMap ensures per-request isolation in concurrent environments and automatic cleanup. const requestParentSpans = new WeakMap(); - const fetchChannel = tracingChannel('srvx.request', data => { - const parsedUrl = data.request._url ? parseStringToURLObject(data.request._url.href) : undefined; - const [spanName, urlAttributes] = getHttpSpanDetailsFromUrlObject(parsedUrl, 'server', 'auto.http.nitro.srvx', { - method: data.request.method, - }); + bindTracingChannelToSpan( + tracingChannel('srvx.request'), + data => { + const parsedUrl = data.request._url ? parseStringToURLObject(data.request._url.href) : undefined; + const [spanName, urlAttributes] = getHttpSpanDetailsFromUrlObject(parsedUrl, 'server', 'auto.http.nitro.srvx', { + method: data.request.method, + }); - const headerAttributes = httpHeadersToSpanAttributes( - Object.fromEntries(data.request.headers.entries()), - getClient()?.getDataCollectionOptions() ?? false, - ); + const headerAttributes = httpHeadersToSpanAttributes( + Object.fromEntries(data.request.headers.entries()), + getClient()?.getDataCollectionOptions() ?? false, + ); - return startSpanManual( - { + return startInactiveSpan({ name: spanName, attributes: { ...urlAttributes, @@ -177,43 +167,36 @@ function setupSrvxTracingChannels(): void { }, // Use the same parent span as middleware to make them siblings parentSpan: requestParentSpans.get(data.request) || undefined, - }, - span => span, - ); - }); - - // Subscribe to events (span already created in bindStore) - fetchChannel.subscribe({ - asyncEnd: data => { - onTraceEnd(data); - - // Clean up parent span reference after the fetch handler completes. - requestParentSpans.delete(data.request); - }, - error: data => { - onTraceError(data); - // Clean up parent span reference on error too - requestParentSpans.delete(data.request); + }); }, - }); + { + beforeSpanEnd(span, data) { + applyResponseStatus(span, data); - const middlewareChannel = tracingChannel('srvx.middleware', data => { - // For the first middleware, capture the current parent span per-request - if (data.middleware?.index === 0) { - const activeSpan = getActiveSpan(); - if (activeSpan) { - requestParentSpans.set(data.request, activeSpan); + // Clean up parent span reference after the fetch handler completes (success or error). + requestParentSpans.delete(data.request); + }, + }, + ); + + bindTracingChannelToSpan( + tracingChannel('srvx.middleware'), + data => { + // For the first middleware, capture the current parent span per-request + if (data.middleware?.index === 0) { + const activeSpan = getActiveSpan(); + if (activeSpan) { + requestParentSpans.set(data.request, activeSpan); + } } - } - const parsedUrl = data.request._url ? parseStringToURLObject(data.request._url.href) : undefined; - const [, urlAttributes] = getHttpSpanDetailsFromUrlObject(parsedUrl, 'server', 'auto.http.nitro.srvx', { - method: data.request.method, - }); + const parsedUrl = data.request._url ? parseStringToURLObject(data.request._url.href) : undefined; + const [, urlAttributes] = getHttpSpanDetailsFromUrlObject(parsedUrl, 'server', 'auto.http.nitro.srvx', { + method: data.request.method, + }); - // Create span as a child of the original parent, not the previous middleware - return startSpanManual( - { + // Create span as a child of the original parent, not the previous middleware + return startInactiveSpan({ name: `${data.middleware?.handler.name ?? 'unknown'} - ${data.request.method} ${data.request._url?.pathname}`, attributes: { ...urlAttributes, @@ -221,16 +204,14 @@ function setupSrvxTracingChannels(): void { [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'middleware.nitro', }, parentSpan: requestParentSpans.get(data.request) || undefined, + }); + }, + { + beforeSpanEnd(span, data) { + applyResponseStatus(span, data); }, - span => span, - ); - }); - - // Subscribe to events (span already created in bindStore) - middlewareChannel.subscribe({ - asyncEnd: onTraceEnd, - error: onTraceError, - }); + }, + ); } /** diff --git a/packages/node/src/integrations/tracing/redis/index.ts b/packages/node/src/integrations/tracing/redis/index.ts index 64e29eed53b9..92c33ce37990 100644 --- a/packages/node/src/integrations/tracing/redis/index.ts +++ b/packages/node/src/integrations/tracing/redis/index.ts @@ -8,9 +8,9 @@ import { spanToJSON, truncate, } from '@sentry/core'; -import { subscribeRedisDiagnosticChannels } from '@sentry/server-utils'; +import { tracingChannel } from 'node:diagnostics_channel'; +import { subscribeRedisDiagnosticChannels, type RedisTracingChannelFactory } from '@sentry/server-utils'; import { generateInstrumentOnce } from '@sentry/node-core'; -import { tracingChannel as otelTracingChannel } from '@sentry/opentelemetry/tracing-channel'; import type { IORedisCommandArgs } from '../../../utils/redisCache'; import { calculateCacheItemSize, @@ -122,11 +122,13 @@ export const instrumentRedis = Object.assign( instrumentIORedis(); instrumentRedisModule(); // node-redis >= 5.12.0 and ioredis >= 5.11.0 publish via diagnostics_channel. - // We pass `@sentry/opentelemetry/tracing-channel` as the factory so the span - // becomes the active OTel context via `bindStore`. That factory needs the - // Sentry OTel context manager to be registered, which `initOpenTelemetry()` - // does after integration `setupOnce`, so defer to the next tick. - void Promise.resolve().then(() => subscribeRedisDiagnosticChannels(otelTracingChannel, cacheResponseHook)); + // `bindTracingChannelToSpan` (inside the subscriber) makes the span the active + // OTel context via `bindStore`, which needs the Sentry OTel context manager to + // be registered — `initOpenTelemetry()` does that after integration `setupOnce`, + // so defer to the next tick. + void Promise.resolve().then(() => + subscribeRedisDiagnosticChannels(tracingChannel as RedisTracingChannelFactory, cacheResponseHook), + ); // todo: implement them gradually // new LegacyRedisInstrumentation({}), diff --git a/packages/opentelemetry/package.json b/packages/opentelemetry/package.json index 22465bffa85c..3e7b10b0625b 100644 --- a/packages/opentelemetry/package.json +++ b/packages/opentelemetry/package.json @@ -42,16 +42,6 @@ "types": "./build/types/index.d.ts", "default": "./build/cjs/index.js" } - }, - "./tracing-channel": { - "import": { - "types": "./build/types/tracingChannel.d.ts", - "default": "./build/esm/tracingChannel.js" - }, - "require": { - "types": "./build/types/tracingChannel.d.ts", - "default": "./build/cjs/tracingChannel.js" - } } }, "typesVersions": { diff --git a/packages/opentelemetry/rollup.npm.config.mjs b/packages/opentelemetry/rollup.npm.config.mjs index 593c4fe0e1a0..e694fb8d1001 100644 --- a/packages/opentelemetry/rollup.npm.config.mjs +++ b/packages/opentelemetry/rollup.npm.config.mjs @@ -2,9 +2,7 @@ import { makeBaseNPMConfig, makeNPMConfigVariants } from '@sentry-internal/rollu export default makeNPMConfigVariants( makeBaseNPMConfig({ - // `tracingChannel` is a Node.js-only subpath so `node:diagnostics_channel` - // isn't pulled into the main bundle (breaks edge/browser builds). - entrypoints: ['src/index.ts', 'src/tracingChannel.ts', 'src/index.browser.ts'], + entrypoints: ['src/index.ts', 'src/index.browser.ts'], packageSpecificConfig: { output: { // set exports to 'named' or 'auto' so that rollup doesn't warn diff --git a/packages/opentelemetry/src/tracingChannel.ts b/packages/opentelemetry/src/tracingChannel.ts deleted file mode 100644 index 88d0d384c58f..000000000000 --- a/packages/opentelemetry/src/tracingChannel.ts +++ /dev/null @@ -1,93 +0,0 @@ -/** - * Vendored and adapted from https://github.com/logaretm/otel-tracing-channel - * - * Creates a TracingChannel with proper OpenTelemetry context propagation - * using Node.js diagnostic_channel's `bindStore` mechanism. - */ -import type { TracingChannel, TracingChannelSubscribers } from 'node:diagnostics_channel'; -import * as diagnosticsChannel from 'node:diagnostics_channel'; -import type { Span } from '@opentelemetry/api'; -import { context, trace } from '@opentelemetry/api'; -import { logger } from '@sentry/core'; -import type { SentryAsyncLocalStorageContextManager } from './asyncLocalStorageContextManager'; -import type { AsyncLocalStorageLookup } from './contextManager'; -import { DEBUG_BUILD } from './debug-build'; - -/** - * Transform function that creates a span from the channel data. - */ -export type OtelTracingChannelTransform = (data: TData) => Span; - -export type TracingChannelContextWithSpan = TContext & { _sentrySpan?: Span }; - -/** - * A TracingChannel whose `subscribe` / `unsubscribe` accept partial subscriber - * objects — you only need to provide handlers for the events you care about. - */ -export interface OtelTracingChannel< - TData extends object = object, - TDataWithSpan extends object = TracingChannelContextWithSpan, -> extends Omit, 'subscribe' | 'unsubscribe'> { - subscribe(subscribers: Partial>): void; - unsubscribe(subscribers: Partial>): void; -} - -interface ContextApi { - _getContextManager(): SentryAsyncLocalStorageContextManager; -} - -/** - * Creates a new tracing channel with proper OTel context propagation. - * - * When the channel's `tracePromise` / `traceSync` / `traceCallback` is called, - * the `transformStart` function runs inside `bindStore` so that: - * 1. A new span is created from the channel data. - * 2. The span is set on the OTel context stored in AsyncLocalStorage. - * 3. Downstream code (including Sentry's span processor) sees the correct parent. - * - * @param channelNameOrInstance - Either a channel name string or an existing TracingChannel instance. - * @param transformStart - Function that creates an OpenTelemetry span from the channel data. - * @returns The tracing channel with OTel context bound. - */ -export function tracingChannel( - channelNameOrInstance: string, - transformStart: OtelTracingChannelTransform, -): OtelTracingChannel> { - const channel = diagnosticsChannel.tracingChannel< - TracingChannelContextWithSpan, - TracingChannelContextWithSpan - >(channelNameOrInstance) as unknown as OtelTracingChannel>; - - let lookup: AsyncLocalStorageLookup | undefined; - try { - const contextManager = (context as unknown as ContextApi)._getContextManager(); - lookup = contextManager.getAsyncLocalStorageLookup(); - } catch { - // getAsyncLocalStorageLookup may not exist if using a non-Sentry context manager - } - - if (!lookup) { - DEBUG_BUILD && - logger.warn( - '[TracingChannel] Could not access OpenTelemetry AsyncLocalStorage, context propagation will not work.', - ); - return channel; - } - - const otelStorage = lookup.asyncLocalStorage; - - // Bind the start channel so that each trace invocation runs the transform - // and stores the resulting context (with span) in AsyncLocalStorage. - // @ts-expect-error bindStore types don't account for AsyncLocalStorage of a different generic type - channel.start.bindStore(otelStorage, (data: TracingChannelContextWithSpan) => { - const span = transformStart(data); - - // Store the span on data so downstream event handlers (asyncEnd, error, etc.) can access it. - data._sentrySpan = span; - - // Return the context with the span set — this is what gets stored in AsyncLocalStorage. - return trace.setSpan(context.active(), span); - }); - - return channel; -} diff --git a/packages/opentelemetry/test/tracingChannel.test.ts b/packages/opentelemetry/test/tracingChannel.test.ts deleted file mode 100644 index 2b5f72327352..000000000000 --- a/packages/opentelemetry/test/tracingChannel.test.ts +++ /dev/null @@ -1,251 +0,0 @@ -import { context, trace } from '@opentelemetry/api'; -import type { ReadableSpan } from '@opentelemetry/sdk-trace-base'; -import { type Span, spanToJSON } from '@sentry/core'; -import { afterEach, beforeEach, describe, expect, it } from 'vitest'; -import { startSpanManual } from '../src/trace'; -import { tracingChannel } from '../src/tracingChannel'; -import { getActiveSpan } from '../src/utils/getActiveSpan'; -import { getParentSpanId } from '../src/utils/getParentSpanId'; -import { cleanupOtel, mockSdkInit } from './helpers/mockSdkInit'; - -describe('tracingChannel', () => { - beforeEach(() => { - mockSdkInit({ tracesSampleRate: 1 }); - }); - - afterEach(async () => { - await cleanupOtel(); - }); - - it('sets the created span as the active span inside traceSync', () => { - const channel = tracingChannel<{ op: string }>('test:sync:active', data => { - return startSpanManual({ name: 'channel-span', op: data.op }, span => span); - }); - - channel.subscribe({ - end: data => { - data._sentrySpan?.end(); - }, - }); - - channel.traceSync( - () => { - const active = getActiveSpan(); - expect(active).toBeDefined(); - expect(spanToJSON(active!).op).toBe('test.op'); - }, - { op: 'test.op' }, - ); - }); - - it('sets the created span as the active span inside tracePromise', async () => { - const channel = tracingChannel<{ op: string }>('test:promise:active', data => { - return startSpanManual({ name: 'channel-span', op: data.op }, span => span); - }); - - channel.subscribe({ - asyncEnd: data => { - data._sentrySpan?.end(); - }, - }); - - await channel.tracePromise( - async () => { - const active = getActiveSpan(); - expect(active).toBeDefined(); - expect(spanToJSON(active!).op).toBe('test.op'); - }, - { op: 'test.op' }, - ); - }); - - it('creates correct parent-child relationship with nested tracing channels', () => { - const outerChannel = tracingChannel<{ name: string }>('test:nested:outer', data => { - return startSpanManual({ name: data.name, op: 'outer' }, span => span); - }); - - const innerChannel = tracingChannel<{ name: string }>('test:nested:inner', data => { - return startSpanManual({ name: data.name, op: 'inner' }, span => span); - }); - - outerChannel.subscribe({ - end: data => { - data._sentrySpan?.end(); - }, - }); - - innerChannel.subscribe({ - end: data => { - data._sentrySpan?.end(); - }, - }); - - let outerSpanId: string | undefined; - let innerParentSpanId: string | undefined; - - outerChannel.traceSync( - () => { - const outerSpan = getActiveSpan(); - outerSpanId = outerSpan?.spanContext().spanId; - - innerChannel.traceSync( - () => { - const innerSpan = getActiveSpan(); - innerParentSpanId = getParentSpanId(innerSpan as unknown as ReadableSpan); - }, - { name: 'inner-span' }, - ); - }, - { name: 'outer-span' }, - ); - - expect(outerSpanId).toBeDefined(); - expect(innerParentSpanId).toBe(outerSpanId); - }); - - it('creates correct parent-child relationship with nested async tracing channels', async () => { - const outerChannel = tracingChannel<{ name: string }>('test:nested-async:outer', data => { - return startSpanManual({ name: data.name, op: 'outer' }, span => span); - }); - - const innerChannel = tracingChannel<{ name: string }>('test:nested-async:inner', data => { - return startSpanManual({ name: data.name, op: 'inner' }, span => span); - }); - - outerChannel.subscribe({ - asyncEnd: data => { - data._sentrySpan?.end(); - }, - }); - - innerChannel.subscribe({ - asyncEnd: data => { - data._sentrySpan?.end(); - }, - }); - - let outerSpanId: string | undefined; - let innerParentSpanId: string | undefined; - - await outerChannel.tracePromise( - async () => { - const outerSpan = getActiveSpan(); - outerSpanId = outerSpan?.spanContext().spanId; - - await innerChannel.tracePromise( - async () => { - const innerSpan = getActiveSpan(); - innerParentSpanId = getParentSpanId(innerSpan as unknown as ReadableSpan); - }, - { name: 'inner-span' }, - ); - }, - { name: 'outer-span' }, - ); - - expect(outerSpanId).toBeDefined(); - expect(innerParentSpanId).toBe(outerSpanId); - }); - - it('creates correct parent when a tracing channel is nested inside startSpanManual', () => { - const channel = tracingChannel<{ name: string }>('test:inside-startspan', data => { - return startSpanManual({ name: data.name, op: 'channel' }, span => span); - }); - - channel.subscribe({ - end: data => { - data._sentrySpan?.end(); - }, - }); - - let manualSpanId: string | undefined; - let channelParentSpanId: string | undefined; - - startSpanManual({ name: 'manual-parent' }, parentSpan => { - manualSpanId = parentSpan.spanContext().spanId; - - channel.traceSync( - () => { - const channelSpan = getActiveSpan(); - channelParentSpanId = getParentSpanId(channelSpan as unknown as ReadableSpan); - }, - { name: 'channel-child' }, - ); - - parentSpan.end(); - }); - - expect(manualSpanId).toBeDefined(); - expect(channelParentSpanId).toBe(manualSpanId); - }); - - it('makes the channel span available on data.span', () => { - let spanFromData: unknown; - - const channel = tracingChannel<{ name: string }>('test:data-span', data => { - return startSpanManual({ name: data.name }, span => span); - }); - - channel.subscribe({ - end: data => { - spanFromData = data._sentrySpan; - data._sentrySpan?.end(); - }, - }); - - channel.traceSync(() => {}, { name: 'test-span' }); - - expect(spanFromData).toBeDefined(); - expect(spanToJSON(spanFromData as unknown as Span).description).toBe('test-span'); - }); - - it('shares the same trace ID across nested channels', () => { - const outerChannel = tracingChannel<{ name: string }>('test:trace-id:outer', data => { - return startSpanManual({ name: data.name }, span => span); - }); - - const innerChannel = tracingChannel<{ name: string }>('test:trace-id:inner', data => { - return startSpanManual({ name: data.name }, span => span); - }); - - outerChannel.subscribe({ end: data => data._sentrySpan?.end() }); - innerChannel.subscribe({ end: data => data._sentrySpan?.end() }); - - let outerTraceId: string | undefined; - let innerTraceId: string | undefined; - - outerChannel.traceSync( - () => { - outerTraceId = getActiveSpan()?.spanContext().traceId; - - innerChannel.traceSync( - () => { - innerTraceId = getActiveSpan()?.spanContext().traceId; - }, - { name: 'inner' }, - ); - }, - { name: 'outer' }, - ); - - expect(outerTraceId).toBeDefined(); - expect(innerTraceId).toBe(outerTraceId); - }); - - it('does not leak context outside of traceSync', () => { - const channel = tracingChannel<{ name: string }>('test:no-leak', data => { - return startSpanManual({ name: data.name }, span => span); - }); - - channel.subscribe({ end: data => data._sentrySpan?.end() }); - - const activeBefore = trace.getSpan(context.active()); - - channel.traceSync(() => {}, { name: 'scoped-span' }); - - const activeAfter = trace.getSpan(context.active()); - - expect(activeBefore).toBeUndefined(); - expect(activeAfter).toBeUndefined(); - }); -}); diff --git a/packages/server-utils/src/index.ts b/packages/server-utils/src/index.ts index 7702ebeebbbb..7a68e0d955a2 100644 --- a/packages/server-utils/src/index.ts +++ b/packages/server-utils/src/index.ts @@ -18,10 +18,7 @@ export type { RedisCommandData, RedisConnectData, RedisDiagnosticChannelResponseHook, - RedisTracingChannel, - RedisTracingChannelContextWithSpan, RedisTracingChannelFactory, - RedisTracingChannelSubscribers, } from './redis/redis-dc-subscriber'; export { bindTracingChannelToSpan } from './tracing-channel'; export type { diff --git a/packages/server-utils/src/redis/redis-dc-subscriber.ts b/packages/server-utils/src/redis/redis-dc-subscriber.ts index 7834fee50af4..1dded05ea4a8 100644 --- a/packages/server-utils/src/redis/redis-dc-subscriber.ts +++ b/packages/server-utils/src/redis/redis-dc-subscriber.ts @@ -1,3 +1,4 @@ +import type { TracingChannel } from 'node:diagnostics_channel'; import { DB_OPERATION_BATCH_SIZE, DB_QUERY_TEXT, @@ -6,14 +7,9 @@ import { SERVER_PORT, } from '@sentry/conventions/attributes'; import type { Span } from '@sentry/core'; -import { - debug, - SEMANTIC_ATTRIBUTE_SENTRY_OP, - SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, - SPAN_STATUS_ERROR, - startSpanManual, -} from '@sentry/core'; +import { debug, SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, startInactiveSpan } from '@sentry/core'; import { DEBUG_BUILD } from '../debug-build'; +import { bindTracingChannelToSpan } from '../tracing-channel'; // Channel names published by node-redis >= 5.12.0 and ioredis >= 5.11.0. // Hardcoded so the subscriber does not have to import either library — the @@ -28,8 +24,6 @@ export const IOREDIS_DC_CHANNEL_CONNECT = 'ioredis:connect'; const ORIGIN = 'auto.db.redis.diagnostic_channel'; const DB_SYSTEM_NAME_VALUE_REDIS = 'redis'; -const NOOP = (): void => {}; - /** * Shape of the `node-redis:command` channel payload published by node-redis. * @@ -104,46 +98,17 @@ export type RedisDiagnosticChannelResponseHook = ( ) => void; /** - * Payload type observed by tracing-channel subscribers — the channel payload - * with `_sentrySpan` stamped on it by the start handler so async/error - * handlers downstream can read it back. - */ -export type RedisTracingChannelContextWithSpan = T & { _sentrySpan?: Span }; - -/** Subscriber object accepted by {@link RedisTracingChannel.subscribe}. */ -export interface RedisTracingChannelSubscribers { - start: (data: RedisTracingChannelContextWithSpan) => void; - asyncStart: (data: RedisTracingChannelContextWithSpan) => void; - asyncEnd: (data: RedisTracingChannelContextWithSpan) => void; - end: (data: RedisTracingChannelContextWithSpan) => void; - error: (data: RedisTracingChannelContextWithSpan) => void; -} - -/** Minimal tracing-channel surface the subscriber depends on. */ -export interface RedisTracingChannel { - subscribe(subs: Partial>): void; -} - -/** - * Platform-provided factory that returns a tracing channel for the given - * channel name. Implementations are responsible for ensuring that, when the - * channel's `start` event fires, the span returned by `transformStart(data)` - * ends up stored on `data._sentrySpan` so the subscriber's `asyncEnd`/`error` - * handlers can read it. + * Platform-provided factory that creates a native tracing channel for the given name. The + * subscriber binds the span and its lifecycle onto the channel via `bindTracingChannelToSpan`, + * which propagates the active span through the runtime's async context. * - * - Node passes `@sentry/opentelemetry/tracing-channel` which uses - * `bindStore` to also propagate the span as the active OTel context. - * - Deno (and other non-OTel runtimes) pass a portable wrapper around - * `node:diagnostics_channel.tracingChannel` that just stamps - * `data._sentrySpan` in `start` without `bindStore`. + * Both Node and Deno pass `node:diagnostics_channel`'s `tracingChannel` directly. */ -export type RedisTracingChannelFactory = ( - name: string, - transformStart: (data: T) => Span, -) => RedisTracingChannel; +export type RedisTracingChannelFactory = (name: string) => TracingChannel; let subscribed = false; let currentResponseHook: RedisDiagnosticChannelResponseHook | undefined; +let activeUnbinds: Array<() => void> = []; /** * Subscribe Sentry span handlers to node-redis and ioredis diagnostics-channel @@ -168,17 +133,18 @@ export function subscribeRedisDiagnosticChannels( try { // node-redis: command name appears as args[0] in the channel payload, so // strip it before the statement and response hook see it. - setupCommandChannel(tracingChannel, REDIS_DC_CHANNEL_COMMAND, data => data.args.slice(1)); - setupBatchChannel(tracingChannel, REDIS_DC_CHANNEL_BATCH, data => - data.batchMode === 'PIPELINE' ? 'PIPELINE' : 'MULTI', + activeUnbinds.push( + setupCommandChannel(tracingChannel, REDIS_DC_CHANNEL_COMMAND, data => data.args.slice(1)), + setupBatchChannel(tracingChannel, REDIS_DC_CHANNEL_BATCH, data => + data.batchMode === 'PIPELINE' ? 'PIPELINE' : 'MULTI', + ), + setupConnectChannel(tracingChannel, REDIS_DC_CHANNEL_CONNECT), + // ioredis: args already exclude the command name; no slicing needed. And + // ioredis has no separate batch channel — pipeline/MULTI metadata rides + // on the per-command payload via `batchMode`/`batchSize`. + setupCommandChannel(tracingChannel, IOREDIS_DC_CHANNEL_COMMAND, data => data.args), + setupConnectChannel(tracingChannel, IOREDIS_DC_CHANNEL_CONNECT), ); - setupConnectChannel(tracingChannel, REDIS_DC_CHANNEL_CONNECT); - - // ioredis: args already exclude the command name; no slicing needed. And - // ioredis has no separate batch channel — pipeline/MULTI metadata rides - // on the per-command payload via `batchMode`/`batchSize`. - setupCommandChannel(tracingChannel, IOREDIS_DC_CHANNEL_COMMAND, data => data.args); - setupConnectChannel(tracingChannel, IOREDIS_DC_CHANNEL_CONNECT); } catch { // The factory may rely on `node:diagnostics_channel`, which isn't always // available. Fail closed; the SDK simply won't emit redis spans here. @@ -190,15 +156,16 @@ function setupCommandChannel( tracingChannel: RedisTracingChannelFactory, channelName: string, getCommandArgs: (data: T) => string[], -): void { - const channel = tracingChannel(channelName, data => { - // `args` is already sanitized by the publishing library (node-redis / - // ioredis call their own `sanitizeArgs` before publishing). Join with - // spaces to mirror the format the libraries themselves intend. - const args = getCommandArgs(data); - const statement = args.length ? `${data.command} ${args.join(' ')}` : data.command; - return startSpanManual( - { +): () => void { + return bindTracingChannelToSpan( + tracingChannel(channelName), + data => { + // `args` is already sanitized by the publishing library (node-redis / + // ioredis call their own `sanitizeArgs` before publishing). Join with + // spaces to mirror the format the libraries themselves intend. + const args = getCommandArgs(data); + const statement = args.length ? `${data.command} ${args.join(' ')}` : data.command; + return startInactiveSpan({ name: `redis-${data.command}`, attributes: { [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: ORIGIN, @@ -208,41 +175,29 @@ function setupCommandChannel( ...(data.serverAddress != null ? { [SERVER_ADDRESS]: data.serverAddress } : {}), ...(data.serverPort != null ? { [SERVER_PORT]: data.serverPort } : {}), }, - }, - span => span, - ); - }); - - channel.subscribe({ - start: NOOP, - asyncStart: NOOP, - end: NOOP, - asyncEnd: data => { - const span = data._sentrySpan; - // Only end here if the error handler isn't going to. - if (!span || data.error) return; - runResponseHook(span, data.command, getCommandArgs(data), data.result); - span.end(); + }); }, - error: data => { - const span = data._sentrySpan; - if (!span) return; - if (data.error) { - span.setStatus({ code: SPAN_STATUS_ERROR, message: data.error.message }); - } - span.end(); + { + // Command failures are surfaced to (and usually handled by) the caller; only annotate the + // span so we don't emit a duplicate error event for every failed command. + captureError: false, + beforeSpanEnd(span, data) { + if ('error' in data) return; + runResponseHook(span, data.command, getCommandArgs(data), data.result); + }, }, - }); + ).unbind; } function setupBatchChannel( tracingChannel: RedisTracingChannelFactory, channelName: string, getOperationName: (data: RedisBatchData) => string, -): void { - const channel = tracingChannel(channelName, data => { - return startSpanManual( - { +): () => void { + return bindTracingChannelToSpan( + tracingChannel(channelName), + data => { + return startInactiveSpan({ name: getOperationName(data), attributes: { [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: ORIGIN, @@ -254,33 +209,17 @@ function setupBatchChannel( ...(data.serverAddress != null ? { [SERVER_ADDRESS]: data.serverAddress } : {}), ...(data.serverPort != null ? { [SERVER_PORT]: data.serverPort } : {}), }, - }, - span => span, - ); - }); - - channel.subscribe({ - start: NOOP, - asyncStart: NOOP, - end: NOOP, - asyncEnd: data => { - if (!data.error) data._sentrySpan?.end(); - }, - error: data => { - const span = data._sentrySpan; - if (!span) return; - if (data.error) { - span.setStatus({ code: SPAN_STATUS_ERROR, message: data.error.message }); - } - span.end(); + }); }, - }); + { captureError: false }, + ).unbind; } -function setupConnectChannel(tracingChannel: RedisTracingChannelFactory, channelName: string): void { - const channel = tracingChannel(channelName, data => { - return startSpanManual( - { +function setupConnectChannel(tracingChannel: RedisTracingChannelFactory, channelName: string): () => void { + return bindTracingChannelToSpan( + tracingChannel(channelName), + data => { + return startInactiveSpan({ name: 'redis-connect', attributes: { [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: ORIGIN, @@ -289,27 +228,10 @@ function setupConnectChannel(tracingChannel: RedisTracingChannelFactory, channel ...(data.serverAddress != null ? { [SERVER_ADDRESS]: data.serverAddress } : {}), ...(data.serverPort != null ? { [SERVER_PORT]: data.serverPort } : {}), }, - }, - span => span, - ); - }); - - channel.subscribe({ - start: NOOP, - asyncStart: NOOP, - end: NOOP, - asyncEnd: data => { - if (!data.error) data._sentrySpan?.end(); - }, - error: data => { - const span = data._sentrySpan; - if (!span) return; - if (data.error) { - span.setStatus({ code: SPAN_STATUS_ERROR, message: data.error.message }); - } - span.end(); + }); }, - }); + { captureError: false }, + ).unbind; } function runResponseHook(span: Span, command: string, args: string[], result: unknown): void { @@ -322,8 +244,10 @@ function runResponseHook(span: Span, command: string, args: string[], result: un } } -/** Test-only: reset module-local subscribe state. */ +/** Test-only: detach all channel bindings and reset module-local subscribe state. */ export function _resetRedisDiagnosticChannelsForTesting(): void { + activeUnbinds.forEach(unbind => unbind()); + activeUnbinds = []; subscribed = false; currentResponseHook = undefined; } diff --git a/packages/server-utils/test/redis/redis-dc-subscriber.test.ts b/packages/server-utils/test/redis/redis-dc-subscriber.test.ts index 39af399549e7..e4de6a17034c 100644 --- a/packages/server-utils/test/redis/redis-dc-subscriber.test.ts +++ b/packages/server-utils/test/redis/redis-dc-subscriber.test.ts @@ -1,3 +1,22 @@ +import { AsyncLocalStorage } from 'node:async_hooks'; +import { tracingChannel } from 'node:diagnostics_channel'; +import type { Scope, Span } from '@sentry/core'; +import * as SentryCore from '@sentry/core'; +import { + _INTERNAL_setSpanForScope, + Client, + createTransport, + getActiveSpan, + getCurrentScope, + getDefaultCurrentScope, + getDefaultIsolationScope, + getGlobalScope, + initAndBind, + resolvedSyncPromise, + setAsyncContextStrategy, + spanToJSON, + startSpan, +} from '@sentry/core'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { _resetRedisDiagnosticChannelsForTesting, @@ -6,304 +25,253 @@ import { REDIS_DC_CHANNEL_BATCH, REDIS_DC_CHANNEL_COMMAND, REDIS_DC_CHANNEL_CONNECT, - subscribeRedisDiagnosticChannels, - type RedisTracingChannel, type RedisTracingChannelFactory, - type RedisTracingChannelSubscribers, + subscribeRedisDiagnosticChannels, } from '../../src/redis/redis-dc-subscriber'; -import { SPAN_STATUS_ERROR } from '@sentry/core'; -interface RecordedChannel { - subs: Partial>; +interface TestStore { + scope: Scope; + isolationScope: Scope; +} + +class TestClient extends Client { + public eventFromException(): PromiseLike { + return resolvedSyncPromise({}); + } + public eventFromMessage(): PromiseLike { + return resolvedSyncPromise({}); + } } -// fake tracing-channel factory that stores subscribers in channels by name -function makeFakeFactory(): { - factory: RedisTracingChannelFactory; - channels: Record; -} { - const channels: Record = {}; - const factory: RedisTracingChannelFactory = (name, _transform) => { - const recorded: RecordedChannel = { subs: {} }; - channels[name] = recorded; - return { - subscribe(subs: Partial>) { - Object.assign(recorded.subs, subs); +function initTestClient(): void { + initAndBind(TestClient, { + dsn: 'https://username@domain/123', + integrations: [], + sendClientReports: false, + stackParser: () => [], + tracesSampleRate: 1, + transport: () => createTransport({ recordDroppedEvent: () => undefined }, () => resolvedSyncPromise({})), + }); +} + +function installTestAsyncContextStrategy(): void { + const asyncStorage = new AsyncLocalStorage(); + + function getScopes(): TestStore { + return ( + asyncStorage.getStore() || { + scope: getDefaultCurrentScope(), + isolationScope: getDefaultIsolationScope(), + } + ); + } + + setAsyncContextStrategy({ + withScope: callback => { + const scope = getScopes().scope.clone(); + const isolationScope = getScopes().isolationScope; + return asyncStorage.run({ scope, isolationScope }, () => callback(scope)); + }, + withSetScope: (scope, callback) => { + const isolationScope = getScopes().isolationScope; + return asyncStorage.run({ scope, isolationScope }, () => callback(scope)); + }, + withIsolationScope: callback => { + const scope = getScopes().scope; + const isolationScope = getScopes().isolationScope.clone(); + return asyncStorage.run({ scope, isolationScope }, () => callback(isolationScope)); + }, + withSetIsolationScope: (isolationScope, callback) => { + const scope = getScopes().scope; + return asyncStorage.run({ scope, isolationScope }, () => callback(isolationScope)); + }, + getCurrentScope: () => getScopes().scope, + getIsolationScope: () => getScopes().isolationScope, + getTracingChannelBinding: () => ({ + asyncLocalStorage: asyncStorage, + getStoreWithActiveSpan: span => { + const scope = getScopes().scope.clone(); + const isolationScope = getScopes().isolationScope; + _INTERNAL_setSpanForScope(scope, span); + return { scope, isolationScope }; }, - } as unknown as RedisTracingChannel; - }; - return { factory, channels }; + }), + }); } -function makeSpan() { - return { - end: vi.fn(), - setStatus: vi.fn(), - setAttribute: vi.fn(), - setAttributes: vi.fn(), - updateName: vi.fn(), - spanContext: () => ({ spanId: 'test-span-id', traceId: 'test-trace-id', traceFlags: 1 }), - }; +/** Drives a channel's `tracePromise` and captures the span bound by the subscriber. */ +async function traceCommand( + channelName: string, + data: Record, + outcome: { result?: unknown; error?: Error }, +): Promise<{ span: Span | undefined; childParentSpanId: string | undefined }> { + const channel = tracingChannel(channelName); + let span: Span | undefined; + let childParentSpanId: string | undefined; + + const run = channel.tracePromise(async () => { + span = getActiveSpan(); + startSpan({ name: 'child' }, child => { + childParentSpanId = spanToJSON(child).parent_span_id; + }); + if (outcome.error) { + throw outcome.error; + } + return outcome.result; + }, data); + + await run.catch(() => undefined); + + return { span, childParentSpanId }; } +const factory = tracingChannel as RedisTracingChannelFactory; + describe('subscribeRedisDiagnosticChannels', () => { - let factory: RedisTracingChannelFactory; - let channels: Record; - let mockSpan: ReturnType; let responseHook: ReturnType; + let captureExceptionSpy: ReturnType; - const subs = (name: string) => - channels[name]!.subs as { - asyncEnd: (data: any) => void; - error: (data: any) => void; - }; - + // `node:diagnostics_channel` channels are process-global. `_reset…` calls each binding's `unbind`, + // so we can subscribe and fully detach per test without handlers leaking across tests. beforeEach(() => { - _resetRedisDiagnosticChannelsForTesting(); - ({ factory, channels } = makeFakeFactory()); - mockSpan = makeSpan(); + installTestAsyncContextStrategy(); + initTestClient(); responseHook = vi.fn(); + captureExceptionSpy = vi.spyOn(SentryCore, 'captureException').mockReturnValue('event-id'); subscribeRedisDiagnosticChannels(factory, responseHook); }); afterEach(() => { + _resetRedisDiagnosticChannelsForTesting(); + setAsyncContextStrategy(undefined); + getCurrentScope().clear(); + getCurrentScope().setClient(undefined); + getGlobalScope().clear(); vi.clearAllMocks(); }); describe('node-redis command channel', () => { - describe('asyncEnd (success path)', () => { - it('calls the response hook with sliced args and ends the span', () => { - const data = { - command: 'GET', - args: ['GET', 'cache:key'], - result: 'hit-value', - _sentrySpan: mockSpan, - }; - subs(REDIS_DC_CHANNEL_COMMAND).asyncEnd(data); - - expect(responseHook).toHaveBeenCalledWith(mockSpan, 'GET', ['cache:key'], 'hit-value'); - expect(mockSpan.end).toHaveBeenCalledTimes(1); - }); - - it('strips the command name from args before passing to the response hook', () => { - const data = { - command: 'MGET', - args: ['MGET', 'key1', 'key2', 'key3'], - result: ['v1', 'v2', 'v3'], - _sentrySpan: mockSpan, - }; - subs(REDIS_DC_CHANNEL_COMMAND).asyncEnd(data); - - expect(responseHook).toHaveBeenCalledWith(mockSpan, 'MGET', ['key1', 'key2', 'key3'], ['v1', 'v2', 'v3']); - }); - - it('bails early when _sentrySpan is absent', () => { - subs(REDIS_DC_CHANNEL_COMMAND).asyncEnd({ command: 'GET', args: ['GET', 'k'], result: 'v' }); - - expect(responseHook).not.toHaveBeenCalled(); - expect(mockSpan.end).not.toHaveBeenCalled(); - }); + it('creates a db.redis span, runs the response hook with sliced args, and ends the span', async () => { + const { span } = await traceCommand( + REDIS_DC_CHANNEL_COMMAND, + { command: 'GET', args: ['GET', 'cache:key'], serverAddress: '127.0.0.1', serverPort: 6379 }, + { result: 'hit-value' }, + ); + + expect(span).toBeDefined(); + const json = spanToJSON(span!); + expect(json.description).toBe('redis-GET'); + expect(json.op).toBe('db.redis'); + expect(json.data['db.system.name']).toBe('redis'); + expect(json.data['db.query.text']).toBe('GET cache:key'); + expect(json.timestamp).toBeDefined(); + + // command name is stripped from args before the hook sees them + expect(responseHook).toHaveBeenCalledWith(span, 'GET', ['cache:key'], 'hit-value'); }); - describe('error path', () => { - it('sets error status and ends the span in the error handler', () => { - const error = new Error('ECONNREFUSED'); - const data = { command: 'SET', args: ['SET', 'k', 'v'], error, _sentrySpan: mockSpan }; - subs(REDIS_DC_CHANNEL_COMMAND).error(data); - - expect(mockSpan.setStatus).toHaveBeenCalledWith({ code: SPAN_STATUS_ERROR, message: 'ECONNREFUSED' }); - expect(mockSpan.end).toHaveBeenCalledTimes(1); - }); - - it('does not call the response hook or end the span a second time in asyncEnd when error is set', () => { - const error = new Error('ECONNREFUSED'); - const data = { command: 'GET', args: ['GET', 'k'], error, _sentrySpan: mockSpan }; - - // TracingChannel fires error first, then asyncEnd, on the same data object. - subs(REDIS_DC_CHANNEL_COMMAND).error(data); - subs(REDIS_DC_CHANNEL_COMMAND).asyncEnd(data); - - expect(responseHook).not.toHaveBeenCalled(); - expect(mockSpan.end).toHaveBeenCalledTimes(1); - }); - - it('bails early in error handler when _sentrySpan is absent', () => { - subs(REDIS_DC_CHANNEL_COMMAND).error({ command: 'GET', args: ['GET', 'k'], error: new Error('x') }); - - expect(mockSpan.setStatus).not.toHaveBeenCalled(); - expect(mockSpan.end).not.toHaveBeenCalled(); - }); - }); - }); - - describe('node-redis batch channel', () => { - describe('asyncEnd (success path)', () => { - it('ends the span', () => { - const data = { batchMode: 'PIPELINE', batchSize: 3, _sentrySpan: mockSpan }; - subs(REDIS_DC_CHANNEL_BATCH).asyncEnd(data); + it('sets error status and does NOT capture an exception on failure', async () => { + const { span } = await traceCommand( + REDIS_DC_CHANNEL_COMMAND, + { command: 'SET', args: ['SET', 'k', 'v'] }, + { error: new Error('ECONNREFUSED') }, + ); - expect(mockSpan.end).toHaveBeenCalledTimes(1); - }); - - it('bails early when _sentrySpan is absent', () => { - subs(REDIS_DC_CHANNEL_BATCH).asyncEnd({ batchMode: 'MULTI' }); - - expect(mockSpan.end).not.toHaveBeenCalled(); - }); - }); - - describe('error path', () => { - it('sets error status and ends the span in the error handler', () => { - const error = new Error('MULTI aborted'); - const data = { batchMode: 'MULTI', error, _sentrySpan: mockSpan }; - subs(REDIS_DC_CHANNEL_BATCH).error(data); - - expect(mockSpan.setStatus).toHaveBeenCalledWith({ code: SPAN_STATUS_ERROR, message: 'MULTI aborted' }); - expect(mockSpan.end).toHaveBeenCalledTimes(1); - }); - - it('does not end the span a second time in asyncEnd when error is set', () => { - const error = new Error('MULTI aborted'); - const data = { batchMode: 'MULTI', error, _sentrySpan: mockSpan }; - - subs(REDIS_DC_CHANNEL_BATCH).error(data); - subs(REDIS_DC_CHANNEL_BATCH).asyncEnd(data); - - expect(mockSpan.end).toHaveBeenCalledTimes(1); - }); - }); - }); - - describe('node-redis connect channel', () => { - describe('asyncEnd (success path)', () => { - it('ends the span', () => { - const data = { serverAddress: '127.0.0.1', serverPort: 6379, _sentrySpan: mockSpan }; - subs(REDIS_DC_CHANNEL_CONNECT).asyncEnd(data); - - expect(mockSpan.end).toHaveBeenCalledTimes(1); - }); - - it('bails early when _sentrySpan is absent', () => { - subs(REDIS_DC_CHANNEL_CONNECT).asyncEnd({ serverAddress: '127.0.0.1' }); - - expect(mockSpan.end).not.toHaveBeenCalled(); - }); + expect(spanToJSON(span!).status).toBe('ECONNREFUSED'); + expect(spanToJSON(span!).timestamp).toBeDefined(); + expect(responseHook).not.toHaveBeenCalled(); + expect(captureExceptionSpy).not.toHaveBeenCalled(); }); - describe('error path', () => { - it('sets error status and ends the span in the error handler', () => { - const error = new Error('connect ECONNREFUSED'); - const data = { serverAddress: '127.0.0.1', serverPort: 6379, error, _sentrySpan: mockSpan }; - subs(REDIS_DC_CHANNEL_CONNECT).error(data); + it('parents the redis span to the surrounding span and parents children to the redis span', async () => { + let outerSpanId: string | undefined; + let result: Awaited> | undefined; - expect(mockSpan.setStatus).toHaveBeenCalledWith({ code: SPAN_STATUS_ERROR, message: 'connect ECONNREFUSED' }); - expect(mockSpan.end).toHaveBeenCalledTimes(1); + await startSpan({ name: 'outer' }, async outer => { + outerSpanId = outer.spanContext().spanId; + result = await traceCommand(REDIS_DC_CHANNEL_COMMAND, { command: 'GET', args: ['GET', 'k'] }, { result: 'v' }); }); - it('does not end the span a second time in asyncEnd when error is set', () => { - const error = new Error('connect ECONNREFUSED'); - const data = { serverAddress: '127.0.0.1', error, _sentrySpan: mockSpan }; - - subs(REDIS_DC_CHANNEL_CONNECT).error(data); - subs(REDIS_DC_CHANNEL_CONNECT).asyncEnd(data); - - expect(mockSpan.end).toHaveBeenCalledTimes(1); - }); + expect(spanToJSON(result!.span!).parent_span_id).toBe(outerSpanId); + expect(result!.childParentSpanId).toBe(result!.span!.spanContext().spanId); }); }); describe('ioredis command channel', () => { - it('calls the response hook with args as published by ioredis (no slicing)', () => { - const data = { - command: 'get', - args: ['cache:key'], - result: 'hit-value', - _sentrySpan: mockSpan, - }; - subs(IOREDIS_DC_CHANNEL_COMMAND).asyncEnd(data); - - expect(responseHook).toHaveBeenCalledWith(mockSpan, 'get', ['cache:key'], 'hit-value'); - expect(mockSpan.end).toHaveBeenCalledTimes(1); + it('does not slice the first arg (ioredis omits the command name)', async () => { + const { span } = await traceCommand( + IOREDIS_DC_CHANNEL_COMMAND, + { command: 'mget', args: ['key1', 'key2'] }, + { result: ['v1', 'v2'] }, + ); + + expect(spanToJSON(span!).data['db.query.text']).toBe('mget key1 key2'); + expect(responseHook).toHaveBeenCalledWith(span, 'mget', ['key1', 'key2'], ['v1', 'v2']); }); + }); - it('does not slice the first arg for ioredis command payloads', () => { - const data = { - command: 'mget', - args: ['key1', 'key2', 'key3'], - result: ['v1', 'v2', 'v3'], - _sentrySpan: mockSpan, - }; - subs(IOREDIS_DC_CHANNEL_COMMAND).asyncEnd(data); - - expect(responseHook).toHaveBeenCalledWith(mockSpan, 'mget', ['key1', 'key2', 'key3'], ['v1', 'v2', 'v3']); - }); - - it('handles batch metadata on ioredis command payloads without a separate batch channel', () => { - const data = { - command: 'set', - args: ['cache:key', '?'], - batchMode: 'MULTI', - batchSize: 2, - result: 'OK', - _sentrySpan: mockSpan, - }; - subs(IOREDIS_DC_CHANNEL_COMMAND).asyncEnd(data); - - expect(channels['ioredis:batch']).toBeUndefined(); - expect(responseHook).toHaveBeenCalledWith(mockSpan, 'set', ['cache:key', '?'], 'OK'); - expect(mockSpan.end).toHaveBeenCalledTimes(1); - }); - - it('sets error status and ends the span in the error handler', () => { - const error = new Error('WRONGTYPE'); - const data = { command: 'hset', args: ['key', 'field', '?'], error, _sentrySpan: mockSpan }; - subs(IOREDIS_DC_CHANNEL_COMMAND).error(data); - - expect(mockSpan.setStatus).toHaveBeenCalledWith({ code: SPAN_STATUS_ERROR, message: 'WRONGTYPE' }); - expect(mockSpan.end).toHaveBeenCalledTimes(1); + describe('batch channel', () => { + it('creates a batch span and ends it', async () => { + const { span } = await traceCommand( + REDIS_DC_CHANNEL_BATCH, + { batchMode: 'PIPELINE', batchSize: 3 }, + { result: ['OK', 'OK', 'OK'] }, + ); + + expect(spanToJSON(span!).description).toBe('PIPELINE'); + expect(spanToJSON(span!).op).toBe('db.redis'); + expect(spanToJSON(span!).timestamp).toBeDefined(); }); - it('does not call the response hook or end the span a second time in asyncEnd when error is set', () => { - const error = new Error('WRONGTYPE'); - const data = { command: 'hset', args: ['key', 'field', '?'], error, _sentrySpan: mockSpan }; - - subs(IOREDIS_DC_CHANNEL_COMMAND).error(data); - subs(IOREDIS_DC_CHANNEL_COMMAND).asyncEnd(data); + it('sets error status without capturing on failure', async () => { + const { span } = await traceCommand( + REDIS_DC_CHANNEL_BATCH, + { batchMode: 'MULTI' }, + { error: new Error('MULTI aborted') }, + ); - expect(responseHook).not.toHaveBeenCalled(); - expect(mockSpan.end).toHaveBeenCalledTimes(1); + expect(spanToJSON(span!).status).toBe('MULTI aborted'); + expect(captureExceptionSpy).not.toHaveBeenCalled(); }); }); - describe('ioredis connect channel', () => { - it('ends the span on success', () => { - const data = { serverAddress: 'localhost', serverPort: 6379, _sentrySpan: mockSpan }; - subs(IOREDIS_DC_CHANNEL_CONNECT).asyncEnd(data); - - expect(mockSpan.end).toHaveBeenCalledTimes(1); + describe('connect channel', () => { + it('creates a db.redis.connect span', async () => { + const { span } = await traceCommand( + REDIS_DC_CHANNEL_CONNECT, + { serverAddress: '127.0.0.1', serverPort: 6379 }, + { result: undefined }, + ); + + expect(spanToJSON(span!).description).toBe('redis-connect'); + expect(spanToJSON(span!).op).toBe('db.redis.connect'); + expect(spanToJSON(span!).timestamp).toBeDefined(); }); - it('sets error status and ends the span in the error handler', () => { - const error = new Error('connect ECONNREFUSED'); - const data = { serverAddress: 'localhost', serverPort: 1, error, _sentrySpan: mockSpan }; - subs(IOREDIS_DC_CHANNEL_CONNECT).error(data); + it('also subscribes the ioredis connect channel', async () => { + const { span } = await traceCommand( + IOREDIS_DC_CHANNEL_CONNECT, + { serverAddress: 'localhost', serverPort: 6379 }, + { result: undefined }, + ); - expect(mockSpan.setStatus).toHaveBeenCalledWith({ code: SPAN_STATUS_ERROR, message: 'connect ECONNREFUSED' }); - expect(mockSpan.end).toHaveBeenCalledTimes(1); + expect(spanToJSON(span!).op).toBe('db.redis.connect'); + expect(spanToJSON(span!).timestamp).toBeDefined(); }); }); describe('idempotency', () => { - it('does not re-subscribe on a second call, but updates the response hook', () => { - // First subscription happened in beforeEach. Replay with a new hook — - // the same channel subscribers stay in place, but the new hook fires. + it('does not re-subscribe on a second call, but updates the response hook', async () => { const secondHook = vi.fn(); subscribeRedisDiagnosticChannels(factory, secondHook); - const data = { command: 'GET', args: ['GET', 'k'], result: 'v', _sentrySpan: mockSpan }; - subs(REDIS_DC_CHANNEL_COMMAND).asyncEnd(data); + const { span } = await traceCommand( + REDIS_DC_CHANNEL_COMMAND, + { command: 'GET', args: ['GET', 'k'] }, + { result: 'v' }, + ); - expect(secondHook).toHaveBeenCalledTimes(1); + expect(secondHook).toHaveBeenCalledWith(span, 'GET', ['k'], 'v'); expect(responseHook).not.toHaveBeenCalled(); }); });