Upgrade golangci-lint to v1.55.2 and fix errors (jaegertracing#5029)

## Description of the changes
- Upgrade linter version
- Update configuration of depguard
  - Disable uber-go/atomic and crossdock as deps
- Replace invalid crossdock imports with stretchr/testify
- Replace uber-go/atomic usage with sync/atomic
- in most cases pointers to atomics are not necessary, and likewise
initialization to zero values
- Fix other test and linter errors revealed by new linter

---------

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

.golangci.yml

@@ -41,7 +41,7 @@ linters:
     # Check declaration order of types, consts, vars and funcs.
     - decorder
 
-    # Checks if package imports are in a list of acceptable packages.
+    # Checks if package imports are in a list of acceptable packages (see cfg below).
     - depguard
 
     # Check for two durations multiplied together.
@@ -90,12 +90,26 @@ linters:
 
 linters-settings:
   depguard:
-    list-type: blacklist
-    include-go-root: true
-    packages:
-      - io/ioutil
-    packages-with-error-message:
-      - io/ioutil: "Use os or io instead of io/ioutil"
+    rules:
+      disallowed-deps:
+        deny:
+          - pkg: go.uber.org/atomic
+            desc: "Use 'sync/atomic' instead of go.uber.org/atomic"
+          - pkg: io/ioutil
+            desc: "Use os or io instead of io/ioutil"
+          - pkg: github.com/hashicorp/go-multierror
+            desc: "Use errors.Join instead of github.com/hashicorp/go-multierror"
+          - pkg: go.uber.org/multierr
+            desc: "Use errors.Join instead of github.com/hashicorp/go-multierror"
+      # crossdock-go provides assert/require similar to stretchr/testify
+      # but we never want to use them outside of the crossdock tests.
+      disallow-crossdock:
+        deny:
+          - pkg: github.com/crossdock/crossdock-go
+            desc: "Do not refer to crossdock from other packages"
+        files:
+          - "!**/crossdock/**"
+
   goimports:
     local-prefixes: github.com/jaegertracing/jaeger
   gosec:

Makefile

@@ -492,7 +492,7 @@ draft-release:
 
 .PHONY: install-test-tools
 install-test-tools:
-	$(GO) install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.52.1
+	$(GO) install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.55.2
 	$(GO) install mvdan.cc/gofumpt@latest
 
 .PHONY: install-build-tools

cmd/agent/app/reporter/client_metrics.go

@@ -17,9 +17,9 @@ package reporter
 import (
 	"context"
 	"sync"
+	"sync/atomic"
 	"time"
 
-	"go.uber.org/atomic"
 	"go.uber.org/zap"
 
 	"github.com/jaegertracing/jaeger/pkg/metrics"
@@ -75,7 +75,7 @@ type ClientMetricsReporter struct {
 	params        ClientMetricsReporterParams
 	clientMetrics *clientMetrics
 	shutdown      chan struct{}
-	closed        *atomic.Bool
+	closed        atomic.Bool
 
 	// map from client-uuid to *lastReceivedClientStats
 	lastReceivedClientStats sync.Map
@@ -104,7 +104,6 @@ func WrapWithClientMetrics(params ClientMetricsReporterParams) *ClientMetricsRep
 		params:        params,
 		clientMetrics: cm,
 		shutdown:      make(chan struct{}),
-		closed:        atomic.NewBool(false),
 	}
 	go r.expireClientMetricsLoop()
 	return r

cmd/agent/app/servers/tbuffered_server_test.go

@@ -19,13 +19,13 @@ import (
 	"context"
 	"io"
 	"sync"
+	"sync/atomic"
 	"testing"
 	"time"
 
 	"github.com/apache/thrift/lib/go/thrift"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
-	"go.uber.org/atomic"
 
 	"github.com/jaegertracing/jaeger/cmd/agent/app/customtransport"
 	"github.com/jaegertracing/jaeger/cmd/agent/app/servers/thriftudp"
@@ -35,7 +35,7 @@ import (
 	"github.com/jaegertracing/jaeger/thrift-gen/zipkincore"
 )
 
-func TestTBufferedServer_SendReceive(t *testing.T) {
+func TestTBufferedServerSendReceive(t *testing.T) {
 	metricsFactory := metricstest.NewFactory(0)
 
 	transport, err := thriftudp.NewTUDPServerTransport("127.0.0.1:0")
@@ -101,7 +101,7 @@ type fakeTransport struct {
 // Second packet is simulated as error.
 // Third packet is returned as normal, but will be dropped as overflow by the server whose queue size = 1.
 func (t *fakeTransport) Read(p []byte) (n int, err error) {
-	packet := t.packet.Inc()
+	packet := t.packet.Add(1)
 	if packet == 2 {
 		// return some error packet, followed by valid one
 		return 0, io.ErrNoProgress
@@ -121,7 +121,7 @@ func (t *fakeTransport) Close() error {
 	return nil
 }
 
-func TestTBufferedServer_Metrics(t *testing.T) {
+func TestTBufferedServerMetrics(t *testing.T) {
 	metricsFactory := metricstest.NewFactory(0)
 
 	transport := new(fakeTransport)

cmd/collector/app/root_span_handler_test.go

@@ -15,10 +15,10 @@
 package app
 
 import (
+	"sync/atomic"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
-	"go.uber.org/atomic"
 	"go.uber.org/zap"
 
 	"github.com/jaegertracing/jaeger/model"
@@ -30,11 +30,11 @@ type mockAggregator struct {
 }
 
 func (t *mockAggregator) RecordThroughput(service, operation string, samplerType model.SamplerType, probability float64) {
-	t.callCount.Inc()
+	t.callCount.Add(1)
 }
 func (t *mockAggregator) Start() {}
 func (t *mockAggregator) Close() error {
-	t.closeCount.Inc()
+	t.closeCount.Add(1)
 	return nil
 }
 

cmd/collector/app/span_processor.go

@@ -18,9 +18,9 @@ package app
 import (
 	"context"
 	"sync"
+	"sync/atomic"
 	"time"
 
-	"go.uber.org/atomic"
 	"go.uber.org/zap"
 
 	"github.com/jaegertracing/jaeger/cmd/collector/app/processor"
@@ -54,8 +54,8 @@ type spanProcessor struct {
 	collectorTags      map[string]string
 	dynQueueSizeWarmup uint
 	dynQueueSizeMemory uint
-	bytesProcessed     *atomic.Uint64
-	spansProcessed     *atomic.Uint64
+	bytesProcessed     atomic.Uint64
+	spansProcessed     atomic.Uint64
 	stopCh             chan struct{}
 }
 
@@ -120,8 +120,6 @@ func newSpanProcessor(spanWriter spanstore.Writer, additional []ProcessSpan, opt
 		stopCh:             make(chan struct{}),
 		dynQueueSizeMemory: options.dynQueueSizeMemory,
 		dynQueueSizeWarmup: options.dynQueueSizeWarmup,
-		bytesProcessed:     atomic.NewUint64(0),
-		spansProcessed:     atomic.NewUint64(0),
 	}
 
 	processSpanFuncs := []ProcessSpan{options.preSave, sp.saveSpan}
@@ -173,7 +171,7 @@ func (sp *spanProcessor) saveSpan(span *model.Span, tenant string) {
 
 func (sp *spanProcessor) countSpan(span *model.Span, tenant string) {
 	sp.bytesProcessed.Add(uint64(span.Size()))
-	sp.spansProcessed.Inc()
+	sp.spansProcessed.Add(1)
 }
 
 func (sp *spanProcessor) ProcessSpans(mSpans []*model.Span, options processor.SpansOptions) ([]bool, error) {

cmd/collector/app/span_processor_test.go

@@ -21,12 +21,12 @@ import (
 	"io"
 	"reflect"
 	"sync"
+	"sync/atomic"
 	"testing"
 	"time"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
-	"go.uber.org/atomic"
 	"go.uber.org/zap"
 
 	"github.com/jaegertracing/jaeger/cmd/collector/app/handler"
@@ -296,10 +296,10 @@ type blockingWriter struct {
 }
 
 func (w *blockingWriter) WriteSpan(ctx context.Context, span *model.Span) error {
-	w.inWriteSpan.Inc()
+	w.inWriteSpan.Add(1)
 	w.Lock()
 	defer w.Unlock()
-	w.inWriteSpan.Dec()
+	w.inWriteSpan.Add(-1)
 	return nil
 }
 
@@ -443,7 +443,10 @@ func TestSpanProcessorCountSpan(t *testing.T) {
 			m := mb.Namespace(metrics.NSOptions{})
 
 			w := &fakeSpanWriter{}
-			opts := []Option{Options.HostMetrics(m), Options.SpanSizeMetricsEnabled(tt.enableSpanMetrics)}
+			opts := []Option{
+				Options.HostMetrics(m),
+				Options.SpanSizeMetricsEnabled(tt.enableSpanMetrics),
+			}
 			if tt.enableDynQueueSizeMem {
 				opts = append(opts, Options.DynQueueSizeMemory(1000))
 			} else {
@@ -456,7 +459,14 @@ func TestSpanProcessorCountSpan(t *testing.T) {
 			p.background(10*time.Millisecond, p.updateGauges)
 
 			p.processSpan(&model.Span{}, "")
-			assert.NotEqual(t, uint64(0), p.bytesProcessed)
+			if tt.enableSpanMetrics {
+				assert.Eventually(t,
+					func() bool { return p.spansProcessed.Load() > 0 },
+					time.Second,
+					time.Millisecond,
+				)
+				assert.Greater(t, p.spansProcessed.Load(), uint64(0))
+			}
 
 			for i := 0; i < 10000; i++ {
 				_, g := mb.Snapshot()
@@ -558,8 +568,8 @@ func TestUpdateDynQueueSize(t *testing.T) {
 			p := newSpanProcessor(w, nil, Options.QueueSize(tt.initialCapacity), Options.DynQueueSizeWarmup(tt.warmup), Options.DynQueueSizeMemory(tt.sizeInBytes))
 			assert.EqualValues(t, tt.initialCapacity, p.queue.Capacity())
 
-			p.spansProcessed = atomic.NewUint64(tt.spansProcessed)
-			p.bytesProcessed = atomic.NewUint64(tt.bytesProcessed)
+			p.spansProcessed.Store(tt.spansProcessed)
+			p.bytesProcessed.Store(tt.bytesProcessed)
 
 			p.updateQueueSize()
 			assert.EqualValues(t, tt.expectedCapacity, p.queue.Capacity())
@@ -579,8 +589,8 @@ func TestStartDynQueueSizeUpdater(t *testing.T) {
 	p := newSpanProcessor(w, nil, Options.QueueSize(100), Options.DynQueueSizeWarmup(1000), Options.DynQueueSizeMemory(oneGiB))
 	assert.EqualValues(t, 100, p.queue.Capacity())
 
-	p.spansProcessed = atomic.NewUint64(1000)
-	p.bytesProcessed = atomic.NewUint64(10 * 1024 * p.spansProcessed.Load()) // 10KiB per span
+	p.spansProcessed.Store(1000)
+	p.bytesProcessed.Store(10 * 1024 * p.spansProcessed.Load()) // 10KiB per span
 
 	// 1024 ^ 3 / (10 * 1024) = 104857,6
 	// ideal queue size = 104857

cmd/es-rollover/app/actions_test.go

@@ -20,9 +20,9 @@ import (
 	"net/http"
 	"testing"
 
-	"github.com/crossdock/crossdock-go/assert"
 	"github.com/spf13/cobra"
 	"github.com/spf13/viper"
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"go.uber.org/zap"
 

cmd/es-rollover/app/index_options_test.go

@@ -17,7 +17,7 @@ package app
 import (
 	"testing"
 
-	"github.com/crossdock/crossdock-go/assert"
+	"github.com/stretchr/testify/assert"
 )
 
 func TestRolloverIndices(t *testing.T) {

cmd/es-rollover/app/init/action_test.go

@@ -20,7 +20,7 @@ import (
 	"strings"
 	"testing"
 
-	"github.com/crossdock/crossdock-go/assert"
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/mock"
 
 	"github.com/jaegertracing/jaeger/cmd/es-rollover/app"

cmd/es-rollover/app/lookback/time_reference_test.go

@@ -18,7 +18,7 @@ import (
 	"testing"
 	"time"
 
-	"github.com/crossdock/crossdock-go/assert"
+	"github.com/stretchr/testify/assert"
 )
 
 func TestGetTimeReference(t *testing.T) {

cmd/es-rollover/app/rollover/action_test.go

@@ -18,7 +18,7 @@ import (
 	"errors"
 	"testing"
 
-	"github.com/crossdock/crossdock-go/assert"
+	"github.com/stretchr/testify/assert"
 
 	"github.com/jaegertracing/jaeger/cmd/es-rollover/app"
 	"github.com/jaegertracing/jaeger/pkg/es/client"

cmd/internal/flags/service_test.go

@@ -17,12 +17,12 @@ import (
 	"flag"
 	"os"
 	"reflect"
+	"sync/atomic"
 	"testing"
 	"time"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
-	"go.uber.org/atomic"
 
 	"github.com/jaegertracing/jaeger/pkg/config"
 	"github.com/jaegertracing/jaeger/pkg/healthcheck"
@@ -86,7 +86,7 @@ func TestStartErrors(t *testing.T) {
 			}
 			assert.NoError(t, err)
 
-			stopped := atomic.NewBool(false)
+			var stopped atomic.Bool
 			shutdown := func() {
 				stopped.Store(true)
 			}

pkg/es/filter/alias_test.go

@@ -17,7 +17,7 @@ package filter
 import (
 	"testing"
 
-	"github.com/crossdock/crossdock-go/assert"
+	"github.com/stretchr/testify/assert"
 
 	"github.com/jaegertracing/jaeger/pkg/es/client"
 )

pkg/es/filter/date_test.go

@@ -18,7 +18,7 @@ import (
 	"testing"
 	"time"
 
-	"github.com/crossdock/crossdock-go/assert"
+	"github.com/stretchr/testify/assert"
 
 	"github.com/jaegertracing/jaeger/pkg/es/client"
 )