logging: remove reamining instances of SetLogger interface (#7572)

This commit is contained in:
Sam Kleinman
2022-01-12 16:56:49 -05:00
committed by GitHub
parent 7a9a38d9d7
commit 2a348cc1e9
21 changed files with 103 additions and 89 deletions
+2 -7
View File
@@ -51,25 +51,20 @@ type NodeService interface {
}
// New configures a client that calls the Node directly.
func New(node NodeService) (*Local, error) {
func New(logger log.Logger, node NodeService) (*Local, error) {
env := node.RPCEnvironment()
if env == nil {
return nil, errors.New("rpc is nil")
}
return &Local{
EventBus: node.EventBus(),
Logger: log.NewNopLogger(),
Logger: logger,
env: env,
}, nil
}
var _ rpcclient.Client = (*Local)(nil)
// SetLogger allows to set a logger on the client.
func (c *Local) SetLogger(l log.Logger) {
c.Logger = l
}
func (c *Local) Status(ctx context.Context) (*coretypes.ResultStatus, error) {
return c.env.Status(ctx)
}
+3 -2
View File
@@ -10,11 +10,12 @@ import (
"github.com/stretchr/testify/require"
"github.com/tendermint/tendermint/abci/example/kvstore"
"github.com/tendermint/tendermint/config"
"github.com/tendermint/tendermint/libs/log"
"github.com/tendermint/tendermint/libs/service"
rpctest "github.com/tendermint/tendermint/rpc/test"
)
func NodeSuite(t *testing.T) (service.Service, *config.Config) {
func NodeSuite(t *testing.T, logger log.Logger) (service.Service, *config.Config) {
t.Helper()
ctx, cancel := context.WithCancel(context.Background())
@@ -26,7 +27,7 @@ func NodeSuite(t *testing.T) (service.Service, *config.Config) {
dir, err := os.MkdirTemp("/tmp", fmt.Sprint("rpc-client-test-", t.Name()))
require.NoError(t, err)
app := kvstore.NewPersistentKVStoreApplication(dir)
app := kvstore.NewPersistentKVStoreApplication(logger, dir)
node, closer, err := rpctest.StartTendermint(ctx, conf, app, rpctest.SuppressStdout)
require.NoError(t, err)
+41 -18
View File
@@ -34,14 +34,14 @@ import (
"github.com/tendermint/tendermint/types"
)
func getHTTPClient(t *testing.T, conf *config.Config) *rpchttp.HTTP {
func getHTTPClient(t *testing.T, logger log.Logger, conf *config.Config) *rpchttp.HTTP {
t.Helper()
rpcAddr := conf.RPC.ListenAddress
c, err := rpchttp.NewWithClient(rpcAddr, http.DefaultClient)
require.NoError(t, err)
c.Logger = log.NewTestingLogger(t)
c.Logger = logger
t.Cleanup(func() {
if c.IsRunning() {
require.NoError(t, c.Stop())
@@ -51,7 +51,7 @@ func getHTTPClient(t *testing.T, conf *config.Config) *rpchttp.HTTP {
return c
}
func getHTTPClientWithTimeout(t *testing.T, conf *config.Config, timeout time.Duration) *rpchttp.HTTP {
func getHTTPClientWithTimeout(t *testing.T, logger log.Logger, conf *config.Config, timeout time.Duration) *rpchttp.HTTP {
t.Helper()
rpcAddr := conf.RPC.ListenAddress
@@ -60,7 +60,7 @@ func getHTTPClientWithTimeout(t *testing.T, conf *config.Config, timeout time.Du
c, err := rpchttp.NewWithClient(rpcAddr, http.DefaultClient)
require.NoError(t, err)
c.Logger = log.NewTestingLogger(t)
c.Logger = logger
t.Cleanup(func() {
http.DefaultClient.Timeout = 0
if c.IsRunning() {
@@ -78,12 +78,13 @@ func GetClients(t *testing.T, ns service.Service, conf *config.Config) []client.
node, ok := ns.(rpclocal.NodeService)
require.True(t, ok)
ncl, err := rpclocal.New(node)
logger := log.NewTestingLogger(t)
ncl, err := rpclocal.New(logger, node)
require.NoError(t, err)
return []client.Client{
ncl,
getHTTPClient(t, conf),
getHTTPClient(t, logger, conf),
}
}
@@ -91,7 +92,9 @@ func TestClientOperations(t *testing.T) {
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
_, conf := NodeSuite(t)
logger := log.NewTestingLogger(t)
_, conf := NodeSuite(t, logger)
t.Run("NilCustomHTTPClient", func(t *testing.T) {
_, err := rpchttp.NewWithClient("http://example.com", nil)
@@ -129,14 +132,16 @@ func TestClientOperations(t *testing.T) {
})
t.Run("Batching", func(t *testing.T) {
t.Run("JSONRPCCalls", func(t *testing.T) {
c := getHTTPClient(t, conf)
logger := log.NewTestingLogger(t)
c := getHTTPClient(t, logger, conf)
testBatchedJSONRPCCalls(ctx, t, c)
})
t.Run("JSONRPCCallsCancellation", func(t *testing.T) {
_, _, tx1 := MakeTxKV()
_, _, tx2 := MakeTxKV()
c := getHTTPClient(t, conf)
logger := log.NewTestingLogger(t)
c := getHTTPClient(t, logger, conf)
batch := c.NewBatch()
_, err := batch.BroadcastTxCommit(ctx, tx1)
require.NoError(t, err)
@@ -150,19 +155,25 @@ func TestClientOperations(t *testing.T) {
require.Equal(t, 0, batch.Count())
})
t.Run("SendingEmptyRequest", func(t *testing.T) {
c := getHTTPClient(t, conf)
logger := log.NewTestingLogger(t)
c := getHTTPClient(t, logger, conf)
batch := c.NewBatch()
_, err := batch.Send(ctx)
require.Error(t, err, "sending an empty batch of JSON RPC requests should result in an error")
})
t.Run("ClearingEmptyRequest", func(t *testing.T) {
c := getHTTPClient(t, conf)
logger := log.NewTestingLogger(t)
c := getHTTPClient(t, logger, conf)
batch := c.NewBatch()
require.Zero(t, batch.Clear(), "clearing an empty batch of JSON RPC requests should result in a 0 result")
})
t.Run("ConcurrentJSONRPC", func(t *testing.T) {
logger := log.NewTestingLogger(t)
var wg sync.WaitGroup
c := getHTTPClient(t, conf)
c := getHTTPClient(t, logger, conf)
for i := 0; i < 50; i++ {
wg.Add(1)
go func() {
@@ -174,7 +185,9 @@ func TestClientOperations(t *testing.T) {
})
})
t.Run("HTTPReturnsErrorIfClientIsNotRunning", func(t *testing.T) {
c := getHTTPClientWithTimeout(t, conf, 100*time.Millisecond)
logger := log.NewTestingLogger(t)
c := getHTTPClientWithTimeout(t, logger, conf, 100*time.Millisecond)
// on Subscribe
_, err := c.Subscribe(ctx, "TestHeaderEvents",
@@ -196,7 +209,9 @@ func TestClientOperations(t *testing.T) {
func TestClientMethodCalls(t *testing.T) {
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
n, conf := NodeSuite(t)
logger := log.NewTestingLogger(t)
n, conf := NodeSuite(t, logger)
// for broadcast tx tests
pool := getMempool(t, n)
@@ -591,7 +606,9 @@ func TestClientMethodCallsAdvanced(t *testing.T) {
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
n, conf := NodeSuite(t)
logger := log.NewTestingLogger(t)
n, conf := NodeSuite(t, logger)
pool := getMempool(t, n)
t.Run("UnconfirmedTxs", func(t *testing.T) {
@@ -654,7 +671,9 @@ func TestClientMethodCallsAdvanced(t *testing.T) {
pool.Flush()
})
t.Run("Tx", func(t *testing.T) {
c := getHTTPClient(t, conf)
logger := log.NewTestingLogger(t)
c := getHTTPClient(t, logger, conf)
// first we broadcast a tx
_, _, tx := MakeTxKV()
@@ -710,7 +729,9 @@ func TestClientMethodCallsAdvanced(t *testing.T) {
}
})
t.Run("TxSearchWithTimeout", func(t *testing.T) {
timeoutClient := getHTTPClientWithTimeout(t, conf, 10*time.Second)
logger := log.NewTestingLogger(t)
timeoutClient := getHTTPClientWithTimeout(t, logger, conf, 10*time.Second)
_, _, tx := MakeTxKV()
_, err := timeoutClient.BroadcastTxCommit(ctx, tx)
@@ -723,7 +744,9 @@ func TestClientMethodCallsAdvanced(t *testing.T) {
})
t.Run("TxSearch", func(t *testing.T) {
t.Skip("Test Asserts Non-Deterministic Results")
c := getHTTPClient(t, conf)
logger := log.NewTestingLogger(t)
c := getHTTPClient(t, logger, conf)
// first we broadcast a few txs
for i := 0; i < 10; i++ {
+6 -6
View File
@@ -8,6 +8,7 @@ package client
import (
"bytes"
"context"
"errors"
"net"
"regexp"
@@ -15,34 +16,33 @@ import (
"time"
"github.com/stretchr/testify/require"
"github.com/tendermint/tendermint/libs/log"
)
func TestWSClientReconnectWithJitter(t *testing.T) {
n := 8
maxReconnectAttempts := 3
var maxReconnectAttempts uint = 3
// Max wait time is ceil(1+0.999) + ceil(2+0.999) + ceil(4+0.999) + ceil(...) = 2 + 3 + 5 = 10s + ...
maxSleepTime := time.Second * time.Duration(((1<<uint(maxReconnectAttempts))-1)+maxReconnectAttempts)
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
var errNotConnected = errors.New("not connected")
clientMap := make(map[int]*WSClient)
buf := new(bytes.Buffer)
logger := log.NewTMLogger(buf)
for i := 0; i < n; i++ {
c, err := NewWS("tcp://foo", "/websocket")
require.NoError(t, err)
c.Dialer = func(string, string) (net.Conn, error) {
return nil, errNotConnected
}
c.SetLogger(logger)
c.maxReconnectAttempts = maxReconnectAttempts
// Not invoking defer c.Stop() because
// after all the reconnect attempts have been
// exhausted, c.Stop is implicitly invoked.
clientMap[i] = c
// Trigger the reconnect routine that performs exponential backoff.
go c.reconnect()
go c.reconnect(ctx)
}
stopCount := 0
+2 -4
View File
@@ -107,8 +107,7 @@ func setup(ctx context.Context) error {
tcpLogger := logger.With("socket", "tcp")
mux := http.NewServeMux()
server.RegisterRPCFuncs(mux, Routes, tcpLogger)
wm := server.NewWebsocketManager(Routes, server.ReadWait(5*time.Second), server.PingPeriod(1*time.Second))
wm.SetLogger(tcpLogger)
wm := server.NewWebsocketManager(tcpLogger, Routes, server.ReadWait(5*time.Second), server.PingPeriod(1*time.Second))
mux.HandleFunc(websocketEndpoint, wm.WebsocketHandler)
config := server.DefaultConfig()
listener1, err := server.Listen(tcpAddr, config.MaxOpenConnections)
@@ -124,8 +123,7 @@ func setup(ctx context.Context) error {
unixLogger := logger.With("socket", "unix")
mux2 := http.NewServeMux()
server.RegisterRPCFuncs(mux2, Routes, unixLogger)
wm = server.NewWebsocketManager(Routes)
wm.SetLogger(unixLogger)
wm = server.NewWebsocketManager(unixLogger, Routes)
mux2.HandleFunc(websocketEndpoint, wm.WebsocketHandler)
listener2, err := server.Listen(unixAddr, config.MaxOpenConnections)
if err != nil {
+2 -6
View File
@@ -41,6 +41,7 @@ type WebsocketManager struct {
// NewWebsocketManager returns a new WebsocketManager that passes a map of
// functions, connection options and logger to new WS connections.
func NewWebsocketManager(
logger log.Logger,
funcMap map[string]*RPCFunc,
wsConnOptions ...func(*wsConnection),
) *WebsocketManager {
@@ -60,16 +61,11 @@ func NewWebsocketManager(
return true
},
},
logger: log.NewNopLogger(),
logger: logger,
wsConnOptions: wsConnOptions,
}
}
// SetLogger sets the logger.
func (wm *WebsocketManager) SetLogger(l log.Logger) {
wm.logger = l
}
// WebsocketHandler upgrades the request/response (via http.Hijack) and starts
// the wsConnection.
func (wm *WebsocketManager) WebsocketHandler(w http.ResponseWriter, r *http.Request) {
+1 -3
View File
@@ -49,9 +49,7 @@ func newWSServer(t *testing.T, logger log.Logger) *httptest.Server {
funcMap := map[string]*RPCFunc{
"c": NewWSRPCFunc(func(ctx context.Context, s string, i int) (string, error) { return "foo", nil }, "s,i"),
}
wm := NewWebsocketManager(funcMap)
wm.SetLogger(logger)
wm := NewWebsocketManager(logger, funcMap)
mux := http.NewServeMux()
mux.HandleFunc("/websocket", wm.WebsocketHandler)