tracing icon indicating copy to clipboard operation
tracing copied to clipboard

Consider implementing `Value` for `T: serde::Serialize + Debug`

Open louismrose opened this issue 2 years ago • 3 comments

Feature Request

Would you accept a contribution to implement Value for T: serde::Serialize + Debug behind feature flag(s). Here's a simple example for JSON:

impl<T> Value for T
where
    T: serde::Serialize + Debug,
{
    fn record(&self, key: &Field, visitor: &mut dyn Visit) {
        let value = serde_json::to_string(self).unwrap_or_else(|_| format!("{:?}", self));
        visitor.record_str(key, &value);
    }
}

Note that alternative, user-land implementation options are somewhat limited because Value is sealed.

Crates

tracing-serde ?

Motivation

I'm working on a large-ish, long-standing Rust code base that use serde heavily to serialize types. We have lots of structs that derive serde::Serialize and use serde's attributes to control serialisation. A simplified example:

#[derive(Debug, serde::Serialize)]
struct Foo {
    bar: Vec<Bar>,
}

#[derive(Debug, serde::Serialize)]
struct Bar {
    #[serde(rename = "baz")]
    foo_baz: Option<String>,
}

I understand that the recommended future of serialising user types with tracing will be to use valuable. However, I think this means that I'd need to port all of my serde attributes to valuable (though happy to be corrected if I've missed something?) If I'm correct, this isn't going to be feasible in a large codebase with an established history of using serde serialisation.

Proposal

My initial, naive suggestion is to add this to tracing-serde behind a Cargo feature flag. This would mean that tracing-serde would need to take a conditional dependency on serde-json.

Alternatives

  • Use valuable -- but this involves duplicating all of my serde serialisation customisation for valuable (I think?)
  • Implement in user-land -- I don't think this is feasible as Value is sealed.

Thanks for your consideration

louismrose avatar Jul 19 '23 14:07 louismrose

IIUC this already exists in form of the valuable-serde bridge and was implemented in #1862. Of course valuable is still unstable. :( One alternative is to create something like the following:

struct DisplayJson<'a, T>(&'a T);

impl<'a, T> std::fmt::Display for DisplayJson<'a, T>
where
    T: serde::Serialize,
{
    fn display(&self, formatter: &mut std::fmt::Formatter) -> std::fmt::Result {
        // Serialization into a string should never fail unless the system is OOM
        f.write_str(&serde_json::to_string(self).map_err(|_| std::fmt::Error)?) 
    }
}

info!(the_object = %DisplayJson(&your_object), "example usage");

Minus potential inefficiencies and only being really pretty when using a json formatter I find this solution the most easily digestible.

Another alternative: impl Display directly for your type in terms of its Serialize impl. Although that might too easily hide the fact that a potentially costly serialization occurs.

SuperFluffy avatar Jul 20 '23 14:07 SuperFluffy

Thanks for the suggestion @SuperFluffy. We have something similar to the pattern you suggest at the moment, but we'd really prefer to write:

info!(the_object, "example usage")

which would be possible if Value were implemented for all T where T: serde::Serialize + Debug

louismrose avatar Jul 20 '23 18:07 louismrose

I'm not sure this is even possible as the other types that already implement Value (e.g. u64) also implement Serialize + Debug so this would be a conflicting implementation.

Also I don't think this is what you'd want as the subscriber would consider the serialized object as a string so for example the FmtSubscriber configured to emit JSON would have a string field containing an escaped JSON.

If this is still something you're interested it, I think you might want to look at the valuable_json example if that's the behaviour you're looking for? Note that valuable support is still unstable though.

mladedav avatar May 08 '24 08:05 mladedav

Yeah, the answer is to use Valuable. I'll close this issue then.

davidbarsky avatar May 25 '24 17:05 davidbarsky