Skip to content

Commit 2a4bf68

Browse files
committed
test(stovepipe): cover freshness metrics
Verify failed-build detection age emission and last-known-green reporting against the source-control and storage boundaries.
1 parent 74c6656 commit 2a4bf68

15 files changed

Lines changed: 364 additions & 3 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

stovepipe/controller/record/BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ go_library(
1111
"//stovepipe/core/loader:go_default_library",
1212
"//stovepipe/core/messagequeue:go_default_library",
1313
"//stovepipe/entity:go_default_library",
14+
"//stovepipe/extension/observability:go_default_library",
1415
"//stovepipe/extension/storage:go_default_library",
1516
"@com_github_uber_go_tally//:go_default_library",
1617
"@org_uber_go_zap//:go_default_library",

stovepipe/controller/record/record.go

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ import (
3333
"github.com/uber/submitqueue/stovepipe/core/loader"
3434
stovepipemq "github.com/uber/submitqueue/stovepipe/core/messagequeue"
3535
"github.com/uber/submitqueue/stovepipe/entity"
36+
"github.com/uber/submitqueue/stovepipe/extension/observability"
3637
"github.com/uber/submitqueue/stovepipe/extension/storage"
3738
"go.uber.org/zap"
3839
)
@@ -44,6 +45,7 @@ type Controller struct {
4445
logger *zap.SugaredLogger
4546
metricsScope tally.Scope
4647
stores storage.Factory
48+
reporter observability.Reporter
4749
topicKey consumer.TopicKey
4850
consumerGroup string
4951
}
@@ -59,21 +61,36 @@ const _opName = "record"
5961
// attribution that this stage does not do, so every fact it writes is whole-repository.
6062
const wholeRepositoryProject = ""
6163

64+
// Option configures a Controller.
65+
type Option func(*Controller)
66+
67+
// WithReporter configures best-effort queue observability reporting.
68+
func WithReporter(reporter observability.Reporter) Option {
69+
return func(c *Controller) {
70+
c.reporter = reporter
71+
}
72+
}
73+
6274
// NewController creates a new record controller.
6375
func NewController(
6476
logger *zap.SugaredLogger,
6577
scope tally.Scope,
6678
stores storage.Factory,
6779
topicKey consumer.TopicKey,
6880
consumerGroup string,
81+
options ...Option,
6982
) *Controller {
70-
return &Controller{
83+
controller := &Controller{
7184
logger: logger.Named("record_controller"),
7285
metricsScope: scope.SubScope("record_controller"),
7386
stores: stores,
7487
topicKey: topicKey,
7588
consumerGroup: consumerGroup,
7689
}
90+
for _, option := range options {
91+
option(controller)
92+
}
93+
return controller
7794
}
7895

7996
// Process loads the request referenced by the delivery and, when its build
@@ -112,6 +129,9 @@ func (c *Controller) Process(ctx context.Context, delivery consumer.Delivery) er
112129
metrics.NamedCounter(c.metricsScope, _opName, "queue_mismatch", 1)
113130
return fmt.Errorf("payload queue %q does not match queue %q of request %s", rec.GetQueueName(), request.Queue, request.ID)
114131
}
132+
if c.reporter != nil {
133+
defer c.reporter.Report(ctx, request.Queue)
134+
}
115135

116136
switch request.State {
117137
case entity.RequestStateSucceeded, entity.RequestStateFailed:
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
load("@rules_go//go:def.bzl", "go_library")
2+
3+
go_library(
4+
name = "go_default_library",
5+
srcs = ["observability.go"],
6+
importpath = "github.com/uber/submitqueue/stovepipe/extension/observability",
7+
visibility = ["//visibility:public"],
8+
)
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
load("@rules_go//go:def.bzl", "go_library", "go_test")
2+
3+
go_library(
4+
name = "go_default_library",
5+
srcs = ["lastgreen.go"],
6+
importpath = "github.com/uber/submitqueue/stovepipe/extension/observability/lastgreen",
7+
visibility = ["//visibility:public"],
8+
deps = [
9+
"//stovepipe/extension/observability:go_default_library",
10+
"//stovepipe/extension/sourcecontrol:go_default_library",
11+
"//stovepipe/extension/storage:go_default_library",
12+
"@com_github_uber_go_tally//:go_default_library",
13+
],
14+
)
15+
16+
go_test(
17+
name = "go_default_test",
18+
srcs = ["lastgreen_test.go"],
19+
embed = [":go_default_library"],
20+
deps = [
21+
"//stovepipe/entity:go_default_library",
22+
"//stovepipe/extension/sourcecontrol:go_default_library",
23+
"//stovepipe/extension/sourcecontrol/mock:go_default_library",
24+
"//stovepipe/extension/storage:go_default_library",
25+
"//stovepipe/extension/storage/mock:go_default_library",
26+
"@com_github_stretchr_testify//assert:go_default_library",
27+
"@com_github_stretchr_testify//require:go_default_library",
28+
"@com_github_uber_go_tally//:go_default_library",
29+
"@org_uber_go_mock//gomock:go_default_library",
30+
],
31+
)

0 commit comments

Comments
 (0)