Record size of streaming request/response bodies - #4823
Conversation
| .build(), | ||
| request_body_size: meter | ||
| .f64_histogram("http.server.request.body_size") | ||
| .u64_histogram("http.server.request.body_size") |
There was a problem hiding this comment.
............. why was this ever a float? lol
There was a problem hiding this comment.
Also, I just noticed there's a slight inconsistency in the metric name versus https://opentelemetry.io/docs/specs/semconv/http/http-metrics/#metric-httpserverrequestbodysize
| /// | ||
| /// This method keeps track of whether an observation has been recorded, so it is safe to call | ||
| /// multiple times. | ||
| fn record_metric(&mut self) { |
There was a problem hiding this comment.
Instead of tracking a state in a boolean, would it be possible for this method to take self by value? Perhaps that's impossible given how http_body::Body works?
There was a problem hiding this comment.
Both call sites only have mutable access to self. I call this from both poll_frame() and from drop() to guard against the possibility that whatever is consuming the stream calls is_end_stream(), gets true, and drops the body without calling poll_frame() a final time to get None. Plus, it's also possible the consumer could call poll_frame() again after getting None.
3 participants
Closes #4822. This changes our metrics middleware to wrap request and response bodies, in order to accurately measure request and response sizes when a Content-Length header is not available.