pinojs / pino

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

Add options to turn off `level` and `v` properties #430

Closed paranoid-android closed 6 years ago

paranoid-android commented 6 years ago

Hello,

Pino lets a developer remove all of the pre-defined log properties except for level and v. For example, a developer can configure a logger like this:

let logger = pino({
  base: null,
  timestamp: false
}, fs.createWriteStream('./log.json'));

logger.info({ message:'hello world'});

This will generate a log that looks like this:

{"level":30,"message":"hello world","v":1}

It would be great to be able to remove all pre-defined properties so only the message or text that a developer wants to log would appear.

mcollina commented 6 years ago

@paranoid-android I'm not sure that adds value. levels play a huge role in pino, and I'm not sure I see the use case. Can you please elaborate?

paranoid-android commented 6 years ago

The reason to remove the 'level' property is just to give a developer full control over the log that's output.

marcbachmann commented 6 years ago

One use case where level might not fit is if you want to track events in an application, but there I would rather use a custom loglevel instead of removing the property. It might also be annoying to see whether something is a line from pino or from somewhere else if we remove all the properties.

To the schema version: I'd support the removal of v as I don't see any benefit in it of it. Isn't the declaration of the npm module version already good enough and a bit redundant to v?

If the root-level properties would be fixed, then it would make sense, but as everything completely depends on the usage of pino, any change could break the log collection on a server.

E.g. A simple value change like

{"customValue": {"test": "foo"}}

to

{"customValue": "foo"}

can break the log pipeline if elasticsearch is in use. Since changing custom values can have a much bigger impact, I see no reason to use v. The custom version attribute or schema-id should be declared on a higher level construct (like with annotations on a service in kubernetes or using the base option).

davidmarkclements commented 6 years ago

v pertains to the version of the log format. This will become important if certain log conventions change. Neither the npm version nor service versioning will cover that

jsumners commented 6 years ago

And it's basically the only property we can use to determine a Pino log line in downstream transports.

davidmarkclements commented 6 years ago

@paranoid-android without the level the semantics of the logger (e.g. logger.info) are meaningless. If you want high speed writes, but don't want the trappings of Pino you can use the underlying lib that pino uses: https://www.npmjs.com/package/sonic-boom

As for removing v we could recommend against it, however if anyone wants to create a PR that configures Pino to log without v (and possibly to rename it as well) we would consider it.

closing for now as we've come to a conclusion on this

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.