diff --git a/docs/getting-started.md b/docs/getting-started.md index e2d2141151..d9838aa13a 100644 --- a/docs/getting-started.md +++ b/docs/getting-started.md @@ -58,6 +58,7 @@ Available unleash options include: - `unsecure` - (default) will use simple cookie based authentication. UI will require the user to specify an email in order to use unleash. - `custom` - use this when you implement your own custom authentication logic. - **ui** (object) - Set of UI specific overrides. You may set the following keys: `headerBackground`, `environment`, `slogan`. +- **getLogger** (function) - Used to register a [custom log provider](#How do I configure the log output). ### 3. Docker @@ -69,14 +70,10 @@ docker run -d -e DATABASE_URL=postgres://user:pass@10.200.221.11:5432/unleash un ## How do I configure the log output? -By default, `unleash` uses [log4js](https://github.com/nomiddlename/log4js-node) to log important information. It is possible to swap out the logger provider (only when using Unleash programmatically). This enables filtering of log levels and easy redirection of output streams. - -### What is a logger provider? - -A logger provider is a function which takes the name of a logger and returns a logger implementation. For instance, the following code snippet shows how a logger provider for the global `console` object could be written: +By default, `unleash` uses [log4js](https://github.com/nomiddlename/log4js-node) to log important information. It is possible to swap out the logger provider (only when using Unleash programmatically). You do this by providing an implementation of the **getLogger** function as This enables filtering of log levels and easy redirection of output streams. ```javascript -function consoleLoggerProvider(name) { +function getLogger(name) { // do something with the name return { debug: console.log, @@ -88,16 +85,3 @@ function consoleLoggerProvider(name) { ``` The logger interface with its `debug`, `info`, `warn` and `error` methods expects format string support as seen in `debug` or the JavaScript `console` object. Many commonly used logging implementations cover this API, e.g., bunyan, pino or winston. - -### How do I set a logger provider? - -Custom logger providers need to be set _before requiring the `unleash-server` module_. The following example shows how this can be done: - -```javascript -// first configure the logger provider -const unleashLogger = require('unleash-server/lib/logger'); -unleashLogger.setLoggerProvider(consoleLoggerProvider); - -// then require unleash-server and continue as normal -const unleash = require('unleash-server'); -``` diff --git a/lib/app.test.js b/lib/app.test.js index dc78f8711c..ce75c44e69 100644 --- a/lib/app.test.js +++ b/lib/app.test.js @@ -3,6 +3,7 @@ const test = require('ava'); const express = require('express'); const proxyquire = require('proxyquire'); +const getLogger = require('../test/fixtures/no-logger'); const getApp = proxyquire('./app', { './routes': class Index { router() { @@ -12,13 +13,14 @@ const getApp = proxyquire('./app', { }); test('should not throw when valid config', t => { - const app = getApp({}); + const app = getApp({ getLogger }); t.true(typeof app.listen === 'function'); }); test('should call preHook', t => { let called = 0; getApp({ + getLogger, preHook: () => { called++; }, @@ -29,6 +31,7 @@ test('should call preHook', t => { test('should call preRouterHook', t => { let called = 0; getApp({ + getLogger, preRouterHook: () => { called++; }, diff --git a/lib/db/client-instance-store.js b/lib/db/client-instance-store.js index 00e9ade47d..849cbeab43 100644 --- a/lib/db/client-instance-store.js +++ b/lib/db/client-instance-store.js @@ -1,7 +1,6 @@ /* eslint camelcase: "off" */ 'use strict'; -const logger = require('../logger')('client-instance-store.js'); const COLUMNS = [ 'app_name', 'instance_id', @@ -24,8 +23,9 @@ const mapRow = row => ({ }); class ClientInstanceStore { - constructor(db) { + constructor(db, getLogger) { this.db = db; + this.logger = getLogger('client-instance-store.js'); const clearer = () => this._removeInstancesOlderThanTwoDays(); setTimeout(clearer, 10).unref(); setInterval(clearer, ONE_DAY).unref(); @@ -35,7 +35,9 @@ class ClientInstanceStore { this.db(TABLE) .whereRaw("created_at < now() - interval '2 days'") .del() - .then(res => res > 0 && logger.info(`Deleted ${res} instances`)); + .then( + res => res > 0 && this.logger.info(`Deleted ${res} instances`) + ); } updateRow(details) { diff --git a/lib/db/client-metrics-db.js b/lib/db/client-metrics-db.js index 3512206f35..90e4a0eff1 100644 --- a/lib/db/client-metrics-db.js +++ b/lib/db/client-metrics-db.js @@ -1,7 +1,5 @@ 'use strict'; -const logger = require('../logger')('client-metrics-db.js'); - const METRICS_COLUMNS = ['id', 'created_at', 'metrics']; const TABLE = 'client_metrics'; @@ -14,8 +12,9 @@ const mapRow = row => ({ }); class ClientMetricsDb { - constructor(db) { + constructor(db, getLogger) { this.db = db; + this.logger = getLogger('client-metrics-db.js'); // Clear old metrics regulary const clearer = () => this.removeMetricsOlderThanOneHour(); @@ -27,7 +26,7 @@ class ClientMetricsDb { this.db(TABLE) .whereRaw("created_at < now() - interval '1 hour'") .del() - .then(res => res > 0 && logger.info(`Deleted ${res} metrics`)); + .then(res => res > 0 && this.logger.info(`Deleted ${res} metrics`)); } // Insert new client metrics diff --git a/lib/db/client-metrics-store.js b/lib/db/client-metrics-store.js index a6443fdbb9..a511b89094 100644 --- a/lib/db/client-metrics-store.js +++ b/lib/db/client-metrics-store.js @@ -1,14 +1,13 @@ 'use strict'; -const logger = require('../logger')('client-metrics-store.js'); - const { EventEmitter } = require('events'); const TEN_SECONDS = 10 * 1000; class ClientMetricsStore extends EventEmitter { - constructor(metricsDb, pollInterval = TEN_SECONDS) { + constructor(metricsDb, getLogger, pollInterval = TEN_SECONDS) { super(); + this.logger = getLogger('client-metrics-store.js'); this.metricsDb = metricsDb; this.highestIdSeen = 0; @@ -20,7 +19,7 @@ class ClientMetricsStore extends EventEmitter { const metrics = await this.metricsDb.getMetricsLastHour(); this._emitMetrics(metrics); } catch (err) { - logger.error('Error fetching metrics last hour', err); + this.logger.error('Error fetching metrics last hour', err); } this._startPoller(pollInterval); this.emit('ready'); diff --git a/lib/db/client-metrics-store.test.js b/lib/db/client-metrics-store.test.js index a9d6d799a4..cbd6a74439 100644 --- a/lib/db/client-metrics-store.test.js +++ b/lib/db/client-metrics-store.test.js @@ -3,6 +3,7 @@ const test = require('ava'); const ClientMetricStore = require('./client-metrics-store'); const lolex = require('lolex'); +const getLogger = require('../../test/fixtures/no-logger'); function getMockDb() { const list = [ @@ -24,7 +25,7 @@ function getMockDb() { test.cb('should call database on startup', t => { const mock = getMockDb(); - const store = new ClientMetricStore(mock); + const store = new ClientMetricStore(mock, getLogger); t.plan(2); @@ -42,7 +43,7 @@ test.cb('should start poller even if inital database fetch fails', t => { const mock = getMockDb(); mock.getMetricsLastHour = () => Promise.reject('oops'); - const store = new ClientMetricStore(mock, 100); + const store = new ClientMetricStore(mock, getLogger, 100); const metrics = []; store.on('metrics', m => metrics.push(m)); @@ -63,7 +64,7 @@ test.cb('should poll for updates', t => { const clock = lolex.install(); const mock = getMockDb(); - const store = new ClientMetricStore(mock, 100); + const store = new ClientMetricStore(mock, getLogger, 100); const metrics = []; store.on('metrics', m => metrics.push(m)); diff --git a/lib/db/feature-toggle-store.js b/lib/db/feature-toggle-store.js index 2c1adaafb9..2ccb7cb4e6 100644 --- a/lib/db/feature-toggle-store.js +++ b/lib/db/feature-toggle-store.js @@ -8,7 +8,6 @@ const { FEATURE_IMPORT, DROP_FEATURES, } = require('../event-type'); -const logger = require('../logger')('client-toggle-store.js'); const NotFoundError = require('../error/notfound-error'); const FEATURE_COLUMNS = [ 'name', @@ -21,8 +20,9 @@ const FEATURE_COLUMNS = [ const TABLE = 'features'; class FeatureToggleStore { - constructor(db, eventStore) { + constructor(db, eventStore, getLogger) { this.db = db; + this.getLogger = getLogger('client-toggle-store.js'); eventStore.on(FEATURE_CREATED, event => this._createFeature(event.data) ); @@ -111,7 +111,7 @@ class FeatureToggleStore { return this.db(TABLE) .insert(this.eventDataToRow(data)) .catch(err => - logger.error('Could not insert feature, error was: ', err) + this.logger.error('Could not insert feature, error: ', err) ); } @@ -120,7 +120,7 @@ class FeatureToggleStore { .where({ name: data.name }) .update(this.eventDataToRow(data)) .catch(err => - logger.error('Could not update feature, error was: ', err) + this.logger.error('Could not update feature, error: ', err) ); } @@ -129,7 +129,7 @@ class FeatureToggleStore { .where({ name }) .update({ archived: 1, enabled: 0 }) .catch(err => { - logger.error('Could not archive feature, error was: ', err); + this.logger.error('Could not archive feature, error: ', err); }); } @@ -138,7 +138,7 @@ class FeatureToggleStore { .where({ name }) .update({ archived: 0, enabled: 0 }) .catch(err => - logger.error('Could not archive feature, error was: ', err) + this.logger.error('Could not archive feature, error: ', err) ); } @@ -151,7 +151,7 @@ class FeatureToggleStore { result === 0 ? this.db(TABLE).insert(rowData) : result ) .catch(err => - logger.error('Could not import feature, error was: ', err) + this.logger.error('Could not import feature, error: ', err) ); } @@ -159,7 +159,7 @@ class FeatureToggleStore { return this.db(TABLE) .delete() .catch(err => - logger.error('Could not drop features, error was: ', err) + this.logger.error('Could not drop features, error: ', err) ); } } diff --git a/lib/db/index.js b/lib/db/index.js index 3db8f6e6cc..f8fd658293 100644 --- a/lib/db/index.js +++ b/lib/db/index.js @@ -10,17 +10,18 @@ const ClientMetricsStore = require('./client-metrics-store'); const ClientApplicationsStore = require('./client-applications-store'); module.exports.createStores = config => { + const getLogger = config.getLogger; const db = createDb(config); - const eventStore = new EventStore(db); - const clientMetricsDb = new ClientMetricsDb(db); + const eventStore = new EventStore(db, getLogger); + const clientMetricsDb = new ClientMetricsDb(db, getLogger); return { db, eventStore, - featureToggleStore: new FeatureToggleStore(db, eventStore), - strategyStore: new StrategyStore(db, eventStore), - clientApplicationsStore: new ClientApplicationsStore(db), - clientInstanceStore: new ClientInstanceStore(db), - clientMetricsStore: new ClientMetricsStore(clientMetricsDb), + featureToggleStore: new FeatureToggleStore(db, eventStore, getLogger), + strategyStore: new StrategyStore(db, eventStore, getLogger), + clientApplicationsStore: new ClientApplicationsStore(db, getLogger), + clientInstanceStore: new ClientInstanceStore(db, getLogger), + clientMetricsStore: new ClientMetricsStore(clientMetricsDb, getLogger), }; }; diff --git a/lib/db/strategy-store.js b/lib/db/strategy-store.js index 7a9ce825fe..578bd02a05 100644 --- a/lib/db/strategy-store.js +++ b/lib/db/strategy-store.js @@ -7,14 +7,14 @@ const { STRATEGY_IMPORT, DROP_STRATEGIES, } = require('../event-type'); -const logger = require('../logger')('strategy-store.js'); const NotFoundError = require('../error/notfound-error'); const STRATEGY_COLUMNS = ['name', 'description', 'parameters', 'built_in']; const TABLE = 'strategies'; class StrategyStore { - constructor(db, eventStore) { + constructor(db, eventStore, getLogger) { this.db = db; + this.logger = getLogger('strategy-store.js'); eventStore.on(STRATEGY_CREATED, event => this._createStrategy(event.data) ); @@ -90,7 +90,7 @@ class StrategyStore { this.db(TABLE) .insert(this.eventDataToRow(data)) .catch(err => - logger.error('Could not insert strategy, error was: ', err) + this.logger.error('Could not insert strategy, error: ', err) ); } @@ -99,7 +99,7 @@ class StrategyStore { .where({ name: data.name }) .update(this.eventDataToRow(data)) .catch(err => - logger.error('Could not update strategy, error was: ', err) + this.logger.error('Could not update strategy, error: ', err) ); } @@ -108,7 +108,7 @@ class StrategyStore { .where({ name }) .del() .catch(err => { - logger.error('Could not delete strategy, error was: ', err); + this.logger.error('Could not delete strategy, error: ', err); }); } @@ -121,7 +121,7 @@ class StrategyStore { result === 0 ? this.db(TABLE).insert(rowData) : result ) .catch(err => - logger.error('Could not import strategy, error was: ', err) + this.logger.error('Could not import strategy, error: ', err) ); } @@ -130,7 +130,7 @@ class StrategyStore { .where({ built_in: 0 }) // eslint-disable-line .delete() .catch(err => - logger.error('Could not drop strategies, error was: ', err) + this.logger.error('Could not drop strategies, error: ', err) ); } } diff --git a/lib/logger.js b/lib/logger.js index 16acf0dc8f..a4758aaf7f 100644 --- a/lib/logger.js +++ b/lib/logger.js @@ -4,22 +4,27 @@ const log4js = require('log4js'); let loggerProvider = getDefaultLogProvider(); -module.exports = exports = function getLogger(name) { - return loggerProvider(name); -}; +module.exports.defaultLogProvider = loggerProvider; -exports.setLoggerProvider = function setLoggerProvider(provider) { +function validateLogProvider(provider) { validate(typeof provider == 'function', 'Provider needs to be a function'); const logger = provider('unleash:logger'); - validate(typeof logger.debug == 'function', 'Logger must implement debug'); validate(typeof logger.info == 'function', 'Logger must implement info'); validate(typeof logger.warn == 'function', 'Logger must implement warn'); validate(typeof logger.error == 'function', 'Logger must implement error'); +} +exports.validateLogProvider = validateLogProvider; + +// Deprecated +exports.setLoggerProvider = function setLoggerProvider(provider) { + validateLogProvider(provider); loggerProvider = provider; - logger.info('Custom Logger Provider initalized.'); + const logger = provider('unleash:logger'); + logger.info(`Your way of configuring a logProvider is depreacted. + See https://unleash.github.io/docs/getting_started for details`); }; function getDefaultLogProvider() { diff --git a/lib/logger.test.js b/lib/logger.test.js index 40462975db..2f1237bc04 100644 --- a/lib/logger.test.js +++ b/lib/logger.test.js @@ -1,29 +1,12 @@ 'use strict'; const test = require('ava'); -const createLogger = require('./logger'); -const logger = require('../logger'); +const logger = require('./logger'); test('should expose a setLoggerProvider function', t => { t.true(logger.setLoggerProvider instanceof Function); }); -test('should create logger via custom logger provider', t => { - const loggerName = 'test'; - const loggerImpl = { - debug: () => {}, - info: () => {}, - warn: () => {}, - error: () => {}, - }; - const provider = () => loggerImpl; - logger.setLoggerProvider(provider); - - const log = createLogger(loggerName); - - t.is(log, loggerImpl); -}); - test('should require custom logger to implement info', t => { const loggerImpl = { debug: () => {}, diff --git a/lib/middleware/permission-checker.test.js b/lib/middleware/permission-checker.test.js index 3bc48014bc..11e4145ab2 100644 --- a/lib/middleware/permission-checker.test.js +++ b/lib/middleware/permission-checker.test.js @@ -5,6 +5,7 @@ const store = require('../../test/fixtures/store'); const checkPermission = require('./permission-checker'); const supertest = require('supertest'); const getApp = require('../app'); +const getLogger = require('../../test/fixtures/no-logger'); const { EventEmitter } = require('events'); const eventBus = new EventEmitter(); @@ -16,6 +17,7 @@ function getSetup(preRouterHook) { baseUriPath: base, stores, eventBus, + getLogger, preRouterHook(_app) { preRouterHook(_app); diff --git a/lib/middleware/request-logger.js b/lib/middleware/request-logger.js index 4916f51d59..8223dd1413 100644 --- a/lib/middleware/request-logger.js +++ b/lib/middleware/request-logger.js @@ -1,9 +1,9 @@ 'use strict'; const url = require('url'); -const logger = require('../logger')('HTTP'); module.exports = function(config) { + const logger = config.getLogger('HTTP'); return (req, res, next) => { next(); if (config.enableRequestLogger) { diff --git a/lib/options.js b/lib/options.js index a68a73d4d5..f6b829031d 100644 --- a/lib/options.js +++ b/lib/options.js @@ -1,6 +1,7 @@ 'use strict'; const { publicFolder } = require('unleash-frontend'); +const { defaultLogProvider, validateLogProvider } = require('./logger'); const isDev = () => process.env.NODE_ENV === 'development'; const THIRTY_DAYS = 30 * 24 * 60 * 60 * 1000; @@ -23,6 +24,7 @@ const DEFAULT_OPTIONS = { ui: {}, importFile: undefined, dropBeforeImport: false, + getLogger: defaultLogProvider, }; module.exports = { @@ -45,6 +47,8 @@ module.exports = { ? { path: options.pipe } : { port: options.port, host: options.host }; + validateLogProvider(options.getLogger); + return options; }, }; diff --git a/lib/routes/admin-api/archive.js b/lib/routes/admin-api/archive.js index 25b4e8ac44..202270fc45 100644 --- a/lib/routes/admin-api/archive.js +++ b/lib/routes/admin-api/archive.js @@ -2,7 +2,6 @@ const Controller = require('../controller'); -const logger = require('../../logger')('/admin-api/archive.js'); const { FEATURE_REVIVED } = require('../../event-type'); const extractUser = require('../../extract-user'); const { UPDATE_FEATURE } = require('../../permissions'); @@ -10,6 +9,7 @@ const { UPDATE_FEATURE } = require('../../permissions'); class ArchiveController extends Controller { constructor(config) { super(config); + this.logger = config.getLogger('/admin-api/archive.js'); this.featureToggleStore = config.stores.featureToggleStore; this.eventStore = config.stores.eventStore; @@ -33,7 +33,7 @@ class ArchiveController extends Controller { }); res.status(200).end(); } catch (error) { - logger.error('Server failed executing request', error); + this.logger.error('Server failed executing request', error); return res.status(500).end(); } } diff --git a/lib/routes/admin-api/archive.test.js b/lib/routes/admin-api/archive.test.js index 54e356e456..0219f3ccd8 100644 --- a/lib/routes/admin-api/archive.test.js +++ b/lib/routes/admin-api/archive.test.js @@ -3,6 +3,7 @@ const test = require('ava'); const store = require('./../../../test/fixtures/store'); const permissions = require('../../../test/fixtures/permissions'); +const getLogger = require('../../../test/fixtures/no-logger'); const supertest = require('supertest'); const getApp = require('../../app'); const { UPDATE_FEATURE } = require('../../permissions'); @@ -20,6 +21,7 @@ function getSetup() { eventBus, extendedPermissions: true, preRouterHook: perms.hook, + getLogger, }); return { diff --git a/lib/routes/admin-api/config.test.js b/lib/routes/admin-api/config.test.js index c7355098f0..7038000cc0 100644 --- a/lib/routes/admin-api/config.test.js +++ b/lib/routes/admin-api/config.test.js @@ -2,6 +2,7 @@ const test = require('ava'); const store = require('./../../../test/fixtures/store'); +const getLogger = require('../../../test/fixtures/no-logger'); const supertest = require('supertest'); const getApp = require('../../app'); @@ -22,6 +23,7 @@ function getSetup() { eventBus, extendedPermissions: false, ui: uiConfig, + getLogger, }); return { diff --git a/lib/routes/admin-api/events.test.js b/lib/routes/admin-api/events.test.js index 0bb35575a2..ff01752fee 100644 --- a/lib/routes/admin-api/events.test.js +++ b/lib/routes/admin-api/events.test.js @@ -3,6 +3,8 @@ const test = require('ava'); const store = require('./../../../test/fixtures/store'); +const getLogger = require('../../../test/fixtures/no-logger'); + const supertest = require('supertest'); const getApp = require('../../app'); @@ -16,6 +18,7 @@ function getSetup() { baseUriPath: base, stores, eventBus, + getLogger, }); return { base, eventStore: stores.eventStore, request: supertest(app) }; diff --git a/lib/routes/admin-api/feature.js b/lib/routes/admin-api/feature.js index c5d492f015..b0ac05337c 100644 --- a/lib/routes/admin-api/feature.js +++ b/lib/routes/admin-api/feature.js @@ -24,6 +24,7 @@ class FeatureController extends Controller { super(config); this.featureToggleStore = config.stores.featureToggleStore; this.eventStore = config.stores.eventStore; + this.logger = config.getLogger('/admin-api/feature.js'); this.get('/', this.getAllToggles); this.post('/', this.createToggle, CREATE_FEATURE); @@ -59,7 +60,7 @@ class FeatureController extends Controller { await this.validateUniqueName(name); res.status(201).end(); } catch (error) { - handleErrors(res, error); + handleErrors(res, this.logger, error); } } @@ -94,7 +95,7 @@ class FeatureController extends Controller { }); res.status(201).end(); } catch (error) { - handleErrors(res, error); + handleErrors(res, this.logger, error); } } @@ -115,7 +116,7 @@ class FeatureController extends Controller { }); res.status(200).end(); } catch (error) { - handleErrors(res, error); + handleErrors(res, this.logger, error); } } @@ -127,7 +128,7 @@ class FeatureController extends Controller { const enabled = !feature.enabled; this._toggle(enabled, req, res); } catch (error) { - handleErrors(res, error); + handleErrors(res, this.logger, error); } } @@ -156,7 +157,7 @@ class FeatureController extends Controller { }); res.json(feature).end(); } catch (error) { - handleErrors(res, error); + handleErrors(res, this.logger, error); } } @@ -175,7 +176,7 @@ class FeatureController extends Controller { }); res.status(200).end(); } catch (error) { - handleErrors(res, error); + handleErrors(res, this.logger, error); } } } diff --git a/lib/routes/admin-api/feature.test.js b/lib/routes/admin-api/feature.test.js index 365ff25930..8268e3e326 100644 --- a/lib/routes/admin-api/feature.test.js +++ b/lib/routes/admin-api/feature.test.js @@ -3,6 +3,7 @@ const test = require('ava'); const store = require('./../../../test/fixtures/store'); const permissions = require('../../../test/fixtures/permissions'); +const getLogger = require('../../../test/fixtures/no-logger'); const supertest = require('supertest'); const getApp = require('../../app'); const { UPDATE_FEATURE, CREATE_FEATURE } = require('../../permissions'); @@ -20,6 +21,7 @@ function getSetup() { eventBus, extendedPermissions: true, preRouterHook: perms.hook, + getLogger, }); return { diff --git a/lib/routes/admin-api/metrics.js b/lib/routes/admin-api/metrics.js index ea32f63160..1bd3d0e6e5 100644 --- a/lib/routes/admin-api/metrics.js +++ b/lib/routes/admin-api/metrics.js @@ -2,7 +2,6 @@ const joi = require('joi'); const Controller = require('../controller'); -const logger = require('../../logger')('/admin-api/metrics.js'); const ClientMetrics = require('../../client-metrics'); const schema = require('./metrics-schema'); const { UPDATE_APPLICATION } = require('../../permissions'); @@ -10,6 +9,7 @@ const { UPDATE_APPLICATION } = require('../../permissions'); class MetricsController extends Controller { constructor(config) { super(config); + this.logger = config.getLogger('/admin-api/metrics.js'); const { clientMetricsStore, clientInstanceStore, @@ -83,7 +83,7 @@ class MetricsController extends Controller { const { value: applicationData, error } = joi.validate(input, schema); if (error) { - logger.warn('Invalid application data posted', error); + this.logger.warn('Invalid application data posted', error); return res.status(400).json(error); } @@ -91,7 +91,7 @@ class MetricsController extends Controller { await this.clientApplicationsStore.upsert(applicationData); res.status(202).end(); } catch (err) { - logger.error(err); + this.logger.error(err); res.status(500).end(); } } @@ -103,7 +103,7 @@ class MetricsController extends Controller { ); res.json({ applications }); } catch (err) { - logger.error(err); + this.logger.error(err); res.status(500).end(); } } @@ -147,7 +147,7 @@ class MetricsController extends Controller { }; res.json(appDetails); } catch (err) { - logger.error(err); + this.logger.error(err); res.status(500).end(); } } diff --git a/lib/routes/admin-api/metrics.test.js b/lib/routes/admin-api/metrics.test.js index 0471aaed47..c68590b785 100644 --- a/lib/routes/admin-api/metrics.test.js +++ b/lib/routes/admin-api/metrics.test.js @@ -3,6 +3,7 @@ const test = require('ava'); const store = require('./../../../test/fixtures/store'); const permissions = require('../../../test/fixtures/permissions'); +const getLogger = require('../../../test/fixtures/no-logger'); const supertest = require('supertest'); const getApp = require('../../app'); const { UPDATE_APPLICATION } = require('../../permissions'); @@ -19,6 +20,7 @@ function getSetup() { eventBus, extendedPermissions: true, preRouterHook: perms.hook, + getLogger, }); return { diff --git a/lib/routes/admin-api/state.js b/lib/routes/admin-api/state.js index 48b143eb98..55221aa48c 100644 --- a/lib/routes/admin-api/state.js +++ b/lib/routes/admin-api/state.js @@ -13,6 +13,7 @@ const upload = multer({ limits: { fileSize: 5242880 } }); class StateController extends Controller { constructor(config) { super(config); + this.logger = config.getLogger('/admin-api/state.js'); this.fileupload('/import', upload.single('file'), this.import, ADMIN); this.get('/export', this.export, ADMIN); } @@ -40,7 +41,7 @@ class StateController extends Controller { }); res.sendStatus(202); } catch (err) { - handleErrors(res, err); + handleErrors(res, this.logger, err); } } @@ -75,7 +76,7 @@ class StateController extends Controller { res.json(data); } } catch (err) { - handleErrors(res, err); + handleErrors(res, this.logger, err); } } } diff --git a/lib/routes/admin-api/strategy.js b/lib/routes/admin-api/strategy.js index 9d079fd352..185fa24e03 100644 --- a/lib/routes/admin-api/strategy.js +++ b/lib/routes/admin-api/strategy.js @@ -18,6 +18,7 @@ const version = 1; class StrategyController extends Controller { constructor(config) { super(config); + this.logger = config.getLogger('/admin-api/strategy.js'); this.strategyStore = config.stores.strategyStore; this.eventStore = config.stores.eventStore; @@ -58,7 +59,7 @@ class StrategyController extends Controller { }); res.status(200).end(); } catch (error) { - handleErrors(res, error); + handleErrors(res, this.logger, error); } } @@ -73,7 +74,7 @@ class StrategyController extends Controller { }); res.status(201).end(); } catch (error) { - handleErrors(res, error); + handleErrors(res, this.logger, error); } } @@ -92,7 +93,7 @@ class StrategyController extends Controller { }); res.status(200).end(); } catch (error) { - handleErrors(res, error); + handleErrors(res, this.logger, error); } } diff --git a/lib/routes/admin-api/strategy.test.js b/lib/routes/admin-api/strategy.test.js index 6b0499c545..af451261e5 100644 --- a/lib/routes/admin-api/strategy.test.js +++ b/lib/routes/admin-api/strategy.test.js @@ -3,6 +3,7 @@ const test = require('ava'); const store = require('./../../../test/fixtures/store'); const permissions = require('../../../test/fixtures/permissions'); +const getLogger = require('../../../test/fixtures/no-logger'); const supertest = require('supertest'); const getApp = require('../../app'); const { @@ -22,6 +23,7 @@ function getSetup() { baseUriPath: base, stores, eventBus, + getLogger, extendedPermissions: true, preRouterHook: perms.hook, }); diff --git a/lib/routes/admin-api/user.test.js b/lib/routes/admin-api/user.test.js index 2cd5aa6ac0..c16e3c47c5 100644 --- a/lib/routes/admin-api/user.test.js +++ b/lib/routes/admin-api/user.test.js @@ -2,6 +2,7 @@ const test = require('ava'); const store = require('./../../../test/fixtures/store'); +const getLogger = require('../../../test/fixtures/no-logger'); const supertest = require('supertest'); const getApp = require('../../app'); const User = require('../../user'); @@ -18,6 +19,7 @@ function getSetup() { baseUriPath: base, stores, eventBus, + getLogger, preHook: a => { a.use((req, res, next) => { req.user = currentUser; diff --git a/lib/routes/admin-api/util.js b/lib/routes/admin-api/util.js index c1a5c31b16..a476ecd0d3 100644 --- a/lib/routes/admin-api/util.js +++ b/lib/routes/admin-api/util.js @@ -1,7 +1,5 @@ 'use strict'; -const logger = require('../../logger')('/admin-api/util.js'); - const joi = require('joi'); const customJoi = joi.extend(j => ({ @@ -36,7 +34,7 @@ const nameType = customJoi .max(100) .required(); -const handleErrors = (res, error) => { +const handleErrors = (res, logger, error) => { logger.warn(error.message); switch (error.name) { case 'NotFoundError': diff --git a/lib/routes/backstage.test.js b/lib/routes/backstage.test.js index e27d268d24..eaf434f7ad 100644 --- a/lib/routes/backstage.test.js +++ b/lib/routes/backstage.test.js @@ -2,6 +2,7 @@ const test = require('ava'); const store = require('./../../test/fixtures/store'); +const getLogger = require('../../test/fixtures/no-logger'); const supertest = require('supertest'); const getApp = require('../app'); @@ -16,6 +17,7 @@ test('should use enable prometheus', t => { serverMetrics: true, stores, eventBus, + getLogger, }); const request = supertest(app); diff --git a/lib/routes/client-api/feature.test.js b/lib/routes/client-api/feature.test.js index cbd54462fb..34d0af154b 100644 --- a/lib/routes/client-api/feature.test.js +++ b/lib/routes/client-api/feature.test.js @@ -2,6 +2,7 @@ const test = require('ava'); const store = require('./../../../test/fixtures/store'); +const getLogger = require('../../../test/fixtures/no-logger'); const supertest = require('supertest'); const getApp = require('../../app'); @@ -15,6 +16,7 @@ function getSetup() { baseUriPath: base, stores, eventBus, + getLogger, }); return { diff --git a/lib/routes/client-api/index.js b/lib/routes/client-api/index.js index 80f7458990..362e58e3a9 100644 --- a/lib/routes/client-api/index.js +++ b/lib/routes/client-api/index.js @@ -11,11 +11,12 @@ class ClientApi extends Controller { super(); const stores = config.stores; + const getLogger = config.getLogger; this.get('/', this.index); - this.use('/features', new FeatureController(stores).router); - this.use('/metrics', new MetricsController(stores).router); - this.use('/register', new RegisterController(stores).router); + this.use('/features', new FeatureController(stores, getLogger).router); + this.use('/metrics', new MetricsController(stores, getLogger).router); + this.use('/register', new RegisterController(stores, getLogger).router); } index(req, res) { diff --git a/lib/routes/client-api/metrics.js b/lib/routes/client-api/metrics.js index bdd128bd14..de1d46b055 100644 --- a/lib/routes/client-api/metrics.js +++ b/lib/routes/client-api/metrics.js @@ -1,14 +1,14 @@ 'use strict'; const joi = require('joi'); -const logger = require('../../logger')('client-api/metrics.js'); const Controller = require('../controller'); const { clientMetricsSchema } = require('./metrics-schema'); class ClientMetricsController extends Controller { - constructor({ clientMetricsStore, clientInstanceStore }) { + constructor({ clientMetricsStore, clientInstanceStore }, getLogger) { super(); + this.logger = getLogger('/api/client/metrics'); this.clientMetricsStore = clientMetricsStore; this.clientInstanceStore = clientInstanceStore; @@ -22,7 +22,7 @@ class ClientMetricsController extends Controller { const { error, value } = joi.validate(data, clientMetricsSchema); if (error) { - logger.warn('Invalid metrics posted', error); + this.logger.warn('Invalid metrics posted', error); return res.status(400).json(error); } @@ -35,7 +35,7 @@ class ClientMetricsController extends Controller { }); res.status(202).end(); } catch (e) { - logger.error('failed to store metrics', e); + this.logger.error('failed to store metrics', e); res.status(500).end(); } } diff --git a/lib/routes/client-api/metrics.test.js b/lib/routes/client-api/metrics.test.js index bbb092aeb2..72bb80a7a0 100644 --- a/lib/routes/client-api/metrics.test.js +++ b/lib/routes/client-api/metrics.test.js @@ -2,6 +2,7 @@ const test = require('ava'); const store = require('./../../../test/fixtures/store'); +const getLogger = require('../../../test/fixtures/no-logger'); const supertest = require('supertest'); const getApp = require('../../app'); @@ -14,6 +15,7 @@ function getSetup() { baseUriPath: '', stores, eventBus, + getLogger, }); return { diff --git a/lib/routes/client-api/register.js b/lib/routes/client-api/register.js index 5595a936ad..132431e1ac 100644 --- a/lib/routes/client-api/register.js +++ b/lib/routes/client-api/register.js @@ -1,14 +1,14 @@ 'use strict'; const joi = require('joi'); -const logger = require('../../logger')('/client-api/register.js'); const Controller = require('../controller'); const { clientRegisterSchema: schema } = require('./register-schema'); class RegisterController extends Controller { - constructor({ clientInstanceStore, clientApplicationsStore }) { + constructor({ clientInstanceStore, clientApplicationsStore }, getLogger) { super(); + this.logger = getLogger('/api/client/register'); this.clientInstanceStore = clientInstanceStore; this.clientApplicationsStore = clientApplicationsStore; @@ -20,7 +20,7 @@ class RegisterController extends Controller { const { value: clientRegistration, error } = joi.validate(data, schema); if (error) { - logger.warn('Invalid client data posted', error); + this.logger.warn('Invalid client data posted', error); return res.status(400).json(error); } @@ -29,14 +29,14 @@ class RegisterController extends Controller { try { await this.clientApplicationsStore.upsert(clientRegistration); await this.clientInstanceStore.insert(clientRegistration); - logger.info( + this.logger.info( `New client registered with appName=${ clientRegistration.appName } and instanceId=${clientRegistration.instanceId}` ); return res.status(202).end(); } catch (err) { - logger.error('failed to register client', err); + this.logger.error('failed to register client', err); return res.status(500).end(); } } diff --git a/lib/routes/client-api/register.test.js b/lib/routes/client-api/register.test.js index 97d7d3dab4..163db88517 100644 --- a/lib/routes/client-api/register.test.js +++ b/lib/routes/client-api/register.test.js @@ -2,6 +2,7 @@ const test = require('ava'); const store = require('./../../../test/fixtures/store'); +const getLogger = require('../../../test/fixtures/no-logger'); const supertest = require('supertest'); const getApp = require('../../app'); @@ -14,6 +15,7 @@ function getSetup() { baseUriPath: '', stores, eventBus, + getLogger, }); return { @@ -89,6 +91,7 @@ test('should fail if store fails', t => { baseUriPath: '', stores, eventBus, + getLogger, }); // --- end custom config diff --git a/lib/routes/health-check.js b/lib/routes/health-check.js index 9d5871a1cc..bfd41a1efd 100644 --- a/lib/routes/health-check.js +++ b/lib/routes/health-check.js @@ -1,12 +1,12 @@ 'use strict'; -const logger = require('../logger')('health-check.js'); const Controller = require('./controller'); class HealthCheckController extends Controller { constructor(config) { super(); this.db = config.stores.db; + this.logger = config.getLogger('health-check.js'); this.get('/', (req, res) => this.index(req, res)); } @@ -16,7 +16,7 @@ class HealthCheckController extends Controller { await this.db.select(1).from('features'); res.json({ health: 'GOOD' }); } catch (e) { - logger.error('Could not select from features, error was: ', e); + this.logger.error('Could not select from features, error was: ', e); res.status(500).json({ health: 'BAD' }); } } diff --git a/lib/routes/health-check.test.js b/lib/routes/health-check.test.js index 4b4190cc07..74ab3d5bf9 100644 --- a/lib/routes/health-check.test.js +++ b/lib/routes/health-check.test.js @@ -2,6 +2,7 @@ const test = require('ava'); const store = require('./../../test/fixtures/store'); +const getLogger = require('../../test/fixtures/no-logger'); const supertest = require('supertest'); const getApp = require('../app'); @@ -15,6 +16,7 @@ function getSetup() { baseUriPath: '', stores, eventBus, + getLogger, }); return { diff --git a/lib/routes/index.test.js b/lib/routes/index.test.js index d9715e2d97..b74f2f25d0 100644 --- a/lib/routes/index.test.js +++ b/lib/routes/index.test.js @@ -2,6 +2,7 @@ const test = require('ava'); const store = require('./../../test/fixtures/store'); +const getLogger = require('../../test/fixtures/no-logger'); const supertest = require('supertest'); const getApp = require('../app'); @@ -16,6 +17,7 @@ function getSetup() { stores, eventBus, enableLegacyRoutes: true, + getLogger, }); return { diff --git a/lib/server-impl.js b/lib/server-impl.js index 9ec4b3171f..d0bf055d92 100644 --- a/lib/server-impl.js +++ b/lib/server-impl.js @@ -2,8 +2,6 @@ const { EventEmitter } = require('events'); -const logFactory = require('./logger'); -const logger = require('./logger')('server-impl.js'); const migrator = require('../migrator'); const getApp = require('./app'); @@ -16,6 +14,7 @@ const AuthenticationRequired = require('./authentication-required'); async function createApp(options) { // Database dependecies (statefull) + const logger = options.getLogger('server-impl.js'); const stores = createStores(options); const eventBus = new EventEmitter(); @@ -23,7 +22,7 @@ async function createApp(options) { { stores, eventBus, - logFactory, + logFactory: options.getLogger, // TODO: remove in v4.x }, options ); @@ -65,6 +64,7 @@ async function createApp(options) { async function start(opts) { const options = createOptions(opts); + const logger = options.getLogger('server-impl.js'); try { await migrator(options); diff --git a/lib/server-impl.test.js b/lib/server-impl.test.js index dbaa39dfc1..711dde924e 100644 --- a/lib/server-impl.test.js +++ b/lib/server-impl.test.js @@ -3,6 +3,7 @@ const test = require('ava'); const proxyquire = require('proxyquire'); const express = require('express'); +const getLogger = require('../test/fixtures/no-logger'); const getApp = proxyquire('./app', { './routes': class Index { @@ -38,6 +39,7 @@ test('should call preHook', async t => { let called = 0; await serverImpl.start({ port: 0, + getLogger, preHook: () => { called++; }, @@ -49,6 +51,7 @@ test('should call preRouterHook', async t => { let called = 0; await serverImpl.start({ port: 0, + getLogger, preRouterHook: () => { called++; }, diff --git a/lib/state-service.js b/lib/state-service.js index cb91e2afc5..7c5a42211e 100644 --- a/lib/state-service.js +++ b/lib/state-service.js @@ -5,7 +5,6 @@ const fs = require('fs'); const mime = require('mime'); const { featureShema } = require('./routes/admin-api/feature-schema'); const strategySchema = require('./routes/admin-api/strategy-schema'); -const getLogger = require('./logger'); const YAML = require('js-yaml'); const { FEATURE_IMPORT, @@ -14,8 +13,6 @@ const { DROP_STRATEGIES, } = require('./event-type'); -const logger = getLogger('state-service.js'); - const dataSchema = joi.object().keys({ version: joi.number(), features: joi @@ -43,6 +40,7 @@ function parseFile(file, data) { class StateService { constructor(config) { this.config = config; + this.logger = config.getLogger('state-service.js'); } importFile({ file, dropBeforeImport, userName }) { @@ -57,9 +55,11 @@ class StateService { const importData = await joi.validate(data, dataSchema); if (importData.features) { - logger.info(`Importing ${importData.features.length} features`); + this.logger.info( + `Importing ${importData.features.length} features` + ); if (dropBeforeImport) { - logger.info(`Dropping existing features`); + this.logger.info(`Dropping existing features`); await eventStore.store({ type: DROP_FEATURES, createdBy: userName, @@ -78,9 +78,11 @@ class StateService { } if (importData.strategies) { - logger.info(`Importing ${importData.strategies.length} strategies`); + this.logger.info( + `Importing ${importData.strategies.length} strategies` + ); if (dropBeforeImport) { - logger.info(`Dropping existing strategies`); + this.logger.info(`Dropping existing strategies`); await eventStore.store({ type: DROP_STRATEGIES, createdBy: userName, diff --git a/lib/state-service.test.js b/lib/state-service.test.js index 26e3e68cc0..306af816c8 100644 --- a/lib/state-service.test.js +++ b/lib/state-service.test.js @@ -3,6 +3,8 @@ const test = require('ava'); const store = require('./../test/fixtures/store'); +const getLogger = require('./../test/fixtures/no-logger'); + const StateService = require('./state-service'); const { FEATURE_IMPORT, @@ -13,7 +15,7 @@ const { function getSetup() { const stores = store.createStores(); - return { stateService: new StateService({ stores }), stores }; + return { stateService: new StateService({ stores, getLogger }), stores }; } test('should import a feature', async t => { diff --git a/logger.js b/logger.js deleted file mode 100644 index fde89611dd..0000000000 --- a/logger.js +++ /dev/null @@ -1,5 +0,0 @@ -'use strict'; - -const logger = require('./lib/logger'); - -exports.setLoggerProvider = logger.setLoggerProvider; diff --git a/package.json b/package.json index ba53ec73ec..6b6762c6bc 100644 --- a/package.json +++ b/package.json @@ -34,7 +34,7 @@ "scripts": { "start": "node server.js", "start:google": "node examples/google-auth-unleash.js", - "start:dev": "NODE_ENV=development supervisor --ignore ./node_modules/ server.js", + "start:dev": "NODE_ENV=development supervisor --ignore ./node_modules/,website server.js", "start:dev:pg": "pg_virtualenv npm run start:dev:pg-chain", "start:dev:pg-chain": "export DATABASE_URL=postgres://$PGUSER:$PGPASSWORD@localhost:$PGPORT/postgres ; db-migrate up && npm run start:dev", "db-migrate": "db-migrate", diff --git a/test/e2e/helpers/database-init.js b/test/e2e/helpers/database-init.js index 55df2fcb07..01b3d46393 100644 --- a/test/e2e/helpers/database-init.js +++ b/test/e2e/helpers/database-init.js @@ -49,12 +49,13 @@ function createFeatures(store) { return dbState.features.map(f => store._createFeature(f)); } -module.exports = async function init(databaseSchema = 'test') { +module.exports = async function init(databaseSchema = 'test', getLogger) { const options = { databaseUrl: require('./database-config').getDatabaseUrl(), databaseSchema, minPool: 0, maxPool: 0, + getLogger, }; const db = createDb(options); diff --git a/test/e2e/helpers/test-helper.js b/test/e2e/helpers/test-helper.js index f36ffd1e78..36c9e742f7 100644 --- a/test/e2e/helpers/test-helper.js +++ b/test/e2e/helpers/test-helper.js @@ -6,6 +6,7 @@ const supertest = require('supertest'); const getApp = require('../../../lib/app'); const dbInit = require('./database-init'); +const getLogger = require('../../fixtures/no-logger'); const StateService = require('../../../lib/state-service'); const { EventEmitter } = require('events'); @@ -19,13 +20,14 @@ function createApp(stores, adminAuthentication = 'none', preHook) { adminAuthentication, secret: 'super-secret', sessionAge: 4000, - stateService: new StateService({ stores }), + stateService: new StateService({ stores, getLogger }), + getLogger, }); } module.exports = { async setupApp(name) { - const stores = await dbInit(name); + const stores = await dbInit(name, getLogger); const app = createApp(stores); return { @@ -34,7 +36,7 @@ module.exports = { }; }, async setupAppWithAuth(name) { - const stores = await dbInit(name); + const stores = await dbInit(name, getLogger); const app = createApp(stores, 'unsecure'); return { @@ -44,7 +46,7 @@ module.exports = { }, async setupAppWithCustomAuth(name, preHook) { - const stores = await dbInit(name); + const stores = await dbInit(name, getLogger); const app = createApp(stores, 'custom', preHook); return { diff --git a/test/fixtures/no-logger.js b/test/fixtures/no-logger.js new file mode 100644 index 0000000000..b0d28d3a6f --- /dev/null +++ b/test/fixtures/no-logger.js @@ -0,0 +1,11 @@ +'use strict'; + +module.exports = function noLoggerProvider() { + // do something with the name + return { + debug: () => {}, + info: () => {}, + warn: () => {}, + error: () => {}, + }; +};