Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: remove fileTransport from consoleLogger #6

Merged
merged 1 commit into from
Aug 11, 2016
Merged

feat: remove fileTransport from consoleLogger #6

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 0 additions & 14 deletions lib/egg/console_logger.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

const Logger = require('../logger');
const ConsoleTransport = require('../transports/console');
const FileTransport = require('../transports/file');
const utils = require('../utils');

/**
Expand All @@ -13,8 +12,6 @@ class ConsoleLogger extends Logger {
/**
* @constructor
* @param {Object} options
* - {String} [errorFile] - error file
* if specify this file, logger will redirect error log to the file.
* - {String} [encoding] - log string encoding, default is 'utf8'
*/
constructor(options) {
Expand All @@ -24,21 +21,10 @@ class ConsoleLogger extends Logger {
level: this.options.level,
formatter: utils.consoleFormatter,
}));

if (this.options.errorFile) {
const fileTransport = new FileTransport({
file: this.options.errorFile,
level: 'ERROR', // 只输出 error 级别的到文件
encoding: this.options.encoding,
formatter: utils.defaultFormatter,
});
this.set('file', fileTransport);
}
}

get defaults() {
return {
errorFile: '',
encoding: 'utf8',
level: process.env.NODE_ENV === 'production' ? 'INFO' : 'WARN',
};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

master 的日志不再输出到 common error 了？那监控还得加上对 stderr 的日志了，这个日志目前监控平台是没有默认加的，要每个应用加，不太好吧。

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

好像也没法写得 common error，要不然 master 也得接收 reload log 的事件。。。

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个挺麻烦的，原因是 master 不会去 load config 了，所以就无法配置了。

但想想也对，master 进程不应该去管理这个，我们现在有些错误其实也是在 stderr 的，比如 master 意外退出，感觉监控 stderr 更加好。

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

嗯，到时候得找找监控的同学，看看能否默认将 stderr 也监控了。

Expand Down
16 changes: 0 additions & 16 deletions test/lib/egg/console_logger.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
'use strict';

const path = require('path');
const fs = require('fs');
const coffee = require('coffee');
const mm = require('mm');
const rimraf = require('rimraf');
Expand Down Expand Up @@ -45,21 +44,6 @@ describe('test/egg/console_logger.test.js', () => {
.end(done);
});

it('error log file print error level log only', done => {
const errorFile = path.join(tmp, 'error.log');
const options = {
errorFile,
};

coffee.fork(consoleLoggerFile, [ JSON.stringify(options) ])
.end(function() {
const content = fs.readFileSync(errorFile, 'utf8');
content.should.containEql('error foo');
content.should.not.containEql('warn foo');
done();
});
});

it('can set level on NODE_ENV = production', done => {
mm(process.env, 'NODE_ENV', 'production');
const options = {
Expand Down