refactor logging to implement #142

irc/config.go

@@ -131,20 +131,6 @@ type ConnectionThrottleConfig struct {
 	Exempted           []string
 }
 
-// LoggingConfig controls a single logging method.
-type LoggingConfig struct {
-	Method        string
-	MethodStdout  bool
-	MethodStderr  bool
-	MethodFile    bool
-	Filename      string
-	TypeString    string       `yaml:"type"`
-	Types         []string     `yaml:"real-types"`
-	ExcludedTypes []string     `yaml:"real-excluded-types"`
-	LevelString   string       `yaml:"level"`
-	Level         logger.Level `yaml:"level-real"`
-}
-
 // LineLenConfig controls line lengths.
 type LineLenConfig struct {
 	Tags int
@@ -219,7 +205,7 @@ type Config struct {
 
 	Opers map[string]*OperConfig
 
-	Logging []LoggingConfig
+	Logging []logger.LoggingConfig
 
 	Debug struct {
 		StackImpact StackImpactConfig
@@ -431,7 +417,7 @@ func LoadConfig(filename string) (config *Config, err error) {
 	if config.Limits.LineLen.Tags < 512 || config.Limits.LineLen.Rest < 512 {
 		return nil, errors.New("Line lengths must be 512 or greater (check the linelen section under server->limits)")
 	}
-	var newLogConfigs []LoggingConfig
+	var newLogConfigs []logger.LoggingConfig
 	for _, logConfig := range config.Logging {
 		// methods
 		methods := make(map[string]bool)

irc/logger/logger.go

@@ -53,30 +53,52 @@ var (
 
 // Manager is the main interface used to log debug/info/error messages.
 type Manager struct {
+	configMutex     sync.RWMutex
 	loggers         []singleLogger
 	stdoutWriteLock sync.Mutex // use one lock for both stdout and stderr
 	fileWriteLock   sync.Mutex
-	DumpingRawInOut bool
+	dumpingRawInOut bool
 }
 
 // Config represents the configuration of a single logger.
-type Config struct {
+type LoggingConfig struct {
-	// logging methods
+	Method        string
 	MethodStdout  bool
 	MethodStderr  bool
 	MethodFile    bool
 	Filename      string
-	// logging level
+	TypeString    string   `yaml:"type"`
-	Level Level
+	Types         []string `yaml:"real-types"`
-	// logging types
+	ExcludedTypes []string `yaml:"real-excluded-types"`
-	Types         []string
+	LevelString   string   `yaml:"level"`
-	ExcludedTypes []string
+	Level         Level    `yaml:"level-real"`
 }
 
 // NewManager returns a new log manager.
-func NewManager(config ...Config) (*Manager, error) {
+func NewManager(config []LoggingConfig) (*Manager, error) {
 	var logger Manager
 
+	if err := logger.ApplyConfig(config); err != nil {
+		return nil, err
+	}
+
+	return &logger, nil
+}
+
+func (logger *Manager) ApplyConfig(config []LoggingConfig) error {
+	logger.configMutex.Lock()
+	defer logger.configMutex.Unlock()
+
+	for _, logger := range logger.loggers {
+		logger.Close()
+	}
+
+	logger.loggers = nil
+	logger.dumpingRawInOut = false
+
+	// for safety, this deep-copies all mutable data in `config`
+	// XXX let's keep it that way
+	var lastErr error
 	for _, logConfig := range config {
 		typeMap := make(map[string]bool)
 		for _, name := range logConfig.Types {
@@ -101,12 +123,12 @@ func NewManager(config ...Config) (*Manager, error) {
 			fileWriteLock:   &logger.fileWriteLock,
 		}
 		if typeMap["userinput"] || typeMap["useroutput"] || (typeMap["*"] && !(excludedTypeMap["userinput"] && excludedTypeMap["useroutput"])) {
-			logger.DumpingRawInOut = true
+			logger.dumpingRawInOut = true
 		}
 		if sLogger.MethodFile.Enabled {
 			file, err := os.OpenFile(sLogger.MethodFile.Filename, os.O_CREATE|os.O_APPEND|os.O_WRONLY, 0666)
 			if err != nil {
-				return nil, fmt.Errorf("Could not open log file %s [%s]", sLogger.MethodFile.Filename, err.Error())
+				lastErr = fmt.Errorf("Could not open log file %s [%s]", sLogger.MethodFile.Filename, err.Error())
 			}
 			writer := bufio.NewWriter(file)
 			sLogger.MethodFile.File = file
@@ -115,11 +137,20 @@ func NewManager(config ...Config) (*Manager, error) {
 		logger.loggers = append(logger.loggers, sLogger)
 	}
 
-	return &logger, nil
+	return lastErr
+}
+
+func (logger *Manager) DumpingRawInOut() bool {
+	logger.configMutex.RLock()
+	defer logger.configMutex.RUnlock()
+	return logger.dumpingRawInOut
 }
 
 // Log logs the given message with the given details.
 func (logger *Manager) Log(level Level, logType string, messageParts ...string) {
+	logger.configMutex.RLock()
+	defer logger.configMutex.RUnlock()
+
 	for _, singleLogger := range logger.loggers {
 		singleLogger.Log(level, logType, messageParts...)
 	}
@@ -127,30 +158,22 @@ func (logger *Manager) Log(level Level, logType string, messageParts ...string)
 
 // Debug logs the given message as a debug message.
 func (logger *Manager) Debug(logType string, messageParts ...string) {
-	for _, singleLogger := range logger.loggers {
+	logger.Log(LogDebug, logType, messageParts...)
-		singleLogger.Log(LogDebug, logType, messageParts...)
-	}
 }
 
 // Info logs the given message as an info message.
 func (logger *Manager) Info(logType string, messageParts ...string) {
-	for _, singleLogger := range logger.loggers {
+	logger.Log(LogInfo, logType, messageParts...)
-		singleLogger.Log(LogInfo, logType, messageParts...)
-	}
 }
 
 // Warning logs the given message as a warning message.
 func (logger *Manager) Warning(logType string, messageParts ...string) {
-	for _, singleLogger := range logger.loggers {
+	logger.Log(LogWarning, logType, messageParts...)
-		singleLogger.Log(LogWarning, logType, messageParts...)
-	}
 }
 
 // Error logs the given message as an error message.
 func (logger *Manager) Error(logType string, messageParts ...string) {
-	for _, singleLogger := range logger.loggers {
+	logger.Log(LogError, logType, messageParts...)
-		singleLogger.Log(LogError, logType, messageParts...)
-	}
 }
 
 // Fatal logs the given message as an error message, then exits.
@@ -179,6 +202,18 @@ type singleLogger struct {
 	ExcludedTypes   map[string]bool
 }
 
+func (logger *singleLogger) Close() error {
+	if logger.MethodFile.Enabled {
+		flushErr := logger.MethodFile.Writer.Flush()
+		closeErr := logger.MethodFile.File.Close()
+		if flushErr != nil {
+			return flushErr
+		}
+		return closeErr
+	}
+	return nil
+}
+
 // Log logs the given message with the given details.
 func (logger *singleLogger) Log(level Level, logType string, messageParts ...string) {
 	// no logging enabled

irc/server.go

@@ -37,6 +37,10 @@ var (
 	couldNotParseIPMsg, _ = (&[]ircmsg.IrcMessage{ircmsg.MakeMessage(nil, "", "ERROR", "Unable to parse your IP address")}[0]).Line()
 )
 
+const (
+	rawOutputNotice = "This server is in debug mode and is logging all user I/O. If you do not wish for everything you send to be readable by the server owner(s), please disconnect."
+)
+
 // Limits holds the maximum limits for various things such as topic lengths.
 type Limits struct {
 	AwayLen        int
@@ -425,8 +429,8 @@ func (server *Server) tryRegister(c *Client) {
 	c.RplISupport()
 	server.MOTD(c)
 	c.Send(nil, c.nickMaskString, RPL_UMODEIS, c.nick, c.ModeString())
-	if server.logger.DumpingRawInOut {
+	if server.logger.DumpingRawInOut() {
-		c.Notice("This server is in debug mode and is logging all user I/O. If you do not wish for everything you send to be readable by the server owner(s), please disconnect.")
+		c.Notice(rawOutputNotice)
 	}
 }
 
@@ -1408,23 +1412,22 @@ func (server *Server) applyConfig(config *Config, initial bool) error {
 	}
 
 	// set RPL_ISUPPORT
+	var newISupportReplies [][]string
 	oldISupportList := server.isupport
 	server.setISupport()
 	if oldISupportList != nil {
-		newISupportReplies := oldISupportList.GetDifference(server.isupport)
+		newISupportReplies = oldISupportList.GetDifference(server.isupport)
-		// push new info to all of our clients
-		server.clients.ByNickMutex.RLock()
-		for _, sClient := range server.clients.ByNick {
-			for _, tokenline := range newISupportReplies {
-				// ugly trickery ahead
-				sClient.Send(nil, server.name, RPL_ISUPPORT, append([]string{sClient.nick}, tokenline...)...)
-			}
-		}
-		server.clients.ByNickMutex.RUnlock()
 	}
 
 	server.loadMOTD(config.Server.MOTD)
 
+	// reload logging config
+	err = server.logger.ApplyConfig(config.Logging)
+	if err != nil {
+		return err
+	}
+	dumpingRawInOut := server.logger.DumpingRawInOut()
+
 	if initial {
 		if err := server.loadDatastore(config.Datastore.Path); err != nil {
 			return err
@@ -1434,6 +1437,21 @@ func (server *Server) applyConfig(config *Config, initial bool) error {
 	// we are now open for business
 	server.setupListeners(config)
 
+	if !initial {
+		// push new info to all of our clients
+		server.clients.ByNickMutex.RLock()
+		for _, sClient := range server.clients.ByNick {
+			for _, tokenline := range newISupportReplies {
+				sClient.Send(nil, server.name, RPL_ISUPPORT, append([]string{sClient.nick}, tokenline...)...)
+			}
+
+			if dumpingRawInOut {
+				sClient.Notice(rawOutputNotice)
+			}
+		}
+		server.clients.ByNickMutex.RUnlock()
+	}
+
 	return nil
 }
 

oragono.go

@@ -46,21 +46,7 @@ Options:
 		log.Fatal("Config file did not load successfully:", err.Error())
 	}
 
-	// assemble separate log configs
+	logger, err := logger.NewManager(config.Logging)
-	var logConfigs []logger.Config
-	for _, lConfig := range config.Logging {
-		logConfigs = append(logConfigs, logger.Config{
-			MethodStdout:  lConfig.MethodStdout,
-			MethodStderr:  lConfig.MethodStderr,
-			MethodFile:    lConfig.MethodFile,
-			Filename:      lConfig.Filename,
-			Level:         lConfig.Level,
-			Types:         lConfig.Types,
-			ExcludedTypes: lConfig.ExcludedTypes,
-		})
-	}
-
-	logger, err := logger.NewManager(logConfigs...)
 	if err != nil {
 		log.Fatal("Logger did not load successfully:", err.Error())
 	}