refactor: Logger handling and logic

.golangci.yml

@@ -44,6 +44,9 @@ linters:
     varnamelen:
       ignore-decls:
         - fs afero.Fs
+    wrapcheck:
+      ignore-package-globs:
+        - backup-rsync/*
 formatters:
   enable:
     - gofmt

backup/cmd/check-coverage.go

@@ -2,7 +2,6 @@ package cmd
 
 import (
 	"fmt"
-	"log"
 	"os"
 
 	"backup-rsync/backup/internal"
@@ -24,7 +23,7 @@ func buildCheckCoverageCommand(fs afero.Fs) *cobra.Command {
 			}
 
 			checker := &internal.CoverageChecker{
-				Logger: log.New(os.Stderr, "", log.LstdFlags),
+				Logger: internal.NewUTCLogger(os.Stderr),
 				Fs:     fs,
 			}
 

backup/cmd/job.go

@@ -11,12 +11,18 @@ import (
 	"github.com/spf13/cobra"
 )
 
+// LoggerFactory creates a logger, returning the logger, log directory path, cleanup function, and any error.
+type LoggerFactory func(fs afero.Fs, configPath string, now time.Time) (*log.Logger, string, func() error, error)
+
+func discardLoggerFactory(_ afero.Fs, _ string, _ time.Time) (*log.Logger, string, func() error, error) {
+	return log.New(io.Discard, "", 0), "", func() error { return nil }, nil
+}
+
 type jobCommandOptions struct {
-	use      string
-	short    string
-	needsLog bool
-	simulate bool
-	factory  func(rsyncPath string, logPath string, out io.Writer) internal.JobCommand
+	use          string
+	short        string
+	factory      func(rsyncPath string, logPath string, out io.Writer) internal.JobCommand
+	createLogger LoggerFactory
 }
 
 func buildJobCommand(fs afero.Fs, opts jobCommandOptions) *cobra.Command {
@@ -33,24 +39,22 @@ func buildJobCommand(fs afero.Fs, opts jobCommandOptions) *cobra.Command {
 			}
 
 			out := cmd.OutOrStdout()
-			logger := log.New(io.Discard, "", 0)
-
-			var logPath string
 
-			if opts.needsLog {
-				var cleanup func() error
-
-				logger, logPath, cleanup, err = internal.CreateMainLogger(fs, configPath, opts.simulate, time.Now())
-				if err != nil {
-					return fmt.Errorf("creating logger: %w", err)
-				}
+			createLogger := opts.createLogger
+			if createLogger == nil {
+				createLogger = discardLoggerFactory
+			}
 
-				defer cleanup()
+			logger, logPath, cleanup, err := createLogger(fs, configPath, time.Now())
+			if err != nil {
+				return fmt.Errorf("creating logger: %w", err)
 			}
 
+			defer cleanup()
+
 			command := opts.factory(rsyncPath, logPath, out)
 
-			return cfg.Apply(command, logger, out)
+			return cfg.Apply(command, logger)
 		},
 	}
 }

backup/cmd/run.go

@@ -3,16 +3,24 @@ package cmd
 import (
 	"backup-rsync/backup/internal"
 	"io"
+	"log"
+	"time"
 
 	"github.com/spf13/afero"
 	"github.com/spf13/cobra"
 )
 
 func buildRunCommand(fs afero.Fs, shell internal.Exec) *cobra.Command {
 	return buildJobCommand(fs, jobCommandOptions{
-		use:      "run",
-		short:    "Execute the sync jobs",
-		needsLog: true,
+		use:   "run",
+		short: "Execute the sync jobs",
+		createLogger: func(fs afero.Fs, configPath string, now time.Time) (*log.Logger, string, func() error, error) {
+			logPath := internal.GetLogPath(configPath, now)
+
+			logger, cleanup, err := internal.CreateMainLogger(fs, logPath)
+
+			return logger, logPath, cleanup, err
+		},
 		factory: func(rsyncPath string, logPath string, out io.Writer) internal.JobCommand {
 			return internal.NewSyncCommand(rsyncPath, logPath, shell, out)
 		},

backup/cmd/simulate.go

@@ -3,17 +3,24 @@ package cmd
 import (
 	"backup-rsync/backup/internal"
 	"io"
+	"log"
+	"time"
 
 	"github.com/spf13/afero"
 	"github.com/spf13/cobra"
 )
 
 func buildSimulateCommand(fs afero.Fs, shell internal.Exec) *cobra.Command {
 	return buildJobCommand(fs, jobCommandOptions{
-		use:      "simulate",
-		short:    "Simulate the sync jobs",
-		needsLog: true,
-		simulate: true,
+		use:   "simulate",
+		short: "Simulate the sync jobs",
+		createLogger: func(fs afero.Fs, configPath string, now time.Time) (*log.Logger, string, func() error, error) {
+			logPath := internal.GetLogPath(configPath, now) + "-sim"
+
+			logger, cleanup, err := internal.CreateMainLogger(fs, logPath)
+
+			return logger, logPath, cleanup, err
+		},
 		factory: func(rsyncPath string, logPath string, out io.Writer) internal.JobCommand {
 			return internal.NewSimulateCommand(rsyncPath, logPath, shell, out)
 		},

backup/cmd/test/commands_test.go

@@ -310,7 +310,8 @@ func TestList_ValidConfig(t *testing.T) {
 
 	require.NoError(t, err)
 	assert.Contains(t, stdout, "Job: docs")
-	assert.Contains(t, stdout, "Status [docs]: SUCCESS")
+	assert.NotContains(t, stdout, "Status [docs]:")
+	assert.NotContains(t, stdout, "Summary:")
 }
 
 // --- run: logger cleanup happens after cfg.Apply completes ---

backup/cmd/test/integration_test.go

@@ -362,7 +362,8 @@ func TestIntegration_List_ShowsCommands(t *testing.T) {
 	assert.Contains(t, stdout, "--exclude=temp")
 	assert.Contains(t, stdout, src+"/")
 	assert.Contains(t, stdout, dst+"/")
-	assert.Contains(t, stdout, "Status [listjob]: SUCCESS")
+	assert.NotContains(t, stdout, "Status [listjob]:")
+	assert.NotContains(t, stdout, "Summary:")
 
 	// list should not actually sync files
 	assert.False(t, fileExists(t, filepath.Join(dst, "x.txt")), "list should not sync files")

backup/internal/config.go

@@ -39,7 +39,7 @@ func (cfg Config) String() string {
 	return string(out)
 }
 
-func (cfg Config) Apply(rsync JobCommand, logger *log.Logger, output io.Writer) error {
+func (cfg Config) Apply(rsync JobCommand, logger *log.Logger) error {
 	versionInfo, fullpath, err := rsync.GetVersionInfo()
 	if err != nil {
 		logger.Printf("Failed to fetch rsync version: %v", err)
@@ -52,15 +52,11 @@ func (cfg Config) Apply(rsync JobCommand, logger *log.Logger, output io.Writer)
 
 	for _, job := range cfg.Jobs {
 		status := job.Apply(rsync)
-		logger.Printf("STATUS [%s]: %s", job.Name, status)
-		fmt.Fprintf(output, "Status [%s]: %s\n", job.Name, status)
+		rsync.ReportJobStatus(job.Name, status, logger)
 		counts[status]++
 	}
 
-	summary := fmt.Sprintf("Summary: %d succeeded, %d failed, %d skipped",
-		counts[Success], counts[Failure], counts[Skipped])
-	logger.Print(summary)
-	fmt.Fprintln(output, summary)
+	rsync.ReportSummary(counts, logger)
 
 	if counts[Failure] > 0 {
 		return fmt.Errorf("%w: %d of %d jobs", ErrJobFailure, counts[Failure], len(cfg.Jobs))

backup/internal/helper.go

@@ -3,6 +3,7 @@ package internal
 
 import (
 	"fmt"
+	"io"
 	"log"
 	"os"
 	"path/filepath"
@@ -12,6 +13,28 @@ import (
 	"github.com/spf13/afero"
 )
 
+// UTCLogWriter wraps an io.Writer and prepends an ISO 8601 UTC timestamp to each write.
+type UTCLogWriter struct {
+	W   io.Writer
+	Now func() time.Time
+}
+
+func (u *UTCLogWriter) Write(data []byte) (int, error) {
+	now := u.Now().UTC().Format(time.RFC3339)
+
+	_, err := fmt.Fprintf(u.W, "%s %s", now, data)
+	if err != nil {
+		return 0, fmt.Errorf("writing log entry: %w", err)
+	}
+
+	return len(data), nil
+}
+
+// NewUTCLogger creates a *log.Logger that writes ISO 8601 UTC timestamps.
+func NewUTCLogger(w io.Writer) *log.Logger {
+	return log.New(&UTCLogWriter{W: w, Now: time.Now}, "", 0)
+}
+
 // Path represents a source or target path with optional exclusions.
 type Path struct {
 	Path       string   `yaml:"path"`
@@ -25,39 +48,33 @@ func NormalizePath(path string) string {
 const LogFilePermission = 0644
 const LogDirPermission = 0755
 
-func getLogPath(simulate bool, configPath string, now time.Time) string {
+func GetLogPath(configPath string, now time.Time) string {
 	filename := filepath.Base(configPath)
 	filename = strings.TrimSuffix(filename, ".yaml")
-	logPath := "logs/sync-" + now.Format("2006-01-02T15-04-05") + "-" + filename
-
-	if simulate {
-		logPath += "-sim"
-	}
 
-	return logPath
+	return "logs/sync-" + now.Format("2006-01-02T15-04-05") + "-" + filename
 }
 
 func CreateMainLogger(
-	fs afero.Fs, configPath string, simulate bool, now time.Time,
-) (*log.Logger, string, func() error, error) {
-	logPath := getLogPath(simulate, configPath, now)
+	fs afero.Fs, logPath string,
+) (*log.Logger, func() error, error) {
 	overallLogPath := logPath + "/summary.log"
 
 	err := fs.MkdirAll(logPath, LogDirPermission)
 	if err != nil {
-		return nil, "", nil, fmt.Errorf("failed to create log directory: %w", err)
+		return nil, nil, fmt.Errorf("failed to create log directory: %w", err)
 	}
 
 	overallLogFile, err := fs.OpenFile(overallLogPath, os.O_CREATE|os.O_WRONLY|os.O_APPEND, LogFilePermission)
 	if err != nil {
-		return nil, "", nil, fmt.Errorf("failed to open overall log file: %w", err)
+		return nil, nil, fmt.Errorf("failed to open overall log file: %w", err)
 	}
 
-	logger := log.New(overallLogFile, "", log.LstdFlags)
+	logger := NewUTCLogger(overallLogFile)
 
 	cleanup := func() error {
 		return overallLogFile.Close()
 	}
 
-	return logger, logPath, cleanup, nil
+	return logger, cleanup, nil
 }

backup/internal/job_command.go

@@ -1,5 +1,7 @@
 package internal
 
+import "log"
+
 // JobStatus represents the outcome of a job execution.
 type JobStatus string
 
@@ -16,4 +18,6 @@ const (
 type JobCommand interface {
 	Run(job Job) JobStatus
 	GetVersionInfo() (string, string, error)
+	ReportJobStatus(jobName string, status JobStatus, logger *log.Logger)
+	ReportSummary(counts map[JobStatus]int, logger *log.Logger)
 }

backup/internal/rsync.go

@@ -4,6 +4,7 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"log"
 	"os"
 	"path/filepath"
 	"strings"
@@ -42,6 +43,18 @@ func (c SharedCommand) PrintArgs(job Job, args []string) {
 	fmt.Fprintf(c.Output, "Command: %s %s\n", c.BinPath, strings.Join(args, " "))
 }
 
+func (c SharedCommand) ReportJobStatus(jobName string, status JobStatus, logger *log.Logger) {
+	logger.Printf("STATUS [%s]: %s", jobName, status)
+	fmt.Fprintf(c.Output, "Status [%s]: %s\n", jobName, status)
+}
+
+func (c SharedCommand) ReportSummary(counts map[JobStatus]int, logger *log.Logger) {
+	summary := fmt.Sprintf("Summary: %d succeeded, %d failed, %d skipped",
+		counts[Success], counts[Failure], counts[Skipped])
+	logger.Print(summary)
+	fmt.Fprintln(c.Output, summary)
+}
+
 func (c SharedCommand) RunWithArgs(job Job, args []string) JobStatus {
 	c.PrintArgs(job, args)
 

backup/internal/rsync_list.go

@@ -1,6 +1,9 @@
 package internal
 
-import "io"
+import (
+	"io"
+	"log"
+)
 
 // ListCommand prints the rsync commands that would be executed without running them.
 type ListCommand struct {
@@ -14,6 +17,10 @@ func NewListCommand(binPath string, shell Exec, output io.Writer) ListCommand {
 	}
 }
 
+func (ListCommand) ReportJobStatus(_ string, _ JobStatus, _ *log.Logger) {}
+
+func (ListCommand) ReportSummary(_ map[JobStatus]int, _ *log.Logger) {}
+
 func (c ListCommand) Run(job Job) JobStatus {
 	logPath := c.JobLogPath(job)
 	args := ArgumentsForJob(job, logPath, false)

backup/internal/test/config_test.go

@@ -367,8 +367,6 @@ func TestLoadResolvedConfig(t *testing.T) {
 func TestConfigApply_VersionInfoSuccess(t *testing.T) {
 	mockCmd := NewMockJobCommand(t)
 
-	var output bytes.Buffer
-
 	var logBuf bytes.Buffer
 
 	logger := log.New(&logBuf, "", 0)
@@ -382,24 +380,20 @@ func TestConfigApply_VersionInfoSuccess(t *testing.T) {
 
 	mockCmd.EXPECT().GetVersionInfo().Return("rsync version 3.2.3", "/usr/bin/rsync", nil).Once()
 	mockCmd.EXPECT().Run(mock.AnythingOfType("internal.Job")).Return(Success).Once()
+	mockCmd.EXPECT().ReportJobStatus("job1", Success, logger).Once()
+	mockCmd.EXPECT().ReportJobStatus("job2", Skipped, logger).Once()
+	mockCmd.EXPECT().ReportSummary(map[JobStatus]int{Success: 1, Skipped: 1}, logger).Once()
 
-	err := cfg.Apply(mockCmd, logger, &output)
+	err := cfg.Apply(mockCmd, logger)
 
 	require.NoError(t, err)
 	assert.Contains(t, logBuf.String(), "Rsync Binary Path: /usr/bin/rsync")
 	assert.Contains(t, logBuf.String(), "Rsync Version Info: rsync version 3.2.3")
-	assert.Contains(t, logBuf.String(), "STATUS [job1]: SUCCESS")
-	assert.Contains(t, logBuf.String(), "STATUS [job2]: SKIPPED")
-	assert.Contains(t, output.String(), "Status [job1]: SUCCESS")
-	assert.Contains(t, output.String(), "Status [job2]: SKIPPED")
-	assert.Contains(t, output.String(), "Summary: 1 succeeded, 0 failed, 1 skipped")
 }
 
 func TestConfigApply_VersionInfoError(t *testing.T) {
 	mockCmd := NewMockJobCommand(t)
 
-	var output bytes.Buffer
-
 	var logBuf bytes.Buffer
 
 	logger := log.New(&logBuf, "", 0)
@@ -412,14 +406,13 @@ func TestConfigApply_VersionInfoError(t *testing.T) {
 
 	mockCmd.EXPECT().GetVersionInfo().Return("", "", errCommandNotFound).Once()
 	mockCmd.EXPECT().Run(mock.AnythingOfType("internal.Job")).Return(Failure).Once()
+	mockCmd.EXPECT().ReportJobStatus("backup", Failure, logger).Once()
+	mockCmd.EXPECT().ReportSummary(map[JobStatus]int{Failure: 1}, logger).Once()
 
-	err := cfg.Apply(mockCmd, logger, &output)
+	err := cfg.Apply(mockCmd, logger)
 
 	require.Error(t, err)
 	require.ErrorIs(t, err, ErrJobFailure)
 	assert.Contains(t, logBuf.String(), "Failed to fetch rsync version: command not found")
 	assert.NotContains(t, logBuf.String(), "Rsync Binary Path")
-	assert.Contains(t, logBuf.String(), "STATUS [backup]: FAILURE")
-	assert.Contains(t, output.String(), "Status [backup]: FAILURE")
-	assert.Contains(t, output.String(), "Summary: 0 succeeded, 1 failed, 0 skipped")
 }