Reduce error logging and make $repsheet more meaningful

thetristan commented 8 years ago

Is there any opposition to me changing the $repsheet variable (or adding a new variable) that indicates the action the repsheet module took for the current request?

Examples:

For a blocked request $repsheet would be set to blacklist
For a marked request $repsheet would be set to mark
For a whitelisted request, $repsheet would be set to whitelist
For all other requests, $repsheet would be left unset

The $repsheet_reason variable would also be updated to include the reason and actor. E.g. IP 127.0.0.1 was marked: bad robot, User foo was blocked: an important reason. Or alternatively $repsheet_reason could be left as is and a new variable $repsheet_actor could be introduced.

This change lets users log the action the repsheet module took for the current request (if any) as part of the access log by specifying $repsheet and/or $repsheet_reason in a log_format directive. This lets users more quickly determine the action repsheet took and see that action in the context of all the other data logged for a given request in the access log.

If the user doesn't want to log the action the repsheet module took for the current request, they can simply omit using any of the $repsheet variables in the format string for their access logs.

Sample error logs (from running the tests):

2016/01/26 16:35:11 [error] 42231#0: *1 [Repsheet] - IP 127.0.0.1 was found on repsheet. Reason: bad robot, client: 127.0.0.1, server: , request: "GET /mark HTTP/1.1", host: "127.0.0.1:8888"
2016/01/26 16:35:11 [error] 42231#0: *1 open() "/Users/tblease/workspace/personal/repsheet-nginx/vendor/nginx-1.8.0/../../build/nginx/html/mark" failed (2: No such file or directory), client: 127.0.0.1, server: , request: "GET /mark HTTP/1.1", host: "127.0.0.1:8888"
2016/01/26 16:35:11 [error] 42231#0: *1 [Repsheet] - IP 127.0.0.1 was found on repsheet. Reason: bad robot, client: 127.0.0.1, server: , request: "GET / HTTP/1.1", host: "127.0.0.1:8888"
2016/01/26 16:35:11 [error] 42231#0: *1 [Repsheet] - IP 127.0.0.1 was found on repsheet. Reason: bad robot, client: 127.0.0.1, server: , request: "GET / HTTP/1.1", host: "127.0.0.1:8888"
2016/01/26 16:35:11 [error] 42231#0: *1 [Repsheet] - IP 1.1.1.1 was blocked by repsheet. Reason: Integration Spec, client: 127.0.0.1, server: , request: "GET /real HTTP/1.1", host: "127.0.0.1:8888"
2016/01/26 16:35:11 [error] 42231#0: *1 [Repsheet] - IP 1.1.1.1 is whitelisted by repsheet. Reason: Integration Spec, client: 127.0.0.1, server: , request: "GET /real HTTP/1.1", host: "127.0.0.1:8888"
2016/01/26 16:35:11 [error] 42231#0: *5 [Repsheet] - IP 1.1.1.1 is whitelisted by repsheet. Reason: Integration Spec, client: 127.0.0.1, server: , request: "GET / HTTP/1.0", host: "127.0.0.1:8888"
2016/01/26 16:35:11 [error] 42231#0: *5 [Repsheet] - IP 1.1.1.1 is whitelisted by repsheet. Reason: Integration Spec, client: 127.0.0.1, server: , request: "GET / HTTP/1.0", host: "127.0.0.1:8888"
2016/01/26 16:35:11 [error] 42231#0: *1 [Repsheet] - Request was blocked by repsheet. Reason: Invalid X-Forwarded-For, client: 127.0.0.1, server: , request: "GET /nofallback HTTP/1.1", host: "127.0.0.1:8888"

davidhanley commented 8 years ago

I like this change. I think it results in more useful logs.

abedra commented 7 years ago

@thetristan @davidhanley are you still interested in this?

davidhanley commented 7 years ago

I like the change, though it's not a priority for us at the moment.

abedra commented 7 years ago

@thetristan did you ever send these changes as a PR? I don't see a record of them?

repsheet / repsheet-nginx

Reduce error logging and make $repsheet more meaningful #40