refactor logger to zerolog (#1239)

* refactor logger to zerolog

* modify dingtalk and discord logger

* fix for lint

* fix for review

* fix for file leak

* fix for review

go.mod

@@ -21,6 +21,7 @@ require (
 	github.com/open-dingtalk/dingtalk-stream-sdk-go v0.9.1
 	github.com/openai/openai-go/v3 v3.22.0
 	github.com/rivo/tview v0.42.0
+	github.com/rs/zerolog v1.34.0
 	github.com/slack-go/slack v0.17.3
 	github.com/spf13/cobra v1.10.2
 	github.com/stretchr/testify v1.11.1
@@ -50,7 +51,6 @@ require (
 	github.com/pmezard/go-difflib v1.0.0 // indirect
 	github.com/remyoudompheng/bigfft v0.0.0-20230129092748-24d4a6f8daec // indirect
 	github.com/rivo/uniseg v0.4.7 // indirect
-	github.com/rs/zerolog v1.34.0 // indirect
 	github.com/segmentio/asm v1.1.3 // indirect
 	github.com/segmentio/encoding v0.5.3 // indirect
 	github.com/spf13/pflag v1.0.10 // indirect

pkg/channels/dingtalk/dingtalk.go

@@ -10,6 +10,7 @@ import (
 
 	"github.com/open-dingtalk/dingtalk-stream-sdk-go/chatbot"
 	"github.com/open-dingtalk/dingtalk-stream-sdk-go/client"
+	dinglog "github.com/open-dingtalk/dingtalk-stream-sdk-go/logger"
 
 	"github.com/sipeed/picoclaw/pkg/bus"
 	"github.com/sipeed/picoclaw/pkg/channels"
@@ -39,6 +40,9 @@ func NewDingTalkChannel(cfg config.DingTalkConfig, messageBus *bus.MessageBus) (
 		return nil, fmt.Errorf("dingtalk client_id and client_secret are required")
 	}
 
+	// Set the logger for the Stream SDK
+	dinglog.SetLogger(logger.NewLogger("dingtalk"))
+
 	base := channels.NewBaseChannel("dingtalk", cfg, messageBus, cfg.AllowFrom,
 		channels.WithMaxMessageLength(20000),
 		channels.WithGroupTrigger(cfg.GroupTrigger),

pkg/channels/discord/discord.go

@@ -45,6 +45,14 @@ type DiscordChannel struct {
 }
 
 func NewDiscordChannel(cfg config.DiscordConfig, bus *bus.MessageBus) (*DiscordChannel, error) {
+	discordgo.Logger = logger.NewLogger("discord").
+		WithLevels(map[int]logger.LogLevel{
+			discordgo.LogError:         logger.ERROR,
+			discordgo.LogWarning:       logger.WARN,
+			discordgo.LogInformational: logger.INFO,
+			discordgo.LogDebug:         logger.DEBUG,
+		}).Log
+
 	session, err := discordgo.New("Bot " + cfg.Token)
 	if err != nil {
 		return nil, fmt.Errorf("failed to create discord session: %w", err)

pkg/channels/qq/qq.go

@@ -78,6 +78,7 @@ func (c *QQChannel) Start(ctx context.Context) error {
 		return fmt.Errorf("QQ app_id and app_secret not configured")
 	}
 
+	botgo.SetLogger(logger.NewLogger("botgo"))
 	logger.InfoC("qq", "Starting QQ bot (WebSocket mode)")
 
 	// Reinitialize shutdown signal for clean restart.

pkg/channels/telegram/telegram.go

@@ -77,6 +77,7 @@ func NewTelegramChannel(cfg *config.Config, bus *bus.MessageBus) (*TelegramChann
 	if baseURL := strings.TrimRight(strings.TrimSpace(telegramCfg.BaseURL), "/"); baseURL != "" {
 		opts = append(opts, telego.WithAPIServer(baseURL))
 	}
+	opts = append(opts, telego.WithLogger(logger.NewLogger("telego")))
 
 	bot, err := telego.NewBot(telegramCfg.Token, opts...)
 	if err != nil {

pkg/logger/logger.go

@@ -1,24 +1,24 @@
 package logger
 
 import (
-	"encoding/json"
 	"fmt"
-	"log"
 	"os"
+	"path/filepath"
 	"runtime"
 	"strings"
 	"sync"
-	"time"
+
+	"github.com/rs/zerolog"
 )
 
-type LogLevel int
+type LogLevel = zerolog.Level
 
 const (
-	DEBUG LogLevel = iota
+	DEBUG = zerolog.DebugLevel
-	INFO
+	INFO  = zerolog.InfoLevel
-	WARN
+	WARN  = zerolog.WarnLevel
-	ERROR
+	ERROR = zerolog.ErrorLevel
-	FATAL
+	FATAL = zerolog.FatalLevel
 )
 
 var (
@@ -31,27 +31,24 @@ var (
 	}
 
 	currentLevel = INFO
-	logger       *Logger
+	logger       zerolog.Logger
+	fileLogger   zerolog.Logger
+	logFile      *os.File
 	once         sync.Once
 	mu           sync.RWMutex
 )
 
-type Logger struct {
-	file *os.File
-}
-
-type LogEntry struct {
-	Level     string         `json:"level"`
-	Timestamp string         `json:"timestamp"`
-	Component string         `json:"component,omitempty"`
-	Message   string         `json:"message"`
-	Fields    map[string]any `json:"fields,omitempty"`
-	Caller    string         `json:"caller,omitempty"`
-}
-
 func init() {
 	once.Do(func() {
-		logger = &Logger{}
+		zerolog.SetGlobalLevel(zerolog.InfoLevel)
+
+		consoleWriter := zerolog.ConsoleWriter{
+			Out:        os.Stdout,
+			TimeFormat: "15:04:05", // TODO: make it configurable???
+		}
+
+		logger = zerolog.New(consoleWriter).With().Timestamp().Logger()
+		fileLogger = zerolog.Logger{}
 	})
 }
 
@@ -59,6 +56,7 @@ func SetLevel(level LogLevel) {
 	mu.Lock()
 	defer mu.Unlock()
 	currentLevel = level
+	zerolog.SetGlobalLevel(level)
 }
 
 func GetLevel() LogLevel {
@@ -71,17 +69,22 @@ func EnableFileLogging(filePath string) error {
 	mu.Lock()
 	defer mu.Unlock()
 
-	file, err := os.OpenFile(filePath, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0o644)
+	if err := os.MkdirAll(filepath.Dir(filePath), 0o755); err != nil {
+		return fmt.Errorf("failed to create log directory: %w", err)
+	}
+
+	newFile, err := os.OpenFile(filePath, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0o644)
 	if err != nil {
 		return fmt.Errorf("failed to open log file: %w", err)
 	}
 
-	if logger.file != nil {
+	// Close old file if exists
-		logger.file.Close()
+	if logFile != nil {
+		logFile.Close()
 	}
 
-	logger.file = file
+	logFile = newFile
-	log.Println("File logging enabled:", filePath)
+	fileLogger = zerolog.New(logFile).With().Timestamp().Caller().Logger()
 	return nil
 }
 
@@ -89,10 +92,57 @@ func DisableFileLogging() {
 	mu.Lock()
 	defer mu.Unlock()
 
-	if logger.file != nil {
+	if logFile != nil {
-		logger.file.Close()
+		logFile.Close()
-		logger.file = nil
+		logFile = nil
-		log.Println("File logging disabled")
+	}
+	fileLogger = zerolog.Logger{}
+}
+
+func getCallerInfo() (string, int, string) {
+	for i := 2; i < 15; i++ {
+		pc, file, line, ok := runtime.Caller(i)
+		if !ok {
+			continue
+		}
+
+		fn := runtime.FuncForPC(pc)
+		if fn == nil {
+			continue
+		}
+
+		// bypass common loggers
+		if strings.HasSuffix(file, "/logger.go") ||
+			strings.HasSuffix(file, "/log.go") {
+			continue
+		}
+
+		funcName := fn.Name()
+		if strings.HasPrefix(funcName, "runtime.") {
+			continue
+		}
+
+		return filepath.Base(file), line, filepath.Base(funcName)
+	}
+
+	return "???", 0, "???"
+}
+
+//nolint:zerologlint
+func getEvent(logger zerolog.Logger, level LogLevel) *zerolog.Event {
+	switch level {
+	case zerolog.DebugLevel:
+		return logger.Debug()
+	case zerolog.InfoLevel:
+		return logger.Info()
+	case zerolog.WarnLevel:
+		return logger.Warn()
+	case zerolog.ErrorLevel:
+		return logger.Error()
+	case zerolog.FatalLevel:
+		return logger.Fatal()
+	default:
+		return logger.Info()
 	}
 }
 
@@ -101,65 +151,41 @@ func logMessage(level LogLevel, component string, message string, fields map[str
 		return
 	}
 
-	entry := LogEntry{
+	callerFile, callerLine, callerFunc := getCallerInfo()
-		Level:     logLevelNames[level],
-		Timestamp: time.Now().UTC().Format(time.RFC3339),
-		Component: component,
-		Message:   message,
-		Fields:    fields,
-	}
 
-	if pc, file, line, ok := runtime.Caller(2); ok {
+	event := getEvent(logger, level)
-		fn := runtime.FuncForPC(pc)
-		if fn != nil {
-			entry.Caller = fmt.Sprintf("%s:%d (%s)", file, line, fn.Name())
-		}
-	}
 
-	if logger.file != nil {
+	// Build combined field with component and caller
-		jsonData, err := json.Marshal(entry)
+	if component != "" {
-		if err == nil {
+		event.Str("caller", fmt.Sprintf("%-6s %s:%d (%s)", component, callerFile, callerLine, callerFunc))
-			logger.file.Write(append(jsonData, '\n'))
-		}
-	}
-
-	var fieldStr string
-	if len(fields) > 0 {
-		fieldStr = " " + formatFields(fields)
 	} else {
-		fieldStr = ""
+		event.Str("caller", fmt.Sprintf("<none> %s:%d (%s)", callerFile, callerLine, callerFunc))
 	}
 
-	logLine := fmt.Sprintf("[%s] [%s]%s %s%s",
+	for k, v := range fields {
-		entry.Timestamp,
+		event.Interface(k, v)
-		logLevelNames[level],
+	}
-		formatComponent(component),
-		message,
-		fieldStr,
-	)
 
-	log.Println(logLine)
+	event.Msg(message)
+
+	// Also log to file if enabled
+	if fileLogger.GetLevel() != zerolog.NoLevel {
+		fileEvent := getEvent(fileLogger, level)
+
+		if component != "" {
+			fileEvent.Str("component", component)
+		}
+		for k, v := range fields {
+			fileEvent.Interface(k, v)
+		}
+		fileEvent.Msg(message)
+	}
 
 	if level == FATAL {
 		os.Exit(1)
 	}
 }
 
-func formatComponent(component string) string {
-	if component == "" {
-		return ""
-	}
-	return fmt.Sprintf(" %s:", component)
-}
-
-func formatFields(fields map[string]any) string {
-	parts := make([]string, 0, len(fields))
-	for k, v := range fields {
-		parts = append(parts, fmt.Sprintf("%s=%v", k, v))
-	}
-	return fmt.Sprintf("{%s}", strings.Join(parts, ", "))
-}
-
 func Debug(message string) {
 	logMessage(DEBUG, "", message, nil)
 }
@@ -232,6 +258,10 @@ func FatalC(component string, message string) {
 	logMessage(FATAL, component, message, nil)
 }
 
+func Fatalf(message string, ss ...any) {
+	logMessage(FATAL, "", fmt.Sprintf(message, ss...), nil)
+}
+
 func FatalF(message string, fields map[string]any) {
 	logMessage(FATAL, "", message, fields)
 }

pkg/logger/logger_3rd_party.go

@@ -0,0 +1,95 @@
+// this file is for compatible with 3rd party loggers, should not be called in PicoClaw project
+
+package logger
+
+import "fmt"
+
+// Logger implements common Logger interface
+type Logger struct {
+	component string
+	levels    map[int]LogLevel
+}
+
+// Debug logs debug messages
+func (b *Logger) Debug(v ...any) {
+	logMessage(DEBUG, b.component, fmt.Sprint(v...), nil)
+}
+
+// Info logs info messages
+func (b *Logger) Info(v ...any) {
+	logMessage(INFO, b.component, fmt.Sprint(v...), nil)
+}
+
+// Warn logs warning messages
+func (b *Logger) Warn(v ...any) {
+	logMessage(WARN, b.component, fmt.Sprint(v...), nil)
+}
+
+// Error logs error messages
+func (b *Logger) Error(v ...any) {
+	logMessage(ERROR, b.component, fmt.Sprint(v...), nil)
+}
+
+// Debugf logs formatted debug messages
+func (b *Logger) Debugf(format string, v ...any) {
+	logMessage(DEBUG, b.component, fmt.Sprintf(format, v...), nil)
+}
+
+// Infof logs formatted info messages
+func (b *Logger) Infof(format string, v ...any) {
+	logMessage(INFO, b.component, fmt.Sprintf(format, v...), nil)
+}
+
+// Warnf logs formatted warning messages
+func (b *Logger) Warnf(format string, v ...any) {
+	logMessage(WARN, b.component, fmt.Sprintf(format, v...), nil)
+}
+
+// Warningf logs formatted warning messages
+func (b *Logger) Warningf(format string, v ...any) {
+	logMessage(WARN, b.component, fmt.Sprintf(format, v...), nil)
+}
+
+// Errorf logs formatted error messages
+func (b *Logger) Errorf(format string, v ...any) {
+	logMessage(ERROR, b.component, fmt.Sprintf(format, v...), nil)
+}
+
+// Fatalf logs formatted fatal messages and exits
+func (b *Logger) Fatalf(format string, v ...any) {
+	logMessage(FATAL, b.component, fmt.Sprintf(format, v...), nil)
+}
+
+// Log logs a message at a given level with caller information
+// the func name must be this because 3rd party loggers expect this
+// msgL: message level (DEBUG, INFO, WARN, ERROR, FATAL)
+// caller: unused parameter reserved for compatibility
+// format: format string
+// a: format arguments
+//
+//nolint:goprintffuncname
+func (b *Logger) Log(msgL, caller int, format string, a ...any) {
+	level := LogLevel(msgL)
+	if b.levels != nil {
+		if lvl, ok := b.levels[msgL]; ok {
+			level = lvl
+		}
+	}
+	logMessage(level, b.component, fmt.Sprintf(format, a...), nil)
+}
+
+// Sync flushes log buffer (no-op for this implementation)
+func (b *Logger) Sync() error {
+	return nil
+}
+
+// WithLevels sets log levels mapping for this logger
+func (b *Logger) WithLevels(levels map[int]LogLevel) *Logger {
+	b.levels = levels
+	return b
+}
+
+// NewLogger creates a new logger instance with optional component name
+func NewLogger(component string) *Logger {
+	return &Logger{component: component}
+}