search

pinojs / pino-http

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

Fix req object placement #322

Closed stychu closed 7 months ago

stychu commented 7 months ago

Fixes https://github.com/pinojs/pino-http/issues/307#issue-1992665710

stychu commented 7 months ago

Unfortunately creating a child logger with the given request set is critical to keep attributing all logs to the given request.

Not sure if I follow. I believe it does keep the request set to all logs having it this way. req is present for all logs unless this condition is meet https://github.com/pinojs/pino-http/blob/77191aedae7d0fb0370d0b5761613ec97be29941/logger.js#L191

that why Im not sure about that one test. If the req should be added to this false evaluated condition then all logs will contain req/res objects

mcollina commented 7 months ago

I've run the example.js file in both master and this PR. It's different.

master

➜  pino-http git:(master) node example.js
{"level":30,"time":1706779462620,"pid":27894,"hostname":"mcl","req":{"id":1,"method":"GET","url":"/","headers":{"host":"localhost:3000","user-agent":"curl/8.4.0","accept":"*/*"},"remoteAddress":"::1","remotePort":56915},"msg":"something else"}
{"level":30,"time":1706779462624,"pid":27894,"hostname":"mcl","req":{"id":1,"method":"GET","url":"/","headers":{"host":"localhost:3000","user-agent":"curl/8.4.0","accept":"*/*"},"remoteAddress":"::1","remotePort":56915},"res":{"statusCode":200,"headers":{}},"responseTime":4,"msg":"request completed"}

This PR:

➜  pino-http git:(fix-nexted-key) node example.js
{"level":30,"time":1706779498790,"pid":28809,"hostname":"mcl","msg":"something else"}
{"level":30,"time":1706779498793,"pid":28809,"hostname":"mcl","req":{"id":1,"method":"GET","url":"/","headers":{"host":"localhost:3000","user-agent":"curl/8.4.0","accept":"*/*"},"remoteAddress":"::1","remotePort":56932},"res":{"statusCode":200,"headers":{}},"responseTime":3,"msg":"request completed"}