From d5ba847d9210e020c03501325d9483d3b10bd048 Mon Sep 17 00:00:00 2001 From: "grafana-delivery-bot[bot]" <132647405+grafana-delivery-bot[bot]@users.noreply.github.com> Date: Mon, 24 Jul 2023 15:44:17 +0200 Subject: [PATCH] [v9.5.x] Alerting: Improve performance of matching captures (#71998) * Alerting: Improve performance of matching captures (#71828) This commit updates eval.go to improve the performance of matching captures in the general case. In some cases we have reduced the runtime of the function from 10s of minutes to a couple 100ms. In the case where no capture matches the exact labels, we revert to the current subset/superset match, but with a reduced search space due to grouping captures. (cherry picked from commit 8dd3eb856de497b53b6065c533a181908286b6f1) * Add label fingerprints from grafana-plugin-sdk-go * Remove unsafe.StringData as we use Go 1.19 * Fix lint --------- Co-authored-by: George Robinson --- pkg/services/ngalert/eval/eval.go | 40 +++++++++---- pkg/services/ngalert/eval/eval_bench_test.go | 59 +++++++++++++++++++ pkg/services/ngalert/eval/fingerprint.go | 40 +++++++++++++ pkg/services/ngalert/eval/fingerprint_test.go | 54 +++++++++++++++++ 4 files changed, 183 insertions(+), 10 deletions(-) create mode 100644 pkg/services/ngalert/eval/eval_bench_test.go create mode 100644 pkg/services/ngalert/eval/fingerprint.go create mode 100644 pkg/services/ngalert/eval/fingerprint_test.go diff --git a/pkg/services/ngalert/eval/eval.go b/pkg/services/ngalert/eval/eval.go index 3a0f2313158..e5b391aa4e6 100644 --- a/pkg/services/ngalert/eval/eval.go +++ b/pkg/services/ngalert/eval/eval.go @@ -300,14 +300,20 @@ type NumberValueCapture struct { } func queryDataResponseToExecutionResults(c models.Condition, execResp *backend.QueryDataResponse) ExecutionResults { - // eval captures for the '__value_string__' annotation and the Value property of the API response. - captures := make([]NumberValueCapture, 0, len(execResp.Responses)) - captureVal := func(refID string, labels data.Labels, value *float64) { - captures = append(captures, NumberValueCapture{ + // captures contains the values of all instant queries and expressions for each dimension + captures := make(map[string]map[fingerprint]NumberValueCapture) + captureFn := func(refID string, labels data.Labels, value *float64) { + m := captures[refID] + if m == nil { + m = make(map[fingerprint]NumberValueCapture) + } + fp := fingerprintLabels(labels) + m[fp] = NumberValueCapture{ Var: refID, Value: value, Labels: labels.Copy(), - }) + } + captures[refID] = m } // datasourceUIDsForRefIDs is a short-lived lookup table of RefID to DatasourceUID @@ -355,7 +361,7 @@ func queryDataResponseToExecutionResults(c models.Condition, execResp *backend.Q if frame.Fields[0].Len() == 1 { v = frame.At(0, 0).(*float64) // type checked above } - captureVal(frame.RefID, frame.Fields[0].Labels, v) + captureFn(frame.RefID, frame.Fields[0].Labels, v) } if refID == c.Condition { @@ -377,13 +383,27 @@ func queryDataResponseToExecutionResults(c models.Condition, execResp *backend.Q if len(frame.Fields) == 1 { theseLabels := frame.Fields[0].Labels - for _, cap := range captures { - // matching labels are equal labels, or when one set of labels includes the labels of the other. - if theseLabels.Equals(cap.Labels) || theseLabels.Contains(cap.Labels) || cap.Labels.Contains(theseLabels) { + fp := fingerprintLabels(theseLabels) + + for _, fps := range captures { + // First look for a capture whose labels are an exact match + if v, ok := fps[fp]; ok { if frame.Meta.Custom == nil { frame.Meta.Custom = []NumberValueCapture{} } - frame.Meta.Custom = append(frame.Meta.Custom.([]NumberValueCapture), cap) + frame.Meta.Custom = append(frame.Meta.Custom.([]NumberValueCapture), v) + } else { + // If no exact match was found, look for captures whose labels are either subsets + // or supersets + for _, v := range fps { + // matching labels are equal labels, or when one set of labels includes the labels of the other. + if theseLabels.Equals(v.Labels) || theseLabels.Contains(v.Labels) || v.Labels.Contains(theseLabels) { + if frame.Meta.Custom == nil { + frame.Meta.Custom = []NumberValueCapture{} + } + frame.Meta.Custom = append(frame.Meta.Custom.([]NumberValueCapture), v) + } + } } } } diff --git a/pkg/services/ngalert/eval/eval_bench_test.go b/pkg/services/ngalert/eval/eval_bench_test.go new file mode 100644 index 00000000000..600120cbe96 --- /dev/null +++ b/pkg/services/ngalert/eval/eval_bench_test.go @@ -0,0 +1,59 @@ +package eval + +import ( + "context" + "strconv" + "testing" + "time" + + "github.com/grafana/grafana-plugin-sdk-go/backend" + "github.com/grafana/grafana-plugin-sdk-go/data" + "github.com/grafana/grafana/pkg/expr" + "github.com/grafana/grafana/pkg/services/ngalert/models" + "github.com/grafana/grafana/pkg/util" +) + +func BenchmarkEvaluate(b *testing.B) { + var dataResp backend.QueryDataResponse + seedDataResponse(&dataResp, 10000) + var evaluator ConditionEvaluator = &conditionEvaluator{ + expressionService: &fakeExpressionService{ + hook: func(ctx context.Context, now time.Time, pipeline expr.DataPipeline) (*backend.QueryDataResponse, error) { + return &dataResp, nil + }, + }, + condition: models.Condition{ + Condition: "B", + }, + } + for i := 0; i < b.N; i++ { + _, err := evaluator.Evaluate(context.Background(), time.Now()) + if err != nil { + b.Fatalf("Unexpected error: %s", err) + } + } +} + +func seedDataResponse(r *backend.QueryDataResponse, n int) { + resps := make(backend.Responses, n) + for i := 0; i < n; i++ { + labels := data.Labels{ + "foo": strconv.Itoa(i), + "bar": strconv.Itoa(i + 1), + } + a, b := resps["A"], resps["B"] + a.Frames = append(a.Frames, &data.Frame{ + Fields: data.Fields{ + data.NewField("Time", labels, []time.Time{time.Now()}), + data.NewField("Value", labels, []*float64{util.Pointer(1.0)}), + }, + }) + b.Frames = append(b.Frames, &data.Frame{ + Fields: data.Fields{ + data.NewField("Value", labels, []*float64{util.Pointer(1.0)}), + }, + }) + resps["A"], resps["B"] = a, b + } + r.Responses = resps +} diff --git a/pkg/services/ngalert/eval/fingerprint.go b/pkg/services/ngalert/eval/fingerprint.go new file mode 100644 index 00000000000..468427e9f39 --- /dev/null +++ b/pkg/services/ngalert/eval/fingerprint.go @@ -0,0 +1,40 @@ +// This code is copied from https://github.com/grafana/grafana-plugin-sdk-go +package eval + +import ( + "fmt" + "hash/fnv" + "sort" + + "github.com/grafana/grafana-plugin-sdk-go/data" +) + +type fingerprint uint64 + +func (f fingerprint) String() string { + return fmt.Sprintf("%016x", uint64(f)) +} + +// Fingerprint calculates a 64-bit FNV-1 hash of the labels. Labels are sorted by key to make sure the hash is stable. +func fingerprintLabels(l data.Labels) fingerprint { + h := fnv.New64() + if len(l) == 0 { + return fingerprint(h.Sum64()) + } + // maps do not guarantee predictable sequence of keys. + // Therefore, to make hash stable, we need to sort keys + keys := make([]string, 0, len(l)) + for labelName := range l { + keys = append(keys, labelName) + } + sort.Strings(keys) + for _, name := range keys { + _, _ = h.Write([]byte(name)) + // ignore errors returned by Write method because fnv never returns them. + _, _ = h.Write([]byte{255}) // use an invalid utf-8 sequence as separator + value := l[name] + _, _ = h.Write([]byte(value)) + _, _ = h.Write([]byte{255}) + } + return fingerprint(h.Sum64()) +} diff --git a/pkg/services/ngalert/eval/fingerprint_test.go b/pkg/services/ngalert/eval/fingerprint_test.go new file mode 100644 index 00000000000..acf40a3ae24 --- /dev/null +++ b/pkg/services/ngalert/eval/fingerprint_test.go @@ -0,0 +1,54 @@ +package eval + +import ( + "testing" + + "github.com/grafana/grafana-plugin-sdk-go/data" + "github.com/stretchr/testify/require" +) + +func TestLabelsFingerprint(t *testing.T) { + testCases := []struct { + name string + labels data.Labels + fingerprint fingerprint + }{ + { + name: "should work if nil", + labels: nil, + fingerprint: fingerprint(0xcbf29ce484222325), + }, + { + name: "should work if empty", + labels: make(data.Labels), + fingerprint: fingerprint(0xcbf29ce484222325), + }, + { + name: "should calculate hash", + labels: data.Labels{"a": "AAA", "b": "BBB", "c": "CCC", "d": "DDD"}, + fingerprint: fingerprint(0xfb4532f90d896635), + }, + } + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + require.Equal(t, testCase.fingerprint, fingerprintLabels(testCase.labels)) + }) + } +} + +func TestLabelsFingerprintString(t *testing.T) { + testCases := []struct { + name string + fingerprint fingerprint + expected string + }{ + {"simple", fingerprint(0x1234567890abcdef), "1234567890abcdef"}, + {"zero", fingerprint(0), "0000000000000000"}, + {"max", fingerprint(0xffffffffffffffff), "ffffffffffffffff"}, + } + for _, testCase := range testCases { + t.Run(testCase.name, func(t *testing.T) { + require.Equal(t, testCase.expected, testCase.fingerprint.String()) + }) + } +}