Take bool env var values into account instead of presence (#5095)

quickwit/quickwit-cli/src/logger.rs

@@ -26,6 +26,7 @@ use opentelemetry::sdk::trace::BatchConfig;
 use opentelemetry::sdk::{trace, Resource};
 use opentelemetry::{global, KeyValue};
 use opentelemetry_otlp::WithExportConfig;
+use quickwit_common::get_bool_from_env;
 use quickwit_serve::{BuildInfo, EnvFilterReloadFn};
 use tracing::Level;
 use tracing_subscriber::fmt::time::UtcTime;
@@ -43,7 +44,7 @@ pub fn setup_logging_and_tracing(
 ) -> anyhow::Result<EnvFilterReloadFn> {
     #[cfg(feature = "tokio-console")]
     {
-        if std::env::var_os(QW_ENABLE_TOKIO_CONSOLE_ENV_KEY).is_some() {
+        if get_bool_from_env(QW_ENABLE_TOKIO_CONSOLE_ENV_KEY, false) {
             console_subscriber::init();
             return Ok(quickwit_serve::do_nothing_env_filter_reload_fn());
         }
@@ -69,7 +70,7 @@ pub fn setup_logging_and_tracing(
         );
     // Note on disabling ANSI characters: setting the ansi boolean on event format is insufficient.
     // It is thus set on layers, see https://github.com/tokio-rs/tracing/issues/1817
-    if std::env::var_os(QW_ENABLE_OPENTELEMETRY_OTLP_EXPORTER_ENV_KEY).is_some() {
+    if get_bool_from_env(QW_ENABLE_OPENTELEMETRY_OTLP_EXPORTER_ENV_KEY, false) {
         let otlp_exporter = opentelemetry_otlp::new_exporter().tonic().with_env();
         // In debug mode, Quickwit can generate a lot of spans, and the default queue size of 2048
         // is too small.

quickwit/quickwit-common/src/lib.rs

@@ -83,26 +83,39 @@ pub fn split_file(split_id: impl Display) -> String {
 pub fn get_from_env<T: FromStr + Debug>(key: &str, default_value: T) -> T {
     if let Ok(value_str) = std::env::var(key) {
         if let Ok(value) = T::from_str(&value_str) {
-            info!(value=?value, "setting `{}` from environment", key);
+            info!(value=?value, "using environment variable `{key}` value");
             return value;
         } else {
-            error!(value_str=%value_str, "failed to parse `{}` from environment", key);
+            error!(value=%value_str, "failed to parse environment variable `{key}` value");
         }
     }
-    info!(value=?default_value, "setting `{}` from default", key);
+    info!(value=?default_value, "using environment variable `{key}` default value");
+    default_value
+}
+
+pub fn get_bool_from_env(key: &str, default_value: bool) -> bool {
+    if let Ok(value_str) = std::env::var(key) {
+        if let Some(value) = parse_bool_lenient(&value_str) {
+            info!(value=%value, "using environment variable `{key}` value");
+            return value;
+        } else {
+            error!(value=%value_str, "failed to parse environment variable `{key}` value");
+        }
+    }
+    info!(value=?default_value, "using environment variable `{key}` default value");
     default_value
 }
 
 pub fn get_from_env_opt<T: FromStr + Debug>(key: &str) -> Option<T> {
     let Some(value_str) = std::env::var(key).ok() else {
-        info!("{key} is not set");
+        info!("environment variable `{key}` is not set");
         return None;
     };
     if let Ok(value) = T::from_str(&value_str) {
-        info!(value=?value, "setting `{}` from environment", key);
+        info!(value=?value, "using environment variable `{key}` value");
         Some(value)
     } else {
-        error!(value_str=%value_str, "failed to parse `{}` from environment", key);
+        error!(value=%value_str, "failed to parse environment variable `{key}` value");
         None
     }
 }
@@ -269,6 +282,22 @@ where
         .unwrap()
 }
 
+pub fn parse_bool_lenient(bool_str: &str) -> Option<bool> {
+    let trimmed_bool_str = bool_str.trim();
+
+    for truthy_value in ["true", "yes", "1"] {
+        if trimmed_bool_str.eq_ignore_ascii_case(truthy_value) {
+            return Some(true);
+        }
+    }
+    for falsy_value in ["false", "no", "0"] {
+        if trimmed_bool_str.eq_ignore_ascii_case(falsy_value) {
+            return Some(false);
+        }
+    }
+    None
+}
+
 #[cfg(test)]
 mod tests {
     use std::io::ErrorKind;
@@ -343,4 +372,21 @@ mod tests {
         assert_eq!(div_ceil_u32(1, 3), 1);
         assert_eq!(div_ceil_u32(0, 3), 0);
     }
+
+    #[test]
+    fn test_parse_bool_lenient() {
+        assert_eq!(parse_bool_lenient("true"), Some(true));
+        assert_eq!(parse_bool_lenient("TRUE"), Some(true));
+        assert_eq!(parse_bool_lenient("True"), Some(true));
+        assert_eq!(parse_bool_lenient("yes"), Some(true));
+        assert_eq!(parse_bool_lenient(" 1"), Some(true));
+
+        assert_eq!(parse_bool_lenient("false"), Some(false));
+        assert_eq!(parse_bool_lenient("FALSE"), Some(false));
+        assert_eq!(parse_bool_lenient("False"), Some(false));
+        assert_eq!(parse_bool_lenient("no"), Some(false));
+        assert_eq!(parse_bool_lenient("0 "), Some(false));
+
+        assert_eq!(parse_bool_lenient("foo"), None);
+    }
 }

quickwit/quickwit-common/src/runtimes.rs

@@ -90,7 +90,7 @@ impl Default for RuntimesConfig {
 fn start_runtimes(config: RuntimesConfig) -> HashMap<RuntimeType, Runtime> {
     let mut runtimes = HashMap::with_capacity(2);
 
-    let disable_lifo_slot: bool = crate::get_from_env("QW_DISABLE_TOKIO_LIFO_SLOT", false);
+    let disable_lifo_slot = crate::get_bool_from_env("QW_DISABLE_TOKIO_LIFO_SLOT", false);
 
     let mut blocking_runtime_builder = tokio::runtime::Builder::new_multi_thread();
     if disable_lifo_slot {

quickwit/quickwit-config/src/lib.rs

@@ -19,12 +19,12 @@
 
 #![deny(clippy::disallowed_methods)]
 
-use std::env;
 use std::str::FromStr;
 
 use anyhow::{bail, ensure, Context};
 use json_comments::StripComments;
 use once_cell::sync::Lazy;
+use quickwit_common::get_bool_from_env;
 use quickwit_common::net::is_valid_hostname;
 use quickwit_common::uri::Uri;
 use quickwit_proto::types::NodeIdRef;
@@ -83,14 +83,16 @@ pub use crate::storage_config::{
 
 /// Returns true if the ingest API v2 is enabled.
 pub fn enable_ingest_v2() -> bool {
-    static ENABLE_INGEST_V2: Lazy<bool> = Lazy::new(|| env::var("QW_ENABLE_INGEST_V2").is_ok());
+    static ENABLE_INGEST_V2: Lazy<bool> =
+        Lazy::new(|| get_bool_from_env("QW_ENABLE_INGEST_V2", false));
     *ENABLE_INGEST_V2
 }
 
 /// Returns true if the ingest API v1 is disabled.
 pub fn disable_ingest_v1() -> bool {
-    static ENABLE_INGEST_V2: Lazy<bool> = Lazy::new(|| env::var("QW_DISABLE_INGEST_V1").is_ok());
-    *ENABLE_INGEST_V2
+    static DISABLE_INGEST_V1: Lazy<bool> =
+        Lazy::new(|| get_bool_from_env("QW_DISABLE_INGEST_V1", false));
+    *DISABLE_INGEST_V1
 }
 
 #[derive(utoipa::OpenApi)]

quickwit/quickwit-config/src/node_config/mod.rs

@@ -122,7 +122,7 @@ impl IndexerConfig {
         }
         #[cfg(not(any(test, feature = "testsuite")))]
         {
-            true
+            quickwit_common::get_bool_from_env("QW_ENABLE_OTLP_ENDPOINT", true)
         }
     }
 
@@ -365,7 +365,7 @@ impl JaegerConfig {
         }
         #[cfg(not(any(test, feature = "testsuite")))]
         {
-            true
+            quickwit_common::get_bool_from_env("QW_ENABLE_JAEGER_ENDPOINT", true)
         }
     }
 

quickwit/quickwit-config/src/storage_config.rs

@@ -22,6 +22,7 @@ use std::{env, fmt};
 
 use anyhow::ensure;
 use itertools::Itertools;
+use quickwit_common::get_bool_from_env;
 use serde::{Deserialize, Serialize};
 use serde_with::{serde_as, EnumMap};
 
@@ -369,7 +370,11 @@ impl S3StorageConfig {
     }
 
     pub fn force_path_style_access(&self) -> Option<bool> {
-        Some(env::var("QW_S3_FORCE_PATH_STYLE_ACCESS").is_ok() || self.force_path_style_access)
+        let force_path_style_access = get_bool_from_env(
+            "QW_S3_FORCE_PATH_STYLE_ACCESS",
+            self.force_path_style_access,
+        );
+        Some(force_path_style_access)
     }
 }
 

quickwit/quickwit-lambda/src/environment.rs

@@ -20,15 +20,17 @@
 use std::env::var;
 
 use once_cell::sync::Lazy;
+use quickwit_common::get_bool_from_env;
 
-pub static INDEX_ID: Lazy<String> =
-    Lazy::new(|| var("QW_LAMBDA_INDEX_ID").expect("QW_LAMBDA_INDEX_ID must be set"));
+pub static INDEX_ID: Lazy<String> = Lazy::new(|| {
+    var("QW_LAMBDA_INDEX_ID").expect("environment variable `QW_LAMBDA_INDEX_ID` should be set")
+});
 
-/// Configure the fmt tracing subscriber to log as json and include span
+/// Configures the fmt tracing subscriber to log as json and include span
 /// boundaries. This is very verbose and is only used to generate advanced KPIs
-/// from Lambda runs (e.g for blog post benchmarks)
+/// from Lambda runs (e.g. for blog post benchmarks)
 pub static ENABLE_VERBOSE_JSON_LOGS: Lazy<bool> =
-    Lazy::new(|| var("QW_LAMBDA_ENABLE_VERBOSE_JSON_LOGS").is_ok_and(|v| v.as_str() == "true"));
+    Lazy::new(|| get_bool_from_env("QW_LAMBDA_ENABLE_VERBOSE_JSON_LOGS", false));
 
 pub static OPENTELEMETRY_URL: Lazy<Option<String>> =
     Lazy::new(|| var("QW_LAMBDA_OPENTELEMETRY_URL").ok());

quickwit/quickwit-lambda/src/indexer/environment.rs

@@ -20,6 +20,7 @@
 use std::env::var;
 
 use once_cell::sync::Lazy;
+use quickwit_common::get_bool_from_env;
 
 pub const CONFIGURATION_TEMPLATE: &str = r#"
 version: 0.8
@@ -31,14 +32,15 @@ data_dir: /tmp
 "#;
 
 pub static INDEX_CONFIG_URI: Lazy<String> = Lazy::new(|| {
-    var("QW_LAMBDA_INDEX_CONFIG_URI").expect("QW_LAMBDA_INDEX_CONFIG_URI must be set")
+    var("QW_LAMBDA_INDEX_CONFIG_URI")
+        .expect("environment variable `QW_LAMBDA_INDEX_CONFIG_URI` should be set")
 });
 
 pub static DISABLE_MERGE: Lazy<bool> =
-    Lazy::new(|| var("QW_LAMBDA_DISABLE_MERGE").is_ok_and(|v| v.as_str() == "true"));
+    Lazy::new(|| get_bool_from_env("QW_LAMBDA_DISABLE_MERGE", false));
 
 pub static DISABLE_JANITOR: Lazy<bool> =
-    Lazy::new(|| var("QW_LAMBDA_DISABLE_JANITOR").is_ok_and(|v| v.as_str() == "true"));
+    Lazy::new(|| get_bool_from_env("QW_LAMBDA_DISABLE_JANITOR", false));
 
 pub static MAX_CHECKPOINTS: Lazy<usize> = Lazy::new(|| {
     var("QW_LAMBDA_MAX_CHECKPOINTS").map_or(100, |v| {

quickwit/quickwit-serve/src/lib.rs

@@ -70,13 +70,13 @@ use quickwit_common::pubsub::{EventBroker, EventSubscriptionHandle};
 use quickwit_common::rate_limiter::RateLimiterSettings;
 use quickwit_common::retry::RetryParams;
 use quickwit_common::runtimes::RuntimesConfig;
-use quickwit_common::spawn_named_task;
 use quickwit_common::tower::{
     BalanceChannel, BoxFutureInfaillible, BufferLayer, Change, ConstantRate, EstimateRateLayer,
     EventListenerLayer, GrpcMetricsLayer, LoadShedLayer, RateLimitLayer, RetryLayer, RetryPolicy,
     SmaRateEstimator,
 };
 use quickwit_common::uri::Uri;
+use quickwit_common::{get_bool_from_env, spawn_named_task};
 use quickwit_config::service::QuickwitService;
 use quickwit_config::{ClusterConfig, NodeConfig};
 use quickwit_control_plane::control_plane::{ControlPlane, ControlPlaneEventSubscriber};
@@ -634,7 +634,7 @@ pub async fn serve_quickwit(
             search_job_placer,
             storage_resolver.clone(),
             event_broker.clone(),
-            std::env::var(DISABLE_DELETE_TASK_SERVICE_ENV_KEY).is_err(),
+            !get_bool_from_env(DISABLE_DELETE_TASK_SERVICE_ENV_KEY, false),
         )
         .await
         .context("failed to start janitor service")?;

quickwit/quickwit-telemetry/Cargo.toml

@@ -26,6 +26,8 @@ uuid = { workspace = true }
 # used by reqwest. 0.8.30 has an unclear license.
 encoding_rs = { workspace = true }
 
+quickwit-common = { workspace = true }
+
 [dev-dependencies]
 serde_json = { workspace = true }
 

quickwit/quickwit-telemetry/src/sender.rs

@@ -309,7 +309,7 @@ pub fn is_telemetry_disabled() -> bool {
 /// Check to see if telemetry is enabled.
 #[cfg(not(test))]
 pub fn is_telemetry_disabled() -> bool {
-    std::env::var_os(crate::DISABLE_TELEMETRY_ENV_KEY).is_some()
+    quickwit_common::get_bool_from_env(crate::DISABLE_TELEMETRY_ENV_KEY, false)
 }
 
 fn start_monitor_if_server_running_task(telemetry_sender: Arc<Inner>) {