diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index ebb2f0f97b05..f8674b6ec3c6 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -773,8 +773,8 @@ jobs: job_node_integration_tests: name: - Node (${{ matrix.node }})${{ (matrix.typescript && format(' (TS {0})', matrix.typescript)) || '' }} Integration - Tests + Node (${{ matrix.node }})${{ (matrix.typescript && format(' (TS {0})', matrix.typescript)) || '' }}${{ + (matrix.use_orchestrion && format(' (Orchestrion)', matrix.use_orchestrion)) || '' }} Integration Tests needs: [job_get_metadata, job_build] if: needs.job_build.outputs.changed_node_integration == 'true' || github.event_name != 'pull_request' runs-on: ubuntu-24.04 @@ -785,10 +785,21 @@ jobs: node: [18, 20, 22, 24, 26] typescript: - false + use_orchestrion: + - false include: # Only check typescript for latest version (to streamline CI) - node: 24 typescript: '3.8' + # No need to test orchestrion for v18 + - node: 20 + use_orchestrion: 'true' + - node: 22 + use_orchestrion: 'true' + - node: 24 + use_orchestrion: 'true' + - node: 26 + use_orchestrion: 'true' steps: - name: Check out current commit (${{ needs.job_get_metadata.outputs.commit_label }}) uses: actions/checkout@v6 @@ -811,6 +822,8 @@ jobs: - name: Run integration tests working-directory: dev-packages/node-integration-tests run: yarn test + env: + INJECT_ORCHESTRION: ${{ matrix.use_orchestrion }} job_node_v18_compat: name: Node v18.0.0 Compatibility Check diff --git a/dev-packages/node-integration-tests/package.json b/dev-packages/node-integration-tests/package.json index aebda7d552a1..1c45cd043c08 100644 --- a/dev-packages/node-integration-tests/package.json +++ b/dev-packages/node-integration-tests/package.json @@ -20,6 +20,7 @@ "lint:fix": "OXLINT_TSGOLINT_DANGEROUSLY_SUPPRESS_PROGRAM_DIAGNOSTICS=true oxlint . --fix --type-aware", "type-check": "tsc", "test": "vitest run", + "test:orchestrion": "INJECT_ORCHESTRION=true yarn test", "test:watch": "yarn test --watch" }, "dependencies": { diff --git a/dev-packages/node-integration-tests/src/index.ts b/dev-packages/node-integration-tests/src/index.ts index ed6a150bd8d6..109d93f7ef19 100644 --- a/dev-packages/node-integration-tests/src/index.ts +++ b/dev-packages/node-integration-tests/src/index.ts @@ -55,3 +55,8 @@ export function getPortAppIsRunningOn(app: Express): number | undefined { // @ts-expect-error It's not defined in the types but we'd like to read it. return app.port; } + +/** Returns true if orchestrion is enabled in env vars. */ +export function isOrchestrionEnabled(): boolean { + return process.env.INJECT_ORCHESTRION === 'true' || process.env.INJECT_ORCHESTRION === '1'; +} diff --git a/dev-packages/node-integration-tests/suites/tracing/lru-memoizer/instrument-orchestrion.mjs b/dev-packages/node-integration-tests/suites/tracing/lru-memoizer/instrument-orchestrion.mjs deleted file mode 100644 index 1565f28886ad..000000000000 --- a/dev-packages/node-integration-tests/suites/tracing/lru-memoizer/instrument-orchestrion.mjs +++ /dev/null @@ -1,17 +0,0 @@ -// Opting in via `experimentalUseDiagnosticsChannelInjection()` (before `init`) -// is all that's needed. -// -// `Sentry.init()` swaps the OTel `lru-memoizer` instrumentation -// for the diagnostics-channel one and synchronously -// installs the module hooks that inject the channels. -import * as Sentry from '@sentry/node'; -import { loggingTransport } from '@sentry-internal/node-integration-tests'; - -Sentry.experimentalUseDiagnosticsChannelInjection(); - -Sentry.init({ - dsn: 'https://public@dsn.ingest.sentry.io/1337', - release: '1.0', - tracesSampleRate: 1.0, - transport: loggingTransport, -}); diff --git a/dev-packages/node-integration-tests/suites/tracing/lru-memoizer/test.ts b/dev-packages/node-integration-tests/suites/tracing/lru-memoizer/test.ts index 71bf4fd50997..56f8961ea9aa 100644 --- a/dev-packages/node-integration-tests/suites/tracing/lru-memoizer/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/lru-memoizer/test.ts @@ -1,103 +1,59 @@ import { afterAll, describe, expect } from 'vitest'; import { cleanupChildProcesses, createEsmAndCjsTests } from '../../../utils/runner'; import { createCjsTests } from '../../../utils/runner/createEsmAndCjsTests'; +import { isOrchestrionEnabled } from '../../../utils'; describe('lru-memoizer', () => { afterAll(() => { cleanupChildProcesses(); }); - // Each case maps to the OpenTelemetry default and the diagnostics-channel opt-in - // variants, mirroring the mysql suite. `flags` are extra Node CLI flags; the - // instrument file is always loaded via `--import` (esm) / `--require` (cjs). - // - // lru-memoizer creates no spans, so there's no `sentry.origin` to - // assert: the opt-in cases prove the channel ran because the opt-in removes the - // OTel integration via `replacedOtelIntegrationNames`. - const CASES = [ - // OpenTelemetry default — no opt-in, no injection. (OTel does not support ESM.) - { label: 'opentelemetry (default)', instrument: 'instrument.mjs', flags: [], failsOnEsm: true }, - // Opt-in via init only. `Sentry.init()` injects the channels synchronously. - { - label: 'diagnostics-channel (init opt-in)', - instrument: 'instrument-orchestrion.mjs', - flags: [], - failsOnEsm: false, - }, - // Opt-in and rely on `node --import @sentry/node/import`. - { - label: 'diagnostics-channel (--import @sentry/node/import opt-in)', - instrument: 'instrument-orchestrion.mjs', - flags: ['--import', '@sentry/node/import'], - failsOnEsm: false, - }, - // Without opt-in: channels are injected unconditionally but not subscribed to, - // so the OTel instrumentation does the work — proves injecting the channels has - // no downside. (OTel does not support ESM.) - { - label: 'opentelemetry (channels injected, no opt-in)', - instrument: 'instrument.mjs', - flags: ['--import', '@sentry/node/import'], - failsOnEsm: true, - }, - ] as const; - - for (const { label, instrument, flags, failsOnEsm } of CASES) { - describe(label, () => { - createEsmAndCjsTests( - __dirname, - 'scenario.mjs', - instrument, - (createTestRunner, test) => { - test('keeps outer context inside the memoized inner functions', async () => { - await createTestRunner() - .withFlags(...flags) - .expect({ - transaction: { - transaction: '', - contexts: { - trace: expect.objectContaining({ - op: 'run', - data: expect.objectContaining({ - 'sentry.op': 'run', - 'sentry.origin': 'manual', - 'memoized.context_preserved': true, - }), - }), - }, - }, - }) - .start() - .completed(); - }); - }, - { failsOnEsm }, - ); - - // CJS-only: the parallel scenario is flaky in ESM (see #21729). - createCjsTests(__dirname, 'scenario-parallel.mjs', instrument, (createTestRunner, test) => { - test('keeps each span context across parallel memoized requests', async () => { - // Each parallel request emits a transaction whose callback must have run in its own context. - // Two identical expectations keep this order-independent. - const expectation = { + createEsmAndCjsTests( + __dirname, + 'scenario.mjs', + 'instrument.mjs', + (createTestRunner, test) => { + test('keeps outer context inside the memoized inner functions', async () => { + await createTestRunner() + .expect({ transaction: { + transaction: '', contexts: { trace: expect.objectContaining({ - op: expect.stringMatching(/^(first|second)$/), - data: expect.objectContaining({ 'memoized.context_preserved': true }), + op: 'run', + data: expect.objectContaining({ + 'sentry.op': 'run', + 'sentry.origin': 'manual', + 'memoized.context_preserved': true, + }), }), }, }, - }; - - await createTestRunner() - .withFlags(...flags) - .expect(expectation) - .expect(expectation) - .start() - .completed(); - }); + }) + .start() + .completed(); }); + }, + { failsOnEsm: !isOrchestrionEnabled() }, + ); + + // CJS-only: the parallel scenario is flaky in ESM (see #21729). + createCjsTests(__dirname, 'scenario-parallel.mjs', 'instrument.mjs', (createTestRunner, test) => { + test('keeps each span context across parallel memoized requests', async () => { + // Each parallel request emits a transaction whose callback must have run in its own context. + // Two identical expectations keep this order-independent. + const expectation = { + transaction: { + contexts: { + trace: expect.objectContaining({ + op: expect.stringMatching(/^(first|second)$/), + data: expect.objectContaining({ 'memoized.context_preserved': true }), + }), + }, + }, + }; + + await createTestRunner().expect(expectation).expect(expectation).start().completed(); }); - } + }); }); diff --git a/dev-packages/node-integration-tests/suites/tracing/mysql/test.ts b/dev-packages/node-integration-tests/suites/tracing/mysql/test.ts index 4a4bb592f24d..0cf5e04c1932 100644 --- a/dev-packages/node-integration-tests/suites/tracing/mysql/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing/mysql/test.ts @@ -4,6 +4,7 @@ import { cleanupChildProcesses, createCjsTests, createEsmAndCjsTests } from '../ import { startMysqlTestServer } from './mysql-test-server'; import type { SerializedStreamedSpanContainer } from '@sentry/core'; import { SEMANTIC_ATTRIBUTE_SENTRY_OP } from '@sentry/core'; +import { isOrchestrionEnabled } from '../../../utils'; describe('mysql auto instrumentation', () => { // A minimal in-process MySQL server (on a random free port) so the client's @@ -58,6 +59,8 @@ describe('mysql auto instrumentation', () => { }; } + // Note: here specifically, we want to ignore the generic orchestrion testing define via INJECT_ORCHESTRION, + // but instead test various different ways to run orchestrion manually const CHANNEL_ORIGIN = 'auto.db.orchestrion.mysql'; // Each case maps to one of the two documented use cases, in opt-in and @@ -137,7 +140,11 @@ describe('mysql auto instrumentation', () => { .completed(); }); }, - { failsOnEsm }, + { + failsOnEsm, + // We handle injection ourselves here + injectOrchestrion: false, + }, ); } @@ -174,7 +181,11 @@ describe('mysql auto instrumentation', () => { .completed(); }); }, - { failsOnEsm }, + { + failsOnEsm, + // We handle injection ourselves here + injectOrchestrion: false, + }, ); }); } @@ -225,7 +236,7 @@ describe('mysql auto instrumentation', () => { }, 'sentry.origin': { type: 'string', - value: 'auto.db.otel.mysql', + value: isOrchestrionEnabled() ? 'auto.db.orchestrion.mysql' : 'auto.db.otel.mysql', }, 'sentry.release': { type: 'string', diff --git a/dev-packages/node-integration-tests/utils/index.ts b/dev-packages/node-integration-tests/utils/index.ts index 92851b42ba5e..c41f365ba8db 100644 --- a/dev-packages/node-integration-tests/utils/index.ts +++ b/dev-packages/node-integration-tests/utils/index.ts @@ -55,3 +55,8 @@ export function conditionalTest(allowedVersion: { export const parseEnvelope = (body: string): Array> => { return body.split('\n').map(e => JSON.parse(e)); }; + +/** Returns true if orchestrion is enabled in env vars. */ +export function isOrchestrionEnabled(): boolean { + return process.env.INJECT_ORCHESTRION === 'true' || process.env.INJECT_ORCHESTRION === '1'; +} diff --git a/dev-packages/node-integration-tests/utils/runner/createEsmAndCjsTests.ts b/dev-packages/node-integration-tests/utils/runner/createEsmAndCjsTests.ts index c5395ee6351b..5def48851504 100644 --- a/dev-packages/node-integration-tests/utils/runner/createEsmAndCjsTests.ts +++ b/dev-packages/node-integration-tests/utils/runner/createEsmAndCjsTests.ts @@ -5,20 +5,37 @@ import { basename, join } from 'path'; import { promisify } from 'util'; import { afterAll, beforeAll, test, type TestAPI } from 'vitest'; import { CLEANUP_STEPS, createRunner } from './createRunner'; +import { isOrchestrionEnabled } from '../../utils'; const execPromise = promisify(exec); interface ScenarioPaths { cjs: { scenario: string; - instrument: string; + flags: string[]; }; esm: { scenario: string; - instrument: string; + flags: string[]; }; } +interface CommonTestOptions { + /** + * `additionalDependencies` to install in the tmp dir. + */ + additionalDependencies?: Record; + /** Copy these files/dirs into the tmp dir. */ + copyPaths?: string[]; + /** If orchestrion should be injected before any instrument file. */ + injectOrchestrion?: boolean; +} + +interface EsmAndCjsTestOptions extends CommonTestOptions { + failsOnCjs?: boolean; + failsOnEsm?: boolean; +} + /** * Run one or multiple tests in ESM and CJS for a given scenario and instrument file. */ @@ -32,28 +49,25 @@ export function createEsmAndCjsTests( mode: 'esm' | 'cjs', cwd: string, ) => void, - options?: { - failsOnCjs?: boolean; - failsOnEsm?: boolean; - /** - * `additionalDependencies` to install in the tmp dir. - */ - additionalDependencies?: Record; - /** Copy these files/dirs into the tmp dir. */ - copyPaths?: string[]; - }, + options?: EsmAndCjsTestOptions, ): void { - const [tmpDirPath, createTmpDir, paths] = prepareTmpDir(cwd, scenarioPath, instrumentPath, options); + const optionsWithDefaults = { + injectOrchestrion: isOrchestrionEnabled() || undefined, + ...options, + }; + const [tmpDirPath, createTmpDir, paths] = prepareTmpDir(cwd, scenarioPath, instrumentPath, optionsWithDefaults); - const esmTestFn = options?.failsOnEsm ? wrapTestApi(test.fails, 'esm - fails') : wrapTestApi(test, 'esm'); - const cjsTestFn = options?.failsOnCjs ? wrapTestApi(test.fails, 'cjs - fails') : wrapTestApi(test, 'cjs'); + const esmTestFn = optionsWithDefaults.failsOnEsm ? wrapTestApi(test.fails, 'esm - fails') : wrapTestApi(test, 'esm'); + const cjsTestFn = optionsWithDefaults.failsOnCjs ? wrapTestApi(test.fails, 'cjs - fails') : wrapTestApi(test, 'cjs'); const createdRunners = new Set>(); callback( () => { - const runner = createRunner(paths.esm.scenario).withFlags('--import', paths.esm.instrument); + const runner = createRunner(paths.esm.scenario) + .withEnv(optionsWithDefaults.injectOrchestrion ? { INJECT_ORCHESTRION: 'true' } : {}) + .withFlags(...paths.esm.flags); // Expected failure — don't dump the child's captured output for these. - if (options?.failsOnEsm) { + if (optionsWithDefaults.failsOnEsm) { runner.suppressErrorLogs(); } return trackRunner(createdRunners, runner); @@ -65,9 +79,11 @@ export function createEsmAndCjsTests( callback( () => { - const runner = createRunner(paths.cjs.scenario).withFlags('--require', paths.cjs.instrument); + const runner = createRunner(paths.cjs.scenario) + .withEnv(optionsWithDefaults.injectOrchestrion ? { INJECT_ORCHESTRION: 'true' } : {}) + .withFlags(...paths.cjs.flags); // Expected failure — don't dump the child's captured output for these. - if (options?.failsOnCjs) { + if (optionsWithDefaults.failsOnCjs) { runner.suppressErrorLogs(); } return trackRunner(createdRunners, runner); @@ -88,20 +104,23 @@ export function createEsmTests( scenarioPath: string, instrumentPath: string, callback: (createTestRunner: () => ReturnType, testFn: typeof test, cwd: string) => void, - options?: { - /** - * `additionalDependencies` to install in the tmp dir. - */ - additionalDependencies?: Record; - /** Copy these files/dirs into the tmp dir. */ - copyPaths?: string[]; - }, + options?: CommonTestOptions, ) { - const [tmpDirPath, createTmpDir, paths] = prepareTmpDir(cwd, scenarioPath, instrumentPath, options); + const optionsWithDefaults = { + injectOrchestrion: isOrchestrionEnabled() || undefined, + ...options, + }; + const [tmpDirPath, createTmpDir, paths] = prepareTmpDir(cwd, scenarioPath, instrumentPath, optionsWithDefaults); const createdRunners = new Set>(); callback( - () => trackRunner(createdRunners, createRunner(paths.esm.scenario).withFlags('--import', paths.esm.instrument)), + () => + trackRunner( + createdRunners, + createRunner(paths.esm.scenario) + .withEnv(optionsWithDefaults.injectOrchestrion ? { INJECT_ORCHESTRION: 'true' } : {}) + .withFlags(...paths.esm.flags), + ), test, tmpDirPath, ); @@ -117,20 +136,23 @@ export function createCjsTests( scenarioPath: string, instrumentPath: string, callback: (createTestRunner: () => ReturnType, testFn: typeof test, cwd: string) => void, - options?: { - /** - * `additionalDependencies` to install in the tmp dir. - */ - additionalDependencies?: Record; - /** Copy these files/dirs into the tmp dir. */ - copyPaths?: string[]; - }, + options?: CommonTestOptions, ) { - const [tmpDirPath, createTmpDir, paths] = prepareTmpDir(cwd, scenarioPath, instrumentPath, options); + const optionsWithDefaults = { + injectOrchestrion: isOrchestrionEnabled() || undefined, + ...options, + }; + const [tmpDirPath, createTmpDir, paths] = prepareTmpDir(cwd, scenarioPath, instrumentPath, optionsWithDefaults); const createdRunners = new Set>(); callback( - () => trackRunner(createdRunners, createRunner(paths.cjs.scenario).withFlags('--require', paths.cjs.instrument)), + () => + trackRunner( + createdRunners, + createRunner(paths.cjs.scenario) + .withEnv(optionsWithDefaults.injectOrchestrion ? { INJECT_ORCHESTRION: 'true' } : {}) + .withFlags(...paths.cjs.flags), + ), test, tmpDirPath, ); @@ -189,8 +211,10 @@ function prepareTmpDir( cwd: string, scenarioPath: string, instrumentPath: string, - options?: { additionalDependencies?: Record; copyPaths?: string[] }, + options?: CommonTestOptions, ): [string, () => Promise, ScenarioPaths] { + const injectOrchestrion = options?.injectOrchestrion ?? false; + const mjsScenarioPath = join(cwd, scenarioPath); const mjsInstrumentPath = join(cwd, instrumentPath); @@ -213,6 +237,9 @@ function prepareTmpDir( const cjsScenarioPath = join(tmpDirPath, esmScenarioBasename.replace('.mjs', '.cjs')); const cjsInstrumentPath = join(tmpDirPath, esmInstrumentBasename.replace('.mjs', '.cjs')); + const cjsFlags: string[] = ['--require', cjsInstrumentPath]; + const esmFlags: string[] = ['--import', esmInstrumentPathForRun]; + async function createTmpDir(): Promise { await mkdir(tmpDirPath); @@ -224,6 +251,28 @@ function prepareTmpDir( await convertEsmFileToCjs(esmScenarioPathForRun, cjsScenarioPath); await convertEsmFileToCjs(esmInstrumentPathForRun, cjsInstrumentPath); + if (injectOrchestrion) { + const mjsInjectOrchetrionPath = join(tmpDirPath, 'inject-orchestrion.mjs'); + const cjsInjectOrchetrionPath = join(tmpDirPath, 'inject-orchestrion.cjs'); + + await writeFile( + mjsInjectOrchetrionPath, + `import {experimentalUseDiagnosticsChannelInjection} from '@sentry/node'; +experimentalUseDiagnosticsChannelInjection();`, + 'utf8', + ); + + await writeFile( + cjsInjectOrchetrionPath, + `const {experimentalUseDiagnosticsChannelInjection} = require('@sentry/node'); +experimentalUseDiagnosticsChannelInjection();`, + 'utf8', + ); + + esmFlags.unshift('--import', mjsInjectOrchetrionPath); + cjsFlags.unshift('--require', cjsInjectOrchetrionPath); + } + // Copy any additional files/dirs into tmp dir if (options?.copyPaths) { for (const path of options.copyPaths) { @@ -260,11 +309,11 @@ function prepareTmpDir( const paths: ScenarioPaths = { cjs: { scenario: cjsScenarioPath, - instrument: cjsInstrumentPath, + flags: cjsFlags, }, esm: { scenario: esmScenarioPathForRun, - instrument: esmInstrumentPathForRun, + flags: esmFlags, }, }; diff --git a/dev-packages/node-integration-tests/utils/runner/createRunner.ts b/dev-packages/node-integration-tests/utils/runner/createRunner.ts index 3b3f57ba35c3..5e576f4ff94e 100644 --- a/dev-packages/node-integration-tests/utils/runner/createRunner.ts +++ b/dev-packages/node-integration-tests/utils/runner/createRunner.ts @@ -191,7 +191,10 @@ export function createRunner(...paths: string[]) { return this; }, withEnv: function (env: Record) { - withEnv = env; + withEnv = { + ...withEnv, + ...env, + }; return this; }, withFlags: function (...args: string[]) { diff --git a/packages/server-utils/src/integrations/tracing-channel/mysql.ts b/packages/server-utils/src/integrations/tracing-channel/mysql.ts index 522bb0b962ce..84be904e9eec 100644 --- a/packages/server-utils/src/integrations/tracing-channel/mysql.ts +++ b/packages/server-utils/src/integrations/tracing-channel/mysql.ts @@ -6,6 +6,7 @@ import { defineIntegration, getCurrentScope, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, + SPAN_KIND, startInactiveSpan, waitForTracingChannelBinding, } from '@sentry/core'; @@ -82,6 +83,7 @@ const _mysqlChannelIntegration = (() => { return startInactiveSpan({ name: sql ?? 'mysql.query', + kind: SPAN_KIND.CLIENT, op: 'db', attributes: { [ATTR_DB_SYSTEM]: 'mysql',