pinojs / pino

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

Cannot use NewRelic Pino formatter in combination with Transport #1621

Closed rbrooksAvetta closed 1 year ago

rbrooksAvetta commented 1 year ago

I am trying to use a custom transporter to override the log.level attribution to use labels instead of the number form, along with new relic local log decoration.

let logger = pino({
  transport: {
    pipeline: [{
      target: './pino-log-level.transport-v2.js'
    }, {
      target: 'pino/file'
    }],
  }
}, nrPino())

This sets the level but does not invoke the new relic log decoration

jsumners commented 1 year ago

Why are you asking us about a New Relic feature?

rbrooksAvetta commented 1 year ago

I am asking because newRelic Pino log decoration works with Pino 7+ but the current constructs isn't working with setting options or a transport. I believe this a problem with the Pino construct.

The following works: Construct with custom transport to override level

let logger = pino({
  transport: {
    pipeline: [{
      target: './pino-log-level.transport-v2.js'
    }, {
      target: 'pino/file'
    }],
  }
})

Construct Pino logger using NewRelic formatter log decorator

let logger = pino(nrPino())

There should be construct for both to work. That's why I am asking

jsumners commented 1 year ago

To be clear: the transport configuration works without supplying New Relic's module and fails when that module is added?

rbrooksAvetta commented 1 year ago

No the transport works either way. The issue I am seeing is with the two argument declare function, constructor, for Pino not initializing the nrPino formatter like it does when I use the single argument constructor.

declare function pino<Options extends LoggerOptions>(options: Options, stream: DestinationStream)
mcollina commented 1 year ago

What is nrPino()?

jsumners commented 1 year ago

Please provide a replication that does not involve third party code. Otherwise, your issue belongs on the New Relic repo.

rbrooksAvetta commented 1 year ago

NewRelic Pino is a formatter to decorate the logs, base on the Pino constructor declaration it is a type of DestinationStream because it accepts and works when it is used with the single argument constructor, but it does not seem to be registered or working with Pino when used as the second argument in the Pino two argument constructor. Hence, the logical conclusion is the DestinationStream for the third-party NRPino is working fine but the Pino construct is not doing what one would expect.

jsumners commented 1 year ago

Hence, the logical conclusion is the DestinationStream for the third-party NRPino is working fine but the Pino construct is not doing what one would expect.

Incorrect. We have a comprehensive test suite that proves our features work the way we advertise they work. You are supplying third party code and claiming ours is broken. The actual logical conclusion is that the third party code is incompatible.

I am closing this as non-reproducible until someone can provide a failing reproduction.

rbrooksAvetta commented 1 year ago

So why would the third-party code work and be compatible with the one arg constructor, a compatible DestinationStream, but not with the two argument constructor? Also, I would expect the Pino code to throw some exception stating incompatibility oppose to just running without errors but not providing consistent working behavior.

github-actions[bot] commented 1 year 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.