From 706985ced8551a553a1d0756f67e4533ca070e95 Mon Sep 17 00:00:00 2001 From: Callum Styan Date: Fri, 31 Jul 2020 12:01:19 -0700 Subject: [PATCH 1/6] Make use of spanlogger when functions that generate spans result in an error, this is so we consistently have the error=true tag set. Signed-off-by: Callum Styan --- pkg/chunk/cache/memcached.go | 15 ++++++++------- pkg/chunk/gcp/bigtable_index_client.go | 8 ++++---- pkg/chunk/util/parallel_chunk_fetch.go | 12 ++++++------ pkg/querier/queryrange/query_range.go | 10 +++++----- pkg/querier/queryrange/results_cache.go | 10 +++++----- pkg/testexporter/correctness/simple.go | 7 +++---- pkg/util/spanlogger/spanlogger.go | 2 +- 7 files changed, 32 insertions(+), 32 deletions(-) diff --git a/pkg/chunk/cache/memcached.go b/pkg/chunk/cache/memcached.go index c2101e69168..71c8094e214 100644 --- a/pkg/chunk/cache/memcached.go +++ b/pkg/chunk/cache/memcached.go @@ -11,13 +11,13 @@ import ( "github.com/bradfitz/gomemcache/memcache" "github.com/go-kit/kit/log" "github.com/go-kit/kit/log/level" - opentracing "github.com/opentracing/opentracing-go" otlog "github.com/opentracing/opentracing-go/log" "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promauto" instr "github.com/weaveworks/common/instrument" "github.com/cortexproject/cortex/pkg/util" + "github.com/cortexproject/cortex/pkg/util/spanlogger" ) type observableVecCollector struct { @@ -146,19 +146,20 @@ func (c *Memcached) Fetch(ctx context.Context, keys []string) (found []string, b func (c *Memcached) fetch(ctx context.Context, keys []string) (found []string, bufs [][]byte, missed []string) { var items map[string]*memcache.Item - err := instr.CollectedRequest(ctx, "Memcache.GetMulti", c.requestDuration, memcacheStatusCode, func(innerCtx context.Context) error { - sp := opentracing.SpanFromContext(innerCtx) - sp.LogFields(otlog.Int("keys requested", len(keys))) + method := "Memcache.GetMulti" + err := instr.CollectedRequest(ctx, method, c.requestDuration, memcacheStatusCode, func(innerCtx context.Context) error { + log, _ := spanlogger.New(innerCtx, method) + log.LogFields(otlog.Int("keys requested", len(keys))) var err error items, err = c.memcache.GetMulti(keys) - sp.LogFields(otlog.Int("keys found", len(items))) + log.LogFields(otlog.Int("keys found", len(items))) // Memcached returns partial results even on error. if err != nil { - sp.LogFields(otlog.Error(err)) - level.Error(c.logger).Log("msg", "Failed to get keys from memcached", "err", err) + log.Error(err) + level.Error(log).Log("msg", "Failed to get keys from memcached", "err", err) } return err }) diff --git a/pkg/chunk/gcp/bigtable_index_client.go b/pkg/chunk/gcp/bigtable_index_client.go index 434bb40c754..c5dd36d1d79 100644 --- a/pkg/chunk/gcp/bigtable_index_client.go +++ b/pkg/chunk/gcp/bigtable_index_client.go @@ -13,13 +13,13 @@ import ( "cloud.google.com/go/bigtable" "github.com/go-kit/kit/log" ot "github.com/opentracing/opentracing-go" - otlog "github.com/opentracing/opentracing-go/log" "github.com/pkg/errors" "github.com/cortexproject/cortex/pkg/chunk" chunk_util "github.com/cortexproject/cortex/pkg/chunk/util" "github.com/cortexproject/cortex/pkg/util" "github.com/cortexproject/cortex/pkg/util/grpcclient" + "github.com/cortexproject/cortex/pkg/util/spanlogger" ) const ( @@ -324,8 +324,8 @@ func (s *storageClientV1) QueryPages(ctx context.Context, queries []chunk.IndexQ func (s *storageClientV1) query(ctx context.Context, query chunk.IndexQuery, callback chunk_util.Callback) error { const null = string('\xff') - sp, ctx := ot.StartSpanFromContext(ctx, "QueryPages", ot.Tag{Key: "tableName", Value: query.TableName}, ot.Tag{Key: "hashValue", Value: query.HashValue}) - defer sp.Finish() + log, _ := spanlogger.New(ctx, "QueryPages", ot.Tag{Key: "tableName", Value: query.TableName}, ot.Tag{Key: "hashValue", Value: query.HashValue}) + defer log.Finish() table := s.client.Open(query.TableName) @@ -358,7 +358,7 @@ func (s *storageClientV1) query(ctx context.Context, query chunk.IndexQuery, cal return true }) if err != nil { - sp.LogFields(otlog.String("error", err.Error())) + log.Error(err) return errors.WithStack(err) } return nil diff --git a/pkg/chunk/util/parallel_chunk_fetch.go b/pkg/chunk/util/parallel_chunk_fetch.go index cd0163ee684..2a68959adc2 100644 --- a/pkg/chunk/util/parallel_chunk_fetch.go +++ b/pkg/chunk/util/parallel_chunk_fetch.go @@ -4,10 +4,10 @@ import ( "context" "sync" - ot "github.com/opentracing/opentracing-go" otlog "github.com/opentracing/opentracing-go/log" "github.com/cortexproject/cortex/pkg/chunk" + "github.com/cortexproject/cortex/pkg/util/spanlogger" ) const maxParallel = 1000 @@ -20,9 +20,9 @@ var decodeContextPool = sync.Pool{ // GetParallelChunks fetches chunks in parallel (up to maxParallel). func GetParallelChunks(ctx context.Context, chunks []chunk.Chunk, f func(context.Context, *chunk.DecodeContext, chunk.Chunk) (chunk.Chunk, error)) ([]chunk.Chunk, error) { - sp, ctx := ot.StartSpanFromContext(ctx, "GetParallelChunks") - defer sp.Finish() - sp.LogFields(otlog.Int("chunks requested", len(chunks))) + log, ctx := spanlogger.New(ctx, "GetParallelChunks") + defer log.Finish() + log.LogFields(otlog.Int("chunks requested", len(chunks))) queuedChunks := make(chan chunk.Chunk) @@ -62,9 +62,9 @@ func GetParallelChunks(ctx context.Context, chunks []chunk.Chunk, f func(context } } - sp.LogFields(otlog.Int("chunks fetched", len(result))) + log.LogFields(otlog.Int("chunks fetched", len(result))) if lastErr != nil { - sp.LogFields(otlog.Error(lastErr)) + log.Error(lastErr) } // Return any chunks we did receive: a partial result may be useful diff --git a/pkg/querier/queryrange/query_range.go b/pkg/querier/queryrange/query_range.go index bfd6a2c9878..353a0c7ceaf 100644 --- a/pkg/querier/queryrange/query_range.go +++ b/pkg/querier/queryrange/query_range.go @@ -21,6 +21,7 @@ import ( "github.com/cortexproject/cortex/pkg/ingester/client" "github.com/cortexproject/cortex/pkg/util" + "github.com/cortexproject/cortex/pkg/util/spanlogger" ) // StatusSuccess Prometheus success result. @@ -238,17 +239,16 @@ func (prometheusCodec) DecodeResponse(ctx context.Context, r *http.Response, _ R body, _ := ioutil.ReadAll(r.Body) return nil, httpgrpc.Errorf(r.StatusCode, string(body)) } - - sp, _ := opentracing.StartSpanFromContext(ctx, "ParseQueryRangeResponse") - defer sp.Finish() + log, _ := spanlogger.New(ctx, "ParseQueryRangeResponse") + defer log.Finish() buf, err := ioutil.ReadAll(r.Body) if err != nil { - sp.LogFields(otlog.Error(err)) + log.Error(err) return nil, httpgrpc.Errorf(http.StatusInternalServerError, "error decoding response: %v", err) } - sp.LogFields(otlog.Int("bytes", len(buf))) + log.LogFields(otlog.Int("bytes", len(buf))) var resp PrometheusResponse if err := json.Unmarshal(buf, &resp); err != nil { diff --git a/pkg/querier/queryrange/results_cache.go b/pkg/querier/queryrange/results_cache.go index 30440d0b910..efdfb3a9028 100644 --- a/pkg/querier/queryrange/results_cache.go +++ b/pkg/querier/queryrange/results_cache.go @@ -450,14 +450,14 @@ func (s resultsCache) get(ctx context.Context, key string) ([]Extent, bool) { } var resp CachedResponse - sp, _ := opentracing.StartSpanFromContext(ctx, "unmarshal-extent") - defer sp.Finish() + log, ctx := spanlogger.New(ctx, "unmarshal-extent") + defer log.Finish() - sp.LogFields(otlog.Int("bytes", len(bufs[0]))) + log.LogFields(otlog.Int("bytes", len(bufs[0]))) if err := proto.Unmarshal(bufs[0], &resp); err != nil { - level.Error(s.logger).Log("msg", "error unmarshalling cached value", "err", err) - sp.LogFields(otlog.Error(err)) + level.Error(log).Log("msg", "error unmarshalling cached value", "err", err) + log.Error(err) return nil, false } diff --git a/pkg/testexporter/correctness/simple.go b/pkg/testexporter/correctness/simple.go index a896f604839..8e46bb6a15a 100644 --- a/pkg/testexporter/correctness/simple.go +++ b/pkg/testexporter/correctness/simple.go @@ -9,7 +9,6 @@ import ( "time" "github.com/go-kit/kit/log/level" - otlog "github.com/opentracing/opentracing-go/log" v1 "github.com/prometheus/client_golang/api/prometheus/v1" "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promauto" @@ -161,7 +160,7 @@ func verifySamples(log *spanlogger.SpanLogger, tc Case, pairs []model.SamplePair } else { sampleResult.WithLabelValues(tc.Name(), fail).Inc() level.Error(log).Log("msg", "wrong value", "at", pair.Timestamp, "expected", tc.ExpectedValueAt(pair.Timestamp.Time()), "actual", pair.Value) - log.LogFields(otlog.Error(fmt.Errorf("wrong value"))) + log.Error(fmt.Errorf("wrong value")) return false } } @@ -171,14 +170,14 @@ func verifySamples(log *spanlogger.SpanLogger, tc Case, pairs []model.SamplePair expectedNumSamples := int(duration / cfg.ScrapeInterval) if !epsilonCorrect(float64(len(pairs)), float64(expectedNumSamples), cfg.samplesEpsilon) { level.Error(log).Log("msg", "wrong number of samples", "expected", expectedNumSamples, "actual", len(pairs)) - log.LogFields(otlog.Error(fmt.Errorf("wrong number of samples"))) + log.Error(fmt.Errorf("wrong value")) return false } } else { expectedNumSamples := int(duration / cfg.ScrapeInterval) if math.Abs(float64(expectedNumSamples-len(pairs))) > 2 { level.Error(log).Log("msg", "wrong number of samples", "expected", expectedNumSamples, "actual", len(pairs)) - log.LogFields(otlog.Error(fmt.Errorf("wrong number of samples"))) + log.Error(fmt.Errorf("wrong value")) return false } } diff --git a/pkg/util/spanlogger/spanlogger.go b/pkg/util/spanlogger/spanlogger.go index a3018b0f38a..b376d2585e5 100644 --- a/pkg/util/spanlogger/spanlogger.go +++ b/pkg/util/spanlogger/spanlogger.go @@ -59,7 +59,7 @@ func (s *SpanLogger) Log(kvps ...interface{}) error { return nil } -// Error sets error flag and logs the error, if non-nil. Returns the err passed in. +// Error sets error flag and logs the error on the span, if non-nil. Returns the err passed in. func (s *SpanLogger) Error(err error) error { if err == nil { return nil From 2935b45cdd500d0bc3d9fdef62d368ea661051ae Mon Sep 17 00:00:00 2001 From: Callum Styan Date: Tue, 4 Aug 2020 13:26:19 -0700 Subject: [PATCH 2/6] Add spanlogger Error to errcheck exclude Signed-off-by: Callum Styan --- pkg/chunk/cache/memcached.go | 2 +- pkg/chunk/gcp/bigtable_index_client.go | 2 +- pkg/chunk/util/parallel_chunk_fetch.go | 2 +- pkg/querier/queryrange/query_range.go | 2 +- pkg/querier/queryrange/results_cache.go | 2 +- pkg/testexporter/correctness/simple.go | 6 +++--- 6 files changed, 8 insertions(+), 8 deletions(-) diff --git a/pkg/chunk/cache/memcached.go b/pkg/chunk/cache/memcached.go index 71c8094e214..60c33f8dc3b 100644 --- a/pkg/chunk/cache/memcached.go +++ b/pkg/chunk/cache/memcached.go @@ -158,7 +158,7 @@ func (c *Memcached) fetch(ctx context.Context, keys []string) (found []string, b // Memcached returns partial results even on error. if err != nil { - log.Error(err) + log.Error(err) //nolint:errcheck level.Error(log).Log("msg", "Failed to get keys from memcached", "err", err) } return err diff --git a/pkg/chunk/gcp/bigtable_index_client.go b/pkg/chunk/gcp/bigtable_index_client.go index c5dd36d1d79..5d7426a0780 100644 --- a/pkg/chunk/gcp/bigtable_index_client.go +++ b/pkg/chunk/gcp/bigtable_index_client.go @@ -358,7 +358,7 @@ func (s *storageClientV1) query(ctx context.Context, query chunk.IndexQuery, cal return true }) if err != nil { - log.Error(err) + log.Error(err) //nolint:errcheck return errors.WithStack(err) } return nil diff --git a/pkg/chunk/util/parallel_chunk_fetch.go b/pkg/chunk/util/parallel_chunk_fetch.go index 2a68959adc2..8b89d7282a8 100644 --- a/pkg/chunk/util/parallel_chunk_fetch.go +++ b/pkg/chunk/util/parallel_chunk_fetch.go @@ -64,7 +64,7 @@ func GetParallelChunks(ctx context.Context, chunks []chunk.Chunk, f func(context log.LogFields(otlog.Int("chunks fetched", len(result))) if lastErr != nil { - log.Error(lastErr) + log.Error(lastErr) //nolint:errcheck } // Return any chunks we did receive: a partial result may be useful diff --git a/pkg/querier/queryrange/query_range.go b/pkg/querier/queryrange/query_range.go index 353a0c7ceaf..4a036adfd0b 100644 --- a/pkg/querier/queryrange/query_range.go +++ b/pkg/querier/queryrange/query_range.go @@ -244,7 +244,7 @@ func (prometheusCodec) DecodeResponse(ctx context.Context, r *http.Response, _ R buf, err := ioutil.ReadAll(r.Body) if err != nil { - log.Error(err) + log.Error(err) //nolint:errcheck return nil, httpgrpc.Errorf(http.StatusInternalServerError, "error decoding response: %v", err) } diff --git a/pkg/querier/queryrange/results_cache.go b/pkg/querier/queryrange/results_cache.go index efdfb3a9028..e61c17f7f50 100644 --- a/pkg/querier/queryrange/results_cache.go +++ b/pkg/querier/queryrange/results_cache.go @@ -457,7 +457,7 @@ func (s resultsCache) get(ctx context.Context, key string) ([]Extent, bool) { if err := proto.Unmarshal(bufs[0], &resp); err != nil { level.Error(log).Log("msg", "error unmarshalling cached value", "err", err) - log.Error(err) + log.Error(err) //nolint:errcheck return nil, false } diff --git a/pkg/testexporter/correctness/simple.go b/pkg/testexporter/correctness/simple.go index 8e46bb6a15a..f8b766f92ed 100644 --- a/pkg/testexporter/correctness/simple.go +++ b/pkg/testexporter/correctness/simple.go @@ -160,7 +160,7 @@ func verifySamples(log *spanlogger.SpanLogger, tc Case, pairs []model.SamplePair } else { sampleResult.WithLabelValues(tc.Name(), fail).Inc() level.Error(log).Log("msg", "wrong value", "at", pair.Timestamp, "expected", tc.ExpectedValueAt(pair.Timestamp.Time()), "actual", pair.Value) - log.Error(fmt.Errorf("wrong value")) + log.Error(fmt.Errorf("wrong value")) //nolint:errcheck return false } } @@ -170,14 +170,14 @@ func verifySamples(log *spanlogger.SpanLogger, tc Case, pairs []model.SamplePair expectedNumSamples := int(duration / cfg.ScrapeInterval) if !epsilonCorrect(float64(len(pairs)), float64(expectedNumSamples), cfg.samplesEpsilon) { level.Error(log).Log("msg", "wrong number of samples", "expected", expectedNumSamples, "actual", len(pairs)) - log.Error(fmt.Errorf("wrong value")) + log.Error(fmt.Errorf("wrong value")) //nolint:errcheck return false } } else { expectedNumSamples := int(duration / cfg.ScrapeInterval) if math.Abs(float64(expectedNumSamples-len(pairs))) > 2 { level.Error(log).Log("msg", "wrong number of samples", "expected", expectedNumSamples, "actual", len(pairs)) - log.Error(fmt.Errorf("wrong value")) + log.Error(fmt.Errorf("wrong value")) //nolint:errcheck return false } } From b51ed19e2b04099dff2ccda25644c4c3be44e262 Mon Sep 17 00:00:00 2001 From: Callum Styan Date: Tue, 4 Aug 2020 20:24:33 -0700 Subject: [PATCH 3/6] Review cleanup. Signed-off-by: Callum Styan --- pkg/chunk/cache/memcached.go | 2 +- pkg/querier/queryrange/results_cache.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/chunk/cache/memcached.go b/pkg/chunk/cache/memcached.go index 60c33f8dc3b..815393bf209 100644 --- a/pkg/chunk/cache/memcached.go +++ b/pkg/chunk/cache/memcached.go @@ -146,7 +146,7 @@ func (c *Memcached) Fetch(ctx context.Context, keys []string) (found []string, b func (c *Memcached) fetch(ctx context.Context, keys []string) (found []string, bufs [][]byte, missed []string) { var items map[string]*memcache.Item - method := "Memcache.GetMulti" + const method = "Memcache.GetMulti" err := instr.CollectedRequest(ctx, method, c.requestDuration, memcacheStatusCode, func(innerCtx context.Context) error { log, _ := spanlogger.New(innerCtx, method) log.LogFields(otlog.Int("keys requested", len(keys))) diff --git a/pkg/querier/queryrange/results_cache.go b/pkg/querier/queryrange/results_cache.go index e61c17f7f50..397b2937f27 100644 --- a/pkg/querier/queryrange/results_cache.go +++ b/pkg/querier/queryrange/results_cache.go @@ -450,7 +450,7 @@ func (s resultsCache) get(ctx context.Context, key string) ([]Extent, bool) { } var resp CachedResponse - log, ctx := spanlogger.New(ctx, "unmarshal-extent") + log, _ := spanlogger.New(ctx, "unmarshal-extent") defer log.Finish() log.LogFields(otlog.Int("bytes", len(bufs[0]))) From b5a91b677b203cfafb92751e5ddc11965aa13541 Mon Sep 17 00:00:00 2001 From: Callum Styan Date: Mon, 10 Aug 2020 11:41:02 -0700 Subject: [PATCH 4/6] Address more review feedback. Signed-off-by: Callum Styan --- pkg/chunk/gcp/bigtable_index_client.go | 2 +- pkg/querier/queryrange/query_range.go | 2 +- pkg/querier/queryrange/results_cache.go | 2 +- pkg/testexporter/correctness/simple.go | 4 ++-- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/pkg/chunk/gcp/bigtable_index_client.go b/pkg/chunk/gcp/bigtable_index_client.go index 5d7426a0780..d4263601d78 100644 --- a/pkg/chunk/gcp/bigtable_index_client.go +++ b/pkg/chunk/gcp/bigtable_index_client.go @@ -324,7 +324,7 @@ func (s *storageClientV1) QueryPages(ctx context.Context, queries []chunk.IndexQ func (s *storageClientV1) query(ctx context.Context, query chunk.IndexQuery, callback chunk_util.Callback) error { const null = string('\xff') - log, _ := spanlogger.New(ctx, "QueryPages", ot.Tag{Key: "tableName", Value: query.TableName}, ot.Tag{Key: "hashValue", Value: query.HashValue}) + log, ctx := spanlogger.New(ctx, "QueryPages", ot.Tag{Key: "tableName", Value: query.TableName}, ot.Tag{Key: "hashValue", Value: query.HashValue}) defer log.Finish() table := s.client.Open(query.TableName) diff --git a/pkg/querier/queryrange/query_range.go b/pkg/querier/queryrange/query_range.go index 4a036adfd0b..38fbc60f3d7 100644 --- a/pkg/querier/queryrange/query_range.go +++ b/pkg/querier/queryrange/query_range.go @@ -239,7 +239,7 @@ func (prometheusCodec) DecodeResponse(ctx context.Context, r *http.Response, _ R body, _ := ioutil.ReadAll(r.Body) return nil, httpgrpc.Errorf(r.StatusCode, string(body)) } - log, _ := spanlogger.New(ctx, "ParseQueryRangeResponse") + log, ctx := spanlogger.New(ctx, "ParseQueryRangeResponse") defer log.Finish() buf, err := ioutil.ReadAll(r.Body) diff --git a/pkg/querier/queryrange/results_cache.go b/pkg/querier/queryrange/results_cache.go index 397b2937f27..e61c17f7f50 100644 --- a/pkg/querier/queryrange/results_cache.go +++ b/pkg/querier/queryrange/results_cache.go @@ -450,7 +450,7 @@ func (s resultsCache) get(ctx context.Context, key string) ([]Extent, bool) { } var resp CachedResponse - log, _ := spanlogger.New(ctx, "unmarshal-extent") + log, ctx := spanlogger.New(ctx, "unmarshal-extent") defer log.Finish() log.LogFields(otlog.Int("bytes", len(bufs[0]))) diff --git a/pkg/testexporter/correctness/simple.go b/pkg/testexporter/correctness/simple.go index f8b766f92ed..ac49d9f5959 100644 --- a/pkg/testexporter/correctness/simple.go +++ b/pkg/testexporter/correctness/simple.go @@ -170,14 +170,14 @@ func verifySamples(log *spanlogger.SpanLogger, tc Case, pairs []model.SamplePair expectedNumSamples := int(duration / cfg.ScrapeInterval) if !epsilonCorrect(float64(len(pairs)), float64(expectedNumSamples), cfg.samplesEpsilon) { level.Error(log).Log("msg", "wrong number of samples", "expected", expectedNumSamples, "actual", len(pairs)) - log.Error(fmt.Errorf("wrong value")) //nolint:errcheck + log.Error(fmt.Errorf("wrong number of samples")) //nolint:errcheck return false } } else { expectedNumSamples := int(duration / cfg.ScrapeInterval) if math.Abs(float64(expectedNumSamples-len(pairs))) > 2 { level.Error(log).Log("msg", "wrong number of samples", "expected", expectedNumSamples, "actual", len(pairs)) - log.Error(fmt.Errorf("wrong value")) //nolint:errcheck + log.Error(fmt.Errorf("wrong number of samples")) //nolint:errcheck return false } } From 075f07cfe5610c0990ac187ee5a08e3a074d3ef1 Mon Sep 17 00:00:00 2001 From: Callum Styan Date: Mon, 10 Aug 2020 16:14:26 -0700 Subject: [PATCH 5/6] We need nolint comments on these lines if we want to assign the ctx in attempt to avoid use of the incorrect ctx in the future. Signed-off-by: Callum Styan --- pkg/querier/queryrange/query_range.go | 2 +- pkg/querier/queryrange/results_cache.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/querier/queryrange/query_range.go b/pkg/querier/queryrange/query_range.go index 38fbc60f3d7..49c379b392a 100644 --- a/pkg/querier/queryrange/query_range.go +++ b/pkg/querier/queryrange/query_range.go @@ -239,7 +239,7 @@ func (prometheusCodec) DecodeResponse(ctx context.Context, r *http.Response, _ R body, _ := ioutil.ReadAll(r.Body) return nil, httpgrpc.Errorf(r.StatusCode, string(body)) } - log, ctx := spanlogger.New(ctx, "ParseQueryRangeResponse") + log, ctx := spanlogger.New(ctx, "ParseQueryRangeResponse") //nolint:ineffassign,staticcheck defer log.Finish() buf, err := ioutil.ReadAll(r.Body) diff --git a/pkg/querier/queryrange/results_cache.go b/pkg/querier/queryrange/results_cache.go index e61c17f7f50..1792331f074 100644 --- a/pkg/querier/queryrange/results_cache.go +++ b/pkg/querier/queryrange/results_cache.go @@ -450,7 +450,7 @@ func (s resultsCache) get(ctx context.Context, key string) ([]Extent, bool) { } var resp CachedResponse - log, ctx := spanlogger.New(ctx, "unmarshal-extent") + log, ctx := spanlogger.New(ctx, "unmarshal-extent") //nolint:ineffassign,staticcheck defer log.Finish() log.LogFields(otlog.Int("bytes", len(bufs[0]))) From f2d02ea2be272e97873b4b351cd675ca91896768 Mon Sep 17 00:00:00 2001 From: Marco Pracucci Date: Tue, 11 Aug 2020 18:10:12 +0200 Subject: [PATCH 6/6] Globally exclude SpanLogger.Error() errcheck Signed-off-by: Marco Pracucci --- .errcheck-exclude | 3 ++- pkg/chunk/cache/memcached.go | 2 +- pkg/chunk/gcp/bigtable_index_client.go | 2 +- pkg/chunk/util/parallel_chunk_fetch.go | 2 +- pkg/querier/queryrange/query_range.go | 2 +- pkg/querier/queryrange/results_cache.go | 2 +- pkg/testexporter/correctness/simple.go | 6 +++--- 7 files changed, 10 insertions(+), 9 deletions(-) diff --git a/.errcheck-exclude b/.errcheck-exclude index d3c9a0d7977..cdf86dc45d5 100644 --- a/.errcheck-exclude +++ b/.errcheck-exclude @@ -2,4 +2,5 @@ io/ioutil.WriteFile io/ioutil.ReadFile (github.com/go-kit/kit/log.Logger).Log io.Copy -(github.com/opentracing/opentracing-go.Tracer).Inject \ No newline at end of file +(github.com/opentracing/opentracing-go.Tracer).Inject +(*github.com/cortexproject/cortex/pkg/util/spanlogger.SpanLogger).Error diff --git a/pkg/chunk/cache/memcached.go b/pkg/chunk/cache/memcached.go index 815393bf209..b8b6cea7d7a 100644 --- a/pkg/chunk/cache/memcached.go +++ b/pkg/chunk/cache/memcached.go @@ -158,7 +158,7 @@ func (c *Memcached) fetch(ctx context.Context, keys []string) (found []string, b // Memcached returns partial results even on error. if err != nil { - log.Error(err) //nolint:errcheck + log.Error(err) level.Error(log).Log("msg", "Failed to get keys from memcached", "err", err) } return err diff --git a/pkg/chunk/gcp/bigtable_index_client.go b/pkg/chunk/gcp/bigtable_index_client.go index d4263601d78..7ea6bd0784e 100644 --- a/pkg/chunk/gcp/bigtable_index_client.go +++ b/pkg/chunk/gcp/bigtable_index_client.go @@ -358,7 +358,7 @@ func (s *storageClientV1) query(ctx context.Context, query chunk.IndexQuery, cal return true }) if err != nil { - log.Error(err) //nolint:errcheck + log.Error(err) return errors.WithStack(err) } return nil diff --git a/pkg/chunk/util/parallel_chunk_fetch.go b/pkg/chunk/util/parallel_chunk_fetch.go index 8b89d7282a8..2a68959adc2 100644 --- a/pkg/chunk/util/parallel_chunk_fetch.go +++ b/pkg/chunk/util/parallel_chunk_fetch.go @@ -64,7 +64,7 @@ func GetParallelChunks(ctx context.Context, chunks []chunk.Chunk, f func(context log.LogFields(otlog.Int("chunks fetched", len(result))) if lastErr != nil { - log.Error(lastErr) //nolint:errcheck + log.Error(lastErr) } // Return any chunks we did receive: a partial result may be useful diff --git a/pkg/querier/queryrange/query_range.go b/pkg/querier/queryrange/query_range.go index 49c379b392a..0864f01a473 100644 --- a/pkg/querier/queryrange/query_range.go +++ b/pkg/querier/queryrange/query_range.go @@ -244,7 +244,7 @@ func (prometheusCodec) DecodeResponse(ctx context.Context, r *http.Response, _ R buf, err := ioutil.ReadAll(r.Body) if err != nil { - log.Error(err) //nolint:errcheck + log.Error(err) return nil, httpgrpc.Errorf(http.StatusInternalServerError, "error decoding response: %v", err) } diff --git a/pkg/querier/queryrange/results_cache.go b/pkg/querier/queryrange/results_cache.go index 1792331f074..097463fd970 100644 --- a/pkg/querier/queryrange/results_cache.go +++ b/pkg/querier/queryrange/results_cache.go @@ -457,7 +457,7 @@ func (s resultsCache) get(ctx context.Context, key string) ([]Extent, bool) { if err := proto.Unmarshal(bufs[0], &resp); err != nil { level.Error(log).Log("msg", "error unmarshalling cached value", "err", err) - log.Error(err) //nolint:errcheck + log.Error(err) return nil, false } diff --git a/pkg/testexporter/correctness/simple.go b/pkg/testexporter/correctness/simple.go index ac49d9f5959..6018d35b4c3 100644 --- a/pkg/testexporter/correctness/simple.go +++ b/pkg/testexporter/correctness/simple.go @@ -160,7 +160,7 @@ func verifySamples(log *spanlogger.SpanLogger, tc Case, pairs []model.SamplePair } else { sampleResult.WithLabelValues(tc.Name(), fail).Inc() level.Error(log).Log("msg", "wrong value", "at", pair.Timestamp, "expected", tc.ExpectedValueAt(pair.Timestamp.Time()), "actual", pair.Value) - log.Error(fmt.Errorf("wrong value")) //nolint:errcheck + log.Error(fmt.Errorf("wrong value")) return false } } @@ -170,14 +170,14 @@ func verifySamples(log *spanlogger.SpanLogger, tc Case, pairs []model.SamplePair expectedNumSamples := int(duration / cfg.ScrapeInterval) if !epsilonCorrect(float64(len(pairs)), float64(expectedNumSamples), cfg.samplesEpsilon) { level.Error(log).Log("msg", "wrong number of samples", "expected", expectedNumSamples, "actual", len(pairs)) - log.Error(fmt.Errorf("wrong number of samples")) //nolint:errcheck + log.Error(fmt.Errorf("wrong number of samples")) return false } } else { expectedNumSamples := int(duration / cfg.ScrapeInterval) if math.Abs(float64(expectedNumSamples-len(pairs))) > 2 { level.Error(log).Log("msg", "wrong number of samples", "expected", expectedNumSamples, "actual", len(pairs)) - log.Error(fmt.Errorf("wrong number of samples")) //nolint:errcheck + log.Error(fmt.Errorf("wrong number of samples")) return false } }