Skip to content

Commit

Permalink
Convert lib/utils to slog (#50340)
Browse files Browse the repository at this point in the history
This only converts the places in the utils package that were
logging with logrus, but it does not fully remove the logrus
package as a dependency. The package is home to the logrus text
formatter which needs to exist until slog is the only logging
mechanism in use.
  • Loading branch information
rosstimothy authored Dec 19, 2024
1 parent a1881e9 commit e9f27d1
Show file tree
Hide file tree
Showing 18 changed files with 155 additions and 110 deletions.
5 changes: 3 additions & 2 deletions lib/utils/addr.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,16 @@
package utils

import (
"context"
"fmt"
"log/slog"
"net"
"net/url"
"strconv"
"strings"
"unicode/utf8"

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

apiutils "github.com/gravitational/teleport/api/utils"
)
Expand Down Expand Up @@ -290,7 +291,7 @@ func GuessHostIP() (ip net.IP, err error) {
for _, iface := range ifaces {
ifadrs, err := iface.Addrs()
if err != nil {
log.Warn(err)
slog.WarnContext(context.Background(), "Unable to get addresses for interface", "interface", iface.Name, "error", err)
} else {
adrs = append(adrs, ifadrs...)
}
Expand Down
5 changes: 3 additions & 2 deletions lib/utils/agentconn/agent_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,10 @@
package agentconn

import (
"context"
"encoding/binary"
"encoding/hex"
"log/slog"
"net"
"os"
"os/exec"
Expand All @@ -33,7 +35,6 @@ import (

"github.com/Microsoft/go-winio"
"github.com/gravitational/trace"
"github.com/sirupsen/logrus"

apiutils "github.com/gravitational/teleport/api/utils"
)
Expand Down Expand Up @@ -214,7 +215,7 @@ func getCygwinUIDFromPS() (uint32, error) {
// preform a successful handshake with it. Handshake details here:
// https://stackoverflow.com/questions/23086038/what-mechanism-is-used-by-msys-cygwin-to-emulate-unix-domain-sockets
func attemptCygwinHandshake(port, key string, uid uint32) (net.Conn, error) {
logrus.Debugf("[KEY AGENT] attempting a handshake with Cygwin ssh-agent socket; port=%s uid=%d", port, uid)
slog.DebugContext(context.Background(), "[KEY AGENT] attempting a handshake with Cygwin ssh-agent socket", "port", port, "uid", uid)

conn, err := net.Dial("tcp", "localhost:"+port)
if err != nil {
Expand Down
9 changes: 6 additions & 3 deletions lib/utils/aws/credentials.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ package aws

import (
"context"
"log/slog"
"sort"
"strings"
"time"
Expand All @@ -33,7 +34,6 @@ import (
"github.com/aws/aws-sdk-go/service/sts"
"github.com/gravitational/trace"
"github.com/jonboulle/clockwork"
"github.com/sirupsen/logrus"

"github.com/gravitational/teleport/lib/modules"
"github.com/gravitational/teleport/lib/utils"
Expand Down Expand Up @@ -70,8 +70,11 @@ func NewCredentialsGetter() CredentialsGetter {
}

// Get obtains STS credentials.
func (g *credentialsGetter) Get(_ context.Context, request GetCredentialsRequest) (*credentials.Credentials, error) {
logrus.Debugf("Creating STS session %q for %q.", request.SessionName, request.RoleARN)
func (g *credentialsGetter) Get(ctx context.Context, request GetCredentialsRequest) (*credentials.Credentials, error) {
slog.DebugContext(ctx, "Creating STS session",
"session_name", request.SessionName,
"role_arn", request.RoleARN,
)
return stscreds.NewCredentials(request.Provider, request.RoleARN,
func(cred *stscreds.AssumeRoleProvider) {
cred.RoleSessionName = MaybeHashRoleSessionName(request.SessionName)
Expand Down
5 changes: 3 additions & 2 deletions lib/utils/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,13 @@
package utils

import (
"context"
"log/slog"
"os"
"path/filepath"
"strings"

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

// TryReadValueAsFile is a utility function to read a value
Expand All @@ -44,7 +45,7 @@ func TryReadValueAsFile(value string) (string, error) {
out := strings.TrimSpace(string(contents))

if out == "" {
log.Warnf("Empty config value file: %v", value)
slog.WarnContext(context.Background(), "Empty config value file", "file", value)
}
return out, nil
}
13 changes: 7 additions & 6 deletions lib/utils/diagnostics/latency/monitor.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,19 +20,20 @@ package latency

import (
"context"
"errors"
"sync/atomic"
"time"

"github.com/google/uuid"
"github.com/gravitational/trace"
"github.com/jonboulle/clockwork"
"github.com/sirupsen/logrus"

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

var log = logrus.WithField(teleport.ComponentKey, "latency")
var logger = logutils.NewPackageLogger(teleport.ComponentKey, "latency")

// Statistics contain latency measurements for both
// legs of a proxied connection.
Expand Down Expand Up @@ -188,8 +189,8 @@ func (m *Monitor) Run(ctx context.Context) {
for {
select {
case <-m.reportTimer.Chan():
if err := m.reporter.Report(ctx, m.GetStats()); err != nil {
log.WithError(err).Warn("failed to report latency stats")
if err := m.reporter.Report(ctx, m.GetStats()); err != nil && !errors.Is(err, context.Canceled) {
logger.WarnContext(ctx, "failed to report latency stats", "error", err)
}
m.reportTimer.Reset(retryutils.SeventhJitter(m.reportInterval))
case <-ctx.Done():
Expand All @@ -205,8 +206,8 @@ func (m *Monitor) pingLoop(ctx context.Context, pinger Pinger, timer clockwork.T
return
case <-timer.Chan():
then := m.clock.Now()
if err := pinger.Ping(ctx); err != nil {
log.WithError(err).Warn("unexpected failure sending ping")
if err := pinger.Ping(ctx); err != nil && !errors.Is(err, context.Canceled) {
logger.WarnContext(ctx, "unexpected failure sending ping", "error", err)
} else {
latency.Store(m.clock.Now().Sub(then).Milliseconds())
}
Expand Down
20 changes: 13 additions & 7 deletions lib/utils/envutils/environment.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,13 @@ package envutils

import (
"bufio"
"context"
"fmt"
"io"
"log/slog"
"os"
"strings"

log "github.com/sirupsen/logrus"

"github.com/gravitational/teleport"
"github.com/gravitational/teleport/lib/utils"
)
Expand All @@ -39,7 +39,10 @@ func ReadEnvironmentFile(filename string) ([]string, error) {
// having this file for the user is optional.
file, err := utils.OpenFileNoUnsafeLinks(filename)
if err != nil {
log.Warnf("Unable to open environment file %v: %v, skipping", filename, err)
slog.WarnContext(context.Background(), "Unable to open environment file, skipping",
"file", filename,
"error", err,
)
return []string{}, nil
}
defer file.Close()
Expand All @@ -51,6 +54,7 @@ func readEnvironment(r io.Reader) ([]string, error) {
var lineno int
env := &SafeEnv{}

ctx := context.Background()
scanner := bufio.NewScanner(r)
for scanner.Scan() {
line := strings.TrimSpace(scanner.Text())
Expand All @@ -59,7 +63,9 @@ func readEnvironment(r io.Reader) ([]string, error) {
// https://github.com/openssh/openssh-portable/blob/master/session.c#L873-L874
lineno = lineno + 1
if lineno > teleport.MaxEnvironmentFileLines {
log.Warnf("Too many lines in environment file, returning first %v lines", teleport.MaxEnvironmentFileLines)
slog.WarnContext(ctx, "Too many lines in environment file, limiting how many are consumed",
"lines_consumed", teleport.MaxEnvironmentFileLines,
)
return *env, nil
}

Expand All @@ -71,15 +77,15 @@ func readEnvironment(r io.Reader) ([]string, error) {
// split on first =, if not found, log it and continue
idx := strings.Index(line, "=")
if idx == -1 {
log.Debugf("Bad line %v while reading environment file: no = separator found", lineno)
slog.DebugContext(ctx, "Bad line while reading environment file: no = separator found", "line_number", lineno)
continue
}

// split key and value and make sure that key has a name
key := line[:idx]
value := line[idx+1:]
if strings.TrimSpace(key) == "" {
log.Debugf("Bad line %v while reading environment file: key without name", lineno)
slog.DebugContext(ctx, "Bad line while reading environment file: key without name", "line_number", lineno)
continue
}

Expand All @@ -88,7 +94,7 @@ func readEnvironment(r io.Reader) ([]string, error) {
}

if err := scanner.Err(); err != nil {
log.Warnf("Unable to read environment file: %v", err)
slog.WarnContext(ctx, "Unable to read environment file", "error", err)
return []string{}, nil
}

Expand Down
4 changes: 2 additions & 2 deletions lib/utils/fanoutbuffer/buffer.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,13 @@ package fanoutbuffer
import (
"context"
"errors"
"log/slog"
"runtime"
"sync"
"sync/atomic"
"time"

"github.com/jonboulle/clockwork"
log "github.com/sirupsen/logrus"
)

// ErrGracePeriodExceeded is an error returned by Cursor.Read indicating that the cursor fell
Expand Down Expand Up @@ -380,7 +380,7 @@ func finalizeCursor[T any](cursor *Cursor[T]) {
}

cursor.closeLocked()
log.Warn("Fanout buffer cursor was never closed. (this is a bug)")
slog.WarnContext(context.Background(), "Fanout buffer cursor was never closed. (this is a bug)")
}

// Close closes the cursor. Close is safe to double-call and should be called as soon as possible if
Expand Down
25 changes: 19 additions & 6 deletions lib/utils/host/hostusers.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,16 @@ package host
import (
"bufio"
"bytes"
"context"
"errors"
"log/slog"
"os"
"os/exec"
"os/user"
"slices"
"strings"

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

// man GROUPADD(8), exit codes section
Expand All @@ -55,7 +56,10 @@ func GroupAdd(groupname string, gid string) (exitCode int, err error) {

cmd := exec.Command(groupaddBin, args...)
output, err := cmd.CombinedOutput()
log.Debugf("%s output: %s", cmd.Path, string(output))
slog.DebugContext(context.Background(), "groupadd command completed",
"command_path", cmd.Path,
"output", string(output),
)

switch code := cmd.ProcessState.ExitCode(); code {
case GroupExistExit:
Expand Down Expand Up @@ -122,15 +126,18 @@ func UserAdd(username string, groups []string, opts UserOpts) (exitCode int, err

if opts.Shell != "" {
if shell, err := exec.LookPath(opts.Shell); err != nil {
log.Warnf("configured shell %q not found, falling back to host default", opts.Shell)
slog.WarnContext(context.Background(), "configured shell not found, falling back to host default", "shell", opts.Shell)
} else {
args = append(args, "--shell", shell)
}
}

cmd := exec.Command(useraddBin, args...)
output, err := cmd.CombinedOutput()
log.Debugf("%s output: %s", cmd.Path, string(output))
slog.DebugContext(context.Background(), "useradd command completed",
"command_path", cmd.Path,
"output", string(output),
)
if cmd.ProcessState.ExitCode() == UserExistExit {
return cmd.ProcessState.ExitCode(), trace.AlreadyExists("user already exists")
}
Expand All @@ -147,7 +154,10 @@ func SetUserGroups(username string, groups []string) (exitCode int, err error) {
// usermod -G (replace groups) (username)
cmd := exec.Command(usermodBin, "-G", strings.Join(groups, ","), username)
output, err := cmd.CombinedOutput()
log.Debugf("%s output: %s", cmd.Path, string(output))
slog.DebugContext(context.Background(), "usermod completed",
"command_path", cmd.Path,
"output", string(output),
)
return cmd.ProcessState.ExitCode(), trace.Wrap(err)
}

Expand All @@ -170,7 +180,10 @@ func UserDel(username string) (exitCode int, err error) {
// userdel --remove (remove home) username
cmd := exec.Command(userdelBin, args...)
output, err := cmd.CombinedOutput()
log.Debugf("%s output: %s", cmd.Path, string(output))
slog.DebugContext(context.Background(), "usedel command completed",
"command_path", cmd.Path,
"output", string(output),
)
return cmd.ProcessState.ExitCode(), trace.Wrap(err)
}

Expand Down
Loading

0 comments on commit e9f27d1

Please sign in to comment.