Skip to content

Commit c91d921

Browse files
authored
feat(stovepipe): Record incomplete build analysis (#915)
## Summary **What**: - Record one history event per build when analysis cannot complete, without rerunning detection or changing validation results. - Retry history recovery failures while acknowledging messages that cannot identify a valid build and request. **Why**: - Make potentially incomplete failure information visible without blocking validation or repeatedly querying the build provider. ## Test Plan - [x] Add unit tests. ## Revert Plan - Revert this PR. No runtime wiring or schema changes are introduced, so existing validation behavior is unchanged. ## Issue Links - [CODEM-607](https://linear.app/uber/issue/CODEM-607/build-analysis) ## Stack 1. #902 1. #907 1. #915 1. #909 1. #916
1 parent 4d3fb9d commit c91d921

7 files changed

Lines changed: 473 additions & 6 deletions

File tree

‎doc/rfc/stovepipe/request-log.md‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,14 @@
22

33
## Summary
44

5-
Stovepipe retains an append-only request log for each validation request. Its internal `RequestLog` is the counterpart of SubmitQueue's `RequestLog`: both retain request status changes and explanatory lifecycle events, while Stovepipe calls its materializer directly instead of sending records through a cross-service log topic. The public API presents these records as request history. The log records every durable `Request.State` transition plus asynchronous milestones needed to explain those transitions, the public verdict, and abandoned terminal-stage work:
5+
Stovepipe retains an append-only request log for each validation request. Its internal `RequestLog` is the counterpart of SubmitQueue's `RequestLog`: both retain request status changes and explanatory lifecycle events, while Stovepipe calls its materializer directly instead of sending records through a cross-service log topic. The public API presents these records as request history. The log records every durable `Request.State` transition plus asynchronous milestones needed to explain those transitions, the public verdict, and abandoned stage work:
66

77
- `build_triggered`;
88
- `build_finished`;
99
- `validation_fact_recorded`;
1010
- `project_facts_recorded`;
11-
- `record_abandoned`.
11+
- `record_abandoned`;
12+
- `build_analysis_abandoned`.
1213

1314
The model deliberately follows SubmitQueue's distinction between statuses describing where a request is and events describing important activity that does not move it. It remains a bounded request-lifecycle log rather than a generic event bus or an audit of every correlated operation.
1415

@@ -126,6 +127,7 @@ Immutable Request context such as URI, build strategy, base URI, and terminal bu
126127
| `validation_fact_recorded` | The immutable whole-repository fact became durable. | Degree metadata and fact creation time |
127128
| `project_facts_recorded` | All project validation facts returned for a request became durable. | Project-fact count metadata and completion time |
128129
| `record_abandoned` | Record-stage work stopped after exhausting primary retries. | Event retention time |
130+
| `build_analysis_abandoned` | Build analysis could not complete; issue observations may be incomplete. This is not a validation verdict. | Build ID metadata and event retention time |
129131

130132
Build running and unchanged polls are not retained. Trigger and terminal result explain the request outcome without turning polling into an unbounded log. Individual project facts remain outside the vocabulary; one batch-completion event summarizes their durable recording.
131133

@@ -214,6 +216,7 @@ Request creation, Build changes, and fact creation use the same source-write, lo
214216
| Buildsignal | Persist terminal Build and retain `build_finished`; CAS the Request outcome and retain its terminal state. | Existing terminal Build and Request outcome each ensure their own entry before record publication. |
215217
| Record | Create or verify the whole-repository fact, then retain `validation_fact_recorded`; after recording project facts, retain one `project_facts_recorded` event. | An identical whole-repository fact and project-fact batch ensure their events before bookmark or promotion work. |
216218
| Record DLQ | Retain `record_abandoned`, then acknowledge the remaining record work. | The stable event ID makes history retention idempotent without replaying facts, bookmarks, promotion, or hooks. |
219+
| Build analysis DLQ | Retain `build_analysis_abandoned` without rerunning detection or changing validation state. | One stable event per build deduplicates recovery deliveries; only state reads and history retention retry. |
217220
| Reconciler | CAS an unrecoverable non-terminal Request to failed, then retain failed. | An existing terminal Request is repaired from its persisted outcome without relabeling it. |
218221

219222
Build running and unchanged polls create no entry. A failed runner trigger that creates no Build creates no event. Cancelled and superseded requests create no validation fact.

‎stovepipe/controller/buildanalysis/BUILD.bazel‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,10 @@ load("@rules_go//go:def.bzl", "go_library", "go_test")
22

33
go_library(
44
name = "go_default_library",
5-
srcs = ["buildanalysis.go"],
5+
srcs = [
6+
"buildanalysis.go",
7+
"dlq.go",
8+
],
69
importpath = "github.com/uber/submitqueue/stovepipe/controller/buildanalysis",
710
visibility = ["//visibility:public"],
811
deps = [
@@ -11,6 +14,7 @@ go_library(
1114
"//platform/metrics:go_default_library",
1215
"//stovepipe/core/loader:go_default_library",
1316
"//stovepipe/core/messagequeue:go_default_library",
17+
"//stovepipe/core/requestlog:go_default_library",
1418
"//stovepipe/entity:go_default_library",
1519
"//stovepipe/extension/buildissue:go_default_library",
1620
"//stovepipe/extension/storage:go_default_library",
@@ -21,14 +25,21 @@ go_library(
2125

2226
go_test(
2327
name = "go_default_test",
24-
srcs = ["buildanalysis_test.go"],
28+
srcs = [
29+
"buildanalysis_test.go",
30+
"dlq_test.go",
31+
],
2532
embed = [":go_default_library"],
2633
deps = [
34+
"//platform/base/failure:go_default_library",
2735
"//platform/base/messagequeue:go_default_library",
36+
"//platform/consumer:go_default_library",
2837
"//platform/consumer/mock:go_default_library",
2938
"//platform/errs:go_default_library",
3039
"//platform/metrics:go_default_library",
3140
"//stovepipe/core/messagequeue:go_default_library",
41+
"//stovepipe/core/requestlog:go_default_library",
42+
"//stovepipe/core/requestlog/mock:go_default_library",
3243
"//stovepipe/entity:go_default_library",
3344
"//stovepipe/extension/buildissue:go_default_library",
3445
"//stovepipe/extension/buildissue/mock:go_default_library",

‎stovepipe/controller/buildanalysis/README.md‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,4 +18,12 @@ Status polling and updates remain the responsibility of buildsignal. Analysis ne
1818

1919
Detection, persistence, configuration, state-loading, and message-validation errors are returned to the consumer for its normal retry or DLQ policy, whether the Build is running or terminal. Analysis errors do not use the two-minute polling hold. The consumer classifies errors and applies its configured retry limits and backoff; a final analysis can still fail and dead-letter.
2020

21-
The message and controller are defined without runtime producers or consumer registration. Host integration and detector implementations are separate work.
21+
## DLQ recovery
22+
23+
The recovery controller records a `build_analysis_abandoned` event in the Request's history, with the build ID, to indicate that analysis could not complete and issue observations may be incomplete. It does not rerun detection, delete earlier observations, change Request or Build state, or release a build slot. Original failure details are logged rather than included in the immutable history event.
24+
25+
History identity is stable per build, so repeated recovery deliveries and separate analysis notices for the same build retain one event. Different builds retain different events, even for the same Request. A later successful analysis does not erase the historical occurrence.
26+
27+
State-read and history-write errors are returned for retry. Hosts must run recovery with the DLQ consumer's always-retryable processor and unlimited-retry subscription; only recovery work is retried, not analysis. Unusable payloads, invalid or unresolvable queues, missing Build or Request records, and queue mismatches are logged and acknowledged rather than retried indefinitely.
28+
29+
The controllers are defined without runtime producers or consumer registration. Host integration and detector implementations are separate work.
Lines changed: 185 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,185 @@
1+
// Copyright (c) 2026 Uber Technologies, Inc.
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
package buildanalysis
16+
17+
import (
18+
"context"
19+
"errors"
20+
"fmt"
21+
22+
"github.com/uber-go/tally"
23+
entityqueue "github.com/uber/submitqueue/platform/base/messagequeue"
24+
"github.com/uber/submitqueue/platform/consumer"
25+
"github.com/uber/submitqueue/platform/metrics"
26+
stovepipemq "github.com/uber/submitqueue/stovepipe/core/messagequeue"
27+
"github.com/uber/submitqueue/stovepipe/core/requestlog"
28+
"github.com/uber/submitqueue/stovepipe/entity"
29+
"github.com/uber/submitqueue/stovepipe/extension/storage"
30+
"go.uber.org/zap"
31+
)
32+
33+
const _dlqOpName = "buildanalysis_dlq"
34+
35+
// DLQController records incomplete analysis without rerunning detection or
36+
// changing the validation outcome.
37+
type DLQController struct {
38+
logger *zap.SugaredLogger
39+
metricsScope tally.Scope
40+
stores storage.Factory
41+
materializer requestlog.Materializer
42+
topicKey consumer.TopicKey
43+
consumerGroup string
44+
}
45+
46+
// NewDLQController creates a controller for incomplete build analysis.
47+
func NewDLQController(
48+
logger *zap.SugaredLogger,
49+
scope tally.Scope,
50+
stores storage.Factory,
51+
materializer requestlog.Materializer,
52+
topicKey consumer.TopicKey,
53+
consumerGroup string,
54+
) *DLQController {
55+
name := string(topicKey) + "_controller"
56+
return &DLQController{
57+
logger: logger.Named(name),
58+
metricsScope: scope.SubScope(name),
59+
stores: stores,
60+
materializer: materializer,
61+
topicKey: topicKey,
62+
consumerGroup: consumerGroup,
63+
}
64+
}
65+
66+
// Process retains incomplete-analysis history and acknowledges unusable messages.
67+
// State-read and history-write failures are returned for DLQ recovery retry.
68+
func (c *DLQController) Process(ctx context.Context, delivery consumer.Delivery) error {
69+
msg := delivery.Message()
70+
analysis := &stovepipemq.BuildAnalysis{}
71+
if err := stovepipemq.Unmarshal(msg.Payload, analysis); err != nil {
72+
metrics.NamedCounter(c.metricsScope, _dlqOpName, "deserialize_errors", 1, metrics.TagsFromContext(ctx)...)
73+
c.logger.Errorw("discarding malformed analysis dlq message",
74+
"message_id", msg.ID,
75+
"error", err,
76+
)
77+
return nil
78+
}
79+
80+
if err := entityqueue.ValidatePayloadQueue(msg, analysis.GetQueueName()); err != nil {
81+
metrics.NamedCounter(c.metricsScope, _dlqOpName, "queue_identity_errors", 1, metrics.TagsFromContext(ctx)...)
82+
c.logger.Errorw("discarding analysis dlq message with invalid queue identity",
83+
"message_id", msg.ID,
84+
"build_id", analysis.GetId(),
85+
"error", err,
86+
)
87+
return nil
88+
}
89+
90+
if analysis.GetId() == "" {
91+
metrics.NamedCounter(c.metricsScope, _dlqOpName, "empty_id_errors", 1, metrics.TagsFromContext(ctx)...)
92+
c.logger.Errorw("discarding analysis dlq message with empty build id",
93+
"message_id", msg.ID,
94+
"queue", analysis.GetQueueName(),
95+
)
96+
return nil
97+
}
98+
99+
store, err := c.stores.For(storage.Config{QueueName: analysis.GetQueueName()})
100+
if err != nil {
101+
metrics.NamedCounter(c.metricsScope, _dlqOpName, "storage_resolve_errors", 1, metrics.TagsFromContext(ctx)...)
102+
c.logger.Errorw("discarding analysis dlq message for unresolvable queue",
103+
"message_id", msg.ID,
104+
"build_id", analysis.GetId(),
105+
"queue", analysis.GetQueueName(),
106+
"error", err,
107+
)
108+
return nil
109+
}
110+
111+
build, err := store.GetBuildStore().Get(ctx, analysis.GetId())
112+
if err != nil {
113+
if errors.Is(err, storage.ErrNotFound) {
114+
metrics.NamedCounter(c.metricsScope, _dlqOpName, "build_not_found", 1, metrics.TagsFromContext(ctx)...)
115+
c.logger.Errorw("discarding analysis dlq message for missing build",
116+
"message_id", msg.ID, "build_id", analysis.GetId(), "queue", analysis.GetQueueName())
117+
return nil
118+
}
119+
120+
return fmt.Errorf("failed to load build %s for analysis dlq: %w", analysis.GetId(), err)
121+
}
122+
123+
if build.RequestID == "" {
124+
metrics.NamedCounter(c.metricsScope, _dlqOpName, "empty_request_id", 1, metrics.TagsFromContext(ctx)...)
125+
c.logger.Errorw("discarding analysis dlq message for build without a request",
126+
"message_id", msg.ID, "build_id", analysis.GetId())
127+
return nil
128+
}
129+
130+
request, err := store.GetRequestStore().Get(ctx, build.RequestID)
131+
if err != nil {
132+
if errors.Is(err, storage.ErrNotFound) {
133+
metrics.NamedCounter(c.metricsScope, _dlqOpName, "request_not_found", 1, metrics.TagsFromContext(ctx)...)
134+
c.logger.Errorw("discarding analysis dlq message for missing request",
135+
"message_id", msg.ID, "build_id", analysis.GetId(), "request_id", build.RequestID)
136+
return nil
137+
}
138+
139+
return fmt.Errorf("failed to load request %s for analysis dlq: %w", build.RequestID, err)
140+
}
141+
142+
if request.Queue != analysis.GetQueueName() {
143+
metrics.NamedCounter(c.metricsScope, _dlqOpName, "queue_identity_errors", 1, metrics.TagsFromContext(ctx)...)
144+
c.logger.Errorw("discarding analysis dlq message for request in another queue",
145+
"message_id", msg.ID, "request_id", request.ID, "queue", request.Queue,
146+
"message_queue", analysis.GetQueueName())
147+
return nil
148+
}
149+
150+
log := requestlog.NewRequestEventLog(request, entity.RequestEventBuildAnalysisAbandoned, analysis.GetId(),
151+
map[string]string{requestlog.MetadataKeyBuildID: analysis.GetId()})
152+
if err := c.materializer.PersistLog(ctx, store, log); err != nil {
153+
metrics.NamedCounter(c.metricsScope, _dlqOpName, "history_errors", 1, metrics.TagsFromContext(ctx)...)
154+
return fmt.Errorf("failed to retain incomplete build analysis for request %s: %w", request.ID, err)
155+
}
156+
157+
originalFailure, hasFailure := delivery.Failure()
158+
metrics.NamedCounter(c.metricsScope, _dlqOpName, "analysis_abandoned", 1, metrics.TagsFromContext(ctx)...)
159+
fields := []any{
160+
"message_id", msg.ID,
161+
"request_id", request.ID,
162+
"build_id", analysis.GetId(),
163+
"queue", request.Queue,
164+
"request_state", request.State,
165+
"history_event", entity.RequestEventBuildAnalysisAbandoned,
166+
"attempt", delivery.Attempt(),
167+
}
168+
if hasFailure {
169+
fields = append(fields,
170+
"failure", originalFailure.Message,
171+
"failure_subjects", originalFailure.Subjects,
172+
"failure_detail", originalFailure.Detail,
173+
)
174+
}
175+
c.logger.Errorw("build analysis could not complete; issue observations may be incomplete", fields...)
176+
return nil
177+
}
178+
179+
func (c *DLQController) Name() string { return string(c.topicKey) }
180+
181+
func (c *DLQController) TopicKey() consumer.TopicKey { return c.topicKey }
182+
183+
func (c *DLQController) ConsumerGroup() string { return c.consumerGroup }
184+
185+
var _ consumer.Controller = (*DLQController)(nil)

0 commit comments

Comments
 (0)