From 0b093b404a8890151be66e5c6d53a810007b059f Mon Sep 17 00:00:00 2001 From: Bryan English Date: Fri, 21 Jun 2024 17:26:14 -0400 Subject: [PATCH] test on a swath of versions, in CI only --- .github/workflows/project.yml | 14 ++++ init.js | 21 +++-- initialize.mjs | 13 +-- integration-tests/helpers.js | 48 ++++------- integration-tests/init.spec.js | 142 +++++++++++++++++++-------------- 5 files changed, 135 insertions(+), 103 deletions(-) diff --git a/.github/workflows/project.yml b/.github/workflows/project.yml index 258d827bdf2..a74051512ba 100644 --- a/.github/workflows/project.yml +++ b/.github/workflows/project.yml @@ -30,6 +30,20 @@ jobs: - run: sudo sysctl -w kernel.core_pattern='|/bin/false' - run: yarn test:integration + # We'll run these separately for earlier (i.e. unsupported) versions + integration-guardrails: + strategy: + matrix: + version: [12, 14, 16] + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-node@v3 + with: + node-version: ${{ matrix.version }} + - run: yarn install --ignore-engines + - run: node node_modules/.bin/mocha --colors --timeout 30000 -r packages/dd-trace/test/setup/core.js integration-tests/init.spec.js + integration-ci: strategy: matrix: diff --git a/init.js b/init.js index 871fdd26375..7ddc8dbe91e 100644 --- a/init.js +++ b/init.js @@ -3,15 +3,25 @@ const path = require('path') const Module = require('module') const telemetry = require('./packages/dd-trace/src/telemetry/init-telemetry') -const Config = require('./packages/dd-trace/src/config') -const log = require('./packages/dd-trace/src/log') +const semver = require('semver') -// eslint-disable-next-line no-new -new Config() // we need this to initialize the logger +function isTrue (envVar) { + return ['1', 'true', 'True'].includes(envVar) +} + +// eslint-disable-next-line no-console +let log = { info: isTrue(process.env.DD_TRACE_DEBUG) ? console.log : () => {} } +if (semver.satisfies(process.versions.node, '>=16')) { + const Config = require('./packages/dd-trace/src/config') + log = require('./packages/dd-trace/src/log') + + // eslint-disable-next-line no-new + new Config() // we need this to initialize the logger +} let initBailout = false let clobberBailout = false -const forced = ['1', 'true', 'True'].includes(process.env.DD_INJECT_FORCE) +const forced = isTrue(process.env.DD_INJECT_FORCE) if (process.env.DD_INJECTION_ENABLED) { // If we're running via single-step install, and we're not in the app's @@ -36,7 +46,6 @@ if (process.env.DD_INJECTION_ENABLED) { if (!clobberBailout) { const { engines } = require('./package.json') const version = process.versions.node - const semver = require('semver') if (!semver.satisfies(version, engines.node)) { initBailout = true telemetry([ diff --git a/initialize.mjs b/initialize.mjs index e7b33de492b..777f45cc046 100644 --- a/initialize.mjs +++ b/initialize.mjs @@ -44,9 +44,12 @@ export async function getSource (...args) { } if (isMainThread) { - await import('./init.js') - const { register } = await import('node:module') - if (register) { - register('./loader-hook.mjs', import.meta.url) - } + // Need this IIFE for versions of Node.js without top-level await. + (async () => { + await import('./init.js') + const { register } = await import('node:module') + if (register) { + register('./loader-hook.mjs', import.meta.url) + } + })() } diff --git a/integration-tests/helpers.js b/integration-tests/helpers.js index a298e829a90..656a4bb84cd 100644 --- a/integration-tests/helpers.js +++ b/integration-tests/helpers.js @@ -7,10 +7,10 @@ const msgpack = require('msgpack-lite') const codec = msgpack.createCodec({ int64: true }) const EventEmitter = require('events') const childProcess = require('child_process') -const { fork, spawn, execSync } = childProcess +const { fork, spawn } = childProcess const exec = promisify(childProcess.exec) const http = require('http') -const fs = require('fs/promises') +const fs = require('fs') const os = require('os') const path = require('path') const rimraf = promisify(require('rimraf')) @@ -163,17 +163,8 @@ class FakeAgent extends EventEmitter { } } -let nodeBinary = process.argv[0] - async function runAndCheckOutput (filename, cwd, expectedOut) { - const oldOpts = process.env.NODE_OPTIONS - delete process.env.NODE_OPTIONS - const nodeVersion = nodeBinary === process.argv[0] - ? process.versions.node - : execSync(`${nodeBinary} -p process.versions.node`).toString().trim() - process.env.NODE_OPTIONS = oldOpts - - const proc = spawn(nodeBinary, [filename], { cwd, stdio: 'pipe' }) + const proc = spawn('node', [filename], { cwd, stdio: 'pipe' }) const pid = proc.pid let out = await new Promise((resolve, reject) => { proc.on('error', reject) @@ -181,6 +172,7 @@ async function runAndCheckOutput (filename, cwd, expectedOut) { proc.stdout.on('data', data => { out = Buffer.concat([out, data]) }) + proc.stderr.pipe(process.stdout) proc.on('exit', () => resolve(out.toString('utf8'))) setTimeout(() => { if (proc.exitCode === null) proc.kill() @@ -195,7 +187,7 @@ async function runAndCheckOutput (filename, cwd, expectedOut) { } assert.strictEqual(out, expectedOut) } - return [pid, nodeVersion] + return pid } // This is set by the useSandbox function @@ -205,7 +197,7 @@ let sandbox async function runAndCheckWithTelemetry (filename, expectedOut, ...expectedTelemetryPoints) { const cwd = sandbox.folder const cleanup = telemetryForwarder(expectedTelemetryPoints) - const [pid, nodeVersion] = await runAndCheckOutput(filename, cwd, expectedOut) + const pid = await runAndCheckOutput(filename, cwd, expectedOut) const msgs = await cleanup() if (expectedTelemetryPoints.length === 0) { // assert no telemetry sent @@ -251,9 +243,9 @@ async function runAndCheckWithTelemetry (filename, expectedOut, ...expectedTelem function meta (pid) { return { language_name: 'nodejs', - language_version: nodeVersion, + language_version: process.versions.node, runtime_name: 'nodejs', - runtime_version: nodeVersion, + runtime_version: process.versions.node, tracer_version: require('../package.json').version, pid: Number(pid) } @@ -303,9 +295,9 @@ async function createSandbox (dependencies = [], isGitRepo = false, // We might use NODE_OPTIONS to init the tracer. We don't want this to affect this operations const { NODE_OPTIONS, ...restOfEnv } = process.env - await fs.mkdir(folder) + fs.mkdirSync(folder) await exec(`yarn pack --filename ${out}`, { env: restOfEnv }) // TODO: cache this - await exec(`npm install ${allDependencies.join(' ')}`, { cwd: folder, env: restOfEnv }) + await exec(`yarn add ${allDependencies.join(' ')} --ignore-engines`, { cwd: folder, env: restOfEnv }) for (const path of integrationTestsPaths) { if (process.platform === 'win32') { @@ -327,7 +319,7 @@ async function createSandbox (dependencies = [], isGitRepo = false, if (isGitRepo) { await exec('git init', { cwd: folder }) - await fs.writeFile(path.join(folder, '.gitignore'), 'node_modules/', { flush: true }) + fs.writeFileSync(path.join(folder, '.gitignore'), 'node_modules/', { flush: true }) await exec('git config user.email "john@doe.com"', { cwd: folder }) await exec('git config user.name "John Doe"', { cwd: folder }) await exec('git config commit.gpgsign false', { cwd: folder }) @@ -356,10 +348,10 @@ function telemetryForwarder (expectedTelemetryPoints) { return cleanup() } - const cleanup = async function () { + const cleanup = function () { let msgs try { - msgs = (await fs.readFile(process.env.FORWARDER_OUT, 'utf8')).trim().split('\n') + msgs = fs.readFileSync(process.env.FORWARDER_OUT, 'utf8').trim().split('\n') } catch (e) { if (expectedTelemetryPoints.length && e.code === 'ENOENT' && retries < 10) { return tryAgain() @@ -382,7 +374,7 @@ function telemetryForwarder (expectedTelemetryPoints) { } msgs[i] = [telemetryType, parsed] } - await fs.unlink(process.env.FORWARDER_OUT) + fs.unlinkSync(process.env.FORWARDER_OUT) delete process.env.FORWARDER_OUT delete process.env.DD_TELEMETRY_FORWARDER_PATH return msgs @@ -484,15 +476,6 @@ function sandboxCwd () { return sandbox.folder } -function useSandboxedNode () { - before(() => { - nodeBinary = path.join(sandbox.folder, 'node_modules/.bin/node') - }) - after(() => { - nodeBinary = process.argv[0] - }) -} - module.exports = { FakeAgent, spawnProc, @@ -506,6 +489,5 @@ module.exports = { spawnPluginIntegrationTestProc, useEnv, useSandbox, - sandboxCwd, - useSandboxedNode + sandboxCwd } diff --git a/integration-tests/init.spec.js b/integration-tests/init.spec.js index 077046d0f19..e61237311b8 100644 --- a/integration-tests/init.spec.js +++ b/integration-tests/init.spec.js @@ -1,10 +1,12 @@ +const semver = require('semver') const { runAndCheckWithTelemetry: testFile, useEnv, useSandbox, - useSandboxedNode + sandboxCwd } = require('./helpers') const path = require('path') +const fs = require('fs') const DD_INJECTION_ENABLED = 'tracing' const DD_INJECT_FORCE = 'true' @@ -14,11 +16,12 @@ const telemetryAbort = ['abort', 'reason:incompatible_runtime', 'abort.runtime', const telemetryForced = ['complete', 'injection_forced:true'] const telemetryGood = ['complete', 'injection_forced:false'] -// Node.js versions prior to 16 aren't installable with `npm install node`, but -// this is early enough to test the version check. -const oldNodeVersion = '16.20.2' +const { engines } = require('../package.json') +const supportedRange = engines.node +const currentVersionIsSupported = semver.satisfies(process.versions.node, supportedRange) function testInjectionScenarios (arg, filename, esmWorks = false) { + if (!currentVersionIsSupported) return const doTest = (file, ...args) => testFile(file, ...args) context('preferring app-dir dd-trace', () => { context('when dd-trace is not in the app dir', () => { @@ -74,80 +77,101 @@ function testRuntimeVersionChecks (arg, filename) { } } - context('when node version is less than engines field', () => { - useSandboxedNode() - useEnv({ NODE_OPTIONS }) - - it('should initialize the tracer, if no DD_INJECTION_ENABLED', () => - doTest('true\n')) - context('with DD_INJECTION_ENABLED', () => { - useEnv({ DD_INJECTION_ENABLED }) - - context('without debug', () => { - it('should not initialize the tracer', () => doTest('false\n', ...telemetryAbort)) - it('should initialize the tracer, if DD_INJECT_FORCE', () => doTestForced('true\n', ...telemetryForced)) - }) - context('with debug', () => { - useEnv({ DD_TRACE_DEBUG }) - - it('should not initialize the tracer', () => - doTest(`Aborting application instrumentation due to incompatible_runtime. -Found incompatible runtime nodejs ${oldNodeVersion}, Supported runtimes: nodejs >=18. + if (!currentVersionIsSupported) { + context('when node version is less than engines field', () => { + useEnv({ NODE_OPTIONS }) + + it('should initialize the tracer, if no DD_INJECTION_ENABLED', () => + doTest('true\n')) + context('with DD_INJECTION_ENABLED', () => { + useEnv({ DD_INJECTION_ENABLED }) + + context('without debug', () => { + it('should not initialize the tracer', () => doTest('false\n', ...telemetryAbort)) + it('should initialize the tracer, if DD_INJECT_FORCE', () => doTestForced('true\n', ...telemetryForced)) + }) + context('with debug', () => { + useEnv({ DD_TRACE_DEBUG }) + + it('should not initialize the tracer', () => + doTest(`Aborting application instrumentation due to incompatible_runtime. +Found incompatible runtime nodejs ${process.versions.node}, Supported runtimes: nodejs >=18. false `, ...telemetryAbort)) - it('should initialize the tracer, if DD_INJECT_FORCE', () => - doTestForced(`Aborting application instrumentation due to incompatible_runtime. -Found incompatible runtime nodejs ${oldNodeVersion}, Supported runtimes: nodejs >=18. + it('should initialize the tracer, if DD_INJECT_FORCE', () => + doTestForced(`Aborting application instrumentation due to incompatible_runtime. +Found incompatible runtime nodejs ${process.versions.node}, Supported runtimes: nodejs >=18. DD_INJECT_FORCE enabled, allowing unsupported runtimes and continuing. Application instrumentation bootstrapping complete true `, ...telemetryForced)) + }) }) }) - }) - context('when node version is more than engines field', () => { - useEnv({ NODE_OPTIONS }) - - it('should initialize the tracer, if no DD_INJECTION_ENABLED', () => doTest('true\n')) - context('with DD_INJECTION_ENABLED', () => { - useEnv({ DD_INJECTION_ENABLED }) - - context('without debug', () => { - it('should initialize the tracer', () => doTest('true\n', ...telemetryGood)) - it('should initialize the tracer, if DD_INJECT_FORCE', () => - doTestForced('true\n', ...telemetryGood)) - }) - context('with debug', () => { - useEnv({ DD_TRACE_DEBUG }) - - it('should initialize the tracer', () => - doTest('Application instrumentation bootstrapping complete\ntrue\n', ...telemetryGood)) - it('should initialize the tracer, if DD_INJECT_FORCE', () => - doTestForced('Application instrumentation bootstrapping complete\ntrue\n', ...telemetryGood)) + } else { + context('when node version is more than engines field', () => { + useEnv({ NODE_OPTIONS }) + + it('should initialize the tracer, if no DD_INJECTION_ENABLED', () => doTest('true\n')) + context('with DD_INJECTION_ENABLED', () => { + useEnv({ DD_INJECTION_ENABLED }) + + context('without debug', () => { + it('should initialize the tracer', () => doTest('true\n', ...telemetryGood)) + it('should initialize the tracer, if DD_INJECT_FORCE', () => + doTestForced('true\n', ...telemetryGood)) + }) + context('with debug', () => { + useEnv({ DD_TRACE_DEBUG }) + + it('should initialize the tracer', () => + doTest('Application instrumentation bootstrapping complete\ntrue\n', ...telemetryGood)) + it('should initialize the tracer, if DD_INJECT_FORCE', () => + doTestForced('Application instrumentation bootstrapping complete\ntrue\n', ...telemetryGood)) + }) }) }) - }) + } }) } +function stubTracerIfNeeded () { + if (!currentVersionIsSupported) { + before(() => { + // Stub out the tracer in the sandbox, since it will not likely load properly. + // We're only doing this on versions we don't support, since the forcing + // action results in undefined behavior in the tracer. + fs.writeFileSync( + path.join(sandboxCwd(), 'node_modules/dd-trace/index.js'), + 'exports.init = () => { Object.assign(global, { _ddtrace: true }) }' + ) + }) + } +} + describe('init.js', () => { - useSandbox([`node@${oldNodeVersion}`]) + useSandbox() + stubTracerIfNeeded() testInjectionScenarios('require', 'init.js', false) testRuntimeVersionChecks('require', 'init.js') }) -describe('initialize.mjs', () => { - useSandbox([`node@${oldNodeVersion}`]) +// ESM is not supportable prior to Node.js 12 +if (semver.satisfies(process.versions.node, '>=12')) { + describe('initialize.mjs', () => { + useSandbox() + stubTracerIfNeeded() - context('as --loader', () => { - testInjectionScenarios('loader', 'initialize.mjs', true) - testRuntimeVersionChecks('loader', 'initialize.mjs') - }) - if (Number(process.versions.node.split('.')[0]) >= 18) { - context('as --import', () => { - testInjectionScenarios('import', 'initialize.mjs', true) + context('as --loader', () => { + testInjectionScenarios('loader', 'initialize.mjs', true) testRuntimeVersionChecks('loader', 'initialize.mjs') }) - } -}) + if (Number(process.versions.node.split('.')[0]) >= 18) { + context('as --import', () => { + testInjectionScenarios('import', 'initialize.mjs', true) + testRuntimeVersionChecks('loader', 'initialize.mjs') + }) + } + }) +}