From 5a1d1be84554c65871ee738ad98597355793ba86 Mon Sep 17 00:00:00 2001 From: lueurxax Date: Tue, 30 Apr 2024 11:10:20 +0200 Subject: [PATCH 1/7] Support multiple alerts in Graph and TimeSeries panels 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. --- graph/graph.go | 9 +++++---- graph/graph_test.go | 17 +++++++++++++++-- row/row.go | 24 ++++++++++++++---------- timeseries/timeseries.go | 9 +++++---- timeseries/timeseries_test.go | 17 +++++++++++++++-- 5 files changed, 54 insertions(+), 22 deletions(-) diff --git a/graph/graph.go b/graph/graph.go index 8bc3f1f4..7f0b80f7 100644 --- a/graph/graph.go +++ b/graph/graph.go @@ -74,12 +74,12 @@ const ( // Graph represents a graph panel. type Graph struct { Builder *sdk.Panel - Alert *alert.Alert + Alerts []*alert.Alert } // New creates a new graph panel. func New(title string, options ...Option) (*Graph, error) { - panel := &Graph{Builder: sdk.NewGraph(title)} + panel := &Graph{Builder: sdk.NewGraph(title), Alerts: make([]*alert.Alert, 0)} panel.Builder.AliasColors = make(map[string]interface{}) panel.Builder.IsNew = false @@ -266,8 +266,9 @@ func XAxis(opts ...axis.Option) Option { // Alert creates an alert for this graph. func Alert(name string, opts ...alert.Option) Option { return func(graph *Graph) error { - graph.Alert = alert.New(graph.Builder.Title, append(opts, alert.Summary(name))...) - graph.Alert.Builder.Name = graph.Builder.Title + al := alert.New(graph.Builder.Title, append(opts, alert.Summary(name))...) + al.Builder.Name = graph.Builder.Title + graph.Alerts = append(graph.Alerts, al) return nil } diff --git a/graph/graph_test.go b/graph/graph_test.go index 961c9ec2..097dc7f4 100644 --- a/graph/graph_test.go +++ b/graph/graph_test.go @@ -158,8 +158,21 @@ func TestAlertsCanBeConfigured(t *testing.T) { panel, err := New("panel title", Alert("some alert")) req.NoError(err) - req.NotNil(panel.Alert) - req.Equal("panel title", panel.Alert.Builder.Name) + req.NotNil(panel.Alerts) + req.Len(panel.Alerts, 1) + req.Equal("panel title", panel.Alerts[0].Builder.Name) +} + +func TestTwoAlertsCanBeConfigured(t *testing.T) { + req := require.New(t) + + panel, err := New("panel title", Alert("some alert"), Alert("other alert")) + + req.NoError(err) + req.NotNil(panel.Alerts) + req.Len(panel.Alerts, 2) + req.Equal("panel title", panel.Alerts[0].Builder.Name) + req.Equal("panel title", panel.Alerts[1].Builder.Name) } func TestDrawModeCanBeConfigured(t *testing.T) { diff --git a/row/row.go b/row/row.go index d65105aa..8df34c0a 100644 --- a/row/row.go +++ b/row/row.go @@ -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 { return nil } - if panel.Builder.Datasource != nil { - panel.Alert.Datasource = panel.Builder.Datasource.LegacyName - } + for _, al := range panel.Alerts { + if panel.Builder.Datasource != nil { + al.Datasource = panel.Builder.Datasource.LegacyName + } - row.alerts = append(row.alerts, panel.Alert) + row.alerts = append(row.alerts, al) + } return nil } @@ -82,15 +84,17 @@ func WithTimeSeries(title string, options ...timeseries.Option) Option { row.builder.Add(panel.Builder) - if panel.Alert == nil { + if panel.Alerts == nil || len(panel.Alerts) == 0 { return nil } - if panel.Builder.Datasource != nil { - panel.Alert.Datasource = panel.Builder.Datasource.LegacyName - } + for _, al := range panel.Alerts { + if panel.Builder.Datasource != nil { + al.Datasource = panel.Builder.Datasource.LegacyName + } - row.alerts = append(row.alerts, panel.Alert) + row.alerts = append(row.alerts, al) + } return nil } diff --git a/timeseries/timeseries.go b/timeseries/timeseries.go index 1fc2a236..b6d9a9e1 100644 --- a/timeseries/timeseries.go +++ b/timeseries/timeseries.go @@ -123,12 +123,12 @@ const ( // TimeSeries represents a time series panel. type TimeSeries struct { Builder *sdk.Panel - Alert *alert.Alert + Alerts []*alert.Alert } // New creates a new time series panel. func New(title string, options ...Option) (*TimeSeries, error) { - panel := &TimeSeries{Builder: sdk.NewTimeseries(title)} + panel := &TimeSeries{Builder: sdk.NewTimeseries(title), Alerts: make([]*alert.Alert, 0)} panel.Builder.IsNew = false for _, opt := range append(defaults(), options...) { @@ -409,8 +409,9 @@ func Transparent() Option { // Alert creates an alert for this graph. func Alert(name string, opts ...alert.Option) Option { return func(timeseries *TimeSeries) error { - timeseries.Alert = alert.New(timeseries.Builder.Title, append(opts, alert.Summary(name))...) - timeseries.Alert.Builder.Name = timeseries.Builder.Title + al := alert.New(timeseries.Builder.Title, append(opts, alert.Summary(name))...) + al.Builder.Name = timeseries.Builder.Title + timeseries.Alerts = append(timeseries.Alerts, al) return nil } diff --git a/timeseries/timeseries_test.go b/timeseries/timeseries_test.go index a26974af..419d7bdc 100644 --- a/timeseries/timeseries_test.go +++ b/timeseries/timeseries_test.go @@ -144,8 +144,21 @@ func TestAlertsCanBeConfigured(t *testing.T) { panel, err := New("panel title", Alert("some alert")) req.NoError(err) - req.NotNil(panel.Alert) - req.Equal("panel title", panel.Alert.Builder.Name) + req.NotNil(panel.Alerts) + req.Len(panel.Alerts, 1) + req.Equal("panel title", panel.Alerts[0].Builder.Name) +} + +func TestTwoAlertsCanBeConfigured(t *testing.T) { + req := require.New(t) + + panel, err := New("panel title", Alert("some alert"), Alert("other alert")) + + req.NoError(err) + req.NotNil(panel.Alerts) + req.Len(panel.Alerts, 2) + req.Equal("panel title", panel.Alerts[0].Builder.Name) + req.Equal("panel title", panel.Alerts[1].Builder.Name) } func TestLineWidthCanBeConfigured(t *testing.T) { From 44c4aad58fd10986c5054708351ca47a9f0edb26 Mon Sep 17 00:00:00 2001 From: lueurxax Date: Tue, 30 Apr 2024 11:14:17 +0200 Subject: [PATCH 2/7] Add new alert for no heap allocations 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. --- row/row_test.go | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/row/row_test.go b/row/row_test.go index 2bad20a7..65a52a71 100644 --- a/row/row_test.go +++ b/row/row_test.go @@ -69,12 +69,20 @@ func TestRowsCanHaveGraphsAndAlert(t *testing.T) { ), alert.If(alert.Avg, "A", alert.IsAbove(3)), ), + graph.Alert( + "No heap allocations", + alert.WithPrometheusQuery( + "A", + "sum(go_memstats_heap_alloc_bytes{app!=\"\"}) by (app)", + ), + alert.If(alert.Avg, "A", alert.IsBelow(0.01)), + ), ), ) req.NoError(err) req.Len(panel.builder.Panels, 1) - req.Len(panel.Alerts(), 1) + req.Len(panel.Alerts(), 2) req.Equal("Prometheus", panel.Alerts()[0].Datasource) } @@ -107,12 +115,20 @@ func TestRowsCanHaveTimeSeriesAndAlert(t *testing.T) { ), alert.If(alert.Avg, "A", alert.IsAbove(3)), ), + timeseries.Alert( + "No heap allocations", + alert.WithPrometheusQuery( + "A", + "sum(go_memstats_heap_alloc_bytes{app!=\"\"}) by (app)", + ), + alert.If(alert.Avg, "A", alert.IsBelow(0.01)), + ), ), ) req.NoError(err) req.Len(panel.builder.Panels, 1) - req.Len(panel.Alerts(), 1) + req.Len(panel.Alerts(), 2) req.Equal("Prometheus", panel.Alerts()[0].Datasource) } From 966d976e7ce5744d5869c1e4429a4cdcaae956e7 Mon Sep 17 00:00:00 2001 From: lueurxax Date: Tue, 30 Apr 2024 17:32:40 +0200 Subject: [PATCH 3/7] Add PanelName to Alert structure 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. --- alert/alert.go | 4 +++- cmd/builder-example/main.go | 1 + dashboards.go | 2 +- decoder/graph.go | 6 +++++- decoder/timeseries.go | 6 +++++- graph/graph.go | 2 +- timeseries/timeseries.go | 2 +- 7 files changed, 17 insertions(+), 6 deletions(-) diff --git a/alert/alert.go b/alert/alert.go index 4f6e09f6..ef2d090d 100644 --- a/alert/alert.go +++ b/alert/alert.go @@ -51,10 +51,11 @@ type Alert struct { Datasource string DashboardUID string PanelID string + PanelName string } // New creates a new alert. -func New(name string, options ...Option) *Alert { +func New(name, panelName string, options ...Option) *Alert { nope := false alert := &Alert{ @@ -87,6 +88,7 @@ func New(name string, options ...Option) *Alert { Labels: map[string]string{}, }, }, + PanelName: panelName, }, } diff --git a/cmd/builder-example/main.go b/cmd/builder-example/main.go index 1bf6c02b..94d5196b 100644 --- a/cmd/builder-example/main.go +++ b/cmd/builder-example/main.go @@ -113,6 +113,7 @@ func main() { timeseries.Legend(timeseries.Last, timeseries.AsTable), timeseries.Alert( "Too many heap allocations", + alert.Summary("Too many heap allocations"), alert.Description("Yup, too much of {{ app }}"), alert.Runbook("https://google.com"), alert.Tags(map[string]string{ diff --git a/dashboards.go b/dashboards.go index 3af5cb19..78659373 100644 --- a/dashboards.go +++ b/dashboards.go @@ -129,7 +129,7 @@ func (client *Client) UpsertDashboard(ctx context.Context, folder *Folder, build alert := *alerts[i] alert.HookDashboardUID(dashboardFromGrafana.UID) - alert.HookPanelID(panelIDByTitle(dashboardFromGrafana, alert.Builder.Name)) + alert.HookPanelID(panelIDByTitle(dashboardFromGrafana, alert.Builder.PanelName)) if err := client.AddAlert(ctx, folder.Title, alert, datasourcesMap); err != nil { return nil, fmt.Errorf("could not add new alerts for dashboard: %w", err) diff --git a/decoder/graph.go b/decoder/graph.go index 4edc1d0b..a5d2b0e4 100644 --- a/decoder/graph.go +++ b/decoder/graph.go @@ -3,6 +3,7 @@ package decoder import ( "fmt" + "github.com/K-Phoen/grabana/alert" "github.com/K-Phoen/grabana/axis" "github.com/K-Phoen/grabana/graph" "github.com/K-Phoen/grabana/graph/series" @@ -82,7 +83,10 @@ func (graphPanel DashboardGraph) toOption() (row.Option, error) { return nil, err } - opts = append(opts, graph.Alert(graphPanel.Alert.Summary, alertOpts...)) + opts = append(opts, graph.Alert( + graphPanel.Alert.Summary, + append(alertOpts, alert.Summary(graphPanel.Alert.Summary))...), + ) } if graphPanel.Visualization != nil { opts = append(opts, graphPanel.Visualization.toOptions()...) diff --git a/decoder/timeseries.go b/decoder/timeseries.go index 29615d16..6dd94cb0 100644 --- a/decoder/timeseries.go +++ b/decoder/timeseries.go @@ -3,6 +3,7 @@ package decoder import ( "fmt" + "github.com/K-Phoen/grabana/alert" "github.com/K-Phoen/grabana/row" "github.com/K-Phoen/grabana/timeseries" "github.com/K-Phoen/grabana/timeseries/axis" @@ -80,7 +81,10 @@ func (timeseriesPanel DashboardTimeSeries) toOption() (row.Option, error) { return nil, err } - opts = append(opts, timeseries.Alert(timeseriesPanel.Alert.Summary, alertOpts...)) + opts = append(opts, timeseries.Alert( + timeseriesPanel.Alert.Summary, + append(alertOpts, alert.Summary(timeseriesPanel.Alert.Summary))..., + )) } if timeseriesPanel.Visualization != nil { vizOpts, err := timeseriesPanel.Visualization.toOptions() diff --git a/graph/graph.go b/graph/graph.go index 7f0b80f7..eafc0468 100644 --- a/graph/graph.go +++ b/graph/graph.go @@ -266,7 +266,7 @@ func XAxis(opts ...axis.Option) Option { // Alert creates an alert for this graph. func Alert(name string, opts ...alert.Option) Option { return func(graph *Graph) error { - al := alert.New(graph.Builder.Title, append(opts, alert.Summary(name))...) + al := alert.New(name, graph.Builder.Title, opts...) al.Builder.Name = graph.Builder.Title graph.Alerts = append(graph.Alerts, al) diff --git a/timeseries/timeseries.go b/timeseries/timeseries.go index b6d9a9e1..150b6b1c 100644 --- a/timeseries/timeseries.go +++ b/timeseries/timeseries.go @@ -409,7 +409,7 @@ func Transparent() Option { // Alert creates an alert for this graph. func Alert(name string, opts ...alert.Option) Option { return func(timeseries *TimeSeries) error { - al := alert.New(timeseries.Builder.Title, append(opts, alert.Summary(name))...) + al := alert.New(name, timeseries.Builder.Title, opts...) al.Builder.Name = timeseries.Builder.Title timeseries.Alerts = append(timeseries.Alerts, al) From eda0f96f9506e68d87abc1af982c1971ff75c894 Mon Sep 17 00:00:00 2001 From: lueurxax Date: Tue, 30 Apr 2024 18:13:55 +0200 Subject: [PATCH 4/7] Refactor alert constructor to include panelTitle argument 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. --- alert/alert.go | 2 +- alert/alert_test.go | 30 ++++++++++++++++-------------- alert/queries_test.go | 10 +++++----- dashboards.go | 2 +- dashboards_test.go | 3 ++- decoder/alert_targets_test.go | 14 +++++++------- 6 files changed, 32 insertions(+), 29 deletions(-) diff --git a/alert/alert.go b/alert/alert.go index ef2d090d..93797088 100644 --- a/alert/alert.go +++ b/alert/alert.go @@ -88,8 +88,8 @@ func New(name, panelName string, options ...Option) *Alert { Labels: map[string]string{}, }, }, - PanelName: panelName, }, + PanelName: panelName, } for _, opt := range append(defaults(), options...) { diff --git a/alert/alert_test.go b/alert/alert_test.go index fd40b715..343f0ee2 100644 --- a/alert/alert_test.go +++ b/alert/alert_test.go @@ -9,8 +9,9 @@ import ( func TestNewAlertCanBeCreated(t *testing.T) { req := require.New(t) alertTitle := "some alert" + panelTitle := "some panel" - a := New(alertTitle) + a := New(alertTitle, panelTitle) req.Len(a.Builder.Rules, 1) @@ -25,6 +26,7 @@ func TestConditionsCanBeCombined(t *testing.T) { req := require.New(t) a := New( + "", "", IfOr(Avg, "A", IsBelow(10)), IfOr(Avg, "B", IsBelow(8)), @@ -36,7 +38,7 @@ func TestConditionsCanBeCombined(t *testing.T) { func TestPanelIDCanBeHooked(t *testing.T) { req := require.New(t) - a := New("") + a := New("", "") a.HookPanelID("id") @@ -46,7 +48,7 @@ func TestPanelIDCanBeHooked(t *testing.T) { func TestDashboardUIDCanBeHooked(t *testing.T) { req := require.New(t) - a := New("") + a := New("", "") a.HookDashboardUID("uid") @@ -57,7 +59,7 @@ func TestDatasourceUIDCanBeHooked(t *testing.T) { req := require.New(t) a := New( - "", + "", "", WithPrometheusQuery("A", "some prometheus query"), IfOr(Avg, "1", IsBelow(10)), ) @@ -87,7 +89,7 @@ func TestDatasourceUIDCanBeHooked(t *testing.T) { func TestSummaryCanBeSet(t *testing.T) { req := require.New(t) - a := New("", Summary("summary content")) + a := New("", "", Summary("summary content")) req.Equal("summary content", a.Builder.Rules[0].Annotations["summary"]) } @@ -95,7 +97,7 @@ func TestSummaryCanBeSet(t *testing.T) { func TestDescriptionCanBeSet(t *testing.T) { req := require.New(t) - a := New("", Description("description content")) + a := New("", "", Description("description content")) req.Equal("description content", a.Builder.Rules[0].Annotations["description"]) } @@ -103,7 +105,7 @@ func TestDescriptionCanBeSet(t *testing.T) { func TestRunbookCanBeSet(t *testing.T) { req := require.New(t) - a := New("", Runbook("runbook url")) + a := New("", "", Runbook("runbook url")) req.Equal("runbook url", a.Builder.Rules[0].Annotations["runbook_url"]) } @@ -111,7 +113,7 @@ func TestRunbookCanBeSet(t *testing.T) { func TestForIntervalCanBeSet(t *testing.T) { req := require.New(t) - a := New("", For("1m")) + a := New("", "", For("1m")) req.Equal("1m", a.Builder.Rules[0].For) } @@ -119,7 +121,7 @@ func TestForIntervalCanBeSet(t *testing.T) { func TestFrequencyCanBeSet(t *testing.T) { req := require.New(t) - a := New("", EvaluateEvery("1m")) + a := New("", "", EvaluateEvery("1m")) req.Equal("1m", a.Builder.Interval) } @@ -127,7 +129,7 @@ func TestFrequencyCanBeSet(t *testing.T) { func TestErrorModeCanBeSet(t *testing.T) { req := require.New(t) - a := New("", OnExecutionError(ErrorKO)) + a := New("", "", OnExecutionError(ErrorKO)) req.Equal(string(ErrorKO), a.Builder.Rules[0].GrafanaAlert.ExecutionErrorState) } @@ -135,7 +137,7 @@ func TestErrorModeCanBeSet(t *testing.T) { func TestNoDataModeCanBeSet(t *testing.T) { req := require.New(t) - a := New("", OnNoData(NoDataAlerting)) + a := New("", "", OnNoData(NoDataAlerting)) req.Equal(string(NoDataAlerting), a.Builder.Rules[0].GrafanaAlert.NoDataState) } @@ -143,7 +145,7 @@ func TestNoDataModeCanBeSet(t *testing.T) { func TestTagsCanBeSet(t *testing.T) { req := require.New(t) - a := New("", Tags(map[string]string{ + a := New("", "", Tags(map[string]string{ "severity": "warning", })) @@ -154,7 +156,7 @@ func TestTagsCanBeSet(t *testing.T) { func TestConditionsCanBeSet(t *testing.T) { req := require.New(t) - a := New("", If(Avg, "1", IsBelow(10))) + a := New("", "", If(Avg, "1", IsBelow(10))) req.Len(a.Builder.Rules[0].GrafanaAlert.Data, 1) } @@ -162,7 +164,7 @@ func TestConditionsCanBeSet(t *testing.T) { func TestOrConditionsCanBeSet(t *testing.T) { req := require.New(t) - a := New("", IfOr(Avg, "1", IsBelow(10))) + a := New("", "", IfOr(Avg, "1", IsBelow(10))) req.Len(a.Builder.Rules[0].GrafanaAlert.Data, 1) } diff --git a/alert/queries_test.go b/alert/queries_test.go index fb64083f..c893c3b1 100644 --- a/alert/queries_test.go +++ b/alert/queries_test.go @@ -10,7 +10,7 @@ import ( func TestPrometheusQueriesCanBeAdded(t *testing.T) { req := require.New(t) - a := New("", WithPrometheusQuery("A", "some prometheus query")) + a := New("", "", WithPrometheusQuery("A", "some prometheus query")) req.Len(a.Builder.Rules[0].GrafanaAlert.Data, 2) } @@ -18,7 +18,7 @@ func TestPrometheusQueriesCanBeAdded(t *testing.T) { func TestGraphiteQueriesCanBeAdded(t *testing.T) { req := require.New(t) - a := New("", WithGraphiteQuery("A", "some graphite query")) + a := New("", "", WithGraphiteQuery("A", "some graphite query")) req.Len(a.Builder.Rules[0].GrafanaAlert.Data, 2) } @@ -26,7 +26,7 @@ func TestGraphiteQueriesCanBeAdded(t *testing.T) { func TestLokiQueriesCanBeAdded(t *testing.T) { req := require.New(t) - a := New("", WithLokiQuery("A", "some loki query")) + a := New("", "", WithLokiQuery("A", "some loki query")) req.Len(a.Builder.Rules[0].GrafanaAlert.Data, 2) } @@ -34,7 +34,7 @@ func TestLokiQueriesCanBeAdded(t *testing.T) { func TestStackdriverQueriesCanBeAdded(t *testing.T) { req := require.New(t) - a := New("", WithStackdriverQuery(stackdriver.Gauge("A", "cloudsql.googleapis.com/database/cpu/utilization"))) + a := New("", "", WithStackdriverQuery(stackdriver.Gauge("A", "cloudsql.googleapis.com/database/cpu/utilization"))) req.Len(a.Builder.Rules[0].GrafanaAlert.Data, 2) } @@ -42,7 +42,7 @@ func TestStackdriverQueriesCanBeAdded(t *testing.T) { func TestInfluxDBQueriesCanBeAdded(t *testing.T) { req := require.New(t) - a := New("", WithInfluxDBQuery("A", "some influxdb query")) + a := New("", "", WithInfluxDBQuery("A", "some influxdb query")) req.Len(a.Builder.Rules[0].GrafanaAlert.Data, 2) } diff --git a/dashboards.go b/dashboards.go index 78659373..5ada40b3 100644 --- a/dashboards.go +++ b/dashboards.go @@ -129,7 +129,7 @@ func (client *Client) UpsertDashboard(ctx context.Context, folder *Folder, build alert := *alerts[i] alert.HookDashboardUID(dashboardFromGrafana.UID) - alert.HookPanelID(panelIDByTitle(dashboardFromGrafana, alert.Builder.PanelName)) + alert.HookPanelID(panelIDByTitle(dashboardFromGrafana, alert.PanelName)) if err := client.AddAlert(ctx, folder.Title, alert, datasourcesMap); err != nil { return nil, fmt.Errorf("could not add new alerts for dashboard: %w", err) diff --git a/dashboards_test.go b/dashboards_test.go index ded065c9..cb10f1ff 100644 --- a/dashboards_test.go +++ b/dashboards_test.go @@ -636,6 +636,7 @@ func TestDashboardsCanBeCreatedWithNewAlertsAndDeletesPreviousAlerts(t *testing. ), timeseries.Alert( "Too many heap allocations", + alert.Summary("Too many heap allocations"), alert.WithPrometheusQuery( "A", "sum(go_memstats_heap_alloc_bytes{app!=\"\"}) by (app)", @@ -849,7 +850,7 @@ func TestDashboardsCanBeCreatedWithNewAlertsAndDeletesPreviousAlerts(t *testing. { "for": "5m", "grafana_alert": { - "title": "Heap allocations", + "title": "Too many heap allocations", "condition": "_alert_condition_", "no_data_state": "NoData", "exec_err_state": "Alerting", diff --git a/decoder/alert_targets_test.go b/decoder/alert_targets_test.go index 0cb86f6b..8bbceb8b 100644 --- a/decoder/alert_targets_test.go +++ b/decoder/alert_targets_test.go @@ -45,7 +45,7 @@ func TestDecodingAPrometheusTarget(t *testing.T) { req.NoError(err) - alert := alertBuilder.New("", opt) + alert := alertBuilder.New("", "", opt) req.Len(alert.Builder.Rules, 1) req.Len(alert.Builder.Rules[0].GrafanaAlert.Data, 2) // the query and the condition @@ -101,7 +101,7 @@ func TestDecodingALokiTarget(t *testing.T) { req.NoError(err) - alert := alertBuilder.New("", opt) + alert := alertBuilder.New("", "", opt) req.Len(alert.Builder.Rules, 1) req.Len(alert.Builder.Rules[0].GrafanaAlert.Data, 2) // the query and the condition @@ -156,7 +156,7 @@ func TestDecodingAGraphiteTarget(t *testing.T) { req.NoError(err) - alert := alertBuilder.New("", opt) + alert := alertBuilder.New("", "", opt) req.Len(alert.Builder.Rules, 1) req.Len(alert.Builder.Rules[0].GrafanaAlert.Data, 2) // the query and the condition @@ -269,7 +269,7 @@ func TestDecodingStackdriverTarget(t *testing.T) { req.NoError(err, ErrInvalidStackdriverType) - alert := alertBuilder.New("", opt) + alert := alertBuilder.New("", "", opt) req.Len(alert.Builder.Rules, 1) req.Len(alert.Builder.Rules[0].GrafanaAlert.Data, 2) // the query and the condition @@ -335,7 +335,7 @@ func TestDecodingStackdriverPreprocessor(t *testing.T) { req.NoError(err) - alert := alertBuilder.New("", opt) + alert := alertBuilder.New("", "", opt) query := alert.Builder.Rules[0].GrafanaAlert.Data[1] req.Equal(tc.expected, query.Model.MetricQuery.Preprocessor) @@ -449,7 +449,7 @@ func TestDecodingStackdriverAggregation(t *testing.T) { req.NoError(err) - alert := alertBuilder.New("", opt) + alert := alertBuilder.New("", "", opt) query := alert.Builder.Rules[0].GrafanaAlert.Data[1] req.Equal(string(tc.expected), query.Model.MetricQuery.CrossSeriesReducer) @@ -591,7 +591,7 @@ func TestDecodingStackdriverAlignment(t *testing.T) { req.NoError(err) - alert := alertBuilder.New("", opt) + alert := alertBuilder.New("", "", opt) query := alert.Builder.Rules[0].GrafanaAlert.Data[1] req.Equal(string(tc.expected), query.Model.MetricQuery.PerSeriesAligner) From 2e021d14477b4a71405adb8ee2151de01c94b226 Mon Sep 17 00:00:00 2001 From: lueurxax Date: Tue, 30 Apr 2024 18:31:38 +0200 Subject: [PATCH 5/7] Remove redundant alert name assignment 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. --- decoder/alert_test.go | 2 +- graph/graph.go | 1 - timeseries/timeseries.go | 1 - 3 files changed, 1 insertion(+), 3 deletions(-) diff --git a/decoder/alert_test.go b/decoder/alert_test.go index 59226657..a993fbc1 100644 --- a/decoder/alert_test.go +++ b/decoder/alert_test.go @@ -32,7 +32,7 @@ func TestDecodingSimpleAlert(t *testing.T) { opts, err := alertDef.toOptions() req.NoError(err) - alertBuilder := alert.New("", opts...) + alertBuilder := alert.New("", "", opts...) rule := alertBuilder.Builder.Rules[0] req.Equal("description", rule.Annotations["description"]) diff --git a/graph/graph.go b/graph/graph.go index eafc0468..4c730f3d 100644 --- a/graph/graph.go +++ b/graph/graph.go @@ -267,7 +267,6 @@ func XAxis(opts ...axis.Option) Option { func Alert(name string, opts ...alert.Option) Option { return func(graph *Graph) error { al := alert.New(name, graph.Builder.Title, opts...) - al.Builder.Name = graph.Builder.Title graph.Alerts = append(graph.Alerts, al) return nil diff --git a/timeseries/timeseries.go b/timeseries/timeseries.go index 150b6b1c..e18152f8 100644 --- a/timeseries/timeseries.go +++ b/timeseries/timeseries.go @@ -410,7 +410,6 @@ func Transparent() Option { func Alert(name string, opts ...alert.Option) Option { return func(timeseries *TimeSeries) error { al := alert.New(name, timeseries.Builder.Title, opts...) - al.Builder.Name = timeseries.Builder.Title timeseries.Alerts = append(timeseries.Alerts, al) return nil From ccb65962a4a8612c3babd2cfd20f7ba55415a00f Mon Sep 17 00:00:00 2001 From: lueurxax Date: Tue, 30 Apr 2024 18:55:54 +0200 Subject: [PATCH 6/7] Remove redundant alert name assignment 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. --- alerts.go | 5 ----- dashboards.go | 13 +++++++++++++ dashboards_test.go | 2 +- graph/graph_test.go | 9 ++++++--- timeseries/timeseries_test.go | 9 ++++++--- 5 files changed, 26 insertions(+), 12 deletions(-) diff --git a/alerts.go b/alerts.go index 99c73c37..a74a816d 100644 --- a/alerts.go +++ b/alerts.go @@ -57,11 +57,6 @@ func (client *Client) AddAlert(ctx context.Context, namespace string, alertDefin alertDefinition.HookDatasourceUID(datasourceUID) - // Before we can add this alert, we need to delete any other alert that might exist for this dashboard and panel - if err := client.DeleteAlertGroup(ctx, namespace, alertDefinition.Builder.Name); err != nil && !errors.Is(err, ErrAlertNotFound) { - return fmt.Errorf("could not delete existing alerts: %w", err) - } - buf, err := json.Marshal(alertDefinition.Builder) if err != nil { return err diff --git a/dashboards.go b/dashboards.go index 5ada40b3..88ad20cf 100644 --- a/dashboards.go +++ b/dashboards.go @@ -125,6 +125,19 @@ func (client *Client) UpsertDashboard(ctx context.Context, folder *Folder, build return nil, err } + alertPanelNames := make(map[string]struct{}) + for _, al := range alerts { + alertPanelNames[al.PanelName] = struct{}{} + } + + // Clean alerts by panel name + for panelName := range alertPanelNames { + // Before we can add this alert, we need to delete any other alert that might exist for this dashboard and panel + if err := client.DeleteAlertGroup(ctx, folder.Title, panelName); err != nil && !errors.Is(err, ErrAlertNotFound) { + return nil, fmt.Errorf("could not delete existing alerts: %w", err) + } + } + for i := range alerts { alert := *alerts[i] diff --git a/dashboards_test.go b/dashboards_test.go index cb10f1ff..9bd516aa 100644 --- a/dashboards_test.go +++ b/dashboards_test.go @@ -844,7 +844,7 @@ func TestDashboardsCanBeCreatedWithNewAlertsAndDeletesPreviousAlerts(t *testing. req.NoError(err) req.JSONEq(`{ - "name": "Heap allocations", + "name": "Too many heap allocations", "interval": "1m", "rules": [ { diff --git a/graph/graph_test.go b/graph/graph_test.go index 097dc7f4..8e1748c2 100644 --- a/graph/graph_test.go +++ b/graph/graph_test.go @@ -160,7 +160,8 @@ func TestAlertsCanBeConfigured(t *testing.T) { req.NoError(err) req.NotNil(panel.Alerts) req.Len(panel.Alerts, 1) - req.Equal("panel title", panel.Alerts[0].Builder.Name) + req.Equal("some alert", panel.Alerts[0].Builder.Name) + req.Equal("panel title", panel.Alerts[0].PanelName) } func TestTwoAlertsCanBeConfigured(t *testing.T) { @@ -171,8 +172,10 @@ func TestTwoAlertsCanBeConfigured(t *testing.T) { req.NoError(err) req.NotNil(panel.Alerts) req.Len(panel.Alerts, 2) - req.Equal("panel title", panel.Alerts[0].Builder.Name) - req.Equal("panel title", panel.Alerts[1].Builder.Name) + req.Equal("panel title", panel.Alerts[0].PanelName) + req.Equal("panel title", panel.Alerts[1].PanelName) + req.Equal("some alert", panel.Alerts[0].Builder.Name) + req.Equal("other alert", panel.Alerts[1].Builder.Name) } func TestDrawModeCanBeConfigured(t *testing.T) { diff --git a/timeseries/timeseries_test.go b/timeseries/timeseries_test.go index 419d7bdc..2a268bab 100644 --- a/timeseries/timeseries_test.go +++ b/timeseries/timeseries_test.go @@ -146,7 +146,8 @@ func TestAlertsCanBeConfigured(t *testing.T) { req.NoError(err) req.NotNil(panel.Alerts) req.Len(panel.Alerts, 1) - req.Equal("panel title", panel.Alerts[0].Builder.Name) + req.Equal("panel title", panel.Alerts[0].PanelName) + req.Equal("some alert", panel.Alerts[0].Builder.Name) } func TestTwoAlertsCanBeConfigured(t *testing.T) { @@ -157,8 +158,10 @@ func TestTwoAlertsCanBeConfigured(t *testing.T) { req.NoError(err) req.NotNil(panel.Alerts) req.Len(panel.Alerts, 2) - req.Equal("panel title", panel.Alerts[0].Builder.Name) - req.Equal("panel title", panel.Alerts[1].Builder.Name) + req.Equal("panel title", panel.Alerts[0].PanelName) + req.Equal("panel title", panel.Alerts[1].PanelName) + req.Equal("some alert", panel.Alerts[0].Builder.Name) + req.Equal("other alert", panel.Alerts[1].Builder.Name) } func TestLineWidthCanBeConfigured(t *testing.T) { From 155d6a5eed86a932eb06d15a3fa10524500de570 Mon Sep 17 00:00:00 2001 From: lueurxax Date: Wed, 1 May 2024 13:02:21 +0300 Subject: [PATCH 7/7] Optimize handling of alert panel names and panel alerts 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. --- dashboards.go | 2 +- row/row.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/dashboards.go b/dashboards.go index 88ad20cf..68cd305c 100644 --- a/dashboards.go +++ b/dashboards.go @@ -125,7 +125,7 @@ func (client *Client) UpsertDashboard(ctx context.Context, folder *Folder, build return nil, err } - alertPanelNames := make(map[string]struct{}) + alertPanelNames := make(map[string]struct{}, len(alerts)) for _, al := range alerts { alertPanelNames[al.PanelName] = struct{}{} } diff --git a/row/row.go b/row/row.go index 8df34c0a..1f9f1a9b 100644 --- a/row/row.go +++ b/row/row.go @@ -58,7 +58,7 @@ func WithGraph(title string, options ...graph.Option) Option { row.builder.Add(panel.Builder) - if panel.Alerts == nil || len(panel.Alerts) == 0 { + if len(panel.Alerts) == 0 { return nil } @@ -84,7 +84,7 @@ func WithTimeSeries(title string, options ...timeseries.Option) Option { row.builder.Add(panel.Builder) - if panel.Alerts == nil || len(panel.Alerts) == 0 { + if len(panel.Alerts) == 0 { return nil }