diff --git a/doc/rfc/stovepipe/request-log.md b/doc/rfc/stovepipe/request-log.md index 979cc17f6..54af5ccda 100644 --- a/doc/rfc/stovepipe/request-log.md +++ b/doc/rfc/stovepipe/request-log.md @@ -10,7 +10,12 @@ Stovepipe retains an append-only request log for each validation request. Its in - `project_facts_recorded`; - `record_abandoned`; - `build_analysis_abandoned`; -- `issue_tracking_abandoned`. +- `issue_tracking_abandoned`; +- `build_analysis_completed`; +- `issues_reconciled`; +- `fix_submitted`; +- `fix_submission_skipped`; +- `fix_submission_abandoned`. 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. @@ -29,7 +34,7 @@ State changes alone are insufficient. A request remains processing while a build An occurrence belongs in the request log only when it is either: 1. a durable `Request.State` transition; or -2. an asynchronous durable milestone required to explain the next request transition or public verdict. +2. an asynchronous durable milestone explaining validation, issue reconciliation, or a repair submission for the request. The following remain with their owning entities, metrics, or structured logs: @@ -130,9 +135,18 @@ Immutable Request context such as URI, build strategy, base URI, and terminal bu | `record_abandoned` | Record-stage work stopped after exhausting primary retries. | Event retention time | | `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 | | `issue_tracking_abandoned` | Issue tracking stopped before completing a pass; existing issue changes remain, but reconciliation may be incomplete. This is not a validation verdict. | Build ID metadata and event retention time | +| `build_analysis_completed` | Terminal detection completed and its complete issue set was persisted. | Build ID, issue count, and event retention time | +| `issues_reconciled` | A final issue-tracking pass completed, including any deliberate suppression of absent-issue closure. | Build ID, closure mode, and event retention time | +| `fix_submitted` | An identified repair attempt was durably associated with a build issue. It does not imply a patch exists or the issue recovered. | Build, task, unit, and fix IDs and event retention time | +| `fix_submission_skipped` | No eligible open issue existed for the build. | Build ID, bounded reason, and event retention time | +| `fix_submission_abandoned` | Repair processing stopped without a durable fix ID; external acceptance remains unknown. | Build ID, bounded reason, and event retention time | 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. Its `project_fact_count` and `broken_project_fact_count` metadata contain the total and the number with degree above zero, counted from persisted facts after their indexes are durable. Project completion projection requires both counts and assigns them together in `RequestSummary` from the retained event. +`build_analysis_completed`, `issues_reconciled`, `fix_submission_skipped`, and `fix_submission_abandoned` use the build ID as their occurrence key. `fix_submitted` uses a digest of the executor fix ID to fit the storage key limit, retaining the verbatim fix ID in metadata. Existing issue-tracking abandonment uses a digest of the failed notification ID to fit the storage key limit. The materializer prefixes each key with its event kind, and request identity scopes all entries. Replays retain the first stored timestamp. + +Analysis and repair events do not advance Request state or alter validation facts. An empty terminal analysis records an issue count of zero. `issues_reconciled` records closure mode `reconciled`, `suppressed_cancelled`, or `suppressed_task_wide_issue`; suppression does not mean reconciliation failed. Skipped repair uses reason `no_eligible_issue`; abandoned repair uses `acceptance_unknown`. Raw dependency errors remain in operational logs. + ### Evolution State and event strings never change meaning or get reused. A new event must satisfy the scope rule and update required-field validation, storage mapping, public projection, and compatibility tests. @@ -220,8 +234,12 @@ The `project_facts_recorded` and `record_abandoned` events project completion of | 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. | | 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. | | Record DLQ | Retain `record_abandoned` and project the end of project-result production; for failed Requests, create or verify the repository fact and retain `validation_fact_recorded` before publishing its hook. | Existing facts remain authoritative; stable log and hook IDs make replay idempotent without resolving projects or replaying promotion. | +| Build analysis | Persist complete terminal detection results, then retain `build_analysis_completed` before issue-tracking publication. | Repeat detection and issue creation, then reconcile the stable per-build event before publication. Nonterminal polls add no event. | | 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. | | Issue tracking DLQ | Protect the current cursor from delayed writes, retain its issue history and `issue_tracking_abandoned`, then mark the pass abandoned. | A stable hash of the notification ID deduplicates the event. Recovery retains accepted issue changes and never processes the remaining range. | +| Issue tracking | Finalize the durable checkpoint, then retain `issues_reconciled`. | Reconstruct history from the finalized checkpoint before it can be replaced by another pass. | +| Fix | Associate the returned fix ID, then retain `fix_submitted`; retain `fix_submission_skipped` when no issue is eligible. | Ensure submitted history from any existing association before skipping executor work. | +| Fix DLQ | Repair submitted or skipped history from current durable state, or retain `fix_submission_abandoned` when acceptance is unknown. | Only read sources and retain history; never call the executor or change issue state. | | 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. | 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. @@ -280,7 +298,7 @@ Rejected because bookmarks, promotion, project facts, hooks, and operational boo ### Use a generic versioned envelope -Rejected for six request states and three explanatory events. Explicit fields are simpler to inspect, constrain, test, and expose. A future migration should be justified by actual vocabulary growth. +Rejected because explicit request states and event fields are simpler to inspect than a generic envelope. A future migration should be justified by actual vocabulary growth. ### Copy SubmitQueue's log topic diff --git a/doc/rfc/stovepipe/steps/buildanalysis.md b/doc/rfc/stovepipe/steps/buildanalysis.md index 474dd84d4..bf68f7b45 100644 --- a/doc/rfc/stovepipe/steps/buildanalysis.md +++ b/doc/rfc/stovepipe/steps/buildanalysis.md @@ -12,7 +12,7 @@ Publication errors are returned through the producing stage's retry/DLQ policy. ## Build issues and polling -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. Terminal detection must return the complete cumulative issue set, including issues returned by earlier calls, and analysis persists that set before publishing final reconciliation. Issue tracking applies all accumulated build issues before permitting any absence-based closure; a task-wide failure preserves existing issues across all tasks. See the controller documentation for handoff and deduplication rules. +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. Terminal detection must return the complete cumulative issue set, including issues returned by earlier calls. Analysis persists that set and retains `build_analysis_completed` with the issue count before publishing final reconciliation. Nonterminal polls add no completion event. Issue tracking applies all accumulated build issues before permitting any absence-based closure; a task-wide failure preserves existing issues across all tasks. See the controller documentation for handoff and deduplication rules. Status-polling DLQ recovery does not publish a build-completion notice. The analysis polling loop observes the ended Request through storage instead. diff --git a/service/stovepipe/server/main.go b/service/stovepipe/server/main.go index c6e2009cb..d57fbce17 100644 --- a/service/stovepipe/server/main.go +++ b/service/stovepipe/server/main.go @@ -417,19 +417,19 @@ func registerPrimaryControllers( } count++ - analysisController := buildanalysis.NewController(logger, scope, store, detectorFactory, registry, stovepipemq.TopicKeyBuildAnalysis, "stovepipe-buildanalysis") + analysisController := buildanalysis.NewController(logger, scope, store, materializer, detectorFactory, registry, stovepipemq.TopicKeyBuildAnalysis, "stovepipe-buildanalysis") if err := c.Register(analysisController); err != nil { return count, fmt.Errorf("failed to register buildanalysis controller: %w", err) } count++ - issueTrackController := issuetrack.NewController(logger, scope, store, registry, stovepipemq.TopicKeyIssueTrack, "stovepipe-issuetrack") + issueTrackController := issuetrack.NewController(logger, scope, store, materializer, registry, stovepipemq.TopicKeyIssueTrack, "stovepipe-issuetrack") if err := c.Register(issueTrackController); err != nil { return count, fmt.Errorf("failed to register issuetrack controller: %w", err) } count++ - fixController := fix.NewController(logger, scope, store, repairExecutors, stovepipemq.TopicKeyFix, "stovepipe-fix") + fixController := fix.NewController(logger, scope, store, materializer, repairExecutors, stovepipemq.TopicKeyFix, "stovepipe-fix") if err := c.Register(fixController); err != nil { return count, fmt.Errorf("failed to register fix controller: %w", err) } @@ -492,6 +492,12 @@ func registerDLQControllers( } count++ + fixDLQController := fix.NewDLQController(logger, scope, store, materializer, dlq.TopicKey(stovepipemq.TopicKeyFix), "stovepipe-fix-dlq") + if err := c.Register(fixDLQController); err != nil { + return count, fmt.Errorf("failed to register fix dlq controller: %w", err) + } + count++ + recordDLQController := record.NewDLQController(logger, scope, store, materializer, registry, dlq.TopicKey(stovepipemq.TopicKeyRecord), "stovepipe-record-dlq") if err := c.Register(recordDLQController); err != nil { return count, fmt.Errorf("failed to register record dlq controller: %w", err) @@ -606,6 +612,12 @@ func newTopicRegistry(q extqueue.Queue, subscriberName string) (consumer.TopicRe Queue: q, Subscription: extqueue.ExtendedSubscriptionConfig(subscriberName, "stovepipe-fix"), }, + { + Key: dlq.TopicKey(stovepipemq.TopicKeyFix), + Name: "fix_dlq", + Queue: q, + Subscription: extqueue.DLQSubscriptionConfig(subscriberName, "stovepipe-fix-dlq"), + }, { Key: stovepipemq.TopicKeyBuildAnalysis, Name: "buildanalysis", diff --git a/service/stovepipe/server/main_test.go b/service/stovepipe/server/main_test.go index 3a1907762..a27c62e7c 100644 --- a/service/stovepipe/server/main_test.go +++ b/service/stovepipe/server/main_test.go @@ -120,9 +120,6 @@ func TestValidationStagesHaveRecovery(t *testing.T) { _, primary, deadLetter := registeredControllers(t) recoveryKeys := topicKeys(deadLetter) for _, controller := range primary { - if controller.TopicKey() == stovepipemq.TopicKeyFix { - continue - } t.Run(controller.Name(), func(t *testing.T) { assert.Contains(t, recoveryKeys, dlq.TopicKey(controller.TopicKey())) }) @@ -135,7 +132,7 @@ func TestFixRegistration(t *testing.T) { require.True(t, ok) assert.Equal(t, "fix", name) assert.Contains(t, topicKeys(primary), stovepipemq.TopicKeyFix) - assert.NotContains(t, topicKeys(deadLetter), dlq.TopicKey(stovepipemq.TopicKeyFix)) + assert.Contains(t, topicKeys(deadLetter), dlq.TopicKey(stovepipemq.TopicKeyFix)) } func TestRegisteredFixControllerRecordsNoopRepairOncePerBuild(t *testing.T) { @@ -150,6 +147,27 @@ func TestRegisteredFixControllerRecordsNoopRepairOncePerBuild(t *testing.T) { store.EXPECT().GetBuildIssueStore().Return(observations).AnyTimes() store.EXPECT().GetIssueStateStore().Return(issues).AnyTimes() + builds := storagemock.NewMockBuildStore(ctrl) + requests := storagemock.NewMockRequestStore(ctrl) + history := storagemock.NewMockRequestLogStore(ctrl) + store.EXPECT().GetBuildStore().Return(builds).AnyTimes() + store.EXPECT().GetRequestStore().Return(requests).AnyTimes() + store.EXPECT().GetRequestLogStore().Return(history).AnyTimes() + builds.EXPECT().Get(gomock.Any(), buildID).Return(entity.Build{ID: buildID, RequestID: "1"}, nil).AnyTimes() + requests.EXPECT().Get(gomock.Any(), "1").Return(entity.Request{ID: "1", Queue: queueName}, nil).AnyTimes() + var retained entity.RequestLog + history.EXPECT().Create(gomock.Any(), gomock.Any()).DoAndReturn(func(_ context.Context, log entity.RequestLog) error { + if retained.ID != "" { + return storage.ErrAlreadyExists + } + retained = log + return nil + }).Times(2) + history.EXPECT().Get(gomock.Any(), "1", gomock.Any()).DoAndReturn(func(_ context.Context, _, id string) (entity.RequestLog, error) { + require.Equal(t, retained.ID, id) + return retained, nil + }) + observation := entity.BuildIssue{TaskID: "test", UnitID: "unit", Version: 1} observations.EXPECT().List(gomock.Any(), buildID, gomock.Any()).DoAndReturn( func(_ context.Context, _ string, bounds storage.BuildIssueRange) ([]entity.BuildIssue, error) { @@ -196,6 +214,8 @@ func TestRegisteredFixControllerRecordsNoopRepairOncePerBuild(t *testing.T) { delivery.EXPECT().Message().Return(message) require.NoError(t, fixController.Process(context.Background(), delivery)) } + require.Equal(t, entity.RequestEventFixSubmitted, retained.Event) + require.Equal(t, "noop/"+buildID, retained.Metadata[requestlog.MetadataKeyFixID]) } func TestHookStage(t *testing.T) { diff --git a/stovepipe/controller/build/build_analysis_test.go b/stovepipe/controller/build/build_analysis_test.go index 0609168d0..bf1cc0002 100644 --- a/stovepipe/controller/build/build_analysis_test.go +++ b/stovepipe/controller/build/build_analysis_test.go @@ -119,7 +119,7 @@ func TestBuildAnalysisPipeline(t *testing.T) { ) analysisController := buildanalysis.NewController(zap.NewNop().Sugar(), tally.NoopScope, - staticStorageFactory{store: m.store}, detectorFactory, buildController.registry, stovepipemq.TopicKeyBuildAnalysis, "analysis") + staticStorageFactory{store: m.store}, m.materializer, detectorFactory, buildController.registry, stovepipemq.TopicKeyBuildAnalysis, "analysis") statusController := buildsignal.NewController(zap.NewNop().Sugar(), tally.NoopScope, staticStorageFactory{store: m.store}, m.materializer, m.runnerFactory, buildController.registry, stovepipemq.TopicKeyBuildSignal, "signal") diff --git a/stovepipe/controller/buildanalysis/BUILD.bazel b/stovepipe/controller/buildanalysis/BUILD.bazel index ae5acadb4..39f5ea62d 100644 --- a/stovepipe/controller/buildanalysis/BUILD.bazel +++ b/stovepipe/controller/buildanalysis/BUILD.bazel @@ -29,6 +29,7 @@ go_test( srcs = [ "buildanalysis_test.go", "dlq_test.go", + "history_test.go", ], embed = [":go_default_library"], deps = [ diff --git a/stovepipe/controller/buildanalysis/README.md b/stovepipe/controller/buildanalysis/README.md index 65be8d984..889d38e0c 100644 --- a/stovepipe/controller/buildanalysis/README.md +++ b/stovepipe/controller/buildanalysis/README.md @@ -4,6 +4,8 @@ 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. +After successful terminal detection and persistence, the controller retains `build_analysis_completed` with the build ID and complete issue count, including zero. History retention must succeed before the issue-tracking handoff; retries reconcile the same per-build entry. Nonterminal polls add no completion entry. + 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. ## Issue-tracking handoff diff --git a/stovepipe/controller/buildanalysis/buildanalysis.go b/stovepipe/controller/buildanalysis/buildanalysis.go index 8f28044fa..c7758119e 100644 --- a/stovepipe/controller/buildanalysis/buildanalysis.go +++ b/stovepipe/controller/buildanalysis/buildanalysis.go @@ -20,6 +20,7 @@ import ( "context" "errors" "fmt" + "strconv" "github.com/uber-go/tally" entityqueue "github.com/uber/submitqueue/platform/base/messagequeue" @@ -28,6 +29,7 @@ import ( "github.com/uber/submitqueue/platform/publish" "github.com/uber/submitqueue/stovepipe/core/loader" stovepipemq "github.com/uber/submitqueue/stovepipe/core/messagequeue" + "github.com/uber/submitqueue/stovepipe/core/requestlog" "github.com/uber/submitqueue/stovepipe/entity" "github.com/uber/submitqueue/stovepipe/extension/buildissue" "github.com/uber/submitqueue/stovepipe/extension/storage" @@ -44,6 +46,7 @@ type Controller struct { logger *zap.SugaredLogger metricsScope tally.Scope stores storage.Factory + materializer requestlog.Materializer detectorFactory buildissue.DetectorFactory registry consumer.TopicRegistry topicKey consumer.TopicKey @@ -54,6 +57,7 @@ func NewController( logger *zap.SugaredLogger, scope tally.Scope, stores storage.Factory, + materializer requestlog.Materializer, detectorFactory buildissue.DetectorFactory, registry consumer.TopicRegistry, topicKey consumer.TopicKey, @@ -63,6 +67,7 @@ func NewController( logger: logger.Named("buildanalysis_controller"), metricsScope: scope.SubScope("buildanalysis_controller"), stores: stores, + materializer: materializer, detectorFactory: detectorFactory, registry: registry, topicKey: topicKey, @@ -165,6 +170,17 @@ func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) (r hold = !finalize && !request.State.IsTerminal() } + if build.Status.IsTerminal() { + log := requestlog.NewRequestEventLog(request, entity.RequestEventBuildAnalysisCompleted, build.ID, + map[string]string{ + requestlog.MetadataKeyBuildID: build.ID, + requestlog.MetadataKeyIssueCount: strconv.Itoa(len(issues)), + }) + if err := c.materializer.PersistLog(ctx, store, log); err != nil { + return fmt.Errorf("retain completed analysis for build %q: %w", build.ID, err) + } + } + if err := c.publishIssueTracking(ctx, request.Queue, build.ID, finalize); err != nil { return err } diff --git a/stovepipe/controller/buildanalysis/buildanalysis_test.go b/stovepipe/controller/buildanalysis/buildanalysis_test.go index 42d9a0ac3..13cdc59aa 100644 --- a/stovepipe/controller/buildanalysis/buildanalysis_test.go +++ b/stovepipe/controller/buildanalysis/buildanalysis_test.go @@ -30,6 +30,7 @@ import ( "github.com/uber/submitqueue/platform/metrics" "github.com/uber/submitqueue/platform/publish" stovepipemq "github.com/uber/submitqueue/stovepipe/core/messagequeue" + "github.com/uber/submitqueue/stovepipe/core/requestlog" "github.com/uber/submitqueue/stovepipe/entity" "github.com/uber/submitqueue/stovepipe/extension/buildissue" buildissuemock "github.com/uber/submitqueue/stovepipe/extension/buildissue/mock" @@ -45,6 +46,9 @@ const ( ) type analysisMocks struct { + history map[string]entity.RequestLog + historyFailure *error + materializer requestlog.Materializer request *entity.Request ctrl *gomock.Controller stores *storagemock.MockFactory @@ -82,6 +86,28 @@ func newTestController(t *testing.T) (*Controller, analysisMocks) { m.requestRead = m.requests.EXPECT().Get(gomock.Any(), gomock.Any()).DoAndReturn(func(context.Context, string) (entity.Request, error) { return *m.request, nil }).AnyTimes() m.store.EXPECT().GetBuildIssueStore().Return(m.issues).AnyTimes() + m.history = map[string]entity.RequestLog{} + m.historyFailure = new(error) + m.materializer = requestlog.NewMaterializer(tally.NoopScope) + history := storagemock.NewMockRequestLogStore(ctrl) + m.store.EXPECT().GetRequestLogStore().Return(history).AnyTimes() + history.EXPECT().Create(gomock.Any(), gomock.Any()).DoAndReturn(func(_ context.Context, log entity.RequestLog) error { + if *m.historyFailure != nil { + return *m.historyFailure + } + if _, exists := m.history[log.ID]; exists { + return storage.ErrAlreadyExists + } + m.history[log.ID] = log + return nil + }).AnyTimes() + history.EXPECT().Get(gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn(func(_ context.Context, requestID, logID string) (entity.RequestLog, error) { + log, exists := m.history[logID] + if !exists || log.RequestID != requestID { + return entity.RequestLog{}, storage.ErrNotFound + } + return log, nil + }).AnyTimes() queue := mqmock.NewMockQueue(ctrl) queue.EXPECT().Publisher().Return(m.publisher).AnyTimes() registry, err := consumer.NewTopicRegistry([]consumer.TopicConfig{ @@ -89,7 +115,7 @@ func newTestController(t *testing.T) (*Controller, analysisMocks) { }) require.NoError(t, err) - c := NewController(zap.NewNop().Sugar(), m.scope, m.stores, m.detectorFactory, registry, + c := NewController(zap.NewNop().Sugar(), m.scope, m.stores, m.materializer, m.detectorFactory, registry, stovepipemq.TopicKeyBuildAnalysis, "stovepipe-buildanalysis") return c, m @@ -177,6 +203,11 @@ func TestProcessPersistsBeforeHoldingOrCompleting(t *testing.T) { } require.NoError(t, c.Process(context.Background(), d)) + if status.IsTerminal() { + require.Len(t, m.history, 1) + } else { + require.Empty(t, m.history) + } }) } } diff --git a/stovepipe/controller/buildanalysis/history_test.go b/stovepipe/controller/buildanalysis/history_test.go new file mode 100644 index 000000000..9673c23ca --- /dev/null +++ b/stovepipe/controller/buildanalysis/history_test.go @@ -0,0 +1,53 @@ +// Copyright (c) 2026 Uber Technologies, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package buildanalysis + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + entityqueue "github.com/uber/submitqueue/platform/base/messagequeue" + "github.com/uber/submitqueue/stovepipe/core/requestlog" + "github.com/uber/submitqueue/stovepipe/entity" + "github.com/uber/submitqueue/stovepipe/extension/storage" + "go.uber.org/mock/gomock" +) + +func TestCompletedAnalysisHistoryPrecedesHandoffAndRepairsOnRetry(t *testing.T) { + c, m := newTestController(t) + issue := entity.BuildIssue{TaskID: "task", UnitID: "unit"} + *m.historyFailure = assert.AnError + detected := m.expectPass(entity.BuildStatusFailed, []entity.BuildIssue{issue}, nil) + m.issues.EXPECT().Create(gomock.Any(), testBuildID, issue).Return(nil).After(detected) + require.ErrorIs(t, c.Process(context.Background(), m.delivery(t)), assert.AnError) + require.Empty(t, m.history) + + *m.historyFailure = nil + for i := 0; i < 2; i++ { + detected = m.expectPass(entity.BuildStatusFailed, []entity.BuildIssue{issue}, nil) + saved := m.issues.EXPECT().Create(gomock.Any(), testBuildID, issue).Return(storage.ErrAlreadyExists).After(detected) + m.expectNotification(t, true, saved).Do(func(context.Context, string, entityqueue.Message) { + require.Len(t, m.history, 1) + }) + require.NoError(t, c.Process(context.Background(), m.delivery(t))) + } + log := m.history["event/build_analysis_completed/"+testBuildID] + require.Equal(t, entity.RequestEventBuildAnalysisCompleted, log.Event) + require.Equal(t, map[string]string{requestlog.MetadataKeyBuildID: testBuildID, requestlog.MetadataKeyIssueCount: "1"}, log.Metadata) + require.Equal(t, m.request.ID, log.RequestID) + require.Empty(t, log.State) +} diff --git a/stovepipe/controller/fix/BUILD.bazel b/stovepipe/controller/fix/BUILD.bazel index 0d8d045d1..863978491 100644 --- a/stovepipe/controller/fix/BUILD.bazel +++ b/stovepipe/controller/fix/BUILD.bazel @@ -4,15 +4,20 @@ go_library( name = "go_default_library", srcs = [ "delivery.go", + "dlq.go", "fix.go", + "history.go", ], importpath = "github.com/uber/submitqueue/stovepipe/controller/fix", visibility = ["//visibility:public"], deps = [ "//platform/base/messagequeue:go_default_library", "//platform/consumer:go_default_library", + "//platform/errs:go_default_library", "//platform/metrics:go_default_library", + "//stovepipe/core/loader:go_default_library", "//stovepipe/core/messagequeue:go_default_library", + "//stovepipe/core/requestlog:go_default_library", "//stovepipe/entity:go_default_library", "//stovepipe/extension/fixer:go_default_library", "//stovepipe/extension/storage:go_default_library", @@ -26,6 +31,7 @@ go_test( srcs = [ "delivery_test.go", "fix_test.go", + "history_test.go", ], embed = [":go_default_library"], deps = [ @@ -34,6 +40,7 @@ go_test( "//platform/consumer/mock:go_default_library", "//platform/errs:go_default_library", "//stovepipe/core/messagequeue:go_default_library", + "//stovepipe/core/requestlog:go_default_library", "//stovepipe/entity:go_default_library", "//stovepipe/extension/fixer:go_default_library", "//stovepipe/extension/fixer/mock:go_default_library", diff --git a/stovepipe/controller/fix/README.md b/stovepipe/controller/fix/README.md index 489766326..9ed812de0 100644 --- a/stovepipe/controller/fix/README.md +++ b/stovepipe/controller/fix/README.md @@ -7,3 +7,7 @@ A returned fix ID is associated with the issue through `BuildIssueStore.Update`, Issue tracking calls [`fixrequest.Publish`](../../core/fixrequest/publish.go) after completing a nonempty opening pass and persisting its issue state and history. Publications share a build partition, while the message identity includes the pass notification ID so a later pass can trigger another evaluation. A pass with no eligible issue completes without submitting a repair; a recorded fix ID prevents later passes from submitting another repair for the build. The one-per-build limit is controller policy. Submission precedes persistence, so overlapping deliveries, crashes, and failed writes can still produce duplicate executor calls. Issue recovery remains separate from repair association. + +A durable association retains `fix_submitted` in the owning Request's history, with build, task, unit, and fix IDs. A digest of the fix ID identifies the occurrence and fits the history key limit; metadata retains the verbatim fix ID. An existing association repairs missing history before skipping submission, including when the issue has since recovered. Without an eligible issue, the controller retains `fix_submission_skipped` once per build with reason `no_eligible_issue`. These entries do not change Request outcome or establish issue recovery. + +The DLQ controller reads the same durable decision without calling the executor. It repairs submitted or skipped history when possible; otherwise, an eligible issue without a fix ID retains `fix_submission_abandoned` once per build with reason `acceptance_unknown`. An executor error without an ID can follow acceptance, so this event does not claim rejection. Source and history errors retry. Unusable payloads, unresolvable queues, and missing or mismatched sources are logged and acknowledged. The reference host registers recovery with unlimited, always-retryable DLQ processing to preserve history repair through a storage outage. diff --git a/stovepipe/controller/fix/dlq.go b/stovepipe/controller/fix/dlq.go new file mode 100644 index 000000000..65130a8fb --- /dev/null +++ b/stovepipe/controller/fix/dlq.go @@ -0,0 +1,99 @@ +// Copyright (c) 2026 Uber Technologies, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package fix + +import ( + "context" + "fmt" + + "github.com/uber-go/tally" + entityqueue "github.com/uber/submitqueue/platform/base/messagequeue" + "github.com/uber/submitqueue/platform/consumer" + "github.com/uber/submitqueue/platform/errs" + "github.com/uber/submitqueue/platform/metrics" + mq "github.com/uber/submitqueue/stovepipe/core/messagequeue" + "github.com/uber/submitqueue/stovepipe/core/requestlog" + "github.com/uber/submitqueue/stovepipe/entity" + "github.com/uber/submitqueue/stovepipe/extension/storage" + "go.uber.org/zap" +) + +// DLQController retains repair submission outcomes without submitting more work. +type DLQController struct { + logger *zap.SugaredLogger + metricsScope tally.Scope + stores storage.Factory + materializer requestlog.Materializer + topicKey consumer.TopicKey + consumerGroup string +} + +// NewDLQController creates repair history recovery. Hosts must use unlimited, +// always-retryable DLQ processing so a history outage cannot discard its retry trigger. +func NewDLQController(logger *zap.SugaredLogger, scope tally.Scope, stores storage.Factory, materializer requestlog.Materializer, topicKey consumer.TopicKey, consumerGroup string) *DLQController { + return &DLQController{ + logger: logger.Named("fix_dlq_controller"), metricsScope: scope.SubScope("fix_dlq_controller"), + stores: stores, materializer: materializer, topicKey: topicKey, consumerGroup: consumerGroup, + } +} + +func (c *DLQController) Process(ctx context.Context, delivery consumer.Delivery) (retErr error) { + op := metrics.Begin(c.metricsScope, "process", metrics.StorageLatencyBuckets, metrics.TagsFromContext(ctx)...) + defer func() { op.Complete(retErr) }() + msg := delivery.Message() + fix := &mq.FixRequest{} + if err := mq.Unmarshal(msg.Payload, fix); err != nil { + c.logger.Errorw("discarding malformed repair recovery", "error", err) + return nil + } + if err := entityqueue.ValidatePayloadQueue(msg, fix.GetQueueName()); err != nil || fix.GetId() == "" { + c.logger.Errorw("discarding repair recovery with invalid identity", "error", err) + return nil + } + store, err := c.stores.For(storage.Config{QueueName: fix.GetQueueName()}) + if err != nil { + c.logger.Errorw("discarding repair recovery for an unresolvable queue", "queue", fix.GetQueueName(), "error", err) + return nil + } + request, err := loadRequestForBuild(ctx, store, fix.GetQueueName(), fix.GetId()) + if err != nil { + if storage.IsNotFound(err) || errs.IsUserError(err) { + c.logger.Errorw("discarding repair recovery without a matching request", "build_id", fix.GetId(), "error", err) + return nil + } + return err + } + observation, eligible, err := findIssueToFix(ctx, store.GetBuildIssueStore(), store.GetIssueStateStore(), fix.GetId()) + if err != nil { + return fmt.Errorf("load repair decision for recovery: %w", err) + } + if observation.FixID != "" { + return persistFixSubmittedLog(ctx, c.materializer, store, request, fix.GetId(), observation) + } + if !eligible { + return persistFixSubmissionDecision(ctx, c.materializer, store, request, fix.GetId(), + entity.RequestEventFixSubmissionSkipped, "no_eligible_issue") + } + // A provider can have accepted work even when no ID was returned. Preserve + // that uncertainty and leave repair associations and issue status alone. + return persistFixSubmissionDecision(ctx, c.materializer, store, request, fix.GetId(), + entity.RequestEventFixSubmissionAbandoned, "acceptance_unknown") +} + +func (c *DLQController) Name() string { return string(c.topicKey) } +func (c *DLQController) TopicKey() consumer.TopicKey { return c.topicKey } +func (c *DLQController) ConsumerGroup() string { return c.consumerGroup } + +var _ consumer.Controller = (*DLQController)(nil) diff --git a/stovepipe/controller/fix/fix.go b/stovepipe/controller/fix/fix.go index 55543d220..cca9abc54 100644 --- a/stovepipe/controller/fix/fix.go +++ b/stovepipe/controller/fix/fix.go @@ -23,6 +23,7 @@ import ( "github.com/uber-go/tally" "github.com/uber/submitqueue/platform/consumer" "github.com/uber/submitqueue/platform/metrics" + "github.com/uber/submitqueue/stovepipe/core/requestlog" "github.com/uber/submitqueue/stovepipe/entity" "github.com/uber/submitqueue/stovepipe/extension/fixer" "github.com/uber/submitqueue/stovepipe/extension/storage" @@ -35,6 +36,7 @@ type Controller struct { logger *zap.SugaredLogger metricsScope tally.Scope stores storage.Factory + materializer requestlog.Materializer executors fixer.Factory topicKey consumer.TopicKey consumerGroup string @@ -44,6 +46,7 @@ func NewController( logger *zap.SugaredLogger, scope tally.Scope, stores storage.Factory, + materializer requestlog.Materializer, executors fixer.Factory, topicKey consumer.TopicKey, consumerGroup string, @@ -52,6 +55,7 @@ func NewController( logger: logger.Named("fix_controller"), metricsScope: scope.SubScope("fix_controller"), stores: stores, + materializer: materializer, executors: executors, topicKey: topicKey, consumerGroup: consumerGroup, @@ -67,8 +71,16 @@ func (c *Controller) processBuild(ctx context.Context, queueName, buildID string if err != nil { return fmt.Errorf("failed to select an issue for build %q: %w", buildID, err) } + request, err := loadRequestForBuild(ctx, store, queueName, buildID) + if err != nil { + return err + } + if observation.FixID != "" { + return persistFixSubmittedLog(ctx, c.materializer, store, request, buildID, observation) + } if !found { - return nil + return persistFixSubmissionDecision(ctx, c.materializer, store, request, buildID, + entity.RequestEventFixSubmissionSkipped, "no_eligible_issue") } executor, err := c.executors.For(fixer.Config{QueueName: queueName}) @@ -116,8 +128,9 @@ func (c *Controller) processBuild(ctx context.Context, queueName, buildID string "queue", queueName, "build_id", buildID, "task_id", issue.TaskID, "unit_id", issue.UnitID, "fix_id", current.FixID, "submitted_fix_id", fixID) + updated.FixID = current.FixID } - return nil + return persistFixSubmittedLog(ctx, c.materializer, store, request, buildID, updated) } func (c *Controller) Name() string { return "fix" } @@ -140,7 +153,7 @@ func findIssueToFix(ctx context.Context, observations storage.BuildIssueStore, i } for _, record := range page { if record.FixID != "" { - return entity.BuildIssue{}, false, nil + return record, false, nil } if found { continue diff --git a/stovepipe/controller/fix/fix_test.go b/stovepipe/controller/fix/fix_test.go index 6d4cdea4a..84d325361 100644 --- a/stovepipe/controller/fix/fix_test.go +++ b/stovepipe/controller/fix/fix_test.go @@ -23,6 +23,7 @@ import ( "github.com/uber-go/tally" "github.com/uber/submitqueue/platform/consumer" "github.com/uber/submitqueue/platform/errs" + "github.com/uber/submitqueue/stovepipe/core/requestlog" "github.com/uber/submitqueue/stovepipe/entity" "github.com/uber/submitqueue/stovepipe/extension/fixer" fixermock "github.com/uber/submitqueue/stovepipe/extension/fixer/mock" @@ -39,14 +40,19 @@ const ( ) type controllerMocks struct { - ctrl *gomock.Controller - stores *storagemock.MockFactory - store *storagemock.MockStorage - issues *storagemock.MockIssueStateStore - observations *storagemock.MockBuildIssueStore - executors *fixermock.MockFactory - executor *fixermock.MockRepairExecutor - logs *observer.ObservedLogs + history map[string]entity.RequestLog + historyFailure *error + materializer requestlog.Materializer + builds *storagemock.MockBuildStore + requests *storagemock.MockRequestStore + ctrl *gomock.Controller + stores *storagemock.MockFactory + store *storagemock.MockStorage + issues *storagemock.MockIssueStateStore + observations *storagemock.MockBuildIssueStore + executors *fixermock.MockFactory + executor *fixermock.MockRepairExecutor + logs *observer.ObservedLogs } func newTestController(t *testing.T) (*Controller, controllerMocks) { @@ -65,7 +71,37 @@ func newTestController(t *testing.T) (*Controller, controllerMocks) { } m.store.EXPECT().GetIssueStateStore().Return(m.issues).AnyTimes() m.store.EXPECT().GetBuildIssueStore().Return(m.observations).AnyTimes() - return NewController(zap.New(core).Sugar(), tally.NoopScope, m.stores, m.executors, + + m.history = map[string]entity.RequestLog{} + m.historyFailure = new(error) + m.materializer = requestlog.NewMaterializer(tally.NoopScope) + history := storagemock.NewMockRequestLogStore(ctrl) + m.store.EXPECT().GetRequestLogStore().Return(history).AnyTimes() + history.EXPECT().Create(gomock.Any(), gomock.Any()).DoAndReturn(func(_ context.Context, log entity.RequestLog) error { + if *m.historyFailure != nil { + return *m.historyFailure + } + if _, exists := m.history[log.ID]; exists { + return storage.ErrAlreadyExists + } + m.history[log.ID] = log + return nil + }).AnyTimes() + history.EXPECT().Get(gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn(func(_ context.Context, requestID, logID string) (entity.RequestLog, error) { + log, exists := m.history[logID] + if !exists || log.RequestID != requestID { + return entity.RequestLog{}, storage.ErrNotFound + } + return log, nil + }).AnyTimes() + + m.builds = storagemock.NewMockBuildStore(ctrl) + m.requests = storagemock.NewMockRequestStore(ctrl) + m.store.EXPECT().GetBuildStore().Return(m.builds).AnyTimes() + m.store.EXPECT().GetRequestStore().Return(m.requests).AnyTimes() + m.builds.EXPECT().Get(gomock.Any(), testBuildID).Return(entity.Build{ID: testBuildID, RequestID: "1"}, nil).AnyTimes() + m.requests.EXPECT().Get(gomock.Any(), "1").Return(entity.Request{ID: "1", Queue: testQueue, State: entity.RequestStateFailed}, nil).AnyTimes() + return NewController(zap.New(core).Sugar(), tally.NoopScope, m.stores, m.materializer, m.executors, consumer.TopicKey("fix"), "stovepipe-fix"), m } diff --git a/stovepipe/controller/fix/history.go b/stovepipe/controller/fix/history.go new file mode 100644 index 000000000..6f1817395 --- /dev/null +++ b/stovepipe/controller/fix/history.go @@ -0,0 +1,74 @@ +// Copyright (c) 2026 Uber Technologies, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package fix + +import ( + "context" + "crypto/sha256" + "fmt" + + "github.com/uber/submitqueue/platform/errs" + "github.com/uber/submitqueue/stovepipe/core/loader" + "github.com/uber/submitqueue/stovepipe/core/requestlog" + "github.com/uber/submitqueue/stovepipe/entity" + "github.com/uber/submitqueue/stovepipe/extension/storage" +) + +func loadRequestForBuild(ctx context.Context, store storage.Storage, queue, buildID string) (entity.Request, error) { + build, err := loader.ByID(ctx, buildID, store.GetBuildStore().Get, "build") + if err != nil { + return entity.Request{}, err + } + if build.RequestID == "" { + return entity.Request{}, errs.NewUserError(fmt.Errorf("build %q has no request", buildID)) + } + request, err := loader.ByID(ctx, build.RequestID, store.GetRequestStore().Get, "request") + if err != nil { + return entity.Request{}, err + } + if request.Queue != queue { + return entity.Request{}, errs.NewUserError(fmt.Errorf("repair request belongs to another queue")) + } + return request, nil +} + +func persistFixSubmittedLog(ctx context.Context, materializer requestlog.Materializer, store storage.Storage, request entity.Request, buildID string, observation entity.BuildIssue) error { + // The queue-scoped executor's opaque fix ID identifies the accepted attempt. + // Restoring this from the durable association must precede any replay skip. + // Opaque fix IDs can fill the storage column; the event prefix must still fit its own key. + identity := sha256.Sum256([]byte(observation.FixID)) + log := requestlog.NewRequestEventLog(request, entity.RequestEventFixSubmitted, fmt.Sprintf("%x", identity), + map[string]string{ + requestlog.MetadataKeyBuildID: buildID, + requestlog.MetadataKeyTaskID: observation.TaskID, + requestlog.MetadataKeyUnitID: observation.UnitID, + requestlog.MetadataKeyFixID: observation.FixID, + }) + if err := materializer.PersistLog(ctx, store, log); err != nil { + return fmt.Errorf("retain submitted repair %q for build %q: %w", observation.FixID, buildID, err) + } + return nil +} + +func persistFixSubmissionDecision(ctx context.Context, materializer requestlog.Materializer, store storage.Storage, request entity.Request, buildID string, event entity.RequestEvent, reason string) error { + log := requestlog.NewRequestEventLog(request, event, buildID, map[string]string{ + requestlog.MetadataKeyBuildID: buildID, + requestlog.MetadataKeyReason: reason, + }) + if err := materializer.PersistLog(ctx, store, log); err != nil { + return fmt.Errorf("retain repair decision for build %q: %w", buildID, err) + } + return nil +} diff --git a/stovepipe/controller/fix/history_test.go b/stovepipe/controller/fix/history_test.go new file mode 100644 index 000000000..e36ea2568 --- /dev/null +++ b/stovepipe/controller/fix/history_test.go @@ -0,0 +1,148 @@ +// Copyright (c) 2026 Uber Technologies, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package fix + +import ( + "context" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/uber-go/tally" + consumermock "github.com/uber/submitqueue/platform/consumer/mock" + "github.com/uber/submitqueue/stovepipe/core/messagequeue" + "github.com/uber/submitqueue/stovepipe/core/requestlog" + "github.com/uber/submitqueue/stovepipe/entity" + "github.com/uber/submitqueue/stovepipe/extension/storage" + "go.uber.org/mock/gomock" + "go.uber.org/zap" +) + +func TestSubmittedFixHistoryRepairsWithoutResubmitting(t *testing.T) { + c, m := newTestController(t) + issue := testIssue() + m.expectIssue(issue) + m.executor.EXPECT().Submit(gomock.Any(), issue).Return("fix/1", assert.AnError) + saved := issueWithFixID(issue, "fix/1") + m.observations.EXPECT().Update(gomock.Any(), testBuildID, saved, int32(0), int32(1)).Return(nil) + *m.historyFailure = assert.AnError + require.ErrorIs(t, c.processBuild(context.Background(), testQueue, testBuildID), assert.AnError) + require.Empty(t, m.history) + *m.historyFailure = nil + for i := 0; i < 2; i++ { + m.stores.EXPECT().For(storage.Config{QueueName: testQueue}).Return(m.store, nil) + m.observations.EXPECT().List(gomock.Any(), testBuildID, storage.BuildIssueRange{Limit: buildIssuePageSize}).Return([]entity.BuildIssue{saved}, nil) + require.NoError(t, c.processBuild(context.Background(), testQueue, testBuildID)) + } + require.Len(t, m.history, 1) + var log entity.RequestLog + for _, retained := range m.history { + log = retained + } + require.Equal(t, entity.RequestEventFixSubmitted, log.Event) + require.Equal(t, map[string]string{ + requestlog.MetadataKeyBuildID: testBuildID, requestlog.MetadataKeyFixID: "fix/1", + requestlog.MetadataKeyTaskID: issue.TaskID, requestlog.MetadataKeyUnitID: issue.UnitID, + }, log.Metadata) + require.Empty(t, log.State) +} + +func TestFixDLQRetainsDecisionWithoutSubmitting(t *testing.T) { + for _, decision := range []string{"associated", "eligible", "ineligible"} { + t.Run(decision, func(t *testing.T) { + _, m := newTestController(t) + c := NewDLQController(zap.NewNop().Sugar(), tally.NoopScope, m.stores, m.materializer, messagequeue.TopicKeyFix, "recovery") + issue := testIssue() + observation := issueIdentity(issue) + expected := entity.RequestEventFixSubmissionAbandoned + switch decision { + case "associated": + observation.FixID = "fix/1" + expected = entity.RequestEventFixSubmitted + case "ineligible": + expected = entity.RequestEventFixSubmissionSkipped + } + for i := 0; i < 3; i++ { + m.stores.EXPECT().For(storage.Config{QueueName: testQueue}).Return(m.store, nil) + m.observations.EXPECT().List(gomock.Any(), testBuildID, storage.BuildIssueRange{Limit: buildIssuePageSize}).Return([]entity.BuildIssue{observation}, nil) + if decision != "associated" { + current := storedIssue(issue) + if decision == "ineligible" { + current.Status = entity.IssueStatusClosed + } + m.issues.EXPECT().Get(gomock.Any(), issue.TaskID, issue.UnitID).Return(current, nil) + m.observations.EXPECT().List(gomock.Any(), testBuildID, afterIssue(observation)).Return(nil, nil) + } + d := consumermock.NewMockDelivery(m.ctrl) + d.EXPECT().Message().Return(repairMessage(t)) + if i == 0 { + *m.historyFailure = assert.AnError + require.ErrorIs(t, c.Process(context.Background(), d), assert.AnError) + require.Empty(t, m.history) + *m.historyFailure = nil + } else { + require.NoError(t, c.Process(context.Background(), d)) + } + } + require.Len(t, m.history, 1) + for _, log := range m.history { + require.Equal(t, expected, log.Event) + require.Equal(t, testBuildID, log.Metadata[requestlog.MetadataKeyBuildID]) + if decision == "eligible" { + require.Equal(t, "acceptance_unknown", log.Metadata[requestlog.MetadataKeyReason]) + } + } + }) + } +} + +func TestFixDLQDiscardsInvalidMessages(t *testing.T) { + for _, problem := range []string{"payload", "tenant", "missing build"} { + t.Run(problem, func(t *testing.T) { + _, m := newTestController(t) + c := NewDLQController(zap.NewNop().Sugar(), tally.NoopScope, m.stores, m.materializer, messagequeue.TopicKeyFix, "recovery") + msg := repairMessage(t) + switch problem { + case "payload": + msg.Payload = []byte("invalid") + case "tenant": + msg.Tenant = "other" + case "missing build": + var err error + msg.Payload, err = messagequeue.Marshal(&messagequeue.FixRequest{Id: "missing", QueueName: testQueue}) + require.NoError(t, err) + m.stores.EXPECT().For(storage.Config{QueueName: testQueue}).Return(m.store, nil) + m.builds.EXPECT().Get(gomock.Any(), "missing").Return(entity.Build{}, storage.ErrNotFound) + } + require.NoError(t, c.Process(context.Background(), m.delivery(msg))) + require.Empty(t, m.history) + }) + } +} + +func TestSubmittedFixHistoryFitsStorageKeyWithMaximumLengthFixID(t *testing.T) { + _, m := newTestController(t) + observation := issueWithFixID(testIssue(), strings.Repeat("f", 255)) + request := entity.Request{ID: "1", Queue: testQueue} + for i := 0; i < 2; i++ { + require.NoError(t, persistFixSubmittedLog(context.Background(), m.materializer, m.store, request, testBuildID, observation)) + } + require.Len(t, m.history, 1) + for _, log := range m.history { + require.LessOrEqual(t, len(log.ID), 255) + require.Equal(t, observation.FixID, log.Metadata[requestlog.MetadataKeyFixID]) + } +} diff --git a/stovepipe/controller/issuetrack/BUILD.bazel b/stovepipe/controller/issuetrack/BUILD.bazel index f0a302ea4..44115858c 100644 --- a/stovepipe/controller/issuetrack/BUILD.bazel +++ b/stovepipe/controller/issuetrack/BUILD.bazel @@ -4,6 +4,7 @@ go_library( name = "go_default_library", srcs = [ "dlq.go", + "history.go", "issuetrack.go", ], importpath = "github.com/uber/submitqueue/stovepipe/controller/issuetrack", @@ -32,6 +33,7 @@ go_test( "dlq_test.go", "fix_handoff_test.go", "fixture_test.go", + "history_test.go", "issuetrack_test.go", ], embed = [":go_default_library"], diff --git a/stovepipe/controller/issuetrack/README.md b/stovepipe/controller/issuetrack/README.md index 701af184f..87e7bed73 100644 --- a/stovepipe/controller/issuetrack/README.md +++ b/stovepipe/controller/issuetrack/README.md @@ -44,6 +44,8 @@ An unfinished pass completes or is safely abandoned before another source is adm Build IDs are opaque; source order comes from Request IDs. All scans are bounded reads under leading primary-key prefixes. There is no transaction spanning progress, issue state, and history. +A finalized checkpoint retains `issues_reconciled` in its Request's history before acknowledgement or replacement by a newer pass. The build ID identifies the occurrence. Metadata distinguishes normal reconciliation from cancellation or task-wide suppression of absent-issue closure. A completed nonterminal refresh adds no request-history entry. If history fails after checkpoint persistence, the next primary or recovery delivery repairs that entry before replacing the checkpoint. + ## Recovery and activation An error returns to normal consumer retry. The next delivery resumes the cursor before advancing. Once that retry policy dead-letters a notification, the recovery controller abandons the failed pass rather than rerunning its remaining batches: @@ -61,7 +63,7 @@ A notification can fail while helping an older pass. Recovery abandons that save Storage and history failures return for retry; hosts must pair recovery with an always-retryable, unlimited-retry DLQ consumer. Only recovery work repeats. Malformed message identities, unresolvable queues, and missing source records are logged and acknowledged. -The reference service registers queue-partitioned `issuetrack` and `issuetrack_dlq` subscriptions. Normal processing uses backend error classification and the extended bounded retry policy, while recovery uses the existing always-retryable DLQ consumer. The DLQ consumer starts before normal consumption and stops after it. Deploy recovery-aware controllers and provision the issue tables before enabling consumption; older controllers do not recognize the abandonment phases. Completed opening passes publish repair notifications; recovery does not. The reference host also consumes repair notifications with a no-op executor; fix dead-letter recovery remains separate. +The reference service registers queue-partitioned `issuetrack` and `issuetrack_dlq` subscriptions. Normal processing uses backend error classification and the extended bounded retry policy, while recovery uses the existing always-retryable DLQ consumer. The DLQ consumer starts before normal consumption and stops after it. Deploy recovery-aware controllers and provision the issue tables before enabling consumption; older controllers do not recognize the abandonment phases. Completed opening passes publish repair notifications; recovery does not. The reference host also consumes repair notifications with a no-op executor; fix dead-letter recovery retains submission outcomes without calling the executor. ## Analysis handoff diff --git a/stovepipe/controller/issuetrack/dlq.go b/stovepipe/controller/issuetrack/dlq.go index 14440e55c..ec6c47043 100644 --- a/stovepipe/controller/issuetrack/dlq.go +++ b/stovepipe/controller/issuetrack/dlq.go @@ -45,7 +45,7 @@ type DLQController struct { func NewDLQController(logger *zap.SugaredLogger, scope tally.Scope, stores storage.Factory, materializer requestlog.Materializer, topicKey consumer.TopicKey, consumerGroup string) *DLQController { return &DLQController{ - controller: NewController(logger, scope, stores, consumer.TopicRegistry{}, topicKey, consumerGroup), + controller: NewController(logger, scope, stores, materializer, consumer.TopicRegistry{}, topicKey, consumerGroup), materializer: materializer, } } @@ -130,6 +130,9 @@ func (c *DLQController) abandonPass(ctx context.Context, store storage.Storage, if err := progress.Validate(); err != nil { return entity.IssueTrackingState{}, fmt.Errorf("invalid issue tracking recovery: %w", err) } + if err := persistIssuesReconciledLog(ctx, c.materializer, store, request.Queue, progress); err != nil { + return entity.IssueTrackingState{}, err + } if progress.NotificationID == notificationID { switch progress.Phase { case entity.IssueTrackingPhaseRefreshed, entity.IssueTrackingPhaseFinalized: diff --git a/stovepipe/controller/issuetrack/dlq_test.go b/stovepipe/controller/issuetrack/dlq_test.go index af723ae5f..0c8ad3bd5 100644 --- a/stovepipe/controller/issuetrack/dlq_test.go +++ b/stovepipe/controller/issuetrack/dlq_test.go @@ -46,10 +46,13 @@ func newRecoveryFixture(t *testing.T) *recoveryFixture { f := &recoveryFixture{fixture: newFixture(t), events: map[string]entity.RequestLog{}} materializer := requestlogmock.NewMockMaterializer(f.mocks) materializer.EXPECT().PersistLog(gomock.Any(), f.store, gomock.Any()).DoAndReturn( - func(_ context.Context, _ storage.Storage, log entity.RequestLog) error { + func(ctx context.Context, _ storage.Storage, log entity.RequestLog) error { if err := f.fail("recovery/history"); err != nil { return err } + if log.Event == entity.RequestEventIssuesReconciled { + return f.materializer.PersistLog(ctx, f.store, log) + } log.TimestampMs = 1000 require.NoError(t, log.Validate()) require.Equal(t, entity.RequestEventIssueTrackingAbandoned, log.Event) diff --git a/stovepipe/controller/issuetrack/fixture_test.go b/stovepipe/controller/issuetrack/fixture_test.go index 232abd91b..fe1c60b4a 100644 --- a/stovepipe/controller/issuetrack/fixture_test.go +++ b/stovepipe/controller/issuetrack/fixture_test.go @@ -26,6 +26,7 @@ import ( "github.com/uber/submitqueue/platform/consumer" queuemock "github.com/uber/submitqueue/platform/extension/messagequeue/mock" "github.com/uber/submitqueue/stovepipe/core/messagequeue" + "github.com/uber/submitqueue/stovepipe/core/requestlog" "github.com/uber/submitqueue/stovepipe/entity" "github.com/uber/submitqueue/stovepipe/extension/storage" storagemock "github.com/uber/submitqueue/stovepipe/extension/storage/mock" @@ -42,24 +43,26 @@ type logKey struct { } type fixture struct { - failOnPhase entity.IssueTrackingPhase - t *testing.T - mocks *gomock.Controller - controller *Controller - factory *storagemock.MockFactory - store storage.Storage - states map[issueKey]entity.IssueState - summaries map[issueKey]entity.IssueSummary - openIssues map[issueKey]entity.OpenIssue - logs map[logKey]entity.IssueLog - progress entity.IssueTrackingState - hasProgress bool - failures map[string]int - initialLogs map[logKey]entity.IssueLog - builds map[string]entity.Build - requests map[string]entity.Request - buildIssues map[string]map[issueKey]entity.BuildIssue - sequence int + requestHistory map[string]entity.RequestLog + materializer requestlog.Materializer + failOnPhase entity.IssueTrackingPhase + t *testing.T + mocks *gomock.Controller + controller *Controller + factory *storagemock.MockFactory + store storage.Storage + states map[issueKey]entity.IssueState + summaries map[issueKey]entity.IssueSummary + openIssues map[issueKey]entity.OpenIssue + logs map[logKey]entity.IssueLog + progress entity.IssueTrackingState + hasProgress bool + failures map[string]int + initialLogs map[logKey]entity.IssueLog + builds map[string]entity.Build + requests map[string]entity.Request + buildIssues map[string]map[issueKey]entity.BuildIssue + sequence int fixMessages []entityqueue.Message onFixPublish func(entityqueue.Message) @@ -351,7 +354,29 @@ func newFixture(t *testing.T) *fixture { }).AnyTimes() registry, err := consumer.NewTopicRegistry([]consumer.TopicConfig{{Key: messagequeue.TopicKeyFix, Name: "fix", Queue: queue}}) require.NoError(t, err) - f.controller = NewController(zap.NewNop().Sugar(), tally.NoopScope, f.factory, registry, messagequeue.TopicKeyIssueTrack, "test") + f.requestHistory = map[string]entity.RequestLog{} + f.materializer = requestlog.NewMaterializer(tally.NoopScope) + requestHistory := storagemock.NewMockRequestLogStore(f.mocks) + store.EXPECT().GetRequestLogStore().Return(requestHistory).AnyTimes() + requestHistory.EXPECT().Create(gomock.Any(), gomock.Any()).DoAndReturn(func(_ context.Context, log entity.RequestLog) error { + if err := f.fail("request-history/create"); err != nil { + return err + } + key := log.RequestID + ":" + log.ID + if _, exists := f.requestHistory[key]; exists { + return storage.ErrAlreadyExists + } + f.requestHistory[key] = log + return nil + }).AnyTimes() + requestHistory.EXPECT().Get(gomock.Any(), gomock.Any(), gomock.Any()).DoAndReturn(func(_ context.Context, requestID, logID string) (entity.RequestLog, error) { + log, exists := f.requestHistory[requestID+":"+logID] + if !exists { + return entity.RequestLog{}, storage.ErrNotFound + } + return log, nil + }).AnyTimes() + f.controller = NewController(zap.NewNop().Sugar(), tally.NoopScope, f.factory, f.materializer, registry, messagequeue.TopicKeyIssueTrack, "test") f.controller.nowMs = func() int64 { return 1000 } return f } diff --git a/stovepipe/controller/issuetrack/history.go b/stovepipe/controller/issuetrack/history.go new file mode 100644 index 000000000..66a5489b6 --- /dev/null +++ b/stovepipe/controller/issuetrack/history.go @@ -0,0 +1,59 @@ +// Copyright (c) 2026 Uber Technologies, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package issuetrack + +import ( + "context" + "fmt" + + "github.com/uber/submitqueue/stovepipe/core/requestlog" + "github.com/uber/submitqueue/stovepipe/entity" + "github.com/uber/submitqueue/stovepipe/extension/storage" +) + +func persistIssuesReconciledLog(ctx context.Context, materializer requestlog.Materializer, store storage.Storage, queue string, progress entity.IssueTrackingState) error { + if progress.Phase != entity.IssueTrackingPhaseFinalized { + return nil + } + if err := progress.Validate(); err != nil { + return fmt.Errorf("invalid completed issue tracking progress: %w", err) + } + build, request, err := loadBuildAndRequest(ctx, store, queue, progress.BuildID) + if err != nil { + return err + } + if request.ID != progress.RequestID { + return fmt.Errorf("completed issue tracking progress belongs to another request") + } + if err := requireTerminalBuild(build, request); err != nil { + return err + } + mode := "reconciled" + switch { + case build.Status == entity.BuildStatusCancelled: + mode = "suppressed_cancelled" + case progress.HasTaskWideIssues: + mode = "suppressed_task_wide_issue" + } + log := requestlog.NewRequestEventLog(request, entity.RequestEventIssuesReconciled, build.ID, + map[string]string{ + requestlog.MetadataKeyBuildID: build.ID, + requestlog.MetadataKeyClosureMode: mode, + }) + if err := materializer.PersistLog(ctx, store, log); err != nil { + return fmt.Errorf("retain issue reconciliation for build %q: %w", build.ID, err) + } + return nil +} diff --git a/stovepipe/controller/issuetrack/history_test.go b/stovepipe/controller/issuetrack/history_test.go new file mode 100644 index 000000000..21ba6ca58 --- /dev/null +++ b/stovepipe/controller/issuetrack/history_test.go @@ -0,0 +1,110 @@ +// Copyright (c) 2026 Uber Technologies, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package issuetrack + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/uber-go/tally" + "github.com/uber/submitqueue/platform/base/failure" + entityqueue "github.com/uber/submitqueue/platform/base/messagequeue" + consumermock "github.com/uber/submitqueue/platform/consumer/mock" + "github.com/uber/submitqueue/stovepipe/core/messagequeue" + "github.com/uber/submitqueue/stovepipe/core/requestlog" + "github.com/uber/submitqueue/stovepipe/entity" + "go.uber.org/zap" +) + +func (f *fixture) recoverHistory(msg entityqueue.Message, original failure.Failure) error { + f.t.Helper() + c := NewDLQController(zap.NewNop().Sugar(), tally.NoopScope, f.factory, f.materializer, messagequeue.TopicKeyIssueTrack, "recovery") + d := consumermock.NewMockDelivery(f.mocks) + d.EXPECT().Message().Return(msg) + d.EXPECT().Failure().Return(original, true).AnyTimes() + return c.Process(context.Background(), d) +} + +func TestReconciliationHistoryClosureModes(t *testing.T) { + for _, mode := range []string{"reconciled", "suppressed_cancelled", "suppressed_task_wide_issue"} { + t.Run(mode, func(t *testing.T) { + f := newFixture(t) + status := entity.BuildStatusFailed + if mode == "suppressed_cancelled" { + status = entity.BuildStatusCancelled + } + f.source("build", "10", status) + f.seedIssue("old", "unit", entity.IssueStatusOpen) + if mode == "suppressed_task_wide_issue" { + f.reportIssue("build", "task", "") + } + msg := f.message("build", true) + require.NoError(t, f.deliver(msg, mode == "reconciled")) + if mode == "reconciled" { + require.NoError(t, f.deliver(msg, false)) + } + require.Len(t, f.requestHistory, 1) + log := f.requestHistory["10:event/issues_reconciled/build"] + require.Equal(t, entity.RequestEventIssuesReconciled, log.Event) + require.Equal(t, mode, log.Metadata[requestlog.MetadataKeyClosureMode]) + expectedStatus := entity.IssueStatusOpen + if mode == "reconciled" { + expectedStatus = entity.IssueStatusClosed + } + require.Equal(t, expectedStatus, f.states[issueKey{"old", "unit"}].Status) + require.NoError(t, f.deliver(msg, false)) + require.Equal(t, log, f.requestHistory["10:event/issues_reconciled/build"]) + }) + } +} + +func TestCompletedReconciliationRepairsHistoryBeforeReplacingCheckpoint(t *testing.T) { + f := newFixture(t) + f.source("old", "10", entity.BuildStatusSucceeded) + msg := f.message("old", true) + require.NoError(t, f.deliver(msg, true)) + f.failures["request-history/create"] = 1 + require.ErrorIs(t, f.deliver(msg, false), assert.AnError) + require.Equal(t, entity.IssueTrackingPhaseFinalized, f.progress.Phase) + require.Empty(t, f.requestHistory) + completed := f.progress + f.source("new", "11", entity.BuildStatusRunning) + newer := f.message("new", false) + f.failures["request-history/create"] = 1 + err := f.deliver(newer, false) + require.ErrorIs(t, err, assert.AnError) + require.Equal(t, completed, f.progress) + require.NoError(t, f.deliver(newer, false)) + require.Equal(t, "new", f.progress.BuildID) + require.Contains(t, f.requestHistory, "10:event/issues_reconciled/old") + require.Len(t, f.requestHistory, 1) // A nonterminal refresh adds no event. +} + +func TestIssueTrackingDLQRepairsCompletedPassInsteadOfAbandoning(t *testing.T) { + f := newFixture(t) + f.source("build", "10", entity.BuildStatusSucceeded) + msg := f.message("build", true) + require.NoError(t, f.deliver(msg, true)) + f.failures["request-history/create"] = 1 + err := f.deliver(msg, false) + require.ErrorIs(t, err, assert.AnError) + checkpoint := f.progress + require.NoError(t, f.recoverHistory(msg, failure.Failure{})) + require.Equal(t, checkpoint, f.progress) + require.Len(t, f.requestHistory, 1) + require.Contains(t, f.requestHistory, "10:event/issues_reconciled/build") +} diff --git a/stovepipe/controller/issuetrack/issuetrack.go b/stovepipe/controller/issuetrack/issuetrack.go index f6beedf3b..8ea62d643 100644 --- a/stovepipe/controller/issuetrack/issuetrack.go +++ b/stovepipe/controller/issuetrack/issuetrack.go @@ -31,6 +31,7 @@ import ( "github.com/uber/submitqueue/stovepipe/core/issuelog" "github.com/uber/submitqueue/stovepipe/core/loader" stovepipemq "github.com/uber/submitqueue/stovepipe/core/messagequeue" + "github.com/uber/submitqueue/stovepipe/core/requestlog" "github.com/uber/submitqueue/stovepipe/entity" "github.com/uber/submitqueue/stovepipe/extension/storage" "go.uber.org/zap" @@ -43,26 +44,28 @@ type issueKey struct{ task, unit string } // Controller retains issue transitions and their queue-scoped progress. type Controller struct { - logger *zap.SugaredLogger - metricsScope tally.Scope - stores storage.Factory - materializer issuelog.Materializer - registry consumer.TopicRegistry - topicKey consumer.TopicKey - consumerGroup string - nowMs func() int64 + logger *zap.SugaredLogger + metricsScope tally.Scope + stores storage.Factory + materializer issuelog.Materializer + registry consumer.TopicRegistry + requestMaterializer requestlog.Materializer + topicKey consumer.TopicKey + consumerGroup string + nowMs func() int64 } -func NewController(logger *zap.SugaredLogger, scope tally.Scope, stores storage.Factory, registry consumer.TopicRegistry, topicKey consumer.TopicKey, consumerGroup string) *Controller { +func NewController(logger *zap.SugaredLogger, scope tally.Scope, stores storage.Factory, materializer requestlog.Materializer, registry consumer.TopicRegistry, topicKey consumer.TopicKey, consumerGroup string) *Controller { return &Controller{ - logger: logger.Named("issuetrack_controller"), - metricsScope: scope.SubScope("issuetrack_controller"), - stores: stores, - materializer: issuelog.NewMaterializer(scope), - registry: registry, - topicKey: topicKey, - consumerGroup: consumerGroup, - nowMs: func() int64 { return time.Now().UnixMilli() }, + logger: logger.Named("issuetrack_controller"), + metricsScope: scope.SubScope("issuetrack_controller"), + stores: stores, + materializer: issuelog.NewMaterializer(scope), + registry: registry, + requestMaterializer: materializer, + topicKey: topicKey, + consumerGroup: consumerGroup, + nowMs: func() int64 { return time.Now().UnixMilli() }, } } @@ -107,6 +110,11 @@ func (c *Controller) trackBuild(ctx context.Context, delivery consumer.Delivery, } else if err != nil { return fmt.Errorf("load issue tracking progress: %w", err) } + // A completed checkpoint is the repair source until its history exists. + // Retain that occurrence before another notification can replace it. + if err := persistIssuesReconciledLog(ctx, c.requestMaterializer, store, queue, progress); err != nil { + return err + } if progress.Phase == entity.IssueTrackingPhaseAbandoning { delivery.Hold(_recoveryDelayMs) @@ -167,6 +175,9 @@ func (c *Controller) trackBuild(ctx context.Context, delivery consumer.Delivery, if err != nil { return err } + if err := persistIssuesReconciledLog(ctx, c.requestMaterializer, store, queue, checkpoint); err != nil { + return err + } if checkpoint.NotificationID != notificationID || checkpoint.Phase == entity.IssueTrackingPhaseRefreshing || checkpoint.Phase == entity.IssueTrackingPhaseFinalizing { diff --git a/stovepipe/core/requestlog/materializer.go b/stovepipe/core/requestlog/materializer.go index a31aa90e0..c96747156 100644 --- a/stovepipe/core/requestlog/materializer.go +++ b/stovepipe/core/requestlog/materializer.go @@ -46,6 +46,18 @@ const ( MetadataKeyBrokenProjectFactCount = "broken_project_fact_count" // MetadataKeyBaseURI records the selected build baseline; an empty value means a full build. MetadataKeyBaseURI = "base_uri" + // MetadataKeyIssueCount records the complete issue count from terminal build analysis. + MetadataKeyIssueCount = "issue_count" + // MetadataKeyClosureMode records whether absent issues were reconciled or why closure was suppressed. + MetadataKeyClosureMode = "closure_mode" + // MetadataKeyFixID identifies an opaque repair attempt. + MetadataKeyFixID = "fix_id" + // MetadataKeyTaskID identifies the validation task associated with a repair attempt. + MetadataKeyTaskID = "task_id" + // MetadataKeyUnitID identifies the validation unit; empty means a task-wide issue. + MetadataKeyUnitID = "unit_id" + // MetadataKeyReason is a bounded explanation for a skipped or abandoned operation. + MetadataKeyReason = "reason" ) // Materializer retains request-log occurrences and advances their derived read models. diff --git a/stovepipe/core/requestlog/materializer_test.go b/stovepipe/core/requestlog/materializer_test.go index e12ed0fe5..4586a82be 100644 --- a/stovepipe/core/requestlog/materializer_test.go +++ b/stovepipe/core/requestlog/materializer_test.go @@ -400,3 +400,30 @@ func TestMaterializerRetainsOtherEventsWithoutProjectingSummary(t *testing.T) { }) } } + +func TestRepairEventsRemainHistoryOnlyAndDeduplicate(t *testing.T) { + for _, event := range []entity.RequestEvent{ + entity.RequestEventBuildAnalysisCompleted, entity.RequestEventIssuesReconciled, + entity.RequestEventIssueTrackingAbandoned, entity.RequestEventFixSubmitted, + entity.RequestEventFixSubmissionSkipped, entity.RequestEventFixSubmissionAbandoned, + } { + t.Run(string(event), func(t *testing.T) { + m, stores, logs, _, _ := newTestMaterializer(t) + request := entity.Request{ID: testRequestID, Queue: testQueue, State: entity.RequestStateFailed, Version: 3} + candidate := NewRequestEventLog(request, event, "occurrence/1", map[string]string{MetadataKeyBuildID: "build/1"}) + stored := candidate + stored.TimestampMs = testNowMs + gomock.InOrder( + logs.EXPECT().Create(gomock.Any(), stored).Return(nil), + logs.EXPECT().Create(gomock.Any(), gomock.Any()).Return(storage.ErrAlreadyExists), + logs.EXPECT().Get(gomock.Any(), testRequestID, candidate.ID).Return(stored, nil), + ) + require.NoError(t, m.PersistLog(context.Background(), stores, candidate)) + m.now = func() time.Time { return time.UnixMilli(testNowMs + 1000) } + require.NoError(t, m.PersistLog(context.Background(), stores, candidate)) + require.Empty(t, candidate.State) + require.Zero(t, candidate.RequestVersion) + require.Empty(t, candidate.OutcomeReason) + }) + } +} diff --git a/stovepipe/entity/request_log.go b/stovepipe/entity/request_log.go index 1c15d67a9..9e6927398 100644 --- a/stovepipe/entity/request_log.go +++ b/stovepipe/entity/request_log.go @@ -34,8 +34,18 @@ const ( RequestEventRecordAbandoned RequestEvent = "record_abandoned" // RequestEventBuildAnalysisAbandoned records incomplete build analysis, not a validation outcome. RequestEventBuildAnalysisAbandoned RequestEvent = "build_analysis_abandoned" - // RequestEventIssueTrackingAbandoned records incomplete issue tracking, not a validation outcome. + // RequestEventBuildAnalysisCompleted records complete terminal issue analysis for a build. + RequestEventBuildAnalysisCompleted RequestEvent = "build_analysis_completed" + // RequestEventIssuesReconciled records completed final issue reconciliation for a build. + RequestEventIssuesReconciled RequestEvent = "issues_reconciled" + // RequestEventIssueTrackingAbandoned records unfinished issue reconciliation after primary processing stopped. RequestEventIssueTrackingAbandoned RequestEvent = "issue_tracking_abandoned" + // RequestEventFixSubmitted records an identified repair attempt associated with a build issue. + RequestEventFixSubmitted RequestEvent = "fix_submitted" + // RequestEventFixSubmissionSkipped records a repair decision with no eligible open issue. + RequestEventFixSubmissionSkipped RequestEvent = "fix_submission_skipped" + // RequestEventFixSubmissionAbandoned records stopped repair submission with an unknown acceptance outcome. + RequestEventFixSubmissionAbandoned RequestEvent = "fix_submission_abandoned" ) // RequestOutcomeReason identifies the durable domain reason for a terminal request state. @@ -141,7 +151,10 @@ func (e RequestLog) validateEvent() error { return fmt.Errorf("event log must not contain request-state context") } switch e.Event { - case RequestEventBuildTriggered, RequestEventBuildFinished, RequestEventValidationFactRecorded, RequestEventProjectFactsRecorded, RequestEventRecordAbandoned, RequestEventBuildAnalysisAbandoned, RequestEventIssueTrackingAbandoned: + case RequestEventBuildTriggered, RequestEventBuildFinished, RequestEventValidationFactRecorded, RequestEventProjectFactsRecorded, + RequestEventRecordAbandoned, RequestEventBuildAnalysisAbandoned, RequestEventBuildAnalysisCompleted, + RequestEventIssuesReconciled, RequestEventIssueTrackingAbandoned, + RequestEventFixSubmitted, RequestEventFixSubmissionSkipped, RequestEventFixSubmissionAbandoned: default: return fmt.Errorf("unknown request event %q", e.Event) }