Alerting: Accurately set value for prom-compatible APIs (#47216)

* Alerting: Accurately set value for prom-compatible APIs

Sets the value fields for the prometheus compatible API based on a combination of condition `refID` and the values extracted from the different frames.

* Fix an extra test

* Ensure a consitent ordering

* Address review comments

* address review comments

pkg/services/ngalert/api/api_prometheus.go

@@ -5,6 +5,7 @@ import (
 	"errors"
 	"fmt"
 	"net/http"
+	"sort"
 	"strconv"
 	"strings"
 	"time"
@@ -47,8 +48,9 @@ func (srv PrometheusSrv) RouteGetAlertStatuses(c *models.ReqContext) response.Re
 	for _, alertState := range srv.manager.GetAll(c.OrgId) {
 		startsAt := alertState.StartsAt
 		valString := ""
-		if alertState.State == eval.Alerting {
-			valString = alertState.LastEvaluationString
+
+		if alertState.State == eval.Alerting || alertState.State == eval.Pending {
+			valString = formatValues(alertState)
 		}
 
 		alertResponse.Data.Alerts = append(alertResponse.Data.Alerts, &apimodels.Alert{
@@ -63,6 +65,34 @@ func (srv PrometheusSrv) RouteGetAlertStatuses(c *models.ReqContext) response.Re
 	return response.JSON(http.StatusOK, alertResponse)
 }
 
+func formatValues(alertState *state.State) string {
+	var fv string
+	values := alertState.GetLastEvaluationValuesForCondition()
+
+	switch len(values) {
+	case 0:
+		fv = alertState.LastEvaluationString
+	case 1:
+		for _, v := range values {
+			fv = strconv.FormatFloat(v, 'e', -1, 64)
+			break
+		}
+
+	default:
+		vs := make([]string, 0, len(values))
+
+		for k, v := range values {
+			vs = append(vs, fmt.Sprintf("%s: %s", k, strconv.FormatFloat(v, 'e', -1, 64)))
+		}
+
+		// Ensure we have a consistent natural ordering after formatting e.g. A0, A1, A10, A11, A3, etc.
+		sort.Strings(vs)
+		fv = strings.Join(vs, ", ")
+	}
+
+	return fv
+}
+
 func getPanelIDFromRequest(r *http.Request) (int64, error) {
 	if s := strings.TrimSpace(r.URL.Query().Get("panel_id")); s != "" {
 		return strconv.ParseInt(s, 10, 64)
@@ -178,8 +208,8 @@ func (srv PrometheusSrv) RouteGetRuleStatuses(c *models.ReqContext) response.Res
 		for _, alertState := range srv.manager.GetStatesForRuleUID(c.OrgId, rule.UID) {
 			activeAt := alertState.StartsAt
 			valString := ""
-			if alertState.State == eval.Alerting {
-				valString = alertState.LastEvaluationString
+			if alertState.State == eval.Alerting || alertState.State == eval.Pending {
+				valString = formatValues(alertState)
 			}
 
 			alert := &apimodels.Alert{
@@ -187,7 +217,7 @@ func (srv PrometheusSrv) RouteGetRuleStatuses(c *models.ReqContext) response.Res
 				Annotations: alertState.Annotations,
 				State:       alertState.State.String(),
 				ActiveAt:    &activeAt,
-				Value:       valString, // TODO: set this once it is added to the evaluation results
+				Value:       valString,
 			}
 
 			if alertState.LastEvaluationTime.After(newRule.LastEvaluation) {

pkg/services/ngalert/api/api_prometheus_test.go

@@ -10,6 +10,7 @@ import (
 
 	"github.com/grafana/grafana/pkg/infra/log"
 	"github.com/grafana/grafana/pkg/models"
+	"github.com/grafana/grafana/pkg/services/ngalert/eval"
 	ngmodels "github.com/grafana/grafana/pkg/services/ngalert/models"
 	"github.com/grafana/grafana/pkg/services/ngalert/state"
 	"github.com/grafana/grafana/pkg/services/ngalert/store"
@@ -20,6 +21,64 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
+func Test_FormatValues(t *testing.T) {
+	val1 := 1.1
+	val2 := 1.4
+
+	tc := []struct {
+		name       string
+		alertState *state.State
+		expected   string
+	}{
+		{
+			name: "with no value, it renders the evaluation string",
+			alertState: &state.State{
+				LastEvaluationString: "[ var='A' metric='vector(10) + time() % 50' labels={} value=1.1 ]",
+				Results: []state.Evaluation{
+					{Condition: "A", Values: map[string]*float64{}},
+				},
+			},
+			expected: "[ var='A' metric='vector(10) + time() % 50' labels={} value=1.1 ]",
+		},
+		{
+			name: "with one value, it renders the single value",
+			alertState: &state.State{
+				LastEvaluationString: "[ var='A' metric='vector(10) + time() % 50' labels={} value=1.1 ]",
+				Results: []state.Evaluation{
+					{Condition: "A", Values: map[string]*float64{"A": &val1}},
+				},
+			},
+			expected: "1.1e+00",
+		},
+		{
+			name: "with two values, it renders the value based on their refID and position",
+			alertState: &state.State{
+				LastEvaluationString: "[ var='B0' metric='vector(10) + time() % 50' labels={} value=1.1 ], [ var='B1' metric='vector(10) + time() % 50' labels={} value=1.4 ]",
+				Results: []state.Evaluation{
+					{Condition: "B", Values: map[string]*float64{"B0": &val1, "B1": &val2}},
+				},
+			},
+			expected: "B0: 1.1e+00, B1: 1.4e+00",
+		},
+		{
+			name: "with a high number of values, it renders the value based on their refID and position using a natural order",
+			alertState: &state.State{
+				LastEvaluationString: "[ var='B0' metric='vector(10) + time() % 50' labels={} value=1.1 ], [ var='B1' metric='vector(10) + time() % 50' labels={} value=1.4 ]",
+				Results: []state.Evaluation{
+					{Condition: "B", Values: map[string]*float64{"B0": &val1, "B1": &val2, "B2": &val1, "B10": &val2, "B11": &val1}},
+				},
+			},
+			expected: "B0: 1.1e+00, B10: 1.4e+00, B11: 1.1e+00, B1: 1.4e+00, B2: 1.1e+00",
+		},
+	}
+
+	for _, tt := range tc {
+		t.Run(tt.name, func(t *testing.T) {
+			require.Equal(t, tt.expected, formatValues(tt.alertState))
+		})
+	}
+}
+
 func TestRouteGetAlertStatuses(t *testing.T) {
 	orgID := int64(1)
 
@@ -83,6 +142,48 @@ func TestRouteGetAlertStatuses(t *testing.T) {
 }`, string(r.Body()))
 	})
 
+	t.Run("with two firing alerts", func(t *testing.T) {
+		_, fakeAIM, api := setupAPI(t)
+		fakeAIM.GenerateAlertInstances(1, util.GenerateShortUID(), 2, withAlertingState())
+		req, err := http.NewRequest("GET", "/api/v1/alerts", nil)
+		require.NoError(t, err)
+		c := &models.ReqContext{Context: &web.Context{Req: req}, SignedInUser: &models.SignedInUser{OrgId: orgID}}
+
+		r := api.RouteGetAlertStatuses(c)
+		require.Equal(t, http.StatusOK, r.Status())
+		require.JSONEq(t, `
+{
+	"status": "success",
+	"data": {
+		"alerts": [{
+			"labels": {
+				"alertname": "test_title_0",
+				"instance_label": "test",
+				"label": "test"
+			},
+			"annotations": {
+				"annotation": "test"
+			},
+			"state": "Alerting",
+			"activeAt": "0001-01-01T00:00:00Z",
+			"value": "1.1e+00"
+		}, {
+			"labels": {
+				"alertname": "test_title_1",
+				"instance_label": "test",
+				"label": "test"
+			},
+			"annotations": {
+				"annotation": "test"
+			},
+			"state": "Alerting",
+			"activeAt": "0001-01-01T00:00:00Z",
+			"value": "1.1e+00"
+		}]
+	}
+}`, string(r.Body()))
+	})
+
 	t.Run("with the inclusion of internal labels", func(t *testing.T) {
 		_, fakeAIM, api := setupAPI(t)
 		fakeAIM.GenerateAlertInstances(orgID, util.GenerateShortUID(), 2)
@@ -130,6 +231,20 @@ func TestRouteGetAlertStatuses(t *testing.T) {
 	})
 }
 
+func withAlertingState() forEachState {
+	return func(s *state.State) *state.State {
+		s.State = eval.Alerting
+		value := float64(1.1)
+		s.Results = append(s.Results, state.Evaluation{
+			EvaluationState: eval.Alerting,
+			EvaluationTime:  timeNow(),
+			Values:          map[string]*float64{"B": &value},
+			Condition:       "B",
+		})
+		return s
+	}
+}
+
 func TestRouteGetRuleStatuses(t *testing.T) {
 	timeNow = func() time.Time { return time.Date(2022, 3, 10, 14, 0, 0, 0, time.UTC) }
 	orgID := int64(1)

pkg/services/ngalert/eval/eval.go

@@ -12,6 +12,7 @@ import (
 	"time"
 
 	"github.com/grafana/grafana/pkg/components/simplejson"
+	"github.com/grafana/grafana/pkg/expr"
 	"github.com/grafana/grafana/pkg/expr/classic"
 	"github.com/grafana/grafana/pkg/infra/log"
 	m "github.com/grafana/grafana/pkg/models"
@@ -22,8 +23,6 @@ import (
 
 	"github.com/grafana/grafana-plugin-sdk-go/backend"
 	"github.com/grafana/grafana-plugin-sdk-go/data"
-
-	"github.com/grafana/grafana/pkg/expr"
 )
 
 //go:generate mockery --name Evaluator --structname FakeEvaluator --inpackage --filename evaluator_mock.go --with-expecter

pkg/services/ngalert/eval/extract_md.go

@@ -20,7 +20,6 @@ func extractEvalString(frame *data.Frame) (s string) {
 	if evalMatches, ok := frame.Meta.Custom.([]classic.EvalMatch); ok {
 		sb := strings.Builder{}
 
-		// TODO: Should we simplify when we only have one match and use the name notation of $labels.A?
 		for i, m := range evalMatches {
 			sb.WriteString("[ ")
 			sb.WriteString(fmt.Sprintf("var='%s%v' ", frame.RefID, i))

pkg/services/ngalert/state/manager.go

@@ -171,6 +171,7 @@ func (st *Manager) setNextState(ctx context.Context, alertRule *ngModels.AlertRu
 		EvaluationTime:  result.EvaluatedAt,
 		EvaluationState: result.State,
 		Values:          NewEvaluationValues(result.Values),
+		Condition:       alertRule.Condition,
 	})
 	currentState.LastEvaluationString = result.EvaluationString
 	currentState.TrimResults(alertRule)

pkg/services/ngalert/state/manager_test.go

@@ -1719,6 +1719,7 @@ func TestStaleResultsHandler(t *testing.T) {
 							EvaluationTime:  evaluationTime.Add(3 * time.Minute),
 							EvaluationState: eval.Normal,
 							Values:          make(map[string]*float64),
+							Condition:       "A",
 						},
 					},
 					LastEvaluationTime: evaluationTime.Add(3 * time.Minute),

pkg/services/ngalert/state/state.go

@@ -2,6 +2,7 @@ package state
 
 import (
 	"errors"
+	"strings"
 	"time"
 
 	"github.com/grafana/grafana-plugin-sdk-go/data"
@@ -36,6 +37,8 @@ type Evaluation struct {
 	// Classic conditions can have different values for the same RefID as they can include multiple conditions.
 	// For these, we use the index of the condition in addition RefID as the key e.g. "A0, A1, A2, etc.".
 	Values map[string]*float64
+	// Condition is the refID specified as the condition in the alerting rule at the time of the evaluation.
+	Condition string
 }
 
 // NewEvaluationValues returns the labels and values for each RefID in the capture.
@@ -203,3 +206,20 @@ func (a *State) GetLabels(opts ...ngModels.LabelOption) map[string]string {
 
 	return labels
 }
+
+func (a *State) GetLastEvaluationValuesForCondition() map[string]float64 {
+	if len(a.Results) <= 0 {
+		return nil
+	}
+
+	lastResult := a.Results[len(a.Results)-1]
+	r := make(map[string]float64, len(lastResult.Values))
+
+	for refID, value := range lastResult.Values {
+		if strings.Contains(refID, lastResult.Condition) {
+			r[refID] = *value
+		}
+	}
+
+	return r
+}