Skip to content

Commit f6f8707

Browse files
johnjbartonjohnjbarton
authored
fix(logging): Upgrade to log4js 2.x API. (#2868)
Support log4js 2.x as well as 1.x for configuration. Fixes #2858
1 parent 35f03b3 commit f6f8707

4 files changed

Lines changed: 79 additions & 29 deletions

File tree

‎lib/logger.js‎

Lines changed: 45 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -17,29 +17,45 @@ var constant = require('./constants')
1717
//
1818
// * `logLevel`: *String* Defines the global log level.
1919
// * `colors`: *Boolean* Use colors in the stdout or not.
20-
// * `appenders`: *Array* This will be passed as appenders to log4js
20+
// * `appenders`: *Object* This will be passed as appenders to log4js
2121
// to allow for fine grained configuration of log4js. For more information
2222
// see https://github.com/nomiddlename/log4js-node.
23+
// *Array* is also accepted for backwards compatibility.
2324
var setup = function (level, colors, appenders) {
2425
// Turn color on/off on the console appenders with pattern layout
2526
var pattern = colors ? constant.COLOR_PATTERN : constant.NO_COLOR_PATTERN
26-
27-
// If there are no appenders use the default one
28-
appenders = helper.isDefined(appenders) ? appenders : [constant.CONSOLE_APPENDER]
29-
30-
appenders = appenders.map(function (appender) {
31-
if (appender.type === 'console') {
32-
if (helper.isDefined(appender.layout) && appender.layout.type === 'pattern') {
33-
appender.layout.pattern = pattern
27+
if (appenders) {
28+
// Convert Array to Object for backwards compatibility.
29+
if (appenders['map']) {
30+
if (appenders.length === 0) {
31+
appenders = [constant.CONSOLE_APPENDER]
3432
}
33+
const v1Appenders = appenders
34+
appenders = {}
35+
v1Appenders.forEach(function (appender, index) {
36+
if (appender.type === 'console') {
37+
appenders['console'] = appender
38+
if (helper.isDefined(appender.layout) && appender.layout.type === 'pattern') {
39+
appender.layout.pattern = pattern
40+
}
41+
} else {
42+
appenders[index + ''] = appender
43+
}
44+
return appender
45+
})
3546
}
36-
return appender
37-
})
47+
} else {
48+
appenders = {'console' : constant.CONSOLE_APPENDER}
49+
}
3850

39-
// Pass the values to log4js
40-
log4js.setGlobalLogLevel(level)
4151
log4js.configure({
42-
appenders: appenders
52+
appenders: appenders,
53+
categories: {
54+
'default': {
55+
'appenders': Object.keys(appenders),
56+
'level': level
57+
}
58+
}
4359
})
4460
}
4561

@@ -49,9 +65,10 @@ var setup = function (level, colors, appenders) {
4965
// setupFromConfig(config, appenders)
5066
//
5167
// * `config`: *Object* The configuration object.
52-
// * `appenders`: *Array* This will be passed as appenders to log4js
68+
// * `appenders`: *Object* This will be passed as appenders to log4js
5369
// to allow for fine grained configuration of log4js. For more information
5470
// see https://github.com/nomiddlename/log4js-node.
71+
// *Array* is also accepted for backwards compatibility.
5572
var setupFromConfig = function (config, appenders) {
5673
var useColors = true
5774
var logLevel = constant.LOG_INFO
@@ -66,13 +83,22 @@ var setupFromConfig = function (config, appenders) {
6683
setup(logLevel, useColors, appenders)
6784
}
6885

86+
const loggerCache = {}
87+
6988
// Create a new logger. There are two optional arguments
7089
// * `name`, which defaults to `karma` and
7190
// If the `name = 'socket.io'` this will create a special wrapper
7291
// to be used as a logger for socket.io.
7392
// * `level`, which defaults to the global level.
7493
var create = function (name, level) {
75-
var logger = log4js.getLogger(name || 'karma')
94+
name = name || 'karma'
95+
var logger
96+
if (loggerCache.hasOwnProperty(name)) {
97+
logger = loggerCache[name]
98+
} else {
99+
logger = log4js.getLogger(name)
100+
loggerCache[name] = logger
101+
}
76102
if (helper.isDefined(level)) {
77103
logger.setLevel(level)
78104
}
@@ -84,3 +110,6 @@ var create = function (name, level) {
84110
exports.create = create
85111
exports.setup = setup
86112
exports.setupFromConfig = setupFromConfig
113+
exports._rebindLog4js4testing = function(mockLog4js) {
114+
log4js = mockLog4js
115+
}

‎package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -342,7 +342,7 @@
342342
"http-proxy": "^1.13.0",
343343
"isbinaryfile": "^3.0.0",
344344
"lodash": "^4.17.4",
345-
"log4js": "^1.1.1",
345+
"log4js": "^2.3.9",
346346
"mime": "^1.3.4",
347347
"minimatch": "^3.0.2",
348348
"optimist": "^0.6.1",

‎test/unit/config.spec.js‎

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -71,8 +71,7 @@ describe('config', () => {
7171
var logSpy
7272

7373
beforeEach(() => {
74-
logSpy = sinon.spy()
75-
logger.create('config').on('log', logSpy)
74+
logSpy = sinon.spy(logger.create('config'), 'error')
7675
})
7776

7877
it('should resolve relative basePath to config directory', () => {
@@ -114,19 +113,17 @@ describe('config', () => {
114113
e.parseConfig('/conf/not-exist.js', {})
115114

116115
expect(logSpy).to.have.been.called
117-
var event = logSpy.lastCall.args[0]
118-
expect(event.level.toString()).to.be.equal('ERROR')
119-
expect(event.data).to.be.deep.equal(['File %s does not exist!', '/conf/not-exist.js'])
116+
var event = logSpy.lastCall.args
117+
expect(event).to.be.deep.equal(['File %s does not exist!', '/conf/not-exist.js'])
120118
expect(mocks.process.exit).to.have.been.calledWith(1)
121119
})
122120

123121
it('should throw and log error if invalid file', () => {
124122
e.parseConfig('/conf/invalid.js', {})
125123

126124
expect(logSpy).to.have.been.called
127-
var event = logSpy.lastCall.args[0]
128-
expect(event.level.toString()).to.be.equal('ERROR')
129-
expect(event.data).to.be.deep.equal([
125+
var event = logSpy.lastCall.args
126+
expect(event).to.be.deep.equal([
130127
'Error in config file!\n',
131128
new SyntaxError('Unexpected token =')
132129
])

‎test/unit/logger.spec.js‎

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,19 +3,43 @@ import path from 'path'
33

44
describe('logger', () => {
55
var m
6+
let configuration
67

78
beforeEach(() => {
8-
m = loadFile(path.join(__dirname, '/../../lib/logger.js'))
9+
const mockLog4Js = {
10+
configure: function (config) {
11+
configuration = config
12+
}
13+
}
14+
m = loadFile(path.join(__dirname, '/../../lib/logger.js'), {'log4js': mockLog4Js})
915
})
1016

1117
describe('setup', () => {
12-
it('should allow for configuration via setup() using an array', () => {
18+
it('should allow for configuration via setup() using an array for back-compat', () => {
1319
m.setup('INFO', true, [{
1420
type: 'file',
1521
filename: 'test/unit/test.log'
1622
}])
17-
18-
expect(m.log4js.appenders).to.have.keys(['console', 'file', 'stdout'])
23+
expect(configuration).to.have.keys(['appenders', 'categories'])
24+
expect(configuration.appenders).to.have.keys(['0'])
25+
expect(configuration.appenders['0'].type).to.equal('file')
26+
expect(configuration.categories).to.have.keys(['default'])
27+
expect(configuration.categories.default.appenders).to.have.keys(['0'])
28+
expect(configuration.categories.default.level).to.equal('INFO')
29+
})
30+
it('should allow setup() using log4js v2 object', () => {
31+
m.setup('WARN', true, {
32+
'fileAppender': {
33+
type: 'file',
34+
filename: 'test/unit/test.log'
35+
}
36+
})
37+
expect(configuration).to.have.keys(['appenders', 'categories'])
38+
expect(configuration.appenders).to.have.keys(['fileAppender'])
39+
expect(configuration.appenders['fileAppender'].type).to.equal('file')
40+
expect(configuration.categories).to.have.keys(['default'])
41+
expect(configuration.categories.default.appenders[0]).to.equal('fileAppender')
42+
expect(configuration.categories.default.level).to.equal('WARN')
1943
})
2044
})
2145
})

0 commit comments

Comments
 (0)