esbuild: ensure aws-sdk can be built (#3591)

* fix esbuild aws-sdk issue by forgoing use of proxy file in favor of injecting instrumentation code directly into the module code --------- Co-authored-by: Ayan Khan <[email protected]>
DataDog · Sep 26, 2023 · 5494f6b · 5494f6b
1 parent ed839d9
commit 5494f6b
Show file tree

Hide file tree

Showing 7 changed files with 92 additions and 35 deletions.
diff --git a/LICENSE-3rdparty.csv b/LICENSE-3rdparty.csv
@@ -32,6 +32,7 @@ require,retry,MIT,Copyright 2011 Tim Koschützki Felix Geisendörfer
 require,semver,ISC,Copyright Isaac Z. Schlueter and Contributors
 dev,@types/node,MIT,Copyright Authors
 dev,autocannon,MIT,Copyright 2016 Matteo Collina
+dev,aws-sdk,Apache 2.0,Copyright 2012-2017 Amazon.com, Inc. or its affiliates. All Rights Reserved.
 dev,axios,MIT,Copyright 2014-present Matt Zabriskie
 dev,benchmark,MIT,Copyright 2010-2016 Mathias Bynens Robert Kieffer John-David Dalton
 dev,body-parser,MIT,Copyright 2014 Jonathan Ong 2014-2015 Douglas Christopher Wilson

diff --git a/integration-tests/esbuild.spec.js b/integration-tests/esbuild.spec.js
@@ -1,5 +1,7 @@
 #!/usr/bin/env node
 
+/* eslint-disable no-console */
+
 'use strict'
 
 const chproc = require('child_process')
@@ -10,22 +12,18 @@ const TEST_DIR = path.join(__dirname, 'esbuild')
 
 describe('esbuild', () => {
   it('works', () => {
-    // eslint-disable-next-line no-console
     console.log(`cd ${TEST_DIR}`)
     process.chdir(TEST_DIR)
 
-    // eslint-disable-next-line no-console
     console.log('npm run build')
     chproc.execSync('npm run build')
 
-    // eslint-disable-next-line no-console
     console.log('npm run built')
     try {
       chproc.execSync('npm run built', {
         timeout: 1000 * 30
       })
     } catch (err) {
-      // eslint-disable-next-line no-console
       console.error(err)
       process.exit(1)
     } finally {
@@ -34,18 +32,15 @@ describe('esbuild', () => {
   })
 
   it('does not bundle modules listed in .external', () => {
-    // eslint-disable-next-line no-console
     console.log(`cd ${TEST_DIR}`)
     process.chdir(TEST_DIR)
 
     try {
-      // eslint-disable-next-line no-console
       console.log('node ./build-and-test-skip-external.js')
       chproc.execSync('node ./build-and-test-skip-external.js', {
         timeout: 1000 * 30
       })
     } catch (err) {
-      // eslint-disable-next-line no-console
       console.error(err)
       process.exit(1)
     } finally {
@@ -54,18 +49,32 @@ describe('esbuild', () => {
   })
 
   it('handles typescript apps that import without file extensions', () => {
-    // eslint-disable-next-line no-console
     console.log(`cd ${TEST_DIR}`)
     process.chdir(TEST_DIR)
 
     try {
-      // eslint-disable-next-line no-console
       console.log('node ./build-and-test-typescript.mjs')
       chproc.execSync('node ./build-and-test-typescript.mjs', {
         timeout: 1000 * 30
       })
     } catch (err) {
-      // eslint-disable-next-line no-console
+      console.error(err)
+      process.exit(1)
+    } finally {
+      process.chdir(CWD)
+    }
+  })
+
+  it('handles the complex aws-sdk package with dynamic requires', () => {
+    console.log(`cd ${TEST_DIR}`)
+    process.chdir(TEST_DIR)
+
+    try {
+      console.log('node ./build-and-test-aws-sdk.js')
+      chproc.execSync('node ./build-and-test-aws-sdk.js', {
+        timeout: 1000 * 30
+      })
+    } catch (err) {
       console.error(err)
       process.exit(1)
     } finally {

diff --git a/integration-tests/esbuild/aws-sdk.js b/integration-tests/esbuild/aws-sdk.js
@@ -0,0 +1,5 @@
+require('../../').init() // dd-trace
+
+const aws = require('aws-sdk')
+
+void aws.util.inherit
diff --git a/integration-tests/esbuild/build-and-test-aws-sdk.js b/integration-tests/esbuild/build-and-test-aws-sdk.js
@@ -0,0 +1,35 @@
+#!/usr/bin/env node
+/* eslint-disable no-console */
+const { spawnSync } = require('child_process')
+
+const ddPlugin = require('../../esbuild') // dd-trace/esbuild
+const esbuild = require('esbuild')
+
+const SCRIPT = './aws-sdk-out.js'
+
+esbuild.build({
+  entryPoints: ['aws-sdk.js'],
+  bundle: true,
+  outfile: SCRIPT,
+  plugins: [ddPlugin],
+  platform: 'node',
+  target: ['node16'],
+  external: [ ]
+}).then(() => {
+  const { status, stdout, stderr } = spawnSync('node', [SCRIPT])
+  if (stdout.length) {
+    console.log(stdout.toString())
+  }
+  if (stderr.length) {
+    console.error(stderr.toString())
+  }
+  if (status) {
+    throw new Error('generated script failed to run')
+  }
+  console.log('ok')
+}).catch((err) => {
+  console.error(err)
+  process.exit(1)
+}).finally(() => {
+  // fs.rmSync(SCRIPT)
+})
diff --git a/integration-tests/esbuild/package.json b/integration-tests/esbuild/package.json
@@ -18,6 +18,7 @@
   "author": "Thomas Hunter II <[email protected]>",
   "license": "ISC",
   "dependencies": {
+    "aws-sdk": "^2.1446.0",
     "axios": "^0.21.2",
     "esbuild": "0.16.12",
     "express": "^4.16.2",

diff --git a/package.json b/package.json
@@ -103,6 +103,7 @@
   "devDependencies": {
     "@types/node": ">=16",
     "autocannon": "^4.5.2",
+    "aws-sdk": "^2.1446.0",
     "axios": "^0.21.2",
     "benchmark": "^2.1.4",
     "body-parser": "^1.20.2",

diff --git a/packages/datadog-esbuild/index.js b/packages/datadog-esbuild/index.js
@@ -23,11 +23,12 @@ for (const instrumentation of Object.values(instrumentations)) {
   }
 }
 
-const NAMESPACE = 'datadog'
 const NM = 'node_modules/'
 const INSTRUMENTED = Object.keys(instrumentations)
 const RAW_BUILTINS = require('module').builtinModules
 const CHANNEL = 'dd-trace:bundler:load'
+const path = require('path')
+const fs = require('fs')
 
 const builtins = new Set()
 
@@ -109,55 +110,59 @@ module.exports.setup = function (build) {
       // https://esbuild.github.io/plugins/#on-resolve-arguments
       return {
         path: fullPathToModule,
-        namespace: NAMESPACE,
         pluginData: {
           version: packageJson.version,
           pkg: extracted.pkg,
           path: extracted.path,
           full: fullPathToModule,
           raw: args.path,
+          pkgOfInterest: true,
           internal
         }
       }
-    } else if (args.namespace === NAMESPACE) {
-      // The datadog namespace is used when requiring files that are injected during the onLoad stage
-
-      if (builtins.has(args.path)) return
-
-      return {
-        path: require.resolve(args.path, { paths: [ args.resolveDir ] }),
-        namespace: 'file'
-      }
     }
   })
 
-  build.onLoad({ filter: /.*/, namespace: NAMESPACE }, args => {
+  build.onLoad({ filter: /.*/ }, args => {
+    if (!args.pluginData?.pkgOfInterest) {
+      return
+    }
+
     const data = args.pluginData
 
     if (DEBUG) console.log(`LOAD: ${data.pkg}@${data.version}, pkg "${data.path}"`)
 
-    const path = data.raw !== data.pkg
+    const pkgPath = data.raw !== data.pkg
       ? `${data.pkg}/${data.path}`
       : data.pkg
 
+    // Read the content of the module file of interest
+    const fileCode = fs.readFileSync(args.path, 'utf8')
+
     const contents = `
-      const dc = require('diagnostics_channel');
-      const ch = dc.channel('${CHANNEL}');
-      const mod = require('${args.path}');
-      const payload = {
-        module: mod,
-        version: '${data.version}',
-        package: '${data.pkg}',
-        path: '${path}'
-      };
-      ch.publish(payload);
-      module.exports = payload.module;
+      (function() {
+        ${fileCode}
+      })(...arguments);
+      {
+        const dc = require('diagnostics_channel');
+        const ch = dc.channel('${CHANNEL}');
+        const mod = module.exports
+        const payload = {
+          module: mod,
+          version: '${data.version}',
+          package: '${data.pkg}',
+          path: '${pkgPath}'
+        };
+        ch.publish(payload);
+        module.exports = payload.module;
+    }
     `
 
     // https://esbuild.github.io/plugins/#on-load-results
     return {
       contents,
-      loader: 'js'
+      loader: 'js',
+      resolveDir: path.dirname(args.path)
     }
   })
 }