client_rust
client_rust copied to clipboard
Consider removing text::Encode in favour of serde::Serialize
Serde's Serialize trait is implemented for dozens of types in the Rust ecosystem, its derive code is fairly well optimised, and it is the most popular serialization framework we have for Rust.
What do you think of removing the Encode trait in favour of Serialize, to get all the nice things there are in Serde for free?
I would be in favor of that. I should have thought of it before.
@nox would you like to write a first proposal? Before you go all the way, I would suggest writing a small proof-of-concept first to make sure using serde is a viable path forward.
Sure, I'll write a struct Bridge<T>(T)
that implements Encode
using T
's implementation of Serialize
, in a separate crate.
I pushed https://github.com/nox/serde_prometheus_labels earlier today.
I published serde_prometheus_labels which provides its own Family<Labels, Metric>
type using serde as the encoder.
How would this interact with a problem like the one in #75? I mean, ()
also implements Serialize/Deserialize but you probably shouldn't be able to use ()
as a label, for instance. So I'm not sure Encode
can be entirely replaced by serde's traits, as serde's traits are basically implemented for too many types that aren't suitable.
I mean,
()
also implements Serialize/Deserialize but you probably shouldn't be able to use()
as a label, for instance.
Why not? ()
should mean "no labels at all".
So I'm not sure
Encode
can be entirely replaced by serde's traits, as serde's traits are basically implemented for too many types that aren't suitable.
I agree to this, but I would rather rely on serde and not be blocked on some missing implementation. It made the job for us, that's why I published that stuff as a separate crate.
With https://github.com/prometheus/client_rust/pull/105 I don't think we should proceed here, i.e. I don't see the value of replacing the new traits.
@nox I am closing here for now. Please comment here in case I am missing something.
The above said I do believe implementing EncodeLabelSet
, EncodeLabel
, EncodeLabelKey
and EncodeLabelValue
for more types in std
would be helpful.