Support for converting KV from log crate

Author: SpriteOvO

spdlog/benches/spdlog-rs/log_crate_proxy.rs

@@ -29,3 +29,9 @@ fn bench_log_crate_proxy(bencher: &mut Bencher) {
     init();
     bencher.iter(|| log::info!(bench_log_message!()))
 }
+
+#[bench]
+fn bench_log_crate_proxy_kv(bencher: &mut Bencher) {
+    init();
+    bencher.iter(|| log::info!(key1 = 42, key2 = true; bench_log_message!()))
+}

spdlog/src/kv.rs

@@ -26,7 +26,7 @@ impl<'a> Key<'a> {
         Key(KeyInner::StaticStr(key))
     }
 
-    fn from_str(key: &'a str) -> Self {
+    pub(crate) fn from_str(key: &'a str) -> Self {
         Key(KeyInner::Str(key))
     }
 
@@ -176,16 +176,16 @@ impl<'a> Iterator for KeyValuesIter<'a> {
 pub(crate) type Pair<'a> = (Key<'a>, Value<'a>);
 
 #[cfg(feature = "log")]
-pub(crate) struct LogCrateConverter<'a>(Vec<(Key<'a>, Value<'a>)>);
+pub(crate) struct LogCrateConverter<'a>(Vec<(log::kv::Key<'a>, ValueOwned)>);
 
 #[cfg(feature = "log")]
 impl<'a> LogCrateConverter<'a> {
     pub(crate) fn new(capacity: usize) -> Self {
         Self(Vec::with_capacity(capacity))
     }
 
-    pub(crate) fn finalize(self) -> Cow<'a, [Pair<'a>]> {
-        Cow::Owned(self.0)
+    pub(crate) fn finalize(self) -> Vec<(log::kv::Key<'a>, ValueOwned)> {
+        self.0
     }
 }
 
@@ -196,10 +196,46 @@ impl<'a> log::kv::VisitSource<'a> for LogCrateConverter<'a> {
         key: log::kv::Key<'a>,
         value: log::kv::Value<'a>,
     ) -> Result<(), log::kv::Error> {
-        self.0.push((
-            Key::from_str(key.as_str()),
-            todo!("convert `lov::kv::Value` to `Value`"),
-        ));
+        struct Visitor(Option<ValueOwned>);
+
+        macro_rules! visit_fn {
+            ( $($fn:ident: $ty:ty => $from:ident),+$(,)? ) => {
+                $(fn $fn(&mut self, value: $ty) -> Result<(), log::kv::Error> {
+                    self.0 = Some(Value::$from(value).to_owned());
+                    Ok(())
+                })+
+            };
+        }
+
+        impl log::kv::VisitValue<'_> for Visitor {
+            fn visit_any(&mut self, value: log::kv::Value) -> Result<(), log::kv::Error> {
+                // Since we have no way to extract the underlying `&dyn Display`, we have to
+                // `to_owned()` here
+                self.0 = Some(Value::from_display(&value).to_owned());
+                Ok(())
+            }
+
+            fn visit_null(&mut self) -> Result<(), log::kv::Error> {
+                self.0 = Some(Value::empty().to_owned());
+                Ok(())
+            }
+
+            visit_fn! {
+                visit_u64: u64 => from_u64,
+                visit_i64: i64 => from_i64,
+                visit_u128: u128 => from_u128,
+                visit_i128: i128 => from_i128,
+                visit_f64: f64 => from_f64,
+                visit_bool: bool => from_bool,
+                visit_str: &str => from_str,
+                visit_borrowed_str: &str => from_str,
+                visit_char: char => from_char,
+            }
+        }
+
+        let mut visitor = Visitor(None);
+        value.visit(&mut visitor)?;
+        self.0.push((key, visitor.0.unwrap()));
         Ok(())
     }
 }

spdlog/src/log_crate_proxy.rs

@@ -1,6 +1,6 @@
 use std::time::SystemTime;
 
-use crate::{default_logger, sync::*, Logger, Record};
+use crate::{default_logger, sync::*, LogCrateRecord, Logger};
 
 /// Proxy layer for compatible [log crate].
 ///
@@ -76,8 +76,8 @@ impl log::Log for LogCrateProxy {
 
     fn log(&self, record: &log::Record) {
         let logger = self.logger();
-        let record = Record::from_log_crate_record(&logger, record, SystemTime::now());
-        logger.log(&record)
+        let record = LogCrateRecord::new(&logger, record, SystemTime::now());
+        logger.log(&record.as_record())
     }
 
     fn flush(&self) {

spdlog/src/record.rs

@@ -22,7 +22,7 @@ use crate::{kv, Level, SourceLocation};
 // possible to correct.
 #[derive(Clone, Debug)]
 pub struct Record<'a> {
-    logger_name: Option<Cow<'a, str>>,
+    logger_name: Option<&'a str>,
     payload: Cow<'a, str>,
     kvs: Cow<'a, [kv::Pair<'a>]>,
     inner: Cow<'a, RecordInner>,
@@ -46,7 +46,7 @@ impl<'a> Record<'a> {
         kvs: &'a [(kv::Key<'a>, kv::Value<'a>)],
     ) -> Record<'a> {
         Record {
-            logger_name: logger_name.map(Cow::Borrowed),
+            logger_name,
             payload: payload.into(),
             kvs: Cow::Borrowed(kvs),
             inner: Cow::Owned(RecordInner {
@@ -62,7 +62,7 @@ impl<'a> Record<'a> {
     #[must_use]
     pub fn to_owned(&self) -> RecordOwned {
         RecordOwned {
-            logger_name: self.logger_name.clone().map(|n| n.into_owned()),
+            logger_name: self.logger_name.map(|n| n.to_owned()),
             payload: self.payload.to_string(),
             kvs: self
                 .kvs
@@ -126,47 +126,6 @@ impl<'a> Record<'a> {
         }
     }
 
-    #[cfg(feature = "log")]
-    #[must_use]
-    pub(crate) fn from_log_crate_record(
-        logger: &'a crate::Logger,
-        record: &'a log::Record,
-        time: SystemTime,
-    ) -> Self {
-        let args = record.args();
-
-        Self {
-            // If the logger has a name configured, use that name. Otherwise, the name can also be
-            // given by the target of the log record.
-            logger_name: logger.name().map(Cow::Borrowed).or_else(|| {
-                let log_target = record.target();
-                if log_target.is_empty() {
-                    None
-                } else {
-                    Some(Cow::Owned(String::from(log_target)))
-                }
-            }),
-            kvs: {
-                let kvs = record.key_values();
-                let mut cvt = kv::LogCrateConverter::new(kvs.count());
-                assert!(kvs.visit(&mut cvt).is_ok());
-                cvt.finalize()
-            },
-            payload: match args.as_str() {
-                Some(literal_str) => literal_str.into(),
-                None => args.to_string().into(),
-            },
-            inner: Cow::Owned(RecordInner {
-                level: record.level().into(),
-                source_location: SourceLocation::from_log_crate_record(record),
-                time,
-                // For records from `log` crate, they never seem to come from different threads, so
-                // getting the current TID here should be correct
-                tid: get_current_tid(),
-            }),
-        }
-    }
-
     #[cfg(test)]
     pub(crate) fn set_time(&mut self, new: SystemTime) {
         self.inner.to_mut().time = new;
@@ -189,7 +148,7 @@ impl RecordOwned {
     #[must_use]
     pub fn as_ref(&self) -> Record {
         Record {
-            logger_name: self.logger_name.as_deref().map(Cow::Borrowed),
+            logger_name: self.logger_name.as_deref(),
             payload: Cow::Borrowed(&self.payload),
             kvs: Cow::Owned(
                 self.kvs
@@ -245,6 +204,65 @@ impl RecordOwned {
     // When adding more getters, also add to `Record`
 }
 
+#[cfg(feature = "log")]
+#[derive(Clone, Debug)]
+pub(crate) struct LogCrateRecord<'a> {
+    logger_name: Option<&'a str>,
+    payload: Cow<'a, str>,
+    kvs: Vec<(log::kv::Key<'a>, kv::ValueOwned)>,
+    inner: Cow<'a, RecordInner>,
+}
+
+#[cfg(feature = "log")]
+impl<'a> LogCrateRecord<'a> {
+    #[must_use]
+    pub(crate) fn new(
+        logger: &'a crate::Logger,
+        record: &'a log::Record,
+        time: SystemTime,
+    ) -> Self {
+        let args = record.args();
+
+        Self {
+            // If the logger has a name configured, use that name. Otherwise, the name can also be
+            // given by the target of the log record.
+            logger_name: logger.name().or_else(|| Some(record.target())),
+            kvs: {
+                let kvs = record.key_values();
+                let mut cvt = kv::LogCrateConverter::new(kvs.count());
+                assert!(kvs.visit(&mut cvt).is_ok());
+                cvt.finalize()
+            },
+            payload: match args.as_str() {
+                Some(literal_str) => literal_str.into(),
+                None => args.to_string().into(),
+            },
+            inner: Cow::Owned(RecordInner {
+                level: record.level().into(),
+                source_location: SourceLocation::from_log_crate_record(record),
+                time,
+                // For records from `log` crate, they never seem to come from different threads, so
+                // getting the current TID here should be correct
+                tid: get_current_tid(),
+            }),
+        }
+    }
+
+    #[must_use]
+    pub(crate) fn as_record(&self) -> Record {
+        Record {
+            logger_name: self.logger_name,
+            payload: self.payload.clone(),
+            kvs: self
+                .kvs
+                .iter()
+                .map(|(k, v)| (kv::Key::from_str(k.as_str()), v.by_ref()))
+                .collect(),
+            inner: self.inner.clone(),
+        }
+    }
+}
+
 fn get_current_tid() -> u64 {
     #[cfg(target_os = "linux")]
     #[must_use]

spdlog/tests/log_crate_proxy.rs

@@ -49,3 +49,22 @@ fn test_target() {
     log::info!(target: "MyLogger", "body");
     assert_eq!(sink.clone_string(), format!("[MyLogger] body{__EOL}"));
 }
+
+#[cfg(feature = "log")]
+#[test]
+fn test_kv() {
+    let formatter = Box::new(PatternFormatter::new(pattern!("{payload} {kv}{eol}")));
+    let sink = Arc::new(StringSink::with(|b| b.formatter(formatter)));
+    let logger = Arc::new(build_test_logger(|b| b.sink(sink.clone())));
+
+    let _guard = GLOBAL_LOG_CRATE_PROXY_MUTEX.lock().unwrap();
+    spdlog::init_log_crate_proxy().ok();
+    spdlog::log_crate_proxy().set_logger(Some(logger));
+    log::set_max_level(log::LevelFilter::Trace);
+
+    log::info!(key1 = 42, key2 = true; "a {} event", "log");
+    assert_eq!(
+        sink.clone_string(),
+        format!("a log event {{ key1=42, key2=true }}{__EOL}")
+    );
+}