Alerting: Small readability improvements to template.go (#63422)

* Alerting: Small readability improvements to template.go

* Fix lint

pkg/services/ngalert/state/cache.go

@@ -142,7 +142,7 @@ func (rs *ruleStates) expandRuleLabelsAndAnnotations(ctx context.Context, log lo
 	expand := func(original map[string]string) map[string]string {
 		expanded := make(map[string]string, len(original))
 		for k, v := range original {
-			ev, err := template.Expand(ctx, alertRule.Title, v, templateLabels, alertInstance, externalURL)
+			ev, err := template.Expand(ctx, alertRule.Title, v, template.NewData(templateLabels, alertInstance), externalURL, alertInstance.EvaluatedAt)
 			expanded[k] = ev
 			if err != nil {
 				log.Error("Error in expanding template", "name", k, "value", v, "error", err)

pkg/services/ngalert/state/template/template.go

@@ -21,13 +21,11 @@ type Labels map[string]string
 
 // String returns the labels as k=v, comma separated, in increasing order.
 func (l Labels) String() string {
-	// sort the names of the labels in increasing order
 	sorted := make([]string, 0, len(l))
 	for k := range l {
 		sorted = append(sorted, k)
 	}
 	sort.Strings(sorted)
-	// create the string from the sorted labels
 	b := strings.Builder{}
 	for i, k := range sorted {
 		b.WriteString(k)
@@ -51,59 +49,63 @@ func (v Value) String() string {
 	return strconv.FormatFloat(v.Value, 'f', -1, 64)
 }
 
-func NewValues(values map[string]eval.NumberValueCapture) map[string]Value {
-	m := make(map[string]Value)
-	for k, v := range values {
+func NewValues(caps map[string]eval.NumberValueCapture) map[string]Value {
+	values := make(map[string]Value)
+	for refID, cap := range caps {
 		var f float64
-		if v.Value != nil {
-			f = *v.Value
+		// A RefID might be missing a value if there was no data or an error.
+		// If that is the case, use "not a number". We don't use 0, or -1, as
+		// either of those are possible values for a RefID.
+		if cap.Value != nil {
+			f = *cap.Value
 		} else {
 			f = math.NaN()
 		}
-		m[k] = Value{
-			Labels: Labels(v.Labels),
+		values[refID] = Value{
+			Labels: Labels(cap.Labels),
 			Value:  f,
 		}
 	}
-	return m
+	return values
 }
 
-func Expand(
-	ctx context.Context,
-	name, tmpl string,
-	labels map[string]string,
-	res eval.Result,
-	externalURL *url.URL) (string, error) {
-	name = "__alert_" + name
-	tmpl = "{{- $labels := .Labels -}}{{- $values := .Values -}}{{- $value := .Value -}}" + tmpl
-	data := struct {
-		Labels map[string]string
-		Values map[string]Value
-		Value  string
-	}{
+type Data struct {
+	Labels map[string]string
+	Values map[string]Value
+	Value  string
+}
+
+func NewData(labels map[string]string, res eval.Result) Data {
+	return Data{
 		Labels: labels,
 		Values: NewValues(res.Values),
 		Value:  res.EvaluationString,
 	}
+}
 
-	expander := template.NewTemplateExpander(
-		ctx, // This context is only used with the `query()` function - which we don't support yet.
-		tmpl,
-		name,
-		data,
-		model.Time(timestamp.FromTime(res.EvaluatedAt)),
-		func(context.Context, string, time.Time) (promql.Vector, error) {
-			return nil, nil
-		},
-		externalURL,
-		[]string{"missingkey=invalid"},
-	)
+func Expand(ctx context.Context, name, tmpl string, data Data, externalURL *url.URL, evaluatedAt time.Time) (string, error) {
+	// add __alert_ to avoid possible conflicts with other templates
+	name = "__alert_" + name
+	// add variables for the labels and values to the beginning of the template
+	tmpl = "{{- $labels := .Labels -}}{{- $values := .Values -}}{{- $value := .Value -}}" + tmpl
+	// ctx and queryFunc are no-ops as `query()` is not supported in Grafana
+	queryFunc := func(context.Context, string, time.Time) (promql.Vector, error) {
+		return nil, nil
+	}
+	tm := model.Time(timestamp.FromTime(evaluatedAt))
+	// Use missingkey=invalid so missing data shows <no value> instead of the type's default value
+	options := []string{"missingkey=invalid"}
+
+	expander := template.NewTemplateExpander(ctx, tmpl, name, data, tm, queryFunc, externalURL, options)
 	expander.Funcs(defaultFuncs)
 
 	result, err := expander.Expand()
-	// Replace missing key value to one that does not look like an HTML tag. This can cause problems downstream in some notifiers.
-	// For example, Telegram in HTML mode rejects requests with unsupported tags.
-	result = strings.ReplaceAll(result, "<no value>", "[no value]")
+	if err != nil {
+		return "", err
+	}
 
-	return result, err
+	// We need to replace <no value> with [no value] as some integrations think <no value> is invalid HTML. For example,
+	// Telegram in HTML mode rejects messages with unsupported tags.
+	result = strings.ReplaceAll(result, "<no value>", "[no value]")
+	return result, nil
 }

pkg/services/ngalert/state/template/template_test.go

@@ -431,7 +431,7 @@ func TestExpandTemplate(t *testing.T) {
 
 	for _, c := range cases {
 		t.Run(c.name, func(t *testing.T) {
-			v, err := Expand(context.Background(), "test", c.text, c.labels, c.alertInstance, externalURL)
+			v, err := Expand(context.Background(), "test", c.text, NewData(c.labels, c.alertInstance), externalURL, c.alertInstance.EvaluatedAt)
 			if c.expectedError != nil {
 				require.NotNil(t, err)
 				require.EqualError(t, c.expectedError, err.Error())