Skip to content

Commit 5aa1dae

Browse files
authored
feat(metrics): instrument extension implementations with core/metrics (#112)
## Summary - Add `metrics.Begin`/`Complete` lifecycle tracking to all uninstrumented extension implementations: `counter/mysql`, `storage/mysql` (all 6 stores), `scorer/heuristic`, and `scorer/composite` - Each constructor now accepts a `tally.Scope` and every interface method emits called/succeeded/failed counters plus latency timers with error classification tags - Update all call sites (gateway, orchestrator, integration tests, unit tests) to pass the scope parameter ## Test plan - [x] All unit tests pass (`make test` — 24/24 passing) - [x] Full build succeeds (`make build`) - [x] `make gazelle` confirms BUILD files are in sync - [x] Integration tests pass with `tally.NoopScope` (Docker-based, run in CI)
1 parent 4edb8d7 commit 5aa1dae

22 files changed

Lines changed: 173 additions & 64 deletions

File tree

example/server/gateway/main.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ func run() error {
102102
defer appDB.Close()
103103

104104
// Initialize counter from shared app database connection
105-
cnt := mysqlcounter.NewCounter(appDB)
105+
cnt := mysqlcounter.NewCounter(appDB, scope.SubScope("counter"))
106106

107107
// Open queue database connection
108108
queueDSN := os.Getenv("QUEUE_MYSQL_DSN")

example/server/orchestrator/main.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -112,9 +112,9 @@ func run() error {
112112
}
113113
defer appDB.Close()
114114

115-
cnt := mysqlcounter.NewCounter(appDB)
115+
cnt := mysqlcounter.NewCounter(appDB, scope.SubScope("counter"))
116116

117-
store, err := mysqlstorage.NewStorage(appDB)
117+
store, err := mysqlstorage.NewStorage(appDB, scope.SubScope("storage"))
118118
if err != nil {
119119
return fmt.Errorf("failed to create storage: %w", err)
120120
}

extension/counter/mysql/BUILD.bazel

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ go_library(
66
importpath = "github.com/uber/submitqueue/extension/counter/mysql",
77
visibility = ["//visibility:public"],
88
deps = [
9+
"//core/metrics",
910
"//extension/counter",
11+
"@com_github_uber_go_tally_v4//:tally",
1012
],
1113
)

extension/counter/mysql/counter.go

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,21 +5,26 @@ import (
55
"database/sql"
66
"fmt"
77

8+
"github.com/uber-go/tally/v4"
9+
"github.com/uber/submitqueue/core/metrics"
810
"github.com/uber/submitqueue/extension/counter"
911
)
1012

1113
type mysqlCounter struct {
12-
db *sql.DB
14+
db *sql.DB
15+
scope tally.Scope
1316
}
1417

1518
// NewCounter creates a new MySQL-backed Counter.
16-
func NewCounter(db *sql.DB) counter.Counter {
17-
return &mysqlCounter{db: db}
19+
func NewCounter(db *sql.DB, scope tally.Scope) counter.Counter {
20+
return &mysqlCounter{db: db, scope: scope}
1821
}
1922

2023
// Next atomically increments the counter for the given domain and returns the new value.
2124
// Uses MySQL's LAST_INSERT_ID() to set the value atomically and read the incremented value.
22-
func (c *mysqlCounter) Next(ctx context.Context, domain string) (int64, error) {
25+
func (c *mysqlCounter) Next(ctx context.Context, domain string) (ret int64, retErr error) {
26+
op := metrics.Begin(c.scope, "next")
27+
defer func() { op.Complete(retErr) }()
2328
result, err := c.db.ExecContext(ctx,
2429
"INSERT INTO counter (domain, value) VALUES (?, LAST_INSERT_ID(1)) ON DUPLICATE KEY UPDATE value = LAST_INSERT_ID(value + 1)",
2530
domain,

extension/scorer/composite/BUILD.bazel

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,10 @@ go_library(
66
importpath = "github.com/uber/submitqueue/extension/scorer/composite",
77
visibility = ["//visibility:public"],
88
deps = [
9+
"//core/metrics",
910
"//entity",
1011
"//extension/scorer",
12+
"@com_github_uber_go_tally_v4//:tally",
1113
],
1214
)
1315

@@ -20,5 +22,6 @@ go_test(
2022
"//extension/scorer",
2123
"@com_github_stretchr_testify//assert",
2224
"@com_github_stretchr_testify//require",
25+
"@com_github_uber_go_tally_v4//:tally",
2326
],
2427
)

extension/scorer/composite/scorer.go

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@ package composite
33
import (
44
"context"
55

6+
"github.com/uber-go/tally/v4"
7+
"github.com/uber/submitqueue/core/metrics"
68
"github.com/uber/submitqueue/entity"
79
"github.com/uber/submitqueue/extension/scorer"
810
)
@@ -51,12 +53,14 @@ type compositeScorer struct {
5153
scorers map[string]scorer.Scorer
5254
// reduce combines named scores into a single value.
5355
reduce ReduceFunc
56+
// scope is the tally scope for emitting metrics.
57+
scope tally.Scope
5458
}
5559

5660
// New creates a composite Scorer that evaluates all named child scorers and combines
5761
// their results using the given reduce function.
5862
// Panics if scorers is empty or reduce is nil.
59-
func New(scorers map[string]scorer.Scorer, reduce ReduceFunc) scorer.Scorer {
63+
func New(scorers map[string]scorer.Scorer, reduce ReduceFunc, scope tally.Scope) scorer.Scorer {
6064
if len(scorers) == 0 {
6165
panic("composite.New: scorers must not be empty")
6266
}
@@ -66,12 +70,16 @@ func New(scorers map[string]scorer.Scorer, reduce ReduceFunc) scorer.Scorer {
6670
return &compositeScorer{
6771
scorers: scorers,
6872
reduce: reduce,
73+
scope: scope,
6974
}
7075
}
7176

7277
// Score evaluates all child scorers and combines their results using the reduce function.
7378
// If any child scorer returns an error, that error is returned immediately.
74-
func (c *compositeScorer) Score(ctx context.Context, change entity.Change) (float64, error) {
79+
func (c *compositeScorer) Score(ctx context.Context, change entity.Change) (ret float64, retErr error) {
80+
op := metrics.Begin(c.scope, "score")
81+
defer func() { op.Complete(retErr) }()
82+
7583
scores := make(map[string]float64, len(c.scorers))
7684
for name, s := range c.scorers {
7785
score, err := s.Score(ctx, change)

extension/scorer/composite/scorer_test.go

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77

88
"github.com/stretchr/testify/assert"
99
"github.com/stretchr/testify/require"
10+
"github.com/uber-go/tally/v4"
1011
"github.com/uber/submitqueue/entity"
1112
"github.com/uber/submitqueue/extension/scorer"
1213
)
@@ -83,7 +84,7 @@ func TestScorer_Score(t *testing.T) {
8384

8485
for _, tt := range tests {
8586
t.Run(tt.name, func(t *testing.T) {
86-
s := New(tt.scorers, tt.reduce)
87+
s := New(tt.scorers, tt.reduce, tally.NoopScope)
8788
got, err := s.Score(context.Background(), entity.Change{})
8889
require.NoError(t, err)
8990
assert.InDelta(t, tt.want, got, 1e-9)
@@ -95,20 +96,20 @@ func TestScorer_Score_ChildError(t *testing.T) {
9596
s := New(map[string]scorer.Scorer{
9697
"error": &errorScorer{},
9798
"files": &fixedScorer{0.9},
98-
}, Min)
99+
}, Min, tally.NoopScope)
99100
_, err := s.Score(context.Background(), entity.Change{})
100101
require.Error(t, err)
101102
}
102103

103104
func TestNew_EmptyScorers(t *testing.T) {
104105
assert.Panics(t, func() {
105-
New(map[string]scorer.Scorer{}, Min)
106+
New(map[string]scorer.Scorer{}, Min, tally.NoopScope)
106107
})
107108
}
108109

109110
func TestNew_NilReduce(t *testing.T) {
110111
assert.Panics(t, func() {
111-
New(map[string]scorer.Scorer{"files": &fixedScorer{0.9}}, nil)
112+
New(map[string]scorer.Scorer{"files": &fixedScorer{0.9}}, nil, tally.NoopScope)
112113
})
113114
}
114115

@@ -124,7 +125,7 @@ func TestReduceFunc_ReceivesNames(t *testing.T) {
124125
s := New(map[string]scorer.Scorer{
125126
"files": &fixedScorer{0.9},
126127
"deps": &fixedScorer{0.95},
127-
}, custom)
128+
}, custom, tally.NoopScope)
128129
got, err := s.Score(context.Background(), entity.Change{})
129130
require.NoError(t, err)
130131
assert.Equal(t, 0.9, got)

extension/scorer/heuristic/BUILD.bazel

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,10 @@ go_library(
66
importpath = "github.com/uber/submitqueue/extension/scorer/heuristic",
77
visibility = ["//visibility:public"],
88
deps = [
9+
"//core/metrics",
910
"//entity",
1011
"//extension/scorer",
12+
"@com_github_uber_go_tally_v4//:tally",
1113
],
1214
)
1315

@@ -19,5 +21,6 @@ go_test(
1921
"//entity",
2022
"@com_github_stretchr_testify//assert",
2123
"@com_github_stretchr_testify//require",
24+
"@com_github_uber_go_tally_v4//:tally",
2225
],
2326
)

extension/scorer/heuristic/scorer.go

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ import (
44
"context"
55
"fmt"
66

7+
"github.com/uber-go/tally/v4"
8+
"github.com/uber/submitqueue/core/metrics"
79
"github.com/uber/submitqueue/entity"
810
"github.com/uber/submitqueue/extension/scorer"
911
)
@@ -28,23 +30,28 @@ type heuristicScorer struct {
2830
buckets []Bucket
2931
// valueFunc extracts the numeric value from a Change.
3032
valueFunc ValueFunc
33+
// scope is the tally scope for emitting metrics.
34+
scope tally.Scope
3135
}
3236

3337
// New creates a new heuristic Scorer with the given buckets and value function.
3438
// Panics if valueFunc is nil.
35-
func New(buckets []Bucket, valueFunc ValueFunc) scorer.Scorer {
39+
func New(buckets []Bucket, valueFunc ValueFunc, scope tally.Scope) scorer.Scorer {
3640
if valueFunc == nil {
3741
panic("heuristic.New: valueFunc must not be nil")
3842
}
3943
return &heuristicScorer{
4044
buckets: buckets,
4145
valueFunc: valueFunc,
46+
scope: scope,
4247
}
4348
}
4449

4550
// Score extracts the value from the change, then returns the probability score for the first
4651
// bucket whose range [Min, Max] contains the value. Returns an error if no bucket matches.
47-
func (s *heuristicScorer) Score(ctx context.Context, change entity.Change) (float64, error) {
52+
func (s *heuristicScorer) Score(ctx context.Context, change entity.Change) (ret float64, retErr error) {
53+
op := metrics.Begin(s.scope, "score")
54+
defer func() { op.Complete(retErr) }()
4855
value, err := s.valueFunc(ctx, change)
4956
if err != nil {
5057
return 0, err

extension/scorer/heuristic/scorer_test.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66

77
"github.com/stretchr/testify/assert"
88
"github.com/stretchr/testify/require"
9+
"github.com/uber-go/tally/v4"
910
"github.com/uber/submitqueue/entity"
1011
)
1112

@@ -91,7 +92,7 @@ func TestScorer_Score(t *testing.T) {
9192

9293
for _, tt := range tests {
9394
t.Run(tt.name, func(t *testing.T) {
94-
s := New(tt.buckets, tt.valueFunc)
95+
s := New(tt.buckets, tt.valueFunc, tally.NoopScope)
9596
got, err := s.Score(context.Background(), entity.Change{})
9697
if tt.wantErr {
9798
require.Error(t, err)
@@ -107,13 +108,13 @@ func TestScorer_Score_ValueFuncError(t *testing.T) {
107108
failing := func(_ context.Context, _ entity.Change) (int, error) {
108109
return 0, assert.AnError
109110
}
110-
s := New([]Bucket{{Min: 0, Max: 10, Score: 0.9}}, failing)
111+
s := New([]Bucket{{Min: 0, Max: 10, Score: 0.9}}, failing, tally.NoopScope)
111112
_, err := s.Score(context.Background(), entity.Change{})
112113
require.Error(t, err)
113114
}
114115

115116
func TestNew_NilValueFunc(t *testing.T) {
116117
assert.Panics(t, func() {
117-
New([]Bucket{{Min: 0, Max: 10, Score: 0.85}}, nil)
118+
New([]Bucket{{Min: 0, Max: 10, Score: 0.85}}, nil, tally.NoopScope)
118119
})
119120
}

0 commit comments

Comments
 (0)