diff --git a/submitqueue/orchestrator/controller/speculate/finalize.go b/submitqueue/orchestrator/controller/speculate/finalize.go index fb61e91a..d823c93e 100644 --- a/submitqueue/orchestrator/controller/speculate/finalize.go +++ b/submitqueue/orchestrator/controller/speculate/finalize.go @@ -94,13 +94,14 @@ func (c *Controller) finalize(ctx context.Context, snap *snapshot) error { continue } + bypassed := false if decision == outcomeMerge { // The winning path carries the head out of the queue; its // siblings cannot help it any more and are still holding CI // slots the rest of the queue could use. winner, ok := mergeablePath(set, *snap) if !ok { - winner, _ = bypassablePath(batch, set, *snap) + winner, bypassed = bypassablePath(batch, set, *snap) } if supersede(&set, winner.ID, nowMs) { snap.pathSets[batch.ID] = set @@ -119,6 +120,9 @@ func (c *Controller) finalize(ctx context.Context, snap *snapshot) error { // not be offered a head whose set we could not write. continue } + if bypassed { + metrics.NamedCounter(c.metricsScope, opName, "bypass", 1) + } c.recordOutcome(snap, batch.ID, decision) decided++ } diff --git a/submitqueue/orchestrator/controller/speculate/run_test.go b/submitqueue/orchestrator/controller/speculate/run_test.go index 0ae432fb..2c96b74e 100644 --- a/submitqueue/orchestrator/controller/speculate/run_test.go +++ b/submitqueue/orchestrator/controller/speculate/run_test.go @@ -89,6 +89,8 @@ type runHarness struct { logs []entity.RequestLog // failTopic, when set, makes every publish to that topic fail. failTopic string + // metrics records the controller's emitted counters for outcome assertions. + metrics tally.TestScope } // failPublishTo makes publishes to one topic fail, leaving the others working, @@ -117,7 +119,7 @@ func (h *runHarness) run(triggerID string) error { func newRunHarness(t *testing.T, ctrl *gomock.Controller, spec *scriptedSpeculator, inFlight []entity.Batch) *runHarness { t.Helper() - h := &runHarness{spec: spec} + h := &runHarness{spec: spec, metrics: tally.NewTestScope("test", nil)} h.batches = storagemock.NewMockBatchStore(ctrl) for _, b := range inFlight { @@ -193,7 +195,7 @@ func newRunHarness(t *testing.T, ctrl *gomock.Controller, spec *scriptedSpeculat require.NoError(t, err) h.controller = NewController( - zaptest.NewLogger(t).Sugar(), tally.NoopScope, staticStorageFactory{store: store}, + zaptest.NewLogger(t).Sugar(), h.metrics, staticStorageFactory{store: store}, staticSpeculatorFactory{s: spec}, registry, topickey.TopicKeySpeculate, "orchestrator-speculate", ) return h @@ -1547,6 +1549,7 @@ func TestRun_MergingHeadReportsSpeculatedAndNoWait(t *testing.T) { assert.Equal(t, entity.RequestLogTypeStatus, h.logs[0].Type) assert.Equal(t, entity.RequestStatusSpeculated, h.logs[0].Status) assert.Equal(t, head, h.logs[0].Metadata["batch_id"]) + assert.NotContains(t, h.metrics.Snapshot().Counters(), "test.speculate_controller.process.bypass+") } func TestRun_BypassesUnsettledDependenciesWithFullCoverage(t *testing.T) { @@ -1578,6 +1581,10 @@ func TestRun_BypassesUnsettledDependenciesWithFullCoverage(t *testing.T) { assert.Zero(t, h.spec.calls) require.Len(t, h.logs, 1) assert.Equal(t, entity.RequestStatusSpeculated, h.logs[0].Status) + + counter, ok := h.metrics.Snapshot().Counters()["test.speculate_controller.process.bypass+"] + require.True(t, ok) + assert.EqualValues(t, 1, counter.Value()) } // The merge stage publishes landing as its first act on the dispatch. Both