From 2cdc5a17df34b723158983a966b0d1a2e9377fe7 Mon Sep 17 00:00:00 2001 From: Ryan Levick Date: Mon, 20 Oct 2025 15:14:16 +0200 Subject: [PATCH 1/4] Add metric when acquiring permit to make outbound request Signed-off-by: Ryan Levick --- crates/factor-outbound-http/src/spin.rs | 23 ++++++++++++++++++++++- crates/factor-outbound-http/src/wasi.rs | 23 ++++++++++++++++++++++- 2 files changed, 44 insertions(+), 2 deletions(-) diff --git a/crates/factor-outbound-http/src/spin.rs b/crates/factor-outbound-http/src/spin.rs index 3eb9408320..3ace01bab1 100644 --- a/crates/factor-outbound-http/src/spin.rs +++ b/crates/factor-outbound-http/src/spin.rs @@ -105,7 +105,28 @@ impl spin_http::Host for crate::InstanceState { // Note: since we don't have access to the underlying connection, we can only // limit the number of concurrent requests, not connections. let permit = match &self.concurrent_outbound_connections_semaphore { - Some(s) => s.acquire().await.ok(), + Some(s) => { + // Try to acquire a permit without waiting first + // Keep track of whether we had to wait for metrics purposes. + let mut waited = false; + let permit = match s.try_acquire() { + Ok(p) => Ok(p), + // No available permits right now; wait for one + Err(tokio::sync::TryAcquireError::NoPermits) => { + waited = true; + s.acquire().await.map_err(|_| ()) + } + Err(_) => Err(()), + }; + if permit.is_ok() { + spin_telemetry::monotonic_counter!( + outbound_http.acquired_permits = 1, + interface = "spin", + waited = waited + ); + } + permit.ok() + } None => None, }; let resp = client.execute(req).await.map_err(log_reqwest_error)?; diff --git a/crates/factor-outbound-http/src/wasi.rs b/crates/factor-outbound-http/src/wasi.rs index ef8ec62a7d..d76a81188b 100644 --- a/crates/factor-outbound-http/src/wasi.rs +++ b/crates/factor-outbound-http/src/wasi.rs @@ -591,7 +591,28 @@ impl ConnectOptions { // If we're limiting concurrent outbound requests, acquire a permit let permit = match &self.concurrent_outbound_connections_semaphore { - Some(s) => s.clone().acquire_owned().await.ok(), + Some(s) => { + // Try to acquire a permit without waiting first + // Keep track of whether we had to wait for metrics purposes. + let mut waited = false; + let permit = match s.clone().try_acquire_owned() { + Ok(p) => Ok(p), + // No available permits right now; wait for one + Err(tokio::sync::TryAcquireError::NoPermits) => { + waited = true; + s.clone().acquire_owned().await.map_err(|_| ()) + } + Err(_) => Err(()), + }; + if permit.is_ok() { + spin_telemetry::monotonic_counter!( + outbound_http.acquired_permits = 1, + interface = "wasi", + waited = waited + ); + } + permit.ok() + } None => None, }; From 94c0c08a322222bda4a0bc3d0e645d1ee8379e6c Mon Sep 17 00:00:00 2001 From: Ryan Levick Date: Mon, 20 Oct 2025 16:12:14 +0200 Subject: [PATCH 2/4] Dedup impl Signed-off-by: Ryan Levick --- crates/factor-outbound-http/src/lib.rs | 60 +++++++++++++++++++++++++ crates/factor-outbound-http/src/spin.rs | 30 +++---------- crates/factor-outbound-http/src/wasi.rs | 31 +++---------- 3 files changed, 71 insertions(+), 50 deletions(-) diff --git a/crates/factor-outbound-http/src/lib.rs b/crates/factor-outbound-http/src/lib.rs index 094b9a21ef..7fd8f44fb3 100644 --- a/crates/factor-outbound-http/src/lib.rs +++ b/crates/factor-outbound-http/src/lib.rs @@ -142,6 +142,66 @@ impl InstanceState { impl SelfInstanceBuilder for InstanceState {} +/// Helper module for acquiring permits from the outbound connections semaphore. +/// +/// This is used by the outbound HTTP implementations to limit concurrent outbound connections. +mod concurrent_outbound_connections { + use super::*; + + /// Acquires a semaphore permit for the given interface, if a semaphore is configured. + pub async fn acquire_semaphore<'a>( + interface: &str, + semaphore: &'a Option>, + ) -> Option> { + let s = semaphore.as_ref()?; + acquire(interface, || s.try_acquire(), async || s.acquire().await).await + } + + /// Acquires an owned semaphore permit for the given interface, if a semaphore is configured. + pub async fn acquire_owned_semaphore( + interface: &str, + semaphore: &Option>, + ) -> Option { + let s = semaphore.as_ref()?; + acquire( + interface, + || s.clone().try_acquire_owned(), + async || s.clone().acquire_owned().await, + ) + .await + } + + /// Helper function to acquire a semaphore permit, either immediately or by waiting. + /// + /// Allows getting either a borrowed or owned permit. + async fn acquire( + interface: &str, + try_acquire: impl Fn() -> Result, + acquire: impl AsyncFnOnce() -> Result, + ) -> Option { + // Try to acquire a permit without waiting first + // Keep track of whether we had to wait for metrics purposes. + let mut waited = false; + let permit = match try_acquire() { + Ok(p) => Ok(p), + // No available permits right now; wait for one + Err(tokio::sync::TryAcquireError::NoPermits) => { + waited = true; + acquire().await.map_err(|_| ()) + } + Err(_) => Err(()), + }; + if permit.is_ok() { + spin_telemetry::monotonic_counter!( + outbound_http.acquired_permits = 1, + interface = interface, + waited = waited + ); + } + permit.ok() + } +} + pub type Request = http::Request; pub type Response = http::Response; diff --git a/crates/factor-outbound-http/src/spin.rs b/crates/factor-outbound-http/src/spin.rs index 3ace01bab1..08d725632c 100644 --- a/crates/factor-outbound-http/src/spin.rs +++ b/crates/factor-outbound-http/src/spin.rs @@ -104,31 +104,11 @@ impl spin_http::Host for crate::InstanceState { // If we're limiting concurrent outbound requests, acquire a permit // Note: since we don't have access to the underlying connection, we can only // limit the number of concurrent requests, not connections. - let permit = match &self.concurrent_outbound_connections_semaphore { - Some(s) => { - // Try to acquire a permit without waiting first - // Keep track of whether we had to wait for metrics purposes. - let mut waited = false; - let permit = match s.try_acquire() { - Ok(p) => Ok(p), - // No available permits right now; wait for one - Err(tokio::sync::TryAcquireError::NoPermits) => { - waited = true; - s.acquire().await.map_err(|_| ()) - } - Err(_) => Err(()), - }; - if permit.is_ok() { - spin_telemetry::monotonic_counter!( - outbound_http.acquired_permits = 1, - interface = "spin", - waited = waited - ); - } - permit.ok() - } - None => None, - }; + let permit = crate::concurrent_outbound_connections::acquire_semaphore( + "spin", + &self.concurrent_outbound_connections_semaphore, + ) + .await; let resp = client.execute(req).await.map_err(log_reqwest_error)?; drop(permit); diff --git a/crates/factor-outbound-http/src/wasi.rs b/crates/factor-outbound-http/src/wasi.rs index d76a81188b..15ae413d80 100644 --- a/crates/factor-outbound-http/src/wasi.rs +++ b/crates/factor-outbound-http/src/wasi.rs @@ -590,31 +590,12 @@ impl ConnectOptions { crate::remove_blocked_addrs(&self.blocked_networks, &mut socket_addrs)?; // If we're limiting concurrent outbound requests, acquire a permit - let permit = match &self.concurrent_outbound_connections_semaphore { - Some(s) => { - // Try to acquire a permit without waiting first - // Keep track of whether we had to wait for metrics purposes. - let mut waited = false; - let permit = match s.clone().try_acquire_owned() { - Ok(p) => Ok(p), - // No available permits right now; wait for one - Err(tokio::sync::TryAcquireError::NoPermits) => { - waited = true; - s.clone().acquire_owned().await.map_err(|_| ()) - } - Err(_) => Err(()), - }; - if permit.is_ok() { - spin_telemetry::monotonic_counter!( - outbound_http.acquired_permits = 1, - interface = "wasi", - waited = waited - ); - } - permit.ok() - } - None => None, - }; + + let permit = crate::concurrent_outbound_connections::acquire_owned_semaphore( + "wasi", + &self.concurrent_outbound_connections_semaphore, + ) + .await; let stream = timeout(self.connect_timeout, TcpStream::connect(&*socket_addrs)) .await From 85d66c1f74ff16c34768ecaf239b2501f3e8ba98 Mon Sep 17 00:00:00 2001 From: Ryan Levick Date: Mon, 20 Oct 2025 18:55:07 +0200 Subject: [PATCH 3/4] Change the metric name Signed-off-by: Ryan Levick --- crates/factor-outbound-http/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/factor-outbound-http/src/lib.rs b/crates/factor-outbound-http/src/lib.rs index 7fd8f44fb3..d30215c4f1 100644 --- a/crates/factor-outbound-http/src/lib.rs +++ b/crates/factor-outbound-http/src/lib.rs @@ -193,7 +193,7 @@ mod concurrent_outbound_connections { }; if permit.is_ok() { spin_telemetry::monotonic_counter!( - outbound_http.acquired_permits = 1, + outbound_http.concurrent_connection_permits_acquired = 1, interface = interface, waited = waited ); From fe1da82018a9e4cd2ce5d288ff5e6049ac363477 Mon Sep 17 00:00:00 2001 From: Ryan Levick Date: Fri, 7 Nov 2025 15:33:59 +0100 Subject: [PATCH 4/4] Document better how metrics are supposed to work Signed-off-by: Ryan Levick --- crates/telemetry/src/metrics.rs | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/crates/telemetry/src/metrics.rs b/crates/telemetry/src/metrics.rs index 2dc2ed811c..e01e90c70c 100644 --- a/crates/telemetry/src/metrics.rs +++ b/crates/telemetry/src/metrics.rs @@ -61,6 +61,8 @@ pub(crate) fn otel_metrics_layer LookupSpan<'span>>( /// /// The increment may only be an i64 or f64. You must not mix types for the same metric. /// +/// Takes advantage of counter support in [tracing-opentelemetry](https://docs.rs/tracing-opentelemetry/0.32.0/tracing_opentelemetry/struct.MetricsLayer.html). +/// /// ```no_run /// # use spin_telemetry::metrics::counter; /// counter!(spin.metric_name = 1, metric_attribute = "value"); @@ -76,6 +78,8 @@ macro_rules! counter { /// /// The increment may only be an i64 or f64. You must not mix types for the same metric. /// +/// Takes advantage of histogram support in [tracing-opentelemetry](https://docs.rs/tracing-opentelemetry/0.32.0/tracing_opentelemetry/struct.MetricsLayer.html). +/// /// ```no_run /// # use spin_telemetry::metrics::histogram; /// histogram!(spin.metric_name = 1.5, metric_attribute = "value"); @@ -91,6 +95,8 @@ macro_rules! histogram { /// /// The increment may only be a positive i64 or f64. You must not mix types for the same metric. /// +/// Takes advantage of monotonic counter support in [tracing-opentelemetry](https://docs.rs/tracing-opentelemetry/0.32.0/tracing_opentelemetry/struct.MetricsLayer.html). +/// /// ```no_run /// # use spin_telemetry::metrics::monotonic_counter; /// monotonic_counter!(spin.metric_name = 1, metric_attribute = "value"); @@ -101,6 +107,23 @@ macro_rules! monotonic_counter { } } +#[macro_export] +/// Records an increment to the named monotonic counter with the given attributes. +/// +/// The increment may only be a positive i64 or f64. You must not mix types for the same metric. +/// +/// Takes advantage of gauge support in [tracing-opentelemetry](https://docs.rs/tracing-opentelemetry/0.32.0/tracing_opentelemetry/struct.MetricsLayer.html). +/// +/// ```no_run +/// # use spin_telemetry::metrics::gauge; +/// gauge!(spin.metric_name = 1, metric_attribute = "value"); +/// ``` +macro_rules! gauge { + ($metric:ident $(. $suffixes:ident)* = $metric_value:expr $(, $attrs:ident=$values:expr)*) => { + tracing::trace!(gauge.$metric $(. $suffixes)* = $metric_value $(, $attrs=$values)*); + } +} + pub use counter; pub use histogram; pub use monotonic_counter;