-
Notifications
You must be signed in to change notification settings - Fork 95
feat(http): expose upstream_states and request timing/byte accessors #291
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,3 +8,4 @@ pub use conf::*; | |
| pub use module::*; | ||
| pub use request::*; | ||
| pub use status::*; | ||
| pub use upstream::*; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,6 +9,7 @@ use crate::core::*; | |
| use crate::ffi::*; | ||
| use crate::http::HttpPhase; | ||
| use crate::http::status::*; | ||
| use crate::http::upstream::UpstreamState; | ||
|
|
||
| /// Define a static request handler. | ||
| /// | ||
|
|
@@ -221,6 +222,65 @@ impl Request { | |
| Some(self.0.upstream) | ||
| } | ||
|
|
||
| /// Per-attempt upstream states recorded for this request. Empty | ||
| /// when no upstream was contacted (no `proxy_pass` / | ||
| /// `fastcgi_pass` / etc., or a cache hit served directly from | ||
| /// disk). | ||
| /// | ||
| /// One entry per attempt in chronological order. A request that | ||
| /// exercises `proxy_next_upstream` yields multiple entries (e.g. | ||
| /// the failed peer, then the successful one); a request that | ||
| /// succeeds on the first try yields exactly one entry — which is | ||
| /// the same `ngx_http_upstream_state_t` that `u->state` points | ||
| /// at. | ||
| pub fn upstream_states(&self) -> &[UpstreamState] { | ||
| let arr = self.0.upstream_states; | ||
| if arr.is_null() { | ||
| return &[]; | ||
| } | ||
| // SAFETY: `upstream_states`, when non-null, points to an | ||
| // `ngx_array_t` allocated in the request pool, valid for the | ||
| // lifetime of the request. | ||
| let raw = unsafe { &*arr }; | ||
| if raw.nelts == 0 { | ||
| return &[]; | ||
| } | ||
| // SAFETY: `UpstreamState` is `#[repr(transparent)]` over | ||
| // `ngx_http_upstream_state_t`, so the layouts are identical | ||
| // and the slice cast is sound. | ||
| unsafe { slice::from_raw_parts(raw.elts.cast::<UpstreamState>(), raw.nelts) } | ||
| } | ||
|
|
||
| /// Wall-clock seconds (since the epoch) at which nginx began | ||
| /// processing this request. Pair with [`Request::start_msec`] | ||
| /// to compute request latency. | ||
| pub fn start_sec(&self) -> time_t { | ||
| self.0.start_sec | ||
| } | ||
|
|
||
| /// Milliseconds-since-second component of the request start | ||
| /// time, complementing [`Request::start_sec`]. | ||
| pub fn start_msec(&self) -> ngx_msec_t { | ||
| self.0.start_msec as ngx_msec_t | ||
| } | ||
|
|
||
| /// Bytes received from the client for this request, including | ||
| /// the request line and headers. Useful as the source label | ||
| /// for `ingress` byte counters. | ||
| pub fn request_length(&self) -> off_t { | ||
| self.0.request_length | ||
| } | ||
|
|
||
| /// Bytes sent to the client on this request's connection. | ||
| /// Reads `r->connection->sent`, the byte total nginx itself uses | ||
| /// to render `$bytes_sent` in `log_format`. | ||
| pub fn bytes_sent(&self) -> off_t { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We may prefer in the future to have a separate type for surfacing |
||
| // SAFETY: `connection` is non-null for the lifetime of a | ||
| // valid `Request` (set by nginx core in | ||
| // `ngx_http_create_request`). | ||
| unsafe { (*self.0.connection).sent } | ||
| } | ||
|
|
||
| /// Pointer to a [`ngx_connection_t`] client connection object. | ||
| /// | ||
| /// [`ngx_connection_t`]: https://nginx.org/en/docs/dev/development_guide.html#connection | ||
|
|
@@ -800,3 +860,100 @@ enum MethodInner { | |
| Trace, | ||
| Connect, | ||
| } | ||
|
|
||
| #[cfg(test)] | ||
| mod tests { | ||
| use core::mem::MaybeUninit; | ||
|
|
||
| extern crate alloc; | ||
| use alloc::vec::Vec; | ||
|
|
||
| use super::*; | ||
| use crate::ffi::{ngx_array_t, ngx_connection_t, ngx_http_upstream_state_t}; | ||
|
|
||
| fn zeroed_request() -> ngx_http_request_t { | ||
| // SAFETY: `ngx_http_request_t` is a `#[repr(C)]` aggregate of | ||
| // pointers and scalars for which the all-zero bit pattern is | ||
| // a defined (if mostly meaningless) value. Tests only call | ||
| // accessors that read fields populated below. | ||
| unsafe { MaybeUninit::zeroed().assume_init() } | ||
| } | ||
|
|
||
| fn request_from(r: &mut ngx_http_request_t) -> &mut Request { | ||
| // SAFETY: `Request` is `#[repr(transparent)]` over | ||
| // `ngx_http_request_t`, so the reference is sound. | ||
| unsafe { Request::from_ngx_http_request(r) } | ||
| } | ||
|
|
||
| #[test] | ||
| fn upstream_states_empty_when_array_null() { | ||
| let mut r = zeroed_request(); | ||
| // upstream_states is already null after zero-init. | ||
| let req = request_from(&mut r); | ||
| assert!(req.upstream_states().is_empty()); | ||
| } | ||
|
|
||
| #[test] | ||
| fn upstream_states_empty_when_nelts_zero() { | ||
| let mut arr: ngx_array_t = unsafe { MaybeUninit::zeroed().assume_init() }; | ||
| // `nelts == 0` should be reported as empty even if `elts` | ||
| // happens to be non-null (nginx leaves it pointing at the | ||
| // initial allocation). | ||
| arr.elts = (&raw mut arr).cast(); | ||
| arr.nelts = 0; | ||
|
|
||
| let mut r = zeroed_request(); | ||
| r.upstream_states = &raw mut arr; | ||
| let req = request_from(&mut r); | ||
| assert!(req.upstream_states().is_empty()); | ||
| } | ||
|
|
||
| #[test] | ||
| fn upstream_states_returns_each_attempt_in_order() { | ||
| // Two upstream attempts: a 502 followed by a 200, mirroring a | ||
| // `proxy_next_upstream` retry. | ||
| let mut states: [ngx_http_upstream_state_t; 2] = | ||
| unsafe { MaybeUninit::zeroed().assume_init() }; | ||
| states[0].status = 502; | ||
| states[0].response_time = 3; | ||
| states[1].status = 200; | ||
| states[1].response_time = 12; | ||
|
|
||
| let mut arr: ngx_array_t = unsafe { MaybeUninit::zeroed().assume_init() }; | ||
| arr.elts = states.as_mut_ptr().cast(); | ||
| arr.nelts = states.len(); | ||
|
|
||
| let mut r = zeroed_request(); | ||
| r.upstream_states = &raw mut arr; | ||
| let req = request_from(&mut r); | ||
|
|
||
| let collected: Vec<(u16, u64)> = | ||
| req.upstream_states().iter().map(|s| (s.status(), s.response_time() as u64)).collect(); | ||
| let expected: Vec<(u16, u64)> = [(502u16, 3u64), (200, 12)].into_iter().collect(); | ||
| assert_eq!(collected, expected); | ||
| } | ||
|
|
||
| #[test] | ||
| fn timing_and_length_accessors_read_underlying_fields() { | ||
| let mut r = zeroed_request(); | ||
| r.start_sec = 1_700_000_000; | ||
| r.start_msec = 250; | ||
| r.request_length = 4096; | ||
|
|
||
| let req = request_from(&mut r); | ||
| assert_eq!(req.start_sec(), 1_700_000_000); | ||
| assert_eq!(req.start_msec(), 250); | ||
| assert_eq!(req.request_length(), 4096); | ||
| } | ||
|
|
||
| #[test] | ||
| fn bytes_sent_reads_through_connection() { | ||
| let mut conn: ngx_connection_t = unsafe { MaybeUninit::zeroed().assume_init() }; | ||
| conn.sent = 8192; | ||
| let mut r = zeroed_request(); | ||
| r.connection = &raw mut conn; | ||
|
|
||
| let req = request_from(&mut r); | ||
| assert_eq!(req.bytes_sent(), 8192); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,91 @@ | ||
| use crate::core::NgxStr; | ||
| use crate::ffi::{ngx_http_upstream_state_t, ngx_msec_t, off_t}; | ||
|
|
||
| /// Per-attempt state nginx records for each upstream contact a | ||
| /// request makes. A request that succeeds on the first try has | ||
| /// exactly one entry; a request that exercises `proxy_next_upstream` | ||
| /// has one entry per attempt (e.g. failed peer, then successful | ||
| /// peer) in chronological order. | ||
| /// | ||
| /// Yielded by [`crate::http::Request::upstream_states`]. Each | ||
| /// instance borrows from the underlying `ngx_http_request_t`'s pool | ||
| /// and is valid for the lifetime of the request. | ||
| /// | ||
| /// See <https://nginx.org/en/docs/dev/development_guide.html#http_request> | ||
| /// for the per-attempt fields nginx tracks. | ||
| #[repr(transparent)] | ||
| pub struct UpstreamState(ngx_http_upstream_state_t); | ||
|
|
||
| impl UpstreamState { | ||
| /// Peer address contacted for this attempt (typically `host:port` | ||
| /// or `unix:/path`), or `None` when no peer was ever selected. | ||
| /// | ||
| /// `None` is the cache-HIT path where `r->upstream` exists (nginx | ||
| /// uses the upstream framework to consult the cache) but no | ||
| /// backend was contacted, plus init-time slots before peer | ||
| /// selection in failure cases. | ||
| pub fn peer(&self) -> Option<&NgxStr> { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. again here: I wonder if we can set the lifetime of the NgxStr to an identical lifetime attached to self. |
||
| if self.0.peer.is_null() { | ||
| return None; | ||
| } | ||
| // SAFETY: `peer` points to an `ngx_str_t` owned by the request | ||
| // pool when non-null; it lives at least as long as `&self`. | ||
| let s = unsafe { *self.0.peer }; | ||
| if s.len == 0 { | ||
| return None; | ||
| } | ||
| Some(unsafe { NgxStr::from_ngx_str(s) }) | ||
| } | ||
|
|
||
| /// HTTP status returned by the peer. Zero when the attempt | ||
| /// failed before any response was received (e.g. connect error, | ||
| /// timeout). | ||
| pub fn status(&self) -> u16 { | ||
| self.0.status as u16 | ||
| } | ||
|
|
||
| /// Total time (in milliseconds) spent on this attempt, from | ||
| /// connect through the last byte of the response. | ||
| pub fn response_time(&self) -> ngx_msec_t { | ||
| self.0.response_time | ||
| } | ||
|
|
||
| /// Time (in milliseconds) the connect() syscall took. Useful | ||
| /// for distinguishing connection setup latency from full | ||
| /// response latency. | ||
| pub fn connect_time(&self) -> ngx_msec_t { | ||
| self.0.connect_time | ||
| } | ||
|
|
||
| /// Time (in milliseconds) waiting for the peer to start sending | ||
| /// the response header. | ||
| pub fn header_time(&self) -> ngx_msec_t { | ||
| self.0.header_time | ||
| } | ||
|
|
||
| /// Time (in milliseconds) the request spent queued before nginx | ||
| /// dispatched it to this peer. | ||
| pub fn queue_time(&self) -> ngx_msec_t { | ||
| self.0.queue_time | ||
| } | ||
|
|
||
| /// Bytes sent to the peer (the proxied request body). | ||
| pub fn bytes_sent(&self) -> off_t { | ||
| self.0.bytes_sent | ||
| } | ||
|
|
||
| /// Bytes received from the peer (the response, including | ||
| /// headers). | ||
| pub fn bytes_received(&self) -> off_t { | ||
| self.0.bytes_received | ||
| } | ||
|
|
||
| /// Length of the response body advertised by the peer. | ||
| pub fn response_length(&self) -> off_t { | ||
| self.0.response_length | ||
| } | ||
| } | ||
|
|
||
| /// Define a static upstream peer initializer | ||
| /// | ||
| /// Initializes the upstream 'get', 'free', and 'session' callbacks and gives the module writer an | ||
|
|
@@ -22,3 +110,86 @@ macro_rules! http_upstream_init_peer_pt { | |
| } | ||
| }; | ||
| } | ||
|
|
||
| #[cfg(test)] | ||
| mod tests { | ||
| use core::mem::MaybeUninit; | ||
|
|
||
| use super::*; | ||
| use crate::ffi::ngx_str_t; | ||
|
|
||
| /// Build a zero-initialised `UpstreamState` for tests. Real | ||
| /// nginx allocates these from the request pool; here we just | ||
| /// need the bytes to be zero so each accessor sees a defined | ||
| /// starting point. | ||
| fn zeroed_state() -> UpstreamState { | ||
| // SAFETY: `ngx_http_upstream_state_t` is `#[repr(C)]` and | ||
| // every field is a scalar or raw pointer for which all-zero | ||
| // bytes is a valid (and defined) value. | ||
| unsafe { MaybeUninit::zeroed().assume_init() } | ||
| } | ||
|
|
||
| #[test] | ||
| fn peer_none_when_pointer_null() { | ||
| let state = zeroed_state(); | ||
| assert!(state.peer().is_none()); | ||
| } | ||
|
|
||
| #[test] | ||
| fn peer_none_when_str_empty() { | ||
| // The pointer is non-null but the `ngx_str_t` it targets | ||
| // is the zero-length sentinel cache lookups leave behind. | ||
| let empty: ngx_str_t = unsafe { MaybeUninit::zeroed().assume_init() }; | ||
| let mut state = zeroed_state(); | ||
| state.0.peer = (&raw const empty).cast_mut(); | ||
| assert!(state.peer().is_none()); | ||
| } | ||
|
|
||
| #[test] | ||
| fn peer_some_when_populated() { | ||
| let bytes = b"10.0.0.1:8080"; | ||
| let mut peer = ngx_str_t { len: bytes.len(), data: bytes.as_ptr().cast_mut() }; | ||
| let mut state = zeroed_state(); | ||
| state.0.peer = &raw mut peer; | ||
|
|
||
| let got = state.peer().expect("peer should resolve"); | ||
| assert_eq!(got.as_bytes(), bytes); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this final assert statement is simply testing |
||
| } | ||
|
|
||
| #[test] | ||
| fn accessors_read_underlying_fields() { | ||
| let mut state = zeroed_state(); | ||
| state.0.status = 502; | ||
| state.0.response_time = 75; | ||
| state.0.connect_time = 5; | ||
| state.0.header_time = 10; | ||
| state.0.queue_time = 1; | ||
| state.0.bytes_sent = 1024; | ||
| state.0.bytes_received = 4096; | ||
| state.0.response_length = 2048; | ||
|
|
||
| assert_eq!(state.status(), 502); | ||
| assert_eq!(state.response_time(), 75); | ||
| assert_eq!(state.connect_time(), 5); | ||
| assert_eq!(state.header_time(), 10); | ||
| assert_eq!(state.queue_time(), 1); | ||
| assert_eq!(state.bytes_sent(), 1024); | ||
| assert_eq!(state.bytes_received(), 4096); | ||
| assert_eq!(state.response_length(), 2048); | ||
| } | ||
|
|
||
| #[test] | ||
| fn upstream_state_size_matches_underlying_struct() { | ||
| // Guards the slice cast in `Request::upstream_states`: the | ||
| // `#[repr(transparent)]` newtype must have the same layout | ||
| // as the raw nginx struct so the slice elements line up. | ||
| assert_eq!( | ||
| core::mem::size_of::<UpstreamState>(), | ||
| core::mem::size_of::<ngx_http_upstream_state_t>(), | ||
| ); | ||
| assert_eq!( | ||
| core::mem::align_of::<UpstreamState>(), | ||
| core::mem::align_of::<ngx_http_upstream_state_t>(), | ||
| ); | ||
| } | ||
| } | ||
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.
I wonder if we can use a lifetime on the returned slice to inform rust that the lifetime is equivalent to the lifetime of the self reference.