pinojs / pino

🌲 super fast, all natural json logger
http://getpino.io
MIT License
14.21k stars 875 forks source link

[question] How to create custom destination based on log level #1271

Closed robertsLando closed 1 year ago

robertsLando commented 2 years ago

Hi, I would like to log errors to stderr and everything else to stdout. I know I could archieve this in multiple ways (using transports or multi stream) BTW that would cause logs to be duplicated, I would like to customize the destination of pino pretty and redirect logs based on the level. Could anyone suggest me how? I think this could also be a useful option to add, something like errorsDestination that by default is set to destination

mcollina commented 2 years ago

This is covered by the dedupe flag in pino.multistream(): https://getpino.io/#/docs/api?id=pino-multistream.

Currently we are not passing this flag through pino.transport(), but it's definitely possible to implement it as it internally uses multistream(). Would you like to send a PR for this?

robertsLando commented 2 years ago

My application is a little bit more complicated and I don't want to dedupe logs in every situation, that's why I would prefer to be able to customize the destination based on the level. Could you give me some tips? I tried to dig into docs all the morning and I don't think it's so difficult but I'm blocked right now

My idea was to create a Writeable stram that is pipelined with split2 to parse the output but I think there could be some better solutions

mcollina commented 2 years ago

My application is a little bit more complicated and I don't want to dedupe logs in every situation, that's why I would prefer to be able to customize the destination based on the level.

That's what dedupe does if I understand your request correct. Can you make a more concrete example?

robertsLando commented 2 years ago

@mcollina IMO instead of creating multiple transports (and dedupe logs) being able to redirect the stream based on the log level would be better in terms of performances. Do you think so or am I wrong? Is this difficult to implement?

IMO this could be easily archived by making destination option to accept a function that returns a destination

robertsLando commented 2 years ago

By checking code it could be done here: https://github.com/pinojs/pino/blob/master/lib/multistream.js#L53

stream = typeof dest.stream === 'function' ? dest.stream(level) : dest.stream

If you are ok with this I could submit a PR

mcollina commented 2 years ago

I don't understand how that would solve your problem, but go ahead anyway - I'd like to see a PR.

robertsLando commented 2 years ago

PR ready @mcollina

robertsLando commented 2 years ago

Like I said in PR this is something that should be handled in pino pretty

mcollina commented 2 years ago

Like I said in PR this is something that should be handled in pino pretty

I don't understand, we closed the PR as this is something that can already be implemented with the current code. What's missing in pino-pretty?

robertsLando commented 2 years ago

Like I said here:

I actually have a pretty stream transport and a file stream transport (that is only working in production). I would like the pretty stream to use the stdout for all levels except errors, I cannot use dedupe as this would prevent logs to be logged on fileStream in production

If I remove dedupe I would have the logs duplicated on error and stdout, If i add dedupe, in production, I would miss the logs in file. I just would like to have an option in pretty stream to say log error levels to stderr and others to stdout, or create a custom stream to handle this

mcollina commented 2 years ago

I think a good solution to this problem is to add support for a maxLevel in https://github.com/pinojs/pino/blob/20afab9a9fbe211e5c43260b84f77ef39fff4e7c/lib/multistream.js#L52.

if (dest.level <= level && (dest.maxLevel || 0) > level) {

The implementation is likely a bit more complex than that, but I just wanted to give you an idea.

In this way you could write:

var fs = require('fs')
var pino = require('pino')
var pretty = require('pino-pretty')
var streams = [
  {stream: fs.createWriteStream('/tmp/info.stream.out')},
  {stream: pretty(), maxLevel: 'warn' }, // log to stdout
  {level: 'error', stream: pretty() }, // log to stderr
  {level: 'debug', stream: fs.createWriteStream('/tmp/debug.stream.out')},
  {level: 'fatal', stream: fs.createWriteStream('/tmp/fatal.stream.out')}
]

var log = pino({
  level: 'debug' // this MUST be set at the lowest level of the
                 // destinations
}, pino.multistream(streams))

log.debug('this will be written to /tmp/debug.stream.out')
log.info('this will be written to /tmp/debug.stream.out and /tmp/info.stream.out')
log.fatal('this will be written to /tmp/debug.stream.out, /tmp/info.stream.out and /tmp/fatal.stream.out')

Wdyt? Would you like to send a PR for this?

github-actions[bot] commented 1 year ago

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.