IntenseWebs/grafana
3
0
Fork 0
mirror of https://github.com/grafana/grafana.git synced 2025-02-25 18:55:37 -06:00
Code Issues Packages Projects Releases Wiki Activity

Alerting: Return errors when expanding templates (#63662)

Browse Source 
This commit changes the state package so that errors encountered while
expanding templates for custom labels and annotations are returned
from the function. This is not used at present, but will be used in the
future as we look at how to offer better feedback to users who don't
have access to logs, for example our customers who use Hosted Grafana.
This commit is contained in:
George Robinson 2023-03-08 12:25:02 +00:00 committed by GitHub
parent 986a1c2a1b
commit 0c8876c3a2
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 145 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

51
pkg/services/ngalert/state/cache.go
View File

@ -6,8 +6,10 @@ import (
"net/url"
"strings"
"sync"
"time"
"github.com/grafana/grafana-plugin-sdk-go/data"
"github.com/hashicorp/go-multierror"
"github.com/grafana/grafana/pkg/infra/log"
"github.com/grafana/grafana/pkg/services/ngalert/eval"
@ -49,7 +51,14 @@ func (c *cache) getOrCreate(ctx context.Context, log log.Logger, alertRule *ngMo
}
func (rs *ruleStates) getOrCreate(ctx context.Context, log log.Logger, alertRule *ngModels.AlertRule, result eval.Result, extraLabels data.Labels, externalURL *url.URL) *State {
ruleLabels, annotations := rs.expandRuleLabelsAndAnnotations(ctx, log, alertRule, result, extraLabels, externalURL)
// Merge both the extra labels and the labels from the evaluation into a common set
// of labels that can be expanded in custom labels and annotations.
templateData := template.NewData(mergeLabels(extraLabels, result.Instance), result)
// For now, do nothing with these errors as they are already logged in expand.
// In the future, we want to show these errors to the user somehow.
labels, _ := expand(ctx, log, alertRule.Title, alertRule.Labels, templateData, externalURL, result.EvaluatedAt)
annotations, _ := expand(ctx, log, alertRule.Title, alertRule.Annotations, templateData, externalURL, result.EvaluatedAt)
values := make(map[string]float64)
for refID, v := range result.Values {
@ -60,12 +69,12 @@ func (rs *ruleStates) getOrCreate(ctx context.Context, log log.Logger, alertRule
}
}
lbs := make(data.Labels, len(extraLabels)+len(ruleLabels)+len(result.Instance))
lbs := make(data.Labels, len(extraLabels)+len(labels)+len(result.Instance))
dupes := make(data.Labels)
for key, val := range extraLabels {
lbs[key] = val
}
for key, val := range ruleLabels {
for key, val := range labels {
ruleVal, ok := lbs[key]
// if duplicate labels exist, reserved label will take precedence
if ok {
@ -135,25 +144,27 @@ func (rs *ruleStates) getOrCreate(ctx context.Context, log log.Logger, alertRule
return newState
}
func (rs *ruleStates) expandRuleLabelsAndAnnotations(ctx context.Context, log log.Logger, alertRule *ngModels.AlertRule, alertInstance eval.Result, extraLabels data.Labels, externalURL *url.URL) (data.Labels, data.Labels) {
// use labels from the result and extra labels to expand the labels and annotations declared by the rule
templateLabels := mergeLabels(extraLabels, alertInstance.Instance)
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, 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)
// Store the original template on error.
expanded[k] = v
}
// expand returns the expanded templates of all annotations or labels for the template data.
// If a template cannot be expanded due to an error in the template the original template is
// maintained and an error is added to the multierror. All errors in the multierror are
// template.ExpandError errors.
func expand(ctx context.Context, log log.Logger, name string, original map[string]string, data template.Data, externalURL *url.URL, evaluatedAt time.Time) (map[string]string, error) {
var (
errs *multierror.Error
expanded = make(map[string]string, len(original))
)
for k, v := range original {
result, err := template.Expand(ctx, name, v, data, externalURL, evaluatedAt)
if err != nil {
log.Error("Error in expanding template", "error", err)
errs = multierror.Append(errs, err)
// keep the original template on error
expanded[k] = v
} else {
expanded[k] = result
}
return expanded
}
return expand(alertRule.Labels), expand(alertRule.Annotations)
return expanded, errs.ErrorOrNil()
}
func (rs *ruleStates) deleteStates(predicate func(s *State) bool) []*State {

92
pkg/services/ngalert/state/cache_test.go
View File

@ -2,20 +2,112 @@ package state
import (
"context"
"errors"
"fmt"
"net/url"
"testing"
"time"
"github.com/grafana/grafana-plugin-sdk-go/data"
"github.com/hashicorp/go-multierror"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/grafana/grafana/pkg/infra/log"
"github.com/grafana/grafana/pkg/services/ngalert/eval"
"github.com/grafana/grafana/pkg/services/ngalert/models"
"github.com/grafana/grafana/pkg/services/ngalert/state/template"
"github.com/grafana/grafana/pkg/util"
)
func Test_expand(t *testing.T) {
ctx := context.Background()
logger := log.NewNopLogger()
// This test asserts that multierror returns a nil error if there are no errors.
// If the expand function forgets to use ErrorOrNil() then the error returned will
// be non-nil even if no errors have been added to the multierror.
t.Run("err is nil if there are no errors", func(t *testing.T) {
result, err := expand(ctx, logger, "test", map[string]string{}, template.Data{}, nil, time.Now())
require.NoError(t, err)
require.Len(t, result, 0)
})
t.Run("original is expanded with template data", func(t *testing.T) {
original := map[string]string{"Summary": `Instance {{ $labels.instance }} has been down for more than 5 minutes`}
expected := map[string]string{"Summary": "Instance host1 has been down for more than 5 minutes"}
data := template.Data{Labels: map[string]string{"instance": "host1"}}
results, err := expand(ctx, logger, "test", original, data, nil, time.Now())
require.NoError(t, err)
require.Equal(t, expected, results)
})
t.Run("original is returned with an error", func(t *testing.T) {
original := map[string]string{
"Summary": `Instance {{ $labels. }} has been down for more than 5 minutes`,
}
data := template.Data{Labels: map[string]string{"instance": "host1"}}
results, err := expand(ctx, logger, "test", original, data, nil, time.Now())
require.NotNil(t, err)
require.Equal(t, original, results)
// err should be an ExpandError that contains the template for the Summary and an error
var expandErr template.ExpandError
require.True(t, errors.As(err, &expandErr))
require.EqualError(t, expandErr, "failed to expand template '{{- $labels := .Labels -}}{{- $values := .Values -}}{{- $value := .Value -}}Instance {{ $labels. }} has been down for more than 5 minutes': error parsing template __alert_test: template: __alert_test:1: unexpected <.> in operand")
})
t.Run("originals are returned with two errors", func(t *testing.T) {
original := map[string]string{
"Summary": `Instance {{ $labels. }} has been down for more than 5 minutes`,
"Description": "The instance has been down for {{ $value minutes, please check the instance is online",
}
data := template.Data{Labels: map[string]string{"instance": "host1"}}
results, err := expand(ctx, logger, "test", original, data, nil, time.Now())
require.NotNil(t, err)
require.Equal(t, original, results)
// TODO: Please update this test in issue https://github.com/grafana/grafana/issues/63686
var multierr *multierror.Error
require.True(t, errors.As(err, &multierr))
require.Equal(t, multierr.Len(), 2)
// assert each error matches the expected error
var expandErr1 template.ExpandError
require.True(t, errors.As(multierr.Errors[0], &expandErr1))
require.EqualError(t, expandErr1, "failed to expand template '{{- $labels := .Labels -}}{{- $values := .Values -}}{{- $value := .Value -}}Instance {{ $labels. }} has been down for more than 5 minutes': error parsing template __alert_test: template: __alert_test:1: unexpected <.> in operand")
var expandErr2 template.ExpandError
require.True(t, errors.As(multierr.Errors[1], &expandErr2))
require.EqualError(t, expandErr2, "failed to expand template '{{- $labels := .Labels -}}{{- $values := .Values -}}{{- $value := .Value -}}The instance has been down for {{ $value minutes, please check the instance is online': error parsing template __alert_test: template: __alert_test:1: function \"minutes\" not defined")
})
t.Run("expanded and original is returned when there is one error", func(t *testing.T) {
original := map[string]string{
"Summary": `Instance {{ $labels.instance }} has been down for more than 5 minutes`,
"Description": "The instance has been down for {{ $value minutes, please check the instance is online",
}
expected := map[string]string{
"Summary": "Instance host1 has been down for more than 5 minutes",
"Description": "The instance has been down for {{ $value minutes, please check the instance is online",
}
data := template.Data{Labels: map[string]string{"instance": "host1"}}
results, err := expand(ctx, logger, "test", original, data, nil, time.Now())
require.NotNil(t, err)
require.Equal(t, expected, results)
// TODO: Please update this test in issue https://github.com/grafana/grafana/issues/63686
var multierr *multierror.Error
require.True(t, errors.As(err, &multierr))
require.Equal(t, multierr.Len(), 1)
// assert each error matches the expected error
var expandErr template.ExpandError
require.True(t, errors.As(err, &expandErr))
require.EqualError(t, expandErr, "failed to expand template '{{- $labels := .Labels -}}{{- $values := .Values -}}{{- $value := .Value -}}The instance has been down for {{ $value minutes, please check the instance is online': error parsing template __alert_test: template: __alert_test:1: function \"minutes\" not defined")
})
}
func Test_getOrCreate(t *testing.T) {
url := &url.URL{
Scheme: "http",

14
pkg/services/ngalert/state/template/template.go
View File

@ -2,6 +2,7 @@ package template
import (
"context"
"fmt"
"math"
"net/url"
"sort"
@ -83,6 +84,17 @@ func NewData(labels map[string]string, res eval.Result) Data {
}
}
// ExpandError is an error containing the template and the error that occurred
// while expanding it.
type ExpandError struct {
Tmpl string
Err error
}
func (e ExpandError) Error() string {
return fmt.Sprintf("failed to expand template '%s': %s", e.Tmpl, e.Err)
}
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
@ -101,7 +113,7 @@ func Expand(ctx context.Context, name, tmpl string, data Data, externalURL *url.
result, err := expander.Expand()
if err != nil {
return "", err
return "", ExpandError{Tmpl: tmpl, Err: err}
}
// We need to replace <no value> with [no value] as some integrations think <no value> is invalid HTML. For example,

13
pkg/services/ngalert/state/template/template_test.go
View File

@ -66,6 +66,11 @@ func TestValueString(t *testing.T) {
}
}
func TestExpandError(t *testing.T) {
err := ExpandError{Tmpl: "{{", Err: errors.New("unexpected {{")}
assert.Equal(t, "failed to expand template '{{': unexpected {{", err.Error())
}
func TestExpandTemplate(t *testing.T) {
pathPrefix := "/path/prefix"
externalURL, err := url.Parse("http://localhost" + pathPrefix)
@ -167,7 +172,7 @@ func TestExpandTemplate(t *testing.T) {
alertInstance: eval.Result{
EvaluationString: "invalid",
},
expectedError: errors.New(`error executing template __alert_test: template: __alert_test:1:79: executing "__alert_test" at <humanize $value>: error calling humanize: strconv.ParseFloat: parsing "invalid": invalid syntax`),
expectedError: errors.New(`failed to expand template '{{- $labels := .Labels -}}{{- $values := .Values -}}{{- $value := .Value -}}{{ humanize $value }}': error executing template __alert_test: template: __alert_test:1:79: executing "__alert_test" at <humanize $value>: error calling humanize: strconv.ParseFloat: parsing "invalid": invalid syntax`),
}, {
name: "humanize1024 float64",
text: "{{ range $key, $val := $values }}{{ humanize1024 .Value }}:{{ end }}",
@ -202,7 +207,7 @@ func TestExpandTemplate(t *testing.T) {
alertInstance: eval.Result{
EvaluationString: "invalid",
},
expectedError: errors.New(`error executing template __alert_test: template: __alert_test:1:79: executing "__alert_test" at <humanize1024 $value>: error calling humanize1024: strconv.ParseFloat: parsing "invalid": invalid syntax`),
expectedError: errors.New(`failed to expand template '{{- $labels := .Labels -}}{{- $values := .Values -}}{{- $value := .Value -}}{{ humanize1024 $value }}': error executing template __alert_test: template: __alert_test:1:79: executing "__alert_test" at <humanize1024 $value>: error calling humanize1024: strconv.ParseFloat: parsing "invalid": invalid syntax`),
}, {
name: "humanizeDuration - seconds - float64",
text: "{{ range $key, $val := $values }}{{ humanizeDuration .Value }}:{{ end }}",
@ -321,7 +326,7 @@ func TestExpandTemplate(t *testing.T) {
alertInstance: eval.Result{
EvaluationString: "invalid",
},
expectedError: errors.New(`error executing template __alert_test: template: __alert_test:1:79: executing "__alert_test" at <humanizeDuration $value>: error calling humanizeDuration: strconv.ParseFloat: parsing "invalid": invalid syntax`),
expectedError: errors.New(`failed to expand template '{{- $labels := .Labels -}}{{- $values := .Values -}}{{- $value := .Value -}}{{ humanizeDuration $value }}': error executing template __alert_test: template: __alert_test:1:79: executing "__alert_test" at <humanizeDuration $value>: error calling humanizeDuration: strconv.ParseFloat: parsing "invalid": invalid syntax`),
}, {
name: "humanizePercentage - float64",
text: "{{ -0.22222 | humanizePercentage }}:{{ 0.0 | humanizePercentage }}:{{ 0.1234567 | humanizePercentage }}:{{ 1.23456 | humanizePercentage }}",
@ -333,7 +338,7 @@ func TestExpandTemplate(t *testing.T) {
}, {
name: "humanizePercentage - string with error",
text: `{{ "invalid" | humanizePercentage }}`,
expectedError: errors.New(`error executing template __alert_test: template: __alert_test:1:91: executing "__alert_test" at <humanizePercentage>: error calling humanizePercentage: strconv.ParseFloat: parsing "invalid": invalid syntax`),
expectedError: errors.New(`failed to expand template '{{- $labels := .Labels -}}{{- $values := .Values -}}{{- $value := .Value -}}{{ "invalid" | humanizePercentage }}': error executing template __alert_test: template: __alert_test:1:91: executing "__alert_test" at <humanizePercentage>: error calling humanizePercentage: strconv.ParseFloat: parsing "invalid": invalid syntax`),
}, {
name: "humanizeTimestamp - float64",
text: "{{ 1435065584.128 | humanizeTimestamp }}",