Skip to content

Conversation

alexander-akhmetov
Copy link
Contributor

@alexander-akhmetov alexander-akhmetov commented Apr 8, 2025

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 :)

@alexander-akhmetov alexander-akhmetov force-pushed the alexander-akhmetov/firing-alerts branch 8 times, most recently from 121d7b2 to cf79fec Compare April 12, 2025 22:11
- [ ] Create and change alert rules
- [ ] List contact points
- [x] List contact points
Copy link
Contributor Author

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

@alexander-akhmetov alexander-akhmetov force-pushed the alexander-akhmetov/firing-alerts branch 2 times, most recently from cd5bf84 to 53871c8 Compare April 14, 2025 21:09
@alexander-akhmetov alexander-akhmetov force-pushed the alexander-akhmetov/firing-alerts branch from 53871c8 to e1b4eb9 Compare April 14, 2025 21:10
@alexander-akhmetov alexander-akhmetov marked this pull request as ready for review April 15, 2025 06:56
@alexander-akhmetov alexander-akhmetov requested a review from a team as a code owner April 15, 2025 06:56
Copy link
Collaborator

@sd2k sd2k left a 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!

rulesEndpointPath = "/api/prometheus/grafana/api/v1/rules"
)

type AlertingClient struct {
Copy link
Collaborator

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!)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense! Un-exported :)

@alexander-akhmetov
Copy link
Contributor Author

I am curious if any of the changes in #95 have affected the use of base URL here - sorry if so!

Nope, all good!

@alexander-akhmetov
Copy link
Contributor Author

alexander-akhmetov commented Apr 15, 2025

Does it make sense to have something similar to IncidentClient in mcpgrafana.go, AlertingClientFromContext? Or is it fine to initialize the client as it is now since it's the same as Grafana client basically?

@sd2k
Copy link
Collaborator

sd2k commented Apr 15, 2025

Hm I think we have that separate IncidentClient in the context to enable connection reuse in the stdio case, but it doesn't feel like a big enough deal really (and annoyingly we need to remember to add the client to the ContextFunc for every transport, which is easy to forget). So I'm happy to keep it where it is (I may even stop storing the IncidentClient in context one day).

Copy link
Collaborator

@sd2k sd2k left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks again! 🥳

@sd2k sd2k merged commit 45425a1 into main Apr 15, 2025
5 checks passed
@sd2k sd2k deleted the alexander-akhmetov/firing-alerts branch April 15, 2025 17:54
ioanarm pushed a commit that referenced this pull request Apr 22, 2025
* Add state retrieval for alert rules

* Make alerting client and its structs private
@sd2k sd2k mentioned this pull request May 9, 2025
2 tasks
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

Successfully merging this pull request may close these issues.

2 participants