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

Does this crate implement Prometheus or OpenMetrics? #111

Open
08d2 opened this issue Dec 6, 2022 · 7 comments
Open

Does this crate implement Prometheus or OpenMetrics? #111

08d2 opened this issue Dec 6, 2022 · 7 comments

Comments

@08d2
Copy link

08d2 commented Dec 6, 2022

There is a relationship between OpenMetrics and Prometheus, but as far as I understand, they are not the same thing, and are definitely not fully compatible with each other.

lib.rs currently states that this repo is a "Client library implementation of the Open Metrics specification".

If that's true, then I propose that this repo should not exist underneath the prometheus GitHub organization. If it will remain in the prometheus org, then I propose that it should change its mandate such that Prometheus compatibility is the first-order goal, and OpenMetrics is a secondary and e.g. nice-to-have goal.

@mxinden
Copy link
Member

mxinden commented Dec 6, 2022

and are definitely not fully compatible with each other.

Can you expand in which way recent Prometheus versions are not compatible with the OpenMetrics specification?

Note that this has also been discussed in #66 (comment) and in #51. I don't understand why this requires a new GitHub issue.

@08d2
Copy link
Author

08d2 commented Dec 11, 2022

Can you expand in which way recent Prometheus versions are not compatible with the OpenMetrics specification?

Consider a counter metric exported via curl <hostport>/metrics, and specifically maybe e.g. the mxinden/kademlia-exporter metric kademlia_exporter_meta_random_node_lookup_triggered. Running that program and then fetching the /metrics endpoint returns

# HELP kademlia_exporter_meta_random_node_lookup_triggered Number of times a random Kademlia node lookup was triggered.
# TYPE kademlia_exporter_meta_random_node_lookup_triggered counter
kademlia_exporter_meta_random_node_lookup_triggered_total 1

Note that the HELP and TYPE lines have metric name "kademlia_exporter_xxx" and that the actual metric data line has a metric name "kadmelia_exporter_xxx_total". This may be valid for OpenMetrics, but it is not valid for Prometheus. Prometheus requires consistent metric names, e.g.

# HELP kademlia_exporter_meta_random_node_lookup_triggered_total Number of times a random Kademlia node lookup was triggered.
# TYPE kademlia_exporter_meta_random_node_lookup_triggered_total counter
kademlia_exporter_meta_random_node_lookup_triggered_total 1

@08d2
Copy link
Author

08d2 commented Dec 11, 2022

I don't understand why this requires a new GitHub issue.

Consider this issue a superset of the other issues. OpenMetrics and Prometheus define specs which are not equivalent. If this repo lives in the Prometheus org, it should implement the Prometheus spec. If it doesn't implement the Prometheus spec, then it shouldn't live in the Prometheus org. That's my only point.

@kalabukdima
Copy link

kalabukdima commented Apr 25, 2024

I was also trying to understand why the metrics exported by this crate can't be scraped by Prometheus.

In particular, the issue is with the Info metric that is not part of the default set and leads to the error invalid metric type "info" preventing all other metrics to be scraped too (tested with prometheus 2.51.2 and some other verions — none of them could deal with this format).

So I think it is confusing indeed that the official Prometheus client doesn't work with Prometheus. I can see the following possible solutions:

  • rename this crate to something like open-metrics-client or
  • add a possibility to export metrics in Prometheus format as done in Python client

Upd: after setting the correct header with the response, Prometheus could parse the output, but it's still confusing that those formats are not compatible and the naming doesn't suggest which one is used.

@mxinden
Copy link
Member

mxinden commented Apr 29, 2024

@08d2 and @kalabukdima I understand your concerns.

@RichiH do you have additional input here as one of the authors of the open metrics specification? How should the Rust client proceed to be consistent within the Prometheus ecosystem.

In the meantime, @08d2 and @kalabukdima, how about very prominently calling out on the main docs.rs page (i.e. in src/lib.rs) that application/openmetrics-text needs to be set as a content type? Does either of you want to create a pull request?

@RichiH
Copy link
Member

RichiH commented Apr 29, 2024

FYI, OpenMetrics is now part of Prometheus officially

I am on the road right now, but this feels like a bug in Prometheus. Potentially dev summit material to discuss?

@mxinden
Copy link
Member

mxinden commented Apr 29, 2024

FYI, OpenMetrics is now part of Prometheus officially

Can you point us to some docs @RichiH? Is OpenMetrics the default? Is setting the HTTP content type still needed?

Related: #194 and prometheus/prometheus#13944

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

4 participants