Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 1.8k
feat(node): move dc integration swap to _init#22173
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -30,32 +30,14 @@ 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 | ||
| // This means that generally request isolation will work (because that is done by httpIntegration) | ||
| // 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, | ||
| ]; | ||
| } | ||
isaacs marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| const client = initNodeCore({ | ||
| ...options, | ||
| // Only use Node SDK defaults if none provided | ||
| defaultIntegrations: options.defaultIntegrations ?? getDefaultIntegrationsImpl(options), | ||
| defaultIntegrations, | ||
| }); | ||
| // Add Node SDK specific OpenTelemetry setup | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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(); | ||
| }); | ||
| }); | ||
cursor[bot] marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.