Skip to content

Commit 0b396b6

Browse files
authored
Fix esbuild not instrumenting namespaced modules (#5588)
* Fix esbuild not instrumenting namespaced modules As drive-by it simplifies the tests and moved the test to the dedicated esbuild part. * Install esbuild dependencies before running tests That prevents the need to add these dependencies to the root.
1 parent 54316ae commit 0b396b6

File tree

11 files changed

+151
-109
lines changed

11 files changed

+151
-109
lines changed

integration-tests/esbuild.spec.js

Lines changed: 0 additions & 84 deletions
This file was deleted.

integration-tests/esbuild/.gitignore

Lines changed: 0 additions & 1 deletion
This file was deleted.

integration-tests/esbuild/build-and-test-aws-sdk.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
#!/usr/bin/env node
22
/* eslint-disable no-console */
3+
const fs = require('fs')
34
const { spawnSync } = require('child_process')
45

56
const ddPlugin = require('../../esbuild') // dd-trace/esbuild
@@ -31,5 +32,5 @@ esbuild.build({
3132
console.error(err)
3233
process.exit(1)
3334
}).finally(() => {
34-
// fs.rmSync(SCRIPT)
35+
fs.rmSync(SCRIPT, { force: true })
3536
})
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
#!/usr/bin/env node
2+
3+
import fs from 'fs/promises'
4+
5+
import * as esbuild from 'esbuild'
6+
import assert from 'assert'
7+
8+
import ddPlugin from '../../esbuild.js'
9+
10+
try {
11+
await esbuild.build({
12+
entryPoints: ['./koa.mjs'],
13+
bundle: true,
14+
outfile: './outfile.js',
15+
minify: false,
16+
sourcemap: false,
17+
platform: 'node',
18+
target: 'es2022',
19+
plugins: [ddPlugin],
20+
external: [
21+
'graphql/language/visitor',
22+
'graphql/language/printer',
23+
'graphql/utilities'
24+
]
25+
})
26+
27+
// Verify instrumentation
28+
const data = await fs.readFile('./outfile.js', 'utf8')
29+
30+
assert.match(data, /^ {8}package: "koa",$/m, 'Bundle should contain the koa instrumentation')
31+
assert.match(data, /^ {8}package: "@koa\/router",$/m, 'Bundle should contain the @koa/router instrumentation')
32+
33+
console.log('ok') // eslint-disable-line no-console
34+
} finally {
35+
await fs.rm('./outfile.js', { force: true })
36+
}

integration-tests/esbuild/build-and-test-skip-external.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,9 @@ esbuild.build({
2121
assert(output.includes('require("knex")'), 'bundle should contain a require call to non-bundled knex')
2222
assert(!output.includes('require("axios")'), 'bundle should not contain a require call to bundled axios')
2323
console.log('ok') // eslint-disable-line no-console
24-
fs.rmSync('./skip-external-out.js')
2524
}).catch((err) => {
2625
console.error(err) // eslint-disable-line no-console
2726
process.exit(1)
27+
}).finally(() => {
28+
fs.rmSync('./skip-external-out.js', { force: true })
2829
})

integration-tests/esbuild/build-and-test-typescript.mjs

Lines changed: 20 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -6,21 +6,23 @@ import * as esbuild from 'esbuild'
66

77
import ddPlugin from '../../esbuild.js'
88

9-
await esbuild.build({
10-
entryPoints: ['typescript-app.ts'],
11-
bundle: true,
12-
outfile: 'typescript-app-out.js',
13-
minify: false,
14-
sourcemap: false,
15-
platform: 'node',
16-
target: 'es2022',
17-
plugins: [ddPlugin],
18-
external: [
19-
'graphql/language/visitor',
20-
'graphql/language/printer',
21-
'graphql/utilities'
22-
]
23-
})
24-
25-
console.log('ok') // eslint-disable-line no-console
26-
fs.rmSync('typescript-app-out.js')
9+
try {
10+
await esbuild.build({
11+
entryPoints: ['typescript-app.ts'],
12+
bundle: true,
13+
outfile: 'typescript-app-out.js',
14+
minify: false,
15+
sourcemap: false,
16+
platform: 'node',
17+
target: 'es2022',
18+
plugins: [ddPlugin],
19+
external: [
20+
'graphql/language/visitor',
21+
'graphql/language/printer',
22+
'graphql/utilities'
23+
]
24+
})
25+
console.log('ok') // eslint-disable-line no-console
26+
} finally {
27+
fs.rmSync('typescript-app-out.js', { force: true })
28+
}
Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
#!/usr/bin/env node
2+
3+
/* eslint-disable no-console */
4+
5+
'use strict'
6+
7+
const chproc = require('child_process')
8+
const path = require('path')
9+
const fs = require('fs')
10+
11+
const TEST_DIR = path.join(__dirname, '.')
12+
process.chdir(TEST_DIR)
13+
14+
describe('esbuild', () => {
15+
before(() => {
16+
chproc.execSync('npm install', {
17+
timeout: 1000 * 30
18+
})
19+
})
20+
21+
it('works', () => {
22+
console.log('npm run build')
23+
chproc.execSync('npm run build')
24+
25+
console.log('npm run built')
26+
try {
27+
chproc.execSync('npm run built', {
28+
timeout: 1000 * 30
29+
})
30+
} catch (err) {
31+
console.error(err)
32+
process.exit(1)
33+
} finally {
34+
fs.rmSync('./out.js', { force: true })
35+
}
36+
})
37+
38+
it('does not bundle modules listed in .external', () => {
39+
const command = 'node ./build-and-test-skip-external.js'
40+
console.log(command)
41+
chproc.execSync(command, {
42+
timeout: 1000 * 30
43+
})
44+
})
45+
46+
it('handles typescript apps that import without file extensions', () => {
47+
const command = 'node ./build-and-test-typescript.mjs'
48+
console.log(command)
49+
chproc.execSync(command, {
50+
timeout: 1000 * 30
51+
})
52+
})
53+
54+
it('handles the complex aws-sdk package with dynamic requires', () => {
55+
const command = 'node ./build-and-test-aws-sdk.js'
56+
console.log(command)
57+
chproc.execSync(command, {
58+
timeout: 1000 * 30
59+
})
60+
})
61+
62+
it('handles scoped node_modules', () => {
63+
const command = 'node ./build-and-test-koa.mjs'
64+
console.log(command)
65+
chproc.execSync(command, {
66+
timeout: 1000 * 30
67+
})
68+
})
69+
})

integration-tests/esbuild/koa.mjs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
import Koa from 'koa'
2+
import Router from '@koa/router'
3+
4+
const app = new Koa()
5+
6+
const fooRouter = new Router()
7+
8+
fooRouter.get('/foo', (ctx) => {
9+
ctx.body = 'foo'
10+
})
11+
12+
app.use(fooRouter.routes())
13+
14+
app.listen(3000)

integration-tests/esbuild/package.json

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,12 @@
1919
"license": "ISC",
2020
"dependencies": {
2121
"@apollo/server": "^4.11.0",
22+
"@koa/router": "^10.0.0",
2223
"aws-sdk": "^2.1446.0",
2324
"axios": "^1.6.7",
2425
"esbuild": "0.16.12",
2526
"express": "^4.16.2",
26-
"knex": "^2.4.2"
27+
"knex": "^2.4.2",
28+
"koa": "^2.13.4"
2729
}
2830
}

package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@
4848
"test:integration:cucumber": "mocha --timeout 60000 -r \"packages/dd-trace/test/setup/core.js\" \"integration-tests/cucumber/*.spec.js\"",
4949
"test:integration:cypress": "mocha --timeout 60000 -r \"packages/dd-trace/test/setup/core.js\" \"integration-tests/cypress/*.spec.js\"",
5050
"test:integration:debugger": "mocha --timeout 60000 -r \"packages/dd-trace/test/setup/core.js\" \"integration-tests/debugger/*.spec.js\"",
51+
"test:integration:esbuild": "mocha --timeout 60000 -r \"packages/dd-trace/test/setup/core.js\" \"integration-tests/esbuild/*.spec.js\"",
5152
"test:integration:jest": "mocha --timeout 60000 -r \"packages/dd-trace/test/setup/core.js\" \"integration-tests/jest/*.spec.js\"",
5253
"test:integration:mocha": "mocha --timeout 60000 -r \"packages/dd-trace/test/setup/core.js\" \"integration-tests/mocha/*.spec.js\"",
5354
"test:integration:playwright": "mocha --timeout 60000 -r \"packages/dd-trace/test/setup/core.js\" \"integration-tests/playwright/*.spec.js\"",

0 commit comments

Comments
 (0)