Skip to content

Commit fe7175a

Browse files
authored
refactor(esbuild): extract logging into module (#7439)
Extract console logging into a dedicated log module that: - Uses printf-style formatting for consistency - Conditionally logs based on DD_TRACE_DEBUG env var - Eliminates scattered DEBUG conditionals throughout code - Adds consistent prefixes for better log identification Also cleans up the `require` section of the code to modernizes imports with `node:` prefix.
1 parent 3c1cd8a commit fe7175a

File tree

2 files changed

+68
-49
lines changed

2 files changed

+68
-49
lines changed

packages/datadog-esbuild/index.js

Lines changed: 36 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,16 @@
11
'use strict'
22

3-
/* eslint-disable no-console */
4-
5-
const { execSync } = require('child_process')
6-
const fs = require('fs')
7-
const RAW_BUILTINS = require('module').builtinModules
8-
const path = require('path')
9-
const { pathToFileURL, fileURLToPath } = require('url')
10-
11-
const instrumentations = require('../datadog-instrumentations/src/helpers/instrumentations.js')
12-
13-
const extractPackageAndModulePath = require(
14-
'../datadog-instrumentations/src/helpers/extract-package-and-module-path.js'
15-
)
16-
const hooks = require('../datadog-instrumentations/src/helpers/hooks.js')
17-
const { processModule, isESMFile } = require('./src/utils.js')
3+
const { execSync } = require('node:child_process')
4+
const fs = require('node:fs')
5+
const RAW_BUILTINS = require('node:module').builtinModules
6+
const path = require('node:path')
7+
const { pathToFileURL, fileURLToPath } = require('node:url')
8+
9+
const instrumentations = require('../datadog-instrumentations/src/helpers/instrumentations')
10+
const extractPackageAndModulePath = require('../datadog-instrumentations/src/helpers/extract-package-and-module-path')
11+
const hooks = require('../datadog-instrumentations/src/helpers/hooks')
12+
const { processModule, isESMFile } = require('./src/utils')
13+
const log = require('./src/log')
1814

1915
const ESM_INTERCEPTED_SUFFIX = '._dd_esbuild_intercepted'
2016
const INTERNAL_ESM_INTERCEPTED_PREFIX = '/_dd_esm_internal_/'
@@ -50,8 +46,6 @@ for (const builtin of RAW_BUILTINS) {
5046
builtins.add(`node:${builtin}`)
5147
}
5248

53-
// eslint-disable-next-line eslint-rules/eslint-process-env
54-
const DEBUG = !!process.env.DD_TRACE_DEBUG
5549
// eslint-disable-next-line eslint-rules/eslint-process-env
5650
const DD_IAST_ENABLED = process.env.DD_IAST_ENABLED?.toLowerCase() === 'true' || process.env.DD_IAST_ENABLED === '1'
5751

@@ -83,9 +77,7 @@ function getGitMetadata () {
8377
cwd: process.cwd(),
8478
}).trim()
8579
} catch (e) {
86-
if (DEBUG) {
87-
console.warn('Warning: failed to get git repository URL:', e.message)
88-
}
80+
log.warn('failed to get git repository URL:', e.message)
8981
}
9082

9183
try {
@@ -95,9 +87,7 @@ function getGitMetadata () {
9587
cwd: process.cwd(),
9688
}).trim()
9789
} catch (e) {
98-
if (DEBUG) {
99-
console.warn('Warning: failed to get git commit SHA:', e.message)
100-
}
90+
log.warn('failed to get git commit SHA:', e.message)
10191
}
10292

10393
return gitMetadata
@@ -162,13 +152,13 @@ ${build.initialOptions.banner.js}`
162152
}
163153
${build.initialOptions.banner.js}`
164154

165-
if (DEBUG) {
166-
console.log('Info: automatically injected git metadata:')
167-
console.log(`DD_GIT_REPOSITORY_URL: ${gitMetadata.repositoryURL || 'not available'}`)
168-
console.log(`DD_GIT_COMMIT_SHA: ${gitMetadata.commitSHA || 'not available'}`)
169-
}
170-
} else if (DEBUG) {
171-
console.warn('Warning: No git metadata available - skipping injection')
155+
log.debug(
156+
'Automatically injected git metadata (DD_GIT_REPOSITORY_URL: %s, DD_GIT_COMMIT_SHA: %s)',
157+
gitMetadata.repositoryURL || 'not available',
158+
gitMetadata.commitSHA || 'not available'
159+
)
160+
} else {
161+
log.warn('No git metadata available - skipping injection')
172162
}
173163

174164
// first time is intercepted, proxy should be created, next time the original should be loaded
@@ -177,7 +167,7 @@ ${build.initialOptions.banner.js}`
177167
build.onResolve({ filter: /.*/ }, args => {
178168
if (externalModules.has(args.path)) {
179169
// Internal Node.js packages will still be instrumented via require()
180-
if (DEBUG) console.log(`EXTERNAL: ${args.path}`)
170+
log.debug('EXTERNAL: %s', args.path)
181171
return
182172
}
183173

@@ -186,24 +176,21 @@ ${build.initialOptions.banner.js}`
186176
args.path.startsWith('@') &&
187177
!args.importer.includes('node_modules/')) {
188178
// This is the Next.js convention for loading local files
189-
if (DEBUG) console.log(`@LOCAL: ${args.path}`)
179+
log.debug('@LOCAL: %s', args.path)
190180
return
191181
}
192182

193183
let fullPathToModule
194184
try {
195185
fullPathToModule = dotFriendlyResolve(args.path, args.resolveDir, args.kind === 'import-statement')
196186
} catch {
197-
if (DEBUG) {
198-
console.warn(`Warning: Unable to find "${args.path}".` +
199-
"Unless it's dead code this could cause a problem at runtime.")
200-
}
187+
log.warn('Unable to find "%s". Unless it\'s dead code this could cause a problem at runtime.', args.path)
201188
return
202189
}
203190

204191
if (args.path.startsWith('.') && !args.importer.includes('node_modules/')) {
205192
// It is local application code, not an instrumented package
206-
if (DEBUG) console.log(`APP: ${args.path}`)
193+
log.debug('APP: %s', args.path)
207194

208195
return {
209196
path: fullPathToModule,
@@ -248,9 +235,11 @@ ${build.initialOptions.banner.js}`
248235
pathToPackageJson = extractPackageAndModulePath(pathToPackageJson).pkgJson
249236
} catch (err) {
250237
if (err.code === 'MODULE_NOT_FOUND') {
251-
if (!internal && DEBUG) {
252-
console.warn(`Warning: Unable to find "${extracted.pkg}/package.json".` +
253-
"Unless it's dead code this could cause a problem at runtime.")
238+
if (!internal) {
239+
log.warn(
240+
'Unable to find "%s/package.json". Unless it\'s dead code this could cause a problem at runtime.',
241+
extracted.pkg
242+
)
254243
}
255244
return
256245
}
@@ -265,7 +254,7 @@ ${build.initialOptions.banner.js}`
265254
fullPathToModule += ESM_INTERCEPTED_SUFFIX
266255
}
267256

268-
if (DEBUG) console.log(`RESOLVE: ${args.path}@${packageJson.version}`)
257+
log.debug('RESOLVE: %s@%s', args.path, packageJson.version)
269258

270259
// https://esbuild.github.io/plugins/#on-resolve-arguments
271260
return {
@@ -288,12 +277,10 @@ ${build.initialOptions.banner.js}`
288277
// since those files should be treated as regular files and not modules
289278
// even though they are in a `node_modules` folder.
290279
if (e.code === 'ENOENT') {
291-
if (DEBUG) {
292-
console.log([
293-
'Skipping `package.json` lookup.',
294-
'This usually means the package was vendored but could indicate an issue otherwise.',
295-
].join(' '))
296-
}
280+
log.debug(
281+
// eslint-disable-next-line @stylistic/max-len
282+
'Skipping `package.json` lookup. This usually means the package was vendored but could indicate an issue otherwise.'
283+
)
297284
} else {
298285
throw e
299286
}
@@ -305,7 +292,7 @@ ${build.initialOptions.banner.js}`
305292
if (args.pluginData?.pkgOfInterest) {
306293
const data = args.pluginData
307294

308-
if (DEBUG) console.log(`LOAD: ${data.pkg}@${data.version}, pkg "${data.path}"`)
295+
log.debug('LOAD: %s@%s, pkg "%s"', data.pkg, data.version, data.path)
309296

310297
const pkgPath = data.raw === data.pkg
311298
? data.pkg
@@ -381,7 +368,7 @@ register(${JSON.stringify(toRegister)}, _, set, get, ${JSON.stringify(data.raw)}
381368
const isJs = /^\.(js|mjs|cjs)$/.test(ext)
382369
if (!isJs) return
383370

384-
if (DEBUG) console.log(`REWRITE: ${args.path}`)
371+
log.debug('REWRITE: %s', args.path)
385372
const fileCode = fs.readFileSync(args.path, 'utf8')
386373
const rewritten = rewriter.rewrite(fileCode, args.path, ['iast'])
387374
return {
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
'use strict'
2+
3+
const { format } = require('util')
4+
5+
// eslint-disable-next-line eslint-rules/eslint-process-env
6+
const DD_TRACE_DEBUG = (process.env.DD_TRACE_DEBUG || '').trim().toLowerCase()
7+
const DEBUG = DD_TRACE_DEBUG === 'true' || DD_TRACE_DEBUG === '1'
8+
9+
const noop = () => {}
10+
11+
const formatWithLogPrefix = (prefix, str, ...args) => {
12+
if (typeof str === 'string') {
13+
return format(`${prefix} ${str}`, ...args)
14+
}
15+
return format(prefix, str, ...args)
16+
}
17+
18+
module.exports = DEBUG
19+
? {
20+
debug (...args) {
21+
// eslint-disable-next-line no-console
22+
console.log(formatWithLogPrefix('[dd-trace/esbuild]', ...args))
23+
},
24+
warn (...args) {
25+
// eslint-disable-next-line no-console
26+
console.warn(formatWithLogPrefix('[dd-trace/esbuild] Warning:', ...args))
27+
},
28+
}
29+
: {
30+
debug: noop,
31+
warn: noop,
32+
}

0 commit comments

Comments
 (0)