Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Jun 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions quickwit/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 3 additions & 2 deletions quickwit/quickwit-cli/src/logger.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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());
}
Expand All @@ -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.
Expand Down
58 changes: 52 additions & 6 deletions quickwit/quickwit-common/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
}
2 changes: 1 addition & 1 deletion quickwit/quickwit-common/src/runtimes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
10 changes: 6 additions & 4 deletions quickwit/quickwit-config/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)]
Expand Down
4 changes: 2 additions & 2 deletions quickwit/quickwit-config/src/node_config/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ impl IndexerConfig {
}
#[cfg(not(any(test, feature = "testsuite")))]
{
true
quickwit_common::get_bool_from_env("QW_ENABLE_OTLP_ENDPOINT", true)
}
}

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

Expand Down
7 changes: 6 additions & 1 deletion quickwit/quickwit-config/src/storage_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down Expand Up @@ -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)
}
}

Expand Down
12 changes: 7 additions & 5 deletions quickwit/quickwit-lambda/src/environment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
8 changes: 5 additions & 3 deletions quickwit/quickwit-lambda/src/indexer/environment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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| {
Expand Down
4 changes: 2 additions & 2 deletions quickwit/quickwit-serve/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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")?;
Expand Down
2 changes: 2 additions & 0 deletions quickwit/quickwit-telemetry/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }

Expand Down
2 changes: 1 addition & 1 deletion quickwit/quickwit-telemetry/src/sender.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>) {
Expand Down
Loading