Skip to content

Commit b81aae1

Browse files
committed
feat(stovepipe): Wire build analysis pipeline
1 parent eb615c1 commit b81aae1

21 files changed

Lines changed: 750 additions & 69 deletions

File tree

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
# Build issue observation
2+
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.
4+
5+
After persisting a Build and its trigger occurrence, build publishes to buildsignal and buildanalysis. On completion, buildsignal persists status, releases the build slot, finalizes the Request, and publishes to record and buildanalysis. At each point, both publications are attempted even if one fails. Any errors are returned to the consumer framework for its retry or DLQ policy. Publication is not atomic: one destination can receive its message while the other publication fails. Start and terminal analysis notices use distinct deterministic message IDs and the same build-ID partition; retries of one notice deduplicate without suppressing the other.
6+
7+
Analysis reloads the Build and Request on each delivery, detects available issues, and creates immutable build-scoped observations. Observations already stored for the same build, task, and unit within a queue are not persisted again. A nonterminal Build holds for 30 seconds after each detection or persistence attempt, including failed attempts. Analysis errors remain visible in logs and metrics but do not consume queue retries while the build is active. A terminal Build receives detection and persistence before the delivery completes; failures use the subscription's normal bounded retry or DLQ policy. Malformed messages, invalid configuration, state-loading failures, and shutdown cancellation still return errors. The decision uses the status loaded before detection, so completion during a failed nonterminal pass leads to terminal processing on the next delivery.
8+
9+
If build-status polling cannot recover, its DLQ handler fails the Request and releases its slot, but leaves the Build at its last observed status because the actual build result is unknown. If analysis observes a terminal Request with a nonterminal Build, it reloads the Build in case completion advanced it between the two reads. If the Build is still nonterminal, analysis stops rather than continuing to poll a request that has ended. The analysis subscription keeps dead-lettering enabled. No consumer automatically replays analysis dead letters; failed messages remain available for investigation and an explicit replay decision. Analysis never changes request outcome or releases a build slot. Record remains independent of analysis success.
10+
11+
This branch records historical observations only; it does not maintain open issues, infer recovery from absence, or track issue lifecycle history.

‎doc/rfc/stovepipe/workflow.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ The pipeline is a queue-driven chain of small, single-purpose controllers, in th
66

77
> **poll for a new head → ingest → process the build strategy → build → buildsignal → record whole-repo greenness (and any project results the resolver returns) → hook.**
88
9+
The [build issue observation](steps/buildanalysis.md) branch runs alongside this chain. It accumulates issues while builds run and performs terminal analysis without gating record. The reference host wires a no-op detector; deployments supply a real detector factory.
10+
911
There is no `analyze` controller and no `analyze` topic. A later project-analysis stage that would map a target graph onto another `build` pass is design only, described under [Designed, not built](#designed-not-built-project-analysis). `record` resolves project results inline on the same delivery that writes the repository fact.
1012

1113
## What Stovepipe is agnostic about

‎service/stovepipe/server/BUILD.bazel‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ go_library(
2626
"//service/stovepipe/server/handler:go_default_library",
2727
"//stovepipe/controller:go_default_library",
2828
"//stovepipe/controller/build:go_default_library",
29+
"//stovepipe/controller/buildanalysis:go_default_library",
2930
"//stovepipe/controller/buildsignal:go_default_library",
3031
"//stovepipe/controller/dlq:go_default_library",
3132
"//stovepipe/controller/process:go_default_library",
@@ -34,6 +35,8 @@ go_library(
3435
"//stovepipe/core/queuelog:go_default_library",
3536
"//stovepipe/core/queuestate:go_default_library",
3637
"//stovepipe/core/requestlog:go_default_library",
38+
"//stovepipe/extension/buildissue:go_default_library",
39+
"//stovepipe/extension/buildissue/noop:go_default_library",
3740
"//stovepipe/extension/buildrunner:go_default_library",
3841
"//stovepipe/extension/buildrunner/fake:go_default_library",
3942
"//stovepipe/extension/projectstatus/noop:go_default_library",
@@ -89,6 +92,9 @@ go_test(
8992
"//stovepipe/controller/dlq:go_default_library",
9093
"//stovepipe/core/messagequeue:go_default_library",
9194
"//stovepipe/core/requestlog:go_default_library",
95+
"//stovepipe/entity:go_default_library",
96+
"//stovepipe/extension/buildissue:go_default_library",
97+
"//stovepipe/extension/buildissue/noop:go_default_library",
9298
"@com_github_stretchr_testify//assert:go_default_library",
9399
"@com_github_stretchr_testify//require:go_default_library",
94100
"@com_github_uber_go_tally//:go_default_library",

‎service/stovepipe/server/main.go‎

Lines changed: 30 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ import (
4848
"github.com/uber/submitqueue/service/stovepipe/server/handler"
4949
"github.com/uber/submitqueue/stovepipe/controller"
5050
"github.com/uber/submitqueue/stovepipe/controller/build"
51+
"github.com/uber/submitqueue/stovepipe/controller/buildanalysis"
5152
"github.com/uber/submitqueue/stovepipe/controller/buildsignal"
5253
"github.com/uber/submitqueue/stovepipe/controller/dlq"
5354
"github.com/uber/submitqueue/stovepipe/controller/process"
@@ -56,6 +57,8 @@ import (
5657
"github.com/uber/submitqueue/stovepipe/core/queuelog"
5758
"github.com/uber/submitqueue/stovepipe/core/queuestate"
5859
"github.com/uber/submitqueue/stovepipe/core/requestlog"
60+
"github.com/uber/submitqueue/stovepipe/extension/buildissue"
61+
buildissuenoop "github.com/uber/submitqueue/stovepipe/extension/buildissue/noop"
5962
"github.com/uber/submitqueue/stovepipe/extension/buildrunner"
6063
buildrunnerfake "github.com/uber/submitqueue/stovepipe/extension/buildrunner/fake"
6164
projectstatusnoop "github.com/uber/submitqueue/stovepipe/extension/projectstatus/noop"
@@ -214,18 +217,15 @@ func run() error {
214217
subscriberName = fmt.Sprintf("stovepipe-%d", time.Now().Unix())
215218
}
216219

220+
detectorFactory := buildissuenoop.New()
217221
registry, err := newTopicRegistry(mysqlQueue, subscriberName)
218222
if err != nil {
219223
return fmt.Errorf("failed to create topic registry: %w", err)
220224
}
221225

222-
// Two consumers share the topic registry but apply different error classification
223-
// policies. The primary consumer runs the standard classifier walk. The DLQ consumer
224-
// uses AlwaysRetryableProcessor so every non-nil error from a DLQ controller is
225-
// forced retryable — reconciliation must redeliver on any failure because the DLQ
226-
// subscription is a final destination (DLQ.Enabled is false on it, so there is no
227-
// further DLQ to fall back on). Stovepipe has no gated deployment yet, so both
228-
// consumers use the no-op gate.
226+
// The primary consumer applies backend classification. The DLQ consumer
227+
// retries recovery work without a second-level DLQ; analysis dead letters
228+
// are retained without automatic replay. Both consumers use no-op gates.
229229
primaryConsumer := consumer.New(logger.Sugar(), scope.SubScope("consumer"), registry,
230230
errs.NewClassifierProcessor(
231231
genericerrs.Classifier,
@@ -254,7 +254,7 @@ func run() error {
254254
}
255255
}
256256
materializer := requestlog.NewMaterializer(scope)
257-
primaryCount, err := registerPrimaryControllers(primaryConsumer, logger.Sugar(), scope, storageFty, materializer, registry, sourceControl, brf, hookResolver{})
257+
primaryCount, err := registerPrimaryControllers(primaryConsumer, logger.Sugar(), scope, storageFty, materializer, registry, sourceControl, brf, hookResolver{}, detectorFactory)
258258
if err != nil {
259259
return err
260260
}
@@ -373,6 +373,7 @@ func registerPrimaryControllers(
373373
sourceControl sourcecontrol.Factory,
374374
brf buildrunner.Factory,
375375
hooks hookext.Hooks,
376+
detectorFactory buildissue.DetectorFactory,
376377
) (int, error) {
377378
var count int
378379

@@ -404,6 +405,12 @@ func registerPrimaryControllers(
404405
}
405406
count++
406407

408+
analysisController := buildanalysis.NewController(logger, scope, store, detectorFactory, stovepipemq.TopicKeyBuildAnalysis, "stovepipe-buildanalysis")
409+
if err := c.Register(analysisController); err != nil {
410+
return count, fmt.Errorf("failed to register buildanalysis controller: %w", err)
411+
}
412+
count++
413+
407414
recordController := record.NewController(logger, scope, store, materializer, projectstatusnoop.New(), sourceControl, registry, stovepipemq.TopicKeyRecord, "stovepipe-record")
408415
if err := c.Register(recordController); err != nil {
409416
return count, fmt.Errorf("failed to register record controller: %w", err)
@@ -419,8 +426,8 @@ func registerPrimaryControllers(
419426
return count, nil
420427
}
421428

422-
// registerDLQControllers creates one DLQ reconciler per primary stage and
423-
// registers them with c, returning how many were registered.
429+
// registerDLQControllers registers recovery for validation and hook stages.
430+
// Analysis dead letters are retained without an automatic replay consumer.
424431
func registerDLQControllers(
425432
c consumer.Consumer,
426433
logger *zap.SugaredLogger,
@@ -463,15 +470,8 @@ func registerDLQControllers(
463470
return count, nil
464471
}
465472

466-
// newTopicRegistry builds the TopicRegistry for Stovepipe's internal pipeline queues. ingest
467-
// publishes to the process topic and the process consumer subscribes to it; process publishes
468-
// to the build topic and the build consumer subscribes to it; build publishes to the buildsignal
469-
// topic and the buildsignal consumer subscribes to it, and also republishes to itself while
470-
// polling. buildsignal publishes to the record topic once a build reaches a terminal status,
471-
// and the record consumer subscribes to it.
472-
//
473-
// The hook topic name is domain-qualified because its key is shared across domains: two
474-
// domains pointed at one queue backend would otherwise consume each other's events.
473+
// newTopicRegistry includes every pipeline topic and its dead-letter destination.
474+
// Hook topic names are domain-qualified because the hook key is shared across domains.
475475
func newTopicRegistry(q extqueue.Queue, subscriberName string) (consumer.TopicRegistry, error) {
476476
return consumer.NewTopicRegistry([]consumer.TopicConfig{
477477
{
@@ -544,6 +544,17 @@ func newTopicRegistry(q extqueue.Queue, subscriberName string) (consumer.TopicRe
544544
Queue: q,
545545
Subscription: extqueue.DLQSubscriptionConfig(subscriberName, "stovepipe-hook-dlq"),
546546
},
547+
{
548+
Key: stovepipemq.TopicKeyBuildAnalysis,
549+
Name: "buildanalysis",
550+
Queue: q,
551+
Subscription: extqueue.ExtendedSubscriptionConfig(subscriberName, "stovepipe-buildanalysis"),
552+
},
553+
{
554+
Key: dlq.TopicKey(stovepipemq.TopicKeyBuildAnalysis),
555+
Name: "buildanalysis_dlq",
556+
Queue: q,
557+
},
547558
})
548559
}
549560

‎service/stovepipe/server/main_test.go‎

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,9 @@ import (
2828
"github.com/uber/submitqueue/stovepipe/controller/dlq"
2929
stovepipemq "github.com/uber/submitqueue/stovepipe/core/messagequeue"
3030
"github.com/uber/submitqueue/stovepipe/core/requestlog"
31+
"github.com/uber/submitqueue/stovepipe/entity"
32+
"github.com/uber/submitqueue/stovepipe/extension/buildissue"
33+
buildissuenoop "github.com/uber/submitqueue/stovepipe/extension/buildissue/noop"
3134
"go.uber.org/zap/zaptest"
3235
)
3336

@@ -60,7 +63,7 @@ func registeredControllers(t *testing.T) (consumer.TopicRegistry, []consumer.Con
6063
deadLetter := &recordingConsumer{}
6164

6265
_, err = registerPrimaryControllers(primary, logger, tally.NoopScope, store, requestlog.NewMaterializer(tally.NoopScope), registry,
63-
fakeSourceControlFactory{}, fakeBuildRunnerFactory{}, hookResolver{})
66+
fakeSourceControlFactory{}, fakeBuildRunnerFactory{}, hookResolver{}, buildissuenoop.New())
6467
require.NoError(t, err)
6568

6669
_, err = registerDLQControllers(deadLetter, logger, tally.NoopScope, store, requestlog.NewMaterializer(tally.NoopScope))
@@ -129,6 +132,7 @@ func TestPipelineSubscriptionRetryPolicies(t *testing.T) {
129132
{name: "process", topicKey: stovepipemq.TopicKeyProcess, consumerGroup: "stovepipe-process", wantRetry: defaultRetry},
130133
{name: "build", topicKey: stovepipemq.TopicKeyBuild, consumerGroup: "stovepipe-build", wantRetry: defaultRetry},
131134
{name: "buildsignal", topicKey: stovepipemq.TopicKeyBuildSignal, consumerGroup: "stovepipe-buildsignal", wantRetry: extendedRetry},
135+
{name: "buildanalysis", topicKey: stovepipemq.TopicKeyBuildAnalysis, consumerGroup: "stovepipe-buildanalysis", wantRetry: extendedRetry},
132136
{name: "record", topicKey: stovepipemq.TopicKeyRecord, consumerGroup: "stovepipe-record", wantRetry: extendedRetry},
133137
{name: "hook", topicKey: basehook.TopicKeyHook, consumerGroup: "stovepipe-hook", wantRetry: defaultRetry},
134138
}
@@ -154,3 +158,24 @@ func TestDLQSubscriptionRetryPoliciesRemainUnlimited(t *testing.T) {
154158
})
155159
}
156160
}
161+
162+
func TestBuildAnalysisIsWiredWithNoopDetector(t *testing.T) {
163+
registry, primary, deadLetter := registeredControllers(t)
164+
assert.Contains(t, topicKeys(primary), stovepipemq.TopicKeyBuildAnalysis)
165+
assert.NotContains(t, topicKeys(deadLetter), dlq.TopicKey(stovepipemq.TopicKeyBuildAnalysis))
166+
dlqName, ok := registry.TopicName(dlq.TopicKey(stovepipemq.TopicKeyBuildAnalysis))
167+
require.True(t, ok)
168+
assert.Equal(t, "buildanalysis_dlq", dlqName)
169+
_, subscribed := registry.SubscriptionConfig(dlq.TopicKey(stovepipemq.TopicKeyBuildAnalysis), "stovepipe-buildanalysis-dlq")
170+
assert.False(t, subscribed)
171+
cfg, ok := registry.SubscriptionConfig(stovepipemq.TopicKeyBuildAnalysis, "stovepipe-buildanalysis")
172+
require.True(t, ok)
173+
assert.True(t, cfg.DLQ.Enabled)
174+
assert.Equal(t, "_dlq", cfg.DLQ.TopicSuffix)
175+
176+
detector, err := (buildissuenoop.New()).For(buildissue.DetectorConfig{QueueName: "validation/main"})
177+
require.NoError(t, err)
178+
issues, err := detector.Detect(context.Background(), entity.BuildID{ID: "build/42"})
179+
require.NoError(t, err)
180+
assert.Empty(t, issues)
181+
}

‎stovepipe/controller/build/BUILD.bazel‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,10 @@ go_library(
2424

2525
go_test(
2626
name = "go_default_test",
27-
srcs = ["build_test.go"],
27+
srcs = [
28+
"build_analysis_test.go",
29+
"build_test.go",
30+
],
2831
embed = [":go_default_library"],
2932
deps = [
3033
"//platform/base/messagequeue:go_default_library",
@@ -33,10 +36,15 @@ go_test(
3336
"//platform/errs:go_default_library",
3437
"//platform/extension/messagequeue/mock:go_default_library",
3538
"//platform/metrics:go_default_library",
39+
"//platform/publish:go_default_library",
40+
"//stovepipe/controller/buildanalysis:go_default_library",
41+
"//stovepipe/controller/buildsignal:go_default_library",
3642
"//stovepipe/core/messagequeue:go_default_library",
3743
"//stovepipe/core/requestlog:go_default_library",
3844
"//stovepipe/core/requestlog/mock:go_default_library",
3945
"//stovepipe/entity:go_default_library",
46+
"//stovepipe/extension/buildissue:go_default_library",
47+
"//stovepipe/extension/buildissue/mock:go_default_library",
4048
"//stovepipe/extension/buildrunner:go_default_library",
4149
"//stovepipe/extension/buildrunner/mock:go_default_library",
4250
"//stovepipe/extension/storage:go_default_library",

‎stovepipe/controller/build/build.go‎

Lines changed: 28 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,12 @@
1515
// Package build holds the build-stage queue controller. It consumes BuildRequest
1616
// messages (a request id), reloads the Request, triggers the build-runner for
1717
// the scope process already decided, persists a Build row, and publishes the
18-
// build id to buildsignal.
18+
// build id to buildsignal and buildanalysis.
1919
package build
2020

2121
import (
2222
"context"
23+
"errors"
2324
"fmt"
2425

2526
"github.com/uber-go/tally"
@@ -39,7 +40,7 @@ import (
3940

4041
// Controller consumes BuildRequest messages, reloads the referenced Request,
4142
// triggers a build for its already-decided scope, and publishes the resulting
42-
// build id to buildsignal. Implements consumer.Controller.
43+
// build id to status polling and analysis. Implements consumer.Controller.
4344
type Controller struct {
4445
logger *zap.SugaredLogger
4546
metricsScope tally.Scope
@@ -51,9 +52,6 @@ type Controller struct {
5152
consumerGroup string
5253
}
5354

54-
// Verify Controller implements consumer.Controller interface at compile time.
55-
var _ consumer.Controller = (*Controller)(nil)
56-
5755
// _opName is the metric operation name shared by every emit in this file.
5856
const _opName = "build"
5957

@@ -155,8 +153,13 @@ func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) er
155153
return err
156154
}
157155

158-
if err := c.publishBuildSignal(ctx, build.ID, request.Queue); err != nil {
159-
return fmt.Errorf("failed to publish build signal for %s: %w", build.ID, err)
156+
signalErr := c.publishBuildSignal(ctx, build.ID, request.Queue)
157+
if signalErr != nil {
158+
signalErr = fmt.Errorf("failed to publish build signal for %s: %w", build.ID, signalErr)
159+
}
160+
analysisErr := c.publishBuildAnalysis(ctx, build.ID, request.Queue)
161+
if err := errors.Join(signalErr, analysisErr); err != nil {
162+
return err
160163
}
161164

162165
c.logger.Debugw("triggered build",
@@ -219,3 +222,21 @@ func (c *Controller) TopicKey() consumer.TopicKey {
219222
func (c *Controller) ConsumerGroup() string {
220223
return c.consumerGroup
221224
}
225+
226+
func (c *Controller) publishBuildAnalysis(ctx context.Context, buildID, queue string) error {
227+
payload, err := stovepipemq.Marshal(&stovepipemq.BuildAnalysis{Id: buildID, QueueName: queue})
228+
if err != nil {
229+
return fmt.Errorf("failed to serialize build analysis: %w", err)
230+
}
231+
if err := publish.Message(ctx, c.registry, stovepipemq.TopicKeyBuildAnalysis, publish.MessageParams{
232+
Tenant: queue,
233+
ID: publish.IntentID(buildID, "started"),
234+
Payload: payload,
235+
PartitionKey: buildID,
236+
}); err != nil {
237+
return fmt.Errorf("failed to publish build analysis for %s: %w", buildID, err)
238+
}
239+
return nil
240+
}
241+
242+
var _ consumer.Controller = (*Controller)(nil)

0 commit comments

Comments
 (0)