Re-add support for standard console-style parameter logging

[taoeffect]

Problem

Pino looks like a great project, but I would imagine that most projects that want to use it would want to use it to capture all of the logs that their apps produce, right? Not just some, but all of the logs.

However, according to the docs, since v7 this is no longer the case because Pino no longer conforms to the console interface.

What this means is that if an app wants to use Pino, it will be impossible for it to guarantee that all logs will be captured if that app uses any libraries. The reason is, some libraries will use console.log and console.debug to log info. In order to capture these logs, one needs to do something like:

console.debug = logger.debug.bind(logger)
console.info = logger.info.bind(logger)
console.log = logger.info.bind(logger)
console.warn = logger.warn.bind(logger)
console.error = logger.error.bind(logger)

Where logger is an instance of pino.

However, this still won't capture all logs for logging done like so:

console.log('message:', number)

Vital information will be missing from the logs.

The docs suggest using a hook to modify the way pino behaves. And indeed this is what we're considering doing for our app with the following hook:

// support regular console.log('asdf', 'adsf', 'adsf') style logging that might be used by libraries
// https://github.com/pinojs/pino/blob/master/docs/api.md#interpolationvalues-any
function logMethod (args, method) {
  const stringIdx = typeof args[0] === 'string' ? 0 : 1
  if (args.length > 1) {
    for (let i = stringIdx + 1; i < args.length; ++i) {
      args[stringIdx] += typeof args[i] === 'string' ? ' %s' : ' %o'
    }
  }
  method.apply(this, args)
}

However, one has to remember to add this bit of ugly code to their app. And anyone who wants to capture all of the logs of their app must do this.

Solution

Please consider reverting to the v6 behavior of Pino, so that all logs are captured by default, including those of imported libraries that do not use Pino.

[jsumners]

Pino has never patched, and will never patch, console.log to intercept any messages written with it. We do not patch globals. Any library that intends to provide logging should accept a logger instance that conforms to a common interface, e.g. abstract-logging. If a library you are using does not do this, you should file an issue with them. It is very bad practice to ship a library that has console.log in its execution path.

[mcollina]

I think a separate module can be created to do exactly this: patching console.log with a pino instance.

[taoeffect]

I think you guys misunderstood me. I wasn't asking or suggesting that Pino patch any globals.

The issue isn't about patching globals.

The issue is that Pino's interface doesn't conform to the console interface.

EDIT: To be even clearer - patching globals wouldn't address the issue. Please consider re-opening. 🙏

[jsumners]

What this means is that if an app wants to use Pino, it will be impossible for it to guarantee that all logs will be captured if that app uses any libraries. The reason is, some libraries will use console.log and console.debug to log info. In order to capture these logs, one needs to do something like:

I don't know how to read this any other way than the console global needs to be patched. As @mcollina suggested, if this is desired, it should be done in a separate module. Said module would utilize the hook code you provided.

[taoeffect]

I don't know how to read this any other way than the console global needs to be patched.

Patching the global is not something that you guys need to do. The issue is that Pino doesn't conform to the console interface. It used to conform to it, but now it doesn't. If I were to patch the global console, it wouldn't fix the problem, as mentioned in the issue. I would need to additionally make Pino conform the to the console interface.

Patching the global console variable is something developers who use Pino can choose to do themselves. But you guys are more than capable of conforming to the console interface, so that if someone decides to capture all log messages it will work properly. Again, you used to conform to the interface (according to the docs), in v6.

Please consider bringing this back, so that then when developers decide they'd like to capture all logs, they can actually do so without additionally having to modify Pino's behavior.

[github-actions]

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.