From 01262b8ca94ed089380fa47f09f3d857114db76c Mon Sep 17 00:00:00 2001 From: "M. J. Fromberger" Date: Fri, 11 Feb 2022 13:19:56 -0800 Subject: [PATCH] rpc: remove unused latency metric (#7810) We have this one solitary metric from the go-metrics package. In principle this statistic could be useful, but the way we have it hooked up, nothing can observe the value: We don't export it, we don't log it, and it does not auto publish anywhere. Given that this state of affairs has not changed since the metric was first added in 2017 (c08618f), I think we can safely discard it. No one is now or has ever gotten any data out of this metric. --- go.mod | 1 - go.sum | 2 -- rpc/jsonrpc/client/ws_client.go | 28 +++------------------------- rpc/jsonrpc/client/ws_client_test.go | 8 -------- 4 files changed, 3 insertions(+), 36 deletions(-) diff --git a/go.mod b/go.mod index 5e7a24d94..2119ad291 100644 --- a/go.mod +++ b/go.mod @@ -23,7 +23,6 @@ require ( github.com/oasisprotocol/curve25519-voi v0.0.0-20210609091139-0a56a4bca00b github.com/ory/dockertest v3.3.5+incompatible github.com/prometheus/client_golang v1.12.1 - github.com/rcrowley/go-metrics v0.0.0-20200313005456-10cdbea86bc0 github.com/rs/cors v1.8.2 github.com/rs/zerolog v1.26.1 github.com/snikch/goodman v0.0.0-20171125024755-10e37e294daa diff --git a/go.sum b/go.sum index 85daf9da7..3a84f802a 100644 --- a/go.sum +++ b/go.sum @@ -865,8 +865,6 @@ github.com/quasilyte/gogrep v0.0.0-20220103110004-ffaa07af02e3/go.mod h1:wSEyW6O github.com/quasilyte/regex/syntax v0.0.0-20200407221936-30656e2c4a95 h1:L8QM9bvf68pVdQ3bCFZMDmnt9yqcMBro1pC7F+IPYMY= github.com/quasilyte/regex/syntax v0.0.0-20200407221936-30656e2c4a95/go.mod h1:rlzQ04UMyJXu/aOvhd8qT+hvDrFpiwqp8MRXDY9szc0= github.com/rcrowley/go-metrics v0.0.0-20181016184325-3113b8401b8a/go.mod h1:bCqnVzQkZxMG4s8nGwiZ5l3QUCyqpo9Y+/ZMZ9VjZe4= -github.com/rcrowley/go-metrics v0.0.0-20200313005456-10cdbea86bc0 h1:MkV+77GLUNo5oJ0jf870itWm3D0Sjh7+Za9gazKc5LQ= -github.com/rcrowley/go-metrics v0.0.0-20200313005456-10cdbea86bc0/go.mod h1:bCqnVzQkZxMG4s8nGwiZ5l3QUCyqpo9Y+/ZMZ9VjZe4= github.com/rogpeppe/fastuuid v0.0.0-20150106093220-6724a57986af/go.mod h1:XWv6SoW27p1b0cqNHllgS5HIMJraePCO15w5zCzIWYg= github.com/rogpeppe/fastuuid v1.2.0/go.mod h1:jVj6XXZzXRy/MSR5jhDC/2q6DgLz+nrA6LYCDYWNEvQ= github.com/rogpeppe/go-internal v1.3.0/go.mod h1:M8bDsm7K2OlrFYOpmOWEs/qY81heoFRclV5y23lUDJ4= diff --git a/rpc/jsonrpc/client/ws_client.go b/rpc/jsonrpc/client/ws_client.go index cf04e704d..3a626e43a 100644 --- a/rpc/jsonrpc/client/ws_client.go +++ b/rpc/jsonrpc/client/ws_client.go @@ -11,7 +11,6 @@ import ( "time" "github.com/gorilla/websocket" - metrics "github.com/rcrowley/go-metrics" "github.com/tendermint/tendermint/libs/log" rpctypes "github.com/tendermint/tendermint/rpc/jsonrpc/types" @@ -66,10 +65,9 @@ type WSClient struct { // nolint: maligned wg sync.WaitGroup - mtx sync.RWMutex - sentLastPingAt time.Time - reconnecting bool - nextReqID int + mtx sync.RWMutex + reconnecting bool + nextReqID int // sentIDs map[types.JSONRPCIntID]bool // IDs of the requests currently in flight // Time allowed to write a message to the server. 0 means block until operation succeeds. @@ -80,10 +78,6 @@ type WSClient struct { // nolint: maligned // Send pings to server with this period. Must be less than readWait. If 0, no pings will be sent. pingPeriod time.Duration - - // Time between sending a ping and receiving a pong. See - // https://godoc.org/github.com/rcrowley/go-metrics#Timer. - PingPongLatencyTimer metrics.Timer } // NewWS returns a new client with default options. The endpoint argument must @@ -117,8 +111,6 @@ func NewWS(remoteAddr, endpoint string) (*WSClient, error) { // sentIDs: make(map[types.JSONRPCIntID]bool), } - - c.PingPongLatencyTimer = metrics.NewTimer() return c, nil } @@ -165,7 +157,6 @@ func (c *WSClient) Start(ctx context.Context) error { func (c *WSClient) Stop() error { // only close user-facing channels when we can't write to them c.wg.Wait() - c.PingPongLatencyTimer.Stop() close(c.ResponsesCh) return nil } @@ -386,9 +377,6 @@ func (c *WSClient) writeRoutine(ctx context.Context) { c.reconnectAfter <- err return } - c.mtx.Lock() - c.sentLastPingAt = time.Now() - c.mtx.Unlock() case <-c.readRoutineQuit: return case <-ctx.Done(): @@ -411,16 +399,6 @@ func (c *WSClient) readRoutine(ctx context.Context) { c.wg.Done() }() - c.conn.SetPongHandler(func(string) error { - // gather latency stats - c.mtx.RLock() - t := c.sentLastPingAt - c.mtx.RUnlock() - c.PingPongLatencyTimer.UpdateSince(t) - - return nil - }) - for { // reset deadline for every message type (control or data) if c.readWait > 0 { diff --git a/rpc/jsonrpc/client/ws_client_test.go b/rpc/jsonrpc/client/ws_client_test.go index 37bd64b22..5bbb5fc25 100644 --- a/rpc/jsonrpc/client/ws_client_test.go +++ b/rpc/jsonrpc/client/ws_client_test.go @@ -11,20 +11,12 @@ import ( "github.com/fortytw2/leaktest" "github.com/gorilla/websocket" - metrics "github.com/rcrowley/go-metrics" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" rpctypes "github.com/tendermint/tendermint/rpc/jsonrpc/types" ) -func init() { - // Disable go-metrics metrics in tests, since they start unsupervised - // goroutines that trip the leak tester. Calling Stop on the metric is not - // sufficient, as that does not wait for the goroutine. - metrics.UseNilMetrics = true -} - const wsCallTimeout = 5 * time.Second type myTestHandler struct {