iamolegga / nestjs-pino

Platform agnostic logger for NestJS based on Pino with REQUEST CONTEXT IN EVERY LOG
MIT License
1.26k stars 98 forks source link

[FEATURE REQUEST] expose request.log.setBindings() (or request.log in general) #1280

Open naseemkullah opened 1 year ago

naseemkullah commented 1 year ago

Is your feature request related to a problem? Please describe. I'd like to add context to the "request completed|errored" log, with vanilla pino-http I can do this via req.log.setBindings(). This is similar to this lib's assign() method which creates a child logger under the hood, except the difference is that we can use it to add context to the onResponse log, allowing us to have just 1 log per request with all required context.

Describe the solution you'd like I'd this the PinoLogger to expose req.log or req.log.setBindings() method, where req is the current request, if any current request is present.

maybe something like this:

// sets bindings on current request if this code is part of handling a request
this.logger.setReqLogBindings({foo: "bar})

Describe alternatives you've considered Threading through the request object deep into different parts of the code base and doing req.log.setBindings() there.

Additional context

setBindings() ref:

iamolegga commented 1 year ago

I couldn't find such feature of pino-http. Could you please add the link?

naseemkullah commented 1 year ago

Hi @iamolegga - I too soon noticed it is not documented, so I added the links in an edit of my original post, here they are again:

https://github.com/pinojs/pino/blob/553c66ba3e33410c203a50e5460365bdc9ec61a9/lib/proto.js#L157-L161) https://github.com/pinojs/pino/blob/013dc0667de75154553f763f781fb0d2d7146574/pino.d.ts#L116-L122

It is actually a feature of pino itself, then pino-http attaches a logger prop to all Requests, that is where I make most use of it.

naseemkullah commented 1 year ago

If you have any personal or work projects using nestjs-pino (I'm sure you do 😛 ) - then anywhere in a Controller where you have access to the req object, try this:

req.log.setBindings({ hello: 123 })
iamolegga commented 1 year ago

Sorry, I still could not find a time to check it out. Do I understand correctly that exposing setBindings is a drop in replacement for assign method for the end user?

I mean here if it works the same way when call setBindings or assign in request context

  1. calling logger on next level (new fields should be added for both cases)
  2. onResponse log (new fields should be added (or ignored?) for both cases)
naseemkullah commented 1 year ago

so every request has its own logger instance attached to it req.log ... if we set bindings to it, those will be in the onResponse log, or any use of req.log before the onResponse log

That said I found a good way to access req.log is by using the @Req decorator which works for my use case, which is to reduce the number of logs during request processing and just have all the context in the onResponse log. Thanks!

iamolegga commented 1 year ago

I believe this feature request could be open. I hope I can find time to play with it and replace assign with the built-in feature

meirkeller commented 8 months ago

This is something that would be very helpful. I tried it out with one of my projects that is using NestJS with Fastify.

@Get('foo')
foo(@Req() req: FastifyRequest) {
    req.log.setBindings({ hello: 123 });
    return 'foo';
}

I got this error req.log.setBindings is not a function. What am i missing?

iamolegga commented 8 months ago

It looks like this is still not in a public API. No mentions in docs