Make metrics collection opt-in (Issue #5) #4

This PR makes metrics collection opt-in as described as a possible advancement in the description for issue #5.

We add a new --collect-metrics (presence) flag. The new default is to not collect any metrics if this flag is not provided, i.e. neither spawning the monitoring thread nor starting the metrics server server that exposes these.

This is my first PR I opened here on Tangled. I feel comfortable contributing in Rust, but am by no means an expert, so if anything seems off or you have other kind of feedback I will gladly take that.

+19 -3

1 changed file

Diff round #0

expand all

unified split

constellation

src

bin

main.rs

+19 -3

constellation/src/bin/main.rs

··· 26 #[arg(long)] 27 #[clap(default_value = "0.0.0.0:6789")] 28 bind: SocketAddr, 29 /// metrics server's listen address 30 #[arg(long)] 31 #[clap(default_value = "0.0.0.0:8765")] ··· 92 let bind = args.bind; 93 let metrics_bind = args.bind_metrics; 94 95 let stay_alive = CancellationToken::new(); 96 97 match args.backend { ··· 102 stream, 103 bind, 104 metrics_bind, 105 stay_alive, 106 ), 107 #[cfg(feature = "rocks")] ··· 136 stream, 137 bind, 138 metrics_bind, 139 stay_alive, 140 ); 141 eprintln!("run finished: {r:?}"); ··· 147 } 148 } 149 150 fn run( 151 mut storage: impl LinkStorage, 152 fixture: Option<PathBuf>, ··· 154 stream: String, 155 bind: SocketAddr, 156 metrics_bind: SocketAddr, 157 stay_alive: CancellationToken, 158 ) -> Result<()> { 159 ctrlc::set_handler({ ··· 198 .build() 199 .expect("axum startup") 200 .block_on(async { 201 - install_metrics_server(metrics_bind)?; 202 serve(readable, bind, staying_alive).await 203 }) 204 .unwrap(); ··· 206 } 207 }); 208 209 - s.spawn(move || { // monitor thread 210 let stay_alive = stay_alive.clone(); 211 let check_alive = stay_alive.clone(); 212 ··· 258 } 259 } 260 stay_alive.drop_guard(); 261 - }); 262 }); 263 264 println!("byeeee");

··· 26 #[arg(long)] 27 #[clap(default_value = "0.0.0.0:6789")] 28 bind: SocketAddr, 29 + /// optionally disable the metrics server 30 + #[arg(long)] 31 + #[clap(default_value_t = false)] 32 + collect_metrics: bool, 33 /// metrics server's listen address 34 #[arg(long)] 35 #[clap(default_value = "0.0.0.0:8765")] ··· 96 let bind = args.bind; 97 let metrics_bind = args.bind_metrics; 98 99 + let collect_metrics = args.collect_metrics; 100 let stay_alive = CancellationToken::new(); 101 102 match args.backend { ··· 107 stream, 108 bind, 109 metrics_bind, 110 + collect_metrics, 111 stay_alive, 112 ), 113 #[cfg(feature = "rocks")] ··· 142 stream, 143 bind, 144 metrics_bind, 145 + collect_metrics, 146 stay_alive, 147 ); 148 eprintln!("run finished: {r:?}"); ··· 154 } 155 } 156 157 + #[allow(clippy::too_many_lines)] 158 + #[allow(clippy::too_many_arguments)] 159 fn run( 160 mut storage: impl LinkStorage, 161 fixture: Option<PathBuf>, ··· 163 stream: String, 164 bind: SocketAddr, 165 metrics_bind: SocketAddr, 166 + collect_metrics: bool, 167 stay_alive: CancellationToken, 168 ) -> Result<()> { 169 ctrlc::set_handler({ ··· 208 .build() 209 .expect("axum startup") 210 .block_on(async { 211 + // Install metrics server only if requested 212 + if collect_metrics { 213 + install_metrics_server(metrics_bind)?; 214 + } 215 serve(readable, bind, staying_alive).await 216 }) 217 .unwrap(); ··· 219 } 220 }); 221 222 + // only spawn monitoring thread if the metrics server is running 223 + if collect_metrics { 224 + s.spawn(move || { // monitor thread 225 let stay_alive = stay_alive.clone(); 226 let check_alive = stay_alive.clone(); 227 ··· 273 } 274 } 275 stay_alive.drop_guard(); 276 + }); 277 + } 278 }); 279 280 println!("byeeee");

History

1 round 1 comment

seoul.systems submitted #0 3mo

1 commit

expand

d4a39074

Make metrics collection opt-in

expand 1 comment

bad-example.com 2mo

oh yeah this is great, thank you!

pull request successfully merged