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

signal-handling to allow proper log-rotation #12

Open
amirmasud opened this issue Jul 22, 2018 · 11 comments
Open

signal-handling to allow proper log-rotation #12

amirmasud opened this issue Jul 22, 2018 · 11 comments

Comments

@amirmasud
Copy link

It seems pino-tee (pino in general) doesn't support singal-handling to reopen file after log-rotation. (Does it?)
It surely supports log-rotation if we have copytruncate option in configuration which is not such good for production (performance and data loss issues as mentioned in logrotate man page).

One good solution would be reopening log file on SIGUSR2 or SIGHUP (also adding a postrotate script on logrotate config to signal the process).

Thanks in advance

@davidmarkclements
Copy link
Member

@mcollina is this something for sonic boom?

@mcollina
Copy link
Member

@davidmarkclements this is definitely something we would need to add to pino@5 before it's released.

@davidmarkclements
Copy link
Member

we should be able to add this after as a minor, shouldn't cause any breakage? I'd rather not hold back the release at this point...

@davidmarkclements
Copy link
Member

also would we need to add the path re-opening capability to sonic boom first?

also, do we know how this would be done in bash with redirection? cc @jsumners

@davidmarkclements
Copy link
Member

davidmarkclements commented Jul 24, 2018

To me, it seems this could be fully implemented in sonic-boom,
we could then document in pino v5 and release a minor

const dest = pino.destination('/log/file')
const logger = require('pino')(dest)
process.on('SIGUSR2', () => dest.reopen())

@mcollina
Copy link
Member

Here it is, please check it out: pinojs/sonic-boom#10.

@davidmarkclements
Copy link
Member

ok that's awesome. I'll add the above to the docs.

next question is, how do we get pino-tee to benefit, it might need to start using sonic-boom and then support the SIGUSR2 or SIGHUP handler...

@mcollina
Copy link
Member

I think we can have pino-tee depend on sonic-boom. I won't have time for a PR for a bit of time.

@matthiasg
Copy link

did somebody do this already ?

@vemilyus
Copy link

I implemented this functionality in PR #16

@zcchew1202
Copy link

Hi, any idea what the ETA is for PR #16 to be merged & which release it'll be in?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants