Skip to content

Commit

Permalink
Enable gofumpt linter (jaegertracing#3729)
Browse files Browse the repository at this point in the history
Signed-off-by: Matthieu MOREL <[email protected]>

Co-authored-by: Matthieu MOREL <[email protected]>
  • Loading branch information
mmorel-35 and mmorel-35 authored Jun 5, 2022
1 parent 456da8b commit 458c1ce
Show file tree
Hide file tree
Showing 154 changed files with 646 additions and 542 deletions.
10 changes: 6 additions & 4 deletions .golangci.yml
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
run:
timeout: 10m
skip-dirs:
- swagger-gen
- pkg/cassandra/mocks
- storage/mocks
- cmd/ingester/app/consumer/mocks
- mocks
- thrift-0.9.2
- .*-gen
skip-files:
- ".*.pb.go$"

linters-settings:
depguard:
Expand Down Expand Up @@ -32,6 +33,7 @@ linters:
- contextcheck
- depguard
- gofmt
- gofumpt
- goimports
- gosec
- govet
Expand Down
6 changes: 5 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ GOARCH ?= $(shell go env GOARCH)
GOBUILD=CGO_ENABLED=0 installsuffix=cgo go build -trimpath
GOTEST=go test -v $(RACE)
GOFMT=gofmt
GOFUMPT=gofumpt
FMT_LOG=.fmt.log
IMPORT_LOG=.import.log

Expand Down Expand Up @@ -145,8 +146,10 @@ nocover:
.PHONY: fmt
fmt:
./scripts/import-order-cleanup.sh inplace
@echo Running go fmt on ALL_SRC ...
@echo Running gofmt on ALL_SRC ...
@$(GOFMT) -e -s -l -w $(ALL_SRC)
@echo Running gofumpt on ALL_SRC ...
@$(GOFUMPT) -e -l -w $(ALL_SRC)
./scripts/updateLicenses.sh

.PHONY: lint
Expand Down Expand Up @@ -371,6 +374,7 @@ install-tools:
# FIXME: pin to f5b92e1 until v1.45.3 is available to pick up fixes for staticheck
# go install github.com/golangci/golangci-lint/cmd/[email protected]
go install github.com/golangci/golangci-lint/cmd/golangci-lint@f5b92e1
go install mvdan.cc/gofumpt@latest

.PHONY: install-ci
install-ci: install-tools
Expand Down
10 changes: 4 additions & 6 deletions cmd/agent/app/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,12 +55,10 @@ type Model string
// Protocol used to distinguish the data transfer protocol
type Protocol string

var (
protocolFactoryMap = map[Protocol]thrift.TProtocolFactory{
compactProtocol: thrift.NewTCompactProtocolFactoryConf(&thrift.TConfiguration{}),
binaryProtocol: thrift.NewTBinaryProtocolFactoryConf(&thrift.TConfiguration{}),
}
)
var protocolFactoryMap = map[Protocol]thrift.TProtocolFactory{
compactProtocol: thrift.NewTCompactProtocolFactoryConf(&thrift.TConfiguration{}),
binaryProtocol: thrift.NewTBinaryProtocolFactoryConf(&thrift.TConfiguration{}),
}

// CollectorProxy provides access to Reporter and ClientConfigManager
type CollectorProxy interface {
Expand Down
7 changes: 5 additions & 2 deletions cmd/agent/app/builder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -181,29 +181,32 @@ func TestMultipleCollectorProxies(t *testing.T) {
assert.Equal(t, ra, mr[1])
}

type fakeCollectorProxy struct {
}
type fakeCollectorProxy struct{}

func (f fakeCollectorProxy) GetReporter() reporter.Reporter {
return fakeCollectorProxy{}
}

func (f fakeCollectorProxy) GetManager() configmanager.ClientConfigManager {
return fakeCollectorProxy{}
}

func (fakeCollectorProxy) EmitZipkinBatch(_ context.Context, _ []*zipkincore.Span) (err error) {
return nil
}

func (fakeCollectorProxy) EmitBatch(_ context.Context, _ *jaeger.Batch) (err error) {
return nil
}

func (fakeCollectorProxy) Close() error {
return nil
}

func (f fakeCollectorProxy) GetSamplingStrategy(_ context.Context, _ string) (*sampling.SamplingStrategyResponse, error) {
return nil, errors.New("no peers available")
}

func (fakeCollectorProxy) GetBaggageRestrictions(_ context.Context, _ string) ([]*baggage.BaggageRestriction, error) {
return nil, nil
}
Expand Down
3 changes: 1 addition & 2 deletions cmd/agent/app/configmanager/grpc/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,7 @@ func TestSamplingManager_GetBaggageRestrictions(t *testing.T) {
assert.EqualError(t, err, "baggage not implemented")
}

type mockSamplingHandler struct {
}
type mockSamplingHandler struct{}

func (*mockSamplingHandler) GetSamplingStrategy(context.Context, *api_v2.SamplingStrategyParameters) (*api_v2.SamplingStrategyResponse, error) {
return &api_v2.SamplingStrategyResponse{StrategyType: api_v2.SamplingStrategyType_PROBABILISTIC}, nil
Expand Down
4 changes: 2 additions & 2 deletions cmd/agent/app/configmanager/metrics_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,15 @@ import (
"github.com/jaegertracing/jaeger/thrift-gen/sampling"
)

type noopManager struct {
}
type noopManager struct{}

func (noopManager) GetSamplingStrategy(_ context.Context, s string) (*sampling.SamplingStrategyResponse, error) {
if s == "failed" {
return nil, errors.New("failed")
}
return &sampling.SamplingStrategyResponse{StrategyType: sampling.SamplingStrategyType_PROBABILISTIC}, nil
}

func (noopManager) GetBaggageRestrictions(_ context.Context, s string) ([]*baggage.BaggageRestriction, error) {
if s == "failed" {
return nil, errors.New("failed")
Expand Down
2 changes: 1 addition & 1 deletion cmd/agent/app/processors/thrift_processor.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ func NewThriftProcessor(
return nil, fmt.Errorf(
"number of processors must be greater than 0, called with %d", numProcessors)
}
var protocolPool = &sync.Pool{
protocolPool := &sync.Pool{
New: func() interface{} {
trans := &customtransport.TBufferedReadTransport{}
return factory.GetProtocol(trans)
Expand Down
1 change: 0 additions & 1 deletion cmd/agent/app/reporter/flags_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,6 @@ func TestBindFlags(t *testing.T) {
}

func TestBindFlagsAllInOne(t *testing.T) {

setupcontext.SetAllInOne()
defer setupcontext.UnsetAllInOne()

Expand Down
1 change: 0 additions & 1 deletion cmd/agent/app/reporter/grpc/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,6 @@ func (b *ConnBuilder) CreateConnection(logger *zap.Logger, mFactory metrics.Fact
dialOptions = append(dialOptions, grpc.WithDefaultServiceConfig(grpcresolver.GRPCServiceConfig))
dialOptions = append(dialOptions, grpc.WithUnaryInterceptor(grpc_retry.UnaryClientInterceptor(grpc_retry.WithMax(b.MaxRetry))))
conn, err := grpc.Dial(dialTarget, dialOptions...)

if err != nil {
return nil, err
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/agent/app/reporter/grpc/collector_proxy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ func TestMultipleCollectors(t *testing.T) {
assert.NotNil(t, proxy.GetManager())
assert.NotNil(t, proxy.GetConn())

var bothServers = false
bothServers := false
r := proxy.GetReporter()
// TODO do not iterate, just create two batches
for i := 0; i < 100; i++ {
Expand Down
18 changes: 12 additions & 6 deletions cmd/agent/app/reporter/grpc/flags_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,18 @@ func TestBindFlags(t *testing.T) {
cOpts []string
expected *ConnBuilder
}{
{cOpts: []string{"--reporter.grpc.host-port=localhost:1111", "--reporter.grpc.retry.max=15"},
expected: &ConnBuilder{CollectorHostPorts: []string{"localhost:1111"}, MaxRetry: 15, DiscoveryMinPeers: 3}},
{cOpts: []string{"--reporter.grpc.host-port=localhost:1111,localhost:2222"},
expected: &ConnBuilder{CollectorHostPorts: []string{"localhost:1111", "localhost:2222"}, MaxRetry: defaultMaxRetry, DiscoveryMinPeers: 3}},
{cOpts: []string{"--reporter.grpc.host-port=localhost:1111,localhost:2222", "--reporter.grpc.discovery.min-peers=5"},
expected: &ConnBuilder{CollectorHostPorts: []string{"localhost:1111", "localhost:2222"}, MaxRetry: defaultMaxRetry, DiscoveryMinPeers: 5}},
{
cOpts: []string{"--reporter.grpc.host-port=localhost:1111", "--reporter.grpc.retry.max=15"},
expected: &ConnBuilder{CollectorHostPorts: []string{"localhost:1111"}, MaxRetry: 15, DiscoveryMinPeers: 3},
},
{
cOpts: []string{"--reporter.grpc.host-port=localhost:1111,localhost:2222"},
expected: &ConnBuilder{CollectorHostPorts: []string{"localhost:1111", "localhost:2222"}, MaxRetry: defaultMaxRetry, DiscoveryMinPeers: 3},
},
{
cOpts: []string{"--reporter.grpc.host-port=localhost:1111,localhost:2222", "--reporter.grpc.discovery.min-peers=5"},
expected: &ConnBuilder{CollectorHostPorts: []string{"localhost:1111", "localhost:2222"}, MaxRetry: defaultMaxRetry, DiscoveryMinPeers: 5},
},
}
for _, test := range tests {
v := viper.New()
Expand Down
17 changes: 12 additions & 5 deletions cmd/agent/app/reporter/grpc/reporter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,11 +71,16 @@ func TestReporter_EmitZipkinBatch(t *testing.T) {
err string
}{
{in: &zipkincore.Span{}, err: "cannot find service name in Zipkin span [traceID=0, spanID=0]"},
{in: &zipkincore.Span{Name: "jonatan", TraceID: 1, ID: 2, Timestamp: &a, Annotations: []*zipkincore.Annotation{{Value: zipkincore.CLIENT_SEND, Host: &zipkincore.Endpoint{ServiceName: "spring"}}}},
{
in: &zipkincore.Span{Name: "jonatan", TraceID: 1, ID: 2, Timestamp: &a, Annotations: []*zipkincore.Annotation{{Value: zipkincore.CLIENT_SEND, Host: &zipkincore.Endpoint{ServiceName: "spring"}}}},
expected: model.Batch{
Spans: []*model.Span{{TraceID: model.NewTraceID(0, 1), SpanID: model.NewSpanID(2), OperationName: "jonatan", Duration: time.Microsecond * 1,
Spans: []*model.Span{{
TraceID: model.NewTraceID(0, 1), SpanID: model.NewSpanID(2), OperationName: "jonatan", Duration: time.Microsecond * 1,
Tags: model.KeyValues{{Key: "span.kind", VStr: "client", VType: model.StringType}},
Process: &model.Process{ServiceName: "spring"}, StartTime: tm.UTC()}}}},
Process: &model.Process{ServiceName: "spring"}, StartTime: tm.UTC(),
}},
},
},
}
for _, test := range tests {
err = rep.EmitZipkinBatch(context.Background(), []*zipkincore.Span{test.in})
Expand Down Expand Up @@ -106,8 +111,10 @@ func TestReporter_EmitBatch(t *testing.T) {
expected model.Batch
err string
}{
{in: &jThrift.Batch{Process: &jThrift.Process{ServiceName: "node"}, Spans: []*jThrift.Span{{OperationName: "foo", StartTime: int64(model.TimeAsEpochMicroseconds(tm))}}},
expected: model.Batch{Process: &model.Process{ServiceName: "node"}, Spans: []*model.Span{{OperationName: "foo", StartTime: tm.UTC()}}}},
{
in: &jThrift.Batch{Process: &jThrift.Process{ServiceName: "node"}, Spans: []*jThrift.Span{{OperationName: "foo", StartTime: int64(model.TimeAsEpochMicroseconds(tm))}}},
expected: model.Batch{Process: &model.Process{ServiceName: "node"}, Spans: []*model.Span{{OperationName: "foo", StartTime: tm.UTC()}}},
},
}
for _, test := range tests {
err = rep.EmitBatch(context.Background(), test.in)
Expand Down
5 changes: 3 additions & 2 deletions cmd/agent/app/servers/tbuffered_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,13 +78,14 @@ func NewTBufferedServer(
) (*TBufferedServer, error) {
dataChan := make(chan *ReadBuf, maxQueueSize)

var readBufPool = &sync.Pool{
readBufPool := &sync.Pool{
New: func() interface{} {
return &ReadBuf{bytes: make([]byte, maxPacketSize)}
},
}

res := &TBufferedServer{dataChan: dataChan,
res := &TBufferedServer{
dataChan: dataChan,
transport: transport,
maxQueueSize: maxQueueSize,
maxPacketSize: maxPacketSize,
Expand Down
2 changes: 1 addition & 1 deletion cmd/agent/app/servers/thriftudp/transport.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import (
"github.com/apache/thrift/lib/go/thrift"
)

//MaxLength of UDP packet
// MaxLength of UDP packet
const (
MaxLength = 65000
)
Expand Down
14 changes: 7 additions & 7 deletions cmd/agent/app/servers/thriftudp/transport_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ func TestNewTUDPClientTransport(t *testing.T) {
require.True(t, trans.IsOpen())
require.NotNil(t, trans.Addr())

//Check address
// Check address
assert.True(t, strings.HasPrefix(trans.Addr().String(), "127.0.0.1:"), "address check")
require.Equal(t, "udp", trans.Addr().Network())

Expand All @@ -64,10 +64,10 @@ func TestNewTUDPServerTransport(t *testing.T) {
require.True(t, trans.IsOpen())
require.Equal(t, ^uint64(0), trans.RemainingBytes())

//Ensure a second server can't be created on the same address
// Ensure a second server can't be created on the same address
trans2, err := NewTUDPServerTransport(trans.Addr().String())
if trans2 != nil {
//close the second server if one got created
// close the second server if one got created
trans2.Close()
}
require.NotNil(t, err)
Expand Down Expand Up @@ -152,7 +152,7 @@ func TestDoubleCloseError(t *testing.T) {
require.Nil(t, err)
require.True(t, trans.IsOpen())

//Close connection object directly
// Close connection object directly
conn := trans.Conn()
require.NotNil(t, conn)
conn.Close()
Expand Down Expand Up @@ -185,7 +185,7 @@ func TestHugeWrite(t *testing.T) {
_, err = trans.Write(hugeMessage)
require.Nil(t, err)

//expect buffer to exceed max
// expect buffer to exceed max
_, err = trans.Write(hugeMessage)
require.NotNil(t, err)
})
Expand All @@ -196,12 +196,12 @@ func TestFlushErrors(t *testing.T) {
trans, err := NewTUDPClientTransport(addr, "")
require.Nil(t, err)

//flushing closed transport
// flushing closed transport
trans.Close()
err = trans.Flush(context.Background())
require.NotNil(t, err)

//error when trying to write in flush
// error when trying to write in flush
trans, err = NewTUDPClientTransport(addr, "")
require.Nil(t, err)
trans.conn.Close()
Expand Down
2 changes: 1 addition & 1 deletion cmd/agent/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func main() {
svc.NoStorage = true

v := viper.New()
var command = &cobra.Command{
command := &cobra.Command{
Use: "jaeger-agent",
Short: "Jaeger agent is a local daemon program which collects tracing data.",
Long: `Jaeger agent is a daemon program that runs on every host and receives tracing data submitted by Jaeger client libraries.`,
Expand Down
8 changes: 3 additions & 5 deletions cmd/all-in-one/all_in_one_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,11 +51,9 @@ const (
getServicesAPIV3URL = queryURL + "/api/v3/services"
)

var (
httpClient = &http.Client{
Timeout: time.Second,
}
)
var httpClient = &http.Client{
Timeout: time.Second,
}

func TestAllInOne(t *testing.T) {
// Check if the query service is available
Expand Down
3 changes: 0 additions & 3 deletions cmd/all-in-one/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ import (

// all-in-one/main is a standalone full-stack jaeger backend, backed by a memory store
func main() {

setupcontext.SetAllInOne()

svc := flags.NewService(ports.CollectorAdminHTTP)
Expand Down Expand Up @@ -245,7 +244,6 @@ func startAgent(
logger *zap.Logger,
baseFactory metrics.Factory,
) *agentApp.Agent {

agent, err := b.CreateAgent(cp, logger, baseFactory)
if err != nil {
logger.Fatal("Unable to initialize Jaeger Agent", zap.Error(err))
Expand Down Expand Up @@ -315,7 +313,6 @@ func createMetricsQueryService(
logger *zap.Logger,
metricsReaderMetricsFactory metrics.Factory,
) (querysvc.MetricsQueryService, error) {

if err := metricsReaderFactory.Initialize(logger); err != nil {
return nil, fmt.Errorf("failed to init metrics reader factory: %w", err)
}
Expand Down
4 changes: 2 additions & 2 deletions cmd/anonymizer/app/uiconv/extractor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,9 @@ func TestExtractor_TraceOutputFileError(t *testing.T) {
)
require.NoError(t, err)

err = os.Chmod("fixtures", 0000)
err = os.Chmod("fixtures", 0o000)
require.NoError(t, err)
defer os.Chmod("fixtures", 0755)
defer os.Chmod("fixtures", 0o755)

_, err = NewExtractor(
outputFile,
Expand Down
4 changes: 2 additions & 2 deletions cmd/anonymizer/app/uiconv/module_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,9 @@ func TestModule_TraceOutputFileError(t *testing.T) {
TraceID: "2be38093ead7a083",
}

err := os.Chmod("fixtures", 0550)
err := os.Chmod("fixtures", 0o550)
require.NoError(t, err)
defer os.Chmod("fixtures", 0755)
defer os.Chmod("fixtures", 0o755)

err = Extract(config, zap.NewNop())
require.Contains(t, err.Error(), "cannot create output file")
Expand Down
4 changes: 2 additions & 2 deletions cmd/anonymizer/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,9 @@ import (
var logger, _ = zap.NewDevelopment()

func main() {
var options = app.Options{}
options := app.Options{}

var command = &cobra.Command{
command := &cobra.Command{
Use: "jaeger-anonymizer",
Short: "Jaeger anonymizer hashes fields of a trace for easy sharing",
Long: `Jaeger anonymizer queries Jaeger query for a trace, anonymizes fields, and store in file`,
Expand Down
3 changes: 1 addition & 2 deletions cmd/collector/app/collector_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,8 +115,7 @@ func TestCollector_StartErrors(t *testing.T) {
run("OTLP/HTTP", options, "could not start OTLP receiver")
}

type mockStrategyStore struct {
}
type mockStrategyStore struct{}

func (m *mockStrategyStore) GetSamplingStrategy(_ context.Context, serviceName string) (*sampling.SamplingStrategyResponse, error) {
return &sampling.SamplingStrategyResponse{}, nil
Expand Down
Loading

0 comments on commit 458c1ce

Please sign in to comment.