Extend linting configuration file

.drone/drone.yml

@@ -16,7 +16,7 @@ trigger:
 
 steps:
   - name: lint
-    image: golangci/golangci-lint:v1.43
+    image: golangci/golangci-lint:v1.44
     commands:
       - apt-get update -y && apt-get install -y libsystemd-dev
       - make lint
@@ -271,6 +271,6 @@ get:
   name: pat
 ---
 kind: signature
-hmac: 3da266cfeead0295f439b0ec44ba62b6b9c53f2774aef54fe58585e4d51d8990
+hmac: 76ba44a90ab332caf4cd3bc11529e7ea674d4706ab8b4928b17b8c7dcda8abea
 
 ...

.golangci.yml

@@ -1,74 +1,83 @@
-# This file contains all available configuration options
-# with their default values.
+# Full list of configuration options: https://golangci-lint.run/usage/configuration/
 
-# options for analysis running
 run:
-  # default concurrency is a available CPU number
-  concurrency: 16
-
-  # timeout for analysis, e.g. 30s, 5m, default is 1m
   timeout: 5m
 
-  # exit code when at least one issue was found, default is 1
-  issues-exit-code: 1
-
-  # include test files or not, default is true
-  tests: true
-
-  # list of build2 tags, all linters use it. Default is empty list.
-  build-tags:
-
-  # which dirs to skip: they won't be analyzed;
-  # can use regexp here: generated.*, regexp is applied on full path;
-  # default value is empty list, but next dirs are always skipped independently
-  # from this option's value:
-  #     vendor$, third_party$, testdata$, examples$, Godeps$, builtin$
-  skip-dirs:
-  # which files to skip: they will be analyzed, but issues from them
-  # won't be reported. Default value is empty list, but there is
-  # no need to include all autogenerated files, we confidently recognize
-  # autogenerated files. If it's not please let us know.
-  skip-files:
-# output configuration options
 output:
-  # colored-line-number|line-number|json|tab|checkstyle, default is "colored-line-number"
-  format: colored-line-number
-
-  # print lines of code with issue, default is true
-  print-issued-lines: true
-
-  # print linter name in the end of issue text, default is true
-  print-linter-name: true
+  sort-results: true
 
 linters:
   enable:
-    - deadcode
-    - errcheck
-    - goconst
-    - gofmt
-    - goimports
-    - golint
-    - gosimple
-    - ineffassign
-    - megacheck
-    - misspell
-    - structcheck
-    - unconvert
-    - unparam
-    - varcheck
-    - govet
-    - unused # new from here.
-    - interfacer
-    - typecheck
+    - deadcode    # Report on unused code
+    - errcheck    # Report unchecked errors
+    - goconst     # Find repeated strings that could be replaced by constant
+    - gofmt       # Check whether code was gofmt-ed
+    - goimports   # Check imports were formatted with gofmt
+    - revive      # Broad set of rules; replaces deprecated golint
+    - gosimple    # Check whether code can be simplified
+    - ineffassign # Detect when assignment to variable is never used
+    - misspell    # Report on commonly misspelled English words
+    - structcheck # Report on unused struct fields
+    - unconvert   # Remove unnecessary type conversions
+    - unparam     # Detect unused function parameters
+    - varcheck    # Find unused global variables/constants
+    - govet       # `go vet`
+    - unused      # Detect unused constants/variables/functions/types
+    - typecheck   # Ensure code typechecks
+    - depguard    # Allow/denylist specific imports
+    - makezero    # Detect misuse of make with non-zero length and append
+    - tenv        # Use testing.(*T).Setenv instead of os.Setenv
+    - whitespace  # Report unnecessary blank lines
 
 issues:
-  # golangci-lint excludes some stuff we want by default (i.e., proper go-style comments).
-  # We exclude the defaults and then manually exclude the subset of defaults we truly don't
-  # care about.
+  # We want to use our own exclusion rules and ignore all the defaults.
   exclude-use-default: false
 
+  exclude-rules:
+    # It's fine if tests ignore errors.
+    - path: _test.go
+      linters:
+        - errcheck
+
   exclude:
-    # EXC0001 errcheck: Almost all programs ignore errors on these functions
-    # and in most cases it's ok. This is copied from the golangci-lint defaults
-    # mut modified to include go-kit logging.
-    - Error return value of .((os\.)?std(out|err)\..*|.*Close|.*Flush|os\.Remove(All)?|.*print(f|ln)?|os\.(Un)?Setenv|.*\.Log). is not checked
+    # Ignoring errors on Close, Log, and removing files is OK in most cases.
+    - "Error return value of `(.*\\.Close|.*\\.Log|os.Remove)` is not checked"
+    # Packages for integrations are named matching their upstream counterpart,
+    # which almost always have underscores.
+    - "var-naming: don't use an underscore in package name"
+
+# Linter settings options: https://golangci-lint.run/usage/linters/
+linters-settings:
+  depguard:
+    # We want to report errors on stdlib packages, not just third party modules
+    include-go-root: true
+
+    packages-with-error-message:
+      - sync/atomic: "Use go.uber.org/atomic instead of sync/atomic"
+      - github.com/pkg/errors: "Use errors instead of github.com/pkg/errors"
+      - github.com/go-kit/kit/log: "Use github.com/go-kit/log instead of github.com/go-kit/kit/log"
+      - golang.org/x/sync/errgroup: "Use github.com/oklog/run instead of golang.org/x/sync/errgroup"
+
+  whitespace:
+    # While there normally shouldn't be extra redundant leading/trailing
+    # whitespace, if statement conditions and function headers that cross
+    # multiple lines are an exception.
+    #
+    #   if true ||
+    #      false {
+    #
+    #       // ... ^ must have empty line above
+    #    }
+    #
+    #   func foo(
+    #     a int,
+    #   ) {
+    #
+    #     // ... ^ must have empty line above
+    #   }
+    #
+    # This helps readers easily separate where the multi-line if/function ends
+    # at a glance.
+    multi-if: true
+    multi-func: true
+

pkg/config/config.go

@@ -20,7 +20,6 @@ import (
 	"github.com/grafana/agent/pkg/util"
 	"github.com/grafana/dskit/kv/consul"
 	"github.com/grafana/dskit/kv/etcd"
-	"github.com/pkg/errors"
 	"github.com/prometheus/common/config"
 	"github.com/prometheus/common/version"
 	"github.com/stretchr/testify/require"
@@ -219,7 +218,7 @@ func (c *Config) RegisterFlags(f *flag.FlagSet) {
 func LoadFile(filename string, expandEnvVars bool, c *Config) error {
 	buf, err := ioutil.ReadFile(filename)
 	if err != nil {
-		return errors.Wrap(err, "error reading config file")
+		return fmt.Errorf("error reading config file: %w", err)
 	}
 	return LoadBytes(buf, expandEnvVars, c)
 }

pkg/config/config_test.go

@@ -3,7 +3,6 @@ package config
 import (
 	"flag"
 	"net/url"
-	"os"
 	"strings"
 	"testing"
 	"time"
@@ -99,7 +98,7 @@ metrics:
 			EvaluationInterval: model.Duration(1 * time.Minute),
 		},
 	}
-	_ = os.Setenv("SCRAPE_TIMEOUT", "33s")
+	t.Setenv("SCRAPE_TIMEOUT", "33s")
 
 	fs := flag.NewFlagSet("test", flag.ExitOnError)
 	c, err := load(fs, []string{"-config.file", "test"}, func(_ string, _ bool, c *Config) error {

pkg/integrations/cadvisor/common.go

@@ -9,7 +9,7 @@ import (
 const name = "cadvisor"
 
 // DefaultConfig holds the default settings for the cadvisor integration
-var DefaultConfig Config = Config{
+var DefaultConfig = Config{
 	// Common cadvisor config defaults
 	StoreContainerLabels: true,
 	ResctrlInterval:      0,

pkg/integrations/dnsmasq_exporter/dnsmasq_exporter.go

@@ -11,7 +11,7 @@ import (
 )
 
 // DefaultConfig is the default config for dnsmasq_exporter.
-var DefaultConfig Config = Config{
+var DefaultConfig = Config{
 	DnsmasqAddress: "localhost:53",
 	LeasesPath:     "/var/lib/misc/dnsmasq.leases",
 }

pkg/integrations/github_exporter/github_exporter.go

@@ -15,7 +15,7 @@ import (
 )
 
 // DefaultConfig holds the default settings for the github_exporter integration
-var DefaultConfig Config = Config{
+var DefaultConfig = Config{
 	APIURL: "https://api.github.com",
 }
 
@@ -74,7 +74,6 @@ func init() {
 
 // New creates a new github_exporter integration.
 func New(logger log.Logger, c *Config) (integrations.Integration, error) {
-
 	conf := gh_config.Config{}
 	err := conf.SetAPIURL(c.APIURL)
 	if err != nil {

pkg/integrations/manager_test.go

@@ -36,8 +36,8 @@ use_hostname_label: true
 `
 	var (
 		cfg        ManagerConfig
-		listenPort int    = 12345
-		listenHost string = "127.0.0.1"
+		listenPort = 12345
+		listenHost = "127.0.0.1"
 	)
 	require.NoError(t, yaml.Unmarshal([]byte(cfgText), &cfg))
 
@@ -66,8 +66,8 @@ test:
 `
 	var (
 		cfg        ManagerConfig
-		listenPort int    = 12345
-		listenHost string = "127.0.0.1"
+		listenPort = 12345
+		listenHost = "127.0.0.1"
 	)
 	require.NoError(t, yaml.Unmarshal([]byte(cfgText), &cfg))
 
@@ -89,8 +89,8 @@ agent:
 
 	var (
 		cfg        ManagerConfig
-		listenPort int    = 12345
-		listenHost string = "127.0.0.1"
+		listenPort = 12345
+		listenHost = "127.0.0.1"
 	)
 	require.NoError(t, yaml.Unmarshal([]byte(cfgText), &cfg))
 

pkg/integrations/memcached_exporter/memcached_exporter.go

@@ -12,7 +12,7 @@ import (
 )
 
 // DefaultConfig is the default config for memcached_exporter.
-var DefaultConfig Config = Config{
+var DefaultConfig = Config{
 	MemcachedAddress: "localhost:11211",
 	Timeout:          time.Second,
 }

pkg/integrations/mongodb_exporter/mongodb_exporter.go

@@ -20,7 +20,6 @@ type Config struct {
 
 // UnmarshalYAML implements yaml.Unmarshaler for Config
 func (c *Config) UnmarshalYAML(unmarshal func(interface{}) error) error {
-
 	type plain Config
 	return unmarshal((*plain)(c))
 }

pkg/integrations/postgres_exporter/postgres_test.go

@@ -17,5 +17,4 @@ integrations:
 `
 	config.CheckSecret(t, stringCfg, "secret_password_in_uri")
 	config.CheckSecret(t, stringCfg, "secret_password_in_uri_2")
-
 }

pkg/integrations/redis_exporter/redis_exporter.go

@@ -144,9 +144,7 @@ func New(log log.Logger, c *Config) (integrations.Integration, error) {
 
 	if (c.TLSClientKeyFile != "") != (c.TLSClientCertFile != "") {
 		return nil, errors.New("TLS client key file and cert file should both be present")
-	}
-	if c.TLSClientKeyFile != "" && c.TLSClientCertFile != "" {
-
+	} else if c.TLSClientKeyFile != "" && c.TLSClientCertFile != "" {
 		exporterConfig.ClientKeyFile = c.TLSClientKeyFile
 		exporterConfig.ClientCertFile = c.TLSClientCertFile
 	}

pkg/integrations/redis_exporter/redis_exporter_test.go

@@ -104,7 +104,6 @@ func TestRedisCases(t *testing.T) {
 	logger := log.NewNopLogger()
 
 	for _, test := range tt {
-
 		t.Run(test.name, func(t *testing.T) {
 			integration, err := New(logger, &test.cfg)
 
@@ -151,7 +150,6 @@ func TestRedisCases(t *testing.T) {
 				require.True(t, exists, "could not find metric %s", metric)
 			}
 		})
-
 	}
 }
 

pkg/integrations/v2/autoscrape/autoscrape.go

@@ -19,7 +19,7 @@ import (
 )
 
 // DefaultGlobal holds default values for Global.
-var DefaultGlobal Global = Global{
+var DefaultGlobal = Global{
 	Enable:          true,
 	MetricsInstance: "default",
 }
@@ -197,8 +197,8 @@ func newInstanceScraper(
 	s InstanceStore,
 	instanceName string,
 ) *instanceScraper {
-	ctx, cancel := context.WithCancel(ctx)
 
+	ctx, cancel := context.WithCancel(ctx)
 	l = log.With(l, "target_instance", instanceName)
 
 	sd := discovery.NewManager(ctx, l, discovery.Name("autoscraper/"+instanceName))

pkg/integrations/v2/autoscrape/autoscrape_test.go

@@ -3,7 +3,6 @@ package autoscrape
 import (
 	"context"
 	"net/http/httptest"
-	"sync/atomic"
 	"testing"
 	"time"
 
@@ -18,6 +17,7 @@ import (
 	"github.com/prometheus/prometheus/storage"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"go.uber.org/atomic"
 )
 
 // TestAutoscrape is a basic end-to-end test of the autoscraper.
@@ -68,15 +68,15 @@ func TestAutoscrape(t *testing.T) {
 	require.NoError(t, wt.Wait(5*time.Second), "timed out waiting for scrape")
 }
 
-var globalRef uint64
+var globalRef atomic.Uint64
 var noOpAppender = mockAppender{
 	AppendFunc: func(ref uint64, l labels.Labels, t int64, v float64) (uint64, error) {
-		return atomic.AddUint64(&globalRef, 1), nil
+		return globalRef.Inc(), nil
 	},
 	CommitFunc:   func() error { return nil },
 	RollbackFunc: func() error { return nil },
 	AppendExemplarFunc: func(ref uint64, l labels.Labels, e exemplar.Exemplar) (uint64, error) {
-		return atomic.AddUint64(&globalRef, 1), nil
+		return globalRef.Inc(), nil
 	},
 }
 

pkg/integrations/v2/metricsutils/metricshandler_integration.go

@@ -26,6 +26,7 @@ func NewMetricsHandlerIntegration(
 	globals integrations.Globals,
 	h http.Handler,
 ) (integrations.MetricsIntegration, error) {
+
 	id, err := c.Identifier(globals)
 	if err != nil {
 		return nil, err

pkg/integrations/v2/subsystem_test.go

@@ -52,7 +52,6 @@ func TestSubsystemOptions_Unmarshal(t *testing.T) {
 
 	for _, tc := range tt {
 		t.Run(tc.name, func(t *testing.T) {
-
 			var so SubsystemOptions
 			err := yaml.UnmarshalStrict([]byte(tc.in), &so)