.*: replace linters with golangci-lint

.circleci/config.yml

@@ -23,7 +23,7 @@ jobs:
               echo "Awesome! GCS integration tests are enabled."
             fi
       - run: make check-go-mod
-      - run: make errcheck
+      - run: make lint
       - run: make check-docs
       - run: make format
       - run:
@@ -131,4 +131,4 @@ workflows:
           tags:
             only: /^v[0-9]+(\.[0-9]+){2}(-.+|[^-.]*)$/
           branches:
-            ignore: /.*/
+            ignore: /.*/

.golangci.yml

@@ -0,0 +1,21 @@
+linters-settings:
+    errcheck:
+        exclude: ./.errcheck_excludes.txt
+
+issues:
+    exclude-rules:
+        # TODO: move away from deprecated prometheus.InstrumentHandler{,Func}
+        - linters:
+              - staticcheck
+          text: "SA1019:"
+        # These are not being checked since these methods exist
+        # so that no one else could implement them.
+        - linters:
+              - unused
+          text: "SourceStoreAPI.implementsStoreAPI"
+        - linters:
+              - unused
+          text: "SourceStoreAPI.producesBlocks"
+        - linters:
+              - unused
+          text: "Source.producesBlocks"

Makefile

@@ -1,5 +1,4 @@
 PREFIX            ?= $(shell pwd)
-FILES_TO_FMT      ?= $(shell find . -path ./vendor -prune -o -name '*.go' -print)
 
 DOCKER_IMAGE_NAME ?= thanos
 DOCKER_IMAGE_TAG  ?= $(subst /,-,$(shell git rev-parse --abbrev-ref HEAD))-$(shell date +%Y-%m-%d)-$(shell git rev-parse --short HEAD)
@@ -15,13 +14,8 @@ export GOPROXY
 EMBEDMD           ?= $(GOBIN)/embedmd-$(EMBEDMD_VERSION)
 # v2.0.0
 EMBEDMD_VERSION   ?= 97c13d6e41602fc6e397eb51c45f38069371a969
-ERRCHECK          ?= $(GOBIN)/errcheck-$(ERRCHECK_VERSION)
-# v1.2.0
-ERRCHECK_VERSION  ?= e14f8d59a22d460d56c5ee92507cd94c78fbf274
 LICHE             ?= $(GOBIN)/liche-$(LICHE_VERSION)
 LICHE_VERSION     ?= 2a2e6e56f6c615c17b2e116669c4cdb31b5453f3
-GOIMPORTS         ?= $(GOBIN)/goimports-$(GOIMPORTS_VERSION)
-GOIMPORTS_VERSION ?= 9d4d845e86f14303813298ede731a971dd65b593
 PROMU             ?= $(GOBIN)/promu-$(PROMU_VERSION)
 PROMU_VERSION     ?= 9583e5a6448f97c6294dca72dd1d173e28f8d4a4
 PROTOC            ?= $(GOBIN)/protoc-$(PROTOC_VERSION)
@@ -33,6 +27,10 @@ HUGO              ?= $(GOBIN)/hugo-$(HUGO_VERSION)
 GOBINDATA_VERSION ?= a9c83481b38ebb1c4eb8f0168fd4b10ca1d3c523
 GOBINDATA         ?= $(GOBIN)/go-bindata-$(GOBINDATA_VERSION)
 GIT               ?= $(shell which git)
+# golangci-lint which includes errcheck, goimports
+# and more. v1.16.0
+GOLANGCILINT_VERSION ?= 97ea1cbb21bbf5e4d0e8bcc0f9243385e9262dcc
+GOLANGCILINT ?= $(GOBIN)/golangci-lint-$(GOLANGCILINT_VERSION)
 
 WEB_DIR           ?= website
 WEBSITE_BASE_URL  ?= https://thanos.io
@@ -151,19 +149,14 @@ check-docs: $(EMBEDMD) $(LICHE) build
 	@$(LICHE) --recursive docs --exclude "cloud.tencent.com" --document-root .
 	@$(LICHE) --exclude "cloud.tencent.com" --document-root . *.md
 
-# errcheck performs static analysis and returns error if any of the errors is not checked.
-.PHONY: errcheck
-errcheck: $(ERRCHECK)
-	@echo ">> errchecking the code"
-	$(ERRCHECK) -verbose -exclude .errcheck_excludes.txt ./cmd/... ./pkg/... ./test/...
-
 # format formats the code (including imports format).
-# NOTE: format requires deps to not remove imports that are used, just not resolved.
-# This is not encoded, because it is often used in IDE onSave logic.
+# # NOTE: format requires deps to not remove imports that are used, just not resolved.
+# # This is not encoded, because it is often used in IDE onSave logic.
 .PHONY: format
-format: $(GOIMPORTS)
+format: check-git $(GOLANGCILINT)
 	@echo ">> formatting code"
-	@$(GOIMPORTS) -w $(FILES_TO_FMT)
+	@$(GOLANGCILINT) run --disable-all -E goimports ./...
+
 
 # proto generates golang files from Thanos proto files.
 .PHONY: proto
@@ -195,12 +188,6 @@ test-deps:
 	$(call fetch_go_bin_version,github.com/prometheus/alertmanager/cmd/alertmanager,$(ALERTMANAGER_VERSION))
 	$(call fetch_go_bin_version,github.com/minio/minio,$(MINIO_SERVER_VERSION))
 
-# vet vets the code.
-.PHONY: vet
-vet: check-git
-	@echo ">> vetting code"
-	@go vet ./...
-
 # go mod related
 .PHONY: go-mod-tidy
 go-mod-tidy: check-git
@@ -231,6 +218,11 @@ web: web-pre-process $(HUGO)
 	# TODO(bwplotka): Make it --gc
 	@cd $(WEB_DIR) && HUGO_ENV=production $(HUGO) --config hugo.yaml --minify -v -b $(WEBSITE_BASE_URL)
 
+.PHONY: lint
+lint: check-git $(GOLANGCILINT)
+	@echo ">> linting all of the Go files"
+	@$(GOLANGCILINT) run ./...
+
 .PHONY: web-serve
 web-serve: web-pre-process $(HUGO)
 	@echo ">> serving documentation website"
@@ -240,9 +232,6 @@ web-serve: web-pre-process $(HUGO)
 $(EMBEDMD):
 	$(call fetch_go_bin_version,github.com/campoy/embedmd,$(EMBEDMD_VERSION))
 
-$(ERRCHECK):
-	$(call fetch_go_bin_version,github.com/kisielk/errcheck,$(ERRCHECK_VERSION))
-
 $(GOIMPORTS):
 	$(call fetch_go_bin_version,golang.org/x/tools/cmd/goimports,$(GOIMPORTS_VERSION))
 
@@ -260,6 +249,9 @@ $(HUGO):
 $(GOBINDATA):
 	$(call fetch_go_bin_version,github.com/go-bindata/go-bindata/go-bindata,$(GOBINDATA_VERSION))
 
+$(GOLANGCILINT):
+	$(call fetch_go_bin_version,github.com/golangci/golangci-lint/cmd/golangci-lint,$(GOLANGCILINT_VERSION))
+
 $(PROTOC):
 	@mkdir -p $(TMP_GOPATH)
 	@echo ">> fetching protoc@${PROTOC_VERSION}"

cmd/thanos/bucket.go

@@ -60,7 +60,6 @@ func registerBucket(m map[string]setupFunc, app *kingpin.Application, name strin
 	registerBucketLs(m, cmd, name, objStoreConfig)
 	registerBucketInspect(m, cmd, name, objStoreConfig)
 	registerBucketWeb(m, cmd, name, objStoreConfig)
-	return
 }
 
 func registerBucketVerify(m map[string]setupFunc, root *kingpin.CmdClause, name string, objStoreConfig *pathOrContent) {

cmd/thanos/compact.go

@@ -206,6 +206,7 @@ func runCompact(
 	// are in milliseconds.
 	comp, err := tsdb.NewLeveledCompactor(ctx, reg, logger, levels, downsample.NewPool())
 	if err != nil {
+		cancel()
 		return errors.Wrap(err, "create compactor")
 	}
 
@@ -216,11 +217,13 @@ func runCompact(
 	)
 
 	if err := os.RemoveAll(downsamplingDir); err != nil {
+		cancel()
 		return errors.Wrap(err, "clean working downsample directory")
 	}
 
 	compactor, err := compact.NewBucketCompactor(logger, sy, comp, compactDir, bkt, concurrency)
 	if err != nil {
+		cancel()
 		return errors.Wrap(err, "create bucket compactor")
 	}
 

cmd/thanos/downsample.go

@@ -298,8 +298,6 @@ func processDownsampling(ctx context.Context, logger log.Logger, bkt objstore.Bu
 
 	level.Info(logger).Log("msg", "uploaded block", "id", id, "duration", time.Since(begin))
 
-	begin = time.Now()
-
 	// It is not harmful if these fails.
 	if err := os.RemoveAll(bdir); err != nil {
 		level.Warn(logger).Log("msg", "failed to clean directory", "dir", bdir, "err", err)

cmd/thanos/main.go

@@ -145,6 +145,10 @@ func main() {
 		var closer io.Closer
 		var confContentYaml []byte
 		confContentYaml, err = tracingConfig.Content()
+		if err != nil {
+			level.Error(logger).Log("msg", "getting tracing config failed", "err", err)
+			os.Exit(1)
+		}
 
 		if len(confContentYaml) == 0 {
 			level.Info(logger).Log("msg", "Tracing will be disabled")

cmd/thanos/receive.go

@@ -257,10 +257,7 @@ func runReceive(
 			err error
 		)
 		g.Add(func() error {
-			select {
-			case <-dbOpen:
-				break
-			}
+			<-dbOpen
 
 			l, err = net.Listen("tcp", grpcBindAddr)
 			if err != nil {

cmd/thanos/rule.go

@@ -762,14 +762,14 @@ func queryFunc(
 
 			if err != nil {
 				level.Error(logger).Log("err", err, "query", q)
+			} else {
+				if len(warns) > 0 {
+					ruleEvalWarnings.WithLabelValues(strings.ToLower(partialResponseStrategy.String())).Inc()
+					// TODO(bwplotka): Propagate those to UI, probably requires changing rule manager code ):
+					level.Warn(logger).Log("warnings", strings.Join(warns, ", "), "query", q)
+				}
+				return v, nil
 			}
-
-			if err == nil && len(warns) > 0 {
-				ruleEvalWarnings.WithLabelValues(strings.ToLower(partialResponseStrategy.String())).Inc()
-				// TODO(bwplotka): Propagate those to UI, probably requires changing rule manager code ):
-				level.Warn(logger).Log("warnings", strings.Join(warns, ", "), "query", q)
-			}
-			return v, err
 		}
 		return nil, errors.Errorf("no query peer reachable")
 	}

pkg/alert/alert.go

@@ -360,7 +360,6 @@ func (s *Sender) Send(ctx context.Context, alerts []*Alert) {
 
 	s.dropped.Add(float64(len(alerts)))
 	level.Warn(s.logger).Log("msg", "failed to send alerts to all alertmanagers", "alertmanagers", amrs, "alerts", string(b))
-	return
 }
 
 func (s *Sender) sendOne(ctx context.Context, url string, b []byte) error {

pkg/block/index.go

@@ -382,7 +382,7 @@ func GatherIndexIssueStats(logger log.Logger, fn string, minTime int64, maxTime
 				stats.OutOfOrderLabels++
 				level.Warn(logger).Log("msg",
 					"out-of-order label set: known bug in Prometheus 2.8.0 and below",
-					"labelset", fmt.Sprintf("%s", lset),
+					"labelset", lset.String(),
 					"series", fmt.Sprintf("%d", id),
 				)
 			}
@@ -727,11 +727,6 @@ func (ss stringset) set(s string) {
 	ss[s] = struct{}{}
 }
 
-func (ss stringset) has(s string) bool {
-	_, ok := ss[s]
-	return ok
-}
-
 func (ss stringset) String() string {
 	return strings.Join(ss.slice(), ",")
 }

pkg/block/metadata/meta.go

@@ -44,8 +44,6 @@ const (
 // Meta describes the a block's meta. It wraps the known TSDB meta structure and
 // extends it by Thanos-specific fields.
 type Meta struct {
-	Version int `json:"version"`
-
 	tsdb.BlockMeta
 
 	Thanos Thanos `json:"thanos"`

pkg/compact/compact.go

@@ -62,9 +62,6 @@ type syncerMetrics struct {
 	garbageCollectionDuration prometheus.Histogram
 	compactions               *prometheus.CounterVec
 	compactionFailures        *prometheus.CounterVec
-	indexCacheBlocks          prometheus.Counter
-	indexCacheTraverse        prometheus.Counter
-	indexCacheFailures        prometheus.Counter
 }
 
 func newSyncerMetrics(reg prometheus.Registerer) *syncerMetrics {
@@ -985,6 +982,7 @@ func (c *BucketCompactor) Compact(ctx context.Context) error {
 			finishedAllGroups      = true
 			mtx                    sync.Mutex
 		)
+		defer workCtxCancel()
 
 		// Set up workers who will compact the groups when the groups are ready.
 		// They will compact available groups until they encounter an error, after which they will stop.

pkg/compact/downsample/downsample.go

@@ -241,7 +241,6 @@ func (a *aggregator) add(v float64) {
 // aggrChunkBuilder builds chunks for multiple different aggregates.
 type aggrChunkBuilder struct {
 	mint, maxt int64
-	isCounter  bool
 	added      int
 
 	chunks [5]chunkenc.Chunk

pkg/compact/retention_test.go

@@ -254,11 +254,11 @@ func TestApplyRetentionPolicyByResolution(t *testing.T) {
 func uploadMockBlock(t *testing.T, bkt objstore.Bucket, id string, minTime, maxTime time.Time, resolutionLevel int64) {
 	t.Helper()
 	meta1 := metadata.Meta{
-		Version: 1,
 		BlockMeta: tsdb.BlockMeta{
 			ULID:    ulid.MustParse(id),
 			MinTime: minTime.Unix() * 1000,
 			MaxTime: maxTime.Unix() * 1000,
+			Version: 1,
 		},
 		Thanos: metadata.Thanos{
 			Downsample: metadata.ThanosDownsample{

pkg/discovery/dns/provider.go

@@ -97,7 +97,6 @@ func (p *Provider) Resolve(ctx context.Context, addrs []string) {
 		p.resolverLookupsCount.Inc()
 		if err != nil {
 			// The DNS resolution failed. Continue without modifying the old records.
-			p.resolved[addr] = p.resolved[addr] // Ensure metrics capture the result even if empty.
 			p.resolverFailuresCount.Inc()
 			level.Error(p.logger).Log("msg", "dns resolution failed", "addr", addr, "err", err)
 			continue

pkg/objstore/azure/helpers.go

@@ -25,7 +25,7 @@ func getContainerURL(ctx context.Context, conf Config) (blob.ContainerURL, error
 		MaxTries: int32(conf.MaxRetries),
 	}
 	if deadline, ok := ctx.Deadline(); ok {
-		retryOptions.TryTimeout = deadline.Sub(time.Now())
+		retryOptions.TryTimeout = time.Until(deadline)
 	}
 
 	p := blob.NewPipeline(c, blob.PipelineOptions{
@@ -73,7 +73,7 @@ func getBlobURL(ctx context.Context, conf Config, blobName string) (blob.BlockBl
 
 func parseError(errorCode string) string {
 	match := errorCodeRegex.FindStringSubmatch(errorCode)
-	if match != nil && len(match) == 2 {
+	if len(match) == 2 {
 		return match[1]
 	}
 	return errorCode

pkg/objstore/gcs/gcs.go

@@ -168,6 +168,7 @@ func (b *Bucket) Close() error {
 // In a close function it empties and deletes the bucket.
 func NewTestBucket(t testing.TB, project string) (objstore.Bucket, func(), error) {
 	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
 	src := rand.NewSource(time.Now().UnixNano())
 	gTestConfig := Config{
 		Bucket: fmt.Sprintf("test_%s_%x", strings.ToLower(t.Name()), src.Int63()),
@@ -180,12 +181,10 @@ func NewTestBucket(t testing.TB, project string) (objstore.Bucket, func(), error
 
 	b, err := NewBucket(ctx, log.NewNopLogger(), bc, "thanos-e2e-test")
 	if err != nil {
-		cancel()
 		return nil, nil, err
 	}
 
 	if err = b.bkt.Create(ctx, project, nil); err != nil {
-		cancel()
 		_ = b.Close()
 		return nil, nil, err
 	}
@@ -196,7 +195,6 @@ func NewTestBucket(t testing.TB, project string) (objstore.Bucket, func(), error
 		if err := b.bkt.Delete(ctx); err != nil {
 			t.Logf("deleting bucket failed: %s", err)
 		}
-		cancel()
 		if err := b.Close(); err != nil {
 			t.Logf("closing bucket failed: %s", err)
 		}