winstonjs / winston

A logger for just about everything.
http://github.com/winstonjs/winston
MIT License
22.74k stars 1.81k forks source link

Reopen log file on SIGUSR1 (log rotation) #943

Open Vanuan opened 7 years ago

Vanuan commented 7 years ago

Does Winston handle external log rotation? It looks like it continues writing to renamed file.

blongstreth commented 7 years ago

I am new to Winston myself and don't think this feature exists and would love to see it implemented ASAP. I found the following which could be used as a workaround for now:

https://gist.github.com/suprememoocow/5133080

SIGUSR1 is reserved by Node.js debugger. Might be better to use SIGUSR2 or SIGHUP. See: https://nodejs.org/dist/latest-v6.x/docs/api/process.html#process_signal_events

Vanuan commented 7 years ago

SIGUSR2 is reserved to restarting a cluster.

ajmas commented 7 years ago

For 'internal' log rotation there is also: https://www.npmjs.com/package/winston-daily-rotate-file

crussell52 commented 6 years ago

I think this would be a really valuable addition. There are external, specialized tools for rotating logs (e.g. logrotated) which are impractical to use in conjunction with winston.

"workarounds" which reach into the internal methods are fragile as the internal implementations could change at any moment.

I don't think winston should attach to a specific signal -- instead it should simply provide a public method for reopening its file descriptor. Then it will be up to the app to listen for the appropriate signal and call the public method on the transport. This is the approach that bunyan takes (see note near bottom of this section)

robsongajunior commented 5 years ago

Agree with @crussell52

instead it should simply provide a public method for reopening its file descriptor.

ajmas commented 5 years ago

Relying on external rotation would require a listener to be added, which may or not work depending on the platform, though the addition of a reopen() method or equivalent would likely be needed?

A small untested mockup, assuming existence of the function:

import {createLogger, format, transports} from 'winston';
import fs from 'fs';

const filename = 'main.log';
const mainLogFile = new transports.File({filename: filename, level: 'info'}),
const logger = createLogger({ transports: [mainLogFile] });

fs.watch(filename, (eventType, filename) => {
  if (eventType === 'rename') {
     mainLogFile.reopen();
  }
});

There could be other external scenarios that could trigger a need to reopen the file, with a different handle, so wondering if simply providing a reopen() method, would be workable approach?

nitzanav commented 5 years ago

any update?

thw0rted commented 5 years ago

It sounds like pino supports this already, if you're watching this issue and haven't switched yet.

matttowerssonos commented 4 years ago

This was previously possible with version 2.x but the revisions to the File transport no longer support it. See: https://gist.github.com/suprememoocow/5133080

The workaround we discovered is to use the copytruncate option with logrotate. That prevents the need to refresh file handles.