test: Add more linters (jaegertracing#4881)

## Description of the changes
- Enable more linters
- Fix issues currently reported

## How was this change tested?
- `make lint`

---------

Signed-off-by: Yuri Shkuro <github@ysh.us>

.golangci.yml

@@ -3,7 +3,10 @@ issues:
   exclude-rules:
     # Exclude some linters from running on tests files.
     - path: _test\.go
-      linters: [gosec]
+      linters: [gosec, bodyclose, noctx]
+
+    - path: crossdock
+      linters: [noctx]
 
     # See https://github.com/jaegertracing/jaeger/issues/4488
     - path: internal/grpctest/
@@ -22,16 +25,70 @@ linters:
   disable:
     - errcheck
   enable:
+    # Plain ASCII identifiers.
+    - asciicheck
+
+    # Checks for dangerous unicode character sequences.
     - bidichk
+
+    # Checks whether HTTP response body is closed successfully.
+    # TODO enable this but maybe find a way to disable in tests.
+    - bodyclose
+
+    # Check whether the function uses a non-inherited context.
     - contextcheck
+
+    # Check declaration order of types, consts, vars and funcs.
+    - decorder
+
+    # Checks if package imports are in a list of acceptable packages.
     - depguard
+
+    # Check for two durations multiplied together.
+    - durationcheck
+
+    # Checks `Err-` prefix for var and `-Error` suffix for error type.
+    - errname
+
+    # Suggests to use `%w` for error-wrapping.
+    # TODO enable this. Curently fails in about 20 places.
+    # - errorlint
+
+    # Checks for pointers to enclosing loop variables.
+    - exportloopref
+
     - gocritic
+    - gofmt
     - gofumpt
     - goimports
+
+    # Allow or ban replace directives in go.mod
+    # or force explanation for retract directives.
+    # Maybe enable once we get rid of old sarama.
+    # - gomoddirectives
+
     - gosec
+
+    # Linter that specializes in simplifying code.
+    - gosimple
     - govet
+
+    # Detects when assignments to existing variables are not used.
+    - ineffassign
+
     - misspell
 
+    # Finds naked/bare returns and requires change them.
+    - nakedret
+
+    # Require a bit more explicit returns.
+    - nilerr
+
+    # Finds sending HTTP request without context.Context.
+    - noctx
+
+    # TODO consider adding more linters, cf. https://olegk.dev/go-linters-configuration-the-right-version
+
 linters-settings:
   depguard:
     list-type: blacklist

cmd/internal/status/command.go

@@ -15,11 +15,13 @@
 package status
 
 import (
+	"context"
 	"flag"
 	"fmt"
 	"io"
 	"net/http"
 	"strings"
+	"time"
 
 	"github.com/spf13/cobra"
 	"github.com/spf13/viper"
@@ -37,7 +39,10 @@ func Command(v *viper.Viper, adminPort int) *cobra.Command {
 		Long:  `Print Jaeger component status information, exit non-zero on any error.`,
 		RunE: func(cmd *cobra.Command, args []string) error {
 			url := convert(v.GetString(statusHTTPHostPort))
-			resp, err := http.Get(url)
+			ctx, cx := context.WithTimeout(context.Background(), time.Second)
+			defer cx()
+			req, _ := http.NewRequestWithContext(ctx, "GET", url, nil)
+			resp, err := http.DefaultClient.Do(req)
 			if err != nil {
 				return err
 			}

crossdock/main.go

@@ -111,6 +111,7 @@ func is2xxStatusCode(statusCode int) bool {
 func httpHealthCheck(logger *zap.Logger, service, healthURL string) {
 	for i := 0; i < 240; i++ {
 		res, err := http.Get(healthURL)
+		res.Body.Close()
 		if err == nil && is2xxStatusCode(res.StatusCode) {
 			logger.Info("Health check successful", zap.String("service", service))
 			return

internal/metricstest/local.go

@@ -251,7 +251,7 @@ func (b *Backend) Snapshot() (counters, gauges map[string]int64) {
 		}
 	}
 
-	return
+	return counters, gauges
 }
 
 // Stop cleanly closes the background goroutine spawned by NewBackend.

pkg/es/client/client.go

@@ -87,6 +87,7 @@ func (c *Client) request(esRequest elasticRequest) ([]byte, error) {
 	if err != nil {
 		return []byte{}, err
 	}
+	defer res.Body.Close()
 
 	if res.StatusCode != http.StatusOK {
 		return []byte{}, c.handleFailedRequest(res)

plugin/metrics/disabled/reader.go

@@ -27,14 +27,14 @@ type (
 	// the METRICS_STORAGE_TYPE has not been set.
 	MetricsReader struct{}
 
-	// errMetricsQueryDisabled is the error returned by disabledMetricsQueryService.
-	errMetricsQueryDisabled struct{}
+	// errMetricsQueryDisabledError is the error returned by disabledMetricsQueryService.
+	errMetricsQueryDisabledError struct{}
 )
 
 // ErrDisabled is the error returned by a "disabled" MetricsQueryService on all of its endpoints.
-var ErrDisabled = &errMetricsQueryDisabled{}
+var ErrDisabled = &errMetricsQueryDisabledError{}
 
-func (m *errMetricsQueryDisabled) Error() string {
+func (m *errMetricsQueryDisabledError) Error() string {
 	return "metrics querying is currently disabled"
 }
 

plugin/sampling/strategystore/static/strategy_store.go

@@ -97,12 +97,19 @@ func (h *strategyStore) Close() {
 
 func (h *strategyStore) downloadSamplingStrategies(url string) ([]byte, error) {
 	h.logger.Info("Downloading sampling strategies", zap.String("url", url))
-	resp, err := http.Get(url)
+
+	ctx, cx := context.WithTimeout(context.Background(), time.Second)
+	defer cx()
+	req, err := http.NewRequestWithContext(ctx, "GET", url, nil)
+	if err != nil {
+		return nil, fmt.Errorf("cannot construct HTTP request: %w", err)
+	}
+	resp, err := http.DefaultClient.Do(req)
 	if err != nil {
 		return nil, fmt.Errorf("failed to download sampling strategies: %w", err)
 	}
-
 	defer resp.Body.Close()
+
 	buf := new(bytes.Buffer)
 	if _, err = buf.ReadFrom(resp.Body); err != nil {
 		return nil, fmt.Errorf("failed to read sampling strategies HTTP response body: %w", err)