Merge pull request #815 from drawks/linting-fixes

Updates for automated linting config and various code fixes to match

Author: deniszh

.github/workflows/golangci-lint.yml

@@ -11,10 +11,10 @@ jobs:
     steps:
       - uses: actions/checkout@v5
       - name: golangci-lint
-        uses: golangci/golangci-lint-action@v6
+        uses: golangci/golangci-lint-action@v9
         with:
           # Required: the version of golangci-lint is required and must be specified without patch version: we always use the latest patch version.
-          version: v1.64.7
+          version: v2.8.0
 
           # Optional: working directory, useful for monorepos
           # working-directory: somedir

.golangci.yml

@@ -1,18 +1,20 @@
+version: "2"
 linters:
-  disable-all: true
+  default: none
   enable:
-    - gofmt
-    - goimports
     - gocritic
     - ineffassign
+    - asciicheck
+    - misspell
+    - promlinter
+    - errorlint
+    - govet
+    - unparam
+    - bodyclose
+    - gochecknoinits
 
     # enabled in carbonapi
-    # - asciicheck
-    # - bodyclose
-    # - gochecknoinits
     # - gosec
-    # - misspell
-    # - unparam
 
     # The linters we need to enable ASAP.
     # Enabling would require significant changes.
@@ -23,9 +25,29 @@ linters:
     # The linters that would be nice to have in order of decreasing priority.
     # They are disabled now because they cause a lot of warnings that would require multiple changes.
     # - revive
-    # - errorlint
-    # - promlinter
     # - cyclop or gocyclo
     # - whitespace
     # - gomnd
     # - lll
+
+  exclusions:
+    generated: lax
+    presets:
+      - comments
+      - common-false-positives
+      - legacy
+      - std-error-handling
+    paths:
+      - third_party$
+      - builtin$
+      - examples$
+formatters:
+  enable:
+    - gofmt
+    - goimports
+  exclusions:
+    generated: lax
+    paths:
+      - third_party$
+      - builtin$
+      - examples$

carbon/app.go

@@ -24,12 +24,12 @@ import (
 	"github.com/go-graphite/go-carbon/tags"
 	"github.com/lomik/zapwriter"
 
-	// register receivers
-	_ "github.com/go-graphite/go-carbon/receiver/http"
-	_ "github.com/go-graphite/go-carbon/receiver/kafka"
-	_ "github.com/go-graphite/go-carbon/receiver/pubsub"
-	_ "github.com/go-graphite/go-carbon/receiver/tcp"
-	_ "github.com/go-graphite/go-carbon/receiver/udp"
+	// receivers
+	http_receiver "github.com/go-graphite/go-carbon/receiver/http"
+	kafka_receiver "github.com/go-graphite/go-carbon/receiver/kafka"
+	pubsub_receiver "github.com/go-graphite/go-carbon/receiver/pubsub"
+	tcp_receiver "github.com/go-graphite/go-carbon/receiver/tcp"
+	udp_receiver "github.com/go-graphite/go-carbon/receiver/udp"
 )
 
 type NamedReceiver struct {
@@ -68,6 +68,8 @@ type App struct {
 	FlushTraces    func()
 }
 
+var registerPluginsOnce sync.Once
+
 // New App instance
 func New(configFilename string) *App {
 	app := &App{
@@ -77,6 +79,15 @@ func New(configFilename string) *App {
 		exit:           make(chan bool),
 	}
 
+	// Register all receivers explicitly
+	registerPluginsOnce.Do(func() {
+		http_receiver.Register()
+		kafka_receiver.Register()
+		pubsub_receiver.Register()
+		tcp_receiver.Register()
+		udp_receiver.Register()
+	})
+
 	return app
 }
 
@@ -456,7 +467,7 @@ func (app *App) Start() (err error) {
 
 		if conf.Carbonserver.TrigramIndex || conf.Carbonserver.TrieIndex {
 			if fi, err := os.Lstat(conf.Whisper.DataDir); err != nil {
-				return fmt.Errorf("failed to stat whisper data directory: %s", err)
+				return fmt.Errorf("failed to stat whisper data directory: %w", err)
 			} else if fi.Mode()&os.ModeSymlink == 1 {
 				return fmt.Errorf("whisper data directory is a symlink")
 			}
@@ -474,7 +485,7 @@ func (app *App) Start() (err error) {
 		for _, rl := range conf.Carbonserver.HeavyGlobQueryRateLimiters {
 			gqrl, err := carbonserver.NewGlobQueryRateLimiter(rl.Pattern, rl.MaxInflightRequests)
 			if err != nil {
-				return fmt.Errorf("failed to init Carbonserver.HeavyGlobQueryRateLimiters %s: %s", rl.Pattern, err)
+				return fmt.Errorf("failed to init Carbonserver.HeavyGlobQueryRateLimiters %s: %w", rl.Pattern, err)
 			}
 			globQueryRateLimiters = append(globQueryRateLimiters, gqrl)
 		}

carbonserver/cache_index_test.go

@@ -168,7 +168,7 @@ func (f *testInfo) commonCacheIdxTestHelper(t *testing.T) {
 
 	// assert [4] comes first, it has the smallest timestamp
 	if !p1.Eq(points.OnePoint(addMetrics[4], 4, timestamps[4])) {
-		fmt.Printf("error - recived wrong point - %v\n", p1)
+		fmt.Printf("error - received wrong point - %v\n", p1)
 		t.FailNow()
 	}
 
@@ -191,7 +191,7 @@ func (f *testInfo) commonCacheIdxTestHelper(t *testing.T) {
 
 	// assert [0] comes second, it has the second smallest timestamp
 	if !p2.Eq(points.OnePoint(addMetrics[0], 0, timestamps[0])) {
-		fmt.Printf("error - recived wrong point - %v\n", p2)
+		fmt.Printf("error - received wrong point - %v\n", p2)
 		t.FailNow()
 	}
 	f.checkExpandGlobs(t, addMetrics[0], true)

carbonserver/carbonserver.go

@@ -147,7 +147,7 @@ var statusCodes = map[string][]uint64{
 	"capabilities": make([]uint64, 5),
 }
 
-// interface to retrive retention and aggregation
+// interface to retrieve retention and aggregation
 // schema from persister.
 type configRetriever interface {
 	MetricRetentionPeriod(string) (int, bool)
@@ -520,14 +520,14 @@ func (listener *CarbonserverListener) SetDoNotLog404s(doNotLog404s bool) {
 func (listener *CarbonserverListener) SetFailOnMaxGlobs(failOnMaxGlobs bool) {
 	listener.failOnMaxGlobs = failOnMaxGlobs
 }
-func (listener *CarbonserverListener) SetMaxMetricsGlobbed(max int) {
-	listener.maxMetricsGlobbed = max
+func (listener *CarbonserverListener) SetMaxMetricsGlobbed(m int) {
+	listener.maxMetricsGlobbed = m
 }
-func (listener *CarbonserverListener) SetMaxMetricsRendered(max int) {
-	listener.maxMetricsRendered = max
+func (listener *CarbonserverListener) SetMaxMetricsRendered(m int) {
+	listener.maxMetricsRendered = m
 }
-func (listener *CarbonserverListener) SetMaxFetchDataGoroutines(max int) {
-	listener.maxFetchDataGoroutines = max
+func (listener *CarbonserverListener) SetMaxFetchDataGoroutines(m int) {
+	listener.maxFetchDataGoroutines = m
 }
 func (listener *CarbonserverListener) SetFLock(flock bool) {
 	listener.flock = flock
@@ -637,8 +637,8 @@ func (listener *CarbonserverListener) ShouldThrottleMetric(ps *points.Points, in
 
 	return throttled
 }
-func (listener *CarbonserverListener) SetMaxInflightRequests(max uint64) {
-	listener.MaxInflightRequests = max
+func (listener *CarbonserverListener) SetMaxInflightRequests(m uint64) {
+	listener.MaxInflightRequests = m
 }
 func (listener *CarbonserverListener) SetNoServiceWhenIndexIsNotReady(no bool) {
 	listener.NoServiceWhenIndexIsNotReady = no
@@ -964,13 +964,14 @@ func (listener *CarbonserverListener) updateFileList(dir string, cacheMetricName
 			readFromCache = true
 			for {
 				entry, err := flc.Read()
+				if errors.Is(err, io.EOF) {
+					break
+				}
 				if err != nil {
-					if !errors.Is(err, io.EOF) {
-						infos = append(infos, zap.NamedError("file_list_cache_read_error", err))
+					infos = append(infos, zap.NamedError("file_list_cache_read_error", err))
 
-						readFromCache = false
-						trieIdx = newTrie(".wsp", listener.maxCreatesPerSecond, listener.estimateSize)
-					}
+					readFromCache = false
+					trieIdx = newTrie(".wsp", listener.maxCreatesPerSecond, listener.estimateSize)
 
 					break
 				}
@@ -1258,7 +1259,8 @@ func (listener *CarbonserverListener) updateFileList(dir string, cacheMetricName
 
 func (*CarbonserverListener) logTrieInsertError(logger *zap.Logger, msg, metric string, err error) {
 	zfields := []zap.Field{zap.Error(err), zap.String("metric", metric)}
-	if ierr, ok := err.(*trieInsertError); ok {
+	var ierr *trieInsertError
+	if errors.As(err, &ierr) {
 		zfields = append(zfields, zap.String("err_info", ierr.info))
 	}
 	logger.Error(msg, zfields...)
@@ -2053,13 +2055,13 @@ type GlobQueryRateLimiter struct {
 	maxInflightRequests chan struct{}
 }
 
-func NewGlobQueryRateLimiter(pattern string, max uint) (*GlobQueryRateLimiter, error) {
+func NewGlobQueryRateLimiter(pattern string, m uint) (*GlobQueryRateLimiter, error) {
 	exp, err := regexp.Compile(pattern)
 	if err != nil {
 		return nil, err
 	}
 
-	return &GlobQueryRateLimiter{pattern: exp, maxInflightRequests: make(chan struct{}, max)}, nil
+	return &GlobQueryRateLimiter{pattern: exp, maxInflightRequests: make(chan struct{}, m)}, nil
 }
 
 type ApiPerPathRatelimiter struct {

carbonserver/carbonserver_test.go

@@ -1,6 +1,7 @@
 package carbonserver
 
 import (
+	"errors"
 	"fmt"
 	"math"
 	"os"
@@ -489,7 +490,7 @@ func TestGetMetricsListEmpty(t *testing.T) {
 	}
 
 	metrics, err := carbonserver.getMetricsList()
-	if err != errMetricsListEmpty {
+	if !errors.Is(err, errMetricsListEmpty) {
 		t.Errorf("err: '%v', expected: '%v'", err, errMetricsListEmpty)
 	}
 	if metrics != nil {

carbonserver/details.go

@@ -107,7 +107,7 @@ func (listener *CarbonserverListener) detailsHandler(wr http.ResponseWriter, req
 			zap.String("reason", "response encode failed"),
 			zap.Error(err),
 		)
-		http.Error(wr, fmt.Sprintf("An internal error has occured: %s", err), http.StatusInternalServerError)
+		http.Error(wr, fmt.Sprintf("An internal error has occurred: %s", err), http.StatusInternalServerError)
 		return
 	}
 	wr.Header().Set("Content-Type", contentType)

carbonserver/find.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io"
 	"net/http"
@@ -149,7 +150,8 @@ func (listener *CarbonserverListener) findHandler(wr http.ResponseWriter, req *h
 	if err != nil || response == nil {
 		var code int
 		var reason string
-		if _, ok := err.(errorNotFound); ok {
+		var nf errorNotFound
+		if errors.Is(err, &nf) {
 			reason = "Not Found"
 			code = http.StatusNotFound
 		} else {
@@ -328,7 +330,7 @@ GATHER:
 			}
 			glob.Files, glob.Leafs, glob.TrieNodes, glob.Lookups, err = expandedResult.Files, expandedResult.Leafs, expandedResult.TrieNodes, expandedResult.Lookups, expandedResult.Err
 			if err != nil {
-				errors = append(errors, fmt.Errorf("%s: %s", expandedResult.Name, err))
+				errors = append(errors, fmt.Errorf("%s: %w", expandedResult.Name, err))
 			}
 			expandedGlobs = append(expandedGlobs, glob)
 			if responseCount == len(names) {
@@ -492,7 +494,8 @@ func (listener *CarbonserverListener) Find(ctx context.Context, req *protov2.Glo
 	if err != nil || finalRes == nil {
 		var code codes.Code
 		var reason string
-		if _, ok := err.(errorNotFound); ok {
+		var nf errorNotFound
+		if errors.As(err, &nf) {
 			reason = "Not Found"
 			code = codes.NotFound
 		} else {

carbonserver/flc.go

@@ -112,10 +112,10 @@ func ReadFileListCache(p string, version FLCVersion, writer io.Writer) error {
 
 	for {
 		entry, err := flc.Read()
+		if errors.Is(err, io.EOF) {
+			return nil
+		}
 		if err != nil {
-			if errors.Is(err, io.EOF) {
-				return nil
-			}
 			return err
 		}
 
@@ -160,7 +160,7 @@ func NewFileListCache(p string, version FLCVersion, mode byte) (FileListCache, e
 		case FLCVersionUnspecified, FLCVersion2:
 			flcc.version = FLCVersion2                  // for supporting FLCVersionUnspecified
 			flc, err = newFileListCacheV2ReadOnly(flcc) // flcc is already closed if there is error.
-			if err != nil && errors.Is(err, errFLCFallbackToV1) {
+			if errors.Is(err, errFLCFallbackToV1) {
 				// transparently detect which cache version is it.
 				return NewFileListCache(p, FLCVersion1, mode)
 			}

carbonserver/list.go

@@ -124,7 +124,7 @@ func (listener *CarbonserverListener) listHandler(wr http.ResponseWriter, req *h
 			zap.String("reason", "response encode failed"),
 			zap.Error(err),
 		)
-		http.Error(wr, fmt.Sprintf("An internal error has occured: %s", err), http.StatusInternalServerError)
+		http.Error(wr, fmt.Sprintf("An internal error has occurred: %s", err), http.StatusInternalServerError)
 		return
 	}
 	wr.Header().Set("Content-Type", contentType)
@@ -295,7 +295,7 @@ func (listener *CarbonserverListener) listQueryHandler(wr http.ResponseWriter, r
 			zap.String("reason", "response encode failed"),
 			zap.Error(err),
 		)
-		http.Error(wr, fmt.Sprintf("An internal error has occured: %s", err), http.StatusInternalServerError)
+		http.Error(wr, fmt.Sprintf("An internal error has occurred: %s", err), http.StatusInternalServerError)
 		return
 	}
 	wr.Header().Set("Content-Type", contentType)