Skip to content

Commit ed471cb

Browse files
sbalabanov-zzsbalabanov-zz
committed
refactor(logging): Do not double log and do not classify errors
1 parent e9fe957 commit ed471cb

20 files changed

Lines changed: 16 additions & 181 deletions

File tree

‎gateway/controller/land.go‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -124,11 +124,6 @@ func (c *LandController) Land(ctx context.Context, req *pb.LandRequest) (*pb.Lan
124124

125125
// Publish to queue for async processing
126126
if err := c.publishToQueue(ctx, landRequest); err != nil {
127-
c.logger.Errorw("failed to publish request to queue",
128-
"queue", req.Queue,
129-
"sqid", landRequest.ID,
130-
"error", err,
131-
)
132127
return nil, fmt.Errorf("LandController failed to publish request to queue: %w", err)
133128
}
134129

‎orchestrator/controller/batch/BUILD.bazel‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ go_library(
77
visibility = ["//visibility:public"],
88
deps = [
99
"//core/consumer",
10-
"//core/errs",
1110
"//entity",
1211
"//entity/queue",
1312
"//extension/counter",

‎orchestrator/controller/batch/batch.go‎

Lines changed: 1 addition & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ import (
2020

2121
"github.com/uber-go/tally/v4"
2222
"github.com/uber/submitqueue/core/consumer"
23-
"github.com/uber/submitqueue/core/errs"
2423
"github.com/uber/submitqueue/entity"
2524
entityqueue "github.com/uber/submitqueue/entity/queue"
2625
"github.com/uber/submitqueue/extension/counter"
@@ -76,12 +75,6 @@ func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) er
7675
// Deserialize request entity
7776
request, err := entity.RequestFromBytes(msg.Payload)
7877
if err != nil {
79-
c.logger.Errorw("failed to deserialize request",
80-
"message_id", msg.ID,
81-
"partition_key", msg.PartitionKey,
82-
"attempt", delivery.Attempt(),
83-
"error", err,
84-
)
8578
c.metricsScope.Counter("deserialize_errors").Inc(1)
8679
// Non-retryable: malformed messages will never succeed regardless of retry count
8780
return fmt.Errorf("failed to deserialize request: %w", err)
@@ -99,11 +92,6 @@ func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) er
9992
// Generate a globally unique batch ID.
10093
seq, err := c.counter.Next(ctx, "batch/"+request.Queue)
10194
if err != nil {
102-
c.logger.Errorw("failed to generate batch ID",
103-
"request_id", request.ID,
104-
"queue", request.Queue,
105-
"error", err,
106-
)
10795
c.metricsScope.Counter("counter_errors").Inc(1)
10896
return fmt.Errorf("failed to generate batch ID for queue=%s: %w", request.Queue, err)
10997
}
@@ -123,11 +111,6 @@ func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) er
123111
entity.BatchStateFinalizing,
124112
})
125113
if err != nil {
126-
c.logger.Errorw("failed to get active batches",
127-
"request_id", request.ID,
128-
"queue", request.Queue,
129-
"error", err,
130-
)
131114
c.metricsScope.Counter("batch_store_errors").Inc(1)
132115
return fmt.Errorf("failed to get active batches for queue=%s: %w", request.Queue, err)
133116
}
@@ -150,10 +133,6 @@ func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) er
150133

151134
existing, err := c.store.GetBatchDependentStore().Get(ctx, dep.ID)
152135
if err != nil && !storage.IsNotFound(err) {
153-
c.logger.Errorw("failed to get existing batch dependent",
154-
"batch_id", dep.ID,
155-
"error", err,
156-
)
157136
c.metricsScope.Counter("batch_dependent_store_errors").Inc(1)
158137
return fmt.Errorf("failed to get batch dependent for batchID=%s: %w", dep.ID, err)
159138
}
@@ -175,13 +154,8 @@ func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) er
175154

176155
// Publish to score topic
177156
if err := c.publish(ctx, consumer.TopicKeyScore, batch); err != nil {
178-
c.logger.Errorw("failed to publish output",
179-
"batch_id", batch.ID,
180-
"topic_key", consumer.TopicKeyScore,
181-
"error", err,
182-
)
183157
c.metricsScope.Counter("publish_errors").Inc(1)
184-
return errs.NewRetryableError(fmt.Errorf("failed to publish to score: %w", err))
158+
return fmt.Errorf("failed to publish to score: %w", err)
185159
}
186160

187161
c.logger.Infow("published batch to score",

‎orchestrator/controller/build/BUILD.bazel‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ go_library(
77
visibility = ["//visibility:public"],
88
deps = [
99
"//core/consumer",
10-
"//core/errs",
1110
"//entity",
1211
"//entity/queue",
1312
"@com_github_uber_go_tally_v4//:tally",

‎orchestrator/controller/build/build.go‎

Lines changed: 1 addition & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ import (
2020

2121
"github.com/uber-go/tally/v4"
2222
"github.com/uber/submitqueue/core/consumer"
23-
"github.com/uber/submitqueue/core/errs"
2423
"github.com/uber/submitqueue/entity"
2524
entityqueue "github.com/uber/submitqueue/entity/queue"
2625
"go.uber.org/zap"
@@ -68,12 +67,6 @@ func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) er
6867
// Deserialize batch entity
6968
batch, err := entity.BatchFromBytes(msg.Payload)
7069
if err != nil {
71-
c.logger.Errorw("failed to deserialize batch",
72-
"message_id", msg.ID,
73-
"partition_key", msg.PartitionKey,
74-
"attempt", delivery.Attempt(),
75-
"error", err,
76-
)
7770
c.metricsScope.Counter("deserialize_errors").Inc(1)
7871
// Non-retryable: malformed messages will never succeed regardless of retry count
7972
return fmt.Errorf("failed to deserialize batch: %w", err)
@@ -100,14 +93,8 @@ func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) er
10093

10194
// Publish build to build signal topic
10295
if err := c.publish(ctx, consumer.TopicKeyBuildSignal, build); err != nil {
103-
c.logger.Errorw("failed to publish output",
104-
"batch_id", batch.ID,
105-
"build_id", build.ID,
106-
"topic_key", consumer.TopicKeyBuildSignal,
107-
"error", err,
108-
)
10996
c.metricsScope.Counter("publish_errors").Inc(1)
110-
return errs.NewRetryableError(fmt.Errorf("failed to publish to buildsignal: %w", err))
97+
return fmt.Errorf("failed to publish to buildsignal: %w", err)
11198
}
11299

113100
c.logger.Infow("published build to buildsignal",

‎orchestrator/controller/buildsignal/BUILD.bazel‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ go_library(
77
visibility = ["//visibility:public"],
88
deps = [
99
"//core/consumer",
10-
"//core/errs",
1110
"//entity",
1211
"//entity/queue",
1312
"@com_github_uber_go_tally_v4//:tally",

‎orchestrator/controller/buildsignal/buildsignal.go‎

Lines changed: 1 addition & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ import (
2020

2121
"github.com/uber-go/tally/v4"
2222
"github.com/uber/submitqueue/core/consumer"
23-
"github.com/uber/submitqueue/core/errs"
2423
"github.com/uber/submitqueue/entity"
2524
entityqueue "github.com/uber/submitqueue/entity/queue"
2625
"go.uber.org/zap"
@@ -68,12 +67,6 @@ func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) er
6867
// Deserialize build entity
6968
build, err := entity.BuildFromBytes(msg.Payload)
7069
if err != nil {
71-
c.logger.Errorw("failed to deserialize build",
72-
"message_id", msg.ID,
73-
"partition_key", msg.PartitionKey,
74-
"attempt", delivery.Attempt(),
75-
"error", err,
76-
)
7770
c.metricsScope.Counter("deserialize_errors").Inc(1)
7871
// Non-retryable: malformed messages will never succeed regardless of retry count
7972
return fmt.Errorf("failed to deserialize build: %w", err)
@@ -97,14 +90,8 @@ func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) er
9790

9891
// Publish batch to speculate topic
9992
if err := c.publish(ctx, consumer.TopicKeySpeculate, batch); err != nil {
100-
c.logger.Errorw("failed to publish output",
101-
"build_id", build.ID,
102-
"batch_id", build.BatchID,
103-
"topic_key", consumer.TopicKeySpeculate,
104-
"error", err,
105-
)
10693
c.metricsScope.Counter("publish_errors").Inc(1)
107-
return errs.NewRetryableError(fmt.Errorf("failed to publish to speculate: %w", err))
94+
return fmt.Errorf("failed to publish to speculate: %w", err)
10895
}
10996

11097
c.logger.Infow("published batch to speculate",

‎orchestrator/controller/conclude/BUILD.bazel‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ go_library(
77
visibility = ["//visibility:public"],
88
deps = [
99
"//core/consumer",
10-
"//core/errs",
1110
"//core/metrics",
1211
"//entity",
1312
"//extension/storage",

‎orchestrator/controller/conclude/conclude.go‎

Lines changed: 2 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ import (
2020

2121
"github.com/uber-go/tally/v4"
2222
"github.com/uber/submitqueue/core/consumer"
23-
"github.com/uber/submitqueue/core/errs"
2423
"github.com/uber/submitqueue/core/metrics"
2524
"github.com/uber/submitqueue/entity"
2625
"github.com/uber/submitqueue/extension/storage"
@@ -73,12 +72,6 @@ func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) (r
7372
// Deserialize batch entity
7473
batch, err := entity.BatchFromBytes(msg.Payload)
7574
if err != nil {
76-
c.logger.Errorw("failed to deserialize batch",
77-
"message_id", msg.ID,
78-
"partition_key", msg.PartitionKey,
79-
"attempt", delivery.Attempt(),
80-
"error", err,
81-
)
8275
// Non-retryable: malformed messages will never succeed regardless of retry count
8376
metrics.NamedCounter(c.metricsScope, "process", "deserialize_errors", 1)
8477
return fmt.Errorf("failed to deserialize batch: %w", err)
@@ -100,10 +93,6 @@ func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) (r
10093
// as updated by the merge controller.
10194
requestState, err := batchStateToRequestState(batch.State)
10295
if err != nil {
103-
c.logger.Errorw("unexpected batch state",
104-
"batch_id", batch.ID,
105-
"state", string(batch.State),
106-
)
10796
metrics.NamedCounter(c.metricsScope, "process", "unexpected_state_errors", 1)
10897
return fmt.Errorf("unexpected batch state %q for batch %s: %w", batch.State, batch.ID, err)
10998
}
@@ -112,25 +101,13 @@ func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) (r
112101
for _, requestID := range batch.Contains {
113102
request, err := c.store.GetRequestStore().Get(ctx, requestID)
114103
if err != nil {
115-
c.logger.Errorw("failed to get request from storage",
116-
"batch_id", batch.ID,
117-
"request_id", requestID,
118-
"error", err,
119-
)
120104
metrics.NamedCounter(c.metricsScope, "process", "request_store_errors", 1)
121-
return errs.NewRetryableError(fmt.Errorf("failed to get request %s: %w", requestID, err))
105+
return fmt.Errorf("failed to get request %s: %w", requestID, err)
122106
}
123107

124108
if err := c.store.GetRequestStore().UpdateState(ctx, requestID, request.Version, requestState); err != nil {
125-
c.logger.Errorw("failed to update request state",
126-
"batch_id", batch.ID,
127-
"request_id", requestID,
128-
"from_version", request.Version,
129-
"to_state", string(requestState),
130-
"error", err,
131-
)
132109
metrics.NamedCounter(c.metricsScope, "process", "request_update_errors", 1)
133-
return errs.NewRetryableError(fmt.Errorf("failed to update request %s state to %s: %w", requestID, requestState, err))
110+
return fmt.Errorf("failed to update request %s state to %s: %w", requestID, requestState, err)
134111
}
135112

136113
c.logger.Infow("updated request state",

‎orchestrator/controller/conclude/conclude_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,7 @@ func TestController_Process(t *testing.T) {
156156
return mockStorage
157157
},
158158
wantErr: true,
159-
retryable: true,
159+
retryable: false,
160160
},
161161
{
162162
name: "request store update failure is retryable",
@@ -179,7 +179,7 @@ func TestController_Process(t *testing.T) {
179179
return mockStorage
180180
},
181181
wantErr: true,
182-
retryable: true,
182+
retryable: false,
183183
},
184184
{
185185
name: "empty contains list succeeds",

0 commit comments

Comments
 (0)