fix(instrumentation): preserve ESM export aliases (#9436)

Named ESM imports could retain the original function because IITM only replaced the default binding after instrumentation. Preserve aliases that reference the original default export and synchronize patched builtin exports so every supported import form observes instrumentation.

Author: BridgeAR

.github/CODEOWNERS

@@ -353,6 +353,7 @@
 /integration-tests/coverage-fixtures/ @DataDog/lang-platform-js
 /integration-tests/crashtracking/ @DataDog/lang-platform-js
 /integration-tests/helpers/ @DataDog/lang-platform-js
+/integration-tests/import-variants.spec.js @DataDog/lang-platform-js
 /integration-tests/init/ @DataDog/lang-platform-js
 /integration-tests/init.spec.js @DataDog/lang-platform-js
 /integration-tests/memory-leak/ @DataDog/lang-platform-js

integration-tests/helpers/index.js

@@ -670,80 +670,119 @@ async function createSandbox (
 }
 
 /**
- * @typedef {{ default?: string, star: string, destructure: string }} Variants
+ * @typedef {'destructure' | 'direct' | 'namespace'} NamedExportBinding
+ * @typedef {object} ImportVariantOptions
+ * @property {string} bindingName
+ * @property {string} packageName
+ * @property {boolean} defaultExport
+ * @property {string[]} namedExports
+ * @property {NamedExportBinding} [namedExportBinding]
+ * @typedef {Record<string, string>} ImportVariants
+ * @typedef {Record<string, string>} ImportVariantFiles
  */
+
 /**
- * @overload
- * @param {string} filename - The file that will be copied and modified for each variant.
- * @param {string} bindingName - The binding name that will be use to bind to the packageName.
- * @param {string} [namedExport] - The name of the named variant to use.
- * @param {string} [packageName] - The name of the package. If not provided, the binding name will be used.
- * @param {boolean} [byPassDefault] - Skip default export variant generation.
- * @returns {Variants} A map from variant names to resulting filenames
+ * @param {ImportVariantOptions} options
+ * @returns {ImportVariants}
  */
+function createImportVariants (options) {
+  const {
+    bindingName,
+    packageName,
+    defaultExport,
+    namedExports,
+    namedExportBinding,
+  } = options
+  assert(defaultExport || namedExports.length, 'At least one default or named export is required')
+  assert(!namedExports.length || namedExportBinding, 'Named exports require a binding style')
+  assert(
+    !namedExportBinding ||
+    namedExportBinding === 'destructure' ||
+    namedExportBinding === 'direct' ||
+    namedExportBinding === 'namespace',
+    `Unknown named export binding style: ${namedExportBinding}`
+  )
+  assert(
+    namedExportBinding !== 'direct' || namedExports.length === 1,
+    'Direct named export bindings require exactly one export'
+  )
+
+  const variants = {}
+  const namespaceName = `mod${bindingName[0].toUpperCase()}${bindingName.slice(1)}`
+
+  if (defaultExport) {
+    variants.default = `import ${bindingName} from '${packageName}'`
+    variants['default-as-named'] = `import { default as ${bindingName} } from '${packageName}'`
+    variants['default-from-namespace'] =
+      `import * as ${namespaceName} from '${packageName}'; const ${bindingName} = ${namespaceName}.default`
+  }
+
+  if (namedExports.length) {
+    if (namedExportBinding === 'direct') {
+      const [namedExport] = namedExports
+      const importBinding = namedExport === bindingName ? namedExport : `${namedExport} as ${bindingName}`
+      variants.named = `import { ${importBinding} } from '${packageName}'`
+      variants['named-from-namespace'] =
+        `import * as ${namespaceName} from '${packageName}'; const ${bindingName} = ${namespaceName}.${namedExport}`
+    } else if (namedExportBinding === 'namespace') {
+      const exportsList = namedExports.join(', ')
+      variants.named = `import { ${exportsList} } from '${packageName}'; const ${bindingName} = { ${exportsList} }`
+      variants['named-from-namespace'] = `import * as ${bindingName} from '${packageName}'`
+    } else {
+      const exportsList = namedExports.join(', ')
+      variants.named = `import { ${exportsList} } from '${packageName}'`
+      variants['named-from-namespace'] =
+        `import * as ${bindingName} from '${packageName}'; const { ${exportsList} } = ${bindingName}`
+    }
+  }
+
+  return variants
+}
+
 /**
- * Creates a bunch of files based on an original file in sandbox. Useful for varying test files
- * without having to create a bunch of them yourself.
- *
- * The variants object should have keys that are named variants, and values that are the text
- * in the file that's different in each variant. There must always be a "default" variant,
- * whose value is the original text within the file that will be replaced.
- *
  * @param {string} filename - The file that will be copied and modified for each variant.
- * @param {Variants|string} variants - The variants or binding name.
- * @param {string} [namedExport] - Named export to use for star/destructure variants.
- * @param {string} [packageName] - Module specifier for the import.
- * @param {boolean} [byPassDefault] - Skip default export variant generation.
- * @returns {Variants} A map from variant names to resulting filenames
+ * @param {ImportVariants} variants - Import statements keyed by variant name.
+ * @param {ImportVariantFiles} variantFilenames - Resulting filenames keyed by variant name.
  */
-function varySandbox (filename, variants, namedExport, packageName, byPassDefault) {
-  if (typeof variants === 'string') {
-    const bindingName = variants
-    const resolvedName = packageName || bindingName
-    // Default namedVariant to bindingName when bypassing default export
-    if (byPassDefault && !namedExport) namedExport = bindingName
-    variants = byPassDefault
-      ? {
-          // eslint-disable-next-line @stylistic/max-len
-          star: `import * as mod${bindingName} from '${resolvedName}'; const ${bindingName} = mod${bindingName}.${namedExport}`,
-          destructure: `import { ${namedExport} } from '${resolvedName}'`,
-        }
-      : {
-          default: `import ${bindingName} from '${resolvedName}'`,
-          star: namedExport
-            ? `import * as ${bindingName} from '${resolvedName}'`
-            : `import * as mod${bindingName} from '${resolvedName}'; const ${bindingName} = mod${bindingName}.default`,
-          destructure: namedExport
-            ? `import { ${namedExport} } from '${resolvedName}'; const ${bindingName} = { ${namedExport} }`
-            : `import { default as ${bindingName}} from '${resolvedName}'`,
-        }
-  }
-
+function writeSandboxVariants (filename, variants, variantFilenames) {
   const origFileData = readFileSync(path.join(sandbox.folder, filename), 'utf8')
-  const { name: prefix, ext: suffix } = path.parse(filename)
-  const variantFilenames = /** @type {Variants} */ ({})
-  const baseVariant = byPassDefault ? 'destructure' : 'default'
+  const baseVariant = variants.default ? 'default' : 'named'
 
   for (const [variant, value] of Object.entries(variants)) {
-    const variantFilename = `${prefix}-${variant}${suffix}`
-    variantFilenames[variant] = variantFilename
+    const variantFilename = variantFilenames[variant]
     let newFileData = origFileData
     if (variant !== baseVariant) {
       const baseValue = variants[baseVariant]
       assert(baseValue, `Missing ${baseVariant} variant`)
       newFileData = origFileData.replace(baseValue, `${value}`)
-      // Error out when the default import does not match that of server.mjs
       if (newFileData === origFileData) throw Error(`Unable to match ${baseVariant}`)
     }
     writeFileSync(path.join(sandbox.folder, variantFilename), newFileData)
   }
-  return variantFilenames
 }
 
 /**
- * @type {['default', 'star', 'destructure']}
+ * Call after useSandbox so variant materialization runs after the sandbox setup.
+ *
+ * @param {string} filename - The file that will be copied and modified for each import variant.
+ * @param {ImportVariantOptions} options
+ * @returns {ImportVariantFiles} Resulting filenames keyed by variant name.
  */
-varySandbox.VARIANTS = ['default', 'star', 'destructure']
+function varySandbox (filename, options) {
+  const variants = createImportVariants(options)
+  const { name: prefix, ext: suffix } = path.parse(filename)
+  const variantFilenames = {}
+
+  for (const variant of Object.keys(variants)) {
+    variantFilenames[variant] = `${prefix}-${variant}${suffix}`
+  }
+
+  before(function () {
+    writeSandboxVariants(filename, variants, variantFilenames)
+  })
+
+  return variantFilenames
+}
 
 /**
  * @param {boolean} shouldExpectTelemetryPoints

integration-tests/import-variants.spec.js

@@ -0,0 +1,134 @@
+'use strict'
+
+const assert = require('node:assert/strict')
+
+const sinon = require('sinon')
+
+const { varySandbox } = require('./helpers')
+
+describe('varySandbox', () => {
+  beforeEach(() => {
+    sinon.stub(global, 'before')
+  })
+
+  afterEach(() => {
+    sinon.restore()
+  })
+
+  it('creates every default export form', () => {
+    assert.deepStrictEqual(varySandbox('logger.mjs', {
+      bindingName: 'logger',
+      packageName: 'logger',
+      defaultExport: true,
+      namedExports: [],
+    }), {
+      default: 'logger-default.mjs',
+      'default-as-named': 'logger-default-as-named.mjs',
+      'default-from-namespace': 'logger-default-from-namespace.mjs',
+    })
+  })
+
+  it('creates every direct named export form', () => {
+    assert.deepStrictEqual(varySandbox('logger.mjs', {
+      bindingName: 'logger',
+      packageName: 'logger',
+      defaultExport: true,
+      namedExports: ['createLogger'],
+      namedExportBinding: 'direct',
+    }), {
+      default: 'logger-default.mjs',
+      'default-as-named': 'logger-default-as-named.mjs',
+      'default-from-namespace': 'logger-default-from-namespace.mjs',
+      named: 'logger-named.mjs',
+      'named-from-namespace': 'logger-named-from-namespace.mjs',
+    })
+  })
+
+  it('creates every namespace-shaped named export form', () => {
+    assert.deepStrictEqual(varySandbox('logger.mjs', {
+      bindingName: 'logger',
+      packageName: 'logger',
+      defaultExport: true,
+      namedExports: ['createLogger', 'levels'],
+      namedExportBinding: 'namespace',
+    }), {
+      default: 'logger-default.mjs',
+      'default-as-named': 'logger-default-as-named.mjs',
+      'default-from-namespace': 'logger-default-from-namespace.mjs',
+      named: 'logger-named.mjs',
+      'named-from-namespace': 'logger-named-from-namespace.mjs',
+    })
+  })
+
+  it('creates named-only forms', () => {
+    assert.deepStrictEqual(varySandbox('logger.mjs', {
+      bindingName: 'Logger',
+      packageName: 'logger',
+      defaultExport: false,
+      namedExports: ['Logger'],
+      namedExportBinding: 'direct',
+    }), {
+      named: 'logger-named.mjs',
+      'named-from-namespace': 'logger-named-from-namespace.mjs',
+    })
+  })
+
+  it('creates separately bound named export forms', () => {
+    assert.deepStrictEqual(varySandbox('logger.mjs', {
+      bindingName: 'loggerModule',
+      packageName: 'logger',
+      defaultExport: false,
+      namedExports: ['createLogger', 'levels'],
+      namedExportBinding: 'destructure',
+    }), {
+      named: 'logger-named.mjs',
+      'named-from-namespace': 'logger-named-from-namespace.mjs',
+    })
+  })
+
+  it('rejects an impossible export configuration', () => {
+    assert.throws(() => varySandbox('logger.mjs', {
+      bindingName: 'logger',
+      packageName: 'logger',
+      defaultExport: false,
+      namedExports: [],
+    }), {
+      message: 'At least one default or named export is required',
+    })
+  })
+
+  it('requires a named export binding style', () => {
+    assert.throws(() => varySandbox('logger.mjs', {
+      bindingName: 'logger',
+      packageName: 'logger',
+      defaultExport: false,
+      namedExports: ['createLogger'],
+    }), {
+      message: 'Named exports require a binding style',
+    })
+  })
+
+  it('rejects an unknown named export binding style', () => {
+    assert.throws(() => varySandbox('logger.mjs', {
+      bindingName: 'logger',
+      packageName: 'logger',
+      defaultExport: false,
+      namedExports: ['createLogger'],
+      namedExportBinding: 'unknown',
+    }), {
+      message: 'Unknown named export binding style: unknown',
+    })
+  })
+
+  it('rejects multiple direct named exports', () => {
+    assert.throws(() => varySandbox('logger.mjs', {
+      bindingName: 'logger',
+      packageName: 'logger',
+      defaultExport: false,
+      namedExports: ['Logger', 'levels'],
+      namedExportBinding: 'direct',
+    }), {
+      message: 'Direct named export bindings require exactly one export',
+    })
+  })
+})

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

@@ -49,10 +49,19 @@ function Hook (modules, hookOptions, onrequire) {
   this._patched = Object.create(null)
   const patched = new WeakMap()
 
+  /**
+   * @param {object|Function|undefined} moduleExports
+   * @param {string} moduleName
+   * @param {string|undefined} moduleBaseDir
+   * @param {string|undefined} moduleVersion
+   * @param {boolean|undefined} isIitm
+   */
   const safeHook = (moduleExports, moduleName, moduleBaseDir, moduleVersion, isIitm) => {
     const parts = [moduleBaseDir, moduleName].filter(Boolean)
     const filename = path.join(...parts)
 
+    const defaultExport = isIitm && moduleExports.default
+    let defaultExportAliases
     let defaultWrapResult
 
     const wrappedOnrequire = (moduleExports, ...args) => {
@@ -77,17 +86,29 @@ function Hook (modules, hookOptions, onrequire) {
     }
 
     if (
-      isIitm &&
-      moduleExports.default &&
-      (typeof moduleExports.default === 'object' ||
-      typeof moduleExports.default === 'function')
+      defaultExport &&
+      (typeof defaultExport === 'object' ||
+      typeof defaultExport === 'function')
     ) {
-      defaultWrapResult = wrappedOnrequire(moduleExports.default, moduleName, moduleBaseDir, moduleVersion, isIitm)
+      defaultWrapResult = wrappedOnrequire(defaultExport, moduleName, moduleBaseDir, moduleVersion, isIitm)
+      if (defaultWrapResult && defaultWrapResult !== defaultExport) {
+        defaultExportAliases = []
+        for (const exportName of Object.keys(moduleExports)) {
+          if (exportName !== 'default' && moduleExports[exportName] === defaultExport) {
+            defaultExportAliases.push(exportName)
+          }
+        }
+      }
     }
 
     const newExports = wrappedOnrequire(moduleExports, moduleName, moduleBaseDir, moduleVersion, isIitm)
 
-    if (defaultWrapResult) newExports.default = defaultWrapResult
+    if (defaultWrapResult && defaultExportAliases) {
+      newExports.default = defaultWrapResult
+      for (const exportName of defaultExportAliases) {
+        moduleExports[exportName] = defaultWrapResult
+      }
+    }
 
     this._patched[filename] = true
 

packages/datadog-instrumentations/src/process.js

@@ -1,5 +1,7 @@
 'use strict'
 
+const { syncBuiltinESMExports } = require('node:module')
+
 const { channel } = require('dc-polyfill')
 const shimmer = require('../../datadog-shimmer')
 
@@ -26,4 +28,5 @@ if (process.setUncaughtExceptionCaptureCallback) {
         return result
       }
     })
+  syncBuiltinESMExports()
 }