-
-
Notifications
You must be signed in to change notification settings - Fork 42
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
Add prometheus metric #57
base: main
Are you sure you want to change the base?
Conversation
Thanks! I'll look into it soon! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks again for this PR. Your approach seems to be to add the Prometheus related functionality right into the core library itself. Because Prometheus is only one tool out of many out there, I'd like the idea of separating this functionality out into it's own handler extension. Do you think we could use the ResultWriter interface for this and allow to export Prometheus specific health check status using the WithResultWriter config option instead?
not sure how you imagine this to work. Cause who will register the metrics (gauges/counters) cause that is how prometheus works. if you expect it so send back some details, then it is still up to the user to set the gauges and counters. imho prometheus is one of the most commonly used metrics tools out there. |
Agree, adding a bunch of extra deps to About the metrics format, I don't feel mapping discrete
This way, we can create a fine grained and unambiguous prom query/aggregation. |
a format like that will result in more timeseries overall (3 for each check), also you need to reset all the gauges before you update the correct one, it's a bit more complex to do. i get that you don't want to inject into the lib itself, but i'm not sure how to do this |
Sure, I can provide some more details. What I was referring to was that there is a Lines 49 to 64 in 09f6eab
The same way, we probably could have a The user can then configure the library to use the health/examples/showcase/main.go Lines 105 to 109 in 09f6eab
@TomL-dev What do you think? |
Just taken a look at So I created a lib myself: https://github.com/chickenzord/go-health-prometheus Please let me know what you think, thanks |
Looks nice! As of now it seems to only expose the aggregated/overall system status. I wonder if it would help if |
@alexliesenfeld it's already showing per-component healths... actually I haven't found a way to get the overall status 😅 for the aggregated/overall, instead of only up/down/unknown, I'd love to add one more |
you can solve this by creating prom queries and generate alerts if the result (count or something) drops below a x percentage |
Any updates on this? :D One more thing I think would be useful – keeping track of how long each probe takes to respond |
No description provided.