From 2d0e3746ac9e541f72a3c23702ec54817be58c33 Mon Sep 17 00:00:00 2001 From: Fabian Reinartz Date: Thu, 23 Nov 2017 13:04:54 +0100 Subject: [PATCH] rules: remove dependency on promql.Engine --- cmd/prometheus/main.go | 5 +- rules/alerting.go | 10 +-- rules/manager.go | 37 +++++++++-- rules/manager_test.go | 135 ++++++++++++++++++++++++-------------- rules/recording.go | 25 +------ rules/recording_test.go | 2 +- template/template.go | 43 +++++------- template/template_test.go | 108 ++++++++++++++++-------------- web/web.go | 20 +++++- 9 files changed, 220 insertions(+), 165 deletions(-) diff --git a/cmd/prometheus/main.go b/cmd/prometheus/main.go index e0f03e6f8..4b19c9b61 100644 --- a/cmd/prometheus/main.go +++ b/cmd/prometheus/main.go @@ -96,6 +96,9 @@ func main() { notifier: notifier.Options{ Registerer: prometheus.DefaultRegisterer, }, + queryEngine: promql.EngineOptions{ + Metrics: prometheus.DefaultRegisterer, + }, } a := kingpin.New(filepath.Base(os.Args[0]), "The Prometheus monitoring server") @@ -234,7 +237,7 @@ func main() { ruleManager := rules.NewManager(&rules.ManagerOptions{ Appendable: fanoutStorage, Notifier: notifier, - QueryEngine: queryEngine, + Query: rules.EngineQueryFunc(queryEngine), Context: ctx, ExternalURL: cfg.web.ExternalURL, Logger: log.With(logger, "component", "rule manager"), diff --git a/rules/alerting.go b/rules/alerting.go index f90413713..3b8b61e9f 100644 --- a/rules/alerting.go +++ b/rules/alerting.go @@ -171,12 +171,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, engine *promql.Engine, externalURL *url.URL) (promql.Vector, error) { - query, err := engine.NewInstantQuery(r.vector.String(), ts) - if err != nil { - return nil, err - } - res, err := query.Exec(ctx).Vector() +func (r *AlertingRule) Eval(ctx context.Context, ts time.Time, query QueryFunc, externalURL *url.URL) (promql.Vector, error) { + res, err := query(ctx, r.vector.String(), ts) if err != nil { return nil, err } @@ -213,7 +209,7 @@ func (r *AlertingRule) Eval(ctx context.Context, ts time.Time, engine *promql.En "__alert_"+r.Name(), tmplData, model.Time(timestamp.FromTime(ts)), - engine, + template.QueryFunc(query), externalURL, ) result, err := tmpl.Expand() diff --git a/rules/manager.go b/rules/manager.go index 43f316056..3d7107036 100644 --- a/rules/manager.go +++ b/rules/manager.go @@ -107,12 +107,42 @@ const ( ruleTypeRecording = "recording" ) +// QueryFunc processes PromQL queries. +type QueryFunc func(ctx context.Context, q string, t time.Time) (promql.Vector, error) + +// EngineQueryFunc returns a new query function that executes instant queries against +// the given engine. +// It converts scaler into vector results. +func EngineQueryFunc(engine *promql.Engine) QueryFunc { + return func(ctx context.Context, qs string, t time.Time) (promql.Vector, error) { + q, err := engine.NewInstantQuery(qs, t) + if err != nil { + return nil, err + } + res := q.Exec(ctx) + if res.Err != nil { + return nil, res.Err + } + switch v := res.Value.(type) { + case promql.Vector: + return v, nil + case promql.Scalar: + return promql.Vector{promql.Sample{ + Point: promql.Point(v), + Metric: labels.Labels{}, + }}, nil + default: + return nil, fmt.Errorf("rule result is not a vector or scalar") + } + } +} + // A Rule encapsulates a vector expression which is evaluated at a specified // interval and acted upon (currently either recorded or used for alerting). type Rule interface { Name() string // eval evaluates the rule, including any associated recording or alerting actions. - Eval(context.Context, time.Time, *promql.Engine, *url.URL) (promql.Vector, error) + Eval(context.Context, time.Time, QueryFunc, *url.URL) (promql.Vector, error) // String returns a human-readable string representation of the rule. String() string @@ -220,7 +250,6 @@ func (g *Group) hash() uint64 { labels.Label{"name", g.name}, labels.Label{"file", g.file}, ) - return l.Hash() } @@ -319,7 +348,7 @@ func (g *Group) Eval(ts time.Time) { evalTotal.WithLabelValues(rtyp).Inc() - vector, err := rule.Eval(g.opts.Context, ts, g.opts.QueryEngine, g.opts.ExternalURL) + vector, err := rule.Eval(g.opts.Context, ts, g.opts.Query, g.opts.ExternalURL) if err != nil { // Canceled queries are intentional termination of queries. This normally // happens on shutdown and thus we skip logging of any errors here. @@ -439,7 +468,7 @@ type Appendable interface { // ManagerOptions bundles options for the Manager. type ManagerOptions struct { ExternalURL *url.URL - QueryEngine *promql.Engine + Query QueryFunc Context context.Context Notifier *notifier.Notifier Appendable Appendable diff --git a/rules/manager_test.go b/rules/manager_test.go index 5ff947129..b9cb2ba49 100644 --- a/rules/manager_test.go +++ b/rules/manager_test.go @@ -17,7 +17,7 @@ import ( "context" "fmt" "math" - "strings" + "sort" "testing" "time" @@ -55,75 +55,108 @@ func TestAlertingRule(t *testing.T) { labels.FromStrings("severity", "{{\"c\"}}ritical"), nil, nil, ) + result := promql.Vector{ + { + Metric: labels.FromStrings( + "__name__", "ALERTS", + "alertname", "HTTPRequestRateLow", + "alertstate", "pending", + "group", "canary", + "instance", "0", + "job", "app-server", + "severity", "critical", + ), + Point: promql.Point{V: 1}, + }, + { + Metric: labels.FromStrings( + "__name__", "ALERTS", + "alertname", "HTTPRequestRateLow", + "alertstate", "pending", + "group", "canary", + "instance", "1", + "job", "app-server", + "severity", "critical", + ), + Point: promql.Point{V: 1}, + }, + { + Metric: labels.FromStrings( + "__name__", "ALERTS", + "alertname", "HTTPRequestRateLow", + "alertstate", "firing", + "group", "canary", + "instance", "0", + "job", "app-server", + "severity", "critical", + ), + Point: promql.Point{V: 1}, + }, + { + Metric: labels.FromStrings( + "__name__", "ALERTS", + "alertname", "HTTPRequestRateLow", + "alertstate", "firing", + "group", "canary", + "instance", "1", + "job", "app-server", + "severity", "critical", + ), + Point: promql.Point{V: 1}, + }, + } baseTime := time.Unix(0, 0) var tests = []struct { time time.Duration - result []string + result promql.Vector }{ { - time: 0, - result: []string{ - `{__name__="ALERTS", alertname="HTTPRequestRateLow", alertstate="pending", group="canary", instance="0", job="app-server", severity="critical"} => 1 @[%v]`, - `{__name__="ALERTS", alertname="HTTPRequestRateLow", alertstate="pending", group="canary", instance="1", job="app-server", severity="critical"} => 1 @[%v]`, - }, + time: 0, + result: result[:2], }, { - time: 5 * time.Minute, - result: []string{ - `{__name__="ALERTS", alertname="HTTPRequestRateLow", alertstate="firing", group="canary", instance="0", job="app-server", severity="critical"} => 1 @[%v]`, - `{__name__="ALERTS", alertname="HTTPRequestRateLow", alertstate="firing", group="canary", instance="1", job="app-server", severity="critical"} => 1 @[%v]`, - }, + time: 5 * time.Minute, + result: result[2:], }, { - time: 10 * time.Minute, - result: []string{ - `{__name__="ALERTS", alertname="HTTPRequestRateLow", alertstate="firing", group="canary", instance="0", job="app-server", severity="critical"} => 1 @[%v]`, - }, + time: 10 * time.Minute, + result: result[2:3], }, { time: 15 * time.Minute, - result: []string{}, + result: nil, }, { time: 20 * time.Minute, - result: []string{}, + result: nil, }, { - time: 25 * time.Minute, - result: []string{ - `{__name__="ALERTS", alertname="HTTPRequestRateLow", alertstate="pending", group="canary", instance="0", job="app-server", severity="critical"} => 1 @[%v]`, - }, + time: 25 * time.Minute, + result: result[:1], }, { - time: 30 * time.Minute, - result: []string{ - `{__name__="ALERTS", alertname="HTTPRequestRateLow", alertstate="firing", group="canary", instance="0", job="app-server", severity="critical"} => 1 @[%v]`, - }, + time: 30 * time.Minute, + result: result[2:3], }, } for i, test := range tests { + t.Logf("case %d", i) + evalTime := baseTime.Add(test.time) - res, err := rule.Eval(suite.Context(), evalTime, suite.QueryEngine(), nil) + res, err := rule.Eval(suite.Context(), evalTime, EngineQueryFunc(suite.QueryEngine()), nil) testutil.Ok(t, err) - actual := strings.Split(res.String(), "\n") - expected := annotateWithTime(test.result, evalTime) - if actual[0] == "" { - actual = []string{} - } - testutil.Assert(t, len(expected) == len(actual), "%d. Number of samples in expected and actual output don't match (%d vs. %d)", i, len(expected), len(actual)) - - for j, expectedSample := range expected { - found := false - for _, actualSample := range actual { - if actualSample == expectedSample { - found = true - } - } - testutil.Assert(t, found, "%d.%d. Couldn't find expected sample in output: '%v'", i, j, expectedSample) + for i := range test.result { + test.result[i].T = timestamp.FromTime(evalTime) } + testutil.Assert(t, len(test.result) == len(res), "%d. Number of samples in expected and actual output don't match (%d vs. %d)", i, len(test.result), len(res)) + + sort.Slice(res, func(i, j int) bool { + return labels.Compare(res[i].Metric, res[j].Metric) < 0 + }) + testutil.Equals(t, test.result, res) for _, aa := range rule.ActiveAlerts() { testutil.Assert(t, aa.Labels.Get(model.MetricNameLabel) == "", "%s label set on active alert: %s", model.MetricNameLabel, aa.Labels) @@ -144,10 +177,10 @@ func TestStaleness(t *testing.T) { defer storage.Close() engine := promql.NewEngine(storage, nil) opts := &ManagerOptions{ - QueryEngine: engine, - Appendable: storage, - Context: context.Background(), - Logger: log.NewNopLogger(), + Query: EngineQueryFunc(engine), + Appendable: storage, + Context: context.Background(), + Logger: log.NewNopLogger(), } expr, err := promql.ParseExpr("a + 1") @@ -271,11 +304,11 @@ func TestApplyConfig(t *testing.T) { conf, err := config.LoadFile("../config/testdata/conf.good.yml") testutil.Ok(t, err) ruleManager := NewManager(&ManagerOptions{ - Appendable: nil, - Notifier: nil, - QueryEngine: nil, - Context: context.Background(), - Logger: log.NewNopLogger(), + Appendable: nil, + Notifier: nil, + Query: nil, + Context: context.Background(), + Logger: log.NewNopLogger(), }) ruleManager.Run() diff --git a/rules/recording.go b/rules/recording.go index 26a202445..438f78780 100644 --- a/rules/recording.go +++ b/rules/recording.go @@ -53,32 +53,11 @@ func (rule *RecordingRule) Name() string { } // Eval evaluates the rule and then overrides the metric names and labels accordingly. -func (rule *RecordingRule) Eval(ctx context.Context, ts time.Time, engine *promql.Engine, _ *url.URL) (promql.Vector, error) { - query, err := engine.NewInstantQuery(rule.vector.String(), ts) +func (rule *RecordingRule) Eval(ctx context.Context, ts time.Time, query QueryFunc, _ *url.URL) (promql.Vector, error) { + vector, err := query(ctx, rule.vector.String(), ts) if err != nil { return nil, err } - - var ( - result = query.Exec(ctx) - vector promql.Vector - ) - if result.Err != nil { - return nil, err - } - - switch v := result.Value.(type) { - case promql.Vector: - vector = v - case promql.Scalar: - vector = promql.Vector{promql.Sample{ - Point: promql.Point(v), - Metric: labels.Labels{}, - }} - default: - return nil, fmt.Errorf("rule result is not a vector or scalar") - } - // Override the metric name and labels. for i := range vector { sample := &vector[i] diff --git a/rules/recording_test.go b/rules/recording_test.go index 0826767ca..cc997d9b2 100644 --- a/rules/recording_test.go +++ b/rules/recording_test.go @@ -62,7 +62,7 @@ func TestRuleEval(t *testing.T) { for _, test := range suite { rule := NewRecordingRule(test.name, test.expr, test.labels) - result, err := rule.Eval(ctx, now, engine, nil) + result, err := rule.Eval(ctx, now, EngineQueryFunc(engine), nil) testutil.Ok(t, err) testutil.Equals(t, result, test.result) } diff --git a/template/template.go b/template/template.go index b8a87f1a0..71bc486e1 100644 --- a/template/template.go +++ b/template/template.go @@ -30,7 +30,6 @@ import ( "github.com/prometheus/common/model" - "github.com/prometheus/prometheus/pkg/labels" "github.com/prometheus/prometheus/promql" "github.com/prometheus/prometheus/util/strutil" ) @@ -59,34 +58,14 @@ func (q queryResultByLabelSorter) Swap(i, j int) { q.results[i], q.results[j] = q.results[j], q.results[i] } -func query(ctx context.Context, q string, ts time.Time, queryEngine *promql.Engine) (queryResult, error) { - query, err := queryEngine.NewInstantQuery(q, ts) +// QueryFunc executes a PromQL query at the given time. +type QueryFunc func(context.Context, string, time.Time) (promql.Vector, error) + +func query(ctx context.Context, q string, ts time.Time, queryFn QueryFunc) (queryResult, error) { + vector, err := queryFn(ctx, q, ts) if err != nil { return nil, err } - res := query.Exec(ctx) - if res.Err != nil { - return nil, res.Err - } - var vector promql.Vector - - switch v := res.Value.(type) { - case promql.Matrix: - return nil, errors.New("matrix return values not supported") - case promql.Vector: - vector = v - case promql.Scalar: - vector = promql.Vector{promql.Sample{ - Point: promql.Point(v), - }} - case promql.String: - vector = promql.Vector{promql.Sample{ - Metric: labels.FromStrings("__value__", v.V), - Point: promql.Point{T: v.T}, - }} - default: - panic("template.query: unhandled result value type") - } // promql.Vector is hard to work with in templates, so convert to // base data types. @@ -111,14 +90,22 @@ type Expander struct { } // NewTemplateExpander returns a template expander ready to use. -func NewTemplateExpander(ctx context.Context, text string, name string, data interface{}, timestamp model.Time, queryEngine *promql.Engine, externalURL *url.URL) *Expander { +func NewTemplateExpander( + ctx context.Context, + text string, + name string, + data interface{}, + timestamp model.Time, + queryFunc QueryFunc, + externalURL *url.URL, +) *Expander { return &Expander{ text: text, name: name, data: data, funcMap: text_template.FuncMap{ "query": func(q string) (queryResult, error) { - return query(ctx, q, timestamp.Time(), queryEngine) + return query(ctx, q, timestamp.Time(), queryFunc) }, "first": func(v queryResult) (*sample, error) { if len(v) > 0 { diff --git a/template/template_test.go b/template/template_test.go index 686d820f6..6a2b81d1a 100644 --- a/template/template_test.go +++ b/template/template_test.go @@ -18,21 +18,19 @@ import ( "math" "net/url" "testing" - - "github.com/prometheus/common/model" - "github.com/stretchr/testify/require" + "time" "github.com/prometheus/prometheus/pkg/labels" "github.com/prometheus/prometheus/promql" - "github.com/prometheus/prometheus/util/testutil" ) type testTemplatesScenario struct { - text string - output string - input interface{} - shouldFail bool - html bool + text string + output string + input interface{} + queryResult promql.Vector + shouldFail bool + html bool } func TestTemplateExpansion(t *testing.T) { @@ -70,42 +68,72 @@ func TestTemplateExpansion(t *testing.T) { output: "1 2", }, { - text: "{{ query \"1.5\" | first | value }}", - output: "1.5", - }, - { - // Get value from scalar query. - text: "{{ query \"scalar(count(metric))\" | first | value }}", - output: "2", + text: "{{ query \"1.5\" | first | value }}", + output: "1.5", + queryResult: promql.Vector{{Point: promql.Point{T: 0, V: 1.5}}}, }, { // Get value from query. - text: "{{ query \"metric{instance='a'}\" | first | value }}", + text: "{{ query \"metric{instance='a'}\" | first | value }}", + queryResult: promql.Vector{ + { + Metric: labels.FromStrings(labels.MetricName, "metric", "instance", "a"), + Point: promql.Point{T: 0, V: 11}, + }}, output: "11", }, { // Get label from query. - text: "{{ query \"metric{instance='a'}\" | first | label \"instance\" }}", + text: "{{ query \"metric{instance='a'}\" | first | label \"instance\" }}", + + queryResult: promql.Vector{ + { + Metric: labels.FromStrings(labels.MetricName, "metric", "instance", "a"), + Point: promql.Point{T: 0, V: 11}, + }}, output: "a", }, { // Missing label is empty when using label function. - text: "{{ query \"metric{instance='a'}\" | first | label \"foo\" }}", + text: "{{ query \"metric{instance='a'}\" | first | label \"foo\" }}", + queryResult: promql.Vector{ + { + Metric: labels.FromStrings(labels.MetricName, "metric", "instance", "a"), + Point: promql.Point{T: 0, V: 11}, + }}, output: "", }, { // Missing label is empty when not using label function. - text: "{{ $x := query \"metric\" | first }}{{ $x.Labels.foo }}", + text: "{{ $x := query \"metric\" | first }}{{ $x.Labels.foo }}", + queryResult: promql.Vector{ + { + Metric: labels.FromStrings(labels.MetricName, "metric", "instance", "a"), + Point: promql.Point{T: 0, V: 11}, + }}, output: "", }, { - text: "{{ $x := query \"metric\" | first }}{{ $x.Labels.foo }}", + text: "{{ $x := query \"metric\" | first }}{{ $x.Labels.foo }}", + queryResult: promql.Vector{ + { + Metric: labels.FromStrings(labels.MetricName, "metric", "instance", "a"), + Point: promql.Point{T: 0, V: 11}, + }}, output: "", html: true, }, { // Range over query and sort by label. - text: "{{ range query \"metric\" | sortByLabel \"instance\" }}{{.Labels.instance}}:{{.Value}}: {{end}}", + text: "{{ range query \"metric\" | sortByLabel \"instance\" }}{{.Labels.instance}}:{{.Value}}: {{end}}", + queryResult: promql.Vector{ + { + Metric: labels.FromStrings(labels.MetricName, "metric", "instance", "a"), + Point: promql.Point{T: 0, V: 11}, + }, { + Metric: labels.FromStrings(labels.MetricName, "metric", "instance", "b"), + Point: promql.Point{T: 0, V: 21}, + }}, output: "a:11: b:21: ", }, { @@ -115,13 +143,15 @@ func TestTemplateExpansion(t *testing.T) { }, { // Error in function. - text: "{{ query \"missing\" | first }}", - shouldFail: true, + text: "{{ query \"missing\" | first }}", + queryResult: promql.Vector{}, + shouldFail: true, }, { // Panic. - text: "{{ (query \"missing\").banana }}", - shouldFail: true, + text: "{{ (query \"missing\").banana }}", + queryResult: promql.Vector{}, + shouldFail: true, }, { // Regex replacement. @@ -211,36 +241,18 @@ func TestTemplateExpansion(t *testing.T) { }, } - time := model.Time(0) - - storage := testutil.NewStorage(t) - defer storage.Close() - - app, err := storage.Appender() - if err != nil { - t.Fatalf("get appender: %s", err) - } - - _, err = app.Add(labels.FromStrings(labels.MetricName, "metric", "instance", "a"), 0, 11) - require.NoError(t, err) - _, err = app.Add(labels.FromStrings(labels.MetricName, "metric", "instance", "b"), 0, 21) - require.NoError(t, err) - - if err := app.Commit(); err != nil { - t.Fatalf("commit samples: %s", err) - } - - engine := promql.NewEngine(storage, nil) - extURL, err := url.Parse("http://testhost:9090/path/prefix") if err != nil { panic(err) } for i, s := range scenarios { + queryFunc := func(_ context.Context, _ string, _ time.Time) (promql.Vector, error) { + return s.queryResult, nil + } var result string var err error - expander := NewTemplateExpander(context.Background(), s.text, "test", s.input, time, engine, extURL) + expander := NewTemplateExpander(context.Background(), s.text, "test", s.input, 0, queryFunc, extURL) if s.html { result, err = expander.ExpandHTML(nil) } else { diff --git a/web/web.go b/web/web.go index 1ded9650e..3ad90dd69 100644 --- a/web/web.go +++ b/web/web.go @@ -527,7 +527,15 @@ func (h *Handler) consoles(w http.ResponseWriter, r *http.Request) { Path: strings.TrimLeft(name, "/"), } - tmpl := template.NewTemplateExpander(h.context, string(text), "__console_"+name, data, h.now(), h.queryEngine, h.options.ExternalURL) + tmpl := template.NewTemplateExpander( + h.context, + string(text), + "__console_"+name, + data, + h.now(), + template.QueryFunc(rules.EngineQueryFunc(h.queryEngine)), + h.options.ExternalURL, + ) filenames, err := filepath.Glob(h.options.ConsoleLibrariesPath + "/*.lib") if err != nil { http.Error(w, err.Error(), http.StatusInternalServerError) @@ -732,7 +740,15 @@ func (h *Handler) executeTemplate(w http.ResponseWriter, name string, data inter http.Error(w, err.Error(), http.StatusInternalServerError) } - tmpl := template.NewTemplateExpander(h.context, text, name, data, h.now(), h.queryEngine, h.options.ExternalURL) + tmpl := template.NewTemplateExpander( + h.context, + text, + name, + data, + h.now(), + template.QueryFunc(rules.EngineQueryFunc(h.queryEngine)), + h.options.ExternalURL, + ) tmpl.Funcs(tmplFuncs(h.consolesPath(), h.options)) result, err := tmpl.ExpandHTML(nil)