From 3513e29943f6a2686fcda880d90641de0b5480c8 Mon Sep 17 00:00:00 2001 From: Igor Unanua Date: Wed, 20 Nov 2024 16:07:00 +0100 Subject: [PATCH 01/11] Remove template literals --- .../src/helpers/bundler-register.js | 4 ++-- .../src/helpers/register.js | 3 +-- packages/datadog-plugin-grpc/src/util.js | 2 +- .../exporters/ci-visibility-exporter.js | 2 +- packages/dd-trace/src/debugger/index.js | 4 ++-- .../dd-trace/src/exporters/common/request.js | 2 +- packages/dd-trace/src/opentracing/span.js | 2 +- packages/dd-trace/src/plugins/ci_plugin.js | 6 +++--- packages/dd-trace/src/plugins/util/git.js | 4 ++-- packages/dd-trace/src/plugins/util/test.js | 4 ++-- packages/dd-trace/src/serverless.js | 2 +- packages/dd-trace/src/span_processor.js | 20 +++++++++---------- .../test/exporters/common/request.spec.js | 2 +- 13 files changed, 28 insertions(+), 29 deletions(-) diff --git a/packages/datadog-instrumentations/src/helpers/bundler-register.js b/packages/datadog-instrumentations/src/helpers/bundler-register.js index a5dfead9669..491a0ba0600 100644 --- a/packages/datadog-instrumentations/src/helpers/bundler-register.js +++ b/packages/datadog-instrumentations/src/helpers/bundler-register.js @@ -30,12 +30,12 @@ dc.subscribe(CHANNEL, (payload) => { try { hooks[payload.package]() } catch (err) { - log.error(`esbuild-wrapped ${payload.package} missing in list of hooks`) + log.error('esbuild-wrapped %s missing in list of hooks', payload.package) throw err } if (!instrumentations[payload.package]) { - log.error(`esbuild-wrapped ${payload.package} missing in list of instrumentations`) + log.error('esbuild-wrapped %s missing in list of instrumentations', payload.package) return } diff --git a/packages/datadog-instrumentations/src/helpers/register.js b/packages/datadog-instrumentations/src/helpers/register.js index 171db91e224..c4f9e0f3841 100644 --- a/packages/datadog-instrumentations/src/helpers/register.js +++ b/packages/datadog-instrumentations/src/helpers/register.js @@ -103,8 +103,7 @@ for (const packageName of names) { try { version = version || getVersion(moduleBaseDir) } catch (e) { - log.error(`Error getting version for "${name}": ${e.message}`) - log.error(e) + log.error('Error getting version for "%s": %s', name, e.message, e) continue } if (typeof namesAndSuccesses[`${name}@${version}`] === 'undefined') { diff --git a/packages/datadog-plugin-grpc/src/util.js b/packages/datadog-plugin-grpc/src/util.js index 1c1937e7ea7..ec7d0f33570 100644 --- a/packages/datadog-plugin-grpc/src/util.js +++ b/packages/datadog-plugin-grpc/src/util.js @@ -54,7 +54,7 @@ module.exports = { } if (config.hasOwnProperty(filter)) { - log.error(`Expected '${filter}' to be an array or function.`) + log.error('Expected \'%s\' to be an array or function.', filter) } return () => ({}) diff --git a/packages/dd-trace/src/ci-visibility/exporters/ci-visibility-exporter.js b/packages/dd-trace/src/ci-visibility/exporters/ci-visibility-exporter.js index 0a12d5f8c5a..444f08d3130 100644 --- a/packages/dd-trace/src/ci-visibility/exporters/ci-visibility-exporter.js +++ b/packages/dd-trace/src/ci-visibility/exporters/ci-visibility-exporter.js @@ -225,7 +225,7 @@ class CiVisibilityExporter extends AgentInfoExporter { repositoryUrl, (err) => { if (err) { - log.error(`Error uploading git metadata: ${err.message}`) + log.error('Error uploading git metadata: %s', err.message) } else { log.debug('Successfully uploaded git metadata') } diff --git a/packages/dd-trace/src/debugger/index.js b/packages/dd-trace/src/debugger/index.js index 3638119c6f1..8e3fa56c2e8 100644 --- a/packages/dd-trace/src/debugger/index.js +++ b/packages/dd-trace/src/debugger/index.js @@ -33,8 +33,8 @@ function start (config, rc) { const ack = rcAckCallbacks.get(ackId) if (ack === undefined) { // This should never happen, but just in case something changes in the future, we should guard against it - log.error(`Received an unknown ackId: ${ackId}`) - if (error) log.error(error) + log.error('Received an unknown ackId: %s', ackId) + if (error) log.error('Error starting Dynamic Instrumentation client', error) return } ack(error) diff --git a/packages/dd-trace/src/exporters/common/request.js b/packages/dd-trace/src/exporters/common/request.js index ab8b697eef6..2ff90236ee8 100644 --- a/packages/dd-trace/src/exporters/common/request.js +++ b/packages/dd-trace/src/exporters/common/request.js @@ -86,7 +86,7 @@ function request (data, options, callback) { if (isGzip) { zlib.gunzip(buffer, (err, result) => { if (err) { - log.error(`Could not gunzip response: ${err.message}`) + log.error('Could not gunzip response: %s', err.message) callback(null, '', res.statusCode) } else { callback(null, result.toString(), res.statusCode) diff --git a/packages/dd-trace/src/opentracing/span.js b/packages/dd-trace/src/opentracing/span.js index e855e504e17..00fd51da027 100644 --- a/packages/dd-trace/src/opentracing/span.js +++ b/packages/dd-trace/src/opentracing/span.js @@ -214,7 +214,7 @@ class DatadogSpan { if (DD_TRACE_EXPERIMENTAL_STATE_TRACKING === 'true') { if (!this._spanContext._tags['service.name']) { - log.error(`Finishing invalid span: ${this}`) + log.error('Finishing invalid span: %s', this) } } diff --git a/packages/dd-trace/src/plugins/ci_plugin.js b/packages/dd-trace/src/plugins/ci_plugin.js index f6692fa4b23..8d32eabe069 100644 --- a/packages/dd-trace/src/plugins/ci_plugin.js +++ b/packages/dd-trace/src/plugins/ci_plugin.js @@ -47,7 +47,7 @@ module.exports = class CiPlugin extends Plugin { } this.tracer._exporter.getLibraryConfiguration(this.testConfiguration, (err, libraryConfig) => { if (err) { - log.error(`Library configuration could not be fetched. ${err.message}`) + log.error('Library configuration could not be fetched. %s', err.message) } else { this.libraryConfig = libraryConfig } @@ -61,7 +61,7 @@ module.exports = class CiPlugin extends Plugin { } this.tracer._exporter.getSkippableSuites(this.testConfiguration, (err, skippableSuites, itrCorrelationId) => { if (err) { - log.error(`Skippable suites could not be fetched. ${err.message}`) + log.error('Skippable suites could not be fetched. %s', err.message) } else { this.itrCorrelationId = itrCorrelationId } @@ -150,7 +150,7 @@ module.exports = class CiPlugin extends Plugin { } this.tracer._exporter.getKnownTests(this.testConfiguration, (err, knownTests) => { if (err) { - log.error(`Known tests could not be fetched. ${err.message}`) + log.error('Known tests could not be fetched. %s', err.message) this.libraryConfig.isEarlyFlakeDetectionEnabled = false } onDone({ err, knownTests }) diff --git a/packages/dd-trace/src/plugins/util/git.js b/packages/dd-trace/src/plugins/util/git.js index 06b9521817f..841755e9246 100644 --- a/packages/dd-trace/src/plugins/util/git.js +++ b/packages/dd-trace/src/plugins/util/git.js @@ -196,7 +196,7 @@ function getLatestCommits () { distributionMetric(TELEMETRY_GIT_COMMAND_MS, { command: 'get_local_commits' }, Date.now() - startTime) return result } catch (err) { - log.error(`Get latest commits failed: ${err.message}`) + log.error('Get latest commits failed: %s', err.message) incrementCountMetric( TELEMETRY_GIT_COMMAND_ERRORS, { command: 'get_local_commits', errorType: err.status } @@ -229,7 +229,7 @@ function getCommitsRevList (commitsToExclude, commitsToInclude) { .split('\n') .filter(commit => commit) } catch (err) { - log.error(`Get commits to upload failed: ${err.message}`) + log.error('Get commits to upload failed: %s', err.message) incrementCountMetric( TELEMETRY_GIT_COMMAND_ERRORS, { command: 'get_objects', errorType: err.code, exitCode: err.status || err.errno } // err.status might be null diff --git a/packages/dd-trace/src/plugins/util/test.js b/packages/dd-trace/src/plugins/util/test.js index 8719c916915..633b1f14361 100644 --- a/packages/dd-trace/src/plugins/util/test.js +++ b/packages/dd-trace/src/plugins/util/test.js @@ -218,13 +218,13 @@ function removeInvalidMetadata (metadata) { return Object.keys(metadata).reduce((filteredTags, tag) => { if (tag === GIT_REPOSITORY_URL) { if (!validateGitRepositoryUrl(metadata[GIT_REPOSITORY_URL])) { - log.error(`Repository URL is not a valid repository URL: ${metadata[GIT_REPOSITORY_URL]}.`) + log.error('Repository URL is not a valid repository URL: %s.', metadata[GIT_REPOSITORY_URL]) return filteredTags } } if (tag === GIT_COMMIT_SHA) { if (!validateGitCommitSha(metadata[GIT_COMMIT_SHA])) { - log.error(`Git commit SHA must be a full-length git SHA: ${metadata[GIT_COMMIT_SHA]}.`) + log.error('Git commit SHA must be a full-length git SHA: %s.', metadata[GIT_COMMIT_SHA]) return filteredTags } } diff --git a/packages/dd-trace/src/serverless.js b/packages/dd-trace/src/serverless.js index d352cae899e..415df38fc2c 100644 --- a/packages/dd-trace/src/serverless.js +++ b/packages/dd-trace/src/serverless.js @@ -23,7 +23,7 @@ function maybeStartServerlessMiniAgent (config) { try { require('child_process').spawn(rustBinaryPath, { stdio: 'inherit' }) } catch (err) { - log.error(`Error spawning mini agent process: ${err}`) + log.error('Error spawning mini agent process: %s', err.message) } } diff --git a/packages/dd-trace/src/span_processor.js b/packages/dd-trace/src/span_processor.js index deb92c02f34..46cf51b162b 100644 --- a/packages/dd-trace/src/span_processor.js +++ b/packages/dd-trace/src/span_processor.js @@ -87,22 +87,22 @@ class SpanProcessor { const id = context.toSpanId() if (finished.has(span)) { - log.error(`Span was already finished in the same trace: ${span}`) + log.error('Span was already finished in the same trace: %s', span) } else { finished.add(span) if (finishedIds.has(id)) { - log.error(`Another span with the same ID was already finished in the same trace: ${span}`) + log.error('Another span with the same ID was already finished in the same trace: %s', span) } else { finishedIds.add(id) } if (context._trace !== trace) { - log.error(`A span was finished in the wrong trace: ${span}.`) + log.error('A span was finished in the wrong trace: %s', span) } if (finishedSpans.has(span)) { - log.error(`Span was already finished in a different trace: ${span}`) + log.error('Span was already finished in a different trace: %s', span) } else { finishedSpans.add(span) } @@ -114,35 +114,35 @@ class SpanProcessor { const id = context.toSpanId() if (started.has(span)) { - log.error(`Span was already started in the same trace: ${span}`) + log.error('Span was already started in the same trace: %s', span) } else { started.add(span) if (startedIds.has(id)) { - log.error(`Another span with the same ID was already started in the same trace: ${span}`) + log.error('Another span with the same ID was already started in the same trace: %s', span) } else { startedIds.add(id) } if (context._trace !== trace) { - log.error(`A span was started in the wrong trace: ${span}.`) + log.error('A span was started in the wrong trace: %s', span) } if (startedSpans.has(span)) { - log.error(`Span was already started in a different trace: ${span}`) + log.error('Span was already started in a different trace: %s', span) } else { startedSpans.add(span) } } if (!finished.has(span)) { - log.error(`Span started in one trace but was finished in another trace: ${span}`) + log.error('Span started in one trace but was finished in another trace: %s', span) } } for (const span of trace.finished) { if (!started.has(span)) { - log.error(`Span finished in one trace but was started in another trace: ${span}`) + log.error('Span finished in one trace but was started in another trace: %s', span) } } } diff --git a/packages/dd-trace/test/exporters/common/request.spec.js b/packages/dd-trace/test/exporters/common/request.spec.js index 55bcb603a27..a6efcc45fa6 100644 --- a/packages/dd-trace/test/exporters/common/request.spec.js +++ b/packages/dd-trace/test/exporters/common/request.spec.js @@ -429,7 +429,7 @@ describe('request', function () { 'accept-encoding': 'gzip' } }, (err, res) => { - expect(log.error).to.have.been.calledWith('Could not gunzip response: unexpected end of file') + expect(log.error).to.have.been.calledWith('Could not gunzip response: %s', 'unexpected end of file') expect(res).to.equal('') done(err) }) From dda2ec2ebc43a582f3c319b760e925dd63d56e7b Mon Sep 17 00:00:00 2001 From: Igor Unanua Date: Wed, 20 Nov 2024 17:09:06 +0100 Subject: [PATCH 02/11] Enable log collection by default --- packages/dd-trace/src/config.js | 8 +------- packages/dd-trace/src/telemetry/index.js | 1 + packages/dd-trace/test/config.spec.js | 17 +++-------------- 3 files changed, 5 insertions(+), 21 deletions(-) diff --git a/packages/dd-trace/src/config.js b/packages/dd-trace/src/config.js index 73cac449546..7a92a611fcb 100644 --- a/packages/dd-trace/src/config.js +++ b/packages/dd-trace/src/config.js @@ -553,7 +553,7 @@ class Config { this._setValue(defaults, 'telemetry.dependencyCollection', true) this._setValue(defaults, 'telemetry.enabled', true) this._setValue(defaults, 'telemetry.heartbeatInterval', 60000) - this._setValue(defaults, 'telemetry.logCollection', false) + this._setValue(defaults, 'telemetry.logCollection', true) this._setValue(defaults, 'telemetry.metrics', true) this._setValue(defaults, 'traceEnabled', true) this._setValue(defaults, 'traceId128BitGenerationEnabled', true) @@ -1142,12 +1142,6 @@ class Config { 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) - } if (this._env.injectionEnabled?.length > 0) { this._setBoolean(calc, 'crashtracking.enabled', true) } diff --git a/packages/dd-trace/src/telemetry/index.js b/packages/dd-trace/src/telemetry/index.js index 5df7d6fcae3..eb1fe376c67 100644 --- a/packages/dd-trace/src/telemetry/index.js +++ b/packages/dd-trace/src/telemetry/index.js @@ -137,6 +137,7 @@ function appClosing () { sendData(config, application, host, reqType, payload) // We flush before shutting down. metricsManager.send(config, application, host) + telemetryLogger.send(config, application, host) } function onBeforeExit () { diff --git a/packages/dd-trace/test/config.spec.js b/packages/dd-trace/test/config.spec.js index 1720c4a5c91..9c255d722bd 100644 --- a/packages/dd-trace/test/config.spec.js +++ b/packages/dd-trace/test/config.spec.js @@ -380,7 +380,7 @@ describe('Config', () => { { name: 'telemetry.dependencyCollection', value: true, origin: 'default' }, { name: 'telemetry.enabled', value: true, origin: 'env_var' }, { name: 'telemetry.heartbeatInterval', value: 60000, origin: 'default' }, - { name: 'telemetry.logCollection', value: false, origin: 'default' }, + { name: 'telemetry.logCollection', value: true, origin: 'default' }, { name: 'telemetry.metrics', value: true, origin: 'default' }, { name: 'traceId128BitGenerationEnabled', value: true, origin: 'default' }, { name: 'traceId128BitLoggingEnabled', value: false, origin: 'default' }, @@ -1577,7 +1577,7 @@ describe('Config', () => { expect(config.telemetry).to.not.be.undefined expect(config.telemetry.enabled).to.be.true expect(config.telemetry.heartbeatInterval).to.eq(60000) - expect(config.telemetry.logCollection).to.be.false + expect(config.telemetry.logCollection).to.be.true expect(config.telemetry.debug).to.be.false expect(config.telemetry.metrics).to.be.true }) @@ -1615,7 +1615,7 @@ describe('Config', () => { process.env.DD_TELEMETRY_METRICS_ENABLED = origTelemetryMetricsEnabledValue }) - it('should not set DD_TELEMETRY_LOG_COLLECTION_ENABLED', () => { + it('should disable log collection if DD_TELEMETRY_LOG_COLLECTION_ENABLED is false', () => { const origLogsValue = process.env.DD_TELEMETRY_LOG_COLLECTION_ENABLED process.env.DD_TELEMETRY_LOG_COLLECTION_ENABLED = 'false' @@ -1626,17 +1626,6 @@ describe('Config', () => { process.env.DD_TELEMETRY_LOG_COLLECTION_ENABLED = origLogsValue }) - it('should set DD_TELEMETRY_LOG_COLLECTION_ENABLED if DD_IAST_ENABLED', () => { - const origIastEnabledValue = process.env.DD_IAST_ENABLED - process.env.DD_IAST_ENABLED = 'true' - - const config = new Config() - - expect(config.telemetry.logCollection).to.be.true - - process.env.DD_IAST_ENABLED = origIastEnabledValue - }) - it('should set DD_TELEMETRY_DEBUG', () => { const origTelemetryDebugValue = process.env.DD_TELEMETRY_DEBUG process.env.DD_TELEMETRY_DEBUG = 'true' From 9dbde4f5d4956a43e9868336f4a6bd20613bcd81 Mon Sep 17 00:00:00 2001 From: Igor Unanua Date: Thu, 21 Nov 2024 11:22:26 +0100 Subject: [PATCH 03/11] Add a message to some log.error(err) --- packages/dd-trace/src/config.js | 4 ++-- packages/dd-trace/src/datastreams/writer.js | 4 ++-- packages/dd-trace/src/dogstatsd.js | 4 ++-- packages/dd-trace/src/exporters/agent/writer.js | 6 +++--- packages/dd-trace/src/exporters/span-stats/writer.js | 2 +- packages/dd-trace/src/opentracing/tracer.js | 4 ++-- packages/dd-trace/src/proxy.js | 4 ++-- packages/dd-trace/src/runtime_metrics.js | 2 +- packages/dd-trace/src/telemetry/send-data.js | 4 ++-- packages/dd-trace/test/config.spec.js | 9 ++++++--- packages/dd-trace/test/exporters/agent/writer.spec.js | 4 +++- .../dd-trace/test/exporters/span-stats/writer.spec.js | 2 +- packages/dd-trace/test/proxy.spec.js | 3 ++- 13 files changed, 29 insertions(+), 23 deletions(-) diff --git a/packages/dd-trace/src/config.js b/packages/dd-trace/src/config.js index 7a92a611fcb..14bdd84b94d 100644 --- a/packages/dd-trace/src/config.js +++ b/packages/dd-trace/src/config.js @@ -145,7 +145,7 @@ function maybeFile (filepath) { try { return fs.readFileSync(filepath, 'utf8') } catch (e) { - log.error(e) + log.error('Error reading file %s', filepath, e) return undefined } } @@ -378,7 +378,7 @@ class Config { } catch (e) { // Only log error if the user has set a git.properties path if (process.env.DD_GIT_PROPERTIES_FILE) { - log.error(e) + log.error('Error reading DD_GIT_PROPERTIES_FILE: %s', DD_GIT_PROPERTIES_FILE, e) } } if (gitPropertiesString) { diff --git a/packages/dd-trace/src/datastreams/writer.js b/packages/dd-trace/src/datastreams/writer.js index f8c9e021ecc..5f789f2e056 100644 --- a/packages/dd-trace/src/datastreams/writer.js +++ b/packages/dd-trace/src/datastreams/writer.js @@ -45,13 +45,13 @@ class DataStreamsWriter { zlib.gzip(encodedPayload, { level: 1 }, (err, compressedData) => { if (err) { - log.error(err) + log.error('Error zipping datastream', err) return } makeRequest(compressedData, this._url, (err, res) => { log.debug(`Response from the agent: ${res}`) if (err) { - log.error(err) + log.error('Error sending datastream', err) } }) }) diff --git a/packages/dd-trace/src/dogstatsd.js b/packages/dd-trace/src/dogstatsd.js index ba84de71341..a396c9e98a4 100644 --- a/packages/dd-trace/src/dogstatsd.js +++ b/packages/dd-trace/src/dogstatsd.js @@ -71,7 +71,7 @@ class DogStatsDClient { const buffer = Buffer.concat(queue) request(buffer, this._httpOptions, (err) => { if (err) { - log.error('HTTP error from agent: ' + err.stack) + log.error('DogStatsDClient: HTTP error from agent: %s', err.message, err) if (err.status === 404) { // Inside this if-block, we have connectivity to the agent, but // we're not getting a 200 from the proxy endpoint. If it's a 404, @@ -89,7 +89,7 @@ class DogStatsDClient { this._sendUdpFromQueue(queue, this._host, this._family) } else { lookup(this._host, (err, address, family) => { - if (err) return log.error(err) + if (err) return log.error('DogStatsDClient: Host not found', err) this._sendUdpFromQueue(queue, address, family) }) } diff --git a/packages/dd-trace/src/exporters/agent/writer.js b/packages/dd-trace/src/exporters/agent/writer.js index 82a28647778..8fac323e614 100644 --- a/packages/dd-trace/src/exporters/agent/writer.js +++ b/packages/dd-trace/src/exporters/agent/writer.js @@ -41,17 +41,17 @@ class Writer extends BaseWriter { startupLog({ agentError: err }) if (err) { - log.error(err) + log.error('Error sending payload to the agent (status code: %s)', err.status, err) done() return } - log.debug(`Response from the agent: ${res}`) + log.debug('Response from the agent: %s', res) try { this._prioritySampler.update(JSON.parse(res).rate_by_service) } catch (e) { - log.error(e) + log.error('Error updating prioritySampler rates', e) runtimeMetrics.increment(`${METRIC_PREFIX}.errors`, true) runtimeMetrics.increment(`${METRIC_PREFIX}.errors.by.name`, `name:${e.name}`, true) diff --git a/packages/dd-trace/src/exporters/span-stats/writer.js b/packages/dd-trace/src/exporters/span-stats/writer.js index 3ece6d221b4..37cd6c77d5e 100644 --- a/packages/dd-trace/src/exporters/span-stats/writer.js +++ b/packages/dd-trace/src/exporters/span-stats/writer.js @@ -16,7 +16,7 @@ class Writer extends BaseWriter { _sendPayload (data, _, done) { makeRequest(data, this._url, (err, res) => { if (err) { - log.error(err) + log.error('Error sending span stats', err) done() return } diff --git a/packages/dd-trace/src/opentracing/tracer.js b/packages/dd-trace/src/opentracing/tracer.js index 2d854442cc3..4ae30ca93ac 100644 --- a/packages/dd-trace/src/opentracing/tracer.js +++ b/packages/dd-trace/src/opentracing/tracer.js @@ -91,7 +91,7 @@ class DatadogTracer { } this._propagators[format].inject(context, carrier) } catch (e) { - log.error(e) + log.error('Error injecting trace', e) runtimeMetrics.increment('datadog.tracer.node.inject.errors', true) } } @@ -100,7 +100,7 @@ class DatadogTracer { try { return this._propagators[format].extract(carrier) } catch (e) { - log.error(e) + log.error('Error extracting trace', e) runtimeMetrics.increment('datadog.tracer.node.extract.errors', true) return null } diff --git a/packages/dd-trace/src/proxy.js b/packages/dd-trace/src/proxy.js index 81d003eebb7..fd814c9d6e3 100644 --- a/packages/dd-trace/src/proxy.js +++ b/packages/dd-trace/src/proxy.js @@ -187,7 +187,7 @@ class Tracer extends NoopProxy { testVisibilityDynamicInstrumentation.start() } } catch (e) { - log.error(e) + log.error('Error initialising tracer', e) } return this @@ -198,7 +198,7 @@ class Tracer extends NoopProxy { try { return require('./profiler').start(config) } catch (e) { - log.error(e) + log.error('Error starting profiler', e) } } diff --git a/packages/dd-trace/src/runtime_metrics.js b/packages/dd-trace/src/runtime_metrics.js index b2711879a05..49e724eb11c 100644 --- a/packages/dd-trace/src/runtime_metrics.js +++ b/packages/dd-trace/src/runtime_metrics.js @@ -32,7 +32,7 @@ module.exports = { nativeMetrics = require('@datadog/native-metrics') nativeMetrics.start() } catch (e) { - log.error(e) + log.error('Error starting native metrics', e) nativeMetrics = null } diff --git a/packages/dd-trace/src/telemetry/send-data.js b/packages/dd-trace/src/telemetry/send-data.js index 813fa427812..81406910c27 100644 --- a/packages/dd-trace/src/telemetry/send-data.js +++ b/packages/dd-trace/src/telemetry/send-data.js @@ -57,7 +57,7 @@ function sendData (config, application, host, reqType, payload = {}, cb = () => try { url = url || new URL(getAgentlessTelemetryEndpoint(config.site)) } catch (err) { - log.error(err) + log.error('Telemetry endpoint url is invalid', err) // No point to do the request if the URL is invalid return cb(err, { payload, reqType }) } @@ -100,7 +100,7 @@ function sendData (config, application, host, reqType, payload = {}, cb = () => path: '/api/v2/apmtelemetry' } if (backendUrl) { - request(data, backendOptions, (error) => { log.error(error) }) + request(data, backendOptions, (error) => { log.error('Error sending telemetry data', error) }) } else { log.error('Invalid Telemetry URL') } diff --git a/packages/dd-trace/test/config.spec.js b/packages/dd-trace/test/config.spec.js index 9c255d722bd..1eba985d04b 100644 --- a/packages/dd-trace/test/config.spec.js +++ b/packages/dd-trace/test/config.spec.js @@ -1781,9 +1781,12 @@ describe('Config', () => { }) expect(log.error).to.be.callCount(3) - expect(log.error.firstCall).to.have.been.calledWithExactly(error) - expect(log.error.secondCall).to.have.been.calledWithExactly(error) - expect(log.error.thirdCall).to.have.been.calledWithExactly(error) + expect(log.error.firstCall) + .to.have.been.calledWithExactly('Error reading file %s', 'DOES_NOT_EXIST.json', error) + expect(log.error.secondCall) + .to.have.been.calledWithExactly('Error reading file %s', 'DOES_NOT_EXIST.html', error) + expect(log.error.thirdCall) + .to.have.been.calledWithExactly('Error reading file %s', 'DOES_NOT_EXIST.json', error) expect(config.appsec.enabled).to.be.true expect(config.appsec.rules).to.eq('path/to/rules.json') diff --git a/packages/dd-trace/test/exporters/agent/writer.spec.js b/packages/dd-trace/test/exporters/agent/writer.spec.js index ce7a62d49bf..aad8749ef37 100644 --- a/packages/dd-trace/test/exporters/agent/writer.spec.js +++ b/packages/dd-trace/test/exporters/agent/writer.spec.js @@ -149,6 +149,7 @@ function describeWriter (protocolVersion) { it('should log request errors', done => { const error = new Error('boom') + error.status = 42 request.yields(error) @@ -156,7 +157,8 @@ function describeWriter (protocolVersion) { writer.flush() setTimeout(() => { - expect(log.error).to.have.been.calledWith(error) + expect(log.error) + .to.have.been.calledWith('Error sending payload to the agent (status code: %s)', error.status, error) done() }) }) diff --git a/packages/dd-trace/test/exporters/span-stats/writer.spec.js b/packages/dd-trace/test/exporters/span-stats/writer.spec.js index d65d480409d..f8e65500e04 100644 --- a/packages/dd-trace/test/exporters/span-stats/writer.spec.js +++ b/packages/dd-trace/test/exporters/span-stats/writer.spec.js @@ -106,7 +106,7 @@ describe('span-stats writer', () => { encoder.count.returns(1) writer.flush(() => { - expect(log.error).to.have.been.calledWith(error) + expect(log.error).to.have.been.calledWith('Error sending span stats', error) done() }) }) diff --git a/packages/dd-trace/test/proxy.spec.js b/packages/dd-trace/test/proxy.spec.js index 4836e99787f..dd145390245 100644 --- a/packages/dd-trace/test/proxy.spec.js +++ b/packages/dd-trace/test/proxy.spec.js @@ -520,8 +520,9 @@ describe('TracerProxy', () => { const profilerImportFailureProxy = new ProfilerImportFailureProxy() profilerImportFailureProxy.init() + sinon.assert.calledOnce(log.error) const expectedErr = sinon.match.instanceOf(Error).and(sinon.match.has('code', 'MODULE_NOT_FOUND')) - sinon.assert.calledWith(log.error, sinon.match(expectedErr)) + sinon.assert.match(log.error.firstCall.lastArg, sinon.match(expectedErr)) }) it('should start telemetry', () => { From beec7aa8f66717b6c2708e7887be3411969bb809 Mon Sep 17 00:00:00 2001 From: Igor Unanua Date: Mon, 25 Nov 2024 16:32:06 +0100 Subject: [PATCH 04/11] remove template lit --- packages/datadog-instrumentations/src/helpers/register.js | 2 +- packages/datadog-plugin-avsc/src/schema_iterator.js | 2 +- packages/dd-trace/src/plugins/plugin.js | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/datadog-instrumentations/src/helpers/register.js b/packages/datadog-instrumentations/src/helpers/register.js index c4f9e0f3841..5a28f066c1f 100644 --- a/packages/datadog-instrumentations/src/helpers/register.js +++ b/packages/datadog-instrumentations/src/helpers/register.js @@ -145,7 +145,7 @@ for (const packageName of names) { `integration:${name}`, `integration_version:${version}` ]) - log.info(`Found incompatible integration version: ${nameVersion}`) + log.info('Found incompatible integration version: %s', nameVersion) seenCombo.add(nameVersion) } } diff --git a/packages/datadog-plugin-avsc/src/schema_iterator.js b/packages/datadog-plugin-avsc/src/schema_iterator.js index c748bbf9e75..44fce95a765 100644 --- a/packages/datadog-plugin-avsc/src/schema_iterator.js +++ b/packages/datadog-plugin-avsc/src/schema_iterator.js @@ -110,7 +110,7 @@ class SchemaExtractor { } for (const field of schema.fields) { if (!this.extractProperty(field, schemaName, field.name, builder, depth)) { - log.warn(`DSM: Unable to extract field with name: ${field.name} from Avro schema with name: ${schemaName}`) + log.warn('DSM: Unable to extract field with name: %s from Avro schema with name: %s', field.name, schemaName) } } } diff --git a/packages/dd-trace/src/plugins/plugin.js b/packages/dd-trace/src/plugins/plugin.js index 78a49b62b14..e8d9c911a69 100644 --- a/packages/dd-trace/src/plugins/plugin.js +++ b/packages/dd-trace/src/plugins/plugin.js @@ -79,7 +79,7 @@ module.exports = class Plugin { return handler.apply(this, arguments) } catch (e) { logger.error('Error in plugin handler:', e) - logger.info('Disabling plugin:', plugin.id) + logger.info('Disabling plugin: %s', plugin.id) plugin.configure(false) } } From 3b668ce08f58f589d8f6b54149f819d436f247a4 Mon Sep 17 00:00:00 2001 From: Igor Unanua Date: Wed, 27 Nov 2024 13:30:41 +0100 Subject: [PATCH 05/11] Add messages to request errors --- .../src/ci-visibility/exporters/agentless/coverage-writer.js | 2 +- .../src/ci-visibility/exporters/agentless/di-logs-writer.js | 2 +- .../dd-trace/src/ci-visibility/exporters/agentless/writer.js | 2 +- packages/dd-trace/src/debugger/devtools_client/status.js | 2 +- packages/dd-trace/src/flare/index.js | 2 +- packages/dd-trace/src/llmobs/writers/base.js | 4 ++-- .../ci-visibility/exporters/agentless/coverage-writer.spec.js | 2 +- .../test/ci-visibility/exporters/agentless/writer.spec.js | 2 +- 8 files changed, 9 insertions(+), 9 deletions(-) diff --git a/packages/dd-trace/src/ci-visibility/exporters/agentless/coverage-writer.js b/packages/dd-trace/src/ci-visibility/exporters/agentless/coverage-writer.js index 98eff61a6fd..a36b07201e1 100644 --- a/packages/dd-trace/src/ci-visibility/exporters/agentless/coverage-writer.js +++ b/packages/dd-trace/src/ci-visibility/exporters/agentless/coverage-writer.js @@ -63,7 +63,7 @@ class Writer extends BaseWriter { TELEMETRY_ENDPOINT_PAYLOAD_DROPPED, { endpoint: 'code_coverage' } ) - log.error(err) + log.error('Error sending CI coverage payload', err) done() return } diff --git a/packages/dd-trace/src/ci-visibility/exporters/agentless/di-logs-writer.js b/packages/dd-trace/src/ci-visibility/exporters/agentless/di-logs-writer.js index eebc3c5e6a9..7d8c5ba47a0 100644 --- a/packages/dd-trace/src/ci-visibility/exporters/agentless/di-logs-writer.js +++ b/packages/dd-trace/src/ci-visibility/exporters/agentless/di-logs-writer.js @@ -40,7 +40,7 @@ class DynamicInstrumentationLogsWriter extends BaseWriter { request(data, options, (err, res) => { if (err) { - log.error(err) + log.error('Error sending DI logs payload', err) done() return } diff --git a/packages/dd-trace/src/ci-visibility/exporters/agentless/writer.js b/packages/dd-trace/src/ci-visibility/exporters/agentless/writer.js index 466c5230b22..34cad3862bc 100644 --- a/packages/dd-trace/src/ci-visibility/exporters/agentless/writer.js +++ b/packages/dd-trace/src/ci-visibility/exporters/agentless/writer.js @@ -64,7 +64,7 @@ class Writer extends BaseWriter { TELEMETRY_ENDPOINT_PAYLOAD_DROPPED, { endpoint: 'test_cycle' } ) - log.error(err) + log.error('Error sending CI agentless payload', err) done() return } diff --git a/packages/dd-trace/src/debugger/devtools_client/status.js b/packages/dd-trace/src/debugger/devtools_client/status.js index e4ba10d8c55..b28fc3c281e 100644 --- a/packages/dd-trace/src/debugger/devtools_client/status.js +++ b/packages/dd-trace/src/debugger/devtools_client/status.js @@ -87,7 +87,7 @@ function send (payload) { } request(form, options, (err) => { - if (err) log.error(err) + if (err) log.error('Error sending debugger payload', err) }) } diff --git a/packages/dd-trace/src/flare/index.js b/packages/dd-trace/src/flare/index.js index 70ec4ccd75e..4a5166d45e1 100644 --- a/packages/dd-trace/src/flare/index.js +++ b/packages/dd-trace/src/flare/index.js @@ -83,7 +83,7 @@ const flare = { headers: form.getHeaders() }, (err) => { if (err) { - log.error(err) + log.error('Error sending flare payload', err) } }) } diff --git a/packages/dd-trace/src/llmobs/writers/base.js b/packages/dd-trace/src/llmobs/writers/base.js index 8a6cdae9c2f..1d33bc653ad 100644 --- a/packages/dd-trace/src/llmobs/writers/base.js +++ b/packages/dd-trace/src/llmobs/writers/base.js @@ -74,11 +74,11 @@ class BaseLLMObsWriter { request(payload, options, (err, resp, code) => { if (err) { logger.error( - `Error sending ${events.length} LLMObs ${this._eventType} events to ${this._url}: ${err.message}` + 'Error sending %d LLMObs %s events to %s: %s', events.length, this._eventType, this._url, err.message, err ) } else if (code >= 300) { logger.error( - `Error sending ${events.length} LLMObs ${this._eventType} events to ${this._url}: ${code}` + 'Error sending %d LLMObs %s events to %s: %s', events.length, this._eventType, this._url, code ) } else { logger.debug(`Sent ${events.length} LLMObs ${this._eventType} events to ${this._url}`) diff --git a/packages/dd-trace/test/ci-visibility/exporters/agentless/coverage-writer.spec.js b/packages/dd-trace/test/ci-visibility/exporters/agentless/coverage-writer.spec.js index 62e10e9753e..61ffee21181 100644 --- a/packages/dd-trace/test/ci-visibility/exporters/agentless/coverage-writer.spec.js +++ b/packages/dd-trace/test/ci-visibility/exporters/agentless/coverage-writer.spec.js @@ -111,7 +111,7 @@ describe('CI Visibility Coverage Writer', () => { encoder.makePayload.returns(payload) coverageWriter.flush(() => { - expect(log.error).to.have.been.calledWith(error) + expect(log.error).to.have.been.calledWith('Error sending CI coverage payload', error) done() }) }) diff --git a/packages/dd-trace/test/ci-visibility/exporters/agentless/writer.spec.js b/packages/dd-trace/test/ci-visibility/exporters/agentless/writer.spec.js index 85765c6bf3a..29ac58fbd31 100644 --- a/packages/dd-trace/test/ci-visibility/exporters/agentless/writer.spec.js +++ b/packages/dd-trace/test/ci-visibility/exporters/agentless/writer.spec.js @@ -113,7 +113,7 @@ describe('CI Visibility Writer', () => { encoder.count.returns(1) writer.flush(() => { - expect(log.error).to.have.been.calledWith(error) + expect(log.error).to.have.been.calledWith('Error sending CI agentless payload', error) done() }) }) From 76e135f55a570473b5f4c7d045e5aeee77622a1b Mon Sep 17 00:00:00 2001 From: Igor Unanua Date: Wed, 27 Nov 2024 13:46:23 +0100 Subject: [PATCH 06/11] Fix llmobs test --- packages/dd-trace/test/llmobs/writers/base.spec.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/dd-trace/test/llmobs/writers/base.spec.js b/packages/dd-trace/test/llmobs/writers/base.spec.js index 8b971b2748a..a2880251f4c 100644 --- a/packages/dd-trace/test/llmobs/writers/base.spec.js +++ b/packages/dd-trace/test/llmobs/writers/base.spec.js @@ -138,14 +138,16 @@ describe('BaseLLMObsWriter', () => { writer.append({ foo: 'bar' }) const error = new Error('boom') + let reqUrl request.callsFake((url, options, callback) => { + reqUrl = options.url callback(error) }) writer.flush() expect(logger.error).to.have.been.calledWith( - 'Error sending 1 LLMObs undefined events to https://llmobs-intake.datadoghq.com/api/v2/llmobs: boom' + 'Error sending %d LLMObs %s events to %s: %s', 1, undefined, reqUrl, 'boom', error ) }) From f7c170163fedaae999fd6429dcf33445b6a5ccf3 Mon Sep 17 00:00:00 2001 From: Igor Unanua Date: Wed, 27 Nov 2024 15:31:17 +0100 Subject: [PATCH 07/11] Some more messages --- packages/datadog-instrumentations/src/http/client.js | 2 +- .../dd-trace/src/ci-visibility/exporters/agentless/index.js | 2 +- .../src/ci-visibility/exporters/ci-visibility-exporter.js | 2 +- packages/dd-trace/src/crashtracking/crashtracker.js | 4 ++-- packages/dd-trace/src/lambda/runtime/ritm.js | 4 ++-- packages/dd-trace/src/tagger.js | 2 +- 6 files changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/datadog-instrumentations/src/http/client.js b/packages/datadog-instrumentations/src/http/client.js index 29547df61dc..6ab01a34513 100644 --- a/packages/datadog-instrumentations/src/http/client.js +++ b/packages/datadog-instrumentations/src/http/client.js @@ -39,7 +39,7 @@ function patch (http, methodName) { try { args = normalizeArgs.apply(null, arguments) } catch (e) { - log.error(e) + log.error('Error normalising http req arguments', e) return request.apply(this, arguments) } diff --git a/packages/dd-trace/src/ci-visibility/exporters/agentless/index.js b/packages/dd-trace/src/ci-visibility/exporters/agentless/index.js index 5895bb573cd..a5b677ef98b 100644 --- a/packages/dd-trace/src/ci-visibility/exporters/agentless/index.js +++ b/packages/dd-trace/src/ci-visibility/exporters/agentless/index.js @@ -38,7 +38,7 @@ class AgentlessCiVisibilityExporter extends CiVisibilityExporter { apiUrl = new URL(apiUrl) this._apiUrl = apiUrl } catch (e) { - log.error(e) + log.error('Error setting CI exporter api url', e) } } diff --git a/packages/dd-trace/src/ci-visibility/exporters/ci-visibility-exporter.js b/packages/dd-trace/src/ci-visibility/exporters/ci-visibility-exporter.js index 444f08d3130..dde5955bc75 100644 --- a/packages/dd-trace/src/ci-visibility/exporters/ci-visibility-exporter.js +++ b/packages/dd-trace/src/ci-visibility/exporters/ci-visibility-exporter.js @@ -345,7 +345,7 @@ class CiVisibilityExporter extends AgentInfoExporter { this._writer.setUrl(url) this._coverageWriter.setUrl(coverageUrl) } catch (e) { - log.error(e) + log.error('Error setting CI exporter url', e) } } diff --git a/packages/dd-trace/src/crashtracking/crashtracker.js b/packages/dd-trace/src/crashtracking/crashtracker.js index fc42195c953..a2d3ec2eb52 100644 --- a/packages/dd-trace/src/crashtracking/crashtracker.js +++ b/packages/dd-trace/src/crashtracking/crashtracker.js @@ -20,7 +20,7 @@ class Crashtracker { binding.updateConfig(this._getConfig(config)) binding.updateMetadata(this._getMetadata(config)) } catch (e) { - log.error(e) + log.error('Error configuring crashtracker', e) } } @@ -36,7 +36,7 @@ class Crashtracker { this._getMetadata(config) ) } catch (e) { - log.error(e) + log.error('Error initialising crashtracker', e) } } diff --git a/packages/dd-trace/src/lambda/runtime/ritm.js b/packages/dd-trace/src/lambda/runtime/ritm.js index 4dd27713a0b..ec50a4a80be 100644 --- a/packages/dd-trace/src/lambda/runtime/ritm.js +++ b/packages/dd-trace/src/lambda/runtime/ritm.js @@ -101,7 +101,7 @@ const registerLambdaHook = () => { try { moduleExports = hook(moduleExports) } catch (e) { - log.error(e) + log.error('Error executing lambda hook', e) } } @@ -120,7 +120,7 @@ const registerLambdaHook = () => { try { moduleExports = hook(moduleExports) } catch (e) { - log.error(e) + log.error('Error executing lambda hook for datadog-lambda-js', e) } } } diff --git a/packages/dd-trace/src/tagger.js b/packages/dd-trace/src/tagger.js index 41c8616a086..bbd8a187940 100644 --- a/packages/dd-trace/src/tagger.js +++ b/packages/dd-trace/src/tagger.js @@ -44,7 +44,7 @@ function add (carrier, keyValuePairs, parseOtelTags = false) { Object.assign(carrier, keyValuePairs) } } catch (e) { - log.error(e) + log.error('Error adding tags', e) } } From b1424e1ffd15f2a98992e49e28157b733d65d34a Mon Sep 17 00:00:00 2001 From: Igor Unanua Date: Wed, 27 Nov 2024 16:24:04 +0100 Subject: [PATCH 08/11] do not send 'Generic Error' with empty stack --- packages/dd-trace/src/telemetry/logs/log-collector.js | 2 +- .../dd-trace/test/telemetry/logs/log-collector.spec.js | 9 +++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/packages/dd-trace/src/telemetry/logs/log-collector.js b/packages/dd-trace/src/telemetry/logs/log-collector.js index 9103fd1c47d..d5a1ded9ff1 100644 --- a/packages/dd-trace/src/telemetry/logs/log-collector.js +++ b/packages/dd-trace/src/telemetry/logs/log-collector.js @@ -48,7 +48,7 @@ function sanitize (logEntry) { .map(line => line.replace(ddBasePath, '')) logEntry.stack_trace = stackLines.join(EOL) - if (logEntry.stack_trace === '' && !logEntry.message) { + if (logEntry.stack_trace === '' && (!logEntry.message || logEntry.message === 'Generic Error')) { // If entire stack was removed and there is no message we'd rather not log it at all. return null } diff --git a/packages/dd-trace/test/telemetry/logs/log-collector.spec.js b/packages/dd-trace/test/telemetry/logs/log-collector.spec.js index 1cb99cef518..05ce21f65f6 100644 --- a/packages/dd-trace/test/telemetry/logs/log-collector.spec.js +++ b/packages/dd-trace/test/telemetry/logs/log-collector.spec.js @@ -43,6 +43,15 @@ describe('telemetry log collector', () => { expect(logCollector.add({ message: 'Error 1', level: 'DEBUG', stack_trace: `stack 1\n${ddFrame}` })).to.be.true }) + it('should not store logs with empty stack and \'Generic Error\' message', () => { + expect(logCollector.add({ + message: 'Generic Error', + level: 'ERROR', + stack_trace: 'stack 1\n/not/a/dd/frame' + }) + ).to.be.false + }) + it('should include original message and dd frames', () => { const ddFrame = `at T (${ddBasePath}packages/dd-trace/test/telemetry/logs/log_collector.spec.js:29:21)` const stack = new Error('Error 1') From d23f868ab401c5e7eb0e9640bb7297e4740a732c Mon Sep 17 00:00:00 2001 From: Igor Unanua Date: Thu, 28 Nov 2024 16:38:46 +0100 Subject: [PATCH 09/11] Remove error type from message --- packages/dd-trace/src/telemetry/logs/index.js | 3 +- .../src/telemetry/logs/log-collector.js | 6 ++++ .../test/telemetry/logs/index.spec.js | 7 +++- .../test/telemetry/logs/log-collector.spec.js | 35 +++++++++++++------ 4 files changed, 37 insertions(+), 14 deletions(-) diff --git a/packages/dd-trace/src/telemetry/logs/index.js b/packages/dd-trace/src/telemetry/logs/index.js index c535acb9cdd..d8fa1969e55 100644 --- a/packages/dd-trace/src/telemetry/logs/index.js +++ b/packages/dd-trace/src/telemetry/logs/index.js @@ -47,8 +47,7 @@ function onErrorLog (msg) { if (cause) { telLog.stack_trace = cause.stack - const errorType = cause.name ?? 'Error' - telLog.message = `${errorType}: ${telLog.message}` + telLog.errorType = cause.constructor.name } onLog(telLog) diff --git a/packages/dd-trace/src/telemetry/logs/log-collector.js b/packages/dd-trace/src/telemetry/logs/log-collector.js index d5a1ded9ff1..a15f5ba4b3e 100644 --- a/packages/dd-trace/src/telemetry/logs/log-collector.js +++ b/packages/dd-trace/src/telemetry/logs/log-collector.js @@ -47,6 +47,12 @@ function sanitize (logEntry) { .filter((line, index) => (isDDCode && index < firstIndex) || line.includes(ddBasePath)) .map(line => line.replace(ddBasePath, '')) + if (!isDDCode && logEntry.errorType && stackLines.length) { + stackLines = [`${logEntry.errorType}: redacted`, ...stackLines] + } + + delete logEntry.errorType + logEntry.stack_trace = stackLines.join(EOL) if (logEntry.stack_trace === '' && (!logEntry.message || logEntry.message === 'Generic Error')) { // If entire stack was removed and there is no message we'd rather not log it at all. diff --git a/packages/dd-trace/test/telemetry/logs/index.spec.js b/packages/dd-trace/test/telemetry/logs/index.spec.js index 0d18b6e847b..e865644e960 100644 --- a/packages/dd-trace/test/telemetry/logs/index.spec.js +++ b/packages/dd-trace/test/telemetry/logs/index.spec.js @@ -145,7 +145,12 @@ describe('telemetry logs', () => { errorLog.publish({ cause: error }) expect(logCollectorAdd) - .to.be.calledOnceWith(match({ message: `${error.name}: Generic Error`, level: 'ERROR', stack_trace: stack })) + .to.be.calledOnceWith(match({ + message: 'Generic Error', + level: 'ERROR', + errorType: 'Error', + stack_trace: stack + })) }) it('should be called when an error string is published to datadog:log:error', () => { diff --git a/packages/dd-trace/test/telemetry/logs/log-collector.spec.js b/packages/dd-trace/test/telemetry/logs/log-collector.spec.js index 05ce21f65f6..6f4d5bbb9d6 100644 --- a/packages/dd-trace/test/telemetry/logs/log-collector.spec.js +++ b/packages/dd-trace/test/telemetry/logs/log-collector.spec.js @@ -54,7 +54,7 @@ describe('telemetry log collector', () => { it('should include original message and dd frames', () => { const ddFrame = `at T (${ddBasePath}packages/dd-trace/test/telemetry/logs/log_collector.spec.js:29:21)` - const stack = new Error('Error 1') + const stack = new TypeError('Error 1') .stack.replace(`Error 1${EOL}`, `Error 1${EOL}${ddFrame}${EOL}`) const ddFrames = stack @@ -63,28 +63,41 @@ describe('telemetry log collector', () => { .map(line => line.replace(ddBasePath, '')) .join(EOL) - expect(logCollector.add({ message: 'Error 1', level: 'ERROR', stack_trace: stack })).to.be.true + expect(logCollector.add({ + message: 'Error 1', + level: 'ERROR', + stack_trace: stack, + errorType: 'TypeError' + })).to.be.true expect(logCollector.hasEntry({ message: 'Error 1', level: 'ERROR', - stack_trace: `Error: Error 1${EOL}${ddFrames}` + stack_trace: `TypeError: Error 1${EOL}${ddFrames}` })).to.be.true }) - it('should include original message if first frame is not a dd frame', () => { + it('should redact stack message if first frame is not a dd frame', () => { const thirdPartyFrame = `at callFn (/this/is/not/a/dd/frame/runnable.js:366:21) at T (${ddBasePath}packages/dd-trace/test/telemetry/logs/log_collector.spec.js:29:21)` - const stack = new Error('Error 1') + const stack = new TypeError('Error 1') .stack.replace(`Error 1${EOL}`, `Error 1${EOL}${thirdPartyFrame}${EOL}`) - const ddFrames = stack - .split(EOL) - .filter(line => line.includes(ddBasePath)) - .map(line => line.replace(ddBasePath, '')) - .join(EOL) + const ddFrames = [ + 'TypeError: redacted', + ...stack + .split(EOL) + .filter(line => line.includes(ddBasePath)) + .map(line => line.replace(ddBasePath, '')) + ].join(EOL) + + expect(logCollector.add({ + message: 'Error 1', + level: 'ERROR', + stack_trace: stack, + errorType: 'TypeError' + })).to.be.true - expect(logCollector.add({ message: 'Error 1', level: 'ERROR', stack_trace: stack })).to.be.true expect(logCollector.hasEntry({ message: 'Error 1', level: 'ERROR', From 1c253ece2519f6beca073ca4525095b55171437d Mon Sep 17 00:00:00 2001 From: Igor Unanua Date: Fri, 29 Nov 2024 16:00:29 +0100 Subject: [PATCH 10/11] A bunch more messages --- .../src/helpers/bundler-register.js | 2 +- packages/datadog-instrumentations/src/jest.js | 6 +++--- packages/datadog-instrumentations/src/playwright.js | 4 ++-- .../datadog-plugin-aws-sdk/src/services/eventbridge.js | 2 +- .../datadog-plugin-aws-sdk/src/services/kinesis.js | 2 +- packages/datadog-plugin-aws-sdk/src/services/lambda.js | 2 +- packages/datadog-plugin-aws-sdk/src/services/sqs.js | 2 +- packages/datadog-plugin-oracledb/src/index.js | 2 +- packages/datadog-shimmer/src/shimmer.js | 4 ++-- .../src/ci-visibility/dynamic-instrumentation/index.js | 4 ++-- .../dd-trace/src/debugger/devtools_client/config.js | 2 +- .../dd-trace/src/debugger/devtools_client/index.js | 2 +- .../src/debugger/devtools_client/remote_config.js | 2 +- .../dd-trace/src/debugger/devtools_client/status.js | 2 +- packages/dd-trace/src/debugger/index.js | 8 ++++---- packages/dd-trace/src/plugins/util/git.js | 10 +++++----- packages/dd-trace/src/plugins/util/web.js | 4 ++-- 17 files changed, 30 insertions(+), 30 deletions(-) diff --git a/packages/datadog-instrumentations/src/helpers/bundler-register.js b/packages/datadog-instrumentations/src/helpers/bundler-register.js index 491a0ba0600..6c11329bc36 100644 --- a/packages/datadog-instrumentations/src/helpers/bundler-register.js +++ b/packages/datadog-instrumentations/src/helpers/bundler-register.js @@ -47,7 +47,7 @@ dc.subscribe(CHANNEL, (payload) => { loadChannel.publish({ name, version: payload.version, file }) payload.module = hook(payload.module, payload.version) } catch (e) { - log.error(e) + log.error('Error executing bundler hook', e) } } }) diff --git a/packages/datadog-instrumentations/src/jest.js b/packages/datadog-instrumentations/src/jest.js index 0841ab4783a..fd13d2fc805 100644 --- a/packages/datadog-instrumentations/src/jest.js +++ b/packages/datadog-instrumentations/src/jest.js @@ -451,7 +451,7 @@ function cliWrapper (cli, jestVersion) { earlyFlakeDetectionFaultyThreshold = libraryConfig.earlyFlakeDetectionFaultyThreshold } } catch (err) { - log.error(err) + log.error('Jest library configuration error', err) } if (isEarlyFlakeDetectionEnabled) { @@ -472,7 +472,7 @@ function cliWrapper (cli, jestVersion) { isEarlyFlakeDetectionEnabled = false } } catch (err) { - log.error(err) + log.error('Jest known tests error', err) } } @@ -491,7 +491,7 @@ function cliWrapper (cli, jestVersion) { skippableSuites = receivedSkippableSuites } } catch (err) { - log.error(err) + log.error('Jest test-suite skippable error', err) } } diff --git a/packages/datadog-instrumentations/src/playwright.js b/packages/datadog-instrumentations/src/playwright.js index e8332d65c8d..ea6fc822a43 100644 --- a/packages/datadog-instrumentations/src/playwright.js +++ b/packages/datadog-instrumentations/src/playwright.js @@ -412,7 +412,7 @@ function runnerHook (runnerExport, playwrightVersion) { } } catch (e) { isEarlyFlakeDetectionEnabled = false - log.error(e) + log.error('Playwright session start error', e) } if (isEarlyFlakeDetectionEnabled && semver.gte(playwrightVersion, MINIMUM_SUPPORTED_VERSION_EFD)) { @@ -425,7 +425,7 @@ function runnerHook (runnerExport, playwrightVersion) { } } catch (err) { isEarlyFlakeDetectionEnabled = false - log.error(err) + log.error('Playwright known tests error', err) } } diff --git a/packages/datadog-plugin-aws-sdk/src/services/eventbridge.js b/packages/datadog-plugin-aws-sdk/src/services/eventbridge.js index b316f75e6be..a5ca5f08de1 100644 --- a/packages/datadog-plugin-aws-sdk/src/services/eventbridge.js +++ b/packages/datadog-plugin-aws-sdk/src/services/eventbridge.js @@ -45,7 +45,7 @@ class EventBridge extends BaseAwsSdkPlugin { } request.params.Entries[0].Detail = finalData } catch (e) { - log.error(e) + log.error('EventBridge error injecting request', e) } } } diff --git a/packages/datadog-plugin-aws-sdk/src/services/kinesis.js b/packages/datadog-plugin-aws-sdk/src/services/kinesis.js index 64a67d768ea..cdbd7c077e9 100644 --- a/packages/datadog-plugin-aws-sdk/src/services/kinesis.js +++ b/packages/datadog-plugin-aws-sdk/src/services/kinesis.js @@ -97,7 +97,7 @@ class Kinesis extends BaseAwsSdkPlugin { parsedAttributes: decodedData._datadog } } catch (e) { - log.error(e) + log.error('Kinesis error extracting response', e) } } diff --git a/packages/datadog-plugin-aws-sdk/src/services/lambda.js b/packages/datadog-plugin-aws-sdk/src/services/lambda.js index f6ea874872e..b5fe1981c20 100644 --- a/packages/datadog-plugin-aws-sdk/src/services/lambda.js +++ b/packages/datadog-plugin-aws-sdk/src/services/lambda.js @@ -43,7 +43,7 @@ class Lambda extends BaseAwsSdkPlugin { const newContextBase64 = Buffer.from(JSON.stringify(clientContext)).toString('base64') request.params.ClientContext = newContextBase64 } catch (err) { - log.error(err) + log.error('Lambda error injecting request', err) } } } diff --git a/packages/datadog-plugin-aws-sdk/src/services/sqs.js b/packages/datadog-plugin-aws-sdk/src/services/sqs.js index e3a76c3e0b9..9857e46bf28 100644 --- a/packages/datadog-plugin-aws-sdk/src/services/sqs.js +++ b/packages/datadog-plugin-aws-sdk/src/services/sqs.js @@ -163,7 +163,7 @@ class Sqs extends BaseAwsSdkPlugin { return JSON.parse(buffer) } } catch (e) { - log.error(e) + log.error('Sqs error parsing DD attributes', e) } } diff --git a/packages/datadog-plugin-oracledb/src/index.js b/packages/datadog-plugin-oracledb/src/index.js index 7c2f1da029f..eb4fa037cac 100644 --- a/packages/datadog-plugin-oracledb/src/index.js +++ b/packages/datadog-plugin-oracledb/src/index.js @@ -33,7 +33,7 @@ function getUrl (connectString) { try { return new URL(`http://${connectString}`) } catch (e) { - log.error(e) + log.error('Invalid oracle connection string', e) return {} } } diff --git a/packages/datadog-shimmer/src/shimmer.js b/packages/datadog-shimmer/src/shimmer.js index d12c4c130ef..0285c5e5083 100644 --- a/packages/datadog-shimmer/src/shimmer.js +++ b/packages/datadog-shimmer/src/shimmer.js @@ -136,7 +136,7 @@ function wrapMethod (target, name, wrapper, noAssert) { if (callState.completed) { // error was thrown after original function returned/resolved, so // it was us. log it. - log.error(e) + log.error('Shimmer error was thrown after original function returned/resolved', e) // original ran and returned something. return it. return callState.retVal } @@ -144,7 +144,7 @@ function wrapMethod (target, name, wrapper, noAssert) { if (!callState.called) { // error was thrown before original function was called, so // it was us. log it. - log.error(e) + log.error('Shimmer error was thrown before original function was called', e) // original never ran. call it unwrapped. return original.apply(this, args) } diff --git a/packages/dd-trace/src/ci-visibility/dynamic-instrumentation/index.js b/packages/dd-trace/src/ci-visibility/dynamic-instrumentation/index.js index ef65489e60d..ec6e2a1fd75 100644 --- a/packages/dd-trace/src/ci-visibility/dynamic-instrumentation/index.js +++ b/packages/dd-trace/src/ci-visibility/dynamic-instrumentation/index.js @@ -90,8 +90,8 @@ class TestVisDynamicInstrumentation { } }).unref() - this.worker.on('error', (err) => log.error(err)) - this.worker.on('messageerror', (err) => log.error(err)) + this.worker.on('error', (err) => log.error('ci-visibility DI worker error', err)) + this.worker.on('messageerror', (err) => log.error('ci-visibility DI worker messageerror', err)) } } diff --git a/packages/dd-trace/src/debugger/devtools_client/config.js b/packages/dd-trace/src/debugger/devtools_client/config.js index 838a1a76cca..fa48779f313 100644 --- a/packages/dd-trace/src/debugger/devtools_client/config.js +++ b/packages/dd-trace/src/debugger/devtools_client/config.js @@ -15,7 +15,7 @@ const config = module.exports = { updateUrl(parentConfig) configPort.on('message', updateUrl) -configPort.on('messageerror', (err) => log.error(err)) +configPort.on('messageerror', (err) => log.error('Debugger config messageerror', err)) function updateUrl (updates) { config.url = updates.url || format({ diff --git a/packages/dd-trace/src/debugger/devtools_client/index.js b/packages/dd-trace/src/debugger/devtools_client/index.js index db71e7028e7..116688c2183 100644 --- a/packages/dd-trace/src/debugger/devtools_client/index.js +++ b/packages/dd-trace/src/debugger/devtools_client/index.js @@ -93,7 +93,7 @@ session.on('Debugger.paused', async ({ params }) => { // TODO: Process template (DEBUG-2628) send(probe.template, logger, snapshot, (err) => { - if (err) log.error(err) + if (err) log.error('Debugger error', err) else ackEmitting(probe) }) } diff --git a/packages/dd-trace/src/debugger/devtools_client/remote_config.js b/packages/dd-trace/src/debugger/devtools_client/remote_config.js index 8a7d7386e33..3db13dc39a6 100644 --- a/packages/dd-trace/src/debugger/devtools_client/remote_config.js +++ b/packages/dd-trace/src/debugger/devtools_client/remote_config.js @@ -44,7 +44,7 @@ rcPort.on('message', async ({ action, conf: probe, ackId }) => { ackError(err, probe) } }) -rcPort.on('messageerror', (err) => log.error(err)) +rcPort.on('messageerror', (err) => log.error('Debugger RC message error', err)) async function start () { sessionStarted = true diff --git a/packages/dd-trace/src/debugger/devtools_client/status.js b/packages/dd-trace/src/debugger/devtools_client/status.js index b28fc3c281e..3fb41b63b11 100644 --- a/packages/dd-trace/src/debugger/devtools_client/status.js +++ b/packages/dd-trace/src/debugger/devtools_client/status.js @@ -55,7 +55,7 @@ function ackEmitting ({ id: probeId, version }) { } function ackError (err, { id: probeId, version }) { - log.error(err) + log.error('Debugger ackError', err) onlyUniqueUpdates(STATUSES.ERROR, probeId, version, () => { const payload = statusPayload(probeId, version, STATUSES.ERROR) diff --git a/packages/dd-trace/src/debugger/index.js b/packages/dd-trace/src/debugger/index.js index 8e3fa56c2e8..cdfa2744451 100644 --- a/packages/dd-trace/src/debugger/index.js +++ b/packages/dd-trace/src/debugger/index.js @@ -40,7 +40,7 @@ function start (config, rc) { ack(error) rcAckCallbacks.delete(ackId) }) - rcChannel.port2.on('messageerror', (err) => log.error(err)) + rcChannel.port2.on('messageerror', (err) => log.error('Debugger RC messageerror', err)) worker = new Worker( join(__dirname, 'devtools_client', 'index.js'), @@ -63,13 +63,13 @@ function start (config, rc) { log.debug(`Dynamic Instrumentation worker thread started successfully (thread id: ${worker.threadId})`) }) - worker.on('error', (err) => log.error(err)) - worker.on('messageerror', (err) => log.error(err)) + worker.on('error', (err) => log.error('Debugger worker error', err)) + worker.on('messageerror', (err) => log.error('Debugger worker messageerror', err)) worker.on('exit', (code) => { const error = new Error(`Dynamic Instrumentation worker thread exited unexpectedly with code ${code}`) - log.error(error) + log.error('Debugger worker exited unexpectedly', error) // Be nice, clean up now that the worker thread encounted an issue and we can't continue rc.removeProductHandler('LIVE_DEBUGGING') diff --git a/packages/dd-trace/src/plugins/util/git.js b/packages/dd-trace/src/plugins/util/git.js index 841755e9246..47707a48679 100644 --- a/packages/dd-trace/src/plugins/util/git.js +++ b/packages/dd-trace/src/plugins/util/git.js @@ -61,7 +61,7 @@ function sanitizedExec ( exitCode: err.status || err.errno }) } - log.error(err) + log.error('Git plugin error executing command', err) return '' } finally { storage.enterWith(store) @@ -144,7 +144,7 @@ function unshallowRepository () { ], { stdio: 'pipe' }) } catch (err) { // If the local HEAD is a commit that has not been pushed to the remote, the above command will fail. - log.error(err) + log.error('Git plugin error executing git command', err) incrementCountMetric( TELEMETRY_GIT_COMMAND_ERRORS, { command: 'unshallow', errorType: err.code, exitCode: err.status || err.errno } @@ -157,7 +157,7 @@ function unshallowRepository () { ], { stdio: 'pipe' }) } catch (err) { // If the CI is working on a detached HEAD or branch tracking hasn’t been set up, the above command will fail. - log.error(err) + log.error('Git plugin error executing fallback git command', err) incrementCountMetric( TELEMETRY_GIT_COMMAND_ERRORS, { command: 'unshallow', errorType: err.code, exitCode: err.status || err.errno } @@ -272,7 +272,7 @@ function generatePackFilesForCommits (commitsToUpload) { try { result = execGitPackObjects(temporaryPath) } catch (err) { - log.error(err) + log.error('Git plugin error executing git pack-objects command', err) incrementCountMetric( TELEMETRY_GIT_COMMAND_ERRORS, { command: 'pack_objects', exitCode: err.status || err.errno, errorType: err.code } @@ -292,7 +292,7 @@ function generatePackFilesForCommits (commitsToUpload) { try { result = execGitPackObjects(cwdPath) } catch (err) { - log.error(err) + log.error('Git plugin error executing fallback git pack-objects command', err) incrementCountMetric( TELEMETRY_GIT_COMMAND_ERRORS, { command: 'pack_objects', exitCode: err.status || err.errno, errorType: err.code } diff --git a/packages/dd-trace/src/plugins/util/web.js b/packages/dd-trace/src/plugins/util/web.js index 5bfb1d6fad4..2d92c74ea91 100644 --- a/packages/dd-trace/src/plugins/util/web.js +++ b/packages/dd-trace/src/plugins/util/web.js @@ -546,7 +546,7 @@ function getHeadersToRecord (config) { .map(h => h.split(':')) .map(([key, tag]) => [key.toLowerCase(), tag]) } catch (err) { - log.error(err) + log.error('Web plugin error getting headers', err) } } else if (config.hasOwnProperty('headers')) { log.error('Expected `headers` to be an array of strings.') @@ -595,7 +595,7 @@ function getQsObfuscator (config) { try { return new RegExp(obfuscator, 'gi') } catch (err) { - log.error(err) + log.error('Web plugin error getting qs obfuscator', err) } } From 5b14d579b4c12643bac747d3900be031f49900ab Mon Sep 17 00:00:00 2001 From: Igor Unanua Date: Fri, 29 Nov 2024 16:14:09 +0100 Subject: [PATCH 11/11] Missing cypress plugin messages --- packages/datadog-plugin-cypress/src/cypress-plugin.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/datadog-plugin-cypress/src/cypress-plugin.js b/packages/datadog-plugin-cypress/src/cypress-plugin.js index 0a7d0debe48..2ed62070fda 100644 --- a/packages/datadog-plugin-cypress/src/cypress-plugin.js +++ b/packages/datadog-plugin-cypress/src/cypress-plugin.js @@ -223,7 +223,7 @@ class CypressPlugin { this.libraryConfigurationPromise = getLibraryConfiguration(this.tracer, this.testConfiguration) .then((libraryConfigurationResponse) => { if (libraryConfigurationResponse.err) { - log.error(libraryConfigurationResponse.err) + log.error('Cypress plugin library config response error', libraryConfigurationResponse.err) } else { const { libraryConfig: { @@ -360,7 +360,7 @@ class CypressPlugin { this.testConfiguration ) if (knownTestsResponse.err) { - log.error(knownTestsResponse.err) + log.error('Cypress known tests response error', knownTestsResponse.err) this.isEarlyFlakeDetectionEnabled = false } else { // We use TEST_FRAMEWORK_NAME for the name of the module @@ -374,7 +374,7 @@ class CypressPlugin { this.testConfiguration ) if (skippableTestsResponse.err) { - log.error(skippableTestsResponse.err) + log.error('Cypress skippable tests response error', skippableTestsResponse.err) } else { const { skippableTests, correlationId } = skippableTestsResponse this.testsToSkip = skippableTests || []