From 554c97a8cb3e4169ee5069c0fefa8ff8780c90d2 Mon Sep 17 00:00:00 2001 From: Dalton Date: Mon, 15 Jun 2020 17:00:37 -0500 Subject: [PATCH] AUTH-2813 adds back a single file support a cloudflared log file --- logger/file_writer.go | 30 +++++++++++++++--- logger/file_writer_test.go | 62 +++++++++++++++++++++++++++++++++++--- logger/formatter.go | 23 ++++++++++---- 3 files changed, 99 insertions(+), 16 deletions(-) diff --git a/logger/file_writer.go b/logger/file_writer.go index 123c0b9e..a1736ff1 100644 --- a/logger/file_writer.go +++ b/logger/file_writer.go @@ -33,7 +33,7 @@ func NewFileRollingWriter(directory, baseFileName string, maxFileSize int64, max // Write is an implementation of io.writer the rolls the file once it reaches its max size // It is expected the caller to Write is doing so in a thread safe manner (as WriteManager does). func (w *FileRollingWriter) Write(p []byte) (n int, err error) { - logFile := buildPath(w.directory, w.baseFileName) + logFile, isSingleFile := buildPath(w.directory, w.baseFileName) if w.fileHandle == nil { h, err := os.OpenFile(logFile, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0664) if err != nil { @@ -55,7 +55,7 @@ func (w *FileRollingWriter) Write(p []byte) (n int, err error) { written, err := w.fileHandle.Write(p) // check if the file needs to be rolled - if err == nil && info.Size()+int64(written) > w.maxFileSize { + if err == nil && info.Size()+int64(written) > w.maxFileSize && !isSingleFile { // close the file handle than do the renaming. A new one will be opened on the next write w.Close() w.rename(logFile, 1) @@ -77,7 +77,10 @@ func (w *FileRollingWriter) Close() { // but if cloudflared-1.log already exists, it is renamed to cloudflared-2.log, // then the other files move in to their postion func (w *FileRollingWriter) rename(sourcePath string, index uint) { - destinationPath := buildPath(w.directory, fmt.Sprintf("%s-%d", w.baseFileName, index)) + destinationPath, isSingleFile := buildPath(w.directory, fmt.Sprintf("%s-%d", w.baseFileName, index)) + if isSingleFile { + return //don't need to rename anything, it is a single file + } // rolled to the max amount of files allowed on disk if index >= w.maxFileCount { @@ -93,8 +96,13 @@ func (w *FileRollingWriter) rename(sourcePath string, index uint) { os.Rename(sourcePath, destinationPath) } -func buildPath(directory, fileName string) string { - return filepath.Join(directory, fileName+".log") +// return the path to the log file and if it is a single file or not. +// true means a single file. false means a rolled file +func buildPath(directory, fileName string) (string, bool) { + if !isDirectory(directory) { // not a directory, so try and treat it as a single file for backwards compatibility sake + return directory, true + } + return filepath.Join(directory, fileName+".log"), false } func exists(filePath string) bool { @@ -103,3 +111,15 @@ func exists(filePath string) bool { } return true } + +func isDirectory(path string) bool { + if path == "" { + return true + } + + fileInfo, err := os.Stat(path) + if err != nil { + return false + } + return fileInfo.IsDir() +} diff --git a/logger/file_writer_test.go b/logger/file_writer_test.go index 5ceae609..3c7a66a1 100644 --- a/logger/file_writer_test.go +++ b/logger/file_writer_test.go @@ -4,6 +4,7 @@ import ( "fmt" "io/ioutil" "os" + "path/filepath" "testing" "github.com/stretchr/testify/assert" @@ -31,18 +32,23 @@ func TestFileWrite(t *testing.T) { } func TestRolling(t *testing.T) { + dirName := "testdir" + err := os.Mkdir(dirName, 0755) + assert.NoError(t, err) + fileName := "test_file" - firstFile := fileName + ".log" - secondFile := fileName + "-1.log" - thirdFile := fileName + "-2.log" + firstFile := filepath.Join(dirName, fileName+".log") + secondFile := filepath.Join(dirName, fileName+"-1.log") + thirdFile := filepath.Join(dirName, fileName+"-2.log") defer func() { + os.RemoveAll(dirName) os.Remove(firstFile) os.Remove(secondFile) os.Remove(thirdFile) }() - w := NewFileRollingWriter("", fileName, 1000, 2) + w := NewFileRollingWriter(dirName, fileName, 1000, 2) defer w.Close() for i := 99; i >= 1; i-- { @@ -52,5 +58,51 @@ func TestRolling(t *testing.T) { assert.FileExists(t, firstFile, "first file doesn't exist as expected") assert.FileExists(t, secondFile, "second file doesn't exist as expected") assert.FileExists(t, thirdFile, "third file doesn't exist as expected") - assert.False(t, exists(fileName+"-3.log"), "limited to two files and there is more") + assert.False(t, exists(filepath.Join(dirName, fileName+"-3.log")), "limited to two files and there is more") +} + +func TestSingleFile(t *testing.T) { + fileName := "test_file" + testData := []byte(string("hello Dalton, how are you doing?")) + defer func() { + os.Remove(fileName) + }() + + w := NewFileRollingWriter(fileName, fileName, 1000, 2) + defer w.Close() + + l, err := w.Write(testData) + + assert.NoError(t, err) + assert.Equal(t, l, len(testData), "expected write length and data length to match") + + d, err := ioutil.ReadFile(fileName) + assert.FileExists(t, fileName, "file doesn't exist at expected path") + assert.Equal(t, d, testData, "expected data in file to match test data") +} + +func TestSingleFileInDirectory(t *testing.T) { + dirName := "testdir" + err := os.Mkdir(dirName, 0755) + assert.NoError(t, err) + + fileName := "test_file" + fullPath := filepath.Join(dirName, fileName+".log") + testData := []byte(string("hello Dalton, how are you doing?")) + defer func() { + os.Remove(fullPath) + os.RemoveAll(dirName) + }() + + w := NewFileRollingWriter(fullPath, fileName, 1000, 2) + defer w.Close() + + l, err := w.Write(testData) + + assert.NoError(t, err) + assert.Equal(t, l, len(testData), "expected write length and data length to match") + + d, err := ioutil.ReadFile(fullPath) + assert.FileExists(t, fullPath, "file doesn't exist at expected path") + assert.Equal(t, d, testData, "expected data in file to match test data") } diff --git a/logger/formatter.go b/logger/formatter.go index bbcdbdff..4e609af5 100644 --- a/logger/formatter.go +++ b/logger/formatter.go @@ -2,6 +2,7 @@ package logger import ( "fmt" + "runtime" "time" "github.com/acmacalister/skittles" @@ -58,14 +59,17 @@ func (f *DefaultFormatter) Content(l Level, c string) string { // TerminalFormatter is setup for colored output type TerminalFormatter struct { - format string + format string + supportsColor bool } // NewTerminalFormatter creates a Terminal formatter for colored output // format is the time format to use for timestamp formatting func NewTerminalFormatter(format string) Formatter { + supportsColor := (runtime.GOOS != "windows") return &TerminalFormatter{ - format: format, + format: format, + supportsColor: supportsColor, } } @@ -74,13 +78,13 @@ func (f *TerminalFormatter) Timestamp(l Level, d time.Time) string { t := "" switch l { case InfoLevel: - t = skittles.Cyan("[INFO] ") + t = f.output("[INFO] ", skittles.Cyan) case ErrorLevel: - t = skittles.Red("[ERROR] ") + t = f.output("[ERROR] ", skittles.Red) case DebugLevel: - t = skittles.Yellow("[DEBUG] ") + t = f.output("[DEBUG] ", skittles.Yellow) case FatalLevel: - t = skittles.Red("[FATAL] ") + t = f.output("[FATAL] ", skittles.Red) } return t } @@ -89,3 +93,10 @@ func (f *TerminalFormatter) Timestamp(l Level, d time.Time) string { func (f *TerminalFormatter) Content(l Level, c string) string { return c } + +func (f *TerminalFormatter) output(msg string, colorFunc func(interface{}) string) string { + if f.supportsColor { + return colorFunc(msg) + } + return msg +}