Skip to content

Commit fbbe3ae

Browse files
committed
fix(node): do not pass options to orchestrion opt-in
This resolves the API surface wart where options for orchestrion integrations had to be passed into the `experimentalUseDiagnosticsChannelInjection` method, and returns that to being an argument-free void-returning opt-in that can be easily no-op'ed in the future, and all options are passed in via `Sentry.init()` as they were in the prior OTel implementation.
1 parent 9d241da commit fbbe3ae

6 files changed

Lines changed: 150 additions & 42 deletions

File tree

packages/node/src/integrations/tracing/mysql/index.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,20 @@ import { MySQLInstrumentation } from './vendored/instrumentation';
22
import type { IntegrationFn } from '@sentry/core';
33
import { defineIntegration } from '@sentry/core';
44
import { generateInstrumentOnce } from '@sentry/node-core';
5+
import { getChannelIntegrationFactory } from '../../../sdk/diagnosticsChannelInjection';
56

67
const INTEGRATION_NAME = 'Mysql' as const;
78

89
export const instrumentMysql = generateInstrumentOnce(INTEGRATION_NAME, () => new MySQLInstrumentation({}));
910

1011
const _mysqlIntegration = (() => {
12+
// When opted into diagnostics-channel injection, build
13+
// orchestrion implementation instead, with same options.
14+
const channelIntegration = getChannelIntegrationFactory(INTEGRATION_NAME);
15+
if (channelIntegration) {
16+
return channelIntegration();
17+
}
18+
1119
return {
1220
name: INTEGRATION_NAME,
1321
setupOnce() {

packages/node/src/integrations/tracing/postgres/index.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { PgInstrumentation } from './vendored/instrumentation';
22
import type { IntegrationFn } from '@sentry/core';
33
import { defineIntegration } from '@sentry/core';
44
import { generateInstrumentOnce } from '@sentry/node-core';
5+
import { getChannelIntegrationFactory } from '../../../sdk/diagnosticsChannelInjection';
56

67
interface PostgresIntegrationOptions {
78
ignoreConnectSpans?: boolean;
@@ -18,6 +19,13 @@ export const instrumentPostgres = generateInstrumentOnce(
1819
);
1920

2021
const _postgresIntegration = ((options?: PostgresIntegrationOptions) => {
22+
// When opted into diagnostics-channel injection, build
23+
// orchestrion implementation instead, with same options.
24+
const channelIntegration = getChannelIntegrationFactory(INTEGRATION_NAME);
25+
if (channelIntegration) {
26+
return channelIntegration(options);
27+
}
28+
2129
return {
2230
name: INTEGRATION_NAME,
2331
setupOnce() {

packages/node/src/sdk/diagnosticsChannelInjection.ts

Lines changed: 32 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import type { Integration } from '@sentry/core';
1+
import type { IntegrationFn } from '@sentry/core';
22

33
/**
44
* The orchestrion-driven pieces, resolved lazily by the opt-in loader.
@@ -13,10 +13,15 @@ import type { Integration } from '@sentry/core';
1313
* normally.
1414
*/
1515
export interface DiagnosticsChannelInjection {
16-
/** Channel-based integrations to register, replacing their OTel equivalents. */
17-
integrations: Integration[];
18-
/** OTel integration names these replace; filtered out of the default set. */
19-
replacedOtelIntegrationNames: string[];
16+
/**
17+
* Channel-based integration factories, keyed by the OTel integration name
18+
* they replace (e.g. `Postgres`). The matching OTel integration factory
19+
* (e.g. `postgresIntegration()`) looks itself up here and, when present,
20+
* builds the channel implementation instead, forwarding the user's options.
21+
* This is how opting in swaps implementations without the OTel factory
22+
* ever importing the orchestrion code (which would defeat tree-shaking).
23+
*/
24+
integrationFactories: Record<string, IntegrationFn>;
2025
/** Installs the module hooks that inject the diagnostics channels. */
2126
register: () => void;
2227
/** Warns (DEBUG only) about missing or doubled channel injection. */
@@ -35,6 +40,8 @@ let cached: DiagnosticsChannelInjection | undefined;
3540
*/
3641
export function setDiagnosticsChannelInjectionLoader(load: () => DiagnosticsChannelInjection): void {
3742
loader = load;
43+
// A new loader invalidates anything memoized from a previous one.
44+
cached = undefined;
3845
}
3946

4047
/** Whether `experimentalUseDiagnosticsChannelInjection()` was called. */
@@ -54,3 +61,23 @@ export function resolveDiagnosticsChannelInjection(): DiagnosticsChannelInjectio
5461
}
5562
return (cached ??= loader());
5663
}
64+
65+
/**
66+
* The channel-based integration factory registered to replace the OTel
67+
* integration named `name` (e.g. `Postgres`), or `undefined` when the app
68+
* didn't opt into diagnostics-channel injection. Node integration factories
69+
* call this to decide whether to build the orchestrion implementation (with
70+
* the user's options) instead of the OTel one, without ever importing
71+
* orchestrion directly.
72+
*
73+
* @internal
74+
*/
75+
export function getChannelIntegrationFactory(name: string): IntegrationFn | undefined {
76+
return resolveDiagnosticsChannelInjection()?.integrationFactories[name];
77+
}
78+
79+
/** Test-only: clear the registered loader and memoized result. */
80+
export function _resetDiagnosticsChannelInjectionForTesting(): void {
81+
loader = undefined;
82+
cached = undefined;
83+
}

packages/node/src/sdk/experimentalUseDiagnosticsChannelInjection.ts

Lines changed: 26 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -27,35 +27,45 @@ import { setDiagnosticsChannelInjectionLoader } from './diagnosticsChannelInject
2727
* OpenTelemetry ones, and installs the module hooks that inject the channels
2828
* (so libraries imported after `init()` publish the channel events).
2929
*
30+
* The swap is per-integration and transparent: the OTel integration factories
31+
* build their channel equivalent when this has been called, so you configure
32+
* them exactly as before.
33+
*
34+
* For example, to suppress pg connect spans on the orchestrion path:
35+
*
36+
* ```ts
37+
* Sentry.experimentalUseDiagnosticsChannelInjection();
38+
* Sentry.init({
39+
* dsn: '__DSN__',
40+
* integrations: [Sentry.postgresIntegration({ ignoreConnectSpans: true })],
41+
* });
42+
* ```
43+
*
3044
* This is a standalone function rather than an `init()` option so that a
3145
* bundler drops all of it (and its transitive deps) when this function isn't
32-
* called. `init()` reads the loader registered below.
46+
* called. The integration factories reach the channel implementations only via
47+
* the registry populated below, so they never import the orchestrion code
48+
* themselves, keeping the tree-shaking boundary at this function.
3349
*
3450
* An app that DOES call it gets the orchestrion code bundled as intended.
3551
*
3652
* In an unbundled (server-side runtime) app this eagerly loads only the small
3753
* subscriber/channel modules; the heavy code-transform dependencies stay lazy
3854
* inside `register()` and load only when injection actually runs.
3955
*
40-
* Per-integration options are passed here rather than via the OTel
41-
* `xxxIntegration({...})` instances, because those are swapped out wholesale for
42-
* their channel equivalents (and a user-provided OTel instance would otherwise
43-
* win integration de-duplication, silently keeping the OTel path). For example,
44-
* to suppress pg connect spans on the orchestrion path:
45-
*
46-
* ```ts
47-
* Sentry.experimentalUseDiagnosticsChannelInjection({ postgres: { ignoreConnectSpans: true } });
48-
* ```
49-
*
5056
* @experimental May change or be removed in any release.
5157
*/
52-
export function experimentalUseDiagnosticsChannelInjection(
53-
options: { postgres?: { ignoreConnectSpans?: boolean } } = {},
54-
): void {
58+
// Note: This function is to remain argument-free and void-returning so that
59+
// it can be easily no-op'ed (or provided with a `false` flag or something to
60+
// opt-out) when orchestrion becomes the default.
61+
export function experimentalUseDiagnosticsChannelInjection(): void {
5562
setDiagnosticsChannelInjectionLoader(
5663
(): DiagnosticsChannelInjection => ({
57-
integrations: [mysqlChannelIntegration(), postgresChannelIntegration(options.postgres)],
58-
replacedOtelIntegrationNames: ['Mysql', 'Postgres'],
64+
// Keyed by the OTel integration name each one replaces.
65+
integrationFactories: {
66+
Mysql: mysqlChannelIntegration,
67+
Postgres: postgresChannelIntegration,
68+
},
5969
register: registerDiagnosticsChannelInjection,
6070
detect: detectOrchestrionSetup,
6171
}),

packages/node/src/sdk/index.ts

Lines changed: 5 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -30,32 +30,16 @@ export function getDefaultIntegrationsWithoutPerformance(): Integration[] {
3030

3131
/** Get the default integrations for the Node SDK. */
3232
export function getDefaultIntegrations(options: Options): Integration[] {
33-
const integrations: Integration[] = [
33+
return [
3434
...getDefaultIntegrationsWithoutPerformance(),
3535
// We only add performance integrations if tracing is enabled
36-
// Note that this means that without tracing enabled, e.g. `expressIntegration()` will not be added
37-
// This means that generally request isolation will work (because that is done by httpIntegration)
36+
// Note that this means that without tracing enabled, e.g.
37+
// `expressIntegration()` will not be added
38+
// This means that generally request isolation will work (because that is
39+
// done by httpIntegration)
3840
// But `transactionName` will not be set automatically
3941
...(hasSpansEnabled(options) ? getAutoPerformanceIntegrations() : []),
4042
];
41-
42-
// When the app opted into diagnostics-channel injection (via
43-
// `experimentalUseDiagnosticsChannelInjection()`) AND span recording is
44-
// enabled, swap the channel-based integrations in place of OTel equivalents
45-
// so the two don't both instrument the same library.
46-
//
47-
// Every channel-based integration we ship today is a 1:1 replacement for an
48-
// OTel performance/tracing integration and produces nothing but spans (those
49-
// only come from `getAutoPerformanceIntegrations()` above), so it's gated on
50-
// span recording.
51-
if (isDiagnosticsChannelInjectionEnabled() && hasSpansEnabled(options)) {
52-
const diagnosticsChannelInjection = resolveDiagnosticsChannelInjection();
53-
if (diagnosticsChannelInjection) {
54-
const replaced = new Set(diagnosticsChannelInjection.replacedOtelIntegrationNames);
55-
return [...integrations.filter(i => !replaced.has(i.name)), ...diagnosticsChannelInjection.integrations];
56-
}
57-
}
58-
return integrations;
5943
}
6044

6145
/**
Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
import type { Integration } from '@sentry/core';
2+
import { afterEach, describe, expect, it, vi } from 'vitest';
3+
import { mysqlIntegration } from '../../src/integrations/tracing/mysql';
4+
import { postgresIntegration } from '../../src/integrations/tracing/postgres';
5+
import {
6+
_resetDiagnosticsChannelInjectionForTesting,
7+
setDiagnosticsChannelInjectionLoader,
8+
} from '../../src/sdk/diagnosticsChannelInjection';
9+
10+
// `experimentalUseDiagnosticsChannelInjection()` registers
11+
// channel-integration factories into a runtime registry; the OTel
12+
// `xxxIntegration()` factories look themselves up there and build the
13+
// channel implementation instead. We exercise that swap by registering fakes
14+
// directly, so this file never imports the orchestrion code (which is exactly
15+
// the property that keeps it tree-shakeable).
16+
17+
afterEach(() => {
18+
_resetDiagnosticsChannelInjectionForTesting();
19+
vi.restoreAllMocks();
20+
});
21+
22+
describe('OTel integration factory <-> channel swap', () => {
23+
it('returns the OTel implementation when injection is not opted into', () => {
24+
// No loader registered (the default), so no channel factory is found and
25+
// the factories build their own OTel implementation.
26+
const pg = postgresIntegration();
27+
const mysql = mysqlIntegration();
28+
29+
expect(pg.name).toBe('Postgres');
30+
expect(typeof pg.setupOnce).toBe('function');
31+
expect(mysql.name).toBe('Mysql');
32+
expect(typeof mysql.setupOnce).toBe('function');
33+
});
34+
35+
it('builds the registered channel implementation, forwarding options, when opted in', () => {
36+
const pgChannel: Integration = { name: 'Postgres', setupOnce: vi.fn() };
37+
const mysqlChannel: Integration = { name: 'Mysql', setupOnce: vi.fn() };
38+
const pgFactory = vi.fn(() => pgChannel);
39+
const mysqlFactory = vi.fn(() => mysqlChannel);
40+
41+
setDiagnosticsChannelInjectionLoader(() => ({
42+
integrationFactories: { Postgres: pgFactory, Mysql: mysqlFactory },
43+
register: vi.fn(),
44+
detect: vi.fn(),
45+
}));
46+
47+
const pg = postgresIntegration({ ignoreConnectSpans: true });
48+
const mysql = mysqlIntegration();
49+
50+
// The factory's instance is returned verbatim
51+
expect(pg).toBe(pgChannel);
52+
expect(mysql).toBe(mysqlChannel);
53+
// The user's options reach the channel factory unchanged.
54+
expect(pgFactory).toHaveBeenCalledWith({ ignoreConnectSpans: true });
55+
});
56+
57+
it('only swaps the integrations that have a registered factory', () => {
58+
const pgChannel: Integration = { name: 'Postgres', setupOnce: vi.fn() };
59+
60+
setDiagnosticsChannelInjectionLoader(() => ({
61+
integrationFactories: { Postgres: () => pgChannel },
62+
register: vi.fn(),
63+
detect: vi.fn(),
64+
}));
65+
66+
// Postgres has a registered factory; mysql does not, so it stays OTel.
67+
expect(postgresIntegration()).toBe(pgChannel);
68+
expect(mysqlIntegration().name).toBe('Mysql');
69+
expect(mysqlIntegration()).not.toBe(pgChannel);
70+
});
71+
});

0 commit comments

Comments
 (0)