Skip to content

Commit eb208cf

Browse files
authored
refactor(stovepipe): align DLQ controller conventions (#619)
## Summary Intent: - Align the existing Stovepipe DLQ controllers with the repository-wide convention used by the parent PR. - Keep the public construction API consistent as additional DLQ stages are added. Changes: - Rename the process and buildsignal constructors to the NewDLQ<Stage>Controller pattern. - Hide controller implementations behind consumer.Controller and derive controller names from their topic keys. - Update service wiring and test fixtures for the new constructors. This PR is stacked on #618, which adds build-stage DLQ reconciliation using the same convention. ## Test Plan - Run the Stovepipe DLQ controller test target. - Build the Stovepipe server target. ## Revert Plan - Revert this PR to restore the prior Stovepipe DLQ constructor and implementation type names. ## Issues
1 parent 9f8a3ef commit eb208cf

5 files changed

Lines changed: 38 additions & 36 deletions

File tree

service/stovepipe/server/main.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -450,7 +450,7 @@ func registerDLQControllers(
450450
) (int, error) {
451451
var count int
452452

453-
processDLQController := dlq.NewController(logger, scope, store, dlq.TopicKey(stovepipemq.TopicKeyProcess), "stovepipe-process-dlq")
453+
processDLQController := dlq.NewDLQRequestController(logger, scope, store, dlq.TopicKey(stovepipemq.TopicKeyProcess), "stovepipe-process-dlq")
454454
if err := c.Register(processDLQController); err != nil {
455455
return count, fmt.Errorf("failed to register process dlq controller: %w", err)
456456
}
@@ -462,7 +462,7 @@ func registerDLQControllers(
462462
}
463463
count++
464464

465-
buildSignalDLQController := dlq.NewBuildSignalController(logger, scope, store, dlq.TopicKey(stovepipemq.TopicKeyBuildSignal), "stovepipe-buildsignal-dlq")
465+
buildSignalDLQController := dlq.NewDLQBuildSignalController(logger, scope, store, dlq.TopicKey(stovepipemq.TopicKeyBuildSignal), "stovepipe-buildsignal-dlq")
466466
if err := c.Register(buildSignalDLQController); err != nil {
467467
return count, fmt.Errorf("failed to register buildsignal dlq controller: %w", err)
468468
}

stovepipe/controller/dlq/buildsignal.go

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ import (
3030
// _buildSignalOpName is the metric operation name shared by every emit in this file.
3131
const _buildSignalOpName = "buildsignal_dlq"
3232

33-
// BuildSignalController is the DLQ reconciler for the buildsignal stage. The
33+
// buildSignalController is the DLQ reconciler for the buildsignal stage. The
3434
// payload names a build, not a request, so it takes one more step than the
3535
// process reconciler: read the build to get its RequestID, then fail that
3636
// request via failRequest.
@@ -49,30 +49,31 @@ const _buildSignalOpName = "buildsignal_dlq"
4949
// There is nothing useful to fix: record decides greenness from Request.State,
5050
// not Build.Status, and writing a terminal status here would claim we saw an
5151
// outcome we never saw.
52-
type BuildSignalController struct {
52+
type buildSignalController struct {
5353
logger *zap.SugaredLogger
5454
metricsScope tally.Scope
5555
stores storage.Factory
5656
topicKey consumer.TopicKey
5757
consumerGroup string
5858
}
5959

60-
// Verify BuildSignalController implements consumer.Controller at compile time.
61-
var _ consumer.Controller = (*BuildSignalController)(nil)
60+
// Verify buildSignalController implements consumer.Controller at compile time.
61+
var _ consumer.Controller = (*buildSignalController)(nil)
6262

63-
// NewBuildSignalController creates a DLQ controller for the buildsignal stage's
63+
// NewDLQBuildSignalController creates a DLQ controller for the buildsignal stage's
6464
// dead-letter topic. topicKey is typically
6565
// dlq.TopicKey(stovepipemq.TopicKeyBuildSignal).
66-
func NewBuildSignalController(
66+
func NewDLQBuildSignalController(
6767
logger *zap.SugaredLogger,
6868
scope tally.Scope,
6969
stores storage.Factory,
7070
topicKey consumer.TopicKey,
7171
consumerGroup string,
72-
) *BuildSignalController {
73-
return &BuildSignalController{
74-
logger: logger.Named("buildsignal_dlq_controller"),
75-
metricsScope: scope.SubScope("buildsignal_dlq_controller"),
72+
) consumer.Controller {
73+
name := string(topicKey) + "_controller"
74+
return &buildSignalController{
75+
logger: logger.Named(name),
76+
metricsScope: scope.SubScope(name),
7677
stores: stores,
7778
topicKey: topicKey,
7879
consumerGroup: consumerGroup,
@@ -83,7 +84,7 @@ func NewBuildSignalController(
8384
// to ack (success) or an error to nack (retry) — pair this controller only with a
8485
// consumer wired with errs.AlwaysRetryableProcessor so a transient reconcile
8586
// failure retries instead of dead-lettering the DLQ message itself.
86-
func (c *BuildSignalController) Process(ctx context.Context, delivery consumer.Delivery) error {
87+
func (c *buildSignalController) Process(ctx context.Context, delivery consumer.Delivery) error {
8788
msg := delivery.Message()
8889

8990
sig := &stovepipemq.BuildSignal{}
@@ -152,16 +153,16 @@ func (c *BuildSignalController) Process(ctx context.Context, delivery consumer.D
152153
}
153154

154155
// Name returns the controller name for logging and metrics.
155-
func (c *BuildSignalController) Name() string {
156-
return "buildsignal_dlq"
156+
func (c *buildSignalController) Name() string {
157+
return string(c.topicKey)
157158
}
158159

159160
// TopicKey returns the topic key this controller subscribes to.
160-
func (c *BuildSignalController) TopicKey() consumer.TopicKey {
161+
func (c *buildSignalController) TopicKey() consumer.TopicKey {
161162
return c.topicKey
162163
}
163164

164165
// ConsumerGroup returns the consumer group for offset tracking.
165-
func (c *BuildSignalController) ConsumerGroup() string {
166+
func (c *buildSignalController) ConsumerGroup() string {
166167
return c.consumerGroup
167168
}

stovepipe/controller/dlq/buildsignal_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ type buildSignalDLQMocks struct {
3838
buildStore *storagemock.MockBuildStore
3939
}
4040

41-
func newBuildSignalController(t *testing.T, ctrl *gomock.Controller) (*BuildSignalController, buildSignalDLQMocks) {
41+
func newBuildSignalController(t *testing.T, ctrl *gomock.Controller) (consumer.Controller, buildSignalDLQMocks) {
4242
t.Helper()
4343

4444
m := buildSignalDLQMocks{
@@ -52,7 +52,7 @@ func newBuildSignalController(t *testing.T, ctrl *gomock.Controller) (*BuildSign
5252
store.EXPECT().GetQueueStore().Return(m.queueStore).AnyTimes()
5353
store.EXPECT().GetBuildStore().Return(m.buildStore).AnyTimes()
5454

55-
c := NewBuildSignalController(
55+
c := NewDLQBuildSignalController(
5656
zap.NewNop().Sugar(),
5757
tally.NewTestScope("test", nil),
5858
staticStorageFactory{store: store},

stovepipe/controller/dlq/dlq_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ type staticStorageFactory struct{ store storage.Storage }
4848
// For returns the fixed store aggregate for any queue.
4949
func (f staticStorageFactory) For(storage.Config) (storage.Storage, error) { return f.store, nil }
5050

51-
func newController(t *testing.T, ctrl *gomock.Controller) (*Controller, dlqMocks) {
51+
func newController(t *testing.T, ctrl *gomock.Controller) (consumer.Controller, dlqMocks) {
5252
t.Helper()
5353

5454
m := dlqMocks{
@@ -60,7 +60,7 @@ func newController(t *testing.T, ctrl *gomock.Controller) (*Controller, dlqMocks
6060
store.EXPECT().GetRequestStore().Return(m.reqStore).AnyTimes()
6161
store.EXPECT().GetQueueStore().Return(m.queueStore).AnyTimes()
6262

63-
c := NewController(zap.NewNop().Sugar(), tally.NewTestScope("test", nil), staticStorageFactory{store: store}, TopicKey(stovepipemq.TopicKeyProcess), "stovepipe-process-dlq")
63+
c := NewDLQRequestController(zap.NewNop().Sugar(), tally.NewTestScope("test", nil), staticStorageFactory{store: store}, TopicKey(stovepipemq.TopicKeyProcess), "stovepipe-process-dlq")
6464
return c, m
6565
}
6666

stovepipe/controller/dlq/request.go

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -26,36 +26,37 @@ import (
2626
"go.uber.org/zap"
2727
)
2828

29-
// Controller is the DLQ reconciler for the process stage. It is registered against the
29+
// requestController is the DLQ reconciler for the process stage. It is registered against the
3030
// process topic's DLQ (see TopicKey) and, on each delivery, decodes the request id from
3131
// the same ProcessRequest payload the primary process controller consumes, then drives
3232
// the referenced request to a terminal failed state via failRequest.
33-
type Controller struct {
33+
type requestController struct {
3434
logger *zap.SugaredLogger
3535
metricsScope tally.Scope
3636
stores storage.Factory
3737
topicKey consumer.TopicKey
3838
consumerGroup string
3939
}
4040

41-
// Verify Controller implements consumer.Controller at compile time.
42-
var _ consumer.Controller = (*Controller)(nil)
41+
// Verify requestController implements consumer.Controller at compile time.
42+
var _ consumer.Controller = (*requestController)(nil)
4343

4444
// _opName is the metric operation name shared by every emit in this file.
4545
const _opName = "process_dlq"
4646

47-
// NewController creates a new DLQ controller for the process stage's dead-letter topic.
47+
// NewDLQRequestController creates a DLQ controller for the process stage's dead-letter topic.
4848
// topicKey is typically dlq.TopicKey(stovepipemq.TopicKeyProcess).
49-
func NewController(
49+
func NewDLQRequestController(
5050
logger *zap.SugaredLogger,
5151
scope tally.Scope,
5252
stores storage.Factory,
5353
topicKey consumer.TopicKey,
5454
consumerGroup string,
55-
) *Controller {
56-
return &Controller{
57-
logger: logger.Named("process_dlq_controller"),
58-
metricsScope: scope.SubScope("process_dlq_controller"),
55+
) consumer.Controller {
56+
name := string(topicKey) + "_controller"
57+
return &requestController{
58+
logger: logger.Named(name),
59+
metricsScope: scope.SubScope(name),
5960
stores: stores,
6061
topicKey: topicKey,
6162
consumerGroup: consumerGroup,
@@ -66,7 +67,7 @@ func NewController(
6667
// (success) or an error to nack (retry) — pair this controller only with a consumer
6768
// wired with errs.AlwaysRetryableProcessor so a transient reconcile failure retries
6869
// instead of dead-lettering the DLQ message itself.
69-
func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) error {
70+
func (c *requestController) Process(ctx context.Context, delivery consumer.Delivery) error {
7071
msg := delivery.Message()
7172

7273
pr := &stovepipemq.ProcessRequest{}
@@ -113,16 +114,16 @@ func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) er
113114
}
114115

115116
// Name returns the controller name for logging and metrics.
116-
func (c *Controller) Name() string {
117-
return "process_dlq"
117+
func (c *requestController) Name() string {
118+
return string(c.topicKey)
118119
}
119120

120121
// TopicKey returns the topic key this controller subscribes to.
121-
func (c *Controller) TopicKey() consumer.TopicKey {
122+
func (c *requestController) TopicKey() consumer.TopicKey {
122123
return c.topicKey
123124
}
124125

125126
// ConsumerGroup returns the consumer group for offset tracking.
126-
func (c *Controller) ConsumerGroup() string {
127+
func (c *requestController) ConsumerGroup() string {
127128
return c.consumerGroup
128129
}

0 commit comments

Comments
 (0)