Skip to content

Commit

Permalink
Don't log errors that cause OOMs, using interface
Browse files Browse the repository at this point in the history
Using weaveworks/common#299 which is a more
flexible approach.

Note the check disappeared from `logging.go`, because it was a mistake
to check that error.  It comes from `io.Writer`, it won't be an app-
level error.
  • Loading branch information
bboreham committed Jul 28, 2023
1 parent 12792f6 commit 1b8913d
Show file tree
Hide file tree
Showing 9 changed files with 23 additions and 26 deletions.
5 changes: 1 addition & 4 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ require (
github.com/spf13/afero v1.9.5
github.com/stretchr/testify v1.8.4
github.com/uber/jaeger-client-go v2.30.0+incompatible
github.com/weaveworks/common v0.0.0-20230714173453-d1f8877b91ce
github.com/weaveworks/common v0.0.0-20230728070032-dd9e68f319d5
go.uber.org/atomic v1.11.0
go.uber.org/goleak v1.2.1
golang.org/x/crypto v0.11.0
Expand Down Expand Up @@ -274,6 +274,3 @@ replace github.com/open-telemetry/opentelemetry-collector-contrib/pkg/translator

// Replace github.com/open-telemetry/opentelemetry-collector-contrib/pkg/translator/prometheus with a fork until https://github.com/open-telemetry/opentelemetry-collector-contrib/pull/24288 is merged.
replace github.com/open-telemetry/opentelemetry-collector-contrib/pkg/translator/prometheus => github.com/open-telemetry/opentelemetry-collector-contrib/pkg/translator/prometheus v0.0.0-20230718193318-2bc9904b7ae8

// Use a fork of weaveworks/common while we work out if there is a better design for https://github.com/weaveworks/common/pull/293
replace github.com/weaveworks/common => github.com/weaveworks/common v0.0.0-20230714173453-d1f8877b91ce
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -1264,8 +1264,8 @@ github.com/uber/jaeger-lib v2.2.0+incompatible/go.mod h1:ComeNDZlWwrWnDv8aPp0Ba6
github.com/uber/jaeger-lib v2.4.1+incompatible h1:td4jdvLcExb4cBISKIpHuGoVXh+dVKhn2Um6rjCsSsg=
github.com/uber/jaeger-lib v2.4.1+incompatible/go.mod h1:ComeNDZlWwrWnDv8aPp0Ba6+uUTzImX/AauajbLI56U=
github.com/vultr/govultr/v2 v2.17.2 h1:gej/rwr91Puc/tgh+j33p/BLR16UrIPnSr+AIwYWZQs=
github.com/weaveworks/common v0.0.0-20230714173453-d1f8877b91ce h1:8Gh0psRT42deLI53RVwLq2idJtRtDbdjhdX5LqnVznU=
github.com/weaveworks/common v0.0.0-20230714173453-d1f8877b91ce/go.mod h1:rgbeLfJUtEr+G74cwFPR1k/4N0kDeaeSv/qhUNE4hm8=
github.com/weaveworks/common v0.0.0-20230728070032-dd9e68f319d5 h1:nORobjToZAvi54wcuUXLq+XG2Rsr0XEizy5aHBHvqWQ=
github.com/weaveworks/common v0.0.0-20230728070032-dd9e68f319d5/go.mod h1:rgbeLfJUtEr+G74cwFPR1k/4N0kDeaeSv/qhUNE4hm8=
github.com/weaveworks/promrus v1.2.0 h1:jOLf6pe6/vss4qGHjXmGz4oDJQA+AOCqEL3FvvZGz7M=
github.com/weaveworks/promrus v1.2.0/go.mod h1:SaE82+OJ91yqjrE1rsvBWVzNZKcHYFtMUyS1+Ogs/KA=
github.com/xdg-go/pbkdf2 v1.0.0/go.mod h1:jrpuAogTd400dnrH08LKmI/xc1MbPOebTwRqcT5RDeI=
Expand Down
4 changes: 2 additions & 2 deletions pkg/distributor/distributor.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ import (
"github.com/prometheus/prometheus/scrape"
"github.com/weaveworks/common/httpgrpc"
"github.com/weaveworks/common/instrument"
"github.com/weaveworks/common/middleware"
"github.com/weaveworks/common/mtime"
"github.com/weaveworks/common/user"
"go.uber.org/atomic"
Expand All @@ -45,6 +44,7 @@ import (
"github.com/grafana/mimir/pkg/mimirpb"
"github.com/grafana/mimir/pkg/util"
"github.com/grafana/mimir/pkg/util/globalerror"
util_log "github.com/grafana/mimir/pkg/util/log"
util_math "github.com/grafana/mimir/pkg/util/math"
"github.com/grafana/mimir/pkg/util/pool"
"github.com/grafana/mimir/pkg/util/push"
Expand Down Expand Up @@ -1021,7 +1021,7 @@ func (d *Distributor) limitsMiddleware(next push.Func) push.Func {
il := d.getInstanceLimits()
if il.MaxInflightPushRequests > 0 && inflight > int64(il.MaxInflightPushRequests) {
d.rejectedRequests.WithLabelValues(reasonDistributorMaxInflightPushRequests).Inc()
return nil, middleware.DoNotLogError{Err: errMaxInflightRequestsReached}
return nil, util_log.DoNotLogError{Err: errMaxInflightRequestsReached}
}

if il.MaxIngestionRate > 0 {
Expand Down
3 changes: 1 addition & 2 deletions pkg/ingester/ingester.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ import (
"github.com/prometheus/prometheus/util/zeropool"
"github.com/thanos-io/objstore"
"github.com/weaveworks/common/httpgrpc"
"github.com/weaveworks/common/middleware"
"go.uber.org/atomic"
"golang.org/x/exp/slices"
"golang.org/x/sync/errgroup"
Expand Down Expand Up @@ -715,7 +714,7 @@ func (i *Ingester) PushWithCleanup(ctx context.Context, pushReq *push.Request) (
defer pushReq.CleanUp()

if err := i.checkRunning(); err != nil {
return nil, middleware.DoNotLogError{Err: err}
return nil, util_log.DoNotLogError{Err: err}
}

// We will report *this* request in the error too.
Expand Down
4 changes: 2 additions & 2 deletions pkg/ingester/instance_limits.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@ import (
"flag"

"github.com/pkg/errors"
"github.com/weaveworks/common/middleware"
"gopkg.in/yaml.v3"

"github.com/grafana/mimir/pkg/util/globalerror"
util_log "github.com/grafana/mimir/pkg/util/log"
)

const (
Expand All @@ -27,7 +27,7 @@ var (
errMaxIngestionRateReached = errors.New(globalerror.IngesterMaxIngestionRate.MessageWithPerInstanceLimitConfig("the write request has been rejected because the ingester exceeded the samples ingestion rate limit", maxIngestionRateFlag))
errMaxTenantsReached = errors.New(globalerror.IngesterMaxTenants.MessageWithPerInstanceLimitConfig("the write request has been rejected because the ingester exceeded the allowed number of tenants", maxInMemoryTenantsFlag))
errMaxInMemorySeriesReached = errors.New(globalerror.IngesterMaxInMemorySeries.MessageWithPerInstanceLimitConfig("the write request has been rejected because the ingester exceeded the allowed number of in-memory series", maxInMemorySeriesFlag))
errMaxInflightRequestsReached = middleware.DoNotLogError{Err: errors.New(globalerror.IngesterMaxInflightPushRequests.MessageWithPerInstanceLimitConfig("the write request has been rejected because the ingester exceeded the allowed number of inflight push requests", maxInflightPushRequestsFlag))}
errMaxInflightRequestsReached = util_log.DoNotLogError{Err: errors.New(globalerror.IngesterMaxInflightPushRequests.MessageWithPerInstanceLimitConfig("the write request has been rejected because the ingester exceeded the allowed number of inflight push requests", maxInflightPushRequestsFlag))}
)

// InstanceLimits describes limits used by ingester. Reaching any of these will result in Push method to return
Expand Down
7 changes: 7 additions & 0 deletions pkg/util/log/log.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
package log

import (
"context"
"fmt"
"io"
"os"
Expand Down Expand Up @@ -105,3 +106,9 @@ func Flush() error {

return nil
}

type DoNotLogError struct{ Err error }

func (i DoNotLogError) Error() string { return i.Err.Error() }
func (i DoNotLogError) Unwrap() error { return i.Err }
func (i DoNotLogError) ShouldLog(_ context.Context, _ time.Duration) bool { return false }

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

11 changes: 0 additions & 11 deletions vendor/github.com/weaveworks/common/middleware/logging.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 1 addition & 2 deletions vendor/modules.txt

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 1b8913d

Please sign in to comment.