libs/service: pass logger explicitly (#7288)

This is a very small change, but removes a method from the
`service.Service` interface (a win!) and forces callers to explicitly
pass loggers in to objects during construction rather than (later)
injecting them. There's not a real need for this kind of lazy
construction of loggers, and I think a decent potential for confusion
for mutable loggers.

The main concern I have is that this changes the constructor API for
ABCI clients. I think this is fine, and I suspect that as we plumb
contexts through, and make changes to the RPC services there'll be a
number of similar sorts of changes to various (quasi) public
interfaces, which I think we should welcome.
This commit is contained in:
Sam Kleinman
2021-11-16 11:20:56 -05:00
committed by GitHub
parent dbac109d01
commit d7606777cf
57 changed files with 347 additions and 356 deletions

View File

@@ -48,11 +48,11 @@ var SOCKET = "socket"
func TestEcho(t *testing.T) {
sockPath := fmt.Sprintf("unix:///tmp/echo_%v.sock", tmrand.Str(6))
clientCreator := abciclient.NewRemoteCreator(sockPath, SOCKET, true)
logger := log.TestingLogger()
clientCreator := abciclient.NewRemoteCreator(logger, sockPath, SOCKET, true)
// Start server
s := server.NewSocketServer(sockPath, kvstore.NewApplication())
s.SetLogger(log.TestingLogger().With("module", "abci-server"))
s := server.NewSocketServer(logger.With("module", "abci-server"), sockPath, kvstore.NewApplication())
if err := s.Start(); err != nil {
t.Fatalf("Error starting socket server: %v", err.Error())
}
@@ -63,11 +63,11 @@ func TestEcho(t *testing.T) {
})
// Start client
cli, err := clientCreator()
cli, err := clientCreator(logger.With("module", "abci-client"))
if err != nil {
t.Fatalf("Error creating ABCI client: %v", err.Error())
}
cli.SetLogger(log.TestingLogger().With("module", "abci-client"))
if err := cli.Start(); err != nil {
t.Fatalf("Error starting ABCI client: %v", err.Error())
}
@@ -96,11 +96,11 @@ func TestEcho(t *testing.T) {
func BenchmarkEcho(b *testing.B) {
b.StopTimer() // Initialize
sockPath := fmt.Sprintf("unix:///tmp/echo_%v.sock", tmrand.Str(6))
clientCreator := abciclient.NewRemoteCreator(sockPath, SOCKET, true)
logger := log.TestingLogger()
clientCreator := abciclient.NewRemoteCreator(logger, sockPath, SOCKET, true)
// Start server
s := server.NewSocketServer(sockPath, kvstore.NewApplication())
s.SetLogger(log.TestingLogger().With("module", "abci-server"))
s := server.NewSocketServer(logger.With("module", "abci-server"), sockPath, kvstore.NewApplication())
if err := s.Start(); err != nil {
b.Fatalf("Error starting socket server: %v", err.Error())
}
@@ -111,11 +111,11 @@ func BenchmarkEcho(b *testing.B) {
})
// Start client
cli, err := clientCreator()
cli, err := clientCreator(logger.With("module", "abci-client"))
if err != nil {
b.Fatalf("Error creating ABCI client: %v", err.Error())
}
cli.SetLogger(log.TestingLogger().With("module", "abci-client"))
if err := cli.Start(); err != nil {
b.Fatalf("Error starting ABCI client: %v", err.Error())
}
@@ -149,11 +149,11 @@ func BenchmarkEcho(b *testing.B) {
func TestInfo(t *testing.T) {
sockPath := fmt.Sprintf("unix:///tmp/echo_%v.sock", tmrand.Str(6))
clientCreator := abciclient.NewRemoteCreator(sockPath, SOCKET, true)
logger := log.TestingLogger()
clientCreator := abciclient.NewRemoteCreator(logger, sockPath, SOCKET, true)
// Start server
s := server.NewSocketServer(sockPath, kvstore.NewApplication())
s.SetLogger(log.TestingLogger().With("module", "abci-server"))
s := server.NewSocketServer(logger.With("module", "abci-server"), sockPath, kvstore.NewApplication())
if err := s.Start(); err != nil {
t.Fatalf("Error starting socket server: %v", err.Error())
}
@@ -164,11 +164,11 @@ func TestInfo(t *testing.T) {
})
// Start client
cli, err := clientCreator()
cli, err := clientCreator(logger.With("module", "abci-client"))
if err != nil {
t.Fatalf("Error creating ABCI client: %v", err.Error())
}
cli.SetLogger(log.TestingLogger().With("module", "abci-client"))
if err := cli.Start(); err != nil {
t.Fatalf("Error starting ABCI client: %v", err.Error())
}

View File

@@ -6,6 +6,7 @@ import (
abciclient "github.com/tendermint/tendermint/abci/client"
"github.com/tendermint/tendermint/abci/example/kvstore"
"github.com/tendermint/tendermint/abci/types"
"github.com/tendermint/tendermint/libs/log"
e2e "github.com/tendermint/tendermint/test/e2e/app"
)
@@ -15,7 +16,7 @@ import (
//
// The Closer is a noop except for persistent_kvstore applications,
// which will clean up the store.
func DefaultClientCreator(addr, transport, dbDir string) (abciclient.Creator, io.Closer) {
func DefaultClientCreator(logger log.Logger, addr, transport, dbDir string) (abciclient.Creator, io.Closer) {
switch addr {
case "kvstore":
return abciclient.NewLocalCreator(kvstore.NewApplication()), noopCloser{}
@@ -32,7 +33,7 @@ func DefaultClientCreator(addr, transport, dbDir string) (abciclient.Creator, io
return abciclient.NewLocalCreator(types.NewBaseApplication()), noopCloser{}
default:
mustConnect := false // loop retrying
return abciclient.NewRemoteCreator(addr, transport, mustConnect), noopCloser{}
return abciclient.NewRemoteCreator(logger, addr, transport, mustConnect), noopCloser{}
}
}

View File

@@ -6,7 +6,7 @@ import (
"syscall"
abciclient "github.com/tendermint/tendermint/abci/client"
tmlog "github.com/tendermint/tendermint/libs/log"
"github.com/tendermint/tendermint/libs/log"
"github.com/tendermint/tendermint/libs/service"
)
@@ -33,8 +33,8 @@ type AppConns interface {
}
// NewAppConns calls NewMultiAppConn.
func NewAppConns(clientCreator abciclient.Creator, metrics *Metrics) AppConns {
return NewMultiAppConn(clientCreator, metrics)
func NewAppConns(clientCreator abciclient.Creator, logger log.Logger, metrics *Metrics) AppConns {
return NewMultiAppConn(clientCreator, logger, metrics)
}
// multiAppConn implements AppConns.
@@ -60,12 +60,12 @@ type multiAppConn struct {
}
// NewMultiAppConn makes all necessary abci connections to the application.
func NewMultiAppConn(clientCreator abciclient.Creator, metrics *Metrics) AppConns {
func NewMultiAppConn(clientCreator abciclient.Creator, logger log.Logger, metrics *Metrics) AppConns {
multiAppConn := &multiAppConn{
metrics: metrics,
clientCreator: clientCreator,
}
multiAppConn.BaseService = *service.NewBaseService(nil, "multiAppConn", multiAppConn)
multiAppConn.BaseService = *service.NewBaseService(logger, "multiAppConn", multiAppConn)
return multiAppConn
}
@@ -128,7 +128,7 @@ func (app *multiAppConn) OnStop() {
}
func (app *multiAppConn) killTMOnClientError() {
killFn := func(conn string, err error, logger tmlog.Logger) {
killFn := func(conn string, err error, logger log.Logger) {
logger.Error(
fmt.Sprintf("%s connection terminated. Did the application crash? Please restart tendermint", conn),
"err", err)
@@ -181,11 +181,12 @@ func (app *multiAppConn) stopAllClients() {
}
func (app *multiAppConn) abciClientFor(conn string) (abciclient.Client, error) {
c, err := app.clientCreator()
c, err := app.clientCreator(app.Logger.With(
"module", "abci-client",
"connection", conn))
if err != nil {
return nil, fmt.Errorf("error creating ABCI client (%s connection): %w", conn, err)
}
c.SetLogger(app.Logger.With("module", "abci-client", "connection", conn))
if err := c.Start(); err != nil {
return nil, fmt.Errorf("error starting ABCI client (%s connection): %w", conn, err)
}

View File

@@ -14,24 +14,24 @@ import (
abciclient "github.com/tendermint/tendermint/abci/client"
abcimocks "github.com/tendermint/tendermint/abci/client/mocks"
"github.com/tendermint/tendermint/libs/log"
)
func TestAppConns_Start_Stop(t *testing.T) {
quitCh := make(<-chan struct{})
clientMock := &abcimocks.Client{}
clientMock.On("SetLogger", mock.Anything).Return().Times(4)
clientMock.On("Start").Return(nil).Times(4)
clientMock.On("Stop").Return(nil).Times(4)
clientMock.On("Quit").Return(quitCh).Times(4)
creatorCallCount := 0
creator := func() (abciclient.Client, error) {
creator := func(logger log.Logger) (abciclient.Client, error) {
creatorCallCount++
return clientMock, nil
}
appConns := NewAppConns(creator, NopMetrics())
appConns := NewAppConns(creator, log.TestingLogger(), NopMetrics())
err := appConns.Start()
require.NoError(t, err)
@@ -68,11 +68,11 @@ func TestAppConns_Failure(t *testing.T) {
clientMock.On("Quit").Return(recvQuitCh)
clientMock.On("Error").Return(errors.New("EOF")).Once()
creator := func() (abciclient.Client, error) {
creator := func(log.Logger) (abciclient.Client, error) {
return clientMock, nil
}
appConns := NewAppConns(creator, NopMetrics())
appConns := NewAppConns(creator, log.TestingLogger(), NopMetrics())
err := appConns.Start()
require.NoError(t, err)