Skip to content

Commit 4f64e0d

Browse files
authored
fix(server): Track status for content-length terminated bodies (#580)
Streaming responses that declare a `content-length` were recorded as client disconnects (499) in `server.requests.duration`, a regression from #555. Hyper stops polling a response body once the declared number of bytes has been written, so the wrapper never saw an end-of-stream poll and its guard fell back to 499 when dropped. `MetricsBody` now counts the bytes it forwards and completes the request once the declared length is reached, alongside the existing end-of-stream and trailers cases. Responses whose body hyper never polls at all are completed up front when they are wrapped. The tests were reworked around a helper that serves a real axum handler through the middleware and returns the tracked status and duration, which made room for a paused-clock test asserting the duration spans body streaming rather than ending at the response headers. Capturing metrics across await points needs a new `with_capturing_test_client_async` in `objectstore-metrics`.
1 parent 725ec28 commit 4f64e0d

5 files changed

Lines changed: 253 additions & 215 deletions

File tree

objectstore-metrics/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -318,7 +318,7 @@ pub fn init(config: &MetricsConfig) -> Result<(), Error> {
318318
Ok(())
319319
}
320320

321-
pub use mock::with_capturing_test_client;
321+
pub use mock::{with_capturing_test_client, with_capturing_test_client_async};
322322

323323
// ---------------------------------------------------------------------------
324324
// Macros

objectstore-metrics/src/mock.rs

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
//! Provides [`with_capturing_test_client`], which installs a thread-local
44
//! recorder that captures all emitted metrics as DogStatsD-format strings.
55
6+
use std::future::Future;
67
use std::sync::{Arc, Mutex};
78

89
use metrics::{Counter, Gauge, Histogram, Key, KeyName, Metadata, Recorder, SharedString, Unit};
@@ -26,6 +27,30 @@ pub fn with_capturing_test_client(f: impl FnOnce()) -> Vec<String> {
2627
recorder.consume()
2728
}
2829

30+
/// Awaits `future` with a thread-local mock recorder installed, then returns all captured
31+
/// metrics as `"name:value|type|#key:value,key:value"` strings.
32+
///
33+
/// The recorder stays installed across await points, so metrics emitted while the future is
34+
/// suspended are captured as well. Since it is thread-local, the future must not migrate between
35+
/// threads: run it on a current-thread runtime, as `#[tokio::test]` does by default.
36+
///
37+
/// # Example
38+
///
39+
/// ```ignore
40+
/// let captured = objectstore_metrics::with_capturing_test_client_async(async {
41+
/// objectstore_metrics::count!("test.counter");
42+
/// })
43+
/// .await;
44+
/// assert!(captured.iter().any(|m| m.starts_with("test.counter:")));
45+
/// ```
46+
pub async fn with_capturing_test_client_async(future: impl Future<Output = ()>) -> Vec<String> {
47+
let recorder = MockRecorder::default();
48+
let guard = metrics::set_default_local_recorder(&recorder);
49+
future.await;
50+
drop(guard);
51+
recorder.consume()
52+
}
53+
2954
/// A metrics recorder that formats and stores every operation as a string.
3055
#[derive(Clone, Default)]
3156
struct MockRecorder {

objectstore-server/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ objectstore-options = { workspace = true, features = ["testing"] }
6464
objectstore-test = { workspace = true }
6565
stresstest = { workspace = true }
6666
tempfile = { workspace = true }
67+
tokio = { workspace = true, features = ["test-util"] }
6768

6869
[[bin]]
6970
name = "objectstore"

0 commit comments

Comments
 (0)