Skip to content

Commit f429a85

Browse files
szegediclaude
szegedi
and
claude
authored
feat(profiling): Start profilers synchronously within tracer initialization (#5906)
* feat(profiler): make profiler startup synchronous Replace `await SourceMapper.create()` with a synchronous `new SourceMapper()` constructor call followed by a fire-and-forget `loadDirectory()`. The mapper is handed to profilers immediately (with an initially empty `infoMap`); `#sourceMapCount` is updated in the background `.then()` callback once the filesystem scan completes. This removes the only `await` from `_start()`, making it — and the entire profiler start path — synchronous. `start()` now uses try/catch instead of `.catch()` chaining. `proxy.js` is updated accordingly: `_profilerStarted` is stored as a plain boolean and `profilerStarted()` wraps it in `Promise.resolve()` for backwards compatibility with existing callers. We also move zlib/compression setup out of the synchronous startup path into a lazy `#getCompressionFn()` private method, initialized on first call. `zlib` and `util.promisify` are now required inside that method rather than at module load time, keeping the startup path free of I/O-adjacent module loading. Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
1 parent 123fee9 commit f429a85

File tree

4 files changed

+99
-85
lines changed

4 files changed

+99
-85
lines changed

‎integration-tests/profiler/profiler.spec.js‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -278,8 +278,9 @@ async function gatherTimelineEvents (cwd, scriptFilePath, agentPort, eventType,
278278
// Gather only tested events
279279
if (event === eventValue) {
280280
if (process.platform !== 'win32') {
281-
assert.notStrictEqual(spanId, undefined, encoded)
282-
assert.notStrictEqual(localRootSpanId, undefined, encoded)
281+
// Skip events without span IDs: these are from internal operations (e.g. background
282+
// source map loading) that occur outside any user span and are not relevant to the test.
283+
if (spanId === undefined || localRootSpanId === undefined) continue
283284
} else {
284285
assert.strictEqual(spanId, undefined, encoded)
285286
assert.strictEqual(localRootSpanId, undefined, encoded)

‎packages/dd-trace/src/profiling/profiler.js‎

Lines changed: 66 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
'use strict'
22

33
const { EventEmitter } = require('events')
4-
const { promisify } = require('util')
5-
const zlib = require('zlib')
64
const dc = require('dc-polyfill')
75
const crashtracker = require('../crashtracking')
86
const log = require('../log')
@@ -42,6 +40,7 @@ function processInfo (infos, info, type) {
4240

4341
class Profiler extends EventEmitter {
4442
#compressionFn
43+
#compressionFnInitialized = false
4544
#compressionOptions
4645
#config
4746
#enabled = false
@@ -116,11 +115,13 @@ class Profiler extends EventEmitter {
116115
reportHostname,
117116
}
118117

119-
return this._start(options).catch((err) => {
118+
try {
119+
return this._start(options)
120+
} catch (err) {
120121
logError(logger, 'Error starting profiler. For troubleshooting tips, see ' +
121122
'<https://dtdg.co/nodejs-profiler-troubleshooting>', err)
122123
return false
123-
})
124+
}
124125
}
125126

126127
get enabled () {
@@ -131,7 +132,50 @@ class Profiler extends EventEmitter {
131132
logError(this.#logger, err)
132133
}
133134

134-
async _start (options) {
135+
#getCompressionFn () {
136+
if (!this.#compressionFnInitialized) {
137+
this.#compressionFnInitialized = true
138+
try {
139+
const { promisify } = require('util')
140+
const zlib = require('zlib')
141+
const { method, level: clevel } = this.#config.uploadCompression
142+
switch (method) {
143+
case 'gzip':
144+
this.#compressionFn = promisify(zlib.gzip)
145+
if (clevel !== undefined) {
146+
this.#compressionOptions = {
147+
level: clevel,
148+
}
149+
}
150+
break
151+
case 'zstd':
152+
// eslint-disable-next-line n/no-unsupported-features/node-builtins
153+
if (typeof zlib.zstdCompress === 'function') {
154+
// eslint-disable-next-line n/no-unsupported-features/node-builtins
155+
this.#compressionFn = promisify(zlib.zstdCompress)
156+
if (clevel !== undefined) {
157+
this.#compressionOptions = {
158+
params: {
159+
// eslint-disable-next-line n/no-unsupported-features/node-builtins
160+
[zlib.constants.ZSTD_c_compressionLevel]: clevel,
161+
},
162+
}
163+
}
164+
} else {
165+
const zstdCompress = require('@datadog/libdatadog').load('datadog-js-zstd').zstd_compress
166+
const level = clevel ?? 0 // 0 is zstd default compression level
167+
this.#compressionFn = (buffer) => Promise.resolve(Buffer.from(zstdCompress(buffer, level)))
168+
}
169+
break
170+
}
171+
} catch (err) {
172+
this.#logError(err)
173+
}
174+
}
175+
return this.#compressionFn
176+
}
177+
178+
_start (options) {
135179
if (this.enabled) return true
136180

137181
const config = this.#config = new Config(options)
@@ -148,45 +192,21 @@ class Profiler extends EventEmitter {
148192
setLogger(config.logger)
149193

150194
if (config.sourceMap) {
151-
mapper = await SourceMapper.create([process.cwd()], config.debugSourceMaps)
152-
this.#sourceMapCount = mapper.infoMap.size
153-
if (config.debugSourceMaps) {
154-
this.#logger.debug(() => {
155-
return this.#sourceMapCount === 0
156-
? 'Found no source maps'
157-
: `Found source maps for following files: [${[...mapper.infoMap.keys()].join(', ')}]`
158-
})
159-
}
160-
}
161-
162-
const clevel = config.uploadCompression.level
163-
switch (config.uploadCompression.method) {
164-
case 'gzip':
165-
this.#compressionFn = promisify(zlib.gzip)
166-
if (clevel !== undefined) {
167-
this.#compressionOptions = {
168-
level: clevel,
169-
}
170-
}
171-
break
172-
case 'zstd':
173-
if (typeof zlib.zstdCompress === 'function') { // eslint-disable-line n/no-unsupported-features/node-builtins
174-
// eslint-disable-next-line n/no-unsupported-features/node-builtins
175-
this.#compressionFn = promisify(zlib.zstdCompress)
176-
if (clevel !== undefined) {
177-
this.#compressionOptions = {
178-
params: {
179-
// eslint-disable-next-line n/no-unsupported-features/node-builtins
180-
[zlib.constants.ZSTD_c_compressionLevel]: clevel,
181-
},
182-
}
195+
mapper = new SourceMapper(config.debugSourceMaps)
196+
mapper.loadDirectory(process.cwd())
197+
.then(() => {
198+
this.#sourceMapCount = mapper.infoMap.size
199+
if (config.debugSourceMaps) {
200+
this.#logger.debug(() => {
201+
return this.#sourceMapCount === 0
202+
? 'Found no source maps'
203+
: `Found source maps for following files: [${[...mapper.infoMap.keys()].join(', ')}]`
204+
})
183205
}
184-
} else {
185-
const zstdCompress = require('@datadog/libdatadog').load('datadog-js-zstd').zstd_compress
186-
const level = clevel ?? 0 // 0 is zstd default compression level
187-
this.#compressionFn = (buffer) => Promise.resolve(Buffer.from(zstdCompress(buffer, level)))
188-
}
189-
break
206+
})
207+
.catch((err) => {
208+
this.#logError(err)
209+
})
190210
}
191211
} catch (err) {
192212
this.#logError(err)
@@ -332,13 +352,14 @@ class Profiler extends EventEmitter {
332352

333353
const encodedProfiles = {}
334354
const infos = this.#createInitialInfos()
355+
const compressionFn = this.#getCompressionFn()
335356

336357
// encode and export asynchronously
337358
await Promise.all(profiles.map(async ({ profiler, profile, info }) => {
338359
try {
339360
const encoded = await profiler.encode(profile)
340-
const compressed = encoded instanceof Buffer && this.#compressionFn !== undefined
341-
? await this.#compressionFn(encoded, this.#compressionOptions)
361+
const compressed = encoded instanceof Buffer && compressionFn !== undefined
362+
? await compressionFn(encoded, this.#compressionOptions)
342363
: encoded
343364
encodedProfiles[profiler.type] = compressed
344365
processInfo(infos, info, profiler.type)

‎packages/dd-trace/src/proxy.js‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,7 @@ class Tracer extends NoopProxy {
180180
if (config.profiling.enabled === 'true') {
181181
this._profilerStarted = this._startProfiler(config)
182182
} else {
183-
this._profilerStarted = Promise.resolve(false)
183+
this._profilerStarted = false
184184
if (config.profiling.enabled === 'auto') {
185185
const { SSIHeuristics } = require('./profiling/ssi-heuristics')
186186
const ssiHeuristics = new SSIHeuristics(config)
@@ -252,6 +252,7 @@ class Tracer extends NoopProxy {
252252
'Error starting profiler. For troubleshooting tips, see <https://dtdg.co/nodejs-profiler-troubleshooting>',
253253
e
254254
)
255+
return false
255256
}
256257
}
257258

@@ -329,11 +330,11 @@ class Tracer extends NoopProxy {
329330
* @override
330331
*/
331332
profilerStarted () {
332-
if (!this._profilerStarted) {
333+
if (this._profilerStarted === undefined) {
333334
// injection hardening: this is only ever invoked from tests.
334335
throw new Error('profilerStarted() must be called after init()')
335336
}
336-
return this._profilerStarted
337+
return Promise.resolve(this._profilerStarted)
337338
}
338339

339340
/**

‎packages/dd-trace/test/profiling/profiler.spec.js‎

Lines changed: 26 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,8 @@ describe('profiler', function () {
2424
let profilers
2525
let consoleLogger
2626
let logger
27-
let sourceMapCreate
27+
let SourceMapperStub
28+
let mapperInstance
2829
let interval
2930

3031
function waitForExport () {
@@ -79,7 +80,11 @@ describe('profiler', function () {
7980
exporters = [exporter]
8081
profilers = [wallProfiler, spaceProfiler]
8182

82-
sourceMapCreate = sinon.stub()
83+
mapperInstance = {
84+
infoMap: new Map(),
85+
loadDirectory: sinon.stub().resolves(),
86+
}
87+
SourceMapperStub = sinon.stub().returns(mapperInstance)
8388
}
8489

8590
function makeStartOptions (overrides = {}) {
@@ -95,9 +100,7 @@ describe('profiler', function () {
95100
function initProfiler () {
96101
Profiler = proxyquire('../../src/profiling/profiler', {
97102
'@datadog/pprof': {
98-
SourceMapper: {
99-
create: sourceMapCreate,
100-
},
103+
SourceMapper: SourceMapperStub,
101104
},
102105
}).Profiler
103106

@@ -347,19 +350,19 @@ describe('profiler', function () {
347350
})
348351

349352
it('should pass source mapper to profilers when enabled', async () => {
350-
const mapper = { infoMap: new Map() }
351-
sourceMapCreate.returns(Promise.resolve(mapper))
352-
await profiler._start(makeStartOptions({ sourceMap: true }))
353+
profiler._start(makeStartOptions({ sourceMap: true }))
353354

354355
const options = profilers[0].start.args[0][0]
355356
assert.ok(Object.hasOwn(options, 'mapper'))
356-
assert.strictEqual(mapper, options.mapper)
357+
assert.strictEqual(mapperInstance, options.mapper)
357358
})
358359

359360
it('should work with a root working dir and source maps on', async () => {
360361
const error = new Error('fail')
361-
sourceMapCreate.rejects(error)
362-
await profiler._start(makeStartOptions({ logger, sourceMap: true }))
362+
mapperInstance.loadDirectory.rejects(error)
363+
profiler._start(makeStartOptions({ logger, sourceMap: true }))
364+
await Promise.resolve() // let .then() propagate the rejection
365+
await Promise.resolve() // let .catch() callback run
363366
assert.strictEqual(consoleLogger.error.args[0][0], error)
364367
assert.strictEqual(profiler.enabled, true)
365368
})
@@ -403,17 +406,15 @@ describe('profiler', function () {
403406
})
404407

405408
it('should include sourceMapCount: 0 when no source maps are found', async () => {
406-
const mapper = { infoMap: new Map() }
407-
sourceMapCreate.returns(Promise.resolve(mapper))
408-
409409
exporterPromise = new Promise(resolve => {
410410
exporter.export = (exportSpec) => {
411411
resolve(exportSpec)
412412
return Promise.resolve()
413413
}
414414
})
415415

416-
await profiler._start(makeStartOptions({ sourceMap: true }))
416+
profiler._start(makeStartOptions({ sourceMap: true }))
417+
await Promise.resolve() // flush .then() callback so #sourceMapCount is set
417418

418419
clock.tick(interval)
419420

@@ -423,14 +424,9 @@ describe('profiler', function () {
423424
})
424425

425426
it('should include sourceMapCount with the number of loaded source maps', async () => {
426-
const mapper = {
427-
infoMap: new Map([
428-
['file1.js', {}],
429-
['file2.js', {}],
430-
['file3.js', {}],
431-
]),
432-
}
433-
sourceMapCreate.returns(Promise.resolve(mapper))
427+
mapperInstance.infoMap.set('file1.js', {})
428+
mapperInstance.infoMap.set('file2.js', {})
429+
mapperInstance.infoMap.set('file3.js', {})
434430

435431
exporterPromise = new Promise(resolve => {
436432
exporter.export = (exportSpec) => {
@@ -439,7 +435,8 @@ describe('profiler', function () {
439435
}
440436
})
441437

442-
await profiler._start(makeStartOptions({ sourceMap: true }))
438+
profiler._start(makeStartOptions({ sourceMap: true }))
439+
await Promise.resolve() // flush .then() callback so #sourceMapCount is set
443440

444441
clock.tick(interval)
445442

@@ -455,9 +452,7 @@ describe('profiler', function () {
455452
function initServerlessProfiler () {
456453
Profiler = proxyquire('../../src/profiling/profiler', {
457454
'@datadog/pprof': {
458-
SourceMapper: {
459-
create: sourceMapCreate,
460-
},
455+
SourceMapper: SourceMapperStub,
461456
},
462457
}).ServerlessProfiler
463458

@@ -526,13 +521,8 @@ describe('profiler', function () {
526521
})
527522

528523
it('should include sourceMapCount in export infos', async () => {
529-
const mapper = {
530-
infoMap: new Map([
531-
['file1.js', {}],
532-
['file2.js', {}],
533-
]),
534-
}
535-
sourceMapCreate.returns(Promise.resolve(mapper))
524+
mapperInstance.infoMap.set('file1.js', {})
525+
mapperInstance.infoMap.set('file2.js', {})
536526

537527
exporterPromise = new Promise(resolve => {
538528
exporter.export = (exportSpec) => {
@@ -541,7 +531,8 @@ describe('profiler', function () {
541531
}
542532
})
543533

544-
await profiler._start(makeStartOptions({ sourceMap: true }))
534+
profiler._start(makeStartOptions({ sourceMap: true }))
535+
await Promise.resolve() // flush .then() callback so #sourceMapCount is set
545536

546537
// flushAfterIntervals + 1 because it flushes after last interval
547538
for (let i = 0; i < flushAfterIntervals + 1; i++) {

0 commit comments

Comments
 (0)