Skip to content

Commit

Permalink
Merge pull request #613 from rust-lang/feat/kv-cleanup
Browse files Browse the repository at this point in the history
Get structured logging API ready for stabilization
  • Loading branch information
KodrAus authored Feb 19, 2024
2 parents 7cb6a01 + 2b220bf commit f0f7494
Show file tree
Hide file tree
Showing 14 changed files with 1,406 additions and 494 deletions.
36 changes: 19 additions & 17 deletions .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down
24 changes: 15 additions & 9 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down
17 changes: 11 additions & 6 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion benches/value.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#![cfg(feature = "kv_unstable")]
#![cfg(feature = "kv")]
#![feature(test)]

use log::kv::Value;
Expand Down
66 changes: 51 additions & 15 deletions src/__private_api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Expand All @@ -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();
Expand All @@ -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());
Expand All @@ -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::*;
22 changes: 13 additions & 9 deletions src/kv/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}

Expand All @@ -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"),
}
}
}
Expand All @@ -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),
Expand Down
6 changes: 3 additions & 3 deletions src/kv/key.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down Expand Up @@ -93,7 +93,7 @@ mod std_support {
}
}

#[cfg(feature = "kv_unstable_sval")]
#[cfg(feature = "kv_sval")]
mod sval_support {
use super::*;

Expand All @@ -116,7 +116,7 @@ mod sval_support {
}
}

#[cfg(feature = "kv_unstable_serde")]
#[cfg(feature = "kv_serde")]
mod serde_support {
use super::*;

Expand Down
Loading

0 comments on commit f0f7494

Please sign in to comment.