search

tokio-rs / tracing

Application level tracing for Rust.
https://tracing.rs
MIT License
5.51k stars 723 forks source link

`Value::from_serde` ? #819

Open anp opened 4 years ago

anp commented 4 years ago

Feature Request

Motivation

I'd like to allow my subscriber to record arbitrarily nested objects from users' macros.

Proposal

I'd like some way to create a Value from the erased-serde traits (erased to reduce binary bloat). The basic sketch proposed by @KodrAus in https://github.com/rust-lang/log/issues/328#issuecomment-657275448 for the log crate seems compelling to me but I haven't worked through all of the implications for tracing.

Alternatives

The most significant assumption of this request is that serde-based traits should be the public API. It's possible that we would be better off extending Value to support nesting in a different way (and offering a polyfill for serde implementors?).

It's also possible that Value is not the right place to include this compatibility but my naive impression is that it's probably the right call.

Another potential alternative would be to reuse the Value type from the log crate's kv support or to work with them to extract that type into a lower-level shared crate. This seems like a great target to drive towards but I suspect both projects will be better served by converging on a shared type once they both have their respective use cases understood and addressed.

hawkw commented 4 years ago

Related: #824 (which also deals with adding Value implementations for more structured types).

hawkw commented 4 years ago

In general, while this sounds pretty simple, it's a pretty big issue, since it kind of depends on two tasks:

This is definitely something I think we all want to see --- currently, Value is sealed primarily to reserve the ability to make these changes without breaking existing uses! I just wanted to note what is required.

An ideal solution would meet the following goals:

Some of these are more negotiable than others --- in particular, a feature-flagged serde dependency would be acceptable if absolutely necessary, since serde is 1.0.

Complete serde support would probably require tracing's Visit trait to include serde's entire data model. In the past, we opted to restrict the number of primitives significantly (we don't support floats, or anything smaller than u64, for example). It'd be worth getting @carllerche's take on whether or not the motivations for this are still relevant.

KodrAus commented 4 years ago

I just merged in some refactoring to log's Value type that you might find interesting. @hawkw that includes the const-fn based specialization we checked out a while ago.

I've been wanting to pull the Value type out of log and turning it into a library for capturing some value in a structured bag. You mentioned you don't want tracing-core to depend on serde, which I think makes a lot of sense. Would you consider having tracing-core (possibly optionally) depend on this structured bag library that itself optionally depends on serde? So instead of having a series of methods on Visit to support nested values (I wrote a library for streaming nested values and there's a lot of design space there you may or may not want to take on) it has just one method for passing along one of these structured bags and the consumer can decide whether they want to treat it like a Serialize or something else.

hawkw commented 4 years ago

I just merged in some refactoring to log's Value type that you might find interesting. @hawkw that includes the const-fn based specialization we checked out a while ago.

Very cool. I need to take a closer look at this strategy. Haven't had time yet, but I'm definitely interested in seeing what you've done. :)

hawkw commented 4 years ago

Another possible solution for supporting serde (and pretty much anything else, it turns out), while requiring a pretty minimal change to the existing Value system, is what @davidbarsky suggested on issue https://github.com/tokio-rs/tracing/issues/845#issuecomment-667724338 (which was not really the right place for this suggestion, but I digress): make dyn Any + 'static a "primitive" value type:

A possible extension to this proposal (as well as #819, perhaps?) would be to allow tracing_core::Value to be implemented on T: dyn std::any::Any + 'static. This would allow layers to opt-in to handle formatting of arbitrary types without requiring crates such as hyperium/http to take a public dependency on tracing_core::Value. I haven't really thought through the ecosystem-wide implications of supporting this, but I suspect tracing-subscriber could support a registry of formatters for arbitrary types, such that subscribers and layers won't need to take an explicit dependency on a given formatter.

Then, we could add a Visit::record_any method to the Visit trait, where the visitor can just downcast the recorded type to whatever it wants (and, eventually, when we introduce ValueSet indexing, ValueSet::get_any). We would probably add a tracing::field::any helper function, and/or a new sigil, for recording something as an Any without having to cast it as a dyn Any.

This does have one significant downside: currently, all the record methods fall through to fmt::Debug. This means that if a subscriber does not have special handling for a particular type, it can just print it. However, an Any trait object is not known to implement fmt::Debug. This means that if you don't have a particular type to downcast to, you can't really do anything of value with an Any.

We could hack around that limitation by making the Any value type be T: Any + Debug, and recording it by wrapping it in a special struct that stores a dyn Any and a closure that downcasts the Any to its original type so that it can be formatted with Debug. But, this is a bit awkward.

This would also, critically, not allow us to record arbitrary Serialize types: only known ones. If the subscriber knows specific Any types it wants to serialize, it could downcast them and use their Serialize implementations. However, it cannot say "i will record anything that serde can serialize".

davidbarsky commented 4 years ago

@hawkw Finally! I made a new issue: https://github.com/tokio-rs/tracing/issues/905

0xForerunner commented 1 year ago

Don't know much about the details, but perhaps erased-serde could be useful here if object safety is the issue? This would be an amazing feature.