p2dq/mitosis/lavd/layered -- cli and logging productionization #3017
Conversation
use clap_main for opts handling in p2dq/mitosis/layered/lavd. also, add an optional run id arg to these schedulers args. arguments such as run id are commonly used by downstream log aggregation systems, so add it and print it. Signed-off-by: Pat Somaru <[email protected]>
migrate logging to tracing. while doing, enable standard rust logging conventions, such as the use of EnvFilter to enable the setting of RUST_LOG and accepting a --log-level argument to set log level. translate log levels into bpf log levels and log them appropriately, do the same w/ libbpf init logging logic. fix a dependency issue in chaos wrt/ logging after migrating p2dq to tracing. explicitly specify dependencies of tracing which we always want to be present, such as tracing-log to ensure that log logs are never silently dropped and parking_lot to ensure that lots of logs don't slow things down from the scheduler binary. enable all the informative things which tracing provides, making logs look like: ``` 2025-11-07T20:35:45.734954Z DEBUG ThreadId(01) scx_utils::libbpf_logger: rust/scx_utils/src/libbpf_logger.rs:10: libbpf: map 'mitosis': created successfully, fd=19 2025-11-07T20:35:45.768205Z WARN ThreadId(01) scx_utils::libbpf_logger: rust/scx_utils/src/libbpf_logger.rs:12: libbpf: map 'mitosis': BPF map skeleton link is uninitialized 2025-11-07T20:35:45.803635Z INFO ThreadId(01) scx_mitosis: scheds/rust/scx_mitosis/src/main.rs:240: Mitosis Scheduler Attached. Run `scx_mitosis --monitor` for metrics. 2025-11-07T20:35:45.804038Z TRACE ThreadId(01) scx_mitosis: scheds/rust/scx_mitosis/src/main.rs:345: Total Decisions: 27 100.0% | Local:74.1% From: CPU:25.9% Cell: 0.0% | V: 0.0% ``` Signed-off-by: Pat Somaru <[email protected]>
likewhatevs
requested review from
daidavid,
dforsyth,
dschatzberg,
hodgesds,
htejun,
mkutsevol and
multics69
|
|
||
| /// Optional run ID for tracking scheduler instances. | ||
| #[clap(long)] | ||
| run_id: Option<u64>, |
There was a problem hiding this comment.
I don't really get the run id, is it just for logging? Why not just print the pid, unless you want something deterministic?
There was a problem hiding this comment.
That needs to be very unique -- more/less that'd be the join key for systems which aggregate logs etc. pid could work until it didn't and things came out wrong.
|
|
||
| /// Optional run ID for tracking scheduler instances. | ||
| #[clap(long)] | ||
| run_id: Option<u64>, |
There was a problem hiding this comment.
If we're going to take run_id, should we update the log formatter to always insert it?
There was a problem hiding this comment.
My thinking is no -- for many use cases (i.e. my bazzite handheld), this would just be log noise. For those use cases where this would be used though, this would lessen (possibly eliminate, likely only lessen because of how rust logging works, IIUC) the code delta to enable the use of this flag.
| simplelog::TerminalMode::Stderr, | ||
| simplelog::ColorChoice::Auto, | ||
| )?; | ||
| let env_filter = EnvFilter::try_from_default_env() |
There was a problem hiding this comment.
maybe for a later change -- these are the same in all the schedulers you touched here (I think). Can we just share?
There was a problem hiding this comment.
tbh not sure. i think for these 4 that makes sense, but at the same time there's a constant tension between having schedulers be independent and sharing code. idk.
4 participants
This pr productionizes lavd/mitosis/p2dq/layered a bit.
It has CLI args be handled by clap_main and adds a run_id argument to enable passing in a run id to enable log correlation etc. wrt when scheduler binaries are exec'ed.
It has logging in those schedulers be handled by tracing, following conventions such as
--log-levelandRUST_LOG, while ensuring that logging log logs are not silently dropped.