From 2910ce7f406c5d5a49a8a3c93e902a46cf427c5f Mon Sep 17 00:00:00 2001 From: Kevin Smathers Date: Mon, 24 Aug 2015 10:18:21 -0400 Subject: [PATCH 1/8] Add rotation of log files by size. Configuration requires user to provide only rotation by period or size - configuration by both is not allowed. Because of the buffering done by streams, the rotation will be approximate. This is especially true if log messages are coming in at a very high rate. --- lib/bunyan.js | 48 +++++++++++++++++++++++++++++++----------------- 1 file changed, 31 insertions(+), 17 deletions(-) diff --git a/lib/bunyan.js b/lib/bunyan.js index 24ff3bc1..66cb0938 100644 --- a/lib/bunyan.js +++ b/lib/bunyan.js @@ -1102,6 +1102,9 @@ function RotatingFileStream(options) { format('rotating-file stream "count" is not >= 0: %j in %j', this.count, this)); + assert.ok((options.period && !options.size) || (!options.period && options.size), + format('period and size setting on rotating-file are mutually exclusive')); + // Parse `options.period`. if (options.period) { // where scope is: @@ -1125,6 +1128,8 @@ function RotatingFileStream(options) { } this.periodNum = Number(m[1]); this.periodScope = m[2]; + } else if (options.size) { + this.rotateFileSize = options.size; } else { this.periodNum = 1; this.periodScope = 'd'; @@ -1154,20 +1159,22 @@ function RotatingFileStream(options) { util.inherits(RotatingFileStream, EventEmitter); RotatingFileStream.prototype._setupNextRot = function () { - var self = this; - this.rotAt = this._nextRotTime(); - var delay = this.rotAt - Date.now(); - // Cap timeout to Node's max setTimeout, see - // . - var TIMEOUT_MAX = 2147483647; // 2^31-1 - if (delay > TIMEOUT_MAX) { - delay = TIMEOUT_MAX; - } - this.timeout = setTimeout( - function () { self.rotate(); }, - delay); - if (typeof (this.timeout.unref) === 'function') { - this.timeout.unref(); + if (this.periodNum && this.periodScope) { + var self = this; + this.rotAt = this._nextRotTime(); + var delay = this.rotAt - Date.now(); + // Cap timeout to Node's max setTimeout, see + // . + var TIMEOUT_MAX = 2147483647; // 2^31-1 + if (delay > TIMEOUT_MAX) { + delay = TIMEOUT_MAX; + } + this.timeout = setTimeout( + function () { self.rotate(); }, + delay); + if (typeof (this.timeout.unref) === 'function') { + this.timeout.unref(); + } } } @@ -1258,8 +1265,10 @@ RotatingFileStream.prototype.rotate = function rotate() { // If rotation period is > ~25 days, we have to break into multiple // setTimeout's. See . - if (self.rotAt && self.rotAt > Date.now()) { - return self._setupNextRot(); + if (this.periodNum && this.periodScope) { + if (self.rotAt && self.rotAt > Date.now()) { + return self._setupNextRot(); + } } if (_DEBUG) { @@ -1339,7 +1348,12 @@ RotatingFileStream.prototype.write = function write(s) { this.rotQueue.push(s); return false; } else { - return this.stream.write(s); + if (this.stream.bytesWritten > this.rotateFileSize) { + this.rotate(); + return false; + } else { + return this.stream.write(s); + } } }; From 3f5cd4cfe6a757238e602595fc9a789956d9d6cf Mon Sep 17 00:00:00 2001 From: Kevin Smathers Date: Mon, 24 Aug 2015 17:20:19 -0400 Subject: [PATCH 2/8] Validate size option value. Rotate the initial log file immediately upon initialization if it is already above the configured size threshold. --- lib/bunyan.js | 36 +++++++++++++++++++++++++++++++++--- 1 file changed, 33 insertions(+), 3 deletions(-) diff --git a/lib/bunyan.js b/lib/bunyan.js index 66cb0938..3adecb54 100644 --- a/lib/bunyan.js +++ b/lib/bunyan.js @@ -1092,9 +1092,8 @@ if (mv) { function RotatingFileStream(options) { this.path = options.path; - this.stream = fs.createWriteStream(this.path, - {flags: 'a', encoding: 'utf8'}); this.count = (options.count == null ? 10 : options.count); + assert.equal(typeof (this.count), 'number', format('rotating-file stream "count" is not a number: %j (%s) in %j', this.count, typeof (this.count), this)); @@ -1129,6 +1128,10 @@ function RotatingFileStream(options) { this.periodNum = Number(m[1]); this.periodScope = m[2]; } else if (options.size) { + var m = /^[0-9]+$/; + if (!m) { + throw new Error(format('invalid file size: "%s"', options.size)); + } this.rotateFileSize = options.size; } else { this.periodNum = 1; @@ -1154,6 +1157,29 @@ function RotatingFileStream(options) { this.rotQueue = []; this.rotating = false; this._setupNextRot(); + + var self = this; + var createStream = true; + + + if (this.rotateFileSize) { + // Check current log file size. If already too large, go ahead and rotate. + if (fs.existsSync(this.path)) { + var stats = fs.statSync(this.path); + if (stats.size > this.rotateFileSize) { + // End of the rotate call will create a new write stream, let's not do that twice. Use flag to track + // whether we need to init here or if rotate will do it for us. + createStream = false; + self.rotate(); + } + } + } + + if (createStream) { + this.stream = fs.createWriteStream(this.path, + {flags: 'a', encoding: 'utf8'}); + } + } util.inherits(RotatingFileStream, EventEmitter); @@ -1280,7 +1306,11 @@ RotatingFileStream.prototype.rotate = function rotate() { } self.rotating = true; - self.stream.end(); // XXX can do moves sync after this? test at high rate + // Rotate file by size may rotate before the stream is created. Let's make sure we have a stream before trying to + // end it. + if (self.stream) { + self.stream.end(); // XXX can do moves sync after this? test at high rate + } function del() { var toDel = self.path + '.' + String(n - 1); From a7701ead406a062b767658f6131947ca325cbc5b Mon Sep 17 00:00:00 2001 From: Kevin Smathers Date: Mon, 24 Aug 2015 17:28:40 -0400 Subject: [PATCH 3/8] Better validation for max file size option. --- lib/bunyan.js | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/lib/bunyan.js b/lib/bunyan.js index 3adecb54..b15d9732 100644 --- a/lib/bunyan.js +++ b/lib/bunyan.js @@ -1128,11 +1128,10 @@ function RotatingFileStream(options) { this.periodNum = Number(m[1]); this.periodScope = m[2]; } else if (options.size) { - var m = /^[0-9]+$/; - if (!m) { - throw new Error(format('invalid file size: "%s"', options.size)); - } this.rotateFileSize = options.size; + assert.equal(typeof (this.rotateFileSize), 'number', + format('rotating-file stream "size" is not a number: %j (%s) in %j', + this.rotateFileSize, typeof (this.rotateFileSize), this)); } else { this.periodNum = 1; this.periodScope = 'd'; From 0121697e8fde53fcb9149722a37e6c96d4fc406b Mon Sep 17 00:00:00 2001 From: Kevin Smathers Date: Mon, 24 Aug 2015 17:47:37 -0400 Subject: [PATCH 4/8] Take into consideration initial file size. If not greater than max file size, continue to truncate to the file. --- lib/bunyan.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/bunyan.js b/lib/bunyan.js index b15d9732..1c98860f 100644 --- a/lib/bunyan.js +++ b/lib/bunyan.js @@ -1129,6 +1129,7 @@ function RotatingFileStream(options) { this.periodScope = m[2]; } else if (options.size) { this.rotateFileSize = options.size; + this.alreadyWritten = 0; assert.equal(typeof (this.rotateFileSize), 'number', format('rotating-file stream "size" is not a number: %j (%s) in %j', this.rotateFileSize, typeof (this.rotateFileSize), this)); @@ -1170,6 +1171,8 @@ function RotatingFileStream(options) { // whether we need to init here or if rotate will do it for us. createStream = false; self.rotate(); + } else { + self.alreadyWritten = stats.size; } } } @@ -1377,8 +1380,9 @@ RotatingFileStream.prototype.write = function write(s) { this.rotQueue.push(s); return false; } else { - if (this.stream.bytesWritten > this.rotateFileSize) { + if ((this.alreadyWritten + this.stream.bytesWritten) > this.rotateFileSize) { this.rotate(); + this.alreadyWritten = 0; return false; } else { return this.stream.write(s); From 89abb97e60600e06bf136cfa1ab5e009eba7b818 Mon Sep 17 00:00:00 2001 From: Kevin Smathers Date: Tue, 1 Sep 2015 14:57:31 -0400 Subject: [PATCH 5/8] Tweak to config to allow for config without size or period (will use default) --- lib/bunyan.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/bunyan.js b/lib/bunyan.js index 1c98860f..edfbcc71 100644 --- a/lib/bunyan.js +++ b/lib/bunyan.js @@ -1101,7 +1101,7 @@ function RotatingFileStream(options) { format('rotating-file stream "count" is not >= 0: %j in %j', this.count, this)); - assert.ok((options.period && !options.size) || (!options.period && options.size), + assert.ok((options.period && !options.size) || (!options.period && options.size) || (!options.period && !options.size), format('period and size setting on rotating-file are mutually exclusive')); // Parse `options.period`. From 4d14700c4023d771c4a379a9627c8a0feffd8f24 Mon Sep 17 00:00:00 2001 From: Kevin Smathers Date: Mon, 21 Sep 2015 11:18:42 -0400 Subject: [PATCH 6/8] Remove check for neither size nor period - was not allowed default to be applied --- lib/bunyan.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/bunyan.js b/lib/bunyan.js index edfbcc71..1c98860f 100644 --- a/lib/bunyan.js +++ b/lib/bunyan.js @@ -1101,7 +1101,7 @@ function RotatingFileStream(options) { format('rotating-file stream "count" is not >= 0: %j in %j', this.count, this)); - assert.ok((options.period && !options.size) || (!options.period && options.size) || (!options.period && !options.size), + assert.ok((options.period && !options.size) || (!options.period && options.size), format('period and size setting on rotating-file are mutually exclusive')); // Parse `options.period`. From 959deabe9e83d7ca2ec47dcf9a690382f7e2fd77 Mon Sep 17 00:00:00 2001 From: Kevin Smathers Date: Sat, 6 Feb 2016 13:29:42 -0500 Subject: [PATCH 7/8] Push message to queue before rotation to avoid losing it. --- lib/bunyan.js | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/bunyan.js b/lib/bunyan.js index 1c98860f..30f48d4a 100644 --- a/lib/bunyan.js +++ b/lib/bunyan.js @@ -1381,6 +1381,7 @@ RotatingFileStream.prototype.write = function write(s) { return false; } else { if ((this.alreadyWritten + this.stream.bytesWritten) > this.rotateFileSize) { + this.rotQueue.push(s); this.rotate(); this.alreadyWritten = 0; return false; From 4d12e540554a44ae8ea72251c9add30c9b2949bc Mon Sep 17 00:00:00 2001 From: Dan Varga Date: Tue, 6 Sep 2016 10:13:17 -0400 Subject: [PATCH 8/8] Re-enable close function --- lib/bunyan.js | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/lib/bunyan.js b/lib/bunyan.js index 30f48d4a..6910c7dd 100644 --- a/lib/bunyan.js +++ b/lib/bunyan.js @@ -650,29 +650,26 @@ Logger.prototype.reopenFileStreams = function () { }; -/* BEGIN JSSTYLED */ /** * Close this logger. * * This closes streams (that it owns, as per 'endOnClose' attributes on * streams), etc. Typically you **don't** need to bother calling this. + */ Logger.prototype.close = function () { if (this._closed) { return; } if (!this._isSimpleChild) { - self.streams.forEach(function (s) { - if (s.endOnClose) { - xxx('closing stream s:', s); + this.streams.forEach(function (s) { + if (s.closeOnExit) { s.stream.end(); - s.endOnClose = false; + s.closeOnExit = false; } }); } this._closed = true; } - */ -/* END JSSTYLED */ /**