From c7725ff01c59a969c12cc79bcfa71b64e2e75c55 Mon Sep 17 00:00:00 2001 From: Ruslan Pislari Date: Mon, 11 May 2026 13:21:55 +0300 Subject: [PATCH 1/2] refactor: remove unused request IDs and streamline request handling - Eliminated `request_id` parameter from `handle_request` method to simplify the interface. - Replaced `request_id` usage with `traceparent` for consistent tracing. - Improved hostname resolution logic for backend, prioritizing `server_name` header. - Enhanced tracing with `in_current_span` for better debugging context. - Updated test cases to align with the new method signature. --- crates/http-service/src/executor/http.rs | 28 ++++---- crates/http-service/src/executor/wasi_http.rs | 67 ++++++++++++------- crates/http-service/src/lib.rs | 29 ++++---- 3 files changed, 71 insertions(+), 53 deletions(-) diff --git a/crates/http-service/src/executor/http.rs b/crates/http-service/src/executor/http.rs index cf20a9e..1db67b1 100644 --- a/crates/http-service/src/executor/http.rs +++ b/crates/http-service/src/executor/http.rs @@ -426,7 +426,7 @@ mod tests { let http_service: HttpService = assert_ok!(ServiceBuilder::new(context).build()); - let res = assert_ok!(http_service.handle_request("1".to_smolstr(), req).await); + let res = assert_ok!(http_service.handle_request(req).await); assert_eq!(StatusCode::OK, res.status()); let headers = res.headers(); assert_eq!(4, headers.len()); @@ -481,7 +481,7 @@ mod tests { let http_service: HttpService = assert_ok!(ServiceBuilder::new(context).build()); - let res = assert_ok!(http_service.handle_request("2".to_smolstr(), req).await); + let res = assert_ok!(http_service.handle_request(req).await); assert_eq!(FASTEDGE_EXECUTION_TIMEOUT, res.status()); let headers = res.headers(); assert_eq!(4, headers.len()); @@ -545,7 +545,7 @@ mod tests { let http_service: HttpService = assert_ok!(ServiceBuilder::new(context).build()); - let res = assert_ok!(http_service.handle_request("3".to_smolstr(), req).await); + let res = assert_ok!(http_service.handle_request(req).await); assert_eq!(FASTEDGE_OUT_OF_MEMORY, res.status()); let headers = res.headers(); assert_eq!(4, headers.len()); @@ -586,7 +586,7 @@ mod tests { let http_service: HttpService = assert_ok!(ServiceBuilder::new(context).build()); - let res = assert_ok!(http_service.handle_request("4".to_smolstr(), req).await); + let res = assert_ok!(http_service.handle_request(req).await); assert_eq!(StatusCode::NOT_FOUND, res.status()); assert_eq!(0, res.headers().len()); } @@ -614,7 +614,7 @@ mod tests { let http_service: HttpService = assert_ok!(ServiceBuilder::new(context).build()); - let res = assert_ok!(http_service.handle_request("5".to_smolstr(), req).await); + let res = assert_ok!(http_service.handle_request(req).await); assert_eq!(StatusCode::NOT_FOUND, res.status()); assert_eq!(0, res.headers().len()); } @@ -642,7 +642,7 @@ mod tests { let http_service: HttpService = assert_ok!(ServiceBuilder::new(context).build()); - let res = assert_ok!(http_service.handle_request("6".to_smolstr(), req).await); + let res = assert_ok!(http_service.handle_request(req).await); assert_eq!(StatusCode::TOO_MANY_REQUESTS, res.status()); assert_eq!(0, res.headers().len()); } @@ -670,7 +670,7 @@ mod tests { let http_service: HttpService = assert_ok!(ServiceBuilder::new(context).build()); - let res = assert_ok!(http_service.handle_request("7".to_smolstr(), req).await); + let res = assert_ok!(http_service.handle_request(req).await); assert_eq!(StatusCode::NOT_ACCEPTABLE, res.status()); assert_eq!(0, res.headers().len()); } @@ -703,7 +703,7 @@ mod tests { let http_service: HttpService = assert_ok!(ServiceBuilder::new(context).build()); - let res = assert_ok!(http_service.handle_request("8".to_smolstr(), req).await); + let res = assert_ok!(http_service.handle_request(req).await); assert_eq!(StatusCode::OK, res.status()); let headers = res.headers(); assert_eq!(4, headers.len()); @@ -742,7 +742,7 @@ mod tests { let http_service: HttpService = assert_ok!(ServiceBuilder::new(context).build()); - let res = assert_ok!(http_service.handle_request("9".to_smolstr(), req).await); + let res = assert_ok!(http_service.handle_request(req).await); // Reaching OK proves we resolved via lookup_by_id (otherwise our mock for // lookup_by_name path would have produced the same status, but we also // verify in `test_fastedge_app_id_invalid_returns_not_found` below that @@ -776,7 +776,7 @@ mod tests { let http_service: HttpService = assert_ok!(ServiceBuilder::new(context).build()); - let res = assert_ok!(http_service.handle_request("10".to_smolstr(), req).await); + let res = assert_ok!(http_service.handle_request(req).await); assert_eq!(StatusCode::NOT_FOUND, res.status()); assert_eq!(0, res.headers().len()); } @@ -807,7 +807,7 @@ mod tests { let http_service: HttpService = assert_ok!(ServiceBuilder::new(context).build()); - let res = assert_ok!(http_service.handle_request("11".to_smolstr(), req).await); + let res = assert_ok!(http_service.handle_request(req).await); assert_eq!(StatusCode::NOT_FOUND, res.status()); assert_eq!(0, res.headers().len()); } @@ -837,7 +837,7 @@ mod tests { let http_service: HttpService = assert_ok!(ServiceBuilder::new(context).build()); - let res = assert_ok!(http_service.handle_request("12".to_smolstr(), req).await); + let res = assert_ok!(http_service.handle_request(req).await); assert_eq!(StatusCode::NOT_FOUND, res.status()); assert_eq!(0, res.headers().len()); } @@ -869,7 +869,7 @@ mod tests { let http_service: HttpService = assert_ok!(ServiceBuilder::new(context).build()); - let res = assert_ok!(http_service.handle_request("13".to_smolstr(), req).await); + let res = assert_ok!(http_service.handle_request(req).await); assert_eq!(StatusCode::NOT_FOUND, res.status()); assert_eq!(0, res.headers().len()); } @@ -895,7 +895,7 @@ mod tests { let http_service: HttpService = assert_ok!(ServiceBuilder::new(context).build()); - let res = assert_ok!(http_service.handle_request("14".to_smolstr(), req).await); + let res = assert_ok!(http_service.handle_request(req).await); assert_eq!(StatusCode::NOT_FOUND, res.status()); assert_eq!(0, res.headers().len()); } diff --git a/crates/http-service/src/executor/wasi_http.rs b/crates/http-service/src/executor/wasi_http.rs index 0c57624..3c3e219 100644 --- a/crates/http-service/src/executor/wasi_http.rs +++ b/crates/http-service/src/executor/wasi_http.rs @@ -4,18 +4,19 @@ use std::time::Duration; use crate::executor; use crate::executor::HttpExecutor; use crate::state::HttpState; -use ::http::{HeaderMap, Request, Response, Uri, header}; -use anyhow::{Context, anyhow, bail}; +use anyhow::{anyhow, bail, Context}; use async_trait::async_trait; +use ::http::{header, HeaderMap, Request, Response, Uri}; use http_backend::Backend; use http_body_util::{BodyExt, Full}; use hyper::body::Body; use runtime::util::stats::{StatsTimer, StatsVisitor}; -use runtime::{InstancePre, store::StoreBuilder}; +use runtime::{store::StoreBuilder, InstancePre}; use smol_str::SmolStr; -use wasmtime_wasi_http::bindings::ProxyPre; +use tracing::Instrument; use wasmtime_wasi_http::bindings::http::types::Scheme; -use wasmtime_wasi_http::{WasiHttpView, body::HyperOutgoingBody}; +use wasmtime_wasi_http::bindings::ProxyPre; +use wasmtime_wasi_http::{body::HyperOutgoingBody, WasiHttpView}; /// Execute context used by ['HttpService'] #[derive(Clone)] @@ -46,7 +47,20 @@ where let (mut parts, body) = req.into_parts(); const LOCALHOST: SmolStr = SmolStr::new_inline("localhost"); - let backend_hostname = self.backend.hostname().unwrap_or(LOCALHOST); + const SERVER_NAME: &'static str = "server_name"; + + // Resolve backend hostname using the following precedence: + // 1. `server_name` request header (if set and valid UTF-8) + // 2. backend's configured hostname + // 3. fallback to "localhost" + let backend_hostname: SmolStr = parts + .headers + .get(SERVER_NAME) + .and_then(|v| v.to_str().ok()) + .map(SmolStr::from) + .or_else(|| self.backend.hostname()) + .unwrap_or(LOCALHOST); + let hostname = match backend_hostname.find('.') { None => backend_hostname.as_str(), Some(i) => { @@ -123,25 +137,28 @@ where let proxy = proxy_pre.instantiate_async(&mut store).await?; let task_stats = stats.clone(); - let task = tokio::task::spawn(async move { - let duration = Duration::from_millis(store.data().timeout); - if let Err(e) = tokio::time::timeout( - duration, - proxy - .wasi_http_incoming_handler() - .call_handle(&mut store, req, out), - ) - .await? - { - tracing::warn!(cause=?e, "incoming handler"); - return Err(e); - }; - - drop(stats_timer); // Stop timing for stats - task_stats.memory_used(store.memory_used() as u64); - - Ok(()) - }); + let task = tokio::task::spawn( + async move { + let duration = Duration::from_millis(store.data().timeout); + if let Err(e) = tokio::time::timeout( + duration, + proxy + .wasi_http_incoming_handler() + .call_handle(&mut store, req, out), + ) + .await? + { + tracing::warn!(cause=?e, "incoming handler"); + return Err(e); + }; + + drop(stats_timer); // Stop timing for stats + task_stats.memory_used(store.memory_used() as u64); + + Ok(()) + } + .in_current_span(), + ); match receiver.await { Ok(Ok(response)) => { diff --git a/crates/http-service/src/lib.rs b/crates/http-service/src/lib.rs index b1d5ee3..d66adaf 100644 --- a/crates/http-service/src/lib.rs +++ b/crates/http-service/src/lib.rs @@ -8,11 +8,11 @@ use wasmtime_wasi_nn::wit::WasiNnView; pub use crate::executor::ExecutorFactory; use crate::executor::HttpExecutor; -use anyhow::{Context, Error, Result, bail}; +use anyhow::{bail, Context, Error, Result}; use bytes::Bytes; use http::{ - HeaderMap, HeaderName, HeaderValue, StatusCode, - header::{ACCESS_CONTROL_ALLOW_ORIGIN, CACHE_CONTROL}, + header::{ACCESS_CONTROL_ALLOW_ORIGIN, CACHE_CONTROL}, HeaderMap, HeaderName, HeaderValue, + StatusCode, }; use http_backend::SERVER_NAME_HEADER; use http_body_util::{BodyExt, Empty, Full}; @@ -22,7 +22,7 @@ use hyper_util::{client::legacy::connect::Connect, rt::TokioIo}; use runtime::util::metrics; use runtime::util::stats::StatsVisitor; use runtime::{ - App, AppResult, ContextT, Router, WasmEngine, WasmEngineBuilder, app::Status, service::Service, + app::Status, service::Service, App, AppResult, ContextT, Router, WasmEngine, WasmEngineBuilder, }; use smol_str::{SmolStr, ToSmolStr}; use state::HttpState; @@ -150,16 +150,13 @@ where Ok((stream, _)) => { tracing::debug!(remote=?stream.peer_addr(), "new http connection"); let connection = self_.clone(); - use tracing::Instrument; let io = TokioIo::new(stream); let service = service_fn(move |req| { let self_ = connection.clone(); - let request_id = remote_traceparent(&req); async move { self_ - .handle_request(request_id.clone(), req) - .instrument(tracing::debug_span!("http", ?request_id)) + .handle_request( req) .await } }); @@ -269,7 +266,6 @@ where /// handle HTTP request. async fn handle_request( &self, - request_id: SmolStr, mut request: hyper::Request, ) -> Result> where @@ -291,7 +287,8 @@ where Ok(app_name) => app_name, }; - let span = tracing::info_span!("handle", app = %app_name); + let traceparent = remote_traceparent(&request); + let span = tracing::info_span!("handle", app = %app_name, traceparent = %traceparent); let _enter = span.enter(); // lookup for application config and binary_id @@ -359,7 +356,7 @@ where } }; - let stats = self.context.new_stats_row(&request_id, &app_name, &cfg); + let stats = self.context.new_stats_row(&traceparent, &app_name, &cfg); let response = match executor .execute(request, stats.clone()) @@ -383,7 +380,7 @@ where let (status_code, fail_reason, msg, internal_code) = map_err(error); stats.status_code(status_code); stats.fail_reason(fail_reason as i32); - tracing::debug!(?fail_reason, ?request_id, "stats"); + tracing::debug!(?fail_reason, ?traceparent, "stats"); #[cfg(feature = "metrics")] metrics::metrics( @@ -487,7 +484,11 @@ fn map_err(error: Error) -> (u16, AppResult, HyperOutgoingBody, u16) { (status_code, fail_reason, msg, internal_code) } -fn remote_traceparent(req: &hyper::Request) -> SmolStr { +fn remote_traceparent(req: &hyper::Request) -> SmolStr +where + B: BodyExt + Send, + ::Data: Send, +{ req.headers() .get(TRACEPARENT) .and_then(|v| v.to_str().ok()) @@ -669,8 +670,8 @@ pub(crate) mod signal { mod tests { use test_case::test_case; - use crate::AppName; use crate::app_name_from_request; + use crate::AppName; use bytes::Bytes; use claims::{assert_err, assert_ok}; use http_backend::SERVER_NAME_HEADER; From e69b6dfd7b9a8a53f90b7c96a523885c3d9d1930 Mon Sep 17 00:00:00 2001 From: Ruslan Pislari Date: Mon, 11 May 2026 13:34:46 +0300 Subject: [PATCH 2/2] refactor: improve header handling and tracing consistency - Reordered and cleaned up imports across modules for readability. - Added `traceparent` to tracing logs for enhanced observability. - Adjusted `handle_request` tracing span to use `http` for clarity. - Fixed formatting issues in `handle_request`. - Replaced `SERVER_NAME` with `SERVER_NAME_HEADER` for hostname resolution consistency. - Streamlined `remote_traceparent` method signature. - Updated tests to reflect changes in import order. --- crates/http-service/src/executor/wasi_http.rs | 15 ++++++------ crates/http-service/src/lib.rs | 24 ++++++++----------- 2 files changed, 17 insertions(+), 22 deletions(-) diff --git a/crates/http-service/src/executor/wasi_http.rs b/crates/http-service/src/executor/wasi_http.rs index 3c3e219..56d56bc 100644 --- a/crates/http-service/src/executor/wasi_http.rs +++ b/crates/http-service/src/executor/wasi_http.rs @@ -4,19 +4,19 @@ use std::time::Duration; use crate::executor; use crate::executor::HttpExecutor; use crate::state::HttpState; -use anyhow::{anyhow, bail, Context}; +use ::http::{HeaderMap, Request, Response, Uri, header}; +use anyhow::{Context, anyhow, bail}; use async_trait::async_trait; -use ::http::{header, HeaderMap, Request, Response, Uri}; -use http_backend::Backend; +use http_backend::{Backend, SERVER_NAME_HEADER}; use http_body_util::{BodyExt, Full}; use hyper::body::Body; use runtime::util::stats::{StatsTimer, StatsVisitor}; -use runtime::{store::StoreBuilder, InstancePre}; +use runtime::{InstancePre, store::StoreBuilder}; use smol_str::SmolStr; use tracing::Instrument; -use wasmtime_wasi_http::bindings::http::types::Scheme; use wasmtime_wasi_http::bindings::ProxyPre; -use wasmtime_wasi_http::{body::HyperOutgoingBody, WasiHttpView}; +use wasmtime_wasi_http::bindings::http::types::Scheme; +use wasmtime_wasi_http::{WasiHttpView, body::HyperOutgoingBody}; /// Execute context used by ['HttpService'] #[derive(Clone)] @@ -47,7 +47,6 @@ where let (mut parts, body) = req.into_parts(); const LOCALHOST: SmolStr = SmolStr::new_inline("localhost"); - const SERVER_NAME: &'static str = "server_name"; // Resolve backend hostname using the following precedence: // 1. `server_name` request header (if set and valid UTF-8) @@ -55,7 +54,7 @@ where // 3. fallback to "localhost" let backend_hostname: SmolStr = parts .headers - .get(SERVER_NAME) + .get(SERVER_NAME_HEADER) .and_then(|v| v.to_str().ok()) .map(SmolStr::from) .or_else(|| self.backend.hostname()) diff --git a/crates/http-service/src/lib.rs b/crates/http-service/src/lib.rs index d66adaf..7eb4bd3 100644 --- a/crates/http-service/src/lib.rs +++ b/crates/http-service/src/lib.rs @@ -8,11 +8,11 @@ use wasmtime_wasi_nn::wit::WasiNnView; pub use crate::executor::ExecutorFactory; use crate::executor::HttpExecutor; -use anyhow::{bail, Context, Error, Result}; +use anyhow::{Context, Error, Result, bail}; use bytes::Bytes; use http::{ - header::{ACCESS_CONTROL_ALLOW_ORIGIN, CACHE_CONTROL}, HeaderMap, HeaderName, HeaderValue, - StatusCode, + HeaderMap, HeaderName, HeaderValue, StatusCode, + header::{ACCESS_CONTROL_ALLOW_ORIGIN, CACHE_CONTROL}, }; use http_backend::SERVER_NAME_HEADER; use http_body_util::{BodyExt, Empty, Full}; @@ -22,7 +22,7 @@ use hyper_util::{client::legacy::connect::Connect, rt::TokioIo}; use runtime::util::metrics; use runtime::util::stats::StatsVisitor; use runtime::{ - app::Status, service::Service, App, AppResult, ContextT, Router, WasmEngine, WasmEngineBuilder, + App, AppResult, ContextT, Router, WasmEngine, WasmEngineBuilder, app::Status, service::Service, }; use smol_str::{SmolStr, ToSmolStr}; use state::HttpState; @@ -156,7 +156,7 @@ where let self_ = connection.clone(); async move { self_ - .handle_request( req) + .handle_request(req) .await } }); @@ -272,6 +272,7 @@ where B: BodyExt + Send, ::Data: Send, { + let traceparent = remote_traceparent(&request); request .headers_mut() .extend(app_req_headers(self.context.append_headers())); @@ -281,14 +282,13 @@ where Err(error) => { #[cfg(feature = "metrics")] metrics::metrics(AppResult::UNKNOWN, HTTP_LABEL, None, None); - tracing::info!(cause=?error, "App name not provided"); + tracing::info!(cause=?error, traceparent = %traceparent, "App name not provided"); return not_found(); } Ok(app_name) => app_name, }; - let traceparent = remote_traceparent(&request); - let span = tracing::info_span!("handle", app = %app_name, traceparent = %traceparent); + let span = tracing::info_span!("http", app = %app_name, traceparent = %traceparent); let _enter = span.enter(); // lookup for application config and binary_id @@ -484,11 +484,7 @@ fn map_err(error: Error) -> (u16, AppResult, HyperOutgoingBody, u16) { (status_code, fail_reason, msg, internal_code) } -fn remote_traceparent(req: &hyper::Request) -> SmolStr -where - B: BodyExt + Send, - ::Data: Send, -{ +fn remote_traceparent(req: &hyper::Request) -> SmolStr { req.headers() .get(TRACEPARENT) .and_then(|v| v.to_str().ok()) @@ -670,8 +666,8 @@ pub(crate) mod signal { mod tests { use test_case::test_case; - use crate::app_name_from_request; use crate::AppName; + use crate::app_name_from_request; use bytes::Bytes; use claims::{assert_err, assert_ok}; use http_backend::SERVER_NAME_HEADER;