feat(node): Only add span listeners for instrumentation when used (#15802)

Related to
#15725 (comment),
this PR is an idea to avoid registering `on('spanStart')` hooks for all
the node libraries that do not have proper hooks, unless they are used.

Today, for OTEL instrumentation that does not provide span hooks, we
fall back to `client.on('spanStart')` to enhance/mutate spans emitted by
the instrumentation. This PR improved this by only registering the
`spanStart` client hook if we detect that the OTEL instrumentation is
actually wrapping something. This avoids us calling a bunch of span
callbacks each time a span is started, when it is not even needed.

For this, a new `instrumentWhenWrapped` utility is added which can be
passed an instrumentation. This works by monkey-patching `_wrap` on the
instrumentation, and ensuring a callback is only called conditionally.

Note: For some (e.g. connect, fastify) we register `spanStart` in the
error handler, which is even better, so there we do not need this logic.

Author: mydea

packages/node/src/integrations/tracing/connect.ts

@@ -104,7 +104,7 @@ function addConnectSpanAttributes(span: Span): void {
     [SEMANTIC_ATTRIBUTE_SENTRY_OP]: `${type}.connect`,
   });
 
-  // Also update the name, we don't need to "middleware - " prefix
+  // Also update the name, we don't need the "middleware - " prefix
   const name = attributes['connect.name'];
   if (typeof name === 'string') {
     span.updateName(name);

packages/node/src/integrations/tracing/dataloader.ts

@@ -6,7 +6,7 @@ import {
   spanToJSON,
 } from '@sentry/core';
 import type { IntegrationFn } from '@sentry/core';
-import { generateInstrumentOnce } from '../../otel/instrument';
+import { instrumentWhenWrapped, generateInstrumentOnce } from '../../otel/instrument';
 
 const INTEGRATION_NAME = 'Dataloader';
 
@@ -19,31 +19,37 @@ export const instrumentDataloader = generateInstrumentOnce(
 );
 
 const _dataloaderIntegration = (() => {
+  let instrumentationWrappedCallback: undefined | ((callback: () => void) => void);
+
   return {
     name: INTEGRATION_NAME,
     setupOnce() {
-      instrumentDataloader();
+      const instrumentation = instrumentDataloader();
+      instrumentationWrappedCallback = instrumentWhenWrapped(instrumentation);
     },
 
     setup(client) {
-      client.on('spanStart', span => {
-        const spanJSON = spanToJSON(span);
-        if (spanJSON.description?.startsWith('dataloader')) {
-          span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, 'auto.db.otel.dataloader');
-        }
+      // This is called either immediately or when the instrumentation is wrapped
+      instrumentationWrappedCallback?.(() => {
+        client.on('spanStart', span => {
+          const spanJSON = spanToJSON(span);
+          if (spanJSON.description?.startsWith('dataloader')) {
+            span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, 'auto.db.otel.dataloader');
+          }
 
-        // These are all possible dataloader span descriptions
-        // Still checking for the future versions
-        // in case they add support for `clear` and `prime`
-        if (
-          spanJSON.description === 'dataloader.load' ||
-          spanJSON.description === 'dataloader.loadMany' ||
-          spanJSON.description === 'dataloader.batch'
-        ) {
-          span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_OP, 'cache.get');
-          // TODO: We can try adding `key` to the `data` attribute upstream.
-          // Or alternatively, we can add `requestHook` to the dataloader instrumentation.
-        }
+          // These are all possible dataloader span descriptions
+          // Still checking for the future versions
+          // in case they add support for `clear` and `prime`
+          if (
+            spanJSON.description === 'dataloader.load' ||
+            spanJSON.description === 'dataloader.loadMany' ||
+            spanJSON.description === 'dataloader.batch'
+          ) {
+            span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_OP, 'cache.get');
+            // TODO: We can try adding `key` to the `data` attribute upstream.
+            // Or alternatively, we can add `requestHook` to the dataloader instrumentation.
+          }
+        });
       });
     },
   };

packages/node/src/integrations/tracing/genericPool.ts

@@ -1,33 +1,38 @@
 import { GenericPoolInstrumentation } from '@opentelemetry/instrumentation-generic-pool';
 import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, defineIntegration, spanToJSON } from '@sentry/core';
 import type { IntegrationFn } from '@sentry/core';
-import { generateInstrumentOnce } from '../../otel/instrument';
+import { generateInstrumentOnce, instrumentWhenWrapped } from '../../otel/instrument';
 
 const INTEGRATION_NAME = 'GenericPool';
 
 export const instrumentGenericPool = generateInstrumentOnce(INTEGRATION_NAME, () => new GenericPoolInstrumentation({}));
 
 const _genericPoolIntegration = (() => {
+  let instrumentationWrappedCallback: undefined | ((callback: () => void) => void);
+
   return {
     name: INTEGRATION_NAME,
     setupOnce() {
-      instrumentGenericPool();
+      const instrumentation = instrumentGenericPool();
+      instrumentationWrappedCallback = instrumentWhenWrapped(instrumentation);
     },
 
     setup(client) {
-      client.on('spanStart', span => {
-        const spanJSON = spanToJSON(span);
+      instrumentationWrappedCallback?.(() =>
+        client.on('spanStart', span => {
+          const spanJSON = spanToJSON(span);
 
-        const spanDescription = spanJSON.description;
+          const spanDescription = spanJSON.description;
 
-        // typo in emitted span for version <= 0.38.0 of @opentelemetry/instrumentation-generic-pool
-        const isGenericPoolSpan =
-          spanDescription === 'generic-pool.aquire' || spanDescription === 'generic-pool.acquire';
+          // typo in emitted span for version <= 0.38.0 of @opentelemetry/instrumentation-generic-pool
+          const isGenericPoolSpan =
+            spanDescription === 'generic-pool.aquire' || spanDescription === 'generic-pool.acquire';
 
-        if (isGenericPoolSpan) {
-          span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, 'auto.db.otel.generic_pool');
-        }
-      });
+          if (isGenericPoolSpan) {
+            span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, 'auto.db.otel.generic_pool');
+          }
+        }),
+      );
     },
   };
 }) satisfies IntegrationFn;

packages/node/src/integrations/tracing/knex.ts

@@ -1,7 +1,7 @@
 import { KnexInstrumentation } from '@opentelemetry/instrumentation-knex';
 import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, defineIntegration, spanToJSON } from '@sentry/core';
 import type { IntegrationFn } from '@sentry/core';
-import { generateInstrumentOnce } from '../../otel/instrument';
+import { generateInstrumentOnce, instrumentWhenWrapped } from '../../otel/instrument';
 
 const INTEGRATION_NAME = 'Knex';
 
@@ -11,21 +11,26 @@ export const instrumentKnex = generateInstrumentOnce(
 );
 
 const _knexIntegration = (() => {
+  let instrumentationWrappedCallback: undefined | ((callback: () => void) => void);
+
   return {
     name: INTEGRATION_NAME,
     setupOnce() {
-      instrumentKnex();
+      const instrumentation = instrumentKnex();
+      instrumentationWrappedCallback = instrumentWhenWrapped(instrumentation);
     },
 
     setup(client) {
-      client.on('spanStart', span => {
-        const { data } = spanToJSON(span);
-        // knex.version is always set in the span data
-        // https://github.com/open-telemetry/opentelemetry-js-contrib/blob/0309caeafc44ac9cb13a3345b790b01b76d0497d/plugins/node/opentelemetry-instrumentation-knex/src/instrumentation.ts#L138
-        if ('knex.version' in data) {
-          span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, 'auto.db.otel.knex');
-        }
-      });
+      instrumentationWrappedCallback?.(() =>
+        client.on('spanStart', span => {
+          const { data } = spanToJSON(span);
+          // knex.version is always set in the span data
+          // https://github.com/open-telemetry/opentelemetry-js-contrib/blob/0309caeafc44ac9cb13a3345b790b01b76d0497d/plugins/node/opentelemetry-instrumentation-knex/src/instrumentation.ts#L138
+          if ('knex.version' in data) {
+            span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, 'auto.db.otel.knex');
+          }
+        }),
+      );
     },
   };
 }) satisfies IntegrationFn;

packages/node/src/integrations/tracing/prisma.ts

@@ -19,6 +19,18 @@ function isPrismaV6TracingHelper(helper: unknown): helper is PrismaV6TracingHelp
   return !!helper && typeof helper === 'object' && 'dispatchEngineSpans' in helper;
 }
 
+function getPrismaTracingHelper(): unknown | undefined {
+  const prismaInstrumentationObject = (globalThis as Record<string, unknown>).PRISMA_INSTRUMENTATION;
+  const prismaTracingHelper =
+    prismaInstrumentationObject &&
+    typeof prismaInstrumentationObject === 'object' &&
+    'helper' in prismaInstrumentationObject
+      ? prismaInstrumentationObject.helper
+      : undefined;
+
+  return prismaTracingHelper;
+}
+
 class SentryPrismaInteropInstrumentation extends EsmInteropPrismaInstrumentation {
   public constructor() {
     super();
@@ -30,13 +42,7 @@ class SentryPrismaInteropInstrumentation extends EsmInteropPrismaInstrumentation
     // The PrismaIntegration (super class) defines a global variable `global["PRISMA_INSTRUMENTATION"]` when `enable()` is called. This global variable holds a "TracingHelper" which Prisma uses internally to create tracing data. It's their way of not depending on OTEL with their main package. The sucky thing is, prisma broke the interface of the tracing helper with the v6 major update. This means that if you use Prisma 5 with the v6 instrumentation (or vice versa) Prisma just blows up, because tries to call methods on the helper that no longer exist.
     // Because we actually want to use the v6 instrumentation and not blow up in Prisma 5 user's faces, what we're doing here is backfilling the v5 method (`createEngineSpan`) with a noop so that no longer crashes when it attempts to call that function.
     // We still won't fully emit all the spans, but this could potentially be implemented in the future.
-    const prismaInstrumentationObject = (globalThis as Record<string, unknown>).PRISMA_INSTRUMENTATION;
-    const prismaTracingHelper =
-      prismaInstrumentationObject &&
-      typeof prismaInstrumentationObject === 'object' &&
-      'helper' in prismaInstrumentationObject
-        ? prismaInstrumentationObject.helper
-        : undefined;
+    const prismaTracingHelper = getPrismaTracingHelper();
 
     let emittedWarning = false;
 
@@ -119,6 +125,12 @@ export const prismaIntegration = defineIntegration(
         instrumentPrisma({ prismaInstrumentation });
       },
       setup(client) {
+        // If no tracing helper exists, we skip any work here
+        // this means that prisma is not being used
+        if (!getPrismaTracingHelper()) {
+          return;
+        }
+
         client.on('spanStart', span => {
           const spanJSON = spanToJSON(span);
           if (spanJSON.description?.startsWith('prisma:')) {

packages/node/src/integrations/tracing/tedious.ts

@@ -1,7 +1,7 @@
 import { TediousInstrumentation } from '@opentelemetry/instrumentation-tedious';
 import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, defineIntegration, spanToJSON } from '@sentry/core';
 import type { IntegrationFn } from '@sentry/core';
-import { generateInstrumentOnce } from '../../otel/instrument';
+import { generateInstrumentOnce, instrumentWhenWrapped } from '../../otel/instrument';
 
 const TEDIUS_INSTRUMENTED_METHODS = new Set([
   'callProcedure',
@@ -17,25 +17,30 @@ const INTEGRATION_NAME = 'Tedious';
 export const instrumentTedious = generateInstrumentOnce(INTEGRATION_NAME, () => new TediousInstrumentation({}));
 
 const _tediousIntegration = (() => {
+  let instrumentationWrappedCallback: undefined | ((callback: () => void) => void);
+
   return {
     name: INTEGRATION_NAME,
     setupOnce() {
-      instrumentTedious();
+      const instrumentation = instrumentTedious();
+      instrumentationWrappedCallback = instrumentWhenWrapped(instrumentation);
     },
 
     setup(client) {
-      client.on('spanStart', span => {
-        const { description, data } = spanToJSON(span);
-        // Tedius integration always set a span name and `db.system` attribute to `mssql`.
-        if (!description || data['db.system'] !== 'mssql') {
-          return;
-        }
-
-        const operation = description.split(' ')[0] || '';
-        if (TEDIUS_INSTRUMENTED_METHODS.has(operation)) {
-          span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, 'auto.db.otel.tedious');
-        }
-      });
+      instrumentationWrappedCallback?.(() =>
+        client.on('spanStart', span => {
+          const { description, data } = spanToJSON(span);
+          // Tedius integration always set a span name and `db.system` attribute to `mssql`.
+          if (!description || data['db.system'] !== 'mssql') {
+            return;
+          }
+
+          const operation = description.split(' ')[0] || '';
+          if (TEDIUS_INSTRUMENTED_METHODS.has(operation)) {
+            span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, 'auto.db.otel.tedious');
+          }
+        }),
+      );
     },
   };
 }) satisfies IntegrationFn;