From 99b6d215a2e58b84862ad07198502cb6b6f807d7 Mon Sep 17 00:00:00 2001 From: Ben Reedy Date: Sun, 12 May 2024 09:27:13 +1000 Subject: [PATCH 1/8] chore(ci): order linters alphabetically Signed-off-by: Ben Reedy --- .golangci.yaml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.golangci.yaml b/.golangci.yaml index aa4ec6df..2b568be2 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -2,15 +2,15 @@ linters: disable-all: true enable: - errcheck - - revive - govet - gofmt - ineffassign - - unconvert + - loggercheck - nilnil - nilerr + - revive + - unconvert - unparam - - loggercheck issues: exclude: From c713bed4e3e617d95a608db31084ea339b2d0502 Mon Sep 17 00:00:00 2001 From: Ben Reedy Date: Sun, 12 May 2024 09:24:05 +1000 Subject: [PATCH 2/8] chore(ci): update golangci-lint to latest version Signed-off-by: Ben Reedy --- .github/workflows/lint.yml | 4 ++-- exporter.go | 4 ++-- pkg/wmi/wmi_test.go | 6 +++--- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 02fea970..74874c66 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -98,9 +98,9 @@ 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 + version: v1.58 args: "--timeout=5m" # golangci-lint action doesn't always provide helpful output, so re-run without the action for diff --git a/exporter.go b/exporter.go index 37ae7892..38e48f90 100644 --- a/exporter.go +++ b/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{ diff --git a/pkg/wmi/wmi_test.go b/pkg/wmi/wmi_test.go index dc07eacf..b6da7215 100644 --- a/pkg/wmi/wmi_test.go +++ b/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 { From a49dee606b874ea7ec69ee6a523239640c5fea8c Mon Sep 17 00:00:00 2001 From: Ben Reedy Date: Mon, 13 May 2024 08:31:11 +1000 Subject: [PATCH 3/8] perf: pre-allocate slices Signed-off-by: Ben Reedy --- .golangci.yaml | 1 + pkg/collector/collector.go | 7 ++++--- pkg/collector/dfsr/dfsr.go | 5 +++-- pkg/collector/textfile/textfile.go | 2 +- 4 files changed, 9 insertions(+), 6 deletions(-) diff --git a/.golangci.yaml b/.golangci.yaml index 2b568be2..25f72094 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -8,6 +8,7 @@ linters: - loggercheck - nilnil - nilerr + - prealloc - revive - unconvert - unparam diff --git a/pkg/collector/collector.go b/pkg/collector/collector.go index d53daa69..1a7d355b 100644 --- a/pkg/collector/collector.go +++ b/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 { diff --git a/pkg/collector/dfsr/dfsr.go b/pkg/collector/dfsr/dfsr.go index c68a5346..002d5772 100644 --- a/pkg/collector/dfsr/dfsr.go +++ b/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)) } diff --git a/pkg/collector/textfile/textfile.go b/pkg/collector/textfile/textfile.go index 6d2b238a..1ce2933f 100644 --- a/pkg/collector/textfile/textfile.go +++ b/pkg/collector/textfile/textfile.go @@ -373,7 +373,7 @@ 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) From 1239fbf7198486bd0119de9abb98e2d61143550d Mon Sep 17 00:00:00 2001 From: Ben Reedy Date: Mon, 13 May 2024 09:18:14 +1000 Subject: [PATCH 4/8] perf: run perfsprint fixes Signed-off-by: Ben Reedy --- .golangci.yaml | 1 + pkg/collector/container/container.go | 7 +++---- pkg/collector/iis/iis.go | 2 +- pkg/collector/os/os.go | 11 ++++++----- pkg/collector/service/service.go | 4 ++-- pkg/collector/textfile/textfile.go | 17 +++++++++-------- pkg/config/flatten.go | 7 +++++-- pkg/perflib/unmarshal.go | 3 ++- pkg/wmi/wmi.go | 10 +++++----- 9 files changed, 34 insertions(+), 28 deletions(-) diff --git a/.golangci.yaml b/.golangci.yaml index 25f72094..610632e0 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -8,6 +8,7 @@ linters: - loggercheck - nilnil - nilerr + - perfsprint - prealloc - revive - unconvert diff --git a/pkg/collector/container/container.go b/pkg/collector/container/container.go index 7c992fd4..29771e0b 100644 --- a/pkg/collector/container/container.go +++ b/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 } diff --git a/pkg/collector/iis/iis.go b/pkg/collector/iis/iis.go index 691fb070..1bead913 100644 --- a/pkg/collector/iis/iis.go +++ b/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) } }() diff --git a/pkg/collector/os/os.go b/pkg/collector/os/os.go index a1633729..b6407de9 100644 --- a/pkg/collector/os/os.go +++ b/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( diff --git a/pkg/collector/service/service.go b/pkg/collector/service/service.go index aa4ec829..d123fa31 100644 --- a/pkg/collector/service/service.go +++ b/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, diff --git a/pkg/collector/textfile/textfile.go b/pkg/collector/textfile/textfile.go index 1ce2933f..264e90e2 100644 --- a/pkg/collector/textfile/textfile.go +++ b/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 } } @@ -379,18 +380,18 @@ func scrapeFile(path string, log log.Logger) ([]*dto.MetricFamily, error) { 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 { diff --git a/pkg/config/flatten.go b/pkg/config/flatten.go index 382f22d9..9c97286d 100644 --- a/pkg/config/flatten.go +++ b/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 diff --git a/pkg/perflib/unmarshal.go b/pkg/perflib/unmarshal.go index e022f93e..f1bc41bc 100644 --- a/pkg/perflib/unmarshal.go +++ b/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() { diff --git a/pkg/wmi/wmi.go b/pkg/wmi/wmi.go index c46e7b73..be51a221 100644 --- a/pkg/wmi/wmi.go +++ b/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() } From 965be334bc73b0aae3bd2b2de8f40dc699ba449d Mon Sep 17 00:00:00 2001 From: Ben Reedy Date: Wed, 15 May 2024 05:54:08 +1000 Subject: [PATCH 5/8] ci: reverse default linter logic Signed-off-by: Ben Reedy --- .golangci.yaml | 112 ++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 98 insertions(+), 14 deletions(-) diff --git a/.golangci.yaml b/.golangci.yaml index 610632e0..aa33b2c9 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -1,18 +1,102 @@ linters: - disable-all: true - enable: - - errcheck - - govet - - gofmt - - ineffassign - - loggercheck - - nilnil - - nilerr - - perfsprint - - prealloc - - revive - - unconvert - - unparam + 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 + - promlinter + - protogetter + - reassign + - rowserrcheck + - sloglint + - spancheck + - sqlclosecheck + - staticcheck + - stylecheck + - tagalign + - tagliatelle + - tenv + - testableexamples + - testifylint + - testpackage + - thelper + - tparallel + - unused + - usestdlibvars + - varnamelen + - wastedassign + - whitespace + - wrapcheck + - wsl + - zerologlint + - execinquery + - gomnd issues: exclude: From a2575b93a99fab51035e8ddc0c85c0c08d51f5d3 Mon Sep 17 00:00:00 2001 From: Ben Reedy Date: Wed, 15 May 2024 05:55:37 +1000 Subject: [PATCH 6/8] feat(ci): add unused linter Signed-off-by: Ben Reedy --- .golangci.yaml | 1 - pkg/collector/smbclient/smbclient.go | 8 -------- 2 files changed, 9 deletions(-) diff --git a/.golangci.yaml b/.golangci.yaml index aa33b2c9..dd056c7b 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -87,7 +87,6 @@ linters: - testpackage - thelper - tparallel - - unused - usestdlibvars - varnamelen - wastedassign diff --git a/pkg/collector/smbclient/smbclient.go b/pkg/collector/smbclient/smbclient.go index 94cb6ded..93381b6c 100644 --- a/pkg/collector/smbclient/smbclient.go +++ b/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 -} From 8b515ba54aa25b0aeac25593e083336a0b923d32 Mon Sep 17 00:00:00 2001 From: Ben Reedy Date: Wed, 15 May 2024 06:03:49 +1000 Subject: [PATCH 7/8] feat(ci): enable promlinter Signed-off-by: Ben Reedy --- .golangci.yaml | 1 - 1 file changed, 1 deletion(-) diff --git a/.golangci.yaml b/.golangci.yaml index dd056c7b..f44c7357 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -70,7 +70,6 @@ linters: - nosprintfhostport - paralleltest - predeclared - - promlinter - protogetter - reassign - rowserrcheck From d6c24d1500d54a5de6d87852bd8fb076ee39fbb8 Mon Sep 17 00:00:00 2001 From: Ben Reedy Date: Wed, 15 May 2024 06:58:45 +1000 Subject: [PATCH 8/8] feat(ci): enable useful golangci output Signed-off-by: Ben Reedy --- .github/workflows/lint.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index 74874c66..98fd6bc7 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -101,7 +101,7 @@ jobs: uses: golangci/golangci-lint-action@v6 with: version: v1.58 - args: "--timeout=5m" + 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.