Skip to content

Commit

Permalink
Add more linters (#733)
Browse files Browse the repository at this point in the history
  • Loading branch information
@lucacome
lucacome committed Jun 17, 2024
1 parent a32ca97 commit 2877168
Show file tree
Hide file tree
Showing 5 changed files with 50 additions and 31 deletions.
3 changes: 1 addition & 2 deletions .github/PULL_REQUEST_TEMPLATE.md
View file
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
View file
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
View file
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
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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)
Expand Down Expand Up @@ -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()
Expand All @@ -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"))
)

Expand Down Expand Up @@ -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 {
Expand All @@ -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 == "" {
Expand Down
2 changes: 2 additions & 0 deletions exporter_test.go
View file
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

0 comments on commit 2877168

Please sign in to comment.