feat: add generator code #203
Conversation
| metrics_controller.force_flush()?; | ||
| } | ||
| drop(metrics_controller); | ||
| shutdown_meter_provider(); |
There was a problem hiding this comment.
the drop/shutdown should maybe be in the if let statement since it's not used otherwise but it doesn't look like it's caused an issue
There was a problem hiding this comment.
I think we should add back the drop(metrics_controller);. Probably doesn't matter now but it might cause some confusion when metrics are fixed and prevent them from showing up as expected.
There was a problem hiding this comment.
LGTM other than the one comment about dropping the metrics_controller. Probably need to run a cargo update to fix CI. It looks like the new clippy/rust release doesn't like the old version of time and cargo update -p time should do it, but updating all dependencies is probably fine... normally I'd suggest we should do that in another PR and merge it right away, but as this is close so I think doing it here is fine.
| @@ -4,7 +4,7 @@ CARGO = CARGO_REGISTRIES_CRATES_IO_PROTOCOL=sparse RUSTFLAGS="--cfg tokio_unstab | |||
| all: build check-fmt check-clippy test | |||
|
|
|||
| .PHONY: test | |||
| test: | |||
| test: update | |||
There was a problem hiding this comment.
I don't think we want to run update as part of these tasks. Adding an update step is fine, but we should run make update and commit the lockfile in a commit. I suspect this will cause unstaged changes in CI that could fail the pipeline.
runner/src/main.rs
Outdated
| pub enum Command { | ||
| /// Bootstrap peers in the network | ||
| Bootstrap(bootstrap::Opts), | ||
| /// Simulate a load scenario against the network | ||
| Simulate(simulate::Opts), | ||
| /// Do nothing and exit | ||
| Noop, | ||
| /// Generate load, currently this command is not used |
There was a problem hiding this comment.
nit: mind moving noop to the last position in the enum?
runner/src/main.rs
Outdated
| mod scenario; | ||
| mod simulate; | ||
| mod utils; | ||
|
|
||
| use keramik_common::telemetry; | ||
|
|
||
| use crate::gen::simulate_load; |
There was a problem hiding this comment.
minor: Can you move the crate use statements to after the external deps?
runner/src/load_generator/gen.rs
Outdated
| // so this is unused | ||
| #[allow(dead_code)] | ||
| #[derive(Clone, Debug)] | ||
| pub enum WeekLongSimulationScenarios { |
There was a problem hiding this comment.
What about a different term instead of WeekLong since the run time is configurable? Maybe LoadGenScenarios? If so, can replace that throughout this file.
feat: lgen operator
This PR introduces the initial logic for the runner. It includes the following key features:
This PR does not introduce any functional changes. It primarily adds interfaces and structures that will be used in future work. Once the operator changes are made, we will be able to call this command effectively.
This PR is part of a larger effort and contains several TODOs that will be addressed in subsequent PRs.
For a quick review, the focus is on the runner logic and the new command implementation, rust coding practices/improvements.