Enhancement proposals for rust-prometheus

[mxinden]

During the last 10 month, helping to maintain this library, I had several ideas how to enhance rust-prometheus. I prototyped my ideas, starting from scratch end of last year, somewhat accidentally ending up with a full-featured client library - rust-open-metrics-client. Below I want to describe the differences between rust-prometheus and rust-open-metrics-client.

I am posting this here first off to collect feedback. In case people like some of my ideas, we could port them to rust-prometheus.

• Strongly typed label sets: While rust-prometheus treats both label keys and values as strings, rust-open-metrics-client allows any Rust type to act as a label set. More concretely with rust-open-metrics-client one can use both (String, String) as a label set, as well as a defined custom type. Example:

  #[derive(Clone, Hash, PartialEq, Eq, Encode)]
  struct Labels {
    method: Method,
    path: String,
  };
  
  #[derive(Clone, Hash, PartialEq, Eq, Encode)]
  enum Method {
    GET,
    PUT,
  };
  
  let http_requests = Family::<Labels, Counter<AtomicU64>>::default();
  
  http_requests.get_or_create(
      &Labels { method: Method::GET, path: "/metrics".to_string() }
  ).inc();

  Using a custom type instead of some string type gives the user all of the safety benefits the Rust type system provides. In addition, using simple enums instead of a string allows various optimizations, as the former can potentially be stack allocated.

• Reduced allocations in hot-path: There are two performance critical paths when instrumenting applications, metric tracking and metric encoding. I think rust-prometheus can improve in the latter case. I.e. the Collector trait forces string allocations of the metric type, help,name, ... on each collect call. rust-open-metrics-client instead uses the visitor pattern, requiring no allocations when collecting, instead, directly writing the encoded metrics out to the provided I/O resource. This leads to a large performance improvement of ~40% (see benchmarks).

• Smaller scoped metric metadata: In rust-prometheus metric metadata (name, help, type) is both carried by the metric reference passed to business logic as well as the metric reference owned by the metric registry. rust-open-meitrcs-client tracks metric metadata in the registry only. The metric reference owned by the business logic e.g. compiles down to a plain AtomicXXX for a counter.

• Access to underlying atomics: rust-open-metrics-client allows advanced users to access the underlying atomic. Useful e.g. for https://github.com/tikv/rust-prometheus/issues/358.

• Open Metrics Compliance: rust-open-metrics-client is compatible with the Open Metrics specification. Given that the Open Metrics format is backwardds-compatible with the Prometheus format, rust-open-metrics-client is compatible with both the Open Metrics format as well as the Prometheus format.

You can find the code of rust-open-metrics-client here. In addition I hope I did a reasonable job documenting the crate.

Curious what you think and whether you would like to see any of the features of rust-open-metrics-client to be ported to rust-prometheus.

[breezewish]

I agree with most of the ideas.

1. I think a whole code refactoring is necessarily, considering that previously the interfaces are designed to be as much close with Golang's library as possible, which brings some performance pitfalls and not respecting some Rust best practices or utilizing some Rust nice features.

2. We now provide different performance hacks for improving metric tracking performance: int metrics, local metrics, static metrics, thread local metrics. It would be nice to reduce these features to only one or two. This would make users more easily to follow the path of best performance, and reduce our maintenance work. Since we have so many performance hacks, they are not well maintained as well, for example:

   • static-metrics are using some outdated techniques. Static labels can be written by using #[drive(...)] nowadays.
   • Thread Local metrics is still hard to use, requires many boilerplate code.
   • IntHistograms is missing.
   • local histograms are actually not local when using its timer guard.

3. Considering that OpenMetrics is very similar to the Prometheus format, I'm also thinking about sharing some common code base for both projects :)

[lucab]

That's great to see! It looks like there is large low-level rework behind these changes, that is dropping the protobuf internal structures, am I reading that right? If so, is there any problem in dropping PB from this crate?

[Yisaer]

/ping

[mxinden]

That's great to see! It looks like there is large low-level rework behind these changes, that is dropping the protobuf internal structures, am I reading that right?

@lucab I am not sure I understand correctly. Are you referring to rust-prometheus or rust-open-metrics-client? In case you are referring to the former (rust-prometheus), I would be in favor of at least dropping the intermediary protobuf format in rust-prometheus, maybe even drop protobuf support all together. See https://github.com/tikv/rust-prometheus/pull/387#issuecomment-780697444 for details. In case you are referring to the latter (rust-open-metrics-client), this crate is written from scratch and indeed does not use the protobuf data structures as its in-memory data representation. Actually, as of today rust-open-metrics-client does not support protobuf at all, but likely will support the Open Metrics Protobuf format in the future.

[Yisaer]

Hi,@mxinden .I wonder whether you are working on this proposal now, if not, I'd like to take this issue. Most of the ideas LGTM.

[mxinden]

Hi,@mxinden .I wonder whether you are working on this proposal now, if not, I'd like to take this issue. Most of the ideas LGTM.

@Yisaer I am not working on any of these proposals for rust-prometheus right now, so please feel free to tackle any of them. While a bit cumbersome, I would suggest writing a detailed change proposal before getting started on any of them. That should make sure everyone is on the same page and non of the coding work is in vain.

[Yisaer]

Hi,@mxinden .I wonder whether you are working on this proposal now, if not, I'd like to take this issue. Most of the ideas LGTM.

@Yisaer I am not working on any of these proposals for rust-prometheus right now, so please feel free to tackle any of them. While a bit cumbersome, I would suggest writing a detailed change proposal before getting started on any of them. That should make sure everyone is on the same page and non of the coding work is in vain.

Thanks for your advice. I will list a change proposal before I started to code.

[Yisaer]

/assign

[roidelapluie]

Hello o/

Julien from the Prometheus team here. I'd love to have an officially supported Prometheus rust client. Based on this discussion, we are likely to adopt https://github.com/mxinden/rust-open-metrics-client as an official implementation, under the Prometheus organisation.

it does not mean that rust-prometheus has to go away, some programming languages have different client libraries. However, I think that rather than re-writing this library to address all the issues in OP's comment, we would rather build on the work already done my @mxinden. We would welcome contributions on that library from the community.

It's not an easy choice to chose something different from the most used library, but thinking long term, I think if we can rally the Rust community around Max's project, we will win over the long term. Rust is here to stay.

I want to finish this message by a really big thank you for everyone caring about rust-prometheus and bringing it to the point it is now. Let's build the future together.

[lucab]

For reference, we are already starting to see users stumbling on old-Prometheus vs OpenMetrics incompatibilities (eg. https://github.com/tikv/rust-prometheus/issues/425 on timestamps format). Making a clearer split between old-Prometheus and new-OpenMetrics may possibly help in that regard.

This library is still deeply rooted into Prometheus v1 world (with v1 protobuf et al.), so I kinda agree that it's easier to move into the new world with a clean table.

Migration will be long and painful though, I fear.

If the larger community is looking for pushing forward an official client library, it could be a good time to reach out to the owner of https://crates.io/crates/openmetrics and see if a name-switch can be agreed.

[roidelapluie]

Interestingly our client libraries are called prometheus, not openmetrics.

In Python, there is "prometheus" which is community maintained and "prometheus-client" which is official. We could have the same pattern.