more resilient builtin require check

Author: tlhunter

packages/datadog-esbuild/index.js

@@ -1,8 +1,16 @@
 const NAMESPACE = 'datadog'
 
 const instrumented = Object.keys(require('../datadog-instrumentations/src/helpers/hooks.js'))
-const builtins = new Set(require('module').builtinModules)
-const packages = new Set()
+const rawBuiltins = require('module').builtinModules
+
+const builtins = new Set()
+
+for (const builtin of rawBuiltins) {
+  builtins.add(builtin)
+  builtins.add(`node:${builtin}`)
+}
+
+const packagesOfInterest = new Set()
 
 const DEBUG = !!process.env.DD_TRACE_DEBUG
 
@@ -12,7 +20,7 @@ const DEBUG = !!process.env.DD_TRACE_DEBUG
 for (const pkg of instrumented) {
   if (builtins.has(pkg)) continue
   if (pkg.startsWith('node:')) continue
-  packages.add(pkg)
+  packagesOfInterest.add(pkg)
 }
 
 const DC_CHANNEL = 'dd-trace:bundledModuleLoadStart'
@@ -23,7 +31,7 @@ module.exports.setup = function (build) {
   build.onResolve({ filter: /.*/ }, args => {
     const packageName = args.path
 
-    if (args.namespace === 'file' && packages.has(packageName)) {
+    if (args.namespace === 'file' && packagesOfInterest.has(packageName)) {
       // The file namespace is used when requiring files from disk in userland
       const pathToPackageJson = require.resolve(`${packageName}/package.json`, { paths: [ args.resolveDir ] })
       const pkg = require(pathToPackageJson)
@@ -45,7 +53,7 @@ module.exports.setup = function (build) {
       // The datadog namespace is used when requiring files that are injected during the onLoad stage
       // see note in onLoad
 
-      if (packageName.startsWith('node:')) return
+      if (builtins.has(packageName)) return
 
       return {
         path: require.resolve(packageName, { paths: [ args.resolveDir ] }),
@@ -59,8 +67,7 @@ module.exports.setup = function (build) {
       // eslint-disable-next-line no-console
       console.log(`load ${args.path}@${args.pluginData.version}`)
     }
-    // TODO: relying on prefixing internal packages with `node:` in this intermediary module for now.
-    // If this causes an issue we'll need to update the logic for determining if a module is internal.
+
     // JSON.stringify adds double quotes. For perf gain could simply add in quotes when we know it's safe.
     const contents = `
       const dc = require('node:diagnostics_channel');
@@ -71,29 +78,13 @@ module.exports.setup = function (build) {
         path: ${JSON.stringify(args.path)},
         version: ${JSON.stringify(args.pluginData.version)}
       };
-      // if (!ch.hasSubscribers) console.error('missing subscriber! ${JSON.stringify(DC_CHANNEL + ':' + args.path)}');
-      ch.publish(payload); // allow subscriber to mutate payload
+      ch.publish(payload);
       module.exports = payload.module;
-      // module.exports.__DATADOG_VERSION = ${JSON.stringify(args.pluginData.version)};
     `
     // https://esbuild.github.io/plugins/#on-load-results
     return {
       contents,
       loader: 'js'
     }
   })
-}
-
-/**
- * This could be a convenience to expose a list of known externals to the application.
- * Devs are free to use this list, ignore it, or merge it with their application-specific list.
- * TODO: Sadly, esbuild does not allow unanticipated keys on the exported object.
- *       Could do `{ plugin: { name, setup }, knownExternals }`
- *       and have app call `plugins: [require('dd-trace/esbuild').plugin]` instead...
- */
-// module.exports.knownExternals = [
-//   'pg-native', // peer dep
-//   'graphql/language/visitor',
-//   'graphql/language/printer',
-//   'graphql/utilities',
-// ];
+}