Skip to content

Commit

Permalink
Migrate logging via cfg.Log to cfg.Logger
Browse files Browse the repository at this point in the history 
Converts logging via the servicecfg.Config to use slog instead of
logrus.
  • Loading branch information
@rosstimothy
rosstimothy committed Mar 28, 2024
1 parent 6c7a700 commit 344e9c6
Show file tree
Hide file tree
Showing 5 changed files with 41 additions and 35 deletions.
56 changes: 30 additions & 26 deletions lib/service/service.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -733,7 +733,7 @@ func waitAndReload(ctx context.Context, cfg servicecfg.Config, srv Process, newT
if !errors.Is(err, ErrTeleportReloading) {
return nil, trace.Wrap(err)
}
cfg.Log.Infof("Started in-process service reload.")
cfg.Logger.InfoContext(ctx, "Started in-process service reload.")
fileDescriptors, err := srv.ExportFileDescriptors()
if err != nil {
warnOnErr(ctx, srv.Close(), cfg.Logger)
Expand All @@ -749,7 +749,7 @@ func waitAndReload(ctx context.Context, cfg servicecfg.Config, srv Process, newT
warnOnErr(ctx, srv.Close(), cfg.Logger)
return nil, trace.Wrap(err, "failed to create a new service")
}
cfg.Log.Infof("Created new process.")
cfg.Logger.InfoContext(ctx, "Created new process.")
if err := newSrv.Start(); err != nil {
warnOnErr(ctx, srv.Close(), cfg.Logger)
return nil, trace.Wrap(err, "failed to start a new service")
Expand All @@ -769,7 +769,7 @@ func waitAndReload(ctx context.Context, cfg servicecfg.Config, srv Process, newT
warnOnErr(ctx, srv.Close(), cfg.Logger)
return nil, trace.BadParameter("the new service has failed to start")
}
cfg.Log.Infof("New service has started successfully.")
cfg.Logger.InfoContext(ctx, "New service has started successfully.")
startCancel()

shutdownTimeout := cfg.Testing.ShutdownTimeout
Expand All @@ -778,7 +778,7 @@ func waitAndReload(ctx context.Context, cfg servicecfg.Config, srv Process, newT
// longer running connections.
shutdownTimeout = defaults.DefaultGracefulShutdownTimeout
}
cfg.Log.Infof("Shutting down the old service with timeout %v.", shutdownTimeout)
cfg.Logger.InfoContext(ctx, "Gracefully shutting down the old service.", "grace_period", shutdownTimeout)
// After the new process has started, initiate the graceful shutdown of the old process
// new process could have generated connections to the new process's server
// so not all connections can be kept forever.
Expand All @@ -791,7 +791,7 @@ func waitAndReload(ctx context.Context, cfg servicecfg.Config, srv Process, newT
// connections can keep hanging the old auth service and prevent
// the services from shutting down, so abort the graceful way
// after some time to keep going.
cfg.Log.Infof("Some connections to the old service were aborted after timeout of %v.", shutdownTimeout)
cfg.Logger.InfoContext(ctx, "Some connections to the old service were aborted after exceeding grace period.", "grace_period", shutdownTimeout)
// Make sure that all parts of the service have exited, this function
// can not allow execution to continue if the shutdown is not complete,
// otherwise subsequent Run executions will hold system resources in case
Expand All @@ -803,7 +803,7 @@ func waitAndReload(ctx context.Context, cfg servicecfg.Config, srv Process, newT
return nil, trace.BadParameter("the old service has failed to exit.")
}
} else {
cfg.Log.Infof("The old service was successfully shut down gracefully.")
cfg.Logger.InfoContext(ctx, "The old service was successfully shut down gracefully.")
}

return newSrv, nil
Expand Down Expand Up @@ -860,7 +860,7 @@ func NewTeleport(cfg *servicecfg.Config) (*TeleportProcess, error) {
err := os.MkdirAll(cfg.DataDir, os.ModeDir|0o700)
if err != nil {
if errors.Is(err, fs.ErrPermission) {
cfg.Log.Errorf("Teleport does not have permission to write to: %v. Ensure that you are running as a user with appropriate permissions.", cfg.DataDir)
cfg.Logger.ErrorContext(context.Background(), "Teleport does not have permission to write to the data directory. Ensure that you are running as a user with appropriate permissions.", "data_dir", cfg.DataDir)
}
return nil, trace.ConvertSystemError(err)
}
Expand Down Expand Up @@ -899,7 +899,7 @@ func NewTeleport(cfg *servicecfg.Config) (*TeleportProcess, error) {

_, err = uuid.Parse(cfg.HostUUID)
if err != nil && !aws.IsEC2NodeID(cfg.HostUUID) {
cfg.Log.Warnf("Host UUID %q is not a true UUID (not eligible for UUID-based proxying)", cfg.HostUUID)
cfg.Logger.WarnContext(supervisor.ExitContext(), "Host UUID is not a true UUID (not eligible for UUID-based proxying)", "host_uuid", cfg.HostUUID)
}

if cfg.Clock == nil {
Expand All @@ -926,12 +926,12 @@ func NewTeleport(cfg *servicecfg.Config) (*TeleportProcess, error) {
if err == nil {
cloudHostname = strings.ReplaceAll(cloudHostname, " ", "_")
if utils.IsValidHostname(cloudHostname) {
cfg.Log.Infof("Found %q tag in cloud instance. Using %q as hostname.", types.CloudHostnameTag, cloudHostname)
cfg.Logger.InfoContext(supervisor.ExitContext(), "Overriding hostname with value from cloud tag TeleportHostname.", "hostname", cloudHostname)
cfg.Hostname = cloudHostname

// cloudHostname exists but is not a valid hostname.
} else if cloudHostname != "" {
cfg.Log.Infof("Found %q tag in cloud instance, but %q is not a valid hostname.", types.CloudHostnameTag, cloudHostname)
cfg.Logger.InfoContext(supervisor.ExitContext(), "Found invalid hostname in cloud tag TeleportHostname.", "hostname", cloudHostname)
}
} else if !trace.IsNotFound(err) {
return nil, trace.Wrap(err)
Expand Down Expand Up @@ -4221,7 +4221,7 @@ func (process *TeleportProcess) initProxyEndpoint(conn *Connector) error {
})

if listeners.reverseTunnelMux != nil {
if minimalWebServer, err = process.initMinimalReverseTunnel(listeners, tlsConfigWeb, cfg, webConfig, process.log.WithField(teleport.ComponentKey, teleport.Component(teleport.ComponentReverseTunnelServer, process.id))); err != nil {
if minimalWebServer, err = process.initMinimalReverseTunnel(listeners, tlsConfigWeb, cfg, webConfig); err != nil {
return trace.Wrap(err)
}
}
Expand Down Expand Up @@ -4886,7 +4886,8 @@ func (process *TeleportProcess) getPROXYSigner(ident *auth.Identity) (multiplexe
return proxySigner, nil
}

func (process *TeleportProcess) initMinimalReverseTunnel(listeners *proxyListeners, tlsConfigWeb *tls.Config, cfg *servicecfg.Config, webConfig web.Config, log *logrus.Entry) (*web.Server, error) {
func (process *TeleportProcess) initMinimalReverseTunnel(listeners *proxyListeners, tlsConfigWeb *tls.Config, cfg *servicecfg.Config, webConfig web.Config) (*web.Server, error) {
logger := process.logger.With(teleport.ComponentKey, teleport.Component(teleport.ComponentReverseTunnelServer, process.id))
internalListener := listeners.minimalWeb
if !cfg.Proxy.DisableTLS {
internalListener = tls.NewListener(internalListener, tlsConfigWeb)
Expand All @@ -4912,14 +4913,16 @@ func (process *TeleportProcess) initMinimalReverseTunnel(listeners *proxyListene
minimalProxyLimiter.WrapHandle(minimalWebHandler)

process.RegisterCriticalFunc("proxy.reversetunnel.tls", func() error {
log.Infof("TLS multiplexer is starting on %v.", cfg.Proxy.ReverseTunnelListenAddr.Addr)
logger.InfoContext(process.ExitContext(), "TLS multiplexer is starting.", "listen_address", cfg.Proxy.ReverseTunnelListenAddr.Addr)
if err := minimalListener.Serve(); !trace.IsConnectionProblem(err) {
log.WithError(err).Warn("TLS multiplexer error.")
logger.WarnContext(process.ExitContext(), "TLS multiplexer error.", "error", err)
}
log.Info("TLS multiplexer exited.")
logger.InfoContext(process.ExitContext(), "TLS multiplexer exited.")
return nil
})

log := process.log.WithField(teleport.ComponentKey, teleport.Component(teleport.ComponentReverseTunnelServer, process.id))

minimalWebServer, err := web.NewServer(web.ServerConfig{
Server: &http.Server{
Handler: httplib.MakeTracingHandler(minimalProxyLimiter, teleport.ComponentProxy),
Expand All @@ -4937,12 +4940,12 @@ func (process *TeleportProcess) initMinimalReverseTunnel(listeners *proxyListene
}

process.RegisterCriticalFunc("proxy.reversetunnel.web", func() error {
log.Infof("Minimal web proxy service %s:%s is starting on %v.", teleport.Version, teleport.Gitref, cfg.Proxy.ReverseTunnelListenAddr.Addr)
logger.InfoContext(process.ExitContext(), "Minimal web proxy service is starting.", "version", teleport.Version, "git_ref", teleport.Gitref, "listen_address", cfg.Proxy.ReverseTunnelListenAddr.Addr)
defer minimalWebHandler.Close()
if err := minimalWebServer.Serve(minimalListener.Web()); err != nil && !errors.Is(err, http.ErrServerClosed) {
log.Warningf("Error while serving web requests: %v", err)
logger.WarnContext(process.ExitContext(), "Error while serving web requests", "error", err)
}
log.Info("Exited.")
logger.InfoContext(process.ExitContext(), "Exited.")
return nil
})

Expand All @@ -4965,7 +4968,7 @@ func (process *TeleportProcess) setupProxyTLSConfig(conn *Connector, tsrv revers
var tlsConfig *tls.Config
acmeCfg := process.Config.Proxy.ACME
if acmeCfg.Enabled {
process.Config.Log.Infof("Managing certs using ACME https://datatracker.ietf.org/doc/rfc8555/.")
process.Config.Logger.InfoContext(process.ExitContext(), "Managing certs using ACME https://datatracker.ietf.org/doc/rfc8555/.")

acmePath := filepath.Join(process.Config.DataDir, teleport.ComponentACME)
if err := os.MkdirAll(acmePath, teleport.PrivateDirMode); err != nil {
Expand Down Expand Up @@ -5648,7 +5651,8 @@ func (process *TeleportProcess) Close() error {
// initSelfSignedHTTPSCert generates and self-signs a TLS key+cert pair for https connection
// to the proxy server.
func initSelfSignedHTTPSCert(cfg *servicecfg.Config) (err error) {
cfg.Log.Warningf("No TLS Keys provided, using self-signed certificate.")
ctx := context.Background()
cfg.Logger.WarnContext(ctx, "No TLS Keys provided, using self-signed certificate.")

keyPath := filepath.Join(cfg.DataDir, defaults.SelfSignedKeyPath)
certPath := filepath.Join(cfg.DataDir, defaults.SelfSignedCertPath)
Expand All @@ -5666,7 +5670,7 @@ func initSelfSignedHTTPSCert(cfg *servicecfg.Config) (err error) {
if !os.IsNotExist(err) {
return trace.Wrap(err, "unrecognized error reading certs")
}
cfg.Log.Warningf("Generating self-signed key and cert to %v %v.", keyPath, certPath)
cfg.Logger.WarnContext(ctx, "Generating self-signed key and cert.", "key_path", keyPath, "cert_path", certPath)

hosts := []string{cfg.Hostname, "localhost"}
var ips []string
Expand All @@ -5676,7 +5680,7 @@ func initSelfSignedHTTPSCert(cfg *servicecfg.Config) (err error) {
proxyHost, _, err := net.SplitHostPort(addr.String())
if err != nil {
// log and skip error since this is a nice to have
cfg.Log.Warnf("Error parsing proxy.public_address %v, skipping adding to self-signed cert: %v", addr.String(), err)
cfg.Logger.WarnContext(ctx, "Error parsing proxy.public_address, skipping adding to self-signed cert", "public_address", addr.String(), "error", err)
continue
}
// If the address is a IP have it added as IP SAN
Expand Down Expand Up @@ -5841,15 +5845,15 @@ func readOrGenerateHostID(ctx context.Context, cfg *servicecfg.Config, kubeBacke
if err != nil {
if !trace.IsNotFound(err) {
if errors.Is(err, fs.ErrPermission) {
cfg.Log.Errorf("Teleport does not have permission to write to: %v. Ensure that you are running as a user with appropriate permissions.", cfg.DataDir)
cfg.Logger.ErrorContext(ctx, "Teleport does not have permission to write to the data directory. Ensure that you are running as a user with appropriate permissions.", "data_dir", cfg.DataDir)
}
return trace.Wrap(err)
}
// if there's no host uuid initialized yet, try to read one from the
// one of the identities
if len(cfg.Identities) != 0 {
cfg.HostUUID = cfg.Identities[0].ID.HostUUID
cfg.Log.Infof("Taking host UUID from first identity: %v.", cfg.HostUUID)
cfg.Logger.InfoContext(ctx, "Taking host UUID from first identity.", "host_uuid", cfg.HostUUID)
} else {
switch cfg.JoinMethod {
case types.JoinMethodToken,
Expand Down Expand Up @@ -5881,7 +5885,7 @@ func readOrGenerateHostID(ctx context.Context, cfg *servicecfg.Config, kubeBacke
default:
return trace.BadParameter("unknown join method %q", cfg.JoinMethod)
}
cfg.Log.Infof("Generating new host UUID: %v.", cfg.HostUUID)
cfg.Logger.InfoContext(ctx, "Generating new host UUID", "host_uuid", cfg.HostUUID)
}
// persistHostUUIDToStorages will persist the host_uuid to the local storage
// and to Kubernetes Secret if this process is running on a Kubernetes Cluster.
Expand Down Expand Up @@ -5936,7 +5940,7 @@ func readHostIDFromStorages(ctx context.Context, dataDir string, kubeBackend kub
func persistHostIDToStorages(ctx context.Context, cfg *servicecfg.Config, kubeBackend kubernetesBackend) error {
if err := utils.WriteHostUUID(cfg.DataDir, cfg.HostUUID); err != nil {
if errors.Is(err, fs.ErrPermission) {
cfg.Log.Errorf("Teleport does not have permission to write to: %v. Ensure that you are running as a user with appropriate permissions.", cfg.DataDir)
cfg.Logger.ErrorContext(ctx, "Teleport does not have permission to write to the data directory. Ensure that you are running as a user with appropriate permissions.", "data_dir", cfg.DataDir)
}
return trace.Wrap(err)
}
Expand Down
4 changes: 2 additions & 2 deletions lib/service/service_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ func TestServiceSelfSignedHTTPS(t *testing.T) {
cfg := &servicecfg.Config{
DataDir: t.TempDir(),
Hostname: "example.com",
Log: utils.WrapLogger(logrus.New().WithField("test", "TestServiceSelfSignedHTTPS")),
Logger: slog.Default(),
}
require.NoError(t, initSelfSignedHTTPSCert(cfg))
require.Len(t, cfg.Proxy.KeyPairs, 1)
Expand Down Expand Up @@ -1157,7 +1157,7 @@ func Test_readOrGenerateHostID(t *testing.T) {

cfg := &servicecfg.Config{
DataDir: dataDir,
Log: logrus.New(),
Logger: slog.Default(),
JoinMethod: types.JoinMethodToken,
Identities: tt.args.identity,
}
Expand Down
7 changes: 4 additions & 3 deletions lib/service/servicecfg/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
package servicecfg

import (
"context"
"io"
"log/slog"
"net"
Expand Down Expand Up @@ -536,7 +537,7 @@ func ApplyDefaults(cfg *Config) {
hostname, err := os.Hostname()
if err != nil {
hostname = "localhost"
cfg.Log.Errorf("Failed to determine hostname: %v.", err)
cfg.Logger.ErrorContext(context.Background(), "Failed to determine hostname", "error", err)
}

// Global defaults.
Expand Down Expand Up @@ -721,7 +722,7 @@ func validateAuthOrProxyServices(cfg *Config) error {
if haveProxyServer {
port := cfg.ProxyServer.Port(0)
if port == defaults.AuthListenPort {
cfg.Log.Warnf("config: proxy_server is pointing to port %d, is this the auth server address?", defaults.AuthListenPort)
cfg.Logger.WarnContext(context.Background(), "config: proxy_server is pointing to port 3025, is this the auth server address?")
}
}

Expand All @@ -730,7 +731,7 @@ func validateAuthOrProxyServices(cfg *Config) error {
checkPorts := []int{defaults.HTTPListenPort, teleport.StandardHTTPSPort}
for _, port := range checkPorts {
if authServerPort == port {
cfg.Log.Warnf("config: auth_server is pointing to port %d, is this the proxy server address?", port)
cfg.Logger.WarnContext(context.Background(), "config: auth_server is pointing to port 3080 or 443, is this the proxy server address?")
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion lib/tbot/testhelpers/srv.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ func MakeAndRunTestAuthServer(t *testing.T, log utils.Logger, fc *config.FileCon
require.NoError(t, auth.Start())

t.Cleanup(func() {
cfg.Log.Info("Cleaning up Auth Server.")
cfg.Logger.InfoContext(context.Background(), "Cleaning up Auth Server.")
auth.Close()
})

Expand Down
7 changes: 4 additions & 3 deletions tool/teleport/common/teleport.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -640,12 +640,13 @@ func OnStart(clf config.CommandLineFlags, config *servicecfg.Config) error {
configFileUsed = defaults.ConfigFilePath
}

ctx := context.Background()
if configFileUsed == "" {
config.Log.Infof("Starting Teleport v%s", teleport.Version)
config.Logger.InfoContext(ctx, "Starting Teleport", "version", teleport.Version)
} else {
config.Log.Infof("Starting Teleport v%s with a config file located at %q", teleport.Version, configFileUsed)
config.Logger.InfoContext(ctx, "Starting Teleport with a config file", "version", teleport.Version, "config_file", configFileUsed)
}
return service.Run(context.TODO(), *config, nil)
return service.Run(ctx, *config, nil)
}

// onStatus is the handler for "status" CLI command
Expand Down

0 comments on commit 344e9c6

Please sign in to comment.