From 11155c702874de25c78fdea99191bc5596b38a3d Mon Sep 17 00:00:00 2001
From: Chris Marchbanks <csmarchbanks@gmail.com>
Date: Wed, 15 Aug 2018 01:52:08 -0600
Subject: [PATCH] Existing alert labels will update based on templates (#4500)

Signed-off-by: Chris Marchbanks <csmarchbanks@gmail.com>
---
 rules/alerting.go      |   5 +-
 rules/alerting_test.go | 105 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 108 insertions(+), 2 deletions(-)

diff --git a/rules/alerting.go b/rules/alerting.go
index 5f8b347b0..fe61583c3 100644
--- a/rules/alerting.go
+++ b/rules/alerting.go
@@ -318,7 +318,8 @@ func (r *AlertingRule) Eval(ctx context.Context, ts time.Time, query QueryFunc,
 			annotations = append(annotations, labels.Label{Name: a.Name, Value: expand(a.Value)})
 		}
 
-		h := smpl.Metric.Hash()
+		lbs := lb.Labels()
+		h := lbs.Hash()
 		resultFPs[h] = struct{}{}
 
 		// Check whether we already have alerting state for the identifying label set.
@@ -330,7 +331,7 @@ func (r *AlertingRule) Eval(ctx context.Context, ts time.Time, query QueryFunc,
 		}
 
 		r.active[h] = &Alert{
-			Labels:      lb.Labels(),
+			Labels:      lbs,
 			Annotations: annotations,
 			ActiveAt:    ts,
 			State:       StatePending,
diff --git a/rules/alerting_test.go b/rules/alerting_test.go
index e3ec1da09..edfb794ab 100644
--- a/rules/alerting_test.go
+++ b/rules/alerting_test.go
@@ -15,8 +15,10 @@ package rules
 
 import (
 	"testing"
+	"time"
 
 	"github.com/prometheus/prometheus/pkg/labels"
+	"github.com/prometheus/prometheus/pkg/timestamp"
 	"github.com/prometheus/prometheus/promql"
 	"github.com/prometheus/prometheus/util/testutil"
 )
@@ -37,3 +39,106 @@ annotations:
 	got := rule.HTMLSnippet("/test/prefix")
 	testutil.Assert(t, want == got, "incorrect HTML snippet; want:\n\n|%v|\n\ngot:\n\n|%v|", want, got)
 }
+
+func TestAlertingRuleLabelsUpdate(t *testing.T) {
+	suite, err := promql.NewTest(t, `
+		load 1m
+			http_requests{job="app-server", instance="0"}	75 85 70 70
+	`)
+	testutil.Ok(t, err)
+	defer suite.Close()
+
+	err = suite.Run()
+	testutil.Ok(t, err)
+
+	expr, err := promql.ParseExpr(`http_requests < 100`)
+	testutil.Ok(t, err)
+
+	rule := NewAlertingRule(
+		"HTTPRequestRateLow",
+		expr,
+		time.Minute,
+		// Basing alerting rule labels off of a value that can change is a very bad idea.
+		// If an alert is going back and forth between two label values it will never fire.
+		// Instead, you should write two alerts with constant labels.
+		labels.FromStrings("severity", "{{ if lt $value 80.0 }}critical{{ else }}warning{{ end }}"),
+		nil, true, nil,
+	)
+
+	results := []promql.Vector{
+		promql.Vector{
+			{
+				Metric: labels.FromStrings(
+					"__name__", "ALERTS",
+					"alertname", "HTTPRequestRateLow",
+					"alertstate", "pending",
+					"instance", "0",
+					"job", "app-server",
+					"severity", "critical",
+				),
+				Point: promql.Point{V: 1},
+			},
+		},
+		promql.Vector{
+			{
+				Metric: labels.FromStrings(
+					"__name__", "ALERTS",
+					"alertname", "HTTPRequestRateLow",
+					"alertstate", "pending",
+					"instance", "0",
+					"job", "app-server",
+					"severity", "warning",
+				),
+				Point: promql.Point{V: 1},
+			},
+		},
+		promql.Vector{
+			{
+				Metric: labels.FromStrings(
+					"__name__", "ALERTS",
+					"alertname", "HTTPRequestRateLow",
+					"alertstate", "pending",
+					"instance", "0",
+					"job", "app-server",
+					"severity", "critical",
+				),
+				Point: promql.Point{V: 1},
+			},
+		},
+		promql.Vector{
+			{
+				Metric: labels.FromStrings(
+					"__name__", "ALERTS",
+					"alertname", "HTTPRequestRateLow",
+					"alertstate", "firing",
+					"instance", "0",
+					"job", "app-server",
+					"severity", "critical",
+				),
+				Point: promql.Point{V: 1},
+			},
+		},
+	}
+
+	baseTime := time.Unix(0, 0)
+	for i, result := range results {
+		t.Logf("case %d", i)
+		evalTime := baseTime.Add(time.Duration(i) * time.Minute)
+		result[0].Point.T = timestamp.FromTime(evalTime)
+		res, err := rule.Eval(suite.Context(), evalTime, EngineQueryFunc(suite.QueryEngine(), suite.Storage()), nil)
+		testutil.Ok(t, err)
+
+		var filteredRes promql.Vector // After removing 'ALERTS_FOR_STATE' samples.
+		for _, smpl := range res {
+			smplName := smpl.Metric.Get("__name__")
+			if smplName == "ALERTS" {
+				filteredRes = append(filteredRes, smpl)
+			} else {
+				// If not 'ALERTS', it has to be 'ALERTS_FOR_STATE'.
+				testutil.Equals(t, smplName, "ALERTS_FOR_STATE")
+			}
+		}
+
+		testutil.Equals(t, result, filteredRes)
+	}
+}