Enable errorlint linter and fix errors (jaegertracing#5028)

## Which problem is this PR solving?
Resolves jaegertracing#5027 

## Description of the changes
- Use `%w` when wrapping errors
- use `errors.Is` and `errors.As` instead of explicit cast or comparison
- Fix a bug 249c959 where direct and wrapped io.EOF were conflicting
- Enable error linting

## How was this change tested?
- 

## Checklist
- [x] I have read
https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md
- [x] I have signed all commits
- [x] I have added unit tests for the new functionality
- [x] I have run lint and test steps successfully
  - for `jaeger`: `make lint test`
  - for `jaeger-ui`: `yarn lint` and `yarn test`

---------

Signed-off-by: 1-8 <code_personal@163.com>
Signed-off-by: Yuri Shkuro <github@ysh.us>
Co-authored-by: Yuri Shkuro <github@ysh.us>

.golangci.yml

@@ -51,8 +51,7 @@ linters:
     - errname
 
     # Suggests to use `%w` for error-wrapping.
-    # TODO enable this. Curently fails in about 20 places.
-    # - errorlint
+    - errorlint
 
     # Checks for pointers to enclosing loop variables.
     - exportloopref
@@ -109,6 +108,17 @@ linters-settings:
       - G601
   gosimple:
     go: "1.20"
+  govet:
+    check-shadowing: false
+    enable-all: true
+    disable:
+      # There is rarely performance differences due to padding,
+      # the most noticable impact is memory usage. However,
+      # the main trace data is Protobuf-generated and we ignore
+      # those files from linting, so this linter is not useful.
+      - fieldalignment
+      # Disable shadow
+      - shadow
 
 run:
   go: "1.20"

cmd/anonymizer/app/query/query.go

@@ -16,6 +16,7 @@ package query
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"io"
 	"time"
@@ -76,7 +77,7 @@ func (q *Query) QueryTrace(traceID string) ([]model.Span, error) {
 	}
 
 	var spans []model.Span
-	for received, err := stream.Recv(); err != io.EOF; received, err = stream.Recv() {
+	for received, err := stream.Recv(); !errors.Is(err, io.EOF); received, err = stream.Recv() {
 		if err != nil {
 			return nil, unwrapNotFoundErr(err)
 		}

cmd/anonymizer/app/uiconv/extractor.go

@@ -16,32 +16,32 @@ package uiconv
 
 import (
 	"encoding/json"
+	"errors"
 	"fmt"
-	"io"
 	"os"
 
 	"go.uber.org/zap"
 
 	uimodel "github.com/jaegertracing/jaeger/model/json"
 )
 
-// Extractor reads the spans from reader, filters by traceID, and stores as JSON into uiFile.
-type Extractor struct {
+// extractor reads the spans from reader, filters by traceID, and stores as JSON into uiFile.
+type extractor struct {
 	uiFile  *os.File
 	traceID string
-	reader  *Reader
+	reader  *spanReader
 	logger  *zap.Logger
 }
 
-// NewExtractor creates Extractor.
-func NewExtractor(uiFile string, traceID string, reader *Reader, logger *zap.Logger) (*Extractor, error) {
+// newExtractor creates extractor.
+func newExtractor(uiFile string, traceID string, reader *spanReader, logger *zap.Logger) (*extractor, error) {
 	f, err := os.OpenFile(uiFile, os.O_CREATE|os.O_WRONLY, os.ModePerm)
 	if err != nil {
 		return nil, fmt.Errorf("cannot create output file: %w", err)
 	}
 	logger.Sugar().Infof("Writing spans to UI file %s", uiFile)
 
-	return &Extractor{
+	return &extractor{
 		uiFile:  f,
 		traceID: traceID,
 		reader:  reader,
@@ -50,7 +50,7 @@ func NewExtractor(uiFile string, traceID string, reader *Reader, logger *zap.Log
 }
 
 // Run executes the extraction.
-func (e *Extractor) Run() error {
+func (e *extractor) Run() error {
 	e.logger.Info("Parsing captured file for trace", zap.String("trace_id", e.traceID))
 
 	var (
@@ -63,7 +63,7 @@ func (e *Extractor) Run() error {
 			spans = append(spans, *span)
 		}
 	}
-	if err != io.EOF {
+	if !errors.Is(err, errNoMoreSpans) {
 		return fmt.Errorf("failed when scanning the file: %w", err)
 	}
 	trace := uimodel.Trace{

cmd/anonymizer/app/uiconv/extractor_test.go

@@ -30,18 +30,15 @@ type UITrace struct {
 	Data []model.Trace
 }
 
-func TestExtractor_TraceSuccess(t *testing.T) {
+func TestExtractorTraceSuccess(t *testing.T) {
 	inputFile := "fixtures/trace_success.json"
 	outputFile := "fixtures/trace_success_ui_anonymized.json"
 	defer os.Remove(outputFile)
 
-	reader, err := NewReader(
-		inputFile,
-		zap.NewNop(),
-	)
+	reader, err := newSpanReader(inputFile, zap.NewNop())
 	require.NoError(t, err)
 
-	extractor, err := NewExtractor(
+	extractor, err := newExtractor(
 		outputFile,
 		"2be38093ead7a083",
 		reader,
@@ -62,22 +59,19 @@ func TestExtractor_TraceSuccess(t *testing.T) {
 	}
 }
 
-func TestExtractor_TraceOutputFileError(t *testing.T) {
+func TestExtractorTraceOutputFileError(t *testing.T) {
 	inputFile := "fixtures/trace_success.json"
 	outputFile := "fixtures/trace_success_ui_anonymized.json"
 	defer os.Remove(outputFile)
 
-	reader, err := NewReader(
-		inputFile,
-		zap.NewNop(),
-	)
+	reader, err := newSpanReader(inputFile, zap.NewNop())
 	require.NoError(t, err)
 
 	err = os.Chmod("fixtures", 0o000)
 	require.NoError(t, err)
 	defer os.Chmod("fixtures", 0o755)
 
-	_, err = NewExtractor(
+	_, err = newExtractor(
 		outputFile,
 		"2be38093ead7a083",
 		reader,
@@ -86,18 +80,15 @@ func TestExtractor_TraceOutputFileError(t *testing.T) {
 	require.Contains(t, err.Error(), "cannot create output file")
 }
 
-func TestExtractor_TraceScanError(t *testing.T) {
+func TestExtractorTraceScanError(t *testing.T) {
 	inputFile := "fixtures/trace_scan_error.json"
 	outputFile := "fixtures/trace_scan_error_ui_anonymized.json"
 	defer os.Remove(outputFile)
 
-	reader, err := NewReader(
-		inputFile,
-		zap.NewNop(),
-	)
+	reader, err := newSpanReader(inputFile, zap.NewNop())
 	require.NoError(t, err)
 
-	extractor, err := NewExtractor(
+	extractor, err := newExtractor(
 		outputFile,
 		"2be38093ead7a083",
 		reader,

cmd/anonymizer/app/uiconv/module.go

@@ -28,11 +28,11 @@ type Config struct {
 // Extract reads anonymized file, finds spans for a given trace,
 // and writes out that trace in the UI format.
 func Extract(config Config, logger *zap.Logger) error {
-	reader, err := NewReader(config.CapturedFile, logger)
+	reader, err := newSpanReader(config.CapturedFile, logger)
 	if err != nil {
 		return err
 	}
-	ext, err := NewExtractor(config.UIFile, config.TraceID, reader, logger)
+	ext, err := newExtractor(config.UIFile, config.TraceID, reader, logger)
 	if err != nil {
 		return err
 	}

cmd/anonymizer/app/uiconv/reader.go

@@ -18,42 +18,43 @@ import (
 	"bufio"
 	"encoding/json"
 	"fmt"
-	"io"
 	"os"
 
 	"go.uber.org/zap"
 
 	uimodel "github.com/jaegertracing/jaeger/model/json"
 )
 
-// Reader loads previously captured spans from a file.
-type Reader struct {
+var errNoMoreSpans = fmt.Errorf("no more spans")
+
+// spanReader loads previously captured spans from a file.
+type spanReader struct {
 	logger       *zap.Logger
 	capturedFile *os.File
 	reader       *bufio.Reader
 	spansRead    int
 	eofReached   bool
 }
 
-// NewReader creates a Reader.
-func NewReader(capturedFile string, logger *zap.Logger) (*Reader, error) {
+// newSpanReader creates a spanReader.
+func newSpanReader(capturedFile string, logger *zap.Logger) (*spanReader, error) {
 	cf, err := os.OpenFile(capturedFile, os.O_RDONLY, os.ModePerm)
 	if err != nil {
 		return nil, fmt.Errorf("cannot open captured file: %w", err)
 	}
 	logger.Sugar().Infof("Reading captured spans from file %s", capturedFile)
 
-	return &Reader{
+	return &spanReader{
 		logger:       logger,
 		capturedFile: cf,
 		reader:       bufio.NewReader(cf),
 	}, nil
 }
 
-// NextSpan reads the next span from the capture file, or returns io.EOF.
-func (r *Reader) NextSpan() (*uimodel.Span, error) {
+// NextSpan reads the next span from the capture file, or returns errNoMoreSpans.
+func (r *spanReader) NextSpan() (*uimodel.Span, error) {
 	if r.eofReached {
-		return nil, io.EOF
+		return nil, errNoMoreSpans
 	}
 	if r.spansRead == 0 {
 		b, err := r.reader.ReadByte()

cmd/anonymizer/app/uiconv/reader_test.go

@@ -15,20 +15,16 @@
 package uiconv
 
 import (
-	"io"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"go.uber.org/zap"
 )
 
-func TestReader_TraceSuccess(t *testing.T) {
+func TestReaderTraceSuccess(t *testing.T) {
 	inputFile := "fixtures/trace_success.json"
-	r, err := NewReader(
-		inputFile,
-		zap.NewNop(),
-	)
+	r, err := newSpanReader(inputFile, zap.NewNop())
 	require.NoError(t, err)
 
 	s1, err := r.NextSpan()
@@ -46,26 +42,20 @@ func TestReader_TraceSuccess(t *testing.T) {
 	assert.Equal(t, true, r.eofReached)
 
 	_, err = r.NextSpan()
-	require.Equal(t, io.EOF, err)
+	require.Equal(t, errNoMoreSpans, err)
 	assert.Equal(t, 1000, r.spansRead)
 	assert.Equal(t, true, r.eofReached)
 }
 
-func TestReader_TraceNonExistent(t *testing.T) {
+func TestReaderTraceNonExistent(t *testing.T) {
 	inputFile := "fixtures/trace_non_existent.json"
-	_, err := NewReader(
-		inputFile,
-		zap.NewNop(),
-	)
+	_, err := newSpanReader(inputFile, zap.NewNop())
 	require.Contains(t, err.Error(), "cannot open captured file")
 }
 
-func TestReader_TraceEmpty(t *testing.T) {
+func TestReaderTraceEmpty(t *testing.T) {
 	inputFile := "fixtures/trace_empty.json"
-	r, err := NewReader(
-		inputFile,
-		zap.NewNop(),
-	)
+	r, err := newSpanReader(inputFile, zap.NewNop())
 	require.NoError(t, err)
 
 	_, err = r.NextSpan()
@@ -74,12 +64,9 @@ func TestReader_TraceEmpty(t *testing.T) {
 	assert.Equal(t, true, r.eofReached)
 }
 
-func TestReader_TraceWrongFormat(t *testing.T) {
+func TestReaderTraceWrongFormat(t *testing.T) {
 	inputFile := "fixtures/trace_wrong_format.json"
-	r, err := NewReader(
-		inputFile,
-		zap.NewNop(),
-	)
+	r, err := newSpanReader(inputFile, zap.NewNop())
 	require.NoError(t, err)
 
 	_, err = r.NextSpan()
@@ -88,12 +75,9 @@ func TestReader_TraceWrongFormat(t *testing.T) {
 	assert.Equal(t, true, r.eofReached)
 }
 
-func TestReader_TraceInvalidJson(t *testing.T) {
+func TestReaderTraceInvalidJson(t *testing.T) {
 	inputFile := "fixtures/trace_invalid_json.json"
-	r, err := NewReader(
-		inputFile,
-		zap.NewNop(),
-	)
+	r, err := newSpanReader(inputFile, zap.NewNop())
 	require.NoError(t, err)
 
 	_, err = r.NextSpan()

cmd/collector/app/handler/grpc_handler.go

@@ -16,6 +16,7 @@ package handler
 
 import (
 	"context"
+	"errors"
 
 	"go.uber.org/zap"
 	"google.golang.org/grpc/codes"
@@ -91,7 +92,7 @@ func (c *batchConsumer) consume(ctx context.Context, batch *model.Batch) error {
 		Tenant:           tenant,
 	})
 	if err != nil {
-		if err == processor.ErrBusy {
+		if errors.Is(err, processor.ErrBusy) {
 			return status.Errorf(codes.ResourceExhausted, err.Error())
 		}
 		c.logger.Error("cannot process spans", zap.Error(err))

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

@@ -16,6 +16,7 @@ package init
 
 import (
 	"encoding/json"
+	"errors"
 	"fmt"
 	"net/http"
 	"strings"
@@ -84,7 +85,8 @@ func (c Action) Do() error {
 func createIndexIfNotExist(c client.IndexAPI, index string) error {
 	err := c.CreateIndex(index)
 	if err != nil {
-		if esErr, ok := err.(client.ResponseError); ok {
+		var esErr client.ResponseError
+		if errors.As(err, &esErr) {
 			if esErr.StatusCode != http.StatusBadRequest || esErr.Body == nil {
 				return esErr.Err
 			}

cmd/internal/flags/admin.go

@@ -17,6 +17,7 @@ package flags
 import (
 	"context"
 	"crypto/tls"
+	"errors"
 	"flag"
 	"fmt"
 	"io"
@@ -148,10 +149,7 @@ func (s *AdminServer) serveWithListener(l net.Listener) {
 		} else {
 			err = s.server.Serve(l)
 		}
-		switch err {
-		case nil, http.ErrServerClosed:
-			// normal exit, nothing to do
-		default:
+		if err != nil && !errors.Is(err, http.ErrServerClosed) {
 			s.logger.Error("failed to serve", zap.Error(err))
 			s.hc.Set(healthcheck.Broken)
 		}

cmd/query/app/flags.go

@@ -17,6 +17,7 @@ package app
 
 import (
 	"bufio"
+	"errors"
 	"flag"
 	"fmt"
 	"io"
@@ -175,7 +176,7 @@ func stringSliceAsHeader(slice []string) (http.Header, error) {
 	tp := textproto.NewReader(reader)
 
 	header, err := tp.ReadMIMEHeader()
-	if err != nil && err != io.EOF {
+	if err != nil && !errors.Is(err, io.EOF) {
 		return nil, fmt.Errorf("failed to parse headers")
 	}