-
Notifications
You must be signed in to change notification settings - Fork 3
Closed as not planned
Closed as not planned
Copy link
Description
🔍 Duplicate Code Pattern: Logger Initialization and Cleanup
Part of duplicate code analysis: #264
Summary
Three logger implementations (FileLogger, JSONLLogger, MarkdownLogger) share nearly identical initialization and cleanup code with minor variations. This pattern appears in 4 files and represents ~120 lines of duplicated logic.
Duplication Details
Pattern: Init*Logger Functions
- Severity: High
- Occurrences: 3 instances
- Locations:
internal/logger/file_logger.go(lines 30-63)internal/logger/jsonl_logger.go(lines 39-64)internal/logger/markdown_logger.go(lines 28-56)
Code Sample from file_logger.go:
func InitFileLogger(logDir, fileName string) error {
globalLoggerMu.Lock()
defer globalLoggerMu.Unlock()
if globalFileLogger != nil {
// Close existing logger
globalFileLogger.Close()
}
fl := &FileLogger{
logDir: logDir,
fileName: fileName,
}
// Try to initialize the log file
file, err := initLogFile(logDir, fileName, os.O_APPEND)
if err != nil {
// File initialization failed - fallback to stdout
log.Printf("WARNING: Failed to initialize log file: %v", err)
log.Printf("WARNING: Falling back to stdout for logging")
fl.useFallback = true
fl.logger = log.New(os.Stdout, "", 0)
globalFileLogger = fl
return nil
}
fl.logFile = file
fl.logger = log.New(file, "", 0)
log.Printf("Logging to file: %s", filepath.Join(logDir, fileName))
globalFileLogger = fl
return nil
}Similar patterns in:
InitJSONLLogger()- Nearly identical structureInitMarkdownLogger()- Same pattern with minor variations
Pattern: Close*Logger Functions
- Severity: High
- Occurrences: 3 instances
- Locations:
internal/logger/file_logger.go(lines 156-167)internal/logger/jsonl_logger.go(lines 104-115)internal/logger/markdown_logger.go(lines 223-234)
Code Sample:
func CloseGlobalLogger() error {
globalLoggerMu.Lock()
defer globalLoggerMu.Unlock()
if globalFileLogger != nil {
err := globalFileLogger.Close()
globalFileLogger = nil
return err
}
return nil
}Identical implementations for:
CloseJSONLLogger()CloseMarkdownLogger()
Pattern: Log Level Functions (LogInfo, LogWarn, LogError, LogDebug)
- Severity: Medium
- Occurrences: 12 functions (4 levels × 3 loggers)
- Locations:
internal/logger/file_logger.go(lines 116-154) - 4 functionsinternal/logger/markdown_logger.go(lines 167-221) - 4 functions (with dual logging)
Code Sample:
func LogInfo(category, format string, args ...interface{}) {
globalLoggerMu.RLock()
defer globalLoggerMu.RUnlock()
if globalFileLogger != nil {
globalFileLogger.Log(LogLevelInfo, category, format, args...)
}
}
func LogWarn(category, format string, args ...interface{}) {
globalLoggerMu.RLock()
defer globalLoggerMu.RUnlock()
if globalFileLogger != nil {
globalFileLogger.Log(LogLevelWarn, category, format, args...)
}
}
// ... LogError and LogDebug follow identical patternAdditional duplication in markdown_logger.go:
LogInfoMd(),LogWarnMd(),LogErrorMd(),LogDebugMd()- These duplicate the above pattern with additional markdown logger calls
Impact Analysis
- Maintainability: High impact - Any change to initialization logic requires updating 3 files consistently
- Bug Risk: High - Inconsistent updates between loggers can lead to subtle bugs (e.g., different fallback behavior)
- Code Bloat: ~120 lines of duplicated logic across logger implementations
- Testing Overhead: Each logger implementation requires separate but nearly identical tests
Refactoring Recommendations
1. Create Base Logger Interface and Common Implementation
- Extract common initialization logic to:
internal/logger/base_logger.go - Estimated effort: 4-6 hours
- Benefits:
- Single source of truth for logger lifecycle
- Consistent error handling across all loggers
- Reduced test duplication
Suggested structure:
// baseLogger provides common initialization and cleanup for all logger types
type baseLogger struct {
logFile *os.File
mu sync.Mutex
logDir string
fileName string
}
func (b *baseLogger) init(logDir, fileName string, flags int) error {
file, err := initLogFile(logDir, fileName, flags)
if err != nil {
return err
}
b.logFile = file
b.logDir = logDir
b.fileName = fileName
return nil
}
func (b *baseLogger) close() error {
return closeLogFile(b.logFile, &b.mu, "base")
}2. Use Generic Global Logger Manager
- Create a registry pattern for managing global logger instances
- Estimated effort: 3-4 hours
- Benefits:
- Eliminates duplicate Close*Logger functions
- Centralized lifecycle management
- Easier to add new logger types
Example:
type LoggerRegistry struct {
loggers map[string]io.Closer
mu sync.RWMutex
}
func (r *LoggerRegistry) Register(name string, logger io.Closer) {
r.mu.Lock()
defer r.mu.Unlock()
r.loggers[name] = logger
}
func (r *LoggerRegistry) Close(name string) error {
r.mu.Lock()
defer r.mu.Unlock()
if logger, ok := r.loggers[name]; ok {
err := logger.Close()
delete(r.loggers, name)
return err
}
return nil
}3. Consolidate Log Level Functions
- Use function composition or method embedding
- Estimated effort: 2-3 hours
- Benefits:
- Single implementation for all log levels
- Consistent formatting and behavior
Example:
func logWithLevel(level LogLevel, category, format string, args ...interface{}) {
globalLoggerMu.RLock()
defer globalLoggerMu.RUnlock()
if globalFileLogger != nil {
globalFileLogger.Log(level, category, format, args...)
}
}
func LogInfo(category, format string, args ...interface{}) {
logWithLevel(LogLevelInfo, category, format, args...)
}
// ... similar for other levelsImplementation Checklist
- Review duplication findings with team
- Design base logger interface and common implementation
- Create
base_logger.gowith shared initialization logic - Refactor
FileLoggerto use base implementation - Refactor
JSONLLoggerto use base implementation - Refactor
MarkdownLoggerto use base implementation - Create
LoggerRegistryfor global instance management - Consolidate log level functions
- Update all tests to work with new structure
- Run full test suite to verify no functionality broken
- Update documentation if needed
Parent Issue
See parent analysis report: #264
Related to #264
AI generated by Duplicate Code Detector
Reactions are currently unavailable