pinojs / pino

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

Proposal: add format support #764

Closed delvedor closed 4 years ago

delvedor commented 4 years ago

Hello everyone! At Elastic, we have a custom format for JSON logs named ECS, it helps us run some automation behind the scenes and improve the user experience. I'm working on a set of libraries that make the most commonly used logging frameworks to log directly into an ECS compliant format, but currently, it is not possible doing it for Pino, since the json structure is fixed, and the serializers are not powerful enough for solving the issue.

What do you think about introducing the format concept? It would be an object that allows you to hook into Pino and specify how you want the data to be serialized. Following an example, so it's more clear what I would like to implement.

const Pino = require('pino')({
  format: {
      // called everytime createChild is executed
      onChild (level, pid) {
        return `'{"level":"${level}","pid":"${pid}"}`
      },
      // what the user logs via log.info(‘messge’, { data })
      onLog (mergingObject, message, ...interpolationValues) {
        // return serialized data
      },
      onTime (time) {
        return `'{"@timestamp":"${new Date(time).toISOString()}"`
      }
  }
})

With this feature, we can also start logging format different than JSON:

const Pino = require('pino')({
  format: {
      onBase (level, pid) {
        return `'level: ${level} - pid ${pid}`
      },
      onLog (mergingObject, message, ...interpolationValues) {
        // return serialized data
      },
      onTime (time) {
        return `@timestamp - ${new Date(time).toISOString()}`
      }
  }
})

What do you think? If you would like to move forward with this feature, I can work on it.

Some additional points:

jsumners commented 4 years ago

I suspect this will incur undesirable performance impacts. But it also feels related to https://github.com/pinojs/pino/pull/614

However, we are not opposed to any improvements that do not negatively impact performance.

nuragic commented 4 years ago

Ciao Tomas @delvedor,

Is there any work in progress on this proposal? Just in case you have any experimental branch or something like that, and you would like to let people test it out... I'd be interested... 😄 Thanks!

delvedor commented 4 years ago

Hello @nuragic! Not yet, I'll keep you posted in this issue :)

nuragic commented 4 years ago

🙏 Many thanks!

zekth commented 4 years ago

I suspect this will incur undesirable performance impacts. But it also feels related to #614

The performance impact is not a problem IMO as if you're implementing it you're aware of the cost of the formating. Like mentionned for the Date format for ISO for example.

delvedor commented 4 years ago

Closed in https://github.com/pinojs/pino/pull/775.

github-actions[bot] commented 2 years 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.