Pass through non-pino lines #95
Conversation
| @@ -46,6 +46,13 @@ function filter (value) { | |||
| return result | |||
| } | |||
|
|
|||
| function isPinoLine (line) { | |||
| var requiredProperties = [ 'time', 'name', 'hostname', 'pid' ] | |||
There was a problem hiding this comment.
time is not required anymore #91 :).
Maybe we should adjust this further.
There was a problem hiding this comment.
Duh. I'm the one that removed that.
There was a problem hiding this comment.
There has to be a way to determine if the incoming line is a pino line. I have updated it to depend only on hostname and pid properties. They are used to build the (pid on hostname): prefix, which is not conditional as the code currently stands.
| return Object.keys(line).filter(function (k) { | ||
| return requiredProperties.indexOf(k) > -1 | ||
| }).length > 0 | ||
| } |
There was a problem hiding this comment.
I think we can remove the Object.keys and loop, and just use a couple of conditions with &&. we should also check that v is 0.
There was a problem hiding this comment.
I have thought about that myself. Before I do it: what do you think of a new common property named pino, like the v property? It could be set to the current version. Then the check only needs to be for the presence of the pino property.
|
I would leave the discussion about "pino" property to another PR/issue. However, I think we should just have a chops thing that also works with bole, bunyan etc. |
| @@ -46,6 +46,10 @@ function filter (value) { | |||
| return result | |||
| } | |||
|
|
|||
| function isPinoLine (line) { | |||
| return line.hasOwnProperty('hostname') && line.hasOwnProperty('pid') | |||
| } | |||
There was a problem hiding this comment.
we should check on v to be 0 as well.
There was a problem hiding this comment.
If v is even present it's currently at 1.
What I am basing the conditions on is https://github.com/mcollina/pino/blob/master/pretty.js#L100 . Without those properties pretty.js will not construct a valid reformatted log line. The v property is not considered at all for reformatting.
There was a problem hiding this comment.
I know but it should :). My bad when writing this for the first time :/.
|
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Fixes #94