From 6e558fb2833fe853ce48ac6a2ffa3af75f6981d2 Mon Sep 17 00:00:00 2001
From: jholdstock <jholdstock@decred.org>
Date: Tue, 21 May 2024 08:59:49 +0100
Subject: [PATCH] vspd: Decouple config loading & log initialization

Loading config and preparing loggers are two separate concepts which
should be handled individually. Nesting one inside the other makes
little sense and complicates reusing the code.
---
 cmd/vspd/config.go | 25 +++----------------------
 cmd/vspd/main.go   | 37 +++++++++++++++++++++++++++++++++----
 2 files changed, 36 insertions(+), 26 deletions(-)

diff --git a/cmd/vspd/config.go b/cmd/vspd/config.go
index aff54784..3946b034 100644
--- a/cmd/vspd/config.go
+++ b/cmd/vspd/config.go
@@ -18,7 +18,6 @@ import (
 
 	"github.com/decred/dcrd/dcrutil/v4"
 	"github.com/decred/dcrd/hdkeychain/v3"
-	"github.com/decred/slog"
 	"github.com/decred/vspd/database"
 	"github.com/decred/vspd/internal/config"
 	"github.com/decred/vspd/internal/version"
@@ -60,9 +59,7 @@ type vspdConfig struct {
 	HomeDir     string `long:"homedir" no-ini:"true" description:"Path to application home directory. Used for storing VSP database and logs."`
 	ConfigFile  string `long:"configfile" no-ini:"true" description:"DEPRECATED: This behavior is no longer available and this option will be removed in a future version of the software."`
 
-	logBackend *slog.Backend
-	logLevel   slog.Level
-
+	logPath                                   string
 	dbPath                                    string
 	network                                   *config.Network
 	dcrdCert                                  []byte
@@ -379,18 +376,8 @@ func loadConfig() (*vspdConfig, error) {
 		return nil, fmt.Errorf("failed to create data directory: %w", err)
 	}
 
-	// Initialize loggers and log rotation.
-	logDir := filepath.Join(cfg.HomeDir, "logs", cfg.network.Name)
-	cfg.logBackend, err = newLogBackend(logDir, appName, cfg.MaxLogSize, cfg.LogsToKeep)
-	if err != nil {
-		return nil, fmt.Errorf("failed to initialize logger: %w", err)
-	}
-
-	var ok bool
-	cfg.logLevel, ok = slog.LevelFromString(cfg.LogLevel)
-	if !ok {
-		return nil, fmt.Errorf("unknown log level: %s", cfg.LogLevel)
-	}
+	// Set the log path.
+	cfg.logPath = filepath.Join(cfg.HomeDir, "logs", cfg.network.Name)
 
 	// Set the database path.
 	cfg.dbPath = filepath.Join(dataDir, dbFilename)
@@ -429,9 +416,3 @@ func loadConfig() (*vspdConfig, error) {
 
 	return &cfg, nil
 }
-
-func (cfg *vspdConfig) logger(subsystem string) slog.Logger {
-	log := cfg.logBackend.Logger(subsystem)
-	log.SetLevel(cfg.logLevel)
-	return log
-}
diff --git a/cmd/vspd/main.go b/cmd/vspd/main.go
index 68a5d9e1..f3e4dbac 100644
--- a/cmd/vspd/main.go
+++ b/cmd/vspd/main.go
@@ -12,6 +12,7 @@ import (
 	"time"
 
 	"github.com/decred/dcrd/wire"
+	"github.com/decred/slog"
 	"github.com/decred/vspd/database"
 	"github.com/decred/vspd/internal/config"
 	"github.com/decred/vspd/internal/signal"
@@ -33,6 +34,28 @@ func main() {
 	os.Exit(run())
 }
 
+// initLogging uses the provided vspd config to create a logging backend, and
+// returns a function which can be used to create ready-to-use subsystem
+// loggers.
+func initLogging(cfg *vspdConfig) (func(subsystem string) slog.Logger, error) {
+	backend, err := newLogBackend(cfg.logPath, "vspd", cfg.MaxLogSize, cfg.LogsToKeep)
+	if err != nil {
+		return nil, fmt.Errorf("failed to initialize logger: %w", err)
+	}
+
+	var ok bool
+	level, ok := slog.LevelFromString(cfg.LogLevel)
+	if !ok {
+		return nil, fmt.Errorf("unknown log level: %q", cfg.LogLevel)
+	}
+
+	return func(subsystem string) slog.Logger {
+		log := backend.Logger(subsystem)
+		log.SetLevel(level)
+		return log
+	}, nil
+}
+
 // run is the real main function for vspd. It is necessary to work around the
 // fact that deferred functions do not run when os.Exit() is called.
 func run() int {
@@ -43,7 +66,13 @@ func run() int {
 		return 1
 	}
 
-	log := cfg.logger("VSP")
+	makeLogger, err := initLogging(cfg)
+	if err != nil {
+		fmt.Fprintf(os.Stderr, "initLogging error: %v\n", err)
+		return 1
+	}
+
+	log := makeLogger("VSP")
 
 	// Create a context that is canceled when a shutdown request is received
 	// through an interrupt signal such as SIGINT (Ctrl+C).
@@ -72,7 +101,7 @@ func run() int {
 	}
 
 	// Open database.
-	db, err := database.Open(cfg.dbPath, cfg.logger(" DB"), maxVoteChangeRecords)
+	db, err := database.Open(cfg.dbPath, makeLogger(" DB"), maxVoteChangeRecords)
 	if err != nil {
 		log.Errorf("Failed to open database: %v", err)
 		return 1
@@ -80,7 +109,7 @@ func run() int {
 	const writeBackup = true
 	defer db.Close(writeBackup)
 
-	rpcLog := cfg.logger("RPC")
+	rpcLog := makeLogger("RPC")
 
 	// Create a channel to receive blockConnected notifications from dcrd.
 	blockNotifChan := make(chan *wire.BlockHeader)
@@ -108,7 +137,7 @@ func run() int {
 		MaxVoteChangeRecords: maxVoteChangeRecords,
 		VspdVersion:          version.String(),
 	}
-	api, err := webapi.New(db, cfg.logger("API"), dcrd, wallets, apiCfg)
+	api, err := webapi.New(db, makeLogger("API"), dcrd, wallets, apiCfg)
 	if err != nil {
 		log.Errorf("Failed to initialize webapi: %v", err)
 		return 1