From 79300340af1c44fa12b483df2899aaa37b98cfa0 Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Mon, 9 Jan 2023 10:14:37 +0200 Subject: [PATCH 1/7] Adding recording/alerting rule origin context This will allow correlation of executed rule queries with their associated rule names and type Signed-off-by: Danny Kopping --- rules/alerting.go | 2 ++ rules/alerting_test.go | 32 ++++++++++++++++++++++++++++++++ rules/origin.go | 34 ++++++++++++++++++++++++++++++++++ rules/recording.go | 2 ++ rules/recording_test.go | 22 ++++++++++++++++++++++ 5 files changed, 92 insertions(+) create mode 100644 rules/origin.go diff --git a/rules/alerting.go b/rules/alerting.go index d45666266..730e5a76f 100644 --- a/rules/alerting.go +++ b/rules/alerting.go @@ -311,6 +311,8 @@ const resolvedRetention = 15 * time.Minute // Eval evaluates the rule expression and then creates pending alerts and fires // or removes previously pending alerts accordingly. func (r *AlertingRule) Eval(ctx context.Context, ts time.Time, query QueryFunc, externalURL *url.URL, limit int) (promql.Vector, error) { + ctx = NewOriginContext(ctx, NewRuleDetail(r.Name(), r.Query().String(), KindAlerting)) + res, err := query(ctx, r.vector.String(), ts) if err != nil { return nil, err diff --git a/rules/alerting_test.go b/rules/alerting_test.go index 4f5f5e683..bc41c0ff1 100644 --- a/rules/alerting_test.go +++ b/rules/alerting_test.go @@ -712,3 +712,35 @@ func TestSendAlertsDontAffectActiveAlerts(t *testing.T) { // But the labels with the AlertingRule should not be changed. require.Equal(t, labels.FromStrings("a1", "1"), rule.active[h].Labels) } + +func TestAlertingEvalWithOrigin(t *testing.T) { + ctx := context.Background() + now := time.Now() + + const name = "my-recording-rule" + const query = `count(metric{foo="bar"}) > 0` + + var detail RuleDetail + + expr, err := parser.ParseExpr(query) + require.NoError(t, err) + + rule := NewAlertingRule( + name, + expr, + time.Minute, + labels.FromStrings("test", "test"), + nil, + nil, + "", + true, log.NewNopLogger(), + ) + + _, err = rule.Eval(ctx, now, func(ctx context.Context, qs string, _ time.Time) (promql.Vector, error) { + detail = FromOriginContext(ctx) + return nil, nil + }, nil, 0) + + require.NoError(t, err) + require.Equal(t, detail, NewRuleDetail(name, query, KindAlerting)) +} diff --git a/rules/origin.go b/rules/origin.go new file mode 100644 index 000000000..c972244a0 --- /dev/null +++ b/rules/origin.go @@ -0,0 +1,34 @@ +package rules + +import "context" + +type ruleOrigin struct{} + +type RuleDetail struct { + Name string + Kind string + Query string +} + +const KindAlerting = "alerting" +const KindRecording = "recording" + +func NewRuleDetail(name, query, kind string) RuleDetail { + return RuleDetail{ + Name: name, + Query: query, + Kind: kind, + } +} + +// NewOriginContext returns a new context with data about the origin attached. +func NewOriginContext(ctx context.Context, rule RuleDetail) context.Context { + return context.WithValue(ctx, ruleOrigin{}, rule) +} + +func FromOriginContext(ctx context.Context) RuleDetail { + if rule, ok := ctx.Value(ruleOrigin{}).(RuleDetail); ok { + return rule + } + return RuleDetail{} +} diff --git a/rules/recording.go b/rules/recording.go index 8e7eefa1d..81ca47c91 100644 --- a/rules/recording.go +++ b/rules/recording.go @@ -73,6 +73,8 @@ func (rule *RecordingRule) Labels() labels.Labels { // Eval evaluates the rule and then overrides the metric names and labels accordingly. func (rule *RecordingRule) Eval(ctx context.Context, ts time.Time, query QueryFunc, _ *url.URL, limit int) (promql.Vector, error) { + ctx = NewOriginContext(ctx, NewRuleDetail(rule.Name(), rule.Query().String(), KindRecording)) + vector, err := query(ctx, rule.vector.String(), ts) if err != nil { return nil, err diff --git a/rules/recording_test.go b/rules/recording_test.go index 366dac52a..54c391215 100644 --- a/rules/recording_test.go +++ b/rules/recording_test.go @@ -156,3 +156,25 @@ func TestRecordingRuleLimit(t *testing.T) { } } } + +func TestRecordingEvalWithOrigin(t *testing.T) { + ctx := context.Background() + now := time.Now() + + const name = "my-recording-rule" + const query = `count(metric{foo="bar"})` + + var detail RuleDetail + + expr, err := parser.ParseExpr(query) + require.NoError(t, err) + + rule := NewRecordingRule(name, expr, []labels.Label{}) + _, err = rule.Eval(ctx, now, func(ctx context.Context, qs string, _ time.Time) (promql.Vector, error) { + detail = FromOriginContext(ctx) + return nil, nil + }, nil, 0) + + require.NoError(t, err) + require.Equal(t, detail, NewRuleDetail(name, query, KindRecording)) +} From d8f3e7d16cbcbbd705791753993eb288fb741a65 Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Mon, 9 Jan 2023 10:42:13 +0200 Subject: [PATCH 2/7] gofumpt Signed-off-by: Danny Kopping --- rules/origin.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/rules/origin.go b/rules/origin.go index c972244a0..e78a07de5 100644 --- a/rules/origin.go +++ b/rules/origin.go @@ -10,8 +10,10 @@ type RuleDetail struct { Query string } -const KindAlerting = "alerting" -const KindRecording = "recording" +const ( + KindAlerting = "alerting" + KindRecording = "recording" +) func NewRuleDetail(name, query, kind string) RuleDetail { return RuleDetail{ From 72527b5f12d95ebbb0f5806a996391779994a8da Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Mon, 9 Jan 2023 10:53:49 +0200 Subject: [PATCH 3/7] Refactoring for simplicity Include labels Signed-off-by: Danny Kopping --- rules/alerting.go | 2 +- rules/alerting_test.go | 17 +++++++++++------ rules/origin.go | 35 +++++++++++++++++++++++++++-------- rules/recording.go | 2 +- rules/recording_test.go | 16 +++++++++++----- 5 files changed, 51 insertions(+), 21 deletions(-) diff --git a/rules/alerting.go b/rules/alerting.go index 730e5a76f..e862ee241 100644 --- a/rules/alerting.go +++ b/rules/alerting.go @@ -311,7 +311,7 @@ const resolvedRetention = 15 * time.Minute // Eval evaluates the rule expression and then creates pending alerts and fires // or removes previously pending alerts accordingly. func (r *AlertingRule) Eval(ctx context.Context, ts time.Time, query QueryFunc, externalURL *url.URL, limit int) (promql.Vector, error) { - ctx = NewOriginContext(ctx, NewRuleDetail(r.Name(), r.Query().String(), KindAlerting)) + ctx = NewOriginContext(ctx, NewRuleDetail(r)) res, err := query(ctx, r.vector.String(), ts) if err != nil { diff --git a/rules/alerting_test.go b/rules/alerting_test.go index bc41c0ff1..6e6b72bce 100644 --- a/rules/alerting_test.go +++ b/rules/alerting_test.go @@ -713,14 +713,19 @@ func TestSendAlertsDontAffectActiveAlerts(t *testing.T) { require.Equal(t, labels.FromStrings("a1", "1"), rule.active[h].Labels) } +// TestAlertingEvalWithOrigin checks that the alerting rule details are passed through the context. func TestAlertingEvalWithOrigin(t *testing.T) { ctx := context.Background() now := time.Now() - const name = "my-recording-rule" - const query = `count(metric{foo="bar"}) > 0` - - var detail RuleDetail + const ( + name = "my-recording-rule" + query = `count(metric{foo="bar"}) > 0` + ) + var ( + detail RuleDetail + lbs = labels.FromStrings("test", "test") + ) expr, err := parser.ParseExpr(query) require.NoError(t, err) @@ -729,7 +734,7 @@ func TestAlertingEvalWithOrigin(t *testing.T) { name, expr, time.Minute, - labels.FromStrings("test", "test"), + lbs, nil, nil, "", @@ -742,5 +747,5 @@ func TestAlertingEvalWithOrigin(t *testing.T) { }, nil, 0) require.NoError(t, err) - require.Equal(t, detail, NewRuleDetail(name, query, KindAlerting)) + require.Equal(t, detail, NewRuleDetail(rule)) } diff --git a/rules/origin.go b/rules/origin.go index e78a07de5..edc7471c1 100644 --- a/rules/origin.go +++ b/rules/origin.go @@ -1,13 +1,19 @@ package rules -import "context" +import ( + "context" + + "github.com/prometheus/prometheus/model/labels" +) type ruleOrigin struct{} +// RuleDetail contains information about the rule that is being evaluated. type RuleDetail struct { - Name string - Kind string - Query string + Name string + Query string + Labels labels.Labels + Kind string } const ( @@ -15,11 +21,23 @@ const ( KindRecording = "recording" ) -func NewRuleDetail(name, query, kind string) RuleDetail { +// NewRuleDetail creates a RuleDetail from a given Rule. +func NewRuleDetail(r Rule) RuleDetail { + var kind string + switch r.(type) { + case *AlertingRule: + kind = KindAlerting + case *RecordingRule: + kind = KindRecording + default: + kind = "unknown" + } + return RuleDetail{ - Name: name, - Query: query, - Kind: kind, + Name: r.Name(), + Query: r.Query().String(), + Labels: r.Labels(), + Kind: kind, } } @@ -28,6 +46,7 @@ func NewOriginContext(ctx context.Context, rule RuleDetail) context.Context { return context.WithValue(ctx, ruleOrigin{}, rule) } +// FromOriginContext returns the RuleDetail origin data from the context. func FromOriginContext(ctx context.Context) RuleDetail { if rule, ok := ctx.Value(ruleOrigin{}).(RuleDetail); ok { return rule diff --git a/rules/recording.go b/rules/recording.go index 81ca47c91..49ffa2d6d 100644 --- a/rules/recording.go +++ b/rules/recording.go @@ -73,7 +73,7 @@ func (rule *RecordingRule) Labels() labels.Labels { // Eval evaluates the rule and then overrides the metric names and labels accordingly. func (rule *RecordingRule) Eval(ctx context.Context, ts time.Time, query QueryFunc, _ *url.URL, limit int) (promql.Vector, error) { - ctx = NewOriginContext(ctx, NewRuleDetail(rule.Name(), rule.Query().String(), KindRecording)) + ctx = NewOriginContext(ctx, NewRuleDetail(rule)) vector, err := query(ctx, rule.vector.String(), ts) if err != nil { diff --git a/rules/recording_test.go b/rules/recording_test.go index 54c391215..ea33fbdd6 100644 --- a/rules/recording_test.go +++ b/rules/recording_test.go @@ -157,24 +157,30 @@ func TestRecordingRuleLimit(t *testing.T) { } } +// TestRecordingEvalWithOrigin checks that the recording rule details are passed through the context. func TestRecordingEvalWithOrigin(t *testing.T) { ctx := context.Background() now := time.Now() - const name = "my-recording-rule" - const query = `count(metric{foo="bar"})` + const ( + name = "my-recording-rule" + query = `count(metric{foo="bar"})` + ) - var detail RuleDetail + var ( + detail RuleDetail + lbs = labels.FromStrings("foo", "bar") + ) expr, err := parser.ParseExpr(query) require.NoError(t, err) - rule := NewRecordingRule(name, expr, []labels.Label{}) + rule := NewRecordingRule(name, expr, lbs) _, err = rule.Eval(ctx, now, func(ctx context.Context, qs string, _ time.Time) (promql.Vector, error) { detail = FromOriginContext(ctx) return nil, nil }, nil, 0) require.NoError(t, err) - require.Equal(t, detail, NewRuleDetail(name, query, KindRecording)) + require.Equal(t, detail, NewRuleDetail(rule)) } From 4d8478d9ac1b7d30c18c6870a617fe4fe022f2f4 Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Mon, 9 Jan 2023 11:05:56 +0200 Subject: [PATCH 4/7] Add license header to appease CI Signed-off-by: Danny Kopping --- rules/origin.go | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/rules/origin.go b/rules/origin.go index edc7471c1..7957777ab 100644 --- a/rules/origin.go +++ b/rules/origin.go @@ -1,3 +1,16 @@ +// Copyright 2023 The Prometheus Authors +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + package rules import ( From 6486d28c7a2e681e54f170719761e4fcd751afb5 Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Fri, 20 Jan 2023 10:27:50 +0200 Subject: [PATCH 5/7] Panic if rule type was not expected Signed-off-by: Danny Kopping --- rules/origin.go | 3 ++- rules/origin_test.go | 37 +++++++++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-) create mode 100644 rules/origin_test.go diff --git a/rules/origin.go b/rules/origin.go index 7957777ab..996538767 100644 --- a/rules/origin.go +++ b/rules/origin.go @@ -15,6 +15,7 @@ package rules import ( "context" + "fmt" "github.com/prometheus/prometheus/model/labels" ) @@ -43,7 +44,7 @@ func NewRuleDetail(r Rule) RuleDetail { case *RecordingRule: kind = KindRecording default: - kind = "unknown" + panic(fmt.Sprintf(`unknown rule type "%T"`, r)) } return RuleDetail{ diff --git a/rules/origin_test.go b/rules/origin_test.go new file mode 100644 index 000000000..0ea654815 --- /dev/null +++ b/rules/origin_test.go @@ -0,0 +1,37 @@ +package rules + +import ( + "context" + "net/url" + "testing" + "time" + + "github.com/prometheus/prometheus/model/labels" + "github.com/prometheus/prometheus/promql" + "github.com/prometheus/prometheus/promql/parser" + "github.com/stretchr/testify/require" +) + +type unknownRule struct{} + +func (u unknownRule) Name() string { return "" } +func (u unknownRule) Labels() labels.Labels { return nil } +func (u unknownRule) Eval(ctx context.Context, time time.Time, queryFunc QueryFunc, url *url.URL, i int) (promql.Vector, error) { + return nil, nil +} +func (u unknownRule) String() string { return "" } +func (u unknownRule) Query() parser.Expr { return nil } +func (u unknownRule) SetLastError(err error) {} +func (u unknownRule) LastError() error { return nil } +func (u unknownRule) SetHealth(health RuleHealth) {} +func (u unknownRule) Health() RuleHealth { return "" } +func (u unknownRule) SetEvaluationDuration(duration time.Duration) {} +func (u unknownRule) GetEvaluationDuration() time.Duration { return 0 } +func (u unknownRule) SetEvaluationTimestamp(time time.Time) {} +func (u unknownRule) GetEvaluationTimestamp() time.Time { return time.Time{} } + +func TestNewRuleDetailPanics(t *testing.T) { + require.PanicsWithValue(t, `unknown rule type "rules.unknownRule"`, func() { + NewRuleDetail(unknownRule{}) + }) +} From c4ca791f1828a5f5f05e7dbce01f86cbba45039f Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Fri, 20 Jan 2023 10:51:41 +0200 Subject: [PATCH 6/7] Appeasing the linter Signed-off-by: Danny Kopping --- rules/origin_test.go | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/rules/origin_test.go b/rules/origin_test.go index 0ea654815..f11caf2f6 100644 --- a/rules/origin_test.go +++ b/rules/origin_test.go @@ -1,3 +1,16 @@ +// Copyright 2023 The Prometheus Authors +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + package rules import ( @@ -6,10 +19,11 @@ import ( "testing" "time" + "github.com/stretchr/testify/require" + "github.com/prometheus/prometheus/model/labels" "github.com/prometheus/prometheus/promql" "github.com/prometheus/prometheus/promql/parser" - "github.com/stretchr/testify/require" ) type unknownRule struct{} From 98c70e1817b5d9dfdb35761142ceeb567e80872f Mon Sep 17 00:00:00 2001 From: Danny Kopping Date: Thu, 26 Jan 2023 13:21:50 +0200 Subject: [PATCH 7/7] Correcting NewAlertingRule args Signed-off-by: Danny Kopping --- rules/alerting_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/rules/alerting_test.go b/rules/alerting_test.go index 050110e6e..11f4f07f3 100644 --- a/rules/alerting_test.go +++ b/rules/alerting_test.go @@ -916,6 +916,7 @@ func TestAlertingEvalWithOrigin(t *testing.T) { rule := NewAlertingRule( name, expr, + time.Second, time.Minute, lbs, nil,