From f823127825c638c640f2c4a18e2bf8d906a281b3 Mon Sep 17 00:00:00 2001 From: Jason Kulatunga Date: Wed, 3 Aug 2022 22:51:44 -0700 Subject: [PATCH] simplify logger creation (move logic into a function in `main` packages) Ensure logger creation is consistent between Web and Collector Create logger in main, pass down to downstream functions (like gin) In debug mode, print a copy of AppConfig Better debugging for logger. --- .../collector-metrics/collector-metrics.go | 48 ++++++++++++------- webapp/backend/cmd/scrutiny/scrutiny.go | 42 +++++++++++++++- .../backend/pkg/web/handler/delete_device.go | 2 +- .../pkg/web/handler/get_device_details.go | 2 +- .../pkg/web/handler/get_devices_summary.go | 2 +- .../get_devices_summary_temp_history.go | 2 +- .../backend/pkg/web/handler/get_settings.go | 2 +- .../pkg/web/handler/register_devices.go | 2 +- .../backend/pkg/web/handler/save_settings.go | 2 +- .../pkg/web/handler/send_test_notification.go | 2 +- .../pkg/web/handler/upload_device_metrics.go | 2 +- webapp/backend/pkg/web/middleware/logger.go | 2 +- webapp/backend/pkg/web/server.go | 31 +++--------- 13 files changed, 88 insertions(+), 53 deletions(-) diff --git a/collector/cmd/collector-metrics/collector-metrics.go b/collector/cmd/collector-metrics/collector-metrics.go index a170f16..39d0a49 100644 --- a/collector/cmd/collector-metrics/collector-metrics.go +++ b/collector/cmd/collector-metrics/collector-metrics.go @@ -1,6 +1,7 @@ package main import ( + "encoding/json" "fmt" "github.com/analogj/scrutiny/collector/pkg/collector" "github.com/analogj/scrutiny/collector/pkg/config" @@ -120,26 +121,16 @@ OPTIONS: config.Set("api.endpoint", apiEndpoint) } - collectorLogger := logrus.WithFields(logrus.Fields{ - "type": "metrics", - }) - - if level, err := logrus.ParseLevel(config.GetString("log.level")); err == nil { - logrus.SetLevel(level) - } else { - logrus.SetLevel(logrus.InfoLevel) - } - - if config.IsSet("log.file") && len(config.GetString("log.file")) > 0 { - logFile, err := os.OpenFile(config.GetString("log.file"), os.O_CREATE|os.O_WRONLY, 0644) - if err != nil { - logrus.Errorf("Failed to open log file %s for output: %s", config.GetString("log.file"), err) - return err - } + collectorLogger, logFile, err := CreateLogger(config) + if logFile != nil { defer logFile.Close() - logrus.SetOutput(io.MultiWriter(os.Stderr, logFile)) + } + if err != nil { + return err } + settingsData, err := json.MarshalIndent(config.AllSettings(), "", "\t") + collectorLogger.Debug(string(settingsData), err) metricCollector, err := collector.CreateMetricsCollector( config, collectorLogger, @@ -192,5 +183,28 @@ OPTIONS: if err != nil { log.Fatal(color.HiRedString("ERROR: %v", err)) } +} + +func CreateLogger(appConfig config.Interface) (*logrus.Entry, *os.File, error) { + logger := logrus.WithFields(logrus.Fields{ + "type": "metrics", + }) + + if level, err := logrus.ParseLevel(appConfig.GetString("log.level")); err == nil { + logger.Logger.SetLevel(level) + } else { + logger.Logger.SetLevel(logrus.InfoLevel) + } + var logFile *os.File + var err error + if appConfig.IsSet("log.file") && len(appConfig.GetString("log.file")) > 0 { + logFile, err = os.OpenFile(appConfig.GetString("log.file"), os.O_CREATE|os.O_WRONLY, 0644) + if err != nil { + logger.Logger.Errorf("Failed to open log file %s for output: %s", appConfig.GetString("log.file"), err) + return nil, logFile, err + } + logger.Logger.SetOutput(io.MultiWriter(os.Stderr, logFile)) + } + return logger, logFile, nil } diff --git a/webapp/backend/cmd/scrutiny/scrutiny.go b/webapp/backend/cmd/scrutiny/scrutiny.go index 103fd6b..ae22677 100644 --- a/webapp/backend/cmd/scrutiny/scrutiny.go +++ b/webapp/backend/cmd/scrutiny/scrutiny.go @@ -1,12 +1,15 @@ package main import ( + "encoding/json" "fmt" "github.com/analogj/scrutiny/webapp/backend/pkg/config" "github.com/analogj/scrutiny/webapp/backend/pkg/errors" "github.com/analogj/scrutiny/webapp/backend/pkg/version" "github.com/analogj/scrutiny/webapp/backend/pkg/web" - log "github.com/sirupsen/logrus" + "github.com/sirupsen/logrus" + "io" + "log" "os" "time" @@ -107,7 +110,18 @@ OPTIONS: config.Set("log.file", c.String("log-file")) } - webServer := web.AppEngine{Config: config} + webLogger, logFile, err := CreateLogger(config) + if logFile != nil { + defer logFile.Close() + } + if err != nil { + return err + } + + settingsData, err := json.Marshal(config.AllSettings()) + webLogger.Debug(string(settingsData), err) + + webServer := web.AppEngine{Config: config, Logger: webLogger} return webServer.Start() }, @@ -140,3 +154,27 @@ OPTIONS: } } + +func CreateLogger(appConfig config.Interface) (*logrus.Entry, *os.File, error) { + logger := logrus.WithFields(logrus.Fields{ + "type": "web", + }) + //set default log level + if level, err := logrus.ParseLevel(appConfig.GetString("log.level")); err == nil { + logger.Logger.SetLevel(level) + } else { + logger.Logger.SetLevel(logrus.InfoLevel) + } + + var logFile *os.File + var err error + if appConfig.IsSet("log.file") && len(appConfig.GetString("log.file")) > 0 { + logFile, err = os.OpenFile(appConfig.GetString("log.file"), os.O_CREATE|os.O_WRONLY, 0644) + if err != nil { + logger.Logger.Errorf("Failed to open log file %s for output: %s", appConfig.GetString("log.file"), err) + return nil, logFile, err + } + logger.Logger.SetOutput(io.MultiWriter(os.Stderr, logFile)) + } + return logger, logFile, nil +} diff --git a/webapp/backend/pkg/web/handler/delete_device.go b/webapp/backend/pkg/web/handler/delete_device.go index 63336c9..f8a507d 100644 --- a/webapp/backend/pkg/web/handler/delete_device.go +++ b/webapp/backend/pkg/web/handler/delete_device.go @@ -8,7 +8,7 @@ import ( ) func DeleteDevice(c *gin.Context) { - logger := c.MustGet("LOGGER").(logrus.FieldLogger) + logger := c.MustGet("LOGGER").(*logrus.Entry) deviceRepo := c.MustGet("DEVICE_REPOSITORY").(database.DeviceRepo) err := deviceRepo.DeleteDevice(c, c.Param("wwn")) diff --git a/webapp/backend/pkg/web/handler/get_device_details.go b/webapp/backend/pkg/web/handler/get_device_details.go index 864b842..b4e24ee 100644 --- a/webapp/backend/pkg/web/handler/get_device_details.go +++ b/webapp/backend/pkg/web/handler/get_device_details.go @@ -9,7 +9,7 @@ import ( ) func GetDeviceDetails(c *gin.Context) { - logger := c.MustGet("LOGGER").(logrus.FieldLogger) + logger := c.MustGet("LOGGER").(*logrus.Entry) deviceRepo := c.MustGet("DEVICE_REPOSITORY").(database.DeviceRepo) device, err := deviceRepo.GetDeviceDetails(c, c.Param("wwn")) diff --git a/webapp/backend/pkg/web/handler/get_devices_summary.go b/webapp/backend/pkg/web/handler/get_devices_summary.go index 56e3eb5..a256f4b 100644 --- a/webapp/backend/pkg/web/handler/get_devices_summary.go +++ b/webapp/backend/pkg/web/handler/get_devices_summary.go @@ -8,7 +8,7 @@ import ( ) func GetDevicesSummary(c *gin.Context) { - logger := c.MustGet("LOGGER").(logrus.FieldLogger) + logger := c.MustGet("LOGGER").(*logrus.Entry) deviceRepo := c.MustGet("DEVICE_REPOSITORY").(database.DeviceRepo) summary, err := deviceRepo.GetSummary(c) diff --git a/webapp/backend/pkg/web/handler/get_devices_summary_temp_history.go b/webapp/backend/pkg/web/handler/get_devices_summary_temp_history.go index 631b9ec..b822dc9 100644 --- a/webapp/backend/pkg/web/handler/get_devices_summary_temp_history.go +++ b/webapp/backend/pkg/web/handler/get_devices_summary_temp_history.go @@ -8,7 +8,7 @@ import ( ) func GetDevicesSummaryTempHistory(c *gin.Context) { - logger := c.MustGet("LOGGER").(logrus.FieldLogger) + logger := c.MustGet("LOGGER").(*logrus.Entry) deviceRepo := c.MustGet("DEVICE_REPOSITORY").(database.DeviceRepo) durationKey, exists := c.GetQuery("duration_key") diff --git a/webapp/backend/pkg/web/handler/get_settings.go b/webapp/backend/pkg/web/handler/get_settings.go index b6969ea..82a1bc6 100644 --- a/webapp/backend/pkg/web/handler/get_settings.go +++ b/webapp/backend/pkg/web/handler/get_settings.go @@ -8,7 +8,7 @@ import ( ) func GetSettings(c *gin.Context) { - logger := c.MustGet("LOGGER").(logrus.FieldLogger) + logger := c.MustGet("LOGGER").(*logrus.Entry) deviceRepo := c.MustGet("DEVICE_REPOSITORY").(database.DeviceRepo) settings, err := deviceRepo.LoadSettings(c) diff --git a/webapp/backend/pkg/web/handler/register_devices.go b/webapp/backend/pkg/web/handler/register_devices.go index cb0c59b..38132ad 100644 --- a/webapp/backend/pkg/web/handler/register_devices.go +++ b/webapp/backend/pkg/web/handler/register_devices.go @@ -13,7 +13,7 @@ import ( // This function is run everytime a collector is about to start a run. It can be used to update device metadata. func RegisterDevices(c *gin.Context) { deviceRepo := c.MustGet("DEVICE_REPOSITORY").(database.DeviceRepo) - logger := c.MustGet("LOGGER").(logrus.FieldLogger) + logger := c.MustGet("LOGGER").(*logrus.Entry) var collectorDeviceWrapper models.DeviceWrapper err := c.BindJSON(&collectorDeviceWrapper) diff --git a/webapp/backend/pkg/web/handler/save_settings.go b/webapp/backend/pkg/web/handler/save_settings.go index 16de020..6706aaa 100644 --- a/webapp/backend/pkg/web/handler/save_settings.go +++ b/webapp/backend/pkg/web/handler/save_settings.go @@ -9,7 +9,7 @@ import ( ) func SaveSettings(c *gin.Context) { - logger := c.MustGet("LOGGER").(logrus.FieldLogger) + logger := c.MustGet("LOGGER").(*logrus.Entry) deviceRepo := c.MustGet("DEVICE_REPOSITORY").(database.DeviceRepo) var settings models.Settings diff --git a/webapp/backend/pkg/web/handler/send_test_notification.go b/webapp/backend/pkg/web/handler/send_test_notification.go index dae55ab..4d8e56d 100644 --- a/webapp/backend/pkg/web/handler/send_test_notification.go +++ b/webapp/backend/pkg/web/handler/send_test_notification.go @@ -13,7 +13,7 @@ import ( // Send test notification func SendTestNotification(c *gin.Context) { appConfig := c.MustGet("CONFIG").(config.Interface) - logger := c.MustGet("LOGGER").(logrus.FieldLogger) + logger := c.MustGet("LOGGER").(*logrus.Entry) testNotify := notify.New( logger, diff --git a/webapp/backend/pkg/web/handler/upload_device_metrics.go b/webapp/backend/pkg/web/handler/upload_device_metrics.go index 82d5850..f58d6ed 100644 --- a/webapp/backend/pkg/web/handler/upload_device_metrics.go +++ b/webapp/backend/pkg/web/handler/upload_device_metrics.go @@ -14,7 +14,7 @@ import ( func UploadDeviceMetrics(c *gin.Context) { //db := c.MustGet("DB").(*gorm.DB) - logger := c.MustGet("LOGGER").(logrus.FieldLogger) + logger := c.MustGet("LOGGER").(*logrus.Entry) appConfig := c.MustGet("CONFIG").(config.Interface) //influxWriteDb := c.MustGet("INFLUXDB_WRITE").(*api.WriteAPIBlocking) deviceRepo := c.MustGet("DEVICE_REPOSITORY").(database.DeviceRepo) diff --git a/webapp/backend/pkg/web/middleware/logger.go b/webapp/backend/pkg/web/middleware/logger.go index 7568efe..8540d5f 100644 --- a/webapp/backend/pkg/web/middleware/logger.go +++ b/webapp/backend/pkg/web/middleware/logger.go @@ -28,7 +28,7 @@ import ( var timeFormat = "02/Jan/2006:15:04:05 -0700" // Logger is the logrus logger handler -func LoggerMiddleware(logger logrus.FieldLogger) gin.HandlerFunc { +func LoggerMiddleware(logger *logrus.Entry) gin.HandlerFunc { hostname, err := os.Hostname() if err != nil { diff --git a/webapp/backend/pkg/web/server.go b/webapp/backend/pkg/web/server.go index 0ef8bc8..bb82405 100644 --- a/webapp/backend/pkg/web/server.go +++ b/webapp/backend/pkg/web/server.go @@ -9,18 +9,17 @@ import ( "github.com/analogj/scrutiny/webapp/backend/pkg/web/middleware" "github.com/gin-gonic/gin" "github.com/sirupsen/logrus" - "io" "net/http" - "os" "path/filepath" "strings" ) type AppEngine struct { Config config.Interface + Logger *logrus.Entry } -func (ae *AppEngine) Setup(logger logrus.FieldLogger) *gin.Engine { +func (ae *AppEngine) Setup(logger *logrus.Entry) *gin.Engine { r := gin.New() r.Use(middleware.LoggerMiddleware(logger)) @@ -36,6 +35,10 @@ func (ae *AppEngine) Setup(logger logrus.FieldLogger) *gin.Engine { api := base.Group("/api") { api.GET("/health", func(c *gin.Context) { + //TODO: + // check if the /web folder is populated. + // check if access to influxdb + // check if access to sqlitedb. c.JSON(http.StatusOK, gin.H{ "success": true, }) @@ -77,26 +80,6 @@ func (ae *AppEngine) Start() error { gin.SetMode(gin.DebugMode) } - logger := logrus.New() - //set default log level - logLevel, err := logrus.ParseLevel(ae.Config.GetString("log.level")) - if err != nil { - return err - } - logger.SetLevel(logLevel) - //set the log file if present - if len(ae.Config.GetString("log.file")) != 0 { - logFile, err := os.OpenFile(ae.Config.GetString("log.file"), os.O_CREATE|os.O_WRONLY, 0644) - defer logFile.Close() - if err != nil { - logrus.Errorf("Failed to open log file %s for output: %s", ae.Config.GetString("log.file"), err) - return err - } - - //configure the logrus default - logger.SetOutput(io.MultiWriter(os.Stderr, logFile)) - } - //check if the database parent directory exists, fail here rather than in a handler. if !utils.FileExists(filepath.Dir(ae.Config.GetString("web.database.location"))) { return errors.ConfigValidationError(fmt.Sprintf( @@ -104,7 +87,7 @@ func (ae *AppEngine) Start() error { filepath.Dir(ae.Config.GetString("web.database.location")))) } - r := ae.Setup(logger) + r := ae.Setup(ae.Logger) return r.Run(fmt.Sprintf("%s:%s", ae.Config.GetString("web.listen.host"), ae.Config.GetString("web.listen.port"))) }