Skip to content

Commit 3b64de7

Browse files
authored
Fix and improve esbuild and add it to the CI (#5877)
* Test latest versions together with esbuild * Fix esbuild not picking up all files and test more This makes sure we run our tests with the oldest and newest esbuild version as well as adding the tests to our CI. They did not yet run in the CI before. It seems like esbuild does not pick up dynamic require calls, so I changed the ones I could find. This should fix the profiler to also be included in esbuild outputs in the future.
1 parent ed2469e commit 3b64de7

5 files changed

Lines changed: 73 additions & 52 deletions

File tree

.github/workflows/platform.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -281,6 +281,7 @@ jobs:
281281
- uses: ./.github/actions/install
282282
- run: sudo sysctl -w kernel.core_pattern='|/bin/false'
283283
- run: yarn test:integration
284+
- run: yarn test:integration:esbuild
284285

285286
# We'll run these separately for earlier (i.e. unsupported) versions
286287
integration-guardrails:

integration-tests/esbuild/index.spec.js

Lines changed: 52 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -11,59 +11,69 @@ const fs = require('fs')
1111
const TEST_DIR = path.join(__dirname, '.')
1212
process.chdir(TEST_DIR)
1313

14-
describe('esbuild', () => {
15-
before(() => {
16-
chproc.execSync('npm install', {
17-
timeout: 1000 * 30
14+
// This should switch to our withVersion helper. The order here currently matters.
15+
const esbuildVersions = ['latest', '0.16.12']
16+
17+
esbuildVersions.forEach((version) => {
18+
describe(`esbuild ${version}`, () => {
19+
before(() => {
20+
chproc.execSync('npm install', {
21+
timeout: 1000 * 30
22+
})
23+
if (version !== 'latest') {
24+
chproc.execSync(`npm install esbuild@${version}`, {
25+
timeout: 1000 * 30
26+
})
27+
}
1828
})
19-
})
2029

21-
it('works', () => {
22-
console.log('npm run build')
23-
chproc.execSync('npm run build')
30+
it('works', () => {
31+
console.log('npm run build')
32+
chproc.execSync('npm run build')
2433

25-
console.log('npm run built')
26-
try {
27-
chproc.execSync('npm run built', {
34+
console.log('npm run built')
35+
try {
36+
chproc.execSync('npm run built', {
37+
timeout: 1000 * 30
38+
})
39+
} catch (err) {
40+
console.error(err)
41+
process.exit(1)
42+
} finally {
43+
fs.rmSync('./out.js', { force: true })
44+
}
45+
})
46+
47+
it('does not bundle modules listed in .external', () => {
48+
const command = 'node ./build-and-test-skip-external.js'
49+
console.log(command)
50+
chproc.execSync(command, {
2851
timeout: 1000 * 30
2952
})
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
4353
})
44-
})
4554

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
55+
it('handles typescript apps that import without file extensions', () => {
56+
const command = 'node ./build-and-test-typescript.mjs'
57+
console.log(command)
58+
chproc.execSync(command, {
59+
timeout: 1000 * 30
60+
})
5161
})
52-
})
5362

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
63+
it('handles the complex aws-sdk package with dynamic requires', () => {
64+
const command = 'node ./build-and-test-aws-sdk.js'
65+
console.log(command)
66+
chproc.execSync(command, {
67+
timeout: 1000 * 30
68+
})
5969
})
60-
})
6170

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
71+
it('handles scoped node_modules', () => {
72+
const command = 'node ./build-and-test-koa.mjs'
73+
console.log(command)
74+
chproc.execSync(command, {
75+
timeout: 1000 * 30
76+
})
6777
})
6878
})
6979
})

integration-tests/esbuild/package.json

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,13 +18,13 @@
1818
"author": "Thomas Hunter II <tlhunter@datadog.com>",
1919
"license": "ISC",
2020
"dependencies": {
21-
"@apollo/server": "^4.11.0",
22-
"@koa/router": "^10.0.0",
23-
"aws-sdk": "^2.1446.0",
24-
"axios": "^1.6.7",
25-
"esbuild": "0.16.12",
21+
"@apollo/server": "*",
22+
"@koa/router": "*",
23+
"aws-sdk": "*",
24+
"axios": "*",
25+
"esbuild": "*",
2626
"express": "^4.16.2",
27-
"knex": "^2.4.2",
28-
"koa": "^2.13.4"
27+
"knex": "*",
28+
"koa": "*"
2929
}
3030
}

packages/dd-trace/src/exporters/agent/writer.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,9 @@ function setHeader (headers, key, value) {
6969
}
7070

7171
function getEncoder (protocolVersion) {
72-
return require(`../../encode/${protocolVersion === '0.5' ? '0.5' : '0.4'}`).AgentEncoder
72+
return protocolVersion === '0.5'
73+
? require('../../encode/0.5').AgentEncoder
74+
: require('../../encode/0.4').AgentEncoder
7375
}
7476

7577
function makeRequest (version, data, count, url, headers, lookup, needsStartupLog, cb) {

packages/dd-trace/src/profiling/profilers/events.js

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -291,8 +291,16 @@ class NodeApiEventSource {
291291

292292
class DatadogInstrumentationEventSource {
293293
constructor (eventHandler, eventFilter) {
294-
this.plugins = ['dns_lookup', 'dns_lookupservice', 'dns_resolve', 'dns_reverse', 'fs', 'net'].map(m => {
295-
const Plugin = require(`./event_plugins/${m}`)
294+
// List all entries explicitly for bundlers to pick up the require calls correctly.
295+
const plugins = [
296+
require('./event_plugins/dns_lookup'),
297+
require('./event_plugins/dns_lookupservice'),
298+
require('./event_plugins/dns_resolve'),
299+
require('./event_plugins/dns_reverse'),
300+
require('./event_plugins/fs'),
301+
require('./event_plugins/net')
302+
]
303+
this.plugins = plugins.map((Plugin) => {
296304
return new Plugin(eventHandler, eventFilter)
297305
})
298306

0 commit comments

Comments
 (0)