Skip to content
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

chore(volo-http): optimize user experience #483

Merged
merged 2 commits into from
Aug 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion volo-http/Cargo.toml
Original file line number Diff line number Diff line change
@@ -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
Expand Down
2 changes: 1 addition & 1 deletion volo-http/src/client/loadbalance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
}
Expand Down
85 changes: 70 additions & 15 deletions volo-http/src/client/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<ClientTransport>;
/// Default [`Client`] without any extra [`Layer`]s
pub type DefaultClient = Client<DefaultLBService<ClientMetaService>>;
pub type DefaultClient<IL = Identity, OL = Identity> =
Client<<OL as Layer<DefaultLBService<<IL as Layer<ClientMetaService>>::Service>>>::Service>;

/// A builder for configuring an HTTP [`Client`].
pub struct ClientBuilder<IL, OL, C, LB> {
Expand All @@ -88,7 +89,11 @@ pub struct ClientBuilder<IL, OL, C, LB> {
tls_config: Option<volo::net::tls::TlsConnector>,
}

struct BuilderConfig {
/// Configuration for [`ClientBuilder`]
///
/// This is unstable now and may be changed in the future.
#[doc(hidden)]
pub struct BuilderConfig {
timeout: Option<Duration>,
stat_enable: bool,
fail_on_error_status: bool,
Expand Down Expand Up @@ -511,16 +516,26 @@ impl<IL, OL, C, LB> ClientBuilder<IL, OL, C, LB> {
&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 {
Expand Down Expand Up @@ -590,29 +605,29 @@ impl<IL, OL, C, LB> ClientBuilder<IL, OL, C, LB> {
}

/// 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<Duration>) -> &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<Duration>) -> &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<Duration>) -> &mut Self {
self.connector.set_write_timeout(timeout);
self
}

/// Set the maximin idle time for the request.
///
/// 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<Duration>) -> &mut Self {
self.builder_config.timeout = timeout;
self
}

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -956,7 +972,46 @@ mod client_tests {
const USER_AGENT_VAL: &str = "volo-http-unit-test";

fn client_types_check() {
struct TestLayer;
struct TestService<S> {
inner: S,
}

impl<S> Layer<S> for TestLayer {
type Service = TestService<S>;

fn layer(self, inner: S) -> Self::Service {
TestService { inner }
}
}

impl<S, Cx, Req> Service<Cx, Req> for TestService<S>
where
S: Service<Cx, Req>,
{
type Response = S::Response;
type Error = S::Error;

fn call(
&self,
cx: &mut Cx,
req: Req,
) -> impl Future<Output = Result<Self::Response, Self::Error>> + Send {
self.inner.call(cx, req)
}
}

let _: DefaultClient = ClientBuilder::new().build();
let _: DefaultClient<TestLayer> = ClientBuilder::new().layer_inner(TestLayer).build();
let _: DefaultClient<TestLayer> = ClientBuilder::new().layer_inner_front(TestLayer).build();
let _: DefaultClient<Identity, TestLayer> =
ClientBuilder::new().layer_outer(TestLayer).build();
let _: DefaultClient<Identity, TestLayer> =
ClientBuilder::new().layer_outer_front(TestLayer).build();
let _: DefaultClient<TestLayer, TestLayer> = ClientBuilder::new()
.layer_inner(TestLayer)
.layer_outer(TestLayer)
.build();
}

#[tokio::test]
Expand Down
1 change: 0 additions & 1 deletion volo-http/src/client/transport.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion volo-http/src/server/layer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
}
Expand Down
25 changes: 12 additions & 13 deletions volo-http/src/server/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand Down Expand Up @@ -274,7 +273,7 @@ impl<S, L> Server<S, L> {
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));
Expand Down Expand Up @@ -322,15 +321,15 @@ impl<S, L> Server<S, L> {
}

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;
}
}

// 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.
Expand All @@ -345,9 +344,9 @@ impl<S, L> Server<S, L> {
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;
}
Expand Down Expand Up @@ -389,7 +388,7 @@ async fn serve<I, S, E>(
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;
}
};
Expand All @@ -401,11 +400,11 @@ async fn serve<I, S, E>(

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;
}
};
Expand Down Expand Up @@ -449,20 +448,20 @@ async fn serve_conn<S>(

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)
);
// 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);
}
},
}
Expand Down
4 changes: 2 additions & 2 deletions volo-http/src/server/utils/serve_dir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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());
}

Expand Down
Loading