Skip to content

Commit dd298d5

Browse files
committed
chore: update iitm and fix initialize (#7387)
The initialize hook was expecting that it could import a cjs module. That expectation was wrong due to only working due to iitm formerly instrumenting the top level module. Since that is not the case anymore it broke our instrumentation, since that would not be possible by Node.js itself. As drive by, this removes code from not supported versions as well as making some parts faster and cleaning up code.
1 parent fcf3c3b commit dd298d5

13 files changed

Lines changed: 131 additions & 95 deletions

File tree

.github/workflows/platform.yml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -443,7 +443,7 @@ jobs:
443443
strategy:
444444
fail-fast: false
445445
matrix:
446-
version: [14.0.0, 14, 16.0.0, 18.0.0, 20.0.0, 22.0.0, 24.0.0]
446+
version: [16.0.0, 18.0.0, 20.0.0, 22.0.0, 24.0.0, 25]
447447
runs-on: ubuntu-latest
448448
steps:
449449
- uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
@@ -464,7 +464,7 @@ jobs:
464464
strategy:
465465
fail-fast: false
466466
matrix:
467-
version: ['0.8', '0.10', '0.12', '4', '6', '8', '10', '12']
467+
version: ['0.8', '0.10', '0.12', '4', '6', '8', '10', '12', '14.0.0', '14']
468468
runs-on: ubuntu-latest
469469
env:
470470
DD_TRACE_DEBUG: 'true' # This exercises more of the guardrails code

eslint.config.mjs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -475,6 +475,7 @@ export default [
475475
'unicorn/explicit-length-check': 'off', // 68 errors
476476
'unicorn/filename-case': ['off', { case: 'kebabCase' }], // 59 errors
477477
'unicorn/prefer-at': 'off', // 17 errors | Difficult to fix
478+
'unicorn/prefer-export-from': ['error', { ignoreUsedVariables: true }],
478479
'unicorn/prevent-abbreviations': 'off', // too strict
479480

480481
// These rules require a newer Node.js version than we support

initialize.mjs

Lines changed: 46 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,7 @@
22
* This file serves one of two purposes, depending on how it's used.
33
*
44
* If used with --import, it will import init.js and register the loader hook.
5-
* If used with --loader, it will act as the loader hook, except that it will
6-
* also import init.js inside the source code of the entrypoint file.
5+
* If used with --loader, it will act as the loader hook.
76
*
87
* The result is that no matter how this file is used, so long as it's with
98
* one of the two flags, the tracer will always be initialized, and the loader
@@ -12,49 +11,68 @@
1211

1312
/* eslint n/no-unsupported-features/node-builtins: ['error', { ignores: ['module.register'] }] */
1413

14+
import { Buffer } from 'buffer'
15+
import * as Module from 'module'
16+
import { types } from 'util'
1517
import { isMainThread } from 'worker_threads'
1618

17-
import * as Module from 'node:module'
18-
import { fileURLToPath } from 'node:url'
19+
// This file must support Node.js 12.0.0 syntax
20+
1921
import {
20-
load as origLoad,
21-
resolve as origResolve,
22-
getSource as origGetSource,
23-
} from 'import-in-the-middle/hook.mjs'
22+
iitmExclusions,
23+
load as hookLoad,
24+
resolve as hookResolve,
25+
} from './loader-hook.mjs'
2426

2527
let hasInsertedInit = false
26-
function insertInit (result) {
27-
if (!hasInsertedInit) {
28-
hasInsertedInit = true
29-
result.source = `
30-
import '${fileURLToPath(new URL('init.js', import.meta.url))}';
31-
${result.source}`
28+
const initJsUrl = new URL('init.js', import.meta.url).href
29+
// `--loader` only reliably influences ESM entrypoints; for CJS apps use `--import`/`--require`.
30+
31+
/**
32+
* @param {{ source?: string|Buffer|Uint8Array, format?: string }} result
33+
* @param {unknown} _url_
34+
* @param {{ format?: string, isMain?: boolean }} context
35+
* @returns {{ source?: string|Buffer|Uint8Array, format?: string }}
36+
*/
37+
function insertInit (result, _url_, context) {
38+
if (hasInsertedInit) return result
39+
// If Node provides `isMain`, only inject into the entrypoint module.
40+
if (context && context.isMain === false) return result
41+
42+
let { source } = result
43+
if (typeof source !== 'string') {
44+
// Fast decode: handle bytes sources without extra copies when possible.
45+
if (Buffer.isBuffer(source)) {
46+
source = source.toString('utf8')
47+
} else if (types.isUint8Array(source)) {
48+
// Create a Buffer view over the same ArrayBuffer segment (no copy).
49+
source = Buffer.from(source.buffer, source.byteOffset, source.byteLength).toString('utf8')
50+
} else {
51+
return result
52+
}
3253
}
54+
55+
const format = result.format || (context && context.format)
56+
if (format !== 'module') return result
57+
58+
hasInsertedInit = true
59+
60+
result.source = `import ${JSON.stringify(initJsUrl)};\n${source}`
61+
3362
return result
3463
}
3564

3665
const [NODE_MAJOR, NODE_MINOR] = process.versions.node.split('.').map(Number)
3766

3867
const brokenLoaders = NODE_MAJOR === 18 && NODE_MINOR === 0
39-
const iitmExclusions = [
40-
/langsmith/,
41-
/openai\/_shims/,
42-
/openai\/resources\/chat\/completions\/messages/,
43-
/openai\/agents-core\/dist\/shims/,
44-
/@anthropic-ai\/sdk\/_shims/,
45-
]
4668

4769
export async function load (url, context, nextLoad) {
4870
const iitmExclusionsMatch = iitmExclusions.some((exclusion) => exclusion.test(url))
49-
const loadHook = (brokenLoaders || iitmExclusionsMatch) ? nextLoad : origLoad
50-
return insertInit(await loadHook(url, context, nextLoad))
71+
const loadHook = (brokenLoaders || iitmExclusionsMatch) ? nextLoad : hookLoad
72+
return insertInit(await loadHook(url, context, nextLoad), url, context)
5173
}
5274

53-
export const resolve = brokenLoaders ? undefined : origResolve
54-
55-
export async function getSource (...args) {
56-
return insertInit(await origGetSource(...args))
57-
}
75+
export const resolve = brokenLoaders ? undefined : hookResolve
5876

5977
if (isMainThread) {
6078
const require = Module.createRequire(import.meta.url)
@@ -65,5 +83,3 @@ if (isMainThread) {
6583
})
6684
}
6785
}
68-
69-
export { getFormat } from 'import-in-the-middle/hook.mjs'

integration-tests/helpers/index.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ async function runAndCheckOutput (filename, cwd, expectedOut, expectedSource) {
6060
// Debug adds this, which we don't care about in these tests
6161
out = out.replace('Flushing 0 metrics via HTTP\n', '')
6262
}
63-
assert.match(out, new RegExp(expectedOut), `output "${out} does not contain expected output "${expectedOut}"`)
63+
assert.match(out, new RegExp(expectedOut), `output "${out}" does not contain expected output "${expectedOut}"`)
6464
}
6565

6666
if (expectedSource) {

integration-tests/init.spec.js

Lines changed: 24 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,12 @@ delete process.env.DD_INJECT_FORCE
3131

3232
function testInjectionScenarios (arg, filename, esmWorks = false) {
3333
if (!currentVersionIsSupported) return
34-
const doTest = (file, ...args) => testFile(file, ...args)
34+
35+
// For `--loader`, we generally want ESM fixtures to ensure the loader hook actually applies.
36+
// However, Node 18.0.0 is a known outlier where ESM via custom loaders is not supportable.
37+
const isNode1800 = process.versions.node === '18.0.0'
38+
const tracerFile = arg === 'loader' && !isNode1800 ? 'init/trace.mjs' : 'init/trace.js'
39+
const instrFile = arg === 'loader' && !isNode1800 ? 'init/instrument.mjs' : 'init/instrument.js'
3540

3641
context('preferring app-dir dd-trace', () => {
3742
context('when dd-trace is not in the app dir', () => {
@@ -40,23 +45,23 @@ function testInjectionScenarios (arg, filename, esmWorks = false) {
4045

4146
if (currentVersionIsSupported) {
4247
context('without DD_INJECTION_ENABLED', () => {
43-
it('should initialize the tracer', () => doTest('init/trace.js', 'true\n', [], 'manual'))
48+
it('should initialize the tracer', () => testFile(tracerFile, 'true\n', [], 'manual'))
4449

45-
it('should initialize instrumentation', () => doTest('init/instrument.js', 'true\n', [], 'manual'))
50+
it('should initialize instrumentation', () => testFile(instrFile, 'true\n', [], 'manual'))
4651

4752
it(`should ${esmWorks ? '' : 'not '}initialize ESM instrumentation`, () =>
48-
doTest('init/instrument.mjs', `${esmWorks}\n`, [], 'manual'))
53+
testFile('init/instrument.mjs', `${esmWorks}\n`, [], 'manual'))
4954
})
5055
}
5156

5257
context('with DD_INJECTION_ENABLED', () => {
5358
useEnv({ DD_INJECTION_ENABLED })
5459

55-
it('should not initialize the tracer', () => doTest('init/trace.js', 'false\n', []))
60+
it('should not initialize the tracer', () => testFile(tracerFile, 'false\n', [], ''))
5661

57-
it('should not initialize instrumentation', () => doTest('init/instrument.js', 'false\n', []))
62+
it('should not initialize instrumentation', () => testFile(instrFile, 'false\n', [], ''))
5863

59-
it('should not initialize ESM instrumentation', () => doTest('init/instrument.mjs', 'false\n', []))
64+
it('should not initialize ESM instrumentation', () => testFile('init/instrument.mjs', 'false\n', [], ''))
6065
})
6166
})
6267

@@ -65,23 +70,23 @@ function testInjectionScenarios (arg, filename, esmWorks = false) {
6570
useEnv({ NODE_OPTIONS })
6671

6772
context('without DD_INJECTION_ENABLED', () => {
68-
it('should initialize the tracer', () => doTest('init/trace.js', 'true\n', [], 'manual'))
73+
it('should initialize the tracer', () => testFile(tracerFile, 'true\n', [], 'manual'))
6974

70-
it('should initialize instrumentation', () => doTest('init/instrument.js', 'true\n', [], 'manual'))
75+
it('should initialize instrumentation', () => testFile(instrFile, 'true\n', [], 'manual'))
7176

7277
it(`should ${esmWorks ? '' : 'not '}initialize ESM instrumentation`, () =>
73-
doTest('init/instrument.mjs', `${esmWorks}\n`, [], 'manual'))
78+
testFile('init/instrument.mjs', `${esmWorks}\n`, [], 'manual'))
7479
})
7580

7681
context('with DD_INJECTION_ENABLED', () => {
7782
useEnv({ DD_INJECTION_ENABLED, DD_TRACE_DEBUG })
7883

79-
it('should initialize the tracer', () => doTest('init/trace.js', 'true\n', telemetryGood, 'ssi'))
84+
it('should initialize the tracer', () => testFile(tracerFile, 'true\n', telemetryGood, 'ssi'))
8085

81-
it('should initialize instrumentation', () => doTest('init/instrument.js', 'true\n', telemetryGood, 'ssi'))
86+
it('should initialize instrumentation', () => testFile(instrFile, 'true\n', telemetryGood, 'ssi'))
8287

8388
it(`should ${esmWorks ? '' : 'not '}initialize ESM instrumentation`, () =>
84-
doTest('init/instrument.mjs', `${esmWorks}\n`, telemetryGood, 'ssi'))
89+
testFile('init/instrument.mjs', `${esmWorks}\n`, telemetryGood, 'ssi'))
8590
})
8691
})
8792
})
@@ -90,11 +95,13 @@ function testInjectionScenarios (arg, filename, esmWorks = false) {
9095
function testRuntimeVersionChecks (arg, filename) {
9196
context('runtime version check', () => {
9297
const NODE_OPTIONS = `--${arg} dd-trace/${filename}`
93-
const doTest = (...args) => testFile('init/trace.js', ...args)
94-
const doTestForced = async (...args) => {
98+
const entryFile = arg === 'loader' ? 'init/trace.mjs' : 'init/trace.js'
99+
const doTest = (expectedOut, expectedTelemetryPoints, expectedSource) =>
100+
testFile(entryFile, expectedOut, expectedTelemetryPoints, expectedSource)
101+
const doTestForced = async (expectedOut, expectedTelemetryPoints, expectedSource) => {
95102
Object.assign(process.env, { DD_INJECT_FORCE })
96103
try {
97-
await testFile('init/trace.js', ...args)
104+
await testFile(entryFile, expectedOut, expectedTelemetryPoints, expectedSource)
98105
} finally {
99106
delete process.env.DD_INJECT_FORCE
100107
}
@@ -279,7 +286,7 @@ if (semver.satisfies(process.versions.node, '>=14.13.1')) {
279286
if (semver.satisfies(process.versions.node, '>=20.6.0')) {
280287
context('as --import', () => {
281288
testInjectionScenarios('import', 'initialize.mjs', true)
282-
testRuntimeVersionChecks('loader', 'initialize.mjs')
289+
testRuntimeVersionChecks('import', 'initialize.mjs')
283290
})
284291
}
285292
})

integration-tests/init/trace.mjs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
// eslint-disable-next-line no-console
2+
console.log(!!global._ddtrace)
3+
// eslint-disable-next-line no-console
4+
console.log('instrumentation source:', global._ddtrace._tracer._config.instrumentationSource)
5+
process.exit()

integration-tests/package-guardrails.spec.js

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@ const NODE_OPTIONS = '--require dd-trace/init.js'
1414
const DD_TRACE_DEBUG = 'true'
1515
const DD_INJECTION_ENABLED = 'tracing'
1616
const DD_LOG_LEVEL = 'error'
17+
const NODE_MAJOR = Number(process.versions.node.split('.')[0])
18+
const FASTIFY_DEP = NODE_MAJOR < 20 ? 'fastify@4' : 'fastify'
1719

1820
// These are on by default in release tests, so we'll turn them off for
1921
// more fine-grained control of these variables in these tests.
@@ -22,8 +24,8 @@ delete process.env.DD_INJECT_FORCE
2224

2325
describe('package guardrails', () => {
2426
useEnv({ NODE_OPTIONS })
25-
const runTest = (...args) =>
26-
testFile('package-guardrails/index.js', ...args)
27+
const runTest = (expectedOut, expectedTelemetryPoints, expectedSource = '') =>
28+
testFile('package-guardrails/index.js', expectedOut, expectedTelemetryPoints, expectedSource)
2729

2830
context('when package is out of range', () => {
2931
useSandbox(['bluebird@1.0.0'])
@@ -70,13 +72,13 @@ false
7072

7173
context('when package is in range (fastify)', () => {
7274
context('when fastify is latest', () => {
73-
useSandbox(['fastify'])
75+
useSandbox([FASTIFY_DEP])
7476

7577
it('should instrument the package', () => runTest('true\n', [], 'manual'))
7678
})
7779

7880
context('when fastify is latest and logging enabled', () => {
79-
useSandbox(['fastify'])
81+
useSandbox([FASTIFY_DEP])
8082
useEnv({ DD_TRACE_DEBUG })
8183

8284
it('should instrument the package', () =>

integration-tests/startup.spec.js

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,10 @@ const execArgvs = [
2323
execArgv: ['--import', 'dd-trace/register.js'],
2424
skip: semver.satisfies(process.versions.node, '<20.6'),
2525
},
26+
{
27+
execArgv: ['--import', 'dd-trace/loader-hook.mjs'],
28+
skip: semver.satisfies(process.versions.node, '<20.6'),
29+
},
2630
{
2731
execArgv: ['--loader', 'dd-trace/loader-hook.mjs'],
2832
skip: semver.satisfies(process.versions.node, '>=20.6'),

loader-hook.mjs

Lines changed: 35 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,28 +1,30 @@
1-
import * as iitm from 'import-in-the-middle/hook.mjs'
1+
import { initialize as origInitialize, load as origLoad, resolve } from 'import-in-the-middle/hook.mjs'
22
import regexpEscapeModule from './vendor/dist/escape-string-regexp/index.js'
33
import hooks from './packages/datadog-instrumentations/src/helpers/hooks.js'
44
import configHelper from './packages/dd-trace/src/config/helper.js'
55
import * as rewriterLoader from './packages/datadog-instrumentations/src/helpers/rewriter/loader.mjs'
66
import { isRelativeRequire } from './packages/datadog-instrumentations/src/helpers/shared-utils.js'
77

8+
// This file must support Node.js 12.0.0 syntax
9+
810
const regexpEscape = regexpEscapeModule.default
911

1012
// For some reason `getEnvironmentVariable` is not otherwise available to ESM.
1113
const env = configHelper.getEnvironmentVariable
1214

1315
function initialize (data = {}) {
14-
data.include ??= []
15-
data.exclude ??= []
16+
if (data.include == null) data.include = []
17+
if (data.exclude == null) data.exclude = []
1618

1719
addInstrumentations(data)
1820
addSecurityControls(data)
1921
addExclusions(data)
2022

21-
return iitm.initialize(data)
23+
return origInitialize(data)
2224
}
2325

2426
function load (url, context, nextLoad) {
25-
return rewriterLoader.load(url, context, (url, context) => iitm.load(url, context, nextLoad))
27+
return rewriterLoader.load(url, context, (url, context) => origLoad(url, context, nextLoad))
2628
}
2729

2830
function addInstrumentations (data) {
@@ -38,27 +40,38 @@ function addInstrumentations (data) {
3840
}
3941

4042
function addSecurityControls (data) {
41-
const securityControls = (env('DD_IAST_SECURITY_CONTROLS_CONFIGURATION') || '')
42-
.split(';')
43-
.map(sc => sc.trim().split(':')[2])
44-
.filter(Boolean)
45-
.map(sc => sc.trim())
43+
const raw = env('DD_IAST_SECURITY_CONTROLS_CONFIGURATION')
44+
if (!raw) return
45+
// Parse `;`-separated entries and take the 3rd `:`-separated segment.
46+
// Expected form (per entry): `<...>:<...>:<subpath>:<...>`
47+
const entries = raw.split(';')
48+
for (const entry of entries) {
49+
if (entry) {
50+
const first = entry.indexOf(':')
51+
if (first === -1) continue
52+
const second = entry.indexOf(':', first + 1)
53+
if (second === -1) continue
54+
const third = entry.indexOf(':', second + 1)
4655

47-
for (const subpath of securityControls) {
48-
data.include.push(new RegExp(regexpEscape(subpath)))
56+
const subpath = entry.slice(second + 1, third === -1 ? undefined : third).trim()
57+
if (subpath) {
58+
data.include.push(new RegExp(regexpEscape(subpath)))
59+
}
60+
}
4961
}
5062
}
5163

5264
function addExclusions (data) {
53-
data.exclude.push(
54-
/middle/,
55-
/langsmith/,
56-
/openai\/_shims/,
57-
/openai\/resources\/chat\/completions\/messages/,
58-
/openai\/agents-core\/dist\/shims/,
59-
/@anthropic-ai\/sdk\/_shims/
60-
)
65+
data.exclude.push(...iitmExclusions)
6166
}
6267

63-
export { initialize, load }
64-
export { getFormat, resolve, getSource } from 'import-in-the-middle/hook.mjs'
68+
export const iitmExclusions = [
69+
/middle/,
70+
/langsmith/,
71+
/openai\/_shims/,
72+
/openai\/resources\/chat\/completions\/messages/,
73+
/openai\/agents-core\/dist\/shims/,
74+
/@anthropic-ai\/sdk\/_shims/,
75+
]
76+
77+
export { initialize, load, resolve }

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@
131131
],
132132
"dependencies": {
133133
"dc-polyfill": "^0.1.10",
134-
"import-in-the-middle": "2.0.3"
134+
"import-in-the-middle": "^2.0.6"
135135
},
136136
"optionalDependencies": {
137137
"@datadog/libdatadog": "0.7.0",

0 commit comments

Comments
 (0)