-
Notifications
You must be signed in to change notification settings - Fork 137
Add state retrieval for alert rules #87
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
Conversation
121d7b2
to
cf79fec
Compare
- [ ] Create and change alert rules | ||
- [ ] List contact points | ||
- [x] List contact points |
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.
Noticed this was done in another PR: #81
cd5bf84
to
53871c8
Compare
53871c8
to
e1b4eb9
Compare
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.
This LGTM! I am curious if any of the changes in #95 have affected the use of base URL here - sorry if so!
tools/alerting_client.go
Outdated
rulesEndpointPath = "/api/prometheus/grafana/api/v1/rules" | ||
) | ||
|
||
type AlertingClient struct { |
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.
This probably doesn't need exporting - ideally we'd just have the actual tools exported from this tools
package. (we may have missed some already but I'll try and fix that soon 😅)
(same goes for the various structs below!)
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.
Makes sense! Un-exported :)
Nope, all good! |
Does it make sense to have something similar to IncidentClient in mcpgrafana.go, |
Hm I think we have that separate |
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.
LGTM, thanks again! 🥳
* Add state retrieval for alert rules * Make alerting client and its structs private
Updates the existing alert tools to retrieve state information (firing, normal, pending, etc.) for alerts.
Since the Alerting provisioning API does not expose this information, I had to add a custom alerting client that uses the same API used by the UI. This API is not part of the officially stable public APIs, but the client makes it possible to add other endpoints, for example alert state history.
This API supports pagination of the alert rule groups, but I haven't added anything for it yet. Can do a follow-up :)