Skip to content

Commit 210c04c

Browse files
committed
feat(stovepipe): report build failure detection latency
Emit the elapsed time from the base change timestamp when a validation build fails, with queue and build-strategy attribution.
1 parent dac4f91 commit 210c04c

5 files changed

Lines changed: 119 additions & 2 deletions

File tree

platform/metrics/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ h := metrics.NamedHistogram(c.scope, "process", "duration", metrics.FastLatencyB
5757
h.RecordDuration(elapsed)
5858
```
5959

60-
Do not emit gauges or timers. Represent operation latency and completion count with lifecycle histograms, and represent instantaneous quantities as sampled histogram values when needed.
60+
Do not emit timers. Represent operation latency and completion count with lifecycle histograms. Use a gauge only for a periodically refreshed, current-state value whose latest observation is the query result; use a histogram for distributions of observations over time.
6161

6262
### Why histograms, not timers
6363

platform/metrics/metrics.go

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,25 @@ var (
106106
2 * time.Hour,
107107
4 * time.Hour,
108108
}
109+
110+
// ChangeAgeBuckets suits age-based signals for source-control changes,
111+
// including time to failure detection and last-known-green freshness.
112+
ChangeAgeBuckets = tally.DurationBuckets{
113+
1 * time.Minute,
114+
5 * time.Minute,
115+
15 * time.Minute,
116+
30 * time.Minute,
117+
1 * time.Hour,
118+
2 * time.Hour,
119+
4 * time.Hour,
120+
8 * time.Hour,
121+
12 * time.Hour,
122+
24 * time.Hour,
123+
48 * time.Hour,
124+
7 * 24 * time.Hour,
125+
14 * 24 * time.Hour,
126+
30 * 24 * time.Hour,
127+
}
109128
)
110129

111130
// Op tracks the lifecycle of a named operation. It captures the start time on

stovepipe/controller/buildsignal/BUILD.bazel

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ go_library(
1313
"//stovepipe/core/messagequeue:go_default_library",
1414
"//stovepipe/entity:go_default_library",
1515
"//stovepipe/extension/buildrunner:go_default_library",
16+
"//stovepipe/extension/sourcecontrol:go_default_library",
1617
"//stovepipe/extension/storage:go_default_library",
1718
"@com_github_uber_go_tally//:go_default_library",
1819
"@org_uber_go_zap//:go_default_library",
@@ -33,6 +34,8 @@ go_test(
3334
"//stovepipe/entity:go_default_library",
3435
"//stovepipe/extension/buildrunner:go_default_library",
3536
"//stovepipe/extension/buildrunner/mock:go_default_library",
37+
"//stovepipe/extension/sourcecontrol:go_default_library",
38+
"//stovepipe/extension/sourcecontrol/mock:go_default_library",
3639
"//stovepipe/extension/storage:go_default_library",
3740
"//stovepipe/extension/storage/mock:go_default_library",
3841
"@com_github_stretchr_testify//assert:go_default_library",

stovepipe/controller/buildsignal/buildsignal.go

Lines changed: 61 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import (
2424
"context"
2525
"errors"
2626
"fmt"
27+
"time"
2728

2829
"github.com/uber-go/tally"
2930
entityqueue "github.com/uber/submitqueue/platform/base/messagequeue"
@@ -33,6 +34,7 @@ import (
3334
stovepipemq "github.com/uber/submitqueue/stovepipe/core/messagequeue"
3435
"github.com/uber/submitqueue/stovepipe/entity"
3536
"github.com/uber/submitqueue/stovepipe/extension/buildrunner"
37+
"github.com/uber/submitqueue/stovepipe/extension/sourcecontrol"
3638
"github.com/uber/submitqueue/stovepipe/extension/storage"
3739
"go.uber.org/zap"
3840
)
@@ -61,6 +63,7 @@ type Controller struct {
6163
metricsScope tally.Scope
6264
stores storage.Factory
6365
buildRunners buildrunner.Factory
66+
sourceControl sourcecontrol.Factory
6467
registry consumer.TopicRegistry
6568
topicKey consumer.TopicKey
6669
consumerGroup string
@@ -72,6 +75,16 @@ var _ consumer.Controller = (*Controller)(nil)
7275
// _opName is the metric operation name shared by every emit in this file.
7376
const _opName = "buildsignal"
7477

78+
// Option configures a Controller.
79+
type Option func(*Controller)
80+
81+
// WithSourceControl enables base-change-age metrics for failed builds.
82+
func WithSourceControl(factory sourcecontrol.Factory) Option {
83+
return func(c *Controller) {
84+
c.sourceControl = factory
85+
}
86+
}
87+
7588
// NewController creates a new buildsignal controller.
7689
func NewController(
7790
logger *zap.SugaredLogger,
@@ -81,8 +94,9 @@ func NewController(
8194
registry consumer.TopicRegistry,
8295
topicKey consumer.TopicKey,
8396
consumerGroup string,
97+
options ...Option,
8498
) *Controller {
85-
return &Controller{
99+
controller := &Controller{
86100
logger: logger.Named("buildsignal_controller"),
87101
metricsScope: scope.SubScope("buildsignal_controller"),
88102
stores: stores,
@@ -91,6 +105,10 @@ func NewController(
91105
topicKey: topicKey,
92106
consumerGroup: consumerGroup,
93107
}
108+
for _, option := range options {
109+
option(controller)
110+
}
111+
return controller
94112
}
95113

96114
// Process reloads the build referenced by the delivery, polls its runner for
@@ -270,10 +288,52 @@ func (c *Controller) markOutcome(ctx context.Context, store storage.Storage, req
270288
metrics.NamedCounter(c.metricsScope, _opName, "outcomes", 1,
271289
metrics.NewTag("state", string(state)),
272290
)
291+
if state == entity.RequestStateFailed {
292+
c.emitBaseChangeAge(ctx, request)
293+
}
273294
return nil
274295
}
275296
}
276297

298+
func (c *Controller) emitBaseChangeAge(ctx context.Context, request *entity.Request) {
299+
if c.sourceControl == nil || request.BaseURI == "" {
300+
metrics.NamedCounter(c.metricsScope, "build_failure", "base_change_unavailable", 1,
301+
metrics.NewTag("queue", request.Queue),
302+
metrics.NewTag("strategy", string(request.BuildStrategy)),
303+
)
304+
return
305+
}
306+
307+
control, err := c.sourceControl.For(sourcecontrol.Config{QueueName: request.Queue})
308+
if err != nil {
309+
metrics.NamedCounter(c.metricsScope, "build_failure", "change_info_errors", 1,
310+
metrics.NewTag("queue", request.Queue),
311+
metrics.NewTag("stage", "resolve_source_control"),
312+
)
313+
return
314+
}
315+
info, err := control.ChangeInfo(ctx, request.BaseURI)
316+
if err != nil || info.CreatedAt.IsZero() {
317+
metrics.NamedCounter(c.metricsScope, "build_failure", "change_info_errors", 1,
318+
metrics.NewTag("queue", request.Queue),
319+
metrics.NewTag("stage", "get_change_info"),
320+
)
321+
return
322+
}
323+
age := time.Since(info.CreatedAt)
324+
if age < 0 {
325+
metrics.NamedCounter(c.metricsScope, "build_failure", "change_info_errors", 1,
326+
metrics.NewTag("queue", request.Queue),
327+
metrics.NewTag("stage", "future_change"),
328+
)
329+
return
330+
}
331+
metrics.NamedHistogram(c.metricsScope, "build_failure", "time_to_detection", metrics.ChangeAgeBuckets,
332+
metrics.NewTag("queue", request.Queue),
333+
metrics.NewTag("strategy", string(request.BuildStrategy)),
334+
).RecordDuration(age)
335+
}
336+
277337
// releaseBuildSlot CAS-decrements the queue's in_flight_count, reopening the process
278338
// concurrency gate now that this request's build is over. It decrements relatively
279339
// (preserving concurrent updates), clamps at zero, and retries on version conflicts.

stovepipe/controller/buildsignal/buildsignal_test.go

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818
"context"
1919
"errors"
2020
"testing"
21+
"time"
2122

2223
"github.com/stretchr/testify/assert"
2324
"github.com/stretchr/testify/require"
@@ -31,6 +32,8 @@ import (
3132
"github.com/uber/submitqueue/stovepipe/entity"
3233
"github.com/uber/submitqueue/stovepipe/extension/buildrunner"
3334
buildrunnermock "github.com/uber/submitqueue/stovepipe/extension/buildrunner/mock"
35+
"github.com/uber/submitqueue/stovepipe/extension/sourcecontrol"
36+
sourcecontrolmock "github.com/uber/submitqueue/stovepipe/extension/sourcecontrol/mock"
3437
"github.com/uber/submitqueue/stovepipe/extension/storage"
3538
storagemock "github.com/uber/submitqueue/stovepipe/extension/storage/mock"
3639
"go.uber.org/mock/gomock"
@@ -143,6 +146,38 @@ func expectFinish(m buildsignalMocks, state entity.RequestState) {
143146
m.reqStore.EXPECT().Update(gomock.Any(), requestWithState(state), int32(1), int32(2)).Return(nil)
144147
}
145148

149+
func TestEmitBaseChangeAge(t *testing.T) {
150+
ctrl := gomock.NewController(t)
151+
scope := tally.NewTestScope("test", nil)
152+
sourceControls := sourcecontrolmock.NewMockFactory(ctrl)
153+
source := sourcecontrolmock.NewMockSourceControl(ctrl)
154+
baseURI := "git://github.com/uber-code/repo/refs%2Fheads%2Fmain/abc"
155+
156+
sourceControls.EXPECT().For(sourcecontrol.Config{QueueName: testQueue}).Return(source, nil)
157+
source.EXPECT().ChangeInfo(gomock.Any(), baseURI).Return(sourcecontrol.ChangeInfo{
158+
CreatedAt: time.Now().Add(-time.Hour),
159+
}, nil)
160+
161+
controller := NewController(
162+
zap.NewNop().Sugar(),
163+
scope,
164+
nil,
165+
nil,
166+
consumer.TopicRegistry{},
167+
stovepipemq.TopicKeyBuildSignal,
168+
"stovepipe-buildsignal",
169+
WithSourceControl(sourceControls),
170+
)
171+
controller.emitBaseChangeAge(context.Background(), &entity.Request{
172+
Queue: testQueue,
173+
BaseURI: baseURI,
174+
BuildStrategy: entity.BuildStrategyIncrementalSinceGreen,
175+
})
176+
177+
_, ok := scope.Snapshot().Histograms()["test.buildsignal_controller.build_failure.time_to_detection+queue=monorepo/main,strategy=incremental_since_green"]
178+
assert.True(t, ok)
179+
}
180+
146181
func TestProcess(t *testing.T) {
147182
tests := []struct {
148183
name string

0 commit comments

Comments
 (0)