[v16] Replace logrus with slog in most of lib/srv/app (#47885)

* Replace logrus with slog in most of `lib/srv/app` * move things * avoid slog.Logger().With * Update lib/srv/app/aws/handler.go Co-authored-by: rosstimothy <[email protected]> * fix: use legacy logger for reconciler --------- Co-authored-by: rosstimothy <[email protected]>
gravitational · Oct 25, 2024 · 07ff292 · 07ff292
1 parent 3cdb1c0
commit 07ff292
Show file tree

Hide file tree

Showing 12 changed files with 116 additions and 75 deletions.
diff --git a/lib/srv/app/aws/handler.go b/lib/srv/app/aws/handler.go
@@ -22,6 +22,7 @@ import (
 	"bytes"
 	"context"
 	"io"
+	"log/slog"
 	"net/http"
 	"net/url"
 	"strings"
@@ -53,8 +54,12 @@ type signerHandler struct {
 
 // SignerHandlerConfig is the awsSignerHandler configuration.
 type SignerHandlerConfig struct {
+	// LegacyLogger is the old logger.
+	// Should be removed gradually.
+	// Deprecated: use Log instead.
+	LegacyLogger logrus.FieldLogger
 	// Log is a logger for the handler.
-	Log logrus.FieldLogger
+	Log *slog.Logger
 	// RoundTripper is an http.RoundTripper instance used for requests.
 	RoundTripper http.RoundTripper
 	// SigningService is used to sign requests before forwarding them.
@@ -77,8 +82,11 @@ func (cfg *SignerHandlerConfig) CheckAndSetDefaults() error {
 		}
 		cfg.RoundTripper = tr
 	}
+	if cfg.LegacyLogger == nil {
+		cfg.LegacyLogger = logrus.WithField(teleport.ComponentKey, "aws:signer")
+	}
 	if cfg.Log == nil {
-		cfg.Log = logrus.WithField(teleport.ComponentKey, "aws:signer")
+		cfg.Log = slog.With(teleport.ComponentKey, "aws:signer")
 	}
 	if cfg.Clock == nil {
 		cfg.Clock = clockwork.NewRealClock()
@@ -106,7 +114,7 @@ func NewAWSSignerHandler(ctx context.Context, config SignerHandlerConfig) (http.
 	var err error
 	handler.fwd, err = reverseproxy.New(
 		reverseproxy.WithRoundTripper(config.RoundTripper),
-		reverseproxy.WithLogger(config.Log),
+		reverseproxy.WithLogger(config.LegacyLogger),
 		reverseproxy.WithErrorHandler(handler.formatForwardResponseError),
 	)
 
@@ -115,7 +123,7 @@ func NewAWSSignerHandler(ctx context.Context, config SignerHandlerConfig) (http.
 
 // formatForwardResponseError converts an error to a status code and writes the code to a response.
 func (s *signerHandler) formatForwardResponseError(rw http.ResponseWriter, r *http.Request, err error) {
-	s.Log.WithError(err).Debugf("Failed to process request.")
+	s.Log.DebugContext(r.Context(), "Failed to process request", "error", err)
 	common.SetTeleportAPIErrorHeader(rw, err)
 
 	// Convert trace error type to HTTP and write response.
@@ -217,7 +225,7 @@ func (s *signerHandler) emitAudit(sessCtx *common.SessionContext, req *http.Requ
 	}
 	if auditErr != nil {
 		// log but don't return the error, because we already handed off request/response handling to the oxy forwarder.
-		s.Log.WithError(auditErr).Warn("Failed to emit audit event.")
+		s.Log.WarnContext(req.Context(), "Failed to emit audit event.", "error", auditErr)
 	}
 }
 

diff --git a/lib/srv/app/azure/credential.go b/lib/srv/app/azure/credential.go
@@ -89,7 +89,7 @@ func findDefaultCredentialProvider(ctx context.Context, logger *slog.Logger) (cr
 	defaultWorkloadIdentity, err := azidentity.NewWorkloadIdentityCredential(nil)
 	if err != nil {
 		// If no workload identity is found, fall back to regular managed identity.
-		logger.With("error", err).DebugContext(ctx, "Failed to load azure workload identity.")
+		logger.DebugContext(ctx, "Failed to load azure workload identity.", "error", err)
 		logger.InfoContext(ctx, "Using azure managed identity.")
 		return managedIdentityCredentialProvider{}, nil
 	}

diff --git a/lib/srv/app/azure/handler.go b/lib/srv/app/azure/handler.go
@@ -51,11 +51,12 @@ const ComponentKey = "azure:fwd"
 type HandlerConfig struct {
 	// RoundTripper is the underlying transport given to an oxy Forwarder.
 	RoundTripper http.RoundTripper
-	// Log is the Logger.
-	// TODO(greedy52) replace with slog.
-	Log logrus.FieldLogger
-	// Logger is the slog.Logger.
-	Logger *slog.Logger
+	// LegacyLogger is the old logger.
+	// Should be removed gradually.
+	// Deprecated: use Log instead.
+	LegacyLogger logrus.FieldLogger
+	// Log is a logger for the handler.
+	Log *slog.Logger
 	// Clock is used to override time in tests.
 	Clock clockwork.Clock
 
@@ -75,14 +76,14 @@ func (s *HandlerConfig) CheckAndSetDefaults(ctx context.Context) error {
 	if s.Clock == nil {
 		s.Clock = clockwork.NewRealClock()
 	}
-	if s.Log == nil {
-		s.Log = logrus.WithField(teleport.ComponentKey, ComponentKey)
+	if s.LegacyLogger == nil {
+		s.LegacyLogger = logrus.WithField(teleport.ComponentKey, ComponentKey)
 	}
-	if s.Logger == nil {
-		s.Logger = slog.Default().With(teleport.ComponentKey, ComponentKey)
+	if s.Log == nil {
+		s.Log = slog.With(teleport.ComponentKey, ComponentKey)
 	}
 	if s.getAccessToken == nil {
-		s.getAccessToken = lazyGetAccessTokenFromDefaultCredentialProvider(s.Logger)
+		s.getAccessToken = lazyGetAccessTokenFromDefaultCredentialProvider(s.Log)
 	}
 	return nil
 }
@@ -127,7 +128,7 @@ func newAzureHandler(ctx context.Context, config HandlerConfig) (*handler, error
 
 	svc.fwd, err = reverseproxy.New(
 		reverseproxy.WithRoundTripper(config.RoundTripper),
-		reverseproxy.WithLogger(config.Log),
+		reverseproxy.WithLogger(config.LegacyLogger),
 		reverseproxy.WithErrorHandler(svc.formatForwardResponseError),
 	)
 
@@ -161,13 +162,13 @@ func (s *handler) serveHTTP(w http.ResponseWriter, req *http.Request) error {
 
 	if err := sessionCtx.Audit.OnRequest(req.Context(), sessionCtx, fwdRequest, status, nil); err != nil {
 		// log but don't return the error, because we already handed off request/response handling to the oxy forwarder.
-		s.Log.WithError(err).Warn("Failed to emit audit event.")
+		s.Log.WarnContext(req.Context(), "Failed to emit audit event.", "error", err)
 	}
 	return nil
 }
 
 func (s *handler) formatForwardResponseError(rw http.ResponseWriter, r *http.Request, err error) {
-	s.Log.WithError(err).Debugf("Failed to process request.")
+	s.Log.DebugContext(r.Context(), "Failed to process request.", "error", err)
 	common.SetTeleportAPIErrorHeader(rw, err)
 
 	// Convert trace error type to HTTP and write response.
@@ -224,7 +225,7 @@ func getPeerKey(certs []*x509.Certificate) (crypto.PublicKey, error) {
 func (s *handler) replaceAuthHeaders(r *http.Request, sessionCtx *common.SessionContext, reqCopy *http.Request) error {
 	auth := reqCopy.Header.Get("Authorization")
 	if auth == "" {
-		s.Log.Debugf("No Authorization header present, skipping replacement.")
+		s.Log.DebugContext(r.Context(), "No Authorization header present, skipping replacement.")
 		return nil
 	}
 
@@ -238,7 +239,11 @@ func (s *handler) replaceAuthHeaders(r *http.Request, sessionCtx *common.Session
 		return trace.Wrap(err, "failed to parse Authorization header")
 	}
 
-	s.Log.Debugf("Processing request, sessionId = %q, azureIdentity = %q, claims = %v", sessionCtx.Identity.RouteToApp.SessionID, sessionCtx.Identity.RouteToApp.AzureIdentity, claims)
+	s.Log.DebugContext(r.Context(), "Processing request.",
+		"session_id", sessionCtx.Identity.RouteToApp.SessionID,
+		"azure_identity", sessionCtx.Identity.RouteToApp.AzureIdentity,
+		"claims", claims,
+	)
 	token, err := s.getToken(r.Context(), sessionCtx.Identity.RouteToApp.AzureIdentity, claims.Resource)
 	if err != nil {
 		return trace.Wrap(err)

diff --git a/lib/srv/app/cloud.go b/lib/srv/app/cloud.go
@@ -35,9 +35,7 @@ import (
 	awssession "github.com/aws/aws-sdk-go/aws/session"
 	"github.com/gravitational/trace"
 	"github.com/jonboulle/clockwork"
-	"github.com/sirupsen/logrus"
 
-	"github.com/gravitational/teleport"
 	"github.com/gravitational/teleport/api/constants"
 	"github.com/gravitational/teleport/lib/tlsca"
 	awsutils "github.com/gravitational/teleport/lib/utils/aws"
@@ -110,7 +108,6 @@ func (c *CloudConfig) CheckAndSetDefaults() error {
 
 type cloud struct {
 	cfg CloudConfig
-	log logrus.FieldLogger
 }
 
 // NewCloud creates a new cloud service.
@@ -120,7 +117,6 @@ func NewCloud(cfg CloudConfig) (Cloud, error) {
 	}
 	return &cloud{
 		cfg: cfg,
-		log: logrus.WithField(teleport.ComponentKey, "cloud"),
 	}, nil
 }
 

diff --git a/lib/srv/app/common/audit.go b/lib/srv/app/common/audit.go
@@ -20,11 +20,11 @@ package common
 
 import (
 	"context"
+	"log/slog"
 	"net/http"
 
 	"github.com/aws/aws-sdk-go/aws/endpoints"
 	"github.com/gravitational/trace"
-	"github.com/sirupsen/logrus"
 
 	"github.com/gravitational/teleport"
 	apidefaults "github.com/gravitational/teleport/api/defaults"
@@ -75,7 +75,7 @@ type audit struct {
 	// cfg is the audit events emitter configuration.
 	cfg AuditConfig
 	// log is used for logging
-	log logrus.FieldLogger
+	log *slog.Logger
 }
 
 // NewAudit returns a new instance of the audit events emitter.
@@ -85,7 +85,7 @@ func NewAudit(config AuditConfig) (Audit, error) {
 	}
 	return &audit{
 		cfg: config,
-		log: logrus.WithField(teleport.ComponentKey, "app:audit"),
+		log: slog.With(teleport.ComponentKey, "app:audit"),
 	}, nil
 }
 
@@ -199,7 +199,7 @@ func (a *audit) OnDynamoDBRequest(ctx context.Context, sessionCtx *SessionContex
 	// If this fails, we still want to emit the rest of the event info; the request event Body is nullable, so it's ok if body is left nil here.
 	body, err := awsutils.UnmarshalRequestBody(req)
 	if err != nil {
-		a.log.WithError(err).Warn("Failed to read request body as JSON, omitting the body from the audit event.")
+		a.log.WarnContext(ctx, "Failed to read request body as JSON, omitting the body from the audit event.", "error", err)
 	}
 	// get the API target from the request header, according to the API request format documentation:
 	// https://docs.aws.amazon.com/amazondynamodb/latest/developerguide/Programming.LowLevelAPI.html#Programming.LowLevelAPI.RequestFormat

diff --git a/lib/srv/app/connections_handler.go b/lib/srv/app/connections_handler.go
@@ -225,7 +225,7 @@ func NewConnectionsHandler(closeContext context.Context, cfg *ConnectionsHandler
 	}
 
 	azureHandler, err := appazure.NewAzureHandler(closeContext, appazure.HandlerConfig{
-		Logger: cfg.Logger.With(teleport.ComponentKey, appazure.ComponentKey),
+		Log: cfg.Logger.With(teleport.ComponentKey, appazure.ComponentKey),
 	})
 	if err != nil {
 		return nil, trace.Wrap(err)

diff --git a/lib/srv/app/gcp/handler.go b/lib/srv/app/gcp/handler.go
@@ -22,6 +22,7 @@ import (
 	"bytes"
 	"context"
 	"fmt"
+	"log/slog"
 	"net/http"
 	"time"
 
@@ -68,8 +69,12 @@ var _ cloudClientGCP = (*cloudClientGCPImpl[iamCredentialsClient])(nil)
 type HandlerConfig struct {
 	// RoundTripper is the underlying transport given to an oxy Forwarder.
 	RoundTripper http.RoundTripper
-	// Log is the Logger.
-	Log logrus.FieldLogger
+	// LegacyLogger is the old logger.
+	// Should be removed gradually.
+	// Deprecated: use Log instead.
+	LegacyLogger logrus.FieldLogger
+	// Log is a logger for the handler.
+	Log *slog.Logger
 	// Clock is used to override time in tests.
 	Clock clockwork.Clock
 	// cloudClientGCP holds a reference to GCP IAM client. Normally set in CheckAndSetDefaults, it is overridden in tests.
@@ -89,7 +94,10 @@ func (s *HandlerConfig) CheckAndSetDefaults() error {
 		s.Clock = clockwork.NewRealClock()
 	}
 	if s.Log == nil {
-		s.Log = logrus.WithField(teleport.ComponentKey, "gcp:fwd")
+		s.Log = slog.With(teleport.ComponentKey, "gcp:fwd")
+	}
+	if s.LegacyLogger == nil {
+		s.LegacyLogger = logrus.WithField(teleport.ComponentKey, "gcp:fwd")
 	}
 	if s.cloudClientGCP == nil {
 		clients, err := cloud.NewClients()
@@ -141,7 +149,7 @@ func newGCPHandler(ctx context.Context, config HandlerConfig) (*handler, error)
 
 	svc.fwd, err = reverseproxy.New(
 		reverseproxy.WithRoundTripper(config.RoundTripper),
-		reverseproxy.WithLogger(config.Log),
+		reverseproxy.WithLogger(config.LegacyLogger),
 		reverseproxy.WithErrorHandler(svc.formatForwardResponseError),
 	)
 	return svc, trace.Wrap(err)
@@ -164,7 +172,10 @@ func (s *handler) serveHTTP(w http.ResponseWriter, req *http.Request) error {
 	if err != nil {
 		return trace.Wrap(err)
 	}
-	s.Log.Debugf("Processing request, sessionId = %q, gcpServiceAccount = %q", sessionCtx.Identity.RouteToApp.SessionID, sessionCtx.Identity.RouteToApp.GCPServiceAccount)
+	s.Log.DebugContext(req.Context(), "Processing request",
+		"session_id", sessionCtx.Identity.RouteToApp.SessionID,
+		"gcp_service_account", sessionCtx.Identity.RouteToApp.GCPServiceAccount,
+	)
 
 	fwdRequest, err := s.prepareForwardRequest(req, sessionCtx)
 	if err != nil {
@@ -176,13 +187,13 @@ func (s *handler) serveHTTP(w http.ResponseWriter, req *http.Request) error {
 
 	if err := sessionCtx.Audit.OnRequest(req.Context(), sessionCtx, fwdRequest, status, nil); err != nil {
 		// log but don't return the error, because we already handed off request/response handling to the oxy forwarder.
-		s.Log.WithError(err).Warn("Failed to emit audit event.")
+		s.Log.WarnContext(req.Context(), "Failed to emit audit event.", "error", err)
 	}
 	return nil
 }
 
 func (s *handler) formatForwardResponseError(rw http.ResponseWriter, r *http.Request, err error) {
-	s.Log.WithError(err).Debugf("Failed to process request.")
+	s.Log.DebugContext(r.Context(), "Failed to process request.", "error", err)
 	common.SetTeleportAPIErrorHeader(rw, err)
 
 	// Convert trace error type to HTTP and write response.
@@ -224,7 +235,7 @@ func (s *handler) prepareForwardRequest(r *http.Request, sessionCtx *common.Sess
 func (s *handler) replaceAuthHeaders(r *http.Request, sessionCtx *common.SessionContext, reqCopy *http.Request) error {
 	auth := reqCopy.Header.Get("Authorization")
 	if auth == "" {
-		s.Log.Debugf("No Authorization header present, skipping replacement.")
+		s.Log.DebugContext(r.Context(), "No Authorization header present, skipping replacement.")
 		return nil
 	}