Make logging transports configurable (#2231)

* feat: Make logging optional

Signed-off-by: wilmardo <info@wilmardenouden.nl>

* feat: be more verbose about what logging is enabled

Signed-off-by: wilmardo <info@wilmardenouden.nl>

* feat: adds tests

Signed-off-by: wilmardo <info@wilmardenouden.nl>

* Updates.

* Updates

* fix tests.

* Remove onlythis.

* Remove another onlythis

* Improve test stability

* Remove another onlythis

lib/controller.js

@@ -82,8 +82,6 @@ class Controller {
             process.exit(1);
         }
 
-        logger.info(`Logging to directory: '${logger.directory}'`);
-        logger.cleanup();
         this.state.start();
 
         const info = await utils.getZigbee2mqttVersion();

lib/util/logger.js

@@ -7,12 +7,17 @@ const fx = require('mkdir-recursive');
 const rimraf = require('rimraf');
 const colorizer = winston.format.colorize();
 
-// Directoy to log to
+// What transports to enable
+const output = settings.get().advanced.log_output;
+
+// Directory to log to
 const timestamp = moment(Date.now()).format('YYYY-MM-DD.HH-mm-ss');
 const directory = settings.get().advanced.log_directory.replace('%TIMESTAMP%', timestamp);
 
-// Make sure that log directoy exsists
-fx.mkdirSync(directory);
+// Make sure that log directoy exsists when not logging to stdout only
+if (output.includes('file')) {
+    fx.mkdirSync(directory);
+}
 
 // Determine the log level.
 const level = settings.get().advanced.log_level;
@@ -45,6 +50,7 @@ const transports = {
     }),
     console: new winston.transports.Console({
         level,
+        silent: !output.includes('console'),
         format: winston.format.combine(
             winston.format.timestamp({format: timestampFormat}),
             winston.format.printf(/* istanbul ignore next */(info) => {
@@ -56,12 +62,13 @@ const transports = {
     }),
 };
 
-const logger = winston.createLogger({
-    transports: [
-        transports.console,
-        transports.file,
-    ],
-});
+// Create logger without transports
+const transportsToUse = [transports.console];
+if (output.includes('file')) {
+    transportsToUse.push(transports.file);
+}
+
+const logger = winston.createLogger({transports: transportsToUse});
 
 // Cleanup any old log directory.
 function cleanup() {
@@ -83,11 +90,22 @@ function cleanup() {
 }
 
 logger.cleanup = cleanup;
-logger.directory = directory;
 logger.getLevel = () => transports.console.level;
 logger.setLevel = (level) => {
     transports.console.level = level;
     transports.file.level = level;
 };
 
+// Print to user what logging is enabled
+if (output.includes('file')) {
+    if (output.includes('console')) {
+        logger.info(`Logging to console and directory: '${directory}'`);
+    } else {
+        logger.info(`Logging to directory: '${directory}'`);
+    }
+    logger.cleanup();
+} else if (output.includes('console')) {
+    logger.info(`Logging to console only'`);
+}
+
 module.exports = logger;

lib/util/settings.js

@@ -42,6 +42,7 @@ const defaults = {
         output: 'json',
     },
     advanced: {
+        log_output: ['console', 'file'],
         log_directory: path.join(data.getPath(), 'log', '%TIMESTAMP%'),
         log_level: /* istanbul ignore next */ process.env.DEBUG ? 'debug' : 'info',
         soft_reset_timeout: 0,
@@ -153,6 +154,7 @@ const schema = {
                 channel: {type: 'number', minimum: 11, maximum: 26},
                 cache_state: {type: 'boolean'},
                 log_level: {type: 'string', enum: ['info', 'warn', 'error', 'debug']},
+                log_output: {type: 'array', items: {type: 'string'}},
                 log_directory: {type: 'string'},
                 baudrate: {type: 'number'},
                 rtscts: {type: 'boolean'},

test/controller.test.js

@@ -30,7 +30,6 @@ describe('Controller', () => {
 
     it('Start controller', async () => {
         await controller.start();
-        expect(logger.cleanup).toHaveBeenCalledTimes(1);
         expect(zigbeeHerdsman.constructor).toHaveBeenCalledWith({"network":{"panID":6754,"extendedPanID":[221,221,221,221,221,221,221,221],"channelList":[11],"networkKey":[1,3,5,7,9,11,13,15,0,2,4,6,8,10,12,13]},"databasePath":path.join(data.mockDir, "database.db"), "databaseBackupPath":path.join(data.mockDir, "database.db.backup"),"backupPath":path.join(data.mockDir, "coordinator_backup.json"),"acceptJoiningDeviceHandler": expect.any(Function),"serialPort":{"baudRate":115200,"rtscts":true,"path":"/dev/dummy"}});
         expect(zigbeeHerdsman.start).toHaveBeenCalledTimes(1);
         expect(zigbeeHerdsman.setLED).toHaveBeenCalledTimes(0);

test/logger.test.js

@@ -1,20 +1,41 @@
 const tmp = require('tmp');
-const data = require('./stub/data');
 const dir = tmp.dirSync();
-const settings = require('../lib/util/settings');
-settings.set(['advanced', 'log_directory'], dir.name + '/%TIMESTAMP%');
-const logger = require('../lib/util/logger.js');
+let settings;
 const fs = require('fs');
 const path = require('path');
+const data = require('./stub/data');
+let stdOutWriteOriginal;
+const rimraf = require('rimraf');
 
 describe('Logger', () => {
+    beforeEach(async () => {
+        data.writeDefaultConfiguration();
+        jest.resetModules();
+        settings = require('../lib/util/settings');
+        settings.set(['advanced', 'log_directory'], dir.name + '/%TIMESTAMP%');
+        settings._reRead();
+        stdOutWriteOriginal = console._stdout.write;
+        console._stdout.write = () => {};
+    });
+
+    afterEach(async () => {
+        console._stdout.write = stdOutWriteOriginal;
+    });
+
     it('Create log directory', () => {
+        const logger = require('../lib/util/logger.js');
         const dirs = fs.readdirSync(dir.name);
         expect(dirs.length).toBe(1);
     });
 
     it('Should cleanup', () => {
-        for (let i = 0; i < 20; i++) {
+        const logger = require('../lib/util/logger.js');
+
+        for (const d of fs.readdirSync(dir.name)) {
+            rimraf.sync(path.join(dir.name, d));
+        }
+
+        for (let i = 0; i < 21; i++) {
             fs.mkdirSync(path.join(dir.name, `log_${i}`));
         }
 
@@ -24,6 +45,7 @@ describe('Logger', () => {
     })
 
     it('Should not cleanup when there is no timestamp set', () => {
+        const logger = require('../lib/util/logger.js');
         for (let i = 30; i < 40; i++) {
             fs.mkdirSync(path.join(dir.name, `log_${i}`));
         }
@@ -35,7 +57,45 @@ describe('Logger', () => {
     })
 
     it('Set and get log level', () => {
+        const logger = require('../lib/util/logger.js');
         logger.setLevel('debug');
         expect(logger.getLevel()).toBe('debug');
     });
+
+    it('Logger should be console and file by default', () => {
+        const logger = require('../lib/util/logger.js');
+        const pipes = logger._readableState.pipes;
+        expect(pipes.length).toBe(2);
+        expect(pipes[0].constructor.name).toBe('Console');
+        expect(pipes[0].silent).toBe(false);
+        expect(pipes[1].constructor.name).toBe('File');
+        expect(pipes[1].dirname.startsWith(dir.name)).toBeTruthy();
+    });
+
+    it('Logger can be file only', () => {
+        settings.set(['advanced', 'log_output'], ['file']);
+        const logger = require('../lib/util/logger.js');
+        const pipes = logger._readableState.pipes;
+        expect(pipes.length).toBe(2);
+        expect(pipes[0].constructor.name).toBe('Console');
+        expect(pipes[0].silent).toBe(true);
+        expect(pipes[1].constructor.name).toBe('File');
+        expect(pipes[1].dirname.startsWith(dir.name)).toBeTruthy();
+    });
+
+    it('Logger can be console only', () => {
+        settings.set(['advanced', 'log_output'], ['console']);
+        const logger = require('../lib/util/logger.js');
+        const pipes = logger._readableState.pipes;
+        expect(pipes.constructor.name).toBe('Console');
+        expect(pipes.silent).toBe(false);
+    });
+
+    it('Logger can be nothing', () => {
+        settings.set(['advanced', 'log_output'], []);
+        const logger = require('../lib/util/logger.js');
+        const pipes = logger._readableState.pipes;
+        expect(pipes.constructor.name).toBe('Console');
+        expect(pipes.silent).toBe(true);
+    });
 });