Refactor GrafanaAlertmanager to not depend on Grafana #19
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
gotjosh
commented
Nov 2, 2022
| type Notifier = notify.Notifier | ||
|
|
||
| // Configuration is an interface for accessing Alertmanager configuration. | ||
| type Configuration interface { |
There was a problem hiding this comment.
This is the biggest piece of this work - we've abstracted everything that could be Grafana-specific in an interface, there's probably more than will either go to Grafana or come back here but I've tried to do the minimum for now to get something done.
gotjosh
commented
Nov 2, 2022
| @@ -135,7 +112,32 @@ type MaintenanceOptions interface { | |||
| MaintenanceFunc() (int64, error) | |||
| } | |||
|
|
|||
| type Template = template.Template | |||
There was a problem hiding this comment.
gotjosh
commented
Nov 2, 2022
| @@ -21,11 +22,17 @@ var ( | |||
| ErrGetAlertGroupsBadPayload = fmt.Errorf("unable to retrieve alerts groups") | |||
| ) | |||
|
|
|||
| func (am *GrafanaAlertmanager) GetAlerts(active, silenced, inhibited bool, filter []string, receivers string) (apimodels.GettableAlerts, error) { | |||
| type GettableAlerts = amv2.GettableAlerts | |||
There was a problem hiding this comment.
There will be a better place to organise this, but for now, I want to make the diffs minimal until we know for sure that this works for Grafana. Then we can refactor away at our liking.
JacobsonMT
approved these changes
Nov 23, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is the first substantial PR at decoupling the Alertmanager logic from the Grafana-specific logic - you can reference that by the removal of the Grafana-based imports from the code and the scope of an interface for that specific logic.
There's a LOT more work TODO (hence the number of comments in the code), but these should be followed up immediately and are very obvious that things don't work. For the ones that we think would be more long term and I'm happy to create an issue to track them.