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

Prometheus text exposition format support #80

Open
byxorna opened this issue Jul 10, 2019 · 6 comments
Open

Prometheus text exposition format support #80

byxorna opened this issue Jul 10, 2019 · 6 comments

Comments

@byxorna
Copy link

byxorna commented Jul 10, 2019

It would be great if gostats supported the prometheus text exposition format on a new HTTP route /metrics. This would enable services like https://github.com/lyft/ratelimit to be more easily deployed in environments that are adopting the CNCF best practices for how to monitor containerized services via Prometheus.

Ive taken a cursory look at the stats_handler.go but am having a hard time making heads or tails of how to shim in a small reformatting of the current stats into a format that aligns with the prom exposition format.

@charlievieth
Copy link
Contributor

@byxorna I like the idea of this. I'm swamped at the moment, but will give it some thought and get back to you. It looks like the biggest change is that tags which we currently store as .__KEY=VALUE need to be {KEY="VALUE"}? That could be parsed out by something like a prometheus Sink, but thats kinda ugly - maybe there is a better way.

Feel free to ping me if you don't hear back.

@byxorna
Copy link
Author

byxorna commented Jul 12, 2019

@charlievieth great, thanks! I think that not all of the metrics will map over perfectly, but the core counters and gauges are the most important imo. Ill remind myself to check in in a week or so :)

I am happy to hack on this as well, but would love guidance on how you would like to see this implemented

@byxorna
Copy link
Author

byxorna commented Jul 19, 2019

@charlievieth hey, just checking in. Any thoughts on how/when to best approach this?

@charlievieth
Copy link
Contributor

@byxorna I think one of the challenges here is that if we want to add support for the prometheus exposition format we'll probably want to store a stats tags, instead of just the formatted statsd metric name (this is if we don't want to parse out the tags on each request to the prometheus /metrics endpoint). This will lead to a non-trivial increase in memory usage. That said, this might not matter - unless the cardinality of your apps metrics is very high.

Basically, changing the current stat types to store their tags would be the easiest way to achieve this. And I would provide an HTTP handler the consumers of this lib can register to the route of their choice.

@byxorna
Copy link
Author

byxorna commented Aug 11, 2019

@charlievieth that sounds great. My usecase is specifically for the downstream lyft/ratelimit project - they do not expose a ton of whitebox metrics, so I am certainly not worried about memory consumption. FWIW, i believe the https://github.com/prometheus/client_golang does the same thing you describe above :)

It sounds like a great idea to give clients of this library the choice to register the prom-compatible http handler whereever they would like - that seems very flexible.

@charlievieth
Copy link
Contributor

charlievieth commented Aug 12, 2019

Cool, feel free to ping me with any PRs. I'm fairly hesitant to introduce anything that will have an adverse impact on the performance of statsd metrics, but I think this can be done fairly efficiently using slices and the tagPair struct.

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

2 participants