From 15cb268440b54bcbdcae9d64e1d5d083e99d74e7 Mon Sep 17 00:00:00 2001 From: jwijenbergh Date: Tue, 28 Feb 2023 10:37:00 +0100 Subject: Log: Use a global logger instance --- client/client.go | 21 +++++++++------------ client/fsm.go | 9 +++++---- client/server.go | 15 ++++++++------- internal/failover/failover.go | 6 ++---- internal/failover/monitor.go | 23 ++++++++++------------- internal/log/log.go | 7 +++++++ 6 files changed, 41 insertions(+), 40 deletions(-) diff --git a/client/client.go b/client/client.go index 1a1e881..6e25694 100644 --- a/client/client.go +++ b/client/client.go @@ -25,9 +25,9 @@ type ( func (c *Client) logError(err error) { // Logs the error with the same level/verbosity as the error if c.Debug { - c.Logger.Inherit(err, fmt.Sprintf("\nwith stacktrace: %s\n", err.(*errors.Error).ErrorStack())) + log.Logger.Inherit(err, fmt.Sprintf("\nwith stacktrace: %s\n", err.(*errors.Error).ErrorStack())) } else { - c.Logger.Inherit(err, "") + log.Logger.Inherit(err, "") } } @@ -77,9 +77,6 @@ type Client struct { // The fsm FSM fsm.FSM `json:"-"` - // The logger - Logger log.FileLogger `json:"-"` - // The config Config config.Config `json:"-"` @@ -90,7 +87,7 @@ type Client struct { Debug bool `json:"-"` // The Failover monitor for the current VPN connection - Failover *failover.DroppedConMon + Failover *failover.DroppedConMon `json:"-"` } // Register initializes the clientwith the following parameters: @@ -132,7 +129,7 @@ func (c *Client) Register( lvl = log.LevelDebug } - if err = c.Logger.Init(lvl, directory); err != nil { + if err = log.Logger.Init(lvl, directory); err != nil { return err } @@ -151,7 +148,7 @@ func (c *Client) Register( // Try to load the previous configuration if c.Config.Load(&c) != nil { // This error can be safely ignored, as when the config does not load, the struct will not be filled - c.Logger.Infof("Previous configuration not found") + log.Logger.Infof("Previous configuration not found") } // Go to the No Server state with the saved servers after we're done @@ -164,11 +161,11 @@ func (c *Client) Register( // Check if we are able to fetch discovery, and log if something went wrong if _, err := c.DiscoServers(); err != nil { - c.Logger.Warningf("Failed to get discovery servers: %v", err) + log.Logger.Warningf("Failed to get discovery servers: %v", err) } if _, err := c.DiscoOrganizations(); err != nil { - c.Logger.Warningf("Failed to get discovery organizations: %v", err) + log.Logger.Warningf("Failed to get discovery organizations: %v", err) } return nil @@ -177,11 +174,11 @@ func (c *Client) Register( // Deregister 'deregisters' the client, meaning saving the log file and the config and emptying out the client struct. func (c *Client) Deregister() { // Close the log file - _ = c.Logger.Close() + _ = log.Logger.Close() // Save the config if err := c.Config.Save(&c); err != nil { - c.Logger.Infof("c.Config.Save failed: %s\nstacktrace:\n%s", err.Error(), err.(*errors.Error).ErrorStack()) + log.Logger.Infof("c.Config.Save failed: %s\nstacktrace:\n%s", err.Error(), err.(*errors.Error).ErrorStack()) } // Empty out the state diff --git a/client/fsm.go b/client/fsm.go index e69652f..b2e846a 100644 --- a/client/fsm.go +++ b/client/fsm.go @@ -2,6 +2,7 @@ package client import ( "github.com/eduvpn/eduvpn-common/internal/fsm" + "github.com/eduvpn/eduvpn-common/internal/log" "github.com/eduvpn/eduvpn-common/internal/server" "github.com/go-errors/errors" ) @@ -247,7 +248,7 @@ func (c *Client) SetConnected() error { func (c *Client) SetConnecting() error { if c.InFSMState(StateConnecting) { // already loading connection, show no error - c.Logger.Warningf("Already connecting") + log.Logger.Warningf("Already connecting") return nil } if err := c.FSM.CheckTransition(StateConnecting); err != nil { @@ -271,7 +272,7 @@ func (c *Client) SetConnecting() error { func (c *Client) SetDisconnecting() error { if c.InFSMState(StateDisconnecting) { // already disconnecting, show no error - c.Logger.Warningf("Already disconnecting") + log.Logger.Warningf("Already disconnecting") return nil } if err := c.FSM.CheckTransition(StateDisconnecting); err != nil { @@ -296,7 +297,7 @@ func (c *Client) SetDisconnecting() error { func (c *Client) SetDisconnected() error { if c.InFSMState(StateDisconnected) { // already disconnected, show no error - c.Logger.Warningf("Already disconnected") + log.Logger.Warningf("Already disconnected") return nil } if err := c.FSM.CheckTransition(StateDisconnected); err != nil { @@ -320,7 +321,7 @@ func (c *Client) goBackInternal() { err := c.GoBack() if err != nil { // TODO(jwijenbergh): Bit suspicious - logging level INFO, yet stacktrace logged. - c.Logger.Infof("failed going back: %s\nstacktrace:\n%s", err.Error(), err.(*errors.Error).ErrorStack()) + log.Logger.Infof("failed going back: %s\nstacktrace:\n%s", err.Error(), err.(*errors.Error).ErrorStack()) } } diff --git a/client/server.go b/client/server.go index d400d0b..fe4c57d 100644 --- a/client/server.go +++ b/client/server.go @@ -3,6 +3,7 @@ package client import ( "github.com/eduvpn/eduvpn-common/internal/failover" "github.com/eduvpn/eduvpn-common/internal/http" + "github.com/eduvpn/eduvpn-common/internal/log" "github.com/eduvpn/eduvpn-common/internal/oauth" "github.com/eduvpn/eduvpn-common/internal/server" "github.com/eduvpn/eduvpn-common/types" @@ -68,7 +69,7 @@ func (c *Client) getConfig(srv server.Server, preferTCP bool, t oauth.Token) (*C // This is the best effort err := server.RefreshEndpoints(srv) if err != nil { - c.Logger.Warningf("failed to refresh server endpoints: %v", err) + log.Logger.Warningf("failed to refresh server endpoints: %v", err) } cfg, err := c.retryConfigAuth(srv, preferTCP, t) @@ -88,7 +89,7 @@ func (c *Client) getConfig(srv server.Server, preferTCP bool, t oauth.Token) (*C if err = c.Config.Save(&c); err != nil { // TODO(jwijenbergh): Not sure why INFO level, yet stacktrace... // TODO(jwijenbergh): Even worse, why logging it but then return nil? The calling code will think that everything went well. - c.Logger.Infof("c.Config.Save failed: %s\nstacktrace:\n%s", + log.Logger.Infof("c.Config.Save failed: %s\nstacktrace:\n%s", err.Error(), err.(*errors.Error).ErrorStack()) } @@ -159,7 +160,7 @@ func (c *Client) RemoveSecureInternet() error { if err := c.Config.Save(&c); err != nil { // TODO(jwijenbergh): Not sure why INFO level, yet stacktrace... // TODO(jwijenbergh): Even worse, why logging it but then return nil? The calling code will think that everything went well. - c.Logger.Infof("c.Config.Save failed: %s\nstacktrace:\n%s", + log.Logger.Infof("c.Config.Save failed: %s\nstacktrace:\n%s", err.Error(), err.(*errors.Error).ErrorStack()) } return nil @@ -181,7 +182,7 @@ func (c *Client) RemoveInstituteAccess(url string) error { if err := c.Config.Save(&c); err != nil { // TODO(jwijenbergh): Not sure why INFO level, yet stacktrace... // TODO(jwijenbergh): Even worse, why logging it but then return nil? The calling code will think that everything went well. - c.Logger.Infof("c.Config.Save failed: %s\nstacktrace:\n%s", + log.Logger.Infof("c.Config.Save failed: %s\nstacktrace:\n%s", err.Error(), err.(*errors.Error).ErrorStack()) } return nil @@ -203,7 +204,7 @@ func (c *Client) RemoveCustomServer(url string) error { if err := c.Config.Save(&c); err != nil { // TODO(jwijenbergh): Not sure why INFO level, yet stacktrace... // TODO(jwijenbergh): Even worse, why logging it but then return nil? The calling code will think that everything went well. - c.Logger.Infof("c.Config.Save failed: %s\nstacktrace:\n%s", + log.Logger.Infof("c.Config.Save failed: %s\nstacktrace:\n%s", err.Error(), err.(*errors.Error).ErrorStack()) } return nil @@ -557,7 +558,7 @@ func (c *Client) ShouldRenewButton() bool { srv, err := c.Servers.GetCurrentServer() if err != nil { - c.Logger.Infof("no server to renew: %s\nstacktrace:\n%s", err.Error(), err.(*errors.Error).ErrorStack()) + log.Logger.Infof("no server to renew: %s\nstacktrace:\n%s", err.Error(), err.(*errors.Error).ErrorStack()) return false } @@ -649,7 +650,7 @@ func (c *Client) StartFailover(gateway string, wgMTU int, readRxBytes func() (in return false, errors.New("Profile does not support OpenVPN fallback") } - c.Failover = failover.New(readRxBytes, c.Logger) + c.Failover = failover.New(readRxBytes) return c.Failover.Start(gateway, wgMTU) } diff --git a/internal/failover/failover.go b/internal/failover/failover.go index 1c4c32e..97b1da1 100644 --- a/internal/failover/failover.go +++ b/internal/failover/failover.go @@ -2,8 +2,6 @@ package failover import ( "time" - - "github.com/eduvpn/eduvpn-common/internal/log" ) const ( @@ -17,6 +15,6 @@ const ( // New creates a failover monitor for the gateway and the rx bytes function reader // This is a simple wrapper over `NewDroppedMonitor` to create one with the default settings // If this function returns True, the connection is dropped. False means it has exited and we don't know for sure if it's dropped or not -func New(readRxBytes func() (int64, error), logger log.FileLogger) *DroppedConMon { - return NewDroppedMonitor(pInterval, pDropped, readRxBytes, logger) +func New(readRxBytes func() (int64, error)) *DroppedConMon { + return NewDroppedMonitor(pInterval, pDropped, readRxBytes) } diff --git a/internal/failover/monitor.go b/internal/failover/monitor.go index c437d30..f143f94 100644 --- a/internal/failover/monitor.go +++ b/internal/failover/monitor.go @@ -20,13 +20,10 @@ type DroppedConMon struct { // The cancel context // This is used to cancel the dropped connection monitor cancel context.CancelFunc - - // logger is the logger for debugging purposes - logger log.FileLogger } -func NewDroppedMonitor(pingInterval time.Duration, pDropped int, readRxBytes func() (int64, error), logger log.FileLogger) *DroppedConMon { - return &DroppedConMon{pInterval: pingInterval, pDropped: pDropped, readRxBytes: readRxBytes, logger: logger} +func NewDroppedMonitor(pingInterval time.Duration, pDropped int, readRxBytes func() (int64, error)) *DroppedConMon { + return &DroppedConMon{pInterval: pingInterval, pDropped: pDropped, readRxBytes: readRxBytes} } // Dropped checks whether or not the connection is 'dropped' @@ -36,7 +33,7 @@ func (m *DroppedConMon) dropped(startBytes int64) (bool, error) { if err != nil { return false, err } - m.logger.Debugf("[Failover] Alive check, current Rx bytes: %d, start Rx bytes: %d", b, startBytes) + log.Logger.Debugf("[Failover] Alive check, current Rx bytes: %d, start Rx bytes: %d", b, startBytes) return b <= startBytes, nil } @@ -68,17 +65,17 @@ func (m *DroppedConMon) Start(gateway string, mtuSize int) (bool, error) { // Send a ping and wait for max 2 seconds // If we have then increased Rx bytes we return early if err = p.Send(gateway, 1); err != nil { - m.logger.Debugf("[Failover] First ping failed, exiting...") + log.Logger.Debugf("[Failover] First ping failed, exiting...") return false, err } - m.logger.Debugf("[Failover] Now we are doing alive check") + log.Logger.Debugf("[Failover] Now we are doing alive check") // Read the pong, if we got the echo reply then everything is fine, early return if err = p.Read(time.Now().Add(m.pInterval)); err == nil { - m.logger.Debugf("[Failover] Got early pong, exiting...") + log.Logger.Debugf("[Failover] Got early pong, exiting...") return false, err } - m.logger.Debugf("[Failover] Error reading pong: %v", err) + log.Logger.Debugf("[Failover] Error reading pong: %v", err) // Create a new ticker that executes our ping function every 'interval' seconds // It starts immediately and stops when we reach the end @@ -86,15 +83,15 @@ func (m *DroppedConMon) Start(gateway string, mtuSize int) (bool, error) { defer ticker.Stop() // Otherwise send n pings, without waiting for pong and then check if dropped - m.logger.Debugf("[Failover] Starting by sending pings and not waiting for pong...") + log.Logger.Debugf("[Failover] Starting by sending pings and not waiting for pong...") // Loop until the max drop counter // We begin with 2 as this is used as the sequence number for ping // and we have already sent a ping for s := 2; s <= m.pDropped; s++ { - m.logger.Debugf("[Failover] Sending ping: %d, with size: %d", s, mtuSize) + log.Logger.Debugf("[Failover] Sending ping: %d, with size: %d", s, mtuSize) // Send a ping and return if an error occurs if err := p.Send(gateway, s); err != nil { - m.logger.Debugf("[Failover] A ping failed, exiting...") + log.Logger.Debugf("[Failover] A ping failed, exiting...") return false, err } // Wait for the next tick to continue diff --git a/internal/log/log.go b/internal/log/log.go index 2fb2444..16cf1fd 100644 --- a/internal/log/log.go +++ b/internal/log/log.go @@ -47,6 +47,9 @@ type FileLogger struct { file *os.File } +// Logger is the global logger instance +var Logger *FileLogger + type Level int8 const ( @@ -174,3 +177,7 @@ func (logger *FileLogger) log(lvl Level, msg string, params ...interface{}) { log.Println(f) } } + +func init() { + Logger = &FileLogger{} +} -- cgit v1.2.3