Skip to content

Commit e883765

Browse files
BridgeARrochdev
authored andcommitted
fix(rewriter): emit valid ESM imports for instrumented modules (#8526)
The local `traceAsyncIterator` transform read `state.sourceType`, but the vendored matcher only exposes `state.moduleType`. The resulting `undefined` fell into the CJS branch on every ESM file and baked `require()` into pure ESM modules such as `@langchain/langgraph/dist/pregel/index.js`. When the vendored ESM branch did fire, the injected specifier was the absolute filesystem path returned by `require.resolve('dc-polyfill')`, which Node's ESM resolver rejects with `ERR_INVALID_MODULE_SPECIFIER`. Three coordinated pieces close it: 1. The local transform reads `moduleType` and emits the default-import-plus-destructure shape used by the vendored transform. Named imports from `dc-polyfill` are not recoverable via Node's CJS to ESM interop because the module does `module.exports = dc` at runtime. 2. The rewriter pre-computes a path form (for `require()`) and a `file://` URL (for `import`) and dispatches to one of two matchers by `moduleType`. 3. `compiler.parse` forwards `isModule` to oxc as `sourceType` so the option is honoured by both parser backends instead of silently lost. Fixes: #7991
1 parent 57efa50 commit e883765

6 files changed

Lines changed: 97 additions & 22 deletions

File tree

packages/datadog-instrumentations/src/helpers/rewriter/compiler.js

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,10 @@ const compiler = {
1919
// TODO: Figure out ESBuild `createRequire` issue and remove this hack.
2020
const oxc = runtimeRequire(['oxc', 'parser'].join('-'))
2121

22-
compiler.parse = (sourceText, options) => {
22+
compiler.parse = (sourceText, { range, isModule } = {}) => {
2323
const { program, errors } = oxc.parseSync('index.js', sourceText, {
24-
...options,
24+
range,
25+
sourceType: isModule ? 'module' : 'script',
2526
preserveParens: false,
2627
})
2728

packages/datadog-instrumentations/src/helpers/rewriter/index.js

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,25 @@
22

33
const { readFileSync } = require('fs')
44
const { join } = require('path')
5+
const { pathToFileURL } = require('url')
56
const log = require('../../../../dd-trace/src/log')
67
const { create } = require('../../../../../vendor/dist/@apm-js-collab/code-transformer')
78
const { traceAsyncIterator, traceIterator } = require('./transforms')
89
const instrumentations = require('./instrumentations')
910

10-
let dcPolyfill
11+
// `dc-polyfill` is referenced from injected `require()` (CJS) and `import`
12+
// (ESM) statements that the transformer splices into the rewritten module.
13+
// `require()` accepts an absolute filesystem path; the ESM resolver rejects it
14+
// with `ERR_INVALID_MODULE_SPECIFIER` and needs a `file://` URL instead. We
15+
// pre-compute both forms here so each matcher hands the transformer a
16+
// specifier that is valid for the module type it is rewriting.
17+
let dcPolyfillCjs
18+
let dcPolyfillEsm
1119

1220
try {
13-
dcPolyfill = require.resolve('dc-polyfill').replaceAll('\\', '/')
21+
const resolved = require.resolve('dc-polyfill')
22+
dcPolyfillCjs = resolved.replaceAll('\\', '/')
23+
dcPolyfillEsm = pathToFileURL(resolved).href
1424
} catch {
1525
// The `dc-polyfill` module is unavailable for some reason (like bundling).
1626
// Let's just keep the default of using `diagnostics-channel` as a fallback
@@ -20,10 +30,13 @@ try {
2030
/** @type {Record<string, string>} map of module base name to version */
2131
const moduleVersions = {}
2232
const disabled = new Set()
23-
const matcher = create(instrumentations, dcPolyfill)
33+
const matcherCjs = create(instrumentations, dcPolyfillCjs)
34+
const matcherEsm = create(instrumentations, dcPolyfillEsm)
2435

25-
matcher.addTransform('traceIterator', traceIterator)
26-
matcher.addTransform('traceAsyncIterator', traceAsyncIterator)
36+
for (const matcher of [matcherCjs, matcherEsm]) {
37+
matcher.addTransform('traceIterator', traceIterator)
38+
matcher.addTransform('traceAsyncIterator', traceAsyncIterator)
39+
}
2740

2841
function rewrite (content, filename, format) {
2942
if (!content) return content
@@ -41,6 +54,7 @@ function rewrite (content, filename, format) {
4154

4255
if (disabled.has(moduleName)) return content
4356

57+
const matcher = moduleType === 'esm' ? matcherEsm : matcherCjs
4458
const transformer = matcher.getTransformer(moduleName, version, filePath)
4559

4660
if (!transformer) return content

packages/datadog-instrumentations/src/helpers/rewriter/transforms.js

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -10,17 +10,25 @@ const tracingChannelPredicate = (node) => (
1010
)
1111

1212
const transforms = module.exports = {
13-
tracingChannelImport ({ dcModule, sourceType }, node) {
13+
/**
14+
* @param {{ dcModule: string, moduleType: 'esm' | 'cjs' }} state
15+
* @param {import('estree').Program} node
16+
*/
17+
tracingChannelImport ({ dcModule, moduleType }, node) {
1418
if (node.body.some(tracingChannelPredicate)) return
1519

20+
// The vendored matcher state exposes `moduleType` (`esm` / `cjs`), so we
21+
// read that field directly. Naming it `sourceType` here used to silently
22+
// pick the CJS branch for every ESM file, leaving `require()` baked into
23+
// pure ESM modules like `@langchain/langgraph/dist/pregel/index.js`.
24+
const isModule = moduleType === 'esm'
25+
1626
const index = node.body.findIndex(child => child.directive === 'use strict')
17-
const code = isModuleSourceType(sourceType)
18-
? `import { tracingChannel as tr_ch_apm_tracingChannel } from "${dcModule}"`
27+
const code = isModule
28+
? `import tr_ch_apm_dc from "${dcModule}"; const {tracingChannel: tr_ch_apm_tracingChannel} = tr_ch_apm_dc`
1929
: `const {tracingChannel: tr_ch_apm_tracingChannel} = require("${dcModule}")`
2030

21-
node.body.splice(index + 1, 0, parse(code, {
22-
isModule: isModuleSourceType(sourceType),
23-
}).body[0])
31+
node.body.splice(index + 1, 0, ...parse(code, { isModule }).body)
2432
},
2533

2634
tracingChannelDeclaration (state, node) {
@@ -53,13 +61,6 @@ function traceAny (state, node, _parent, ancestry) {
5361
}
5462
}
5563

56-
/**
57-
* @param {string} sourceType
58-
*/
59-
function isModuleSourceType (sourceType) {
60-
return sourceType === 'module' || sourceType === 'esm'
61-
}
62-
6364
function traceFunction (state, node, program) {
6465
transforms.tracingChannelDeclaration(state, program)
6566

packages/datadog-instrumentations/test/helpers/rewriter/index.spec.js

Lines changed: 51 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
11
'use strict'
22

3-
const { readFileSync } = require('node:fs')
3+
const { mkdtempSync, readFileSync, writeFileSync } = require('node:fs')
4+
const { tmpdir } = require('node:os')
45
const { resolve, join, dirname } = require('node:path')
56
const Module = require('node:module')
67
const assert = require('node:assert')
8+
const { pathToFileURL } = require('node:url')
79
const { beforeEach, describe, it } = require('mocha')
810
const proxyquire = require('proxyquire')
911
const sinon = require('sinon')
@@ -261,6 +263,19 @@ describe('check-require-cache', () => {
261263
},
262264
channelName: 'trace_class_private_method',
263265
},
266+
{
267+
module: {
268+
name: 'test-esm',
269+
versionRange: '>=0.1',
270+
filePath: 'pregel-class.js',
271+
},
272+
functionQuery: {
273+
methodName: 'stream',
274+
className: 'Pregel',
275+
},
276+
channelName: 'pregel_stream',
277+
transform: 'traceAsyncIterator',
278+
},
264279
],
265280
})
266281
})
@@ -522,8 +537,42 @@ describe('check-require-cache', () => {
522537
content = readFileSync(filename, 'utf8')
523538
content = rewriter.rewrite(content, filename, 'module')
524539

525-
assert.match(content, /\bimport\s+.+\s+from\s+"/)
540+
assert.match(content, /\bimport\s+.+\s+from\s+"file:\/\//)
526541
assert.match(content, /tr_ch_apm_tracingChannel/)
527542
assert.doesNotMatch(content, /require\("/)
528543
})
544+
545+
// Covers the local `traceAsyncIterator` transform shape used by the langgraph
546+
// integration. Goes through `addTransform`, which the iterator-transform path
547+
// unique to dd-trace uses, not the vendored orchestrion transform that the
548+
// `kind: 'AsyncIterator'` test above happens to hit.
549+
it('should rewrite ESM modules without injecting require() for the traceAsyncIterator transform', async () => {
550+
const filename = resolve(__dirname, 'node_modules', 'test-esm', 'pregel-class.js')
551+
const source = readFileSync(filename, 'utf8')
552+
553+
const rewritten = rewriter.rewrite(source, filename, 'module')
554+
555+
assert.match(rewritten, /^import\s/m, 'expected an ESM import in the rewritten output')
556+
assert.doesNotMatch(rewritten, /\brequire\s*\(/, 'CJS require() must not appear in ESM output')
557+
assert.match(rewritten, /from\s+"file:\/\/[^"]+"/, 'dc-polyfill specifier must be a file:// URL for ESM')
558+
559+
// End-to-end: write the rewritten module to disk and dynamic-import it.
560+
// This is what fails at runtime today when the local transform emits
561+
// `require()` (no `require` in ESM scope) or a bare absolute path (Node
562+
// rejects with ERR_INVALID_MODULE_SPECIFIER).
563+
const dir = mkdtempSync(join(tmpdir(), 'dd-rewriter-esm-'))
564+
writeFileSync(join(dir, 'package.json'), '{"type":"module"}')
565+
const outFile = join(dir, 'pregel-class.mjs')
566+
writeFileSync(outFile, rewritten)
567+
568+
ch = tracingChannel('orchestrion:test-esm:pregel_stream')
569+
subs = { start: sinon.spy() }
570+
ch.subscribe(subs)
571+
572+
const mod = await import(pathToFileURL(outFile).href)
573+
const iter = new mod.Pregel().stream()
574+
await iter.next()
575+
576+
assert.ok(subs.start.calledOnce, 'instrumented start channel should fire once')
577+
})
529578
})

packages/datadog-instrumentations/test/helpers/rewriter/node_modules/test-esm/package.json

Lines changed: 5 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/datadog-instrumentations/test/helpers/rewriter/node_modules/test-esm/pregel-class.js

Lines changed: 5 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)