Merge pull request #322 from Kuadrant/env_vars

Env vars consolidation & docs update
Kuadrant · May 15, 2024 · 372fd7a · 372fd7a
2 parents 37bb70b + 5767c5f
commit 372fd7a
Show file tree

Hide file tree

Showing 3 changed files with 38 additions and 45 deletions.
diff --git a/doc/server/configuration.md b/doc/server/configuration.md
@@ -224,11 +224,11 @@ Arguments:
   <URL>  Redis URL to use
 
 Options:
-      --ttl <TTL>             TTL for cached counters in milliseconds [default: 5000]
-      --ratio <ratio>         Ratio to apply to the TTL from Redis on cached counters [default: 10000]
-      --flush-period <flush>  Flushing period for counters in milliseconds [default: 1000]
-      --max-cached <max>      Maximum amount of counters cached [default: 10000]
-  -h, --help                  Print help
+      --batch-size <batch>          Size of entries to flush in as single flush [default: 100]
+      --flush-period <flush>        Flushing period for counters in milliseconds [default: 1000]
+      --max-cached <max>            Maximum amount of counters cached [default: 10000]
+      --response-timeout <timeout>  Timeout for Redis commands in milliseconds [default: 350]
+  -h, --help                        Print help
 ```
 
 #### `disk`
@@ -375,29 +375,20 @@ sacrifices some rate-limit accuracy. This mode does two things:
 
 #### `REDIS_LOCAL_CACHE_FLUSHING_PERIOD_MS`
 
-- Used to configure the local cache when using Redis. See
+- Used to configure the maximum flushing period. See
 [`REDIS_LOCAL_CACHE_ENABLED`](#redis_local_cache_enabled). This env only applies
 when `"REDIS_LOCAL_CACHE_ENABLED" == 1`.
 - Optional. Defaults to `1000`.
 - Format: `integer`. Duration in milliseconds.
 
 
-#### `REDIS_LOCAL_CACHE_MAX_TTL_CACHED_COUNTERS_MS`
+#### `REDIS_LOCAL_CACHE_BATCH_SIZE`
 
-- Used to configure the local cache when using Redis. See
+- Used to configure the maximum number of counters to update in a flush. See
 [`REDIS_LOCAL_CACHE_ENABLED`](#redis_local_cache_enabled). This env only applies
 when `"REDIS_LOCAL_CACHE_ENABLED" == 1`.
-- Optional. Defaults to `5000`.
-- Format: `integer`. Duration in milliseconds.
-
-
-#### `REDIS_LOCAL_CACHE_TTL_RATIO_CACHED_COUNTERS`
-
-- Used to configure the local cache when using Redis. See
-[`REDIS_LOCAL_CACHE_ENABLED`](#redis_local_cache_enabled). This env only applies
-when `"REDIS_LOCAL_CACHE_ENABLED" == 1`.
-- Optional. Defaults to `10`.
-- Format: `integer`.
+- Optional. Defaults to `100`.
+- Format: `integer`. 
 
 
 #### `REDIS_URL`

diff --git a/limitador-server/src/config.rs b/limitador-server/src/config.rs
@@ -5,8 +5,7 @@
 // REDIS_URL: StorageType { String }
 // └ REDIS_LOCAL_CACHE_ENABLED: bool
 //   └ REDIS_LOCAL_CACHE_FLUSHING_PERIOD_MS: i64 ?!
-//   └ REDIS_LOCAL_CACHE_MAX_TTL_CACHED_COUNTERS_MS: u64 -> Duration
-//   └ REDIS_LOCAL_CACHE_TTL_RATIO_CACHED_COUNTERS: u64
+//   └ REDIS_LOCAL_CACHE_BATCH_SIZE: u64
 //
 // INFINISPAN_URL: StorageType { String }
 //  └ INFINISPAN_CACHE_NAME: String
@@ -39,6 +38,7 @@ pub struct Configuration {
 
 pub mod env {
     use lazy_static::lazy_static;
+    use std::env;
 
     lazy_static! {
         pub static ref LIMITS_FILE: Option<&'static str> = value_for("LIMITS_FILE");
@@ -47,18 +47,19 @@ pub mod env {
         pub static ref HTTP_API_HOST: Option<&'static str> = value_for("HTTP_API_HOST");
         pub static ref HTTP_API_PORT: Option<&'static str> = value_for("HTTP_API_PORT");
         pub static ref TRACING_ENDPOINT: Option<&'static str> = value_for("TRACING_ENDPOINT");
+        pub static ref LIMIT_NAME_IN_PROMETHEUS_LABELS: bool =
+            env_option_is_enabled("LIMIT_NAME_IN_PROMETHEUS_LABELS");
         pub static ref DISK_PATH: Option<&'static str> = value_for("DISK_PATH");
         pub static ref DISK_OPTIMIZE: Option<&'static str> = value_for("DISK_OPTIMIZE");
         pub static ref REDIS_URL: Option<&'static str> = value_for("REDIS_URL");
-        pub static ref REDIS_LOCAL_CACHE_MAX_TTL_CACHED_COUNTERS_MS: Option<&'static str> =
-            value_for("REDIS_LOCAL_CACHE_MAX_TTL_CACHED_COUNTERS_MS");
+        pub static ref REDIS_LOCAL_CACHE_ENABLED: bool =
+            env_option_is_enabled("REDIS_LOCAL_CACHE_ENABLED");
         pub static ref REDIS_LOCAL_CACHE_FLUSHING_PERIOD_MS: Option<&'static str> =
             value_for("REDIS_LOCAL_CACHE_FLUSHING_PERIOD_MS");
         pub static ref REDIS_LOCAL_CACHE_BATCH_SIZE: Option<&'static str> =
             value_for("REDIS_LOCAL_CACHE_BATCH_SIZE");
-        pub static ref REDIS_LOCAL_CACHE_TTL_RATIO_CACHED_COUNTERS: Option<&'static str> =
-            value_for("REDIS_LOCAL_CACHE_TTL_RATIO_CACHED_COUNTERS");
         pub static ref RATE_LIMIT_HEADERS: Option<&'static str> = value_for("RATE_LIMIT_HEADERS");
+        pub static ref INFINISPAN_URL: Option<&'static str> = value_for("INFINISPAN_URL");
         pub static ref INFINISPAN_CACHE_NAME: Option<&'static str> =
             value_for("INFINISPAN_CACHE_NAME");
         pub static ref INFINISPAN_COUNTERS_CONSISTENCY: Option<&'static str> =
@@ -71,6 +72,13 @@ pub mod env {
             Err(_) => None,
         }
     }
+
+    fn env_option_is_enabled(env_name: &str) -> bool {
+        match env::var(env_name) {
+            Ok(value) => value == "1",
+            Err(_) => false,
+        }
+    }
 }
 
 impl Configuration {

diff --git a/limitador-server/src/main.rs b/limitador-server/src/main.rs
@@ -777,8 +777,7 @@ fn create_config() -> (Configuration, &'static str) {
         *matches.get_one::<u16>("port").unwrap(),
         matches.get_one::<String>("http_ip").unwrap().into(),
         *matches.get_one::<u16>("http_port").unwrap(),
-        matches.get_flag("limit_name_in_labels")
-            || env_option_is_enabled("LIMIT_NAME_IN_PROMETHEUS_LABELS"),
+        matches.get_flag("limit_name_in_labels") || *config::env::LIMIT_NAME_IN_PROMETHEUS_LABELS,
         matches
             .get_one::<String>("tracing_endpoint")
             .unwrap()
@@ -800,25 +799,27 @@ fn create_config() -> (Configuration, &'static str) {
 }
 
 fn storage_config_from_env() -> Result<StorageConfiguration, ()> {
-    let redis_url = env::var("REDIS_URL");
+    let redis_url = config::env::REDIS_URL.ok_or(());
     let infinispan_url = if cfg!(feature = "infinispan") {
-        env::var("INFINISPAN_URL")
+        config::env::INFINISPAN_URL.ok_or(VarError::NotPresent)
     } else {
         Err(VarError::NotPresent)
     };
 
     match (redis_url, infinispan_url) {
         (Ok(_), Ok(_)) => Err(()),
         (Ok(url), Err(_)) => Ok(StorageConfiguration::Redis(RedisStorageConfiguration {
-            url,
-            cache: if env_option_is_enabled("REDIS_LOCAL_CACHE_ENABLED") {
+            url: url.to_owned(),
+            cache: if *config::env::REDIS_LOCAL_CACHE_ENABLED {
                 Some(RedisStorageCacheConfiguration {
-                    batch_size: env::var("REDIS_LOCAL_CACHE_BATCH_SIZE")
-                        .unwrap_or_else(|_| (DEFAULT_BATCH_SIZE).to_string())
+                    batch_size: config::env::REDIS_LOCAL_CACHE_BATCH_SIZE
+                        .map(str::to_owned)
+                        .unwrap_or_else(|| (DEFAULT_BATCH_SIZE).to_string())
                         .parse()
                         .expect("Expected an usize"),
-                    flushing_period: env::var("REDIS_LOCAL_CACHE_FLUSHING_PERIOD_MS")
-                        .unwrap_or_else(|_| (DEFAULT_FLUSHING_PERIOD_SEC * 1000).to_string())
+                    flushing_period: config::env::REDIS_LOCAL_CACHE_FLUSHING_PERIOD_MS
+                        .map(str::to_owned)
+                        .unwrap_or_else(|| (DEFAULT_FLUSHING_PERIOD_SEC * 1000).to_string())
                         .parse()
                         .expect("Expected an i64"),
                     max_counters: DEFAULT_MAX_CACHED_COUNTERS,
@@ -831,9 +832,9 @@ fn storage_config_from_env() -> Result<StorageConfiguration, ()> {
         #[cfg(feature = "infinispan")]
         (Err(_), Ok(url)) => Ok(StorageConfiguration::Infinispan(
             InfinispanStorageConfiguration {
-                url,
-                cache: env::var("INFINISPAN_CACHE_NAME").ok(),
-                consistency: env::var("INFINISPAN_COUNTERS_CONSISTENCY").ok(),
+                url: url.to_owned(),
+                cache: config::env::INFINISPAN_CACHE_NAME.map(str::to_owned),
+                consistency: config::env::INFINISPAN_COUNTERS_CONSISTENCY.map(str::to_owned),
             },
         )),
         _ => Ok(StorageConfiguration::InMemory(
@@ -858,13 +859,6 @@ fn guess_cache_size() -> Option<u64> {
     Some(size)
 }
 
-fn env_option_is_enabled(env_name: &str) -> bool {
-    match env::var(env_name) {
-        Ok(value) => value == "1",
-        Err(_) => false,
-    }
-}
-
 fn leak<D: Display>(s: D) -> &'static str {
     return Box::leak(format!("{}", s).into_boxed_str());
 }