Convert auth rpc services to use slog (#50513)

gravitational · Dec 20, 2024 · 4be8ef4 · 4be8ef4
1 parent 1cab8d9
commit 4be8ef4
Show file tree

Hide file tree

Showing 9 changed files with 78 additions and 63 deletions.
diff --git a/lib/auth/auth.go b/lib/auth/auth.go
@@ -652,7 +652,7 @@ func NewServer(cfg *InitConfig, opts ...ServerOption) (*Server, error) {
 
 	// Add in a login hook for generating state during user login.
 	as.ulsGenerator, err = userloginstate.NewGenerator(userloginstate.GeneratorConfig{
-		Log:         log,
+		Log:         as.logger,
 		AccessLists: &as,
 		Access:      &as,
 		UsageEvents: &as,

diff --git a/lib/auth/grpcserver.go b/lib/auth/grpcserver.go
@@ -5119,6 +5119,7 @@ func NewGRPCServer(cfg GRPCServerConfig) (*GRPCServer, error) {
 		Emitter:    cfg.Emitter,
 		Reporter:   cfg.AuthServer.Services.UsageReporter,
 		Clock:      cfg.AuthServer.GetClock(),
+		Logger:     cfg.AuthServer.logger.With(teleport.ComponentKey, "users.service"),
 	})
 	if err != nil {
 		return nil, trace.Wrap(err)
@@ -5133,6 +5134,7 @@ func NewGRPCServer(cfg GRPCServerConfig) (*GRPCServer, error) {
 		Emitter:    cfg.Emitter,
 		Reporter:   cfg.AuthServer.Services.UsageReporter,
 		Clock:      cfg.AuthServer.GetClock(),
+		Logger:     cfg.AuthServer.logger.With(teleport.ComponentKey, "presence.service"),
 	})
 	if err != nil {
 		return nil, trace.Wrap(err)

diff --git a/lib/auth/presence/presencev1/service.go b/lib/auth/presence/presencev1/service.go
@@ -20,10 +20,10 @@ package presencev1
 
 import (
 	"context"
+	"log/slog"
 
 	"github.com/gravitational/trace"
 	"github.com/jonboulle/clockwork"
-	"github.com/sirupsen/logrus"
 	"google.golang.org/protobuf/types/known/emptypb"
 
 	"github.com/gravitational/teleport"
@@ -34,6 +34,7 @@ import (
 	"github.com/gravitational/teleport/lib/services"
 	usagereporter "github.com/gravitational/teleport/lib/usagereporter/teleport"
 	"github.com/gravitational/teleport/lib/utils"
+	logutils "github.com/gravitational/teleport/lib/utils/log"
 )
 
 // Backend is the subset of the backend resources that the Service modifies.
@@ -65,7 +66,7 @@ type ServiceConfig struct {
 	AuthServer AuthServer
 	Backend    Backend
 	Cache      Cache
-	Logger     logrus.FieldLogger
+	Logger     *slog.Logger
 	Emitter    apievents.Emitter
 	Reporter   usagereporter.UsageReporter
 	Clock      clockwork.Clock
@@ -79,7 +80,7 @@ type Service struct {
 	authServer AuthServer
 	backend    Backend
 	cache      Cache
-	logger     logrus.FieldLogger
+	logger     *slog.Logger
 	emitter    apievents.Emitter
 	reporter   usagereporter.UsageReporter
 	clock      clockwork.Clock
@@ -103,7 +104,7 @@ func NewService(cfg ServiceConfig) (*Service, error) {
 	}
 
 	if cfg.Logger == nil {
-		cfg.Logger = logrus.WithField(teleport.ComponentKey, "presence.service")
+		cfg.Logger = slog.With(teleport.ComponentKey, "presence.service")
 	}
 	if cfg.Clock == nil {
 		cfg.Clock = clockwork.NewRealClock()
@@ -149,7 +150,11 @@ func (s *Service) GetRemoteCluster(
 
 	v3, ok := rc.(*types.RemoteClusterV3)
 	if !ok {
-		s.logger.Warnf("expected type RemoteClusterV3, got %T for %q", rc, rc.GetName())
+		s.logger.WarnContext(ctx, "unexpected remote cluster type",
+			"got_type", logutils.TypeAttr(rc),
+			"expected_type", "RemoteClusterV3",
+			"remote_cluster", rc.GetName(),
+		)
 		return nil, trace.BadParameter("encountered unexpected remote cluster type")
 	}
 
@@ -180,7 +185,11 @@ func (s *Service) ListRemoteClusters(
 	for _, rc := range page {
 		v3, ok := rc.(*types.RemoteClusterV3)
 		if !ok {
-			s.logger.Warnf("expected type RemoteClusterV3, got %T for %q", rc, rc.GetName())
+			s.logger.WarnContext(ctx, "unexpected remote cluster type",
+				"got_type", logutils.TypeAttr(rc),
+				"expected_type", "RemoteClusterV3",
+				"remote_cluster", rc.GetName(),
+			)
 			continue
 		}
 		concretePage = append(concretePage, v3)
@@ -234,7 +243,11 @@ func (s *Service) UpdateRemoteCluster(
 		}
 		v3, ok := rc.(*types.RemoteClusterV3)
 		if !ok {
-			s.logger.Warnf("expected type RemoteClusterV3, got %T for user %q", rc, rc.GetName())
+			s.logger.WarnContext(ctx, "unexpected remote cluster type",
+				"got_type", logutils.TypeAttr(rc),
+				"expected_type", "RemoteClusterV3",
+				"remote_cluster", rc.GetName(),
+			)
 			return nil, trace.BadParameter("encountered unexpected remote cluster type")
 		}
 		return v3, nil
@@ -271,7 +284,11 @@ func (s *Service) UpdateRemoteCluster(
 	}
 	v3, ok := rc.(*types.RemoteClusterV3)
 	if !ok {
-		s.logger.Warnf("expected type RemoteClusterV3, got %T for user %q", rc, rc.GetName())
+		s.logger.WarnContext(ctx, "unexpected remote cluster type",
+			"got_type", logutils.TypeAttr(rc),
+			"expected_type", "RemoteClusterV3",
+			"remote_cluster", rc.GetName(),
+		)
 		return nil, trace.BadParameter("encountered unexpected remote cluster type")
 	}
 
@@ -330,7 +347,11 @@ func (s *Service) ListReverseTunnels(
 	for _, rc := range page {
 		v3, ok := rc.(*types.ReverseTunnelV2)
 		if !ok {
-			s.logger.Warnf("expected type ReverseTunnelV2, got %T for %q", rc, rc.GetName())
+			s.logger.WarnContext(ctx, "unexpected reverse tunnel type",
+				"got_type", logutils.TypeAttr(rc),
+				"expected_type", "ReverseTunnelV2",
+				"reverse_tunnel", rc.GetName(),
+			)
 			continue
 		}
 		concretePage = append(concretePage, v3)

diff --git a/lib/auth/trust/trustv1/service.go b/lib/auth/trust/trustv1/service.go
@@ -23,10 +23,8 @@ import (
 	"time"
 
 	"github.com/gravitational/trace"
-	"github.com/sirupsen/logrus"
 	"google.golang.org/protobuf/types/known/emptypb"
 
-	"github.com/gravitational/teleport"
 	trustpb "github.com/gravitational/teleport/api/gen/proto/go/teleport/trust/v1"
 	"github.com/gravitational/teleport/api/types"
 	"github.com/gravitational/teleport/lib/authz"
@@ -52,7 +50,6 @@ type ServiceConfig struct {
 	Authorizer authz.Authorizer
 	Cache      services.AuthorityGetter
 	Backend    services.TrustInternal
-	Logger     *logrus.Entry
 	AuthServer authServer
 }
 
@@ -63,7 +60,6 @@ type Service struct {
 	cache      services.AuthorityGetter
 	backend    services.TrustInternal
 	authServer authServer
-	logger     *logrus.Entry
 }
 
 // NewService returns a new trust gRPC service.
@@ -77,12 +73,9 @@ func NewService(cfg *ServiceConfig) (*Service, error) {
 		return nil, trace.BadParameter("authorizer is required")
 	case cfg.AuthServer == nil:
 		return nil, trace.BadParameter("authServer is required")
-	case cfg.Logger == nil:
-		cfg.Logger = logrus.WithField(teleport.ComponentKey, "trust.service")
 	}
 
 	return &Service{
-		logger:     cfg.Logger,
 		authorizer: cfg.Authorizer,
 		cache:      cfg.Cache,
 		backend:    cfg.Backend,

diff --git a/lib/auth/userloginstate/generator.go b/lib/auth/userloginstate/generator.go
@@ -21,10 +21,10 @@ package userloginstate
 import (
 	"context"
 	"fmt"
+	"log/slog"
 
 	"github.com/gravitational/trace"
 	"github.com/jonboulle/clockwork"
-	"github.com/sirupsen/logrus"
 
 	"github.com/gravitational/teleport/api/client/proto"
 	usageeventsv1 "github.com/gravitational/teleport/api/gen/proto/go/usageevents/v1"
@@ -49,7 +49,7 @@ type AccessListsAndLockGetter interface {
 // GeneratorConfig is the configuration for the user login state generator.
 type GeneratorConfig struct {
 	// Log is a logger to use for the generator.
-	Log *logrus.Entry
+	Log *slog.Logger
 
 	// AccessLists is a service for retrieving access lists and locks from the backend.
 	AccessLists AccessListsAndLockGetter
@@ -107,7 +107,7 @@ func (g *GeneratorConfig) CheckAndSetDefaults() error {
 
 // Generator will generate a user login state from a user.
 type Generator struct {
-	log         *logrus.Entry
+	log         *slog.Logger
 	accessLists AccessListsAndLockGetter
 	access      services.Access
 	usageEvents UsageEventsClient
@@ -191,7 +191,7 @@ func (g *Generator) Generate(ctx context.Context, user types.User, ulsService se
 	if g.usageEvents != nil {
 		// Emit the usage event metadata.
 		if err := g.emitUsageEvent(ctx, user, uls, inheritedRoles, inheritedTraits); err != nil {
-			g.log.WithError(err).Debug("Error emitting usage event during user login state generation, skipping")
+			g.log.DebugContext(ctx, "Error emitting usage event during user login state generation, skipping", "error", err)
 		}
 	}
 
@@ -245,7 +245,7 @@ func (g *Generator) handleAccessListMembership(ctx context.Context, user types.U
 	if err != nil || membershipKind == accesslists.MembershipOrOwnershipTypeNone {
 		// Log any error.
 		if err != nil {
-			g.log.WithError(err).Warn("checking access list membership")
+			g.log.WarnContext(ctx, "checking access list membership", "error", err)
 		}
 		return inheritedRoles, inheritedTraits, nil
 	}
@@ -290,7 +290,7 @@ func (g *Generator) handleAccessListOwnership(ctx context.Context, user types.Us
 	if err != nil || ownershipType == accesslists.MembershipOrOwnershipTypeNone {
 		// Log any error.
 		if err != nil {
-			g.log.WithError(err).Warn("checking access list ownership")
+			g.log.WarnContext(ctx, "checking access list ownership", "error", err)
 		}
 		return inheritedRoles, inheritedTraits, nil
 	}
@@ -497,6 +497,6 @@ func (g *Generator) emitSkippedAccessListEvent(ctx context.Context, accessListNa
 			UserMessage: "access list skipped because it references non-existent role(s)",
 		},
 	}); err != nil {
-		g.log.WithError(err).Warn("Failed to emit access list skipped warning audit event.")
+		g.log.WarnContext(ctx, "Failed to emit access list skipped warning audit event", "error", err)
 	}
 }
diff --git a/lib/auth/userloginstate/generator_test.go b/lib/auth/userloginstate/generator_test.go
@@ -26,7 +26,6 @@ import (
 	"github.com/google/go-cmp/cmp"
 	"github.com/google/go-cmp/cmp/cmpopts"
 	"github.com/jonboulle/clockwork"
-	"github.com/sirupsen/logrus"
 	"github.com/stretchr/testify/require"
 
 	"github.com/gravitational/teleport/api/client/proto"
@@ -42,6 +41,7 @@ import (
 	"github.com/gravitational/teleport/lib/modules"
 	"github.com/gravitational/teleport/lib/services"
 	"github.com/gravitational/teleport/lib/services/local"
+	"github.com/gravitational/teleport/lib/utils"
 )
 
 const ownerUser = "owner"
@@ -729,7 +729,6 @@ func initGeneratorSvc(t *testing.T) (*Generator, *svc) {
 	ulsService, err := local.NewUserLoginStateService(mem)
 	require.NoError(t, err)
 
-	log := logrus.WithField("test", "logger")
 	svc := &svc{
 		AccessLists:     accessListsSvc,
 		Access:          accessSvc,
@@ -739,7 +738,7 @@ func initGeneratorSvc(t *testing.T) (*Generator, *svc) {
 	emitter := &eventstest.MockRecorderEmitter{}
 
 	generator, err := NewGenerator(GeneratorConfig{
-		Log:         log,
+		Log:         utils.NewSlogLoggerForTests(),
 		AccessLists: svc,
 		Access:      svc,
 		UsageEvents: svc,

diff --git a/lib/auth/userloginstate/userloginstatev1/service.go b/lib/auth/userloginstate/userloginstatev1/service.go
@@ -23,10 +23,8 @@ import (
 
 	"github.com/gravitational/trace"
 	"github.com/jonboulle/clockwork"
-	"github.com/sirupsen/logrus"
 	"google.golang.org/protobuf/types/known/emptypb"
 
-	"github.com/gravitational/teleport"
 	userloginstatev1 "github.com/gravitational/teleport/api/gen/proto/go/teleport/userloginstate/v1"
 	"github.com/gravitational/teleport/api/types"
 	conv "github.com/gravitational/teleport/api/types/userloginstate/convert/v1"
@@ -36,8 +34,6 @@ import (
 
 // ServiceConfig is the service config for the Access Lists gRPC service.
 type ServiceConfig struct {
-	// Logger is the logger to use.
-	Logger logrus.FieldLogger
 
 	// Authorizer is the authorizer to use.
 	Authorizer authz.Authorizer
@@ -58,10 +54,6 @@ func (c *ServiceConfig) checkAndSetDefaults() error {
 		return trace.BadParameter("user login states service is missing")
 	}
 
-	if c.Logger == nil {
-		c.Logger = logrus.WithField(teleport.ComponentKey, "user_login_state_crud_service")
-	}
-
 	if c.Clock == nil {
 		c.Clock = clockwork.NewRealClock()
 	}
@@ -72,7 +64,6 @@ func (c *ServiceConfig) checkAndSetDefaults() error {
 type Service struct {
 	userloginstatev1.UnimplementedUserLoginStateServiceServer
 
-	log             logrus.FieldLogger
 	authorizer      authz.Authorizer
 	userLoginStates services.UserLoginStates
 	clock           clockwork.Clock
@@ -85,7 +76,6 @@ func NewService(cfg ServiceConfig) (*Service, error) {
 	}
 
 	return &Service{
-		log:             cfg.Logger,
 		authorizer:      cfg.Authorizer,
 		userLoginStates: cfg.UserLoginStates,
 		clock:           cfg.Clock,

diff --git a/lib/auth/userpreferences/userpreferencesv1/service.go b/lib/auth/userpreferences/userpreferencesv1/service.go
@@ -22,10 +22,8 @@ import (
 	"context"
 
 	"github.com/gravitational/trace"
-	"github.com/sirupsen/logrus"
 	"google.golang.org/protobuf/types/known/emptypb"
 
-	"github.com/gravitational/teleport"
 	userpreferences "github.com/gravitational/teleport/api/gen/proto/go/userpreferences/v1"
 	"github.com/gravitational/teleport/lib/authz"
 	"github.com/gravitational/teleport/lib/services"
@@ -35,7 +33,6 @@ import (
 type ServiceConfig struct {
 	Backend    services.UserPreferences
 	Authorizer authz.Authorizer
-	Logger     *logrus.Entry
 }
 
 // Service implements the teleport.userpreferences.v1.UserPreferencesService RPC service.
@@ -44,7 +41,6 @@ type Service struct {
 
 	backend    services.UserPreferences
 	authorizer authz.Authorizer
-	log        *logrus.Entry
 }
 
 // NewService returns a new user preferences gRPC service.
@@ -54,14 +50,11 @@ func NewService(cfg *ServiceConfig) (*Service, error) {
 		return nil, trace.BadParameter("backend is required")
 	case cfg.Authorizer == nil:
 		return nil, trace.BadParameter("authorizer is required")
-	case cfg.Logger == nil:
-		cfg.Logger = logrus.WithField(teleport.ComponentKey, "userpreferences.service")
 	}
 
 	return &Service{
 		backend:    cfg.Backend,
 		authorizer: cfg.Authorizer,
-		log:        cfg.Logger,
 	}, nil
 }