Skip to content

Commit

Permalink
revert changes to metrics
Browse files Browse the repository at this point in the history
  • Loading branch information
smatthewenglish committed Jul 12, 2024
1 parent 30662f1 commit 0ca715c
Show file tree
Hide file tree
Showing 6 changed files with 35 additions and 52 deletions.
2 changes: 0 additions & 2 deletions crates/rpc/rpc-builder/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,6 @@ reth-tracing.workspace = true
reth-transaction-pool = { workspace = true, features = ["test-utils"] }
reth-tokio-util.workspace = true

reth-metrics = { workspace = true, features = ["common"] }
metrics.workspace = true
tokio = { workspace = true, features = ["rt", "rt-multi-thread"] }
serde_json.workspace = true
clap = { workspace = true, features = ["derive"] }
31 changes: 23 additions & 8 deletions crates/rpc/rpc-builder/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -208,8 +208,7 @@ pub use eth::{
};

// Rpc server metrics
#[allow(missing_docs)]
pub mod metrics;
mod metrics;

/// Convenience function for starting a server in one step.
#[allow(clippy::too_many_arguments)]
Expand Down Expand Up @@ -1142,6 +1141,7 @@ pub struct RpcServerConfig<RpcMiddleware = Identity> {
/// JWT secret for authentication
jwt_secret: Option<JwtSecret>,
/// Configurable RPC middleware
#[allow(dead_code)]
rpc_middleware: RpcServiceBuilder<RpcMiddleware>,
}

Expand All @@ -1165,7 +1165,7 @@ impl Default for RpcServerConfig<Identity> {
}
}

impl RpcServerConfig<Identity> {
impl RpcServerConfig {
/// Creates a new config with only http set
pub fn http(config: ServerBuilder<Identity, Identity>) -> Self {
Self::default().with_http(config)
Expand Down Expand Up @@ -1395,7 +1395,16 @@ impl<RpcMiddleware> RpcServerConfig<RpcMiddleware> {
.option_layer(Self::maybe_cors_layer(cors)?)
.option_layer(Self::maybe_jwt_layer(self.jwt_secret)),
)
.set_rpc_middleware(self.rpc_middleware)
.set_rpc_middleware(
RpcServiceBuilder::new().layer(
modules
.http
.as_ref()
.or(modules.ws.as_ref())
.map(RpcRequestMetrics::same_port)
.unwrap_or_default(),
),
)
.build(http_socket_addr)
.await
.map_err(|err| {
Expand Down Expand Up @@ -1425,7 +1434,6 @@ impl<RpcMiddleware> RpcServerConfig<RpcMiddleware> {
let mut ws_server = None;
let mut http_local_addr = None;
let mut http_server = None;
let rpc_middleware_clone = self.rpc_middleware.clone();

if let Some(builder) = self.ws_server_config {
let server = builder
Expand All @@ -1435,7 +1443,10 @@ impl<RpcMiddleware> RpcServerConfig<RpcMiddleware> {
.option_layer(Self::maybe_cors_layer(self.ws_cors_domains.clone())?)
.option_layer(Self::maybe_jwt_layer(self.jwt_secret)),
)
.set_rpc_middleware(rpc_middleware_clone.clone())
.set_rpc_middleware(
RpcServiceBuilder::new()
.layer(modules.ws.as_ref().map(RpcRequestMetrics::ws).unwrap_or_default()),
)
.build(ws_socket_addr)
.await
.map_err(|err| RpcError::server_error(err, ServerKind::WS(ws_socket_addr)))?;
Expand All @@ -1456,7 +1467,11 @@ impl<RpcMiddleware> RpcServerConfig<RpcMiddleware> {
.option_layer(Self::maybe_cors_layer(self.http_cors_domains.clone())?)
.option_layer(Self::maybe_jwt_layer(self.jwt_secret)),
)
.set_rpc_middleware(rpc_middleware_clone)
.set_rpc_middleware(
RpcServiceBuilder::new().layer(
modules.http.as_ref().map(RpcRequestMetrics::http).unwrap_or_default(),
),
)
.build(http_socket_addr)
.await
.map_err(|err| RpcError::server_error(err, ServerKind::Http(http_socket_addr)))?;
Expand Down Expand Up @@ -1623,7 +1638,7 @@ pub struct TransportRpcModules<Context = ()> {
/// The original config
config: TransportRpcModuleConfig,
/// rpcs module for http
pub http: Option<RpcModule<Context>>,
http: Option<RpcModule<Context>>,
/// rpcs module for ws
ws: Option<RpcModule<Context>>,
/// rpcs module for ipc
Expand Down
33 changes: 10 additions & 23 deletions crates/rpc/rpc-builder/src/metrics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,22 +21,12 @@ use tower::Layer;
/// - Request metrics: metrics for each RPC method (e.g. number of calls started, time taken to
/// process a call)
#[derive(Default, Debug, Clone)]
#[allow(unreachable_pub)]
pub struct RpcRequestMetrics {
pub(crate) struct RpcRequestMetrics {
inner: Arc<RpcServerMetricsInner>,
}

#[allow(dead_code, unreachable_pub)]
impl RpcRequestMetrics {
/// Creates a new `RpcRequestMetrics`.
///
/// # Parameters
/// - `module`: The RPC module.
/// - `transport`: The transport protocol.
///
/// # Returns
/// A new `RpcRequestMetrics` instance.
pub fn new(module: &RpcModule<()>, transport: RpcTransport) -> Self {
pub(crate) fn new(module: &RpcModule<()>, transport: RpcTransport) -> Self {
Self {
inner: Arc::new(RpcServerMetricsInner {
connection_metrics: transport.connection_metrics(),
Expand All @@ -48,25 +38,25 @@ impl RpcRequestMetrics {
}

/// Creates a new instance of the metrics layer for HTTP.
pub fn http(module: &RpcModule<()>) -> Self {
pub(crate) fn http(module: &RpcModule<()>) -> Self {
Self::new(module, RpcTransport::Http)
}

/// Creates a new instance of the metrics layer for same port.
///
/// Note: currently it's not possible to track transport specific metrics for a server that runs http and ws on the same port: <https://github.com/paritytech/jsonrpsee/issues/1345> until we have this feature we will use the http metrics for this case.
pub fn same_port(module: &RpcModule<()>) -> Self {
pub(crate) fn same_port(module: &RpcModule<()>) -> Self {
Self::http(module)
}

/// Creates a new instance of the metrics layer for Ws.
pub fn ws(module: &RpcModule<()>) -> Self {
pub(crate) fn ws(module: &RpcModule<()>) -> Self {
Self::new(module, RpcTransport::WebSocket)
}

/// Creates a new instance of the metrics layer for Ws.
#[allow(unused)]
pub fn ipc(module: &RpcModule<()>) -> Self {
pub(crate) fn ipc(module: &RpcModule<()>) -> Self {
Self::new(module, RpcTransport::Ipc)
}
}
Expand All @@ -91,9 +81,8 @@ struct RpcServerMetricsInner {
/// A [`RpcServiceT`] middleware that captures RPC metrics for the server.
///
/// This is created per connection and captures metrics for each request.
#[derive(Clone, Debug)]
#[allow(unreachable_pub)]
pub struct RpcRequestMetricsService<S> {
#[derive(Clone)]
pub(crate) struct RpcRequestMetricsService<S> {
metrics: RpcRequestMetrics,
inner: S,
}
Expand Down Expand Up @@ -136,8 +125,7 @@ impl<S> Drop for RpcRequestMetricsService<S> {

/// Response future to update the metrics for a single request/response pair.
#[pin_project::pin_project]
#[allow(unreachable_pub)]
pub struct MeteredRequestFuture<F> {
pub(crate) struct MeteredRequestFuture<F> {
#[pin]
fut: F,
/// time when the request started
Expand Down Expand Up @@ -186,8 +174,7 @@ impl<F: Future<Output = MethodResponse>> Future for MeteredRequestFuture<F> {

/// The transport protocol used for the RPC connection.
#[derive(Debug, Clone, Copy, Eq, PartialEq)]
#[allow(unreachable_pub)]
pub enum RpcTransport {
pub(crate) enum RpcTransport {
Http,
WebSocket,
#[allow(unused)]
Expand Down
11 changes: 2 additions & 9 deletions crates/rpc/rpc-builder/tests/it/startup.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,8 @@
use std::io;

use jsonrpsee::server::RpcServiceBuilder;
use reth_rpc_builder::{
error::{RpcError, ServerKind, WsHttpSamePortError},
metrics::RpcRequestMetrics,
EthApiBuild, RpcServerConfig, TransportRpcModuleConfig,
};
use reth_rpc_server_types::RethRpcModule;
Expand All @@ -30,13 +28,8 @@ async fn test_http_addr_in_use() {
let builder = test_rpc_builder();
let server = builder
.build(TransportRpcModuleConfig::set_http(vec![RethRpcModule::Admin]), EthApiBuild::build);
let rpc_middleware = RpcServiceBuilder::new()
.layer(server.http.as_ref().map(RpcRequestMetrics::http).unwrap_or_default());
let result = RpcServerConfig::http(Default::default())
.with_http_address(addr)
.set_rpc_middleware(rpc_middleware)
.start(&server)
.await;
let result =
RpcServerConfig::http(Default::default()).with_http_address(addr).start(&server).await;
let err = result.unwrap_err();
assert!(is_addr_in_use_kind(&err, ServerKind::Http(addr)), "{err}");
}
Expand Down
5 changes: 0 additions & 5 deletions crates/rpc/rpc-builder/tests/it/utils.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
use std::net::{Ipv4Addr, SocketAddr, SocketAddrV4};

use jsonrpsee::server::RpcServiceBuilder;
use reth_beacon_consensus::BeaconConsensusEngineHandle;
use reth_chainspec::MAINNET;
use reth_ethereum_engine_primitives::EthEngineTypes;
Expand All @@ -10,7 +9,6 @@ use reth_payload_builder::test_utils::spawn_test_payload_service;
use reth_provider::test_utils::{NoopProvider, TestCanonStateSubscriptions};
use reth_rpc_builder::{
auth::{AuthRpcModule, AuthServerConfig, AuthServerHandle},
metrics::RpcRequestMetrics,
EthApiBuild, RpcModuleBuilder, RpcServerConfig, RpcServerHandle, TransportRpcModuleConfig,
};
use reth_rpc_engine_api::{capabilities::EngineCapabilities, EngineApi};
Expand Down Expand Up @@ -56,11 +54,8 @@ pub async fn launch_auth(secret: JwtSecret) -> AuthServerHandle {
pub async fn launch_http(modules: impl Into<RpcModuleSelection>) -> RpcServerHandle {
let builder = test_rpc_builder();
let server = builder.build(TransportRpcModuleConfig::set_http(modules), EthApiBuild::build);
let rpc_middleware = RpcServiceBuilder::new()
.layer(server.http.as_ref().map(RpcRequestMetrics::http).unwrap_or_default());
RpcServerConfig::http(Default::default())
.with_http_address(test_address())
.set_rpc_middleware(rpc_middleware)
.start(&server)
.await
.unwrap()
Expand Down
5 changes: 0 additions & 5 deletions examples/rpc-db/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,10 @@ use reth::rpc::builder::{
EthApiBuild, RethRpcModule, RpcModuleBuilder, RpcServerConfig, TransportRpcModuleConfig,
};
// Configuring the network parts, ideally also wouldn't need to think about this.
use jsonrpsee::server::RpcServiceBuilder;
use myrpc_ext::{MyRpcExt, MyRpcExtApiServer};
use reth::{blockchain_tree::noop::NoopBlockchainTree, tasks::TokioTaskExecutor};
use reth_node_ethereum::EthEvmConfig;
use reth_provider::test_utils::TestCanonStateSubscriptions;
use reth_rpc_builder::metrics::RpcRequestMetrics;

// Custom rpc extension
pub mod myrpc_ext;
Expand Down Expand Up @@ -79,11 +77,8 @@ async fn main() -> eyre::Result<()> {
server.merge_configured(custom_rpc.into_rpc())?;

// Start the server & keep it alive
let rpc_middleware = RpcServiceBuilder::new()
.layer(server.http.as_ref().map(RpcRequestMetrics::http).unwrap_or_default());
let _handle = RpcServerConfig::http(Default::default())
.with_http_address("0.0.0.0:8545".parse()?)
.set_rpc_middleware(rpc_middleware)
.start(&server)
.await?;
futures::future::pending::<()>().await;
Expand Down

0 comments on commit 0ca715c

Please sign in to comment.