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.

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

Add rotation of log files by size. Configuration requires user to pr… #288

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
87 changes: 67 additions & 20 deletions lib/bunyan.js
Original file line number Diff line number Diff line change
Expand Up @@ -1092,16 +1092,18 @@ 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));
assert.ok(this.count >= 0,
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) {
// <number><scope> where scope is:
Expand All @@ -1125,6 +1127,12 @@ function RotatingFileStream(options) {
}
this.periodNum = Number(m[1]);
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));
} else {
this.periodNum = 1;
this.periodScope = 'd';
Expand All @@ -1149,25 +1157,52 @@ 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();
} else {
self.alreadyWritten = stats.size;
}
}
}

if (createStream) {
this.stream = fs.createWriteStream(this.path,
{flags: 'a', encoding: 'utf8'});
}

}

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
// <https://github.com/joyent/node/issues/8656>.
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
// <https://github.com/joyent/node/issues/8656>.
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();
}
}
}

Expand Down Expand Up @@ -1258,8 +1293,10 @@ RotatingFileStream.prototype.rotate = function rotate() {

// If rotation period is > ~25 days, we have to break into multiple
// setTimeout's. See <https://github.com/joyent/node/issues/8656>.
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) {
Expand All @@ -1271,7 +1308,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);
Expand Down Expand Up @@ -1339,7 +1380,13 @@ RotatingFileStream.prototype.write = function write(s) {
this.rotQueue.push(s);
return false;
} else {
return this.stream.write(s);
if ((this.alreadyWritten + this.stream.bytesWritten) > this.rotateFileSize) {
Copy link

Choose a reason for hiding this comment

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

By checking the file and rotating immediately, we lose the log message that caused the rotation.
Can be resolved by adding a "this.rotQueue.push(s);" before the call to rotate.

Although the function returns false, this isn't an indicator that the caller should retry - simply an indicator that the data has been buffered.

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for catching this! Fixed!

On Sat, Feb 6, 2016 at 10:55 AM, Jim Tupper [email protected]
wrote:

In lib/bunyan.js
#288 (comment):

@@ -1339,7 +1380,13 @@ RotatingFileStream.prototype.write = function write(s) {
this.rotQueue.push(s);
return false;
} else {

  •    return this.stream.write(s);
    
  •    if ((this.alreadyWritten + this.stream.bytesWritten) > this.rotateFileSize) {
    

By checking the file and rotating immediately, we lose the log message
that caused the rotation


Reply to this email directly or view it on GitHub
https://github.com/trentm/node-bunyan/pull/288/files#r52103575.

Kevin Smathers
Product Development Engineer
Tanium, Inc.
[email protected]

this.rotate();
this.alreadyWritten = 0;
return false;
} else {
return this.stream.write(s);
}
}
};

Expand Down