Repository: alibaba/sentinel-golang
Pull request #552 (2023-12-15 03:48:23)

Upgrade the latest Metric log file name to a fixed format
<!--  Thanks for submitting a pull request! Here are some tips for you:
1. Please make sure you have read and understood the contributing guidelines: https://github.com/alibaba/blob/master/CONTRIBUTING.md
2. Please make sure the PR has a corresponding issue.
-->

### Describe what this PR does / why we need it

impove sentinel latest metric log file name to xxx-metrics.log. 

Fix the latest file name format for convenient log collection.

### Does this pull request fix one issue?

NONE

### Describe how you did it

1. replace func `nextFileNameOfTime`  with func `rotateWithDateAndNewFileName` 
2. init `latestOpSec` before roll log file when app starting

### Describe how to verify it

The implementation can be verified via go test.

### Special notes for reviews

CLAassistant: [![CLA assistant check](https://cla-assistant.io/pull/badge/not_signed)](https://cla-assistant.io/alibaba/sentinel-golang?pullRequest=552) <br/>Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our [Contributor License Agreement](https://cla-assistant.io/alibaba/sentinel-golang?pullRequest=552) before we can accept your contribution.<br/><hr/>**followtry** seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please [add the email address used for this commit to your account](https://help.github.com/articles/why-are-my-commits-linked-to-the-wrong-user/#commits-are-not-linked-to-any-user).<br/><sub>You have signed the CLA already but the status is still pending? Let us [recheck](https://cla-assistant.io/check/alibaba/sentinel-golang?pullRequest=552) it.</sub>
codecov-commenter: ## [Codecov](https://app.codecov.io/gh/alibaba/sentinel-golang/pull/552?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=alibaba) Report
Attention: `7 lines` in your changes are missing coverage. Please review.
> Comparison is base [(`0807185`)](https://app.codecov.io/gh/alibaba/sentinel-golang/commit/08071855bc67423777353c6e02f1390b419172b1?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=alibaba) 53.17% compared to head [(`54305b7`)](https://app.codecov.io/gh/alibaba/sentinel-golang/pull/552?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=alibaba) 53.33%.

> :exclamation: Current head 54305b7 differs from pull request most recent head 2340281. Consider uploading reports for the commit 2340281 to get more accurate results

| [Files](https://app.codecov.io/gh/alibaba/sentinel-golang/pull/552?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=alibaba) | Patch % | Lines |
|---|---|---|
| [util/file.go](https://app.codecov.io/gh/alibaba/sentinel-golang/pull/552?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=alibaba#diff-dXRpbC9maWxlLmdv) | 0.00% | [5 Missing :warning: ](https://app.codecov.io/gh/alibaba/sentinel-golang/pull/552?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=alibaba) |
| [core/log/metric/writer.go](https://app.codecov.io/gh/alibaba/sentinel-golang/pull/552?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=alibaba#diff-Y29yZS9sb2cvbWV0cmljL3dyaXRlci5nbw==) | 90.00% | [1 Missing and 1 partial :warning: ](https://app.codecov.io/gh/alibaba/sentinel-golang/pull/552?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=alibaba) |

<details><summary>Additional details and impacted files</summary>


```diff
@@                 Coverage Diff                 @@
##           amap-customized     #552      +/-   ##
===================================================
+ Coverage            53.17%   53.33%   +0.16%     
===================================================
  Files                   91       91              
  Lines                 5894     5932      +38     
===================================================
+ Hits                  3134     3164      +30     
- Misses                2413     2422       +9     
+ Partials               347      346       -1     
```



</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/alibaba/sentinel-golang/pull/552?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=alibaba).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=alibaba).


Patch:
diff --git a/core/log/metric/common.go b/core/log/metric/common.go
index 4b14fbc..c67cf5b 100644
--- a/core/log/metric/common.go
+++ b/core/log/metric/common.go
@@ -24,6 +24,7 @@ import (
 	"strings"
 
 	"github.com/alibaba/sentinel-golang/core/base"
+	"github.com/alibaba/sentinel-golang/util"
 )
 
 const (
@@ -73,6 +74,27 @@ func formMetricIdxFileName(metricFilename string) string {
 	return metricFilename + MetricIdxSuffix
 }
 
+func getLastFileSuffixIndex(list []string) uint32 {
+	if len(list) == 0 {
+		return 0
+	}
+	last := list[len(list)-1]
+	var n uint32 = 0
+	items := strings.Split(last, ".")
+	if len(items) > 0 {
+		v, err := strconv.ParseUint(items[len(items)-1], 10, 32)
+		if err == nil {
+			n = uint32(v)
+		}
+	}
+	return n
+}
+func doRotateBaseAndIndexFile(fileName, pattern string, index int) {
+	newFileName := fileName + "." + pattern + "." + strconv.Itoa(index)
+	util.FileRename(fileName, newFileName)
+	util.FileRename(formMetricIdxFileName(fileName), formMetricIdxFileName(newFileName))
+}
+
 func filenameMatches(filename, baseFilename string) bool {
 	if !strings.HasPrefix(filename, baseFilename) {
 		return false
diff --git a/core/log/metric/writer.go b/core/log/metric/writer.go
index 7cddd77..e34c42c 100644
--- a/core/log/metric/writer.go
+++ b/core/log/metric/writer.go
@@ -24,11 +24,12 @@ import (
 	"strings"
 	"sync"
 
+	"github.com/pkg/errors"
+
 	"github.com/alibaba/sentinel-golang/core/base"
 	"github.com/alibaba/sentinel-golang/core/config"
 	"github.com/alibaba/sentinel-golang/logging"
 	"github.com/alibaba/sentinel-golang/util"
-	"github.com/pkg/errors"
 )
 
 type DefaultMetricLogWriter struct {
@@ -148,7 +149,7 @@ func (d *DefaultMetricLogWriter) rollFileIfSizeExceeded(time uint64) error {
 }
 
 func (d *DefaultMetricLogWriter) rollToNextFile(time uint64) error {
-	newFilename, err := d.nextFileNameOfTime(time)
+	newFilename, err := d.rotateWithDateAndNewFileName(time)
 	if err != nil {
 		return err
 	}
@@ -199,6 +200,31 @@ func (d *DefaultMetricLogWriter) removeDeprecatedFiles() error {
 	return err
 }
 
+func (d *DefaultMetricLogWriter) rotateWithDateAndNewFileName(time uint64) (string, error) {
+	isNewDay := d.isNewDay(d.latestOpSec, int64(time/1000))
+	dateStr := util.FormatDate(time)
+	if isNewDay {
+		//format yesterday time as 2006-01-02
+		dateStr = util.FormatDate(time - 86400000)
+	}
+	filePattern := d.baseFilename + "." + dateStr
+	list, err := listMetricFilesConditional(d.baseDir, filePattern, func(fn string, p string) bool {
+		return strings.Contains(fn, p)
+	})
+	if err != nil {
+		return "", err
+	}
+	newFileName := filepath.Join(d.baseDir, d.baseFilename)
+	//file no exist then return baseFileName as new file name
+	exists, _ := util.FileExists(newFileName)
+	if len(list) == 0 && !exists {
+		return newFileName, nil
+	}
+	n := getLastFileSuffixIndex(list)
+	doRotateBaseAndIndexFile(newFileName, dateStr, int(n+1))
+	return newFileName, nil
+}
+
 func (d *DefaultMetricLogWriter) nextFileNameOfTime(time uint64) (string, error) {
 	dateStr := util.FormatDate(time)
 	filePattern := d.baseFilename + "." + dateStr
@@ -273,10 +299,10 @@ func (d *DefaultMetricLogWriter) initialize() error {
 		return nil
 	}
 	ts := util.CurrentTimeMillis()
+	d.latestOpSec = int64(ts / 1000)
 	if err := d.rollToNextFile(ts); err != nil {
 		return errors.Wrap(err, "failed to initialize metric log writer")
 	}
-	d.latestOpSec = int64(ts / 1000)
 	return nil
 }
 
diff --git a/util/file.go b/util/file.go
index de520f2..7109cb8 100644
--- a/util/file.go
+++ b/util/file.go
@@ -47,3 +47,11 @@ func CreateDirIfNotExists(dirname string) error {
 	}
 	return nil
 }
+
+func FileRename(oldName, newName string) bool {
+	err := os.Rename(oldName, newName)
+	if err != nil {
+		return false
+	}
+	return true
+}


Test patch:
diff --git a/core/log/metric/writer_test.go b/core/log/metric/writer_test.go
new file mode 100644
index 0000000..50f8616
--- /dev/null
+++ b/core/log/metric/writer_test.go
@@ -0,0 +1,334 @@
+package metric
+
+import (
+	"bufio"
+	"fmt"
+	"os"
+	"sync"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+
+	"github.com/alibaba/sentinel-golang/util"
+)
+
+func TestDefaultMetricLogWriter_rotateWithDateAndNewFileName(t *testing.T) {
+	_, offset := util.Now().Zone()
+	fmt.Println("old offset:", offset)
+	type fields struct {
+		baseDir           string
+		baseFilename      string
+		maxSingleSize     uint64
+		maxFileAmount     uint32
+		timezoneOffsetSec int64
+		latestOpSec       int64
+		curMetricFile     *os.File
+		curMetricIdxFile  *os.File
+		metricOut         *bufio.Writer
+		idxOut            *bufio.Writer
+		mux               *sync.RWMutex
+	}
+	type args struct {
+		time   uint64
+		before func()
+		file   func()
+		clear  func()
+	}
+	baseDir := "/tmp/logs/"
+	metricsLog := "test-metrics.log"
+	perm := 0755
+	tests := []struct {
+		name    string
+		fields  fields
+		args    args
+		want    string
+		wantErr assert.ErrorAssertionFunc
+	}{
+		{
+			name: "no any file in baseDir,create new file",
+			fields: fields{
+				baseDir:           baseDir,
+				baseFilename:      metricsLog,
+				maxSingleSize:     1024,
+				maxFileAmount:     10,
+				timezoneOffsetSec: 0,
+				latestOpSec:       0,
+				curMetricFile:     nil,
+				curMetricIdxFile:  nil,
+				metricOut:         nil,
+				idxOut:            nil,
+				mux:               nil,
+			},
+			args: args{
+				time: uint64(time.Date(2023, 12, 15, 0, 0, 0, 0, time.Local).Unix() * 1000),
+				before: func() {
+					_ = os.RemoveAll(baseDir)
+					_ = os.MkdirAll(baseDir, os.FileMode(perm))
+				},
+				clear: func() {
+					_ = os.RemoveAll(baseDir)
+				},
+			},
+			want:    baseDir + metricsLog,
+			wantErr: assert.NoError,
+		},
+		{
+			name: "have today date file in baseDir,rotate it and create new file",
+			fields: fields{
+				baseDir:           baseDir,
+				baseFilename:      metricsLog,
+				maxSingleSize:     1024,
+				maxFileAmount:     10,
+				timezoneOffsetSec: int64(offset),
+				latestOpSec:       time.Date(2023, 12, 15, 10, 0, 0, 0, time.Local).Unix(),
+				curMetricFile:     nil,
+				curMetricIdxFile:  nil,
+				metricOut:         nil,
+				idxOut:            nil,
+				mux:               nil,
+			},
+			args: args{
+				time: uint64(time.Date(2023, 12, 15, 10, 0, 1, 0, time.Local).Unix() * 1000),
+				before: func() {
+					_ = os.RemoveAll(baseDir)
+					_ = os.MkdirAll(baseDir, os.FileMode(perm))
+				},
+				file: func() {
+					_, _ = os.Create(baseDir + metricsLog)
+				},
+				clear: func() {
+					_ = os.RemoveAll(baseDir)
+				},
+			},
+			want:    baseDir + metricsLog,
+			wantErr: assert.NoError,
+		},
+		{
+			name: "have yesterday date file in baseDir,rotate it to yesterday date file and create new today date file",
+			fields: fields{
+				baseDir:           baseDir,
+				baseFilename:      metricsLog,
+				maxSingleSize:     1024,
+				maxFileAmount:     10,
+				timezoneOffsetSec: int64(offset),
+				latestOpSec:       time.Date(2023, 12, 14, 23, 59, 59, 0, time.Local).Unix(), //昨日
+				curMetricFile:     nil,
+				curMetricIdxFile:  nil,
+				metricOut:         nil,
+				idxOut:            nil,
+				mux:               nil,
+			},
+			args: args{
+				time: uint64(time.Date(2023, 12, 15, 0, 0, 0, 0, time.Local).Unix() * 1000),
+				before: func() {
+					_ = os.RemoveAll(baseDir)
+					_ = os.MkdirAll(baseDir, os.FileMode(perm))
+				},
+				file: func() {
+					yesterday := time.Date(2023, 12, 14, 23, 59, 59, 0, time.Local).Unix() * 1000
+					dateStr := util.FormatDate(uint64(yesterday))
+					_, _ = os.Create(baseDir + metricsLog)
+					_, _ = os.Create(baseDir + metricsLog + "." + dateStr + ".1")
+				},
+				clear: func() {
+					_ = os.RemoveAll(baseDir)
+				},
+			},
+			want:    baseDir + metricsLog,
+			wantErr: assert.NoError,
+		},
+		{
+			name: "have yesterday archive file and today date file in baseDir,rotate it and create new file",
+			fields: fields{
+				baseDir:           baseDir,
+				baseFilename:      metricsLog,
+				maxSingleSize:     1024,
+				maxFileAmount:     10,
+				timezoneOffsetSec: int64(offset),
+				latestOpSec:       time.Date(2023, 12, 15, 10, 0, 0, 0, time.Local).Unix(),
+				curMetricFile:     nil,
+				curMetricIdxFile:  nil,
+				metricOut:         nil,
+				idxOut:            nil,
+				mux:               nil,
+			},
+			args: args{
+				time: uint64(time.Date(2023, 12, 15, 10, 0, 1, 0, time.Local).Unix() * 1000),
+				before: func() {
+					_ = os.RemoveAll(baseDir)
+					_ = os.MkdirAll(baseDir, os.FileMode(perm))
+				},
+				file: func() {
+					yesterday := time.Now().Add(-24*time.Hour).Unix() * 1000
+					dateStr := util.FormatDate(uint64(yesterday))
+					_, _ = os.Create(baseDir + metricsLog)
+					_, _ = os.Create(baseDir + metricsLog + "." + dateStr + ".1")
+				},
+				clear: func() {
+					_ = os.RemoveAll(baseDir)
+				},
+			},
+			want:    baseDir + metricsLog,
+			wantErr: assert.NoError,
+		},
+		{
+			name: "have yesterday archive file and today archive file and today date file in baseDir,rotate it and create new file",
+			fields: fields{
+				baseDir:           baseDir,
+				baseFilename:      metricsLog,
+				maxSingleSize:     1024,
+				maxFileAmount:     10,
+				timezoneOffsetSec: int64(offset),
+				latestOpSec:       time.Date(2023, 12, 15, 10, 0, 0, 0, time.Local).Unix(),
+				curMetricFile:     nil,
+				curMetricIdxFile:  nil,
+				metricOut:         nil,
+				idxOut:            nil,
+				mux:               nil,
+			},
+			args: args{
+				time: uint64(time.Date(2023, 12, 15, 10, 1, 1, 0, time.Local).Unix() * 1000),
+				before: func() {
+					_ = os.RemoveAll(baseDir)
+					_ = os.MkdirAll(baseDir, os.FileMode(perm))
+				},
+				file: func() {
+					yesterday := time.Now().Add(-24*time.Hour).Unix() * 1000
+					dateStr := util.FormatDate(uint64(yesterday))
+					todayDateStr := util.FormatDate(uint64(time.Now().Unix() * 1000))
+					_, _ = os.Create(baseDir + metricsLog)
+					_, _ = os.Create(baseDir + metricsLog + "." + todayDateStr + ".1")
+					_, _ = os.Create(baseDir + metricsLog + "." + dateStr + ".1")
+				},
+				clear: func() {
+					_ = os.RemoveAll(baseDir)
+				},
+			},
+			want:    baseDir + metricsLog,
+			wantErr: assert.NoError,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			d := &DefaultMetricLogWriter{
+				baseDir:           tt.fields.baseDir,
+				baseFilename:      tt.fields.baseFilename,
+				maxSingleSize:     tt.fields.maxSingleSize,
+				maxFileAmount:     tt.fields.maxFileAmount,
+				timezoneOffsetSec: tt.fields.timezoneOffsetSec,
+				latestOpSec:       tt.fields.latestOpSec,
+				curMetricFile:     tt.fields.curMetricFile,
+				curMetricIdxFile:  tt.fields.curMetricIdxFile,
+				metricOut:         tt.fields.metricOut,
+				idxOut:            tt.fields.idxOut,
+				mux:               tt.fields.mux,
+			}
+			if tt.args.before != nil {
+				tt.args.before()
+			}
+			if tt.args.file != nil {
+				tt.args.file()
+			}
+			newFileName, err := d.rotateWithDateAndNewFileName(tt.args.time)
+			if !tt.wantErr(t, err, fmt.Sprintf("rotateWithDateAndNewFileName(%v)", tt.args.time)) {
+				return
+			}
+			_, _ = os.Create(newFileName)
+			if tt.args.clear != nil {
+				tt.args.clear()
+			}
+			assert.Equalf(t, tt.want, newFileName, "rotateWithDateAndNewFileName(%v)", tt.args.time)
+		})
+	}
+}
+
+func TestDefaultMetricLogWriter_isNewDay(t *testing.T) {
+	_, offset := util.Now().Zone()
+	fmt.Println("offset:", offset)
+	type fields struct {
+		baseDir           string
+		baseFilename      string
+		maxSingleSize     uint64
+		maxFileAmount     uint32
+		timezoneOffsetSec int64
+		latestOpSec       int64
+		curMetricFile     *os.File
+		curMetricIdxFile  *os.File
+		metricOut         *bufio.Writer
+		idxOut            *bufio.Writer
+		mux               *sync.RWMutex
+	}
+	type args struct {
+		lastSec int64
+		sec     int64
+	}
+	tests := []struct {
+		name   string
+		fields fields
+		args   args
+		want   bool
+	}{
+		{
+			name: "isNewDay true",
+			fields: fields{
+				baseDir:           "/tmp/logs/",
+				baseFilename:      "test-metrics.log",
+				maxSingleSize:     1024,
+				maxFileAmount:     10,
+				timezoneOffsetSec: int64(offset),
+				latestOpSec:       0,
+				curMetricFile:     nil,
+				curMetricIdxFile:  nil,
+				metricOut:         nil,
+				idxOut:            nil,
+				mux:               nil,
+			},
+
+			args: args{
+				lastSec: time.Date(2023, 12, 14, 23, 59, 59, 0, time.Local).Unix(),
+				sec:     time.Date(2023, 12, 15, 0, 0, 0, 0, time.Local).Unix(),
+			},
+			want: true,
+		},
+		{
+			name: "isNewDay false",
+			fields: fields{
+				baseDir:           "/tmp/logs/",
+				baseFilename:      "test-metrics.log",
+				maxSingleSize:     1024,
+				maxFileAmount:     10,
+				timezoneOffsetSec: int64(offset),
+				latestOpSec:       0,
+				curMetricFile:     nil,
+				curMetricIdxFile:  nil,
+				metricOut:         nil,
+				idxOut:            nil,
+				mux:               nil,
+			},
+			args: args{
+				lastSec: time.Date(2023, 12, 15, 0, 0, 0, 0, time.Local).Unix(),
+				sec:     time.Date(2023, 12, 15, 23, 59, 59, 0, time.Local).Unix(),
+			},
+			want: false,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			d := &DefaultMetricLogWriter{
+				baseDir:           tt.fields.baseDir,
+				baseFilename:      tt.fields.baseFilename,
+				maxSingleSize:     tt.fields.maxSingleSize,
+				maxFileAmount:     tt.fields.maxFileAmount,
+				timezoneOffsetSec: tt.fields.timezoneOffsetSec,
+				latestOpSec:       tt.fields.latestOpSec,
+				curMetricFile:     tt.fields.curMetricFile,
+				curMetricIdxFile:  tt.fields.curMetricIdxFile,
+				metricOut:         tt.fields.metricOut,
+				idxOut:            tt.fields.idxOut,
+				mux:               tt.fields.mux,
+			}
+			assert.Equalf(t, tt.want, d.isNewDay(tt.args.lastSec, tt.args.sec), "isNewDay(%v, %v)", tt.args.lastSec, tt.args.sec)
+		})
+	}
+}
diff --git a/util/file_test.go b/util/file_test.go
index 78f90ca..46320a9 100644
--- a/util/file_test.go
+++ b/util/file_test.go
@@ -15,6 +15,7 @@
 package util
 
 import (
+	"os"
 	"testing"
 )
 
@@ -51,3 +52,66 @@ func TestFileExists(t *testing.T) {
 		})
 	}
 }
+
+func TestFileRename(t *testing.T) {
+	type args struct {
+		newNameSuffix string
+		before        func() string
+		clean         func(tempName string)
+	}
+	oldName := "testfile"
+	newName := ".renamed"
+	tests := []struct {
+		name string
+		args args
+		want bool
+	}{
+		{
+			name: "rename success",
+			want: true,
+			args: args{
+				newNameSuffix: newName,
+				before: func() string {
+					oldFile, err := os.CreateTemp("", oldName)
+					if err != nil {
+						t.Fatalf("Failed to create temporary file: %s", err)
+					}
+					tempName := oldFile.Name()
+					return tempName
+				},
+				clean: func(tempName string) {
+					os.RemoveAll(tempName)
+				},
+			},
+		},
+		{
+			name: "rename fail",
+			want: false,
+			args: args{
+				newNameSuffix: newName,
+				before: func() string {
+					err := os.RemoveAll(oldName)
+					if err != nil {
+						t.Fatalf("Failed to remove temporary file: %s", err)
+					}
+					return ""
+				},
+				clean: func(tempName string) {
+
+				},
+			},
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			var oldFileName string
+			if tt.args.before != nil {
+				oldFileName = tt.args.before()
+			}
+			defer tt.args.clean(oldFileName)
+			if got := FileRename(oldFileName, oldFileName+tt.args.newNameSuffix); got != tt.want {
+				t.Errorf("FileRename() = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}