diff --git a/ee/query-service/app/server.go b/ee/query-service/app/server.go index 54eb7bd1e5..7468be4698 100644 --- a/ee/query-service/app/server.go +++ b/ee/query-service/app/server.go @@ -757,7 +757,7 @@ func makeRulesManager( RepoURL: ruleRepoURL, DBConn: db, Context: context.Background(), - Logger: nil, + Logger: zap.L(), DisableRules: disableRules, FeatureFlags: fm, Reader: ch, diff --git a/ee/query-service/rules/anomaly.go b/ee/query-service/rules/anomaly.go index a04bfc2840..08ff3afcda 100644 --- a/ee/query-service/rules/anomaly.go +++ b/ee/query-service/rules/anomaly.go @@ -250,7 +250,7 @@ func (r *AnomalyRule) Eval(ctx context.Context, ts time.Time) (interface{}, erro } lb := labels.NewBuilder(smpl.Metric).Del(labels.MetricNameLabel).Del(labels.TemporalityLabel) - resultLabels := labels.NewBuilder(smpl.MetricOrig).Del(labels.MetricNameLabel).Del(labels.TemporalityLabel).Labels() + resultLabels := labels.NewBuilder(smpl.Metric).Del(labels.MetricNameLabel).Del(labels.TemporalityLabel).Labels() for name, value := range r.Labels().Map() { lb.Set(name, expand(value)) @@ -262,7 +262,7 @@ func (r *AnomalyRule) Eval(ctx context.Context, ts time.Time) (interface{}, erro annotations := make(labels.Labels, 0, len(r.Annotations().Map())) for name, value := range r.Annotations().Map() { - annotations = append(annotations, labels.Label{Name: common.NormalizeLabelName(name), Value: expand(value)}) + annotations = append(annotations, labels.Label{Name: name, Value: expand(value)}) } if smpl.IsMissing { lb.Set(labels.AlertNameLabel, "[No data] "+r.Name()) diff --git a/ee/query-service/rules/manager.go b/ee/query-service/rules/manager.go index 5ed35d4d34..e44bbcf82b 100644 --- a/ee/query-service/rules/manager.go +++ b/ee/query-service/rules/manager.go @@ -73,7 +73,7 @@ func PrepareTaskFunc(opts baserules.PrepareTaskOptions) (baserules.Task, error) task = newTask(baserules.TaskTypeCh, opts.TaskName, time.Duration(opts.Rule.Frequency), rules, opts.ManagerOpts, opts.NotifyFunc, opts.RuleDB) } else { - return nil, fmt.Errorf("unsupported rule type. Supported types: %s, %s", baserules.RuleTypeProm, baserules.RuleTypeThreshold) + return nil, fmt.Errorf("unsupported rule type %s. Supported types: %s, %s", opts.Rule.RuleType, baserules.RuleTypeProm, baserules.RuleTypeThreshold) } return task, nil diff --git a/pkg/query-service/app/server.go b/pkg/query-service/app/server.go index b71df63781..f16597aa31 100644 --- a/pkg/query-service/app/server.go +++ b/pkg/query-service/app/server.go @@ -742,7 +742,7 @@ func makeRulesManager( RepoURL: ruleRepoURL, DBConn: db, Context: context.Background(), - Logger: nil, + Logger: zap.L(), DisableRules: disableRules, FeatureFlags: fm, Reader: ch, diff --git a/pkg/query-service/rules/base_rule.go b/pkg/query-service/rules/base_rule.go index a0b402c5be..b6d2db0a3c 100644 --- a/pkg/query-service/rules/base_rule.go +++ b/pkg/query-service/rules/base_rule.go @@ -8,7 +8,6 @@ import ( "sync" "time" - "go.signoz.io/signoz/pkg/query-service/common" "go.signoz.io/signoz/pkg/query-service/converter" "go.signoz.io/signoz/pkg/query-service/interfaces" "go.signoz.io/signoz/pkg/query-service/model" @@ -339,11 +338,9 @@ func (r *BaseRule) ShouldAlert(series v3.Series) (Sample, bool) { var alertSmpl Sample var shouldAlert bool var lbls qslabels.Labels - var lblsNormalized qslabels.Labels for name, value := range series.Labels { lbls = append(lbls, qslabels.Label{Name: name, Value: value}) - lblsNormalized = append(lblsNormalized, qslabels.Label{Name: common.NormalizeLabelName(name), Value: value}) } series.Points = removeGroupinSetPoints(series) @@ -366,7 +363,7 @@ func (r *BaseRule) ShouldAlert(series v3.Series) (Sample, bool) { if r.compareOp() == ValueIsAbove { for _, smpl := range series.Points { if smpl.Value > r.targetVal() { - alertSmpl = Sample{Point: Point{V: smpl.Value}, Metric: lblsNormalized, MetricOrig: lbls} + alertSmpl = Sample{Point: Point{V: smpl.Value}, Metric: lbls} shouldAlert = true break } @@ -374,7 +371,7 @@ func (r *BaseRule) ShouldAlert(series v3.Series) (Sample, bool) { } else if r.compareOp() == ValueIsBelow { for _, smpl := range series.Points { if smpl.Value < r.targetVal() { - alertSmpl = Sample{Point: Point{V: smpl.Value}, Metric: lblsNormalized, MetricOrig: lbls} + alertSmpl = Sample{Point: Point{V: smpl.Value}, Metric: lbls} shouldAlert = true break } @@ -382,7 +379,7 @@ func (r *BaseRule) ShouldAlert(series v3.Series) (Sample, bool) { } else if r.compareOp() == ValueIsEq { for _, smpl := range series.Points { if smpl.Value == r.targetVal() { - alertSmpl = Sample{Point: Point{V: smpl.Value}, Metric: lblsNormalized, MetricOrig: lbls} + alertSmpl = Sample{Point: Point{V: smpl.Value}, Metric: lbls} shouldAlert = true break } @@ -390,7 +387,7 @@ func (r *BaseRule) ShouldAlert(series v3.Series) (Sample, bool) { } else if r.compareOp() == ValueIsNotEq { for _, smpl := range series.Points { if smpl.Value != r.targetVal() { - alertSmpl = Sample{Point: Point{V: smpl.Value}, Metric: lblsNormalized, MetricOrig: lbls} + alertSmpl = Sample{Point: Point{V: smpl.Value}, Metric: lbls} shouldAlert = true break } @@ -398,7 +395,7 @@ func (r *BaseRule) ShouldAlert(series v3.Series) (Sample, bool) { } else if r.compareOp() == ValueOutsideBounds { for _, smpl := range series.Points { if math.Abs(smpl.Value) >= r.targetVal() { - alertSmpl = Sample{Point: Point{V: smpl.Value}, Metric: lblsNormalized, MetricOrig: lbls} + alertSmpl = Sample{Point: Point{V: smpl.Value}, Metric: lbls} shouldAlert = true break } @@ -407,7 +404,7 @@ func (r *BaseRule) ShouldAlert(series v3.Series) (Sample, bool) { case AllTheTimes: // If all samples match the condition, the rule is firing. shouldAlert = true - alertSmpl = Sample{Point: Point{V: r.targetVal()}, Metric: lblsNormalized, MetricOrig: lbls} + alertSmpl = Sample{Point: Point{V: r.targetVal()}, Metric: lbls} if r.compareOp() == ValueIsAbove { for _, smpl := range series.Points { if smpl.Value <= r.targetVal() { @@ -423,7 +420,7 @@ func (r *BaseRule) ShouldAlert(series v3.Series) (Sample, bool) { minValue = smpl.Value } } - alertSmpl = Sample{Point: Point{V: minValue}, Metric: lblsNormalized, MetricOrig: lbls} + alertSmpl = Sample{Point: Point{V: minValue}, Metric: lbls} } } else if r.compareOp() == ValueIsBelow { for _, smpl := range series.Points { @@ -439,7 +436,7 @@ func (r *BaseRule) ShouldAlert(series v3.Series) (Sample, bool) { maxValue = smpl.Value } } - alertSmpl = Sample{Point: Point{V: maxValue}, Metric: lblsNormalized, MetricOrig: lbls} + alertSmpl = Sample{Point: Point{V: maxValue}, Metric: lbls} } } else if r.compareOp() == ValueIsEq { for _, smpl := range series.Points { @@ -459,7 +456,7 @@ func (r *BaseRule) ShouldAlert(series v3.Series) (Sample, bool) { if shouldAlert { for _, smpl := range series.Points { if !math.IsInf(smpl.Value, 0) && !math.IsNaN(smpl.Value) { - alertSmpl = Sample{Point: Point{V: smpl.Value}, Metric: lblsNormalized, MetricOrig: lbls} + alertSmpl = Sample{Point: Point{V: smpl.Value}, Metric: lbls} break } } @@ -467,7 +464,7 @@ func (r *BaseRule) ShouldAlert(series v3.Series) (Sample, bool) { } else if r.compareOp() == ValueOutsideBounds { for _, smpl := range series.Points { if math.Abs(smpl.Value) >= r.targetVal() { - alertSmpl = Sample{Point: Point{V: smpl.Value}, Metric: lblsNormalized, MetricOrig: lbls} + alertSmpl = Sample{Point: Point{V: smpl.Value}, Metric: lbls} shouldAlert = true break } @@ -484,7 +481,7 @@ func (r *BaseRule) ShouldAlert(series v3.Series) (Sample, bool) { count++ } avg := sum / count - alertSmpl = Sample{Point: Point{V: avg}, Metric: lblsNormalized, MetricOrig: lbls} + alertSmpl = Sample{Point: Point{V: avg}, Metric: lbls} if r.compareOp() == ValueIsAbove { if avg > r.targetVal() { shouldAlert = true @@ -516,7 +513,7 @@ func (r *BaseRule) ShouldAlert(series v3.Series) (Sample, bool) { } sum += smpl.Value } - alertSmpl = Sample{Point: Point{V: sum}, Metric: lblsNormalized, MetricOrig: lbls} + alertSmpl = Sample{Point: Point{V: sum}, Metric: lbls} if r.compareOp() == ValueIsAbove { if sum > r.targetVal() { shouldAlert = true @@ -541,7 +538,7 @@ func (r *BaseRule) ShouldAlert(series v3.Series) (Sample, bool) { case Last: // If the last sample matches the condition, the rule is firing. shouldAlert = false - alertSmpl = Sample{Point: Point{V: series.Points[len(series.Points)-1].Value}, Metric: lblsNormalized, MetricOrig: lbls} + alertSmpl = Sample{Point: Point{V: series.Points[len(series.Points)-1].Value}, Metric: lbls} if r.compareOp() == ValueIsAbove { if series.Points[len(series.Points)-1].Value > r.targetVal() { shouldAlert = true diff --git a/pkg/query-service/rules/manager.go b/pkg/query-service/rules/manager.go index 75b2b5fade..c41d0bbe50 100644 --- a/pkg/query-service/rules/manager.go +++ b/pkg/query-service/rules/manager.go @@ -173,7 +173,7 @@ func defaultPrepareTaskFunc(opts PrepareTaskOptions) (Task, error) { task = newTask(TaskTypeProm, opts.TaskName, taskNamesuffix, time.Duration(opts.Rule.Frequency), rules, opts.ManagerOpts, opts.NotifyFunc, opts.RuleDB) } else { - return nil, fmt.Errorf("unsupported rule type. Supported types: %s, %s", RuleTypeProm, RuleTypeThreshold) + return nil, fmt.Errorf("unsupported rule type %s. Supported types: %s, %s", opts.Rule.RuleType, RuleTypeProm, RuleTypeThreshold) } return task, nil diff --git a/pkg/query-service/rules/prom_rule.go b/pkg/query-service/rules/prom_rule.go index 473fac0d5d..c11525d796 100644 --- a/pkg/query-service/rules/prom_rule.go +++ b/pkg/query-service/rules/prom_rule.go @@ -43,6 +43,7 @@ func NewPromRule( BaseRule: baseRule, pqlEngine: pqlEngine, } + p.logger = logger query, err := p.getPqlQuery() diff --git a/pkg/query-service/rules/result_types.go b/pkg/query-service/rules/result_types.go index 5f39208b01..1f6304c6d5 100644 --- a/pkg/query-service/rules/result_types.go +++ b/pkg/query-service/rules/result_types.go @@ -17,10 +17,6 @@ type Sample struct { Metric labels.Labels - // Label keys as-is from the result query. - // The original labels are used to prepare the related{logs, traces} link in alert notification - MetricOrig labels.Labels - IsMissing bool } diff --git a/pkg/query-service/rules/templates.go b/pkg/query-service/rules/templates.go index 5f29621c68..519ff3859b 100644 --- a/pkg/query-service/rules/templates.go +++ b/pkg/query-service/rules/templates.go @@ -16,6 +16,7 @@ import ( "golang.org/x/text/cases" + "go.signoz.io/signoz/pkg/query-service/common" "go.signoz.io/signoz/pkg/query-service/utils/times" ) @@ -204,17 +205,47 @@ func NewTemplateExpander( // AlertTemplateData returns the interface to be used in expanding the template. func AlertTemplateData(labels map[string]string, value string, threshold string) interface{} { + // This exists here for backwards compatibility. + // The labels map passed in no longer contains the normalized labels. + // To continue supporting the old way of referencing labels, we need to + // add the normalized labels just for the template expander. + // This is done by creating a new map and adding the normalized labels to it. + newLabels := make(map[string]string) + for k, v := range labels { + newLabels[k] = v + newLabels[common.NormalizeLabelName(k)] = v + } + return struct { Labels map[string]string Value string Threshold string }{ - Labels: labels, + Labels: newLabels, Value: value, Threshold: threshold, } } +// preprocessTemplate preprocesses the template to replace our custom $variable syntax with the correct Go template syntax. +// example, $service.name in the template is replaced with {{index $labels "service.name"}} +// While we could use go template functions to do this, we need to keep the syntax +// consistent across the platform. +// If there is a go template block, it won't be replaced. +// The example for existing go template block is: {{$threshold}} or {{$value}} or any other valid go template syntax. +func (te *TemplateExpander) preprocessTemplate() { + re := regexp.MustCompile(`({{.*?}})|(\$(\w+(?:\.\w+)*))`) + te.text = re.ReplaceAllStringFunc(te.text, func(match string) string { + if strings.HasPrefix(match, "{{") { + // If it's a Go template block, leave it unchanged + return match + } + // Otherwise, it's our custom $variable syntax + path := strings.Split(match[1:], ".") + return "{{index $labels \"" + strings.Join(path, ".") + "\"}}" + }) +} + // Funcs adds the functions in fm to the Expander's function map. // Existing functions will be overwritten in case of conflict. func (te TemplateExpander) Funcs(fm text_template.FuncMap) { @@ -237,6 +268,8 @@ func (te TemplateExpander) Expand() (result string, resultErr error) { } }() + te.preprocessTemplate() + tmpl, err := text_template.New(te.name).Funcs(te.funcMap).Option("missingkey=zero").Parse(te.text) if err != nil { return "", fmt.Errorf("error parsing template %v: %v", te.name, err) diff --git a/pkg/query-service/rules/templates_test.go b/pkg/query-service/rules/templates_test.go new file mode 100644 index 0000000000..737a3d20ca --- /dev/null +++ b/pkg/query-service/rules/templates_test.go @@ -0,0 +1,65 @@ +package rules + +import ( + "context" + "testing" + "time" + + "github.com/stretchr/testify/require" + "go.signoz.io/signoz/pkg/query-service/utils/times" +) + +func TestTemplateExpander(t *testing.T) { + defs := "{{$labels := .Labels}}{{$value := .Value}}{{$threshold := .Threshold}}" + data := AlertTemplateData(map[string]string{"service.name": "my-service"}, "100", "200") + expander := NewTemplateExpander(context.Background(), defs+"test $service.name", "test", data, times.Time(time.Now().Unix()), nil) + result, err := expander.Expand() + if err != nil { + t.Fatal(err) + } + require.Equal(t, "test my-service", result) +} + +func TestTemplateExpander_WithThreshold(t *testing.T) { + defs := "{{$labels := .Labels}}{{$value := .Value}}{{$threshold := .Threshold}}" + data := AlertTemplateData(map[string]string{"service.name": "my-service"}, "200", "100") + expander := NewTemplateExpander(context.Background(), defs+"test $service.name exceeds {{$threshold}} and observed at {{$value}}", "test", data, times.Time(time.Now().Unix()), nil) + result, err := expander.Expand() + if err != nil { + t.Fatal(err) + } + require.Equal(t, "test my-service exceeds 100 and observed at 200", result) +} + +func TestTemplateExpanderOldVariableSyntax(t *testing.T) { + defs := "{{$labels := .Labels}}{{$value := .Value}}{{$threshold := .Threshold}}" + data := AlertTemplateData(map[string]string{"service.name": "my-service"}, "200", "100") + expander := NewTemplateExpander(context.Background(), defs+"test {{.Labels.service_name}} exceeds {{$threshold}} and observed at {{$value}}", "test", data, times.Time(time.Now().Unix()), nil) + result, err := expander.Expand() + if err != nil { + t.Fatal(err) + } + require.Equal(t, "test my-service exceeds 100 and observed at 200", result) +} + +func TestTemplateExpander_WithAlreadyNormalizedKey(t *testing.T) { + defs := "{{$labels := .Labels}}{{$value := .Value}}{{$threshold := .Threshold}}" + data := AlertTemplateData(map[string]string{"service_name": "my-service"}, "200", "100") + expander := NewTemplateExpander(context.Background(), defs+"test {{.Labels.service_name}} exceeds {{$threshold}} and observed at {{$value}}", "test", data, times.Time(time.Now().Unix()), nil) + result, err := expander.Expand() + if err != nil { + t.Fatal(err) + } + require.Equal(t, "test my-service exceeds 100 and observed at 200", result) +} + +func TestTemplateExpander_WithMissingKey(t *testing.T) { + defs := "{{$labels := .Labels}}{{$value := .Value}}{{$threshold := .Threshold}}" + data := AlertTemplateData(map[string]string{"service_name": "my-service"}, "200", "100") + expander := NewTemplateExpander(context.Background(), defs+"test {{.Labels.missing_key}} exceeds {{$threshold}} and observed at {{$value}}", "test", data, times.Time(time.Now().Unix()), nil) + result, err := expander.Expand() + if err != nil { + t.Fatal(err) + } + require.Equal(t, "test exceeds 100 and observed at 200", result) +} diff --git a/pkg/query-service/rules/threshold_rule.go b/pkg/query-service/rules/threshold_rule.go index 8453f1a268..72b00e2412 100644 --- a/pkg/query-service/rules/threshold_rule.go +++ b/pkg/query-service/rules/threshold_rule.go @@ -401,7 +401,7 @@ func (r *ThresholdRule) Eval(ctx context.Context, ts time.Time) (interface{}, er } lb := labels.NewBuilder(smpl.Metric).Del(labels.MetricNameLabel).Del(labels.TemporalityLabel) - resultLabels := labels.NewBuilder(smpl.MetricOrig).Del(labels.MetricNameLabel).Del(labels.TemporalityLabel).Labels() + resultLabels := labels.NewBuilder(smpl.Metric).Del(labels.MetricNameLabel).Del(labels.TemporalityLabel).Labels() for name, value := range r.labels.Map() { lb.Set(name, expand(value)) @@ -413,7 +413,7 @@ func (r *ThresholdRule) Eval(ctx context.Context, ts time.Time) (interface{}, er annotations := make(labels.Labels, 0, len(r.annotations.Map())) for name, value := range r.annotations.Map() { - annotations = append(annotations, labels.Label{Name: common.NormalizeLabelName(name), Value: expand(value)}) + annotations = append(annotations, labels.Label{Name: name, Value: expand(value)}) } if smpl.IsMissing { lb.Set(labels.AlertNameLabel, "[No data] "+r.Name()) @@ -423,13 +423,13 @@ func (r *ThresholdRule) Eval(ctx context.Context, ts time.Time) (interface{}, er // is used alert grouping, and we want to group alerts with the same // label set, but different timestamps, together. if r.typ == AlertTypeTraces { - link := r.prepareLinksToTraces(ts, smpl.MetricOrig) + link := r.prepareLinksToTraces(ts, smpl.Metric) if link != "" && r.hostFromSource() != "" { zap.L().Info("adding traces link to annotations", zap.String("link", fmt.Sprintf("%s/traces-explorer?%s", r.hostFromSource(), link))) annotations = append(annotations, labels.Label{Name: "related_traces", Value: fmt.Sprintf("%s/traces-explorer?%s", r.hostFromSource(), link)}) } } else if r.typ == AlertTypeLogs { - link := r.prepareLinksToLogs(ts, smpl.MetricOrig) + link := r.prepareLinksToLogs(ts, smpl.Metric) if link != "" && r.hostFromSource() != "" { zap.L().Info("adding logs link to annotations", zap.String("link", fmt.Sprintf("%s/logs/logs-explorer?%s", r.hostFromSource(), link))) annotations = append(annotations, labels.Label{Name: "related_logs", Value: fmt.Sprintf("%s/logs/logs-explorer?%s", r.hostFromSource(), link)}) diff --git a/pkg/query-service/rules/threshold_rule_test.go b/pkg/query-service/rules/threshold_rule_test.go index e23ba0d05c..d3d84f06a7 100644 --- a/pkg/query-service/rules/threshold_rule_test.go +++ b/pkg/query-service/rules/threshold_rule_test.go @@ -1010,7 +1010,7 @@ func TestThresholdRuleLabelNormalization(t *testing.T) { sample, shoulAlert := rule.ShouldAlert(c.values) for name, value := range c.values.Labels { - assert.Equal(t, value, sample.Metric.Get(common.NormalizeLabelName(name))) + assert.Equal(t, value, sample.Metric.Get(name)) } assert.Equal(t, c.expectAlert, shoulAlert, "Test case %d", idx)