diff --git a/packages/dd-trace/src/exporters/agent/writer.js b/packages/dd-trace/src/exporters/agent/writer.js index 9e3fa008ba6..fd903e27429 100644 --- a/packages/dd-trace/src/exporters/agent/writer.js +++ b/packages/dd-trace/src/exporters/agent/writer.js @@ -42,7 +42,7 @@ class Writer extends BaseWriter { startupLog({ agentError: err }) if (err) { - log.error('Error sending payload to the agent (status code: %s)', err.status, err) + log.error('Error sending payload to the agent (status code: %s)', err.status, err, log.NO_TELEMETRY) done() return } diff --git a/packages/dd-trace/src/exporters/common/request.js b/packages/dd-trace/src/exporters/common/request.js index 9b5cc05d08e..4c262818b43 100644 --- a/packages/dd-trace/src/exporters/common/request.js +++ b/packages/dd-trace/src/exporters/common/request.js @@ -96,22 +96,25 @@ function request (data, options, callback) { callback(null, buffer.toString(), res.statusCode) } } else { - let errorMessage = '' + let logArgs = [''] // use with log.error(...logArgs), it's a sprintf string with placeholder args try { const fullUrl = new URL( options.path, options.url || options.hostname || `http://localhost:${options.port}` ).href - errorMessage = `Error from ${fullUrl}: ${res.statusCode} ${http.STATUS_CODES[res.statusCode]}.` + logArgs[0] += 'Error from agent, url=%s, status=%s %s' + logArgs.push(fullUrl, res.statusCode, http.STATUS_CODES[res.statusCode]) } catch { // ignore error } const responseData = buffer.toString() if (responseData) { - errorMessage += ` Response from the endpoint: "${responseData}"` + logArgs[0] += ', response=%s' + logArgs.push(responseData.substring(0, 100).replaceAll(/[\r\n]/g, ' ')) // can be a 1MB HTML document from an nginx proxy } - const error = new Error(errorMessage) + const error = new Error('error from agent. DO NOT PASS TO log.error() INSTEAD USE log.error(...error.logArgs)') // TODO DON'T MERGE THIS error.status = res.statusCode + error.logArgs = logArgs callback(error, null, res.statusCode) } diff --git a/packages/dd-trace/src/log/index.js b/packages/dd-trace/src/log/index.js index 85b625a10ab..f9f68cb0563 100644 --- a/packages/dd-trace/src/log/index.js +++ b/packages/dd-trace/src/log/index.js @@ -5,7 +5,7 @@ const { inspect } = require('util') const { isTrue } = require('../util') const { traceChannel, debugChannel, infoChannel, warnChannel, errorChannel } = require('./channels') const logWriter = require('./writer') -const { Log } = require('./log') +const { Log, LogConfig } = require('./log') const { memoize } = require('./utils') const { getEnvironmentVariable } = require('../config-helper') @@ -15,7 +15,12 @@ const config = { logLevel: 'debug' } +const NO_TRANSMIT = new LogConfig(false) + const log = { + LogConfig, + NO_TRANSMIT, + /** * @returns Read-only version of logging config. To modify config, call `log.use` and `log.toggle` */ @@ -86,6 +91,7 @@ const log = { }, error (...args) { + console.log('log.error', ...args) if (errorChannel.hasSubscribers) { errorChannel.publish(Log.parse(...args)) } diff --git a/packages/dd-trace/src/log/log.js b/packages/dd-trace/src/log/log.js index dafc5b79b0f..ff79e774b00 100644 --- a/packages/dd-trace/src/log/log.js +++ b/packages/dd-trace/src/log/log.js @@ -3,11 +3,12 @@ const { format } = require('util') class Log { - constructor (message, args, cause, delegate) { + constructor (message, args, cause, delegate, sendViaTelemetry = true) { this.message = message this.args = args this.cause = cause this.delegate = delegate + this.sendViaTelemetry = sendViaTelemetry } get formatted () { @@ -22,10 +23,21 @@ class Log { static parse (...args) { let message, cause, delegate + let sendViaTelemetry = true - const lastArg = args.at(-1) - if (lastArg && typeof lastArg === 'object' && lastArg.stack) { // lastArg instanceof Error? - cause = args.pop() + { + const lastArg = args.at(-1) + if (lastArg instanceof LogConfig) { + args.pop() + sendViaTelemetry = lastArg.transmit + } + } + + { + const lastArg = args.at(-1) + if (lastArg && typeof lastArg === 'object' && lastArg.stack) { // lastArg instanceof Error? + cause = args.pop() + } } const firstArg = args.shift() @@ -43,10 +55,21 @@ class Log { message = String(firstArg) } - return new Log(message, args, cause, delegate) + return new Log(message, args, cause, delegate, sendViaTelemetry) + } +} + +/** + * Pass instances of this class to logger methods when fine-grain control is needed + * @property {boolean} transmit - Whether to send the log via telemetry. + */ +class LogConfig { + constructor (transmit = true) { + this.transmit = transmit } } module.exports = { - Log + Log, + LogConfig } diff --git a/packages/dd-trace/src/remote_config/manager.js b/packages/dd-trace/src/remote_config/manager.js index a95f281fb86..e00aebd9f1d 100644 --- a/packages/dd-trace/src/remote_config/manager.js +++ b/packages/dd-trace/src/remote_config/manager.js @@ -153,7 +153,7 @@ class RemoteConfigManager extends EventEmitter { if (statusCode === 404) return cb() if (err) { - log.error('[RC] Error in request', err) + log.error('[RC] Error in request', err, log.NO_TELEMETRY) return cb() } diff --git a/packages/dd-trace/src/telemetry/logs/log-collector.js b/packages/dd-trace/src/telemetry/logs/log-collector.js index 35b9383c181..bc9c74442be 100644 --- a/packages/dd-trace/src/telemetry/logs/log-collector.js +++ b/packages/dd-trace/src/telemetry/logs/log-collector.js @@ -27,7 +27,8 @@ function createHash (logEntry) { } function isValid (logEntry) { - return logEntry?.level && logEntry.message + console.log('isValid', logEntry) + return logEntry?.level && logEntry.message && logEntry.transmit !== false } const EOL = '\n' diff --git a/packages/dd-trace/test/exporters/agent/writer.spec.js b/packages/dd-trace/test/exporters/agent/writer.spec.js index aad8749ef37..1979fd5f45f 100644 --- a/packages/dd-trace/test/exporters/agent/writer.spec.js +++ b/packages/dd-trace/test/exporters/agent/writer.spec.js @@ -158,7 +158,8 @@ function describeWriter (protocolVersion) { setTimeout(() => { expect(log.error) - .to.have.been.calledWith('Error sending payload to the agent (status code: %s)', error.status, error) + .to.have.been.calledWith('Error sending payload to the agent (status code: %s)', + error.status, error, log.NO_TELEMETRY) done() }) }) diff --git a/packages/dd-trace/test/log.spec.js b/packages/dd-trace/test/log.spec.js index 0221249cda4..2751df3af28 100644 --- a/packages/dd-trace/test/log.spec.js +++ b/packages/dd-trace/test/log.spec.js @@ -245,6 +245,16 @@ describe('log', () => { expect(console.error.secondCall.args[0]).to.be.instanceof(Error) expect(console.error.secondCall.args[0]).to.have.property('message', 'cause') }) + + it('should work with a log config object', () => { + log.error('this is an error', new Error('cause'), log.NO_TELEMETRY) + + expect(console.error).to.have.been.called + expect(console.error.firstCall.args[0]).to.be.instanceof(Error) + expect(console.error.firstCall.args[0]).to.have.property('message', 'this is an error') + expect(console.error.secondCall.args[0]).to.be.instanceof(Error) + expect(console.error.secondCall.args[0]).to.have.property('message', 'cause') + }) }) describe('toggle', () => { 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 7185fe47151..2ea606b5d18 100644 --- a/packages/dd-trace/test/telemetry/logs/log-collector.spec.js +++ b/packages/dd-trace/test/telemetry/logs/log-collector.spec.js @@ -49,6 +49,12 @@ describe('telemetry log collector', () => { ).to.be.false }) + it('should not store logs with transmit flag set to false', () => { + expect(logCollector.add({ message: 'Error 1', level: 'ERROR' })).to.be.true + expect(logCollector.add({ message: 'Error 2', level: 'ERROR', transmit: true })).to.be.true + expect(logCollector.add({ message: 'Error 3', level: 'ERROR', transmit: false })).to.be.false + }) + it('should include original message and dd frames', () => { const ddFrame = `at T (${ddBasePath}path/to/dd/file.js:1:2)` const stack = new TypeError('Error 1')