Skip to content

Commit cac95d6

Browse files
apollo_infra: extract MetricsConfig from MonitoringEndpointConfig
1 parent 6838143 commit cac95d6

File tree

12 files changed

+64
-50
lines changed

12 files changed

+64
-50
lines changed

crates/apollo_deployments/resources/app_configs/monitoring_endpoint_config.json

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,4 @@
11
{
2-
"monitoring_endpoint_config.collect_metrics": true,
3-
"monitoring_endpoint_config.collect_profiling_metrics": true,
42
"monitoring_endpoint_config.ip": "0.0.0.0",
53
"monitoring_config.collect_metrics": true,
64
"monitoring_config.collect_profiling_metrics": true
Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,5 @@
11
{
22
"monitoring_config.collect_metrics": true,
33
"monitoring_config.collect_profiling_metrics": true,
4-
"monitoring_endpoint_config.collect_metrics": true,
5-
"monitoring_endpoint_config.collect_profiling_metrics": true,
64
"monitoring_endpoint_config.ip": "0.0.0.0"
75
}

crates/apollo_infra/src/metrics.rs

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,31 @@ use apollo_metrics::metrics::{
44
MetricGauge,
55
MetricHistogram,
66
};
7+
use serde::{Deserialize, Serialize};
78

89
use crate::requests::LABEL_NAME_REQUEST_VARIANT;
910

11+
/// Configuration for metrics collection.
12+
#[derive(Clone, Copy, Debug, Default, Serialize, Deserialize, PartialEq)]
13+
pub struct MetricsConfig {
14+
/// Whether to collect metrics at all.
15+
pub collect_metrics: bool,
16+
/// Whether to collect profiling metrics.
17+
pub collect_profiling_metrics: bool,
18+
}
19+
20+
impl MetricsConfig {
21+
/// Returns a config with all metrics collection enabled.
22+
pub const fn enabled() -> Self {
23+
Self { collect_metrics: true, collect_profiling_metrics: true }
24+
}
25+
26+
/// Returns a config with all metrics collection disabled.
27+
pub const fn disabled() -> Self {
28+
Self { collect_metrics: false, collect_profiling_metrics: false }
29+
}
30+
}
31+
1032
/// Metrics of a local client.
1133
#[derive(Clone)]
1234
pub struct LocalClientMetrics {

crates/apollo_integration_tests/src/flow_test_setup.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ use apollo_base_layer_tests::anvil_base_layer::AnvilBaseLayer;
66
use apollo_consensus_manager_config::config::ConsensusManagerConfig;
77
use apollo_http_server::test_utils::HttpTestClient;
88
use apollo_http_server_config::config::HttpServerConfig;
9+
use apollo_infra::metrics::MetricsConfig;
910
use apollo_infra_utils::test_utils::AvailablePorts;
1011
use apollo_l1_gas_price_provider_config::config::EthToStrkOracleConfig;
1112
use apollo_mempool_p2p_config::config::MempoolP2pConfig;
@@ -259,12 +260,8 @@ impl FlowSequencerSetup {
259260

260261
let component_config = ComponentConfig::default();
261262

262-
// Explicitly avoid collecting metrics in the monitoring endpoint; metrics are collected
263-
// using a global recorder, which fails when being set multiple times in the same
264-
// process, as in this test.
265263
let monitoring_endpoint_config = MonitoringEndpointConfig {
266264
port: available_ports.get_next_port(),
267-
collect_metrics: false,
268265
..Default::default()
269266
};
270267

@@ -292,7 +289,10 @@ impl FlowSequencerSetup {
292289
num_l1_txs;
293290

294291
debug!("Sequencer config: {:#?}", node_config);
295-
let (clients, servers) = create_node_modules(&node_config, vec![]).await;
292+
// Pass MetricsConfig::disabled() to avoid conflicts when running multiple
293+
// sequencers in the same process (metrics recorder can only be installed once globally).
294+
let (clients, servers) =
295+
create_node_modules(&node_config, MetricsConfig::disabled(), vec![]).await;
296296

297297
let MonitoringEndpointConfig { ip, port, .. } =
298298
node_config.monitoring_endpoint_config.as_ref().unwrap().to_owned();

crates/apollo_integration_tests/src/integration_test_manager.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1132,7 +1132,6 @@ async fn get_sequencer_setup_configs(
11321132
// Set a monitoring endpoint for each executable.
11331133
let monitoring_endpoint_config = MonitoringEndpointConfig {
11341134
port: config_available_ports.get_next_port(),
1135-
collect_metrics: true,
11361135
..Default::default()
11371136
};
11381137

crates/apollo_monitoring_endpoint/src/monitoring_endpoint.rs

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
use std::net::SocketAddr;
22

33
use apollo_infra::component_definitions::ComponentStarter;
4+
use apollo_infra::metrics::MetricsConfig;
45
use apollo_infra::tokio_metrics::setup_tokio_metrics;
56
use apollo_infra::trace_util::{configure_tracing, get_log_directives, set_log_level};
67
use apollo_infra_utils::type_name::short_type_name;
@@ -50,15 +51,16 @@ impl MonitoringEndpoint {
5051
pub fn new(
5152
config: MonitoringEndpointConfig,
5253
version: &'static str,
54+
metrics_config: MetricsConfig,
5355
mempool_client: Option<SharedMempoolClient>,
5456
l1_provider_client: Option<SharedL1ProviderClient>,
5557
) -> Self {
5658
// TODO(Tsabary): consider error handling
57-
let prometheus_handle = if config.collect_metrics {
59+
let prometheus_handle = if metrics_config.collect_metrics {
5860
// TODO(Lev): add tests that show the metrics are collected / not collected based on the
5961
// config value.
6062
COLLECT_SEQUENCER_PROFILING_METRICS
61-
.set(config.collect_profiling_metrics)
63+
.set(metrics_config.collect_profiling_metrics)
6264
.expect("Should be able to set profiling metrics collection.");
6365

6466
Some(
@@ -144,10 +146,17 @@ impl MonitoringEndpoint {
144146
pub fn create_monitoring_endpoint(
145147
config: MonitoringEndpointConfig,
146148
version: &'static str,
149+
metrics_config: MetricsConfig,
147150
mempool_client: Option<SharedMempoolClient>,
148151
l1_provider_client: Option<SharedL1ProviderClient>,
149152
) -> MonitoringEndpoint {
150-
let result = MonitoringEndpoint::new(config, version, mempool_client, l1_provider_client);
153+
let result = MonitoringEndpoint::new(
154+
config,
155+
version,
156+
metrics_config,
157+
mempool_client,
158+
l1_provider_client,
159+
);
151160
setup_tokio_metrics();
152161
result
153162
}

crates/apollo_monitoring_endpoint/src/monitoring_endpoint_test.rs

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ use std::collections::HashMap;
22
use std::net::IpAddr;
33
use std::sync::Arc;
44

5+
use apollo_infra::metrics::MetricsConfig;
56
use apollo_l1_provider_types::{L1ProviderSnapshot, MockL1ProviderClient};
67
use apollo_mempool_types::communication::MockMempoolClient;
78
use apollo_mempool_types::mempool_types::{
@@ -49,13 +50,14 @@ const TEST_VERSION: &str = "1.2.3-dev";
4950
const CONFIG_WITHOUT_METRICS: MonitoringEndpointConfig = MonitoringEndpointConfig {
5051
ip: MONITORING_ENDPOINT_DEFAULT_IP,
5152
port: MONITORING_ENDPOINT_DEFAULT_PORT,
52-
collect_metrics: false,
53-
collect_profiling_metrics: false,
5453
};
5554

55+
const METRICS_CONFIG_DISABLED: MetricsConfig =
56+
MetricsConfig { collect_metrics: false, collect_profiling_metrics: false };
57+
5658
fn setup_monitoring_endpoint(config: Option<MonitoringEndpointConfig>) -> MonitoringEndpoint {
5759
let config = config.unwrap_or(CONFIG_WITHOUT_METRICS);
58-
create_monitoring_endpoint(config, TEST_VERSION, None, None)
60+
create_monitoring_endpoint(config, TEST_VERSION, METRICS_CONFIG_DISABLED, None, None)
5961
}
6062

6163
async fn request_app(app: Router, method: &str) -> Response {
@@ -109,8 +111,9 @@ async fn set_log_level_invalid_level() {
109111

110112
#[tokio::test]
111113
async fn with_metrics() {
112-
let config = MonitoringEndpointConfig { collect_metrics: true, ..Default::default() };
113-
let app = setup_monitoring_endpoint(Some(config)).app();
114+
let config = MonitoringEndpointConfig::default();
115+
let metrics_config = MetricsConfig { collect_metrics: true, collect_profiling_metrics: false };
116+
let app = create_monitoring_endpoint(config, TEST_VERSION, metrics_config, None, None).app();
114117

115118
// Register a metric.
116119
let metric_name = "metric_name";
@@ -163,6 +166,7 @@ fn setup_monitoring_endpoint_with_mempool_client() -> MonitoringEndpoint {
163166
create_monitoring_endpoint(
164167
CONFIG_WITHOUT_METRICS,
165168
TEST_VERSION,
169+
METRICS_CONFIG_DISABLED,
166170
Some(shared_mock_mempool_client),
167171
None,
168172
)
@@ -225,6 +229,7 @@ fn setup_monitoring_endpoint_with_l1_provider_client() -> MonitoringEndpoint {
225229
create_monitoring_endpoint(
226230
CONFIG_WITHOUT_METRICS,
227231
TEST_VERSION,
232+
METRICS_CONFIG_DISABLED,
228233
None,
229234
Some(shared_mock_l1_provider_client),
230235
)

crates/apollo_monitoring_endpoint_config/src/config.rs

Lines changed: 1 addition & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -14,18 +14,11 @@ pub const MONITORING_ENDPOINT_DEFAULT_PORT: u16 = 8082;
1414
pub struct MonitoringEndpointConfig {
1515
pub ip: IpAddr,
1616
pub port: u16,
17-
pub collect_metrics: bool,
18-
pub collect_profiling_metrics: bool,
1917
}
2018

2119
impl MonitoringEndpointConfig {
2220
pub fn deployment() -> Self {
23-
Self {
24-
ip: MONITORING_ENDPOINT_DEFAULT_IP,
25-
port: MONITORING_ENDPOINT_DEFAULT_PORT,
26-
collect_metrics: true,
27-
collect_profiling_metrics: true,
28-
}
21+
Self { ip: MONITORING_ENDPOINT_DEFAULT_IP, port: MONITORING_ENDPOINT_DEFAULT_PORT }
2922
}
3023
}
3124

@@ -50,18 +43,6 @@ impl SerializeConfig for MonitoringEndpointConfig {
5043
"The monitoring endpoint port.",
5144
ParamPrivacyInput::Public,
5245
),
53-
ser_param(
54-
"collect_metrics",
55-
&self.collect_metrics,
56-
"If true, collect and return metrics in the monitoring endpoint.",
57-
ParamPrivacyInput::Public,
58-
),
59-
ser_param(
60-
"collect_profiling_metrics",
61-
&self.collect_profiling_metrics,
62-
"If true, collect and return profiling metrics in the monitoring endpoint.",
63-
ParamPrivacyInput::Public,
64-
),
6546
])
6647
}
6748
}

crates/apollo_node/resources/config_schema.json

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2784,16 +2784,6 @@
27842784
"privacy": "TemporaryValue",
27852785
"value": false
27862786
},
2787-
"monitoring_endpoint_config.collect_metrics": {
2788-
"description": "If true, collect and return metrics in the monitoring endpoint.",
2789-
"privacy": "Public",
2790-
"value": true
2791-
},
2792-
"monitoring_endpoint_config.collect_profiling_metrics": {
2793-
"description": "If true, collect and return profiling metrics in the monitoring endpoint.",
2794-
"privacy": "Public",
2795-
"value": true
2796-
},
27972787
"monitoring_endpoint_config.ip": {
27982788
"description": "The monitoring endpoint ip address.",
27992789
"privacy": "Public",

crates/apollo_node/src/components.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ use apollo_config_manager::config_manager_runner::ConfigManagerRunner;
1010
use apollo_consensus_manager::consensus_manager::{ConsensusManager, ConsensusManagerArgs};
1111
use apollo_gateway::gateway::{create_gateway, Gateway};
1212
use apollo_http_server::http_server::{create_http_server, HttpServer};
13+
use apollo_infra::metrics::MetricsConfig;
1314
use apollo_l1_endpoint_monitor::monitor::L1EndpointMonitor;
1415
use apollo_l1_gas_price::l1_gas_price_provider::L1GasPriceProvider;
1516
use apollo_l1_gas_price::l1_gas_price_scraper::L1GasPriceScraper;
@@ -62,9 +63,12 @@ pub struct SequencerNodeComponents {
6263
pub state_sync_runner: Option<StateSyncRunner>,
6364
}
6465

66+
// TODO(Nadin): metrics_config is a temporary parameter until metrics initialization is moved to
67+
// main.rs
6568
pub async fn create_node_components(
6669
config: &SequencerNodeConfig,
6770
clients: &SequencerNodeClients,
71+
metrics_config: MetricsConfig,
6872
cli_args: Vec<String>,
6973
) -> SequencerNodeComponents {
7074
// TODO(tsabary): consider moving ownership of component configs to the components themselves
@@ -324,6 +328,7 @@ pub async fn create_node_components(
324328
Some(create_monitoring_endpoint(
325329
monitoring_endpoint_config.clone(),
326330
VERSION_FULL,
331+
metrics_config,
327332
mempool_client,
328333
l1_provider_client,
329334
))

0 commit comments

Comments
 (0)