refactor(cli): merge blocking argument and remove no-logging

Author: omarabid

pyroscope_cli/src/cli/lib.rs

@@ -31,7 +31,7 @@ pub struct Cli {
     command: Commands,
 }
 
-/// Clap Derive for pyroscope-cli commands 
+/// Clap Derive for pyroscope-cli commands
 #[derive(Subcommand, Debug)]
 enum Commands {
     #[clap(
@@ -74,14 +74,6 @@ enum Commands {
             help = "[default: info] log level for the application"
         )]
         log_level: Option<LogLevel>,
-        #[clap(
-            name = "no_logging",
-            long = "no-logging",
-            value_name = "NO_LOGGING",
-            help = "disable logging from pyroscope",
-            takes_value = false
-        )]
-        no_logging: bool,
         #[clap(
             name = "pid",
             long = "pid",
@@ -91,21 +83,13 @@ enum Commands {
         )]
         pid: i32,
         #[clap(
-            name = "rbspy_blocking",
-            long = "rbspy-blocking",
-            value_name = "RBSPY_BLOCKING",
-            help = "enable blocking mode for rbspy",
-            takes_value = false
-        )]
-        rbspy_blocking: bool,
-        #[clap(
-            name = "pyspy_blocking",
-            long = "pyspy-blocking",
-            value_name = "PYSPY_BLOCKING",
-            help = "enable blocking mode for pyspy",
+            name = "blocking",
+            long = "blocking",
+            value_name = "BLOCKING",
+            help = "enable blocking mode. [supported by: rbspy, pyspy]",
             takes_value = false
         )]
-        pyspy_blocking: bool,
+        blocking: bool,
         #[clap(
             name = "pyspy_idle",
             long = "pyspy-idle",
@@ -202,29 +186,13 @@ enum Commands {
         )]
         log_level: Option<LogLevel>,
         #[clap(
-            name = "no_logging",
-            long = "no-logging",
-            value_name = "NO_LOGGING",
-            help = "disable logging from pyroscope",
-            takes_value = false
-        )]
-        no_logging: bool,
-        #[clap(
-            name = "rbspy_blocking",
-            long = "rbspy-blocking",
-            value_name = "RBSPY_BLOCKING",
-            help = "enable blocking mode for rbspy",
-            takes_value = false
-        )]
-        rbspy_blocking: bool,
-        #[clap(
-            name = "pyspy_blocking",
-            long = "pyspy-blocking",
-            value_name = "PYSPY_BLOCKING",
-            help = "enable blocking mode for pyspy",
+            name = "blocking",
+            long = "blocking",
+            value_name = "BLOCKING",
+            help = "enable blocking mode. [supported by: rbspy, pyspy]",
             takes_value = false
         )]
-        pyspy_blocking: bool,
+        blocking: bool,
         #[clap(
             name = "pyspy_idle",
             long = "pyspy-idle",

pyroscope_cli/src/core/profiler.rs

@@ -25,9 +25,7 @@ impl Profiler {
 
         let sample_rate: u32 = AppConfig::get::<u32>("sample_rate")?;
 
-        // TODO: CLI should probably unify this into a single argument
-        let rbspy_blocking: bool = AppConfig::get::<bool>("rbspy_blocking")?;
-        let pyspy_blocking: bool = AppConfig::get::<bool>("pyspy_blocking")?;
+        let blocking: bool = AppConfig::get::<bool>("blocking")?;
 
         let pyspy_idle: bool = AppConfig::get::<bool>("pyspy_idle")?;
         let pyspy_gil: bool = AppConfig::get::<bool>("pyspy_gil")?;
@@ -42,7 +40,7 @@ impl Profiler {
             Spy::Pyspy => {
                 let config = PyspyConfig::new(pid)
                     .sample_rate(sample_rate)
-                    .lock_process(pyspy_blocking)
+                    .lock_process(blocking)
                     .with_subprocesses(detect_subprocesses)
                     .include_idle(pyspy_idle)
                     .gil_only(pyspy_gil)
@@ -56,7 +54,7 @@ impl Profiler {
             Spy::Rbspy => {
                 let config = RbspyConfig::new(pid)
                     .sample_rate(sample_rate)
-                    .lock_process(rbspy_blocking)
+                    .lock_process(blocking)
                     .with_subprocesses(detect_subprocesses);
                 let backend = Rbspy::new(config);
                 PyroscopeAgent::builder(server_address, app_name)

pyroscope_cli/src/utils/app_config.rs

@@ -23,8 +23,7 @@ pub struct AppConfig {
     pub no_logging: Option<bool>,
     pub log_level: LogLevel,
     pub no_root_drop: Option<bool>,
-    pub rbspy_blocking: Option<bool>,
-    pub pyspy_blocking: Option<bool>,
+    pub blocking: Option<bool>,
     pub pyspy_idle: Option<bool>,
     pub pyspy_gil: Option<bool>,
     pub pyspy_native: Option<bool>,
@@ -75,11 +74,8 @@ impl AppConfig {
                     AppConfig::set("log_level", log_level)?;
                 }
             }
-            if sub_connect.is_present("no_logging") {
-                AppConfig::set("no_logging", "true")?;
-            }
-            if sub_connect.is_present("pyspy_blocking") {
-                AppConfig::set("pyspy_blocking", "true")?;
+            if sub_connect.is_present("blocking") {
+                AppConfig::set("blocking", "true")?;
             }
             if sub_connect.is_present("pyspy_idle") {
                 AppConfig::set("pyspy_idle", "true")?;
@@ -90,9 +86,6 @@ impl AppConfig {
             if sub_connect.is_present("pyspy_native") {
                 AppConfig::set("pyspy_native", "true")?;
             }
-            if sub_connect.is_present("rbspy_blocking") {
-                AppConfig::set("rbspy_blocking", "true")?;
-            }
             if sub_connect.is_present("sample_rate") {
                 if let Some(sample_rate) = sub_connect.value_of("sample_rate") {
                     AppConfig::set("sample_rate", sample_rate)?;
@@ -161,11 +154,8 @@ impl AppConfig {
                     AppConfig::set("log_level", log_level)?;
                 }
             }
-            if sub_exec.is_present("no_logging") {
-                AppConfig::set("no_logging", "true")?;
-            }
-            if sub_exec.is_present("pyspy_blocking") {
-                AppConfig::set("pyspy_blocking", "true")?;
+            if sub_exec.is_present("blocking") {
+                AppConfig::set("blocking", "true")?;
             }
             if sub_exec.is_present("pyspy_idle") {
                 AppConfig::set("pyspy_idle", "true")?;
@@ -176,9 +166,6 @@ impl AppConfig {
             if sub_exec.is_present("pyspy_native") {
                 AppConfig::set("pyspy_native", "true")?;
             }
-            if sub_exec.is_present("rbspy_blocking") {
-                AppConfig::set("rbspy_blocking", "true")?;
-            }
             if sub_exec.is_present("sample_rate") {
                 if let Some(sample_rate) = sub_exec.value_of("sample_rate") {
                     AppConfig::set("sample_rate", sample_rate)?;