pinojs / pino

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

Improve documentation on global logging level with multiple targets #1895

Open mjarosie opened 9 months ago

mjarosie commented 9 months ago

The "global" level setting isn't taken into account when transport.targets parameter is provided - each target has its own level that defaults to info (as mentioned here). That might be a bit confusing and should probably be mentioned in the documentation.

For example originally having this config:

const logger = pino({
  level: 'debug',
  transport: {
    target: 'pino-pretty',
  },
})

logger.trace('trace') # Doesn't appear
logger.debug('debug')
logger.info('info')
logger.warn('warn')
logger.error('error')
logger.fatal('fatal')

If I switch to the following, I'd lose debug logs:

const logger = pino({
  level: 'debug',
  transport: {
    targets: [{ target: 'pino-pretty' }],
  },
})

logger.trace('trace') # Doesn't appear
logger.debug('debug') # Doesn't appear
logger.info('info')
logger.warn('warn')
logger.error('error')
logger.fatal('fatal')

I appreciate that this might be an expected behaviour, but I'd expect this to be documented explicitly - I couldn't find the mention of it, unless I didn't look in the right place or I somehow missed it? It took me some time to figure out what's happening - it only started making sense when I've found the comment in the GitHub issue linked above.

jsumners commented 9 months ago

https://github.com/pinojs/pino/blob/9259e13309a77f246ad1a766c27cda794e2915d0/docs/api.md?plain=1#L1267

mjarosie commented 9 months ago

https://github.com/pinojs/pino/blob/9259e13309a77f246ad1a766c27cda794e2915d0/docs/api.md?plain=1#L1267

I don't see how the linked option documentation would clear up the confusion. I think that it should be clearly stated here or here to make it clear that if you provide a list of targets to pino(), the top-level level option is ignored.

jsumners commented 9 months ago

Would you like to send a Pull Request to address this issue?

mjarosie commented 9 months ago

Sure, will create one soon.