From 515eaf57440c46b3d2dfda20b67d0e40446e6879 Mon Sep 17 00:00:00 2001 From: popomore Date: Tue, 1 Nov 2016 19:23:51 +0800 Subject: [PATCH] feat: add getLogger to app and ctx it can be override with custom function Closes eggjs/egg#36 --- app/extend/context.js | 49 ++++++++++++------- lib/egg.js | 18 +++++-- package.json | 4 +- test/app/extend/application.test.js | 13 +++++ test/app/extend/context.test.js | 39 +++++++++++++-- test/fixtures/apps/get-logger/app/router.js | 12 +++++ .../fixtures/apps/get-logger/config/config.js | 13 +++++ test/fixtures/apps/get-logger/package.json | 3 ++ test/lib/core/urllib.test.js | 5 +- test/utils.js | 1 + 10 files changed, 123 insertions(+), 34 deletions(-) create mode 100644 test/fixtures/apps/get-logger/app/router.js create mode 100644 test/fixtures/apps/get-logger/config/config.js create mode 100644 test/fixtures/apps/get-logger/package.json diff --git a/app/extend/context.js b/app/extend/context.js index d779e0b27a..51d41756ce 100644 --- a/app/extend/context.js +++ b/app/extend/context.js @@ -7,13 +7,12 @@ const Cookies = require('egg-cookies'); const co = require('co'); const util = require('../../lib/core/util'); -const LOGGER = Symbol('LOGGER'); -const CORE_LOGGER = Symbol('CORE_LOGGER'); const HELPER = Symbol('Context#helper'); const VIEW = Symbol('Context#view'); const LOCALS = Symbol('Context#locals'); const LOCALS_LIST = Symbol('Context#localsList'); const COOKIES = Symbol('Context#cookies'); +const CONTEXT_LOGGERS = Symbol('Context#logger'); const proto = module.exports = { get cookies() { @@ -183,10 +182,31 @@ const proto = module.exports = { }, /** - * 应用 Web 相关日志，用于记录 Web 行为相关的日志， - * 最终日志文件输出到 `{log.dir}/{app.name}-web.log` 中。 - * 每行日志会自动记录上当前请求的一些基本信息， - * 如 `[$logonId/$userId/$ip/$timestamp_ms/$sofaTraceId $use_ms $method $url]` + * Wrap app.loggers with context infomation, + * if a custom logger is defined by naming aLogger, then you can `ctx.getLogger('aLogger')` + * @param {String} name - logger name + * @return {Logger} logger + */ + getLogger(name) { + let cache = this[CONTEXT_LOGGERS]; + if (!cache) { + cache = this[CONTEXT_LOGGERS] = {}; + } + + // read from cache + if (cache[name]) return cache[name]; + + // get no exist logger + const appLogger = this.app.getLogger(name); + if (!appLogger) return null; + + // write to cache + cache[name] = new ContextLogger(this, appLogger); + return cache[name]; + }, + + /** + * Logger for Application, wrapping app.coreLogger with context infomation * @member {ContextLogger} Context#logger * @since 1.0.0 * @example @@ -194,28 +214,19 @@ const proto = module.exports = { * this.logger.info('some request data: %j', this.request.body); * this.logger.warn('WARNING!!!!'); * ``` - * 错误日志记录，直接会将错误日志完整堆栈信息记录下来，并且输出到 `{log.dir}/common-error.log` - * ``` - * this.logger.error(err); - * ``` */ get logger() { - if (!this[LOGGER]) { - this[LOGGER] = new ContextLogger(this, this.app.logger); - } - return this[LOGGER]; + return this.getLogger('logger'); }, /** - * app context 级别的 core logger，适合 core 对当前请求记录日志使用 + * Logger for frameworks and plugins, + * wrapping app.coreLogger with context infomation * @member {ContextLogger} Context#coreLogger * @since 1.0.0 */ get coreLogger() { - if (!this[CORE_LOGGER]) { - this[CORE_LOGGER] = new ContextLogger(this, this.app.coreLogger); - } - return this[CORE_LOGGER]; + return this.getLogger('coreLogger'); }, /** diff --git a/lib/egg.js b/lib/egg.js index e8a43b5e12..b5e717ac7f 100644 --- a/lib/egg.js +++ b/lib/egg.js @@ -154,9 +154,7 @@ class EggApplication extends EggCore { } /** - * logger 集合，包含两个： - * - 应用使用：loggers.logger - * - 框架使用：loggers.coreLogger + * All loggers contain logger, coreLogger and customLogger * @member {Object} * @since 1.0.0 */ @@ -167,13 +165,23 @@ class EggApplication extends EggCore { return this[LOGGERS]; } + /** + * Get logger by name, it's equal to app.loggers['name'], + * but you can extend it with your own logical. + * @param {String} name - logger name + * @return {Logger} logger + */ + getLogger(name) { + return this.loggers[name] || null; + } + /** * 同 {@link Agent#coreLogger} 相同 * @member {Logger} * @since 1.0.0 */ get logger() { - return this.loggers.logger; + return this.getLogger('logger'); } /** @@ -182,7 +190,7 @@ class EggApplication extends EggCore { * @since 1.0.0 */ get coreLogger() { - return this.loggers.coreLogger; + return this.getLogger('coreLogger'); } _unhandledRejectionHandler(err) { diff --git a/package.json b/package.json index 8119f48ec0..c1b18b585f 100644 --- a/package.json +++ b/package.json @@ -60,7 +60,7 @@ "egg-alinode": "^1.0.3", "egg-bin": "^1.3.0", "egg-ci": "^1.0.3", - "egg-mock": "^0.0.8", + "egg-mock": "^1.0.0", "egg-plugin-puml": "1", "egg-view-nunjucks": "^0.5.0", "eslint": "^3.0.0", @@ -83,7 +83,7 @@ "should": "^11.1.1", "stream-wormhole": "^1.0.0", "supertest": "^2.0.1", - "toa": "^2.3.1", + "toa": "^2.4.0", "toa-router": "^1.5.2" }, "main": "index.js", diff --git a/test/app/extend/application.test.js b/test/app/extend/application.test.js index 6e09eadb3f..90e5964591 100644 --- a/test/app/extend/application.test.js +++ b/test/app/extend/application.test.js @@ -5,6 +5,7 @@ const mm = require('egg-mock'); const utils = require('../../utils'); describe('test/app/extend/application.test.js', () => { + describe('app.logger', () => { let app; before(() => { @@ -16,6 +17,18 @@ describe('test/app/extend/application.test.js', () => { it('should alias app.logger => app.loggers.logger', () => { app.logger.should.equal(app.loggers.logger); }); + + it('should alias app.coreLooger => app.loggers.coreLooger', () => { + app.coreLogger.should.equal(app.loggers.coreLogger); + }); + + it('should alias app.getLogger(\'coreLogger\') => app.loggers.coreLooger', () => { + app.getLogger('coreLogger').should.equal(app.loggers.coreLogger); + }); + + it('should alias app.getLogger(\'noexist\') => null', () => { + (app.getLogger('noexist') === null).should.be.true(); + }); }); describe('app.inspect()', () => { diff --git a/test/app/extend/context.test.js b/test/app/extend/context.test.js index b138c0779d..bf840cbad9 100644 --- a/test/app/extend/context.test.js +++ b/test/app/extend/context.test.js @@ -5,6 +5,8 @@ const path = require('path'); const mm = require('egg-mock'); const request = require('supertest'); const sleep = require('ko-sleep'); + + const utils = require('../../utils'); describe('test/app/extend/context.test.js', () => { @@ -26,7 +28,8 @@ describe('test/app/extend/context.test.js', () => { yield request(app.callback()) .get('/logger?message=foo') .expect('logger'); - yield sleep(100); + + yield sleep(5000); const errorContent = fs.readFileSync(path.join(logdir, 'common-error.log'), 'utf8'); errorContent.should.containEql('nodejs.Error: error foo'); @@ -59,7 +62,8 @@ describe('test/app/extend/context.test.js', () => { yield request(app.callback()) .get('/logger?message=foo') .expect('logger'); - yield sleep(100); + + yield sleep(5000); const errorContent = fs.readFileSync(path.join(logdir, 'common-error.log'), 'utf8'); errorContent.should.containEql('nodejs.Error: error foo'); @@ -86,7 +90,8 @@ describe('test/app/extend/context.test.js', () => { yield request(app.callback()) .get('/logger?message=foo') .expect('logger'); - yield sleep(100); + + yield sleep(5000); const errorContent = fs.readFileSync(path.join(logdir, 'common-error.log'), 'utf8'); errorContent.should.containEql('nodejs.Error: error foo'); @@ -104,6 +109,30 @@ describe('test/app/extend/context.test.js', () => { }); }); + describe('ctx.getLogger', () => { + let app; + before(() => { + app = utils.app('apps/get-logger'); + return app.ready(); + }); + after(() => app.close()); + + it('should return null when logger is not found', () => { + return request(app.callback()) + .get('/noExistLogger') + .expect('null'); + }); + + it('should log with padding message', function* () { + yield request(app.callback()) + .get('/logger') + .expect(200); + + const logPath = utils.getFilepath('apps/get-logger/logs/get-logger/a.log'); + fs.readFileSync(logPath, 'utf8').should.match(/\[-\/127.0.0.1\/-\/\d+ms GET \/logger] aaa/); + }); + }); + describe('properties', () => { let app; before(() => { @@ -339,7 +368,7 @@ describe('test/app/extend/context.test.js', () => { .get('/') .expect(200) .expect('hello'); - yield sleep(100); + yield sleep(5000); const logdir = app.config.logger.dir; const log = fs.readFileSync(path.join(logdir, 'ctx-background-web.log'), 'utf8'); log.should.match(/background run result file size: \d+/); @@ -352,7 +381,7 @@ describe('test/app/extend/context.test.js', () => { .get('/error') .expect(200) .expect('hello error'); - yield sleep(100); + yield sleep(5000); const logdir = app.config.logger.dir; const log = fs.readFileSync(path.join(logdir, 'common-error.log'), 'utf8'); log.should.match(/ENOENT: no such file or directory/); diff --git a/test/fixtures/apps/get-logger/app/router.js b/test/fixtures/apps/get-logger/app/router.js new file mode 100644 index 0000000000..1903c0ec42 --- /dev/null +++ b/test/fixtures/apps/get-logger/app/router.js @@ -0,0 +1,12 @@ +'use strict'; + +module.exports = function(app) { + app.get('/logger', function* () { + this.getLogger('aLogger').info('aaa'); + this.body = 'done'; + }); + + app.get('/noExistLogger', function* () { + this.body = this.app.getLogger('noexist') + ''; + }); +}; diff --git a/test/fixtures/apps/get-logger/config/config.js b/test/fixtures/apps/get-logger/config/config.js new file mode 100644 index 0000000000..53fba40f48 --- /dev/null +++ b/test/fixtures/apps/get-logger/config/config.js @@ -0,0 +1,13 @@ +'use strict'; + +const path = require('path'); + +module.exports = appInfo => { + return { + customLogger: { + aLogger: { + file: path.join(appInfo.baseDir, 'logs', appInfo.name, 'a.log'), + }, + }, + }; +}; diff --git a/test/fixtures/apps/get-logger/package.json b/test/fixtures/apps/get-logger/package.json new file mode 100644 index 0000000000..38918ab476 --- /dev/null +++ b/test/fixtures/apps/get-logger/package.json @@ -0,0 +1,3 @@ +{ + "name": "get-logger" +} diff --git a/test/lib/core/urllib.test.js b/test/lib/core/urllib.test.js index 5931d56827..1a437ca551 100644 --- a/test/lib/core/urllib.test.js +++ b/test/lib/core/urllib.test.js @@ -5,7 +5,7 @@ const urllib = require('../../../lib/core/urllib'); describe('test/lib/core/urllib.test.js', () => { let client; - const url = 'https://a.alipayobjects.com/aliBridge/1.0.0/aliBridge.min.js'; + const url = 'http://www.google.com/generate_204'; before(() => { client = urllib({ @@ -23,7 +23,6 @@ describe('test/lib/core/urllib.test.js', () => { it('should request ok with log', done => { const args = { dataType: 'text', - timeout: 10000, }; client.once('response', info => { info.req.options.headers['mock-traceid'].should.equal('mock-traceid'); @@ -32,7 +31,7 @@ describe('test/lib/core/urllib.test.js', () => { }); client.request(url, args); - }).timeout(10000); + }); it('should request callback with log', done => { client.once('response', info => { diff --git a/test/utils.js b/test/utils.js index 7e592a804d..811bf31aa9 100644 --- a/test/utils.js +++ b/test/utils.js @@ -45,6 +45,7 @@ exports.startLocalServer = () => { }); }); }; +process.once('exit', () => localServer.close()); exports.getFilepath = name => { return path.join(fixtures, name);