Skip to content

Commit 494217f

Browse files
authored
fix log publish format (#7293)
1 parent 5b46416 commit 494217f

File tree

4 files changed

+64
-116
lines changed

4 files changed

+64
-116
lines changed

packages/dd-trace/src/log/index.js

Lines changed: 45 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -31,31 +31,31 @@ const log = {
3131
use (logger) {
3232
config.logger = logger
3333
logWriter.use(logger)
34-
return this
34+
return log
3535
},
3636

3737
toggle (enabled, logLevel) {
3838
config.enabled = enabled
3939
config.logLevel = logLevel
4040
logWriter.toggle(enabled, logLevel)
41-
return this
41+
return log
4242
},
4343

4444
reset () {
4545
logWriter.reset()
46-
this._deprecate = memoize((code, message) => {
47-
errorChannel.publish(Log.parse(message))
46+
log._deprecate = memoize((code, message) => {
47+
publishFormatted(errorChannel, null, message)
4848
return true
4949
})
5050

51-
return this
51+
return log
5252
},
5353

5454
trace (...args) {
5555
if (traceChannel.hasSubscribers) {
5656
const logRecord = {}
5757

58-
Error.captureStackTrace(logRecord, this.trace)
58+
Error.captureStackTrace(logRecord, log.trace)
5959

6060
const stack = logRecord.stack.split('\n')
6161
const fn = stack[1].replace(/^\s+at ([^\s]+) .+/, '$1')
@@ -64,49 +64,46 @@ const log = {
6464

6565
stack[0] = `Trace: ${fn}(${params})`
6666

67-
traceChannel.publish(Log.parse(stack.join('\n')))
67+
publishFormatted(traceChannel, null, stack.join('\n'))
6868
}
69-
return this
69+
return log
7070
},
7171

7272
debug (...args) {
73-
if (debugChannel.hasSubscribers) {
74-
debugChannel.publish(Log.parse(...args))
75-
}
76-
return this
73+
publishFormatted(debugChannel, null, ...args)
74+
return log
7775
},
7876

7977
info (...args) {
80-
if (infoChannel.hasSubscribers) {
81-
infoChannel.publish(Log.parse(...args))
82-
}
83-
return this
78+
publishFormatted(infoChannel, null, ...args)
79+
return log
8480
},
8581

8682
warn (...args) {
87-
if (warnChannel.hasSubscribers) {
88-
warnChannel.publish(Log.parse(...args))
89-
}
90-
return this
83+
publishFormatted(warnChannel, null, ...args)
84+
return log
9185
},
9286

9387
error (...args) {
94-
if (errorChannel.hasSubscribers) {
95-
errorChannel.publish(Log.parse(...args))
96-
}
97-
return this
88+
publishFormatted(errorChannel, formatted => {
89+
const stackTraceLimitBackup = Error.stackTraceLimit
90+
Error.stackTraceLimit = 0
91+
const newError = new Error(formatted)
92+
Error.stackTraceLimit = stackTraceLimitBackup
93+
Error.captureStackTrace(newError, log.error)
94+
return newError
95+
}, ...args)
96+
return log
9897
},
9998

10099
errorWithoutTelemetry (...args) {
101100
args.push(NO_TRANSMIT)
102-
if (errorChannel.hasSubscribers) {
103-
errorChannel.publish(Log.parse(...args))
104-
}
105-
return this
101+
publishFormatted(errorChannel, null, ...args)
102+
return log
106103
},
107104

108105
deprecate (code, message) {
109-
return this._deprecate(code, message)
106+
return log._deprecate(code, message)
110107
},
111108

112109
isEnabled (fleetStableConfigValue, localStableConfigValue) {
@@ -133,7 +130,25 @@ const log = {
133130
}
134131
}
135132

136-
logWriter.setStackTraceLimitFunction(log.error)
133+
function publishFormatted (ch, formatter, ...args) {
134+
if (ch.hasSubscribers) {
135+
const log = Log.parse(...args)
136+
const { formatted, cause } = getErrorLog(log)
137+
138+
// calling twice ch.publish() because Error cause is only available in Node.js v16.9.0
139+
// TODO: replace it with Error(message, { cause }) when cause has broad support
140+
if (formatted) ch.publish(formatter?.(formatted) || formatted)
141+
if (cause) ch.publish(cause)
142+
}
143+
}
144+
145+
function getErrorLog (err) {
146+
if (typeof err?.delegate === 'function') {
147+
const result = err.delegate()
148+
return Array.isArray(result) ? Log.parse(...result) : Log.parse(result)
149+
}
150+
return err
151+
}
137152

138153
log.reset()
139154

packages/dd-trace/src/log/writer.js

Lines changed: 13 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22

33
const { storage } = require('../../../datadog-core')
44
const { LogChannel } = require('./channels')
5-
const { Log } = require('./log')
65
const defaultLogger = {
76
debug: msg => console.debug(msg), /* eslint-disable-line no-console */
87
info: msg => console.info(msg), /* eslint-disable-line no-console */
@@ -13,7 +12,6 @@ const defaultLogger = {
1312
let enabled = false
1413
let logger = defaultLogger
1514
let logChannel = new LogChannel()
16-
let stackTraceLimitFunction = onError
1715

1816
function withNoop (fn) {
1917
const store = storage('legacy').getStore()
@@ -24,15 +22,15 @@ function withNoop (fn) {
2422
}
2523

2624
function unsubscribeAll () {
27-
logChannel.unsubscribe({ trace: onTrace, debug: onDebug, info: onInfo, warn: onWarn, error: onError })
25+
logChannel.unsubscribe({ trace, debug, info, warn, error })
2826
}
2927

3028
function toggleSubscription (enable, level) {
3129
unsubscribeAll()
3230

3331
if (enable) {
3432
logChannel = new LogChannel(level)
35-
logChannel.subscribe({ trace: onTrace, debug: onDebug, info: onInfo, warn: onWarn, error: onError })
33+
logChannel.subscribe({ trace, debug, info, warn, error })
3634
}
3735
}
3836

@@ -53,89 +51,26 @@ function reset () {
5351
toggleSubscription(false)
5452
}
5553

56-
function getErrorLog (err) {
57-
if (typeof err?.delegate === 'function') {
58-
const result = err.delegate()
59-
return Array.isArray(result) ? Log.parse(...result) : Log.parse(result)
60-
}
61-
return err
62-
}
63-
64-
function setStackTraceLimitFunction (fn) {
65-
if (typeof fn !== 'function') {
66-
throw new TypeError('stackTraceLimitFunction must be a function')
67-
}
68-
stackTraceLimitFunction = fn
69-
}
70-
71-
function onError (err) {
72-
const { formatted, cause } = getErrorLog(err)
73-
74-
// calling twice logger.error() because Error cause is only available in Node.js v16.9.0
75-
// TODO: replace it with Error(message, { cause }) when cause has broad support
76-
if (formatted) {
77-
withNoop(() => {
78-
const stackTraceLimitBackup = Error.stackTraceLimit
79-
Error.stackTraceLimit = 0
80-
const newError = new Error(formatted)
81-
Error.stackTraceLimit = stackTraceLimitBackup
82-
Error.captureStackTrace(newError, stackTraceLimitFunction)
83-
logger.error(newError)
84-
})
85-
}
86-
if (cause) withNoop(() => logger.error(cause))
54+
function error (err) {
55+
withNoop(() => logger.error(err))
8756
}
8857

89-
function onWarn (log) {
90-
const { formatted, cause } = getErrorLog(log)
91-
if (formatted) withNoop(() => logger.warn(formatted))
92-
if (cause) withNoop(() => logger.warn(cause))
58+
function warn (log) {
59+
withNoop(() => logger.warn ? logger.warn(log) : logger.debug(log))
9360
}
9461

95-
function onInfo (log) {
96-
const { formatted, cause } = getErrorLog(log)
97-
if (formatted) withNoop(() => logger.info(formatted))
98-
if (cause) withNoop(() => logger.info(cause))
62+
function info (log) {
63+
withNoop(() => logger.info ? logger.info(log) : logger.debug(log))
9964
}
10065

101-
function onDebug (log) {
102-
const { formatted, cause } = getErrorLog(log)
103-
if (formatted) withNoop(() => logger.debug(formatted))
104-
if (cause) withNoop(() => logger.debug(cause))
66+
function debug (log) {
67+
withNoop(() => logger.debug(log))
10568
}
10669

107-
function onTrace (log) {
108-
const { formatted, cause } = getErrorLog(log)
70+
function trace (log) {
10971
// Using logger.debug() because not all loggers have trace level,
11072
// and console.trace() has a completely different meaning.
111-
if (formatted) withNoop(() => logger.debug(formatted))
112-
if (cause) withNoop(() => logger.debug(cause))
113-
}
114-
115-
function error (...args) {
116-
onError(Log.parse(...args))
117-
}
118-
119-
function warn (...args) {
120-
const log = Log.parse(...args)
121-
if (!logger.warn) return onDebug(log)
122-
123-
onWarn(log)
124-
}
125-
126-
function info (...args) {
127-
const log = Log.parse(...args)
128-
if (!logger.info) return onDebug(log)
129-
130-
onInfo(log)
131-
}
132-
133-
function debug (...args) {
134-
onDebug(Log.parse(...args))
135-
}
136-
137-
function trace (...args) {
138-
onTrace(Log.parse(...args))
73+
withNoop(() => logger.debug(log))
13974
}
14075

141-
module.exports = { use, toggle, reset, error, warn, info, debug, trace, setStackTraceLimitFunction }
76+
module.exports = { use, toggle, reset, error, warn, info, debug, trace }

packages/dd-trace/test/flare.spec.js

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,18 +3,16 @@
33
const assert = require('node:assert/strict')
44
const http = require('node:http')
55

6-
const { channel } = require('dc-polyfill')
76
const express = require('express')
87
const upload = require('multer')()
98
const proxyquire = require('proxyquire').noCallThru()
109
const { describe, it, beforeEach, afterEach } = require('mocha')
1110

1211
const { assertObjectContains } = require('../../../integration-tests/helpers')
1312
require('./setup/core')
13+
const log = require('../src/log')
1414
const { getConfigFresh } = require('./helpers/config')
1515

16-
const debugChannel = channel('datadog:log:debug')
17-
1816
describe('Flare', () => {
1917
let flare
2018
let startupLog
@@ -155,9 +153,9 @@ describe('Flare', () => {
155153
flare.enable(tracerConfig)
156154
flare.prepare('debug')
157155

158-
debugChannel.publish('foo')
159-
debugChannel.publish('bar')
160-
debugChannel.publish({ foo: 'bar' })
156+
log.debug('foo')
157+
log.debug('bar')
158+
log.debug(JSON.stringify({ foo: 'bar' }))
161159

162160
flare.send(task)
163161
})

packages/dd-trace/test/log.spec.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -457,8 +457,8 @@ describe('log', () => {
457457

458458
sinon.assert.calledOnce(console.error)
459459
const consoleErrorArg = console.error.getCall(0).args[0]
460-
assert.strictEqual(typeof consoleErrorArg, 'object')
461-
assert.strictEqual(consoleErrorArg.message, 'message')
460+
assert.strictEqual(typeof consoleErrorArg, 'string')
461+
assert.strictEqual(consoleErrorArg, 'message')
462462
})
463463

464464
it('should only log once for a given code', () => {

0 commit comments

Comments
 (0)