Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PROF-9250: Enable timeline and CPU profiling by default #4149

Merged
merged 6 commits into from
Apr 8, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion integration-tests/profiler/profiler.spec.js
Original file line number Diff line number Diff line change
@@ -15,8 +15,13 @@ const zlib = require('zlib')
const { Profile } = require('pprof-format')
const semver = require('semver')

const DEFAULT_PROFILE_TYPES = ['wall', 'space']
if (process.platform !== 'win32') {
DEFAULT_PROFILE_TYPES.push('events')
}

function checkProfiles (agent, proc, timeout,
expectedProfileTypes = ['wall', 'space'], expectBadExit = false, multiplicity = 1) {
expectedProfileTypes = DEFAULT_PROFILE_TYPES, expectBadExit = false, multiplicity = 1) {
const fileNames = expectedProfileTypes.map(type => `${type}.pprof`)
const resultPromise = agent.assertMessageReceived(({ headers, payload, files }) => {
assert.propertyVal(headers, 'host', `127.0.0.1:${agent.port}`)
12 changes: 8 additions & 4 deletions packages/dd-trace/src/profiling/config.js
Original file line number Diff line number Diff line change
@@ -21,6 +21,7 @@ class Config {
DD_AGENT_HOST,
DD_ENV,
DD_PROFILING_CODEHOTSPOTS_ENABLED,
DD_PROFILING_CPU_ENABLED,
DD_PROFILING_DEBUG_SOURCE_MAPS,
DD_PROFILING_ENABLED,
DD_PROFILING_ENDPOINT_COLLECTION_ENABLED,
@@ -165,7 +166,7 @@ class Config {

this.timelineEnabled = isTrue(coalesce(options.timelineEnabled,
DD_PROFILING_TIMELINE_ENABLED,
DD_PROFILING_EXPERIMENTAL_TIMELINE_ENABLED, false))
DD_PROFILING_EXPERIMENTAL_TIMELINE_ENABLED, samplingContextsAvailable))
logExperimentalVarDeprecation('TIMELINE_ENABLED')
checkOptionWithSamplingContextAllowed(this.timelineEnabled, 'Timeline view')

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

this.cpuProfilingEnabled = isTrue(coalesce(options.cpuProfilingEnabled,
DD_PROFILING_EXPERIMENTAL_CPU_ENABLED, false))
DD_PROFILING_CPU_ENABLED,
DD_PROFILING_EXPERIMENTAL_CPU_ENABLED, samplingContextsAvailable))
logExperimentalVarDeprecation('CPU_ENABLED')
checkOptionWithSamplingContextAllowed(this.cpuProfilingEnabled, 'CPU profiling')

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

// Events profiler is a profiler for timeline events
if (options.timelineEnabled) {
// Events profiler is a profiler that produces timeline events. It is only
// added if timeline is enabled and there's a wall profiler.
if (options.timelineEnabled && profilers.some(p => p instanceof WallProfiler)) {
profilers.push(new EventsProfiler(options))
}

24 changes: 18 additions & 6 deletions packages/dd-trace/test/profiling/config.spec.js
Original file line number Diff line number Diff line change
@@ -9,6 +9,7 @@ const { AgentExporter } = require('../../src/profiling/exporters/agent')
const { FileExporter } = require('../../src/profiling/exporters/file')
const WallProfiler = require('../../src/profiling/profilers/wall')
const SpaceProfiler = require('../../src/profiling/profilers/space')
const EventsProfiler = require('../../src/profiling/profilers/events')
const { ConsoleLogger } = require('../../src/profiling/loggers/console')

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

it('should support configuration options', () => {
@@ -86,10 +87,13 @@ describe('config', () => {
expect(config.exporters[0]._url.toString()).to.equal(options.url)
expect(config.exporters[1]).to.be.an.instanceof(FileExporter)
expect(config.profilers).to.be.an('array')
expect(config.profilers.length).to.equal(2)
expect(config.profilers.length).to.equal(2 + samplingContextsAvailable)
expect(config.profilers[0]).to.be.an.instanceOf(SpaceProfiler)
expect(config.profilers[1]).to.be.an.instanceOf(WallProfiler)
expect(config.profilers[1].codeHotspotsEnabled()).false
if (samplingContextsAvailable) {
expect(config.profilers[2]).to.be.an.instanceOf(EventsProfiler)
}
})

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

expect(config.profilers).to.be.an('array')
expect(config.profilers.length).to.equal(1)
expect(config.profilers.length).to.equal(1 + samplingContextsAvailable)
expect(config.profilers[0]).to.be.an.instanceOf(WallProfiler)
expect(config.profilers[0].codeHotspotsEnabled()).to.equal(samplingContextsAvailable)
if (samplingContextsAvailable) {
expect(config.profilers[1]).to.be.an.instanceOf(EventsProfiler)
}
expect(config.v8ProfilerBugWorkaroundEnabled).false
expect(config.cpuProfilingEnabled).to.equal(samplingContextsAvailable)
})
@@ -181,8 +188,11 @@ describe('config', () => {
const config = new Config(options)

expect(config.profilers).to.be.an('array')
expect(config.profilers.length).to.equal(1)
expect(config.profilers.length).to.equal(1 + samplingContextsAvailable)
expect(config.profilers[0]).to.be.an.instanceOf(WallProfiler)
if (samplingContextsAvailable) {
expect(config.profilers[1]).to.be.an.instanceOf(EventsProfiler)
}
})

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

expect(config.profilers).to.be.an('array')
expect(config.profilers.length).to.equal(1)
expect(config.profilers.length).to.equal(2)
expect(config.profilers[0]).to.be.an.instanceOf(WallProfiler)
expect(config.profilers[0].codeHotspotsEnabled()).false
expect(config.profilers[0].endpointCollectionEnabled()).false
expect(config.profilers[1]).to.be.an.instanceOf(EventsProfiler)
})

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

expect(config.profilers).to.be.an('array')
expect(config.profilers.length).to.equal(1)
expect(config.profilers.length).to.equal(2)
expect(config.profilers[0]).to.be.an.instanceOf(WallProfiler)
expect(config.profilers[0].codeHotspotsEnabled()).false
expect(config.profilers[0].endpointCollectionEnabled()).false
expect(config.profilers[1]).to.be.an.instanceOf(EventsProfiler)
})

function optionOnlyWorksWithGivenCondition (property, name, condition) {
15 changes: 9 additions & 6 deletions packages/dd-trace/test/profiling/profiler.spec.js
Original file line number Diff line number Diff line change
@@ -7,6 +7,9 @@ const sinon = require('sinon')

const SpaceProfiler = require('../../src/profiling/profilers/space')
const WallProfiler = require('../../src/profiling/profilers/wall')
const EventsProfiler = require('../../src/profiling/profilers/events')

const samplingContextsAvailable = process.platform !== 'win32'

describe('profiler', function () {
let Profiler
@@ -137,12 +140,12 @@ describe('profiler', function () {
it('should allow configuring profilers by string or string arrays', async () => {
const checks = [
['space', SpaceProfiler],
['wall', WallProfiler],
['space,wall', SpaceProfiler, WallProfiler],
['wall,space', WallProfiler, SpaceProfiler],
[['space', 'wall'], SpaceProfiler, WallProfiler],
[['wall', 'space'], WallProfiler, SpaceProfiler]
]
['wall', WallProfiler, EventsProfiler],
['space,wall', SpaceProfiler, WallProfiler, EventsProfiler],
['wall,space', WallProfiler, SpaceProfiler, EventsProfiler],
[['space', 'wall'], SpaceProfiler, WallProfiler, EventsProfiler],
[['wall', 'space'], WallProfiler, SpaceProfiler, EventsProfiler]
].map(profilers => profilers.filter(profiler => samplingContextsAvailable || profiler !== EventsProfiler))

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