Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

quietResLogger clarifications #350

Open
whitlaaa opened this issue Sep 28, 2024 · 3 comments
Open

quietResLogger clarifications #350

whitlaaa opened this issue Sep 28, 2024 · 3 comments

Comments

@whitlaaa
Copy link

Big fan of pino and was looking at leveraging pino-http to replace some outdated custom setup in our apps. I was looking at the recently added quietResLogger option and was hoping you could clarify its behavior (and perhaps its intended usage based on #235/#339?).

According to the README, setting quietResLogger to true will include the request id under reqId and "only contain req" (I assume that last bit is just a typo that is supposed to say "only contain res").

However, reqId is only included under res.log if quietReqLogger is also set. Is the intended usage to set both options as the unit tests seem to indicate?

Reading #235, I assumed the use case was either:

  • Set customReceivedMessage and quietResLogger
    • This gets the request info auto logged when it's received
    • Then they can rely solely on res.log to get quiet logging with reqId during their business logic
  • Set only quietResLogger
    • Then use req.log at the start of a handler followed by only using res.log for quiet logging afterward, e.g.:
function handle (req, res) {
  logger(req, res)
  req.log.info('') // just log the request info
  res.log.info('doing some work') // quiet logs with only reqId
  ...
  res.log.inf('finished doing some work')
  res.end('hello world')
} 

However, I don't think either of the above approaches works if quietReqLogger is set since it suppresses req from both customReceivedMessage and req.log. Obviously I could log that req info in other ways even in quiet mode, but pino's consistent format is preferred.

Thanks and hopefully this made sense. Up way later than I should have been while reading docs and writing this. 😆

@mcollina
Copy link
Member

You likely know more about that option that me know. I really regret merging that PR at this point :).

How would you recommend fixing this?

@randompixel
Copy link

I've also been very confused by these flags. Running Express, the default INFO (12345): [HTTP] request completed message doesn't behave how I'd expect?

From my understanding of the README.md, it sounds like if you set quietReqLogger it replaces the req object with reqId. If you set quietResLogger it removes the res object but leaves the req settings in place?

I'd expect the two flags to work independently of each other, such as:

both false quietReqLogger:true / quietResLogger:false quietReqLogger:false / quietResLogger:true both true
Include reqId
Trims Request
Trims Response
--- --- --- --- ---
Expected Full reqId + res Only req Only reqId

But from a very basic app with no other configuration or logging modifications, as far as I can see the current behaviour works like this?

both false quietReqLogger:true / quietResLogger:false quietReqLogger:false / quietResLogger:true both true
Actual Full reqId + res + req res reqId + res

So

  • tell both to be quiet and you still get res
  • tell onlyreq to be quiet and you still get req
  • tell only res to be quiet and you get res but no req
  • tell both of them to be quiet and you still get res

This doesn't make any sense!

@jsumners
Copy link
Member

You likely know more about that option that me know. I really regret merging that PR at this point :).

How would you recommend fixing this?

Let's rip it out and release a new major.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants