From 069d634e9da15b28cb5006b4b59b3624ff6e8d7f Mon Sep 17 00:00:00 2001 From: Andreas Karis Date: Wed, 27 Sep 2023 17:59:49 +0200 Subject: [PATCH] Make logger state threadsafe Signed-off-by: Andreas Karis --- logging.go | 245 ++++++++++++++++++++++++++++++++++++------------ logging_test.go | 34 +++---- 2 files changed, 204 insertions(+), 75 deletions(-) diff --git a/logging.go b/logging.go index 166fe15..12173c3 100644 --- a/logging.go +++ b/logging.go @@ -21,6 +21,7 @@ import ( "path/filepath" "runtime/debug" "strings" + "sync" "time" lumberjack "gopkg.in/natefinch/lumberjack.v2" @@ -39,12 +40,7 @@ const ( structuredPrefixerOddArguments = "prefixer must return an even number of arguments for structured logging" ) -var logger *lumberjack.Logger -var logWriter io.Writer -var logLevel Level -var logToStderr bool -var prefixer Prefixer -var structuredPrefixer StructuredPrefixer +var loggingState state // LogOptions defines the configuration of the lumberjack logger type LogOptions struct { @@ -59,7 +55,7 @@ func init() { } func initLogger() { - logger = &lumberjack.Logger{} + loggingState.setLogger(&lumberjack.Logger{}) // Set default options. SetLogOptions(nil) @@ -74,12 +70,12 @@ func initLogger() { // SetPrefixer allows overwriting the Prefixer with a custom one. func SetPrefixer(p Prefixer) { - prefixer = p + loggingState.setPrefixer(p) } // SetStructuredPrefixer allows overwriting the StructuredPrefixer with a custom one. func SetStructuredPrefixer(p StructuredPrefixer) { - structuredPrefixer = p + loggingState.setStructuredPrefixer(p) } // SetDefaultPrefixer sets the default Prefixer. @@ -101,29 +97,10 @@ func SetDefaultStructuredPrefixer() { // Set the logging options (LogOptions) func SetLogOptions(options *LogOptions) { - // give some default value - logger.MaxSize = 100 - logger.MaxAge = 5 - logger.MaxBackups = 5 - logger.Compress = true - if options != nil { - if options.MaxAge != nil { - logger.MaxAge = *options.MaxAge - } - if options.MaxSize != nil { - logger.MaxSize = *options.MaxSize - } - if options.MaxBackups != nil { - logger.MaxBackups = *options.MaxBackups - } - if options.Compress != nil { - logger.Compress = *options.Compress - } - } - + loggingState.setLogOptions(options) // Update the logWriter if necessary. - if isFileLoggingEnabled() { - logWriter = logger + if loggingState.isFileLoggingEnabled() { + loggingState.setLoggerAsLogWriter() } } @@ -132,10 +109,10 @@ func SetLogFile(filename string) { // Allow logging to stderr only. Print an error a single time when this is set to the empty string but stderr // logging is off. if filename == "" { - if !logToStderr { + if !loggingState.getLogToStderr() { fmt.Fprint(os.Stderr, logFileReqFailMsg) } - disableFileLogging() + loggingState.setLogFile("") return } @@ -150,30 +127,18 @@ func SetLogFile(filename string) { return } - logger.Filename = filename - logWriter = logger -} - -// disableFileLogging disables file logging. -func disableFileLogging() { - logger.Filename = "" - logWriter = nil -} - -// isFileLoggingEnabled returns true if file logging is enabled. -func isFileLoggingEnabled() bool { - return logWriter != nil + loggingState.setLogFile(filename) } // GetLogLevel gets current logging level func GetLogLevel() Level { - return logLevel + return loggingState.getLogLevel() } // SetLogLevel sets logging level func SetLogLevel(level Level) { if level.IsValid() { - logLevel = level + loggingState.setLogLevel(level) } else { fmt.Fprintf(os.Stderr, setLevelFailMsg, level) } @@ -181,15 +146,15 @@ func SetLogLevel(level Level) { // SetLogStderr sets flag for logging stderr output func SetLogStderr(enable bool) { - if !enable && !isFileLoggingEnabled() { + if !enable && !loggingState.isFileLoggingEnabled() { fmt.Fprint(os.Stderr, logFileReqFailMsg) } - logToStderr = enable + loggingState.setLogToStderr(enable) } // SetOutput set custom output WARNING subsequent call to SetLogFile or SetLogOptions invalidates this setting func SetOutput(out io.Writer) { - logWriter = out + loggingState.setLogWriter(out) } // Panicf prints logging plus stack trace. This should be used only for unrecoverable error @@ -256,7 +221,7 @@ func DebugStructured(msg string, args ...interface{}) { // structuredMessage takes msg and an even list of args and returns a structured message. func structuredMessage(loggingLevel Level, msg string, args ...interface{}) string { - prefixArgs := structuredPrefixer.CreateStructuredPrefix(loggingLevel, msg) + prefixArgs := loggingState.getStructuredPrefixer().CreateStructuredPrefix(loggingLevel, msg) if len(prefixArgs)%2 != 0 { panic(fmt.Sprintf("msg=%q logging_failure=%q", msg, structuredPrefixerOddArguments)) } @@ -297,24 +262,24 @@ func printf(level Level, format string, a ...interface{}) { // printWithPrefixf prints log messages if they match the configured log level. Messages are optionally prepended by a // configured prefix. func printWithPrefixf(level Level, printPrefix bool, format string, a ...interface{}) { - if level > logLevel { + if level > loggingState.getLogLevel() { return } - if !isFileLoggingEnabled() && !logToStderr { + if !loggingState.isFileLoggingEnabled() && !loggingState.getLogToStderr() { return } if printPrefix { - format = prefixer.CreatePrefix(level) + format + format = loggingState.getPrefixer().CreatePrefix(level) + format } - if logToStderr { + if loggingState.getLogToStderr() { doWritef(os.Stderr, format, a...) } - if isFileLoggingEnabled() { - doWritef(logWriter, format, a...) + if loggingState.isFileLoggingEnabled() { + doWritef(loggingState.getLogWriter(), format, a...) } } @@ -367,3 +332,167 @@ func resolvePath(path string) (string, error) { return filepath.Clean(path), nil } + +// state is the struct for the loggingState singleton. It allows us to set all logger attributes in a threadsafe manner +// for as long as we always access all of its attributes via its methods. +type state struct { + loggerMutex sync.RWMutex + logger *lumberjack.Logger + logWriter io.Writer + logLevel Level + logToStderr bool + prefixer Prefixer + structuredPrefixer StructuredPrefixer +} + +// setLogger sets s's logger. +func (s *state) setLogger(logger *lumberjack.Logger) { + s.loggerMutex.Lock() + defer s.loggerMutex.Unlock() + + s.logger = logger +} + +// getLogger gets s's logger. +func (s *state) getLogger() *lumberjack.Logger { + s.loggerMutex.RLock() + defer s.loggerMutex.RUnlock() + + return s.logger +} + +// setLogWriter sets s's logWriter. +func (s *state) setLogWriter(logWriter io.Writer) { + s.loggerMutex.Lock() + defer s.loggerMutex.Unlock() + + s.logWriter = logWriter +} + +// setLoggerAsLogWriter sets s's logWriter to a copy of its current logger. +func (s *state) setLoggerAsLogWriter() { + s.loggerMutex.Lock() + defer s.loggerMutex.Unlock() + + cp := *s.logger + s.logWriter = &cp +} + +// getLogWriter gets s's logWriter. +func (s *state) getLogWriter() io.Writer { + s.loggerMutex.RLock() + defer s.loggerMutex.RUnlock() + + return s.logWriter +} + +// setLogLevel sets s's logLeves. +func (s *state) setLogLevel(logLevel Level) { + s.loggerMutex.Lock() + defer s.loggerMutex.Unlock() + + s.logLevel = logLevel +} + +// getLogLevel gets s's logLeves. +func (s *state) getLogLevel() Level { + s.loggerMutex.RLock() + defer s.loggerMutex.RUnlock() + + return s.logLevel +} + +// setLogToStderr sets s's logToStderr. +func (s *state) setLogToStderr(logToStderr bool) { + s.loggerMutex.Lock() + defer s.loggerMutex.Unlock() + + s.logToStderr = logToStderr +} + +// getLogToStderr gets s's getLogToStderr. +func (s *state) getLogToStderr() bool { + s.loggerMutex.RLock() + defer s.loggerMutex.RUnlock() + + return s.logToStderr +} + +// setPrefixer sets s's prefixer. +func (s *state) setPrefixer(prefixer Prefixer) { + s.loggerMutex.Lock() + defer s.loggerMutex.Unlock() + + s.prefixer = prefixer +} + +// getPrefixer gets s's prefixer. +func (s *state) getPrefixer() Prefixer { + s.loggerMutex.RLock() + defer s.loggerMutex.RUnlock() + + return s.prefixer +} + +// setStructuredPrefixer sets s's structuredPrefixer. +func (s *state) setStructuredPrefixer(structuredPrefixer StructuredPrefixer) { + s.loggerMutex.Lock() + defer s.loggerMutex.Unlock() + + s.structuredPrefixer = structuredPrefixer +} + +// getStructuredPrefixer gets s's structuredPrefixer. +func (s *state) getStructuredPrefixer() StructuredPrefixer { + s.loggerMutex.RLock() + defer s.loggerMutex.RUnlock() + + return s.structuredPrefixer +} + +// isFileLoggingEnabled returns true if files *state is enabled. +func (s *state) isFileLoggingEnabled() bool { + return s.getLogWriter() != nil +} + +// setLogFile sets s's files *state. This method sets s.logger.Filename to the specified value and then sets s.logWriter +// to a copy of s.logger. If filename is the empty string, logWriter is set to nil. +func (s *state) setLogFile(filename string) { + s.loggerMutex.Lock() + defer s.loggerMutex.Unlock() + + if filename == "" { + s.logger.Filename = "" + s.logWriter = nil + return + } + s.logger.Filename = filename + cp := *(s.logger) + s.logWriter = &cp +} + +// setLogOpt8ions sets s's files *state options. +func (s *state) setLogOptions(options *LogOptions) { + s.loggerMutex.Lock() + defer s.loggerMutex.Unlock() + + // give some default value + s.logger.MaxSize = 100 + s.logger.MaxAge = 5 + s.logger.MaxBackups = 5 + s.logger.Compress = true + if options != nil { + if options.MaxAge != nil { + s.logger.MaxAge = *options.MaxAge + } + if options.MaxSize != nil { + s.logger.MaxSize = *options.MaxSize + } + if options.MaxBackups != nil { + s.logger.MaxBackups = *options.MaxBackups + } + if options.Compress != nil { + s.logger.Compress = *options.Compress + } + } +} diff --git a/logging_test.go b/logging_test.go index 89c59e6..aff86dd 100644 --- a/logging_test.go +++ b/logging_test.go @@ -63,11 +63,11 @@ var _ = Describe("CNI Logging Operations", func() { Context("Default settings", func() { When("the defaults are used", func() { It("logs to stderr", func() { - Expect(logToStderr).To(BeTrue()) + Expect(loggingState.getLogToStderr()).To(BeTrue()) }) It("does not log to file", func() { - Expect(isFileLoggingEnabled()).To(BeFalse()) + Expect(loggingState.isFileLoggingEnabled()).To(BeFalse()) }) }) }) @@ -134,7 +134,7 @@ var _ = Describe("CNI Logging Operations", func() { When("the log file name is valid", func() { It("prepares the logger's writer and creates the log file", func() { SetLogFile(logFile) - Expect(logWriter).To(Equal(logger)) + Expect(loggingState.getLogWriter()).To(Equal(loggingState.getLogger())) Expect(logFile).To(BeAnExistingFile()) }) }) @@ -153,7 +153,7 @@ var _ = Describe("CNI Logging Operations", func() { It("should be created", func() { SetLogFile(logFile) - Expect(logWriter).To(Equal(logger)) + Expect(loggingState.getLogWriter()).To(Equal(loggingState.getLogger())) Expect(logFile).To(BeAnExistingFile()) }) }) @@ -221,7 +221,7 @@ var _ = Describe("CNI Logging Operations", func() { Compress: getPrimitivePointer(true), } SetLogOptions(logOpts) - Expect(logger).To(Equal(expectedLogger)) + Expect(loggingState.getLogger()).To(Equal(expectedLogger)) }) }) @@ -240,7 +240,7 @@ var _ = Describe("CNI Logging Operations", func() { Compress: getPrimitivePointer(true), } SetLogOptions(logOpts) - Expect(logger).To(Equal(expectedLogger)) + Expect(loggingState.getLogger()).To(Equal(expectedLogger)) }) }) @@ -256,7 +256,7 @@ var _ = Describe("CNI Logging Operations", func() { } SetLogOptions(nil) - Expect(logger).To(Equal(expectedLogger)) + Expect(loggingState.getLogger()).To(Equal(expectedLogger)) }) }) }) @@ -625,26 +625,26 @@ var _ = Describe("CNI Log Level Operations", func() { It("sets the appropriate log level", func() { // by string SetLogLevel(StringToLevel(debugStr)) - Expect(logLevel).To(Equal(DebugLevel)) + Expect(loggingState.getLogLevel()).To(Equal(DebugLevel)) SetLogLevel(StringToLevel(infoStr)) - Expect(logLevel).To(Equal(InfoLevel)) + Expect(loggingState.getLogLevel()).To(Equal(InfoLevel)) SetLogLevel(StringToLevel(warningStr)) - Expect(logLevel).To(Equal(WarningLevel)) + Expect(loggingState.getLogLevel()).To(Equal(WarningLevel)) SetLogLevel(StringToLevel(errorStr)) - Expect(logLevel).To(Equal(ErrorLevel)) + Expect(loggingState.getLogLevel()).To(Equal(ErrorLevel)) SetLogLevel(StringToLevel(panicStr)) - Expect(logLevel).To(Equal(PanicLevel)) + Expect(loggingState.getLogLevel()).To(Equal(PanicLevel)) // by int for i := 1; i <= 5; i++ { l := Level(i) SetLogLevel(l) - Expect(logLevel).To(Equal(l)) + Expect(loggingState.getLogLevel()).To(Equal(l)) } // by level SetLogLevel(DebugLevel) - Expect(logLevel).To(Equal(DebugLevel)) + Expect(loggingState.getLogLevel()).To(Equal(DebugLevel)) SetLogLevel(WarningLevel) - Expect(logLevel).To(Equal(WarningLevel)) + Expect(loggingState.getLogLevel()).To(Equal(WarningLevel)) }) }) @@ -655,14 +655,14 @@ var _ = Describe("CNI Log Level Operations", func() { loggerOutput := captureStdErr(SetLogLevel, invalidLogLevel) Expect(loggerOutput).To(Equal(expectedLoggerOutput)) - Expect(logLevel).To(Equal(defaultLogLevel)) + Expect(loggingState.getLogLevel()).To(Equal(defaultLogLevel)) invalidLogLevel = Level(10) expectedLoggerOutput = fmt.Sprintf(setLevelFailMsg, invalidLogLevel) loggerOutput = captureStdErr(SetLogLevel, invalidLogLevel) Expect(loggerOutput).To(Equal(expectedLoggerOutput)) - Expect(logLevel).To(Equal(defaultLogLevel)) + Expect(loggingState.getLogLevel()).To(Equal(defaultLogLevel)) }) }) })