not best practice to bind to Node request and response object?

[niftylettuce]

I think that it'd be best practice to not bind to the Node request/response object and instead just solely bind to the Koa request and response object.

https://github.com/pinojs/koa-pino-logger/blob/master/logger.js#L11

-ctx.log = ctx.request.log = ctx.response.log = ctx.req.log
+ctx.log = ctx.request.log = ctx.response.log;

Also not having ctx.res.log to begin with is kind of odd since you're binding to ctx.response.log and ctx.req.log, should be a 1:1 relationship.

[niftylettuce]

if you're open to this I will submit PR to fix, would require a major version bump and also a deprecation notice IMO

[davidmarkclements]

Yeah I’m ok with that -
@mcollina @jsumners ?

[mcollina]

I'm +1, that should be a semver-major change.

[jsumners]

I know nothing of Koa. Do whatever is best.

[niftylettuce]

Thanks, I'll make a PR now! Also feel free to add me to NPM and GitHub so I can help maintain this package. I use it directly in CabinJS https://cabinjs.com and Lad https://lad.js.org.

[niftylettuce]

Actually it looks like pino-http is the culprit as to why this issue exists to begin with - specifically https://github.com/pinojs/pino-http/blob/f8236514c514a85885701cd82667a4b1fbc48a8a/logger.js#L62 sets req.log, which in this context we can't workaround unless we have an ugly delete ctx.req.log afterwards.

The reason we need to pass the Node request and response object to pino-http's wrapping is because of this https://github.com/pinojs/pino-http/blob/master/logger.js#L66-L67 (event listeners on Node's request object).

Any thoughts as to what we can do here?

[mcollina]

I think we should follow the same approach we do in hapi-pino, mainly building a custom module for Koa. Would that make sense? Would you like to send a PR?

[stephanebachelier]

@mcollina are you still interested in building a custom module for Koa ? There are some limitations apart from the fact that this module is only a thin wrapper around pino-http.

[mcollina]

It'd be great if somebody contributed a PR :).

[stephanebachelier]

@mcollina ok I'm in.

[jmealo]

@stephanebachelier: Where did you end up with this? I've built out a fair bit of log enrichment outside of process that I'd like to get rid of.

[jmealo]

@stephanebachelier: @mcollina: #31