Complete lib/srv conversion to slog (#50124)

* Convert lib/srv/server to use slog * Convert lib/srv/desktop to use slog * Add depguard rule to prevent logrus from sneaking back in * fix: authandler log using %v
gravitational · Dec 18, 2024 · 1a7466f · 1a7466f
1 parent 3550c4d
commit 1a7466f
Show file tree

Hide file tree

Showing 7 changed files with 39 additions and 38 deletions.
diff --git a/.golangci.yml b/.golangci.yml
@@ -9,54 +9,54 @@ issues:
   exclude-dirs-use-default: false
   exclude-rules:
     - linters:
-      - gosimple
-      text: "S1002: should omit comparison to bool constant"
+        - gosimple
+      text: 'S1002: should omit comparison to bool constant'
     - linters:
-      - revive
-      text: "exported: exported const"
+        - revive
+      text: 'exported: exported const'
     # TODO(hugoShaka): Remove once https://github.com/dominikh/go-tools/issues/1294 is fixed
     - linters:
-      - unused
+        - unused
       path: 'integrations/operator/controllers/resources/(.+)_controller_test\.go'
     # TODO(codingllama): Remove once we move to grpc.NewClient.
     - linters: [staticcheck]
-      text: "grpc.Dial is deprecated"
+      text: 'grpc.Dial is deprecated'
     - linters: [staticcheck]
-      text: "grpc.DialContext is deprecated"
+      text: 'grpc.DialContext is deprecated'
     # Deprecated gRPC dial options. Related to grpc.NewClient.
     - path: (client/client.go|client/proxy/client_test.go) # api/
       linters: [staticcheck]
       # grpc.FailOnNonTempDialError
       # grpc.WithReturnConnectionError
-      text: "this DialOption is not supported by NewClient"
+      text: 'this DialOption is not supported by NewClient'
     - path: lib/kube/grpc/grpc_test.go
       linters: [staticcheck]
-      text: "grpc.WithBlock is deprecated"
+      text: 'grpc.WithBlock is deprecated'
     - path: lib/observability/tracing/client.go
       linters: [staticcheck]
-      text: "grpc.WithBlock is deprecated"
+      text: 'grpc.WithBlock is deprecated'
     - path: integrations/lib/config.go
       linters: [staticcheck]
-      text: "grpc.WithReturnConnectionError is deprecated"
+      text: 'grpc.WithReturnConnectionError is deprecated'
     - path: lib/service/service_test.go
       linters: [staticcheck]
       # grpc.WithReturnConnectionError
       # grpc.FailOnNonTempDialError
-      text: "this DialOption is not supported by NewClient"
+      text: 'this DialOption is not supported by NewClient'
     - path: integration/client_test.go
       linters: [staticcheck]
-      text: "grpc.WithReturnConnectionError is deprecated"
+      text: 'grpc.WithReturnConnectionError is deprecated'
     - path: integration/integration_test.go
       linters: [staticcheck]
-      text: "grpc.WithBlock is deprecated"
+      text: 'grpc.WithBlock is deprecated'
     - path: lib/multiplexer/multiplexer_test.go
       linters: [staticcheck]
-      text: "grpc.WithBlock is deprecated"
+      text: 'grpc.WithBlock is deprecated'
     - path: provider/provider.go # integrations/terraform
       linters: [staticcheck]
-      text: "grpc.WithReturnConnectionError is deprecated"
+      text: 'grpc.WithReturnConnectionError is deprecated'
     - linters: [govet]
-      text: "non-constant format string in call to github.com/gravitational/trace."
+      text: 'non-constant format string in call to github.com/gravitational/trace.'
   exclude-use-default: true
   max-same-issues: 0
   max-issues-per-linter: 0
@@ -121,6 +121,7 @@ linters-settings:
         files:
           - '**/api/**'
           - '**/e/**'
+          - '**/lib/srv/**'
         deny:
           - pkg: github.com/sirupsen/logrus
             desc: 'use "log/slog" instead'
@@ -130,7 +131,7 @@ linters-settings:
       client-tools:
         files:
           # Tests can do anything
-          - "!$test"
+          - '!$test'
           - '**/tool/tbot/**'
           - '**/lib/tbot/**'
           - '**/tool/tctl/**'
@@ -158,7 +159,7 @@ linters-settings:
       cgo:
         files:
           # Tests can do anything
-          - "!$test"
+          - '!$test'
           - '**/tool/tbot/**'
           - '**/lib/client/**'
           - '!**/lib/integrations/**'
@@ -240,8 +241,8 @@ linters-settings:
     require-specific: true
   revive:
     rules:
-    - name: unused-parameter
-      disabled: true
+      - name: unused-parameter
+        disabled: true
   sloglint:
     context: all
     key-naming-case: snake

diff --git a/lib/auth/windows/certificate_authority.go b/lib/auth/windows/certificate_authority.go
@@ -20,9 +20,9 @@ package windows
 
 import (
 	"context"
+	"log/slog"
 
 	"github.com/gravitational/trace"
-	"github.com/sirupsen/logrus"
 
 	"github.com/gravitational/teleport/api/types"
 	"github.com/gravitational/teleport/lib/auth/authclient"
@@ -45,7 +45,7 @@ type CertificateStoreConfig struct {
 	// LDAPConfig is the ldap configuration
 	LDAPConfig
 	// Log is the logging sink for the service
-	Log logrus.FieldLogger
+	Logger *slog.Logger
 	// ClusterName is the name of this cluster
 	ClusterName string
 	// LC is the LDAPClient
@@ -114,9 +114,9 @@ func (c *CertificateStoreClient) updateCRL(ctx context.Context, crlDER []byte, c
 		); err != nil {
 			return trace.Wrap(err)
 		}
-		c.cfg.Log.Info("Updated CRL for Windows logins via LDAP")
+		c.cfg.Logger.InfoContext(ctx, "Updated CRL for Windows logins via LDAP")
 	} else {
-		c.cfg.Log.Info("Added CRL for Windows logins via LDAP")
+		c.cfg.Logger.InfoContext(ctx, "Added CRL for Windows logins via LDAP")
 	}
 	return nil
 }
diff --git a/lib/srv/authhandlers.go b/lib/srv/authhandlers.go
@@ -633,7 +633,7 @@ func (a *ahLoginChecker) canLoginWithRBAC(cert *ssh.Certificate, ca types.CertAu
 	// Use the server's shutdown context.
 	ctx := a.c.Server.Context()
 
-	a.log.DebugContext(ctx, "Checking permissions for (%v,%v) to login to node with RBAC checks.", teleportUser, osUser)
+	a.log.DebugContext(ctx, "Checking permissions to login to node with RBAC checks", "teleport_user", teleportUser, "os_user", osUser)
 
 	// get roles assigned to this user
 	accessInfo, err := fetchAccessInfo(cert, ca, teleportUser, clusterName)

diff --git a/lib/srv/desktop/rdp/rdpclient/client.go b/lib/srv/desktop/rdp/rdpclient/client.go
@@ -73,6 +73,7 @@ import "C"
 import (
 	"context"
 	"fmt"
+	"log/slog"
 	"os"
 	"runtime/cgo"
 	"sync"
@@ -81,7 +82,6 @@ import (
 	"unsafe"
 
 	"github.com/gravitational/trace"
-	"github.com/sirupsen/logrus"
 
 	"github.com/gravitational/teleport/api/types"
 	"github.com/gravitational/teleport/lib/srv/desktop/tdp"
@@ -98,14 +98,15 @@ func init() {
 	// assume the user knows what they want)
 	rl := os.Getenv("RUST_LOG")
 	if rl == "" {
-		switch l := logrus.GetLevel(); l {
-		case logrus.TraceLevel:
+		ctx := context.Background()
+		switch {
+		case slog.Default().Enabled(ctx, logutils.TraceLevel):
 			rustLogLevel = "trace"
-		case logrus.DebugLevel:
+		case slog.Default().Enabled(ctx, slog.LevelDebug):
 			rustLogLevel = "debug"
-		case logrus.InfoLevel:
+		case slog.Default().Enabled(ctx, slog.LevelInfo):
 			rustLogLevel = "info"
-		case logrus.WarnLevel:
+		case slog.Default().Enabled(ctx, slog.LevelWarn):
 			rustLogLevel = "warn"
 		default:
 			rustLogLevel = "error"

diff --git a/lib/srv/desktop/windows_server.go b/lib/srv/desktop/windows_server.go
@@ -36,7 +36,6 @@ import (
 	"github.com/go-ldap/ldap/v3"
 	"github.com/gravitational/trace"
 	"github.com/jonboulle/clockwork"
-	"github.com/sirupsen/logrus"
 
 	"github.com/gravitational/teleport"
 	apidefaults "github.com/gravitational/teleport/api/defaults"
@@ -382,7 +381,7 @@ func NewWindowsService(cfg WindowsServiceConfig) (*WindowsService, error) {
 	s.ca = windows.NewCertificateStoreClient(windows.CertificateStoreConfig{
 		AccessPoint: s.cfg.AccessPoint,
 		LDAPConfig:  caLDAPConfig,
-		Log:         logrus.NewEntry(logrus.StandardLogger()),
+		Logger:      slog.Default(),
 		ClusterName: s.clusterName,
 		LC:          s.lc,
 	})

diff --git a/lib/srv/server/ec2_watcher.go b/lib/srv/server/ec2_watcher.go
@@ -20,14 +20,14 @@ package server
 
 import (
 	"context"
+	"log/slog"
 	"sync"
 	"time"
 
 	"github.com/aws/aws-sdk-go-v2/aws"
 	"github.com/aws/aws-sdk-go-v2/service/ec2"
 	ec2types "github.com/aws/aws-sdk-go-v2/service/ec2/types"
 	"github.com/gravitational/trace"
-	log "github.com/sirupsen/logrus"
 
 	usageeventsv1 "github.com/gravitational/teleport/api/gen/proto/go/usageevents/v1"
 	"github.com/gravitational/teleport/api/types"
@@ -305,7 +305,7 @@ func newEC2InstanceFetcher(cfg ec2FetcherConfig) *ec2InstanceFetcher {
 			})
 		}
 	} else {
-		log.Debug("Not setting any tag filters as there is a '*:...' tag present and AWS doesnt allow globbing on keys")
+		slog.DebugContext(context.Background(), "Not setting any tag filters as there is a '*:...' tag present and AWS doesnt allow globbing on keys")
 	}
 	var parameters map[string]string
 	if cfg.Matcher.Params == nil {

diff --git a/lib/srv/server/watcher.go b/lib/srv/server/watcher.go
@@ -20,10 +20,10 @@ package server
 
 import (
 	"context"
+	"log/slog"
 	"time"
 
 	"github.com/gravitational/trace"
-	log "github.com/sirupsen/logrus"
 
 	"github.com/gravitational/teleport/api/types"
 )
@@ -80,7 +80,7 @@ func (w *Watcher) sendInstancesOrLogError(instancesColl []Instances, err error)
 		if trace.IsNotFound(err) {
 			return
 		}
-		log.WithError(err).Error("Failed to fetch instances")
+		slog.ErrorContext(context.Background(), "Failed to fetch instances", "error", err)
 		return
 	}
 	for _, inst := range instancesColl {