Merge pull request #613 from rust-lang/feat/kv-cleanup

Get structured logging API ready for stabilization

Author: KodrAus

.github/workflows/main.yml

@@ -44,10 +44,10 @@ jobs:
     - run: cargo test --verbose --all-features
     - run: cargo test --verbose --features serde
     - run: cargo test --verbose --features std
-    - run: cargo test --verbose --features kv_unstable
-    - run: cargo test --verbose --features kv_unstable_sval
-    - run: cargo test --verbose --features kv_unstable_serde
-    - run: cargo test --verbose --features "kv_unstable kv_unstable_std kv_unstable_sval kv_unstable_serde"
+    - run: cargo test --verbose --features kv
+    - run: cargo test --verbose --features kv_sval
+    - run: cargo test --verbose --features kv_serde
+    - run: cargo test --verbose --features "kv kv_std kv_sval kv_serde"
     - run: cargo run --verbose --manifest-path test_max_level_features/Cargo.toml
     - run: cargo run --verbose --manifest-path test_max_level_features/Cargo.toml --release
 
@@ -103,12 +103,12 @@ jobs:
         run: |
           rustup update nightly --no-self-update
           rustup default nightly
-      - run: cargo build --verbose -Z avoid-dev-deps --features kv_unstable
-      - run: cargo build --verbose -Z avoid-dev-deps --features "kv_unstable std"
-      - run: cargo build --verbose -Z avoid-dev-deps --features "kv_unstable kv_unstable_sval"
-      - run: cargo build --verbose -Z avoid-dev-deps --features "kv_unstable kv_unstable_serde"
-      - run: cargo build --verbose -Z avoid-dev-deps --features "kv_unstable kv_unstable_std"
-      - run: cargo build --verbose -Z avoid-dev-deps --features "kv_unstable kv_unstable_sval kv_unstable_serde"
+      - run: cargo build --verbose -Z avoid-dev-deps --features kv
+      - run: cargo build --verbose -Z avoid-dev-deps --features "kv std"
+      - run: cargo build --verbose -Z avoid-dev-deps --features "kv kv_sval"
+      - run: cargo build --verbose -Z avoid-dev-deps --features "kv kv_serde"
+      - run: cargo build --verbose -Z avoid-dev-deps --features "kv kv_std"
+      - run: cargo build --verbose -Z avoid-dev-deps --features "kv kv_sval kv_serde"
 
   minimalv:
     name: Minimal versions
@@ -119,12 +119,12 @@ jobs:
         run: |
           rustup update nightly --no-self-update
           rustup default nightly
-      - run: cargo build --verbose -Z minimal-versions --features kv_unstable
-      - run: cargo build --verbose -Z minimal-versions --features "kv_unstable std"
-      - run: cargo build --verbose -Z minimal-versions --features "kv_unstable kv_unstable_sval"
-      - run: cargo build --verbose -Z minimal-versions --features "kv_unstable kv_unstable_serde"
-      - run: cargo build --verbose -Z minimal-versions --features "kv_unstable kv_unstable_std"
-      - run: cargo build --verbose -Z minimal-versions --features "kv_unstable kv_unstable_sval kv_unstable_serde"
+      - run: cargo build --verbose -Z minimal-versions --features kv
+      - run: cargo build --verbose -Z minimal-versions --features "kv std"
+      - run: cargo build --verbose -Z minimal-versions --features "kv kv_sval"
+      - run: cargo build --verbose -Z minimal-versions --features "kv kv_serde"
+      - run: cargo build --verbose -Z minimal-versions --features "kv kv_std"
+      - run: cargo build --verbose -Z minimal-versions --features "kv kv_sval kv_serde"
 
   msrv:
     name: MSRV
@@ -135,7 +135,9 @@ jobs:
         run: |
           rustup update 1.60.0 --no-self-update
           rustup default 1.60.0
-      - run: cargo test --verbose --manifest-path tests/Cargo.toml
+      - run: |
+          cargo test --verbose --manifest-path tests/Cargo.toml
+          cargo test --verbose --manifest-path tests/Cargo.toml --features kv
 
   embedded:
     name: Embedded

Cargo.toml

@@ -17,7 +17,7 @@ rust-version = "1.60.0"
 edition = "2021"
 
 [package.metadata.docs.rs]
-features = ["std", "serde", "kv_unstable_std", "kv_unstable_sval", "kv_unstable_serde"]
+features = ["std", "serde", "kv_std", "kv_sval", "kv_serde"]
 
 [[test]]
 name = "filters"
@@ -46,25 +46,31 @@ release_max_level_trace = []
 
 std = []
 
-# requires the latest stable
-# this will have a tighter MSRV before stabilization
-kv_unstable = ["value-bag"]
-kv_unstable_sval = ["kv_unstable", "value-bag/sval", "sval", "sval_ref"]
-kv_unstable_std = ["std", "kv_unstable", "value-bag/error"]
-kv_unstable_serde = ["kv_unstable_std", "value-bag/serde", "serde"]
+kv = []
+kv_sval = ["kv", "value-bag/sval", "sval", "sval_ref"]
+kv_std = ["std", "kv", "value-bag/error"]
+kv_serde = ["kv_std", "value-bag/serde", "serde"]
+
+# Deprecated: use `kv_*` instead
+# These `*_unstable` features will be removed in a future release
+kv_unstable = ["kv", "value-bag"]
+kv_unstable_sval = ["kv_sval", "kv_unstable"]
+kv_unstable_std = ["kv_std", "kv_unstable"]
+kv_unstable_serde = ["kv_serde", "kv_unstable_std"]
 
 [dependencies]
 serde = { version = "1.0", optional = true, default-features = false }
 sval = { version = "2.1", optional = true, default-features = false }
 sval_ref = { version = "2.1", optional = true, default-features = false }
-value-bag = { version = "1.4", optional = true, default-features = false }
+value-bag = { version = "1.7", optional = true, default-features = false, features = ["inline-i128"] }
 
 [dev-dependencies]
 serde = { version = "1.0", features = ["derive"] }
+serde_json = "1.0"
 serde_test = "1.0"
 sval = { version = "2.1" }
 sval_derive = { version = "2.1" }
-value-bag = { version = "1.4", features = ["test"] }
+value-bag = { version = "1.7", features = ["test"] }
 
 # NOTE: log doesn't actually depent on this crate. However our dependencies,
 # serde and sval, dependent on version 1.0 of the crate, which has problem fixed

README.md

@@ -100,23 +100,28 @@ The executable itself may use the `log` crate to log as well.
 
 ## Structured logging
 
-If you enable the `kv_unstable` feature, you can associate structured data with your log records:
+If you enable the `kv` feature, you can associate structured data with your log records:
 
 ```rust
-use log::{info, trace, warn, as_serde, as_error};
+use log::{info, trace, warn};
 
 pub fn shave_the_yak(yak: &mut Yak) {
-    trace!(target = "yak_events", yak = as_serde!(yak); "Commencing yak shaving");
+    // `yak:serde` will capture `yak` using its `serde::Serialize` impl
+    //
+    // You could also use `:?` for `Debug`, or `:%` for `Display`. For a
+    // full list, see the `log` crate documentation
+    trace!(target = "yak_events", yak:serde; "Commencing yak shaving");
 
     loop {
         match find_a_razor() {
             Ok(razor) => {
-                info!(razor = razor; "Razor located");
+                info!(razor; "Razor located");
                 yak.shave(razor);
                 break;
             }
-            Err(err) => {
-                warn!(err = as_error!(err); "Unable to locate a razor, retrying");
+            Err(e) => {
+                // `e:err` will capture `e` using its `std::error::Error` impl
+                warn!(e:err; "Unable to locate a razor, retrying");
             }
         }
     }

benches/value.rs

@@ -1,4 +1,4 @@
-#![cfg(feature = "kv_unstable")]
+#![cfg(feature = "kv")]
 #![feature(test)]
 
 use log::kv::Value;

src/__private_api.rs

@@ -5,31 +5,28 @@ use crate::{Level, Metadata, Record};
 use std::fmt::Arguments;
 pub use std::{file, format_args, line, module_path, stringify};
 
-#[cfg(feature = "kv_unstable")]
-pub type Value<'a> = dyn crate::kv::value::ToValue + 'a;
-
-#[cfg(not(feature = "kv_unstable"))]
-pub type Value<'a> = str;
+#[cfg(not(feature = "kv"))]
+pub type Value<'a> = &'a str;
 
 mod sealed {
     /// Types for the `kv` argument.
     pub trait KVs<'a> {
-        fn into_kvs(self) -> Option<&'a [(&'a str, &'a super::Value<'a>)]>;
+        fn into_kvs(self) -> Option<&'a [(&'a str, super::Value<'a>)]>;
     }
 }
 
 // Types for the `kv` argument.
 
-impl<'a> KVs<'a> for &'a [(&'a str, &'a Value<'a>)] {
+impl<'a> KVs<'a> for &'a [(&'a str, Value<'a>)] {
     #[inline]
-    fn into_kvs(self) -> Option<&'a [(&'a str, &'a Value<'a>)]> {
+    fn into_kvs(self) -> Option<&'a [(&'a str, Value<'a>)]> {
         Some(self)
     }
 }
 
 impl<'a> KVs<'a> for () {
     #[inline]
-    fn into_kvs(self) -> Option<&'a [(&'a str, &'a Value<'a>)]> {
+    fn into_kvs(self) -> Option<&'a [(&'a str, Value<'a>)]> {
         None
     }
 }
@@ -41,13 +38,11 @@ fn log_impl(
     level: Level,
     &(target, module_path, file): &(&str, &'static str, &'static str),
     line: u32,
-    kvs: Option<&[(&str, &Value)]>,
+    kvs: Option<&[(&str, Value)]>,
 ) {
-    #[cfg(not(feature = "kv_unstable"))]
+    #[cfg(not(feature = "kv"))]
     if kvs.is_some() {
-        panic!(
-            "key-value support is experimental and must be enabled using the `kv_unstable` feature"
-        )
+        panic!("key-value support is experimental and must be enabled using the `kv` feature")
     }
 
     let mut builder = Record::builder();
@@ -60,7 +55,7 @@ fn log_impl(
         .file_static(Some(file))
         .line(Some(line));
 
-    #[cfg(feature = "kv_unstable")]
+    #[cfg(feature = "kv")]
     builder.key_values(&kvs);
 
     crate::logger().log(&builder.build());
@@ -87,3 +82,44 @@ pub fn log<'a, K>(
 pub fn enabled(level: Level, target: &str) -> bool {
     crate::logger().enabled(&Metadata::builder().level(level).target(target).build())
 }
+
+#[cfg(feature = "kv")]
+mod kv_support {
+    use crate::kv;
+
+    pub type Value<'a> = kv::Value<'a>;
+
+    // NOTE: Many functions here accept a double reference &&V
+    // This is so V itself can be ?Sized, while still letting us
+    // erase it to some dyn Trait (because &T is sized)
+
+    pub fn capture_to_value<'a, V: kv::ToValue + ?Sized>(v: &'a &'a V) -> Value<'a> {
+        v.to_value()
+    }
+
+    pub fn capture_debug<'a, V: core::fmt::Debug + ?Sized>(v: &'a &'a V) -> Value<'a> {
+        Value::from_debug(v)
+    }
+
+    pub fn capture_display<'a, V: core::fmt::Display + ?Sized>(v: &'a &'a V) -> Value<'a> {
+        Value::from_display(v)
+    }
+
+    #[cfg(feature = "kv_std")]
+    pub fn capture_error<'a>(v: &'a (dyn std::error::Error + 'static)) -> Value<'a> {
+        Value::from_dyn_error(v)
+    }
+
+    #[cfg(feature = "kv_sval")]
+    pub fn capture_sval<'a, V: sval::Value + ?Sized>(v: &'a &'a V) -> Value<'a> {
+        Value::from_sval(v)
+    }
+
+    #[cfg(feature = "kv_serde")]
+    pub fn capture_serde<'a, V: serde::Serialize + ?Sized>(v: &'a &'a V) -> Value<'a> {
+        Value::from_serde(v)
+    }
+}
+
+#[cfg(feature = "kv")]
+pub use self::kv_support::*;

src/kv/error.rs

@@ -11,7 +11,8 @@ enum Inner {
     #[cfg(feature = "std")]
     Boxed(std_support::BoxedError),
     Msg(&'static str),
-    Value(value_bag::Error),
+    #[cfg(feature = "value-bag")]
+    Value(crate::kv::value::inner::Error),
     Fmt,
 }
 
@@ -23,21 +24,23 @@ impl Error {
         }
     }
 
-    // Not public so we don't leak the `value_bag` API
-    pub(super) fn from_value(err: value_bag::Error) -> Self {
+    // Not public so we don't leak the `crate::kv::value::inner` API
+    #[cfg(feature = "value-bag")]
+    pub(super) fn from_value(err: crate::kv::value::inner::Error) -> Self {
         Error {
             inner: Inner::Value(err),
         }
     }
 
-    // Not public so we don't leak the `value_bag` API
-    pub(super) fn into_value(self) -> value_bag::Error {
+    // Not public so we don't leak the `crate::kv::value::inner` API
+    #[cfg(feature = "value-bag")]
+    pub(super) fn into_value(self) -> crate::kv::value::inner::Error {
         match self.inner {
             Inner::Value(err) => err,
-            #[cfg(feature = "kv_unstable_std")]
-            _ => value_bag::Error::boxed(self),
-            #[cfg(not(feature = "kv_unstable_std"))]
-            _ => value_bag::Error::msg("error inspecting a value"),
+            #[cfg(feature = "kv_std")]
+            _ => crate::kv::value::inner::Error::boxed(self),
+            #[cfg(not(feature = "kv_std"))]
+            _ => crate::kv::value::inner::Error::msg("error inspecting a value"),
         }
     }
 }
@@ -48,6 +51,7 @@ impl fmt::Display for Error {
         match &self.inner {
             #[cfg(feature = "std")]
             Boxed(err) => err.fmt(f),
+            #[cfg(feature = "value-bag")]
             Value(err) => err.fmt(f),
             Msg(msg) => msg.fmt(f),
             Fmt => fmt::Error.fmt(f),

src/kv/key.rs

@@ -30,7 +30,7 @@ impl ToKey for str {
     }
 }
 
-/// A key in a structured key-value pair.
+/// A key in a key-value.
 // These impls must only be based on the as_str() representation of the key
 // If a new field (such as an optional index) is added to the key they must not affect comparison
 #[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
@@ -93,7 +93,7 @@ mod std_support {
     }
 }
 
-#[cfg(feature = "kv_unstable_sval")]
+#[cfg(feature = "kv_sval")]
 mod sval_support {
     use super::*;
 
@@ -116,7 +116,7 @@ mod sval_support {
     }
 }
 
-#[cfg(feature = "kv_unstable_serde")]
+#[cfg(feature = "kv_serde")]
 mod serde_support {
     use super::*;