Support multiple alerts in Graph and TimeSeries panels #260
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
The previous implementation only supported a single alert per Graph and TimeSeries. The current changes introduce the ability to configure multiple alerts per panel. Also, changes have been propagated to the corresponding tests in graph_test.go and timeseries_test.go to verify the new feature handling multiple alerts.
A new alert for "No heap allocations" has been incorporated into the row/row_test.go file. This alert uses a Prometheus query and triggers if the average is below 0.01. The test assertions are updated accordingly to check for two alerts rather than one.
Added PanelName as a field to the Alert structure. Updated related code to make use of this new field in alert creation and handling. The new field provides more specific information about the panel associated with the alert.
The alert constructor was changed to include 'panelTitle' as a formal argument. Consequently, all the places where the alert constructor, New, was used have been updated with this new argument. Changes were also made to the 'PanelName' property extraction in the 'dashboards.go' file. Furthermore, the alert test was adjusted to reflect changes in the alert constructor.
Removed the redundant line where alert name was being assigned with the Builder title in both graph.go and timeseries.go. Also updated the alert creation test in alert_test.go to reflect this change.
Removed the redundant line where alert name was being assigned with the Builder title in both graph.go and timeseries.go. Also updated the alert creation test in alert_test.go to reflect this change.
malekvictor
reviewed
May 1, 2024
| } | ||
|
|
||
| // New creates a new alert. | ||
| func New(name string, options ...Option) *Alert { | ||
| func New(name, panelName string, options ...Option) *Alert { |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
It's not optional parameter, without panel name we can't match alert with panel
There was a problem hiding this comment.
Ok, I leave it to the discretion of the owners
dashboards.go
Outdated
| @@ -125,11 +125,24 @@ func (client *Client) UpsertDashboard(ctx context.Context, folder *Folder, build | |||
| return nil, err | |||
| } | |||
|
|
|||
| alertPanelNames := make(map[string]struct{}) | |||
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
row/row.go
Outdated
| @@ -58,15 +58,17 @@ func WithGraph(title string, options ...graph.Option) Option { | |||
|
|
|||
| row.builder.Add(panel.Builder) | |||
|
|
|||
| if panel.Alert == nil { | |||
| if panel.Alerts == nil || len(panel.Alerts) == 0 { | |||
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This commit refactors how alert panel names and panel alerts are handled. For the alert panel names, allocation has been optimized by initializing the map with the size of the alerts array. Checking for panels with alerts has also been simplified to just check the length of alerts rather than checking for null and length simultaneously.
malekvictor
approved these changes
May 6, 2024
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.
The previous implementation only supported a single alert per Graph and TimeSeries. The current changes introduce the ability to configure multiple alerts per panel.