service: remove exported logger from base implemenation (#7381)

libs/pubsub/example_test.go

@@ -7,6 +7,7 @@ import (
 	"github.com/stretchr/testify/require"
 
 	abci "github.com/tendermint/tendermint/abci/types"
+	"github.com/tendermint/tendermint/libs/log"
 	"github.com/tendermint/tendermint/libs/pubsub"
 	"github.com/tendermint/tendermint/libs/pubsub/query"
 )
@@ -14,7 +15,8 @@ import (
 func TestExample(t *testing.T) {
 	ctx, cancel := context.WithCancel(context.Background())
 	defer cancel()
-	s := newTestServer(ctx, t)
+
+	s := newTestServer(ctx, t, log.TestingLogger())
 
 	sub := newTestSub(t).must(s.SubscribeWithArgs(ctx, pubsub.SubscribeArgs{
 		ClientID: "example-client",

libs/pubsub/pubsub.go

@@ -41,6 +41,7 @@ import (
 	"sync"
 
 	"github.com/tendermint/tendermint/abci/types"
+	"github.com/tendermint/tendermint/libs/log"
 	"github.com/tendermint/tendermint/libs/service"
 )
 
@@ -103,6 +104,7 @@ func (args UnsubscribeArgs) Validate() error {
 // messages with or without events, and manages internal state.
 type Server struct {
 	service.BaseService
+	logger log.Logger
 
 	queue  chan item
 	done   <-chan struct{} // closed when server should exit
@@ -133,9 +135,10 @@ type Option func(*Server)
 // NewServer returns a new server. See the commentary on the Option functions
 // for a detailed description of how to configure buffering. If no options are
 // provided, the resulting server's queue is unbuffered.
-func NewServer(options ...Option) *Server {
-	s := new(Server)
-	s.BaseService = *service.NewBaseService(nil, "PubSub", s)
+func NewServer(logger log.Logger, options ...Option) *Server {
+	s := &Server{logger: logger}
+
+	s.BaseService = *service.NewBaseService(logger, "PubSub", s)
 	for _, opt := range options {
 		opt(s)
 	}
@@ -167,9 +170,7 @@ func (s *Server) BufferCapacity() int { return cap(s.queue) }
 // If len(capacities) > 0, its first value is used as the queue capacity.
 //
 // Deprecated: Use SubscribeWithArgs. This method will be removed in v0.36.
-func (s *Server) Subscribe(ctx context.Context,
-	clientID string, query Query, capacities ...int) (*Subscription, error) {
-
+func (s *Server) Subscribe(ctx context.Context, clientID string, query Query, capacities ...int) (*Subscription, error) {
 	args := SubscribeArgs{
 		ClientID: clientID,
 		Query:    query,
@@ -384,7 +385,7 @@ func (s *Server) run(ctx context.Context) {
 		// Sender: Service the queue and forward messages to subscribers.
 		for it := range queue {
 			if err := s.send(it.Data, it.Events); err != nil {
-				s.Logger.Error("Error sending event", "err", err)
+				s.logger.Error("Error sending event", "err", err)
 			}
 		}
 		// Terminate all subscribers before exit.

libs/pubsub/pubsub_test.go

@@ -22,7 +22,8 @@ func TestSubscribeWithArgs(t *testing.T) {
 	ctx, cancel := context.WithCancel(context.Background())
 	defer cancel()
 
-	s := newTestServer(ctx, t)
+	logger := log.TestingLogger()
+	s := newTestServer(ctx, t, logger)
 
 	t.Run("DefaultLimit", func(t *testing.T) {
 		sub := newTestSub(t).must(s.SubscribeWithArgs(ctx, pubsub.SubscribeArgs{
@@ -50,8 +51,9 @@ func TestSubscribeWithArgs(t *testing.T) {
 func TestObserver(t *testing.T) {
 	ctx, cancel := context.WithCancel(context.Background())
 	defer cancel()
+	logger := log.TestingLogger()
 
-	s := newTestServer(ctx, t)
+	s := newTestServer(ctx, t, logger)
 
 	done := make(chan struct{})
 	var got interface{}
@@ -71,7 +73,9 @@ func TestObserverErrors(t *testing.T) {
 	ctx, cancel := context.WithCancel(context.Background())
 	defer cancel()
 
-	s := newTestServer(ctx, t)
+	logger := log.TestingLogger()
+
+	s := newTestServer(ctx, t, logger)
 
 	require.Error(t, s.Observe(ctx, nil, query.All))
 	require.NoError(t, s.Observe(ctx, func(pubsub.Message) error { return nil }))
@@ -82,7 +86,9 @@ func TestPublishDoesNotBlock(t *testing.T) {
 	ctx, cancel := context.WithCancel(context.Background())
 	defer cancel()
 
-	s := newTestServer(ctx, t)
+	logger := log.TestingLogger()
+
+	s := newTestServer(ctx, t, logger)
 
 	sub := newTestSub(t).must(s.SubscribeWithArgs(ctx, pubsub.SubscribeArgs{
 		ClientID: clientID,
@@ -110,7 +116,8 @@ func TestSubscribeErrors(t *testing.T) {
 	ctx, cancel := context.WithCancel(context.Background())
 	defer cancel()
 
-	s := newTestServer(ctx, t)
+	logger := log.TestingLogger()
+	s := newTestServer(ctx, t, logger)
 
 	t.Run("EmptyQueryErr", func(t *testing.T) {
 		_, err := s.SubscribeWithArgs(ctx, pubsub.SubscribeArgs{ClientID: clientID})
@@ -130,7 +137,8 @@ func TestSlowSubscriber(t *testing.T) {
 	ctx, cancel := context.WithCancel(context.Background())
 	defer cancel()
 
-	s := newTestServer(ctx, t)
+	logger := log.TestingLogger()
+	s := newTestServer(ctx, t, logger)
 
 	sub := newTestSub(t).must(s.SubscribeWithArgs(ctx, pubsub.SubscribeArgs{
 		ClientID: clientID,
@@ -151,7 +159,8 @@ func TestDifferentClients(t *testing.T) {
 	ctx, cancel := context.WithCancel(context.Background())
 	defer cancel()
 
-	s := newTestServer(ctx, t)
+	logger := log.TestingLogger()
+	s := newTestServer(ctx, t, logger)
 
 	sub1 := newTestSub(t).must(s.SubscribeWithArgs(ctx, pubsub.SubscribeArgs{
 		ClientID: "client-1",
@@ -205,7 +214,8 @@ func TestSubscribeDuplicateKeys(t *testing.T) {
 	ctx, cancel := context.WithCancel(context.Background())
 	defer cancel()
 
-	s := newTestServer(ctx, t)
+	logger := log.TestingLogger()
+	s := newTestServer(ctx, t, logger)
 
 	testCases := []struct {
 		query    string
@@ -260,7 +270,8 @@ func TestClientSubscribesTwice(t *testing.T) {
 	ctx, cancel := context.WithCancel(context.Background())
 	defer cancel()
 
-	s := newTestServer(ctx, t)
+	logger := log.TestingLogger()
+	s := newTestServer(ctx, t, logger)
 
 	q := query.MustCompile(`tm.events.type='NewBlock'`)
 	events := []abci.Event{{
@@ -295,7 +306,8 @@ func TestUnsubscribe(t *testing.T) {
 	ctx, cancel := context.WithCancel(context.Background())
 	defer cancel()
 
-	s := newTestServer(ctx, t)
+	logger := log.TestingLogger()
+	s := newTestServer(ctx, t, logger)
 
 	sub := newTestSub(t).must(s.SubscribeWithArgs(ctx, pubsub.SubscribeArgs{
 		ClientID: clientID,
@@ -319,7 +331,8 @@ func TestClientUnsubscribesTwice(t *testing.T) {
 	ctx, cancel := context.WithCancel(context.Background())
 	defer cancel()
 
-	s := newTestServer(ctx, t)
+	logger := log.TestingLogger()
+	s := newTestServer(ctx, t, logger)
 
 	newTestSub(t).must(s.SubscribeWithArgs(ctx, pubsub.SubscribeArgs{
 		ClientID: clientID,
@@ -340,7 +353,8 @@ func TestResubscribe(t *testing.T) {
 	ctx, cancel := context.WithCancel(context.Background())
 	defer cancel()
 
-	s := newTestServer(ctx, t)
+	logger := log.TestingLogger()
+	s := newTestServer(ctx, t, logger)
 
 	args := pubsub.SubscribeArgs{
 		ClientID: clientID,
@@ -363,7 +377,8 @@ func TestUnsubscribeAll(t *testing.T) {
 	ctx, cancel := context.WithCancel(context.Background())
 	defer cancel()
 
-	s := newTestServer(ctx, t)
+	logger := log.TestingLogger()
+	s := newTestServer(ctx, t, logger)
 
 	sub1 := newTestSub(t).must(s.SubscribeWithArgs(ctx, pubsub.SubscribeArgs{
 		ClientID: clientID,
@@ -383,10 +398,8 @@ func TestUnsubscribeAll(t *testing.T) {
 }
 
 func TestBufferCapacity(t *testing.T) {
-	s := pubsub.NewServer(pubsub.BufferCapacity(2),
-		func(s *pubsub.Server) {
-			s.Logger = log.TestingLogger()
-		})
+	logger := log.TestingLogger()
+	s := pubsub.NewServer(logger, pubsub.BufferCapacity(2))
 
 	require.Equal(t, 2, s.BufferCapacity())
 
@@ -402,12 +415,10 @@ func TestBufferCapacity(t *testing.T) {
 	require.ErrorIs(t, s.Publish(ctx, "Ironclad"), context.DeadlineExceeded)
 }
 
-func newTestServer(ctx context.Context, t testing.TB) *pubsub.Server {
+func newTestServer(ctx context.Context, t testing.TB, logger log.Logger) *pubsub.Server {
 	t.Helper()
 
-	s := pubsub.NewServer(func(s *pubsub.Server) {
-		s.Logger = log.TestingLogger()
-	})
+	s := pubsub.NewServer(logger)
 
 	require.NoError(t, s.Start(ctx))
 	t.Cleanup(s.Wait)

libs/service/service.go

@@ -92,7 +92,7 @@ Typical usage:
 	}
 */
 type BaseService struct {
-	Logger  log.Logger
+	logger  log.Logger
 	name    string
 	started uint32 // atomic
 	stopped uint32 // atomic
@@ -109,7 +109,7 @@ func NewBaseService(logger log.Logger, name string, impl Implementation) *BaseSe
 	}
 
 	return &BaseService{
-		Logger: logger,
+		logger: logger,
 		name:   name,
 		quit:   make(chan struct{}),
 		impl:   impl,
@@ -122,12 +122,12 @@ func NewBaseService(logger log.Logger, name string, impl Implementation) *BaseSe
 func (bs *BaseService) Start(ctx context.Context) error {
 	if atomic.CompareAndSwapUint32(&bs.started, 0, 1) {
 		if atomic.LoadUint32(&bs.stopped) == 1 {
-			bs.Logger.Error("not starting service; already stopped", "service", bs.name, "impl", bs.impl.String())
+			bs.logger.Error("not starting service; already stopped", "service", bs.name, "impl", bs.impl.String())
 			atomic.StoreUint32(&bs.started, 0)
 			return ErrAlreadyStopped
 		}
 
-		bs.Logger.Info("starting service", "service", bs.name, "impl", bs.impl.String())
+		bs.logger.Info("starting service", "service", bs.name, "impl", bs.impl.String())
 
 		if err := bs.impl.OnStart(ctx); err != nil {
 			// revert flag
@@ -151,13 +151,13 @@ func (bs *BaseService) Start(ctx context.Context) error {
 				// the context was cancel and we
 				// should stop.
 				if err := bs.Stop(); err != nil {
-					bs.Logger.Error("stopped service",
+					bs.logger.Error("stopped service",
 						"err", err.Error(),
 						"service", bs.name,
 						"impl", bs.impl.String())
 				}
 
-				bs.Logger.Info("stopped service",
+				bs.logger.Info("stopped service",
 					"service", bs.name,
 					"impl", bs.impl.String())
 			}
@@ -166,7 +166,7 @@ func (bs *BaseService) Start(ctx context.Context) error {
 		return nil
 	}
 
-	bs.Logger.Debug("not starting service; already started", "service", bs.name, "impl", bs.impl.String())
+	bs.logger.Debug("not starting service; already started", "service", bs.name, "impl", bs.impl.String())
 	return ErrAlreadyStarted
 }
 
@@ -175,19 +175,19 @@ func (bs *BaseService) Start(ctx context.Context) error {
 func (bs *BaseService) Stop() error {
 	if atomic.CompareAndSwapUint32(&bs.stopped, 0, 1) {
 		if atomic.LoadUint32(&bs.started) == 0 {
-			bs.Logger.Error("not stopping service; not started yet", "service", bs.name, "impl", bs.impl.String())
+			bs.logger.Error("not stopping service; not started yet", "service", bs.name, "impl", bs.impl.String())
 			atomic.StoreUint32(&bs.stopped, 0)
 			return ErrNotStarted
 		}
 
-		bs.Logger.Info("stopping service", "service", bs.name, "impl", bs.impl.String())
+		bs.logger.Info("stopping service", "service", bs.name, "impl", bs.impl.String())
 		bs.impl.OnStop()
 		close(bs.quit)
 
 		return nil
 	}
 
-	bs.Logger.Debug("not stopping service; already stopped", "service", bs.name, "impl", bs.impl.String())
+	bs.logger.Debug("not stopping service; already stopped", "service", bs.name, "impl", bs.impl.String())
 	return ErrAlreadyStopped
 }