bithavoc / express-winston

express.js middleware for winstonjs
https://www.npmjs.com/package/express-winston
MIT License
797 stars 187 forks source link

Uuid for log messages #103

Closed frnd closed 8 years ago

frnd commented 8 years ago

This pull request adds a logger on the express request object that generates an unique id to log messages so is possible to trace log messages that belongs to a one request. For example a route like this:

router.get('/', function(req, res, next) {
      req._logger.info('begin: %s', 'start', {data: 'data'});
      res.write('This is a normal request, it should be logged to the console too');
      req._logger.info('end: %s', 'finish', {data: 'data'});
      res.end();
});

Will generate a 2 log messages like this:

{
  "data": "data",
  "requestId": "0e7e58bd-4be8-47f1-b68b-d040a8d03c90",
  "level": "info",
  "message": "begin: start"
}
{
  "data": "data",
  "requestId": "0e7e58bd-4be8-47f1-b68b-d040a8d03c90",
  "level": "info",
  "message": "end: finish"
}

The requestId will be also added to request and error logging so is easy to trace a single request from start to end.

floatingLomas commented 8 years ago

Please squash all those commits so it can be more easily reviewed.

bithavoc commented 8 years ago

@frnd please squash your commits

frnd commented 8 years ago

sorry for this messy pull request, but this is my first one I will appreciate any help :smile:

bithavoc commented 8 years ago

Squash your commits so all your 8 it looks like a single commit.

git rebase -i HEAD~8

follow the instructions (add s to all the commits from 2b5f77b to 31519bd)

Close the editor. git log should show a single commit.

frnd commented 8 years ago

thanks!! I have also executed a

git pull -f

I hope everything is ok now.

rosston commented 8 years ago

I don't want to be a pain, but I'm wondering if the functionality here really needs to be within express-winston.

Setting a response header definitely seems outside of the scope of this module. And for the requestId itself, it seems like a user could just as easily do req.requestId = Math.random() (or UUID for robustness, but same idea) by themselves.

frnd commented 8 years ago

Thanks for your comments. No problem, I sent it so you can evaluate. :smile: The idea behind a response header is because this way you can add it to crash report the user can fill in the frontend. This way, you can trace all logs messages for the request that causes the failure.

rosston commented 8 years ago

Closing due to inactivity/feature being out of scope.