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

Consolidate env variable names #81

Closed
alexsnaps opened this issue Jun 16, 2022 · 2 comments
Closed

Consolidate env variable names #81

alexsnaps opened this issue Jun 16, 2022 · 2 comments
Labels
area/server participation/good first issue Good for newcomers status/discussing Further information is requested

Comments

@alexsnaps
Copy link
Member

alexsnaps commented Jun 16, 2022

Currently env variables are used to configure limitador-server. We should consolidate these in some sort of prefix/namespace. LIMITADOR_ maybe? So that e.g. the currently LIMITS_FILE would become LIMITADOR_LIMITS_FILE.
We should also modify the logger to use the same prefix (currently RUST_LOG and RUST_LOG_STYLE)

Config the logger

diff --git a/limitador-server/src/main.rs b/limitador-server/src/main.rs
index 280cdf4..110cf02 100644
--- a/limitador-server/src/main.rs
+++ b/limitador-server/src/main.rs
@@ -5,6 +5,7 @@ extern crate log;
 
 use crate::envoy_rls::server::run_envoy_rls_server;
 use crate::http_api::server::run_http_server;
+use env_logger::Env;
 use limitador::errors::LimitadorError;
 use limitador::limit::Limit;
 use limitador::storage::infinispan::{Consistency, InfinispanStorageBuilder};
@@ -239,7 +240,13 @@ impl Limiter {
 
 #[actix_rt::main]
 async fn main() -> Result<(), Box<dyn std::error::Error>> {
-    env_logger::init();
+    if let Err(e) = env_logger::try_init_from_env(
+        Env::new()
+            .filter("LIMITADOR_LOG")
+            .write_style("LIMITADOR_LOG_STYLE"),
+    ) {
+        eprintln!("Couldn't initializer logger: {}", e);
+    }
 
     let rate_limiter: Arc<Limiter> = match Limiter::new().await {
         Ok(limiter) => Arc::new(limiter),
@alexsnaps alexsnaps added participation/good first issue Good for newcomers status/discussing Further information is requested labels Jun 16, 2022
@alexsnaps
Copy link
Member Author

Might be "overruled" by #83 if we go for command line args… Or should we keep the logging configuration thru env vars maybe?

@alexsnaps
Copy link
Member Author

I think with the work done as part of CLI #92 We can leave this as is (so things remain backwards compatible, yet the new CLI based approached is favored)

@alexsnaps alexsnaps closed this as not planned Won't fix, can't repro, duplicate, stale Jul 29, 2022
Repository owner moved this from In Refinement to Done in Kuadrant Service Protection Jul 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/server participation/good first issue Good for newcomers status/discussing Further information is requested
Projects
No open projects
Status: Done
Development

No branches or pull requests

1 participant