building5 / sails-hook-bunyan

Bunyan integration for Sails
MIT License
5 stars 3 forks source link

Request not serialized correctly #1

Closed JeffAshton closed 9 years ago

JeffAshton commented 9 years ago

When creating a child logger, we are passing ( simple: true )

From: https://github.com/building5/sails-hook-bunyan/blob/master/index.js#L158

req.log = _this.logger.child({req: req}, true);

Reading the documentation in bunyan, for child (https://github.com/trentm/node-bunyan/blob/master/lib/bunyan.js#L584),

Passing true fails condition (b) since we want to serialize the request.

Well I can appreciate we want things to be fast, I can't think of a case where we would want to serialize the entire request. There is way to much crap on the request object otherwise.

leedm777 commented 9 years ago

@JeffAshtonCT Yikes. I thought the non-serialization for simple children was a bug (and submitted a PR to "fix" it).

Well I can appreciate we want things to be fast, I can't think of a case where we would want to serialize the entire request.

This wasn't about being fast. The reopenFileStreams() call on bunyan causes child loggers to crash if they aren't simple (b/c they don't share the stream objects that are being reopened).

Giving it some more thought, I can run the req serializer manually when creating the child logger. That should fix both problems. Thoughts?

leedm777 commented 9 years ago

Fixed in v2.1.1

JeffAshton commented 9 years ago

Looks good. Thanks for fixing.