Skip to content

Commit

Permalink
Consistently use logutils.TypeAttr (#49969)
Browse files Browse the repository at this point in the history
When converting to slog there have been various mechanisms used
to include an objects type in a slog attribute to replace %T from
a logrus message. This unifies all type attribute to use
logutils.typeAttr instead of calling reflect/fmt directly. The former
ensures that the type is only calculated if the message is going
to be logged.
  • Loading branch information
rosstimothy authored Dec 9, 2024
1 parent 637fc65 commit 6029741
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 7 deletions.
10 changes: 5 additions & 5 deletions lib/services/notifications_cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import (
"fmt"
"io"
"log/slog"
"reflect"
"sync"
"time"

Expand All @@ -36,6 +35,7 @@ import (
apiutils "github.com/gravitational/teleport/api/utils"
"github.com/gravitational/teleport/lib/backend"
"github.com/gravitational/teleport/lib/utils"
logutils "github.com/gravitational/teleport/lib/utils/log"
"github.com/gravitational/teleport/lib/utils/sortcache"
)

Expand Down Expand Up @@ -456,14 +456,14 @@ func (c *UserNotificationCache) processEventsAndUpdateCurrent(ctx context.Contex
// to transform the notification into a legacy resource. We now have to use Unwrap() to get the original RFD153-style notification out and add it to the cache.
resource153, ok := event.Resource.(types.Resource153Unwrapper)
if !ok {
slog.WarnContext(ctx, "Unexpected resource type in event (expected types.Resource153Unwrapper)", "resource_type", reflect.TypeOf(resource153))
slog.WarnContext(ctx, "Unexpected resource type in event (expected types.Resource153Unwrapper)", "resource_type", logutils.TypeAttr(resource153))
continue
}
resource := resource153.Unwrap()

notification, ok := resource.(*notificationsv1.Notification)
if !ok {
slog.WarnContext(ctx, "Unexpected resource type in event (expected *notificationsv1.Notification)", "resource_type", reflect.TypeOf(resource))
slog.WarnContext(ctx, "Unexpected resource type in event (expected *notificationsv1.Notification)", "resource_type", logutils.TypeAttr(resource))
continue
}
if evicted := cache.Put(notification); evicted > 1 {
Expand All @@ -487,14 +487,14 @@ func (c *GlobalNotificationCache) processEventsAndUpdateCurrent(ctx context.Cont
case types.OpPut:
resource153, ok := event.Resource.(types.Resource153Unwrapper)
if !ok {
slog.WarnContext(ctx, "Unexpected resource type in event (expected types.Resource153Unwrapper)", "resource_type", reflect.TypeOf(resource153))
slog.WarnContext(ctx, "Unexpected resource type in event (expected types.Resource153Unwrapper)", "resource_type", logutils.TypeAttr(resource153))
continue
}
resource := resource153.Unwrap()

globalNotification, ok := resource.(*notificationsv1.GlobalNotification)
if !ok {
slog.WarnContext(ctx, "Unexpected resource type in event (expected *notificationsv1.GlobalNotification)", "resource_type", reflect.TypeOf(resource))
slog.WarnContext(ctx, "Unexpected resource type in event (expected *notificationsv1.GlobalNotification)", "resource_type", logutils.TypeAttr(resource))
continue
}
if evicted := cache.Put(globalNotification); evicted > 1 {
Expand Down
2 changes: 1 addition & 1 deletion lib/srv/desktop/rdp/rdpclient/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ func (c *Client) readClientUsername() error {
}
u, ok := msg.(tdp.ClientUsername)
if !ok {
c.cfg.Logger.DebugContext(context.Background(), fmt.Sprintf("Expected ClientUsername message, got %T", msg))
c.cfg.Logger.DebugContext(context.Background(), "Received unexpected ClientUsername message", "message_type", logutils.TypeAttr(msg))
continue
}
c.cfg.Logger.DebugContext(context.Background(), "Got RDP username", "username", u.Username)
Expand Down
3 changes: 2 additions & 1 deletion lib/srv/desktop/windows_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ import (
"github.com/gravitational/teleport/lib/srv/desktop/tdp"
"github.com/gravitational/teleport/lib/tlsca"
"github.com/gravitational/teleport/lib/utils"
logutils "github.com/gravitational/teleport/lib/utils/log"
)

const (
Expand Down Expand Up @@ -1140,7 +1141,7 @@ func (s *WindowsService) makeTDPReceiveHandler(
if e.Size() > libevents.MaxProtoMessageSizeBytes {
// screen spec, mouse button, and mouse move are fixed size messages,
// so they cannot exceed the maximum size
s.cfg.Logger.WarnContext(ctx, "refusing to record message", "len", len(b), "type", fmt.Sprintf("%T", m))
s.cfg.Logger.WarnContext(ctx, "refusing to record message", "len", len(b), "type", logutils.TypeAttr(m))
} else {
if err := libevents.SetupAndRecordEvent(ctx, recorder, e); err != nil {
s.cfg.Logger.WarnContext(ctx, "could not record desktop recording event", "error", err)
Expand Down

0 comments on commit 6029741

Please sign in to comment.