Remove env_logger and use telemetry logging in tests (#1247)

* Remove env_logger and reuse telemetry logging in tests

* Use test-captured writer for telemetry logs in tests

* Isolate test logging init from OTEL and keep tests quiet

Author: lovasoa

Cargo.lock

@@ -37,7 +37,6 @@ actix-web = { version = "4", features = ["rustls-0_23", "cookies"] }
 percent-encoding = "2.2.0"
 handlebars = "6.2.0"
 log = "0.4.17"
-env_logger = "0.11.1"
 mime_guess = "2.0.4"
 futures-util = "0.3.21"
 tokio = { version = "1.24.1", features = ["macros", "rt", "process", "sync"] }

Cargo.toml

@@ -7,29 +7,45 @@
 //! fields for human readability and machine parseability.
 
 use std::env;
-use std::sync::OnceLock;
+use std::sync::{Once, OnceLock};
 
 use opentelemetry_sdk::metrics::SdkMeterProvider;
 use opentelemetry_sdk::trace::SdkTracerProvider;
 
 static TRACER_PROVIDER: OnceLock<SdkTracerProvider> = OnceLock::new();
 static METER_PROVIDER: OnceLock<SdkMeterProvider> = OnceLock::new();
+static TEST_LOGGING_INIT: Once = Once::new();
+const DEFAULT_ENV_FILTER_DIRECTIVES: &str = "sqlpage=info,actix_web=info,tracing_actix_web=info";
 
 /// Initializes logging / tracing. Returns `true` if `OTel` was activated.
 #[must_use]
 pub fn init_telemetry() -> bool {
+    init_telemetry_with_log_layer(logfmt::LogfmtLayer::new())
+}
+
+fn init_telemetry_with_log_layer(logfmt_layer: logfmt::LogfmtLayer) -> bool {
     let otel_endpoint = env::var("OTEL_EXPORTER_OTLP_ENDPOINT").ok();
     let otel_active = otel_endpoint.as_deref().is_some_and(|v| !v.is_empty());
 
     if otel_active {
-        init_otel_tracing();
+        init_otel_tracing(logfmt_layer);
     } else {
-        init_tracing();
+        init_tracing(logfmt_layer);
     }
 
     otel_active
 }
 
+/// Initializes logging once for tests using the same formatter as production.
+///
+/// Unlike `init_telemetry`, this does not initialize OTEL exporters and does
+/// not panic on invalid `LOG_LEVEL` / `RUST_LOG` values.
+pub fn init_test_logging() {
+    TEST_LOGGING_INIT.call_once(|| {
+        init_test_tracing();
+    });
+}
+
 /// Shuts down the `OTel` tracer provider, flushing pending spans.
 pub fn shutdown_telemetry() {
     if let Some(provider) = TRACER_PROVIDER.get() {
@@ -45,17 +61,27 @@ pub fn shutdown_telemetry() {
 }
 
 /// Tracing subscriber without `OTel` export — logfmt output only.
-fn init_tracing() {
+fn init_tracing(logfmt_layer: logfmt::LogfmtLayer) {
     use tracing_subscriber::layer::SubscriberExt;
 
     let subscriber = tracing_subscriber::registry()
         .with(default_env_filter())
-        .with(logfmt::LogfmtLayer::new());
+        .with(logfmt_layer);
+
+    set_global_subscriber(subscriber);
+}
+
+fn init_test_tracing() {
+    use tracing_subscriber::layer::SubscriberExt;
+
+    let subscriber = tracing_subscriber::registry()
+        .with(test_env_filter())
+        .with(logfmt::LogfmtLayer::test_writer());
 
     set_global_subscriber(subscriber);
 }
 
-fn init_otel_tracing() {
+fn init_otel_tracing(logfmt_layer: logfmt::LogfmtLayer) {
     use opentelemetry::global;
     use opentelemetry::trace::TracerProvider as _;
     use opentelemetry_sdk::propagation::TraceContextPropagator;
@@ -117,7 +143,7 @@ fn init_otel_tracing() {
 
     let subscriber = tracing_subscriber::registry()
         .with(default_env_filter())
-        .with(logfmt::LogfmtLayer::new())
+        .with(logfmt_layer)
         .with(otel_layer)
         .with(tracing_opentelemetry::MetricsLayer::new(meter_provider));
 
@@ -133,13 +159,26 @@ fn default_env_filter() -> tracing_subscriber::EnvFilter {
     .expect("Invalid log filter value in LOG_LEVEL or RUST_LOG")
 }
 
+fn test_env_filter() -> tracing_subscriber::EnvFilter {
+    env_filter_directives(
+        env::var("LOG_LEVEL").ok().as_deref(),
+        env::var("RUST_LOG").ok().as_deref(),
+    )
+    .parse()
+    .unwrap_or_else(|_| {
+        DEFAULT_ENV_FILTER_DIRECTIVES
+            .parse()
+            .expect("Default filter directives should always be valid")
+    })
+}
+
 fn env_filter_directives(log_level: Option<&str>, rust_log: Option<&str>) -> String {
     match (
         log_level.filter(|value| !value.is_empty()),
         rust_log.filter(|value| !value.is_empty()),
     ) {
         (Some(value), _) | (None, Some(value)) => value.to_owned(),
-        (None, None) => "sqlpage=info,actix_web=info,tracing_actix_web=info".to_owned(),
+        (None, None) => DEFAULT_ENV_FILTER_DIRECTIVES.to_owned(),
     }
 }
 
@@ -217,14 +256,29 @@ mod logfmt {
     const BOLD: &str = "\x1b[1m";
     const RESET: &str = "\x1b[0m";
 
+    #[derive(Copy, Clone)]
+    enum OutputMode {
+        Stderr,
+        TestWriter,
+    }
+
     pub(super) struct LogfmtLayer {
         use_colors: bool,
+        output_mode: OutputMode,
     }
 
     impl LogfmtLayer {
         pub fn new() -> Self {
             Self {
                 use_colors: io::stderr().is_terminal(),
+                output_mode: OutputMode::Stderr,
+            }
+        }
+
+        pub fn test_writer() -> Self {
+            Self {
+                use_colors: false,
+                output_mode: OutputMode::TestWriter,
             }
         }
     }
@@ -280,7 +334,14 @@ mod logfmt {
 
             buf.push('\n');
             write_multiline_message(&mut buf, msg, multiline_msg);
-            let _ = io::Write::write_all(&mut io::stderr().lock(), buf.as_bytes());
+            match self.output_mode {
+                OutputMode::Stderr => {
+                    let _ = io::Write::write_all(&mut io::stderr().lock(), buf.as_bytes());
+                }
+                OutputMode::TestWriter => {
+                    eprint!("{buf}");
+                }
+            }
         }
     }
 

src/telemetry.rs

@@ -174,10 +174,7 @@ mod tests {
     use sqlx::Connection;
 
     fn setup_logging() {
-        let _ = env_logger::builder()
-            .parse_default_env()
-            .is_test(true)
-            .try_init();
+        crate::telemetry::init_test_logging();
     }
 
     fn db_specific_test(db_type: &str) -> Option<String> {

src/webserver/database/sql_to_json.rs

@@ -371,7 +371,7 @@ mod test {
 
     #[actix_web::test]
     async fn test_extract_multipart_form_data() {
-        let _ = env_logger::try_init();
+        crate::telemetry::init_test_logging();
         let config =
             serde_json::from_str::<AppConfig>(r#"{"listen_on": "localhost:1234"}"#).unwrap();
         let mut service_request = TestRequest::get()

src/webserver/http_request_info.rs

@@ -11,6 +11,7 @@ use actix_web::{
 };
 use sqlpage::{
     app_config::{test_database_url, AppConfig},
+    telemetry,
     webserver::http::{form_config, main_handler, payload_config},
     AppState,
 };
@@ -111,10 +112,7 @@ pub fn test_config() -> AppConfig {
 }
 
 pub fn init_log() {
-    let _ = env_logger::builder()
-        .parse_default_env()
-        .is_test(true)
-        .try_init();
+    telemetry::init_test_logging();
 }
 
 fn format_request_line_and_headers(req: &ServiceRequest) -> String {

tests/common/mod.rs

@@ -141,9 +141,10 @@ async fn test_file_upload_too_large() -> actix_web::Result<()> {
         .await
         .expect_err("Expected an error response")
         .to_string();
+    let msg = "max file size";
     assert!(
-        err_str.to_ascii_lowercase().contains("max file size"),
-        "{err_str}\nexpected to contain: File too large"
+        err_str.to_ascii_lowercase().contains(msg),
+        "{err_str}\nexpected to contain: {msg}"
     );
     Ok(())
 }