From fe68dcd09c712b6d551e707c15769f7253d5d6b7 Mon Sep 17 00:00:00 2001 From: Yukiteru Date: Fri, 9 Aug 2024 15:10:03 +0800 Subject: [PATCH] chore(volo-http): optimize user experience (#483) This commit unified log format in `volo-http`, adjusted the timeout interface and add generic types for `DefaultClient`. Signed-off-by: Yu Li --- Cargo.lock | 2 +- volo-http/Cargo.toml | 2 +- volo-http/src/client/loadbalance.rs | 2 +- volo-http/src/client/mod.rs | 85 ++++++++++++++++++++----- volo-http/src/client/transport.rs | 1 - volo-http/src/server/layer.rs | 2 +- volo-http/src/server/mod.rs | 25 ++++---- volo-http/src/server/utils/serve_dir.rs | 4 +- 8 files changed, 88 insertions(+), 35 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 524857b7..7f21560d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3680,7 +3680,7 @@ dependencies = [ [[package]] name = "volo-http" -version = "0.2.11" +version = "0.2.12" dependencies = [ "ahash", "async-broadcast", diff --git a/volo-http/Cargo.toml b/volo-http/Cargo.toml index bab4953d..61fd951b 100644 --- a/volo-http/Cargo.toml +++ b/volo-http/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "volo-http" -version = "0.2.11" +version = "0.2.12" edition.workspace = true homepage.workspace = true repository.workspace = true diff --git a/volo-http/src/client/loadbalance.rs b/volo-http/src/client/loadbalance.rs index f892d06d..06a89823 100644 --- a/volo-http/src/client/loadbalance.rs +++ b/volo-http/src/client/loadbalance.rs @@ -136,7 +136,7 @@ where match channel.recv().await { Ok(recv) => lb.rebalance(recv), Err(err) => { - tracing::warn!("[VOLO] discovering subscription error: {:?}", err) + tracing::warn!("[Volo-HTTP] discovering subscription error: {:?}", err) } } } diff --git a/volo-http/src/client/mod.rs b/volo-http/src/client/mod.rs index abec918e..b2ceb6db 100644 --- a/volo-http/src/client/mod.rs +++ b/volo-http/src/client/mod.rs @@ -67,7 +67,8 @@ const PKG_NAME_WITH_VER: &str = concat!(env!("CARGO_PKG_NAME"), '/', env!("CARGO /// Default inner service of [`Client`] pub type ClientMetaService = MetaService; /// Default [`Client`] without any extra [`Layer`]s -pub type DefaultClient = Client>; +pub type DefaultClient = + Client<
    >::Service>>>::Service>; /// A builder for configuring an HTTP [`Client`]. pub struct ClientBuilder { @@ -88,7 +89,11 @@ pub struct ClientBuilder { tls_config: Option, } -struct BuilderConfig { +/// Configuration for [`ClientBuilder`] +/// +/// This is unstable now and may be changed in the future. +#[doc(hidden)] +pub struct BuilderConfig { timeout: Option, stat_enable: bool, fail_on_error_status: bool, @@ -511,16 +516,26 @@ impl ClientBuilder { &mut self.headers } - /// Get a reference to the HTTP configuration of the client. - pub fn http_config(&self) -> &ClientConfig { + /// Get a reference to HTTP configuration of the client. + pub fn http_config_ref(&self) -> &ClientConfig { &self.http_config } - /// Get a mutable reference to the HTTP configuration of the client. + /// Get a mutable reference to HTTP configuration of the client. pub fn http_config_mut(&mut self) -> &mut ClientConfig { &mut self.http_config } + /// Get a reference to builder configuration of the client. + pub fn builder_config_ref(&self) -> &BuilderConfig { + &self.builder_config + } + + /// Get a mutable reference to builder configuration of the client. + pub fn builder_config_mut(&mut self) -> &mut BuilderConfig { + &mut self.builder_config + } + /// This is unstable now and may be changed in the future. #[doc(hidden)] pub fn stat_enable(&mut self, enable: bool) -> &mut Self { @@ -590,20 +605,20 @@ impl ClientBuilder { } /// Set the maximum idle time for a connection. - pub fn set_connect_timeout(&mut self, timeout: Duration) -> &mut Self { - self.connector.set_connect_timeout(Some(timeout)); + pub fn set_connect_timeout(&mut self, timeout: Option) -> &mut Self { + self.connector.set_connect_timeout(timeout); self } /// Set the maximum idle time for reading data from the connection. - pub fn set_read_timeout(&mut self, timeout: Duration) -> &mut Self { - self.connector.set_read_timeout(Some(timeout)); + pub fn set_read_timeout(&mut self, timeout: Option) -> &mut Self { + self.connector.set_read_timeout(timeout); self } /// Set the maximum idle time for writing data to the connection. - pub fn set_write_timeout(&mut self, timeout: Duration) -> &mut Self { - self.connector.set_write_timeout(Some(timeout)); + pub fn set_write_timeout(&mut self, timeout: Option) -> &mut Self { + self.connector.set_write_timeout(timeout); self } @@ -611,8 +626,8 @@ impl ClientBuilder { /// /// The whole request includes connecting, writting, and reading the whole HTTP protocol /// headers (without reading response body). - pub fn set_request_timeout(&mut self, timeout: Duration) -> &mut Self { - self.builder_config.timeout = Some(timeout); + pub fn set_request_timeout(&mut self, timeout: Option) -> &mut Self { + self.builder_config.timeout = timeout; self } @@ -924,11 +939,12 @@ where mod client_tests { #![allow(unused)] - use std::collections::HashMap; + use std::{collections::HashMap, future::Future}; use http::{header, StatusCode}; + use motore::{layer::Layer, service::Service}; use serde::Deserialize; - use volo::context::Endpoint; + use volo::{context::Endpoint, layer::Identity}; use super::{ callopt::CallOpt, @@ -956,7 +972,46 @@ mod client_tests { const USER_AGENT_VAL: &str = "volo-http-unit-test"; fn client_types_check() { + struct TestLayer; + struct TestService { + inner: S, + } + + impl Layer for TestLayer { + type Service = TestService; + + fn layer(self, inner: S) -> Self::Service { + TestService { inner } + } + } + + impl Service for TestService + where + S: Service, + { + type Response = S::Response; + type Error = S::Error; + + fn call( + &self, + cx: &mut Cx, + req: Req, + ) -> impl Future> + Send { + self.inner.call(cx, req) + } + } + let _: DefaultClient = ClientBuilder::new().build(); + let _: DefaultClient = ClientBuilder::new().layer_inner(TestLayer).build(); + let _: DefaultClient = ClientBuilder::new().layer_inner_front(TestLayer).build(); + let _: DefaultClient = + ClientBuilder::new().layer_outer(TestLayer).build(); + let _: DefaultClient = + ClientBuilder::new().layer_outer_front(TestLayer).build(); + let _: DefaultClient = ClientBuilder::new() + .layer_inner(TestLayer) + .layer_outer(TestLayer) + .build(); } #[tokio::test] diff --git a/volo-http/src/client/transport.rs b/volo-http/src/client/transport.rs index a1b4195e..762d15eb 100644 --- a/volo-http/src/client/transport.rs +++ b/volo-http/src/client/transport.rs @@ -94,7 +94,6 @@ impl ClientTransport { volo::net::conn::ConnStream::Tcp(tcp_stream) => tcp_stream, _ => unreachable!(), }; - println!("target_name: {target_name}"); self.tls_connector .connect(target_name, tcp_stream) .await diff --git a/volo-http/src/server/layer.rs b/volo-http/src/server/layer.rs index ec2b6c15..1ab7eec0 100644 --- a/volo-http/src/server/layer.rs +++ b/volo-http/src/server/layer.rs @@ -121,7 +121,7 @@ where Ok(Err(res)) => Ok(res.into_response()), // something wrong while extracting Err(rej) => { - tracing::warn!("[VOLO] FilterLayer: something wrong while extracting"); + tracing::warn!("[Volo-HTTP] FilterLayer: something wrong while extracting"); Ok(rej.into_response()) } } diff --git a/volo-http/src/server/mod.rs b/volo-http/src/server/mod.rs index 56959031..7a622b94 100644 --- a/volo-http/src/server/mod.rs +++ b/volo-http/src/server/mod.rs @@ -24,7 +24,6 @@ use motore::{ use parking_lot::RwLock; use scopeguard::defer; use tokio::sync::Notify; -use tracing::{info, trace}; #[cfg(feature = "__tls")] use volo::net::{conn::ConnStream, tls::Acceptor, tls::ServerTlsConfig}; use volo::{ @@ -274,7 +273,7 @@ impl Server { let server = Arc::new(self.server); let service = Arc::new(self.layer.layer(self.service)); let incoming = mk_incoming.make_incoming().await?; - info!("[VOLO] server start at: {:?}", incoming); + tracing::info!("[Volo-HTTP] server start at: {:?}", incoming); // count connections, used for graceful shutdown let conn_cnt = Arc::new(AtomicUsize::new(0)); @@ -322,7 +321,7 @@ impl Server { } if !self.shutdown_hooks.is_empty() { - info!("[VOLO] call shutdown hooks"); + tracing::info!("[Volo-HTTP] call shutdown hooks"); for hook in self.shutdown_hooks { (hook)().await; @@ -330,7 +329,7 @@ impl Server { } // received signal, graceful shutdown now - info!("[VOLO] received signal, gracefully exiting now"); + tracing::info!("[Volo-HTTP] received signal, gracefully exiting now"); *exit_flag.write() = true; // Now we won't accept new connections. @@ -345,9 +344,9 @@ impl Server { if conn_cnt.load(Ordering::Relaxed) == 0 { break; } - trace!( - "[VOLO] gracefully exiting, remaining connection count: {}", - conn_cnt.load(Ordering::Relaxed) + tracing::trace!( + "[Volo-HTTP] gracefully exiting, remaining connection count: {}", + conn_cnt.load(Ordering::Relaxed), ); tokio::time::sleep(Duration::from_secs(1)).await; } @@ -389,7 +388,7 @@ async fn serve( let stream = match tls_config.acceptor.accept(stream).await { Ok(conn) => conn, Err(err) => { - trace!("[VOLO] tls handshake error: {err:?}"); + tracing::trace!("[Volo-HTTP] tls handshake error: {err:?}"); continue; } }; @@ -401,11 +400,11 @@ async fn serve( let peer = match conn.info.peer_addr { Some(ref peer) => { - trace!(" accept connection from: {peer:?}"); + tracing::trace!("accept connection from: {peer:?}"); peer.clone() } None => { - info!("no peer address found from server connection"); + tracing::info!("no peer address found from server connection"); continue; } }; @@ -449,7 +448,7 @@ async fn serve_conn( tokio::select! { _ = &mut notified => { - tracing::trace!("[VOLO] closing a pending connection"); + tracing::trace!("[Volo-HTTP] closing a pending connection"); // Graceful shutdown. hyper::server::conn::http1::UpgradeableConnection::graceful_shutdown( Pin::new(&mut http_conn) @@ -457,12 +456,12 @@ async fn serve_conn( // Continue to poll this connection until shutdown can finish. let result = http_conn.await; if let Err(err) = result { - tracing::debug!("[VOLO] connection error: {:?}", err); + tracing::debug!("[Volo-HTTP] connection error: {:?}", err); } } result = &mut http_conn => { if let Err(err) = result { - tracing::debug!("[VOLO] connection error: {:?}", err); + tracing::debug!("[Volo-HTTP] connection error: {:?}", err); } }, } diff --git a/volo-http/src/server/utils/serve_dir.rs b/volo-http/src/server/utils/serve_dir.rs index 7da71452..c1207561 100644 --- a/volo-http/src/server/utils/serve_dir.rs +++ b/volo-http/src/server/utils/serve_dir.rs @@ -93,7 +93,7 @@ where let path = req.uri().path(); let path = path.strip_prefix('/').unwrap_or(path); - tracing::trace!("ServeDir: path: {path}"); + tracing::trace!("[Volo-HTTP] ServeDir: path: {path}"); // Join to the serving directory and canonicalize it let path = self.path.join(path); @@ -103,7 +103,7 @@ where // Reject file which is out of the serving directory if path.strip_prefix(self.path.as_path()).is_err() { - tracing::debug!("ServeDir: illegal path: {}", path.display()); + tracing::debug!("[Volo-HTTP] ServeDir: illegal path: {}", path.display()); return Ok(StatusCode::FORBIDDEN.into_response()); }