Write meaningful logs when remote-write is disabled or is misconfigur…

…ed (#4141)

Signed-off-by: Danny Kopping <danny.kopping@grafana.com>

pkg/ruler/appender.go

@@ -67,14 +67,14 @@ func (a *RemoteWriteAppendable) Appender(ctx context.Context) storage.Appender {
 
 	client, err := NewRemoteWriter(a.cfg, a.userID)
 	if err != nil {
-		level.Error(a.logger).Log("msg", "error creating remote-write client; setting appender as noop", "err", err, "tenant", a.userID)
-		return &NoopAppender{}
+		level.Error(a.logger).Log("msg", "error creating remote-write client; discarding samples", "err", err, "tenant", a.userID)
+		return &DiscardingAppender{err}
 	}
 
 	queue, err := util.NewEvictingQueue(capacity, a.onEvict(a.userID, groupKey))
 	if err != nil {
-		level.Error(a.logger).Log("msg", "queue creation error; setting appender as noop", "err", err, "tenant", a.userID)
-		return &NoopAppender{}
+		level.Error(a.logger).Log("msg", "queue creation error; discarding samples", "err", err, "tenant", a.userID)
+		return &DiscardingAppender{err}
 	}
 
 	appender = &RemoteWriteAppender{
@@ -195,3 +195,19 @@ func retrieveGroupKeyFromContext(ctx context.Context) string {
 
 	return rules.GroupKey(file, name)
 }
+
+// DiscardingAppender is used when remote-write for recording rules is disabled
+type DiscardingAppender struct {
+	reason error
+}
+
+func (a DiscardingAppender) Appender(_ context.Context) storage.Appender             { return a }
+func (a DiscardingAppender) Add(l labels.Labels, t int64, v float64) (uint64, error) { return 0, nil }
+func (a DiscardingAppender) Append(ref uint64, l labels.Labels, t int64, v float64) (uint64, error) {
+	return 0, a.reason
+}
+func (a DiscardingAppender) Commit() error   { return nil }
+func (a DiscardingAppender) Rollback() error { return nil }
+func (a DiscardingAppender) AppendExemplar(ref uint64, l labels.Labels, e exemplar.Exemplar) (uint64, error) {
+	return 0, a.reason
+}

pkg/ruler/compat.go

@@ -30,6 +30,8 @@ import (
 	"github.com/grafana/loki/pkg/logql"
 )
 
+var ErrRemoteWriteDisabled = errors.New("remote-write disabled")
+
 // RulesLimits is the one function we need from limits.Overrides, and
 // is here to limit coupling.
 type RulesLimits interface {
@@ -144,7 +146,7 @@ func MemstoreTenantManager(
 func newAppendable(cfg Config, overrides RulesLimits, logger log.Logger, userID string, metrics *remoteWriteMetrics) storage.Appendable {
 	if !cfg.RemoteWrite.Enabled {
 		level.Info(logger).Log("msg", "remote-write is disabled")
-		return &NoopAppender{}
+		return &DiscardingAppender{ErrRemoteWriteDisabled}
 	}
 
 	return newRemoteWriteAppendable(cfg, overrides, logger, userID, metrics)

pkg/ruler/compat_test.go

@@ -275,7 +275,7 @@ groups:
 	}
 }
 
-// TestNoopAppender tests that a NoopAppender is created when remote-write is disabled
+// TestInvalidRemoteWriteConfig tests that a validation error is raised when config is invalid
 func TestInvalidRemoteWriteConfig(t *testing.T) {
 	// if remote-write is not enabled, validation fails
 	cfg := Config{
@@ -299,8 +299,8 @@ func TestInvalidRemoteWriteConfig(t *testing.T) {
 	require.Error(t, cfg.RemoteWrite.Validate())
 }
 
-// TestNoopAppender tests that a NoopAppender is created when remote-write is disabled
-func TestNoopAppender(t *testing.T) {
+// TestDiscardingAppender tests that a DiscardingAppender is created when remote-write is disabled
+func TestDiscardingAppender(t *testing.T) {
 	cfg := Config{
 		Config: ruler.Config{},
 		RemoteWrite: RemoteWriteConfig{
@@ -311,7 +311,7 @@ func TestNoopAppender(t *testing.T) {
 
 	appendable := newAppendable(cfg, &validation.Overrides{}, log.NewNopLogger(), "fake", metrics)
 	appender := appendable.Appender(context.TODO())
-	require.IsType(t, NoopAppender{}, appender)
+	require.Equal(t, DiscardingAppender{ErrRemoteWriteDisabled}, appender)
 }
 
 // TestNonMetricQuery tests that only metric queries can be executed in the query function,

pkg/ruler/memstore.go

@@ -13,7 +13,6 @@ import (
 	"github.com/prometheus/client_golang/prometheus"
 	"github.com/prometheus/client_golang/prometheus/promauto"
 	"github.com/prometheus/common/model"
-	"github.com/prometheus/prometheus/pkg/exemplar"
 	"github.com/prometheus/prometheus/pkg/labels"
 	"github.com/prometheus/prometheus/promql"
 	"github.com/prometheus/prometheus/rules"
@@ -26,19 +25,6 @@ const (
 	statusFailure           = "failure"
 )
 
-type NoopAppender struct{}
-
-func (a NoopAppender) Appender(_ context.Context) storage.Appender             { return a }
-func (a NoopAppender) Add(l labels.Labels, t int64, v float64) (uint64, error) { return 0, nil }
-func (a NoopAppender) Append(ref uint64, l labels.Labels, t int64, v float64) (uint64, error) {
-	return 0, errors.New("unimplemented")
-}
-func (a NoopAppender) Commit() error   { return nil }
-func (a NoopAppender) Rollback() error { return nil }
-func (a NoopAppender) AppendExemplar(ref uint64, l labels.Labels, e exemplar.Exemplar) (uint64, error) {
-	return 0, errors.New("unimplemented")
-}
-
 func ForStateMetric(base labels.Labels, alertName string) labels.Labels {
 	b := labels.NewBuilder(base)
 	b.Set(labels.MetricName, AlertForStateMetricName)