Skip to content

Commit 319fd52

Browse files
suj-krishnansuj-krishnan
committed
fixup! *: plug in no-op metric implementations for nil fields
1 parent 6f017ef commit 319fd52

6 files changed

Lines changed: 55 additions & 49 deletions

File tree

‎drpcclient/dialoptions.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,6 @@ func DialContext(ctx context.Context, address string, opts ...DialOption) (conn
144144
},
145145
SoftCancel: false,
146146
},
147-
Metrics: options.metrics,
147+
Metrics: *options.metrics,
148148
}), nil
149149
}

‎drpcconn/conn.go‎

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ type Options struct {
3131

3232
// Metrics holds optional metrics the client will populate. If nil, no
3333
// metrics are recorded.
34-
Metrics *drpcmetrics.ClientMetrics
34+
Metrics drpcmetrics.ClientMetrics
3535
}
3636

3737
// Conn is a drpc client connection.
@@ -41,8 +41,7 @@ type Conn struct {
4141
mu sync.Mutex
4242
wbuf []byte
4343

44-
stats map[string]*drpcstats.Stats // TODO (server): deprecate stats
45-
metrics drpcmetrics.ClientMetrics
44+
stats map[string]*drpcstats.Stats // TODO (server): deprecate stats
4645
}
4746

4847
var _ drpc.Conn = (*Conn)(nil)
@@ -57,7 +56,8 @@ func NewWithOptions(tr drpc.Transport, opts Options) *Conn {
5756
tr: tr,
5857
}
5958

60-
c.initClientMetrics(opts.Metrics, tr)
59+
c.tr = drpcmetrics.ToMeteredTransport(tr, opts.Metrics.BytesSent,
60+
opts.Metrics.BytesRecv)
6161

6262
if opts.CollectStats {
6363
drpcopts.SetManagerStatsCB(&opts.Manager.Internal, c.getStats)
@@ -69,17 +69,6 @@ func NewWithOptions(tr drpc.Transport, opts Options) *Conn {
6969
return c
7070
}
7171

72-
// initClientMetrics copies the caller-supplied metrics into the conn and
73-
// wraps the transport with a metered transport if metrics are provided.
74-
func (c *Conn) initClientMetrics(metrics *drpcmetrics.ClientMetrics, tr drpc.Transport) {
75-
if metrics != nil {
76-
c.metrics = *metrics
77-
mt := drpcmetrics.NewMeteredTransport(tr, c.metrics.BytesSent,
78-
c.metrics.BytesRecv)
79-
c.tr = mt
80-
}
81-
}
82-
8372
// Stats returns the collected stats grouped by rpc.
8473
func (c *Conn) Stats() map[string]drpcstats.Stats {
8574
c.mu.Lock()
@@ -121,7 +110,9 @@ func (c *Conn) Close() (err error) { return c.man.Close() }
121110

122111
// Invoke issues the rpc on the transport serializing in, waits for a response, and
123112
// deserializes it into out. Only one Invoke or Stream may be open at a time.
124-
func (c *Conn) Invoke(ctx context.Context, rpc string, enc drpc.Encoding, in, out drpc.Message) (err error) {
113+
func (c *Conn) Invoke(
114+
ctx context.Context, rpc string, enc drpc.Encoding, in, out drpc.Message,
115+
) (err error) {
125116
defer func() { err = drpc.ToRPCErr(err) }()
126117

127118
var metadata []byte
@@ -153,7 +144,14 @@ func (c *Conn) Invoke(ctx context.Context, rpc string, enc drpc.Encoding, in, ou
153144
return nil
154145
}
155146

156-
func (c *Conn) doInvoke(stream *drpcstream.Stream, enc drpc.Encoding, rpc string, data []byte, metadata []byte, out drpc.Message) (err error) {
147+
func (c *Conn) doInvoke(
148+
stream *drpcstream.Stream,
149+
enc drpc.Encoding,
150+
rpc string,
151+
data []byte,
152+
metadata []byte,
153+
out drpc.Message,
154+
) (err error) {
157155
if len(metadata) > 0 {
158156
if err := stream.RawWrite(drpcwire.KindInvokeMetadata, metadata); err != nil {
159157
return err
@@ -176,7 +174,9 @@ func (c *Conn) doInvoke(stream *drpcstream.Stream, enc drpc.Encoding, rpc string
176174

177175
// NewStream begins a streaming rpc on the connection. Only one Invoke or Stream may
178176
// be open at a time.
179-
func (c *Conn) NewStream(ctx context.Context, rpc string, enc drpc.Encoding) (_ drpc.Stream, err error) {
177+
func (c *Conn) NewStream(
178+
ctx context.Context, rpc string, enc drpc.Encoding,
179+
) (_ drpc.Stream, err error) {
180180
defer func() { err = drpc.ToRPCErr(err) }()
181181

182182
var metadata []byte

‎drpcmetrics/metrics.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,10 +60,10 @@ type meteredTransport struct {
6060
bytesRecv Counter
6161
}
6262

63-
// NewMeteredTransport returns a transport that increments bytesSent and
63+
// ToMeteredTransport returns a transport that increments bytesSent and
6464
// bytesRecv on each Write and Read call respectively. Nil counters are
6565
// replaced with no-op implementations.
66-
func NewMeteredTransport(tr drpc.Transport, bytesSent,
66+
func ToMeteredTransport(tr drpc.Transport, bytesSent,
6767
bytesRecv Counter) drpc.Transport {
6868
if bytesSent == nil {
6969
bytesSent = NoOpCounter{}

‎drpcpool/pool.go‎

Lines changed: 23 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,6 @@ type Pool[K comparable, V Conn] struct {
5353
mu sync.Mutex
5454
entries map[K]*list[K, V]
5555
order list[K, V]
56-
metrics PoolMetrics
5756
}
5857

5958
// New constructs a new Pool with the provided Options.
@@ -62,7 +61,7 @@ func New[K comparable, V Conn](opts Options) *Pool[K, V] {
6261
opts: opts,
6362
entries: make(map[K]*list[K, V]),
6463
}
65-
pool.initPoolMetrics(opts.Metrics)
64+
pool.initPoolMetrics()
6665

6766
// emit the metric (0 value) so it shows up as soon as the pool is created
6867
pool.updatePoolSize()
@@ -71,31 +70,41 @@ func New[K comparable, V Conn](opts Options) *Pool[K, V] {
7170

7271
// initPoolMetrics copies the caller-supplied metrics into the pool,
7372
// substituting no-op implementations for any nil fields.
74-
func (p *Pool[K, V]) initPoolMetrics(metrics *PoolMetrics) {
75-
if metrics != nil {
76-
p.metrics = *metrics
73+
func (p *Pool[K, V]) initPoolMetrics() {
74+
metrics := p.opts.Metrics
75+
if metrics == nil {
76+
return
7777
}
78-
if p.metrics.PoolSize == nil {
79-
p.metrics.PoolSize = drpcmetrics.NoOpGauge{}
78+
if metrics.PoolSize == nil {
79+
metrics.PoolSize = drpcmetrics.NoOpGauge{}
8080
}
81-
if p.metrics.ConnectionHitsTotal == nil {
82-
p.metrics.ConnectionHitsTotal = drpcmetrics.NoOpLabeledCounter{}
81+
if metrics.ConnectionHitsTotal == nil {
82+
metrics.ConnectionHitsTotal = drpcmetrics.NoOpLabeledCounter{}
8383
}
84-
if p.metrics.ConnectionMissesTotal == nil {
85-
p.metrics.ConnectionMissesTotal = drpcmetrics.NoOpLabeledCounter{}
84+
if metrics.ConnectionMissesTotal == nil {
85+
metrics.ConnectionMissesTotal = drpcmetrics.NoOpLabeledCounter{}
8686
}
8787
}
8888

8989
func (p *Pool[K, V]) recordHit() {
90-
p.metrics.ConnectionHitsTotal.Inc(p.opts.Labels, 1)
90+
if p.opts.Metrics == nil {
91+
return
92+
}
93+
p.opts.Metrics.ConnectionHitsTotal.Inc(p.opts.Labels, 1)
9194
}
9295

9396
func (p *Pool[K, V]) recordMiss() {
94-
p.metrics.ConnectionMissesTotal.Inc(p.opts.Labels, 1)
97+
if p.opts.Metrics == nil {
98+
return
99+
}
100+
p.opts.Metrics.ConnectionMissesTotal.Inc(p.opts.Labels, 1)
95101
}
96102

97103
func (p *Pool[K, V]) updatePoolSize() {
98-
p.metrics.PoolSize.Update(p.opts.Labels, int64(p.order.count))
104+
if p.opts.Metrics == nil {
105+
return
106+
}
107+
p.opts.Metrics.PoolSize.Update(p.opts.Labels, int64(p.order.count))
99108
}
100109

101110
func (p *Pool[K, V]) log(what string, cb func() string) {

‎drpcserver/server.go‎

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ type Options struct {
4848

4949
// Metrics holds optional metrics the server will populate. If nil, no
5050
// metrics are recorded.
51-
Metrics *ServerMetrics
51+
Metrics ServerMetrics
5252
}
5353

5454
// ServerMetrics holds optional metrics that the server will populate during
@@ -61,17 +61,16 @@ type ServerMetrics struct {
6161

6262
// addTLSHandshakeError increments the TLS handshake error counter.
6363
func (s *Server) addTLSHandshakeError() {
64-
s.metrics.TLSHandshakeErrors.Inc(1)
64+
s.opts.Metrics.TLSHandshakeErrors.Inc(1)
6565
}
6666

6767
// Server is an implementation of drpc.Server to serve drpc connections.
6868
type Server struct {
6969
opts Options
7070
handler drpc.Handler
7171

72-
mu sync.Mutex
73-
stats map[string]*drpcstats.Stats
74-
metrics ServerMetrics
72+
mu sync.Mutex
73+
stats map[string]*drpcstats.Stats
7574
}
7675

7776
// New constructs a new Server.
@@ -97,12 +96,10 @@ func NewWithOptions(handler drpc.Handler, opts Options) *Server {
9796
drpcopts.SetManagerStatsCB(&s.opts.Manager.Internal, s.getStats)
9897
s.stats = make(map[string]*drpcstats.Stats)
9998
}
100-
if s.opts.Metrics != nil {
101-
s.metrics = *s.opts.Metrics
102-
if s.metrics.TLSHandshakeErrors == nil {
103-
s.metrics.TLSHandshakeErrors = drpcmetrics.NoOpCounter{}
104-
}
99+
if s.opts.Metrics.TLSHandshakeErrors == nil {
100+
s.opts.Metrics.TLSHandshakeErrors = drpcmetrics.NoOpCounter{}
105101
}
102+
106103
return s
107104
}
108105

‎internal/integration/metrics_test.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ func (c *testCounter) count() int {
5656
//
5757

5858
func createMeteredClientConnection(
59-
t testing.TB, server DRPCServiceServer, metrics *drpcmetrics.ClientMetrics,
59+
t testing.TB, server DRPCServiceServer, metrics drpcmetrics.ClientMetrics,
6060
) (DRPCServiceClient, func()) {
6161
ctx := drpctest.NewTracker(t)
6262
c1, c2 := net.Pipe()
@@ -84,7 +84,7 @@ func TestClientByteMetrics(t *testing.T) {
8484

8585
sent := &testCounter{}
8686
recv := &testCounter{}
87-
cli, close := createMeteredClientConnection(t, standardImpl, &drpcmetrics.ClientMetrics{
87+
cli, close := createMeteredClientConnection(t, standardImpl, drpcmetrics.ClientMetrics{
8888
BytesSent: sent,
8989
BytesRecv: recv,
9090
})
@@ -121,7 +121,7 @@ func TestClientByteMetricsPartialNil(t *testing.T) {
121121
defer ctx.Close()
122122

123123
sent := &testCounter{}
124-
cli, close := createMeteredClientConnection(t, standardImpl, &drpcmetrics.ClientMetrics{
124+
cli, close := createMeteredClientConnection(t, standardImpl, drpcmetrics.ClientMetrics{
125125
BytesSent: sent,
126126
// BytesRecv intentionally nil.
127127
})
@@ -139,7 +139,7 @@ func TestServerTLSHandshakeErrorMetric(t *testing.T) {
139139
mux := drpcmux.New()
140140
assert.NoError(t, DRPCRegisterService(mux, standardImpl))
141141
srv := drpcserver.NewWithOptions(mux, drpcserver.Options{
142-
Metrics: &drpcserver.ServerMetrics{
142+
Metrics: drpcserver.ServerMetrics{
143143
TLSHandshakeErrors: tlsErrors,
144144
},
145145
})

0 commit comments

Comments
 (0)