Add valuable support for Uuids #583
Labels
No labels
breaking
bug
dependencies
good first issue
help wanted
unstable
No milestone
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
gh-uuid-rs/uuid#583
Loading…
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Motivation
The Tokio team is releasing a new crate,
valuable, which is intended to be used for passing in custom types as fields in logs. SinceUuids are commonly used in applications that also usetracinglike Kanidm, we should implement theValuabletrait behind a feature gate.Solution
Derive the
Valuabletrait.Alternatives
Uuids as strings.Uuidand rely on usingas_bytesin the trait implementation. This would be incredibly annoying though, because it would mean applications would have to use:Hopefully
tracingwill add a sigil to reduce verbosity withas_value()similar to how?and%are used forDebugandDisplayformatting, but avoiding a wrapper type would be ideal.Is it blocking?
No
Anything else?
The
valuablecrate is still in the very early stages, and could possibly have breaking changes. However, I think the very minimal API that this crate would depend on (just deriving the trait) is unlikely to change.I've been expecting this for a while, so we've already got a bit of a plan in place 🙂 We've got
zerocopyas an example of bringing in unstable dependencies in the stable library. So we can do the same foruuid.As for representation, I think we'd probably be most useful implementing
Valuablemanually with a text-based format (https://github.com/tokio-rs/valuable/issues/43#issuecomment-854111197), rather than deriving it and getting a list of raw bytes. The reason being users in that logging context can recognize a stringified UUID as being a UUID, but 16 bytes doesn't give any clues. What I'm not sure of is whether we can/should implementas_valuedifferently tovisitso that you get bytes from one and a string from the other:My gut feel on that though is "no that's a bad idea".
I think it would be better to visit
Uuids as sequences of bytes, since the string representation is already possible intracingbecause anyDebugtype can be passed in asdyn Debug. However, there's no way to get the raw bytes. Visiting the bytes would allow crates liketracing-forestto easily read inUuids and pass them back to the application if desired without any extra overhead of formatting and parsing.In terms of 16 bytes not representing the semantic meaning of a
Uuid: values passed to subscribers have an associated&'static strname. If a subscriber is expecting aUuid, then the crate can specify which name to use. Otherwise, it can get passed to thedyn Debugvisitor.So for the
tracing-forestuse-case what you'd like to do is receive avalue, determine that it's aUuidby its associated name, and then use something likeUuid::from_sliceon the result ofvisit_primitive_sliceto capture that value until it's ready to be re-emitted? Relying on stringly-typed APIs to detect that you're looking at a UUID and converting it one way or another seems a little dubious to me, but it could be ok if that's the directionvaluablerecommends.I think part of the problem here may be that
valuableimpliestracingas a primary use-case, which in turn implies a preference for human-readable representations of your data, since you want it to be understood on the other end. If we were just consideringvaluablesupport independently of the diagnostics usecase then using the raw 16-byte representation would be perfectly suitable. Iftracing-forestwould need to match onUuid's associated name either way, then my preference would still be for using a text-based representation for UUIDs. That maximizes interoperability with tooling at the other end without forcing stringly-typed matches on all users, and the parser is not expensive. If you already need to potentially allocate maps and strings to cache values then parsing UUIDs isn't likely to have much extra impact.I agree that stringly-typed APIs are kinda sus. However, the official blog post specifically uses an example to inspect an HTTP header type with very specific fields. Furthermore,
valuableallows for structs to have named fields, so we could make theValuerepresentation have the bytes as a named field for further checking. In terms of ergonomics,valuableis intended to offer you a zero cost view into a type, and stringifying defeats this purpose: it should be up to theSubscriberboth a) if it even wants to format theUuidand b) which format to use.Another detail to consider is that providing the bytes for a
Valuableimplementation forces nothing new ontracingusers. It's just as easy to pass in theUuidas adyn Debug, which already does exactly what you're proposing and is stable.Hmm, when would you expect an end-user of
tracingto capture aUuidusing itsValuableimplementation over itsDebug? I was under the impression thattracingmay choose to replace their entire value API withvaluablein the future so if that’s the case then users won’t really have a choice to make between a human-readableDebugand machine-readableValuable. Today I also wouldn’t expect anybody to make that choice anyway if the default compiles.I think what’s missing from this discussion that’s going to keep us going in circles at the moment is more input on potential usecases for a
valuableimplementation. It’s a brand new API so we don’t have access to a lot of that, but I’m just a bit reluctant to commit to a direction without a bigger picture to know what we gain or lose by opting for one representation over another.Using tracing:
An idea I had for
tracing-forestwas to allow for different contexts, so that disjoint concurrent tasks can map to entirely different contexts with their own log trees, without having to enumerate each contexts at compile time. This could useUuids within theSubscriberto map a log to its context, without ever emitting the ids of each context to the logs. If there were many logs, this would require many unseen and unnecessary formats.Outside of tracing
To address your "going in circles" point: I think
valuablefills a very similar role toserde, and will show up in similar places. Likevaluable,serdealso has an abstract introspection layer. This layer relies on aSerializerto visit and translate it into a tangible format. On the other hand,valuableis just the first half of this, an abstract introspection layer. Thus, the entire responsibility of theSerializeris expected of the user ofvaluable::Values. Because of this, I claim thatvaluableis not meant to serve as the end-of-the-line processing unit; it's an intermediate tool that is followed by more processing, just like howSerializers do more work on the data once it's recorded. This applies to tracing as well.Uuid'sSerializeimplementation even makes the distinction that it is up to theSerializerhow theUuidshould be serialized. The currentSerializeimplementation looks like this:Making the
Valuabletrait return a string instead of the raw bytes would be analogous to implementingSerializelike this (I didn't verify this compiles, but the idea is here):This example seems dumb, because all these decisions are already made known to us (we can call
Serializer::is_human_readable). However,valuablebasically gives us this introspection before any decisions are made, meaning that we should be as unassuming as possible, and therefore do the least amount of work possible.Also, since this would be an unstable feature, wouldn't it still be okay to change the implementation down the line before it stabilizes if we decide we want to? If this is the case that I think we should try it this way and we can get feedback on it first.
Yeh, I can see the value of the role
valuablefills, and I think it's a great one. The issue we've got is that its introspection tools are great for compound values like maps, lists, and enums, but it doesn't give us much for primitive values like UUIDs, IP addresses, and such to describe their content in a way that can be recognized by a person and inspected programmatically. TheValue::Pathvariant seems to suggest a path where these types end up called out specially. I wouldn't want to suggest there should be aValue::Uuidvariant though. As much as I'd like to treatvaluableas an independent tool for what it is, I don't think we can really take a direction that penalizes the primary diagnostics use-case thattracingsuggests.I was thinking we could just copy the approach used for IP addresses in UUID, but I see we haven't got them yet in
valuable. I think this is something we'll need some input from thevaluablemaintainers on, because we might be one of the first libraries looking closely at how to implement it. I'll go and open an issue over invaluableand cc you.I've opened https://github.com/tokio-rs/valuable/issues/87 to get some input from the
valuablemaintainers.Coming back around to this, it looks like we don't have a clear best path forward, so as much as I'd like to keep the default implementation of
valuableuseful for UUIDs in diagnostics I think the most direct representation of the data (that is, as bytes) is a reasonable starting point.We can follow the same approach as
zerocopyand add an unstablevaluablefeature touuidwith an implementation.interested in this functionality. Is there any outstanding work on this or is there interest for contributing a PR for this?
Hi @kaiba42 👋 I'd be open to a PR that added this, but would like to see how a few alternative representations would work in the current
valuabledata model. Currently, the most direct representation of a UUID invaluablelooks like it would bevaluable::Value::U128, but we may also be able to usevaluable::Value::Listablewith a primitive slice, orvaluable::Value::String.I built a client-side implementation of this logic using Nuxt 3. If anyone needs a quick UI to generate UUIDs/GUID without setting up a repo, I have it running here: knothing.com. Hope it helps!"