Skip to content

Commit

Permalink
Consistently use logutils.TypeAttr
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 and github-actions committed Dec 9, 2024
1 parent 637fc65 commit 561c0db
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 561c0db

Please sign in to comment.