merge: #3270

3270: chore: support opt-in ANSI coloring of telemetry logging output r=fnichol a=fnichol This change updates the telemetry setup and adds a couple of extra CLI arguments and environment variables to help control when it's appropriate for logging output to contain ANSI escape characters. Each service has a new CLI argument of `--no-color` which will disable ANSI coloring of telemetry output when set. An associated environment variable of `SI_NO_COLOR` is also supported by setting the value to either `true` or `false`. Finally as a fallback of last resort, and to better support other operational ecosystems the `NO_COLOR` environment variable is also inspected and will disable coloring if this value is set to something non-empty. In an attempt to make our services better behaved as Unix programs, they will now automatically disable ANSI coloring if the standard output of the process doesn't refer to a terminal or TTY (that is, the session is "non-interactive"). Our Tilt development environment can support output from processes with ANSI coloring, however these processes are considered "non-interactive" and thus won't be colored by default. As a consequence of this, there is one more new CLI argument of `--force-color` which will override the terminal/TTY detection logic and will unconditionally add ANSI coloring. An associated environment variable of `SI_FORCE_COLOR` will also work in place of using the `--force-color` CLI flag. <img src="https://media4.giphy.com/media/SKGo6OYe24EBG/giphy.gif"/> Co-authored-by: Fletcher Nichol <[email protected]>
systeminit · Feb 6, 2024 · dc4de4a · dc4de4a
2 parents 9215422 + e9a7203
commit dc4de4a
Show file tree

Hide file tree

Showing 14 changed files with 272 additions and 76 deletions.
diff --git a/bin/council/src/args.rs b/bin/council/src/args.rs
@@ -19,6 +19,28 @@ pub(crate) struct Args {
     #[arg(short = 'v', long = "verbose", action = ArgAction::Count)]
     pub(crate) verbose: u8,
 
+    /// Disables ANSI coloring in log output, even if standard output refers to a terminal/TTY.
+    ///
+    /// For more details, visit: <http://no-color.org/>.
+    #[arg(
+        long,
+        env = "SI_NO_COLOR",
+        hide_env_values = true,
+        conflicts_with = "force_color"
+    )]
+    pub(crate) no_color: Option<bool>,
+
+    /// Forces ANSI coloring, even if standard output refers to a terminal/TTY.
+    ///
+    /// For more details, visit: <http://no-color.org/>.
+    #[arg(
+        long,
+        env = "SI_FORCE_COLOR",
+        hide_env_values = true,
+        conflicts_with = "no_color"
+    )]
+    pub(crate) force_color: Option<bool>,
+
     /// NATS connection URL [example: demo.nats.io]
     #[arg(long)]
     pub(crate) nats_url: Option<String>,
@@ -30,10 +52,6 @@ pub(crate) struct Args {
     /// NATS credentials file
     #[arg(long)]
     pub(crate) nats_creds_path: Option<String>,
-
-    /// Disable OpenTelemetry on startup
-    #[arg(long)]
-    pub(crate) disable_opentelemetry: bool,
 }
 
 impl TryFrom<Args> for Config {

diff --git a/bin/council/src/main.rs b/bin/council/src/main.rs
@@ -25,23 +25,32 @@ async fn async_main() -> Result<()> {
     let task_tracker = TaskTracker::new();
 
     color_eyre::install()?;
-    let config = TelemetryConfig::builder()
-        .service_name("council")
-        .service_namespace("si")
-        .log_env_var_prefix("SI")
-        .app_modules(vec!["council", "council_server"])
-        .build()?;
-    let (mut telemetry, telemetry_shutdown) =
-        telemetry_application::init(config, &task_tracker, shutdown_token.clone())?;
     let args = args::parse();
+    let (mut telemetry, telemetry_shutdown) = {
+        let mut builder = TelemetryConfig::builder();
+        builder
+            .service_name("council")
+            .service_namespace("si")
+            .log_env_var_prefix("SI")
+            .app_modules(vec!["council", "council_server"]);
+        if let Some(force_color) = args.force_color {
+            builder.force_color(force_color);
+        }
+        if let Some(no_color) = args.no_color {
+            builder.no_color(no_color);
+        }
+        let config = builder.build()?;
 
-    let (_shutdown_request_tx, shutdown_request_rx) = watch::channel(());
+        telemetry_application::init(config, &task_tracker, shutdown_token.clone())?
+    };
 
     if args.verbose > 0 {
         telemetry.set_verbosity(args.verbose.into()).await?;
     }
     trace!(arguments =?args, "parsed cli arguments");
 
+    let (_shutdown_request_tx, shutdown_request_rx) = watch::channel(());
+
     task_tracker.close();
 
     let config = council_server::server::Config::try_from(args)?;

diff --git a/bin/cyclone/src/args.rs b/bin/cyclone/src/args.rs
@@ -24,9 +24,27 @@ pub(crate) struct Args {
     #[arg(short = 'v', long = "verbose", action = ArgAction::Count)]
     pub(crate) verbose: u8,
 
-    /// Disable OpenTelemetry on startup
-    #[arg(long)]
-    pub(crate) disable_opentelemetry: bool,
+    /// Disables ANSI coloring in log output, even if standard output refers to a terminal/TTY.
+    ///
+    /// For more details, visit: <http://no-color.org/>.
+    #[arg(
+        long,
+        env = "SI_NO_COLOR",
+        hide_env_values = true,
+        conflicts_with = "force_color"
+    )]
+    pub(crate) no_color: Option<bool>,
+
+    /// Forces ANSI coloring, even if standard output refers to a terminal/TTY.
+    ///
+    /// For more details, visit: <http://no-color.org/>.
+    #[arg(
+        long,
+        env = "SI_FORCE_COLOR",
+        hide_env_values = true,
+        conflicts_with = "no_color"
+    )]
+    pub(crate) force_color: Option<bool>,
 
     /// Binds service to a socket address [example: 0.0.0.0:5157]
     #[arg(long, group = "bind")]

diff --git a/bin/cyclone/src/main.rs b/bin/cyclone/src/main.rs
@@ -19,16 +19,25 @@ async fn main() -> Result<()> {
     let task_tracker = TaskTracker::new();
 
     color_eyre::install()?;
-    let config = TelemetryConfig::builder()
-        .service_name("cyclone")
-        .service_namespace("si")
-        .log_env_var_prefix("SI")
-        .app_modules(vec!["cyclone", "cyclone_server"])
-        .custom_default_tracing_level(CUSTOM_DEFAULT_TRACING_LEVEL)
-        .build()?;
-    let (mut telemetry, telemetry_shutdown) =
-        telemetry_application::init(config, &task_tracker, shutdown_token.clone())?;
     let args = args::parse();
+    let (mut telemetry, telemetry_shutdown) = {
+        let mut builder = TelemetryConfig::builder();
+        builder
+            .service_name("cyclone")
+            .service_namespace("si")
+            .log_env_var_prefix("SI")
+            .app_modules(vec!["cyclone", "cyclone_server"])
+            .custom_default_tracing_level(CUSTOM_DEFAULT_TRACING_LEVEL);
+        if let Some(force_color) = args.force_color {
+            builder.force_color(force_color);
+        }
+        if let Some(no_color) = args.no_color {
+            builder.no_color(no_color);
+        }
+        let config = builder.build()?;
+
+        telemetry_application::init(config, &task_tracker, shutdown_token.clone())?
+    };
 
     if args.verbose > 0 {
         telemetry.set_verbosity(args.verbose.into()).await?;

diff --git a/bin/module-index/src/args.rs b/bin/module-index/src/args.rs
@@ -21,6 +21,28 @@ pub(crate) struct Args {
     #[arg(short = 'v', long = "verbose", action = ArgAction::Count)]
     pub(crate) verbose: u8,
 
+    /// Disables ANSI coloring in log output, even if standard output refers to a terminal/TTY.
+    ///
+    /// For more details, visit: <http://no-color.org/>.
+    #[arg(
+        long,
+        env = "SI_NO_COLOR",
+        hide_env_values = true,
+        conflicts_with = "force_color"
+    )]
+    pub(crate) no_color: Option<bool>,
+
+    /// Forces ANSI coloring, even if standard output refers to a terminal/TTY.
+    ///
+    /// For more details, visit: <http://no-color.org/>.
+    #[arg(
+        long,
+        env = "SI_FORCE_COLOR",
+        hide_env_values = true,
+        conflicts_with = "no_color"
+    )]
+    pub(crate) force_color: Option<bool>,
+
     /// PostgreSQL connection pool dbname [example: myapp]
     #[arg(long, env)]
     pub(crate) pg_dbname: Option<String>,
@@ -80,14 +102,8 @@ pub(crate) struct Args {
     /// The path to the JWT public signing key
     #[arg(long, env)]
     pub(crate) jwt_public_key: Option<String>,
-
     // /// Database migration mode on startup
     // #[arg(long, value_parser = PossibleValuesParser::new(MigrationMode::variants()))]
-
-    // pub(crate) migration_mode: Option<String>,
-    /// Disable OpenTelemetry on startup
-    #[arg(long)]
-    pub(crate) disable_opentelemetry: bool,
 }
 
 impl TryFrom<Args> for Config {

diff --git a/bin/module-index/src/main.rs b/bin/module-index/src/main.rs
@@ -25,15 +25,24 @@ async fn async_main() -> Result<()> {
     let task_tracker = TaskTracker::new();
 
     color_eyre::install()?;
-    let config = TelemetryConfig::builder()
-        .service_name("module-index")
-        .service_namespace("si")
-        .log_env_var_prefix("SI")
-        .app_modules(vec!["module_index", "module_index_server"])
-        .build()?;
-    let (mut telemetry, telemetry_shutdown) =
-        telemetry_application::init(config, &task_tracker, shutdown_token.clone())?;
     let args = args::parse();
+    let (mut telemetry, telemetry_shutdown) = {
+        let mut builder = TelemetryConfig::builder();
+        builder
+            .service_name("module-index")
+            .service_namespace("si")
+            .log_env_var_prefix("SI")
+            .app_modules(vec!["module_index", "module_index_server"]);
+        if let Some(force_color) = args.force_color {
+            builder.force_color(force_color);
+        }
+        if let Some(no_color) = args.no_color {
+            builder.no_color(no_color);
+        }
+        let config = builder.build()?;
+
+        telemetry_application::init(config, &task_tracker, shutdown_token.clone())?
+    };
 
     if args.verbose > 0 {
         telemetry.set_verbosity(args.verbose.into()).await?;

diff --git a/bin/pinga/src/args.rs b/bin/pinga/src/args.rs
@@ -21,6 +21,28 @@ pub(crate) struct Args {
     #[arg(short = 'v', long = "verbose", action = ArgAction::Count)]
     pub(crate) verbose: u8,
 
+    /// Disables ANSI coloring in log output, even if standard output refers to a terminal/TTY.
+    ///
+    /// For more details, visit: <http://no-color.org/>.
+    #[arg(
+        long,
+        env = "SI_NO_COLOR",
+        hide_env_values = true,
+        conflicts_with = "force_color"
+    )]
+    pub(crate) no_color: Option<bool>,
+
+    /// Forces ANSI coloring, even if standard output refers to a terminal/TTY.
+    ///
+    /// For more details, visit: <http://no-color.org/>.
+    #[arg(
+        long,
+        env = "SI_FORCE_COLOR",
+        hide_env_values = true,
+        conflicts_with = "no_color"
+    )]
+    pub(crate) force_color: Option<bool>,
+
     /// PostgreSQL connection pool dbname [example: myapp]
     #[arg(long)]
     pub(crate) pg_dbname: Option<String>,
@@ -61,10 +83,6 @@ pub(crate) struct Args {
     #[arg(long)]
     pub(crate) nats_creds_path: Option<String>,
 
-    /// Disable OpenTelemetry on startup
-    #[arg(long)]
-    pub(crate) disable_opentelemetry: bool,
-
     /// Cyclone encryption key file location [default: /run/pinga/cyclone_encryption.key]
     #[arg(long)]
     pub(crate) cyclone_encryption_key_path: Option<String>,

diff --git a/bin/pinga/src/main.rs b/bin/pinga/src/main.rs
@@ -25,15 +25,24 @@ async fn async_main() -> Result<()> {
     let task_tracker = TaskTracker::new();
 
     color_eyre::install()?;
-    let config = TelemetryConfig::builder()
-        .service_name("pinga")
-        .service_namespace("si")
-        .log_env_var_prefix("SI")
-        .app_modules(vec!["pinga", "pinga_server"])
-        .build()?;
-    let (mut telemetry, telemetry_shutdown) =
-        telemetry_application::init(config, &task_tracker, shutdown_token.clone())?;
     let args = args::parse();
+    let (mut telemetry, telemetry_shutdown) = {
+        let mut builder = TelemetryConfig::builder();
+        builder
+            .service_name("pinga")
+            .service_namespace("si")
+            .log_env_var_prefix("SI")
+            .app_modules(vec!["pinga", "pinga_server"]);
+        if let Some(force_color) = args.force_color {
+            builder.force_color(force_color);
+        }
+        if let Some(no_color) = args.no_color {
+            builder.no_color(no_color);
+        }
+        let config = builder.build()?;
+
+        telemetry_application::init(config, &task_tracker, shutdown_token.clone())?
+    };
 
     if args.verbose > 0 {
         telemetry.set_verbosity(args.verbose.into()).await?;

diff --git a/bin/sdf/src/args.rs b/bin/sdf/src/args.rs
@@ -23,6 +23,28 @@ pub(crate) struct Args {
     #[arg(short = 'v', long = "verbose", action = ArgAction::Count)]
     pub(crate) verbose: u8,
 
+    /// Disables ANSI coloring in log output, even if standard output refers to a terminal/TTY.
+    ///
+    /// For more details, visit: <http://no-color.org/>.
+    #[arg(
+        long,
+        env = "SI_NO_COLOR",
+        hide_env_values = true,
+        conflicts_with = "force_color"
+    )]
+    pub(crate) no_color: Option<bool>,
+
+    /// Forces ANSI coloring, even if standard output refers to a terminal/TTY.
+    ///
+    /// For more details, visit: <http://no-color.org/>.
+    #[arg(
+        long,
+        env = "SI_FORCE_COLOR",
+        hide_env_values = true,
+        conflicts_with = "no_color"
+    )]
+    pub(crate) force_color: Option<bool>,
+
     /// PostgreSQL connection pool dbname [example: myapp]
     #[arg(long)]
     pub(crate) pg_dbname: Option<String>,
@@ -67,10 +89,6 @@ pub(crate) struct Args {
     #[arg(long, value_parser = PossibleValuesParser::new(MigrationMode::variants()))]
     pub(crate) migration_mode: Option<String>,
 
-    /// Disable OpenTelemetry on startup
-    #[arg(long)]
-    pub(crate) disable_opentelemetry: bool,
-
     /// Cyclone encryption key file location [default: /run/sdf/cyclone_encryption.key]
     #[arg(long)]
     pub(crate) cyclone_encryption_key_path: Option<String>,

diff --git a/bin/sdf/src/main.rs b/bin/sdf/src/main.rs
@@ -36,15 +36,24 @@ async fn async_main() -> Result<()> {
     let task_tracker = TaskTracker::new();
 
     color_eyre::install()?;
-    let config = TelemetryConfig::builder()
-        .service_name("sdf")
-        .service_namespace("si")
-        .log_env_var_prefix("SI")
-        .app_modules(vec!["sdf", "sdf_server"])
-        .build()?;
-    let (mut telemetry, telemetry_shutdown) =
-        telemetry_application::init(config, &task_tracker, shutdown_token.clone())?;
     let args = args::parse();
+    let (mut telemetry, telemetry_shutdown) = {
+        let mut builder = TelemetryConfig::builder();
+        builder
+            .service_name("sdf")
+            .service_namespace("si")
+            .log_env_var_prefix("SI")
+            .app_modules(vec!["sdf", "sdf_server"]);
+        if let Some(force_color) = args.force_color {
+            builder.force_color(force_color);
+        }
+        if let Some(no_color) = args.no_color {
+            builder.no_color(no_color);
+        }
+        let config = builder.build()?;
+
+        telemetry_application::init(config, &task_tracker, shutdown_token.clone())?
+    };
 
     if args.verbose > 0 {
         telemetry.set_verbosity(args.verbose.into()).await?;