-
Notifications
You must be signed in to change notification settings - Fork 180
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add access logging functionality #713
Conversation
87a8dd7
to
b7d6d1c
Compare
Codecov Report
@@ Coverage Diff @@
## master #713 +/- ##
==========================================
+ Coverage 76.04% 76.52% +0.48%
==========================================
Files 36 37 +1
Lines 2371 2437 +66
==========================================
+ Hits 1803 1865 +62
- Misses 568 572 +4
Continue to review full report at Codecov.
|
b7d6d1c
to
fe41270
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Basically LGTM.
Other than a few things, most importantly making sure it doesn't break existing code if it doesn't have to.
7f08722
to
980d5c1
Compare
980d5c1
to
ed8afb1
Compare
Thanks for the review, updated all of it I think (and fixed tests on Windows). |
You have merge rights here, yes? |
Yeah, sorry for the slow review; this looks great! |
This adds access logging functionality to
HTTP.listen
/HTTP.serve
. This includes a string macro that parses NGINX style format strings (chose that since it is pretty readable, self-explanatory, and looks more like Julia code compared to e.g. Apache format strings).Example usage:
which would then log
etc.
Originally I thought this had to be implemented "on the inside" in order to get access to e.g. number of written bytes etc, but (as done in this PR) those things can technically be stored in the
http::Stream
object and you could do:but it is pretty convenient to just have to pass the
access_log
keyword and it does not really cost anything. In addition, if you want manual control you can still do it like above.