Skip to content

Commit

Permalink
migrate log file flag
Browse files Browse the repository at this point in the history
  • Loading branch information
ashmeenkaur committed Jun 25, 2024
1 parent fc0082b commit e92fa74
Show file tree
Hide file tree
Showing 6 changed files with 23 additions and 37 deletions.
17 changes: 2 additions & 15 deletions cmd/flags_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -274,23 +274,18 @@ func (t *FlagsTest) TestResolvePathForTheFlagInContext() {
currentWorkingDir, err := os.Getwd()
assert.Equal(t.T(), nil, err)
app.Action = func(appCtx *cli.Context) {
err = resolvePathForTheFlagInContext("log-file", appCtx)
assert.Equal(t.T(), nil, err)
err = resolvePathForTheFlagInContext("key-file", appCtx)
assert.Equal(t.T(), nil, err)
err = resolvePathForTheFlagInContext("config-file", appCtx)
assert.Equal(t.T(), nil, err)

assert.Equal(t.T(), filepath.Join(currentWorkingDir, "test.txt"),
appCtx.String("log-file"))
assert.Equal(t.T(), filepath.Join(currentWorkingDir, "test.txt"),
appCtx.String("key-file"))
assert.Equal(t.T(), filepath.Join(currentWorkingDir, "config.yaml"),
appCtx.String("config-file"))
}
// Simulate argv.
fullArgs := []string{"some_app", "--log-file=test.txt",
"--key-file=test.txt", "--config-file=config.yaml"}
fullArgs := []string{"some_app", "--key-file=test.txt", "--config-file=config.yaml"}

err = app.Run(fullArgs)

Expand All @@ -304,16 +299,13 @@ func (t *FlagsTest) TestResolvePathForTheFlagsInContext() {
app.Action = func(appCtx *cli.Context) {
resolvePathForTheFlagsInContext(appCtx)

assert.Equal(t.T(), filepath.Join(currentWorkingDir, "test.txt"),
appCtx.String("log-file"))
assert.Equal(t.T(), filepath.Join(currentWorkingDir, "test.txt"),
appCtx.String("key-file"))
assert.Equal(t.T(), filepath.Join(currentWorkingDir, "config.yaml"),
appCtx.String("config-file"))
}
// Simulate argv.
fullArgs := []string{"some_app", "--log-file=test.txt",
"--key-file=test.txt", "--config-file=config.yaml"}
fullArgs := []string{"some_app", "--key-file=test.txt", "--config-file=config.yaml"}

err = app.Run(fullArgs)

Expand Down Expand Up @@ -421,17 +413,13 @@ func (t *FlagsTest) TestValidateFlagsForUnsupportedExperimentalMetadataPrefetchO

func (t *FlagsTest) Test_resolveConfigFilePaths() {
mountConfig := &config.MountConfig{}
mountConfig.LogConfig = config.LogConfig{
FilePath: "~/test.txt",
}
mountConfig.CacheDir = "~/cache-dir"

err := resolveConfigFilePaths(mountConfig)

assert.Equal(t.T(), nil, err)
homeDir, err := os.UserHomeDir()
assert.Equal(t.T(), nil, err)
assert.Equal(t.T(), filepath.Join(homeDir, "test.txt"), mountConfig.LogConfig.FilePath)
assert.EqualValues(t.T(), filepath.Join(homeDir, "cache-dir"), mountConfig.CacheDir)
}

Expand All @@ -441,7 +429,6 @@ func (t *FlagsTest) Test_resolveConfigFilePaths_WithoutSettingPaths() {
err := resolveConfigFilePaths(mountConfig)

assert.Equal(t.T(), nil, err)
assert.Equal(t.T(), "", mountConfig.LogConfig.FilePath)
assert.EqualValues(t.T(), "", mountConfig.CacheDir)
}

Expand Down
2 changes: 1 addition & 1 deletion cmd/legacy_main.go
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@ func runCLIApp(c *cli.Context) (err error) {
}

if flags.Foreground {
err = logger.InitLogFile(mountConfig.LogConfig)
err = logger.InitLogFile(mountConfig.LogConfig, newConfig.Logging)
if err != nil {
return fmt.Errorf("init log file: %w", err)
}
Expand Down
5 changes: 3 additions & 2 deletions cmd/legacy_param_converter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ func (m *MockContext) IsSet(name string) bool {
}

func TestPopulateConfigFromLegacyFlags(t *testing.T) {
currentWorkingDir, _ := os.Getwd()
var populateConfigFromLegacyFlags = []struct {
testName string
legacyFlagStorage *flagStorage
Expand Down Expand Up @@ -179,7 +180,7 @@ func TestPopulateConfigFromLegacyFlags(t *testing.T) {
LogConfig: config.LogConfig{
Severity: "info",
Format: "text",
FilePath: "~/Documents/log-file.txt",
FilePath: "log-file.txt",
LogRotateConfig: config.LogRotateConfig{
MaxFileSizeMB: 20,
BackupFileCount: 2,
Expand Down Expand Up @@ -218,7 +219,7 @@ func TestPopulateConfigFromLegacyFlags(t *testing.T) {
Logging: cfg.LoggingConfig{
Severity: "INFO",
Format: "text",
FilePath: cfg.ResolvedPath(path.Join(os.Getenv("HOME"), "Documents/log-file.txt")),
FilePath: cfg.ResolvedPath(path.Join(currentWorkingDir, "log-file.txt")),
LogRotate: cfg.LogRotateLoggingConfig{
MaxFileSizeMb: 20,
BackupFileCount: 2,
Expand Down
8 changes: 2 additions & 6 deletions internal/config/config_util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,17 +59,15 @@ func (t *ConfigTest) TestOverrideLoggingFlags_WithNonEmptyLogConfigs() {
mountConfig := &MountConfig{}
mountConfig.LogConfig = LogConfig{
Severity: ERROR,
FilePath: "/tmp/hello.txt",
Format: "text",
}
mountConfig.WriteConfig = WriteConfig{
CreateEmptyFile: true,
}

OverrideWithLoggingFlags(mountConfig, f.LogFile, f.LogFormat, f.DebugFuse, f.DebugGCS, f.DebugMutex)
OverrideWithLoggingFlags(mountConfig, f.LogFormat, f.DebugFuse, f.DebugGCS, f.DebugMutex)

assert.Equal(t.T(), "text", mountConfig.LogConfig.Format)
assert.Equal(t.T(), "/tmp/hello.txt", mountConfig.LogConfig.FilePath)
assert.Equal(t.T(), TRACE, mountConfig.LogConfig.Severity)
}

Expand All @@ -81,17 +79,15 @@ func (t *ConfigTest) TestOverrideLoggingFlags_WithEmptyLogConfigs() {
mountConfig := &MountConfig{}
mountConfig.LogConfig = LogConfig{
Severity: INFO,
FilePath: "",
Format: "",
}
mountConfig.WriteConfig = WriteConfig{
CreateEmptyFile: true,
}

OverrideWithLoggingFlags(mountConfig, f.LogFile, f.LogFormat, f.DebugFuse, f.DebugGCS, f.DebugMutex)
OverrideWithLoggingFlags(mountConfig, f.LogFormat, f.DebugFuse, f.DebugGCS, f.DebugMutex)

assert.Equal(t.T(), "json", mountConfig.LogConfig.Format)
assert.Equal(t.T(), "a.txt", mountConfig.LogConfig.FilePath)
assert.Equal(t.T(), INFO, mountConfig.LogConfig.Severity)
}

Expand Down
21 changes: 11 additions & 10 deletions internal/logger/logger.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"os"
"runtime/debug"

"github.com/googlecloudplatform/gcsfuse/v2/cfg"
"github.com/googlecloudplatform/gcsfuse/v2/internal/config"
"gopkg.in/natefinch/lumberjack.v2"
)
Expand Down Expand Up @@ -51,14 +52,14 @@ var (
// config.
// Here, background true means, this InitLogFile has been called for the
// background daemon.
func InitLogFile(logConfig config.LogConfig) error {
func InitLogFile(legacyLogConfig config.LogConfig, newLogConfig cfg.LoggingConfig) error {
var f *os.File
var sysWriter *syslog.Writer
var fileWriter *lumberjack.Logger
var err error
if logConfig.FilePath != "" {
if newLogConfig.FilePath != "" {
f, err = os.OpenFile(
logConfig.FilePath,
string(newLogConfig.FilePath),
os.O_WRONLY|os.O_CREATE|os.O_APPEND,
0644,
)
Expand All @@ -67,9 +68,9 @@ func InitLogFile(logConfig config.LogConfig) error {
}
fileWriter = &lumberjack.Logger{
Filename: f.Name(),
MaxSize: logConfig.LogRotateConfig.MaxFileSizeMB,
MaxBackups: logConfig.LogRotateConfig.BackupFileCount,
Compress: logConfig.LogRotateConfig.Compress,
MaxSize: legacyLogConfig.LogRotateConfig.MaxFileSizeMB,
MaxBackups: legacyLogConfig.LogRotateConfig.BackupFileCount,
Compress: legacyLogConfig.LogRotateConfig.Compress,
}
} else {
if _, ok := os.LookupEnv(GCSFuseInBackgroundMode); ok {
Expand All @@ -90,11 +91,11 @@ func InitLogFile(logConfig config.LogConfig) error {
file: f,
sysWriter: sysWriter,
fileWriter: fileWriter,
format: logConfig.Format,
level: logConfig.Severity,
logRotateConfig: logConfig.LogRotateConfig,
format: legacyLogConfig.Format,
level: legacyLogConfig.Severity,
logRotateConfig: legacyLogConfig.LogRotateConfig,
}
defaultLogger = defaultLoggerFactory.newLogger(logConfig.Severity)
defaultLogger = defaultLoggerFactory.newLogger(legacyLogConfig.Severity)

return nil
}
Expand Down
7 changes: 4 additions & 3 deletions internal/logger/logger_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"regexp"
"testing"

"github.com/googlecloudplatform/gcsfuse/v2/cfg"
"github.com/googlecloudplatform/gcsfuse/v2/internal/config"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/suite"
Expand Down Expand Up @@ -274,18 +275,18 @@ func (t *LoggerTest) TestInitLogFile() {
filePath += "/log.txt"
fileSize := 100
backupFileCount := 2
logConfig := config.LogConfig{
legacyLogConfig := config.LogConfig{
Severity: config.DEBUG,
Format: format,
FilePath: filePath,
LogRotateConfig: config.LogRotateConfig{
MaxFileSizeMB: fileSize,
BackupFileCount: backupFileCount,
Compress: true,
},
}
newLogConfig := cfg.LoggingConfig{FilePath: cfg.ResolvedPath(filePath)}

err := InitLogFile(logConfig)
err := InitLogFile(legacyLogConfig, newLogConfig)

assert.NoError(t.T(), err)
assert.Equal(t.T(), filePath, defaultLoggerFactory.file.Name())
Expand Down

0 comments on commit e92fa74

Please sign in to comment.