From c3f114d677a43b578292af509d1a7bdd0fd4dd02 Mon Sep 17 00:00:00 2001 From: Michael Schurter Date: Tue, 31 Aug 2021 12:15:20 -0700 Subject: [PATCH] do not append timestamp to current log file This patch updates the file_sink to *not* append the timestamp on the current log file. The timestamp is only appended when the file is rotated. This matches the behavior: - Implemented in Nomad v1.1.4 with hashicorp/nomad#11070 - Requested in hashicorp/nomad#11061 - Referenced in Consul's documentation: https://www.consul.io/docs/enterprise/audit-logging > The following example configures a destination called "My Sink" which > stores audit events at the file `/tmp/audit.json`. Nomad's documentation is a little vague but implies the prior behavior of always appending a timestamp. While this is a *backward incompatible change* I think it's worth it for usability and matching Consul's documentation. Nomad's documentation can be fixed and a notice placed in the Upgrade Guide. --- file_sink.go | 20 ++++++++++++++------ file_sink_test.go | 36 ++++++++++++++++++++++++++++++++++-- 2 files changed, 48 insertions(+), 8 deletions(-) diff --git a/file_sink.go b/file_sink.go index 5ac5f84..19f9455 100644 --- a/file_sink.go +++ b/file_sink.go @@ -159,11 +159,9 @@ func (fs *FileSink) open() error { } createTime := time.Now() - // New file name as the format: - // file rotation enabled: filename-timestamp.extension - // file rotation disabled: filename.extension - newfileName := fs.newFileName(createTime) - newfilePath := filepath.Join(fs.Path, newfileName) + // New file name: filename.extension + // Old file name: filename-timestamp.extension + newfilePath := filepath.Join(fs.Path, fs.FileName) var err error fs.f, err = os.OpenFile(newfilePath, os.O_APPEND|os.O_WRONLY|os.O_CREATE, mode) @@ -198,8 +196,18 @@ func (fs *FileSink) rotate() error { ((elapsed > fs.MaxDuration) && (fs.MaxDuration > 0)) { fs.f.Close() + + // Move current log file to a timestamped file. + rotateTime := time.Now().UnixNano() + rotateFileName := fmt.Sprintf(fs.fileNamePattern(), strconv.FormatInt(rotateTime, 10)) + oldPath := filepath.Join(fs.Path, fs.FileName) + newPath := filepath.Join(fs.Path, rotateFileName) + if err := os.Rename(oldPath, newPath); err != nil { + return fmt.Errorf("failed to rotate log file: %v", err) + } + if err := fs.pruneFiles(); err != nil { - return err + return fmt.Errorf("failed to prune log files: %w", err) } return fs.open() } diff --git a/file_sink_test.go b/file_sink_test.go index 3a715dd..e40eae4 100644 --- a/file_sink_test.go +++ b/file_sink_test.go @@ -142,9 +142,25 @@ func TestFileSink_TimeRotate(t *testing.T) { } want := 2 - if got, _ := ioutil.ReadDir(tmpDir); len(got) != want { + got, err := os.ReadDir(tmpDir) + if err != nil { + t.Fatalf("Error listing log directory: %v", err) + } + if len(got) != want { t.Errorf("Expected %d files, got %v file(s)", want, len(got)) } + found := false + names := []string{} + for _, entry := range got { + names = append(names, entry.Name()) + if entry.Name() == fs.FileName { + found = true + break + } + } + if !found { + t.Errorf("Did not find expected file %q -- found: %v", fs.FileName, names) + } } func TestFileSink_ByteRotate(t *testing.T) { @@ -183,9 +199,25 @@ func TestFileSink_ByteRotate(t *testing.T) { } want := 2 - if got, _ := ioutil.ReadDir(tmpDir); len(got) != want { + got, err := os.ReadDir(tmpDir) + if err != nil { + t.Fatalf("Error listing log directory: %v", err) + } + if len(got) != want { t.Errorf("Expected %d files, got %v file(s)", want, len(got)) } + found := false + names := []string{} + for _, entry := range got { + names = append(names, entry.Name()) + if entry.Name() == fs.FileName { + found = true + break + } + } + if !found { + t.Errorf("Did not find expected file %q -- found: %v", fs.FileName, names) + } } func TestFileSink_open(t *testing.T) {