From 10099645ac11a3aa9e15c8179390c42805cc08c9 Mon Sep 17 00:00:00 2001 From: Preetam Dwivedi Date: Fri, 13 Feb 2026 14:51:48 -0800 Subject: [PATCH 1/2] Cleanup From 8deb39716db227052f4270b0a3b9f7eac676fda1 Mon Sep 17 00:00:00 2001 From: Preetam Dwivedi Date: Fri, 13 Feb 2026 15:07:57 -0800 Subject: [PATCH 2/2] chore (logger): remove nil check from controllers --- gateway/controller/BUILD.bazel | 10 ++++++++-- gateway/controller/land.go | 7 ------- gateway/controller/land_test.go | 6 ++++-- gateway/controller/ping.go | 7 ------- gateway/controller/ping_test.go | 14 ++++++++------ orchestrator/controller/ping.go | 7 ------- orchestrator/controller/ping_test.go | 14 ++++++++------ speculator/controller/ping.go | 7 ------- speculator/controller/ping_test.go | 14 ++++++++------ 9 files changed, 36 insertions(+), 50 deletions(-) diff --git a/gateway/controller/BUILD.bazel b/gateway/controller/BUILD.bazel index d625087d..c8f94c86 100644 --- a/gateway/controller/BUILD.bazel +++ b/gateway/controller/BUILD.bazel @@ -2,7 +2,10 @@ load("@rules_go//go:def.bzl", "go_library", "go_test") go_library( name = "controller", - srcs = ["ping.go", "land.go"], + srcs = [ + "land.go", + "ping.go", + ], importpath = "github.com/uber/submitqueue/gateway/controller", visibility = ["//visibility:public"], deps = [ @@ -14,7 +17,10 @@ go_library( go_test( name = "controller_test", - srcs = ["ping_test.go", "land_test.go"], + srcs = [ + "land_test.go", + "ping_test.go", + ], embed = [":controller"], deps = [ "//gateway/protopb", diff --git a/gateway/controller/land.go b/gateway/controller/land.go index e9f090bd..4d6f3a45 100644 --- a/gateway/controller/land.go +++ b/gateway/controller/land.go @@ -18,13 +18,6 @@ type LandController struct { // NewLandController creates a new instance of the gateway land controller func NewLandController(logger *zap.Logger, scope tally.Scope) *LandController { - if logger == nil { - logger = zap.NewNop() - } - if scope == nil { - scope = tally.NoopScope - } - return &LandController{ logger: logger, metricsScope: scope, diff --git a/gateway/controller/land_test.go b/gateway/controller/land_test.go index 233c8b26..539fe138 100644 --- a/gateway/controller/land_test.go +++ b/gateway/controller/land_test.go @@ -5,16 +5,18 @@ import ( "testing" "github.com/stretchr/testify/require" + "github.com/uber-go/tally/v4" pb "github.com/uber/submitqueue/gateway/protopb" + "go.uber.org/zap" ) func TestNewLandController(t *testing.T) { - controller := NewLandController(nil, nil) + controller := NewLandController(zap.NewNop(), tally.NoopScope) require.NotNil(t, controller) } func TestLand_ReturnsSqid(t *testing.T) { - controller := NewLandController(nil, nil) + controller := NewLandController(zap.NewNop(), tally.NoopScope) ctx := context.Background() req := &pb.LandRequest{ diff --git a/gateway/controller/ping.go b/gateway/controller/ping.go index 7f7977f9..6704fc1d 100644 --- a/gateway/controller/ping.go +++ b/gateway/controller/ping.go @@ -18,13 +18,6 @@ type PingController struct { // NewPingController creates a new instance of the gateway ping controller func NewPingController(logger *zap.Logger, scope tally.Scope) *PingController { - if logger == nil { - logger = zap.NewNop() - } - if scope == nil { - scope = tally.NoopScope - } - return &PingController{ logger: logger, metricsScope: scope, diff --git a/gateway/controller/ping_test.go b/gateway/controller/ping_test.go index 0a3d3774..f8168953 100644 --- a/gateway/controller/ping_test.go +++ b/gateway/controller/ping_test.go @@ -7,16 +7,18 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/uber-go/tally/v4" pb "github.com/uber/submitqueue/gateway/protopb" + "go.uber.org/zap" ) func TestNewPingController(t *testing.T) { - controller := NewPingController(nil, nil) + controller := NewPingController(zap.NewNop(), tally.NoopScope) require.NotNil(t, controller) } func TestPing_DefaultMessage(t *testing.T) { - controller := NewPingController(nil, nil) + controller := NewPingController(zap.NewNop(), tally.NoopScope) ctx := context.Background() req := &pb.PingRequest{} @@ -27,7 +29,7 @@ func TestPing_DefaultMessage(t *testing.T) { } func TestPing_CustomMessage(t *testing.T) { - controller := NewPingController(nil, nil) + controller := NewPingController(zap.NewNop(), tally.NoopScope) ctx := context.Background() testCases := []struct { @@ -52,7 +54,7 @@ func TestPing_CustomMessage(t *testing.T) { } func TestPing_ServiceName(t *testing.T) { - controller := NewPingController(nil, nil) + controller := NewPingController(zap.NewNop(), tally.NoopScope) ctx := context.Background() req := &pb.PingRequest{} @@ -63,7 +65,7 @@ func TestPing_ServiceName(t *testing.T) { } func TestPing_Timestamp(t *testing.T) { - controller := NewPingController(nil, nil) + controller := NewPingController(zap.NewNop(), tally.NoopScope) ctx := context.Background() before := time.Now().Unix() @@ -77,7 +79,7 @@ func TestPing_Timestamp(t *testing.T) { } func TestPing_Hostname(t *testing.T) { - controller := NewPingController(nil, nil) + controller := NewPingController(zap.NewNop(), tally.NoopScope) ctx := context.Background() req := &pb.PingRequest{} diff --git a/orchestrator/controller/ping.go b/orchestrator/controller/ping.go index 52ba1a97..f931a563 100644 --- a/orchestrator/controller/ping.go +++ b/orchestrator/controller/ping.go @@ -18,13 +18,6 @@ type PingController struct { // NewPingController creates a new instance of the orchestrator ping controller func NewPingController(logger *zap.Logger, scope tally.Scope) *PingController { - if logger == nil { - logger = zap.NewNop() - } - if scope == nil { - scope = tally.NoopScope - } - return &PingController{ logger: logger, metricsScope: scope, diff --git a/orchestrator/controller/ping_test.go b/orchestrator/controller/ping_test.go index 26dc3506..6a138a7b 100644 --- a/orchestrator/controller/ping_test.go +++ b/orchestrator/controller/ping_test.go @@ -7,16 +7,18 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/uber-go/tally/v4" pb "github.com/uber/submitqueue/orchestrator/protopb" + "go.uber.org/zap" ) func TestNewPingController(t *testing.T) { - controller := NewPingController(nil, nil) + controller := NewPingController(zap.NewNop(), tally.NoopScope) require.NotNil(t, controller) } func TestPing_DefaultMessage(t *testing.T) { - controller := NewPingController(nil, nil) + controller := NewPingController(zap.NewNop(), tally.NoopScope) ctx := context.Background() req := &pb.PingRequest{} @@ -27,7 +29,7 @@ func TestPing_DefaultMessage(t *testing.T) { } func TestPing_CustomMessage(t *testing.T) { - controller := NewPingController(nil, nil) + controller := NewPingController(zap.NewNop(), tally.NoopScope) ctx := context.Background() testCases := []struct { @@ -52,7 +54,7 @@ func TestPing_CustomMessage(t *testing.T) { } func TestPing_ServiceName(t *testing.T) { - controller := NewPingController(nil, nil) + controller := NewPingController(zap.NewNop(), tally.NoopScope) ctx := context.Background() req := &pb.PingRequest{} @@ -63,7 +65,7 @@ func TestPing_ServiceName(t *testing.T) { } func TestPing_Timestamp(t *testing.T) { - controller := NewPingController(nil, nil) + controller := NewPingController(zap.NewNop(), tally.NoopScope) ctx := context.Background() before := time.Now().Unix() @@ -77,7 +79,7 @@ func TestPing_Timestamp(t *testing.T) { } func TestPing_Hostname(t *testing.T) { - controller := NewPingController(nil, nil) + controller := NewPingController(zap.NewNop(), tally.NoopScope) ctx := context.Background() req := &pb.PingRequest{} diff --git a/speculator/controller/ping.go b/speculator/controller/ping.go index a636643b..6f91b58b 100644 --- a/speculator/controller/ping.go +++ b/speculator/controller/ping.go @@ -18,13 +18,6 @@ type PingController struct { // NewPingController creates a new instance of the speculator ping controller func NewPingController(logger *zap.Logger, scope tally.Scope) *PingController { - if logger == nil { - logger = zap.NewNop() - } - if scope == nil { - scope = tally.NoopScope - } - return &PingController{ logger: logger, metricsScope: scope, diff --git a/speculator/controller/ping_test.go b/speculator/controller/ping_test.go index c2a23f85..80e60f01 100644 --- a/speculator/controller/ping_test.go +++ b/speculator/controller/ping_test.go @@ -7,16 +7,18 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/uber-go/tally/v4" pb "github.com/uber/submitqueue/speculator/protopb" + "go.uber.org/zap" ) func TestNewPingController(t *testing.T) { - controller := NewPingController(nil, nil) + controller := NewPingController(zap.NewNop(), tally.NoopScope) require.NotNil(t, controller) } func TestPing_DefaultMessage(t *testing.T) { - controller := NewPingController(nil, nil) + controller := NewPingController(zap.NewNop(), tally.NoopScope) ctx := context.Background() req := &pb.PingRequest{} @@ -27,7 +29,7 @@ func TestPing_DefaultMessage(t *testing.T) { } func TestPing_CustomMessage(t *testing.T) { - controller := NewPingController(nil, nil) + controller := NewPingController(zap.NewNop(), tally.NoopScope) ctx := context.Background() testCases := []struct { @@ -52,7 +54,7 @@ func TestPing_CustomMessage(t *testing.T) { } func TestPing_ServiceName(t *testing.T) { - controller := NewPingController(nil, nil) + controller := NewPingController(zap.NewNop(), tally.NoopScope) ctx := context.Background() req := &pb.PingRequest{} @@ -63,7 +65,7 @@ func TestPing_ServiceName(t *testing.T) { } func TestPing_Timestamp(t *testing.T) { - controller := NewPingController(nil, nil) + controller := NewPingController(zap.NewNop(), tally.NoopScope) ctx := context.Background() before := time.Now().Unix() @@ -77,7 +79,7 @@ func TestPing_Timestamp(t *testing.T) { } func TestPing_Hostname(t *testing.T) { - controller := NewPingController(nil, nil) + controller := NewPingController(zap.NewNop(), tally.NoopScope) ctx := context.Background() req := &pb.PingRequest{}