vmalert: correctly return error for RW failures (#3452)

* vmalert: correctly return error for RW failures

By mistake, in 0989649ad0 the error
for remote write failures weren't return to user.
This change fixes it.

Signed-off-by: hagen1778 <roman@victoriametrics.com>

app/vmalert/group.go

@@ -421,20 +421,26 @@ func (e *executor) exec(ctx context.Context, rule Rule, ts time.Time, resolveDur
 		return fmt.Errorf("rule %q: failed to execute: %w", rule, err)
 	}
 
-	errGr := new(utils.ErrGroup)
 	if e.rw != nil {
-		pushToRW := func(tss []prompbmarshal.TimeSeries) {
+		pushToRW := func(tss []prompbmarshal.TimeSeries) error {
+			var lastErr error
 			for _, ts := range tss {
 				remoteWriteTotal.Inc()
 				if err := e.rw.Push(ts); err != nil {
 					remoteWriteErrors.Inc()
-					errGr.Add(fmt.Errorf("rule %q: remote write failure: %w", rule, err))
+					lastErr = fmt.Errorf("rule %q: remote write failure: %w", rule, err)
 				}
 			}
+			return lastErr
 		}
-		pushToRW(tss)
+		if err := pushToRW(tss); err != nil {
+			return err
+		}
+
 		staleSeries := e.getStaleSeries(rule, tss, ts)
-		pushToRW(staleSeries)
+		if err := pushToRW(staleSeries); err != nil {
+			return err
+		}
 	}
 
 	ar, ok := rule.(*AlertingRule)
@@ -448,6 +454,7 @@ func (e *executor) exec(ctx context.Context, rule Rule, ts time.Time, resolveDur
 	}
 
 	wg := sync.WaitGroup{}
+	errGr := new(utils.ErrGroup)
 	for _, nt := range e.notifiers() {
 		wg.Add(1)
 		go func(nt notifier.Notifier) {

app/vmalert/group_test.go

@@ -3,8 +3,6 @@ package main
 import (
 	"context"
 	"fmt"
-	"github.com/VictoriaMetrics/VictoriaMetrics/lib/decimal"
-	"github.com/VictoriaMetrics/VictoriaMetrics/lib/prompbmarshal"
 	"reflect"
 	"sort"
 	"testing"
@@ -12,6 +10,9 @@ import (
 
 	"github.com/VictoriaMetrics/VictoriaMetrics/app/vmalert/config"
 	"github.com/VictoriaMetrics/VictoriaMetrics/app/vmalert/notifier"
+	"github.com/VictoriaMetrics/VictoriaMetrics/app/vmalert/remotewrite"
+	"github.com/VictoriaMetrics/VictoriaMetrics/lib/decimal"
+	"github.com/VictoriaMetrics/VictoriaMetrics/lib/prompbmarshal"
 	"github.com/VictoriaMetrics/VictoriaMetrics/lib/promutils"
 )
 
@@ -452,3 +453,24 @@ func TestFaultyNotifier(t *testing.T) {
 	}
 	t.Fatalf("alive notifier didn't receive notification by %v", deadline)
 }
+
+func TestFaultyRW(t *testing.T) {
+	fq := &fakeQuerier{}
+	fq.add(metricWithValueAndLabels(t, 1, "__name__", "foo", "job", "bar"))
+
+	r := &RecordingRule{
+		Name:  "test",
+		state: newRuleState(),
+		q:     fq,
+	}
+
+	e := &executor{
+		rw:                       &remotewrite.Client{},
+		previouslySentSeriesToRW: make(map[uint64]map[string][]prompbmarshal.Label),
+	}
+
+	err := e.exec(context.Background(), r, time.Now(), 0, 10)
+	if err == nil {
+		t.Fatalf("expected to get an error from faulty RW client, got nil instead")
+	}
+}

docs/CHANGELOG.md

@@ -58,6 +58,7 @@ The following tip changes can be tested by building VictoriaMetrics components f
 
 * BUGFIX: [vmalert](https://docs.victoriametrics.com/vmalert.html): properly pass HTTP headers during the alert state restore procedure. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/3418).
 * BUGFIX: [vmalert](https://docs.victoriametrics.com/vmalert.html): properly specify rule evaluation step during the [replay mode](https://docs.victoriametrics.com/vmalert.html#rules-backfilling). The `step` value was previously overriden by `-datasource.queryStep` command-line flag.
+* BUGFIX: [vmalert](https://docs.victoriametrics.com/vmalert.html): properly return the error message from remote-write failures. Before, error was ignored and only `vmalert_remotewrite_errors_total` was incremented.
 * BUGFIX: [vmui](https://docs.victoriametrics.com/#vmui): properly put multi-line queries in the url, so it could be copy-n-pasted and opened without issues in a new browser tab. Previously the url for multi-line query couldn't be opened. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/3444).
 * BUGFIX: [vmui](https://docs.victoriametrics.com/#vmui): correctly handle `up` and `down` keypresses when editing multi-line queries. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/3445).