add configs for log rotation (#1505)

* add configs for log rotation

* review comments

* review comments

internal/config/mount_config.go

@@ -14,26 +14,58 @@
 
 package config
 
+const (
+	// Default log rotation config values.
+	defaultMaxFileSizeMB = 512
+	defaultFileCount     = 10
+	defaultCompress      = false
+)
+
 type WriteConfig struct {
 	CreateEmptyFile bool `yaml:"create-empty-file"`
 }
 
 type LogConfig struct {
-	Severity LogSeverity `yaml:"severity"`
-	Format   string      `yaml:"format"`
-	FilePath string      `yaml:"file-path"`
+	Severity        LogSeverity     `yaml:"severity"`
+	Format          string          `yaml:"format"`
+	FilePath        string          `yaml:"file-path"`
+	LogRotateConfig LogRotateConfig `yaml:"log-rotate"`
 }
 
 type MountConfig struct {
 	WriteConfig `yaml:"write"`
 	LogConfig   `yaml:"logging"`
 }
 
+// LogRotateConfig defines the parameters for log rotation. It consists of three
+// configuration options:
+// 1. max-file-size-mb: specifies the maximum size in megabytes that a log file
+// can reach before it is rotated. The default value is 512 megabytes.
+// 2. file-count: determines the maximum number of log files to retain after they
+// have been rotated. The default value is 10.
+// 3. compress: indicates whether the rotated log files should be compressed
+// using gzip. The default value is False.
+type LogRotateConfig struct {
+	MaxFileSizeMB int  `yaml:"max-file-size-mb"`
+	FileCount     int  `yaml:"file-count"`
+	Compress      bool `yaml:"compress"`
+}
+
+func DefaultLogRotateConfig() LogRotateConfig {
+	return LogRotateConfig{
+		MaxFileSizeMB: defaultMaxFileSizeMB,
+		FileCount:     defaultFileCount,
+		Compress:      defaultCompress,
+	}
+}
+
 func NewMountConfig() *MountConfig {
 	mountConfig := &MountConfig{}
 	mountConfig.LogConfig = LogConfig{
 		// Making the default severity as INFO.
 		Severity: INFO,
+		// Setting default values of log rotate config.
+		LogRotateConfig: DefaultLogRotateConfig(),
 	}
 	return mountConfig
 }

internal/config/testdata/invalid_log_rotate_config_1.yaml

@@ -0,0 +1,7 @@
+write:
+  create-empty-file: true
+logging:
+  severity: error
+  log-rotate:
+    max-file-size-mb: -1
+    file-count: -1

internal/config/testdata/invalid_log_rotate_config_2.yaml

@@ -0,0 +1,6 @@
+write:
+  create-empty-file: true
+logging:
+  severity: error
+  log-rotate:
+    file-count: -1

internal/config/testdata/valid_config.yaml

@@ -4,4 +4,9 @@ logging:
   file-path: /tmp/logfile.json
   format: text
   severity: error
+  log-rotate:
+    max-file-size-mb: 100
+    file-count: 5
+    compress: false
+
 

internal/config/yaml_parser.go

@@ -33,6 +33,8 @@ const (
 	WARNING LogSeverity = "WARNING"
 	ERROR   LogSeverity = "ERROR"
 	OFF     LogSeverity = "OFF"
+
+	parseConfigFileErrMsgFormat = "error parsing config file: %v"
 )
 
 func IsValidLogSeverity(severity LogSeverity) bool {
@@ -49,6 +51,16 @@ func IsValidLogSeverity(severity LogSeverity) bool {
 	return false
 }
 
+func IsValidLogRotateConfig(config LogRotateConfig) error {
+	if config.MaxFileSizeMB <= 0 {
+		return fmt.Errorf("max-file-size-mb should be atleast 1")
+	}
+	if config.FileCount <= 0 {
+		return fmt.Errorf("file-count should be atleast 1")
+	}
+	return nil
+}
+
 func ParseConfigFile(fileName string) (mountConfig *MountConfig, err error) {
 	mountConfig = NewMountConfig()
 
@@ -71,7 +83,7 @@ func ParseConfigFile(fileName string) (mountConfig *MountConfig, err error) {
 		if err == io.EOF {
 			return mountConfig, nil
 		}
-		return mountConfig, fmt.Errorf("error parsing config file: %w", err)
+		return mountConfig, fmt.Errorf(parseConfigFileErrMsgFormat, err)
 	}
 
 	// convert log severity to upper-case
@@ -81,5 +93,10 @@ func ParseConfigFile(fileName string) (mountConfig *MountConfig, err error) {
 		return
 	}
 
+	if err = IsValidLogRotateConfig(mountConfig.LogConfig.LogRotateConfig); err != nil {
+		err = fmt.Errorf(parseConfigFileErrMsgFormat, err)
+		return
+	}
+
 	return
 }

internal/config/yaml_parser_test.go

@@ -15,6 +15,7 @@
 package config
 
 import (
+	"fmt"
 	"strings"
 	"testing"
 
@@ -30,10 +31,13 @@ func init() { RegisterTestSuite(&YamlParserTest{}) }
 
 func validateDefaultConfig(mountConfig *MountConfig) {
 	AssertNe(nil, mountConfig)
-	AssertEq(false, mountConfig.CreateEmptyFile)
-	AssertEq("INFO", mountConfig.LogConfig.Severity)
-	AssertEq("", mountConfig.LogConfig.Format)
-	AssertEq("", mountConfig.LogConfig.FilePath)
+	ExpectEq(false, mountConfig.CreateEmptyFile)
+	ExpectEq("INFO", mountConfig.LogConfig.Severity)
+	ExpectEq("", mountConfig.LogConfig.Format)
+	ExpectEq("", mountConfig.LogConfig.FilePath)
+	ExpectEq(512, mountConfig.LogConfig.LogRotateConfig.MaxFileSizeMB)
+	ExpectEq(10, mountConfig.LogConfig.LogRotateConfig.FileCount)
+	ExpectEq(false, mountConfig.LogConfig.LogRotateConfig.Compress)
 }
 
 func (t *YamlParserTest) TestReadConfigFile_EmptyFileName() {
@@ -77,15 +81,35 @@ func (t *YamlParserTest) TestReadConfigFile_ValidConfig() {
 
 	AssertEq(nil, err)
 	AssertNe(nil, mountConfig)
-	AssertEq(true, mountConfig.WriteConfig.CreateEmptyFile)
-	AssertEq(ERROR, mountConfig.LogConfig.Severity)
-	AssertEq("/tmp/logfile.json", mountConfig.LogConfig.FilePath)
-	AssertEq("text", mountConfig.LogConfig.Format)
+	ExpectEq(true, mountConfig.WriteConfig.CreateEmptyFile)
+	ExpectEq(ERROR, mountConfig.LogConfig.Severity)
+	ExpectEq("/tmp/logfile.json", mountConfig.LogConfig.FilePath)
+	ExpectEq("text", mountConfig.LogConfig.Format)
+	ExpectEq(100, mountConfig.LogConfig.LogRotateConfig.MaxFileSizeMB)
+	ExpectEq(5, mountConfig.LogConfig.LogRotateConfig.FileCount)
+	ExpectEq(false, mountConfig.LogConfig.LogRotateConfig.Compress)
 }
 
-func (t *YamlParserTest) TestReadConfigFile_InvalidValidLogConfig() {
+func (t *YamlParserTest) TestReadConfigFile_InvalidLogConfig() {
 	_, err := ParseConfigFile("testdata/invalid_log_config.yaml")
 
 	AssertNe(nil, err)
-	AssertTrue(strings.Contains(err.Error(), "error parsing config file: log severity should be one of [trace, debug, info, warning, error, off]"))
+	AssertTrue(strings.Contains(err.Error(),
+		fmt.Sprintf(parseConfigFileErrMsgFormat, "log severity should be one of [trace, debug, info, warning, error, off]")))
+}
+
+func (t *YamlParserTest) TestReadConfigFile_InvalidLogRotateConfig1() {
+	_, err := ParseConfigFile("testdata/invalid_log_rotate_config_1.yaml")
+
+	AssertNe(nil, err)
+	AssertTrue(strings.Contains(err.Error(),
+		fmt.Sprintf(parseConfigFileErrMsgFormat, "max-file-size-mb should be atleast 1")))
+}
+
+func (t *YamlParserTest) TestReadConfigFile_InvalidLogRotateConfig2() {
+	_, err := ParseConfigFile("testdata/invalid_log_rotate_config_2.yaml")
+
+	AssertNe(nil, err)
+	AssertTrue(strings.Contains(err.Error(),
+		fmt.Sprintf(parseConfigFileErrMsgFormat, "file-count should be atleast 1")))
 }

internal/logger/logger.go

@@ -46,13 +46,13 @@ var (
 // config.
 // Here, background true means, this InitLogFile has been called for the
 // background daemon.
-func InitLogFile(filename string, format string, level config.LogSeverity) error {
+func InitLogFile(logConfig config.LogConfig) error {
 	var f *os.File
 	var sysWriter *syslog.Writer
 	var err error
-	if filename != "" {
+	if logConfig.FilePath != "" {
 		f, err = os.OpenFile(
-			filename,
+			logConfig.FilePath,
 			os.O_WRONLY|os.O_CREATE|os.O_APPEND,
 			0644,
 		)
@@ -75,21 +75,23 @@ func InitLogFile(filename string, format string, level config.LogSeverity) error
 	}
 
 	defaultLoggerFactory = &loggerFactory{
-		file:      f,
-		sysWriter: sysWriter,
-		format:    format,
-		level:     level,
+		file:            f,
+		sysWriter:       sysWriter,
+		format:          logConfig.Format,
+		level:           logConfig.Severity,
+		logRotateConfig: logConfig.LogRotateConfig,
 	}
-	defaultLogger = defaultLoggerFactory.newLogger(level)
+	defaultLogger = defaultLoggerFactory.newLogger(logConfig.Severity)
 
 	return nil
 }
 
 // init initializes the logger factory to use stdout and stderr.
 func init() {
 	defaultLoggerFactory = &loggerFactory{
-		file:  nil,
-		level: config.INFO, // setting log level to INFO by default
+		file:            nil,
+		level:           config.INFO, // setting log level to INFO by default
+		logRotateConfig: config.DefaultLogRotateConfig(),
 	}
 	defaultLogger = defaultLoggerFactory.newLogger(config.INFO)
 }
@@ -162,10 +164,11 @@ func Fatal(format string, v ...interface{}) {
 
 type loggerFactory struct {
 	// If nil, log to stdout or stderr. Otherwise, log to this file.
-	file      *os.File
-	sysWriter *syslog.Writer
-	format    string
-	level     config.LogSeverity
+	file            *os.File
+	sysWriter       *syslog.Writer
+	format          string
+	level           config.LogSeverity
+	logRotateConfig config.LogRotateConfig
 }
 
 func (f *loggerFactory) newLogger(level config.LogSeverity) *slog.Logger {

internal/logger/logger_test.go

@@ -17,6 +17,7 @@ package logger
 import (
 	"bytes"
 	"log/slog"
+	"os"
 	"strings"
 	"testing"
 
@@ -261,3 +262,32 @@ func (t *LoggerTest) TestSetLoggingLevel() {
 		AssertEq(test.programLevel.Level(), test.expectedProgramLevel)
 	}
 }
+
+func (t *LoggerTest) TestInitLogFile() {
+	format := "text"
+	filePath, _ := os.UserHomeDir()
+	filePath += "/log.txt"
+	fileSize := 100
+	fileCount := 2
+	logConfig := config.LogConfig{
+		Severity: config.DEBUG,
+		Format:   format,
+		FilePath: filePath,
+		LogRotateConfig: config.LogRotateConfig{
+			MaxFileSizeMB: fileSize,
+			FileCount:     fileCount,
+			Compress:      true,
+		},
+	}
+
+	err := InitLogFile(logConfig)
+
+	AssertEq(nil, err)
+	ExpectEq(filePath, defaultLoggerFactory.file.Name())
+	ExpectEq(nil, defaultLoggerFactory.sysWriter)
+	ExpectEq(format, defaultLoggerFactory.format)
+	ExpectEq(config.DEBUG, defaultLoggerFactory.level)
+	ExpectEq(fileSize, defaultLoggerFactory.logRotateConfig.MaxFileSizeMB)
+	ExpectEq(fileCount, defaultLoggerFactory.logRotateConfig.FileCount)
+	ExpectEq(true, defaultLoggerFactory.logRotateConfig.Compress)
+}

main.go

@@ -210,7 +210,7 @@ func runCLIApp(c *cli.Context) (err error) {
 	}
 
 	if flags.Foreground {
-		err = logger.InitLogFile(mountConfig.LogConfig.FilePath, mountConfig.LogConfig.Format, mountConfig.LogConfig.Severity)
+		err = logger.InitLogFile(mountConfig.LogConfig)
 		if err != nil {
 			return fmt.Errorf("init log file: %w", err)
 		}