From 31b1d58a706be8ce5cdbe66f1d28878c9b4dc572 Mon Sep 17 00:00:00 2001 From: Srikanth Chekuri Date: Wed, 27 Mar 2024 20:25:18 +0530 Subject: [PATCH] chore: fix alerting options (#4752) --- frontend/public/locales/en-GB/alerts.json | 5 ++ frontend/public/locales/en/alerts.json | 5 ++ .../container/FormAlertRules/RuleOptions.tsx | 47 ++++++++++++------- frontend/src/types/api/alerts/def.ts | 4 +- pkg/query-service/rules/alerting.go | 2 +- pkg/query-service/rules/apiParams.go | 6 +-- pkg/query-service/rules/manager.go | 12 ++--- pkg/query-service/rules/promRule.go | 4 +- pkg/query-service/rules/promrule_test.go | 2 +- pkg/query-service/rules/thresholdRule.go | 6 +-- pkg/query-service/rules/thresholdRule_test.go | 6 +-- 11 files changed, 63 insertions(+), 36 deletions(-) diff --git a/frontend/public/locales/en-GB/alerts.json b/frontend/public/locales/en-GB/alerts.json index 4dffb641d3..0901b14d19 100644 --- a/frontend/public/locales/en-GB/alerts.json +++ b/frontend/public/locales/en-GB/alerts.json @@ -37,11 +37,16 @@ "text_condition1": "Send a notification when", "text_condition2": "the threshold", "text_condition3": "during the last", + "option_1min": "1 min", "option_5min": "5 mins", "option_10min": "10 mins", "option_15min": "15 mins", + "option_30min": "30 mins", "option_60min": "60 mins", "option_4hours": "4 hours", + "option_3hours": "3 hours", + "option_6hours": "6 hours", + "option_12hours": "12 hours", "option_24hours": "24 hours", "field_threshold": "Alert Threshold", "option_allthetimes": "all the times", diff --git a/frontend/public/locales/en/alerts.json b/frontend/public/locales/en/alerts.json index 33714d4429..597cc24096 100644 --- a/frontend/public/locales/en/alerts.json +++ b/frontend/public/locales/en/alerts.json @@ -37,11 +37,16 @@ "text_condition1": "Send a notification when", "text_condition2": "the threshold", "text_condition3": "during the last", + "option_1min": "1 min", "option_5min": "5 mins", "option_10min": "10 mins", "option_15min": "15 mins", + "option_30min": "30 mins", "option_60min": "60 mins", + "option_3hours": "3 hours", "option_4hours": "4 hours", + "option_6hours": "6 hours", + "option_12hours": "12 hours", "option_24hours": "24 hours", "field_threshold": "Alert Threshold", "option_allthetimes": "all the times", diff --git a/frontend/src/container/FormAlertRules/RuleOptions.tsx b/frontend/src/container/FormAlertRules/RuleOptions.tsx index d62b39f30f..2ef6bba4c0 100644 --- a/frontend/src/container/FormAlertRules/RuleOptions.tsx +++ b/frontend/src/container/FormAlertRules/RuleOptions.tsx @@ -20,6 +20,7 @@ import { AlertDef, defaultCompareOp, defaultEvalWindow, + defaultFrequency, defaultMatchType, } from 'types/api/alerts/def'; import { EQueryType } from 'types/common/dashboard'; @@ -206,6 +207,35 @@ function RuleOptions({ }); }; + const onChangeFrequency = (value: string | unknown): void => { + const freq = (value as string) || alertDef.frequency; + setAlertDef({ + ...alertDef, + frequency: freq, + }); + }; + + const renderFrequency = (): JSX.Element => ( + + {t('option_1min')} + {t('option_5min')} + {t('option_10min')} + {t('option_15min')} + {t('option_30min')} + {t('option_60min')} + {t('option_3hours')} + {t('option_6hours')} + {t('option_12hours')} + {t('option_24hours')} + + ); + const selectedCategory = getCategoryByOptionId(currentQuery?.unit || ''); const categorySelectOptions = getCategorySelectOptionByName( @@ -250,22 +280,7 @@ function RuleOptions({ {t('text_alert_frequency')} - - { - setAlertDef({ - ...alertDef, - frequency: Number(value) || 0, - }); - }} - type="number" - onWheel={(e): void => e.currentTarget.blur()} - /> - - {t('text_for')} + {renderFrequency()} diff --git a/frontend/src/types/api/alerts/def.ts b/frontend/src/types/api/alerts/def.ts index 96fa86654f..c773cb78a2 100644 --- a/frontend/src/types/api/alerts/def.ts +++ b/frontend/src/types/api/alerts/def.ts @@ -6,6 +6,8 @@ export const defaultMatchType = '1'; // default eval window export const defaultEvalWindow = '5m0s'; +export const defaultFrequency = '1m0s'; + // default compare op: above export const defaultCompareOp = '1'; @@ -14,7 +16,7 @@ export interface AlertDef { alertType?: string; alert?: string; ruleType?: string; - frequency?: number | undefined; + frequency?: string; condition: RuleCondition; labels?: Labels; annotations?: Labels; diff --git a/pkg/query-service/rules/alerting.go b/pkg/query-service/rules/alerting.go index 3e56c2d0c7..b2f511c6c0 100644 --- a/pkg/query-service/rules/alerting.go +++ b/pkg/query-service/rules/alerting.go @@ -137,7 +137,7 @@ type RuleCondition struct { CompareOp CompareOp `yaml:"op,omitempty" json:"op,omitempty"` Target *float64 `yaml:"target,omitempty" json:"target,omitempty"` AlertOnAbsent bool `yaml:"alertOnAbsent,omitempty" json:"alertOnAbsent,omitempty"` - AbsentFor time.Duration `yaml:"absentFor,omitempty" json:"absentFor,omitempty"` + AbsentFor uint64 `yaml:"absentFor,omitempty" json:"absentFor,omitempty"` MatchType MatchType `json:"matchType,omitempty"` TargetUnit string `json:"targetUnit,omitempty"` SelectedQuery string `json:"selectedQueryName,omitempty"` diff --git a/pkg/query-service/rules/apiParams.go b/pkg/query-service/rules/apiParams.go index 1393f59697..af7e9378f6 100644 --- a/pkg/query-service/rules/apiParams.go +++ b/pkg/query-service/rules/apiParams.go @@ -31,7 +31,7 @@ func newApiErrorBadData(err error) *model.ApiError { // PostableRule is used to create alerting rule from HTTP api type PostableRule struct { - Alert string `yaml:"alert,omitempty" json:"alert,omitempty"` + AlertName string `yaml:"alert,omitempty" json:"alert,omitempty"` AlertType string `yaml:"alertType,omitempty" json:"alertType,omitempty"` Description string `yaml:"description,omitempty" json:"description,omitempty"` RuleType RuleType `yaml:"ruleType,omitempty" json:"ruleType,omitempty"` @@ -188,7 +188,7 @@ func (r *PostableRule) Validate() (errs []error) { } func testTemplateParsing(rl *PostableRule) (errs []error) { - if rl.Alert == "" { + if rl.AlertName == "" { // Not an alerting rule. return errs } @@ -200,7 +200,7 @@ func testTemplateParsing(rl *PostableRule) (errs []error) { tmpl := NewTemplateExpander( context.TODO(), defs+text, - "__alert_"+rl.Alert, + "__alert_"+rl.AlertName, tmplData, times.Time(timestamp.FromTime(time.Now())), nil, diff --git a/pkg/query-service/rules/manager.go b/pkg/query-service/rules/manager.go index d5c6e74dd3..cad02523d7 100644 --- a/pkg/query-service/rules/manager.go +++ b/pkg/query-service/rules/manager.go @@ -503,7 +503,7 @@ func (m *Manager) prepareTask(acquireLock bool, r *PostableRule, taskName string rules := make([]Rule, 0) var task Task - if r.Alert == "" { + if r.AlertName == "" { zap.L().Error("task load failed, at least one rule must be set", zap.String("name", taskName)) return task, fmt.Errorf("task load failed, at least one rule must be set") } @@ -525,7 +525,7 @@ func (m *Manager) prepareTask(acquireLock bool, r *PostableRule, taskName string rules = append(rules, tr) // create ch rule task for evalution - task = newTask(TaskTypeCh, taskName, taskNamesuffix, time.Duration(r.Frequency*Duration(time.Minute)), rules, m.opts, m.prepareNotifyFunc()) + task = newTask(TaskTypeCh, taskName, taskNamesuffix, time.Duration(r.Frequency), rules, m.opts, m.prepareNotifyFunc()) // add rule to memory m.rules[ruleId] = tr @@ -536,7 +536,7 @@ func (m *Manager) prepareTask(acquireLock bool, r *PostableRule, taskName string pr, err := NewPromRule( ruleId, r, - log.With(m.logger, "alert", r.Alert), + log.With(m.logger, "alert", r.AlertName), PromRuleOpts{}, ) @@ -547,7 +547,7 @@ func (m *Manager) prepareTask(acquireLock bool, r *PostableRule, taskName string rules = append(rules, pr) // create promql rule task for evalution - task = newTask(TaskTypeProm, taskName, taskNamesuffix, time.Duration(r.Frequency*Duration(time.Minute)), rules, m.opts, m.prepareNotifyFunc()) + task = newTask(TaskTypeProm, taskName, taskNamesuffix, time.Duration(r.Frequency), rules, m.opts, m.prepareNotifyFunc()) // add rule to memory m.rules[ruleId] = pr @@ -850,7 +850,7 @@ func (m *Manager) TestNotification(ctx context.Context, ruleStr string) (int, *m return 0, newApiErrorBadData(errs[0]) } - var alertname = parsedRule.Alert + var alertname = parsedRule.AlertName if alertname == "" { // alertname is not mandatory for testing, so picking // a random string here @@ -858,7 +858,7 @@ func (m *Manager) TestNotification(ctx context.Context, ruleStr string) (int, *m } // append name to indicate this is test alert - parsedRule.Alert = fmt.Sprintf("%s%s", alertname, TestAlertPostFix) + parsedRule.AlertName = fmt.Sprintf("%s%s", alertname, TestAlertPostFix) var rule Rule var err error diff --git a/pkg/query-service/rules/promRule.go b/pkg/query-service/rules/promRule.go index 8f829e0ad3..a998de243e 100644 --- a/pkg/query-service/rules/promRule.go +++ b/pkg/query-service/rules/promRule.go @@ -71,7 +71,7 @@ func NewPromRule( p := PromRule{ id: id, - name: postableRule.Alert, + name: postableRule.AlertName, source: postableRule.Source, ruleCondition: postableRule.RuleCondition, evalWindow: time.Duration(postableRule.EvalWindow), @@ -612,7 +612,7 @@ func (r *PromRule) shouldAlert(series pql.Series) (pql.Sample, bool) { func (r *PromRule) String() string { ar := PostableRule{ - Alert: r.name, + AlertName: r.name, RuleCondition: r.ruleCondition, EvalWindow: Duration(r.evalWindow), Labels: r.labels.Map(), diff --git a/pkg/query-service/rules/promrule_test.go b/pkg/query-service/rules/promrule_test.go index ee843b9b64..0707933b89 100644 --- a/pkg/query-service/rules/promrule_test.go +++ b/pkg/query-service/rules/promrule_test.go @@ -20,7 +20,7 @@ func (l testLogger) Log(args ...interface{}) error { func TestPromRuleShouldAlert(t *testing.T) { postableRule := PostableRule{ - Alert: "Test Rule", + AlertName: "Test Rule", AlertType: "METRIC_BASED_ALERT", RuleType: RuleTypeProm, EvalWindow: Duration(5 * time.Minute), diff --git a/pkg/query-service/rules/thresholdRule.go b/pkg/query-service/rules/thresholdRule.go index c05c61c57b..05fd526b79 100644 --- a/pkg/query-service/rules/thresholdRule.go +++ b/pkg/query-service/rules/thresholdRule.go @@ -102,7 +102,7 @@ func NewThresholdRule( t := ThresholdRule{ id: id, - name: p.Alert, + name: p.AlertName, source: p.Source, ruleCondition: p.RuleCondition, evalWindow: time.Duration(p.EvalWindow), @@ -713,7 +713,7 @@ func (r *ThresholdRule) runChQuery(ctx context.Context, db clickhouse.Conn, quer zap.L().Debug("resultmap(potential alerts)", zap.String("ruleid", r.ID()), zap.Int("count", len(resultMap))) // if the data is missing for `For` duration then we should send alert - if r.ruleCondition.AlertOnAbsent && r.lastTimestampWithDatapoints.Add(r.Condition().AbsentFor*time.Minute).Before(time.Now()) { + if r.ruleCondition.AlertOnAbsent && r.lastTimestampWithDatapoints.Add(time.Duration(r.Condition().AbsentFor)*time.Minute).Before(time.Now()) { zap.L().Info("no data found for rule condition", zap.String("ruleid", r.ID())) lbls := labels.NewBuilder(labels.Labels{}) if !r.lastTimestampWithDatapoints.IsZero() { @@ -1289,7 +1289,7 @@ func (r *ThresholdRule) Eval(ctx context.Context, ts time.Time, queriers *Querie func (r *ThresholdRule) String() string { ar := PostableRule{ - Alert: r.name, + AlertName: r.name, RuleCondition: r.ruleCondition, EvalWindow: Duration(r.evalWindow), Labels: r.labels.Map(), diff --git a/pkg/query-service/rules/thresholdRule_test.go b/pkg/query-service/rules/thresholdRule_test.go index fde35364bc..b7d3cc5fee 100644 --- a/pkg/query-service/rules/thresholdRule_test.go +++ b/pkg/query-service/rules/thresholdRule_test.go @@ -14,7 +14,7 @@ import ( func TestThresholdRuleCombinations(t *testing.T) { postableRule := PostableRule{ - Alert: "Tricky Condition Tests", + AlertName: "Tricky Condition Tests", AlertType: "METRIC_BASED_ALERT", RuleType: RuleTypeThreshold, EvalWindow: Duration(5 * time.Minute), @@ -339,7 +339,7 @@ func TestNormalizeLabelName(t *testing.T) { func TestPrepareLinksToLogs(t *testing.T) { postableRule := PostableRule{ - Alert: "Tricky Condition Tests", + AlertName: "Tricky Condition Tests", AlertType: "LOGS_BASED_ALERT", RuleType: RuleTypeThreshold, EvalWindow: Duration(5 * time.Minute), @@ -381,7 +381,7 @@ func TestPrepareLinksToLogs(t *testing.T) { func TestPrepareLinksToTraces(t *testing.T) { postableRule := PostableRule{ - Alert: "Links to traces test", + AlertName: "Links to traces test", AlertType: "TRACES_BASED_ALERT", RuleType: RuleTypeThreshold, EvalWindow: Duration(5 * time.Minute),