Merge pull request #1487 from breed808/ci

Additional CI linters

.github/workflows/lint.yml

@@ -98,10 +98,10 @@ jobs:
           go-version-file: 'go.mod'
 
       - name: golangci-lint
-        uses: golangci/golangci-lint-action@v3
+        uses: golangci/golangci-lint-action@v6
         with:
-          version: v1.55.2
-          args: "--timeout=5m"
+          version: v1.58
+          args: "--timeout=5m --out-format github-actions,colored-line-number"
 
       # golangci-lint action doesn't always provide helpful output, so re-run without the action for
       # better output of the problem.

.golangci.yaml

@@ -1,16 +1,100 @@
 linters:
-  disable-all: true
-  enable:
-  - errcheck
-  - revive
-  - govet
-  - gofmt
-  - ineffassign
-  - unconvert
-  - nilnil
-  - nilerr
-  - unparam
-  - loggercheck
+  enable-all: true
+  disable:
+    - asasalint
+    - asciicheck
+    - bidichk
+    - bodyclose
+    - canonicalheader
+    - containedctx
+    - contextcheck
+    - copyloopvar
+    - cyclop
+    - decorder
+    - depguard
+    - dogsled
+    - dupl
+    - dupword
+    - durationcheck
+    - err113
+    - errchkjson
+    - errname
+    - errorlint
+    - exhaustive
+    - exhaustruct
+    - exportloopref
+    - fatcontext
+    - forbidigo
+    - forcetypeassert
+    - funlen
+    - gci
+    - ginkgolinter
+    - gocheckcompilerdirectives
+    - gochecknoglobals
+    - gochecknoinits
+    - gochecksumtype
+    - gocognit
+    - goconst
+    - gocritic
+    - gocyclo
+    - godot
+    - godox
+    - gofumpt
+    - goheader
+    - goimports
+    - gomoddirectives
+    - gomodguard
+    - goprintffuncname
+    - gosec
+    - gosimple
+    - gosmopolitan
+    - grouper
+    - importas
+    - inamedparam
+    - interfacebloat
+    - intrange
+    - ireturn
+    - lll
+    - maintidx
+    - makezero
+    - mirror
+    - misspell
+    - mnd
+    - musttag
+    - nakedret
+    - nestif
+    - nlreturn
+    - noctx
+    - nolintlint
+    - nonamedreturns
+    - nosprintfhostport
+    - paralleltest
+    - predeclared
+    - protogetter
+    - reassign
+    - rowserrcheck
+    - sloglint
+    - spancheck
+    - sqlclosecheck
+    - staticcheck
+    - stylecheck
+    - tagalign
+    - tagliatelle
+    - tenv
+    - testableexamples
+    - testifylint
+    - testpackage
+    - thelper
+    - tparallel
+    - usestdlibvars
+    - varnamelen
+    - wastedassign
+    - whitespace
+    - wrapcheck
+    - wsl
+    - zerologlint
+    - execinquery
+    - gomnd
 
 issues:
   exclude:

exporter.go

@@ -180,14 +180,14 @@ func main() {
 
 	mux := http.NewServeMux()
 	mux.HandleFunc(*metricsPath, withConcurrencyLimit(*maxRequests, collectors.BuildServeHTTP(*disableExporterMetrics, *timeoutMargin)))
-	mux.HandleFunc("/health", func(w http.ResponseWriter, r *http.Request) {
+	mux.HandleFunc("/health", func(w http.ResponseWriter, _ *http.Request) {
 		w.Header().Set("Content-Type", "application/json")
 		_, err := fmt.Fprintln(w, `{"status":"ok"}`)
 		if err != nil {
 			_ = level.Debug(logger).Log("msg", "Failed to write to stream", "err", err)
 		}
 	})
-	mux.HandleFunc("/version", func(w http.ResponseWriter, r *http.Request) {
+	mux.HandleFunc("/version", func(w http.ResponseWriter, _ *http.Request) {
 		// we can't use "version" directly as it is a package, and not an object that
 		// can be serialized.
 		err := json.NewEncoder(w).Encode(prometheusVersion{

pkg/collector/collector.go

@@ -163,11 +163,12 @@ func (c *Collectors) SetPerfCounterQuery() error {
 	var (
 		err error
 
-		perfCounterDependencies []string
-		perfCounterNames        []string
-		perfIndicies            []string
+		perfCounterNames []string
+		perfIndicies     []string
 	)
 
+	perfCounterDependencies := make([]string, 0, len(c.collectors))
+
 	for _, collector := range c.collectors {
 		perfCounterNames, err = collector.GetPerfCounter()
 		if err != nil {

pkg/collector/container/container.go

@@ -3,7 +3,6 @@
 package container
 
 import (
-	"fmt"
 	"strings"
 
 	"github.com/Microsoft/hcsshim"
@@ -326,14 +325,14 @@ func (c *collector) collect(ch chan<- prometheus.Metric) error {
 	}
 
 	if len(hnsEndpoints) == 0 {
-		_ = level.Info(c.logger).Log("msg", fmt.Sprintf("No network stats for containers to collect"))
+		_ = level.Info(c.logger).Log("msg", "No network stats for containers to collect")
 		return nil
 	}
 
 	for _, endpoint := range hnsEndpoints {
 		endpointStats, err := hcsshim.GetHNSEndpointStats(endpoint.Id)
 		if err != nil {
-			_ = level.Warn(c.logger).Log("msg", fmt.Sprintf("Failed to collect network stats for interface %s", endpoint.Id), "err", err)
+			_ = level.Warn(c.logger).Log("msg", "Failed to collect network stats for interface "+endpoint.Id, "err", err)
 			continue
 		}
 
@@ -342,7 +341,7 @@ func (c *collector) collect(ch chan<- prometheus.Metric) error {
 			endpointId := strings.ToUpper(endpoint.Id)
 
 			if !ok {
-				_ = level.Warn(c.logger).Log("msg", fmt.Sprintf("Failed to collect network stats for container %s", containerId))
+				_ = level.Warn(c.logger).Log("msg", "Failed to collect network stats for container "+containerId)
 				continue
 			}
 

pkg/collector/dfsr/dfsr.go

@@ -130,8 +130,9 @@ func (c *collector) SetLogger(logger log.Logger) {
 
 func (c *collector) GetPerfCounter() ([]string, error) {
 	// Perflib sources are dynamic, depending on the enabled child collectors
-	var perflibDependencies []string
-	for _, source := range utils.ExpandEnabledChildCollectors(*c.dfsrEnabledCollectors) {
+	expandedChildCollectors := utils.ExpandEnabledChildCollectors(*c.dfsrEnabledCollectors)
+	perflibDependencies := make([]string, 0, len(expandedChildCollectors))
+	for _, source := range expandedChildCollectors {
 		perflibDependencies = append(perflibDependencies, dfsrGetPerfObjectName(source))
 	}
 

pkg/collector/iis/iis.go

@@ -54,7 +54,7 @@ func getIISVersion(logger log.Logger) simple_version {
 	defer func() {
 		err = k.Close()
 		if err != nil {
-			_ = level.Warn(logger).Log("msg", fmt.Sprintf("Failed to close registry key"), "err", err)
+			_ = level.Warn(logger).Log("msg", "Failed to close registry key", "err", err)
 		}
 	}()
 

pkg/collector/os/os.go

@@ -6,6 +6,7 @@ import (
 	"errors"
 	"fmt"
 	"os"
+	"strconv"
 	"strings"
 	"time"
 
@@ -269,12 +270,12 @@ func (c *collector) collect(ctx *types.ScrapeContext, ch chan<- prometheus.Metri
 		c.OSInformation,
 		prometheus.GaugeValue,
 		1.0,
-		fmt.Sprintf("Microsoft %s", pn), // Caption
+		"Microsoft "+pn, // Caption
 		fmt.Sprintf("%d.%d.%s", nwgi.VersionMajor, nwgi.VersionMinor, bn), // Version
-		fmt.Sprintf("%d", nwgi.VersionMajor),                              // Major Version
-		fmt.Sprintf("%d", nwgi.VersionMinor),                              // Minor Version
-		bn,                                                                // Build number
-		fmt.Sprintf("%d", revision),                                       // Revision
+		strconv.FormatUint(uint64(nwgi.VersionMajor), 10),                 // Major Version
+		strconv.FormatUint(uint64(nwgi.VersionMinor), 10),                 // Minor Version
+		bn,                               // Build number
+		strconv.FormatUint(revision, 10), // Revision
 	)
 
 	ch <- prometheus.MustNewConstMetric(

pkg/collector/service/service.go

@@ -210,7 +210,7 @@ func (c *collector) collectWMI(ch chan<- prometheus.Metric) error {
 		return err
 	}
 	for _, service := range dst {
-		pid := fmt.Sprintf("%d", uint64(service.ProcessId))
+		pid := strconv.FormatUint(uint64(service.ProcessId), 10)
 
 		runAs := ""
 		if service.StartName != nil {
@@ -319,7 +319,7 @@ func (c *collector) collectAPI(ch chan<- prometheus.Metric) error {
 				return
 			}
 
-			pid := fmt.Sprintf("%d", uint64(serviceStatus.ProcessId))
+			pid := strconv.FormatUint(uint64(serviceStatus.ProcessId), 10)
 
 			ch <- prometheus.MustNewConstMetric(
 				c.Information,

pkg/collector/smbclient/smbclient.go

@@ -285,7 +285,6 @@ func (c *collector) collectClientShares(ctx *types.ScrapeContext, ch chan<- prom
 		return err
 	}
 	for _, instance := range data {
-		// labelName := c.toLabelName(instance.Name)
 		if instance.Name == "_Total" {
 			continue
 		}
@@ -446,10 +445,3 @@ func (c *collector) collectClientShares(ctx *types.ScrapeContext, ch chan<- prom
 	}
 	return nil
 }
-
-// toLabelName converts strings to lowercase and replaces all whitespaces and dots with underscores
-func (c *collector) toLabelName(name string) string {
-	s := strings.ReplaceAll(strings.Join(strings.Fields(strings.ToLower(name)), "_"), ".", "_")
-	s = strings.ReplaceAll(s, "__", "_")
-	return s
-}

pkg/collector/textfile/textfile.go

@@ -16,6 +16,7 @@
 package textfile
 
 import (
+	"errors"
 	"fmt"
 	"io"
 	"os"
@@ -100,7 +101,7 @@ func (c *collector) Build() error {
 		c.directories = strings.Trim(*c.textFileDirectories, ",")
 	}
 
-	_ = level.Info(c.logger).Log("msg", fmt.Sprintf("textfile collector directories: %s", c.directories))
+	_ = level.Info(c.logger).Log("msg", "textfile collector directories: "+c.directories)
 
 	c.MtimeDesc = prometheus.NewDesc(
 		prometheus.BuildFQName(types.Namespace, "textfile", "mtime_seconds"),
@@ -296,12 +297,12 @@ func (c *collector) Collect(_ *types.ScrapeContext, ch chan<- prometheus.Metric)
 	for _, directory := range strings.Split(c.directories, ",") {
 		err := filepath.WalkDir(directory, func(path string, dirEntry os.DirEntry, err error) error {
 			if err != nil {
-				_ = level.Error(c.logger).Log("msg", fmt.Sprintf("Error reading directory: %s", path), "err", err)
+				_ = level.Error(c.logger).Log("msg", "Error reading directory: "+path, "err", err)
 				errorMetric = 1.0
 				return nil
 			}
 			if !dirEntry.IsDir() && strings.HasSuffix(dirEntry.Name(), ".prom") {
-				_ = level.Debug(c.logger).Log("msg", fmt.Sprintf("Processing file: %s", path))
+				_ = level.Debug(c.logger).Log("msg", "Processing file: "+path)
 				families_array, err := scrapeFile(path, c.logger)
 				if err != nil {
 					_ = level.Error(c.logger).Log("msg", fmt.Sprintf("Error scraping file: %q. Skip File.", path), "err", err)
@@ -325,7 +326,7 @@ func (c *collector) Collect(_ *types.ScrapeContext, ch chan<- prometheus.Metric)
 			return nil
 		})
 		if err != nil && directory != "" {
-			_ = level.Error(c.logger).Log("msg", fmt.Sprintf("Error reading textfile collector directory: %s", c.directories), "err", err)
+			_ = level.Error(c.logger).Log("msg", "Error reading textfile collector directory: "+c.directories, "err", err)
 			errorMetric = 1.0
 		}
 	}
@@ -373,24 +374,24 @@ func scrapeFile(path string, log log.Logger) ([]*dto.MetricFamily, error) {
 	}
 
 	// Use temporary array to check for duplicates
-	var families_array []*dto.MetricFamily
+	families_array := make([]*dto.MetricFamily, 0, len(parsedFamilies))
 
 	for _, mf := range parsedFamilies {
 		families_array = append(families_array, mf)
 		for _, m := range mf.Metric {
 			if m.TimestampMs != nil {
-				return nil, fmt.Errorf("textfile contains unsupported client-side timestamps")
+				return nil, errors.New("textfile contains unsupported client-side timestamps")
 			}
 		}
 		if mf.Help == nil {
-			help := fmt.Sprintf("Metric read from %s", path)
+			help := "Metric read from " + path
 			mf.Help = &help
 		}
 	}
 
 	// If duplicate metrics are detected in a *single* file, skip processing of file metrics
 	if duplicateMetricEntry(families_array) {
-		return nil, fmt.Errorf("duplicate metrics detected")
+		return nil, errors.New("duplicate metrics detected")
 	}
 	return families_array, nil
 }
@@ -400,7 +401,7 @@ func checkBOM(encoding utfbom.Encoding) error {
 		return nil
 	}
 
-	return fmt.Errorf(encoding.String())
+	return errors.New(encoding.String())
 }
 
 func getDefaultPath() string {

pkg/config/flatten.go

@@ -1,6 +1,9 @@
 package config
 
-import "fmt"
+import (
+	"fmt"
+	"strconv"
+)
 
 // flatten flattens the nested struct.
 //
@@ -46,7 +49,7 @@ func flattenSlice(data []interface{}) map[string]string {
 				ret[fmt.Sprintf("%d,%s", idx, fk)] = fv
 			}
 		default:
-			ret[fmt.Sprint(idx)] = fmt.Sprint(typed)
+			ret[strconv.Itoa(idx)] = fmt.Sprint(typed)
 		}
 	}
 	return ret

pkg/perflib/unmarshal.go

@@ -1,6 +1,7 @@
 package perflib
 
 import (
+	"errors"
 	"fmt"
 	"reflect"
 	"strings"
@@ -17,7 +18,7 @@ const (
 
 func UnmarshalObject(obj *PerfObject, vs interface{}, logger log.Logger) error {
 	if obj == nil {
-		return fmt.Errorf("counter not found")
+		return errors.New("counter not found")
 	}
 	rv := reflect.ValueOf(vs)
 	if rv.Kind() != reflect.Ptr || rv.IsNil() {

pkg/wmi/wmi.go

@@ -2,7 +2,7 @@ package wmi
 
 import (
 	"bytes"
-	"fmt"
+
 	"reflect"
 
 	"github.com/go-kit/log"
@@ -47,7 +47,7 @@ func QueryAll(src interface{}, logger log.Logger) string {
 	b.WriteString("SELECT * FROM ")
 	b.WriteString(className(src))
 
-	_ = level.Debug(logger).Log("msg", fmt.Sprintf("Generated WMI query %s", b.String()))
+	_ = level.Debug(logger).Log("msg", "Generated WMI query "+b.String())
 	return b.String()
 }
 
@@ -56,7 +56,7 @@ func QueryAllForClass(_ interface{}, class string, logger log.Logger) string {
 	b.WriteString("SELECT * FROM ")
 	b.WriteString(class)
 
-	_ = level.Debug(logger).Log("msg", fmt.Sprintf("Generated WMI query %s", b.String()))
+	_ = level.Debug(logger).Log("msg", "Generated WMI query "+b.String())
 	return b.String()
 }
 
@@ -70,7 +70,7 @@ func QueryAllWhere(src interface{}, where string, logger log.Logger) string {
 		b.WriteString(where)
 	}
 
-	_ = level.Debug(logger).Log("msg", fmt.Sprintf("Generated WMI query %s", b.String()))
+	_ = level.Debug(logger).Log("msg", "Generated WMI query "+b.String())
 	return b.String()
 }
 
@@ -84,6 +84,6 @@ func QueryAllForClassWhere(_ interface{}, class string, where string, logger log
 		b.WriteString(where)
 	}
 
-	_ = level.Debug(logger).Log("msg", fmt.Sprintf("Generated WMI query %s", b.String()))
+	_ = level.Debug(logger).Log("msg", "Generated WMI query "+b.String())
 	return b.String()
 }

pkg/wmi/wmi_test.go

@@ -12,13 +12,13 @@ type fakeWmiClass struct {
 }
 
 var (
-	mapQueryAll = func(src interface{}, class string, where string) string {
+	mapQueryAll = func(src interface{}, _ string, _ string) string {
 		return QueryAll(src, log.NewNopLogger())
 	}
-	mapQueryAllWhere = func(src interface{}, class string, where string) string {
+	mapQueryAllWhere = func(src interface{}, _ string, where string) string {
 		return QueryAllWhere(src, where, log.NewNopLogger())
 	}
-	mapQueryAllForClass = func(src interface{}, class string, where string) string {
+	mapQueryAllForClass = func(src interface{}, class string, _ string) string {
 		return QueryAllForClass(src, class, log.NewNopLogger())
 	}
 	mapQueryAllForClassWhere = func(src interface{}, class string, where string) string {