pinojs / pino-http

🌲 high-speed HTTP logger for Node.js
MIT License
527 stars 117 forks source link

Unable to remove whole req object from response logs #327

Closed mzvonar closed 6 months ago

mzvonar commented 6 months ago

Hi, firstly thank you for this amazong library.

I’m trying to remove the req object from succesful response log, because it’s huge.

I saw it was discussed here: https://github.com/pinojs/pino-http/issues/5

But I looked through the code and maybe it has chnhed simce then. It seems the res logger is always set to the fullReqLogger here: https://github.com/pinojs/pino-http/blob/master/logger.js#L150

So using customReceivedObject or serializers has no effect because by the time they are used the req is already added to the fullReqLogger context and is always part of the resulting log.

The only way I found is to use redaction, but it feels kind of hacky.

Shouldn’t the responseLogger variable also be set to log in case the quietReqLogger option is enabled?

Or is there a better way to do this?

Thanks

jsumners commented 6 months ago

Please provide a minimal reproducible example. Doing so will help us diagnose your issue. It should be the bare minimum code needed to trigger the issue, and easily runnable without any changes or extra code.

You may use a GitHub repository to host the code if it is too much to fit in a code block (or two).

mzvonar commented 6 months ago

Please provide a minimal reproducible example. Doing so will help us diagnose your issue. It should be the bare minimum code needed to trigger the issue, and easily runnable without any changes or extra code.

You may use a GitHub repository to host the code if it is too much to fit in a code block (or two).

@jsumners thanks for looking into it. I’m sorry I was extending my pono logger and I messed up this context, so the serializers were never called when I provided my logger instance to pinoHttp. I didn’t get to it today but I will verify thoroughly that everytinhg works after I fixed that.

mzvonar commented 6 months ago

Hi, I can confirm the issue was on my side. Sorry for wasting your time :disappointed: My req object was the raw express request. I was wondering why would anybody want to log the whole request :smiley:

One thing to note. When pino.stdSerializers.wrapRequestSerializer is used with the default wrapSerializers enabled, the request is wrapped twice. The second time it's the already serialized request so it's missing the req.info || req.socket. This will result in remoteAddress and remotePort being overwritten as undefined. Maybe a note could be aded to docs. It took me quite a time to understand how it works. Also as a Pino beginner I wasn't aware that serializers is a pino feature, not specific to pino-http.

Thanks for your help!