Repository: VictoriaMetrics/VictoriaMetrics
Pull request #871 (2020-10-29 19:01:19)

vmalert: skip automatically added labels on alerts restore
Label `alertgroup` was introduced in #611 and automatically added to generated
time series. By mistake, this new label wasn't correctly purged on restore event
and affected alert's ID uniqueness. This commit removes `alertgroup` label
in restore function.

https://github.com/VictoriaMetrics/VictoriaMetrics/issues/870

codecov[bot]: # [Codecov](https://codecov.io/gh/VictoriaMetrics/VictoriaMetrics/pull/871?src=pr&el=h1) Report
> Merging [#871](https://codecov.io/gh/VictoriaMetrics/VictoriaMetrics/pull/871?src=pr&el=desc) into [master](https://codecov.io/gh/VictoriaMetrics/VictoriaMetrics/commit/de3fe22815b78bbb62f0fb8f2bb163c1d009b264?el=desc) will **decrease** coverage by `0.01%`.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/VictoriaMetrics/VictoriaMetrics/pull/871/graphs/tree.svg?width=650&height=150&src=pr&token=YND8dYVcXn)](https://codecov.io/gh/VictoriaMetrics/VictoriaMetrics/pull/871?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master     #871      +/-   ##
==========================================
- Coverage   62.93%   62.91%   -0.02%     
==========================================
  Files         189      189              
  Lines       21534    21536       +2     
==========================================
- Hits        13552    13549       -3     
- Misses       6700     6704       +4     
- Partials     1282     1283       +1     
```


| [Impacted Files](https://codecov.io/gh/VictoriaMetrics/VictoriaMetrics/pull/871?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [app/vmalert/alerting.go](https://codecov.io/gh/VictoriaMetrics/VictoriaMetrics/pull/871/diff?src=pr&el=tree#diff-YXBwL3ZtYWxlcnQvYWxlcnRpbmcuZ28=) | `75.75% <100.00%> (+0.24%)` | :arrow_up: |
| [app/vmselect/promql/aggr\_incremental.go](https://codecov.io/gh/VictoriaMetrics/VictoriaMetrics/pull/871/diff?src=pr&el=tree#diff-YXBwL3Ztc2VsZWN0L3Byb21xbC9hZ2dyX2luY3JlbWVudGFsLmdv) | `84.88% <0.00%> (-3.11%)` | :arrow_down: |
| [lib/storage/partition.go](https://codecov.io/gh/VictoriaMetrics/VictoriaMetrics/pull/871/diff?src=pr&el=tree#diff-bGliL3N0b3JhZ2UvcGFydGl0aW9uLmdv) | `68.38% <0.00%> (-0.26%)` | :arrow_down: |
| [app/vmalert/remotewrite/remotewrite.go](https://codecov.io/gh/VictoriaMetrics/VictoriaMetrics/pull/871/diff?src=pr&el=tree#diff-YXBwL3ZtYWxlcnQvcmVtb3Rld3JpdGUvcmVtb3Rld3JpdGUuZ28=) | `58.49% <0.00%> (+1.88%)` | :arrow_up: |
| [lib/storage/merge.go](https://codecov.io/gh/VictoriaMetrics/VictoriaMetrics/pull/871/diff?src=pr&el=tree#diff-bGliL3N0b3JhZ2UvbWVyZ2UuZ28=) | `77.77% <0.00%> (+2.56%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/VictoriaMetrics/VictoriaMetrics/pull/871?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/VictoriaMetrics/VictoriaMetrics/pull/871?src=pr&el=footer). Last update [de3fe22...15d9bd0](https://codecov.io/gh/VictoriaMetrics/VictoriaMetrics/pull/871?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/app/vmalert/alerting.go b/app/vmalert/alerting.go
index 7e11ad850..51eee637f 100644
--- a/app/vmalert/alerting.go
+++ b/app/vmalert/alerting.go
@@ -403,7 +403,7 @@ func (ar *AlertingRule) Restore(ctx context.Context, q datasource.Querier, lookb
 		labelsFilter += fmt.Sprintf(",%s=%q", k, v)
 	}
 
-	// Get the last datapoint in range via MetricsQL `last_over_time`.
+	// Get the last data point in range via MetricsQL `last_over_time`.
 	// We don't use plain PromQL since Prometheus doesn't support
 	// remote write protocol which is used for state persistence in vmalert.
 	expr := fmt.Sprintf("last_over_time(%s{alertname=%q%s}[%ds])",
@@ -417,11 +417,14 @@ func (ar *AlertingRule) Restore(ctx context.Context, q datasource.Querier, lookb
 		labels := m.Labels
 		m.Labels = make([]datasource.Label, 0)
 		// drop all extra labels, so hash key will
-		// be identical to timeseries received in Exec
+		// be identical to time series received in Exec
 		for _, l := range labels {
 			if l.Name == alertNameLabel {
 				continue
 			}
+			if l.Name == alertGroupNameLabel {
+				continue
+			}
 			// drop all overridden labels
 			if _, ok := ar.Labels[l.Name]; ok {
 				continue
@@ -436,7 +439,7 @@ func (ar *AlertingRule) Restore(ctx context.Context, q datasource.Querier, lookb
 		a.ID = hash(m)
 		a.State = notifier.StatePending
 		ar.alerts[a.ID] = a
-		logger.Infof("alert %q(%d) restored to state at %v", a.Name, a.ID, a.Start)
+		logger.Infof("alert %q (%d) restored to state at %v", a.Name, a.ID, a.Start)
 	}
 	return nil
 }


Test patch:
diff --git a/app/vmalert/alerting_test.go b/app/vmalert/alerting_test.go
index cace27e6d..55aa2b48d 100644
--- a/app/vmalert/alerting_test.go
+++ b/app/vmalert/alerting_test.go
@@ -355,6 +355,7 @@ func TestAlertingRule_Restore(t *testing.T) {
 				metricWithValueAndLabels(t, float64(time.Now().Truncate(time.Hour).Unix()),
 					"__name__", alertForStateMetricName,
 					alertNameLabel, "",
+					alertGroupNameLabel, "groupID",
 					"foo", "bar",
 					"namespace", "baz",
 				),