From f45fec784b9168cd14bdb38916a2432afa24f15b Mon Sep 17 00:00:00 2001
From: Doug Lauder <wiggin77@warpmail.net>
Date: Wed, 3 Apr 2024 11:49:26 -0400
Subject: [PATCH] Improvements to logging start-up. (#26649)

Improvements to logging start-up.
- comments have been added
- order of configuration is adjusted to allow the main logger to capture errors in notification logger
- redundant logger initialization removed; no need to configure the logger just to replace the configuration immediately after
---
 server/channels/app/platform/config.go | 13 ++++++++--
 server/channels/app/platform/log.go    | 36 ++++++++++----------------
 2 files changed, 25 insertions(+), 24 deletions(-)

diff --git a/server/channels/app/platform/config.go b/server/channels/app/platform/config.go
index 8789a7adcc..3411c13bec 100644
--- a/server/channels/app/platform/config.go
+++ b/server/channels/app/platform/config.go
@@ -135,6 +135,8 @@ func (ps *PlatformService) ConfigureLogger(name string, logger *mlog.Logger, log
 	// Advanced logging is E20 only, however logging must be initialized before the license
 	// file is loaded.  If no valid E20 license exists then advanced logging will be
 	// shutdown once license is loaded/checked.
+	var resultLevel = mlog.LvlInfo
+	var resultMsg string
 	var err error
 	var logConfigSrc config.LogConfigSrc
 	dsn := logSettings.GetAdvancedLoggingConfig()
@@ -143,9 +145,10 @@ func (ps *PlatformService) ConfigureLogger(name string, logger *mlog.Logger, log
 		if err != nil {
 			return fmt.Errorf("invalid config source for %s, %w", name, err)
 		}
-		ps.logger.Info("Loaded configuration for "+name, mlog.String("source", dsn))
+		resultMsg = fmt.Sprintf("Loaded Advanced Logging configuration for %s: %s", name, string(dsn))
 	} else {
-		ps.logger.Debug("Advanced logging config not provided for " + name)
+		resultLevel = mlog.LvlDebug
+		resultMsg = fmt.Sprintf("Advanced logging config not provided for %s", name)
 	}
 
 	cfg, err := config.MloggerConfigFromLoggerConfig(logSettings, logConfigSrc, getPath)
@@ -153,9 +156,15 @@ func (ps *PlatformService) ConfigureLogger(name string, logger *mlog.Logger, log
 		return fmt.Errorf("invalid config source for %s, %w", name, err)
 	}
 
+	// this will remove any existing targets and replace with those defined in cfg.
 	if err := logger.ConfigureTargets(cfg, nil); err != nil {
 		return fmt.Errorf("invalid config for %s, %w", name, err)
 	}
+
+	if resultMsg != "" {
+		ps.Log().Log(resultLevel, resultMsg)
+	}
+
 	return nil
 }
 
diff --git a/server/channels/app/platform/log.go b/server/channels/app/platform/log.go
index 87f6c37f31..f9930c9a16 100644
--- a/server/channels/app/platform/log.go
+++ b/server/channels/app/platform/log.go
@@ -7,7 +7,6 @@ import (
 	"context"
 	"encoding/json"
 	"errors"
-	"fmt"
 	"io"
 	"net/http"
 	"os"
@@ -38,17 +37,24 @@ func (ps *PlatformService) initLogging() error {
 		if err != nil {
 			return err
 		}
+	}
 
-		logCfg, err := config.MloggerConfigFromLoggerConfig(&ps.Config().LogSettings, nil, config.GetLogFileLocation)
-		if err != nil {
+	// configure app logger. This will replace any existing targets with new ones as defined in the config.
+	if err := ps.ConfigureLogger("logging", ps.logger, &ps.Config().LogSettings, config.GetLogFileLocation); err != nil {
+		// if the config is locked then a unit test has already configured and locked the logger; not an error.
+		if !errors.Is(err, mlog.ErrConfigurationLock) {
+			// revert to default logger if the config is invalid
+			mlog.InitGlobalLogger(nil)
 			return err
 		}
-
-		if errCfg := ps.logger.ConfigureTargets(logCfg, nil); errCfg != nil {
-			return fmt.Errorf("failed to configure test logger: %w", errCfg)
-		}
 	}
 
+	// redirect default Go logger to app logger.
+	ps.logger.RedirectStdLog(mlog.LvlStdLog)
+
+	// use the app logger as the global logger (eventually remove all instances of global logging).
+	mlog.InitGlobalLogger(ps.logger)
+
 	// create notification logger if needed
 	if ps.notificationsLogger == nil {
 		l, err := mlog.NewLogger()
@@ -58,21 +64,7 @@ func (ps *PlatformService) initLogging() error {
 		ps.notificationsLogger = l.With(mlog.String("logSource", "notifications"))
 	}
 
-	if err := ps.ConfigureLogger("logging", ps.logger, &ps.Config().LogSettings, config.GetLogFileLocation); err != nil {
-		// if the config is locked then a unit test has already configured and locked the logger; not an error.
-		if !errors.Is(err, mlog.ErrConfigurationLock) {
-			// revert to default logger if the config is invalid
-			mlog.InitGlobalLogger(nil)
-			return err
-		}
-	}
-
-	// Redirect default Go logger to app logger.
-	ps.logger.RedirectStdLog(mlog.LvlStdLog)
-
-	// Use the app logger as the global logger (eventually remove all instances of global logging).
-	mlog.InitGlobalLogger(ps.logger)
-
+	// configure notification logger
 	notificationLogSettings := config.GetLogSettingsFromNotificationsLogSettings(&ps.Config().NotificationLogSettings)
 	if err := ps.ConfigureLogger("notification logging", ps.notificationsLogger, notificationLogSettings, config.GetNotificationsLogFileLocation); err != nil {
 		if !errors.Is(err, mlog.ErrConfigurationLock) {