fix

Signed-off-by: Yuri Shkuro <[email protected]>

.golangci.yml

@@ -220,6 +220,9 @@ linters-settings:
         # looks useful, but requires refactoring: "calls to log.Fatal only in main() or init() functions"
         - name: deep-exit
           disabled: true
+        # this rule conflicts with nolintlint which does insist on no-space in //nolint
+        - name: comment-spacings
+          disabled: true
   testifylint:
     disable:
       - float-compare

Makefile

@@ -179,11 +179,6 @@ nocover:
 	@echo Verifying that all packages have test files to count in coverage
 	@scripts/check-test-files.sh $(ALL_PKGS)
 
-.PHONY: goleak
-goleak:
-	@echo Verifying that all packages with tests have goleak in their TestMain
-	@scripts/check-goleak-files.sh $(ALL_PKGS)
-
 .PHONY: fmt
 fmt: $(GOFUMPT)
 	@echo Running import-order-cleanup on ALL_SRC ...
@@ -196,12 +191,35 @@ fmt: $(GOFUMPT)
 	@./scripts/updateLicense.py $(ALL_SRC) $(SCRIPTS_SRC)
 
 .PHONY: lint
-lint: $(LINT) goleak
+lint: lint-license lint-imports lint-semconv lint-goversion lint-goleak lint-go
+
+.PHONY: lint-license
+lint-license:
+	@echo Verifying that all files have license headers
 	@./scripts/updateLicense.py $(ALL_SRC) $(SCRIPTS_SRC) > $(FMT_LOG)
+	@[ ! -s "$(FMT_LOG)" ] || (echo "License check failures, run 'make fmt'" | cat - $(FMT_LOG) && false)
+
+.PHONY: lint-imports
+lint-imports:
+	@echo Verifying that all Go files have correctly ordered imports
 	@./scripts/import-order-cleanup.py -o stdout -t $(ALL_SRC) > $(IMPORT_LOG)
-	@[ ! -s "$(FMT_LOG)" -a ! -s "$(IMPORT_LOG)" ] || (echo "License check or import ordering failures, run 'make fmt'" | cat - $(FMT_LOG) $(IMPORT_LOG) && false)
+	@[ ! -s "$(IMPORT_LOG)" ] || (echo "Import ordering failures, run 'make fmt'" | cat - $(IMPORT_LOG) && false)
+
+.PHONY: lint-semconv
+lint-semconv:
 	./scripts/check-semconv-version.sh
+
+.PHONY: lint-goversion
+lint-goversion:
 	./scripts/check-go-version.sh
+
+.PHONY: lint-goleak
+lint-goleak:
+	@echo Verifying that all packages with tests have goleak in their TestMain
+	@scripts/check-goleak-files.sh $(ALL_PKGS)
+
+.PHONY: lint-go
+lint-go: $(LINT)
 	$(LINT) -v run
 
 .PHONY: build-examples

cmd/collector/app/handler/grpc_handler.go

@@ -93,7 +93,7 @@ func (c *batchConsumer) consume(ctx context.Context, batch *model.Batch) error {
 	})
 	if err != nil {
 		if errors.Is(err, processor.ErrBusy) {
-			return status.Errorf(codes.ResourceExhausted, err.Error())
+			return status.Error(codes.ResourceExhausted, err.Error())
 		}
 		c.logger.Error("cannot process spans", zap.Error(err))
 		return err

cmd/collector/app/span_processor_test.go

@@ -474,7 +474,7 @@ func TestSpanProcessorCountSpan(t *testing.T) {
 					time.Second,
 					time.Millisecond,
 				)
-				assert.Greater(t, p.spansProcessed.Load(), uint64(0))
+				assert.Positive(t, p.spansProcessed.Load())
 			}
 
 			for i := 0; i < 10000; i++ {

cmd/ingester/app/consumer/deadlock_detector_test.go

@@ -45,7 +45,7 @@ func TestNoClosingSignalIfMessagesProcessedInInterval(t *testing.T) {
 	w := f.startMonitoringForPartition(1)
 
 	w.incrementMsgCount()
-	assert.Zero(t, len(w.closePartitionChannel()))
+	assert.Empty(t, w.closePartitionChannel())
 	w.close()
 }
 
@@ -160,6 +160,6 @@ func TestApiCompatibilityWhenDeadlockDetectorDisabled(t *testing.T) {
 
 	w.incrementMsgCount()
 	w.incrementAllPartitionMsgCount()
-	assert.Zero(t, len(w.closePartitionChannel()))
+	assert.Empty(t, w.closePartitionChannel())
 	w.close()
 }

cmd/query/app/apiv3/grpc_handler.go

@@ -16,6 +16,7 @@ package apiv3
 
 import (
 	"context"
+	"errors"
 	"fmt"
 
 	"github.com/gogo/protobuf/types"
@@ -59,11 +60,11 @@ func (h *Handler) GetTrace(request *api_v3.GetTraceRequest, stream api_v3.QueryS
 func (h *Handler) FindTraces(request *api_v3.FindTracesRequest, stream api_v3.QueryService_FindTracesServer) error {
 	query := request.GetQuery()
 	if query == nil {
-		return status.Errorf(codes.InvalidArgument, "missing query")
+		return status.Error(codes.InvalidArgument, "missing query")
 	}
 	if query.GetStartTimeMin() == nil ||
 		query.GetStartTimeMax() == nil {
-		return fmt.Errorf("start time min and max are required parameters")
+		return errors.New("start time min and max are required parameters")
 	}
 
 	queryParams := &spanstore.TraceQueryParameters{

cmd/query/app/apiv3/http_gateway_test.go

@@ -4,6 +4,7 @@
 package apiv3
 
 import (
+	"errors"
 	"fmt"
 	"io"
 	"net/http"
@@ -105,7 +106,7 @@ func TestHTTPGatewayTryHandleError(t *testing.T) {
 	gw.Logger = logger
 	w = httptest.NewRecorder()
 	const e = "some err"
-	assert.True(t, gw.tryHandleError(w, fmt.Errorf(e), http.StatusInternalServerError))
+	assert.True(t, gw.tryHandleError(w, errors.New(e), http.StatusInternalServerError))
 	assert.Contains(t, log.String(), e, "logs error if status code is 500")
 	assert.Contains(t, string(w.Body.String()), e, "writes error message to body")
 }
@@ -118,7 +119,7 @@ func TestHTTPGatewayOTLPError(t *testing.T) {
 	const simErr = "simulated error"
 	gw.returnSpansTestable(nil, w,
 		func(_ []*model.Span) (ptrace.Traces, error) {
-			return ptrace.Traces{}, fmt.Errorf(simErr)
+			return ptrace.Traces{}, errors.New(simErr)
 		},
 	)
 	assert.Contains(t, w.Body.String(), simErr)
@@ -138,7 +139,7 @@ func TestHTTPGatewayGetTraceErrors(t *testing.T) {
 	const simErr = "simulated error"
 	gw.reader.
 		On("GetTrace", matchContext, matchTraceID).
-		Return(nil, fmt.Errorf(simErr)).Once()
+		Return(nil, errors.New(simErr)).Once()
 
 	r, err = http.NewRequest(http.MethodGet, "/api/v3/traces/123", nil)
 	require.NoError(t, err)
@@ -247,7 +248,7 @@ func TestHTTPGatewayFindTracesErrors(t *testing.T) {
 		gw := setupHTTPGatewayNoServer(t, "", tenancy.Options{})
 		gw.reader.
 			On("FindTraces", matchContext, qp).
-			Return(nil, fmt.Errorf(simErr)).Once()
+			Return(nil, errors.New(simErr)).Once()
 
 		gw.router.ServeHTTP(w, r)
 		assert.Contains(t, w.Body.String(), simErr)
@@ -260,7 +261,7 @@ func TestHTTPGatewayGetServicesErrors(t *testing.T) {
 	const simErr = "simulated error"
 	gw.reader.
 		On("GetServices", matchContext).
-		Return(nil, fmt.Errorf(simErr)).Once()
+		Return(nil, errors.New(simErr)).Once()
 
 	r, err := http.NewRequest(http.MethodGet, "/api/v3/services", nil)
 	require.NoError(t, err)
@@ -276,7 +277,7 @@ func TestHTTPGatewayGetOperationsErrors(t *testing.T) {
 	const simErr = "simulated error"
 	gw.reader.
 		On("GetOperations", matchContext, qp).
-		Return(nil, fmt.Errorf(simErr)).Once()
+		Return(nil, errors.New(simErr)).Once()
 
 	r, err := http.NewRequest(http.MethodGet, "/api/v3/operations?service=foo&span_kind=server", nil)
 	require.NoError(t, err)

cmd/query/app/http_handler_test.go

@@ -428,9 +428,9 @@ func TestSearchByTraceIDNotFound(t *testing.T) {
 
 func TestSearchByTraceIDFailure(t *testing.T) {
 	ts := initializeTestServer(t)
-	whatsamattayou := "https://youtu.be/WrKFOCg13QQ"
+	whatsamattayou := "whatsamattayou"
 	ts.spanReader.On("GetTrace", mock.AnythingOfType("*context.valueCtx"), mock.AnythingOfType("model.TraceID")).
-		Return(nil, fmt.Errorf(whatsamattayou)).Once()
+		Return(nil, errors.New(whatsamattayou)).Once()
 
 	var response structuredResponse
 	err := getJSON(ts.server.URL+`/api/traces?traceID=1`, &response)
@@ -763,7 +763,7 @@ func TestMetricsReaderError(t *testing.T) {
 				tc.mockedQueryMethod,
 				mock.AnythingOfType("*context.valueCtx"),
 				mock.AnythingOfType(tc.mockedQueryMethodParamType),
-			).Return(tc.mockedResponse, fmt.Errorf(tc.wantErrorMessage)).Once()
+			).Return(tc.mockedResponse, errors.New(tc.wantErrorMessage)).Once()
 
 			// Test
 			var response metrics.MetricFamily

cmd/query/app/query_parser_test.go

@@ -16,7 +16,6 @@
 package app
 
 import (
-	"fmt"
 	"net/http"
 	"regexp"
 	"testing"
@@ -174,7 +173,7 @@ func TestParseTraceQuery(t *testing.T) {
 			} else {
 				matched, matcherr := regexp.MatchString(test.errMsg, err.Error())
 				require.NoError(t, matcherr)
-				assert.True(t, matched, fmt.Sprintf("Error \"%s\" should match \"%s\"", err.Error(), test.errMsg))
+				assert.True(t, matched, "Error \"%s\" should match \"%s\"", err.Error(), test.errMsg)
 			}
 		})
 	}

crossdock/services/tracehandler_test.go

@@ -276,7 +276,7 @@ func TestCreateTracesLoop(t *testing.T) {
 		}
 		time.Sleep(time.Millisecond)
 	}
-	assert.Greater(t, h.CallCount(), int64(0))
+	assert.Positive(t, h.CallCount())
 }
 
 func TestValidateAdaptiveSamplingTraces(t *testing.T) {

model/keyvalue.go

@@ -218,6 +218,8 @@ func (kv KeyValue) Hash(w io.Writer) error {
 	case StringType:
 		_, err = w.Write([]byte(kv.VStr))
 	case BoolType:
+		// staticcheck incorrectly complains about bool, even though its docs say bool wasn’t supported only before Go 1.8.
+		//nolint: staticcheck
 		err = binary.Write(w, binary.BigEndian, kv.VBool)
 	case Int64Type:
 		err = binary.Write(w, binary.BigEndian, kv.VInt64)

plugin/storage/badger/factory_test.go

@@ -120,7 +120,7 @@ func TestMaintenanceRun(t *testing.T) {
 
 	waiter(runtime)
 	_, gs = mFactory.Snapshot()
-	assert.Greater(t, gs[lastValueLogCleanedName], int64(0))
+	assert.Positive(t, gs[lastValueLogCleanedName])
 
 	err := io.Closer(f).Close()
 	require.NoError(t, err)

plugin/storage/es/spanstore/reader.go

@@ -417,7 +417,7 @@ func (s *SpanReader) multiRead(ctx context.Context, traceIDs []model.TraceID, st
 			return nil, err
 		}
 
-		if results.Responses == nil || len(results.Responses) == 0 {
+		if len(results.Responses) == 0 {
 			break
 		}
 

plugin/storage/grpc/shared/grpc_handler.go

@@ -164,7 +164,7 @@ func (s *GRPCHandler) Close(context.Context, *storage_v1.CloseWriterRequest) (*s
 func (s *GRPCHandler) GetTrace(r *storage_v1.GetTraceRequest, stream storage_v1.SpanReaderPlugin_GetTraceServer) error {
 	trace, err := s.impl.SpanReader().GetTrace(stream.Context(), r.TraceID)
 	if errors.Is(err, spanstore.ErrTraceNotFound) {
-		return status.Errorf(codes.NotFound, spanstore.ErrTraceNotFound.Error())
+		return status.Error(codes.NotFound, spanstore.ErrTraceNotFound.Error())
 	}
 	if err != nil {
 		return err
@@ -289,7 +289,7 @@ func (s *GRPCHandler) GetArchiveTrace(r *storage_v1.GetTraceRequest, stream stor
 	}
 	trace, err := reader.GetTrace(stream.Context(), r.TraceID)
 	if errors.Is(err, spanstore.ErrTraceNotFound) {
-		return status.Errorf(codes.NotFound, spanstore.ErrTraceNotFound.Error())
+		return status.Error(codes.NotFound, spanstore.ErrTraceNotFound.Error())
 	}
 	if err != nil {
 		return err

plugin/storage/integration/es_index_cleaner_test.go

@@ -161,7 +161,7 @@ func runIndexCleanerTest(t *testing.T, client *elastic.Client, v8Client *elastic
 	for _, index := range expectedIndices {
 		expected = append(expected, prefix+index)
 	}
-	assert.ElementsMatch(t, indices, expected, fmt.Sprintf("indices found: %v, expected: %v", indices, expected))
+	assert.ElementsMatch(t, indices, expected, "indices found: %v, expected: %v", indices, expected)
 }
 
 func createAllIndices(client *elastic.Client, prefix string, adaptiveSampling bool) error {

plugin/storage/integration/es_index_rollover_test.go

@@ -16,7 +16,6 @@ package integration
 
 import (
 	"context"
-	"fmt"
 	"strconv"
 	"testing"
 
@@ -129,9 +128,9 @@ func runIndexRolloverWithILMTest(t *testing.T, client *elastic.Client, prefix st
 		actualWriteAliases = append(actualWriteAliases, v.Settings["index.lifecycle.rollover_alias"].(string))
 	}
 	// Check indices created
-	assert.ElementsMatch(t, indices, expected, fmt.Sprintf("indices found: %v, expected: %v", indices, expected))
+	assert.ElementsMatch(t, indices, expected)
 	// Check rollover alias is write alias
-	assert.ElementsMatch(t, actualWriteAliases, expectedWriteAliases, fmt.Sprintf("aliases found: %v, expected: %v", actualWriteAliases, expectedWriteAliases))
+	assert.ElementsMatch(t, actualWriteAliases, expectedWriteAliases)
 }
 
 func getVersion(client *elastic.Client) (uint, error) {

scripts/check-go-version.sh

@@ -19,7 +19,12 @@ while getopts "uvdx" opt; do
 done
 
 # Fetch latest go release version
-go_latest_version=$(curl -s https://go.dev/dl/?mode=json | jq -r '.[0].version' | awk -F'.' '{gsub("go", ""); print $1"."$2}')
+# go_latest_version=$(curl -s https://go.dev/dl/?mode=json | jq -r '.[0].version' | awk -F'.' '{gsub("go", ""); print $1"."$2}')
+#
+# UPDATE: we don't use the logic above because it causes CI to fail when new version of Go is released,
+# which may create circular dependencies when other utilities need to be upgraded. Instead use the toolchain
+# version declared in the main go.mod. Updates to that version will be handled by the bots.
+go_latest_version=$(grep toolchain go.mod | sed 's/^.*go\([0-9]\.[0-9]*\).*/\1/')
 go_previous_version="${go_latest_version%.*}.$((10#${go_latest_version#*.} - 1))"
 
 files_to_update=0