8000 Merge pull request #550 from nomiddlename/cluster-feature-flag · michaeljheath/log4js-node@4f8336f · GitHub
[go: up one dir, main page]
Skip to content

Commit 4f8336f

Browse files
nomiddlenamenomiddlename
authored
Merge pull request log4js-node#550 from nomiddlename/cluster-feature-flag
fix(cluster): add flag to turn off the cluster support
2 parents 103d74d + f266757 commit 4f8336f

File tree

6 files changed

+110
-20
lines changed

6 files changed

+110
-20
lines changed

‎docs/api.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ Properties:
1919
* `level` (string, case insensitive) - the minimum log level that this category will send to the appenders. For example, if set to 'error' then the appenders will only receive log events of level 'error', 'fatal', 'mark' - log events of 'info', 'warn', 'debug', or 'trace' will be ignored.
2020
* `pm2` (boolean) (optional) - set this to true if you're running your app using [pm2](http://pm2.keymetrics.io), otherwise logs will not work (you'll also need to install pm2-intercom as pm2 module: `pm2 install pm2-intercom`)
2121
* `pm2InstanceVar` (string) (optional, defaults to 'NODE_APP_INSTANCE') - set this if you're using pm2 and have changed the default name of the NODE_APP_INSTANCE variable.
22+
* `disableClustering` (boolean) (optional) - set this to true if you liked the way log4js used to just ignore clustered environments, or you're having trouble with PM2 logging. Each worker process will do its own logging. Be careful with this if you're logging to files, weirdness can occur.
2223

2324
## Loggers - `log4js.getLogger([category])`
2425

‎docs/faq.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,9 @@ log4js.configure({
5050
});
5151
```
5252

53+
## FFS, why did you mess with the PM2 stuff? It was working fine for me!
54+
You can turn off the clustering support, with the `disableClustering: true` option in your config. This will make log4js behave more like it did before version 2.x. Each worker process will log its own output, instead of sending it all to the master process. Be careful if you're logging to files though, this could result in weird behaviour.
55+
5356
## NPM complains about nodemailer being deprecated, what should I do?
5457

5558
Nodemailer version 4.0.1 (the not-deprecated version) requires a node version >= 6, but log4js supports node versions >= 4. So until I stop supporting node versions less than 6 I can't update the dependency. It's only an optional dependency anyway, so you're free to install nodemailer@4.0.1 if you want - as far as I know it should work, the API looks the same to me. If you know that the smtp appender definitely doesn't work with nodemailer v4, then please create an issue with some details about the problem.

‎lib/configuration.js

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,17 @@
22

33
const util = require('util');
44
const path = require('path');
5-
const cluster = require('cluster');
65
const levels = require('./levels');
76
const layouts = require('./layouts');
87
const debug = require('debug')('log4js:configuration');
98

9+
let cluster;
10+
try {
11+
cluster = require('cluster'); // eslint-disable-line global-require
12+
} catch (e) {
13+
debug('Clustering support disabled because require(cluster) threw an error: ', e);
14+
}
15+
1016
const validColours = [
1117
'white', 'grey', 'black',
1218
'blue', 'cyan', 'green',
@@ -76,11 +82,11 @@ class Configuration {
7682
debug(`DEPRECATION: Appender ${config.type} exports a shutdown function.`);
7783
}
7884

79-
debug(`cluster.isMaster ? ${cluster.isMaster}`);
80-
debug(`pm2 enabled ? ${this.pm2}`);
81-
debug(`pm2InstanceVar = ${this.pm2InstanceVar}`);
82-
debug(`process.env[${this.pm2InstanceVar}] = ${process.env[this.pm2InstanceVar]}`);
83-
if (cluster.isMaster || (this.pm2 && process.env[this.pm2InstanceVar] === '0')) {
85+
if (this.disableClustering || cluster.isMaster || (this.pm2 && process.env[this.pm2InstanceVar] === '0')) {
86+
debug(`cluster.isMaster ? ${cluster.isMaster}`);
87+
debug(`pm2 enabled ? ${this.pm2}`);
88+
debug(`pm2InstanceVar = ${this.pm2InstanceVar}`);
89+
debug(`process.env[${this.pm2InstanceVar}] = ${process.env[this.pm2InstanceVar]}`);
8490
return appenderModule.configure(
8591
config,
8692
layouts,
@@ -199,6 +205,8 @@ class Configuration {
199205
this.throwExceptionIf(not(anObject(candidate.appenders)), 'must have a property "appenders" of type object.');
200206
this.throwExceptionIf(not(anObject(candidate.categories)), 'must have a property "categories" of type object.');
201207

208+
this.disableClustering = this.candidate.disableClustering || !cluster;
209+
202210
this.pm2 = this.candidate.pm2;
203211
this.pm2InstanceVar = this.candidate.pm2InstanceVar || 'NODE_APP_INSTANCE';
204212

‎lib/log4js.js

Lines changed: 25 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -24,12 +24,18 @@
2424
*/
2525
const debug = require('debug')('log4js:main');
2626
const fs = require('fs');
27-
const cluster = require('cluster');
2827
const Configuration = require('./configuration');
2928
const connectModule = require('./connect-logger');
3029
const logger = require('./logger');
3130
const layouts = require('./layouts');
3231

32+
let cluster;
33+
try {
34+
cluster = require('cluster'); // eslint-disable-line global-require
35+
} catch (e) {
36+
debug('Clustering support disabled because require(cluster) threw an error: ', e);
37+
}
38+
3339
const defaultConfig = {
3440
appenders: {
3541
stdout: { type: 'stdout' }
@@ -147,7 +153,7 @@ function isPM2Master() {
147153
}
148154

149155
function isMaster() {
150-
return cluster.isMaster || isPM2Master();
156+
return config.disableClustering || cluster.isMaster || isPM2Master();
151157
}
152158

153159
/**
@@ -199,16 +205,23 @@ function configure(configurationFileOrObject) {
199205
LoggingEvent = loggerModule.LoggingEvent;
200206
module.exports.connectLogger = connectModule(config.levels).connectLogger;
201207

202-
// PM2 cluster support
203-
// PM2 runs everything as workers - install pm2-intercom for this to work.
204-
// we only want one of the app instances to write logs
205-
if (isPM2Master()) {
206-
debug('listening for PM2 broadcast messages');
207-
process.removeListener('message', receiver);
208-
process.on('message', receiver);
209-
} else if (cluster.isMaster) {
210-
cluster.removeListener('message', receiver);
211-
cluster.on('message', receiver);
208+
if (config.disableClustering) {
209+
debug('Not listening for cluster messages, because clustering disabled.');
210+
} else {
211+
// PM2 cluster support
212+
// PM2 runs everything as workers - install pm2-intercom for this to work.
213+
// we only want one of the app instances to write logs
214+
if (isPM2Master()) {
215+
debug('listening for PM2 broadcast messages');
216+
process.removeListener('message', receiver);
217+
process.on('message', receiver);
218+
} else if (cluster.isMaster) {
219+
debug('listening for cluster messages');
220+
cluster.removeListener('message', receiver);
221+
cluster.on('message', receiver);
222+
} else {
223+
debug('not listening for messages, because we are not a master process');
224+
}
212225
}
213226

214227
enabled = true;

‎lib/logger.js

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,13 @@
33
'use strict';
44

55
const debug = require('debug')('log4js:logger');
6-
const cluster = require('cluster');
6+
7+
let cluster;
8+
try {
9+
cluster = require('cluster'); // eslint-disable-line global-require
10+
} catch (e) {
11+
debug('Clustering support disabled because require(cluster) threw an error: ', e);
12+
}
713

814
/**
915
* @name LoggingEvent
@@ -25,7 +31,7 @@ class LoggingEvent {
2531
this.level = level;
2632
this.context = Object.assign({}, context);
2733
this.pid = process.pid;
28-
if (cluster.isWorker) {
34+
if (cluster && cluster.isWorker) {
2935
this.cluster = {
3036
workerId: cluster.worker.id,
3137
worker: process.pid

‎test/tap/disable-cluster-test.js

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
'use strict';
2+
3+
const test = require('tap').test;
4+
const cluster = require('cluster');
5+
const log4js = require('../../lib/log4js');
6+
const recorder = require('../../lib/appenders/recording');
7+
8+
cluster.removeAllListeners();
9+
10+
log4js.configure({
11+
appenders: {
12+
vcr: { type: 'recording' }
13+
},
14+
categories: { default: { appenders: ['vcr'], level: 'debug' } },
15+
disableClustering: true
16+
});
17+
18+
if (cluster.isMaster) {
19+
cluster.fork();
20+
21+
const masterLogger = log4js.getLogger('master');
22+
const masterPid = process.pid;
23+
masterLogger.info('this is master');
24+
25+
cluster.on('exit', () => {
26+
const logEvents = recorder.replay();
27+
28+
test('cluster master', (batch) => {
29+
batch.test('only master events should be logged', (t) => {
30+
t.equal(logEvents.length, 1);
31+
t.equal(logEvents[0].categoryName, 'master');
32+
t.equal(logEvents[0].pid, masterPid);
33+
t.equal(logEvents[0].data[0], 'this is master');
34+
t.end();
35+
});
36+
37+
batch.end();
38+
});
39+
});
40+
} else {
41+
const workerLogger = log4js.getLogger('worker');
42+
workerLogger.info('this is worker', new Error('oh dear'));
43+
44+
const workerEvents = recorder.replay();
45+
test('cluster worker', (batch) => {
46+
batch.test('should send events to its own appender', (t) => {
47+
t.equal(workerEvents.length, 1);
48+
t.equal(workerEvents[0].categoryName, 'worker');
49+
t.equal(workerEvents[0].data[0], 'this is worker');
50+
t.type(workerEvents[0].data[1], 'Error');
51+
t.contains(workerEvents[0].data[1].stack, 'Error: oh dear');
52+
t.end();
53+
});
54+
batch.end();
55+
});
56+
// test sending a cluster-style log message
57+
process.send({ topic: 'log4js:message', data: { cheese: 'gouda' } });
58+
cluster.worker.disconnect();
59+
}

0 commit comments

Comments
 (0)
0