REFACTOR: Improve logger interface, add eslint

1. Add eslint based on airbnb base rules
2. Change logger interface to receive configuration object instead of variables
3. Add option to provide skip function for setupExpressLogger
4. Upgrade node image in circleci config

Author: v0vs

.circleci/config.yml

@@ -2,7 +2,7 @@ version: 2
 jobs:
   build:
     docker:
-      - image: circleci/node:10.9.0
+      - image: circleci/node:12.15.0
 
     working_directory: ~/repo
 
@@ -25,6 +25,8 @@ jobs:
 
       - run: npm install
 
+      - run: npm run lint
+
       - run: npm run test
 
       - save_cache:

.eslintignore

@@ -0,0 +1,2 @@
+**/*.ts
+node_modules/

.eslintrc.js

@@ -0,0 +1,16 @@
+module.exports = {
+  env: {
+    es6: true,
+    node: true,
+    mocha: true
+  },
+  extends: 'airbnb-base',
+  globals: {
+    Atomics: 'readonly',
+    SharedArrayBuffer: 'readonly',
+  },
+  parserOptions: {
+    ecmaVersion: 2018,
+    sourceType: 'module',
+  }
+};

index.d.ts

@@ -1,9 +1,19 @@
 import { Logger } from 'winston';
 import { Handler } from 'express';
+import { RouteFilter } from 'express-winston';
 
 declare namespace strigoNodeLogger {
-  function setupNodeLogger(env: string, level?: string): Logger;
-  function setupExpressLogger(env: string, level?: string): { logger: Logger, loggerMiddleware: Handler };
+  type NodeLoggerOptions = {
+    env?: string;
+    level?: string;
+  }
+
+  type NodeExpressLoggerOptions = NodeLoggerOptions & {
+    skip?: RouteFilter[]
+  }
+
+  function setupNodeLogger(loggerOptions: NodeLoggerOptions): Logger;
+  function setupExpressLogger(loggerOptions: NodeExpressLoggerOptions): { logger: Logger, loggerMiddleware: Handler };
 }
 
 export = strigoNodeLogger;

index.js

@@ -45,35 +45,39 @@ function configureFormatters(env) {
  * @param {String} env The name of the environment to use. This affects formatting.
  * @param {String} level The level to use when setting the logger up.
  */
-function setupNodeLogger(env, level = DEFAULT_LOG_LEVEL) {
+function setupNodeLogger({ env = '', level = DEFAULT_LOG_LEVEL }) {
   return createLogger({
     level,
     format: format.combine(...configureFormatters(env)),
-    transports: [ new transports.Console() ],
+    transports: [new transports.Console()],
   });
 }
 
+function skipDefaultRequests(req) {
+  // Consul's Health check regularly bombards us with requests, so we should ignore it.
+  return (req.headers['user-agent'] === 'Consul Health Check');
+}
+
 /**
  * Setup an express logger based on the main logger.
  *
  * @param {String} env The name of the environment to use. This affects formatting.
  * @param {String} level The level to use when setting the logger up.
  */
-function setupExpressLogger(env, level = DEFAULT_LOG_LEVEL) {
+function setupExpressLogger({ env = '', level = DEFAULT_LOG_LEVEL, skip = [] }) {
   const logger = setupNodeLogger(env, level);
 
   const loggerMiddleware = expressWinston.logger({
     winstonInstance: logger,
     metaField: null,
     colorize: false,
-    // Consul's Health check regularly bombards us with requests, so we should ignore it.
-    skip: (req) => (req.headers['user-agent'] == 'Consul Health Check' && req.url == '/'),
+    skip: (req) => [skipDefaultRequests, ...skip].every((skipFunc) => skipFunc(req)),
   });
 
   return { logger, loggerMiddleware };
 }
 
 module.exports = {
   setupNodeLogger,
-  setupExpressLogger
-}
+  setupExpressLogger,
+};

package.json

@@ -23,10 +23,14 @@
     "@babel/register": "7.0.0",
     "capture-console": "1.0.1",
     "chai": "4.2.0",
-    "chance": "1.0.18",
+    "chance": "1.1.4",
+    "eslint": "6.8.0",
+    "eslint-config-airbnb-base": "14.0.0",
+    "eslint-plugin-import": "2.20.1",
     "mocha": "5.2.0"
   },
   "scripts": {
+    "lint": "eslint **/*.js",
     "test": "mocha --require @babel/register"
   }
 }

test/logger.test.js

@@ -1,16 +1,15 @@
-import chai from 'chai';
 import capture from 'capture-console';
 import Chance from 'chance';
+import { expect } from 'chai';
 
 import { setupNodeLogger, setupExpressLogger } from '..';
 
-const expect = chai.expect;
 const chance = new Chance();
 
-describe('strigo-node-logger', function() {
-  describe('#setupNodeLogger()', function() {
-    it('should printout the configured severity level', function() {
-      const log = setupNodeLogger(null, 'warn');
+describe('strigo-node-logger', () => {
+  describe('#setupNodeLogger()', () => {
+    it('should printout the configured severity level', () => {
+      const log = setupNodeLogger({ level: 'warn' });
 
       let printout;
       let stdout;
@@ -32,8 +31,8 @@ describe('strigo-node-logger', function() {
       expect(stdout.indexOf('info')).to.be.eq(-1);
     });
 
-    it('should not write debug printout when set up with info level', function() {
-      const log = setupNodeLogger('prod', 'info');
+    it('should not write debug printout when set up with info level', () => {
+      const log = setupNodeLogger({ env: 'prod', level: 'info' });
 
       const printout = chance.string();
       const stdout = capture.captureStdout(() => {
@@ -43,8 +42,8 @@ describe('strigo-node-logger', function() {
       expect(stdout.indexOf(printout)).to.be.eq(-1);
     });
 
-    it('should not write debug printout when configured to info in runtime', function() {
-      const log = setupNodeLogger('prod', 'debug');
+    it('should not write debug printout when configured to info in runtime', () => {
+      const log = setupNodeLogger({ env: 'prod', level: 'debug' });
       log.level = 'info';
 
       const printout = chance.string();
@@ -55,8 +54,8 @@ describe('strigo-node-logger', function() {
       expect(stdout.indexOf(printout)).to.be.eq(-1);
     });
 
-    it('should allow setting up an express logger', function() {
-      const { logger: log, loggerMiddleware: middleware } = setupExpressLogger('prod', 'info');
+    it('should allow setting up an express logger', () => {
+      const { logger: log, loggerMiddleware: middleware } = setupExpressLogger({ env: 'prod', level: 'info' });
 
       // Should be a function. This test can be better if we check for specific
       // expressWinston attributes.