From 3f6870bb045f6aed4b340ad12652f54148719ee5 Mon Sep 17 00:00:00 2001 From: meenahoda Date: Mon, 4 Apr 2022 14:02:11 +0100 Subject: [PATCH 1/3] refactor: initialise logger on boot so it can be used anywhere in Tymly and able to override with plugins --- lib/boot/boot/index.js | 3 ++ lib/boot/index.js | 13 +++++++- .../logger => logger-service}/index.js | 28 ++++++++-------- lib/logger-service/log-levels.js | 1 + .../logger => logger-service}/null-logger.js | 0 .../components/services/logger/doc/index.js | 3 -- .../components/services/logger/schema.json | 5 --- .../components/services/logger/index.js | 33 +++++++++++++++++++ .../alternative-logger-plugin/index.js | 1 + test/logger-spec.js | 15 +++++++++ 10 files changed, 78 insertions(+), 24 deletions(-) rename lib/{plugin/components/services/logger => logger-service}/index.js (81%) create mode 100644 lib/logger-service/log-levels.js rename lib/{plugin/components/services/logger => logger-service}/null-logger.js (100%) delete mode 100644 lib/plugin/components/services/logger/doc/index.js delete mode 100644 lib/plugin/components/services/logger/schema.json create mode 100644 test/fixtures/plugins/alternative-logger-plugin/components/services/logger/index.js create mode 100644 test/fixtures/plugins/alternative-logger-plugin/index.js diff --git a/lib/boot/boot/index.js b/lib/boot/boot/index.js index 5b5907f3..a85f3fac 100644 --- a/lib/boot/boot/index.js +++ b/lib/boot/boot/index.js @@ -12,6 +12,9 @@ async function bootAllServices (options) { const bootedServices = {} + // Register core default logger as a Tymly service + bootedServices.logger = options.logger + const config = options.config || {} for (const serviceComponent of options.parsedServices) { diff --git a/lib/boot/index.js b/lib/boot/index.js index 4642458d..88d0eb72 100644 --- a/lib/boot/index.js +++ b/lib/boot/index.js @@ -7,7 +7,14 @@ const applyMods = require('./mods/apply') const startupMessages = require('./../startup-messages') +const LoggerService = require('./../logger-service') + async function bootTymlyServices (options, callback) { + // Initialise a core default logger + const loggerService = new LoggerService.serviceClass() + loggerService.boot() + options.logger = loggerService + const messages = options.messages || startupMessages() options.messages = messages @@ -55,6 +62,9 @@ async function bootTymly (options, messages) { messages ) + // In case an alternative logger was booted + options.logger = bootedServices.logger + await postBootTymlyRefResolution( loadedComponents, bootedServices, @@ -111,7 +121,8 @@ async function bootServices ( pluginPaths: loadedComponents.pluginPaths, blueprintPaths: loadedComponents.blueprintPaths, config: options.config, - messages: messages + messages: messages, + logger: options.logger }) return bootedServices } catch (err) { diff --git a/lib/plugin/components/services/logger/index.js b/lib/logger-service/index.js similarity index 81% rename from lib/plugin/components/services/logger/index.js rename to lib/logger-service/index.js index 9bf75463..59e8926e 100644 --- a/lib/plugin/components/services/logger/index.js +++ b/lib/logger-service/index.js @@ -1,5 +1,5 @@ -const LEVELS = ['trace', 'debug', 'info', 'warn', 'error', 'fatal'] -const nullLogger = require('./null-logger')(LEVELS) +const LOG_LEVELS = require('./log-levels') +const nullLogger = require('./null-logger')(LOG_LEVELS) const pino = require('pino') const pinoPretty = require('pino-pretty') const moment = require('moment') @@ -7,26 +7,26 @@ const path = require('path') const fs = require('fs') const { isPlainObject } = require('lodash') -class Logger { - boot (options) { - this.shouldLog = [...LEVELS, 'ON', '*'].includes(process.env.LOGGER) +class LoggerService { + boot () { + this.shouldLog = [...LOG_LEVELS, 'ON', '*'].includes(process.env.LOGGER) if (this.shouldLog) { - this.level = LEVELS.includes(process.env.LOGGER) + this.level = LOG_LEVELS.includes(process.env.LOGGER) ? process.env.LOGGER : (process.env.LOGGER === '*' ? 'trace' : 'info') this.loggerOutputFilePath = generateFilePath(process.env.LOGGER_OUTPUT_DIR_PATH) - options.messages.info(`Logger level set to: ${this.level}`) - options.messages.info(this.loggerOutputFilePath ? `Output logs to ${this.loggerOutputFilePath}` : 'No output file') + console.log(`Logger level set to: ${this.level}`) + console.log(this.loggerOutputFilePath ? `Output logs to ${this.loggerOutputFilePath}` : 'No output file') } else { - options.messages.info('Using default logger') + console.log('Using default logger') } this.logger = this.createLogger() const createLogFn = level => { this[level] = msg => this.logger[level](msg) } - LEVELS.forEach(createLogFn) + LOG_LEVELS.forEach(createLogFn) this.info('Logger initialised') } @@ -40,7 +40,7 @@ class Logger { // addLogger (namespace = 'logger') { // const fns = {} // const createLogFn = level => { fns[level] = (...args) => this.logger[level](`[${namespace}] ${args.join(' ')}`) } - // LEVELS.forEach(createLogFn) + // LOG_LEVELS.forEach(createLogFn) // return fns // } @@ -75,7 +75,7 @@ class Logger { } isValidLevel (level) { - return LEVELS.includes(level) + return LOG_LEVELS.includes(level) } } @@ -121,7 +121,5 @@ function createMultistream (filepath, level) { } module.exports = { - schema: require('./schema.json'), - serviceClass: Logger, - bootBefore: ['tymly'] + serviceClass: LoggerService } diff --git a/lib/logger-service/log-levels.js b/lib/logger-service/log-levels.js new file mode 100644 index 00000000..0f34c48a --- /dev/null +++ b/lib/logger-service/log-levels.js @@ -0,0 +1 @@ +module.exports = ['trace', 'debug', 'info', 'warn', 'error', 'fatal'] diff --git a/lib/plugin/components/services/logger/null-logger.js b/lib/logger-service/null-logger.js similarity index 100% rename from lib/plugin/components/services/logger/null-logger.js rename to lib/logger-service/null-logger.js diff --git a/lib/plugin/components/services/logger/doc/index.js b/lib/plugin/components/services/logger/doc/index.js deleted file mode 100644 index 098270b8..00000000 --- a/lib/plugin/components/services/logger/doc/index.js +++ /dev/null @@ -1,3 +0,0 @@ -module.exports = { - description: 'Tymly Logger Service' -} diff --git a/lib/plugin/components/services/logger/schema.json b/lib/plugin/components/services/logger/schema.json deleted file mode 100644 index 23dcd4be..00000000 --- a/lib/plugin/components/services/logger/schema.json +++ /dev/null @@ -1,5 +0,0 @@ -{ - "type": "object", - "properties": {}, - "required": [] -} \ No newline at end of file diff --git a/test/fixtures/plugins/alternative-logger-plugin/components/services/logger/index.js b/test/fixtures/plugins/alternative-logger-plugin/components/services/logger/index.js new file mode 100644 index 00000000..4fdfb6e0 --- /dev/null +++ b/test/fixtures/plugins/alternative-logger-plugin/components/services/logger/index.js @@ -0,0 +1,33 @@ +const { isPlainObject } = require('lodash') +const levels = ['trace', 'debug', 'info', 'warn', 'error', 'fatal'] + +class AlternativeLogger { + boot () { + this.shouldLog = true + this.level = process.env.LOGGER + + this.logs = [] + + const logFn = level => (...args) => { + const message = args.join(' ') + console.log(`[${level}] ${message}`) + this.logs.push({ level, message }) + } + + const logger = {} + levels.forEach(level => { logger[level] = logFn(level) }) + logger.child = () => logger + this.logger = logger + } + + child (arg) { + const namespace = (typeof arg === 'string' ? arg : (isPlainObject(arg) ? arg.namespace : '')) || 'logger' + const opts = { namespace } + return this.logger.child(opts) + } +} + +module.exports = { + serviceClass: AlternativeLogger, + bootBefore: ['tymly'] +} diff --git a/test/fixtures/plugins/alternative-logger-plugin/index.js b/test/fixtures/plugins/alternative-logger-plugin/index.js new file mode 100644 index 00000000..4ba52ba2 --- /dev/null +++ b/test/fixtures/plugins/alternative-logger-plugin/index.js @@ -0,0 +1 @@ +module.exports = {} diff --git a/test/logger-spec.js b/test/logger-spec.js index 57e7311e..f8503274 100644 --- a/test/logger-spec.js +++ b/test/logger-spec.js @@ -254,6 +254,21 @@ describe('Logger tests', function () { }) }) + describe('boot Tymly with an alternative logger service', () => { + it('boot tymly', async () => { + process.env.LOGGER = 'trace' + bootedServices = await tymly.boot({ + pluginPaths: [ + path.resolve(__dirname, './fixtures/plugins/logger-plugin'), + path.resolve(__dirname, './fixtures/plugins/alternative-logger-plugin') + ] + }) + expect(bootedServices.logger.shouldLog).to.eql(true) + expect(bootedServices.logger.level).to.eql('trace') + expect(bootedServices.logger.logs.length).to.eql(6) + }) + }) + after('reset logging environment variables', () => { process.env.LOGGER = null process.env.LOGGER_OUTPUT_DIR_PATH = null From c05cebb4b07e85384077f8978fc8b7521a4c0050 Mon Sep 17 00:00:00 2001 From: meenahoda Date: Mon, 4 Apr 2022 14:07:34 +0100 Subject: [PATCH 2/3] fix: A constructor name should not start with a lowercase letter --- lib/boot/index.js | 2 +- lib/logger-service/index.js | 4 +--- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/lib/boot/index.js b/lib/boot/index.js index 88d0eb72..f64af919 100644 --- a/lib/boot/index.js +++ b/lib/boot/index.js @@ -11,7 +11,7 @@ const LoggerService = require('./../logger-service') async function bootTymlyServices (options, callback) { // Initialise a core default logger - const loggerService = new LoggerService.serviceClass() + const loggerService = new LoggerService() loggerService.boot() options.logger = loggerService diff --git a/lib/logger-service/index.js b/lib/logger-service/index.js index 59e8926e..f3d75b84 100644 --- a/lib/logger-service/index.js +++ b/lib/logger-service/index.js @@ -120,6 +120,4 @@ function createMultistream (filepath, level) { return pino.multistream(streams) } -module.exports = { - serviceClass: LoggerService -} +module.exports = LoggerService From 1feb02ec8b90e80b0205df20a8ad87f22ada11ce Mon Sep 17 00:00:00 2001 From: meenahoda Date: Mon, 4 Apr 2022 14:10:44 +0100 Subject: [PATCH 3/3] test: update to ignore initialisation log --- test/logger-spec.js | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/test/logger-spec.js b/test/logger-spec.js index f8503274..3f725ed7 100644 --- a/test/logger-spec.js +++ b/test/logger-spec.js @@ -13,10 +13,11 @@ function readLogFile (filepath) { .split('\n') .filter(line => line) .map(JSON.parse) + .filter(line => line.msg !== 'Logger initialised') } function expectAllLevels (logs) { - logs.shift() + // logs.shift() const levels = logs.map(l => l.level) const notLogged = Object.values(LEVEL_MAP).filter(l => !levels.includes(l)) @@ -61,7 +62,7 @@ function expectAllLevels (logs) { } function expectSomeLevels (logs, expectedLevels) { - logs.shift() + // logs.shift() const levels = logs.map(l => l.level) for (const [key, value] of Object.keys(LEVEL_MAP)) { @@ -147,7 +148,7 @@ describe('Logger tests', function () { it('check output file contents', () => { const filepath = bootedServices.logger.loggerOutputFilePath const logs = readLogFile(filepath) - expect(logs.length).to.eql(7) + expect(logs.length).to.eql(6) expectAllLevels(logs) }) @@ -178,7 +179,7 @@ describe('Logger tests', function () { it('check output file contents', () => { const filepath = bootedServices.logger.loggerOutputFilePath const logs = readLogFile(filepath) - expect(logs.length).to.eql(5) + expect(logs.length).to.eql(4) expectSomeLevels(logs, ['info', 'warn', 'error', 'fatal']) }) @@ -240,7 +241,7 @@ describe('Logger tests', function () { it('check output file contents', () => { const filepath = bootedServices.logger.loggerOutputFilePath const logs = readLogFile(filepath) - expect(logs.length).to.eql(7) + expect(logs.length).to.eql(6) expectAllLevels(logs) })