Skip to content

Commit 507c4ef

Browse files
committed
Make capture CLI flags optional
This commit migrates the CLI flags to do with capture management to an optional status. We still require that the capture manager be configured, but now either through the CLI or through YAML configuration. Signed-off-by: Brian L. Troutwine <[email protected]>
1 parent 2f252cf commit 507c4ef

File tree

2 files changed

+88
-26
lines changed

2 files changed

+88
-26
lines changed

lading/src/bin/lading.rs

Lines changed: 85 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,10 @@ enum Error {
6161
PrometheusPath,
6262
#[error("Invalid capture format, must be 'jsonl', 'parquet', or 'multi'")]
6363
InvalidCaptureFormat,
64+
#[error(
65+
"Telemetry must be configured either via command-line flags (--capture-path, --prometheus-addr, or --prometheus-path) or in the configuration file"
66+
)]
67+
MissingTelemetry,
6468
#[error(transparent)]
6569
Registration(#[from] lading_signal::RegisterError),
6670
}
@@ -155,7 +159,7 @@ struct CliFlatLegacy {
155159
))]
156160
#[clap(group(
157161
ArgGroup::new("telemetry")
158-
.required(true)
162+
.required(false)
159163
.args(&["capture_path", "prometheus_addr", "prometheus_path"]),
160164
))]
161165
#[clap(group(
@@ -328,15 +332,15 @@ fn get_config(args: &LadingArgs, config: Option<String>) -> Result<Config, Error
328332

329333
let options_global_labels = args.global_labels.clone().unwrap_or_default();
330334
if let Some(ref prom_addr) = args.prometheus_addr {
331-
config.telemetry = Telemetry::Prometheus {
335+
config.telemetry = Some(Telemetry::Prometheus {
332336
addr: prom_addr.parse()?,
333337
global_labels: options_global_labels.inner,
334-
};
338+
});
335339
} else if let Some(ref prom_path) = args.prometheus_path {
336-
config.telemetry = Telemetry::PrometheusSocket {
340+
config.telemetry = Some(Telemetry::PrometheusSocket {
337341
path: prom_path.parse().map_err(|_| Error::PrometheusPath)?,
338342
global_labels: options_global_labels.inner,
339-
};
343+
});
340344
} else if let Some(ref capture_path) = args.capture_path {
341345
let format = match args.capture_format.as_str() {
342346
"jsonl" => config::CaptureFormat::Jsonl {
@@ -353,32 +357,30 @@ fn get_config(args: &LadingArgs, config: Option<String>) -> Result<Config, Error
353357
_ => return Err(Error::InvalidCaptureFormat),
354358
};
355359

356-
config.telemetry = Telemetry::Log {
360+
config.telemetry = Some(Telemetry::Log {
357361
path: capture_path.parse().map_err(|_| Error::CapturePath)?,
358362
global_labels: options_global_labels.inner,
359363
expiration: Duration::from_secs(args.capture_expiriation_seconds.unwrap_or(u64::MAX)),
360364
format,
361-
};
362-
} else {
363-
match config.telemetry {
364-
Telemetry::Prometheus {
365-
ref mut global_labels,
366-
..
367-
}
368-
| Telemetry::PrometheusSocket {
369-
ref mut global_labels,
370-
..
371-
}
372-
| Telemetry::Log {
373-
ref mut global_labels,
374-
..
375-
} => {
365+
});
366+
} else if let Some(ref mut telemetry) = config.telemetry {
367+
// Telemetry was configured in YAML, merge in global_labels from CLI if any
368+
match telemetry {
369+
Telemetry::Prometheus { global_labels, .. }
370+
| Telemetry::PrometheusSocket { global_labels, .. }
371+
| Telemetry::Log { global_labels, .. } => {
376372
for (k, v) in options_global_labels.inner {
377373
global_labels.insert(k, v);
378374
}
379375
}
380376
}
381377
}
378+
379+
// Validate that telemetry was configured somewhere
380+
if config.telemetry.is_none() {
381+
return Err(Error::MissingTelemetry);
382+
}
383+
382384
Ok(config)
383385
}
384386

@@ -399,7 +401,10 @@ async fn inner_main(
399401
// a passive prometheus export and an active log file. Only one can be
400402
// active at a time.
401403
let mut capture_manager_handle: Option<tokio::task::JoinHandle<()>> = None;
402-
match config.telemetry {
404+
match config
405+
.telemetry
406+
.expect("telemetry should be validated in get_config")
407+
{
403408
Telemetry::PrometheusSocket {
404409
path,
405410
global_labels,
@@ -870,4 +875,62 @@ generator: []
870875
"uqhwd:b2xiyw,hf9gy:uwcy04"
871876
);
872877
}
878+
879+
#[tokio::test(flavor = "multi_thread")]
880+
async fn yaml_only_telemetry_configuration_works() {
881+
// Test that telemetry can be configured in YAML without CLI flags
882+
let contents = r#"
883+
generator: []
884+
telemetry:
885+
addr: "0.0.0.0:9876"
886+
global_labels: {}
887+
"#;
888+
889+
let tmp_dir = tempfile::tempdir().expect("directory could not be created");
890+
let config_path = tmp_dir.path().join("config.yaml");
891+
std::fs::write(&config_path, contents).expect("Failed to write config file");
892+
893+
let args = vec![
894+
"lading",
895+
"--no-target",
896+
"--config-path",
897+
config_path.to_str().unwrap(),
898+
];
899+
let legacy_cli = CliFlatLegacy::parse_from(args);
900+
let config =
901+
get_config(&legacy_cli.args, None).expect("Failed to get config with YAML telemetry");
902+
903+
// Verify telemetry was loaded from YAML
904+
assert!(
905+
config.telemetry.is_some(),
906+
"Telemetry should be loaded from YAML"
907+
);
908+
match config.telemetry.unwrap() {
909+
Telemetry::Prometheus { addr, .. } => {
910+
assert_eq!(addr.to_string(), "0.0.0.0:9876");
911+
}
912+
_ => panic!("Expected Prometheus telemetry"),
913+
}
914+
}
915+
916+
#[test]
917+
fn missing_telemetry_returns_error() {
918+
// Test that missing telemetry (no CLI flags, no YAML) returns an error
919+
let contents = r#"
920+
generator: []
921+
"#;
922+
923+
let args = vec!["lading", "--no-target"];
924+
let legacy_cli = CliFlatLegacy::parse_from(args);
925+
let result = get_config(&legacy_cli.args, Some(contents.to_string()));
926+
927+
assert!(
928+
result.is_err(),
929+
"Should return error when telemetry is not configured"
930+
);
931+
match result.unwrap_err() {
932+
Error::MissingTelemetry => {} // Expected error
933+
other => panic!("Expected MissingTelemetry error, got: {:?}", other),
934+
}
935+
}
873936
}

lading/src/config.rs

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -55,8 +55,7 @@ fn default_sample_period() -> u64 {
5555
#[serde(deny_unknown_fields)]
5656
pub struct Config {
5757
/// The method by which to express telemetry
58-
#[serde(default)]
59-
pub telemetry: Telemetry,
58+
pub telemetry: Option<Telemetry>,
6059
/// The generator to apply to the target in-rig
6160
#[serde(default)]
6261
#[serde(with = "serde_yaml::with::singleton_map_recursive")]
@@ -229,7 +228,7 @@ impl Config {
229228
check_duplicate_blackhole_ids(&partial.blackhole)?;
230229

231230
Ok(Self {
232-
telemetry: partial.telemetry.unwrap_or_default(),
231+
telemetry: partial.telemetry,
233232
generator: partial.generator,
234233
observer: observer::Config::default(),
235234
sample_period_milliseconds: partial
@@ -588,7 +587,7 @@ blackhole:
588587
},
589588
],
590589
target: Option::default(),
591-
telemetry: crate::config::Telemetry::default(),
590+
telemetry: None,
592591
observer: observer::Config::default(),
593592
inspector: Option::default(),
594593
target_metrics: Option::default(),

0 commit comments

Comments
 (0)