Redesign windsock cli (#1481)

.github/workflows/windsock_benches.yaml

@@ -37,13 +37,13 @@ jobs:
         echo '1' | sudo tee /proc/sys/kernel/perf_event_paranoid
 
         # run some extra cases that arent handled by nextest
-        cargo windsock --bench-length-seconds 5 --operations-per-second 100 --profilers flamegraph --name cassandra,compression=none,connection_count=1,driver=scylla,operation=read_i64,protocol=v4,shotover=standard,topology=single
-        cargo windsock --bench-length-seconds 5 --operations-per-second 100 --profilers samply --name cassandra,compression=none,connection_count=1,driver=scylla,operation=read_i64,protocol=v4,shotover=standard,topology=single
-        cargo windsock --bench-length-seconds 5 --operations-per-second 100 --profilers sys_monitor --name kafka,shotover=standard,size=1B,topology=single
-        cargo windsock --bench-length-seconds 5 --operations-per-second 100 --profilers shotover_metrics --name redis,encryption=none,operation=get,shotover=standard,topology=single
+        cargo windsock local-run --bench-length-seconds 5 --operations-per-second 100 --profilers flamegraph name=cassandra,compression=none,connection_count=1,driver=scylla,operation=read_i64,protocol=v4,shotover=standard,topology=single
+        cargo windsock local-run --bench-length-seconds 5 --operations-per-second 100 --profilers samply name=cassandra,compression=none,connection_count=1,driver=scylla,operation=read_i64,protocol=v4,shotover=standard,topology=single
+        cargo windsock local-run --bench-length-seconds 5 --operations-per-second 100 --profilers sys_monitor name=kafka,shotover=standard,size=1B,topology=single
+        cargo windsock local-run --bench-length-seconds 5 --operations-per-second 100 --profilers shotover_metrics name=redis,encryption=none,operation=get,shotover=standard,topology=single
 
         # windsock/examples/cassandra.rs - this can stay here until windsock is moved to its own repo
-        cargo run --release --example cassandra -- --bench-length-seconds 5 --operations-per-second 100
+        cargo run --release --example cassandra -- local-run --bench-length-seconds 5 --operations-per-second 100
     - name: Ensure that tests did not create or modify any files that arent .gitignore'd
       run: |
         if [ -n "$(git status --porcelain)" ]; then

windsock/readme.md

@@ -102,16 +102,18 @@ impl BenchTask for BenchTaskCassandra {
 }
 ```
 
+**TODO:** document running windsock as both a standalone crate and as a cargo bench.
+
 This example is simplified for demonstration purposes, refer to `examples/cassandra.rs` for a full working example.
 
 ## Running benches
 
 Then we run our crate to run the benchmarks and view results like:
 
 ```none
-> cargo run
+> cargo windsock run-local
 ... benchmark running logs
-> cargo run -- --results-by-name "cassandra,topology=single cassandra,topology=cluster3"
+> cargo windsock results
 Results for cassandra
            topology   ──single ─cluster3
 Measurements ═══════════════════════════
@@ -142,33 +144,63 @@ and graphs: TODO
 ### Just run every bench
 
 ```shell
-> cargo run
+> cargo windsock
 ```
 
 ### Run benches with matching tags and view all the results in one table
 
 ```shell
-> cargo run -- db=kafka OPS=1000 topology=single # run benchmarks matching some tags
-> cargo run -- --results-by-tag db=kafka OPS=1000 topology=single # view the results of the benchmarks with the same tags in a single table
+> cargo windsock run-local db=kafka OPS=1000 topology=single # run benchmarks matching some tags
+> cargo windsock results # view the results of the benchmarks with the same tags in a single table
 ```
 
 ### Iteratively compare results against a previous implementation
 
 ```shell
 > git checkout main # checkout original implementation
-> cargo run # run all benchmarks
-> cargo run -- --set-baseline # set the last benchmark run as the baseline
+> cargo windsock run-local # run all benchmarks
+> cargo windsock baseline-set # set the last benchmark run as the baseline
 > vim src/main.rs # modify implementation
-> cargo run # run all benchmarks, every result is compared against the baseline
+> cargo windsock run-local # run all benchmarks, every result is compared against the baseline
+> cargo windsock results # view those results in a nice table
 > vim src/main.rs # modify implementation again
-> cargo run # run all benchmarks, every result is compared against the baseline
+> cargo windsock run-local # run all benchmarks, every result is compared against the baseline
+```
+
+### Run benchmarks in the cloud (simple)
+
+```shell
+# create cloud resources, run benchmarks and then cleanup - all in one command
+> cargo windsock cloud-setup-run-cleanup
+```
+
+### Iteratively compare results against a previous implementation (running in a remote cloud)
+
+```shell
+# Setup the cloud resources and then form a baseline
+> git checkout main # checkout original implementation
+> cargo windsock cloud-setup db=kafka # setup the cloud resources required to run all kafka benchmarks
+> cargo windsock cloud-run db=kafka # run all the kafka benchmarks in the cloud
+> cargo windsock baseline-set # set the last benchmark run as the baseline
+
+# Make a change and and measure the effect
+> vim src/main.rs # modify implementation
+> cargo windsock cloud-run db=kafka # run all benchmarks, every result is compared against the baseline
+> cargo windsock results # view those results in a nice table, compared against the baseline
+
+# And again
+> vim src/main.rs # modify implementation again
+> cargo windsock cloud-run db=kafka # run all benchmarks, every result is compared against the baseline
+
+# And finally...
+> cargo windsock cloud-cleanup # Terminate all the cloud resources now that we are done
 ```
 
 ### Generate graph webpage
 
 TODO: not yet implemented
 
 ```shell
-> cargo run # run all benches
-> cargo run -- --generate_webpage # generate a webpage from the results
+> cargo windsock local-run # run all benches
+> cargo windsock generate-webpage # generate a webpage from the results
 ```

windsock/src/bench.rs

@@ -1,4 +1,4 @@
-use crate::cli::Args;
+use crate::cli::RunArgs;
 use crate::report::{report_builder, Report, ReportArchive};
 use crate::tables::ReportColumn;
 use anyhow::Result;
@@ -34,7 +34,7 @@ impl<ResourcesRequired, Resources> BenchState<ResourcesRequired, Resources> {
 
     pub async fn orchestrate(
         &mut self,
-        args: &Args,
+        args: &RunArgs,
         running_in_release: bool,
         cloud_resources: Option<Resources>,
     ) {
@@ -52,10 +52,10 @@ impl<ResourcesRequired, Resources> BenchState<ResourcesRequired, Resources> {
             PathBuf::new()
         };
 
-        if args.cloud {
+        if let Some(cloud_resources) = cloud_resources {
             self.bench
                 .orchestrate_cloud(
-                    cloud_resources.unwrap(),
+                    cloud_resources,
                     running_in_release,
                     Profiling {
                         results_path,
@@ -85,7 +85,7 @@ impl<ResourcesRequired, Resources> BenchState<ResourcesRequired, Resources> {
         }]);
     }
 
-    pub async fn run(&mut self, args: &Args, running_in_release: bool, resources: &str) {
+    pub async fn run(&mut self, args: &RunArgs, running_in_release: bool, resources: &str) {
         let (tx, rx) = tokio::sync::mpsc::unbounded_channel();
         let process = tokio::spawn(report_builder(
             self.tags.clone(),
@@ -173,9 +173,9 @@ pub trait Bench {
 
     /// Call within `Bench::orchestrate_local` to call `Bench::run`
     async fn execute_run(&self, resources: &str, bench_parameters: &BenchParameters) {
-        let internal_run = format!("{} {}", self.name(), resources);
+        let name_and_resources = format!("{} {}", self.name(), resources);
         let output = tokio::process::Command::new(std::env::current_exe().unwrap().as_os_str())
-            .args(run_args_vec(internal_run, bench_parameters))
+            .args(run_args_vec(name_and_resources, bench_parameters))
             .output()
             .await
             .unwrap();
@@ -188,17 +188,18 @@ pub trait Bench {
 
     /// Call within `Bench::orchestrate_cloud` to determine how to invoke the uploaded windsock executable
     fn run_args(&self, resources: &str, bench_parameters: &BenchParameters) -> String {
-        let internal_run = format!("\"{} {}\"", self.name(), resources);
-        run_args_vec(internal_run, bench_parameters).join(" ")
+        let name_and_resources = format!("\"{} {}\"", self.name(), resources);
+        run_args_vec(name_and_resources, bench_parameters).join(" ")
     }
 
     fn name(&self) -> String {
         Tags(self.tags()).get_name()
     }
 }
 
-fn run_args_vec(internal_run: String, bench_parameters: &BenchParameters) -> Vec<String> {
+fn run_args_vec(name_and_resources: String, bench_parameters: &BenchParameters) -> Vec<String> {
     let mut args = vec![];
+    args.push("internal-run".to_owned());
     args.push("--bench-length-seconds".to_owned());
     args.push(bench_parameters.runtime_seconds.to_string());
 
@@ -207,8 +208,7 @@ fn run_args_vec(internal_run: String, bench_parameters: &BenchParameters) -> Vec
         args.push(ops.to_string());
     };
 
-    args.push("--internal-run".to_owned());
-    args.push(internal_run);
+    args.push(name_and_resources);
 
     args
 }
@@ -219,7 +219,7 @@ pub struct BenchParameters {
 }
 
 impl BenchParameters {
-    fn from_args(args: &Args) -> Self {
+    fn from_args(args: &RunArgs) -> Self {
         BenchParameters {
             runtime_seconds: args.bench_length_seconds.unwrap_or(15),
             operations_per_second: args.operations_per_second,
@@ -237,21 +237,15 @@ pub(crate) struct Tags(pub HashMap<String, String>);
 
 impl Tags {
     pub fn get_name(&self) -> String {
-        let mut result = if let Some(name) = self.0.get("name") {
-            name.clone()
-        } else {
-            "".to_string()
-        };
+        let mut result = String::new();
 
         let mut tags: Vec<(&String, &String)> = self.0.iter().collect();
         tags.sort_by_key(|x| x.0);
         for (key, value) in tags {
-            if key != "name" {
-                if !result.is_empty() {
-                    write!(result, ",").unwrap();
-                }
-                write!(result, "{key}={value}").unwrap();
+            if !result.is_empty() {
+                write!(result, ",").unwrap();
             }
+            write!(result, "{key}={value}").unwrap();
         }
         result
     }
@@ -265,10 +259,8 @@ impl Tags {
                 let key = pair.next().unwrap().to_owned();
                 let value = pair.next().unwrap().to_owned();
                 map.insert(key, value);
-            } else if map.contains_key("name") {
-                panic!("The name tag was already set and a tag without an '=' was found")
             } else {
-                map.insert("name".to_owned(), tag.to_owned());
+                panic!("tag without an '=' was found")
             }
         }
         Tags(map)