Welcome to mirror list, hosted at ThFree Co, Russian Federation.

gitlab.com/gitlab-org/gitaly.git - Unnamed repository; edit this file 'description' to name the repository.
summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorPatrick Steinhardt <psteinhardt@gitlab.com>2023-09-21 13:37:21 +0300
committerPatrick Steinhardt <psteinhardt@gitlab.com>2023-09-21 14:27:36 +0300
commit9a75130ccaa40ec64ef4ebee81bff1f444335469 (patch)
tree5f23f1a52bdb7933faa47920321c98c3880d0591
parente52fdb6a6f09a1cfe7192272a7aabff0d7c84c4b (diff)
log: Hide away the `logrus.Entry` interface
The `log.LogrusLogger` type directly wraps the `logrus.Entry`, thus exposing all of its functions. We don't want anybody to call them in the ideal case so that we can ensure that we use our own logging interface almost exclusively throughout the codebase. Let's hide the `logrus.Entry` by making it a private member of the `log.LogrusLogger`. While this requires us to explicitly implement some of the expected interface-level functions, by now it's really only four of them and thus manageable. There are still some very limited callsites in our tests that require access to the `logrus.Entry`, which will be addressed in another iteration. Meanwhile, we expose an explicit `LogrusEntry()` getter that makes the entry accessible to those usecases.
-rw-r--r--client/dial_test.go2
-rw-r--r--internal/command/command_test.go2
-rw-r--r--internal/grpc/middleware/featureflag/featureflag_handler_test.go2
-rw-r--r--internal/log/logger.go55
-rw-r--r--internal/log/middleware.go4
-rw-r--r--internal/praefect/repocleaner/repository_test.go2
-rw-r--r--internal/testhelper/logger.go2
-rw-r--r--internal/testhelper/testserver/praefect.go2
8 files changed, 50 insertions, 21 deletions
diff --git a/client/dial_test.go b/client/dial_test.go
index c04f23046..ff3716e77 100644
--- a/client/dial_test.go
+++ b/client/dial_test.go
@@ -805,5 +805,5 @@ func startFakeGitalyServer(t *testing.T) string {
}
func newLogger(tb testing.TB) *logrus.Entry {
- return testhelper.SharedLogger(tb).Entry
+ return testhelper.SharedLogger(tb).LogrusEntry() //nolint:staticcheck
}
diff --git a/internal/command/command_test.go b/internal/command/command_test.go
index ad1b5fd6c..209e84b3b 100644
--- a/internal/command/command_test.go
+++ b/internal/command/command_test.go
@@ -494,7 +494,7 @@ func TestCommand_logMessage(t *testing.T) {
t.Parallel()
logger := testhelper.NewLogger(t)
- logger.Logger.SetLevel(logrus.DebugLevel)
+ logger.LogrusEntry().Logger.SetLevel(logrus.DebugLevel) //nolint:staticcheck
hook := testhelper.AddLoggerHook(logger)
ctx := logger.ToContext(testhelper.Context(t))
diff --git a/internal/grpc/middleware/featureflag/featureflag_handler_test.go b/internal/grpc/middleware/featureflag/featureflag_handler_test.go
index f0b62f1b8..45eb31d80 100644
--- a/internal/grpc/middleware/featureflag/featureflag_handler_test.go
+++ b/internal/grpc/middleware/featureflag/featureflag_handler_test.go
@@ -39,7 +39,7 @@ func TestFeatureFlagLogs(t *testing.T) {
server := grpc.NewServer(
grpc.ChainUnaryInterceptor(
grpcmwlogrus.UnaryServerInterceptor(
- logger.Entry,
+ logger.LogrusEntry(), //nolint:staticcheck
grpcmwlogrus.WithMessageProducer(
log.MessageProducer(
grpcmwlogrus.DefaultMessageProducer,
diff --git a/internal/log/logger.go b/internal/log/logger.go
index ebed6293e..a1baac5c8 100644
--- a/internal/log/logger.go
+++ b/internal/log/logger.go
@@ -34,69 +34,98 @@ type Logger interface {
// LogrusLogger is an implementation of the Logger interface that is implemented via a `logrus.FieldLogger`.
type LogrusLogger struct {
- *logrus.Entry
+ entry *logrus.Entry
}
// FromLogrusEntry constructs a new Gitaly-specific logger from a `logrus.Logger`.
func FromLogrusEntry(entry *logrus.Entry) LogrusLogger {
- return LogrusLogger{Entry: entry}
+ return LogrusLogger{entry: entry}
+}
+
+// LogrusEntry returns the `logrus.Entry` that backs this logger. Note that this interface only exists during the
+// transition period and will be eventually removed. It is thus heavily discouraged to use it.
+//
+// Deprecated: This will be removed once all callsites have been converted to do something that is independent of the
+// logrus logger.
+func (l LogrusLogger) LogrusEntry() *logrus.Entry {
+ return l.entry
}
// WithField creates a new logger with the given field appended.
func (l LogrusLogger) WithField(key string, value any) Logger {
- return LogrusLogger{Entry: l.Entry.WithField(key, value)}
+ return LogrusLogger{entry: l.entry.WithField(key, value)}
}
// WithFields creates a new logger with the given fields appended.
func (l LogrusLogger) WithFields(fields Fields) Logger {
- return LogrusLogger{Entry: l.Entry.WithFields(fields)}
+ return LogrusLogger{entry: l.entry.WithFields(fields)}
}
// WithError creates a new logger with an appended error field.
func (l LogrusLogger) WithError(err error) Logger {
- return LogrusLogger{Entry: l.Entry.WithError(err)}
+ return LogrusLogger{entry: l.entry.WithError(err)}
+}
+
+// Debugf writes a formatted log message at debug level.
+func (l LogrusLogger) Debugf(format string, args ...any) {
+ l.entry.Debugf(format, args...)
+}
+
+// Infof writes a formatted log message at info level.
+func (l LogrusLogger) Infof(format string, args ...any) {
+ l.entry.Infof(format, args...)
+}
+
+// Warnf writes a formatted log message at warn level.
+func (l LogrusLogger) Warnf(format string, args ...any) {
+ l.entry.Warnf(format, args...)
+}
+
+// Errorf writes a formatted log message at error level.
+func (l LogrusLogger) Errorf(format string, args ...any) {
+ l.entry.Errorf(format, args...)
}
// Debug writes a log message at debug level.
func (l LogrusLogger) Debug(msg string) {
- l.Entry.Debug(msg)
+ l.entry.Debug(msg)
}
// Info writes a log message at info level.
func (l LogrusLogger) Info(msg string) {
- l.Entry.Info(msg)
+ l.entry.Info(msg)
}
// Warn writes a log message at warn level.
func (l LogrusLogger) Warn(msg string) {
- l.Entry.Warn(msg)
+ l.entry.Warn(msg)
}
// Error writes a log message at error level.
func (l LogrusLogger) Error(msg string) {
- l.Entry.Error(msg)
+ l.entry.Error(msg)
}
// ToContext injects the logger into the given context so that it can be retrieved via `FromContext()`.
func (l LogrusLogger) ToContext(ctx context.Context) context.Context {
- return ctxlogrus.ToContext(ctx, l.Entry)
+ return ctxlogrus.ToContext(ctx, l.entry)
}
// StreamServerInterceptor creates a gRPC interceptor that generates log messages for streaming RPC calls.
func (l LogrusLogger) StreamServerInterceptor(opts ...grpcmwlogrus.Option) grpc.StreamServerInterceptor {
- return grpcmwlogrus.StreamServerInterceptor(l.Entry, opts...)
+ return grpcmwlogrus.StreamServerInterceptor(l.entry, opts...)
}
// UnaryServerInterceptor creates a gRPC interceptor that generates log messages for unary RPC calls.
func (l LogrusLogger) UnaryServerInterceptor(opts ...grpcmwlogrus.Option) grpc.UnaryServerInterceptor {
- return grpcmwlogrus.UnaryServerInterceptor(l.Entry, opts...)
+ return grpcmwlogrus.UnaryServerInterceptor(l.entry, opts...)
}
// FromContext extracts the logger from the context. If no logger has been injected then this will return a discarding
// logger.
func FromContext(ctx context.Context) LogrusLogger {
return LogrusLogger{
- Entry: ctxlogrus.Extract(ctx),
+ entry: ctxlogrus.Extract(ctx),
}
}
diff --git a/internal/log/middleware.go b/internal/log/middleware.go
index d177132d2..4cb82541a 100644
--- a/internal/log/middleware.go
+++ b/internal/log/middleware.go
@@ -190,7 +190,7 @@ func UnaryLogDataCatcherServerInterceptor() grpc.UnaryServerInterceptor {
return func(ctx context.Context, req interface{}, info *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (interface{}, error) {
mpp := messageProducerPropagationFrom(ctx)
if mpp != nil {
- mpp.fields = FromContext(ctx).Entry.Data
+ mpp.fields = FromContext(ctx).entry.Data
}
return handler(ctx, req)
}
@@ -203,7 +203,7 @@ func StreamLogDataCatcherServerInterceptor() grpc.StreamServerInterceptor {
ctx := ss.Context()
mpp := messageProducerPropagationFrom(ctx)
if mpp != nil {
- mpp.fields = FromContext(ctx).Entry.Data
+ mpp.fields = FromContext(ctx).entry.Data
}
return handler(srv, ss)
}
diff --git a/internal/praefect/repocleaner/repository_test.go b/internal/praefect/repocleaner/repository_test.go
index ed5f55bb8..94501ea6c 100644
--- a/internal/praefect/repocleaner/repository_test.go
+++ b/internal/praefect/repocleaner/repository_test.go
@@ -287,7 +287,7 @@ func TestRunner_Run_noAvailableStorages(t *testing.T) {
defer close(releaseFirst)
logger := testhelper.NewLogger(t)
- logger.Logger.SetLevel(logrus.DebugLevel)
+ logger.LogrusEntry().Logger.SetLevel(logrus.DebugLevel) //nolint:staticcheck
loggerHook := testhelper.AddLoggerHook(logger)
runner := NewRunner(cfg, logger, praefect.StaticHealthChecker{virtualStorage: []string{storage1}}, nodeSet.Connections(), storageCleanup, storageCleanup, actionStub{
diff --git a/internal/testhelper/logger.go b/internal/testhelper/logger.go
index bac918e51..8e47d4ec1 100644
--- a/internal/testhelper/logger.go
+++ b/internal/testhelper/logger.go
@@ -103,7 +103,7 @@ type LoggerHook struct {
// AddLoggerHook installs a hook on the logger.
func AddLoggerHook(logger log.LogrusLogger) LoggerHook {
- return LoggerHook{hook: test.NewLocal(logger.Logger)}
+ return LoggerHook{hook: test.NewLocal(logger.LogrusEntry().Logger)} //nolint:staticcheck
}
// AllEntries returns all log entries that have been intercepted by the hook.
diff --git a/internal/testhelper/testserver/praefect.go b/internal/testhelper/testserver/praefect.go
index 4024a69e4..59331264c 100644
--- a/internal/testhelper/testserver/praefect.go
+++ b/internal/testhelper/testserver/praefect.go
@@ -64,7 +64,7 @@ func StartPraefect(tb testing.TB, cfg config.Config) PraefectServer {
// Redirect log output of the server to the Praefect server logger. This will cause us to write logs into a
// Praefect-specific logger.
- logWriter := testhelper.NewLogger(tb, testhelper.WithLoggerName("praefect")).Entry.WithField("component", "praefect").Writer()
+ logWriter := testhelper.NewLogger(tb, testhelper.WithLoggerName("praefect")).LogrusEntry().WithField("component", "praefect").Writer() //nolint:staticcheck
tb.Cleanup(func() { testhelper.MustClose(tb, logWriter) })
cmd := exec.Command(binaryPath, "-config", configFilePath)