Skip to content

Commit 64da0b0

Browse files
szegedijuan-fernandez
authored andcommitted
PROF-9250: Enable timeline and CPU profiling by default (#4149)
* Add non-experimental DD_PROFILING_CPU_ENABLED * Turn timelines and CPU profile on by default on non-Windows platforms
1 parent ff5b16f commit 64da0b0

File tree

4 files changed

+41
-17
lines changed

4 files changed

+41
-17
lines changed

integration-tests/profiler/profiler.spec.js

+6-1
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,13 @@ const zlib = require('zlib')
1515
const { Profile } = require('pprof-format')
1616
const semver = require('semver')
1717

18+
const DEFAULT_PROFILE_TYPES = ['wall', 'space']
19+
if (process.platform !== 'win32') {
20+
DEFAULT_PROFILE_TYPES.push('events')
21+
}
22+
1823
function checkProfiles (agent, proc, timeout,
19-
expectedProfileTypes = ['wall', 'space'], expectBadExit = false, multiplicity = 1) {
24+
expectedProfileTypes = DEFAULT_PROFILE_TYPES, expectBadExit = false, multiplicity = 1) {
2025
const fileNames = expectedProfileTypes.map(type => `${type}.pprof`)
2126
const resultPromise = agent.assertMessageReceived(({ headers, payload, files }) => {
2227
assert.propertyVal(headers, 'host', `127.0.0.1:${agent.port}`)

packages/dd-trace/src/profiling/config.js

+8-4
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ class Config {
2121
DD_AGENT_HOST,
2222
DD_ENV,
2323
DD_PROFILING_CODEHOTSPOTS_ENABLED,
24+
DD_PROFILING_CPU_ENABLED,
2425
DD_PROFILING_DEBUG_SOURCE_MAPS,
2526
DD_PROFILING_ENABLED,
2627
DD_PROFILING_ENDPOINT_COLLECTION_ENABLED,
@@ -165,7 +166,7 @@ class Config {
165166

166167
this.timelineEnabled = isTrue(coalesce(options.timelineEnabled,
167168
DD_PROFILING_TIMELINE_ENABLED,
168-
DD_PROFILING_EXPERIMENTAL_TIMELINE_ENABLED, false))
169+
DD_PROFILING_EXPERIMENTAL_TIMELINE_ENABLED, samplingContextsAvailable))
169170
logExperimentalVarDeprecation('TIMELINE_ENABLED')
170171
checkOptionWithSamplingContextAllowed(this.timelineEnabled, 'Timeline view')
171172

@@ -176,7 +177,9 @@ class Config {
176177
checkOptionWithSamplingContextAllowed(this.codeHotspotsEnabled, 'Code hotspots')
177178

178179
this.cpuProfilingEnabled = isTrue(coalesce(options.cpuProfilingEnabled,
179-
DD_PROFILING_EXPERIMENTAL_CPU_ENABLED, false))
180+
DD_PROFILING_CPU_ENABLED,
181+
DD_PROFILING_EXPERIMENTAL_CPU_ENABLED, samplingContextsAvailable))
182+
logExperimentalVarDeprecation('CPU_ENABLED')
180183
checkOptionWithSamplingContextAllowed(this.cpuProfilingEnabled, 'CPU profiling')
181184

182185
this.profilers = ensureProfilers(profilers, this)
@@ -288,8 +291,9 @@ function ensureProfilers (profilers, options) {
288291
}
289292
}
290293

291-
// Events profiler is a profiler for timeline events
292-
if (options.timelineEnabled) {
294+
// Events profiler is a profiler that produces timeline events. It is only
295+
// added if timeline is enabled and there's a wall profiler.
296+
if (options.timelineEnabled && profilers.some(p => p instanceof WallProfiler)) {
293297
profilers.push(new EventsProfiler(options))
294298
}
295299

packages/dd-trace/test/profiling/config.spec.js

+18-6
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ const { AgentExporter } = require('../../src/profiling/exporters/agent')
99
const { FileExporter } = require('../../src/profiling/exporters/file')
1010
const WallProfiler = require('../../src/profiling/profilers/wall')
1111
const SpaceProfiler = require('../../src/profiling/profilers/space')
12+
const EventsProfiler = require('../../src/profiling/profilers/events')
1213
const { ConsoleLogger } = require('../../src/profiling/loggers/console')
1314

1415
const samplingContextsAvailable = process.platform !== 'win32'
@@ -54,7 +55,7 @@ describe('config', () => {
5455
expect(config.profilers[0].codeHotspotsEnabled()).to.equal(samplingContextsAvailable)
5556
expect(config.profilers[1]).to.be.an.instanceof(SpaceProfiler)
5657
expect(config.v8ProfilerBugWorkaroundEnabled).true
57-
expect(config.cpuProfilingEnabled).false
58+
expect(config.cpuProfilingEnabled).to.equal(samplingContextsAvailable)
5859
})
5960

6061
it('should support configuration options', () => {
@@ -86,10 +87,13 @@ describe('config', () => {
8687
expect(config.exporters[0]._url.toString()).to.equal(options.url)
8788
expect(config.exporters[1]).to.be.an.instanceof(FileExporter)
8889
expect(config.profilers).to.be.an('array')
89-
expect(config.profilers.length).to.equal(2)
90+
expect(config.profilers.length).to.equal(2 + samplingContextsAvailable)
9091
expect(config.profilers[0]).to.be.an.instanceOf(SpaceProfiler)
9192
expect(config.profilers[1]).to.be.an.instanceOf(WallProfiler)
9293
expect(config.profilers[1].codeHotspotsEnabled()).false
94+
if (samplingContextsAvailable) {
95+
expect(config.profilers[2]).to.be.an.instanceOf(EventsProfiler)
96+
}
9397
})
9498

9599
it('should filter out invalid profilers', () => {
@@ -145,9 +149,12 @@ describe('config', () => {
145149
const config = new Config(options)
146150

147151
expect(config.profilers).to.be.an('array')
148-
expect(config.profilers.length).to.equal(1)
152+
expect(config.profilers.length).to.equal(1 + samplingContextsAvailable)
149153
expect(config.profilers[0]).to.be.an.instanceOf(WallProfiler)
150154
expect(config.profilers[0].codeHotspotsEnabled()).to.equal(samplingContextsAvailable)
155+
if (samplingContextsAvailable) {
156+
expect(config.profilers[1]).to.be.an.instanceOf(EventsProfiler)
157+
}
151158
expect(config.v8ProfilerBugWorkaroundEnabled).false
152159
expect(config.cpuProfilingEnabled).to.equal(samplingContextsAvailable)
153160
})
@@ -181,8 +188,11 @@ describe('config', () => {
181188
const config = new Config(options)
182189

183190
expect(config.profilers).to.be.an('array')
184-
expect(config.profilers.length).to.equal(1)
191+
expect(config.profilers.length).to.equal(1 + samplingContextsAvailable)
185192
expect(config.profilers[0]).to.be.an.instanceOf(WallProfiler)
193+
if (samplingContextsAvailable) {
194+
expect(config.profilers[1]).to.be.an.instanceOf(EventsProfiler)
195+
}
186196
})
187197

188198
it('should prioritize options over env variables', () => {
@@ -204,10 +214,11 @@ describe('config', () => {
204214
const config = new Config(options)
205215

206216
expect(config.profilers).to.be.an('array')
207-
expect(config.profilers.length).to.equal(1)
217+
expect(config.profilers.length).to.equal(2)
208218
expect(config.profilers[0]).to.be.an.instanceOf(WallProfiler)
209219
expect(config.profilers[0].codeHotspotsEnabled()).false
210220
expect(config.profilers[0].endpointCollectionEnabled()).false
221+
expect(config.profilers[1]).to.be.an.instanceOf(EventsProfiler)
211222
})
212223

213224
it('should prioritize non-experimental env variables and warn about experimental ones', () => {
@@ -245,10 +256,11 @@ describe('config', () => {
245256
'Use DD_PROFILING_CODEHOTSPOTS_ENABLED instead.')
246257

247258
expect(config.profilers).to.be.an('array')
248-
expect(config.profilers.length).to.equal(1)
259+
expect(config.profilers.length).to.equal(2)
249260
expect(config.profilers[0]).to.be.an.instanceOf(WallProfiler)
250261
expect(config.profilers[0].codeHotspotsEnabled()).false
251262
expect(config.profilers[0].endpointCollectionEnabled()).false
263+
expect(config.profilers[1]).to.be.an.instanceOf(EventsProfiler)
252264
})
253265

254266
function optionOnlyWorksWithGivenCondition (property, name, condition) {

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

+9-6
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,9 @@ const sinon = require('sinon')
77

88
const SpaceProfiler = require('../../src/profiling/profilers/space')
99
const WallProfiler = require('../../src/profiling/profilers/wall')
10+
const EventsProfiler = require('../../src/profiling/profilers/events')
11+
12+
const samplingContextsAvailable = process.platform !== 'win32'
1013

1114
describe('profiler', function () {
1215
let Profiler
@@ -137,12 +140,12 @@ describe('profiler', function () {
137140
it('should allow configuring profilers by string or string arrays', async () => {
138141
const checks = [
139142
['space', SpaceProfiler],
140-
['wall', WallProfiler],
141-
['space,wall', SpaceProfiler, WallProfiler],
142-
['wall,space', WallProfiler, SpaceProfiler],
143-
[['space', 'wall'], SpaceProfiler, WallProfiler],
144-
[['wall', 'space'], WallProfiler, SpaceProfiler]
145-
]
143+
['wall', WallProfiler, EventsProfiler],
144+
['space,wall', SpaceProfiler, WallProfiler, EventsProfiler],
145+
['wall,space', WallProfiler, SpaceProfiler, EventsProfiler],
146+
[['space', 'wall'], SpaceProfiler, WallProfiler, EventsProfiler],
147+
[['wall', 'space'], WallProfiler, SpaceProfiler, EventsProfiler]
148+
].map(profilers => profilers.filter(profiler => samplingContextsAvailable || profiler !== EventsProfiler))
146149

147150
for (const [profilers, ...expected] of checks) {
148151
await profiler._start({

0 commit comments

Comments
 (0)