tokio-rs / tracing

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

core: Make `dyn Any + 'static` a primitive, recordable type #905

Open davidbarsky opened 4 years ago

davidbarsky commented 4 years ago

Feature Request

Crates

Motivation

Proposal

David:

Implement 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.

Eliza, continuing in #819:

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".

To address the "I can only record types that I know how to downcast" problem, Layers and Subscribers would be encouraged to provide a registry of formatters + downcasters for std::any::Any that the end-user (e.g., the person creating a binary executable) would register when creating their subscriber or layer.

Alternatives

Don't do this. Based off Eliza's response, I worry that supporting dyn Any + 'static is strictly worse than something like Value::from_serde or the current practice of extensions traits on Spans that allow users to directly write to a Subscriber's extensions.

On the other hand, a registry of formatting/downcasting functions for std::any::Any could serve nice compliment for specialized, associative collections like http::Request and friends, in that it provides a handy escape hatch for types/crates that either can't or won't take a public dependency on tracing_core::Value. That said, I'm not sure this benefit is worth the downsides because Serde's support for implementing Serialize/Deserialize on foreign types would handle this usecase nicely.

davidbarsky commented 4 years ago

I think my biggest motivation for this proposal was supporting recording nested structures, as fmt::Debug recordings of, for instance, http::HeaderMap are non-ideal. This also has interesting implications for the 32 field limit—how would nested keys be treated by the key/value system? Given that:

...there's a rich design space available for us to explore.

anp commented 4 years ago

I'm a bigger fan of Value::from_serde than this approach for that use case. Another one I have is that I'd like to pass a value directly through to the subscriber and it seems like this could be useful there.

the current practice of extensions traits on Spans that allow users to directly write to a Subscriber's extensions

I'm not familiar with this but it seems like it might not work super well for the case of wanting to print JS values to console.log as those should often be events, although I may be misunderstanding this brief description. If it's a useful alternative to this proposal it might be worth writing something about it to contrast.

The ideal, IMO would be to support something like this in tracing-wasm:

let foo: JsValue = ...;
info!(foo, "button clicked");

which would end up calling something like console.log("button clicked foo=", foo) under the hood. It would be fine if we were limited to an extension trait that returned a concrete type the backend can probe for:

let foo: JsValue = ...;
info!(foo = foo.pretty(), "button clicked"); // returns something like ConsoleValue(foo)
hawkw commented 4 years ago

Supporting downcasting for Values is definitely not as powerful as arbitrary serde serialization — and it's not really intended as a replacement for that. But downcasting is something we should probably implement anyway, and it may also cover some of those use cases in the short term.