Skip to content

Commit f7e7f2f

Browse files
Pulkit0729myAlapiDABH
authoredJul 10, 2023
Added Lazy option to file transport (#2317)
* Test for file Transport option rotationFormat * Test for file Transport option rotationFormat * Added Lazy option in file transport * Lint and test * removed only statement * Update test/unit/winston/transports/01-file-maxsize.test.js Co-authored-by: David Hyde <DABH@users.noreply.github.com> * Update test/unit/winston/transports/01-file-maxsize.test.js Co-authored-by: David Hyde <DABH@users.noreply.github.com> --------- Co-authored-by: myAlapi <myalapi2022@gmail.com> Co-authored-by: David Hyde <DABH@users.noreply.github.com>
1 parent de2e887 commit f7e7f2f

File tree

6 files changed

+370
-8
lines changed

6 files changed

+370
-8
lines changed
 

Diff for: ‎docs/transports.md

+1
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,7 @@ looking for daily log rotation see [DailyRotateFile](#dailyrotatefile-transport)
102102
* __level:__ Level of messages that this transport should log (default: level set on parent logger).
103103
* __silent:__ Boolean flag indicating whether to suppress output (default false).
104104
* __eol:__ Line-ending character to use. (default: `os.EOL`).
105+
* __lazy:__ If true, log files will be created on demand, not at the initialization time.
105106
* __filename:__ The filename of the logfile to write output to.
106107
* __maxsize:__ Max size in bytes of the logfile, if the size is exceeded then a new file is created, a counter will become a suffix of the log file.
107108
* __maxFiles:__ Limit the number of files created when the size of the logfile is exceeded.

Diff for: ‎lib/winston/transports/file.js

+37-6
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,7 @@ module.exports = class File extends TransportStream {
7979
this.maxFiles = options.maxFiles || null;
8080
this.eol = (typeof options.eol === 'string') ? options.eol : os.EOL;
8181
this.tailable = options.tailable || false;
82+
this.lazy = options.lazy || false;
8283

8384
// Internal state variables representing the number of files this instance
8485
// has created and the current size (in bytes) of the current logfile.
@@ -88,9 +89,10 @@ module.exports = class File extends TransportStream {
8889
this._drain = false;
8990
this._opening = false;
9091
this._ending = false;
92+
this._fileExist = false;
9193

9294
if (this.dirname) this._createLogDirIfNotExist(this.dirname);
93-
this.open();
95+
if (!this.lazy) this.open();
9496
}
9597

9698
finishIfEnding() {
@@ -107,14 +109,13 @@ module.exports = class File extends TransportStream {
107109
}
108110
}
109111

110-
111112
/**
112113
* Core logging method exposed to Winston. Metadata is optional.
113114
* @param {Object} info - TODO: add param description.
114115
* @param {Function} callback - TODO: add param description.
115116
* @returns {undefined}
116117
*/
117-
log(info, callback = () => {}) {
118+
log(info, callback = () => { }) {
118119
// Remark: (jcrugzz) What is necessary about this callback(null, true) now
119120
// when thinking about 3.x? Should silent be handled in the base
120121
// TransportStream _write method?
@@ -123,6 +124,7 @@ module.exports = class File extends TransportStream {
123124
return true;
124125
}
125126

127+
126128
// Output stream buffer is full and has asked us to wait for the drain event
127129
if (this._drain) {
128130
this._stream.once('drain', () => {
@@ -138,6 +140,32 @@ module.exports = class File extends TransportStream {
138140
});
139141
return;
140142
}
143+
if (this.lazy) {
144+
if (!this._fileExist) {
145+
if (!this._opening) {
146+
this.open();
147+
}
148+
this.once('open', () => {
149+
this._fileExist = true;
150+
this.log(info, callback);
151+
return;
152+
});
153+
return;
154+
}
155+
if (this._needsNewFile(this._pendingSize)) {
156+
this._dest.once('close', () => {
157+
if (!this._opening) {
158+
this.open();
159+
}
160+
this.once('open', () => {
161+
this.log(info, callback);
162+
return;
163+
});
164+
return;
165+
});
166+
return;
167+
}
168+
}
141169

142170
// Grab the raw string and append the expected EOL.
143171
const output = `${info[MESSAGE]}${this.eol}`;
@@ -169,6 +197,10 @@ module.exports = class File extends TransportStream {
169197
if (!this._needsNewFile()) {
170198
return;
171199
}
200+
if (this.lazy) {
201+
this._endStream(() => {this.emit('fileclosed')});
202+
return;
203+
}
172204

173205
// End the current stream, ensure it flushes and create a new one.
174206
// This could potentially be optimized to not run a stat call but its
@@ -508,7 +540,6 @@ module.exports = class File extends TransportStream {
508540
_cleanupStream(stream) {
509541
stream.removeListener('error', this._onError);
510542
stream.destroy();
511-
512543
return stream;
513544
}
514545

@@ -526,7 +557,7 @@ module.exports = class File extends TransportStream {
526557
* @param {function} callback - Callback for when the current file has closed.
527558
* @private
528559
*/
529-
_endStream(callback = () => {}) {
560+
_endStream(callback = () => { }) {
530561
if (this._dest) {
531562
this._stream.unpipe(this._dest);
532563
this._dest.end(() => {
@@ -542,7 +573,7 @@ module.exports = class File extends TransportStream {
542573
* Returns the WritableStream for the active file on this instance. If we
543574
* should gzip the file then a zlib stream is returned.
544575
*
545-
* @param {ReadableStream} source – PassThrough to pipe to the file when open.
576+
* @param {ReadableStream} source –PassThrough to pipe to the file when open.
546577
* @returns {WritableStream} Stream that writes to disk for the active file.
547578
*/
548579
_createStream(source) {

Diff for: ‎test/unit/winston/transports/00-file-stress.test.js

+43
Original file line numberDiff line numberDiff line change
@@ -150,4 +150,47 @@ describe('File (stress)', function () {
150150
});
151151
}, 10000);
152152
});
153+
154+
it('should handle a high volume of writes with lazy option enabled', function (done) {
155+
const logger = winston.createLogger({
156+
transports: [
157+
new winston.transports.File({
158+
filename: fileStressLogFile,
159+
lazy: true
160+
})
161+
]
162+
});
163+
164+
const counters = {
165+
write: 0,
166+
read: 0
167+
};
168+
169+
const interval = setInterval(function () {
170+
logger.info(++counters.write);
171+
}, 0);
172+
173+
setTimeout(function () {
174+
clearInterval(interval);
175+
176+
helpers
177+
.tryRead(fileStressLogFile)
178+
.on('error', function (err) {
179+
assume(err).false();
180+
logger.close();
181+
done();
182+
})
183+
.pipe(split())
184+
.on('data', function (d) {
185+
const json = JSON.parse(d);
186+
assume(json.level).equal('info');
187+
assume(json.message).equal(++counters.read);
188+
})
189+
.on('end', function () {
190+
assume(counters.write).equal(counters.read);
191+
logger.close();
192+
done();
193+
});
194+
}, 10000);
195+
});
153196
});

Diff for: ‎test/unit/winston/transports/01-file-maxsize.test.js

+92-2
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,8 @@ describe('File (maxsize)', function () {
2525
this.timeout(10000);
2626

2727
let testDone = false;
28-
before(removeFixtures);
29-
after(done => {
28+
this.beforeEach(removeFixtures);
29+
this.afterEach(done => {
3030
testDone = true;
3131
removeFixtures(done);
3232
});
@@ -119,4 +119,94 @@ describe('File (maxsize)', function () {
119119
setImmediate(() => logKbytes(4));
120120
});
121121
});
122+
123+
describe('With lazy option enabled', () => {
124+
it('should not create extra file', function (done) {
125+
const fillWith = ['a', 'b', 'c', 'd', 'e'];
126+
const lazyTransport = new winston.transports.File({
127+
format: winston.format.printf(info => info.message),
128+
filename: path.join(testLogFixturesPath, 'testmaxsize.log'),
129+
maxsize: 3072,
130+
lazy: true
131+
});
132+
const logger = winston.createLogger({
133+
transports: [lazyTransport]
134+
});
135+
//
136+
// Setup a list of files which we will later stat.
137+
//
138+
const files = [];
139+
140+
//
141+
// Assets the no of files and all the files have been created with the
142+
// correct filesize
143+
//
144+
function assumeFilesCreated() {
145+
assume(files.length).equals(fillWith.length);
146+
files.map(function (file, i) {
147+
let stats;
148+
try {
149+
stats = fs.statSync(file);
150+
} catch (ex) {
151+
assume(stats).is.an(
152+
'object',
153+
`${file} failed to open: ${ex.message}`
154+
);
155+
}
156+
157+
const text = fs.readFileSync(file, 'utf8');
158+
assume(text[0]).equals(fillWith[i]);
159+
// Either 4096 on Unix or 4100 on Windows
160+
// because of the eol.
161+
if (process.platform === 'win32') {
162+
assume(stats.size).equals(3075);
163+
} else {
164+
assume(stats.size).equals(3072);
165+
}
166+
});
167+
done();
168+
}
169+
170+
//
171+
// Log the specified kbytes to the transport
172+
//
173+
function logKbytes(kbytes) {
174+
//
175+
// Shift the next fill char off the array then push it back
176+
// to rotate the chars.
177+
//
178+
const filler = fillWith.shift();
179+
fillWith.push(filler);
180+
181+
//
182+
//
183+
// To not make each file not fail the assertion of the filesize we can
184+
// make the array 1023 characters long.
185+
//
186+
const kbStr = Array(1023).fill(filler).join('');
187+
for (var i = 0; i < kbytes; i++) {
188+
logger.log({ level: 'info', message: kbStr });
189+
}
190+
}
191+
192+
// Initial Log
193+
let count =1;
194+
logKbytes(3);
195+
196+
// Listen to file close event called when the file is closed
197+
lazyTransport.on('fileclosed', ()=>{
198+
if (count === fillWith.length) {
199+
assumeFilesCreated();
200+
return;
201+
}
202+
count += 1;
203+
setImmediate(()=>{logKbytes(3);});
204+
})
205+
206+
//Listent to file open event called when the file is opened
207+
lazyTransport.on('open', file => {
208+
files.push(file);
209+
});
210+
});
211+
});
122212
});

Diff for: ‎test/unit/winston/transports/file-lazy.test.js

+61
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
'use strict';
2+
3+
const path = require('path');
4+
const winston = require('../../../../lib/winston');
5+
const helpers = require('../../../helpers');
6+
const fs = require('fs');
7+
const { MESSAGE } = require('triple-beam');
8+
const split = require('split2');
9+
const assume = require('assume');
10+
11+
function noop() {}
12+
13+
describe('Lazy Option Test', function () {
14+
this.timeout(10 * 1000);
15+
var logPath = path.join(
16+
__dirname,
17+
'..',
18+
'..',
19+
'..',
20+
'fixtures',
21+
'file',
22+
'lazy.log'
23+
);
24+
25+
beforeEach(function () {
26+
try {
27+
fs.unlinkSync(logPath);
28+
} catch (ex) {
29+
if (ex && ex.code !== 'ENOENT') {
30+
return done(ex);
31+
}
32+
}
33+
});
34+
35+
it('should not create a log file before receiving any logs', function (done) {
36+
var transport = new winston.transports.File({
37+
filename: logPath,
38+
lazy: true
39+
});
40+
41+
setTimeout(function () {
42+
assume(fs.existsSync(logPath)).false();
43+
done();
44+
}, 0);
45+
});
46+
it('should create a log file after receiving log', function (done) {
47+
var transport = new winston.transports.File({
48+
filename: logPath,
49+
lazy: true
50+
});
51+
52+
var info = { [MESSAGE]: 'this is my log message' };
53+
54+
transport.log(info, noop);
55+
56+
setTimeout(function () {
57+
assume(fs.existsSync(logPath));
58+
done();
59+
}, 0);
60+
});
61+
});
+136
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,136 @@
1+
'use strict';
2+
3+
const path = require('path');
4+
const winston = require('../../../../lib/winston');
5+
const helpers = require('../../../helpers');
6+
const fs = require('fs');
7+
const { MESSAGE } = require('triple-beam');
8+
const split = require('split2');
9+
const assume = require('assume');
10+
const rimraf = require('rimraf');
11+
const testFileFixturesPath = path.join(
12+
__dirname,
13+
'..',
14+
'..',
15+
'..',
16+
'fixtures',
17+
'file'
18+
);
19+
20+
//
21+
// Remove all log fixtures
22+
//
23+
function removeFixtures(done) {
24+
rimraf(path.join(testFileFixturesPath, 'rotation*'), done);
25+
}
26+
27+
// Validate Filename according to rotation
28+
function isCorrectFormat(filename) {
29+
let time = filename.split('rotation')[1].split('.')[0];
30+
return new Date(time).getTime() > 0;
31+
}
32+
33+
describe('winston/transports/file/rotationFormat', function () {
34+
this.timeout(10000);
35+
36+
let testDone = false;
37+
before(removeFixtures);
38+
after(done => {
39+
testDone = true;
40+
removeFixtures(done);
41+
});
42+
43+
it('should create multiple files correctly with rotation Function', function (done) {
44+
const fillWith = ['a', 'b', 'c', 'd', 'e'];
45+
const rotationTransport = new winston.transports.File({
46+
level: 'info',
47+
format: winston.format.printf(info => info.message),
48+
filename: path.join(testFileFixturesPath, 'rotation.log'),
49+
maxsize: 4096,
50+
rotationFormat: () => {
51+
return new Date().getTime();
52+
}
53+
});
54+
55+
//
56+
// Have to wait for `fs.stats` to be done in `rotationTransport.open()`.
57+
// Otherwise the rotationTransport._dest is undefined. See https://github.com/winstonjs/winston/issues/1174
58+
//
59+
60+
//
61+
// Setup a list of files which we will later stat.
62+
//
63+
const files = [];
64+
65+
//
66+
// Assets all the files have been created with the
67+
// correct filesize
68+
//
69+
function assumeFilesCreated() {
70+
files.map(function (file, i) {
71+
let stats;
72+
try {
73+
stats = fs.statSync(file);
74+
} catch (ex) {
75+
assume(stats).is.an(
76+
'object',
77+
`${file} failed to open: ${ex.message}`
78+
);
79+
}
80+
81+
const text = fs.readFileSync(file, 'utf8');
82+
assume(text[0]).equals(fillWith[i]);
83+
assume(isCorrectFormat(file));
84+
// Either 4096 on Unix or 4100 on Windows
85+
// because of the eol.
86+
if (process.platform === 'win32') {
87+
assume(stats.size).equals(4100);
88+
} else {
89+
assume(stats.size).equals(4096);
90+
}
91+
});
92+
93+
done();
94+
}
95+
96+
//
97+
// Log the specified kbytes to the transport
98+
//
99+
function logKbytes(kbytes) {
100+
//
101+
// Shift the next fill char off the array then push it back
102+
// to rotate the chars.
103+
//
104+
const filler = fillWith.shift();
105+
fillWith.push(filler);
106+
107+
//
108+
//
109+
// To not make each file not fail the assertion of the filesize we can
110+
// make the array 1023 characters long.
111+
//
112+
const kbStr = Array(1023).fill(filler).join('');
113+
114+
//
115+
// With printf format that displays the message only
116+
// winston adds exactly 0 characters.
117+
//
118+
for (var i = 0; i < kbytes; i++) {
119+
rotationTransport.log({ level: 'info', [MESSAGE]: kbStr });
120+
}
121+
}
122+
123+
rotationTransport.on('open', function (file) {
124+
125+
if (testDone) return; // ignore future notifications
126+
const match = file.match(/(\d+)\.log$/);
127+
const count = match ? match[1] : 0;
128+
if (files.length === 5) {
129+
return assumeFilesCreated();
130+
}
131+
132+
files.push(file);
133+
setImmediate(() => logKbytes(4));
134+
});
135+
});
136+
});

0 commit comments

Comments
 (0)
Please sign in to comment.