Repository: zeromicro/go-zero
Pull request #2181 (2022-07-22 14:14:51)

chore: refactoring logx


Patch:
diff --git a/core/logx/config.go b/core/logx/config.go
index 0e42dd9f..eb79f4ee 100644
--- a/core/logx/config.go
+++ b/core/logx/config.go
@@ -1,12 +1,5 @@
 package logx
 
-type LogRotationRuleType int
-
-const (
-	LogRotationRuleTypeDaily LogRotationRuleType = iota
-	LogRotationRuleTypeSizeLimit
-)
-
 // A LogConf is a logging config.
 type LogConf struct {
 	ServiceName         string `json:",optional"`
@@ -19,15 +12,15 @@ type LogConf struct {
 	KeepDays            int    `json:",optional"`
 	StackCooldownMillis int    `json:",default=100"`
 	// MaxBackups represents how many backup log files will be kept. 0 means all files will be kept forever.
-	// Only take effect when RotationRuleType is `LogRotationRuleTypeSizeLimit`
-	// NOTE: the level of option `KeepDays` will be higher. Even thougth `MaxBackups` sets 0, log files will
-	// still be removed if the `KeepDays` limitation is reached.
+	// Only take effect when RotationRuleType is `size`.
+	// Even thougth `MaxBackups` sets 0, log files will still be removed
+	// if the `KeepDays` limitation is reached.
 	MaxBackups int `json:",default=0"`
 	// MaxSize represents how much space the writing log file takes up. 0 means no limit. The unit is `MB`.
-	// Only take effect when RotationRuleType is `LogRotationRuleTypeSizeLimit`
+	// Only take effect when RotationRuleType is `size`
 	MaxSize int `json:",default=0"`
-	// RotationRuleType represents the type of log rotation rule. Default is DailyRotateRule.
-	// 0: LogRotationRuleTypeDaily
-	// 1: LogRotationRuleTypeSizeLimit
-	RotationRuleType LogRotationRuleType `json:",default=0,options=[0,1]"`
+	// RotationRuleType represents the type of log rotation rule. Default is `daily`.
+	// daily: daily rotation.
+	// size: size limited rotation.
+	Rotation string `json:",default=daily,options=[daily,size]"`
 }
diff --git a/core/logx/logs.go b/core/logx/logs.go
index ea550e73..a9116806 100644
--- a/core/logx/logs.go
+++ b/core/logx/logs.go
@@ -21,9 +21,8 @@ var (
 	encoding   uint32 = jsonEncodingType
 	// use uint32 for atomic operations
 	disableStat uint32
-
-	options logOptions
-	writer  = new(atomicWriter)
+	options     logOptions
+	writer      = new(atomicWriter)
 )
 
 type (
@@ -43,7 +42,7 @@ type (
 		keepDays              int
 		maxBackups            int
 		maxSize               int
-		rotationRule          LogRotationRuleType
+		rotationRule          string
 	}
 
 	// LogField is a key-value pair that will be added to the log entry.
@@ -311,8 +310,8 @@ func WithMaxSize(size int) LogOption {
 	}
 }
 
-// WithLogRotationRuleType customizes which log rotation rule to use.
-func WithLogRotationRuleType(r LogRotationRuleType) LogOption {
+// WithRotation customizes which log rotation rule to use.
+func WithRotation(r string) LogOption {
 	return func(opts *logOptions) {
 		opts.rotationRule = r
 	}
@@ -324,10 +323,7 @@ func createOutput(path string) (io.WriteCloser, error) {
 	}
 
 	switch options.rotationRule {
-	case LogRotationRuleTypeDaily:
-		return NewLogger(path, DefaultRotateRule(path, backupFileDelimiter, options.keepDays,
-			options.gzipEnabled), options.gzipEnabled)
-	case LogRotationRuleTypeSizeLimit:
+	case sizeRotationRule:
 		return NewLogger(path, NewSizeLimitRotateRule(path, backupFileDelimiter, options.keepDays,
 			options.maxSize, options.maxBackups, options.gzipEnabled), options.gzipEnabled)
 	default:
diff --git a/core/logx/readme-cn.md b/core/logx/readme-cn.md
index f7cd73d7..c8087b92 100644
--- a/core/logx/readme-cn.md
+++ b/core/logx/readme-cn.md
@@ -19,7 +19,7 @@ type LogConf struct {
 	StackCooldownMillis int                 `json:",default=100"`
 	MaxBackups          int                 `json:",default=0"`
 	MaxSize             int                 `json:",default=0"`
-	RotationRuleType    LogRotationRuleType `json:",default=0,options=[0,1]"`
+	Rotation            string              `json:",default=daily,options=[daily,size]"`
 }
 ```
 
@@ -40,11 +40,11 @@ type LogConf struct {
 - `Compress`: 是否压缩日志文件，只在 `file` 模式下工作
 - `KeepDays`：日志文件被保留多少天，在给定的天数之后，过期的文件将被自动删除。对 `console` 模式没有影响
 - `StackCooldownMillis`：多少毫秒后再次写入堆栈跟踪。用来避免堆栈跟踪日志过多
-- `MaxBackups`: 多少个日志文件备份将被保存。0代表所有备份都被保存。当`RotationRuleType`被设置为`LogRotationRuleTypeSizeLimit`时才会起作用。注意：`KeepDays`选项的优先级会比`MaxBackups`高，即使`MaxBackups`被设置为0，当达到`KeepDays`上限时备份文件同样会被删除。
-- `MaxSize`: 当前被写入的日志文件最大可占用多少空间。0代表没有上限。单位为`MB`。当`RotationRuleType`被设置为`LogRotationRuleTypeSizeLimit`时才会起作用。
-- `RotationRuleType`: 日志轮转策略类型。默认为`LogRotationRuleTypeDaily`（按天轮转）（整形数值0）。
-  - `LogRotationRuleTypeDaily`（整形数值0）: 按天轮转。
-  - `LogRotationRuleTypeSizeLimit`（整形数值1）: 按日志大小轮转。
+- `MaxBackups`: 多少个日志文件备份将被保存。0代表所有备份都被保存。当`Rotation`被设置为`size`时才会起作用。注意：`KeepDays`选项的优先级会比`MaxBackups`高，即使`MaxBackups`被设置为0，当达到`KeepDays`上限时备份文件同样会被删除。
+- `MaxSize`: 当前被写入的日志文件最大可占用多少空间。0代表没有上限。单位为`MB`。当`Rotation`被设置为`size`时才会起作用。
+- `Rotation`: 日志轮转策略类型。默认为`daily`（按天轮转）。
+  - `daily` 按天轮转。
+  - `size` 按日志大小轮转。
 
 
 ## 打印日志方法
diff --git a/core/logx/readme.md b/core/logx/readme.md
index 1cd078d7..5949cad8 100644
--- a/core/logx/readme.md
+++ b/core/logx/readme.md
@@ -19,7 +19,7 @@ type LogConf struct {
 	StackCooldownMillis int                 `json:",default=100"`
 	MaxBackups          int                 `json:",default=0"`
 	MaxSize             int                 `json:",default=0"`
-	RotationRuleType    LogRotationRuleType `json:",default=0,options=[0,1]"`
+	Rotation            string              `json:",default=daily,options=[daily,size]"`
 }
 ```
 
@@ -40,11 +40,11 @@ type LogConf struct {
 - `Compress`: whether or not to compress log files, only works with `file` mode.
 - `KeepDays`: how many days that the log files are kept, after the given days, the outdated files will be deleted automatically. It has no effect on `console` mode.
 - `StackCooldownMillis`: how many milliseconds to rewrite stacktrace again. It’s used to avoid stacktrace flooding.
-- `MaxBackups`: represents how many backup log files will be kept. 0 means all files will be kept forever. Only take effect when RotationRuleType is `LogRotationRuleTypeSizeLimit`. NOTE: the level of option `KeepDays` will be higher. Even thougth `MaxBackups` sets 0, log files will still be removed if the `KeepDays` limitation is reached.
-- `MaxSize`: represents how much space the writing log file takes up. 0 means no limit. The unit is `MB`. Only take effect when RotationRuleType is `LogRotationRuleTypeSizeLimit`.
-- `RotationRuleType`: represents the type of log rotation rule. Default is LogRotationRuleTypeDaily (int value 0).
-  - `LogRotationRuleTypeDaily` (int value 0): rotate the logs by day.
-  - `LogRotationRuleTypeSizeLimit` (int value 1): rotate the logs by size of logs.
+- `MaxBackups`: represents how many backup log files will be kept. 0 means all files will be kept forever. Only take effect when `Rotation` is `size`. NOTE: the level of option `KeepDays` will be higher. Even thougth `MaxBackups` sets 0, log files will still be removed if the `KeepDays` limitation is reached.
+- `MaxSize`: represents how much space the writing log file takes up. 0 means no limit. The unit is `MB`. Only take effect when `Rotation` is `size`.
+- `Rotation`: represents the type of log rotation rule. Default is `daily`.
+  - `daily` rotate the logs by day.
+  - `size` rotate the logs by size of logs.
 
 ## Logging methods
 
diff --git a/core/logx/rotatelogger.go b/core/logx/rotatelogger.go
index 8330e96f..39798f51 100644
--- a/core/logx/rotatelogger.go
+++ b/core/logx/rotatelogger.go
@@ -26,7 +26,7 @@ const (
 	defaultDirMode    = 0o755
 	defaultFileMode   = 0o600
 	gzipExt           = ".gz"
-	megabyte          = 1024 * 1024
+	megaBytes         = 1 << 20
 )
 
 // ErrLogFileClosed is an error that indicates the log file is already closed.
@@ -38,7 +38,7 @@ type (
 		BackupFileName() string
 		MarkRotated()
 		OutdatedFiles() []string
-		ShallRotate(currentSize, writeLen int) bool
+		ShallRotate(size int64) bool
 	}
 
 	// A RotateLogger is a Logger that can rotate log files with given rules.
@@ -51,10 +51,9 @@ type (
 		rule     RotateRule
 		compress bool
 		// can't use threading.RoutineGroup because of cycle import
-		waitGroup sync.WaitGroup
-		closeOnce sync.Once
-
-		currentSize int
+		waitGroup   sync.WaitGroup
+		closeOnce   sync.Once
+		currentSize int64
 	}
 
 	// A DailyRotateRule is a rule to daily rotate the log files.
@@ -69,7 +68,7 @@ type (
 	// SizeLimitRotateRule a rotation rule that make the log file rotated base on size
 	SizeLimitRotateRule struct {
 		DailyRotateRule
-		maxSize    int
+		maxSize    int64
 		maxBackups int
 	}
 )
@@ -133,7 +132,7 @@ func (r *DailyRotateRule) OutdatedFiles() []string {
 }
 
 // ShallRotate checks if the file should be rotated.
-func (r *DailyRotateRule) ShallRotate(currentSize, writeLen int) bool {
+func (r *DailyRotateRule) ShallRotate(_ int64) bool {
 	return len(r.rotatedTime) > 0 && getNowDate() != r.rotatedTime
 }
 
@@ -147,26 +146,14 @@ func NewSizeLimitRotateRule(filename, delimiter string, days, maxSize, maxBackup
 			days:        days,
 			gzip:        gzip,
 		},
-		maxSize:    maxSize,
+		maxSize:    int64(maxSize) * megaBytes,
 		maxBackups: maxBackups,
 	}
 }
 
-func (r *SizeLimitRotateRule) ShallRotate(currentSize, writeLen int) bool {
-	return r.maxSize > 0 && r.maxSize*megabyte < currentSize+writeLen
-}
-
-func (r *SizeLimitRotateRule) parseFilename(file string) (dir, logname, ext, prefix string) {
-	dir = filepath.Dir(r.filename)
-	logname = filepath.Base(r.filename)
-	ext = filepath.Ext(r.filename)
-	prefix = logname[:len(logname)-len(ext)]
-	return
-}
-
 func (r *SizeLimitRotateRule) BackupFileName() string {
 	dir := filepath.Dir(r.filename)
-	_, _, ext, prefix := r.parseFilename(r.filename)
+	prefix, ext := r.parseFilename()
 	timestamp := getNowDateInRFC3339Format()
 	return filepath.Join(dir, fmt.Sprintf("%s%s%s%s", prefix, r.delimiter, timestamp, ext))
 }
@@ -176,17 +163,20 @@ func (r *SizeLimitRotateRule) MarkRotated() {
 }
 
 func (r *SizeLimitRotateRule) OutdatedFiles() []string {
+	dir := filepath.Dir(r.filename)
+	prefix, ext := r.parseFilename()
+
 	var pattern string
-	dir, _, ext, prefix := r.parseFilename(r.filename)
 	if r.gzip {
-		pattern = fmt.Sprintf("%s%s%s%s*%s%s", dir, string(filepath.Separator), prefix, r.delimiter, ext, gzipExt)
+		pattern = fmt.Sprintf("%s%s%s%s*%s%s", dir, string(filepath.Separator),
+			prefix, r.delimiter, ext, gzipExt)
 	} else {
-		pattern = fmt.Sprintf("%s%s%s%s*%s", dir, string(filepath.Separator), prefix, r.delimiter, ext)
+		pattern = fmt.Sprintf("%s%s%s%s*%s", dir, string(filepath.Separator),
+			prefix, r.delimiter, ext)
 	}
 
 	files, err := filepath.Glob(pattern)
 	if err != nil {
-		fmt.Printf("failed to delete outdated log files, error: %s\n", err)
 		Errorf("failed to delete outdated log files, error: %s", err)
 		return nil
 	}
@@ -206,17 +196,15 @@ func (r *SizeLimitRotateRule) OutdatedFiles() []string {
 	// test if any too old backups
 	if r.days > 0 {
 		boundary := time.Now().Add(-time.Hour * time.Duration(hoursPerDay*r.days)).Format(rfc3339DateFormat)
-		bf := filepath.Join(dir, fmt.Sprintf("%s%s%s%s", prefix, r.delimiter, boundary, ext))
+		boundaryFile := filepath.Join(dir, fmt.Sprintf("%s%s%s%s", prefix, r.delimiter, boundary, ext))
 		if r.gzip {
-			bf += gzipExt
+			boundaryFile += gzipExt
 		}
 		for _, f := range files {
-			if f < bf {
-				outdated[f] = lang.Placeholder
-			} else {
-				// Becase the filenames are sorted. No need to keep looping after the first ineligible item showing up.
+			if f >= boundaryFile {
 				break
 			}
+			outdated[f] = lang.Placeholder
 		}
 	}
 
@@ -227,6 +215,17 @@ func (r *SizeLimitRotateRule) OutdatedFiles() []string {
 	return result
 }
 
+func (r *SizeLimitRotateRule) ShallRotate(size int64) bool {
+	return r.maxSize > 0 && r.maxSize < size
+}
+
+func (r *SizeLimitRotateRule) parseFilename() (prefix, ext string) {
+	logName := filepath.Base(r.filename)
+	ext = filepath.Ext(r.filename)
+	prefix = logName[:len(logName)-len(ext)]
+	return
+}
+
 // NewLogger returns a RotateLogger with given filename and rule, etc.
 func NewLogger(filename string, rule RotateRule, compress bool) (*RotateLogger, error) {
 	l := &RotateLogger{
@@ -385,7 +384,7 @@ func (l *RotateLogger) startWorker() {
 }
 
 func (l *RotateLogger) write(v []byte) {
-	if l.rule.ShallRotate(l.currentSize, len(v)) {
+	if l.rule.ShallRotate(l.currentSize + int64(len(v))) {
 		if err := l.rotate(); err != nil {
 			log.Println(err)
 		} else {
@@ -395,7 +394,7 @@ func (l *RotateLogger) write(v []byte) {
 	}
 	if l.fp != nil {
 		l.fp.Write(v)
-		l.currentSize += len(v)
+		l.currentSize += int64(len(v))
 	}
 }
 
diff --git a/core/logx/vars.go b/core/logx/vars.go
index e9f6772a..26c4fd84 100644
--- a/core/logx/vars.go
+++ b/core/logx/vars.go
@@ -15,9 +15,9 @@ const (
 	jsonEncodingType = iota
 	plainEncodingType
 
-	jsonEncoding     = "json"
 	plainEncoding    = "plain"
 	plainEncodingSep = '\t'
+	sizeRotationRule = "size"
 )
 
 const (
@@ -27,9 +27,8 @@ const (
 	slowFilename   = "slow.log"
 	statFilename   = "stat.log"
 
-	consoleMode = "console"
-	fileMode    = "file"
-	volumeMode  = "volume"
+	fileMode   = "file"
+	volumeMode = "volume"
 
 	levelAlert  = "alert"
 	levelInfo   = "info"
diff --git a/core/logx/writer.go b/core/logx/writer.go
index 0380bbe4..485736d4 100644
--- a/core/logx/writer.go
+++ b/core/logx/writer.go
@@ -116,7 +116,7 @@ func newFileWriter(c LogConf) (Writer, error) {
 		opts = append(opts, WithMaxSize(c.MaxSize))
 	}
 
-	opts = append(opts, WithLogRotationRuleType(c.RotationRuleType))
+	opts = append(opts, WithRotation(c.Rotation))
 
 	accessFile := path.Join(c.Path, accessFilename)
 	errorFile := path.Join(c.Path, errorFilename)


Test patch:
diff --git a/core/logx/rotatelogger_test.go b/core/logx/rotatelogger_test.go
index aa9bebaf..75865c09 100644
--- a/core/logx/rotatelogger_test.go
+++ b/core/logx/rotatelogger_test.go
@@ -29,7 +29,7 @@ func TestDailyRotateRuleOutdatedFiles(t *testing.T) {
 func TestDailyRotateRuleShallRotate(t *testing.T) {
 	var rule DailyRotateRule
 	rule.rotatedTime = time.Now().Add(time.Hour * 24).Format(dateFormat)
-	assert.True(t, rule.ShallRotate(0, 0))
+	assert.True(t, rule.ShallRotate(0))
 }
 
 func TestSizeLimitRotateRuleMarkRotated(t *testing.T) {
@@ -53,10 +53,10 @@ func TestSizeLimitRotateRuleShallRotate(t *testing.T) {
 	var rule SizeLimitRotateRule
 	rule.rotatedTime = time.Now().Add(time.Hour * 24).Format(rfc3339DateFormat)
 	rule.maxSize = 0
-	assert.False(t, rule.ShallRotate(0, 0))
+	assert.False(t, rule.ShallRotate(0))
 	rule.maxSize = 100
-	assert.False(t, rule.ShallRotate(0, 0))
-	assert.True(t, rule.ShallRotate(99*megabyte, 2*megabyte))
+	assert.False(t, rule.ShallRotate(0))
+	assert.True(t, rule.ShallRotate(101*megaBytes))
 }
 
 func TestRotateLoggerClose(t *testing.T) {