search

pinojs / pino-pretty

🌲Basic prettifier for Pino log lines
MIT License
1.27k stars 150 forks source link

Support nested levelKey #407

Closed nikrabaev closed 1 year ago

nikrabaev commented 1 year ago

This PR allows specifying the nested levelKey. Such keys will be treated just like the ignore nested keys. There are two reasons for this change:

  1. Why would the ignore option support nested keys and levelKey don't?
  2. I personally need it because in my project I use the custom format of logs, where the level is placed inside another tags object
nikrabaev commented 1 year ago

Even though this got approved, I've just realized that it may break things for those who assumed the dot notation handling does not apply to the levelKey and didn't escape the dots in their code. I see 3 ways to avoid such issues:

  1. Try to find the property using the non-split levelKey first. If the value is not unknown, return it. Otherwise, assume it's a nested key.
  2. Create a new option like nestedLevelKey while preserving the original behavior of levelKey. In situations when both are used, give higher priority to the nestedLevelKey.
  3. Mark it as a breaking change.
mcollina commented 1 year ago

I think we might just go for a major.

@jsumners wdyt?

nikrabaev commented 1 year ago

@jsumners Applied

krazar commented 1 year ago

I'll comment here, but tell me If it is best I create a new issue for this.

This change may introduce an unexpected situation when using a nested level key. Though the nested key is identified as a level, it is not removed from the object as it is the case with "non nested levelKey" and it could be the intended behavior (at least it is mine).

Using the exclude options could have been a good alternative, but this is applied before loading the level property, so it does not work.

https://github.com/pinojs/pino-pretty/blob/master/index.js#L189