From 56289c46042ed7a5f0964dbbaf45e433e40096b9 Mon Sep 17 00:00:00 2001 From: isaacs Date: Thu, 9 Jul 2026 10:53:32 -0700 Subject: [PATCH] feat(node): move dc integration swap to _init Move the diagnostics-channel integration swap out of `getDefaultIntegrations` and into `_init`, so it also applies to framework SDKs that pass their own `defaultIntegrations` array, and is skipped when defaults are explicitly empty. This will be needed because the NestJS SDK (and others) build on top of the Node SDK, but swap out their default integrations. --- packages/node/src/sdk/index.ts | 48 +++---- .../sdk/diagnosticsChannelInjection.test.ts | 117 ++++++++++++++++++ 2 files changed, 142 insertions(+), 23 deletions(-) create mode 100644 packages/node/test/sdk/diagnosticsChannelInjection.test.ts diff --git a/packages/node/src/sdk/index.ts b/packages/node/src/sdk/index.ts index 8c8d2e887541..77c3c3bc6fcc 100644 --- a/packages/node/src/sdk/index.ts +++ b/packages/node/src/sdk/index.ts @@ -30,7 +30,7 @@ export function getDefaultIntegrationsWithoutPerformance(): Integration[] { /** Get the default integrations for the Node SDK. */ export function getDefaultIntegrations(options: Options): Integration[] { - const integrations: Integration[] = [ + return [ ...getDefaultIntegrationsWithoutPerformance(), // We only add performance integrations if tracing is enabled // Note that this means that without tracing enabled, e.g. `expressIntegration()` will not be added @@ -38,24 +38,6 @@ export function getDefaultIntegrations(options: Options): Integration[] { // But `transactionName` will not be set automatically ...(hasSpansEnabled(options) ? getAutoPerformanceIntegrations() : []), ]; - - // When the app opted into diagnostics-channel injection (via - // `experimentalUseDiagnosticsChannelInjection()`) AND span recording is - // enabled, swap the channel-based integrations in place of OTel equivalents - // so the two don't both instrument the same library. - // - // Every channel-based integration we ship today is a 1:1 replacement for an - // OTel performance/tracing integration and produces nothing but spans (those - // only come from `getAutoPerformanceIntegrations()` above), so it's gated on - // span recording. - if (isDiagnosticsChannelInjectionEnabled() && hasSpansEnabled(options)) { - const diagnosticsChannelInjection = resolveDiagnosticsChannelInjection(); - if (diagnosticsChannelInjection) { - const replaced = new Set(diagnosticsChannelInjection.replacedOtelIntegrationNames); - return [...integrations.filter(i => !replaced.has(i.name)), ...diagnosticsChannelInjection.integrations]; - } - } - return integrations; } /** @@ -77,8 +59,7 @@ function _init( // EXPERIMENTAL: diagnostics-channel injection, opted into via // `experimentalUseDiagnosticsChannelInjection()`. Gated on span recording to // match the OTel integrations it replaces. With tracing off there are no - // channel subscribers, so injecting is pointless work. `resolve...()` is - // memoized, so `getDefaultIntegrations()` (below) sees the same instance. + // channel subscribers, so injecting is pointless work. const diagnosticsChannelInjection = isDiagnosticsChannelInjectionEnabled() && hasSpansEnabled(options) ? resolveDiagnosticsChannelInjection() @@ -90,10 +71,31 @@ function _init( diagnosticsChannelInjection.register(); } + // Only use Node SDK defaults if none provided. + let defaultIntegrations = options.defaultIntegrations ?? getDefaultIntegrationsImpl(options); + + // When opted into diagnostics-channel injection, swap the channel-based + // integrations in place of their OTel equivalents so the two don't both + // instrument the same library. Done here (rather than in + // `getDefaultIntegrations`) so it also covers framework SDKs (e.g. + // `@sentry/nestjs`) that pass their own `defaultIntegrations` array. + // + // Only when there's a non-empty default set to swap: + // `defaultIntegrations: false` (not an array) and `[]` / + // `initWithoutDefaultIntegrations()` (explicitly no defaults) are left + // untouched, as appending channel integrations there would resurrect + // defaults the caller opted out of. + if (diagnosticsChannelInjection && Array.isArray(defaultIntegrations) && defaultIntegrations.length > 0) { + const replaced = new Set(diagnosticsChannelInjection.replacedOtelIntegrationNames); + defaultIntegrations = [ + ...defaultIntegrations.filter(integration => !replaced.has(integration.name)), + ...diagnosticsChannelInjection.integrations, + ]; + } + const client = initNodeCore({ ...options, - // Only use Node SDK defaults if none provided - defaultIntegrations: options.defaultIntegrations ?? getDefaultIntegrationsImpl(options), + defaultIntegrations, }); // Add Node SDK specific OpenTelemetry setup diff --git a/packages/node/test/sdk/diagnosticsChannelInjection.test.ts b/packages/node/test/sdk/diagnosticsChannelInjection.test.ts new file mode 100644 index 000000000000..b1412904e33c --- /dev/null +++ b/packages/node/test/sdk/diagnosticsChannelInjection.test.ts @@ -0,0 +1,117 @@ +import type { Integration } from '@sentry/core'; +import { debug } from '@sentry/core'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { init, initWithoutDefaultIntegrations } from '../../src/sdk'; +import { setDiagnosticsChannelInjectionLoader } from '../../src/sdk/diagnosticsChannelInjection'; +import { cleanupOtel, resetGlobals } from '../helpers/mockSdkInit'; + +// eslint-disable-next-line no-var +declare var global: any; + +const PUBLIC_DSN = 'https://username@domain/123'; + +function mockIntegration(name: string): Integration { + return { name, setupOnce: vi.fn() }; +} + +// These tests run in definition order: the first runs before any loader is set +// (opt-out), the second sets it (opt-in). The module-level loader state is +// isolated per test file by vitest, so it doesn't leak elsewhere. +describe('diagnostics-channel injection integration swap', () => { + beforeEach(() => { + global.__SENTRY__ = {}; + vi.spyOn(debug, 'enable').mockImplementation(() => undefined); + }); + + afterEach(() => { + cleanupOtel(); + resetGlobals(); + vi.clearAllMocks(); + }); + + it('does not swap integrations when not opted in', () => { + // Distinct names from the opt-in test below: `@sentry/core` only runs + // `setupOnce` once per integration name per process, so reusing names across + // tests would suppress later calls. + const otelNest = mockIntegration('OptOutNest'); + const http = mockIntegration('OptOutHttp'); + + init({ + dsn: PUBLIC_DSN, + tracesSampleRate: 1, + skipOpenTelemetrySetup: true, + defaultIntegrations: [otelNest, http], + }); + + // No opt-in -> the supplied defaults are set up untouched. + expect(otelNest.setupOnce).toHaveBeenCalledTimes(1); + expect(http.setupOnce).toHaveBeenCalledTimes(1); + }); + + it('replaces the named OTel integrations with the channel integrations, even when defaultIntegrations are supplied by a framework SDK', () => { + const channelMysql = mockIntegration('Mysql'); + const channelNest = mockIntegration('Nest'); + const register = vi.fn(); + const detect = vi.fn(); + setDiagnosticsChannelInjectionLoader(() => ({ + integrations: [channelMysql, channelNest], + replacedOtelIntegrationNames: ['Mysql', 'Nest'], + register, + detect, + })); + + // Mimics `@sentry/nestjs`, which prepends its OTel `Nest` integration to + // its own `defaultIntegrations` array (so node's `getDefaultIntegrations` + // swap never sees it; swap must happen in `init`). + const otelNest = mockIntegration('Nest'); + const http = mockIntegration('Http'); + + init({ + dsn: PUBLIC_DSN, + tracesSampleRate: 1, + skipOpenTelemetrySetup: true, + defaultIntegrations: [otelNest, http], + }); + + // OTel 'Nest' filtered out, never set up. + expect(otelNest.setupOnce).not.toHaveBeenCalled(); + // Channel replacements set up instead. + expect(channelNest.setupOnce).toHaveBeenCalledTimes(1); + expect(channelMysql.setupOnce).toHaveBeenCalledTimes(1); + // Unrelated default preserved. + expect(http.setupOnce).toHaveBeenCalledTimes(1); + // Hooks installed and detection ran once. + expect(register).toHaveBeenCalledTimes(1); + expect(detect).toHaveBeenCalledTimes(1); + }); + + it('does not add channel integrations when defaults are explicitly empty', () => { + const channelEmptyMysql = mockIntegration('EmptyMysql'); + setDiagnosticsChannelInjectionLoader(() => ({ + integrations: [channelEmptyMysql], + replacedOtelIntegrationNames: ['EmptyMysql'], + register: vi.fn(), + detect: vi.fn(), + })); + + // `defaultIntegrations: []` opts out of all defaults; the swap must not + // resurrect them by appending the channel integrations. + init({ dsn: PUBLIC_DSN, tracesSampleRate: 1, skipOpenTelemetrySetup: true, defaultIntegrations: [] }); + + expect(channelEmptyMysql.setupOnce).not.toHaveBeenCalled(); + }); + + it('does not add channel integrations to initWithoutDefaultIntegrations()', () => { + const channelNoDefaults = mockIntegration('NoDefaultsMysql'); + setDiagnosticsChannelInjectionLoader(() => ({ + integrations: [channelNoDefaults], + replacedOtelIntegrationNames: ['NoDefaultsMysql'], + register: vi.fn(), + detect: vi.fn(), + })); + + initWithoutDefaultIntegrations({ dsn: PUBLIC_DSN, tracesSampleRate: 1, skipOpenTelemetrySetup: true }); + + expect(channelNoDefaults.setupOnce).not.toHaveBeenCalled(); + }); +});