-
Notifications
You must be signed in to change notification settings - Fork 194
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
refactor(katana): separate metrics address and port into 2 args #2537
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -11,14 +11,13 @@ | |||||||||||||||||||||
//! for more info. | ||||||||||||||||||||||
|
||||||||||||||||||||||
use std::collections::HashSet; | ||||||||||||||||||||||
use std::net::{IpAddr, SocketAddr}; | ||||||||||||||||||||||
use std::net::IpAddr; | ||||||||||||||||||||||
use std::path::PathBuf; | ||||||||||||||||||||||
|
||||||||||||||||||||||
use alloy_primitives::U256; | ||||||||||||||||||||||
use anyhow::{Context, Result}; | ||||||||||||||||||||||
use clap::{Args, Parser}; | ||||||||||||||||||||||
use console::Style; | ||||||||||||||||||||||
use dojo_utils::parse::parse_socket_address; | ||||||||||||||||||||||
use katana_core::backend::config::{Environment, StarknetConfig}; | ||||||||||||||||||||||
use katana_core::constants::{ | ||||||||||||||||||||||
DEFAULT_ETH_L1_GAS_PRICE, DEFAULT_INVOKE_MAX_STEPS, DEFAULT_SEQUENCER_ADDRESS, | ||||||||||||||||||||||
|
@@ -27,7 +26,7 @@ | |||||||||||||||||||||
use katana_core::service::messaging::MessagingConfig; | ||||||||||||||||||||||
use katana_node::config::db::DbConfig; | ||||||||||||||||||||||
use katana_node::config::dev::DevConfig; | ||||||||||||||||||||||
use katana_node::config::metrics::MetricsConfig; | ||||||||||||||||||||||
use katana_node::config::metrics::{MetricsConfig, DEFAULT_METRICS_ADDR, DEFAULT_METRICS_PORT}; | ||||||||||||||||||||||
use katana_node::config::rpc::{ | ||||||||||||||||||||||
ApiKind, RpcConfig, DEFAULT_RPC_ADDR, DEFAULT_RPC_MAX_CONNECTIONS, DEFAULT_RPC_PORT, | ||||||||||||||||||||||
}; | ||||||||||||||||||||||
|
@@ -83,12 +82,6 @@ | |||||||||||||||||||||
#[arg(help = "Output logs in JSON format.")] | ||||||||||||||||||||||
pub json_log: bool, | ||||||||||||||||||||||
|
||||||||||||||||||||||
/// Enable Prometheus metrics. | ||||||||||||||||||||||
/// | ||||||||||||||||||||||
/// The metrics will be served at the given interface and port. | ||||||||||||||||||||||
#[arg(long, value_name = "SOCKET", value_parser = parse_socket_address, help_heading = "Metrics")] | ||||||||||||||||||||||
pub metrics: Option<SocketAddr>, | ||||||||||||||||||||||
|
||||||||||||||||||||||
#[arg(long)] | ||||||||||||||||||||||
#[arg(requires = "rpc_url")] | ||||||||||||||||||||||
#[arg(value_name = "BLOCK_NUMBER")] | ||||||||||||||||||||||
|
@@ -105,20 +98,46 @@ | |||||||||||||||||||||
pub messaging: Option<MessagingConfig>, | ||||||||||||||||||||||
|
||||||||||||||||||||||
#[command(flatten)] | ||||||||||||||||||||||
#[command(next_help_heading = "Server options")] | ||||||||||||||||||||||
pub metrics: MetricsOptions, | ||||||||||||||||||||||
|
||||||||||||||||||||||
#[command(flatten)] | ||||||||||||||||||||||
pub server: ServerOptions, | ||||||||||||||||||||||
|
||||||||||||||||||||||
#[command(flatten)] | ||||||||||||||||||||||
#[command(next_help_heading = "Starknet options")] | ||||||||||||||||||||||
pub starknet: StarknetOptions, | ||||||||||||||||||||||
|
||||||||||||||||||||||
#[cfg(feature = "slot")] | ||||||||||||||||||||||
#[command(flatten)] | ||||||||||||||||||||||
#[command(next_help_heading = "Slot options")] | ||||||||||||||||||||||
pub slot: SlotOptions, | ||||||||||||||||||||||
} | ||||||||||||||||||||||
|
||||||||||||||||||||||
#[derive(Debug, Args, Clone)] | ||||||||||||||||||||||
#[command(next_help_heading = "Metrics options")] | ||||||||||||||||||||||
pub struct MetricsOptions { | ||||||||||||||||||||||
/// Whether to enable metrics. | ||||||||||||||||||||||
/// | ||||||||||||||||||||||
/// For now, metrics will still be collected even if this flag is not set. This only | ||||||||||||||||||||||
/// controls whether the metrics server is started or not. | ||||||||||||||||||||||
#[arg(long)] | ||||||||||||||||||||||
pub metrics: bool, | ||||||||||||||||||||||
|
||||||||||||||||||||||
/// The metrics will be served at the given address. | ||||||||||||||||||||||
#[arg(long = "metrics.addr")] | ||||||||||||||||||||||
#[arg(value_name = "ADD")] | ||||||||||||||||||||||
#[arg(requires = "metrics")] | ||||||||||||||||||||||
#[arg(default_value_t = DEFAULT_METRICS_ADDR)] | ||||||||||||||||||||||
pub metrics_addr: IpAddr, | ||||||||||||||||||||||
|
||||||||||||||||||||||
/// The metrics will be served at the given port. | ||||||||||||||||||||||
#[arg(long = "metrics.port")] | ||||||||||||||||||||||
#[arg(value_name = "PORT")] | ||||||||||||||||||||||
#[arg(requires = "metrics")] | ||||||||||||||||||||||
#[arg(default_value_t = DEFAULT_METRICS_PORT)] | ||||||||||||||||||||||
pub metrics_port: u16, | ||||||||||||||||||||||
} | ||||||||||||||||||||||
|
||||||||||||||||||||||
#[derive(Debug, Args, Clone)] | ||||||||||||||||||||||
#[command(next_help_heading = "Server options")] | ||||||||||||||||||||||
pub struct ServerOptions { | ||||||||||||||||||||||
#[arg(short, long)] | ||||||||||||||||||||||
#[arg(default_value_t = DEFAULT_RPC_PORT)] | ||||||||||||||||||||||
|
@@ -142,6 +161,7 @@ | |||||||||||||||||||||
} | ||||||||||||||||||||||
|
||||||||||||||||||||||
#[derive(Debug, Args, Clone)] | ||||||||||||||||||||||
#[command(next_help_heading = "Starknet options")] | ||||||||||||||||||||||
pub struct StarknetOptions { | ||||||||||||||||||||||
#[arg(long)] | ||||||||||||||||||||||
#[arg(default_value = "0")] | ||||||||||||||||||||||
|
@@ -163,7 +183,6 @@ | |||||||||||||||||||||
pub disable_validate: bool, | ||||||||||||||||||||||
|
||||||||||||||||||||||
#[command(flatten)] | ||||||||||||||||||||||
#[command(next_help_heading = "Environment options")] | ||||||||||||||||||||||
pub environment: EnvironmentOptions, | ||||||||||||||||||||||
|
||||||||||||||||||||||
#[arg(long)] | ||||||||||||||||||||||
|
@@ -173,6 +192,7 @@ | |||||||||||||||||||||
} | ||||||||||||||||||||||
|
||||||||||||||||||||||
#[derive(Debug, Args, Clone)] | ||||||||||||||||||||||
#[command(next_help_heading = "Environment options")] | ||||||||||||||||||||||
pub struct EnvironmentOptions { | ||||||||||||||||||||||
#[arg(long)] | ||||||||||||||||||||||
#[arg(help = "The chain ID.")] | ||||||||||||||||||||||
|
@@ -208,6 +228,7 @@ | |||||||||||||||||||||
|
||||||||||||||||||||||
#[cfg(feature = "slot")] | ||||||||||||||||||||||
#[derive(Debug, Args, Clone)] | ||||||||||||||||||||||
#[command(next_help_heading = "Slot options")] | ||||||||||||||||||||||
pub struct SlotOptions { | ||||||||||||||||||||||
#[arg(hide = true)] | ||||||||||||||||||||||
#[arg(long = "slot.controller")] | ||||||||||||||||||||||
|
@@ -364,7 +385,11 @@ | |||||||||||||||||||||
} | ||||||||||||||||||||||
|
||||||||||||||||||||||
fn metrics_config(&self) -> Option<MetricsConfig> { | ||||||||||||||||||||||
self.metrics.map(|addr| MetricsConfig { addr }) | ||||||||||||||||||||||
if self.metrics.metrics { | ||||||||||||||||||||||
Some(MetricsConfig { addr: self.metrics.metrics_addr, port: self.metrics.metrics_port }) | ||||||||||||||||||||||
} else { | ||||||||||||||||||||||
None | ||||||||||||||||||||||
} | ||||||||||||||||||||||
Comment on lines
+388
to
+392
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Ohayo, sensei! Adjusting Following the renaming of the Apply this diff to update the field reference: fn metrics_config(&self) -> Option<MetricsConfig> {
- if self.metrics.metrics {
+ if self.metrics.enabled {
Some(MetricsConfig { addr: self.metrics.metrics_addr, port: self.metrics.metrics_port })
} else {
None
}
} 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||
} | ||||||||||||||||||||||
} | ||||||||||||||||||||||
|
||||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,22 @@ | ||
use std::net::SocketAddr; | ||
use std::net::{IpAddr, Ipv4Addr, SocketAddr}; | ||
|
||
/// Metrics server default address. | ||
pub const DEFAULT_METRICS_ADDR: IpAddr = IpAddr::V4(Ipv4Addr::LOCALHOST); | ||
/// Metrics server default port. | ||
pub const DEFAULT_METRICS_PORT: u16 = 9100; | ||
|
||
/// Node metrics configurations. | ||
#[derive(Debug, Clone)] | ||
pub struct MetricsConfig { | ||
/// The address to bind the metrics server to. | ||
pub addr: SocketAddr, | ||
pub addr: IpAddr, | ||
/// The port to bind the metrics server to. | ||
pub port: u16, | ||
} | ||
|
||
impl MetricsConfig { | ||
/// Returns the [`SocketAddr`] for the metrics server. | ||
pub fn socket_addr(&self) -> SocketAddr { | ||
SocketAddr::new(self.addr, self.port) | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Ohayo, sensei! Possible Typos and Clarity Improvements in
MetricsOptions
Typo in
value_name
formetrics_addr
The
value_name
formetrics_addr
is set to"ADD"
, which seems like a typo. Consider changing it to"ADDR"
for clarity.Apply this diff to fix the typo:
Consider Renaming the
metrics
Field to Improve ClarityUsing
self.metrics.metrics
may cause confusion due to the repetitive naming. Consider renaming the boolean fieldmetrics
inMetricsOptions
toenabled
orenable_metrics
to enhance readability.Apply this diff to rename the field:
Update the references accordingly in the code:
In
metrics_config
method:Update the
#[arg]
attribute:📝 Committable suggestion