Add more linters (#733)

Author: lucacome

.github/PULL_REQUEST_TEMPLATE.md

@@ -7,8 +7,7 @@ that issue here in this description (not in the title of the PR).
 
 Before creating a PR, run through this checklist and mark each as complete.
 
-- [ ] I have read the [CONTRIBUTING](https://github.com/nginxinc/nginx-prometheus-exporter/blob/main/CONTRIBUTING.md)
-  guide
+- [ ] I have read the [CONTRIBUTING](https://github.com/nginxinc/nginx-prometheus-exporter/blob/main/CONTRIBUTING.md) guide
 - [ ] I have proven my fix is effective or that my feature works
 - [ ] I have checked that all unit tests pass after adding my changes
 - [ ] I have ensured the README is up to date

.golangci.yml

@@ -14,7 +14,6 @@ linters-settings:
       - name: error-strings
       - name: errorf
       - name: exported
-      - name: if-return
       - name: increment-decrement
       - name: indent-error-flow
       - name: package-comments
@@ -32,41 +31,57 @@ linters-settings:
     exclude-functions:
       - (github.com/go-kit/log.Logger).Log
   govet:
-    check-shadowing: true
     enable-all: true
-
 linters:
   enable:
+    - asasalint
     - asciicheck
     - bidichk
     - dupword
     - errcheck
+    - errname
     - errorlint
+    - exportloopref
+    - fatcontext
+    - forcetypeassert
+    - gocheckcompilerdirectives
+    - godot
     - gofmt
     - gofumpt
     - goimports
     - gosec
     - gosimple
+    - gosmopolitan
     - govet
     - ineffassign
+    - intrange
     - loggercheck
     - makezero
     - misspell
     - nilerr
     - noctx
+    - nolintlint
+    - paralleltest
     - perfsprint
+    - prealloc
     - predeclared
     - promlinter
+    - reassign
     - revive
     - staticcheck
+    - stylecheck
     - tagalign
+    - tenv
+    - thelper
     - tparallel
     - typecheck
     - unconvert
     - unparam
     - unused
     - usestdlibvars
     - wastedassign
+    - whitespace
+    - wrapcheck
   disable-all: true
 issues:
   max-issues-per-linter: 0

collector/nginx_plus.go

@@ -11,7 +11,7 @@ import (
 	"github.com/prometheus/client_golang/prometheus"
 )
 
-// LabelUpdater updates the labels of upstream server and server zone metrics
+// LabelUpdater updates the labels of upstream server and server zone metrics.
 type LabelUpdater interface {
 	UpdateUpstreamServerPeerLabels(upstreamServerPeerLabels map[string][]string)
 	DeleteUpstreamServerPeerLabels(peers []string)
@@ -64,7 +64,7 @@ type NginxPlusCollector struct {
 	mutex                          sync.Mutex
 }
 
-// UpdateUpstreamServerPeerLabels updates the Upstream Server Peer Labels
+// UpdateUpstreamServerPeerLabels updates the Upstream Server Peer Labels.
 func (c *NginxPlusCollector) UpdateUpstreamServerPeerLabels(upstreamServerPeerLabels map[string][]string) {
 	c.variableLabelsMutex.Lock()
 	for k, v := range upstreamServerPeerLabels {
@@ -73,7 +73,7 @@ func (c *NginxPlusCollector) UpdateUpstreamServerPeerLabels(upstreamServerPeerLa
 	c.variableLabelsMutex.Unlock()
 }
 
-// DeleteUpstreamServerPeerLabels deletes the Upstream Server Peer Labels
+// DeleteUpstreamServerPeerLabels deletes the Upstream Server Peer Labels.
 func (c *NginxPlusCollector) DeleteUpstreamServerPeerLabels(peers []string) {
 	c.variableLabelsMutex.Lock()
 	for _, k := range peers {
@@ -82,7 +82,7 @@ func (c *NginxPlusCollector) DeleteUpstreamServerPeerLabels(peers []string) {
 	c.variableLabelsMutex.Unlock()
 }
 
-// UpdateStreamUpstreamServerPeerLabels updates the Upstream Server Peer Labels
+// UpdateStreamUpstreamServerPeerLabels updates the Upstream Server Peer Labels.
 func (c *NginxPlusCollector) UpdateStreamUpstreamServerPeerLabels(streamUpstreamServerPeerLabels map[string][]string) {
 	c.variableLabelsMutex.Lock()
 	for k, v := range streamUpstreamServerPeerLabels {
@@ -91,7 +91,7 @@ func (c *NginxPlusCollector) UpdateStreamUpstreamServerPeerLabels(streamUpstream
 	c.variableLabelsMutex.Unlock()
 }
 
-// DeleteStreamUpstreamServerPeerLabels deletes the Upstream Server Peer Labels
+// DeleteStreamUpstreamServerPeerLabels deletes the Upstream Server Peer Labels.
 func (c *NginxPlusCollector) DeleteStreamUpstreamServerPeerLabels(peers []string) {
 	c.variableLabelsMutex.Lock()
 	for _, k := range peers {
@@ -100,7 +100,7 @@ func (c *NginxPlusCollector) DeleteStreamUpstreamServerPeerLabels(peers []string
 	c.variableLabelsMutex.Unlock()
 }
 
-// UpdateUpstreamServerLabels updates the Upstream Server Labels
+// UpdateUpstreamServerLabels updates the Upstream Server Labels.
 func (c *NginxPlusCollector) UpdateUpstreamServerLabels(upstreamServerLabelValues map[string][]string) {
 	c.variableLabelsMutex.Lock()
 	for k, v := range upstreamServerLabelValues {
@@ -109,7 +109,7 @@ func (c *NginxPlusCollector) UpdateUpstreamServerLabels(upstreamServerLabelValue
 	c.variableLabelsMutex.Unlock()
 }
 
-// DeleteUpstreamServerLabels deletes the Upstream Server Labels
+// DeleteUpstreamServerLabels deletes the Upstream Server Labels.
 func (c *NginxPlusCollector) DeleteUpstreamServerLabels(upstreamNames []string) {
 	c.variableLabelsMutex.Lock()
 	for _, k := range upstreamNames {
@@ -118,7 +118,7 @@ func (c *NginxPlusCollector) DeleteUpstreamServerLabels(upstreamNames []string)
 	c.variableLabelsMutex.Unlock()
 }
 
-// UpdateStreamUpstreamServerLabels updates the Upstream Server Labels
+// UpdateStreamUpstreamServerLabels updates the Upstream Server Labels.
 func (c *NginxPlusCollector) UpdateStreamUpstreamServerLabels(streamUpstreamServerLabelValues map[string][]string) {
 	c.variableLabelsMutex.Lock()
 	for k, v := range streamUpstreamServerLabelValues {
@@ -127,7 +127,7 @@ func (c *NginxPlusCollector) UpdateStreamUpstreamServerLabels(streamUpstreamServ
 	c.variableLabelsMutex.Unlock()
 }
 
-// DeleteStreamUpstreamServerLabels deletes the Upstream Server Labels
+// DeleteStreamUpstreamServerLabels deletes the Upstream Server Labels.
 func (c *NginxPlusCollector) DeleteStreamUpstreamServerLabels(streamUpstreamNames []string) {
 	c.variableLabelsMutex.Lock()
 	for _, k := range streamUpstreamNames {
@@ -136,7 +136,7 @@ func (c *NginxPlusCollector) DeleteStreamUpstreamServerLabels(streamUpstreamName
 	c.variableLabelsMutex.Unlock()
 }
 
-// UpdateServerZoneLabels updates the Server Zone Labels
+// UpdateServerZoneLabels updates the Server Zone Labels.
 func (c *NginxPlusCollector) UpdateServerZoneLabels(serverZoneLabelValues map[string][]string) {
 	c.variableLabelsMutex.Lock()
 	for k, v := range serverZoneLabelValues {
@@ -145,7 +145,7 @@ func (c *NginxPlusCollector) UpdateServerZoneLabels(serverZoneLabelValues map[st
 	c.variableLabelsMutex.Unlock()
 }
 
-// DeleteServerZoneLabels deletes the Server Zone Labels
+// DeleteServerZoneLabels deletes the Server Zone Labels.
 func (c *NginxPlusCollector) DeleteServerZoneLabels(zoneNames []string) {
 	c.variableLabelsMutex.Lock()
 	for _, k := range zoneNames {
@@ -154,7 +154,7 @@ func (c *NginxPlusCollector) DeleteServerZoneLabels(zoneNames []string) {
 	c.variableLabelsMutex.Unlock()
 }
 
-// UpdateStreamServerZoneLabels updates the Stream Server Zone Labels
+// UpdateStreamServerZoneLabels updates the Stream Server Zone Labels.
 func (c *NginxPlusCollector) UpdateStreamServerZoneLabels(streamServerZoneLabelValues map[string][]string) {
 	c.variableLabelsMutex.Lock()
 	for k, v := range streamServerZoneLabelValues {
@@ -163,7 +163,7 @@ func (c *NginxPlusCollector) UpdateStreamServerZoneLabels(streamServerZoneLabelV
 	c.variableLabelsMutex.Unlock()
 }
 
-// DeleteStreamServerZoneLabels deletes the Stream Server Zone Labels
+// DeleteStreamServerZoneLabels deletes the Stream Server Zone Labels.
 func (c *NginxPlusCollector) DeleteStreamServerZoneLabels(zoneNames []string) {
 	c.variableLabelsMutex.Lock()
 	for _, k := range zoneNames {
@@ -172,7 +172,7 @@ func (c *NginxPlusCollector) DeleteStreamServerZoneLabels(zoneNames []string) {
 	c.variableLabelsMutex.Unlock()
 }
 
-// UpdateCacheZoneLabels updates the Upstream Cache Zone labels
+// UpdateCacheZoneLabels updates the Upstream Cache Zone labels.
 func (c *NginxPlusCollector) UpdateCacheZoneLabels(cacheZoneLabelValues map[string][]string) {
 	c.variableLabelsMutex.Lock()
 	for k, v := range cacheZoneLabelValues {
@@ -181,7 +181,7 @@ func (c *NginxPlusCollector) UpdateCacheZoneLabels(cacheZoneLabelValues map[stri
 	c.variableLabelsMutex.Unlock()
 }
 
-// DeleteCacheZoneLabels deletes the Cache Zone Labels
+// DeleteCacheZoneLabels deletes the Cache Zone Labels.
 func (c *NginxPlusCollector) DeleteCacheZoneLabels(cacheZoneNames []string) {
 	c.variableLabelsMutex.Lock()
 	for _, k := range cacheZoneNames {
@@ -226,7 +226,7 @@ func (c *NginxPlusCollector) getStreamUpstreamServerPeerLabelValues(peer string)
 	return c.streamUpstreamServerPeerLabels[peer]
 }
 
-// UpdateWorkerLabels updates the Worker Labels
+// UpdateWorkerLabels updates the Worker Labels.
 func (c *NginxPlusCollector) UpdateWorkerLabels(workerLabelValues map[string][]string) {
 	c.variableLabelsMutex.Lock()
 	for k, v := range workerLabelValues {
@@ -235,7 +235,7 @@ func (c *NginxPlusCollector) UpdateWorkerLabels(workerLabelValues map[string][]s
 	c.variableLabelsMutex.Unlock()
 }
 
-// DeleteWorkerLabels deletes the Worker Labels
+// DeleteWorkerLabels deletes the Worker Labels.
 func (c *NginxPlusCollector) DeleteWorkerLabels(id []string) {
 	c.variableLabelsMutex.Lock()
 	for _, k := range id {
@@ -250,7 +250,7 @@ func (c *NginxPlusCollector) getWorkerLabelValues(id string) []string {
 	return c.workerLabels[id]
 }
 
-// VariableLabelNames holds all the variable label names for the different metrics
+// VariableLabelNames holds all the variable label names for the different metrics.
 type VariableLabelNames struct {
 	UpstreamServerVariableLabelNames           []string
 	ServerZoneVariableLabelNames               []string
@@ -262,7 +262,7 @@ type VariableLabelNames struct {
 	WorkerPIDVariableLabelNames                []string
 }
 
-// NewVariableLabelNames NewVariableLabels creates a new struct for VariableNames for the collector
+// NewVariableLabelNames NewVariableLabels creates a new struct for VariableNames for the collector.
 func NewVariableLabelNames(upstreamServerVariableLabelNames []string, serverZoneVariableLabelNames []string, upstreamServerPeerVariableLabelNames []string,
 	streamUpstreamServerVariableLabelNames []string, streamServerZoneLabels []string, streamUpstreamServerPeerVariableLabelNames []string, cacheZoneLabelNames []string, workerPIDVariableLabelNames []string,
 ) VariableLabelNames {
@@ -1219,7 +1219,6 @@ func (c *NginxPlusCollector) Collect(ch chan<- prometheus.Metric) {
 	}
 
 	for name, zone := range stats.Caches {
-
 		var cold float64
 		if zone.Cold {
 			cold = 1.0

exporter.go

@@ -33,7 +33,7 @@ import (
 	"github.com/prometheus/exporter-toolkit/web/kingpinflag"
 )
 
-// positiveDuration is a wrapper of time.Duration to ensure only positive values are accepted
+// positiveDuration is a wrapper of time.Duration to ensure only positive values are accepted.
 type positiveDuration struct{ time.Duration }
 
 func (pd *positiveDuration) Set(s string) error {
@@ -49,7 +49,7 @@ func (pd *positiveDuration) Set(s string) error {
 func parsePositiveDuration(s string) (positiveDuration, error) {
 	dur, err := time.ParseDuration(s)
 	if err != nil {
-		return positiveDuration{}, err
+		return positiveDuration{}, fmt.Errorf("failed to parse duration %q: %w", s, err)
 	}
 	if dur < 0 {
 		return positiveDuration{}, fmt.Errorf("negative duration %v is not valid", dur)
@@ -82,7 +82,7 @@ func parseUnixSocketAddress(address string) (string, string, error) {
 var (
 	constLabels = map[string]string{}
 
-	// Command-line flags
+	// Command-line flags.
 	webConfig     = kingpinflag.AddFlags(kingpin.CommandLine, ":9113")
 	metricsPath   = kingpin.Flag("web.telemetry-path", "Path under which to expose metrics.").Default("/metrics").Envar("TELEMETRY_PATH").String()
 	nginxPlus     = kingpin.Flag("nginx.plus", "Start the exporter for NGINX Plus. By default, the exporter is started for NGINX.").Default("false").Envar("NGINX_PLUS").Bool()
@@ -92,7 +92,7 @@ var (
 	sslClientCert = kingpin.Flag("nginx.ssl-client-cert", "Path to the PEM encoded client certificate file to use when connecting to the server.").Default("").Envar("SSL_CLIENT_CERT").String()
 	sslClientKey  = kingpin.Flag("nginx.ssl-client-key", "Path to the PEM encoded client certificate key file to use when connecting to the server.").Default("").Envar("SSL_CLIENT_KEY").String()
 
-	// Custom command-line flags
+	// Custom command-line flags.
 	timeout = createPositiveDurationFlag(kingpin.Flag("nginx.timeout", "A timeout for scraping metrics from NGINX or NGINX Plus.").Default("5s").Envar("TIMEOUT").HintOptions("5s", "10s", "30s", "1m", "5m"))
 )
 
@@ -269,7 +269,11 @@ type userAgentRoundTripper struct {
 func (rt *userAgentRoundTripper) RoundTrip(req *http.Request) (*http.Response, error) {
 	req = cloneRequest(req)
 	req.Header.Set("User-Agent", rt.agent)
-	return rt.rt.RoundTrip(req)
+	roundTrip, err := rt.rt.RoundTrip(req)
+	if err != nil {
+		return nil, fmt.Errorf("round trip failed: %w", err)
+	}
+	return roundTrip, nil
 }
 
 func cloneRequest(req *http.Request) *http.Request {
@@ -287,7 +291,7 @@ func cloneRequest(req *http.Request) *http.Request {
 }
 
 // addMissingEnvironmentFlags sets Envar on any flag which has
-// the "web." prefix which doesn't already have an Envar set
+// the "web." prefix which doesn't already have an Envar set.
 func addMissingEnvironmentFlags(ka *kingpin.Application) {
 	for _, f := range ka.Model().FlagGroupModel.Flags {
 		if strings.HasPrefix(f.Name, "web.") && f.Envar == "" {

exporter_test.go

@@ -112,6 +112,7 @@ func TestParseUnixSocketAddress(t *testing.T) {
 }
 
 func TestAddMissingEnvironmentFlags(t *testing.T) {
+	t.Parallel()
 	expectedMatches := map[string]string{
 		"non-matching-flag":  "",
 		"web.missing-env":    "MISSING_ENV",
@@ -143,6 +144,7 @@ func TestAddMissingEnvironmentFlags(t *testing.T) {
 }
 
 func TestConvertFlagToEnvar(t *testing.T) {
+	t.Parallel()
 	cases := []struct {
 		input  string
 		output string