From 8d351aedb06a96bc27093b2d75cc91788bebb067 Mon Sep 17 00:00:00 2001 From: Ola Bini Date: Thu, 26 Mar 2020 14:00:01 +0000 Subject: [PATCH 1/5] Extract a LogTarget interface to make it easier to manage logging --- cmd/grumble/grumble.go | 2 +- cmd/grumble/server.go | 2 +- pkg/logtarget/logtarget.go | 20 ++++++++++++++++---- 3 files changed, 18 insertions(+), 6 deletions(-) diff --git a/cmd/grumble/grumble.go b/cmd/grumble/grumble.go index 56eaa9a..c47e2de 100644 --- a/cmd/grumble/grumble.go +++ b/cmd/grumble/grumble.go @@ -44,7 +44,7 @@ func main() { } log.SetPrefix("[G] ") log.SetFlags(log.LstdFlags | log.Lmicroseconds) - log.SetOutput(&logtarget.Target) + log.SetOutput(logtarget.Target) log.Printf("Grumble") log.Printf("Using data directory: %s", Args.DataDir) diff --git a/cmd/grumble/server.go b/cmd/grumble/server.go index 64b4dd8..7877830 100644 --- a/cmd/grumble/server.go +++ b/cmd/grumble/server.go @@ -156,7 +156,7 @@ func NewServer(id int64) (s *Server, err error) { s.Channels[0] = NewChannel(0, "Root") s.nextChanId = 1 - s.Logger = log.New(&logtarget.Target, fmt.Sprintf("[%v] ", s.Id), log.LstdFlags|log.Lmicroseconds) + s.Logger = log.New(logtarget.Target, fmt.Sprintf("[%v] ", s.Id), log.LstdFlags|log.Lmicroseconds) return } diff --git a/pkg/logtarget/logtarget.go b/pkg/logtarget/logtarget.go index 46a2eb2..144654c 100644 --- a/pkg/logtarget/logtarget.go +++ b/pkg/logtarget/logtarget.go @@ -7,15 +7,23 @@ package logtarget import ( "bytes" + "io" "os" "sync" ) +type LogTarget interface { + io.Writer + + OpenFile(string) error + Rotate() error +} + // LogTarget implements the io.Writer interface, allowing // LogTarget to be registered with the regular Go log package. // LogTarget multiplexes its incoming writes to multiple optional // output writers, and one main output writer (the log file). -type LogTarget struct { +type FileLogTarget struct { mu sync.Mutex logfn string file *os.File @@ -24,8 +32,12 @@ type LogTarget struct { var Target LogTarget +func init() { + Target = &FileLogTarget{} +} + // Write writes a log message to all registered io.Writers -func (target *LogTarget) Write(in []byte) (int, error) { +func (target *FileLogTarget) Write(in []byte) (int, error) { target.mu.Lock() defer target.mu.Unlock() @@ -48,7 +60,7 @@ func (target *LogTarget) Write(in []byte) (int, error) { // OpenFile opens the main log file for writing. // This method will open the file in append-only mode. -func (target *LogTarget) OpenFile(fn string) (err error) { +func (target *FileLogTarget) OpenFile(fn string) (err error) { target.logfn = fn target.file, err = os.OpenFile(target.logfn, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0650) if err != nil { @@ -61,7 +73,7 @@ func (target *LogTarget) OpenFile(fn string) (err error) { // This method holds a lock while rotating the log file, // and all log writes will be held back until the rotation // is complete. -func (target *LogTarget) Rotate() error { +func (target *FileLogTarget) Rotate() error { target.mu.Lock() defer target.mu.Unlock() From dd6f383d3e57dd3fcd589e289a23ec5d3b292d5f Mon Sep 17 00:00:00 2001 From: Ola Bini Date: Thu, 26 Mar 2020 14:04:14 +0000 Subject: [PATCH 2/5] Rename the default logtarget to not stutter. Also hide the default file log target implementation --- cmd/grumble/grumble.go | 4 ++-- cmd/grumble/server.go | 2 +- cmd/grumble/signal_unix.go | 2 +- pkg/logtarget/logtarget.go | 20 ++++++++++---------- 4 files changed, 14 insertions(+), 14 deletions(-) diff --git a/cmd/grumble/grumble.go b/cmd/grumble/grumble.go index c47e2de..6d2c93a 100644 --- a/cmd/grumble/grumble.go +++ b/cmd/grumble/grumble.go @@ -37,14 +37,14 @@ func main() { dataDir.Close() // Set up logging - err = logtarget.Target.OpenFile(Args.LogPath) + err = logtarget.Default.OpenFile(Args.LogPath) if err != nil { fmt.Fprintf(os.Stderr, "Unable to open log file (%v): %v", Args.LogPath, err) return } log.SetPrefix("[G] ") log.SetFlags(log.LstdFlags | log.Lmicroseconds) - log.SetOutput(logtarget.Target) + log.SetOutput(logtarget.Default) log.Printf("Grumble") log.Printf("Using data directory: %s", Args.DataDir) diff --git a/cmd/grumble/server.go b/cmd/grumble/server.go index 7877830..4f2bb4d 100644 --- a/cmd/grumble/server.go +++ b/cmd/grumble/server.go @@ -156,7 +156,7 @@ func NewServer(id int64) (s *Server, err error) { s.Channels[0] = NewChannel(0, "Root") s.nextChanId = 1 - s.Logger = log.New(logtarget.Target, fmt.Sprintf("[%v] ", s.Id), log.LstdFlags|log.Lmicroseconds) + s.Logger = log.New(logtarget.Default, fmt.Sprintf("[%v] ", s.Id), log.LstdFlags|log.Lmicroseconds) return } diff --git a/cmd/grumble/signal_unix.go b/cmd/grumble/signal_unix.go index 7127f24..9066b49 100644 --- a/cmd/grumble/signal_unix.go +++ b/cmd/grumble/signal_unix.go @@ -20,7 +20,7 @@ func SignalHandler() { signal.Notify(sigchan, syscall.SIGUSR2, syscall.SIGTERM, syscall.SIGINT) for sig := range sigchan { if sig == syscall.SIGUSR2 { - err := logtarget.Target.Rotate() + err := logtarget.Default.Rotate() if err != nil { fmt.Fprintf(os.Stderr, "unable to rotate log file: %v", err) } diff --git a/pkg/logtarget/logtarget.go b/pkg/logtarget/logtarget.go index 144654c..6ad43f5 100644 --- a/pkg/logtarget/logtarget.go +++ b/pkg/logtarget/logtarget.go @@ -12,6 +12,10 @@ import ( "sync" ) +// LogTarget implements the io.Writer interface, allowing +// LogTarget to be registered with the regular Go log package. +// LogTarget multiplexes its incoming writes to multiple optional +// output writers, and one main output writer (the log file). type LogTarget interface { io.Writer @@ -19,25 +23,21 @@ type LogTarget interface { Rotate() error } -// LogTarget implements the io.Writer interface, allowing -// LogTarget to be registered with the regular Go log package. -// LogTarget multiplexes its incoming writes to multiple optional -// output writers, and one main output writer (the log file). -type FileLogTarget struct { +type fileLogTarget struct { mu sync.Mutex logfn string file *os.File memLog *bytes.Buffer } -var Target LogTarget +var Default LogTarget func init() { - Target = &FileLogTarget{} + Default = &fileLogTarget{} } // Write writes a log message to all registered io.Writers -func (target *FileLogTarget) Write(in []byte) (int, error) { +func (target *fileLogTarget) Write(in []byte) (int, error) { target.mu.Lock() defer target.mu.Unlock() @@ -60,7 +60,7 @@ func (target *FileLogTarget) Write(in []byte) (int, error) { // OpenFile opens the main log file for writing. // This method will open the file in append-only mode. -func (target *FileLogTarget) OpenFile(fn string) (err error) { +func (target *fileLogTarget) OpenFile(fn string) (err error) { target.logfn = fn target.file, err = os.OpenFile(target.logfn, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0650) if err != nil { @@ -73,7 +73,7 @@ func (target *FileLogTarget) OpenFile(fn string) (err error) { // This method holds a lock while rotating the log file, // and all log writes will be held back until the rotation // is complete. -func (target *FileLogTarget) Rotate() error { +func (target *fileLogTarget) Rotate() error { target.mu.Lock() defer target.mu.Unlock() From 9c082d3516187cbf2a334f0dbe907717afcaedf1 Mon Sep 17 00:00:00 2001 From: Ola Bini Date: Thu, 26 Mar 2020 14:09:03 +0000 Subject: [PATCH 3/5] Make OpenFile a factory function instead of a method on LogTarget. Use this to initialize the default logging target --- cmd/grumble/grumble.go | 2 +- pkg/logtarget/logtarget.go | 25 +++++++++++-------------- 2 files changed, 12 insertions(+), 15 deletions(-) diff --git a/cmd/grumble/grumble.go b/cmd/grumble/grumble.go index 6d2c93a..04f87c7 100644 --- a/cmd/grumble/grumble.go +++ b/cmd/grumble/grumble.go @@ -37,7 +37,7 @@ func main() { dataDir.Close() // Set up logging - err = logtarget.Default.OpenFile(Args.LogPath) + logtarget.Default, err = logtarget.OpenFile(Args.LogPath) if err != nil { fmt.Fprintf(os.Stderr, "Unable to open log file (%v): %v", Args.LogPath, err) return diff --git a/pkg/logtarget/logtarget.go b/pkg/logtarget/logtarget.go index 6ad43f5..950458a 100644 --- a/pkg/logtarget/logtarget.go +++ b/pkg/logtarget/logtarget.go @@ -19,7 +19,6 @@ import ( type LogTarget interface { io.Writer - OpenFile(string) error Rotate() error } @@ -32,8 +31,17 @@ type fileLogTarget struct { var Default LogTarget -func init() { - Default = &fileLogTarget{} +// OpenFile creates a LogTarget pointing to a log file +// and returns it. +// This method will open the file in append-only mode. +func OpenFile(fileName string) (t LogTarget, err error) { + target := &fileLogTarget{} + target.logfn = fileName + target.file, err = os.OpenFile(fileName, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0650) + if err != nil { + return nil, err + } + return target, nil } // Write writes a log message to all registered io.Writers @@ -58,17 +66,6 @@ func (target *fileLogTarget) Write(in []byte) (int, error) { return len(in), nil } -// OpenFile opens the main log file for writing. -// This method will open the file in append-only mode. -func (target *fileLogTarget) OpenFile(fn string) (err error) { - target.logfn = fn - target.file, err = os.OpenFile(target.logfn, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0650) - if err != nil { - return err - } - return nil -} - // Rotate rotates the current log file. // This method holds a lock while rotating the log file, // and all log writes will be held back until the rotation From d6c4d9f766137c7115eaa38e7a6206bd087e091a Mon Sep 17 00:00:00 2001 From: Ola Bini Date: Thu, 26 Mar 2020 14:10:33 +0000 Subject: [PATCH 4/5] Remove unused field --- pkg/logtarget/logtarget.go | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/pkg/logtarget/logtarget.go b/pkg/logtarget/logtarget.go index 950458a..5cf99da 100644 --- a/pkg/logtarget/logtarget.go +++ b/pkg/logtarget/logtarget.go @@ -6,7 +6,6 @@ package logtarget import ( - "bytes" "io" "os" "sync" @@ -23,10 +22,9 @@ type LogTarget interface { } type fileLogTarget struct { - mu sync.Mutex - logfn string - file *os.File - memLog *bytes.Buffer + mu sync.Mutex + logfn string + file *os.File } var Default LogTarget From 8b2c7901ee2512c4adfd33ee59be5c3b787c05e7 Mon Sep 17 00:00:00 2001 From: Ola Bini Date: Thu, 26 Mar 2020 14:20:13 +0000 Subject: [PATCH 5/5] Use MultiWriter to simplify the writing implementation. Also make it possible to initialize the log target to variable amounts of writers, and doesn't hardcode the use of StdErr as output --- cmd/grumble/grumble.go | 2 +- pkg/logtarget/logtarget.go | 62 ++++++++++++++++++++++---------------- 2 files changed, 37 insertions(+), 27 deletions(-) diff --git a/cmd/grumble/grumble.go b/cmd/grumble/grumble.go index 04f87c7..139eadc 100644 --- a/cmd/grumble/grumble.go +++ b/cmd/grumble/grumble.go @@ -37,7 +37,7 @@ func main() { dataDir.Close() // Set up logging - logtarget.Default, err = logtarget.OpenFile(Args.LogPath) + logtarget.Default, err = logtarget.OpenFile(Args.LogPath, os.Stderr) if err != nil { fmt.Fprintf(os.Stderr, "Unable to open log file (%v): %v", Args.LogPath, err) return diff --git a/pkg/logtarget/logtarget.go b/pkg/logtarget/logtarget.go index 5cf99da..1b69a3b 100644 --- a/pkg/logtarget/logtarget.go +++ b/pkg/logtarget/logtarget.go @@ -21,57 +21,66 @@ type LogTarget interface { Rotate() error } -type fileLogTarget struct { +// logTarget is the default implementation of a log +// target. It can write to more than one writer at the same time +// but only rotate one file +type logTarget struct { mu sync.Mutex logfn string file *os.File + w io.Writer + ws []io.Writer } +// Default is the default log target for the application +// It has to be initialized before used var Default LogTarget +// OpenWriters returns a log target that will +// log to all the given writers at the same time +func OpenWriters(ws ...io.Writer) LogTarget { + target := &logTarget{} + target.w = io.MultiWriter(ws...) + return target +} + // OpenFile creates a LogTarget pointing to a log file // and returns it. // This method will open the file in append-only mode. -func OpenFile(fileName string) (t LogTarget, err error) { - target := &fileLogTarget{} +// It also takes a variable number of writers that are +// other log targets +func OpenFile(fileName string, ws ...io.Writer) (t LogTarget, err error) { + target := &logTarget{} target.logfn = fileName target.file, err = os.OpenFile(fileName, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0650) if err != nil { return nil, err } + target.ws = ws + target.w = io.MultiWriter(append(ws, target.file)...) return target, nil } // Write writes a log message to all registered io.Writers -func (target *fileLogTarget) Write(in []byte) (int, error) { +func (target *logTarget) Write(out []byte) (int, error) { + target.mu.Lock() + defer target.mu.Unlock() + + return target.Write(out) +} + +// Rotate rotates the current log file, if one is opened. +// This method holds a lock while rotating the log file, +// and all log writes will be held back until the rotation +// is complete. +func (target *logTarget) Rotate() error { target.mu.Lock() defer target.mu.Unlock() if target.file == nil { - panic("no log file opened") + return nil } - n, err := os.Stderr.Write(in) - if err != nil { - return n, err - } - - n, err = target.file.Write(in) - if err != nil { - return n, err - } - - return len(in), nil -} - -// Rotate rotates the current log file. -// This method holds a lock while rotating the log file, -// and all log writes will be held back until the rotation -// is complete. -func (target *fileLogTarget) Rotate() error { - target.mu.Lock() - defer target.mu.Unlock() - // Close the existing log file err := target.file.Close() if err != nil { @@ -82,6 +91,7 @@ func (target *fileLogTarget) Rotate() error { if err != nil { return err } + target.w = io.MultiWriter(append(target.ws, target.file)...) return nil }