diff --git a/packages/dd-trace/src/profiling/profiler.js b/packages/dd-trace/src/profiling/profiler.js index 50b6fa13c53..3e6c5d7f618 100644 --- a/packages/dd-trace/src/profiling/profiler.js +++ b/packages/dd-trace/src/profiling/profiler.js @@ -146,6 +146,10 @@ class Profiler extends EventEmitter { const encodedProfiles = {} try { + if (Object.keys(this._config.profilers).length === 0) { + throw new Error('No profile types configured.') + } + // collect profiles synchronously so that profilers can be safely stopped asynchronously for (const profiler of this._config.profilers) { const profile = profiler.profile(restart, startDate, endDate) @@ -156,23 +160,32 @@ class Profiler extends EventEmitter { profiles.push({ profiler, profile }) } + if (restart) { + this._capture(this._timeoutInterval, endDate) + } + // encode and export asynchronously for (const { profiler, profile } of profiles) { - encodedProfiles[profiler.type] = await profiler.encode(profile) - this._logger.debug(() => { - const profileJson = JSON.stringify(profile, (key, value) => { - return typeof value === 'bigint' ? value.toString() : value + try { + encodedProfiles[profiler.type] = await profiler.encode(profile) + this._logger.debug(() => { + const profileJson = JSON.stringify(profile, (key, value) => { + return typeof value === 'bigint' ? value.toString() : value + }) + return `Collected ${profiler.type} profile: ` + profileJson }) - return `Collected ${profiler.type} profile: ` + profileJson - }) + } catch (err) { + // If encoding one of the profile types fails, we should still try to + // encode and submit the other profile types. + this._logError(err) + } } - if (restart) { - this._capture(this._timeoutInterval, endDate) + if (Object.keys(encodedProfiles).length > 0) { + await this._submit(encodedProfiles, startDate, endDate, snapshotKind) + profileSubmittedChannel.publish() + this._logger.debug('Submitted profiles') } - await this._submit(encodedProfiles, startDate, endDate, snapshotKind) - profileSubmittedChannel.publish() - this._logger.debug('Submitted profiles') } catch (err) { this._logError(err) this._stop() @@ -180,9 +193,6 @@ class Profiler extends EventEmitter { } _submit (profiles, start, end, snapshotKind) { - if (!Object.keys(profiles).length) { - return Promise.reject(new Error('No profiles to submit')) - } const { tags } = this._config const tasks = [] diff --git a/packages/dd-trace/src/profiling/profilers/events.js b/packages/dd-trace/src/profiling/profilers/events.js index e1d42484f13..f8f43b06a9a 100644 --- a/packages/dd-trace/src/profiling/profilers/events.js +++ b/packages/dd-trace/src/profiling/profilers/events.js @@ -330,13 +330,13 @@ class EventsProfiler { if (!restart) { this.stop() } - const profile = this.eventSerializer.createProfile(startDate, endDate) + const thatEventSerializer = this.eventSerializer this.eventSerializer = new EventSerializer() - return profile + return () => thatEventSerializer.createProfile(startDate, endDate) } encode (profile) { - return pprof.encode(profile) + return pprof.encode(profile()) } } diff --git a/packages/dd-trace/test/profiling/profiler.spec.js b/packages/dd-trace/test/profiling/profiler.spec.js index dc94061ff1f..d99eb6135ea 100644 --- a/packages/dd-trace/test/profiling/profiler.spec.js +++ b/packages/dd-trace/test/profiling/profiler.spec.js @@ -181,6 +181,21 @@ describe('profiler', function () { }) it('should stop when capturing failed', async () => { + wallProfiler.profile.throws(new Error('boom')) + + await profiler._start({ profilers, exporters, logger }) + + clock.tick(interval) + + sinon.assert.calledOnce(wallProfiler.stop) + sinon.assert.calledOnce(spaceProfiler.stop) + sinon.assert.calledOnce(consoleLogger.error) + sinon.assert.notCalled(wallProfiler.encode) + sinon.assert.notCalled(spaceProfiler.encode) + sinon.assert.notCalled(exporter.export) + }) + + it('should not stop when encoding failed', async () => { const rejected = Promise.reject(new Error('boom')) wallProfiler.encode.returns(rejected) @@ -190,9 +205,25 @@ describe('profiler', function () { await rejected.catch(() => {}) - sinon.assert.calledOnce(wallProfiler.stop) - sinon.assert.calledOnce(spaceProfiler.stop) + sinon.assert.notCalled(wallProfiler.stop) + sinon.assert.notCalled(spaceProfiler.stop) sinon.assert.calledOnce(consoleLogger.error) + sinon.assert.calledOnce(exporter.export) + }) + + it('should not stop when exporting failed', async () => { + const rejected = Promise.reject(new Error('boom')) + exporter.export.returns(rejected) + + await profiler._start({ profilers, exporters, logger }) + + clock.tick(interval) + + await rejected.catch(() => {}) + + sinon.assert.notCalled(wallProfiler.stop) + sinon.assert.notCalled(spaceProfiler.stop) + sinon.assert.calledOnce(exporter.export) }) it('should flush when the interval is reached', async () => { @@ -270,17 +301,6 @@ describe('profiler', function () { sinon.assert.calledWithMatch(submit, 'Submitted profiles') }) - it('should skip submit with no profiles', async () => { - const start = new Date() - const end = new Date() - try { - await profiler._submit({}, start, end) - throw new Error('should have got exception from _submit') - } catch (err) { - expect(err.message).to.equal('No profiles to submit') - } - }) - it('should have a new start time for each capture', async () => { await profiler._start({ profilers, exporters })