Part of duplicate code analysis: #3735
Summary
FileLogger.Log and ServerFileLogger.Log in internal/logger/ share identical core logic: format a log line with formatLogLine, write it with Println, then immediately Sync the underlying file. The sync-and-warn pattern is copy-pasted between the two implementations.
Duplication Details
Pattern: Duplicated write-and-sync core in file-based Log methods
- Severity: Low
- Occurrences: 2
- Locations:
internal/logger/file_logger.go (lines 82–98, FileLogger.Log)
internal/logger/server_file_logger.go (lines 82–105, ServerFileLogger.Log)
- Code Sample:
// FileLogger.Log (file_logger.go:82)
logLine := formatLogLine(level, category, format, args...)
fl.logger.Println(logLine)
if fl.logFile != nil {
if err := fl.logFile.Sync(); err != nil {
log.Printf("WARNING: Failed to sync log file: %v", err)
}
}
// ServerFileLogger.Log (server_file_logger.go:93)
logLine := formatLogLine(level, category, format, args...)
logger.Println(logLine)
sfl.mu.RLock()
if file, exists := sfl.files[serverID]; exists {
if err := file.Sync(); err != nil {
log.Printf("WARNING: Failed to sync log file for server %s: %v", serverID, err)
}
}
sfl.mu.RUnlock()
The formatLogLine + Println + Sync + warning-on-error sequence is the same in both methods, varying only in whether the serverID is included in the warning message.
Impact Analysis
- Maintainability: A change to the sync-and-warn pattern (e.g., adding metrics, changing the warning message format, using
fsync instead of Sync) requires editing 2 separate methods
- Bug Risk: Low but real — the warning messages have drifted slightly (
"sync log file" vs "sync log file for server %s"), suggesting the two copies are already diverging
- Code Bloat: ~12 lines of near-identical logic
Refactoring Recommendations
-
Extract a writeLine helper to common.go
// writeLine writes a formatted log line to logger and syncs the file.
// file may be nil (for stdout-backed loggers).
func writeLine(logger *log.Logger, file *os.File, serverID, line string) {
logger.Println(line)
if file != nil {
if err := file.Sync(); err != nil {
if serverID != "" {
log.Printf("WARNING: Failed to sync log file for server %s: %v", serverID, err)
} else {
log.Printf("WARNING: Failed to sync log file: %v", err)
}
}
}
}
-
Both FileLogger.Log and ServerFileLogger.Log can then call writeLine with appropriate arguments.
-
Estimated effort: 30–45 minutes
-
Benefits: Single sync-and-warn implementation; future changes (e.g., adding write counters, structured errors) apply to one place
Implementation Checklist
Parent Issue
See parent analysis report: #3735
Related to #3735
Generated by Duplicate Code Detector · ● 2.4M · ◷
Part of duplicate code analysis: #3735
Summary
FileLogger.LogandServerFileLogger.Logininternal/logger/share identical core logic: format a log line withformatLogLine, write it withPrintln, then immediatelySyncthe underlying file. The sync-and-warn pattern is copy-pasted between the two implementations.Duplication Details
Pattern: Duplicated write-and-sync core in file-based Log methods
internal/logger/file_logger.go(lines 82–98,FileLogger.Log)internal/logger/server_file_logger.go(lines 82–105,ServerFileLogger.Log)The
formatLogLine+Println+Sync+ warning-on-error sequence is the same in both methods, varying only in whether theserverIDis included in the warning message.Impact Analysis
fsyncinstead ofSync) requires editing 2 separate methods"sync log file"vs"sync log file for server %s"), suggesting the two copies are already divergingRefactoring Recommendations
Extract a
writeLinehelper tocommon.goBoth
FileLogger.LogandServerFileLogger.Logcan then callwriteLinewith appropriate arguments.Estimated effort: 30–45 minutes
Benefits: Single sync-and-warn implementation; future changes (e.g., adding write counters, structured errors) apply to one place
Implementation Checklist
writeLinehelper (or equivalent) tointernal/logger/common.goFileLogger.Logto use the helperServerFileLogger.Logto use the helpermake test-unitto confirm no regressionsParent Issue
See parent analysis report: #3735
Related to #3735