feat: Re-configurable logger

Jonathan Woollett-Light · Jonathan Woollett-Light · commit 28ac83d10d0f · 2023-07-19T14:49:28.000+01:00
Updates tracing to the latest version from GitHub adding
support for re-configuring the logger at run-time and
removing the old logger format.

Signed-off-by: Jonathan Woollett-Light &lt;jcawl@amazon.co.uk&gt;
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/src/api_server/Cargo.toml b/src/api_server/Cargo.toml
@@ -21,7 +21,7 @@ mmds = { path = "../mmds" }
 seccompiler = { path = "../seccompiler" }
 utils = { path = "../utils" }
 vmm = { path = "../vmm" }
-tracing = { version = "0.1.37", features = ["attributes"] }
+tracing = { git = "https://github.com/tokio-rs/tracing", rev = "27f688efb72316a26f3ec1f952c82626692c08ff", features = ["attributes"] }
 
 [dev-dependencies]
 libc = "0.2.117"
diff --git a/src/api_server/src/request/logger.rs b/src/api_server/src/request/logger.rs
@@ -37,11 +37,10 @@ mod tests {
               }"#;
 
         let mut expected_cfg = LoggerConfig {
-            log_path: PathBuf::from("log"),
+            log_path: Some(PathBuf::from("log")),
             level: Some(Level::Warn),
             show_level: Some(false),
             show_log_origin: Some(false),
-            new_format: None,
         };
         match vmm_action_from_request(parse_put_logger(&Body::new(body)).unwrap()) {
             VmmAction::ConfigureLogger(cfg) => assert_eq!(cfg, expected_cfg),
@@ -56,11 +55,10 @@ mod tests {
               }"#;
 
         expected_cfg = LoggerConfig {
-            log_path: PathBuf::from("log"),
+            log_path: Some(PathBuf::from("log")),
             level: Some(Level::Debug),
             show_level: Some(false),
             show_log_origin: Some(false),
-            new_format: None,
         };
         match vmm_action_from_request(parse_put_logger(&Body::new(body)).unwrap()) {
             VmmAction::ConfigureLogger(cfg) => assert_eq!(cfg, expected_cfg),
diff --git a/src/cpu-template-helper/Cargo.toml b/src/cpu-template-helper/Cargo.toml
@@ -17,7 +17,7 @@ libc = "0.2.147"
 serde = { version = "1.0.171", features = ["derive"] }
 serde_json = "1.0.100"
 thiserror = "1.0.43"
-tracing = { version = "0.1.37", features = ["attributes"] }
+tracing = { git = "https://github.com/tokio-rs/tracing", rev = "27f688efb72316a26f3ec1f952c82626692c08ff", features = ["attributes"] }
 
 vmm = { path = "../vmm" }
 
diff --git a/src/dumbo/Cargo.toml b/src/dumbo/Cargo.toml
@@ -11,7 +11,7 @@ bench = false
 [dependencies]
 bitflags = "1.3.2"
 derive_more = { version = "0.99.17", default-features = false, features = ["from"] }
-tracing = { version = "0.1.37", features = ["attributes"] }
+tracing = { git = "https://github.com/tokio-rs/tracing", rev = "27f688efb72316a26f3ec1f952c82626692c08ff", features = ["attributes"] }
 
 logger = { path = "../logger" }
 micro_http = { git = "https://github.com/firecracker-microvm/micro-http", rev = "4b18a04" }
diff --git a/src/firecracker/Cargo.toml b/src/firecracker/Cargo.toml
@@ -19,8 +19,8 @@ log = "0.4.19"
 serde_json = "1.0.100"
 thiserror = "1.0.43"
 timerfd = "1.5.0"
-tracing = { version = "0.1.37", features = ["attributes"] }
-tracing-subscriber = "0.3.17"
+tracing = { git = "https://github.com/tokio-rs/tracing", rev = "27f688efb72316a26f3ec1f952c82626692c08ff", features = ["attributes"] }
+tracing-subscriber = { git = "https://github.com/tokio-rs/tracing", rev = "27f688efb72316a26f3ec1f952c82626692c08ff" }
 
 api_server = { path = "../api_server" }
 logger = { path = "../logger" }
diff --git a/src/firecracker/src/api_server_adapter.rs b/src/firecracker/src/api_server_adapter.rs
@@ -129,6 +129,7 @@ pub(crate) fn run_with_api(
     api_payload_limit: usize,
     mmds_size_limit: usize,
     metadata_json: Option<&str>,
+    logger_handles: vmm::vmm_config::LoggerHandles,
 ) -> FcExitCode {
     // FD to notify of API events. This is a blocking eventfd by design.
     // It is used in the config/pre-boot loop which is a simple blocking loop
@@ -210,6 +211,7 @@ pub(crate) fn run_with_api(
             boot_timer_enabled,
             mmds_size_limit,
             metadata_json,
+            logger_handles,
         ),
     };
 
diff --git a/src/firecracker/src/main.rs b/src/firecracker/src/main.rs
@@ -119,7 +119,7 @@ fn main_exitable() -> FcExitCode {
         .arg(
             Argument::new("id")
                 .takes_value(true)
-                .default_value(logger::DEFAULT_INSTANCE_ID)
+                .default_value("anonymous-instance")
                 .help("MicroVM unique identifier."),
         )
         .arg(
@@ -198,11 +198,6 @@ fn main_exitable() -> FcExitCode {
                     "Whether or not to include the file path and line number of the log's origin.",
                 ),
         )
-        .arg(
-            Argument::new("new-format")
-                .takes_value(false)
-                .help("Whether to use the new logging output format."),
-        )
         .arg(
             Argument::new("metrics-path")
                 .takes_value(true)
@@ -279,9 +274,8 @@ fn main_exitable() -> FcExitCode {
         app_name: "Firecracker".to_string(),
     };
 
-    logger::INSTANCE_ID.set(String::from(instance_id)).unwrap();
-
-    if let Some(log_path) = arguments.single_value("log-path") {
+    // Configure logger, the logger handles can be used to re-configure the logger with the API.
+    let logger_handles = {
         let level_res = arguments
             .single_value("level")
             .map(|s| Level::from_str(s))
@@ -297,18 +291,19 @@ fn main_exitable() -> FcExitCode {
             }
         };
 
-        let logger_config = vmm::vmm_config::LoggerConfig {
-            log_path: PathBuf::from(log_path),
+        let config = vmm::vmm_config::LoggerConfig {
+            log_path: arguments.single_value("log-path").map(PathBuf::from),
             level,
             show_level: Some(arguments.flag_present("show-level")),
             show_log_origin: Some(arguments.flag_present("show-log-origin")),
-            new_format: Some(arguments.flag_present("new-format")),
-        };
-
-        if let Err(err) = logger_config.init() {
-            return generic_error_exit(&format!("Could not initialize logger: {}", err));
         };
-    }
+        match config.init() {
+            Ok(h) => h,
+            Err(err) => {
+                return generic_error_exit(&format!("Could not initialize logger: {}", err));
+            }
+        }
+    };
 
     if let Some(metrics_path) = arguments.single_value("metrics-path") {
         let metrics_config = MetricsConfig {
@@ -398,6 +393,7 @@ fn main_exitable() -> FcExitCode {
             api_payload_limit,
             mmds_size_limit,
             metadata_json.as_deref(),
+            logger_handles,
         )
     } else {
         let seccomp_filters: BpfThreadMap = seccomp_filters
diff --git a/src/jailer/Cargo.toml b/src/jailer/Cargo.toml
@@ -17,6 +17,6 @@ libc = "0.2.147"
 nix = { version = "0.26.2", default-features = false, features = ["dir"] }
 regex = { version = "1.9.1", default-features = false, features = ["std"] }
 thiserror = "1.0.43"
-tracing = { version = "0.1.37", features = ["attributes"] }
+tracing = { git = "https://github.com/tokio-rs/tracing", rev = "27f688efb72316a26f3ec1f952c82626692c08ff", features = ["attributes"] }
 
 utils = { path = "../utils" }
diff --git a/src/logger/Cargo.toml b/src/logger/Cargo.toml
@@ -16,5 +16,5 @@ serde = { version = "1.0.136", features = ["derive", "rc"] }
 serde_json = "1.0.78"
 thiserror = "1.0.32"
 vm-superio = "0.7.0"
-tracing = { version = "0.1.37", features = ["attributes"] }
+tracing = { git = "https://github.com/tokio-rs/tracing", rev = "27f688efb72316a26f3ec1f952c82626692c08ff", features = ["attributes"] }
 utils = { path = "../utils" }
diff --git a/src/logger/src/lib.rs b/src/logger/src/lib.rs
@@ -8,8 +8,6 @@
 
 mod metrics;
 
-use std::sync::OnceLock;
-
 use tracing::warn;
 
 #[cfg(target_arch = "aarch64")]
@@ -26,11 +24,6 @@ pub type FcLineWriter = std::io::LineWriter<std::fs::File>;
 /// that are not generally available.
 const DEV_PREVIEW_LOG_PREFIX: &str = "[DevPreview]";
 
-/// The default instance ID.
-pub const DEFAULT_INSTANCE_ID: &str = "anonymous-instance";
-/// The instance ID to use when initializing the logger.
-pub static INSTANCE_ID: OnceLock<String> = OnceLock::new();
-
 /// Log a standard warning message indicating a given feature name
 /// is in development preview.
 pub fn log_dev_preview_warning(feature_name: &str, msg_opt: Option<String>) {
diff --git a/src/mmds/Cargo.toml b/src/mmds/Cargo.toml
@@ -24,4 +24,4 @@ logger = { path = "../logger" }
 micro_http = { git = "https://github.com/firecracker-microvm/micro-http", rev = "4b18a04" }
 snapshot = { path = "../snapshot" }
 utils = { path = "../utils" }
-tracing = { version = "0.1.37", features = ["attributes"] }
+tracing = { git = "https://github.com/tokio-rs/tracing", rev = "27f688efb72316a26f3ec1f952c82626692c08ff", features = ["attributes"] }
diff --git a/src/rate_limiter/Cargo.toml b/src/rate_limiter/Cargo.toml
@@ -16,4 +16,4 @@ versionize_derive = "0.1.5"
 logger = { path = "../logger" }
 snapshot = { path = "../snapshot" }
 utils = { path = "../utils" }
-tracing = { version = "0.1.37", features = ["attributes"] }
+tracing = { git = "https://github.com/tokio-rs/tracing", rev = "27f688efb72316a26f3ec1f952c82626692c08ff", features = ["attributes"] }
diff --git a/src/rebase-snap/Cargo.toml b/src/rebase-snap/Cargo.toml
@@ -12,6 +12,6 @@ bench = false
 
 [dependencies]
 libc = "0.2.147"
-tracing = { version = "0.1.37", features = ["attributes"] }
+tracing = { git = "https://github.com/tokio-rs/tracing", rev = "27f688efb72316a26f3ec1f952c82626692c08ff", features = ["attributes"] }
 
 utils = { path = "../utils" }
diff --git a/src/seccompiler/Cargo.toml b/src/seccompiler/Cargo.toml
@@ -22,6 +22,6 @@ libc = "0.2.147"
 serde = { version = "1.0.171", features = ["derive"] }
 serde_json = "1.0.100"
 thiserror = "1.0.43"
-tracing = { version = "0.1.37", features = ["attributes"] }
+tracing = { git = "https://github.com/tokio-rs/tracing", rev = "27f688efb72316a26f3ec1f952c82626692c08ff", features = ["attributes"] }
 
 utils = { path = "../utils" }
diff --git a/src/snapshot/Cargo.toml b/src/snapshot/Cargo.toml
@@ -14,7 +14,7 @@ libc = "0.2.117"
 versionize = "0.1.10"
 versionize_derive = "0.1.5"
 thiserror = "1.0.32"
-tracing = { version = "0.1.37", features = ["attributes"] }
+tracing = { git = "https://github.com/tokio-rs/tracing", rev = "27f688efb72316a26f3ec1f952c82626692c08ff", features = ["attributes"] }
 
 [dev-dependencies]
 criterion = { version = "0.5.0", default-features = false }
diff --git a/src/utils/Cargo.toml b/src/utils/Cargo.toml
@@ -17,7 +17,7 @@ versionize = "0.1.10"
 versionize_derive = "0.1.5"
 vmm-sys-util = "0.11.0"
 vm-memory = { version = "0.11.0", features = ["backend-mmap", "backend-bitmap"] }
-tracing = { version = "0.1.37", features = ["attributes"] }
+tracing = { git = "https://github.com/tokio-rs/tracing", rev = "27f688efb72316a26f3ec1f952c82626692c08ff", features = ["attributes"] }
 
 net_gen = { path = "../net_gen" }
 
diff --git a/src/vmm/Cargo.toml b/src/vmm/Cargo.toml
@@ -29,9 +29,8 @@ versionize_derive = "0.1.5"
 vm-allocator = "0.1.0"
 vm-fdt = "0.2.0"
 vm-superio = "0.7.0"
-tracing = { version = "0.1.37", features = ["attributes"] }
-tracing-subscriber = "0.3.17"
-tracing-core = "0.1.31"
+tracing = { git = "https://github.com/tokio-rs/tracing", rev = "27f688efb72316a26f3ec1f952c82626692c08ff", features = ["attributes"] }
+tracing-subscriber = { git = "https://github.com/tokio-rs/tracing", rev = "27f688efb72316a26f3ec1f952c82626692c08ff" }
 log = { version = "0.4.17", features = ["serde"] }
 
 dumbo = { path = "../dumbo" }
diff --git a/src/vmm/src/resources.rs b/src/vmm/src/resources.rs
@@ -27,7 +27,7 @@ use crate::vmm_config::metrics::{init_metrics, MetricsConfig, MetricsConfigError
 use crate::vmm_config::mmds::{MmdsConfig, MmdsConfigError};
 use crate::vmm_config::net::*;
 use crate::vmm_config::vsock::*;
-use crate::vmm_config::{LoggerConfig, LoggerConfigError};
+use crate::vmm_config::{InitLoggerError, LoggerConfig};
 
 /// Errors encountered when configuring microVM resources.
 #[derive(Debug, thiserror::Error, derive_more::From)]
@@ -49,7 +49,7 @@ pub enum ResourcesError {
     InvalidJson(serde_json::Error),
     /// Logger configuration error.
     #[error("Logger error: {0}")]
-    Logger(LoggerConfigError),
+    Logger(InitLoggerError),
     /// Metrics system configuration error.
     #[error("Metrics error: {0}")]
     Metrics(MetricsConfigError),
@@ -863,7 +863,7 @@ mod tests {
                 HTTP_MAX_PAYLOAD_SIZE,
                 None,
             ),
-            Err(Error::Logger(LoggerConfigError::File(_))),
+            Err(ResourcesError::Logger(InitLoggerError::File(_))),
         ));
 
         // Invalid path for metrics pipe.
diff --git a/src/vmm/src/rpc_interface.rs b/src/vmm/src/rpc_interface.rs
diff --git a/src/vmm/src/vmm_config/mod.rs b/src/vmm/src/vmm_config/mod.rs
diff --git a/tests/framework/microvm.py b/tests/framework/microvm.py
diff --git a/tests/integration_tests/functional/test_logging.py b/tests/integration_tests/functional/test_logging.py
