feat(server-utils): Add requiresParentSpan option to bindTracingChannelToSpan (#22113)

A bunch of the diagnostics-channel subscribers repeated the same guard
at the top of their `getSpan` callback.

Usually the logic is to bail out with `undefined` when there's no active
span, so we don't open a top-level db span for queries/connects that
happen outside a request.

This pulls that into a `requiresParentSpan` option on
`bindTracingChannelToSpan` and moves the check into the bind store,
before `getSpan` runs. Behaviorally identical, just one place instead of
five.

If you need to actually inspect the parent span before deciding, that
still belongs in `getSpan`.

Feel free to push back if this over complicates the
`bindTracingChannelToSpan`, I thought it was a nice opportunity to
normalize this behavior but not a biggie.

---------

Co-authored-by: Andrei <168741329+andreiborza@users.noreply.github.com>

Author: logaretm

.size-limit.js

@@ -400,7 +400,7 @@ module.exports = [
     import: createImport('init', 'experimentalUseDiagnosticsChannelInjection'),
     ignore: [...builtinModules, ...nodePrefixedBuiltinModules],
     gzip: true,
-    limit: '135 KB',
+    limit: '136 KB',
     disablePlugins: ['@size-limit/esbuild'],
   },
   {

packages/server-utils/src/integrations/tracing-channel/ioredis.ts

@@ -8,7 +8,6 @@ import type { IntegrationFn, Span } from '@sentry/core';
 import {
   debug,
   defineIntegration,
-  getActiveSpan,
   SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
   startInactiveSpan,
   waitForTracingChannelBinding,
@@ -96,10 +95,6 @@ const _ioredisChannelIntegration = ((options: IORedisChannelIntegrationOptions =
         bindTracingChannelToSpan(
           commandChannel,
           data => {
-            // ioredis' `requireParentSpan` default: only create a span under an active span.
-            if (!getActiveSpan()) {
-              return undefined;
-            }
             const command = data.arguments?.[0] as RedisCommand | undefined;
             if (!command || typeof command !== 'object') {
               return undefined;
@@ -113,6 +108,8 @@ const _ioredisChannelIntegration = ((options: IORedisChannelIntegrationOptions =
             });
           },
           {
+            // ioredis' `requireParentSpan` default: only create a span under an active span.
+            requiresParentSpan: true,
             beforeSpanEnd(span, data) {
               if ('error' in data || !responseHook) {
                 return;
@@ -125,17 +122,18 @@ const _ioredisChannelIntegration = ((options: IORedisChannelIntegrationOptions =
           },
         );
 
-        bindTracingChannelToSpan(connectChannel, data => {
-          if (!getActiveSpan()) {
-            return undefined;
-          }
-          const { host, port } = getConnectionOptions(data.self);
-          return startInactiveSpan({
-            name: 'connect',
-            op: 'db',
-            attributes: { ...connectionAttributes(host, port), [DB_STATEMENT]: 'connect' },
-          });
-        });
+        bindTracingChannelToSpan(
+          connectChannel,
+          data => {
+            const { host, port } = getConnectionOptions(data.self);
+            return startInactiveSpan({
+              name: 'connect',
+              op: 'db',
+              attributes: { ...connectionAttributes(host, port), [DB_STATEMENT]: 'connect' },
+            });
+          },
+          { requiresParentSpan: true },
+        );
       });
     },
   };

packages/server-utils/src/integrations/tracing-channel/postgres-js.ts

@@ -9,7 +9,6 @@ import {
   _INTERNAL_setPostgresOperationName,
   debug,
   defineIntegration,
-  getActiveSpan,
   SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
   SPAN_KIND,
   SPAN_STATUS_ERROR,
@@ -257,15 +256,12 @@ const _postgresJsChannelIntegration = ((options: PostgresJsChannelIntegrationOpt
               return undefined;
             }
 
-            // Opt out of: re-entrant `handle()` calls (then/catch/finally re-invoke
-            // it, guarded by `executed`), queries already wrapped by the portable
-            // `instrumentPostgresJsSql`, and (by default) queries with no parent span.
+            // Opt out of re-entrant `handle()` calls (then/catch/finally re-invoke it, guarded by
+            // `executed`) and queries already wrapped by the portable `instrumentPostgresJsSql`. The
+            // parent-span requirement is applied via `requiresParentSpan` below.
             if (query.executed === true || (query as Record<symbol, unknown>)[QUERY_FROM_INSTRUMENTED_SQL]) {
               return undefined;
             }
-            if (requireParentSpan !== false && !getActiveSpan()) {
-              return undefined;
-            }
 
             const fullQuery = _INTERNAL_reconstructPostgresQuery(query.strings);
             const sanitizedSqlQuery = _INTERNAL_sanitizeSqlQuery(fullQuery);
@@ -306,6 +302,7 @@ const _postgresJsChannelIntegration = ((options: PostgresJsChannelIntegrationOpt
             return span;
           },
           {
+            requiresParentSpan: requireParentSpan !== false,
             deferSpanEnd({ data }) {
               // `handle` is async: its promise settles on dispatch (asyncEnd), long
               // before the query does. The resolve/reject wrappers own the ending.

packages/server-utils/src/integrations/tracing-channel/postgres.ts

@@ -4,7 +4,6 @@ import {
   bindScopeToEmitter,
   debug,
   defineIntegration,
-  getActiveSpan,
   getCurrentScope,
   SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
   SPAN_KIND,
@@ -121,12 +120,6 @@ function subscribeQueryLikeChannel(
   bindTracingChannelToSpan(
     diagnosticsChannel.tracingChannel<PgChannelContext>(channelName),
     data => {
-      // Only instrument when there's an active span; returning `undefined` opts this call out entirely,
-      // leaving the active context untouched (e.g. connects issued during app startup).
-      if (!getActiveSpan()) {
-        return undefined;
-      }
-
       // Capture the caller's scope while still synchronously inside the call, for the streamed path:
       // pg dispatches a `Submittable` emitter's events outside the original async scope, so `deferSpanEnd`
       // replays this scope onto that emitter.
@@ -143,6 +136,9 @@ function subscribeQueryLikeChannel(
     // `query` can return a streamable `Submittable`, so only it defers.
     deferStreamedResult
       ? {
+          // Only instrument under an active span, leaving the context untouched otherwise
+          // (e.g. connects issued during app startup).
+          requiresParentSpan: true,
           // Streamable `Submittable` (e.g. `client.query(new Query())`)
           // returns an emitter that orchestrion stores on `ctx.result` while
           // firing no async events; the query isn't done until the emitter
@@ -169,7 +165,7 @@ function subscribeQueryLikeChannel(
             return true;
           },
         }
-      : undefined,
+      : { requiresParentSpan: true },
   );
 }
 

packages/server-utils/src/mysql2/mysql2-dc-subscriber.ts

@@ -9,7 +9,6 @@ import {
 } from '@sentry/conventions/attributes';
 import {
   _INTERNAL_sanitizeSqlQuery,
-  getActiveSpan,
   SEMANTIC_ATTRIBUTE_SENTRY_OP,
   SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
   startInactiveSpan,
@@ -93,51 +92,49 @@ export function subscribeMysql2DiagnosticChannels(tracingChannel: MySQL2TracingC
 }
 
 function setupQueryChannel(tracingChannel: MySQL2TracingChannelFactory, channelName: string): void {
-  bindTracingChannelToSpan(tracingChannel<MySQL2QueryData>(channelName), data => {
-    // Only instrument when there's an active span
-    if (!getActiveSpan()) {
-      return undefined;
-    }
+  bindTracingChannelToSpan(
+    tracingChannel<MySQL2QueryData>(channelName),
+    data => {
+      // mysql2 does not sanitize its channel payload, so the statement may carry
+      // raw user values (on the `query` channel they are inlined). Strip every
+      // literal before it leaves the process; `values` is never attached.
+      const queryText = data.query ? _INTERNAL_sanitizeSqlQuery(data.query) : undefined;
+      const operation = queryText?.match(SQL_OPERATION_RE)?.[1]?.toUpperCase();
 
-    // mysql2 does not sanitize its channel payload, so the statement may carry
-    // raw user values (on the `query` channel they are inlined). Strip every
-    // literal before it leaves the process; `values` is never attached.
-    const queryText = data.query ? _INTERNAL_sanitizeSqlQuery(data.query) : undefined;
-    const operation = queryText?.match(SQL_OPERATION_RE)?.[1]?.toUpperCase();
-
-    return startInactiveSpan({
-      name: queryText || 'mysql2.query',
-      attributes: {
-        [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: ORIGIN,
-        [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'db',
-        [DB_SYSTEM_NAME]: DB_SYSTEM_NAME_VALUE_MYSQL,
-        [DB_QUERY_TEXT]: queryText,
-        [DB_OPERATION_NAME]: operation,
-        [DB_NAMESPACE]: data.database || undefined,
-        [SERVER_ADDRESS]: data.serverAddress,
-        [SERVER_PORT]: data.serverPort,
-      },
-    });
-  });
+      return startInactiveSpan({
+        name: queryText || 'mysql2.query',
+        attributes: {
+          [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: ORIGIN,
+          [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'db',
+          [DB_SYSTEM_NAME]: DB_SYSTEM_NAME_VALUE_MYSQL,
+          [DB_QUERY_TEXT]: queryText,
+          [DB_OPERATION_NAME]: operation,
+          [DB_NAMESPACE]: data.database || undefined,
+          [SERVER_ADDRESS]: data.serverAddress,
+          [SERVER_PORT]: data.serverPort,
+        },
+      });
+    },
+    { requiresParentSpan: true },
+  );
 }
 
 function setupConnectChannel(tracingChannel: MySQL2TracingChannelFactory, channelName: string, spanName: string): void {
-  bindTracingChannelToSpan(tracingChannel<MySQL2ConnectData>(channelName), data => {
-    // Only instrument when there's an active span.
-    if (!getActiveSpan()) {
-      return undefined;
-    }
-
-    return startInactiveSpan({
-      name: spanName,
-      attributes: {
-        [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: ORIGIN,
-        [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'db',
-        [DB_SYSTEM_NAME]: DB_SYSTEM_NAME_VALUE_MYSQL,
-        [DB_NAMESPACE]: data.database || undefined,
-        [SERVER_ADDRESS]: data.serverAddress,
-        [SERVER_PORT]: data.serverPort,
-      },
-    });
-  });
+  bindTracingChannelToSpan(
+    tracingChannel<MySQL2ConnectData>(channelName),
+    data => {
+      return startInactiveSpan({
+        name: spanName,
+        attributes: {
+          [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: ORIGIN,
+          [SEMANTIC_ATTRIBUTE_SENTRY_OP]: 'db',
+          [DB_SYSTEM_NAME]: DB_SYSTEM_NAME_VALUE_MYSQL,
+          [DB_NAMESPACE]: data.database || undefined,
+          [SERVER_ADDRESS]: data.serverAddress,
+          [SERVER_PORT]: data.serverPort,
+        },
+      });
+    },
+    { requiresParentSpan: true },
+  );
 }

packages/server-utils/src/tracing-channel.ts

@@ -1,7 +1,14 @@
 import type { TracingChannel, TracingChannelSubscribers } from 'node:diagnostics_channel';
 import type { AsyncLocalStorage } from 'node:async_hooks';
 import type { ExclusiveEventHintOrCaptureContext, Span } from '@sentry/core';
-import { debug, captureException, SPAN_STATUS_ERROR, getAsyncContextStrategy, getMainCarrier } from '@sentry/core';
+import {
+  debug,
+  captureException,
+  SPAN_STATUS_ERROR,
+  getAsyncContextStrategy,
+  getMainCarrier,
+  getActiveSpan,
+} from '@sentry/core';
 import { DEBUG_BUILD } from './debug-build';
 import { ERROR_TYPE } from '@sentry/conventions/attributes';
 
@@ -58,6 +65,12 @@ export interface TracingChannelLifeCycleOptions<TData extends object = object> {
     /** Ends the span: `end()` on success, `end(error)` on failure. Idempotent. */
     end: (error?: unknown) => void;
   }) => boolean;
+
+  /**
+   * Apply span/scope binding only if there is a parent span, similar to returning `undefined` in the `getSpan` callback.
+   * If you need to perform some conditional checking on the parent span before deciding, do it in the `getSpan` callback.
+   */
+  requiresParentSpan?: boolean;
 }
 
 /** Returned by {@link bindTracingChannelToSpan}: the bound channel plus a teardown handle. */
@@ -90,7 +103,7 @@ export function bindTracingChannelToSpan<TData extends object>(
   getSpan: (data: TracingChannelPayloadWithSpan<TData>) => Span | undefined,
   opts?: TracingChannelLifeCycleOptions<TData>,
 ): TracingChannelBindingHandle<TData> {
-  const handle = bindSpanToChannelStore(channel, getSpan);
+  const handle = bindSpanToChannelStore(channel, getSpan, opts);
 
   const beforeSpanEnd = opts?.beforeSpanEnd;
   const deferSpanEnd = opts?.deferSpanEnd;
@@ -191,6 +204,7 @@ export function bindTracingChannelToSpan<TData extends object>(
 function bindSpanToChannelStore<TData extends object>(
   channel: TracingChannel<TData, TData>,
   getSpan: (data: TracingChannelPayloadWithSpan<TData>) => Span | undefined,
+  opts?: Pick<TracingChannelLifeCycleOptions, 'requiresParentSpan'>,
 ): TracingChannelBindingHandle<TData> {
   // Grabs the tracing channel binding defined by the AsyncContext strategy implementation
   const binding = getAsyncContextStrategy(getMainCarrier()).getTracingChannelBinding?.();
@@ -219,7 +233,8 @@ function bindSpanToChannelStore<TData extends object>(
     // callback-style channels (see `_sentryCallerStore`).
     data._sentryCallerStore = asyncLocalStorage.getStore();
 
-    const span = getSpan(data);
+    const shouldGetSpan = !opts?.requiresParentSpan || getActiveSpan();
+    const span = shouldGetSpan ? getSpan(data) : undefined;
     if (!span) {
       // Leave the active context untouched so nested operations keep parenting to the enclosing span.
       return data._sentryCallerStore as TData;