Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ruler: write meaningful logs when remote-write is disabled or is misconfigured #4141

Merged
merged 1 commit into from
Aug 19, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 20 additions & 4 deletions pkg/ruler/appender.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down Expand Up @@ -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
}
4 changes: 3 additions & 1 deletion pkg/ruler/compat.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
Expand Down
8 changes: 4 additions & 4 deletions pkg/ruler/compat_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand All @@ -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{
Expand All @@ -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,
Expand Down
14 changes: 0 additions & 14 deletions pkg/ruler/memstore.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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)
Expand Down