Skip to content

Commit

Permalink
Remove logrus
Browse files Browse the repository at this point in the history
All direct logging via logrus is now removed from tbot, tctl, tsh,
and teleport. The last remaining places using logrus are all of
the plugins in `integrations`. Some of the integrations code are
directly imported in teleport.e which prevent logrus from becoming
an indirect dependency for the time being.
  • Loading branch information
rosstimothy committed Jan 7, 2025
1 parent 2c764e1 commit 59146c8
Show file tree
Hide file tree
Showing 27 changed files with 329 additions and 625 deletions.
25 changes: 15 additions & 10 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -98,10 +98,6 @@ linters-settings:
desc: 'use "github.com/google/uuid" instead'
- pkg: github.com/pborman/uuid
desc: 'use "github.com/google/uuid" instead'
- pkg: github.com/siddontang/go-log/log
desc: 'use "github.com/sirupsen/logrus" instead'
- pkg: github.com/siddontang/go/log
desc: 'use "github.com/sirupsen/logrus" instead'
- pkg: github.com/tj/assert
desc: 'use "github.com/stretchr/testify/assert" instead'
- pkg: go.uber.org/atomic
Expand All @@ -116,16 +112,25 @@ linters-settings:
desc: 'use "github.com/gravitational/teleport/lib/msgraph" instead'
- pkg: github.com/cloudflare/cfssl
desc: 'use "crypto" or "x/crypto" instead'
# Prevent logrus from being imported by api and e. Once everything in teleport has been converted
# to use log/slog this should be moved into the main block above.
logrus:
# Prevent importing any additional logging libraries.
logging:
files:
- '**/api/**'
- '**/e/**'
- '**/lib/srv/**'
# Integrations are still allowed to use logrus becuase they haven't
# been converted to slog yet. Once they use slog, remove this exception.
- '!**/integrations/**'
deny:
- pkg: github.com/sirupsen/logrus
desc: 'use "log/slog" instead'
- pkg: github.com/siddontang/go-log/log
desc: 'use "log/slog" instead'
- pkg: github.com/siddontang/go/log
desc: 'use "log/slog" instead'
- pkg: github.com/mailgun/log
desc: 'use "log/slog" instead'
- pkg: github.com/saferwall/pe/log
desc: 'use "log/slog" instead'
- pkg: golang.org/x/exp/slog
desc: 'use "log/slog" instead'
# Prevent importing internal packages in client tools or packages containing
# common interfaces consumed by them that are known to bloat binaries or break builds
# because they only support a single platform.
Expand Down
2 changes: 1 addition & 1 deletion e
Submodule e updated from f00dbc to bc0a10
9 changes: 0 additions & 9 deletions integration/helpers/instance.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ import (
"github.com/gorilla/websocket"
"github.com/gravitational/trace"
"github.com/jonboulle/clockwork"
"github.com/sirupsen/logrus"
"github.com/stretchr/testify/require"
"golang.org/x/crypto/ssh"

Expand Down Expand Up @@ -327,10 +326,6 @@ type InstanceConfig struct {
Priv []byte
// Pub is SSH public key of the instance
Pub []byte
// Log specifies the logger
// Deprecated: Use Logger instead
// TODO(tross): Delete when e is updated
Log utils.Logger
// Logger specifies the logger
Logger *slog.Logger
// Ports is a collection of instance ports.
Expand All @@ -354,10 +349,6 @@ func NewInstance(t *testing.T, cfg InstanceConfig) *TeleInstance {
cfg.Listeners = StandardListenerSetup(t, &cfg.Fds)
}

if cfg.Log == nil {
cfg.Log = logrus.New()
}

if cfg.Logger == nil {
cfg.Logger = slog.New(logutils.DiscardHandler{})
}
Expand Down
4 changes: 2 additions & 2 deletions integration/hostuser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -637,7 +637,7 @@ func TestRootLoginAsHostUser(t *testing.T) {
NodeName: Host,
Priv: privateKey,
Pub: publicKey,
Log: utils.NewLoggerForTests(),
Logger: utils.NewSlogLoggerForTests(),
})

// Create a user that can create a host user.
Expand Down Expand Up @@ -735,7 +735,7 @@ func TestRootStaticHostUsers(t *testing.T) {
NodeName: Host,
Priv: privateKey,
Pub: publicKey,
Log: utils.NewLoggerForTests(),
Logger: utils.NewSlogLoggerForTests(),
})

require.NoError(t, instance.Create(t, nil, false, nil))
Expand Down
62 changes: 62 additions & 0 deletions integrations/lib/logger/buffer.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
// Copyright 2022 The Go Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.

package logger

import "sync"

// buffer adapted from go/src/fmt/print.go
type buffer []byte

// Having an initial size gives a dramatic speedup.
var bufPool = sync.Pool{
New: func() any {
b := make([]byte, 0, 1024)
return (*buffer)(&b)
},
}

func newBuffer() *buffer {
return bufPool.Get().(*buffer)
}

func (b *buffer) Len() int {
return len(*b)
}

func (b *buffer) SetLen(n int) {
*b = (*b)[:n]
}

func (b *buffer) Free() {
// To reduce peak allocation, return only smaller buffers to the pool.
const maxBufferSize = 16 << 10
if cap(*b) <= maxBufferSize {
*b = (*b)[:0]
bufPool.Put(b)
}
}

func (b *buffer) Reset() {
*b = (*b)[:0]
}

func (b *buffer) Write(p []byte) (int, error) {
*b = append(*b, p...)
return len(p), nil
}

func (b *buffer) WriteString(s string) (int, error) {
*b = append(*b, s...)
return len(s), nil
}

func (b *buffer) WriteByte(c byte) error {
*b = append(*b, c)
return nil
}

func (b *buffer) String() string {
return string(*b)
}
10 changes: 5 additions & 5 deletions integrations/lib/logger/logger.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ var extraFields = []string{logutils.LevelField, logutils.ComponentField, logutil
// Init sets up logger for a typical daemon scenario until configuration
// file is parsed
func Init() {
formatter := &logutils.TextFormatter{
formatter := &TextFormatter{
EnableColors: utils.IsTerminal(os.Stderr),
ComponentPadding: 1, // We don't use components so strip the padding
ExtraFields: extraFields,
Expand Down Expand Up @@ -113,11 +113,11 @@ func (conf Config) NewSLogLogger() (*slog.Logger, error) {
var w io.Writer
switch conf.Output {
case "":
w = logutils.NewSharedWriter(os.Stderr)
w = os.Stderr
case "stderr", "error", "2":
w = logutils.NewSharedWriter(os.Stderr)
w = os.Stderr
case "stdout", "out", "1":
w = logutils.NewSharedWriter(os.Stdout)
w = os.Stdout
case teleport.Syslog:
w = os.Stderr
sw, err := utils.NewSyslogWriter()
Expand All @@ -136,7 +136,7 @@ func (conf Config) NewSLogLogger() (*slog.Logger, error) {
if err != nil {
return nil, trace.Wrap(err, "failed to init the log file shared writer")
}
w = logutils.NewWriterFinalizer[*logutils.FileSharedWriter](sharedWriter)
w = logutils.NewWriterFinalizer(sharedWriter)
if err := sharedWriter.RunWatcherReopen(context.Background()); err != nil {
return nil, trace.Wrap(err)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/

package log
package logger

import (
"fmt"
Expand All @@ -25,6 +25,7 @@ import (
"slices"
"strconv"
"strings"
"time"
"unicode"

"github.com/gravitational/trace"
Expand Down Expand Up @@ -57,7 +58,7 @@ type writer struct {
}

func newWriter() *writer {
return &writer{b: &buffer{}}
return &writer{b: newBuffer()}
}

func (w *writer) Len() int {
Expand Down Expand Up @@ -486,3 +487,16 @@ func needsQuoting(text string) bool {
}
return false
}

func appendRFC3339Millis(b []byte, t time.Time) []byte {
// Format according to time.RFC3339Nano since it is highly optimized,
// but truncate it to use millisecond resolution.
// Unfortunately, that format trims trailing 0s, so add 1/10 millisecond
// to guarantee that there are exactly 4 digits after the period.
const prefixLen = len("2006-01-02T15:04:05.000")
n := len(b)
t = t.Truncate(time.Millisecond).Add(time.Millisecond / 10)
b = t.AppendFormat(b, time.RFC3339Nano)
b = append(b[:n+prefixLen], b[n+prefixLen+1:]...) // drop the 4th digit
return b
}
20 changes: 18 additions & 2 deletions lib/client/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import (
"slices"
"strconv"
"strings"
"sync"
"sync/atomic"
"time"
"unicode/utf8"
Expand Down Expand Up @@ -2850,6 +2851,21 @@ type execResult struct {
exitStatus int
}

// sharedWriter is an [io.Writer] implementation that protects
// writes with a mutex. This allows a single [io.Writer] to be shared
// by both logrus and slog without their output clobbering each other.
type sharedWriter struct {
mu sync.Mutex
io.Writer
}

func (s *sharedWriter) Write(p []byte) (int, error) {
s.mu.Lock()
defer s.mu.Unlock()

return s.Writer.Write(p)
}

// runCommandOnNodes executes a given bash command on a bunch of remote nodes.
func (tc *TeleportClient) runCommandOnNodes(ctx context.Context, clt *ClusterClient, nodes []TargetNode, command []string) error {
cluster := clt.ClusterName()
Expand Down Expand Up @@ -2909,10 +2925,10 @@ func (tc *TeleportClient) runCommandOnNodes(ctx context.Context, clt *ClusterCli
}
}

stdout := logutils.NewSharedWriter(tc.Stdout)
stdout := &sharedWriter{Writer: tc.Stdout}
stderr := stdout
if tc.Stdout != tc.Stderr {
stderr = logutils.NewSharedWriter(tc.Stderr)
stderr = &sharedWriter{Writer: tc.Stderr}
}

for _, node := range nodes {
Expand Down
5 changes: 0 additions & 5 deletions lib/service/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@ import (
"github.com/jonboulle/clockwork"
"github.com/prometheus/client_golang/prometheus/promhttp"
"github.com/quic-go/quic-go"
"github.com/sirupsen/logrus"
"go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc"
"go.opentelemetry.io/otel/attribute"
"golang.org/x/crypto/acme"
Expand Down Expand Up @@ -992,10 +991,6 @@ func NewTeleport(cfg *servicecfg.Config) (*TeleportProcess, error) {
}

processID := fmt.Sprintf("%v", nextProcessID())
cfg.Log = utils.WrapLogger(cfg.Log.WithFields(logrus.Fields{
teleport.ComponentKey: teleport.Component(teleport.ComponentProcess, processID),
"pid": fmt.Sprintf("%v.%v", os.Getpid(), processID),
}))
cfg.Logger = cfg.Logger.With(
teleport.ComponentKey, teleport.Component(teleport.ComponentProcess, processID),
"pid", fmt.Sprintf("%v.%v", os.Getpid(), processID),
Expand Down
1 change: 0 additions & 1 deletion lib/service/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -948,7 +948,6 @@ func TestTeleportProcess_reconnectToAuth(t *testing.T) {
cfg.Testing.ConnectFailureC = make(chan time.Duration, 5)
cfg.Testing.ClientTimeout = time.Millisecond
cfg.InstanceMetadataClient = imds.NewDisabledIMDSClient()
cfg.Log = utils.NewLoggerForTests()
cfg.Logger = utils.NewSlogLoggerForTests()
process, err := NewTeleport(cfg)
require.NoError(t, err)
Expand Down
15 changes: 0 additions & 15 deletions lib/service/servicecfg/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ import (
"github.com/ghodss/yaml"
"github.com/gravitational/trace"
"github.com/jonboulle/clockwork"
"github.com/sirupsen/logrus"
"golang.org/x/crypto/ssh"

"github.com/gravitational/teleport"
Expand All @@ -52,7 +51,6 @@ import (
"github.com/gravitational/teleport/lib/sshca"
usagereporter "github.com/gravitational/teleport/lib/usagereporter/teleport"
"github.com/gravitational/teleport/lib/utils"
logutils "github.com/gravitational/teleport/lib/utils/log"
)

// Config contains the configuration for all services that Teleport can run.
Expand Down Expand Up @@ -223,10 +221,6 @@ type Config struct {
// Kube is a Kubernetes API gateway using Teleport client identities.
Kube KubeConfig

// Log optionally specifies the logger.
// Deprecated: use Logger instead.
Log utils.Logger

// Logger outputs messages using slog. The underlying handler respects
// the user supplied logging config.
Logger *slog.Logger
Expand Down Expand Up @@ -518,10 +512,6 @@ func ApplyDefaults(cfg *Config) {

cfg.Version = defaults.TeleportConfigVersionV1

if cfg.Log == nil {
cfg.Log = utils.NewLogger()
}

if cfg.Logger == nil {
cfg.Logger = slog.Default()
}
Expand Down Expand Up @@ -698,10 +688,6 @@ func applyDefaults(cfg *Config) {
cfg.Console = io.Discard
}

if cfg.Log == nil {
cfg.Log = logrus.StandardLogger()
}

if cfg.Logger == nil {
cfg.Logger = slog.Default()
}
Expand Down Expand Up @@ -799,7 +785,6 @@ func verifyEnabledService(cfg *Config) error {
// If called after `config.ApplyFileConfig` or `config.Configure` it will also
// change the global loggers.
func (c *Config) SetLogLevel(level slog.Level) {
c.Log.SetLevel(logutils.SlogLevelToLogrusLevel(level))
c.LoggerLevel.Set(level)
}

Expand Down
Loading

0 comments on commit 59146c8

Please sign in to comment.