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

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

Open
naseemkullah opened this issue Jan 18, 2023 · 8 comments
Assignees
Labels
enhancement New feature or request

Comments

@naseemkullah
Copy link

naseemkullah commented Jan 18, 2023

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:

@naseemkullah naseemkullah added the enhancement New feature or request label Jan 18, 2023
@naseemkullah naseemkullah changed the title [FEATURE REQUEST] expose request.log.setBindings() [FEATURE REQUEST] expose request.log.setBindings() (or request.log in general) Jan 18, 2023
@iamolegga
Copy link
Owner

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

@naseemkullah
Copy link
Author

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
Copy link
Author

naseemkullah commented Jan 18, 2023

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
Copy link
Owner

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
Copy link
Author

naseemkullah commented Feb 13, 2023

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
Copy link
Owner

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

@iamolegga iamolegga reopened this Feb 13, 2023
@meirkeller
Copy link

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
Copy link
Owner

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

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

No branches or pull requests

3 participants