Convert run commands

.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 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 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 kafka,shotover=standard,size=1B,topology=single
+        cargo windsock local-run --bench-length-seconds 5 --operations-per-second 100 --profilers shotover_metrics 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/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,

windsock/src/cli.rs

@@ -1,12 +1,54 @@
 use anyhow::{anyhow, Error};
-use clap::{Parser, Subcommand};
+use clap::{Args, Parser, Subcommand};
+
+const ABOUT: &str = r#"Bench Names:
+    Each benchmark has a unique name, this name is used by many options listed below.
+    The name is derived from its tags so you wont find it directly in the bench implementation but it will be listed in `--list`.
+
+Tag Filters:
+    Many options below take tag filters that specify which benches to include.
+    Tag filters specify which benches to include and the filter results are unioned.
+
+    So:
+    * The filter "foo=some_value" will include only benches with the tag key `foo` and the tag value `some_value`
+    * The filter "foo=some_value bar=another_value" will include only benches that match "foo=some_value" and "bar=another_value"
+    * The filter "" will include all benches"#;
 
 #[derive(Subcommand, Clone)]
 pub enum Command {
     /// List the name of every bench.
     #[clap(verbatim_doc_comment)]
     List,
 
+    /// Create cloud resources for running benches
+    #[clap(verbatim_doc_comment)]
+    CloudSetup {
+        /// e.g. "db=kafka connection_count=100"
+        #[clap(verbatim_doc_comment)]
+        filter: String,
+    },
+
+    /// Run benches in the cloud using the resources created by cloud-setup
+    #[clap(verbatim_doc_comment)]
+    CloudRun(RunArgs),
+
+    /// cleanup cloud resources created by cloud-setup
+    /// Make sure to call this when your benchmarking session is finished!
+    #[clap(verbatim_doc_comment)]
+    CloudCleanup,
+
+    /// cloud-setup, cloud-run and cloud-cleanup combined into a single command.
+    /// Convient for getting a quick understanding of performance.
+    /// However, if you are performing optimization work prefer the individual commands as you will get:
+    /// * more stable results (same cloud instance)
+    /// * faster results (skip recreating and destroying cloud resources)
+    #[clap(verbatim_doc_comment)]
+    CloudSetupRunCleanup(RunArgs),
+
+    /// Run benches entirely on your local machine
+    #[clap(verbatim_doc_comment)]
+    LocalRun(RunArgs),
+
     /// The results of the last benchmarks run becomes the new baseline from which future benchmark runs will be compared.
     #[clap(verbatim_doc_comment)]
     BaselineSet,
@@ -30,7 +72,7 @@ pub enum Command {
         #[clap(long, verbatim_doc_comment)]
         ignore_baseline: bool,
 
-        /// e.g. "db=kafka OPS=10000"
+        /// e.g. "db=kafka connection_count=100"
         #[clap(verbatim_doc_comment)]
         filter: Option<String>,
     },
@@ -39,47 +81,25 @@ pub enum Command {
     ///     Comparing various benches against a specific base bench.
     ///
     /// Usage: First provide the base benchmark name then provide benchmark names to compare against the base.
-    ///     --compare_by_name "base_name other_name1 other_name2"
+    ///     "base_name other_name1 other_name2"
     #[clap(verbatim_doc_comment)]
     CompareByName { filter: String },
 
     /// Display results from the last benchmark run by:
     ///     Comparing benches matching tag filters against a specific base bench.
     ///
     /// Usage: First provide the base benchmark name then provide tag filters
-    ///     --compare_by_tags "base_name db=kafka OPS=10000"
+    ///     "base_name db=kafka connection_count=10"
     #[clap(verbatim_doc_comment)]
     CompareByTags { filter: String },
-}
-
-const ABOUT: &str = r#"Bench Names:
-    Each benchmark has a unique name, this name is used by many options listed below.
-    The name is derived from its tags so you wont find it directly in the bench implementation but it will be listed in `--list`.
-
-Tag Filters:
-    Many options below take tag filters that specify which benches to include.
-    Tag filters specify which benches to include and the filter results are unioned.
-
-    So:
-    * The filter "foo=some_value" will include only benches with the tag key `foo` and the tag value `some_value`
-    * The filter "foo=some_value bar=another_value" will include only benches that match "foo=some_value" and "bar=another_value"
-    * The filter "" will include all benches"#;
 
-#[derive(Parser, Clone)]
-#[clap(about=ABOUT)]
-pub struct Args {
-    /// Run all benches that match the specified tag key/values.
-    /// `tag_key=tag_value foo=bar`
+    /// Not for human use. Call this from your bench orchestration method to launch your bencher.
     #[clap(verbatim_doc_comment)]
-    pub filter: Option<String>,
-
-    #[command(subcommand)]
-    pub command: Option<Command>,
-
-    /// Run a specific bench with the name produced via `--list`.
-    #[clap(long, verbatim_doc_comment)]
-    pub name: Option<String>,
+    InternalRun(RunArgs),
+}
 
+#[derive(Args, Clone)]
+pub struct RunArgs {
     /// Instruct benches to profile the application under test with the specified profilers.
     /// Benches that do not support the specified profilers will be skipped.
     #[clap(long, verbatim_doc_comment, value_delimiter = ',')]
@@ -95,31 +115,28 @@ pub struct Args {
     #[clap(long, verbatim_doc_comment)]
     pub operations_per_second: Option<u64>,
 
-    /// By default windsock will run benches on your local machine.
-    /// Set this flag to have windsock run the benches in your configured cloud.
-    #[clap(long, verbatim_doc_comment)]
-    pub cloud: bool,
-
-    /// Windsock will automatically cleanup cloud resources after benches have been run.
-    /// However this command exists to force cleanup in case windsock panicked before automatic cleanup could occur.
-    #[clap(long, verbatim_doc_comment)]
-    pub cleanup_cloud_resources: bool,
-
-    /// Skip running of benches.
-    /// Skip automatic deletion of cloud resources on bench run completion.
-    /// Instead, just create cloud resources and write details of the resources to disk so they may be restored via `--load-cloud-resources-file`
-    #[clap(long, verbatim_doc_comment)]
-    pub store_cloud_resources_file: bool,
+    /// Run all benches that match the specified tag key/values.
+    /// `tag_key=tag_value foo=bar`
+    #[clap(verbatim_doc_comment)]
+    pub filter: Option<String>,
+}
 
-    /// Skip automatic creation of cloud resources on bench run completion.
-    /// Skip automatic deletion of cloud resources on bench run completion.
-    /// Instead, details of the resources are loaded from disk as saved via a previous run using `--store-cloud-resources-file`
-    #[clap(long, verbatim_doc_comment)]
-    pub load_cloud_resources_file: bool,
+impl RunArgs {
+    pub fn filter(&self) -> String {
+        match &self.filter {
+            // convert a name into a filter by swapping commas for spaces
+            Some(filter) => filter.replace(',', " "),
+            // If not provided use the empty filter
+            None => String::new(),
+        }
+    }
+}
 
-    /// Not for human use. Call this from your bench orchestration method to launch your bencher.
-    #[clap(long, verbatim_doc_comment)]
-    pub internal_run: Option<String>,
+#[derive(Parser)]
+#[clap(about=ABOUT)]
+pub struct WindsockArgs {
+    #[command(subcommand)]
+    pub command: Option<Command>,
 
     #[clap(long, hide(true))]
     list: bool,
@@ -131,7 +148,7 @@ pub struct Args {
     ignored: bool,
 
     #[clap(long, hide(true))]
-    exact: bool,
+    pub exact: Option<String>,
 
     #[clap(long, hide(true))]
     nocapture: bool,
@@ -142,7 +159,7 @@ enum NextestFormat {
     Terse,
 }
 
-impl Args {
+impl WindsockArgs {
     pub fn nextest_list(&self) -> bool {
         self.list
     }
@@ -155,17 +172,21 @@ impl Args {
         self.list && matches!(&self.format, Some(NextestFormat::Terse)) && self.ignored
     }
 
-    pub fn nextest_run_by_name(&self) -> bool {
-        self.nocapture && self.exact
+    pub fn nextest_run_by_name(&self) -> Option<&str> {
+        if self.nocapture {
+            self.exact.as_deref()
+        } else {
+            None
+        }
     }
 
     pub fn nextest_invalid_args(&self) -> Option<Error> {
         if self.format.is_some() && self.list {
             Some(anyhow!("`--format` only exists for nextest compatibility and is not supported without `--list`"))
-        } else if self.nocapture && !self.exact {
+        } else if self.nocapture && self.exact.is_none() {
             Some(anyhow!("`--nocapture` only exists for nextest compatibility and is not supported without `--exact`"))
-        } else if self.exact && !self.nocapture {
-            Some(anyhow!("`--nocapture` only exists for nextest compatibility and is not supported without `--nocapture`"))
+        } else if self.exact.is_some() && !self.nocapture {
+            Some(anyhow!("`--exact` only exists for nextest compatibility and is not supported without `--nocapture`"))
         } else {
             None
         }