Skip to content

Commit

Permalink
Convert lib/teleterm to use slog
Browse files Browse the repository at this point in the history
  • Loading branch information
rosstimothy authored and github-actions committed Jan 8, 2025
1 parent 3a32d04 commit 6bae454
Show file tree
Hide file tree
Showing 29 changed files with 158 additions and 140 deletions.
19 changes: 10 additions & 9 deletions lib/client/clientcache/clientcache.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,11 @@ package clientcache

import (
"context"
"log/slog"
"slices"
"sync"

"github.com/gravitational/trace"
"github.com/sirupsen/logrus"
"golang.org/x/sync/singleflight"

"github.com/gravitational/teleport"
Expand Down Expand Up @@ -53,7 +53,7 @@ type RetryWithReloginFunc func(ctx context.Context, tc *client.TeleportClient, f
type Config struct {
NewClientFunc NewClientFunc
RetryWithReloginFunc RetryWithReloginFunc
Log logrus.FieldLogger
Logger *slog.Logger
}

func (c *Config) checkAndSetDefaults() error {
Expand All @@ -63,8 +63,8 @@ func (c *Config) checkAndSetDefaults() error {
if c.RetryWithReloginFunc == nil {
return trace.BadParameter("RetryWithReloginFunc is required")
}
if c.Log == nil {
c.Log = logrus.WithField(teleport.ComponentKey, "clientcache")
if c.Logger == nil {
c.Logger = slog.With(teleport.ComponentKey, "clientcache")
}
return nil
}
Expand Down Expand Up @@ -99,7 +99,7 @@ func (c *Cache) Get(ctx context.Context, profileName, leafClusterName string) (*
k := key{profile: profileName, leafCluster: leafClusterName}
groupClt, err, _ := c.group.Do(k.String(), func() (any, error) {
if fromCache := c.getFromCache(k); fromCache != nil {
c.cfg.Log.WithField("cluster", k).Debug("Retrieved client from cache.")
c.cfg.Logger.DebugContext(ctx, "Retrieved client from cache", "cluster", k)
return fromCache, nil
}

Expand All @@ -123,7 +123,7 @@ func (c *Cache) Get(ctx context.Context, profileName, leafClusterName string) (*
// Save the client in the cache, so we don't have to build a new connection next time.
c.addToCache(k, newClient)

c.cfg.Log.WithField("cluster", k).Info("Added client to cache.")
c.cfg.Logger.InfoContext(ctx, "Added client to cache", "cluster", k)

return newClient, nil
})
Expand Down Expand Up @@ -159,9 +159,10 @@ func (c *Cache) ClearForRoot(profileName string) error {
}
}

c.cfg.Log.WithFields(
logrus.Fields{"cluster": profileName, "clients": deleted},
).Info("Invalidated cached clients for root cluster.")
c.cfg.Logger.InfoContext(context.Background(), "Invalidated cached clients for root cluster",
"cluster", profileName,
"clients", deleted,
)

return trace.NewAggregate(errors...)

Expand Down
24 changes: 15 additions & 9 deletions lib/client/db/dbcmd/dbcmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ package dbcmd
import (
"context"
"fmt"
"log/slog"
"net/url"
"os"
"os/exec"
Expand All @@ -30,7 +31,6 @@ import (
"strings"

"github.com/gravitational/trace"
"github.com/sirupsen/logrus"
"go.mongodb.org/mongo-driver/x/mongo/driver/connstring"

"github.com/gravitational/teleport/api/constants"
Expand Down Expand Up @@ -143,8 +143,8 @@ func NewCmdBuilder(tc *client.TeleportClient, profile *client.ProfileStatus,
host, port = tc.DatabaseProxyHostPort(db)
}

if options.log == nil {
options.log = logrus.NewEntry(logrus.StandardLogger())
if options.logger == nil {
options.logger = slog.Default()
}

if options.exe == nil {
Expand Down Expand Up @@ -256,8 +256,11 @@ func (c *CLICommandBuilder) getPostgresCommand() *exec.Cmd {
func (c *CLICommandBuilder) getCockroachCommand() *exec.Cmd {
// If cockroach CLI client is not available, fallback to psql.
if _, err := c.options.exe.LookPath(cockroachBin); err != nil {
c.options.log.Debugf("Couldn't find %q client in PATH, falling back to %q: %v.",
cockroachBin, postgresBin, err)
c.options.logger.DebugContext(context.Background(), "Couldn't find cockroach client in PATH, falling back to postgres client",
"cockroach_client", cockroachBin,
"postgres_client", postgresBin,
"error", err,
)
return c.getPostgresCommand()
}
return exec.Command(cockroachBin, "sql", "--url", c.getPostgresConnString())
Expand Down Expand Up @@ -560,7 +563,10 @@ func (c *CLICommandBuilder) getMongoAddress() string {
// force a different timeout for debugging purpose or extreme situations.
serverSelectionTimeoutMS := "5000"
if envValue := os.Getenv(envVarMongoServerSelectionTimeoutMS); envValue != "" {
c.options.log.Infof("Using environment variable %s=%s.", envVarMongoServerSelectionTimeoutMS, envValue)
c.options.logger.InfoContext(context.Background(), "Using server selection timeout value from environment variable",
"environment_variable", envVarMongoServerSelectionTimeoutMS,
"server_selection_timeout", envValue,
)
serverSelectionTimeoutMS = envValue
}
query.Set("serverSelectionTimeoutMS", serverSelectionTimeoutMS)
Expand Down Expand Up @@ -905,7 +911,7 @@ type connectionCommandOpts struct {
noTLS bool
printFormat bool
tolerateMissingCLIClient bool
log *logrus.Entry
logger *slog.Logger
exe Execer
password string
gcp types.GCPCloudSQL
Expand Down Expand Up @@ -969,9 +975,9 @@ func WithPrintFormat() ConnectCommandFunc {

// WithLogger is the connect command option that allows the caller to pass a logger that will be
// used by CLICommandBuilder.
func WithLogger(log *logrus.Entry) ConnectCommandFunc {
func WithLogger(log *slog.Logger) ConnectCommandFunc {
return func(opts *connectionCommandOpts) {
opts.log = log
opts.logger = log
}
}

Expand Down
9 changes: 5 additions & 4 deletions lib/teleterm/apiserver/apiserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,12 @@
package apiserver

import (
"context"
"fmt"
"log/slog"
"net"

"github.com/gravitational/trace"
log "github.com/sirupsen/logrus"
"google.golang.org/grpc"

api "github.com/gravitational/teleport/gen/proto/go/teleport/lib/teleterm/v1"
Expand Down Expand Up @@ -70,7 +71,7 @@ func New(cfg Config) (*APIServer, error) {
}

grpcServer := grpc.NewServer(cfg.TshdServerCreds,
grpc.ChainUnaryInterceptor(withErrorHandling(cfg.Log)),
grpc.ChainUnaryInterceptor(withErrorHandling(cfg.Logger)),
grpc.MaxConcurrentStreams(defaults.GRPCMaxConcurrentStreams),
)

Expand All @@ -96,7 +97,7 @@ func (s *APIServer) Stop() {
// immediate. Closing the VNet service before the gRPC server gives some time for the VNet admin
// process to notice that the client is gone and shut down as well.
if err := s.vnetService.Close(); err != nil {
log.WithError(err).Error("Error while closing VNet service")
slog.ErrorContext(context.Background(), "Error while closing VNet service", "error", err)
}

s.grpcServer.GracefulStop()
Expand All @@ -120,7 +121,7 @@ func newListener(hostAddr string, listeningC chan<- utils.NetAddr) (net.Listener
listeningC <- addr
}

log.Infof("tsh daemon is listening on %v.", addr.FullAddress())
slog.InfoContext(context.Background(), "tsh daemon listener created", "listen_addr", addr.FullAddress())

return lis, nil
}
Expand Down
11 changes: 6 additions & 5 deletions lib/teleterm/apiserver/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,10 @@
package apiserver

import (
"log/slog"

"github.com/gravitational/trace"
"github.com/jonboulle/clockwork"
"github.com/sirupsen/logrus"
"google.golang.org/grpc"

"github.com/gravitational/teleport"
Expand All @@ -39,8 +40,8 @@ type Config struct {
Daemon *daemon.Service
ClusterIDCache *clusteridcache.Cache
InstallationID string
// Log is a component logger
Log logrus.FieldLogger
// Logger is a component logger
Logger *slog.Logger
TshdServerCreds grpc.ServerOption
Clock clockwork.Clock
// ListeningC propagates the address on which the gRPC server listens. Mostly useful in tests, as
Expand All @@ -66,8 +67,8 @@ func (c *Config) CheckAndSetDefaults() error {
return trace.BadParameter("missing TshdServerCreds")
}

if c.Log == nil {
c.Log = logrus.WithField(teleport.ComponentKey, "conn:apiserver")
if c.Logger == nil {
c.Logger = slog.With(teleport.ComponentKey, "conn:apiserver")
}

if c.InstallationID == "" {
Expand Down
6 changes: 3 additions & 3 deletions lib/teleterm/apiserver/middleware.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,14 @@ package apiserver

import (
"context"
"log/slog"

"github.com/gravitational/trace/trail"
"github.com/sirupsen/logrus"
"google.golang.org/grpc"
)

// withErrorHandling is gRPC middleware that maps internal errors to proper gRPC error codes
func withErrorHandling(log logrus.FieldLogger) grpc.UnaryServerInterceptor {
func withErrorHandling(log *slog.Logger) grpc.UnaryServerInterceptor {
return func(
ctx context.Context,
req interface{},
Expand All @@ -36,7 +36,7 @@ func withErrorHandling(log logrus.FieldLogger) grpc.UnaryServerInterceptor {
) (interface{}, error) {
resp, err := handler(ctx, req)
if err != nil {
log.WithError(err).Error("Request failed.")
log.ErrorContext(ctx, "Request failed", "error", err)
return resp, trail.ToGRPC(err)
}

Expand Down
10 changes: 4 additions & 6 deletions lib/teleterm/clusters/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,10 @@ package clusters

import (
"context"
"log/slog"

"github.com/gravitational/trace"
"github.com/jonboulle/clockwork"
"github.com/sirupsen/logrus"
"golang.org/x/sync/errgroup"
"google.golang.org/grpc"
"google.golang.org/grpc/metadata"
Expand All @@ -49,8 +49,8 @@ type Cluster struct {
Name string
// ProfileName is the name of the tsh profile
ProfileName string
// Log is a component logger
Log *logrus.Entry
// Logger is a component logger
Logger *slog.Logger
// dir is the directory where cluster certificates are stored
dir string
// Status is the cluster status
Expand Down Expand Up @@ -192,9 +192,7 @@ func (c *Cluster) GetWithDetails(ctx context.Context, authClient authclient.Clie
return roles, nil
})
if err != nil {
c.Log.
WithError(err).
Warn("Failed to calculate trusted device requirement")
c.Logger.WarnContext(ctx, "Failed to calculate trusted device requirement", "error", err)
}

roleSet := services.NewRoleSet(roles...)
Expand Down
14 changes: 7 additions & 7 deletions lib/teleterm/clusters/cluster_auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,10 @@ import (
"context"
"encoding/json"
"errors"
"log/slog"
"sort"

"github.com/gravitational/trace"
"github.com/sirupsen/logrus"

"github.com/gravitational/teleport/api/client/webclient"
"github.com/gravitational/teleport/api/constants"
Expand All @@ -47,9 +47,9 @@ func (c *Cluster) SyncAuthPreference(ctx context.Context) (*webclient.WebConfigA
}
pingResponseJSON, err := json.Marshal(pingResponse)
if err != nil {
c.Log.WithError(err).Debugln("Could not marshal ping response to JSON")
c.Logger.DebugContext(ctx, "Could not marshal ping response to JSON", "error", err)
} else {
c.Log.WithField("response", string(pingResponseJSON)).Debugln("Got ping response")
c.Logger.DebugContext(ctx, "Got ping response", "response", string(pingResponseJSON))
}

if err := c.clusterClient.SaveProfile(false); err != nil {
Expand Down Expand Up @@ -227,7 +227,7 @@ func (c *Cluster) passwordlessLogin(stream api.TerminalService_LoginPasswordless
response, err := client.SSHAgentPasswordlessLogin(ctx, client.SSHLoginPasswordless{
SSHLogin: sshLogin,
AuthenticatorAttachment: c.clusterClient.AuthenticatorAttachment,
CustomPrompt: newPwdlessLoginPrompt(ctx, c.Log, stream),
CustomPrompt: newPwdlessLoginPrompt(ctx, c.Logger, stream),
WebauthnLogin: c.clusterClient.WebauthnLogin,
})
if err != nil {
Expand All @@ -239,11 +239,11 @@ func (c *Cluster) passwordlessLogin(stream api.TerminalService_LoginPasswordless

// pwdlessLoginPrompt is a implementation for wancli.LoginPrompt for teleterm passwordless logins.
type pwdlessLoginPrompt struct {
log *logrus.Entry
log *slog.Logger
Stream api.TerminalService_LoginPasswordlessServer
}

func newPwdlessLoginPrompt(ctx context.Context, log *logrus.Entry, stream api.TerminalService_LoginPasswordlessServer) *pwdlessLoginPrompt {
func newPwdlessLoginPrompt(ctx context.Context, log *slog.Logger, stream api.TerminalService_LoginPasswordlessServer) *pwdlessLoginPrompt {
return &pwdlessLoginPrompt{
log: log,
Stream: stream,
Expand Down Expand Up @@ -283,7 +283,7 @@ func (p *pwdlessLoginPrompt) ackTouch() error {
// The current gRPC message type switch in teleterm client code will reject
// any new message types, making this difficult to add without breaking
// older clients.
p.log.Debug("Detected security key tap")
p.log.DebugContext(context.Background(), "Detected security key tap")
return nil
}

Expand Down
11 changes: 4 additions & 7 deletions lib/teleterm/clusters/cluster_auth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,20 +20,17 @@ package clusters

import (
"context"
"log/slog"
"testing"

"github.com/gravitational/trace"
"github.com/sirupsen/logrus"
"github.com/stretchr/testify/require"
"google.golang.org/grpc"

"github.com/gravitational/teleport"
api "github.com/gravitational/teleport/gen/proto/go/teleport/lib/teleterm/v1"
wancli "github.com/gravitational/teleport/lib/auth/webauthncli"
)

var log = logrus.WithField(teleport.ComponentKey, "cluster_auth_test")

func TestPwdlessLoginPrompt_PromptPIN(t *testing.T) {
stream := &mockLoginPwdlessStream{}

Expand All @@ -49,7 +46,7 @@ func TestPwdlessLoginPrompt_PromptPIN(t *testing.T) {
}}, nil
}

prompt := newPwdlessLoginPrompt(context.Background(), log, stream)
prompt := newPwdlessLoginPrompt(context.Background(), slog.Default(), stream)
pin, err := prompt.PromptPIN()
require.NoError(t, err)
require.Equal(t, "1234", pin)
Expand All @@ -74,7 +71,7 @@ func TestPwdlessLoginPrompt_PromptTouch(t *testing.T) {
return nil
}

prompt := newPwdlessLoginPrompt(context.Background(), log, stream)
prompt := newPwdlessLoginPrompt(context.Background(), slog.Default(), stream)
ackTouch, err := prompt.PromptTouch()
require.NoError(t, err)
require.NoError(t, ackTouch())
Expand Down Expand Up @@ -110,7 +107,7 @@ func TestPwdlessLoginPrompt_PromptCredential(t *testing.T) {
}}, nil
}

prompt := newPwdlessLoginPrompt(context.Background(), log, stream)
prompt := newPwdlessLoginPrompt(context.Background(), slog.Default(), stream)
cred, err := prompt.PromptCredential(unsortedCreds)
require.NoError(t, err)
require.Equal(t, "foo", cred.User.Name)
Expand Down
Loading

0 comments on commit 6bae454

Please sign in to comment.