Skip to content

Commit 277d7ad

Browse files
arjan-balarjan-bal
committed
Use an empty MetricsRecorderList instead of a NoOpMetricsRecorder in TestClientConn
1 parent 931c661 commit 277d7ad

File tree

9 files changed

+47
-41
lines changed

9 files changed

+47
-41
lines changed

‎balancer/pickfirst/pickfirstleaf/metrics_test.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import (
3030
"google.golang.org/grpc/internal"
3131
"google.golang.org/grpc/internal/stubserver"
3232
"google.golang.org/grpc/internal/testutils"
33+
"google.golang.org/grpc/internal/testutils/stats"
3334
testgrpc "google.golang.org/grpc/interop/grpc_testing"
3435
testpb "google.golang.org/grpc/interop/grpc_testing"
3536
"google.golang.org/grpc/resolver"
@@ -79,7 +80,7 @@ func (s) TestPickFirstMetrics(t *testing.T) {
7980
Addresses: []resolver.Address{{Addr: ss.Address}}},
8081
)
8182

82-
tmr := testutils.NewTestMetricsRecorder()
83+
tmr := stats.NewTestMetricsRecorder()
8384
cc, err := grpc.NewClient(r.Scheme()+":///", grpc.WithStatsHandler(tmr), grpc.WithTransportCredentials(insecure.NewCredentials()), grpc.WithResolvers(r))
8485
if err != nil {
8586
t.Fatalf("NewClient() failed with error: %v", err)
@@ -123,7 +124,7 @@ func (s) TestPickFirstMetricsFailure(t *testing.T) {
123124
Addresses: []resolver.Address{{Addr: "bad address"}}},
124125
)
125126
grpcTarget := r.Scheme() + ":///"
126-
tmr := testutils.NewTestMetricsRecorder()
127+
tmr := stats.NewTestMetricsRecorder()
127128
cc, err := grpc.NewClient(grpcTarget, grpc.WithStatsHandler(tmr), grpc.WithTransportCredentials(insecure.NewCredentials()), grpc.WithResolvers(r))
128129
if err != nil {
129130
t.Fatalf("NewClient() failed with error: %v", err)

‎balancer/pickfirst/pickfirstleaf/pickfirstleaf_ext_test.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ import (
4040
"google.golang.org/grpc/internal/stubserver"
4141
"google.golang.org/grpc/internal/testutils"
4242
"google.golang.org/grpc/internal/testutils/pickfirst"
43+
"google.golang.org/grpc/internal/testutils/stats"
4344
"google.golang.org/grpc/resolver"
4445
"google.golang.org/grpc/resolver/manual"
4546
"google.golang.org/grpc/status"
@@ -884,7 +885,7 @@ func (s) TestPickFirstLeaf_HappyEyeballs_TF_AfterEndOfList(t *testing.T) {
884885
triggerTimer, timeAfter := mockTimer()
885886
pfinternal.TimeAfterFunc = timeAfter
886887

887-
tmr := testutils.NewTestMetricsRecorder()
888+
tmr := stats.NewTestMetricsRecorder()
888889
dialer := testutils.NewBlockingDialer()
889890
opts := []grpc.DialOption{
890891
grpc.WithDefaultServiceConfig(fmt.Sprintf(`{"loadBalancingConfig": [{"%s":{}}]}`, pickfirstleaf.Name)),
@@ -973,7 +974,7 @@ func (s) TestPickFirstLeaf_HappyEyeballs_TriggerConnectionDelay(t *testing.T) {
973974
triggerTimer, timeAfter := mockTimer()
974975
pfinternal.TimeAfterFunc = timeAfter
975976

976-
tmr := testutils.NewTestMetricsRecorder()
977+
tmr := stats.NewTestMetricsRecorder()
977978
dialer := testutils.NewBlockingDialer()
978979
opts := []grpc.DialOption{
979980
grpc.WithDefaultServiceConfig(fmt.Sprintf(`{"loadBalancingConfig": [{"%s":{}}]}`, pickfirstleaf.Name)),
@@ -1033,7 +1034,7 @@ func (s) TestPickFirstLeaf_HappyEyeballs_TF_ThenTimerFires(t *testing.T) {
10331034
triggerTimer, timeAfter := mockTimer()
10341035
pfinternal.TimeAfterFunc = timeAfter
10351036

1036-
tmr := testutils.NewTestMetricsRecorder()
1037+
tmr := stats.NewTestMetricsRecorder()
10371038
dialer := testutils.NewBlockingDialer()
10381039
opts := []grpc.DialOption{
10391040
grpc.WithDefaultServiceConfig(fmt.Sprintf(`{"loadBalancingConfig": [{"%s":{}}]}`, pickfirstleaf.Name)),

‎balancer/rls/cache_test.go

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ import (
2525
"github.com/google/go-cmp/cmp"
2626
"github.com/google/go-cmp/cmp/cmpopts"
2727
"google.golang.org/grpc/internal/backoff"
28-
"google.golang.org/grpc/internal/testutils"
28+
"google.golang.org/grpc/internal/testutils/stats"
2929
)
3030

3131
var (
@@ -120,7 +120,7 @@ func (s) TestLRU_BasicOperations(t *testing.T) {
120120

121121
func (s) TestDataCache_BasicOperations(t *testing.T) {
122122
initCacheEntries()
123-
dc := newDataCache(5, nil, &testutils.NoopMetricsRecorder{}, "")
123+
dc := newDataCache(5, nil, &stats.NoopMetricsRecorder{}, "")
124124
for i, k := range cacheKeys {
125125
dc.addEntry(k, cacheEntries[i])
126126
}
@@ -134,7 +134,7 @@ func (s) TestDataCache_BasicOperations(t *testing.T) {
134134

135135
func (s) TestDataCache_AddForcesResize(t *testing.T) {
136136
initCacheEntries()
137-
dc := newDataCache(1, nil, &testutils.NoopMetricsRecorder{}, "")
137+
dc := newDataCache(1, nil, &stats.NoopMetricsRecorder{}, "")
138138

139139
// The first entry in cacheEntries has a minimum expiry time in the future.
140140
// This entry would stop the resize operation since we do not evict entries
@@ -163,7 +163,7 @@ func (s) TestDataCache_AddForcesResize(t *testing.T) {
163163

164164
func (s) TestDataCache_Resize(t *testing.T) {
165165
initCacheEntries()
166-
dc := newDataCache(5, nil, &testutils.NoopMetricsRecorder{}, "")
166+
dc := newDataCache(5, nil, &stats.NoopMetricsRecorder{}, "")
167167
for i, k := range cacheKeys {
168168
dc.addEntry(k, cacheEntries[i])
169169
}
@@ -194,7 +194,7 @@ func (s) TestDataCache_Resize(t *testing.T) {
194194

195195
func (s) TestDataCache_EvictExpiredEntries(t *testing.T) {
196196
initCacheEntries()
197-
dc := newDataCache(5, nil, &testutils.NoopMetricsRecorder{}, "")
197+
dc := newDataCache(5, nil, &stats.NoopMetricsRecorder{}, "")
198198
for i, k := range cacheKeys {
199199
dc.addEntry(k, cacheEntries[i])
200200
}
@@ -221,7 +221,7 @@ func (s) TestDataCache_ResetBackoffState(t *testing.T) {
221221
}
222222

223223
initCacheEntries()
224-
dc := newDataCache(5, nil, &testutils.NoopMetricsRecorder{}, "")
224+
dc := newDataCache(5, nil, &stats.NoopMetricsRecorder{}, "")
225225
for i, k := range cacheKeys {
226226
dc.addEntry(k, cacheEntries[i])
227227
}
@@ -251,7 +251,7 @@ func (s) TestDataCache_Metrics(t *testing.T) {
251251
{size: 4},
252252
{size: 5},
253253
}
254-
tmr := testutils.NewTestMetricsRecorder()
254+
tmr := stats.NewTestMetricsRecorder()
255255
dc := newDataCache(50, nil, tmr, "")
256256

257257
dc.updateRLSServerTarget("rls-server-target")

‎balancer/rls/picker_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,8 @@ import (
3131
"google.golang.org/grpc/credentials/insecure"
3232
"google.golang.org/grpc/internal/grpcsync"
3333
"google.golang.org/grpc/internal/stubserver"
34-
"google.golang.org/grpc/internal/testutils"
3534
rlstest "google.golang.org/grpc/internal/testutils/rls"
35+
"google.golang.org/grpc/internal/testutils/stats"
3636
"google.golang.org/grpc/metadata"
3737
"google.golang.org/grpc/status"
3838
"google.golang.org/protobuf/types/known/durationpb"
@@ -266,7 +266,7 @@ func (s) Test_RLSDefaultTargetPicksMetric(t *testing.T) {
266266
// Register a manual resolver and push the RLS service config through it.
267267
r := startManualResolverWithConfig(t, rlsConfig)
268268

269-
tmr := testutils.NewTestMetricsRecorder()
269+
tmr := stats.NewTestMetricsRecorder()
270270
cc, err := grpc.NewClient(r.Scheme()+":///", grpc.WithResolvers(r), grpc.WithTransportCredentials(insecure.NewCredentials()), grpc.WithStatsHandler(tmr))
271271
if err != nil {
272272
t.Fatalf("grpc.NewClient() failed: %v", err)
@@ -312,7 +312,7 @@ func (s) Test_RLSTargetPicksMetric(t *testing.T) {
312312
// Register a manual resolver and push the RLS service config through it.
313313
r := startManualResolverWithConfig(t, rlsConfig)
314314

315-
tmr := testutils.NewTestMetricsRecorder()
315+
tmr := stats.NewTestMetricsRecorder()
316316
// Dial the backend.
317317
cc, err := grpc.NewClient(r.Scheme()+":///", grpc.WithResolvers(r), grpc.WithTransportCredentials(insecure.NewCredentials()), grpc.WithStatsHandler(tmr))
318318
if err != nil {
@@ -350,7 +350,7 @@ func (s) Test_RLSFailedPicksMetric(t *testing.T) {
350350
// Register a manual resolver and push the RLS service config through it.
351351
r := startManualResolverWithConfig(t, rlsConfig)
352352

353-
tmr := testutils.NewTestMetricsRecorder()
353+
tmr := stats.NewTestMetricsRecorder()
354354
// Dial the backend.
355355
cc, err := grpc.NewClient(r.Scheme()+":///", grpc.WithResolvers(r), grpc.WithTransportCredentials(insecure.NewCredentials()), grpc.WithStatsHandler(tmr))
356356
if err != nil {

‎balancer/weightedroundrobin/balancer_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,8 @@ import (
3131
"google.golang.org/grpc/internal"
3232
"google.golang.org/grpc/internal/grpctest"
3333
"google.golang.org/grpc/internal/stubserver"
34-
"google.golang.org/grpc/internal/testutils"
3534
"google.golang.org/grpc/internal/testutils/roundrobin"
35+
"google.golang.org/grpc/internal/testutils/stats"
3636
"google.golang.org/grpc/orca"
3737
"google.golang.org/grpc/peer"
3838
"google.golang.org/grpc/resolver"
@@ -224,7 +224,7 @@ func (s) TestWRRMetricsBasic(t *testing.T) {
224224
srv := startServer(t, reportCall)
225225
sc := svcConfig(t, testMetricsConfig)
226226

227-
tmr := testutils.NewTestMetricsRecorder()
227+
tmr := stats.NewTestMetricsRecorder()
228228
if err := srv.StartClient(grpc.WithDefaultServiceConfig(sc), grpc.WithStatsHandler(tmr)); err != nil {
229229
t.Fatalf("Error starting client: %v", err)
230230
}

‎balancer/weightedroundrobin/metrics_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ import (
2424

2525
"google.golang.org/grpc/internal/grpctest"
2626
iserviceconfig "google.golang.org/grpc/internal/serviceconfig"
27-
"google.golang.org/grpc/internal/testutils"
27+
"google.golang.org/grpc/internal/testutils/stats"
2828
)
2929

3030
type s struct {
@@ -108,7 +108,7 @@ func (s) TestWRR_Metrics_SubConnWeight(t *testing.T) {
108108

109109
for _, test := range tests {
110110
t.Run(test.name, func(t *testing.T) {
111-
tmr := testutils.NewTestMetricsRecorder()
111+
tmr := stats.NewTestMetricsRecorder()
112112
wsc := &endpointWeight{
113113
metricsRecorder: tmr,
114114
weightVal: 3,
@@ -136,7 +136,7 @@ func (s) TestWRR_Metrics_SubConnWeight(t *testing.T) {
136136
// with no weights. Both of these should emit a count metric for round robin
137137
// fallback.
138138
func (s) TestWRR_Metrics_Scheduler_RR_Fallback(t *testing.T) {
139-
tmr := testutils.NewTestMetricsRecorder()
139+
tmr := stats.NewTestMetricsRecorder()
140140
ew := &endpointWeight{
141141
metricsRecorder: tmr,
142142
weightVal: 0,

‎internal/stats/metrics_recorder_list_test.go

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ import (
3636
"google.golang.org/grpc/internal"
3737
"google.golang.org/grpc/internal/grpctest"
3838
istats "google.golang.org/grpc/internal/stats"
39-
"google.golang.org/grpc/internal/testutils"
39+
"google.golang.org/grpc/internal/testutils/stats"
4040
testgrpc "google.golang.org/grpc/interop/grpc_testing"
4141
testpb "google.golang.org/grpc/interop/grpc_testing"
4242
"google.golang.org/grpc/resolver"
@@ -146,8 +146,8 @@ func (s) TestMetricsRecorderList(t *testing.T) {
146146

147147
// Create two stats.Handlers which also implement MetricsRecorder, configure
148148
// one as a global dial option and one as a local dial option.
149-
mr1 := testutils.NewTestMetricsRecorder()
150-
mr2 := testutils.NewTestMetricsRecorder()
149+
mr1 := stats.NewTestMetricsRecorder()
150+
mr2 := stats.NewTestMetricsRecorder()
151151

152152
defer internal.ClearGlobalDialOptions()
153153
internal.AddGlobalDialOptions.(func(opt ...grpc.DialOption))(grpc.WithStatsHandler(mr1))
@@ -166,7 +166,7 @@ func (s) TestMetricsRecorderList(t *testing.T) {
166166
// to record.
167167
tsc.UnaryCall(ctx, &testpb.SimpleRequest{})
168168

169-
mdWant := testutils.MetricsData{
169+
mdWant := stats.MetricsData{
170170
Handle: intCountHandle.Descriptor(),
171171
IntIncr: 1,
172172
LabelKeys: []string{"int counter label", "int counter optional label"},
@@ -179,7 +179,7 @@ func (s) TestMetricsRecorderList(t *testing.T) {
179179
t.Fatal(err.Error())
180180
}
181181

182-
mdWant = testutils.MetricsData{
182+
mdWant = stats.MetricsData{
183183
Handle: floatCountHandle.Descriptor(),
184184
FloatIncr: 2,
185185
LabelKeys: []string{"float counter label", "float counter optional label"},
@@ -192,7 +192,7 @@ func (s) TestMetricsRecorderList(t *testing.T) {
192192
t.Fatal(err.Error())
193193
}
194194

195-
mdWant = testutils.MetricsData{
195+
mdWant = stats.MetricsData{
196196
Handle: intHistoHandle.Descriptor(),
197197
IntIncr: 3,
198198
LabelKeys: []string{"int histo label", "int histo optional label"},
@@ -205,7 +205,7 @@ func (s) TestMetricsRecorderList(t *testing.T) {
205205
t.Fatal(err.Error())
206206
}
207207

208-
mdWant = testutils.MetricsData{
208+
mdWant = stats.MetricsData{
209209
Handle: floatHistoHandle.Descriptor(),
210210
FloatIncr: 4,
211211
LabelKeys: []string{"float histo label", "float histo optional label"},
@@ -217,7 +217,7 @@ func (s) TestMetricsRecorderList(t *testing.T) {
217217
if err := mr2.WaitForFloat64Histo(ctx, mdWant); err != nil {
218218
t.Fatal(err.Error())
219219
}
220-
mdWant = testutils.MetricsData{
220+
mdWant = stats.MetricsData{
221221
Handle: intGaugeHandle.Descriptor(),
222222
IntIncr: 5,
223223
LabelKeys: []string{"int gauge label", "int gauge optional label"},

‎internal/testutils/balancer.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@ import (
2929
"google.golang.org/grpc/experimental/stats"
3030
"google.golang.org/grpc/internal/grpcsync"
3131
"google.golang.org/grpc/resolver"
32+
33+
istats "google.golang.org/grpc/internal/stats"
3234
)
3335

3436
// TestSubConn implements the SubConn interface, to be used in tests.
@@ -156,7 +158,7 @@ func (tcc *BalancerClientConn) NewSubConn(a []resolver.Address, o balancer.NewSu
156158

157159
// MetricsRecorder returns a no-op MetricsRecorder.
158160
func (*BalancerClientConn) MetricsRecorder() stats.MetricsRecorder {
159-
return &NoopMetricsRecorder{}
161+
return istats.NewMetricsRecorderList(nil)
160162
}
161163

162164
// RemoveSubConn is a nop; tests should all be updated to use sc.Shutdown()

‎internal/testutils/test_metrics_recorder.go renamed to ‎internal/testutils/stats/test_metrics_recorder.go

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,8 @@
1616
*
1717
*/
1818

19-
package testutils
19+
// Package stats implements a TestMetricsRecorder utility.
20+
package stats
2021

2122
import (
2223
"context"
@@ -25,6 +26,7 @@ import (
2526

2627
"github.com/google/go-cmp/cmp"
2728
estats "google.golang.org/grpc/experimental/stats"
29+
"google.golang.org/grpc/internal/testutils"
2830
"google.golang.org/grpc/stats"
2931
)
3032

@@ -33,11 +35,11 @@ import (
3335
// have taken place. It also persists metrics data keyed on the metrics
3436
// descriptor.
3537
type TestMetricsRecorder struct {
36-
intCountCh *Channel
37-
floatCountCh *Channel
38-
intHistoCh *Channel
39-
floatHistoCh *Channel
40-
intGaugeCh *Channel
38+
intCountCh *testutils.Channel
39+
floatCountCh *testutils.Channel
40+
intHistoCh *testutils.Channel
41+
floatHistoCh *testutils.Channel
42+
intGaugeCh *testutils.Channel
4143

4244
// mu protects data.
4345
mu sync.Mutex
@@ -48,11 +50,11 @@ type TestMetricsRecorder struct {
4850
// NewTestMetricsRecorder returns a new TestMetricsRecorder.
4951
func NewTestMetricsRecorder() *TestMetricsRecorder {
5052
return &TestMetricsRecorder{
51-
intCountCh: NewChannelWithSize(10),
52-
floatCountCh: NewChannelWithSize(10),
53-
intHistoCh: NewChannelWithSize(10),
54-
floatHistoCh: NewChannelWithSize(10),
55-
intGaugeCh: NewChannelWithSize(10),
53+
intCountCh: testutils.NewChannelWithSize(10),
54+
floatCountCh: testutils.NewChannelWithSize(10),
55+
intHistoCh: testutils.NewChannelWithSize(10),
56+
floatHistoCh: testutils.NewChannelWithSize(10),
57+
intGaugeCh: testutils.NewChannelWithSize(10),
5658

5759
data: make(map[string]float64),
5860
}

0 commit comments

Comments
 (0)