Skip to content

Commit

Permalink
chore(volo-http): refactor server and remove common stats (#411)
Browse files Browse the repository at this point in the history
The original server implementation was not elegant enough, so this
commit restructured it.

Additionally, we noticed that common stats and stat tracer could be
replaced with custom layers, so we removed those as well.

Signed-off-by: Yu Li <liyu.yukiteru@bytedance.com>
  • Loading branch information
wfly1998 committed Apr 9, 2024
1 parent d48d7cd commit 6f11f75
Show file tree
Hide file tree
Showing 8 changed files with 174 additions and 356 deletions.
17 changes: 1 addition & 16 deletions examples/src/http/example-http-server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -307,17 +307,6 @@ async fn headers_map_response(response: ServerResponse) -> impl IntoResponse {
)
}

fn tracer(cx: &ServerContext) {
tracing::info!(
"process start at {:?}, end at {:?}, req size: {:?}, resp size: {:?}, resp status: {:?}",
cx.common_stats.process_start_at().unwrap(),
cx.common_stats.process_end_at().unwrap(),
cx.common_stats.req_size().unwrap_or(&0),
cx.common_stats.resp_size().unwrap_or(&0),
cx.common_stats.status_code().unwrap(),
);
}

#[volo::main]
async fn main() {
let subscriber = tracing_subscriber::FmtSubscriber::builder()
Expand All @@ -343,9 +332,5 @@ async fn main() {

println!("Listening on {addr}");

Server::new(app)
.stat_tracer(tracer)
.run(addr)
.await
.unwrap();
Server::new(app).run(addr).await.unwrap();
}
17 changes: 0 additions & 17 deletions volo-http/src/client/meta.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,28 +49,11 @@ where
}
}

let stat_enabled = cx.stat_enabled();

if stat_enabled {
if let Some(req_size) = exact_len {
cx.common_stats.set_req_size(req_size);
}
}

tracing::trace!("sending request: {} {}", req.method(), req.uri());
tracing::trace!("headers: {:?}", req.headers());

let res = self.inner.call(cx, req).await;

if stat_enabled {
if let Ok(response) = res.as_ref() {
cx.common_stats.set_status_code(response.status());
if let Some(resp_size) = response.size_hint().exact() {
cx.common_stats.set_resp_size(resp_size);
}
}
}

if !cx.rpc_info().config().fail_on_error_status {
return res;
}
Expand Down
20 changes: 3 additions & 17 deletions volo-http/src/client/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -708,27 +708,13 @@ where
req.headers_mut().extend(self.inner.headers.clone());

let has_metainfo = METAINFO.try_with(|_| {}).is_ok();
let stat_enabled = cx.stat_enabled();

let mk_call = async {
if stat_enabled {
cx.common_stats.record_process_start_at();
}

let res = self.service.call(cx, req).await;

if stat_enabled {
cx.common_stats.record_process_end_at();
}
res
};
let fut = self.service.call(cx, req);

if has_metainfo {
mk_call.await
fut.await
} else {
METAINFO
.scope(RefCell::new(MetaInfo::default()), mk_call)
.await
METAINFO.scope(RefCell::new(MetaInfo::default()), fut).await
}
}
}
Expand Down
7 changes: 1 addition & 6 deletions volo-http/src/context/client.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,11 @@
use chrono::{DateTime, Local};
use faststr::FastStr;
use paste::paste;
use volo::{
context::{Context, Reusable, Role, RpcCx, RpcInfo},
newtype_impl_context,
};

use super::CommonStats;
use crate::utils::macros::impl_deref_and_deref_mut;
use crate::utils::macros::{impl_deref_and_deref_mut, stat_impl};

#[derive(Debug)]
pub struct ClientContext(pub(crate) RpcCx<ClientCxInner, Config>);
Expand All @@ -21,7 +19,6 @@ impl ClientContext {
#[cfg(feature = "__tls")]
tls,
stats: ClientStats::default(),
common_stats: CommonStats::default(),
},
))
}
Expand All @@ -46,8 +43,6 @@ pub struct ClientCxInner {

/// This is unstable now and may be changed in the future.
pub stats: ClientStats,
/// This is unstable now and may be changed in the future.
pub common_stats: CommonStats,
}

impl ClientCxInner {
Expand Down
79 changes: 0 additions & 79 deletions volo-http/src/context/mod.rs
Original file line number Diff line number Diff line change
@@ -1,51 +1,3 @@
use chrono::{DateTime, Local};
use http::{Method, StatusCode, Uri};
use paste::paste;

// This macro is unused only when both `client` and `server` features are not enabled.
// But no one can use this crate without any of them, maybe :)
#[allow(unused_macros)]
macro_rules! stat_impl {
($t: ident) => {
paste! {
/// This is unstable now and may be changed in the future.
#[inline]
pub fn $t(&self) -> Option<DateTime<Local>> {
self.$t
}

/// This is unstable now and may be changed in the future.
#[doc(hidden)]
#[inline]
pub fn [<set_$t>](&mut self, t: DateTime<Local>) {
self.$t = Some(t)
}

/// This is unstable now and may be changed in the future.
#[inline]
pub fn [<record_ $t>](&mut self) {
self.$t = Some(Local::now())
}
}
};
}

macro_rules! stat_impl_getter_and_setter {
($name: ident, $type: ty) => {
paste! {
#[inline]
pub fn $name(&self) -> Option<&$type> {
self.$name.as_ref()
}

#[inline]
pub fn [<set_ $name>](&mut self, t: $type) {
self.$name = Some(t)
}
}
};
}

#[cfg(feature = "client")]
pub mod client;

Expand All @@ -57,34 +9,3 @@ pub mod server;

#[cfg(feature = "server")]
pub use self::server::{RequestPartsExt, ServerContext};

/// This is unstable now and may be changed in the future.
#[derive(Debug, Default, Clone)]
pub struct CommonStats {
process_start_at: Option<DateTime<Local>>,
process_end_at: Option<DateTime<Local>>,

method: Option<Method>,
uri: Option<Uri>,
status_code: Option<StatusCode>,

req_size: Option<u64>,
resp_size: Option<u64>,
}

impl CommonStats {
stat_impl!(process_start_at);
stat_impl!(process_end_at);

stat_impl_getter_and_setter!(method, Method);
stat_impl_getter_and_setter!(uri, Uri);
stat_impl_getter_and_setter!(status_code, StatusCode);

stat_impl_getter_and_setter!(req_size, u64);
stat_impl_getter_and_setter!(resp_size, u64);

#[inline]
pub fn reset(&mut self) {
*self = Self { ..Self::default() }
}
}
39 changes: 3 additions & 36 deletions volo-http/src/context/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,12 @@ use http::{
request::Parts,
uri::{Authority, PathAndQuery, Scheme, Uri},
};
use paste::paste;
use volo::{
context::{Context, Reusable, Role, RpcCx, RpcInfo},
net::Address,
newtype_impl_context,
};

use super::CommonStats;
use crate::{
server::param::Params,
utils::{
Expand All @@ -29,21 +27,11 @@ impl ServerContext {
RpcInfo::<Config>::with_role(Role::Server),
ServerCxInner {
params: Params::default(),
stats: ServerStats::default(),
common_stats: CommonStats::default(),
},
);
cx.rpc_info_mut().caller_mut().set_address(peer);
Self(cx)
}

pub fn enable_stat(&mut self, enable: bool) {
self.rpc_info_mut().config_mut().stat_enable = enable;
}

pub(crate) fn stat_enabled(&self) -> bool {
self.rpc_info().config().stat_enable
}
}

impl_deref_and_deref_mut!(ServerContext, RpcCx<ServerCxInner, Config>, 0);
Expand All @@ -53,38 +41,17 @@ newtype_impl_context!(ServerContext, Config, 0);
#[derive(Clone, Debug)]
pub struct ServerCxInner {
pub params: Params,

/// This is unstable now and may be changed in the future.
pub stats: ServerStats,
/// This is unstable now and may be changed in the future.
pub common_stats: CommonStats,
}

impl ServerCxInner {
impl_getter!(params, Params);
}

/// This is unstable now and may be changed in the future.
#[derive(Debug, Default, Clone)]
pub struct ServerStats {}

impl ServerStats {}

#[derive(Debug, Clone)]
pub struct Config {
pub(crate) stat_enable: bool,
}

impl Default for Config {
fn default() -> Self {
Self { stat_enable: true }
}
}
#[derive(Clone, Debug, Default)]
pub struct Config {}

impl Reusable for Config {
fn clear(&mut self) {
self.stat_enable = true;
}
fn clear(&mut self) {}
}

pub trait RequestPartsExt {
Expand Down
Loading

0 comments on commit 6f11f75

Please sign in to comment.