Repository: cloudflare/foundations (language: rust)

Issue:
For API consistency make `LogVerbosity` an enum and use it across the API



Discussion / hints:

Fix patch:
diff --git a/foundations/src/telemetry/log/init.rs b/foundations/src/telemetry/log/init.rs
index a0b239a..b893f9f 100644
--- a/foundations/src/telemetry/log/init.rs
+++ b/foundations/src/telemetry/log/init.rs
@@ -140,7 +140,7 @@ where
         drain,
         FieldRedactFilterFactory::new(settings.redact_keys.clone()),
     );
-    let drain = drain.filter_level(*settings.verbosity);
+    let drain = drain.filter_level(settings.verbosity.into());
 
     RateLimitingDrain::new(drain, settings)
 }
diff --git a/foundations/src/telemetry/log/mod.rs b/foundations/src/telemetry/log/mod.rs
index a3a2997..464531d 100644
--- a/foundations/src/telemetry/log/mod.rs
+++ b/foundations/src/telemetry/log/mod.rs
@@ -21,7 +21,7 @@ use self::internal::current_log;
 use crate::telemetry::log::init::build_log_with_drain;
 use crate::telemetry::settings::LogVerbosity;
 use crate::Result;
-use slog::{Level, Logger, OwnedKV};
+use slog::{Logger, OwnedKV};
 use std::ops::Deref;
 use std::sync::Arc;
 
@@ -39,11 +39,11 @@ pub use self::testing::TestLogRecord;
 /// verbosity level.
 ///
 /// [`init`]: crate::telemetry::init
-pub fn set_verbosity(level: Level) -> Result<()> {
+pub fn set_verbosity(verbosity: LogVerbosity) -> Result<()> {
     let harness = LogHarness::get();
 
     let mut settings = harness.settings.clone();
-    settings.verbosity = LogVerbosity(level);
+    settings.verbosity = verbosity;
 
     let current_log = current_log();
     let mut current_log_lock = current_log.write();
diff --git a/foundations/src/telemetry/settings/logging.rs b/foundations/src/telemetry/settings/logging.rs
index 9282aa7..30512bb 100644
--- a/foundations/src/telemetry/settings/logging.rs
+++ b/foundations/src/telemetry/settings/logging.rs
@@ -1,13 +1,10 @@
 use crate::telemetry::settings::rate_limit::RateLimitingSettings;
 use crate::utils::feature_use;
 
-use std::ops::Deref;
 use std::path::PathBuf;
 
 feature_use!(cfg(feature = "settings"), {
-    use crate::settings::{settings, Settings};
-    use serde::{de, Deserialize, Deserializer, Serialize, Serializer};
-    use std::str::FromStr;
+    use crate::settings::settings;
 });
 
 // NOTE: we technically don't need a feature gate here, but if we don't add it then docs don't
@@ -66,21 +63,52 @@ pub enum LogFormat {
     Json,
 }
 
-/// Verbosity level of the log.
-#[derive(Clone, Debug, Copy)]
-pub struct LogVerbosity(pub Level);
+/// Log verbosity levels which match 1:1 with [`slog::Level`].
+#[cfg_attr(
+    feature = "settings",
+    settings(crate_path = "crate", impl_default = false)
+)]
+#[cfg_attr(not(feature = "settings"), derive(Clone, Debug))]
+#[derive(Copy, Default)]
+pub enum LogVerbosity {
+    /// See [`slog::Level::Critical`].
+    Critical,
+    /// See [`slog::Level::Error`].
+    Error,
+    /// See [`slog::Level::Warning`].
+    Warning,
+    /// See [`slog::Level::Info`].
+    #[default]
+    Info,
+    /// See [`slog::Level::Debug`].
+    Debug,
+    /// See [`slog::Level::Trace`].
+    Trace,
+}
 
-impl Default for LogVerbosity {
-    fn default() -> Self {
-        Self(Level::Info)
+impl From<slog::Level> for LogVerbosity {
+    fn from(level: slog::Level) -> Self {
+        match level {
+            Level::Critical => Self::Critical,
+            Level::Warning => Self::Warning,
+            Level::Error => Self::Error,
+            Level::Info => Self::Info,
+            Level::Debug => Self::Debug,
+            Level::Trace => Self::Trace,
+        }
     }
 }
 
-impl Deref for LogVerbosity {
-    type Target = Level;
-
-    fn deref(&self) -> &Self::Target {
-        &self.0
+impl From<LogVerbosity> for slog::Level {
+    fn from(level: LogVerbosity) -> Self {
+        match level {
+            LogVerbosity::Critical => Self::Critical,
+            LogVerbosity::Warning => Self::Warning,
+            LogVerbosity::Error => Self::Error,
+            LogVerbosity::Info => Self::Info,
+            LogVerbosity::Debug => Self::Debug,
+            LogVerbosity::Trace => Self::Trace,
+        }
     }
 }
 
@@ -94,30 +122,3 @@ pub struct LogVolumeMetricSettings {
     /// Whether to enable log volume metrics
     pub enabled: bool,
 }
-
-#[cfg(feature = "settings")]
-mod with_settings_feature {
-    use super::*;
-
-    impl<'de> Deserialize<'de> for LogVerbosity {
-        fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
-        where
-            D: Deserializer<'de>,
-        {
-            Level::from_str(&String::deserialize(deserializer)?)
-                .map_err(|_| de::Error::custom("incorrect verbosity level"))
-                .map(LogVerbosity)
-        }
-    }
-
-    impl Serialize for LogVerbosity {
-        fn serialize<S>(&self, s: S) -> Result<S::Ok, S::Error>
-        where
-            S: Serializer,
-        {
-            s.serialize_str(self.0.as_str())
-        }
-    }
-
-    impl Settings for LogVerbosity {}
-}


Test patch:
diff --git a/foundations/src/telemetry/testing.rs b/foundations/src/telemetry/testing.rs
index b3daa91..7a12165 100644
--- a/foundations/src/telemetry/testing.rs
+++ b/foundations/src/telemetry/testing.rs
@@ -6,7 +6,6 @@ feature_use!(cfg(feature = "logging"), {
     use super::log::testing::{create_test_log, TestLogRecord, TestLogRecords};
     use super::settings::LogVerbosity;
     use super::settings::LoggingSettings;
-    use slog::Level;
     use std::sync::Arc;
     use std::sync::RwLockReadGuard;
 });
@@ -52,7 +51,7 @@ impl TestTelemetryContext {
         #[cfg(feature = "logging")]
         let (log, log_records) = {
             create_test_log(&LoggingSettings {
-                verbosity: LogVerbosity(Level::Trace),
+                verbosity: LogVerbosity::Trace,
                 ..Default::default()
             })
         };
diff --git a/foundations/tests/logging.rs b/foundations/tests/logging.rs
index 6f61c82..424f28f 100644
--- a/foundations/tests/logging.rs
+++ b/foundations/tests/logging.rs
@@ -1,9 +1,8 @@
 use foundations::telemetry::log::internal::LoggerWithKvNestingTracking;
 use foundations::telemetry::log::{add_fields, set_verbosity, warn};
-use foundations::telemetry::settings::{LoggingSettings, RateLimitingSettings};
+use foundations::telemetry::settings::{LogVerbosity, LoggingSettings, RateLimitingSettings};
 use foundations::telemetry::TestTelemetryContext;
 use foundations_macros::with_test_telemetry;
-use slog::Level;
 
 #[with_test_telemetry(test)]
 fn test_rate_limiter(mut ctx: TestTelemetryContext) {
@@ -39,7 +38,7 @@ fn test_exceed_limit_kv_nesting(_ctx: TestTelemetryContext) {
     match std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| {
         for _ in 0..((LoggerWithKvNestingTracking::MAX_NESTING / 2) + 1) {
             add_fields! { "key1" => "hello" }
-            set_verbosity(Level::Info).expect("set_verbosity");
+            set_verbosity(LogVerbosity::Info).expect("set_verbosity");
         }
     })) {
         Ok(_) => panic!("test case exceeded the maximum log KV nesting, but there was no panic"),
@@ -70,6 +69,6 @@ fn test_exceed_limit_kv_nesting(_ctx: TestTelemetryContext) {
 fn test_not_exceed_limit_kv_nesting(_ctx: TestTelemetryContext) {
     for _ in 0..((LoggerWithKvNestingTracking::MAX_NESTING / 2) - 5) {
         add_fields! { "key1" => "hello" }
-        set_verbosity(Level::Info).expect("set_verbosity");
+        set_verbosity(LogVerbosity::Info).expect("set_verbosity");
     }
 }