From 7eaefcf379e1b27c3551c6b698a3b1d6bf24d500 Mon Sep 17 00:00:00 2001 From: Matthieu MOREL Date: Wed, 1 Nov 2023 20:06:46 +0100 Subject: [PATCH] ci(lint): enable errorlint on scrape (#12923) Signed-off-by: Matthieu MOREL Signed-off-by: Jesus Vazquez Co-authored-by: Jesus Vazquez --- .golangci.yml | 3 --- scrape/scrape.go | 42 +++++++++++++++++++++--------------------- scrape/scrape_test.go | 16 ++++++++-------- 3 files changed, 29 insertions(+), 32 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index d585dfc743..871d748c75 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -34,9 +34,6 @@ issues: - path: _test.go linters: - errcheck - - path: scrape/ - linters: - - errorlint - path: tsdb/ linters: - errorlint diff --git a/scrape/scrape.go b/scrape/scrape.go index 0ea740b68e..790ee18af1 100644 --- a/scrape/scrape.go +++ b/scrape/scrape.go @@ -18,6 +18,7 @@ import ( "bytes" "compress/gzip" "context" + "errors" "fmt" "io" "math" @@ -30,7 +31,6 @@ import ( "github.com/go-kit/log" "github.com/go-kit/log/level" - "github.com/pkg/errors" config_util "github.com/prometheus/common/config" "github.com/prometheus/common/model" "github.com/prometheus/common/version" @@ -122,7 +122,7 @@ func newScrapePool(cfg *config.ScrapeConfig, app storage.Appendable, offsetSeed client, err := config_util.NewClientFromConfig(cfg.HTTPClientConfig, cfg.JobName, options.HTTPClientOptions...) if err != nil { - return nil, errors.Wrap(err, "error creating HTTP client") + return nil, fmt.Errorf("error creating HTTP client: %w", err) } buffers := pool.New(1e3, 100e6, 3, func(sz int) interface{} { return make([]byte, 0, sz) }) @@ -250,7 +250,7 @@ func (sp *scrapePool) reload(cfg *config.ScrapeConfig) error { client, err := config_util.NewClientFromConfig(cfg.HTTPClientConfig, cfg.JobName, sp.httpOpts...) if err != nil { sp.metrics.targetScrapePoolReloadsFailed.Inc() - return errors.Wrap(err, "error creating HTTP client") + return fmt.Errorf("error creating HTTP client: %w", err) } reuseCache := reusableCache(sp.config, cfg) @@ -695,7 +695,7 @@ func (s *targetScraper) readResponse(ctx context.Context, resp *http.Response, w }() if resp.StatusCode != http.StatusOK { - return "", errors.Errorf("server returned HTTP status %s", resp.Status) + return "", fmt.Errorf("server returned HTTP status %s", resp.Status) } if s.bodySizeLimit <= 0 { @@ -1549,7 +1549,7 @@ loop: } sampleAdded, err = sl.checkAddError(ce, met, parsedTimestamp, err, &sampleLimitErr, &bucketLimitErr, &appErrs) if err != nil { - if err != storage.ErrNotFound { + if !errors.Is(err, storage.ErrNotFound) { level.Debug(sl.l).Log("msg", "Unexpected error", "series", string(met), "err", err) } break loop @@ -1620,8 +1620,8 @@ loop: sl.cache.forEachStale(func(lset labels.Labels) bool { // Series no longer exposed, mark it stale. _, err = app.Append(0, lset, defTime, math.Float64frombits(value.StaleNaN)) - switch errors.Cause(err) { - case storage.ErrOutOfOrderSample, storage.ErrDuplicateSampleForTimestamp: + switch { + case errors.Is(err, storage.ErrOutOfOrderSample), errors.Is(err, storage.ErrDuplicateSampleForTimestamp): // Do not count these in logging, as this is expected if a target // goes away and comes back again with a new scrape loop. err = nil @@ -1635,35 +1635,35 @@ loop: // Adds samples to the appender, checking the error, and then returns the # of samples added, // whether the caller should continue to process more samples, and any sample or bucket limit errors. func (sl *scrapeLoop) checkAddError(ce *cacheEntry, met []byte, tp *int64, err error, sampleLimitErr, bucketLimitErr *error, appErrs *appendErrors) (bool, error) { - switch errors.Cause(err) { - case nil: + switch { + case err == nil: if (tp == nil || sl.trackTimestampsStaleness) && ce != nil { sl.cache.trackStaleness(ce.hash, ce.lset) } return true, nil - case storage.ErrNotFound: + case errors.Is(err, storage.ErrNotFound): return false, storage.ErrNotFound - case storage.ErrOutOfOrderSample: + case errors.Is(err, storage.ErrOutOfOrderSample): appErrs.numOutOfOrder++ level.Debug(sl.l).Log("msg", "Out of order sample", "series", string(met)) sl.metrics.targetScrapeSampleOutOfOrder.Inc() return false, nil - case storage.ErrDuplicateSampleForTimestamp: + case errors.Is(err, storage.ErrDuplicateSampleForTimestamp): appErrs.numDuplicates++ level.Debug(sl.l).Log("msg", "Duplicate sample for timestamp", "series", string(met)) sl.metrics.targetScrapeSampleDuplicate.Inc() return false, nil - case storage.ErrOutOfBounds: + case errors.Is(err, storage.ErrOutOfBounds): appErrs.numOutOfBounds++ level.Debug(sl.l).Log("msg", "Out of bounds metric", "series", string(met)) sl.metrics.targetScrapeSampleOutOfBounds.Inc() return false, nil - case errSampleLimit: + case errors.Is(err, errSampleLimit): // Keep on parsing output if we hit the limit, so we report the correct // total number of samples scraped. *sampleLimitErr = err return false, nil - case errBucketLimit: + case errors.Is(err, errBucketLimit): // Keep on parsing output if we hit the limit, so we report the correct // total number of samples scraped. *bucketLimitErr = err @@ -1674,10 +1674,10 @@ func (sl *scrapeLoop) checkAddError(ce *cacheEntry, met []byte, tp *int64, err e } func (sl *scrapeLoop) checkAddExemplarError(err error, e exemplar.Exemplar, appErrs *appendErrors) error { - switch errors.Cause(err) { - case storage.ErrNotFound: + switch { + case errors.Is(err, storage.ErrNotFound): return storage.ErrNotFound - case storage.ErrOutOfOrderExemplar: + case errors.Is(err, storage.ErrOutOfOrderExemplar): appErrs.numExemplarOutOfOrder++ level.Debug(sl.l).Log("msg", "Out of order exemplar", "exemplar", fmt.Sprintf("%+v", e)) sl.metrics.targetScrapeExemplarOutOfOrder.Inc() @@ -1789,13 +1789,13 @@ func (sl *scrapeLoop) addReportSample(app storage.Appender, s []byte, t int64, v } ref, err := app.Append(ref, lset, t, v) - switch errors.Cause(err) { - case nil: + switch { + case err == nil: if !ok { sl.cache.addRef(s, ref, lset, lset.Hash()) } return nil - case storage.ErrOutOfOrderSample, storage.ErrDuplicateSampleForTimestamp: + case errors.Is(err, storage.ErrOutOfOrderSample), errors.Is(err, storage.ErrDuplicateSampleForTimestamp): // Do not log here, as this is expected if a target goes away and comes back // again with a new scrape loop. return nil diff --git a/scrape/scrape_test.go b/scrape/scrape_test.go index 08cf37ede1..ccd651f49b 100644 --- a/scrape/scrape_test.go +++ b/scrape/scrape_test.go @@ -18,6 +18,7 @@ import ( "compress/gzip" "context" "encoding/binary" + "errors" "fmt" "io" "math" @@ -31,7 +32,6 @@ import ( "github.com/go-kit/log" "github.com/gogo/protobuf/proto" - "github.com/pkg/errors" "github.com/prometheus/client_golang/prometheus" dto "github.com/prometheus/client_model/go" config_util "github.com/prometheus/common/config" @@ -882,7 +882,7 @@ func TestScrapeLoopRun(t *testing.T) { select { case err := <-errc: - if err != context.DeadlineExceeded { + if !errors.Is(err, context.DeadlineExceeded) { t.Fatalf("Expected timeout error but got: %s", err) } case <-time.After(3 * time.Second): @@ -952,7 +952,7 @@ func TestScrapeLoopForcedErr(t *testing.T) { select { case err := <-errc: - if err != forcedErr { + if !errors.Is(err, forcedErr) { t.Fatalf("Expected forced error but got: %s", err) } case <-time.After(3 * time.Second): @@ -1741,7 +1741,7 @@ func TestScrapeLoopAppendSampleLimit(t *testing.T) { now := time.Now() slApp := sl.appender(context.Background()) total, added, seriesAdded, err := sl.append(app, []byte("metric_a 1\nmetric_b 1\nmetric_c 1\n"), "", now) - if err != errSampleLimit { + if !errors.Is(err, errSampleLimit) { t.Fatalf("Did not see expected sample limit error: %s", err) } require.NoError(t, slApp.Rollback()) @@ -1772,7 +1772,7 @@ func TestScrapeLoopAppendSampleLimit(t *testing.T) { now = time.Now() slApp = sl.appender(context.Background()) total, added, seriesAdded, err = sl.append(slApp, []byte("metric_a 1\nmetric_b 1\nmetric_c{deleteme=\"yes\"} 1\nmetric_d 1\nmetric_e 1\nmetric_f 1\nmetric_g 1\nmetric_h{deleteme=\"yes\"} 1\nmetric_i{deleteme=\"yes\"} 1\n"), "", now) - if err != errSampleLimit { + if !errors.Is(err, errSampleLimit) { t.Fatalf("Did not see expected sample limit error: %s", err) } require.NoError(t, slApp.Rollback()) @@ -1868,7 +1868,7 @@ func TestScrapeLoop_HistogramBucketLimit(t *testing.T) { now = time.Now() total, added, seriesAdded, err = sl.append(app, msg, "application/vnd.google.protobuf", now) - if err != errBucketLimit { + if !errors.Is(err, errBucketLimit) { t.Fatalf("Did not see expected histogram bucket limit error: %s", err) } require.NoError(t, app.Rollback()) @@ -2738,8 +2738,8 @@ func TestTargetScrapeScrapeCancel(t *testing.T) { switch { case err == nil: errc <- errors.New("Expected error but got nil") - case ctx.Err() != context.Canceled: - errc <- errors.Errorf("Expected context cancellation error but got: %s", ctx.Err()) + case !errors.Is(ctx.Err(), context.Canceled): + errc <- fmt.Errorf("Expected context cancellation error but got: %w", ctx.Err()) default: close(errc) }