Skip to content

Commit 2bf201a

Browse files
committed
feat(stovepipe): Publish issue-tracking updates
1 parent e8a500b commit 2bf201a

12 files changed

Lines changed: 324 additions & 37 deletions

File tree

‎doc/rfc/stovepipe/steps/buildanalysis.md‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
1-
# Build issue observation
1+
# Build issue detection
22

3-
Build analysis runs alongside status polling and recording. Hosts supply a queue-resolved detector factory. The reference service uses a no-op detector so the analysis stage is always wired without inspecting provider output; deployments replace the factory with their integration. No-op results do not establish build success or recovery.
3+
Build analysis runs alongside status polling and recording. Hosts supply a queue-resolved detector factory. The reference service uses a no-op detector so the analysis stage is always wired without inspecting provider output; deployments replace the factory with their integration. No-op results do not establish build success. The reference host registers an issue-tracking publication route but does not yet consume those notifications.
44

55
## Start and completion notices
66

@@ -10,9 +10,9 @@ Start and completion notices use distinct deterministic message IDs and the same
1010

1111
Publication errors are returned through the producing stage's retry/DLQ policy. Analysis processing does not gate record, but notification publication is still part of the producing stage: a failed start publication can send the build-stage message to its existing fail-closed recovery path.
1212

13-
## Observations and polling
13+
## Build issues and polling
1414

15-
The [analysis controller](../../../../stovepipe/controller/buildanalysis/README.md) owns detection, per-build observation persistence, polling, and the stop condition for Requests that end without a recorded build result. It only accumulates historical observations; it does not maintain open issues or infer recovery from absence.
15+
The [analysis controller](../../../../stovepipe/controller/buildanalysis/README.md) owns detection, per-build issue persistence, polling, and the stop condition for Requests that end without a recorded build result. It accumulates per-build issues and publishes queue-partitioned issue-tracking notifications after persistence; it does not maintain open issues itself. Successful terminal analysis requests final reconciliation only after the Request records that Build as its winning outcome. Final reconciliation uses the accumulated build issues, including issues omitted from the last detector response. See the controller documentation for handoff and deduplication rules.
1616

1717
Status-polling DLQ recovery does not publish a build-completion notice. The analysis polling loop observes the ended Request through storage instead.
1818

‎service/stovepipe/server/main.go‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -409,7 +409,7 @@ func registerPrimaryControllers(
409409
}
410410
count++
411411

412-
analysisController := buildanalysis.NewController(logger, scope, store, detectorFactory, stovepipemq.TopicKeyBuildAnalysis, "stovepipe-buildanalysis")
412+
analysisController := buildanalysis.NewController(logger, scope, store, detectorFactory, registry, stovepipemq.TopicKeyBuildAnalysis, "stovepipe-buildanalysis")
413413
if err := c.Register(analysisController); err != nil {
414414
return count, fmt.Errorf("failed to register buildanalysis controller: %w", err)
415415
}
@@ -561,6 +561,11 @@ func newTopicRegistry(q extqueue.Queue, subscriberName string) (consumer.TopicRe
561561
Queue: q,
562562
Subscription: extqueue.DLQSubscriptionConfig(subscriberName, "stovepipe-hook-dlq"),
563563
},
564+
{
565+
Key: stovepipemq.TopicKeyIssueTrack,
566+
Name: "issuetrack",
567+
Queue: q,
568+
},
564569
{
565570
Key: stovepipemq.TopicKeyBuildAnalysis,
566571
Name: "buildanalysis",

‎service/stovepipe/server/main_test.go‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,15 @@ func TestEveryRegisteredControllerResolvesInTheTopicRegistry(t *testing.T) {
9292
}
9393
}
9494

95+
func TestIssueTrackingHasPublicationRouteWithoutConsumption(t *testing.T) {
96+
registry, primary, deadLetter := registeredControllers(t)
97+
name, ok := registry.TopicName(stovepipemq.TopicKeyIssueTrack)
98+
require.True(t, ok)
99+
assert.Equal(t, "issuetrack", name)
100+
assert.NotContains(t, topicKeys(primary), stovepipemq.TopicKeyIssueTrack)
101+
assert.NotContains(t, topicKeys(deadLetter), dlq.TopicKey(stovepipemq.TopicKeyIssueTrack))
102+
}
103+
95104
func TestHookStage(t *testing.T) {
96105
registry, primary, deadLetter := registeredControllers(t)
97106

‎stovepipe/controller/build/build_analysis_test.go‎

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -110,15 +110,16 @@ func TestBuildAnalysisPipeline(t *testing.T) {
110110

111111
detectorFactory := buildissuemock.NewMockDetectorFactory(ctrl)
112112
detector := buildissuemock.NewMockDetector(ctrl)
113-
detectorFactory.EXPECT().For(buildissue.DetectorConfig{QueueName: testQueue}).Return(detector, nil).Times(3)
113+
detectorFactory.EXPECT().For(buildissue.DetectorConfig{QueueName: testQueue}).Return(detector, nil).Times(4)
114114
gomock.InOrder(
115115
detector.EXPECT().Detect(gomock.Any(), entity.BuildID{ID: testBuildID}).Return([]entity.BuildIssue{a}, nil),
116-
detector.EXPECT().Detect(gomock.Any(), entity.BuildID{ID: testBuildID}).Return([]entity.BuildIssue{a, b}, nil),
117-
detector.EXPECT().Detect(gomock.Any(), entity.BuildID{ID: testBuildID}).Return([]entity.BuildIssue{a, b}, nil),
116+
detector.EXPECT().Detect(gomock.Any(), entity.BuildID{ID: testBuildID}).Return([]entity.BuildIssue{b}, nil),
117+
detector.EXPECT().Detect(gomock.Any(), entity.BuildID{ID: testBuildID}).Return(nil, nil),
118+
detector.EXPECT().Detect(gomock.Any(), entity.BuildID{ID: testBuildID}).Return(nil, nil),
118119
)
119120

120121
analysisController := buildanalysis.NewController(zap.NewNop().Sugar(), tally.NoopScope,
121-
staticStorageFactory{store: m.store}, detectorFactory, stovepipemq.TopicKeyBuildAnalysis, "analysis")
122+
staticStorageFactory{store: m.store}, detectorFactory, buildController.registry, stovepipemq.TopicKeyBuildAnalysis, "analysis")
122123
statusController := buildsignal.NewController(zap.NewNop().Sugar(), tally.NoopScope,
123124
staticStorageFactory{store: m.store}, m.materializer, m.runnerFactory, buildController.registry,
124125
stovepipemq.TopicKeyBuildSignal, "signal")
@@ -139,17 +140,37 @@ func TestBuildAnalysisPipeline(t *testing.T) {
139140
assert.True(t, persisted[a])
140141
assert.False(t, persisted[b])
141142

143+
secondPoll := delivered(published["buildanalysis"][0])
144+
secondPoll.EXPECT().Hold(buildanalysis.PollDelayMs)
145+
require.NoError(t, analysisController.Process(ctx, secondPoll))
146+
assert.True(t, persisted[b])
147+
142148
require.NoError(t, statusController.Process(ctx, delivered(published["buildsignal"][0])))
143149
require.Len(t, published["record"], 1)
144150
require.Len(t, published["buildanalysis"], 2)
145151
assert.Equal(t, entity.BuildStatusFailed, savedBuild.Status)
146152
assert.Equal(t, entity.RequestStateFailed, request.State)
147153
assert.Zero(t, queue.InFlightCount)
148-
assert.False(t, persisted[b])
154+
assert.True(t, persisted[b])
149155
assert.NotEqual(t, published["buildanalysis"][0].ID, published["buildanalysis"][1].ID)
150156

151157
require.NoError(t, analysisController.Process(ctx, delivered(published["buildanalysis"][0])))
152158
require.NoError(t, analysisController.Process(ctx, delivered(published["buildanalysis"][1])))
153159
assert.Len(t, persisted, 2)
154160
assert.True(t, persisted[b])
161+
assert.True(t, persisted[a])
162+
163+
require.Len(t, published["issuetrack"], 4)
164+
for i, msg := range published["issuetrack"] {
165+
assert.Equal(t, testQueue, msg.PartitionKey)
166+
assert.Equal(t, testQueue, msg.Tenant)
167+
notification := &stovepipemq.IssueTrack{}
168+
require.NoError(t, stovepipemq.Unmarshal(msg.Payload, notification))
169+
assert.Equal(t, testBuildID, notification.GetBuildId())
170+
assert.Equal(t, testQueue, notification.GetQueueName())
171+
assert.Equal(t, i > 1, notification.GetFinalize())
172+
}
173+
assert.NotEqual(t, published["issuetrack"][0].ID, published["issuetrack"][1].ID)
174+
assert.NotEqual(t, published["issuetrack"][1].ID, published["issuetrack"][2].ID)
175+
assert.Equal(t, published["issuetrack"][2].ID, published["issuetrack"][3].ID)
155176
}

‎stovepipe/controller/build/build_test.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,7 @@ func newController(t *testing.T, ctrl *gomock.Controller) (*Controller, buildMoc
9999
registry, err := consumer.NewTopicRegistry([]consumer.TopicConfig{
100100
{Key: stovepipemq.TopicKeyBuildSignal, Name: "buildsignal", Queue: queue},
101101
{Key: stovepipemq.TopicKeyBuildAnalysis, Name: "buildanalysis", Queue: queue},
102+
{Key: stovepipemq.TopicKeyIssueTrack, Name: "issuetrack", Queue: queue},
102103
{Key: stovepipemq.TopicKeyRecord, Name: "record", Queue: queue},
103104
})
104105
require.NoError(t, err)

‎stovepipe/controller/buildanalysis/BUILD.bazel‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ go_library(
1212
"//platform/base/messagequeue:go_default_library",
1313
"//platform/consumer:go_default_library",
1414
"//platform/metrics:go_default_library",
15+
"//platform/publish:go_default_library",
1516
"//stovepipe/core/loader:go_default_library",
1617
"//stovepipe/core/messagequeue:go_default_library",
1718
"//stovepipe/core/requestlog:go_default_library",
@@ -36,7 +37,9 @@ go_test(
3637
"//platform/consumer:go_default_library",
3738
"//platform/consumer/mock:go_default_library",
3839
"//platform/errs:go_default_library",
40+
"//platform/extension/messagequeue/mock:go_default_library",
3941
"//platform/metrics:go_default_library",
42+
"//platform/publish:go_default_library",
4043
"//stovepipe/core/messagequeue:go_default_library",
4144
"//stovepipe/core/requestlog:go_default_library",
4245
"//stovepipe/core/requestlog/mock:go_default_library",
Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,31 +1,39 @@
11
# Build analysis
22

3-
## Observations
3+
## Build issues
44

5-
The controller reloads the queue-scoped Build, invokes the queue's issue detector, and persists each returned observation individually. Observations already stored for the same build, task, and unit within a queue are not persisted again, so repeated polls and retries after partial writes are safe. Detector results accompanying an error are discarded.
5+
The controller reloads the queue-scoped Build, invokes the queue's issue detector, and persists each returned build issue individually. Build issues already stored for the same build, task, and unit within a queue are not persisted again, so repeated polls and retries after partial writes are safe. Detector results accompanying an error are discarded.
66

7-
This controller only accumulates historical observations. An empty detection result neither deletes earlier observations nor establishes recovery. Open-issue projection, lifecycle history, and resolution are outside its scope.
7+
After persistence, the controller publishes an issue-tracking notification partitioned by queue. Issue tracking maintains open issues and lifecycle history; analysis never writes them directly. An empty detection result does not delete earlier build issues.
8+
9+
## Issue-tracking handoff
10+
11+
Refresh notifications let issue tracking open issues reported by the build without closing other open issues. Final notifications let it also close open issues absent from the build's accumulated issues. A successful terminal detection must discover all remaining reportable issues; it need not repeat earlier build issues. An empty terminal response still triggers finalization, but does not retract earlier build issues from the same build. See the [detector contract](../../extension/buildissue/README.md).
12+
13+
All analysis deliveries for a build must share its build-ID partition and consumer group. The controller validates that partition key. Finalization requires a successful pass that began with a terminal Build and the Request's recorded winning build ID. If status becomes terminal before that outcome is recorded, analysis publishes a refresh and holds until the outcome is available. Known losing builds do not publish. A terminal Request without a recorded winning build permits a refresh but not closing issues based on absence.
14+
15+
Every refresh gets a fresh notification ID because held polling deliveries do not identify individual passes. A fixed ID would suppress later refreshes after earlier notifications were consumed. Retries can therefore publish duplicate refreshes; issue tracking is idempotent. Final notifications use one deterministic ID per build, so duplicate terminal passes deduplicate. Publication errors return through the normal analysis retry/DLQ policy; no hold or successful completion occurs before publication succeeds.
816

917
## Polling and completion
1018

11-
Every delivery reloads the Build and its Request before detection. After a successful pass, a nonterminal Build holds the delivery for two minutes; a terminal Build completes the delivery after persistence. The status read before detection determines whether the pass holds: if the Build finishes during detection, another delivery is needed to analyze it after observing terminal status. More than one delivery may analyze the same terminal Build; observation persistence remains idempotent.
19+
Every delivery reloads the Build and its Request before detection. After a successful pass, a nonterminal Build holds the delivery for two minutes; a terminal Build completes after persistence and publication once its Request outcome is known, or the Request has ended without one. A pass that begins with a nonterminal Build always holds, even if the Build finishes during detection, so another delivery analyzes it after observing terminal status. More than one delivery may analyze the same terminal Build; build-issue persistence remains idempotent.
1220

1321
If the Request is terminal but the loaded Build is nonterminal, analysis reloads the Build before deciding to stop. The Build may have finished between the initial Build read and the Request read; reloading avoids skipping its final analysis. If it is still nonterminal, analysis stops without detection or a hold. This can occur when the status-polling DLQ handler fails the Request without learning the actual build result, leaving the Build at its last observed status.
1422

1523
Status polling and updates remain the responsibility of buildsignal. Analysis never changes Request state or releases build slots.
1624

1725
## Errors
1826

19-
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.
27+
Detection, persistence, publication, 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.
2028

2129
## DLQ recovery
2230

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.
31+
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 the recorded build issues may be incomplete. It does not rerun detection, delete earlier build issues, change Request or Build state, or release a build slot. Original failure details are logged rather than included in the immutable history event.
2432

2533
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.
2634

2735
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.
2836

2937
## Host integration
3038

31-
The reference service supplies a no-op detector and publishes analysis notices at build start and completion. It registers the normal analysis controller with bounded retries and the recovery controller with the existing DLQ consumer. See [build issue observation](../../../doc/rfc/stovepipe/steps/buildanalysis.md) for the publication rules and retry configuration.
39+
The reference service supplies a no-op detector and publishes analysis notices at build start and completion. It also registers a publish-only issue-tracking topic; issue-tracking consumption and recovery are not enabled yet. It registers the normal analysis controller with bounded retries and the recovery controller with the existing DLQ consumer. See [build issue detection](../../../doc/rfc/stovepipe/steps/buildanalysis.md) for the publication rules and retry configuration.

0 commit comments

Comments
 (0)