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

Support a prometheus / openmetrics endpoint #285

Open
jespersoderlund opened this issue Aug 24, 2021 · 11 comments
Open

Support a prometheus / openmetrics endpoint #285

jespersoderlund opened this issue Aug 24, 2021 · 11 comments

Comments

@jespersoderlund
Copy link
Contributor

To be a part of an openmetrics centric metrics infrastructure it would be useful if the ratelimit service natively supported a prometheus metrics endpoint without having to go past a statsd bridge

@ysawa0
Copy link
Member

ysawa0 commented Aug 25, 2021

Great idea, that makes sense to me.

@irl-segfault
Copy link

+1, would be super useful! Is there any kind of shim or adapter people can use in the meantime?

@irl-segfault
Copy link

^ answer: use statsd-prom exporter

@ethernoy
Copy link
Contributor

I think I can work on adding Prometheus instrumentation in ratelimit, but I believe we should work out a list of possible metric to be implemented first.

@zirain
Copy link
Contributor

zirain commented Jul 8, 2022

this PR tried to fix this by adding prometheus_sink, but it won't.
a valid metric in stats may be invalid in prometheus, e. g. ratelimit.service.rate_limit.httpbin.header_match_\get.near_limit

@ramaraochavali
Copy link

@zirain Do you know what needs to be done to fix ^^?

@upgle
Copy link
Contributor

upgle commented Apr 13, 2023

Hi, I've abstracted an existing structure that was fully coupled to gostats and applied Prometheus metrics to it.
I would like to contribute this code. (not using gostats prometheus_sink)

I tried to contribute a small change to this project last time, but no one was interested and it closed automatically. I was wondering if there are other procedures like mailing.

Thank you

image

Label based metric for prometheus

image

@jespersoderlund
Copy link
Contributor Author

I've contributed a few things over the last couple of years and the maintainers have picked-it up in reasonable time.

Sorry, to hear that your previous PR was closed, hope that your contributions can get reviewed this time.

Not a maintainer myself, so I can't directly help-out.

I'm keen to see this feature as you probably can guess since I created the issue :)

@jespersoderlund
Copy link
Contributor Author

@upgle
One thing that the current setup going through statsd => prometheus allows us to do is to not publish all metrics since some it can be quite high cardinality.

So we only publish the "near_limit" and "over_limit" metrics.

Will your implementation allow something similar, like having an allow-list of metrics to publish?

@upgle
Copy link
Contributor

upgle commented Apr 14, 2023

Thank you for your kind reply.
I understand your concerns about cardinality.

I checked this out, other metrics other than "near_limit" and "over_limit" are all useful metrics (e.g. redis, memcached, localcache...), and these metrics do not have high cardinality.

Rather, a metric such as near_limit has a high cardinality because it has a descriptor key as a label for prometheus.

And there is currently no implementation such as allow-list, but I think it is a point that can be improved.

I'll be contributing code soon and listening to feedback on slack, github, and more.

@birdayz
Copy link
Contributor

birdayz commented Feb 29, 2024

We could either make each metric configurable (on/off). In addition, anyone can use prometheus labeldrop to reduce cardinality. Do we need anything beyond that?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants