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

Alertmanager: Support uploading Grafana Alertmanager Configuration an… #6682

Merged
merged 19 commits into from
Jan 19, 2024

Conversation

gotjosh
Copy link
Contributor

@gotjosh gotjosh commented Nov 17, 2023

…d State

Grafana's Alertmanager and Mimir Alertmanager are configured differently but have the same set of upstream components running except receivers.

We'd like to enable Grafana to use the Mimir Alertmanager as a backend when Grafana is run with certain configuration so that Grafana can stop leveraging its internal Alertmanager.

One of the first steps in this direction is to allow the Mimir Alertmanager to store two things:

  1. Grafana's Alertmanager Configuration
  2. Grafana's Alertmanager State (Notification log and Silences)

This PR, setups up two sets of APIs to allow the Mimir Alertmanager to support Grafana in this migration path. One for CRUDing the configuration and one for CRUDing the set of States.

Although these APIs are per tenant, they are not meant to be called by tenants and are experimental.

What this PR does

Which issue(s) this PR fixes or relates to

All the tasks are documented in https://github.com/grafana/alerting-squad/issues/566

Checklist

  • Tests updated.
  • Documentation added.
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX].
  • about-versioning.md updated with experimental features.

…d State

Grafana's Alertmanager and Mimir Alertmanager are configured differently but have the same set of upstream components running except receivers.

We'd like to enable Grafana to use the Mimir Alertmanager as a backend when Grafana is run with certain configuration so that Grafana can stop leveraging its internal Alertmanager.

One of the first steps in this direction is to allow the Mimir Alertmanager to store two things:

1. Grafana's Alertmanager Configuration
2. Grafana's Alertmanager State (Notification log and Silences)

This PR, setups up two sets of APIs to allow the Mimir the Mimir Alertmanager to support Grafana in this migration path. One for CRUDing the configuration and one for CRUDing the set of States.

Although these APIs are per tenant, they are not meant to be called by tenants and are experimental.
@gotjosh gotjosh force-pushed the gotjosh/state-config-grafana branch from 663a0ae to 36c3cee Compare January 16, 2024 16:27
@gotjosh gotjosh marked this pull request as ready for review January 17, 2024 18:48
@gotjosh gotjosh requested review from a team as code owners January 17, 2024 18:48
Signed-off-by: gotjosh <[email protected]>
@gotjosh
Copy link
Contributor Author

gotjosh commented Jan 17, 2024

@dimitarvdimitrov, can I get you to quick skim on what I'm doing here to ensure I'm not doing anything stupid (particularly with the feature flag stuff)? We've already tested this end to end in dev and it does what we need it to do (for now, as we'll keep expanding on it) and all the context you need to understand the reasoning is in the description.

@santihernandezc, this is now ready for review.

Copy link
Contributor

@santihernandezc santihernandezc left a comment

Choose a reason for hiding this comment

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

LGTM 👍
Some minor comments, I don't need to review this again.

@dimitarvdimitrov dimitarvdimitrov self-requested a review January 18, 2024 18:15
Copy link
Contributor

@dimitarvdimitrov dimitarvdimitrov left a comment

Choose a reason for hiding this comment

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

LGTM, I left a couple of suggestions & questions. The one for the bucket implementation in bucket_client.go is the major one, the rest are mostly minor.

I suspect that we'd want to update the HTTP API docs at some point too. Perhaps when this is moved out of experimental state?

- Rename created at and default fields to clarity
- Move the feature flag from the API level to the Alertmanager level
- Add missing doc strings for some functions

Signed-off-by: gotjosh <[email protected]>
@gotjosh
Copy link
Contributor Author

gotjosh commented Jan 19, 2024

I suspect that we'd want to update the HTTP API docs at some point too. Perhaps when this is moved out of experimental state?

I have added a task to https://github.com/grafana/alerting-squad/issues/566 to make sure we don't forget.

Signed-off-by: gotjosh <[email protected]>
…eractions and add tests to ensure integrity.

Signed-off-by: gotjosh <[email protected]>
Copy link
Contributor

@dimitarvdimitrov dimitarvdimitrov 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 for addressing my comments!

@gotjosh gotjosh merged commit bc34d2d into main Jan 19, 2024
27 checks passed
@gotjosh gotjosh deleted the gotjosh/state-config-grafana branch January 19, 2024 16:16
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.

3 participants