Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/valuable integration #1608

Merged
merged 27 commits into from
Jan 21, 2022
Merged
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
13f01f8
Initial commit to add Valuable
daniel-emotech Sep 30, 2021
1638739
Implement Value for Valuable expose in tracing
xd009642 Oct 1, 2021
bb74cfb
Create ValuableValue
xd009642 Oct 1, 2021
cfee46a
Add doc comments for `ValuableValue`
xd009642 Oct 4, 2021
3f3a477
Add tracing_unstable config and Debug impl
xd009642 Oct 4, 2021
e228504
Apply suggestions
xd009642 Oct 4, 2021
5919719
Add simple example
xd009642 Oct 5, 2021
344c43e
import valuable function
xd009642 Oct 5, 2021
7d2e431
Make example work without valuable
xd009642 Oct 5, 2021
b848ee0
Fix formatting
xd009642 Oct 5, 2021
bc4d326
Move things to v0.1.x
xd009642 Oct 7, 2021
9735ac5
Add instrument macro example
xd009642 Oct 14, 2021
9ade843
Update to published valuable
xd009642 Jan 3, 2022
5e93474
Merge branch 'v0.1.x' into feature/valuable-integration
xd009642 Jan 5, 2022
3bb628c
Allow dead code
xd009642 Jan 11, 2022
e7770f6
Merge branch 'v0.1.x' into feature/valuable-integration
davidbarsky Jan 19, 2022
0ef179f
Apply final feedback docs/example
xd009642 Jan 20, 2022
d9f1aa8
Merge branch 'feature/valuable-integration' of github.com:xd009642/tr…
xd009642 Jan 20, 2022
e5fc2d3
make valuable example always depend on valuable
xd009642 Jan 20, 2022
aed9534
Fix silly mistake
xd009642 Jan 20, 2022
3177709
Update examples/examples/valuable_instrument.rs
xd009642 Jan 20, 2022
c7ac0ac
Update examples/examples/valuable.rs
xd009642 Jan 20, 2022
a24c78d
cargo fmt
xd009642 Jan 20, 2022
67e1bbd
add a comment explaining the two spans
xd009642 Jan 20, 2022
9d22b47
Update examples/examples/valuable.rs
hawkw Jan 21, 2022
16e0c8b
Merge branch 'v0.1.x' into feature/valuable-integration
hawkw Jan 21, 2022
062a6ea
rustfmt
hawkw Jan 21, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions examples/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -58,3 +58,9 @@ opentelemetry-jaeger = "0.15"
# fmt examples
snafu = "0.6.10"
thiserror = "1.0.26"

# valuable examples
valuable = { version = "0.1.0", features = ["derive"] }

[target.'cfg(tracing_unstable)'.dependencies]
tracing-core = { path = "../tracing-core", version = "0.1", features = ["valuable"]}
57 changes: 57 additions & 0 deletions examples/examples/valuable.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
#![allow(dead_code)]
//! This example shows how a field value may be recorded using the `valuable`
//! crate (https://crates.io/crates/valuable).
//!
//! `valuable` provides a lightweight but flexible way to record structured data, allowing
//! visitors to extract individual fields or elements of structs, maps, arrays, and other
//! nested structures.
//!
//! `tracing`'s support for `valuable` is currently feature flagged. Additionally, `valuable`
//! support is considered an *unstable feature*: in order to use `valuable` with `tracing`,
//! the project must be built with `RUSTFLAGS="--cfg tracing_unstable`.
//!
//! Therefore, when `valuable` support is not enabled, this example falls back to using
//! `fmt::Debug` to record fields that implement `valuable::Valuable`.
#[cfg(tracing_unstable)]
use tracing::field::valuable;
use tracing::{info, info_span};
use valuable::Valuable;

#[derive(Clone, Debug, Valuable)]
struct User {
name: String,
age: u32,
address: Address,
}

#[derive(Clone, Debug, Valuable)]
struct Address {
country: String,
city: String,
street: String,
}

fn main() {
tracing_subscriber::fmt()
.with_max_level(tracing::Level::TRACE)
.init();

let user = User {
name: "Arwen Undomiel".to_string(),
age: 3000,
address: Address {
country: "Middle Earth".to_string(),
city: "Rivendell".to_string(),
street: "leafy lane".to_string(),
},
};

#[cfg(tracing_unstable)]
let span = info_span!("Processing", user = valuable(&user));

#[cfg(not(tracing_unstable))]
let span = info_span!("Processing", user = ?user);
xd009642 marked this conversation as resolved.
Show resolved Hide resolved

let _handle = span.enter();
info!("Nothing to do");
}
45 changes: 45 additions & 0 deletions examples/examples/valuable_instrument.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
#[cfg(tracing_unstable)]
mod app {
use std::collections::HashMap;
use tracing::field::valuable;
use tracing::{info, info_span, instrument};
use valuable::Valuable;

#[derive(Valuable)]
struct Headers<'a> {
headers: HashMap<&'a str, &'a str>,
}

// Current there's no way to automatically apply valuable to a type, so we need to make use of
// the fields argument for instrument
#[instrument(fields(headers=valuable(&headers)))]
fn process(headers: Headers) {
info!("Handle request")
}

pub fn run() {
let headers = [
("content-type", "application/json"),
("content-length", "568"),
("server", "github.com"),
]
.iter()
.cloned()
.collect::<HashMap<_, _>>();

let http_headers = Headers { headers };

process(http_headers);
}
}

fn main() {
tracing_subscriber::fmt()
.with_max_level(tracing::Level::TRACE)
.init();

#[cfg(tracing_unstable)]
app::run();
#[cfg(not(tracing_unstable))]
println!("Nothing to do, this example needs --cfg=tracing_unstable to run");
}
5 changes: 4 additions & 1 deletion tracing-core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ edition = "2018"
rust-version = "1.42.0"

[features]
default = ["std"]
default = ["std", "valuable/std"]
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this causes cargo to download the valuable crate even though it isn't actually used without tracing_unstable

std = ["lazy_static"]

[badges]
Expand All @@ -36,6 +36,9 @@ maintenance = { status = "actively-developed" }
[dependencies]
lazy_static = { version = "1", optional = true }

[target.'cfg(tracing_unstable)'.dependencies]
valuable = { version = "0.1.0", optional = true, default_features = false }

[package.metadata.docs.rs]
all-features = true
rustdoc-args = ["--cfg", "docsrs"]
51 changes: 51 additions & 0 deletions tracing-core/src/field.rs
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,15 @@ pub struct Iter {
/// [`Event`]: ../event/struct.Event.html
/// [`ValueSet`]: struct.ValueSet.html
pub trait Visit {
/// Visits an arbitrary type implementing the [`valuable`] crate's `Valuable` trait.
///
/// [`valuable`]: https://docs.rs/valuable
#[cfg(all(tracing_unstable, feature = "valuable"))]
xd009642 marked this conversation as resolved.
Show resolved Hide resolved
#[cfg_attr(docsrs, doc(cfg(all(tracing_unstable, feature = "valuable"))))]
fn record_value(&mut self, field: &Field, value: &dyn valuable::Valuable) {
self.record_debug(field, &value)
}

/// Visit a double-precision floating point value.
fn record_f64(&mut self, field: &Field, value: f64) {
self.record_debug(field, &value)
Expand Down Expand Up @@ -249,6 +258,14 @@ pub struct DisplayValue<T: fmt::Display>(T);
#[derive(Clone)]
pub struct DebugValue<T: fmt::Debug>(T);

/// A `Value` which serializes using [`Valuable`].
///
/// [`Valuable`]: https://docs.rs/valuable/latest/valuable/trait.Valuable.html
#[derive(Clone)]
#[cfg(all(tracing_unstable, feature = "valuable"))]
xd009642 marked this conversation as resolved.
Show resolved Hide resolved
#[cfg_attr(docsrs, doc(cfg(all(tracing_unstable, feature = "valuable"))))]
pub struct ValuableValue<T: valuable::Valuable>(T);

/// Wraps a type implementing `fmt::Display` as a `Value` that can be
/// recorded using its `Display` implementation.
pub fn display<T>(t: T) -> DisplayValue<T>
Expand All @@ -267,6 +284,19 @@ where
DebugValue(t)
}

/// Wraps a type implementing [`Valuable`] as a `Value` that
/// can be recorded using its `Valuable` implementation.
///
/// [`Valuable`]: https://docs.rs/valuable/latest/valuable/trait.Valuable.html
#[cfg(all(tracing_unstable, feature = "valuable"))]
xd009642 marked this conversation as resolved.
Show resolved Hide resolved
#[cfg_attr(docsrs, doc(cfg(all(tracing_unstable, feature = "valuable"))))]
pub fn valuable<T>(t: T) -> ValuableValue<T>
xd009642 marked this conversation as resolved.
Show resolved Hide resolved
where
T: valuable::Valuable,
{
ValuableValue(t)
}

// ===== impl Visit =====

impl<'a, 'b> Visit for fmt::DebugStruct<'a, 'b> {
Expand Down Expand Up @@ -539,6 +569,27 @@ impl<T: fmt::Debug> fmt::Debug for DebugValue<T> {
}
}

// ===== impl ValuableValue =====

#[cfg(all(tracing_unstable, feature = "valuable"))]
impl<T: valuable::Valuable> crate::sealed::Sealed for ValuableValue<T> {}

#[cfg(all(tracing_unstable, feature = "valuable"))]
#[cfg_attr(docsrs, doc(cfg(all(tracing_unstable, feature = "valuable"))))]
impl<T: valuable::Valuable> Value for ValuableValue<T> {
fn record(&self, key: &Field, visitor: &mut dyn Visit) {
visitor.record_value(key, &self.0)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a potential thought, although i'm not sure whether or not it's necessary: valuable's list of primitive types has some overlaps with tracing's. we may want to consider doing something where we call Valuable::as_value, match on the result, and call the Visit type's record_$TY methods for tracing primitive types if the value is a tracing primitive? e.g.

Suggested change
visitor.record_value(key, &self.0)
match self.0.as_value() {
valuable::Value::I64(v) => visitor.record_i64(key, v),
valuable::Value::U64(v) => visitor.record_u64(key, v),
// ...
_ => visitor.record_value(key, &self.0)
}

I'm not totally sure if this is the right approach or not, but the thought process here is that visitors that are aware of Valuable will probably still implement the record_$TY methods for various tracing primitives as well, since a majority of the values recorded will not be Valuables. but, the visitor implementation could always handle the duplicated code itself...idk if this is worth doing here or not. probably worth testing it out in your own code and seeing how it feels...

}
}

#[cfg(all(tracing_unstable, feature = "valuable"))]
xd009642 marked this conversation as resolved.
Show resolved Hide resolved
#[cfg_attr(docsrs, doc(cfg(all(tracing_unstable, feature = "valuable"))))]
impl<T: valuable::Valuable> fmt::Debug for ValuableValue<T> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "{:?}", &self.0 as &dyn valuable::Valuable)
}
}

impl crate::sealed::Sealed for Empty {}
impl Value for Empty {
#[inline]
Expand Down
1 change: 1 addition & 0 deletions tracing/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ async-await = []
std = ["tracing-core/std"]
log-always = ["log"]
attributes = ["tracing-attributes"]
valuable = ["tracing-core/valuable"]

[[bench]]
name = "subscriber"
Expand Down