Don't be generic over numbers #1
Conversation
Adopt encoding style similar to serde. `EncodeXXX` for a type that can be encoded (see serde's `Serialize`) and `XXXEncoder` for a supported encoding i.e. text and protobuf (see serde's `Serializer`). - Compatible with Rust's additive features. Enabling the `protobuf` feature does not change type signatures. - `EncodeMetric` is trait object safe, and thus `Registry` can use dynamic dispatch. - Implement a single encoding trait `EncodeMetric` per metric type instead of one implementation per metric AND per encoding format. - Leverage `std::fmt::Write` instead of `std::io::Write`. The OpenMetrics text format has to be unicode compliant. The OpenMetrics Protobuf format requires labels to be valid strings. Signed-off-by: Max Inden <mail@max-inden.de>
|
Thanks for providing the patch Thomas! Very much appreciate you exploring alternative approaches. I am not convinced this is the best way forward. This patch reduces complexity by cutting out the the I don't expect the majority of all library users to implement any of the With that in mind, I don't think the reduced complexity of this patch is worth the additional methods on Let's keep this pull request open. I want to experiment with the large refactoring in downstream code which might change my mind. |
|
Up to you! My main concern is the use of type parameters for a problem that doesn't need to be "open to extension". The set of types is fixed within prometheus but the usage of type parameters suggests that it is not and should instead be extended.
The bottom line is that I think we are overly abstract here which creates a fair bit of indirection and complexity. |
We only have a limited number of data types in prometheus. Instead of being generic over them, unroll the abstractions manually and one implementation per data type.