From 4df8c4daa7201dde7405aa03b4d64a7f49928223 Mon Sep 17 00:00:00 2001 From: Attila Szegedi Date: Wed, 14 Aug 2024 09:30:46 +0200 Subject: [PATCH] PROF-10320: Separate SSI telemetry and heuristics activation (#4592) * Telemetry emission and heuristics are now independent. * profiling.enabled is four-state now (true, false, auto, undefined) * Adds injectionEnabled config * Removes profiling.{ssi, heuristicsEnabled} config. * Move derived telemetry.logCollection value computation to _applyCalculated() * Tidy _merge() --- packages/dd-trace/src/config.js | 62 +++---- packages/dd-trace/src/profiler.js | 4 +- packages/dd-trace/src/profiling/config.js | 4 - packages/dd-trace/src/profiling/profiler.js | 1 - .../dd-trace/src/profiling/ssi-heuristics.js | 107 ++++++------ packages/dd-trace/src/proxy.js | 36 ++-- packages/dd-trace/test/config.spec.js | 10 +- .../dd-trace/test/profiling/config.spec.js | 3 - .../dd-trace/test/profiling/profiler.spec.js | 7 - .../test/profiling/ssi-heuristics.spec.js | 162 ++++++++++++------ packages/dd-trace/test/proxy.spec.js | 5 +- 11 files changed, 223 insertions(+), 178 deletions(-) diff --git a/packages/dd-trace/src/config.js b/packages/dd-trace/src/config.js index 1c982ef9da..6ff1fdcbe1 100644 --- a/packages/dd-trace/src/config.js +++ b/packages/dd-trace/src/config.js @@ -441,6 +441,7 @@ class Config { this._setValue(defaults, 'iast.redactionValuePattern', null) this._setValue(defaults, 'iast.requestSampling', 30) this._setValue(defaults, 'iast.telemetryVerbosity', 'INFORMATION') + this._setValue(defaults, 'injectionEnabled', []) this._setValue(defaults, 'isAzureFunction', false) this._setValue(defaults, 'isCiVisibility', false) this._setValue(defaults, 'isEarlyFlakeDetectionEnabled', false) @@ -459,8 +460,6 @@ class Config { this._setValue(defaults, 'profiling.enabled', undefined) this._setValue(defaults, 'profiling.exporters', 'agent') this._setValue(defaults, 'profiling.sourceMap', true) - this._setValue(defaults, 'profiling.ssi', false) - this._setValue(defaults, 'profiling.heuristicsEnabled', false) this._setValue(defaults, 'profiling.longLivedThreshold', undefined) this._setValue(defaults, 'protocolVersion', '0.4') this._setValue(defaults, 'queryStringObfuscation', qsRegex) @@ -681,6 +680,7 @@ class Config { } this._envUnprocessed['iast.requestSampling'] = DD_IAST_REQUEST_SAMPLING this._setString(env, 'iast.telemetryVerbosity', DD_IAST_TELEMETRY_VERBOSITY) + this._setArray(env, 'injectionEnabled', DD_INJECTION_ENABLED) this._setBoolean(env, 'isAzureFunction', getIsAzureFunction()) this._setBoolean(env, 'isGCPFunction', getIsGCPFunction()) this._setBoolean(env, 'logInjection', DD_LOGS_INJECTION) @@ -696,18 +696,18 @@ class Config { this._envUnprocessed.peerServiceMapping = DD_TRACE_PEER_SERVICE_MAPPING } this._setString(env, 'port', DD_TRACE_AGENT_PORT) - this._setBoolean(env, 'profiling.enabled', coalesce(DD_EXPERIMENTAL_PROFILING_ENABLED, DD_PROFILING_ENABLED)) + const profilingEnabledEnv = coalesce(DD_EXPERIMENTAL_PROFILING_ENABLED, DD_PROFILING_ENABLED) + const profilingEnabled = isTrue(profilingEnabledEnv) + ? 'true' + : isFalse(profilingEnabledEnv) + ? 'false' + : profilingEnabledEnv === 'auto' ? 'auto' : undefined + this._setString(env, 'profiling.enabled', profilingEnabled) this._setString(env, 'profiling.exporters', DD_PROFILING_EXPORTERS) this._setBoolean(env, 'profiling.sourceMap', DD_PROFILING_SOURCE_MAP && !isFalse(DD_PROFILING_SOURCE_MAP)) - if (DD_PROFILING_ENABLED === 'auto' || DD_INJECTION_ENABLED) { - this._setBoolean(env, 'profiling.ssi', true) - if (DD_PROFILING_ENABLED === 'auto' || DD_INJECTION_ENABLED.split(',').includes('profiler')) { - this._setBoolean(env, 'profiling.heuristicsEnabled', true) - } - if (DD_INTERNAL_PROFILING_LONG_LIVED_THRESHOLD) { - // This is only used in testing to not have to wait 30s - this._setValue(env, 'profiling.longLivedThreshold', Number(DD_INTERNAL_PROFILING_LONG_LIVED_THRESHOLD)) - } + if (DD_INTERNAL_PROFILING_LONG_LIVED_THRESHOLD) { + // This is only used in testing to not have to wait 30s + this._setValue(env, 'profiling.longLivedThreshold', Number(DD_INTERNAL_PROFILING_LONG_LIVED_THRESHOLD)) } this._setString(env, 'protocolVersion', DD_TRACE_AGENT_PROTOCOL_VERSION) @@ -762,12 +762,7 @@ class Config { this._setBoolean(env, 'telemetry.dependencyCollection', DD_TELEMETRY_DEPENDENCY_COLLECTION_ENABLED) this._setValue(env, 'telemetry.heartbeatInterval', maybeInt(Math.floor(DD_TELEMETRY_HEARTBEAT_INTERVAL * 1000))) this._envUnprocessed['telemetry.heartbeatInterval'] = DD_TELEMETRY_HEARTBEAT_INTERVAL * 1000 - const hasTelemetryLogsUsingFeatures = - env['iast.enabled'] || env['profiling.enabled'] || env['profiling.heuristicsEnabled'] - ? true - : undefined - this._setBoolean(env, 'telemetry.logCollection', coalesce(DD_TELEMETRY_LOG_COLLECTION_ENABLED, - hasTelemetryLogsUsingFeatures)) + this._setBoolean(env, 'telemetry.logCollection', DD_TELEMETRY_LOG_COLLECTION_ENABLED) this._setBoolean(env, 'telemetry.metrics', DD_TELEMETRY_METRICS_ENABLED) this._setBoolean(env, 'traceId128BitGenerationEnabled', DD_TRACE_128_BIT_TRACEID_GENERATION_ENABLED) this._setBoolean(env, 'traceId128BitLoggingEnabled', DD_TRACE_128_BIT_TRACEID_LOGGING_ENABLED) @@ -862,7 +857,10 @@ class Config { this._setValue(opts, 'peerServiceMapping', options.peerServiceMapping) this._setBoolean(opts, 'plugins', options.plugins) this._setString(opts, 'port', options.port) - this._setBoolean(opts, 'profiling.enabled', options.profiling) + const strProfiling = String(options.profiling) + if (['true', 'false', 'auto'].includes(strProfiling)) { + this._setString(opts, 'profiling.enabled', strProfiling) + } this._setString(opts, 'protocolVersion', options.protocolVersion) if (options.remoteConfig) { this._setValue(opts, 'remoteConfig.pollInterval', maybeFloat(options.remoteConfig.pollInterval)) @@ -885,10 +883,6 @@ class Config { this._setBoolean(opts, 'spanRemoveIntegrationFromService', options.spanRemoveIntegrationFromService) this._setBoolean(opts, 'startupLogs', options.startupLogs) this._setTags(opts, 'tags', tags) - const hasTelemetryLogsUsingFeatures = - (options.iast && (options.iast === true || options.iast?.enabled === true)) || - (options.profiling && options.profiling === true) - this._setBoolean(opts, 'telemetry.logCollection', hasTelemetryLogsUsingFeatures) this._setBoolean(opts, 'traceId128BitGenerationEnabled', options.traceId128BitGenerationEnabled) this._setBoolean(opts, 'traceId128BitLoggingEnabled', options.traceId128BitLoggingEnabled) this._setString(opts, 'version', options.version || tags.version) @@ -1019,6 +1013,13 @@ class Config { calc['tracePropagationStyle.inject'] = calc['tracePropagationStyle.inject'] || defaultPropagationStyle calc['tracePropagationStyle.extract'] = calc['tracePropagationStyle.extract'] || defaultPropagationStyle } + + const iastEnabled = coalesce(this._options['iast.enabled'], this._env['iast.enabled']) + const profilingEnabled = coalesce(this._options['profiling.enabled'], this._env['profiling.enabled']) + const injectionIncludesProfiler = (this._env.injectionEnabled || []).includes('profiler') + if (iastEnabled || ['auto', 'true'].includes(profilingEnabled) || injectionIncludesProfiler) { + this._setBoolean(calc, 'telemetry.logCollection', true) + } } _applyRemote (options) { @@ -1143,17 +1144,18 @@ class Config { for (const name in this._defaults) { for (let i = 0; i < containers.length; i++) { const container = containers[i] - const origin = origins[i] - const unprocessed = unprocessedValues[i] + const value = container[name] - if ((container[name] !== null && container[name] !== undefined) || container === this._defaults) { - if (get(this, name) === container[name] && has(this, name)) break + if ((value !== null && value !== undefined) || container === this._defaults) { + if (get(this, name) === value && has(this, name)) break - let value = container[name] set(this, name, value) - value = unprocessed[name] || value - changes.push({ name, value, origin }) + changes.push({ + name, + value: unprocessedValues[i][name] || value, + origin: origins[i] + }) break } diff --git a/packages/dd-trace/src/profiler.js b/packages/dd-trace/src/profiler.js index ce08a3d0f6..65c0d00709 100644 --- a/packages/dd-trace/src/profiler.js +++ b/packages/dd-trace/src/profiler.js @@ -9,7 +9,7 @@ process.once('beforeExit', () => { profiler.stop() }) module.exports = { start: config => { const { service, version, env, url, hostname, port, tags, repositoryUrl, commitSHA } = config - const { enabled, sourceMap, exporters, heuristicsEnabled } = config.profiling + const { sourceMap, exporters } = config.profiling const logger = { debug: (message) => log.debug(message), info: (message) => log.info(message), @@ -18,8 +18,6 @@ module.exports = { } return profiler.start({ - enabled, - heuristicsEnabled, service, version, env, diff --git a/packages/dd-trace/src/profiling/config.js b/packages/dd-trace/src/profiling/config.js index 18b85f7c44..f866a925ff 100644 --- a/packages/dd-trace/src/profiling/config.js +++ b/packages/dd-trace/src/profiling/config.js @@ -23,7 +23,6 @@ class Config { DD_PROFILING_CODEHOTSPOTS_ENABLED, DD_PROFILING_CPU_ENABLED, DD_PROFILING_DEBUG_SOURCE_MAPS, - DD_PROFILING_ENABLED, DD_PROFILING_ENDPOINT_COLLECTION_ENABLED, DD_PROFILING_EXPERIMENTAL_CODEHOTSPOTS_ENABLED, DD_PROFILING_EXPERIMENTAL_CPU_ENABLED, @@ -49,7 +48,6 @@ class Config { DD_VERSION } = process.env - const enabled = isTrue(coalesce(options.enabled, DD_PROFILING_ENABLED, true)) const env = coalesce(options.env, DD_ENV) const service = options.service || DD_SERVICE || 'node' const host = os.hostname() @@ -64,8 +62,6 @@ class Config { const pprofPrefix = coalesce(options.pprofPrefix, DD_PROFILING_PPROF_PREFIX, '') - this.enabled = enabled - this.heuristicsEnabled = options.heuristicsEnabled this.service = service this.env = env this.host = host diff --git a/packages/dd-trace/src/profiling/profiler.js b/packages/dd-trace/src/profiling/profiler.js index 773e1cafc8..50b6fa13c5 100644 --- a/packages/dd-trace/src/profiling/profiler.js +++ b/packages/dd-trace/src/profiling/profiler.js @@ -47,7 +47,6 @@ class Profiler extends EventEmitter { if (this._enabled) return true const config = this._config = new Config(options) - if (!config.enabled && !config.heuristicsEnabled) return false this._logger = config.logger this._enabled = true diff --git a/packages/dd-trace/src/profiling/ssi-heuristics.js b/packages/dd-trace/src/profiling/ssi-heuristics.js index f4e10ea462..9910b9273b 100644 --- a/packages/dd-trace/src/profiling/ssi-heuristics.js +++ b/packages/dd-trace/src/profiling/ssi-heuristics.js @@ -7,40 +7,6 @@ const dc = require('dc-polyfill') // If the process lives for at least 30 seconds, it's considered long-lived const DEFAULT_LONG_LIVED_THRESHOLD = 30000 -const EnablementChoice = { - MANUALLY_ENABLED: Symbol('SSITelemetry.EnablementChoice.MANUALLY_ENABLED'), - SSI_ENABLED: Symbol('SSITelemetry.EnablementChoice.SSI_ENABLED'), - SSI_NOT_ENABLED: Symbol('SSITelemetry.EnablementChoice.SSI_NOT_ENABLED'), - DISABLED: Symbol('SSITelemetry.EnablementChoice.DISABLED') -} -Object.freeze(EnablementChoice) - -function getEnablementChoiceFromConfig (config) { - if (config.ssi === false || config.enabled === false) { - return EnablementChoice.DISABLED - } else if (config.heuristicsEnabled === true) { - return EnablementChoice.SSI_ENABLED - } else if (config.enabled === true) { - return EnablementChoice.MANUALLY_ENABLED - } else { - return EnablementChoice.SSI_NOT_ENABLED - } -} - -function enablementChoiceToTagValue (enablementChoice) { - switch (enablementChoice) { - case EnablementChoice.MANUALLY_ENABLED: - return 'manually_enabled' - case EnablementChoice.SSI_ENABLED: - return 'ssi_enabled' - case EnablementChoice.SSI_NOT_ENABLED: - return 'not_enabled' - case EnablementChoice.DISABLED: - // Can't emit this one as a tag - throw new Error('Invalid enablement choice') - } -} - /** * This class embodies the SSI profiler-triggering heuristics and also emits telemetry metrics about * the profiler behavior under SSI. It emits the following metrics: @@ -56,9 +22,23 @@ function enablementChoiceToTagValue (enablementChoice) { */ class SSIHeuristics { constructor (config) { - this.enablementChoice = getEnablementChoiceFromConfig(config) + const injectionIncludesProfiler = config.injectionEnabled.includes('profiler') + this._heuristicsActive = injectionIncludesProfiler || config.profiling.enabled === 'auto' + this._emitsTelemetry = config.injectionEnabled.length > 0 && config.profiling.enabled !== 'false' + + if (this._emitsTelemetry) { + if (config.profiling.enabled === 'true') { + this.enablementChoice = 'manually_enabled' + } else if (injectionIncludesProfiler) { + this.enablementChoice = 'ssi_enabled' + } else if (config.profiling.enabled === 'auto') { + this.enablementChoice = 'auto_enabled' + } else { + this.enablementChoice = 'ssi_not_enabled' + } + } - const longLivedThreshold = config.longLivedThreshold || DEFAULT_LONG_LIVED_THRESHOLD + const longLivedThreshold = config.profiling.longLivedThreshold || DEFAULT_LONG_LIVED_THRESHOLD if (typeof longLivedThreshold !== 'number' || longLivedThreshold <= 0) { throw new Error('Long-lived threshold must be a positive number') } @@ -69,12 +49,16 @@ class SSIHeuristics { this.shortLived = true } - enabled () { - return this.enablementChoice !== EnablementChoice.DISABLED + get emitsTelemetry () { + return this._emitsTelemetry + } + + get heuristicsActive () { + return this._heuristicsActive } start () { - if (this.enabled()) { + if (this.heuristicsActive || this.emitsTelemetry) { // Used to determine short-livedness of the process. We could use the process start time as the // reference point, but the tracer initialization point is more relevant, as we couldn't be // collecting profiles earlier anyway. The difference is not particularly significant if the @@ -85,13 +69,17 @@ class SSIHeuristics { }, this.longLivedThreshold).unref() this._onSpanCreated = this._onSpanCreated.bind(this) - this._onProfileSubmitted = this._onProfileSubmitted.bind(this) - this._onMockProfileSubmitted = this._onMockProfileSubmitted.bind(this) - this._onAppClosing = this._onAppClosing.bind(this) - dc.subscribe('dd-trace:span:start', this._onSpanCreated) - dc.subscribe('datadog:profiling:profile-submitted', this._onProfileSubmitted) - dc.subscribe('datadog:profiling:mock-profile-submitted', this._onMockProfileSubmitted) + + if (this.emitsTelemetry) { + this._onProfileSubmitted = this._onProfileSubmitted.bind(this) + this._onMockProfileSubmitted = this._onMockProfileSubmitted.bind(this) + + dc.subscribe('datadog:profiling:profile-submitted', this._onProfileSubmitted) + dc.subscribe('datadog:profiling:mock-profile-submitted', this._onMockProfileSubmitted) + } + + this._onAppClosing = this._onAppClosing.bind(this) dc.subscribe('datadog:telemetry:app-closing', this._onAppClosing) } } @@ -152,7 +140,7 @@ class SSIHeuristics { const tags = [ 'installation:ssi', - `enablement_choice:${enablementChoiceToTagValue(this.enablementChoice)}`, + `enablement_choice:${this.enablementChoice}`, `has_sent_profiles:${this.hasSentProfiles}`, `heuristic_hypothetical_decision:${decision.join('_')}` ] @@ -163,9 +151,9 @@ class SSIHeuristics { if ( !this._emittedRuntimeId && decision[0] === 'triggered' && - // When enablement choice is SSI_ENABLED, hasSentProfiles can transition from false to true when the + // When heuristics are active, hasSentProfiles can transition from false to true when the // profiler gets started and the first profile is submitted, so we have to wait for it. - (this.enablementChoice !== EnablementChoice.SSI_ENABLED || this.hasSentProfiles) + (!this.heuristicsActive || this.hasSentProfiles) ) { // Tags won't change anymore, so we can emit the runtime ID metric now. this._emittedRuntimeId = true @@ -174,17 +162,20 @@ class SSIHeuristics { } _onAppClosing () { - this._ensureProfileMetrics() - // Last ditch effort to emit a runtime ID count metric - if (!this._emittedRuntimeId) { - this._emittedRuntimeId = true - this._runtimeIdCount.inc() + if (this.emitsTelemetry) { + this._ensureProfileMetrics() + // Last ditch effort to emit a runtime ID count metric + if (!this._emittedRuntimeId) { + this._emittedRuntimeId = true + this._runtimeIdCount.inc() + } + // So we have the metrics in the final state + this._profileCount.inc(0) + + dc.unsubscribe('datadog:profiling:profile-submitted', this._onProfileSubmitted) + dc.unsubscribe('datadog:profiling:mock-profile-submitted', this._onMockProfileSubmitted) } - // So we have the metrics in the final state - this._profileCount.inc(0) - dc.unsubscribe('datadog:profiling:profile-submitted', this._onProfileSubmitted) - dc.unsubscribe('datadog:profiling:mock-profile-submitted', this._onMockProfileSubmitted) dc.unsubscribe('datadog:telemetry:app-closing', this._onAppClosing) if (this.noSpan) { dc.unsubscribe('dd-trace:span:start', this._onSpanCreated) @@ -192,4 +183,4 @@ class SSIHeuristics { } } -module.exports = { SSIHeuristics, EnablementChoice } +module.exports = { SSIHeuristics } diff --git a/packages/dd-trace/src/proxy.js b/packages/dd-trace/src/proxy.js index ac374dc8d4..c3b865226e 100644 --- a/packages/dd-trace/src/proxy.js +++ b/packages/dd-trace/src/proxy.js @@ -116,24 +116,32 @@ class Tracer extends NoopProxy { require('./serverless').maybeStartServerlessMiniAgent(config) } - const ssiHeuristics = new SSIHeuristics(config.profiling) - ssiHeuristics.start() - if (config.profiling.enabled) { - this._profilerStarted = this._startProfiler(config) - } else if (config.profiling.ssi) { - const mockProfiler = require('./profiling/ssi-telemetry-mock-profiler') - mockProfiler.start(config) - - if (config.profiling.heuristicsEnabled) { + if (config.profiling.enabled !== 'false') { + const ssiHeuristics = new SSIHeuristics(config) + ssiHeuristics.start() + let mockProfiler = null + if (config.profiling.enabled === 'true') { + this._profilerStarted = this._startProfiler(config) + } else if (ssiHeuristics.emitsTelemetry) { + // Start a mock profiler that emits mock profile-submitted events for the telemetry. + // It will be stopped if the real profiler is started by the heuristics. + mockProfiler = require('./profiling/ssi-telemetry-mock-profiler') + mockProfiler.start(config) + } + + if (ssiHeuristics.heuristicsActive) { ssiHeuristics.onTriggered(() => { - mockProfiler.stop() + if (mockProfiler) { + mockProfiler.stop() + } this._startProfiler(config) - ssiHeuristics.onTriggered() + ssiHeuristics.onTriggered() // deregister this callback }) } - } - if (!this._profilerStarted) { - this._profilerStarted = Promise.resolve(false) + + if (!this._profilerStarted) { + this._profilerStarted = Promise.resolve(false) + } } if (config.runtimeMetrics) { diff --git a/packages/dd-trace/test/config.spec.js b/packages/dd-trace/test/config.spec.js index a06aaf92fb..4034bc2804 100644 --- a/packages/dd-trace/test/config.spec.js +++ b/packages/dd-trace/test/config.spec.js @@ -312,6 +312,7 @@ describe('Config', () => { { name: 'iast.redactionValuePattern', value: null, origin: 'default' }, { name: 'iast.requestSampling', value: 30, origin: 'default' }, { name: 'iast.telemetryVerbosity', value: 'INFORMATION', origin: 'default' }, + { name: 'injectionEnabled', value: [], origin: 'default' }, { name: 'isCiVisibility', value: false, origin: 'default' }, { name: 'isEarlyFlakeDetectionEnabled', value: false, origin: 'default' }, { name: 'isGCPFunction', value: false, origin: 'env_var' }, @@ -327,9 +328,7 @@ describe('Config', () => { { name: 'port', value: '8126', origin: 'default' }, { name: 'profiling.enabled', value: undefined, origin: 'default' }, { name: 'profiling.exporters', value: 'agent', origin: 'default' }, - { name: 'profiling.heuristicsEnabled', value: false, origin: 'default' }, { name: 'profiling.sourceMap', value: true, origin: 'default' }, - { name: 'profiling.ssi', value: false, origin: 'default' }, { name: 'protocolVersion', value: '0.4', origin: 'default' }, { name: 'queryStringObfuscation', @@ -615,12 +614,11 @@ describe('Config', () => { { name: 'iast.requestSampling', value: '40', origin: 'env_var' }, { name: 'iast.telemetryVerbosity', value: 'DEBUG', origin: 'env_var' }, { name: 'instrumentation_config_id', value: 'abcdef123', origin: 'env_var' }, + { name: 'injectionEnabled', value: ['profiler'], origin: 'env_var' }, { name: 'isGCPFunction', value: false, origin: 'env_var' }, { name: 'peerServiceMapping', value: process.env.DD_TRACE_PEER_SERVICE_MAPPING, origin: 'env_var' }, { name: 'port', value: '6218', origin: 'env_var' }, - { name: 'profiling.enabled', value: true, origin: 'env_var' }, - { name: 'profiling.heuristicsEnabled', value: true, origin: 'env_var' }, - { name: 'profiling.ssi', value: true, origin: 'env_var' }, + { name: 'profiling.enabled', value: 'true', origin: 'env_var' }, { name: 'protocolVersion', value: '0.5', origin: 'env_var' }, { name: 'queryStringObfuscation', value: '.*', origin: 'env_var' }, { name: 'remoteConfig.enabled', value: false, origin: 'env_var' }, @@ -638,7 +636,6 @@ describe('Config', () => { { name: 'spanAttributeSchema', value: 'v1', origin: 'env_var' }, { name: 'spanRemoveIntegrationFromService', value: true, origin: 'env_var' }, { name: 'telemetry.enabled', value: true, origin: 'env_var' }, - { name: 'telemetry.logCollection', value: true, origin: 'env_var' }, { name: 'traceId128BitGenerationEnabled', value: true, origin: 'env_var' }, { name: 'traceId128BitLoggingEnabled', value: true, origin: 'env_var' }, { name: 'tracing', value: false, origin: 'env_var' }, @@ -901,7 +898,6 @@ describe('Config', () => { { name: 'spanComputePeerService', value: true, origin: 'calculated' }, { name: 'spanRemoveIntegrationFromService', value: true, origin: 'code' }, { name: 'stats.enabled', value: false, origin: 'calculated' }, - { name: 'telemetry.logCollection', value: true, origin: 'code' }, { name: 'traceId128BitGenerationEnabled', value: true, origin: 'code' }, { name: 'traceId128BitLoggingEnabled', value: true, origin: 'code' }, { name: 'version', value: '0.1.0', origin: 'code' } diff --git a/packages/dd-trace/test/profiling/config.spec.js b/packages/dd-trace/test/profiling/config.spec.js index 6b62de36df..27023e57b0 100644 --- a/packages/dd-trace/test/profiling/config.spec.js +++ b/packages/dd-trace/test/profiling/config.spec.js @@ -39,7 +39,6 @@ describe('config', () => { const config = new Config() expect(config).to.deep.include({ - enabled: true, service: 'node', flushInterval: 65 * 1000 }) @@ -60,7 +59,6 @@ describe('config', () => { it('should support configuration options', () => { const options = { - enabled: false, service: 'test', version: '1.2.3-test.0', logger: nullLogger, @@ -72,7 +70,6 @@ describe('config', () => { const config = new Config(options) - expect(config.enabled).to.equal(options.enabled) expect(config.service).to.equal(options.service) expect(config.host).to.be.a('string') expect(config.version).to.equal(options.version) diff --git a/packages/dd-trace/test/profiling/profiler.spec.js b/packages/dd-trace/test/profiling/profiler.spec.js index 8e3bbf4e68..dc94061ff1 100644 --- a/packages/dd-trace/test/profiling/profiler.spec.js +++ b/packages/dd-trace/test/profiling/profiler.spec.js @@ -232,13 +232,6 @@ describe('profiler', function () { expect(tags).to.have.property('foo', 'foo') }) - it('should not start when disabled', async () => { - await profiler._start({ profilers, exporters, enabled: false }) - - sinon.assert.notCalled(wallProfiler.start) - sinon.assert.notCalled(spaceProfiler.start) - }) - it('should log exporter errors', async () => { exporter.export.rejects(new Error('boom')) diff --git a/packages/dd-trace/test/profiling/ssi-heuristics.spec.js b/packages/dd-trace/test/profiling/ssi-heuristics.spec.js index be46d3714f..81e91d3fe5 100644 --- a/packages/dd-trace/test/profiling/ssi-heuristics.spec.js +++ b/packages/dd-trace/test/profiling/ssi-heuristics.spec.js @@ -11,29 +11,68 @@ telemetryManagerNamespace.returns() const dc = require('dc-polyfill') const Config = require('../../src/config') -describe('SSI Heuristics', () => { - it('should be disabled without SSI even if the profiler is manually enabled', () => { - delete process.env.DD_INJECTION_ENABLED - process.env.DD_PROFILING_ENABLED = 'true' - testDisabledHeuristics() - }) +describe('Profiling for SSI', () => { + describe('When injection is not present', () => { + describe('Neither telemetry nor heuristics should work when', () => { + it('profiler enablement is unspecified', () => { + delete process.env.DD_INJECTION_ENABLED + testInactive('') + }) - it('should be disabled when SSI is present but the profiler is manually disabled', () => { - process.env.DD_INJECTION_ENABLED = 'tracing' - process.env.DD_PROFILING_ENABLED = 'false' - testDisabledHeuristics() - }) + it('the profiler is explicitly enabled', () => { + delete process.env.DD_INJECTION_ENABLED + testInactive('true') + }) + + it('the profiler is explicitly disabled', () => { + delete process.env.DD_INJECTION_ENABLED + testInactive('false') + }) + }) - it('should be enabled when SSI is present', () => { - process.env.DD_INJECTION_ENABLED = 'tracing' - delete process.env.DD_PROFILING_ENABLED - return testEnabledHeuristics('not_enabled') + describe('Only telemetry should work when', () => { + it('the profiler is explicitly auto-enabled', () => { + delete process.env.DD_INJECTION_ENABLED + process.env.DD_PROFILING_ENABLED = 'auto' + return testEnabledHeuristics(undefined, true) + }) + }) }) - it('should be enabled when SSI is present and profiling is manually enabled', () => { - process.env.DD_INJECTION_ENABLED = 'tracing' - process.env.DD_PROFILING_ENABLED = 'true' - return testEnabledHeuristics('manually_enabled') + describe('When injection is present', () => { + describe('Neither telemetry nor heuristics should work when', () => { + it('the profiler is explicitly disabled', () => { + process.env.DD_INJECTION_ENABLED = 'tracer' + return testInactive('false') + }) + }) + + describe('Only telemetry should work when', () => { + it('profiler enablement is unspecified', () => { + process.env.DD_INJECTION_ENABLED = 'tracer' + delete process.env.DD_PROFILING_ENABLED + return testEnabledHeuristics('ssi_not_enabled', false) + }) + + it('the profiler is explicitly enabled', () => { + process.env.DD_INJECTION_ENABLED = 'tracer' + process.env.DD_PROFILING_ENABLED = 'true' + return testEnabledHeuristics('manually_enabled', false) + }) + }) + + describe('Both telemetry and heuristics should work when', () => { + it('the profiler is explicitly auto-enabled', () => { + process.env.DD_INJECTION_ENABLED = 'tracer' + process.env.DD_PROFILING_ENABLED = 'auto' + return testEnabledHeuristics('auto_enabled', true) + }) + + it('\'profiler\' value is in DD_INJECTION_ENABLED', () => { + process.env.DD_INJECTION_ENABLED = 'tracer,service_name,profiler' + return testEnabledHeuristics('ssi_enabled', true) + }) + }) }) }) @@ -55,7 +94,7 @@ function setupHarness () { } const namespaceFn = sinon.stub().returns(ssiMetricsNamespace) - const { SSIHeuristics, EnablementChoice } = proxyquire('../src/profiling/ssi-heuristics', { + const { SSIHeuristics } = proxyquire('../src/profiling/ssi-heuristics', { '../telemetry/metrics': { manager: { namespace: namespaceFn @@ -68,19 +107,23 @@ function setupHarness () { runtimeIdCountInc: runtimeIdCount.inc, count: ssiMetricsNamespace.count } - return { stubs, SSIHeuristics, EnablementChoice } + return { stubs, SSIHeuristics } } -function testDisabledHeuristics () { - const { stubs, SSIHeuristics, EnablementChoice } = setupHarness() - const heuristics = new SSIHeuristics(new Config().profiling) +function testInactive (profilingEnabledValue) { + process.env.DD_PROFILING_ENABLED = profilingEnabledValue + + const { stubs, SSIHeuristics } = setupHarness() + const heuristics = new SSIHeuristics(new Config()) heuristics.start() + expect(heuristics.emitsTelemetry).to.equal(false) + expect(heuristics.heuristicsActive).to.equal(false) + dc.channel('dd-trace:span:start').publish() dc.channel('datadog:profiling:profile-submitted').publish() dc.channel('datadog:profiling:mock-profile-submitted').publish() dc.channel('datadog:telemetry:app-closing').publish() - expect(heuristics.enablementChoice).to.equal(EnablementChoice.DISABLED) - expect(heuristics.enabled()).to.equal(false) + expect(heuristics.enablementChoice).to.equal(undefined) // When it is disabled, the telemetry should not subscribe to any channel // so the preceding publishes should not have any effect. expect(heuristics._profileCount).to.equal(undefined) @@ -94,6 +137,7 @@ function executeTelemetryEnabledScenario ( profileCount, sentProfiles, enablementChoice, + heuristicsActive, heuristicDecision, longLived = false ) { @@ -102,13 +146,23 @@ function executeTelemetryEnabledScenario ( if (longLived) { config.profiling.longLivedThreshold = 2 } - const heuristics = new SSIHeuristics(config.profiling) + const heuristics = new SSIHeuristics(config) heuristics.start() - expect(heuristics.enabled()).to.equal(true) + expect(heuristics.heuristicsActive).to.equal(heuristicsActive) function runScenarioAndCheck () { scenario(heuristics) - createAndCheckMetrics(stubs, profileCount, sentProfiles, enablementChoice, heuristicDecision) + if (enablementChoice) { + createAndCheckMetrics(stubs, profileCount, sentProfiles, enablementChoice, heuristicDecision) + } else { + // enablementChose being undefined means telemetry should not be active so + // no metrics APIs must've been called + expect(stubs.count.args.length).to.equal(0) + expect(stubs.profileCountCountInc.args.length).to.equal(0) + expect(stubs.count.args.length).to.equal(0) + expect(stubs.runtimeIdCountInc.args.length).to.equal(0) + } + dc.channel('datadog:telemetry:app-closing').publish() } if (longLived) { @@ -118,7 +172,13 @@ function executeTelemetryEnabledScenario ( } } -function createAndCheckMetrics (stubs, profileCount, sentProfiles, enablementChoice, heuristicDecision) { +function createAndCheckMetrics ( + stubs, + profileCount, + sentProfiles, + enablementChoice, + heuristicDecision +) { // Trigger metrics creation dc.channel('datadog:telemetry:app-closing').publish() @@ -134,41 +194,45 @@ function createAndCheckMetrics (stubs, profileCount, sentProfiles, enablementCho expect(stubs.runtimeIdCountInc.args.length).to.equal(1) } -function testEnabledHeuristics (enablementChoice) { - testNoOp(enablementChoice) - testProfilesSent(enablementChoice) - testMockProfilesSent(enablementChoice) - testSpan(enablementChoice) - return testLongLived(enablementChoice).then(() => testTriggered(enablementChoice)) +function testEnabledHeuristics (enablementChoice, heuristicsEnabled) { + testNoOp(enablementChoice, heuristicsEnabled) + testProfilesSent(enablementChoice, heuristicsEnabled) + testMockProfilesSent(enablementChoice, heuristicsEnabled) + testSpan(enablementChoice, heuristicsEnabled) + return testLongLived(enablementChoice, heuristicsEnabled).then( + () => testTriggered(enablementChoice, heuristicsEnabled) + ) } -function testNoOp (enablementChoice) { - executeTelemetryEnabledScenario(_ => {}, 0, false, enablementChoice, 'no_span_short_lived') +function testNoOp (enablementChoice, heuristicsActive) { + executeTelemetryEnabledScenario(_ => {}, 0, false, enablementChoice, heuristicsActive, 'no_span_short_lived') } -function testProfilesSent (enablementChoice) { +function testProfilesSent (enablementChoice, heuristicsActive) { executeTelemetryEnabledScenario(_ => { dc.channel('datadog:profiling:profile-submitted').publish() dc.channel('datadog:profiling:profile-submitted').publish() - }, 2, true, enablementChoice, 'no_span_short_lived') + }, 2, true, enablementChoice, heuristicsActive, 'no_span_short_lived') } -function testMockProfilesSent (enablementChoice) { +function testMockProfilesSent (enablementChoice, heuristicsActive) { executeTelemetryEnabledScenario(_ => { dc.channel('datadog:profiling:mock-profile-submitted').publish() dc.channel('datadog:profiling:mock-profile-submitted').publish() - }, 2, false, enablementChoice, 'no_span_short_lived') + }, 2, false, enablementChoice, heuristicsActive, 'no_span_short_lived') } -function testSpan (enablementChoice) { +function testSpan (enablementChoice, heuristicsActive) { executeTelemetryEnabledScenario(heuristics => { - dc.channel('dd-trace:span:start').publish() + const ch = dc.channel('dd-trace:span:start') + expect(ch.hasSubscribers).to.equal(true) + ch.publish() expect(heuristics.noSpan).to.equal(false) dc.channel('datadog:profiling:profile-submitted').publish() - }, 1, true, enablementChoice, 'short_lived') + }, 1, true, enablementChoice, heuristicsActive, 'short_lived') } -function testLongLived (enablementChoice) { +function testLongLived (enablementChoice, heuristicsActive) { let callbackInvoked = false return executeTelemetryEnabledScenario(heuristics => { heuristics.onTriggered(() => { @@ -176,12 +240,12 @@ function testLongLived (enablementChoice) { heuristics.onTriggered() }) dc.channel('datadog:profiling:profile-submitted').publish() - }, 1, true, enablementChoice, 'no_span', true).then(() => { + }, 1, true, enablementChoice, heuristicsActive, 'no_span', true).then(() => { expect(callbackInvoked).to.equal(false) }) } -function testTriggered (enablementChoice) { +function testTriggered (enablementChoice, heuristicsActive) { let callbackInvoked = false return executeTelemetryEnabledScenario(heuristics => { heuristics.onTriggered(() => { @@ -191,7 +255,7 @@ function testTriggered (enablementChoice) { dc.channel('dd-trace:span:start').publish() expect(heuristics.noSpan).to.equal(false) dc.channel('datadog:profiling:profile-submitted').publish() - }, 1, true, enablementChoice, 'triggered', true).then(() => { + }, 1, true, enablementChoice, heuristicsActive, 'triggered', true).then(() => { expect(callbackInvoked).to.equal(true) }) } diff --git a/packages/dd-trace/test/proxy.spec.js b/packages/dd-trace/test/proxy.spec.js index 6b694ea805..b7df0c6a64 100644 --- a/packages/dd-trace/test/proxy.spec.js +++ b/packages/dd-trace/test/proxy.spec.js @@ -123,6 +123,7 @@ describe('TracerProxy', () => { config = { tracing: true, experimental: {}, + injectionEnabled: [], logger: 'logger', debug: true, profiling: {}, @@ -489,7 +490,7 @@ describe('TracerProxy', () => { }) it('should load profiler when configured', () => { - config.profiling = { enabled: true } + config.profiling = { enabled: 'true' } proxy.init() @@ -497,7 +498,7 @@ describe('TracerProxy', () => { }) it('should throw an error since profiler fails to be imported', () => { - config.profiling = { enabled: true } + config.profiling = { enabled: 'true' } const ProfilerImportFailureProxy = proxyquire('../src/proxy', { './tracer': DatadogTracer,