PR feedback

integration-tests/helpers.js

@@ -7,7 +7,7 @@ const msgpack = require('msgpack-lite')
 const codec = msgpack.createCodec({ int64: true })
 const EventEmitter = require('events')
 const childProcess = require('child_process')
-const { fork } = childProcess
+const { fork, spawn, execSync } = childProcess
 const exec = promisify(childProcess.exec)
 const http = require('http')
 const fs = require('fs/promises')
@@ -163,8 +163,17 @@ class FakeAgent extends EventEmitter {
   }
 }
 
+let nodeBinary = process.argv[0]
+
 async function runAndCheckOutput (filename, cwd, expectedOut) {
-  const proc = fork(filename, { cwd, stdio: 'pipe' })
+  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 pid = proc.pid
   let out = await new Promise((resolve, reject) => {
     proc.on('error', reject)
@@ -186,7 +195,7 @@ async function runAndCheckOutput (filename, cwd, expectedOut) {
     }
     assert.strictEqual(out, expectedOut)
   }
-  return pid
+  return [pid, nodeVersion]
 }
 
 // This is set by the useSandbox function
@@ -196,7 +205,7 @@ let sandbox
 async function runAndCheckWithTelemetry (filename, expectedOut, ...expectedTelemetryPoints) {
   const cwd = sandbox.folder
   const cleanup = telemetryForwarder(expectedTelemetryPoints)
-  const pid = await runAndCheckOutput(filename, cwd, expectedOut)
+  const [pid, nodeVersion] = await runAndCheckOutput(filename, cwd, expectedOut)
   const msgs = await cleanup()
   if (expectedTelemetryPoints.length === 0) {
     // assert no telemetry sent
@@ -242,9 +251,9 @@ async function runAndCheckWithTelemetry (filename, expectedOut, ...expectedTelem
   function meta (pid) {
     return {
       language_name: 'nodejs',
-      language_version: process.env.FAKE_VERSION || process.versions.node,
+      language_version: nodeVersion,
       runtime_name: 'nodejs',
-      runtime_version: process.env.FAKE_VERSION || process.versions.node,
+      runtime_version: nodeVersion,
       tracer_version: require('../package.json').version,
       pid: Number(pid)
     }
@@ -296,7 +305,7 @@ async function createSandbox (dependencies = [], isGitRepo = false,
 
   await fs.mkdir(folder)
   await exec(`yarn pack --filename ${out}`, { env: restOfEnv }) // TODO: cache this
-  await exec(`yarn add ${allDependencies.join(' ')}`, { cwd: folder, env: restOfEnv })
+  await exec(`npm install ${allDependencies.join(' ')}`, { cwd: folder, env: restOfEnv })
 
   for (const path of integrationTestsPaths) {
     if (process.platform === 'win32') {
@@ -475,6 +484,15 @@ 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,
@@ -488,5 +506,6 @@ module.exports = {
   spawnPluginIntegrationTestProc,
   useEnv,
   useSandbox,
-  sandboxCwd
+  sandboxCwd,
+  useSandboxedNode
 }

integration-tests/init.spec.js

@@ -1,7 +1,8 @@
 const {
   runAndCheckWithTelemetry: testFile,
   useEnv,
-  useSandbox
+  useSandbox,
+  useSandboxedNode
 } = require('./helpers')
 const path = require('path')
 
@@ -13,6 +14,10 @@ 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'
+
 function testInjectionScenarios (arg, filename, esmWorks = false) {
   const doTest = (file, ...args) => testFile(file, ...args)
   context('preferring app-dir dd-trace', () => {
@@ -58,7 +63,7 @@ function testInjectionScenarios (arg, filename, esmWorks = false) {
 
 function testRuntimeVersionChecks (arg, filename) {
   context('runtime version check', () => {
-    const NODE_OPTIONS = `--require ${path.join(__dirname, 'init', 'setversion.js')} --${arg} dd-trace/${filename}`
+    const NODE_OPTIONS = `--${arg} dd-trace/${filename}`
     const doTest = (...args) => testFile('init/trace.js', ...args)
     const doTestForced = async (...args) => {
       Object.assign(process.env, { DD_INJECT_FORCE })
@@ -70,7 +75,8 @@ function testRuntimeVersionChecks (arg, filename) {
     }
 
     context('when node version is less than engines field', () => {
-      useEnv({ FAKE_VERSION: '3.0.0', NODE_OPTIONS })
+      useSandboxedNode()
+      useEnv({ NODE_OPTIONS })
 
       it('should initialize the tracer, if no DD_INJECTION_ENABLED', () =>
         doTest('true\n'))
@@ -86,12 +92,12 @@ function testRuntimeVersionChecks (arg, filename) {
 
           it('should not initialize the tracer', () =>
             doTest(`Aborting application instrumentation due to incompatible_runtime.
-Found incompatible runtime nodejs 3.0.0, Supported runtimes: nodejs >=18.
+Found incompatible runtime nodejs ${oldNodeVersion}, 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 3.0.0, Supported runtimes: nodejs >=18.
+Found incompatible runtime nodejs ${oldNodeVersion}, Supported runtimes: nodejs >=18.
 DD_INJECT_FORCE enabled, allowing unsupported runtimes and continuing.
 Application instrumentation bootstrapping complete
 true
@@ -100,7 +106,7 @@ true
       })
     })
     context('when node version is more than engines field', () => {
-      useEnv({ FAKE_VERSION: '50000.0.0', NODE_OPTIONS })
+      useEnv({ NODE_OPTIONS })
 
       it('should initialize the tracer, if no DD_INJECTION_ENABLED', () => doTest('true\n'))
       context('with DD_INJECTION_ENABLED', () => {
@@ -125,25 +131,23 @@ true
 }
 
 describe('init.js', () => {
-  useSandbox()
+  useSandbox([`node@${oldNodeVersion}`])
 
   testInjectionScenarios('require', 'init.js', false)
   testRuntimeVersionChecks('require', 'init.js')
 })
 
 describe('initialize.mjs', () => {
-  useSandbox()
+  useSandbox([`node@${oldNodeVersion}`])
 
   context('as --loader', () => {
     testInjectionScenarios('loader', 'initialize.mjs', true)
-    // TODO uncomment next line and fix this before enabling ESM in SSI
-    // testRuntimeVersionChecks('loader', 'initialize.mjs')
+    testRuntimeVersionChecks('loader', 'initialize.mjs')
   })
   if (Number(process.versions.node.split('.')[0]) >= 18) {
     context('as --import', () => {
       testInjectionScenarios('import', 'initialize.mjs', true)
-      // TODO uncomment next line and fix this before enabling ESM in SSI
-      // testRuntimeVersionChecks('loader', 'initialize.mjs')
+      testRuntimeVersionChecks('loader', 'initialize.mjs')
     })
   }
 })

packages/dd-trace/src/telemetry/init-telemetry.js

@@ -3,6 +3,7 @@
 const fs = require('fs')
 const { spawn } = require('child_process')
 const tracerVersion = require('../../../../package.json').version
+const log = require('../log')
 
 module.exports = sendTelemetry
 
@@ -59,5 +60,16 @@ function sendTelemetry (name, tags = []) {
   const proc = spawn(process.env.DD_TELEMETRY_FORWARDER_PATH, ['library_entrypoint'], {
     stdio: 'pipe'
   })
+  proc.on('error', () => {
+    log.error('Failed to spawn telemetry forwarder')
+  })
+  proc.on('exit', (code) => {
+    if (code !== 0) {
+      log.error(`Telemetry forwarder exited with code ${code}`)
+    }
+  })
+  proc.stdin.on('error', () => {
+    log.error('Failed to write telemetry data to telemetry forwarder')
+  })
   proc.stdin.end(JSON.stringify({ metadata, points }))
 }