Merge branch 'master' into robertomonteromiguel/onboarding_simple_sce…

…nario

integration-tests/profiler/profiler.spec.js

@@ -13,7 +13,6 @@ const fsync = require('fs')
 const net = require('net')
 const zlib = require('zlib')
 const { Profile } = require('pprof-format')
-const semver = require('semver')
 
 const DEFAULT_PROFILE_TYPES = ['wall', 'space']
 if (process.platform !== 'win32') {
@@ -315,61 +314,59 @@ describe('profiler', () => {
       assert.equal(endpoints.size, 3, encoded)
     })
 
-    if (semver.gte(process.version, '16.0.0')) {
-      it('dns timeline events work', async () => {
-        const dnsEvents = await gatherNetworkTimelineEvents(cwd, 'profiler/dnstest.js', 'dns')
-        assert.sameDeepMembers(dnsEvents, [
-          { name: 'lookup', host: 'example.org' },
-          { name: 'lookup', host: 'example.com' },
-          { name: 'lookup', host: 'datadoghq.com' },
-          { name: 'queryA', host: 'datadoghq.com' },
-          { name: 'lookupService', address: '13.224.103.60', port: 80 }
-        ])
-      })
+    it('dns timeline events work', async () => {
+      const dnsEvents = await gatherNetworkTimelineEvents(cwd, 'profiler/dnstest.js', 'dns')
+      assert.sameDeepMembers(dnsEvents, [
+        { name: 'lookup', host: 'example.org' },
+        { name: 'lookup', host: 'example.com' },
+        { name: 'lookup', host: 'datadoghq.com' },
+        { name: 'queryA', host: 'datadoghq.com' },
+        { name: 'lookupService', address: '13.224.103.60', port: 80 }
+      ])
+    })
 
-      it('net timeline events work', async () => {
-        // Simple server that writes a constant message to the socket.
-        const msg = 'cya later!\n'
-        function createServer () {
-          const server = net.createServer((socket) => {
-            socket.end(msg, 'utf8')
-          }).on('error', (err) => {
-            throw err
-          })
-          return server
-        }
-        // Create two instances of the server
-        const server1 = createServer()
+    it('net timeline events work', async () => {
+      // Simple server that writes a constant message to the socket.
+      const msg = 'cya later!\n'
+      function createServer () {
+        const server = net.createServer((socket) => {
+          socket.end(msg, 'utf8')
+        }).on('error', (err) => {
+          throw err
+        })
+        return server
+      }
+      // Create two instances of the server
+      const server1 = createServer()
+      try {
+        const server2 = createServer()
         try {
-          const server2 = createServer()
-          try {
-            // Have the servers listen on ephemeral ports
-            const p = new Promise(resolve => {
-              server1.listen(0, () => {
-                server2.listen(0, async () => {
-                  resolve([server1.address().port, server2.address().port])
-                })
+          // Have the servers listen on ephemeral ports
+          const p = new Promise(resolve => {
+            server1.listen(0, () => {
+              server2.listen(0, async () => {
+                resolve([server1.address().port, server2.address().port])
               })
             })
-            const [port1, port2] = await p
-            const args = [String(port1), String(port2), msg]
-            // Invoke the profiled program, passing it the ports of the servers and
-            // the expected message.
-            const events = await gatherNetworkTimelineEvents(cwd, 'profiler/nettest.js', 'net', args)
-            // The profiled program should have two TCP connection events to the two
-            // servers.
-            assert.sameDeepMembers(events, [
-              { name: 'connect', host: '127.0.0.1', port: port1 },
-              { name: 'connect', host: '127.0.0.1', port: port2 }
-            ])
-          } finally {
-            server2.close()
-          }
+          })
+          const [port1, port2] = await p
+          const args = [String(port1), String(port2), msg]
+          // Invoke the profiled program, passing it the ports of the servers and
+          // the expected message.
+          const events = await gatherNetworkTimelineEvents(cwd, 'profiler/nettest.js', 'net', args)
+          // The profiled program should have two TCP connection events to the two
+          // servers.
+          assert.sameDeepMembers(events, [
+            { name: 'connect', host: '127.0.0.1', port: port1 },
+            { name: 'connect', host: '127.0.0.1', port: port2 }
+          ])
         } finally {
-          server1.close()
+          server2.close()
         }
-      })
-    }
+      } finally {
+        server1.close()
+      }
+    })
   }
 
   context('shutdown', () => {

packages/datadog-instrumentations/src/child_process.js

@@ -13,19 +13,38 @@ const childProcessChannel = dc.tracingChannel('datadog:child_process:execution')
 
 // ignored exec method because it calls to execFile directly
 const execAsyncMethods = ['execFile', 'spawn']
-const execSyncMethods = ['execFileSync', 'spawnSync']
 
 const names = ['child_process', 'node:child_process']
 
 // child_process and node:child_process returns the same object instance, we only want to add hooks once
 let patched = false
+
+function throwSyncError (error) {
+  throw error
+}
+
+function returnSpawnSyncError (error, context) {
+  context.result = {
+    error,
+    status: null,
+    signal: null,
+    output: null,
+    stdout: null,
+    stderr: null,
+    pid: 0
+  }
+
+  return context.result
+}
+
 names.forEach(name => {
   addHook({ name }, childProcess => {
     if (!patched) {
       patched = true
-      shimmer.massWrap(childProcess, execAsyncMethods, wrapChildProcessAsyncMethod())
-      shimmer.massWrap(childProcess, execSyncMethods, wrapChildProcessSyncMethod())
-      shimmer.wrap(childProcess, 'execSync', wrapChildProcessSyncMethod(true))
+      shimmer.massWrap(childProcess, execAsyncMethods, wrapChildProcessAsyncMethod(childProcess.ChildProcess))
+      shimmer.wrap(childProcess, 'execSync', wrapChildProcessSyncMethod(throwSyncError, true))
+      shimmer.wrap(childProcess, 'execFileSync', wrapChildProcessSyncMethod(throwSyncError))
+      shimmer.wrap(childProcess, 'spawnSync', wrapChildProcessSyncMethod(returnSpawnSyncError))
     }
 
     return childProcess
@@ -34,25 +53,43 @@ names.forEach(name => {
 
 function normalizeArgs (args, shell) {
   const childProcessInfo = {
-    command: args[0]
+    command: args[0],
+    file: args[0]
   }
 
   if (Array.isArray(args[1])) {
     childProcessInfo.command = childProcessInfo.command + ' ' + args[1].join(' ')
+    childProcessInfo.fileArgs = args[1]
+
     if (args[2] !== null && typeof args[2] === 'object') {
       childProcessInfo.options = args[2]
     }
   } else if (args[1] !== null && typeof args[1] === 'object') {
     childProcessInfo.options = args[1]
   }
+
   childProcessInfo.shell = shell ||
     childProcessInfo.options?.shell === true ||
     typeof childProcessInfo.options?.shell === 'string'
 
   return childProcessInfo
 }
 
-function wrapChildProcessSyncMethod (shell = false) {
+function createContextFromChildProcessInfo (childProcessInfo) {
+  const context = {
+    command: childProcessInfo.command,
+    file: childProcessInfo.file,
+    shell: childProcessInfo.shell
+  }
+
+  if (childProcessInfo.fileArgs) {
+    context.fileArgs = childProcessInfo.fileArgs
+  }
+
+  return context
+}
+
+function wrapChildProcessSyncMethod (returnError, shell = false) {
   return function wrapMethod (childProcessMethod) {
     return function () {
       if (!childProcessChannel.start.hasSubscribers || arguments.length === 0) {
@@ -63,14 +100,30 @@ function wrapChildProcessSyncMethod (shell = false) {
 
       const innerResource = new AsyncResource('bound-anonymous-fn')
       return innerResource.runInAsyncScope(() => {
-        return childProcessChannel.traceSync(
-          childProcessMethod,
-          {
-            command: childProcessInfo.command,
-            shell: childProcessInfo.shell
-          },
-          this,
-          ...arguments)
+        const context = createContextFromChildProcessInfo(childProcessInfo)
+        const abortController = new AbortController()
+
+        childProcessChannel.start.publish({ ...context, abortController })
+
+        try {
+          if (abortController.signal.aborted) {
+            const error = abortController.signal.reason || new Error('Aborted')
+            // expected behaviors on error are different
+            return returnError(error, context)
+          }
+
+          const result = childProcessMethod.apply(this, arguments)
+          context.result = result
+
+          return result
+        } catch (err) {
+          context.error = err
+          childProcessChannel.error.publish(context)
+
+          throw err
+        } finally {
+          childProcessChannel.end.publish(context)
+        }
       })
     }
   }
@@ -84,18 +137,52 @@ function wrapChildProcessCustomPromisifyMethod (customPromisifyMethod, shell) {
 
     const childProcessInfo = normalizeArgs(arguments, shell)
 
-    return childProcessChannel.tracePromise(
-      customPromisifyMethod,
-      {
-        command: childProcessInfo.command,
-        shell: childProcessInfo.shell
-      },
-      this,
-      ...arguments)
+    const context = createContextFromChildProcessInfo(childProcessInfo)
+
+    const { start, end, asyncStart, asyncEnd, error } = childProcessChannel
+    const abortController = new AbortController()
+
+    start.publish({
+      ...context,
+      abortController
+    })
+
+    let result
+    if (abortController.signal.aborted) {
+      result = Promise.reject(abortController.signal.reason || new Error('Aborted'))
+    } else {
+      try {
+        result = customPromisifyMethod.apply(this, arguments)
+      } catch (error) {
+        error.publish({ ...context, error })
+        throw error
+      } finally {
+        end.publish(context)
+      }
+    }
+
+    function reject (err) {
+      context.error = err
+      error.publish(context)
+      asyncStart.publish(context)
+
+      asyncEnd.publish(context)
+      return Promise.reject(err)
+    }
+
+    function resolve (result) {
+      context.result = result
+      asyncStart.publish(context)
+
+      asyncEnd.publish(context)
+      return result
+    }
+
+    return Promise.prototype.then.call(result, resolve, reject)
   }
 }
 
-function wrapChildProcessAsyncMethod (shell = false) {
+function wrapChildProcessAsyncMethod (ChildProcess, shell = false) {
   return function wrapMethod (childProcessMethod) {
     function wrappedChildProcessMethod () {
       if (!childProcessChannel.start.hasSubscribers || arguments.length === 0) {
@@ -112,9 +199,31 @@ function wrapChildProcessAsyncMethod (shell = false) {
 
       const innerResource = new AsyncResource('bound-anonymous-fn')
       return innerResource.runInAsyncScope(() => {
-        childProcessChannel.start.publish({ command: childProcessInfo.command, shell: childProcessInfo.shell })
+        const context = createContextFromChildProcessInfo(childProcessInfo)
+        const abortController = new AbortController()
+
+        childProcessChannel.start.publish({ ...context, abortController })
+
+        let childProcess
+        if (abortController.signal.aborted) {
+          childProcess = new ChildProcess()
+          childProcess.on('error', () => {}) // Original method does not crash when non subscribers
+
+          process.nextTick(() => {
+            const error = abortController.signal.reason || new Error('Aborted')
+            childProcess.emit('error', error)
+
+            const cb = arguments[arguments.length - 1]
+            if (typeof cb === 'function') {
+              cb(error)
+            }
+
+            childProcess.emit('close')
+          })
+        } else {
+          childProcess = childProcessMethod.apply(this, arguments)
+        }
 
-        const childProcess = childProcessMethod.apply(this, arguments)
         if (childProcess) {
           let errorExecuted = false
 
@@ -129,8 +238,7 @@ function wrapChildProcessAsyncMethod (shell = false) {
               childProcessChannel.error.publish()
             }
             childProcessChannel.asyncEnd.publish({
-              command: childProcessInfo.command,
-              shell: childProcessInfo.shell,
+              ...context,
               result: code
             })
           })