search

pinojs / pino-http

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

`wrapChild` doesn't pass on options to the created child logger as expected #310

Closed ramblingenzyme closed 11 months ago

ramblingenzyme commented 11 months ago

https://github.com/pinojs/pino-http/blob/9109d5446a9c2cc02ba5b98f3bd345d9c24db61a/logger.js#L83 image

Based on the published API of pino, passing option overrides to a child logger is the second argument to .child, not the first. https://github.com/pinojs/pino/blob/master/docs/api.md#loggerchildbindings-options--logger image

I think this bit of code needs an update and tests written for that use case, as well as validating if the user has passed logger & any pino options to pino-http that are ignored by .child.

mcollina commented 11 months ago

Thanks for reporting! Would you like to send a Pull Request to address this issue? Remember to add unit tests.

ramblingenzyme commented 11 months ago

I'm happy to take a stab at it, I'll let you know if I get a PR up or if I back out.

ramblingenzyme commented 11 months ago

@mcollina 🤦🏽 I don't know how I ended up in the commit I linked, but I was looking at 6 year old code, the current code is as expected. Sorry for the incorrect bug report.

https://github.com/pinojs/pino-http/blob/master/logger.js#L218 https://github.com/pinojs/pino-http/blob/44c61b76c0c05dace6b4c701d12c6a546c6ad013/logger.js#L218