cleanup

Author: tlhunter

integration-tests/esbuild/app.js

@@ -1,6 +1,6 @@
 #!/usr/bin/env node
 
-require('dd-trace').init();
+require('dd-trace').init()
 const assert = require('assert')
 const express = require('express')
 const redis = require('redis')
@@ -13,17 +13,17 @@ assert.equal(redis.Graph.name, 'Graph')
 assert.equal(pg.types.builtins.BOOL, 16)
 assert.equal(express.static.mime.types.ogg, 'audio/ogg')
 
-console.log('REDIS INJECTED?', redis.__DATADOG_VERSION);
-console.log('PG INJECTED?', pg.__DATADOG_VERSION);
-console.log('EXPRESS INJECTED?', express.__DATADOG_VERSION);
+// console.log('REDIS INJECTED?', redis.__DATADOG_VERSION)
+// console.log('PG INJECTED?', pg.__DATADOG_VERSION)
+// console.log('EXPRESS INJECTED?', express.__DATADOG_VERSION)
 
 const conn = {
   user: 'postgres',
   host: 'localhost',
   database: 'postgres',
   password: 'hunter2',
   port: 5433,
-};
+}
 
 console.log('pg connect')
 const client = new pg.Client(conn)
@@ -42,5 +42,5 @@ app.get('/', async (req, res) => {
 })
 
 app.listen(PORT, () => {
-    console.log(`Example app listening on port ${PORT}`)
+  console.log(`Example app listening on port ${PORT}`)
 })

integration-tests/esbuild/app2.js

@@ -1,10 +1,11 @@
 {
-  "name": "esbuild-demo",
+  "name": "esbuild-dd-trace-demo",
+  "private": true,
   "version": "1.0.0",
   "description": "basic example app bundling dd-trace via esbuild",
   "main": "app.js",
   "scripts": {
-    "build": "node ./build.js",
+    "build": "DD_TRACE_DEBUG=true node ./build.js",
     "built": "DD_TRACE_DEBUG=true node ./out.js",
     "raw": "DD_TRACE_DEBUG=true node ./app.js",
     "link": "pushd ../.. && yarn link && popd && yarn link dd-trace",

integration-tests/esbuild/package.json

@@ -4,48 +4,33 @@ const instrumented = Object.keys(require('../datadog-instrumentations/src/helper
 const builtins = new Set(require('module').builtinModules)
 const packages = new Set()
 
-// We don't want to handle any built-in packages via DCITM.
-// Those packages will still be handled via RITM.
-// Plust, attempting to instrument them will fail as they have no package.json file.
+const DEBUG = !!process.env.DD_TRACE_DEBUG
+
+// We don't want to handle any built-in packages via DCITM
+// Those packages will still be handled via RITM
+// Attempting to instrument them would fail as they have no package.json file
 for (let pkg of instrumented) {
   if (builtins.has(pkg)) continue
   if (pkg.startsWith('node:')) continue
   packages.add(pkg)
 }
 
-// console.log(packages)
-
 const DC_CHANNEL = 'dd-trace:bundledModuleLoadStart'
 
 module.exports.name = 'datadog-esbuild'
 
 module.exports.setup = function(build) {
   build.onResolve({ filter: /.*/ }, args => {
     const package_name = args.path
-    // first call:
-    /* args = {
-      path: 'pg',
-      importer: '/Users/thomas.hunter/Projects/esbuild-demo/app.js',
-      namespace: 'file',
-      resolveDir: '/Users/thomas.hunter/Projects/esbuild-demo',
-      kind: 'require-call',
-      pluginData: undefined
-    } */
-    // second call:
-    /* args = {
-      path: 'pg',
-      importer: 'pg',
-      namespace: 'datadog',
-      resolveDir: '',
-      kind: 'require-call',
-      pluginData: undefined
-    } */
+
     if (args.namespace === 'file' && packages.has(package_name)) {
+      // The file namespace is used when requiring files from disk in userland
       const pathToPackageJson = require.resolve(`${package_name}/package.json`, { paths: [ args.resolveDir ] })
       const pkg = require(pathToPackageJson)
 
-      console.log('ONRESOLVE', package_name, pkg.version, pathToPackageJson)
-      // console.log(args)
+      if (DEBUG) {
+        console.log(`resolve ${package_name}@${pkg.version}`)
+      }
 
       // https://esbuild.github.io/plugins/#on-resolve-arguments
       return {
@@ -56,9 +41,9 @@ module.exports.setup = function(build) {
         }
       }
     } else if (args.namespace === 'datadog') {
-      console.log('ONRESOLVE DD', package_name)
+      // The datadog namespace is used when requiring files that are injected during the onLoad stage
+      // see note in onLoad
 
-      // @see note in onLoad
       if (package_name.startsWith('node:')) return
 
       return {
@@ -69,29 +54,25 @@ module.exports.setup = function(build) {
   })
 
   build.onLoad({ filter: /.*/, namespace: NAMESPACE }, args => {
-    /* args = {
-    path: 'pg',
-    namespace: 'datadog',
-    suffix: '',
-    pluginData: { version: '8.8.0' }
-    } */
-    console.log('ONLOAD', args.path, args.pluginData.version)
+    if (DEBUG) {
+      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.
     // Note that JSON.stringify adds double quotes for us. For perf gain we can simply add in quotes when we know it's safe.
     let contents = `
-    const dc = require('node:diagnostics_channel');
-    const channel = dc.channel(${JSON.stringify(DC_CHANNEL + ':' + args.path)});
-    const mod = require(${JSON.stringify(args.path)});
-    const payload = {
+      const dc = require('node:diagnostics_channel');
+      const channel = dc.channel(${JSON.stringify(DC_CHANNEL + ':' + args.path)});
+      const mod = require(${JSON.stringify(args.path)});
+      const payload = {
         module: mod,
         path: ${JSON.stringify(args.path)},
         version: ${JSON.stringify(args.pluginData.version)}
-    };
-    if (!channel.hasSubscribers) console.error('NO SUB! ${JSON.stringify(DC_CHANNEL + ':' + args.path)}');
-    channel.publish(payload); // subscriber may mutate payload
-    module.exports = payload.module;
-    module.exports.__DATADOG_VERSION = ${JSON.stringify(args.pluginData.version)}; // TODO: Unneccesary but cool
+      };
+      // if (!channel.hasSubscribers) console.error('missing subscriber! ${JSON.stringify(DC_CHANNEL + ':' + args.path)}');
+      channel.publish(payload); // subscriber may mutate payload
+      module.exports = payload.module;
+      // module.exports.__DATADOG_VERSION = ${JSON.stringify(args.pluginData.version)};
     `
     // https://esbuild.github.io/plugins/#on-load-results
     return {
@@ -102,7 +83,7 @@ module.exports.setup = function(build) {
 }
 
 /**
- * This is just a convenience to expose a list of known externals to the application.
+ * 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 }`

packages/datadog-esbuild/index.js

@@ -13,7 +13,6 @@ const Dcitm = require('../../../dd-trace/src/dcitm')
  * @param {Function} onrequire callback to be executed upon encountering module
  */
 function Hook (modules, onrequire) {
-  console.log('hook.js Hook()', modules)
   if (!(this instanceof Hook)) return new Hook(modules, onrequire)
 
   this._patched = Object.create(null)
@@ -22,7 +21,6 @@ function Hook (modules, onrequire) {
     const parts = [moduleBaseDir, moduleName].filter(v => v)
     const filename = path.join(...parts)
 
-    console.log('IS PATCHED', filename, !!this._patched[filename])
     if (this._patched[filename]) return moduleExports
 
     this._patched[filename] = true

packages/datadog-instrumentations/src/helpers/hook.js

@@ -21,7 +21,6 @@ exports.channel = function (name) {
  * @param Function hook
  */
 exports.addHook = function addHook ({ name, versions, file }, hook) {
-  console.log('ADD HOOK', name, versions, file, hook)
   if (!instrumentations[name]) {
     instrumentations[name] = []
   }

packages/datadog-instrumentations/src/helpers/instrument.js

@@ -16,22 +16,18 @@ const loadChannel = channel('dd-trace:instrumentation:load')
 
 // TODO: make this more efficient
 
-for (const packageName of names) { // for every module that we instrument
-  Hook([packageName], (moduleExports, moduleName, moduleBaseDir, moduleVersion) => { // call *ITM
-    console.log('INSIDE HOOK CALLBACK', moduleVersion)
-    // the hook callback is encountered upon loading a given module
+for (const packageName of names) {
+  Hook([packageName], (moduleExports, moduleName, moduleBaseDir, moduleVersion) => {
     moduleName = moduleName.replace(pathSepExpr, '/')
 
-    hooks[packageName]() // builds out instrumentations database
+    hooks[packageName]()
 
     for (const { name, file, versions, hook } of instrumentations[packageName]) {
       const fullFilename = filename(name, file)
 
       if (moduleName === fullFilename) {
         const version = moduleVersion || getVersion(moduleBaseDir)
 
-        // TODO: how to get register.js to play nicely with dcitm.js?
-        console.log('REGISTER.JS HOOK', moduleName, version);
         if (matchVersion(version, versions)) {
           try {
             loadChannel.publish({ name, version, file })

packages/datadog-instrumentations/src/helpers/register.js

@@ -26,9 +26,7 @@ module.exports = class DiagnosticsChannelInTheMiddle {
     this.modules = modules
     this.options = options
     this.onrequire = onrequire
-    console.log('PREP', modules)
     for (let module of modules) {
-      console.log('SUBSCRIBE', `${CHANNEL_PREFIX}:${module}`)
       dc.subscribe(`${CHANNEL_PREFIX}:${module}`, this._onModuleLoad.bind(this)) // TODO: shouldn't need a .bind
     }
   }
@@ -42,11 +40,9 @@ module.exports = class DiagnosticsChannelInTheMiddle {
   /**
    * @param {string} arg.module name of the module
    * @param {string} arg.path path to the module
-   * @param {string} [arg.version] version of the module
+   * @param {string} arg.version version of the module
    */
   _onModuleLoad(payload) {
-    console.log(`onModuleLoad(${payload.path}@${payload.version})`)
-    // TODO: this returns the same module that was passed in
-    payload.module = this.onrequire(payload.module, payload.path, undefined, payload.version) // TODO: moduleBaseDir
+    payload.module = this.onrequire(payload.module, payload.path, undefined, payload.version)
   }
 }