Skip to content

Add more linters #733

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jun 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions .github/PULL_REQUEST_TEMPLATE.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
21 changes: 18 additions & 3 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
39 changes: 19 additions & 20 deletions collector/nginx_plus.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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 {
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -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 {
Expand Down Expand Up @@ -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 {
Expand All @@ -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 {
Expand All @@ -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
Expand All @@ -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 {
Expand Down Expand Up @@ -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
Expand Down
16 changes: 10 additions & 6 deletions exporter.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
"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 {
Expand All @@ -49,7 +49,7 @@
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)
Expand Down Expand Up @@ -82,7 +82,7 @@
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()
Expand All @@ -92,7 +92,7 @@
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"))
)

Expand Down Expand Up @@ -269,7 +269,11 @@
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)

Check warning on line 274 in exporter.go

View check run for this annotation

Codecov / codecov/patch

exporter.go#L272-L274

Added lines #L272 - L274 were not covered by tests
}
return roundTrip, nil

Check warning on line 276 in exporter.go

View check run for this annotation

Codecov / codecov/patch

exporter.go#L276

Added line #L276 was not covered by tests
}

func cloneRequest(req *http.Request) *http.Request {
Expand All @@ -287,7 +291,7 @@
}

// 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 == "" {
Expand Down
2 changes: 2 additions & 0 deletions exporter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -143,6 +144,7 @@ func TestAddMissingEnvironmentFlags(t *testing.T) {
}

func TestConvertFlagToEnvar(t *testing.T) {
t.Parallel()
cases := []struct {
input string
output string
Expand Down
Loading