Skip to content

Commit

Permalink
Avoid restarting profiling for last collect
Browse files Browse the repository at this point in the history
Upon process exit, profiler collects profiles a last time and was
restarting all profilers before stopping them.
Avoid this useless restart by changing the profiler API and make
`profile()` methode take a restart argument.
  • Loading branch information
nsavoire committed Jan 5, 2024
1 parent 7cd2e42 commit d250c7e
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 34 deletions.
31 changes: 17 additions & 14 deletions packages/dd-trace/src/profiling/profiler.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ class Profiler extends EventEmitter {
mapper,
nearOOMCallback: this._nearOOMExport.bind(this)
})
this._logger.debug(`Started ${profiler.type} profiler in thread ${threadId}`)
this._logger.debug(`Started ${profiler.type} profiler in ${threadNamePrefix} thread`)
}

this._capture(this._timeoutInterval, start)
Expand Down Expand Up @@ -98,7 +98,7 @@ class Profiler extends EventEmitter {

// collect and export current profiles
// once collect returns, profilers can be safely stopped
this._collect(snapshotKinds.ON_SHUTDOWN)
this._collect(snapshotKinds.ON_SHUTDOWN, false)
this._stop()
}

Expand All @@ -109,13 +109,11 @@ class Profiler extends EventEmitter {

for (const profiler of this._config.profilers) {
profiler.stop()
this._logger.debug(`Stopped ${profiler.type} profiler`)
this._logger.debug(`Stopped ${profiler.type} profiler in ${threadNamePrefix} thread`)
}

clearTimeout(this._timer)
this._timer = undefined

return this
}

_capture (timeout, start) {
Expand All @@ -129,18 +127,21 @@ class Profiler extends EventEmitter {
}
}

async _collect (snapshotKind) {
async _collect (snapshotKind, restart = true) {
if (!this._enabled) return

const start = this._lastStart
const end = new Date()
const startDate = this._lastStart
const endDate = new Date()
const profiles = []
const encodedProfiles = {}

try {
// collect profiles synchronously so that profilers can be safely stopped asynchronously
for (const profiler of this._config.profilers) {
const profile = profiler.profile(start, end)
const profile = profiler.profile({ startDate, endDate, restart })
if (!restart) {
this._logger.debug(`Stopped ${profiler.type} profiler in ${threadNamePrefix} thread`)
}
if (!profile) continue
profiles.push({ profiler, profile })
}
Expand All @@ -156,8 +157,10 @@ class Profiler extends EventEmitter {
})
}

this._capture(this._timeoutInterval, end)
await this._submit(encodedProfiles, start, end, snapshotKind)
if (restart) {
this._capture(this._timeoutInterval, endDate)
}
await this._submit(encodedProfiles, startDate, endDate, snapshotKind)
this._logger.debug('Submitted profiles')
} catch (err) {
this._logger.error(err)
Expand Down Expand Up @@ -197,10 +200,10 @@ class ServerlessProfiler extends Profiler {
this._flushAfterIntervals = this._config.flushInterval / 1000
}

async _collect (snapshotKind) {
if (this._profiledIntervals >= this._flushAfterIntervals) {
async _collect (snapshotKind, restart = true) {
if (this._profiledIntervals >= this._flushAfterIntervals || !restart) {
this._profiledIntervals = 0
await super._collect(snapshotKind)
await super._collect(snapshotKind, restart)
} else {
this._profiledIntervals += 1
this._capture(this._timeoutInterval, new Date())
Expand Down
7 changes: 6 additions & 1 deletion packages/dd-trace/src/profiling/profilers/events.js
Original file line number Diff line number Diff line change
Expand Up @@ -171,10 +171,11 @@ class EventsProfiler {
stop () {
if (this._observer) {
this._observer.disconnect()
this._observer = undefined
}
}

profile (startDate, endDate) {
profile ({ startDate, endDate, restart = true }) {
if (this.entries.length === 0) {
// No events in the period; don't produce a profile
return null
Expand Down Expand Up @@ -243,6 +244,10 @@ class EventsProfiler {
unit: stringTable.dedup(pprofValueUnit)
})

if (!restart) {
this.stop()
}

return new Profile({
sampleType: [timeValueType],
timeNanos: endDate.getTime() * MS_TO_NS,
Expand Down
8 changes: 6 additions & 2 deletions packages/dd-trace/src/profiling/profilers/space.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,12 @@ class NativeSpaceProfiler {
}
}

profile () {
return this._pprof.heap.profile(undefined, this._mapper, getThreadLabels)
profile ({ restart = true } = {}) {
const profile = this._pprof.heap.profile(undefined, this._mapper, getThreadLabels)
if (!restart) {
this.stop()
}
return profile
}

encode (profile) {
Expand Down
34 changes: 17 additions & 17 deletions packages/dd-trace/src/profiling/profilers/wall.js
Original file line number Diff line number Diff line change
Expand Up @@ -223,18 +223,32 @@ class NativeWallProfiler {

_stop (restart) {
if (!this._started) return

if (this._captureSpanData) {
// update last sample context if needed
this._enter()
this._lastSampleCount = 0
}
const profile = this._pprof.time.stop(restart, this._generateLabels)

if (restart) {
const v8BugDetected = this._pprof.time.v8ProfilerStuckEventLoopDetected()
if (v8BugDetected !== 0) {
this._reportV8bug(v8BugDetected === 1)
}
} else {
if (this._captureSpanData) {
beforeCh.unsubscribe(this._enter)
enterCh.unsubscribe(this._enter)
spanFinishCh.unsubscribe(this._spanFinished)
this._profilerState = undefined
this._lastSpan = undefined
this._lastStartedSpans = undefined
this._lastWebTags = undefined
}
this._started = false
}

return profile
}

Expand Down Expand Up @@ -276,30 +290,16 @@ class NativeWallProfiler {
return labels
}

profile () {
return this._stop(true)
profile ({ restart = true } = {}) {
return this._stop(restart)
}

encode (profile) {
return this._pprof.encode(profile)
}

stop () {
if (!this._started) return

const profile = this._stop(false)
if (this._captureSpanData) {
beforeCh.unsubscribe(this._enter)
enterCh.unsubscribe(this._enter)
spanFinishCh.unsubscribe(this._spanFinished)
this._profilerState = undefined
this._lastSpan = undefined
this._lastStartedSpans = undefined
this._lastWebTags = undefined
}

this._started = false
return profile
this._stop(false)
}
}

Expand Down

0 comments on commit d250c7e

Please sign in to comment.