Alerting: Fix handling of special floating-point cases when writing observed values to annotations (#61074)

* Fix json serialization of state values

* Simplify two of the tests

* Fix linter complaint

* Don't return error if we fail to look up dashboard, just log it and move on

* Address linter complaint

pkg/services/ngalert/state/historian/annotation.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"math"
 	"sort"
 	"strings"
 	"time"
@@ -45,7 +46,7 @@ func NewAnnotationBackend(annotations annotations.Repository, dashboards dashboa
 func (h *AnnotationBackend) RecordStatesAsync(ctx context.Context, rule history_model.RuleMeta, states []state.StateTransition) <-chan error {
 	logger := h.log.FromContext(ctx)
 	// Build annotations before starting goroutine, to make sure all data is copied and won't mutate underneath us.
-	annotations := h.buildAnnotations(rule, states, logger)
+	annotations := buildAnnotations(rule, states, logger)
 	panel := parsePanelKey(rule, logger)
 	errCh := make(chan error, 1)
 	go func() {
@@ -138,7 +139,7 @@ func (h *AnnotationBackend) QueryStates(ctx context.Context, query ngmodels.Hist
 	return frame, nil
 }
 
-func (h *AnnotationBackend) buildAnnotations(rule history_model.RuleMeta, states []state.StateTransition, logger log.Logger) []annotations.Item {
+func buildAnnotations(rule history_model.RuleMeta, states []state.StateTransition, logger log.Logger) []annotations.Item {
 	items := make([]annotations.Item, 0, len(states))
 	for _, state := range states {
 		if !shouldRecord(state) {
@@ -168,7 +169,7 @@ func (h *AnnotationBackend) recordAnnotationsSync(ctx context.Context, panel *pa
 		dashID, err := h.dashboards.getID(ctx, panel.orgID, panel.dashUID)
 		if err != nil {
 			logger.Error("Error getting dashboard for alert annotation", "dashboardUID", panel.dashUID, "error", err)
-			return fmt.Errorf("error getting dashboard for alert annotation: %w", err)
+			dashID = 0
 		}
 
 		for i := range annotations {
@@ -212,10 +213,27 @@ func buildAnnotationTextAndData(rule history_model.RuleMeta, currentState *state
 		for _, k := range keys {
 			values = append(values, fmt.Sprintf("%s=%f", k, currentState.Values[k]))
 		}
-		jsonData.Set("values", simplejson.NewFromAny(currentState.Values))
+		jsonData.Set("values", jsonifyValues(currentState.Values))
 		value = strings.Join(values, ", ")
 	}
 
 	labels := removePrivateLabels(currentState.Labels)
 	return fmt.Sprintf("%s {%s} - %s", rule.Title, labels.String(), value), jsonData
 }
+
+func jsonifyValues(vs map[string]float64) *simplejson.Json {
+	if vs == nil {
+		return nil
+	}
+
+	j := simplejson.New()
+	for k, v := range vs {
+		switch {
+		case math.IsInf(v, 0), math.IsNaN(v):
+			j.Set(k, fmt.Sprintf("%f", v))
+		default:
+			j.Set(k, v)
+		}
+	}
+	return j
+}

pkg/services/ngalert/state/historian/annotation_test.go

@@ -2,6 +2,8 @@ package historian
 
 import (
 	"context"
+	"encoding/json"
+	"math"
 	"testing"
 	"time"
 
@@ -12,7 +14,10 @@ import (
 	"github.com/grafana/grafana/pkg/services/annotations"
 	"github.com/grafana/grafana/pkg/services/annotations/annotationstest"
 	"github.com/grafana/grafana/pkg/services/dashboards"
+	"github.com/grafana/grafana/pkg/services/ngalert/eval"
 	"github.com/grafana/grafana/pkg/services/ngalert/models"
+	"github.com/grafana/grafana/pkg/services/ngalert/state"
+	history_model "github.com/grafana/grafana/pkg/services/ngalert/state/historian/model"
 	"github.com/grafana/grafana/pkg/services/ngalert/tests/fakes"
 )
 
@@ -64,8 +69,70 @@ func withOrgID(orgId int64) func(rule *models.AlertRule) {
 	}
 }
 
+func TestBuildAnnotations(t *testing.T) {
+	t.Run("data wraps nil values when values are nil", func(t *testing.T) {
+		logger := log.NewNopLogger()
+		rule := history_model.RuleMeta{}
+		states := []state.StateTransition{makeStateTransition()}
+		states[0].State.Values = nil
+
+		items := buildAnnotations(rule, states, logger)
+
+		require.Len(t, items, 1)
+		j := assertValidJSON(t, items[0].Data)
+		require.JSONEq(t, `{"values": null}`, j)
+	})
+
+	t.Run("data approximately contains expected values", func(t *testing.T) {
+		logger := log.NewNopLogger()
+		rule := history_model.RuleMeta{}
+		states := []state.StateTransition{makeStateTransition()}
+		states[0].State.Values = map[string]float64{"a": 1.0, "b": 2.0}
+
+		items := buildAnnotations(rule, states, logger)
+
+		require.Len(t, items, 1)
+		assertValidJSON(t, items[0].Data)
+		// Since we're comparing floats, avoid require.JSONEq to avoid intermittency caused by floating point rounding.
+		vs := items[0].Data.MustMap()["values"]
+		require.NotNil(t, vs)
+		vals := vs.(*simplejson.Json).MustMap()
+		require.InDelta(t, 1.0, vals["a"], 0.1)
+		require.InDelta(t, 2.0, vals["b"], 0.1)
+	})
+
+	t.Run("data handles special float values", func(t *testing.T) {
+		logger := log.NewNopLogger()
+		rule := history_model.RuleMeta{}
+		states := []state.StateTransition{makeStateTransition()}
+		states[0].State.Values = map[string]float64{"nan": math.NaN(), "inf": math.Inf(1), "ninf": math.Inf(-1)}
+
+		items := buildAnnotations(rule, states, logger)
+
+		require.Len(t, items, 1)
+		j := assertValidJSON(t, items[0].Data)
+		require.JSONEq(t, `{"values": {"nan": "NaN", "inf": "+Inf", "ninf": "-Inf"}}`, j)
+	})
+}
+
+func makeStateTransition() state.StateTransition {
+	return state.StateTransition{
+		State: &state.State{
+			State: eval.Alerting,
+		},
+		PreviousState: eval.Normal,
+	}
+}
+
 func withUID(uid string) func(rule *models.AlertRule) {
 	return func(rule *models.AlertRule) {
 		rule.UID = uid
 	}
 }
+
+func assertValidJSON(t *testing.T, j *simplejson.Json) string {
+	require.NotNil(t, j)
+	ser, err := json.Marshal(j)
+	require.NoError(t, err)
+	return string(ser)
+}