From 31ac507beea24d972e6bbe5e8f7668cc4072c10b Mon Sep 17 00:00:00 2001 From: Aliaksandr Valialkin Date: Fri, 5 Nov 2021 19:49:32 +0200 Subject: [PATCH] app/vmalert: remove `rule.type` config, since it doesnt play well with the upcoming default tenants for -clusterMode It is better from the consistency point of view to set up rule types at group level where tenant config is set up. --- app/vmalert/README.md | 18 ++++-------- app/vmalert/alerting.go | 4 +-- app/vmalert/config/config.go | 13 --------- app/vmalert/config/config_test.go | 2 -- .../testdata/dir/rules-update0-good.rules | 15 ---------- .../testdata/dir/rules-update1-good.rules | 12 -------- app/vmalert/config/testdata/rules3-good.rules | 7 ----- app/vmalert/group_test.go | 12 -------- app/vmalert/manager_test.go | 29 ------------------- app/vmalert/recording.go | 4 +-- docs/vmalert.md | 18 ++++-------- 11 files changed, 14 insertions(+), 120 deletions(-) delete mode 100644 app/vmalert/config/testdata/dir/rules-update0-good.rules delete mode 100644 app/vmalert/config/testdata/dir/rules-update1-good.rules diff --git a/app/vmalert/README.md b/app/vmalert/README.md index d7169df6cd..4496ff44b2 100644 --- a/app/vmalert/README.md +++ b/app/vmalert/README.md @@ -85,7 +85,7 @@ name: [ concurrency: | default = 1 ] # Optional type for expressions inside the rules. Supported values: "graphite" and "prometheus". -# By default "prometheus" rule type is used. +# By default "prometheus" type is used. [ type: ] # Optional list of label filters applied to every rule's @@ -130,12 +130,8 @@ The syntax for alerting rule is the following: # The name of the alert. Must be a valid metric name. alert: -# Optional type for the rule. Supported values: "graphite", "prometheus". -# By default "prometheus" rule type is used. -[ type: ] - # The expression to evaluate. The expression language depends on the type value. -# By default PromQL/MetricsQL expression is used. If type="graphite", then the expression +# By default PromQL/MetricsQL expression is used. If group.type="graphite", then the expression # must contain valid Graphite expression. expr: @@ -166,12 +162,8 @@ The syntax for recording rules is following: # The name of the time series to output to. Must be a valid metric name. record: -# Optional type for the rule. Supported values: "graphite", "prometheus". -# By default "prometheus" rule type is used. -[ type: ] - # The expression to evaluate. The expression language depends on the type value. -# By default MetricsQL expression is used. If type="graphite", then the expression +# By default MetricsQL expression is used. If group.type="graphite", then the expression # must contain valid Graphite expression. expr: @@ -308,8 +300,8 @@ max range per request: 8h20m0s In `replay` mode all groups are executed sequentially one-by-one. Rules within the group are executed sequentially as well (`concurrency` setting is ignored). Vmalert sends rule's expression to [/query_range](https://prometheus.io/docs/prometheus/latest/querying/api/#range-queries) endpoint -of the configured `-datasource.url`. Returned data then processed according to the rule type and -backfilled to `-remoteWrite.url` via [Remote Write protocol](https://prometheus.io/docs/prometheus/latest/storage/#remote-storage-integrations). +of the configured `-datasource.url`. Returned data then processed according to the `group.type` and +backfilled to `-remoteWrite.url` via [remote write protocol](https://prometheus.io/docs/prometheus/latest/storage/#remote-storage-integrations). Vmalert respects `evaluationInterval` value set by flag or per-group during the replay. Vmalert automatically disables caching on VictoriaMetrics side by sending `nocache=1` param. It allows to prevent cache pollution and unwanted time range boundaries adjustment during backfilling. diff --git a/app/vmalert/alerting.go b/app/vmalert/alerting.go index d654246c86..9342ebe0f0 100644 --- a/app/vmalert/alerting.go +++ b/app/vmalert/alerting.go @@ -58,7 +58,7 @@ type alertingRuleMetrics struct { func newAlertingRule(qb datasource.QuerierBuilder, group *Group, cfg config.Rule) *AlertingRule { ar := &AlertingRule{ - Type: cfg.Type, + Type: group.Type, RuleID: cfg.ID, Name: cfg.Alert, Expr: cfg.Expr, @@ -69,7 +69,7 @@ func newAlertingRule(qb datasource.QuerierBuilder, group *Group, cfg config.Rule GroupName: group.Name, EvalInterval: group.Interval, q: qb.BuildWithParams(datasource.QuerierParams{ - DataSourceType: &cfg.Type, + DataSourceType: &group.Type, EvaluationInterval: group.Interval, ExtraLabels: group.ExtraFilterLabels, }), diff --git a/app/vmalert/config/config.go b/app/vmalert/config/config.go index 0cf23210dc..a637062597 100644 --- a/app/vmalert/config/config.go +++ b/app/vmalert/config/config.go @@ -56,14 +56,6 @@ func (g *Group) UnmarshalYAML(unmarshal func(interface{}) error) error { if g.Type.Get() == "" { g.Type.Set(datasource.NewPrometheusType()) } - // update rules with empty type. - for i, r := range g.Rules { - if r.Type.Get() == "" { - r.Type.Set(g.Type) - r.ID = HashRule(r) - g.Rules[i] = r - } - } h := md5.New() h.Write(b) @@ -94,9 +86,6 @@ func (g *Group) Validate(validateAnnotations, validateExpressions bool) error { // its needed only for tests. // because correct types must be inherited after unmarshalling. exprValidator := g.Type.ValidateExpr - if r.Type.Get() != "" { - exprValidator = r.Type.ValidateExpr - } if err := exprValidator(r.Expr); err != nil { return fmt.Errorf("invalid expression for rule %q.%q: %w", g.Name, ruleName, err) } @@ -117,7 +106,6 @@ func (g *Group) Validate(validateAnnotations, validateExpressions bool) error { // recording rule or alerting rule. type Rule struct { ID uint64 - Type datasource.Type `yaml:"type,omitempty"` Record string `yaml:"record,omitempty"` Alert string `yaml:"alert,omitempty"` Expr string `yaml:"expr"` @@ -159,7 +147,6 @@ func HashRule(r Rule) uint64 { h.Write([]byte("alerting")) h.Write([]byte(r.Alert)) } - h.Write([]byte(r.Type.Get())) kv := sortMap(r.Labels) for _, i := range kv { h.Write([]byte(i.key)) diff --git a/app/vmalert/config/config_test.go b/app/vmalert/config/config_test.go index 92f0ad1e4c..8acdafb74d 100644 --- a/app/vmalert/config/config_test.go +++ b/app/vmalert/config/config_test.go @@ -263,7 +263,6 @@ func TestGroup_Validate(t *testing.T) { }, { Expr: "sum(up == 0 ) by (host)", - Type: datasource.NewPrometheusType(), }, }, }, @@ -279,7 +278,6 @@ func TestGroup_Validate(t *testing.T) { }, { Expr: "sumSeries(time('foo.bar',10))", - Type: datasource.NewPrometheusType(), }, }, }, diff --git a/app/vmalert/config/testdata/dir/rules-update0-good.rules b/app/vmalert/config/testdata/dir/rules-update0-good.rules deleted file mode 100644 index e91277ad03..0000000000 --- a/app/vmalert/config/testdata/dir/rules-update0-good.rules +++ /dev/null @@ -1,15 +0,0 @@ -groups: - - name: TestUpdateGroup - interval: 2s - concurrency: 2 - type: prometheus - labels: - cluster: main - rules: - - alert: up - expr: up == 0 - for: 30s - - alert: up graphite - expr: filterSeries(time('host.1',20),'>','0') - for: 30s - type: graphite diff --git a/app/vmalert/config/testdata/dir/rules-update1-good.rules b/app/vmalert/config/testdata/dir/rules-update1-good.rules deleted file mode 100644 index 0fcad76877..0000000000 --- a/app/vmalert/config/testdata/dir/rules-update1-good.rules +++ /dev/null @@ -1,12 +0,0 @@ -groups: - - name: TestUpdateGroup - interval: 30s - type: graphite - rules: - - alert: up - expr: filterSeries(time('host.2',20),'>','0') - for: 30s - - alert: up graphite - expr: filterSeries(time('host.1',20),'>','0') - for: 30s - type: graphite diff --git a/app/vmalert/config/testdata/rules3-good.rules b/app/vmalert/config/testdata/rules3-good.rules index 3e260b94d3..0eedbe0e85 100644 --- a/app/vmalert/config/testdata/rules3-good.rules +++ b/app/vmalert/config/testdata/rules3-good.rules @@ -21,10 +21,3 @@ groups: annotations: summary: Too high connection number for {{$labels.instance}} description: "It is {{ $value }} connections for {{$labels.instance}}" - - alert: HostDown - type: graphite - expr: filterSeries(sumSeries(host.receiver.interface.up),'last','=', 0) - for: 3m - annotations: - summary: Too high connection number for {{$labels.instance}} - description: "It is {{ $value }} connections for {{$labels.instance}}" diff --git a/app/vmalert/group_test.go b/app/vmalert/group_test.go index 3910f4170a..b062522563 100644 --- a/app/vmalert/group_test.go +++ b/app/vmalert/group_test.go @@ -8,7 +8,6 @@ import ( "time" "github.com/VictoriaMetrics/VictoriaMetrics/app/vmalert/config" - "github.com/VictoriaMetrics/VictoriaMetrics/app/vmalert/datasource" "github.com/VictoriaMetrics/VictoriaMetrics/app/vmalert/notifier" "github.com/VictoriaMetrics/VictoriaMetrics/app/vmalert/utils" ) @@ -108,17 +107,6 @@ func TestUpdateWith(t *testing.T) { {Record: "foo5"}, }, }, - { - "update datasource type", - []config.Rule{ - {Alert: "foo1", Type: datasource.NewPrometheusType()}, - {Alert: "foo3", Type: datasource.NewGraphiteType()}, - }, - []config.Rule{ - {Alert: "foo1", Type: datasource.NewGraphiteType()}, - {Alert: "foo10", Type: datasource.NewPrometheusType()}, - }, - }, } for _, tc := range testCases { diff --git a/app/vmalert/manager_test.go b/app/vmalert/manager_test.go index 31d8853699..503cd9e53a 100644 --- a/app/vmalert/manager_test.go +++ b/app/vmalert/manager_test.go @@ -113,18 +113,6 @@ func TestManagerUpdate(t *testing.T) { Name: "ExampleAlertAlwaysFiring", Expr: "sum by(job) (up == 1)", } - ExampleAlertGraphite = &AlertingRule{ - Name: "up graphite", - Expr: "filterSeries(time('host.1',20),'>','0')", - Type: datasource.NewGraphiteType(), - For: defaultEvalInterval, - } - ExampleAlertGraphite2 = &AlertingRule{ - Name: "up", - Expr: "filterSeries(time('host.2',20),'>','0')", - Type: datasource.NewGraphiteType(), - For: defaultEvalInterval, - } ) testCases := []struct { @@ -226,23 +214,6 @@ func TestManagerUpdate(t *testing.T) { }, }, }, - { - name: "update prometheus to graphite type", - initPath: "config/testdata/dir/rules-update0-good.rules", - updatePath: "config/testdata/dir/rules-update1-good.rules", - want: []*Group{ - { - File: "config/testdata/dir/rules-update1-good.rules", - Interval: defaultEvalInterval, - Type: datasource.NewGraphiteType(), - Name: "TestUpdateGroup", - Rules: []Rule{ - ExampleAlertGraphite2, - ExampleAlertGraphite, - }, - }, - }, - }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { diff --git a/app/vmalert/recording.go b/app/vmalert/recording.go index 0907be124d..32924d075e 100644 --- a/app/vmalert/recording.go +++ b/app/vmalert/recording.go @@ -60,7 +60,7 @@ func (rr *RecordingRule) ID() uint64 { func newRecordingRule(qb datasource.QuerierBuilder, group *Group, cfg config.Rule) *RecordingRule { rr := &RecordingRule{ - Type: cfg.Type, + Type: group.Type, RuleID: cfg.ID, Name: cfg.Record, Expr: cfg.Expr, @@ -68,7 +68,7 @@ func newRecordingRule(qb datasource.QuerierBuilder, group *Group, cfg config.Rul GroupID: group.ID(), metrics: &recordingRuleMetrics{}, q: qb.BuildWithParams(datasource.QuerierParams{ - DataSourceType: &cfg.Type, + DataSourceType: &group.Type, EvaluationInterval: group.Interval, ExtraLabels: group.ExtraFilterLabels, }), diff --git a/docs/vmalert.md b/docs/vmalert.md index 425e48ffc8..4d9d13a587 100644 --- a/docs/vmalert.md +++ b/docs/vmalert.md @@ -89,7 +89,7 @@ name: [ concurrency: | default = 1 ] # Optional type for expressions inside the rules. Supported values: "graphite" and "prometheus". -# By default "prometheus" rule type is used. +# By default "prometheus" type is used. [ type: ] # Optional list of label filters applied to every rule's @@ -134,12 +134,8 @@ The syntax for alerting rule is the following: # The name of the alert. Must be a valid metric name. alert: -# Optional type for the rule. Supported values: "graphite", "prometheus". -# By default "prometheus" rule type is used. -[ type: ] - # The expression to evaluate. The expression language depends on the type value. -# By default PromQL/MetricsQL expression is used. If type="graphite", then the expression +# By default PromQL/MetricsQL expression is used. If group.type="graphite", then the expression # must contain valid Graphite expression. expr: @@ -170,12 +166,8 @@ The syntax for recording rules is following: # The name of the time series to output to. Must be a valid metric name. record: -# Optional type for the rule. Supported values: "graphite", "prometheus". -# By default "prometheus" rule type is used. -[ type: ] - # The expression to evaluate. The expression language depends on the type value. -# By default MetricsQL expression is used. If type="graphite", then the expression +# By default MetricsQL expression is used. If group.type="graphite", then the expression # must contain valid Graphite expression. expr: @@ -312,8 +304,8 @@ max range per request: 8h20m0s In `replay` mode all groups are executed sequentially one-by-one. Rules within the group are executed sequentially as well (`concurrency` setting is ignored). Vmalert sends rule's expression to [/query_range](https://prometheus.io/docs/prometheus/latest/querying/api/#range-queries) endpoint -of the configured `-datasource.url`. Returned data then processed according to the rule type and -backfilled to `-remoteWrite.url` via [Remote Write protocol](https://prometheus.io/docs/prometheus/latest/storage/#remote-storage-integrations). +of the configured `-datasource.url`. Returned data then processed according to the `group.type` and +backfilled to `-remoteWrite.url` via [remote write protocol](https://prometheus.io/docs/prometheus/latest/storage/#remote-storage-integrations). Vmalert respects `evaluationInterval` value set by flag or per-group during the replay. Vmalert automatically disables caching on VictoriaMetrics side by sending `nocache=1` param. It allows to prevent cache pollution and unwanted time range boundaries adjustment during backfilling.