Skip to content

Replace slog with zap for global logging #1459

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions cmd/thv-operator/controllers/mcpserver_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,7 @@ func (r *MCPServerReconciler) createRBACResource(
) error {
desired := createResource()
if err := controllerutil.SetControllerReference(mcpServer, desired, r.Scheme); err != nil {
logger.Error(fmt.Sprintf("Failed to set controller reference for %s", resourceType), err)
logger.Errorf("Failed to set controller reference for %s: %v", resourceType, err)
return nil
}

Expand All @@ -310,7 +310,7 @@ func (r *MCPServerReconciler) updateRBACResourceIfNeeded(
) error {
desired := createResource()
if err := controllerutil.SetControllerReference(mcpServer, desired, r.Scheme); err != nil {
logger.Error(fmt.Sprintf("Failed to set controller reference for %s", resourceType), err)
logger.Errorf("Failed to set controller reference for %s: %v", resourceType, err)
return nil
}

Expand Down Expand Up @@ -665,7 +665,7 @@ func (r *MCPServerReconciler) deploymentForMCPServer(m *mcpv1alpha1.MCPServer) *

// Set MCPServer instance as the owner and controller
if err := controllerutil.SetControllerReference(m, dep, r.Scheme); err != nil {
logger.Error("Failed to set controller reference for Deployment", err)
logger.Errorf("Failed to set controller reference for Deployment %v", err)
return nil
}
return dep
Expand Down Expand Up @@ -752,7 +752,7 @@ func (r *MCPServerReconciler) serviceForMCPServer(m *mcpv1alpha1.MCPServer) *cor

// Set MCPServer instance as the owner and controller
if err := controllerutil.SetControllerReference(m, svc, r.Scheme); err != nil {
logger.Error("Failed to set controller reference for Service", err)
logger.Errorf("Failed to set controller reference for Service %v", err)
return nil
}
return svc
Expand Down
5 changes: 2 additions & 3 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,12 @@ require (
github.com/docker/docker v28.3.3+incompatible
github.com/docker/go-connections v0.6.0
github.com/go-chi/chi/v5 v5.2.2
github.com/go-logr/zapr v1.3.0
github.com/gofrs/flock v0.12.1
github.com/google/go-containerregistry v0.20.6
github.com/google/uuid v1.6.0
github.com/lestrrat-go/httprc/v3 v3.0.1
github.com/lestrrat-go/jwx/v3 v3.0.10
github.com/lmittmann/tint v1.1.2
github.com/mark3labs/mcp-go v0.37.0
github.com/olekukonko/tablewriter v1.0.9
github.com/onsi/ginkgo/v2 v2.24.0
Expand All @@ -26,7 +26,6 @@ require (
github.com/pkg/browser v0.0.0-20240102092130-5ac0b6a4141c
github.com/prometheus/client_golang v1.23.0
github.com/santhosh-tekuri/jsonschema/v5 v5.3.1
github.com/santhosh-tekuri/jsonschema/v6 v6.0.2
github.com/sigstore/protobuf-specs v0.5.0
github.com/sigstore/sigstore-go v1.1.1
github.com/spf13/viper v1.20.1
Expand All @@ -41,6 +40,7 @@ require (
go.opentelemetry.io/otel/sdk v1.37.0
go.opentelemetry.io/otel/sdk/metric v1.37.0
go.uber.org/mock v0.6.0
go.uber.org/zap v1.27.0
golang.ngrok.com/ngrok/v2 v2.1.0
golang.org/x/exp/jsonrpc2 v0.0.0-20250813145105-42675adae3e6
golang.org/x/mod v0.27.0
Expand Down Expand Up @@ -239,7 +239,6 @@ require (
go.opentelemetry.io/proto/otlp v1.7.0 // indirect
go.uber.org/automaxprocs v1.6.0 // indirect
go.uber.org/multierr v1.11.0 // indirect
go.uber.org/zap v1.27.0 // indirect
go.yaml.in/yaml/v2 v2.4.2 // indirect
golang.ngrok.com/muxado/v2 v2.0.1 // indirect
golang.org/x/exp/event v0.0.0-20250718183923-645b1fa84792 // indirect
Expand Down
3 changes: 0 additions & 3 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -1282,8 +1282,6 @@ github.com/lib/pq v1.1.0/go.mod h1:5WUZQaWbwv1U+lTReE5YruASi9Al49XbQIvNi/34Woo=
github.com/lib/pq v1.2.0/go.mod h1:5WUZQaWbwv1U+lTReE5YruASi9Al49XbQIvNi/34Woo=
github.com/lib/pq v1.10.2/go.mod h1:AlVN5x4E4T544tWzH6hKfbfQvm3HdbOxrmggDNAPY9o=
github.com/lib/pq v1.10.7/go.mod h1:AlVN5x4E4T544tWzH6hKfbfQvm3HdbOxrmggDNAPY9o=
github.com/lmittmann/tint v1.1.2 h1:2CQzrL6rslrsyjqLDwD11bZ5OpLBPU+g3G/r5LSfS8w=
github.com/lmittmann/tint v1.1.2/go.mod h1:HIS3gSy7qNwGCj+5oRjAutErFBl4BzdQP6cJZ0NfMwE=
github.com/lucasb-eyer/go-colorful v1.2.0 h1:1nnpGOrhyZZuNyfu1QjKiUICQ74+3FNCN69Aj6K7nkY=
github.com/lucasb-eyer/go-colorful v1.2.0/go.mod h1:R4dSotOR9KMtayYi1e77YzuveK+i7ruzyGqttikkLy0=
github.com/luna-duclos/instrumentedsql v1.1.3/go.mod h1:9J1njvFds+zN7y85EDhN9XNQLANWwZt2ULeIC8yMNYs=
Expand Down Expand Up @@ -1459,7 +1457,6 @@ github.com/sagikazarmark/locafero v0.7.0 h1:5MqpDsTGNDhY8sGp0Aowyf0qKsPrhewaLSsF
github.com/sagikazarmark/locafero v0.7.0/go.mod h1:2za3Cg5rMaTMoG/2Ulr9AwtFaIppKXTRYnozin4aB5k=
github.com/santhosh-tekuri/jsonschema/v5 v5.3.1 h1:lZUw3E0/J3roVtGQ+SCrUrg3ON6NgVqpn3+iol9aGu4=
github.com/santhosh-tekuri/jsonschema/v5 v5.3.1/go.mod h1:uToXkOrWAZ6/Oc07xWQrPOhJotwFIyu2bBVN41fcDUY=
github.com/santhosh-tekuri/jsonschema/v6 v6.0.2/go.mod h1:JXeL+ps8p7/KNMjDQk3TCwPpBy0wYklyWTfbkIzdIFU=
github.com/sassoftware/relic v7.2.1+incompatible h1:Pwyh1F3I0r4clFJXkSI8bOyJINGqpgjJU3DYAZeI05A=
github.com/sassoftware/relic v7.2.1+incompatible/go.mod h1:CWfAxv73/iLZ17rbyhIEq3K9hs5w6FpNMdUT//qR+zk=
github.com/sassoftware/relic/v7 v7.6.2 h1:rS44Lbv9G9eXsukknS4mSjIAuuX+lMq/FnStgmZlUv4=
Expand Down
198 changes: 82 additions & 116 deletions pkg/logger/logger.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,193 +2,159 @@
package logger

import (
"context"
"fmt"
"log/slog"
"os"
"runtime"
"strconv"
"time"

"github.com/lmittmann/tint"
"github.com/go-logr/logr"
"github.com/go-logr/zapr"
"github.com/spf13/viper"
"go.uber.org/zap"
"go.uber.org/zap/zapcore"
)

// Log is a global logger instance
var log Logger

// Debug logs a message at debug level using the singleton logger.
func Debug(msg string, args ...any) {
log.Debug(msg, args...)
func Debug(msg string) {
zap.S().Debug(msg)
}

// Debugf logs a message at debug level using the singleton logger.
func Debugf(msg string, args ...any) {
log.Debugf(msg, args...)
zap.S().Debugf(msg, args...)
}

// Debugw logs a message at debug level using the singleton logger with additional key-value pairs.
func Debugw(msg string, keysAndValues ...any) {
zap.S().Debugw(msg, keysAndValues...)
}

// Info logs a message at info level using the singleton logger.
func Info(msg string, args ...any) {
log.Info(msg, args...)
func Info(msg string) {
zap.S().Info(msg)
}

// Infof logs a message at info level using the singleton logger.
func Infof(msg string, args ...any) {
log.Infof(msg, args...)
zap.S().Infof(msg, args...)
}

// Infow logs a message at info level using the singleton logger with additional key-value pairs.
func Infow(msg string, keysAndValues ...any) {
zap.S().Infow(msg, keysAndValues...)
}

// Warn logs a message at warning level using the singleton logger.
func Warn(msg string, args ...any) {
log.Warn(msg, args...)
func Warn(msg string) {
zap.S().Warn(msg)
}

// Warnf logs a message at warning level using the singleton logger.
func Warnf(msg string, args ...any) {
log.Warnf(msg, args...)
zap.S().Warnf(msg, args...)
}

// Warnw logs a message at warning level using the singleton logger with additional key-value pairs.
func Warnw(msg string, keysAndValues ...any) {
zap.S().Warnw(msg, keysAndValues...)
}

// Error logs a message at error level using the singleton logger.
func Error(msg string, args ...any) {
log.Error(msg, args...)
func Error(msg string) {
zap.S().Error(msg)
}

// Errorf logs a message at error level using the singleton logger.
func Errorf(msg string, args ...any) {
log.Errorf(msg, args...)
zap.S().Errorf(msg, args...)
}

// Errorw logs a message at error level using the singleton logger with additional key-value pairs.
func Errorw(msg string, keysAndValues ...any) {
zap.S().Errorw(msg, keysAndValues...)
}

// Panic logs a message at error level using the singleton logger and panics the program.
func Panic(msg string) {
log.Panic(msg)
zap.S().Panic(msg)
}

// Panicf logs a message at error level using the singleton logger and panics the program.
func Panicf(msg string, args ...any) {
log.Panicf(msg, args...)
zap.S().Panicf(msg, args...)
}

// Logger provides a unified interface for logging
type Logger interface {
Debug(msg string, args ...any)
Debugf(msg string, args ...any)
Info(msg string, args ...any)
Infof(msg string, args ...any)
Warn(msg string, args ...any)
Warnf(msg string, args ...any)
Error(msg string, args ...any)
Errorf(msg string, args ...any)
Panic(msg string)
Panicf(msg string, args ...any)
// Panicw logs a message at error level using the singleton logger with additional key-value pairs and panics the program.
func Panicw(msg string, keysAndValues ...any) {
zap.S().Panicw(msg, keysAndValues...)
}

// Implementation using slog
type slogLogger struct {
logger *slog.Logger
// DPanic logs a message at error level using the singleton logger and panics the program.
func DPanic(msg string) {
zap.S().DPanic(msg)
}

func (l *slogLogger) Debugf(msg string, args ...any) {
l.logger.Debug(fmt.Sprintf(msg, args...))
// DPanicf logs a message at error level using the singleton logger and panics the program.
func DPanicf(msg string, args ...any) {
zap.S().DPanicf(msg, args...)
}

func (l *slogLogger) Infof(msg string, args ...any) {
l.logger.Info(fmt.Sprintf(msg, args...))
// DPanicw logs a message at error level using the singleton logger with additional key-value pairs and panics the program.
func DPanicw(msg string, keysAndValues ...any) {
zap.S().DPanicw(msg, keysAndValues...)
}

func (l *slogLogger) Warnf(msg string, args ...any) {
l.logger.Warn(fmt.Sprintf(msg, args...))
// Fatal logs a message at error level using the singleton logger and exits the program.
func Fatal(msg string) {
zap.S().Fatal(msg)
}

func (l *slogLogger) Errorf(msg string, args ...any) {
l.logger.Error(fmt.Sprintf(msg, args...))
// Fatalf logs a message at error level using the singleton logger and exits the program.
func Fatalf(msg string, args ...any) {
zap.S().Fatalf(msg, args...)
}

func (l *slogLogger) Panicf(msg string, args ...any) {
l.Panic(fmt.Sprintf(msg, args...))
// Fatalw logs a message at error level using the singleton logger with additional key-value pairs and exits the program.
func Fatalw(msg string, keysAndValues ...any) {
zap.S().Fatalw(msg, keysAndValues...)
}

func (l *slogLogger) Debug(msg string, args ...any) {
l.logger.Debug(msg, args...)
}

func (l *slogLogger) Info(msg string, args ...any) {
l.logger.Info(msg, args...)
}

func (l *slogLogger) Warn(msg string, args ...any) {
l.logger.Warn(msg, args...)
}

func (l *slogLogger) Error(msg string, args ...any) {
l.logger.Error(msg, args...)
}

func (l *slogLogger) Panic(msg string) {
var pcs [1]uintptr
runtime.Callers(2, pcs[:]) // skip [Callers, Panic]
record := slog.NewRecord(time.Now(), slog.LevelError, msg, pcs[0])
_ = l.logger.Handler().Handle(context.Background(), record)
panic(msg)
}

func unstructuredLogs() bool {
unstructuredLogs, err := strconv.ParseBool(os.Getenv("UNSTRUCTURED_LOGS"))
if err != nil {
// at this point if the error is not nil, the env var wasn't set, or is ""
// which means we just default to outputting unstructured logs.
return true
}
return unstructuredLogs
// NewLogr returns a logr.Logger which uses zap logger
func NewLogr() logr.Logger {
return zapr.NewLogger(zap.L())
}

// Initialize creates and configures the appropriate logger.
// If the UNSTRUCTURED_LOGS is set to true, it will output plain log message
// with only time and LogLevelType (INFO, DEBUG, ERROR, WARN)).
// Otherwise it will create a standard structured slog logger
func Initialize() {
var config zap.Config
if unstructuredLogs() {
w := os.Stderr

handler := tint.NewHandler(w, &tint.Options{
Level: getLogLevel(),
TimeFormat: time.Kitchen,
})

slogger := slog.New(handler)

slog.SetDefault(slogger)
log = &slogLogger{logger: slogger}
config = zap.NewDevelopmentConfig()
config.EncoderConfig.EncodeLevel = zapcore.CapitalColorLevelEncoder
config.EncoderConfig.EncodeTime = zapcore.TimeEncoderOfLayout(time.Kitchen)
config.OutputPaths = []string{"stderr"}
} else {
w := os.Stdout

handler := slog.NewJSONHandler(w, &slog.HandlerOptions{
Level: getLogLevel(),
})

slogger := slog.New(handler)

slog.SetDefault(slogger)
log = &slogLogger{logger: slogger}
config = zap.NewProductionConfig()
config.OutputPaths = []string{"stdout"}
}
}

// GetLogger returns a context-specific logger
func GetLogger(component string) Logger {
if slogger, ok := log.(*slogLogger); ok {
return &slogLogger{
logger: slogger.logger.With("component", component),
}
// Set log level based on current debug flag
if viper.GetBool("debug") {
config.Level = zap.NewAtomicLevelAt(zap.DebugLevel)
} else {
config.Level = zap.NewAtomicLevelAt(zap.InfoLevel)
}

return log
zap.ReplaceGlobals(zap.Must(config.Build()))
}

// getLogLevel returns the appropriate slog.Level based on the debug flag
func getLogLevel() slog.Level {
var level slog.Level
if viper.GetBool("debug") {
level = slog.LevelDebug
} else {
level = slog.LevelInfo
func unstructuredLogs() bool {
unstructuredLogs, err := strconv.ParseBool(os.Getenv("UNSTRUCTURED_LOGS"))
if err != nil {
// at this point if the error is not nil, the env var wasn't set, or is ""
// which means we just default to outputting unstructured logs.
return true
}
return level
return unstructuredLogs
}
Loading
Loading