From c54da8f955f53ddf62c17556dc8caa60f287368b Mon Sep 17 00:00:00 2001 From: Alexander Akhmetov Date: Wed, 26 Mar 2025 10:31:38 +0100 Subject: [PATCH] Alerting: Make $value return the query value in case when a single datasource is used (#102301) What is this feature? This PR changes the behavior of the $value and .Value variables in alerting templating to be more compatible with Prometheus templating. When a single datasource is used in the alerting rule, these variables will now return the numeric value from the query instead of the evaluation string. Why do we need this feature? It makes Grafana templating more compatible with Prometheus templates. In Prometheus, $value returns the numeric value of the query, but in Grafana it's the evaluation string: [ var='A' labels={instance=instance1} value=81.234 ]. This is because in Grafana multiple datasources can be used in the alert rule, and it's not always possible to get a single value. This change makes Grafana's behavior consistent with Prometheus when a single datasource is used, and in case when multiple datasources are used in the query, it keeps the old behaviour. Both $value and .Value are not recommended to use (documentation), and it's better to use .Values instead. --- .../alerting-rules/templates/reference.md | 22 +- pkg/services/ngalert/eval/eval.go | 26 ++- pkg/services/ngalert/eval/eval_test.go | 164 +++++++++++++ .../ngalert/state/template/template.go | 45 +++- .../ngalert/state/template/template_test.go | 217 +++++++++++++++++- 5 files changed, 445 insertions(+), 29 deletions(-) diff --git a/docs/sources/alerting/alerting-rules/templates/reference.md b/docs/sources/alerting/alerting-rules/templates/reference.md index 7c9a0e691a6..d638b8e1d3c 100644 --- a/docs/sources/alerting/alerting-rules/templates/reference.md +++ b/docs/sources/alerting/alerting-rules/templates/reference.md @@ -80,11 +80,11 @@ Templates are based on the **Go templating system**. Refer to [Template language The following variables are available when templating annotations and labels: -| Variables | Description | -| ------------------ | --------------------------------------------------------------------------------------------------------------------------------------------------- | -| [$labels](#labels) | Contains all labels from the query, only query labels. | -| [$values](#values) | Contains the labels and floating point values of all instant queries and expressions, indexed by their Ref IDs. | -| [$value](#value) | A string containing the labels and values of all instant queries; threshold, reduce and math expressions, and classic conditions in the alert rule. | +| Variables | Description | +| ------------------ | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| [$labels](#labels) | Contains all labels from the query, only query labels. | +| [$values](#values) | Contains the labels and floating point values of all instant queries and expressions, indexed by their Ref IDs. | +| [$value](#value) | A string containing the labels and values of all instant queries; threshold, reduce and math expressions, and classic conditions in the alert rule. When a single data source is used, it returns the value of the query. It is generally recommended to use [$values](#values). | ### $labels @@ -145,16 +145,24 @@ Alternatively, you can use the `index()` function to retrieve the query value: The `$value` variable is a string containing the labels and values of all instant queries; threshold, reduce and math expressions, and classic conditions in the alert rule. +When a single data source is used in the alert rule, `$value` will return the query value directly. + This example prints the `$value` variable: ``` {{ $value }}: CPU usage has exceeded 80% for the last 5 minutes. ``` -It would display something like this: +When using multiple data sources, it would display something like this: ``` -[ var='A' labels={instance=instance1} value=81.234 ]: CPU usage has exceeded 80% for the last 5 minutes. +[ var='A' labels={instance=instance1} value=81.234, , [ var='B' labels={instance=instance2} value=1 ] ]: CPU usage has exceeded 80% for the last 5 minutes. +``` + +But with a single data source, it would display just the value of the query: + +``` +81.234: CPU usage has exceeded 80% for the last 5 minutes. ``` Instead, we recommend using [$values](#values), which contains the same information as `$value` but is structured in an easier-to-use table format. diff --git a/pkg/services/ngalert/eval/eval.go b/pkg/services/ngalert/eval/eval.go index afe62877f5a..fec321ddd11 100644 --- a/pkg/services/ngalert/eval/eval.go +++ b/pkg/services/ngalert/eval/eval.go @@ -434,8 +434,9 @@ func getExprRequest(ctx EvaluationContext, condition models.Condition, dsCacheSe } type NumberValueCapture struct { - Var string // RefID - Labels data.Labels + Var string // RefID + IsDatasourceNode bool + Labels data.Labels Value *float64 } @@ -461,16 +462,17 @@ func IsNoData(res backend.DataResponse) bool { func queryDataResponseToExecutionResults(c models.Condition, execResp *backend.QueryDataResponse) ExecutionResults { // captures contains the values of all instant queries and expressions for each dimension captures := make(map[string]map[data.Fingerprint]NumberValueCapture) - captureFn := func(refID string, labels data.Labels, value *float64) { + captureFn := func(refID string, datasourceType expr.NodeType, labels data.Labels, value *float64) { m := captures[refID] if m == nil { m = make(map[data.Fingerprint]NumberValueCapture) } fp := labels.Fingerprint() m[fp] = NumberValueCapture{ - Var: refID, - Value: value, - Labels: labels.Copy(), + Var: refID, + IsDatasourceNode: datasourceType == expr.TypeDatasourceNode, + Value: value, + Labels: labels.Copy(), } captures[refID] = m } @@ -487,14 +489,20 @@ func queryDataResponseToExecutionResults(c models.Condition, execResp *backend.Q result.Error = FindConditionError(execResp, c.Condition) for refID, res := range execResp.Responses { + var datasourceType expr.NodeType + datasourceUID, ok := datasourceUIDsForRefIDs[refID] + if ok { + datasourceType = expr.NodeTypeFromDatasourceUID(datasourceUID) + } + if IsNoData(res) { // To make sure NoData is nil when Results are also nil we wait to initialize // NoData until there is at least one query or expression that returned no data if result.NoData == nil { result.NoData = make(map[string]string) } - if s, ok := datasourceUIDsForRefIDs[refID]; ok && expr.NodeTypeFromDatasourceUID(s) == expr.TypeDatasourceNode { // TODO perhaps extract datasource UID from ML expression too. - result.NoData[refID] = s + if datasourceType == expr.TypeDatasourceNode { // TODO perhaps extract datasource UID from ML expression too. + result.NoData[refID] = datasourceUID } } @@ -508,7 +516,7 @@ func queryDataResponseToExecutionResults(c models.Condition, execResp *backend.Q if frame.Fields[0].Len() == 1 { v = frame.At(0, 0).(*float64) // type checked above } - captureFn(refID, frame.Fields[0].Labels, v) + captureFn(refID, datasourceType, frame.Fields[0].Labels, v) } if refID == c.Condition { diff --git a/pkg/services/ngalert/eval/eval_test.go b/pkg/services/ngalert/eval/eval_test.go index bde445c732c..80da967dbb8 100644 --- a/pkg/services/ngalert/eval/eval_test.go +++ b/pkg/services/ngalert/eval/eval_test.go @@ -732,6 +732,67 @@ func TestCreate_HysteresisCommand(t *testing.T) { } } +func TestQueryDataResponseToExecutionResults(t *testing.T) { + t.Run("should set datasource type for captured values", func(t *testing.T) { + c := models.Condition{ + Condition: "B", + Data: []models.AlertQuery{ + { + RefID: "A", + DatasourceUID: "test-ds", + }, + { + RefID: "B", + DatasourceUID: expr.DatasourceUID, + }, + }, + } + + execResp := &backend.QueryDataResponse{ + Responses: backend.Responses{ + "A": { + Frames: []*data.Frame{ + { + RefID: "A", + Fields: []*data.Field{ + data.NewField( + "Value", + data.Labels{"foo": "bar"}, + []*float64{util.Pointer(10.0)}, + ), + }, + }, + }, + }, + "B": { + Frames: []*data.Frame{ + { + RefID: "B", + Fields: []*data.Field{ + data.NewField( + "Value", + data.Labels{"foo": "bar"}, + []*float64{util.Pointer(1.0)}, + ), + }, + }, + }, + }, + }, + } + + results := queryDataResponseToExecutionResults(c, execResp) + evaluatedResults := evaluateExecutionResult(results, time.Now()) + + require.Len(t, evaluatedResults, 1) + result := evaluatedResults[0] + + // Validate that IsDatasourceNode were correctly set + require.True(t, result.Values["A"].IsDatasourceNode) + require.False(t, result.Values["B"].IsDatasourceNode) + }) +} + func TestEvaluate(t *testing.T) { cases := []struct { name string @@ -1054,6 +1115,109 @@ func TestEvaluate(t *testing.T) { EvaluationString: "[ var='A' labels={foo=bar} value=10 ], [ var='B' labels={foo=bar} value=10 ], [ var='C' labels={foo=bar} value=1 ]", }}, }, + { + name: "range query with reducer includes only reducer and condition values in EvaluationString", + cond: models.Condition{ + Condition: "C", + }, + resp: backend.QueryDataResponse{ + Responses: backend.Responses{ + "A": { + // This simulates a range query data response with time series data + Frames: []*data.Frame{{ + RefID: "A", + Fields: []*data.Field{ + data.NewField( + "Time", + nil, + []time.Time{time.Now(), time.Now().Add(10 * time.Second)}, + ), + data.NewField( + "Value", + data.Labels{"foo": "bar"}, + []*float64{util.Pointer(10.0), util.Pointer(20.0)}, + ), + }, + }}, + }, + "B": { + // Reduce node + Frames: []*data.Frame{{ + RefID: "B", + Fields: []*data.Field{ + data.NewField( + "Value", + data.Labels{"foo": "bar"}, + []*float64{util.Pointer(15.0)}, + ), + }, + Meta: &data.FrameMeta{ + Custom: []NumberValueCapture{ + { + Var: "B", + IsDatasourceNode: false, + Labels: data.Labels{"foo": "bar"}, + Value: util.Pointer(15.0), + }, + }, + }, + }}, + }, + "C": { + // Threshold + Frames: []*data.Frame{{ + RefID: "C", + Fields: []*data.Field{ + data.NewField( + "Value", + data.Labels{"foo": "bar"}, + []*float64{util.Pointer(1.0)}, + ), + }, + Meta: &data.FrameMeta{ + Custom: []NumberValueCapture{ + { + Var: "B", + IsDatasourceNode: false, + Labels: data.Labels{"foo": "bar"}, + Value: util.Pointer(15.0), + }, + { + Var: "C", + IsDatasourceNode: false, + Labels: data.Labels{"foo": "bar"}, + Value: util.Pointer(1.0), + }, + }, + }, + }}, + }, + }, + }, + expected: Results{{ + State: Alerting, + Instance: data.Labels{ + "foo": "bar", + }, + Values: map[string]NumberValueCapture{ + "B": { + Var: "B", + IsDatasourceNode: false, + Labels: data.Labels{"foo": "bar"}, + Value: util.Pointer(15.0), + }, + "C": { + Var: "C", + IsDatasourceNode: false, + Labels: data.Labels{"foo": "bar"}, + Value: util.Pointer(1.0), + }, + }, + // Note the absence of "A" in the EvaluationString. + // For range queries, the raw datasource values are not included + EvaluationString: "[ var='B' labels={foo=bar} value=15 ], [ var='C' labels={foo=bar} value=1 ]", + }}, + }, } for _, tc := range cases { diff --git a/pkg/services/ngalert/state/template/template.go b/pkg/services/ngalert/state/template/template.go index ebd541d527d..1a593f76f67 100644 --- a/pkg/services/ngalert/state/template/template.go +++ b/pkg/services/ngalert/state/template/template.go @@ -42,8 +42,9 @@ func (l Labels) String() string { // Value contains the labels and value of a Reduce, Math or Threshold // expression for a series. type Value struct { - Labels Labels - Value float64 + Labels Labels + Value float64 + isDatasourceNode bool } func (v Value) String() string { @@ -63,8 +64,9 @@ func NewValues(captures map[string]eval.NumberValueCapture) map[string]Value { f = math.NaN() } values[refID] = Value{ - Labels: Labels(capture.Labels), - Value: f, + Labels: Labels(capture.Labels), + Value: f, + isDatasourceNode: capture.IsDatasourceNode, } } return values @@ -73,14 +75,43 @@ func NewValues(captures map[string]eval.NumberValueCapture) map[string]Value { type Data struct { Labels Labels Values map[string]Value - Value string + + // Value is the .Value and $value variables in templates. + // For single datasource queries, this will be the numeric value of the query. + // For multiple datasource queries, this will be the evaluation string. + Value string } func NewData(labels map[string]string, res eval.Result) Data { + values := NewValues(res.Values) + + // By default, use the evaluation string as the Value + valueStr := res.EvaluationString + + // If there's exactly one datasource node, use its value instead + // This makes the $value variable compatible with Prometheus templating + // where $value holds the numeric value of the alert query + datasourceNodeCount := 0 + var datasourceNodeValue Value + for _, v := range values { + if v.isDatasourceNode { + datasourceNodeCount++ + if datasourceNodeCount > 1 { + // Multiple datasource nodes found, we'll use the evaluation string + break + } + datasourceNodeValue = v + } + } + + if datasourceNodeCount == 1 { + valueStr = datasourceNodeValue.String() + } + return Data{ Labels: labels, - Values: NewValues(res.Values), - Value: res.EvaluationString, + Values: values, + Value: valueStr, } } diff --git a/pkg/services/ngalert/state/template/template_test.go b/pkg/services/ngalert/state/template/template_test.go index 11982b45505..780090bd96a 100644 --- a/pkg/services/ngalert/state/template/template_test.go +++ b/pkg/services/ngalert/state/template/template_test.go @@ -71,6 +71,157 @@ func TestExpandError(t *testing.T) { assert.Equal(t, "failed to expand template '{{': unexpected {{", err.Error()) } +func TestNewData(t *testing.T) { + t.Run("uses evaluation string when no datasource nodes", func(t *testing.T) { + res := eval.Result{ + EvaluationString: "[ var='A' labels={instance=foo} value=10 ]", + Values: map[string]eval.NumberValueCapture{ + "A": { + Var: "A", + IsDatasourceNode: false, + Labels: data.Labels{"instance": "foo"}, + Value: util.Pointer(10.0), + }, + }, + } + + data := NewData(map[string]string{}, res) + assert.Equal(t, "[ var='A' labels={instance=foo} value=10 ]", data.Value) + }) + + t.Run("uses single datasource node value when exactly one exists", func(t *testing.T) { + res := eval.Result{ + EvaluationString: "[ var='A' labels={instance=foo} value=10 ]", + Values: map[string]eval.NumberValueCapture{ + "A": { + Var: "A", + IsDatasourceNode: true, + Labels: data.Labels{"instance": "foo"}, + Value: util.Pointer(10.0), + }, + "B": { + Var: "B", + IsDatasourceNode: false, + Labels: data.Labels{"instance": "foo"}, + Value: util.Pointer(20.0), + }, + }, + } + + data := NewData(map[string]string{}, res) + assert.Equal(t, "10", data.Value) + }) + + t.Run("uses evaluation string when multiple datasource nodes exist", func(t *testing.T) { + res := eval.Result{ + EvaluationString: "[ var='A' labels={instance=foo} value=10 ]", + Values: map[string]eval.NumberValueCapture{ + "A": { + Var: "A", + IsDatasourceNode: true, + Labels: data.Labels{"instance": "foo"}, + Value: util.Pointer(10.0), + }, + "B": { + Var: "B", + IsDatasourceNode: true, + Labels: data.Labels{"instance": "bar"}, + Value: util.Pointer(20.0), + }, + }, + } + + data := NewData(map[string]string{}, res) + assert.Equal(t, "[ var='A' labels={instance=foo} value=10 ]", data.Value) + }) +} + +// TestDatasourceValueInTemplating tests the behavior of the $value variable in alert templates. +// $value behavior has been changed to return a numeric value from the datasource query +// when only a single datasource is used in the alerting rule. If more datasources are used, +// $value will return the evaluation string. +// +// This change makes Grafana's templating more compatible with Prometheus templating, +// where $value and .Value return the numeric value of the alert query. +func TestDatasourceValueInTemplating(t *testing.T) { + t.Run("nil datasource value is rendered as NaN", func(t *testing.T) { + res := eval.Result{ + EvaluationString: "[ var='A' labels={instance=foo} value=no data ]", + Values: map[string]eval.NumberValueCapture{ + "A": { + Var: "A", + IsDatasourceNode: true, + Labels: data.Labels{"instance": "foo"}, + Value: nil, // nil value + }, + }, + } + + data := NewData(map[string]string{}, res) + // In Prometheus, a nil value would be rendered as NaN + assert.Equal(t, "NaN", data.Value) + }) + + t.Run("single datasource node uses query value", func(t *testing.T) { + res := eval.Result{ + EvaluationString: "[ var='A' labels={instance=foo} value=10, var='B' labels={instance=foo} value=20, var='C' labels={instance=foo} value=30 ]", + Values: map[string]eval.NumberValueCapture{ + "A": { + Var: "A", + IsDatasourceNode: true, + Labels: data.Labels{"instance": "foo"}, + Value: util.Pointer(10.0), + }, + "B": { + Var: "B", + IsDatasourceNode: false, + Labels: data.Labels{"instance": "foo"}, + Value: util.Pointer(20.0), + }, + "C": { + Var: "C", + IsDatasourceNode: false, + Labels: data.Labels{"instance": "foo"}, + Value: util.Pointer(30.0), + }, + }, + } + + data := NewData(map[string]string{}, res) + assert.Equal(t, "10", data.Value) + }) + + t.Run("multiple datasource nodes uses evaluation string", func(t *testing.T) { + evalStr := "[ var='A' labels={instance=foo} value=10, var='B' labels={instance=foo} value=20, var='C' labels={instance=foo} value=30 ]" + res := eval.Result{ + EvaluationString: evalStr, + Values: map[string]eval.NumberValueCapture{ + "A": { + Var: "A", + IsDatasourceNode: true, + Labels: data.Labels{"instance": "foo"}, + Value: util.Pointer(10.0), + }, + "B": { + Var: "B", + IsDatasourceNode: true, + Labels: data.Labels{"instance": "foo"}, + Value: util.Pointer(20.0), + }, + "C": { + Var: "C", + IsDatasourceNode: false, + Labels: data.Labels{"instance": "foo"}, + Value: util.Pointer(30.0), + }, + }, + } + + data := NewData(map[string]string{}, res) + assert.Equal(t, evalStr, data.Value) + }) +} + func TestExpandTemplate(t *testing.T) { pathPrefix := "/path/prefix" externalURL, err := url.Parse("http://localhost" + pathPrefix) @@ -112,9 +263,10 @@ func TestExpandTemplate(t *testing.T) { alertInstance: eval.Result{ Values: map[string]eval.NumberValueCapture{ "A": { - Var: "A", - Labels: data.Labels{"instance": "foo"}, - Value: util.Pointer(1.1), + Var: "A", + Labels: data.Labels{"instance": "foo"}, + Value: util.Pointer(1.1), + IsDatasourceNode: true, }, }, }, @@ -125,9 +277,10 @@ func TestExpandTemplate(t *testing.T) { alertInstance: eval.Result{ Values: map[string]eval.NumberValueCapture{ "A": { - Var: "A", - Labels: data.Labels{}, - Value: util.Pointer(1.0), + Var: "A", + Labels: data.Labels{}, + Value: util.Pointer(1.0), + IsDatasourceNode: true, }, }, }, @@ -145,6 +298,58 @@ func TestExpandTemplate(t *testing.T) { }, }, expected: "foo has value NaN", + }, { + name: "$value is expanded into a number for a single datasource query", + text: ` + current $value is: {{ $value }} + current .Value is: {{ .Value }} + `, + alertInstance: eval.Result{ + Values: map[string]eval.NumberValueCapture{ + "query": { + Var: "query", + Labels: data.Labels{"instance": "foo"}, + Value: util.Pointer(10.123), + IsDatasourceNode: true, + }, + "math": { + Var: "math", + Labels: data.Labels{"instance": "foo"}, + Value: util.Pointer(10.0), + IsDatasourceNode: false, + }, + }, + EvaluationString: "[ var='query' labels={instance=foo} value=10.123, var='math' labels={instance=foo} value=10 ]", + }, + expected: `current $value is: 10.123 + current .Value is: 10.123 + `, + }, { + name: "$value is expanded into a string for multi-datasource query", + text: ` + current $value is: {{ $value }} + current .Value is: {{ .Value }} + `, + alertInstance: eval.Result{ + Values: map[string]eval.NumberValueCapture{ + "query": { + Var: "query", + Labels: data.Labels{"instance": "foo"}, + Value: util.Pointer(10.123), + IsDatasourceNode: true, + }, + "second-query": { + Var: "second-query", + Labels: data.Labels{"instance": "bar"}, + Value: util.Pointer(20.456), + IsDatasourceNode: true, + }, + }, + EvaluationString: "[ var='query' labels={instance=foo} value=10.123, var='second-query' labels={instance=bar} value=20.456 ]", + }, + expected: `current $value is: [ var='query' labels={instance=foo} value=10.123, var='second-query' labels={instance=bar} value=20.456 ] + current .Value is: [ var='query' labels={instance=foo} value=10.123, var='second-query' labels={instance=bar} value=20.456 ] + `, }, { name: "assert value string is expanded into $value", text: "{{ $value }}",