restic / rest-server

Rest Server is a high performance HTTP server that implements restic's REST backend API.
BSD 2-Clause "Simplified" License
889 stars 139 forks source link

rest-server log POST request size is zero #45

Open lingej opened 6 years ago

lingej commented 6 years ago

POST requests are logged with a size of "0" GET requests are logged with the correct size

Is it possible to log the request size for POST requests too? Want to do some stats based on access logs.

Tested with rest-server 0.9.6

BR Joerg

zcalusic commented 6 years ago

Hello @lingej and thanks for participating in the project!

For the moment, I suspected some odd bug in the code, but there's really not that much code around logging. 😄 We're simply using github.com/gorilla/handlers package, and its CombinedLoggingHandler() function to provide logs.

On further inspection of the function that really synthesizes log lines, https://github.com/gorilla/handlers/blob/master/handlers.go#L238 I think I understand what's happening. The function logs response size, and for POST requests it can and probably is in most cases, well.. 0. What you would like to see is request body size for POST request. I agree it would be more useful in this case.

Now, I don't know what combined log format specifies in this case, but if you can find and testify that for POST HTTP method request body size should be logged instead, you basically found a bug in the gorilla toolkit. 😃

Meaning, there's not much to do on "this side", as soon as it's implemented in the library, we can just pull the new version and it will automagically work.

wojas commented 6 years ago

In the combined log format, this number is always "the size of the response to the client (in bytes)".

An extra request body size number would be useful. I'm not sure if anyone is actually depending on the log files being in combined format, but adding the number to the end (after User-Agent) should be safe.

See: https://stackoverflow.com/a/9234855/297793

rawtaz commented 4 years ago

I think we should solve this by adding the ability to select and/or specify the log format instead of hardcoding some workaround.

For lack of better ideas I was about to suggest adding a --log-format flag which takes one of common, combined and a custom format string as per the Apache HTTPd syntax, defaulting to combined. The --log flag will use whatever the --log-format is set to.

There's a problem with this though. In the Apache HTTPd log format, as well as the Gorilla handlers used for logging, there's no syntax for "POST request size without headers", there's only %I which is "Bytes received, including request and headers". This is inconsistent with the current size output which uses %b which is "Size of response in bytes, excluding HTTP headers".

So in summary, from what I can tell there's currently no way that we can use existing de facto log format syntax and get consistent size numbers for both incoming and outgoing data. Or am I missing something?

At the same time I'm wondering how much this is really needed. @lingej how are you dealing with it so far? Do you still have the need for this metric? Is it something you could possibly derive from the Prometheus metrics endpoint (enabled by --prometheus and access at /metrics)?

rawtaz commented 4 years ago

@lingej Ping :)