stats: statistics on request and response body size#4433
stats: statistics on request and response body size#4433zyfjeff wants to merge 5 commits intoenvoyproxy:masterfrom
Conversation
zyfjeff
force-pushed
the
statistics_body_size
branch
from
950ec26 to
339b850
Compare
zyfjeff
force-pushed
the
statistics_body_size
branch
from
e0da368 to
4837b77
Compare
Signed-off-by: tianqian.zyf <[email protected]>
Signed-off-by: zyfjeff <[email protected]>
Signed-off-by: zyfjeff <[email protected]>
Signed-off-by: zyfjeff <[email protected]>
| upstream_cx_destroy_local_with_active_rq, Counter, Total connections destroyed locally with 1+ active request | ||
| upstream_cx_destroy_remote_with_active_rq, Counter, Total connections destroyed remotely with 1+ active request | ||
| upstream_cx_close_notify, Counter, Total connections closed via HTTP/1.1 connection close header or HTTP/2 GOAWAY | ||
| upstream_cx_rx_bytes_total, Histogram, Size of received connection bytes |
There was a problem hiding this comment.
should this say upstream_cx_rx_bytes? and below?
There was a problem hiding this comment.
I think it needs to be fixed here too.
There was a problem hiding this comment.
sorry, I check again carefully
zyfjeff
force-pushed
the
statistics_body_size
branch
from
790e878 to
295f0ec
Compare
Signed-off-by: zyfjeff <[email protected]>
|
Assigning @zuercher for senior review. |
zuercher
left a comment
zuercher
left a comment
There was a problem hiding this comment.
This doesn't seem to actually resolve the referenced ticket (#3621). This is recording a histogram of the size of read and write buffers. Those values are related to the request/response body size, but they're not the same. Partly the distinction is the size of headers in HTTP requests, but also there's no guarantee Envoy will receive an entire request (or request body) in a single read.
This seems like it might be useful for the tcp proxy (since there's no way to distinguish requests there), but for HTTP and redis, tracking the size of actual requests & responses seems more useful to me.
|
|
||
| downstream_cx_total, Counter, Total number of connections handled by the filter | ||
| downstream_cx_no_route, Counter, Number of connections for which no matching route was found or the cluster for the route was not found | ||
| downstream_cx_tx_bytes, Histogram, Bytes written to the downstream connection |
There was a problem hiding this comment.
Nit: Should say "Size of bytes written" (and "Size of bytes read" below) to match the others.
|
@zyfjeff are you still working on this? |
|
@zuercher Is looking at the code and see how to count the body size, don't know what do you suggest? |
|
@zyfjeff I apologize, but I don't understand what you're asking. Could you restate it? In the meantime, I can give you some information that might help. The HTTP connection manager/router already track the number of bytes received and sent at the request level (it's stored in RequestInfo). The HTTP router also tracks the bytes received and sent for each proxied upstream request in a separate RequestInfo. Perhaps it's sufficient to expose those as stats? (We'd probably want to ask the original issue author.) Request body size tracking could be done in a similar way. |
|
This pull request has been automatically marked as stale because it has not had activity in the last 7 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions! |
stale
bot
added
the
stale
|
At present the implementation of the knowledge of each socket read data size, is not really the size of a request, does not meet the demand, so the close off the PR. |
3 participants
Signed-off-by: tianqian.zyf [email protected]
Description: statistics on request and response body size
Risk Level: low
Testing: unittest
Docs Changes: yes
Release Notes: N/A
[Optional Fixes #Issue] #3621
[Optional Deprecated:]