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

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

Open
davidbarsky opened this issue Aug 8, 2020 · 3 comments
Open

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

davidbarsky opened this issue Aug 8, 2020 · 3 comments

Comments

@davidbarsky
Copy link
Member

Feature Request

Crates

  • tracing-core

Motivation

  • More easily record structured key/value pairs using tracing_core::Value without needing to take a public dependency on serde 1.0.

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.

  • Value::from_serde, or something equivalent to Value::from_serde, seems like a more general purpose and correct solution than dyn Any + 'static because it doesn't require Layer or Subscriber-specific knowledge of types.
  • While Subscriber/Layer-specific extension traits are Layer/Subscriber-specific, this approach doesn't introduce a potential source of complexity and poor interoperability between Subscribers to tracing_core.
  • dyn Any + 'static could prevent the usage of non-'static references, requiring a clone to record a value. This is something that tracing has avoided when recording other values and wouldn't hit if tracing_core::Value supported something serde-style serializers.

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 davidbarsky changed the title tracing_core: Make dyn Any + 'static a primitive, recordable type core: Make dyn Any + 'static a primitive, recordable type Aug 8, 2020
@davidbarsky
Copy link
Member Author

davidbarsky commented Aug 8, 2020

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
Copy link
Contributor

anp commented Aug 23, 2020

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
Copy link
Member

hawkw commented Aug 24, 2020

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants