Skip to content

Commit

Permalink
Convert lib/srv/discovery to use slog (#50066)
Browse files Browse the repository at this point in the history
  • Loading branch information
rosstimothy authored Dec 16, 2024
1 parent 79e993b commit de34430
Show file tree
Hide file tree
Showing 38 changed files with 379 additions and 309 deletions.
1 change: 0 additions & 1 deletion lib/service/discovery.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,6 @@ func (process *TeleportProcess) initDiscoveryService() error {
AccessPoint: accessPoint,
ServerID: process.Config.HostUUID,
Log: process.logger,
LegacyLogger: process.log,
ClusterName: conn.ClusterName(),
ClusterFeatures: process.GetClusterFeatures,
PollInterval: process.Config.Discovery.PollInterval,
Expand Down
4 changes: 2 additions & 2 deletions lib/srv/db/watcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,9 @@ package db

import (
"context"
"log/slog"

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

"github.com/gravitational/teleport"
"github.com/gravitational/teleport/api/types"
Expand Down Expand Up @@ -127,7 +127,7 @@ func (s *Server) startCloudWatcher(ctx context.Context) error {

watcher, err := discovery.NewWatcher(ctx, discovery.WatcherConfig{
FetchersFn: discovery.StaticFetchers(allFetchers),
Log: logrus.WithField(teleport.ComponentKey, "watcher:cloud"),
Logger: slog.With(teleport.ComponentKey, "watcher:cloud"),
Origin: types.OriginCloud,
})
if err != nil {
Expand Down
5 changes: 3 additions & 2 deletions lib/srv/discovery/common/database.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@
package common

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

"github.com/Azure/azure-sdk-for-go/sdk/azcore/arm"
Expand All @@ -36,7 +38,6 @@ import (
"github.com/aws/aws-sdk-go/service/redshift"
"github.com/aws/aws-sdk-go/service/redshiftserverless"
"github.com/gravitational/trace"
log "github.com/sirupsen/logrus"

"github.com/gravitational/teleport/api/types"
apiawsutils "github.com/gravitational/teleport/api/utils/aws"
Expand Down Expand Up @@ -1403,7 +1404,7 @@ func labelsFromAzureMySQLFlexServer(server *armmysqlflexibleservers.Server) (map
labels[types.DiscoveryLabelAzureReplicationRole] = role
ssrid, err := arm.ParseResourceID(azure.StringVal(server.Properties.SourceServerResourceID))
if err != nil {
log.WithError(err).Debugf("Skipping malformed %q label for Azure MySQL Flexible server replica.", types.DiscoveryLabelAzureSourceServer)
slog.DebugContext(context.Background(), "Skipping malformed label for Azure MySQL Flexible server replica", "error", err, "label", types.DiscoveryLabelAzureSourceServer)
} else {
labels[types.DiscoveryLabelAzureSourceServer] = ssrid.Name
}
Expand Down
7 changes: 4 additions & 3 deletions lib/srv/discovery/common/kubernetes.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,15 @@
package common

import (
"context"
"fmt"
"log/slog"
"maps"
"strings"

"github.com/aws/aws-sdk-go-v2/aws/arn"
"github.com/aws/aws-sdk-go/aws"
"github.com/gravitational/trace"
"github.com/sirupsen/logrus"

"github.com/gravitational/teleport/api/types"
"github.com/gravitational/teleport/lib/cloud/azure"
Expand Down Expand Up @@ -79,7 +80,7 @@ func awsEKSTagsToLabels(tags map[string]*string) map[string]string {
if types.IsValidLabelKey(key) {
labels[key] = aws.StringValue(val)
} else {
logrus.Debugf("Skipping EKS tag %q, not a valid label key.", key)
slog.DebugContext(context.Background(), "Skipping EKS tag that is not a valid label key", "tag", key)
}
}
return labels
Expand All @@ -97,7 +98,7 @@ func addLabels(labels map[string]string, moreLabels map[string]string) map[strin
if types.IsValidLabelKey(key) {
labels[key] = value
} else {
logrus.Debugf("Skipping %q, not a valid label key.", key)
slog.DebugContext(context.Background(), "Skipping label that is not a valid label key", "label", key)
}
}
return labels
Expand Down
18 changes: 9 additions & 9 deletions lib/srv/discovery/common/watcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,13 @@ package common

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

"github.com/gravitational/trace"
"github.com/jonboulle/clockwork"
"github.com/sirupsen/logrus"
"golang.org/x/sync/errgroup"

"github.com/gravitational/teleport/api/types"
Expand All @@ -49,8 +49,8 @@ type WatcherConfig struct {
Interval time.Duration
// TriggerFetchC can be used to force an instant Poll, instead of waiting for the next poll Interval.
TriggerFetchC chan struct{}
// Log is the watcher logger.
Log logrus.FieldLogger
// Logger is the watcher logger.
Logger *slog.Logger
// Clock is used to control time.
Clock clockwork.Clock
// DiscoveryGroup is the name of the discovery group that the current
Expand All @@ -75,8 +75,8 @@ func (c *WatcherConfig) CheckAndSetDefaults() error {
if c.TriggerFetchC == nil {
c.TriggerFetchC = make(chan struct{})
}
if c.Log == nil {
c.Log = logrus.New()
if c.Logger == nil {
c.Logger = slog.Default()
}
if c.Clock == nil {
c.Clock = clockwork.NewRealClock()
Expand Down Expand Up @@ -117,7 +117,7 @@ func NewWatcher(ctx context.Context, config WatcherConfig) (*Watcher, error) {
func (w *Watcher) Start() {
pollTimer := w.cfg.Clock.NewTimer(w.cfg.Interval)
defer pollTimer.Stop()
w.cfg.Log.Infof("Starting watcher.")
w.cfg.Logger.InfoContext(w.ctx, "Starting watcher")
w.fetchAndSend()
for {
select {
Expand All @@ -135,7 +135,7 @@ func (w *Watcher) Start() {
pollTimer.Reset(w.cfg.Interval)

case <-w.ctx.Done():
w.cfg.Log.Infof("Watcher done.")
w.cfg.Logger.InfoContext(w.ctx, "Watcher done")
return
}
}
Expand Down Expand Up @@ -163,9 +163,9 @@ func (w *Watcher) fetchAndSend() {
// not others. This is acceptable, so make a debug log instead
// of a warning.
if trace.IsAccessDenied(err) || trace.IsNotFound(err) {
w.cfg.Log.WithError(err).WithField("fetcher", lFetcher).Debugf("Skipped fetcher for %s at %s.", lFetcher.ResourceType(), lFetcher.Cloud())
w.cfg.Logger.DebugContext(groupCtx, "Skipped fetcher for resources", "error", err, "fetcher", lFetcher, "resource", lFetcher.ResourceType(), "cloud", lFetcher.Cloud())
} else {
w.cfg.Log.WithError(err).WithField("fetcher", lFetcher).Warnf("Unable to fetch resources for %s at %s.", lFetcher.ResourceType(), lFetcher.Cloud())
w.cfg.Logger.WarnContext(groupCtx, "Unable to fetch resources", "error", err, "fetcher", lFetcher, "resource", lFetcher.ResourceType(), "cloud", lFetcher.Cloud())
}
// never return the error otherwise it will impact other watchers.
return nil
Expand Down
2 changes: 1 addition & 1 deletion lib/srv/discovery/database_watcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ func (s *Server) startDatabaseWatchers() error {
watcher, err := common.NewWatcher(s.ctx,
common.WatcherConfig{
FetchersFn: s.getAllDatabaseFetchers,
Log: s.LegacyLogger.WithField("kind", types.KindDatabase),
Logger: s.Log.With("kind", types.KindDatabase),
DiscoveryGroup: s.DiscoveryGroup,
Interval: s.PollInterval,
TriggerFetchC: s.newDiscoveryConfigChangedSub(),
Expand Down
19 changes: 6 additions & 13 deletions lib/srv/discovery/discovery.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ import (
"github.com/aws/aws-sdk-go/aws/session"
"github.com/gravitational/trace"
"github.com/jonboulle/clockwork"
"github.com/sirupsen/logrus"
"google.golang.org/protobuf/types/known/timestamppb"
"k8s.io/client-go/kubernetes"
"k8s.io/client-go/rest"
Expand Down Expand Up @@ -133,9 +132,6 @@ type Config struct {
AccessPoint authclient.DiscoveryAccessPoint
// Log is the logger.
Log *slog.Logger
// LegacyLogger is the old logger
// Deprecated: use Log instead.
LegacyLogger logrus.FieldLogger
// ServerID identifies the Teleport instance where this service runs.
ServerID string
// onDatabaseReconcile is called after each database resource reconciliation.
Expand Down Expand Up @@ -258,9 +254,7 @@ kubernetes matchers are present.`)
if c.Log == nil {
c.Log = slog.Default()
}
if c.LegacyLogger == nil {
c.LegacyLogger = logrus.New()
}

if c.protocolChecker == nil {
c.protocolChecker = fetchers.NewProtoChecker(false)
}
Expand All @@ -281,7 +275,6 @@ kubernetes matchers are present.`)
}

c.Log = c.Log.With(teleport.ComponentKey, teleport.ComponentDiscovery)
c.LegacyLogger = c.LegacyLogger.WithField(teleport.ComponentKey, teleport.ComponentDiscovery)

if c.DiscoveryGroup == "" {
const warningMessage = "discovery_service.discovery_group is not set. This field is required for the discovery service to work properly.\n" +
Expand Down Expand Up @@ -573,7 +566,7 @@ func (s *Server) initAWSWatchers(matchers []types.AWSMatcher) error {
_, otherMatchers = splitMatchers(otherMatchers, db.IsAWSMatcherType)

// Add non-integration kube fetchers.
kubeFetchers, err := fetchers.MakeEKSFetchersFromAWSMatchers(s.LegacyLogger, s.CloudClients, otherMatchers, noDiscoveryConfig)
kubeFetchers, err := fetchers.MakeEKSFetchersFromAWSMatchers(s.Log, s.CloudClients, otherMatchers, noDiscoveryConfig)
if err != nil {
return trace.Wrap(err)
}
Expand Down Expand Up @@ -601,7 +594,7 @@ func (s *Server) initKubeAppWatchers(matchers []types.KubernetesMatcher) error {
KubernetesClient: kubeClient,
FilterLabels: matcher.Labels,
Namespaces: matcher.Namespaces,
Log: s.LegacyLogger,
Logger: s.Log,
ClusterName: s.DiscoveryGroup,
ProtocolChecker: s.Config.protocolChecker,
})
Expand Down Expand Up @@ -699,7 +692,7 @@ func (s *Server) kubeFetchersFromMatchers(matchers Matchers, discoveryConfigName
return matcherType == types.AWSMatcherEKS
})
if len(awsKubeMatchers) > 0 {
eksFetchers, err := fetchers.MakeEKSFetchersFromAWSMatchers(s.LegacyLogger, s.CloudClients, awsKubeMatchers, discoveryConfigName)
eksFetchers, err := fetchers.MakeEKSFetchersFromAWSMatchers(s.Log, s.CloudClients, awsKubeMatchers, discoveryConfigName)
if err != nil {
return nil, trace.Wrap(err)
}
Expand Down Expand Up @@ -766,7 +759,7 @@ func (s *Server) initAzureWatchers(ctx context.Context, matchers []types.AzureMa
Regions: matcher.Regions,
FilterLabels: matcher.ResourceTags,
ResourceGroups: matcher.ResourceGroups,
Log: s.LegacyLogger,
Logger: s.Log,
DiscoveryConfigName: discoveryConfigName,
})
if err != nil {
Expand Down Expand Up @@ -857,7 +850,7 @@ func (s *Server) initGCPWatchers(ctx context.Context, matchers []types.GCPMatche
Location: location,
FilterLabels: matcher.GetLabels(),
ProjectID: projectID,
Log: s.LegacyLogger,
Logger: s.Log,
})
if err != nil {
return trace.Wrap(err)
Expand Down
15 changes: 0 additions & 15 deletions lib/srv/discovery/discovery_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ import (
"github.com/google/uuid"
"github.com/gravitational/trace"
"github.com/jonboulle/clockwork"
"github.com/sirupsen/logrus"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"google.golang.org/grpc"
Expand Down Expand Up @@ -751,7 +750,6 @@ func TestDiscoveryServer(t *testing.T) {
require.NoError(t, err)
}

legacyLogger := logrus.New()
logger := libutils.NewSlogLoggerForTests()

reporter := &mockUsageReporter{}
Expand Down Expand Up @@ -784,7 +782,6 @@ func TestDiscoveryServer(t *testing.T) {
Matchers: tc.staticMatchers,
Emitter: tc.emitter,
Log: logger,
LegacyLogger: legacyLogger,
DiscoveryGroup: defaultDiscoveryGroup,
CloudClients: tc.cloudClients,
clock: fakeClock,
Expand Down Expand Up @@ -845,7 +842,6 @@ func TestDiscoveryServer(t *testing.T) {
func TestDiscoveryServerConcurrency(t *testing.T) {
t.Parallel()
ctx := context.Background()
legacyLogger := logrus.New()
logger := libutils.NewSlogLoggerForTests()

defaultDiscoveryGroup := "dg01"
Expand Down Expand Up @@ -942,7 +938,6 @@ func TestDiscoveryServerConcurrency(t *testing.T) {
Matchers: staticMatcher,
Emitter: emitter,
Log: logger,
LegacyLogger: legacyLogger,
DiscoveryGroup: defaultDiscoveryGroup,
})
require.NoError(t, err)
Expand Down Expand Up @@ -1440,12 +1435,8 @@ func TestDiscoveryInCloudKube(t *testing.T) {
require.NoError(t, r.Close())
require.NoError(t, w.Close())
})

legacyLogger := logrus.New()
logger := libutils.NewSlogLoggerForTests()

legacyLogger.SetOutput(w)
legacyLogger.SetLevel(logrus.DebugLevel)
clustersNotUpdated := make(chan string, 10)
go func() {
// reconcileRegexp is the regex extractor of a log message emitted by reconciler when
Expand Down Expand Up @@ -1484,7 +1475,6 @@ func TestDiscoveryInCloudKube(t *testing.T) {
},
Emitter: authClient,
Log: logger,
LegacyLogger: legacyLogger,
DiscoveryGroup: mainDiscoveryGroup,
})

Expand Down Expand Up @@ -2852,7 +2842,6 @@ func TestAzureVMDiscovery(t *testing.T) {
require.NoError(t, err)
}

legacyLogger := logrus.New()
logger := libutils.NewSlogLoggerForTests()

emitter := &mockEmitter{}
Expand All @@ -2869,7 +2858,6 @@ func TestAzureVMDiscovery(t *testing.T) {
Matchers: tc.staticMatchers,
Emitter: emitter,
Log: logger,
LegacyLogger: legacyLogger,
DiscoveryGroup: defaultDiscoveryGroup,
})

Expand Down Expand Up @@ -3161,7 +3149,6 @@ func TestGCPVMDiscovery(t *testing.T) {
require.NoError(t, err)
}

legacyLogger := logrus.New()
logger := libutils.NewSlogLoggerForTests()
emitter := &mockEmitter{}
reporter := &mockUsageReporter{}
Expand All @@ -3177,7 +3164,6 @@ func TestGCPVMDiscovery(t *testing.T) {
Matchers: tc.staticMatchers,
Emitter: emitter,
Log: logger,
LegacyLogger: legacyLogger,
DiscoveryGroup: defaultDiscoveryGroup,
})

Expand Down Expand Up @@ -3225,7 +3211,6 @@ func TestServer_onCreate(t *testing.T) {
DiscoveryGroup: "test-cluster",
AccessPoint: accessPoint,
Log: libutils.NewSlogLoggerForTests(),
LegacyLogger: logrus.New(),
},
}

Expand Down
Loading

0 comments on commit de34430

Please sign in to comment.