Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add configs for log rotation #1505

Merged
merged 3 commits into from
Nov 17, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
38 changes: 35 additions & 3 deletions internal/config/mount_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
raj-prince marked this conversation as resolved.
Show resolved Hide resolved
// 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
}
7 changes: 7 additions & 0 deletions internal/config/testdata/invalid_log_rotate_config_1.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
write:
create-empty-file: true
logging:
severity: error
log-rotate:
max-file-size-mb: -1
file-count: -1
raj-prince marked this conversation as resolved.
Show resolved Hide resolved
6 changes: 6 additions & 0 deletions internal/config/testdata/invalid_log_rotate_config_2.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
write:
create-empty-file: true
logging:
severity: error
log-rotate:
file-count: -1
5 changes: 5 additions & 0 deletions internal/config/testdata/valid_config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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


19 changes: 18 additions & 1 deletion internal/config/yaml_parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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()

Expand All @@ -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
Expand All @@ -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
}
44 changes: 34 additions & 10 deletions internal/config/yaml_parser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package config

import (
"fmt"
"strings"
"testing"

Expand All @@ -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() {
Expand Down Expand Up @@ -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")))
}
31 changes: 17 additions & 14 deletions internal/logger/logger.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
Expand All @@ -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)
}
Expand Down Expand Up @@ -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 {
Expand Down
30 changes: 30 additions & 0 deletions internal/logger/logger_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package logger
import (
"bytes"
"log/slog"
"os"
"strings"
"testing"

Expand Down Expand Up @@ -261,3 +262,32 @@ func (t *LoggerTest) TestSetLoggingLevel() {
AssertEq(test.programLevel.Level(), test.expectedProgramLevel)
}
}

func (t *LoggerTest) TestInitLogFile() {
ashmeenkaur marked this conversation as resolved.
Show resolved Hide resolved
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)
AssertEq(filePath, defaultLoggerFactory.file.Name())
AssertEq(nil, defaultLoggerFactory.sysWriter)
AssertEq(format, defaultLoggerFactory.format)
AssertEq(config.DEBUG, defaultLoggerFactory.level)
AssertEq(fileSize, defaultLoggerFactory.logRotateConfig.MaxFileSizeMB)
AssertEq(fileCount, defaultLoggerFactory.logRotateConfig.FileCount)
AssertEq(true, defaultLoggerFactory.logRotateConfig.Compress)
}
2 changes: 1 addition & 1 deletion main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down