Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Convert resource reconciler to use slog #47726

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion lib/kube/proxy/watcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ package proxy

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

Expand All @@ -45,7 +46,8 @@ func (s *TLSServer) startReconciler(ctx context.Context) (err error) {
OnCreate: s.onCreate,
OnUpdate: s.onUpdate,
OnDelete: s.onDelete,
Log: s.log,
// TODO(tross): update to use the server logger once it has been converted to slog
Logger: slog.With("kind", types.KindKubernetesCluster),
})
if err != nil {
return trace.Wrap(err)
Expand Down
40 changes: 20 additions & 20 deletions lib/services/reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,14 @@ package services

import (
"context"
"log/slog"

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

"github.com/gravitational/teleport"
"github.com/gravitational/teleport/api/types"
logutils "github.com/gravitational/teleport/lib/utils/log"
)

// Matcher is used by reconciler to match resources.
Expand Down Expand Up @@ -53,7 +55,10 @@ type GenericReconcilerConfig[K comparable, T any] struct {
// OnDelete is called when an existing resource is deleted.
OnDelete func(context.Context, T) error
// Log is the reconciler's logger.
// TODO(tross) remove this when all components in e have been updated
Log logrus.FieldLogger
// Logger emits log messages.
Logger *slog.Logger
}

// CheckAndSetDefaults validates the reconciler configuration and sets defaults.
Expand All @@ -79,8 +84,8 @@ func (c *GenericReconcilerConfig[K, T]) CheckAndSetDefaults() error {
if c.CompareResources == nil {
c.CompareResources = CompareResources[T]
}
if c.Log == nil {
c.Log = logrus.WithField(teleport.ComponentKey, "reconciler")
if c.Logger == nil {
c.Logger = slog.With(teleport.ComponentKey, "reconciler")
}
return nil
}
Expand All @@ -91,13 +96,8 @@ func NewGenericReconciler[K comparable, T any](cfg GenericReconcilerConfig[K, T]
return nil, trace.Wrap(err)
}
return &GenericReconciler[K, T]{
cfg: cfg,
// We do a WithFields here to force this into a *logrus.Entry, which has the ability to
// log at the Trace level. If we were to change this in ReconcilerConfig, we'd have to
// refactor existing code to use *logrus.Entry instead of logrus.FieldLogger, and with
// the eventual change to slog, it seems easier to do this for now until this can be
// changed to slog.
log: cfg.Log.WithFields(nil),
cfg: cfg,
logger: cfg.Logger,
}, nil
}

Expand All @@ -107,8 +107,8 @@ func NewGenericReconciler[K comparable, T any](cfg GenericReconcilerConfig[K, T]
// It's used in combination with watchers by agents (app, database, desktop)
// to enable dynamically registered resources.
type GenericReconciler[K comparable, T any] struct {
cfg GenericReconcilerConfig[K, T]
log *logrus.Entry
cfg GenericReconcilerConfig[K, T]
logger *slog.Logger
}

// Reconcile reconciles currently registered resources with new resources and
Expand All @@ -117,8 +117,8 @@ func (r *GenericReconciler[K, T]) Reconcile(ctx context.Context) error {
currentResources := r.cfg.GetCurrentResources()
newResources := r.cfg.GetNewResources()

r.log.Debugf("Reconciling %v current resources with %v new resources.",
len(currentResources), len(newResources))
r.logger.DebugContext(ctx, "Reconciling current resources with new resources",
"current_resource_count", len(currentResources), "new_resource_count", len(newResources))

var errs []error

Expand Down Expand Up @@ -151,7 +151,7 @@ func (r *GenericReconciler[K, T]) processRegisteredResource(ctx context.Context,
if err != nil {
return trace.Wrap(err)
}
r.log.Infof("%v %v removed, deleting.", kind, key)
r.logger.InfoContext(ctx, "Resource was removed, deleting", "name", key)
if err := r.cfg.OnDelete(ctx, registered); err != nil {
return trace.Wrap(err, "failed to delete %v %v", kind, key)
}
Expand All @@ -171,13 +171,13 @@ func (r *GenericReconciler[K, T]) processNewResource(ctx context.Context, curren
return trace.Wrap(err)
}
if r.cfg.Matcher(newT) {
r.log.Infof("%v %v matches, creating.", kind, key)
r.logger.InfoContext(ctx, "New resource matches, creating", "name", key)
if err := r.cfg.OnCreate(ctx, newT); err != nil {
return trace.Wrap(err, "failed to create %v %v", kind, key)
}
return nil
}
r.log.Debugf("%v %v doesn't match, not creating.", kind, key)
r.logger.DebugContext(ctx, "New resource doesn't match, not creating", "name", key)
return nil
}

Expand All @@ -191,7 +191,7 @@ func (r *GenericReconciler[K, T]) processNewResource(ctx context.Context, curren
return trace.Wrap(err)
}
if registeredOrigin != newOrigin {
r.log.Warnf("%v has different origin (%v vs %v), not updating.", key, newOrigin, registeredOrigin)
r.logger.WarnContext(ctx, "New resource has different origin, not updating", "name", key, "new_origin", newOrigin, "existing_origin", registeredOrigin)
return nil
}

Expand All @@ -203,20 +203,20 @@ func (r *GenericReconciler[K, T]) processNewResource(ctx context.Context, curren
}
if r.cfg.CompareResources(newT, registered) != Equal {
if r.cfg.Matcher(newT) {
r.log.Infof("%v %v updated, updating.", kind, key)
r.logger.InfoContext(ctx, "Existing resource updated, updating", "name", key)
if err := r.cfg.OnUpdate(ctx, newT, registered); err != nil {
return trace.Wrap(err, "failed to update %v %v", kind, key)
}
return nil
}
r.log.Infof("%v %v updated and no longer matches, deleting.", kind, key)
r.logger.InfoContext(ctx, "Existing resource updated and no longer matches, deleting", "name", key)
if err := r.cfg.OnDelete(ctx, registered); err != nil {
return trace.Wrap(err, "failed to delete %v %v", kind, key)
}
return nil
}

r.log.Tracef("%v %v is already registered.", kind, key)
r.logger.Log(ctx, logutils.TraceLevel, "Existing resource is already registered", "name", key)
return nil
}

Expand Down
4 changes: 3 additions & 1 deletion lib/srv/app/watcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ package app
import (
"context"
"fmt"
"log/slog"

"github.com/gravitational/trace"

Expand All @@ -40,7 +41,8 @@ func (s *Server) startReconciler(ctx context.Context) error {
OnCreate: s.onCreate,
OnUpdate: s.onUpdate,
OnDelete: s.onDelete,
Log: s.log,
// TODO(tross): update to use the server logger once it is converted to slog
Logger: slog.With("kind", types.KindApp),
})
if err != nil {
return trace.Wrap(err)
Expand Down
5 changes: 0 additions & 5 deletions lib/srv/db/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ import (
"github.com/google/uuid"
"github.com/gravitational/trace"
"github.com/jonboulle/clockwork"
"github.com/sirupsen/logrus"
"golang.org/x/sync/errgroup"

"github.com/gravitational/teleport"
Expand Down Expand Up @@ -325,9 +324,6 @@ type Server struct {
reconcileCh chan struct{}
// mu protects access to server infos and databases.
mu sync.RWMutex
// logrusLogger is used for logging.
// Deprecated: use log (*slog.Logger) instead.
logrusLogger *logrus.Entry
// logger is used for logging.
log *slog.Logger
// activeConnections counts the number of database active connections.
Expand Down Expand Up @@ -419,7 +415,6 @@ func New(ctx context.Context, config Config) (*Server, error) {
connCtx, connCancelFunc := context.WithCancel(ctx)
server := &Server{
cfg: config,
logrusLogger: logrus.WithField(teleport.ComponentKey, teleport.ComponentDatabase),
log: slog.With(teleport.ComponentKey, teleport.ComponentDatabase),
closeContext: closeCtx,
closeFunc: closeCancelFunc,
Expand Down
2 changes: 1 addition & 1 deletion lib/srv/db/watcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func (s *Server) startReconciler(ctx context.Context) error {
OnCreate: s.onCreate,
OnUpdate: s.onUpdate,
OnDelete: s.onDelete,
Log: s.logrusLogger,
Logger: s.log.With("kind", types.KindDatabase),
})
if err != nil {
return trace.Wrap(err)
Expand Down
8 changes: 4 additions & 4 deletions lib/srv/desktop/discovery.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,14 @@ import (
"encoding/hex"
"errors"
"fmt"
"log/slog"
"net"
"net/netip"
"strings"
"time"

"github.com/go-ldap/ldap/v3"
"github.com/gravitational/trace"
"github.com/sirupsen/logrus"

apidefaults "github.com/gravitational/teleport/api/defaults"
"github.com/gravitational/teleport/api/types"
Expand All @@ -46,14 +46,14 @@ func (s *WindowsService) startDesktopDiscovery() error {
reconciler, err := services.NewReconciler(services.ReconcilerConfig[types.WindowsDesktop]{
// Use a matcher that matches all resources, since our desktops are
// pre-filtered by nature of using an LDAP search with filters.
Matcher: func(d types.WindowsDesktop) bool { return true },

Matcher: func(d types.WindowsDesktop) bool { return true },
GetCurrentResources: func() map[string]types.WindowsDesktop { return s.lastDiscoveryResults },
GetNewResources: s.getDesktopsFromLDAP,
OnCreate: s.upsertDesktop,
OnUpdate: s.updateDesktop,
OnDelete: s.deleteDesktop,
Log: logrus.NewEntry(logrus.StandardLogger()),
// TODO(tross): update to use the service logger once it is converted to use slog
Logger: slog.With("kind", types.KindWindowsDesktop),
})
if err != nil {
return trace.Wrap(err)
Expand Down
4 changes: 3 additions & 1 deletion lib/srv/discovery/database_watcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ package discovery

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

"github.com/gravitational/trace"
Expand Down Expand Up @@ -52,7 +53,8 @@ func (s *Server) startDatabaseWatchers() error {
defer mu.Unlock()
return utils.FromSlice(newDatabases, types.Database.GetName)
},
Log: s.Log.WithField("kind", types.KindDatabase),
// TODO(tross): update to use the server logger once it is converted to use slog
Logger: slog.With("kind", types.KindDatabase),
OnCreate: s.onDatabaseCreate,
OnUpdate: s.onDatabaseUpdate,
OnDelete: s.onDatabaseDelete,
Expand Down
4 changes: 3 additions & 1 deletion lib/srv/discovery/kube_services_watcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ package discovery

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

Expand Down Expand Up @@ -61,7 +62,8 @@ func (s *Server) startKubeAppsWatchers() error {
defer mu.Unlock()
return utils.FromSlice(appResources, types.Application.GetName)
},
Log: s.Log.WithField("kind", types.KindApp),
// TODO(tross): update to use the server logger once it is converted to use slog
Logger: slog.With("kind", types.KindApp),
OnCreate: s.onAppCreate,
OnUpdate: s.onAppUpdate,
OnDelete: s.onAppDelete,
Expand Down
4 changes: 3 additions & 1 deletion lib/srv/discovery/kube_watcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ package discovery

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

"github.com/gravitational/trace"
Expand Down Expand Up @@ -60,7 +61,8 @@ func (s *Server) startKubeWatchers() error {
defer mu.Unlock()
return utils.FromSlice(kubeResources, types.KubeCluster.GetName)
},
Log: s.Log.WithField("kind", types.KindKubernetesCluster),
// TODO(tross): update to user the server logger once it is converted to use slog
Logger: slog.With("kind", types.KindKubernetesCluster),
OnCreate: s.onKubeCreate,
OnUpdate: s.onKubeUpdate,
OnDelete: s.onKubeDelete,
Expand Down
Loading