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

History

1 round 1 comment

seoul.systems submitted #0 2mo

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