From 7efe407f2e85d2ac6271873d7424ec0f5d28e857 Mon Sep 17 00:00:00 2001 From: Injun Song Date: Tue, 16 May 2023 23:09:15 +0900 Subject: [PATCH] feat(all): add flags for logger This PR adds common flags for logging to package `internal/flags`. Those flags include the followings: - `--logdir` - `--logtostderr` - `--logfile-max-backups` - `--logfile-retention-days` - `--logfile-max-size-mb` - `--logfile-name-utc` - `--logfile-compression` - `--log-human-readable` - `--loglevel` Those are very common. Hence all daemons can use them. Resolves #439 --- cmd/varlogadm/cli.go | 57 ++++------ cmd/varlogadm/flags.go | 26 ----- cmd/varlogmr/flags.go | 7 -- cmd/varlogmr/metadata_repository.go | 29 ++++-- cmd/varlogsn/cli.go | 16 ++- cmd/varlogsn/flags.go | 26 ----- cmd/varlogsn/varlogsn.go | 25 +---- internal/flags/logger.go | 156 ++++++++++++++++++++++++++++ internal/metarepos/config.go | 5 +- 9 files changed, 212 insertions(+), 135 deletions(-) create mode 100644 internal/flags/logger.go diff --git a/cmd/varlogadm/cli.go b/cmd/varlogadm/cli.go index a9edb2218..933627905 100644 --- a/cmd/varlogadm/cli.go +++ b/cmd/varlogadm/cli.go @@ -2,16 +2,15 @@ package main import ( "context" - "path/filepath" "github.com/urfave/cli/v2" "go.uber.org/zap" - "go.uber.org/zap/zapcore" "github.com/kakao/varlog/internal/admin" "github.com/kakao/varlog/internal/admin/mrmanager" "github.com/kakao/varlog/internal/admin/snmanager" "github.com/kakao/varlog/internal/admin/snwatcher" + "github.com/kakao/varlog/internal/flags" "github.com/kakao/varlog/pkg/types" "github.com/kakao/varlog/pkg/util/log" ) @@ -51,11 +50,16 @@ func newStartCommand() *cli.Command { flagSNWatcherHeartbeatCheckDeadline.DurationFlag(false, snwatcher.DefaultHeartbeatDeadline), flagSNWatcherReportDeadline.DurationFlag(false, snwatcher.DefaultReportDeadline), - flagLogDir.StringFlag(false, ""), - flagLogToStderr.BoolFlag(), - flagLogFileRetentionDays.IntFlag(false, 0), - flagLogFileCompression.BoolFlag(), - flagLogLevel.StringFlag(false, "info"), + // logger options + flags.LogDir, + flags.LogToStderr, + flags.LogFileMaxSizeMB, + flags.LogFileMaxBackups, + flags.LogFileRetentionDays, + flags.LogFileNameUTC, + flags.LogFileCompression, + flags.LogHumanReadable, + flags.LogLevel, }, } } @@ -65,7 +69,13 @@ func start(c *cli.Context) error { if err != nil { return err } - logger, err := newLogger(c) + + logOpts, err := flags.ParseLoggerFlags(c, "varlogadm.log") + if err != nil { + return err + } + logOpts = append(logOpts, log.WithZapLoggerOptions(zap.AddStacktrace(zap.DPanicLevel))) + logger, err := log.New(logOpts...) if err != nil { return err } @@ -123,34 +133,3 @@ func start(c *cli.Context) error { } return Main(opts, logger) } - -func newLogger(c *cli.Context) (*zap.Logger, error) { - level, err := zapcore.ParseLevel(c.String(flagLogLevel.Name)) - if err != nil { - return nil, err - } - - opts := []log.Option{ - log.WithHumanFriendly(), - log.WithLocalTime(), - log.WithZapLoggerOptions(zap.AddStacktrace(zap.DPanicLevel)), - log.WithLogLevel(level), - } - if !c.Bool(flagLogToStderr.Name) { - opts = append(opts, log.WithoutLogToStderr()) - } - if logdir := c.String(flagLogDir.Name); len(logdir) != 0 { - absDir, err := filepath.Abs(logdir) - if err != nil { - return nil, err - } - opts = append(opts, log.WithPath(filepath.Join(absDir, "varlogadm.log"))) - } - if c.Bool(flagLogFileCompression.Name) { - opts = append(opts, log.WithCompression()) - } - if retention := c.Int(flagLogFileRetentionDays.Name); retention > 0 { - opts = append(opts, log.WithAgeDays(retention)) - } - return log.New(opts...) -} diff --git a/cmd/varlogadm/flags.go b/cmd/varlogadm/flags.go index 3c4291d76..029426d27 100644 --- a/cmd/varlogadm/flags.go +++ b/cmd/varlogadm/flags.go @@ -72,30 +72,4 @@ var ( Name: "sn-watcher-report-deadline", Envs: []string{"SN_WATCHER_REPORT_DEADLINE"}, } - - flagLogDir = flags.FlagDesc{ - Name: "logdir", - Aliases: []string{"log-dir"}, - Envs: []string{"LOG_DIR", "LOGDIR"}, - } - flagLogToStderr = flags.FlagDesc{ - Name: "logtostderr", - Aliases: []string{"log-to-stderr"}, - Envs: []string{"LOGTOSTDERR", "LOG_TO_STDERR"}, - } - flagLogFileRetentionDays = flags.FlagDesc{ - Name: "logfile-retention-days", - Aliases: []string{"log-file-retention-days"}, - Envs: []string{"LOGFILE_RETENTION_DAYS", "LOG_FILE_RETENTION_DAYS"}, - } - flagLogFileCompression = flags.FlagDesc{ - Name: "logfile-compression", - Aliases: []string{"log-file-compression"}, - Envs: []string{"LOGFILE_COMPRESSION", "LOG_FILE_COMPRESSION"}, - } - flagLogLevel = flags.FlagDesc{ - Name: "loglevel", - Aliases: []string{"log-level"}, - Envs: []string{"LOGLEVEL", "LOG_LEVEL"}, - } ) diff --git a/cmd/varlogmr/flags.go b/cmd/varlogmr/flags.go index 77e3d8619..a70766594 100644 --- a/cmd/varlogmr/flags.go +++ b/cmd/varlogmr/flags.go @@ -150,11 +150,4 @@ var ( Usage: "Collector endpoint", Envs: []string{"COLLECTOR_ENDPOINT"}, } - - flagLogDir = flags.FlagDesc{ - Name: "log-dir", - Aliases: []string{}, - Usage: "Log Dir", - Envs: []string{"LOG_DIR"}, - } ) diff --git a/cmd/varlogmr/metadata_repository.go b/cmd/varlogmr/metadata_repository.go index 1405e8f61..049825ac5 100644 --- a/cmd/varlogmr/metadata_repository.go +++ b/cmd/varlogmr/metadata_repository.go @@ -4,12 +4,13 @@ import ( "fmt" "os" "os/signal" - "path/filepath" "syscall" "github.com/urfave/cli/v2" _ "go.uber.org/automaxprocs" + "go.uber.org/zap" + "github.com/kakao/varlog/internal/flags" "github.com/kakao/varlog/internal/metarepos" "github.com/kakao/varlog/pkg/types" "github.com/kakao/varlog/pkg/util/log" @@ -24,16 +25,14 @@ func main() { } func start(c *cli.Context) error { - logDir, err := filepath.Abs(c.String(flagLogDir.Name)) + logOpts, err := flags.ParseLoggerFlags(c, "varlogmr.log") if err != nil { - return fmt.Errorf("could not create abs path: %w", err) + return err } - logger, err := log.New( - log.WithoutLogToStderr(), - log.WithPath(fmt.Sprintf("%s/log.txt", logDir)), - ) + logOpts = append(logOpts, log.WithZapLoggerOptions(zap.AddStacktrace(zap.DPanicLevel))) + logger, err := log.New(logOpts...) if err != nil { - return fmt.Errorf("could not create logger: %w", err) + return err } defer func() { _ = logger.Sync() @@ -125,7 +124,19 @@ func initCLI() *cli.App { flagMaxLogStreamsCountPerTopic, flagTelemetryCollectorName.StringFlag(false, metarepos.DefaultTelemetryCollectorName), flagTelemetryCollectorEndpoint.StringFlag(false, metarepos.DefaultTelmetryCollectorEndpoint), - flagLogDir.StringFlag(false, metarepos.DefaultLogDir), + + //flagLogDir.StringFlag(false, metarepos.DefaultLogDir), + + // logger options + flags.LogDir, + flags.LogToStderr, + flags.LogFileMaxSizeMB, + flags.LogFileMaxBackups, + flags.LogFileRetentionDays, + flags.LogFileNameUTC, + flags.LogFileCompression, + flags.LogHumanReadable, + flags.LogLevel, }, }}, } diff --git a/cmd/varlogsn/cli.go b/cmd/varlogsn/cli.go index 9f28fdb14..1b87f2240 100644 --- a/cmd/varlogsn/cli.go +++ b/cmd/varlogsn/cli.go @@ -5,6 +5,7 @@ import ( "github.com/urfave/cli/v2" + "github.com/kakao/varlog/internal/flags" "github.com/kakao/varlog/internal/storagenode" "github.com/kakao/varlog/internal/storagenode/logstream" "github.com/kakao/varlog/pkg/types" @@ -72,11 +73,16 @@ func newStartCommand() *cli.Command { flagStorageMetricsLogInterval, flagStorageVerbose.BoolFlag(), - flagLogDir.StringFlag(false, ""), - flagLogToStderr.BoolFlag(), - flagLogFileRetentionDays.IntFlag(false, 0), - flagLogFileCompression.BoolFlag(), - flagLogLevel.StringFlag(false, "info"), + // logger options + flags.LogDir, + flags.LogToStderr, + flags.LogFileMaxSizeMB, + flags.LogFileMaxBackups, + flags.LogFileRetentionDays, + flags.LogFileNameUTC, + flags.LogFileCompression, + flags.LogHumanReadable, + flags.LogLevel, flagExporterType.StringFlag(false, "noop"), flagExporterStopTimeout.DurationFlag(false, 5*time.Second), diff --git a/cmd/varlogsn/flags.go b/cmd/varlogsn/flags.go index 52f1d28c5..039cba922 100644 --- a/cmd/varlogsn/flags.go +++ b/cmd/varlogsn/flags.go @@ -172,32 +172,6 @@ var ( Value: storage.DefaultMetricsLogInterval, } - // flags for logging. - flagLogDir = flags.FlagDesc{ - Name: "log-dir", - Aliases: []string{"logdir"}, - Envs: []string{"LOG_DIR", "LOGDIR"}, - } - flagLogToStderr = flags.FlagDesc{ - Name: "logtostderr", - Envs: []string{"LOGTOSTDERR"}, - } - flagLogFileRetentionDays = flags.FlagDesc{ - Name: "logfile-retention-days", - Aliases: []string{"log-file-retention-days"}, - Envs: []string{"LOGFILE_RETENTION_DAYS", "LOG_FILE_RETENTION_DAYS"}, - } - flagLogFileCompression = flags.FlagDesc{ - Name: "logfile-compression", - Aliases: []string{"log-file-compression"}, - Envs: []string{"LOGFILE_COMPRESSION", "LOG_FILE_COMPRESSION"}, - } - flagLogLevel = flags.FlagDesc{ - Name: "loglevel", - Aliases: []string{"log-level"}, - Envs: []string{"LOGLEVEL", "LOG_LEVEL"}, - } - // flags for telemetry. flagExporterType = flags.FlagDesc{ Name: "exporter-type", diff --git a/cmd/varlogsn/varlogsn.go b/cmd/varlogsn/varlogsn.go index 26ab77a2a..01f7de11c 100644 --- a/cmd/varlogsn/varlogsn.go +++ b/cmd/varlogsn/varlogsn.go @@ -6,7 +6,6 @@ import ( "fmt" "os" "os/signal" - "path/filepath" "strings" "syscall" @@ -21,9 +20,9 @@ import ( _ "go.uber.org/automaxprocs" "go.uber.org/multierr" "go.uber.org/zap" - "go.uber.org/zap/zapcore" "golang.org/x/sync/errgroup" + "github.com/kakao/varlog/internal/flags" "github.com/kakao/varlog/internal/storage" "github.com/kakao/varlog/internal/storagenode" "github.com/kakao/varlog/internal/storagenode/logstream" @@ -47,29 +46,11 @@ func run() int { } func start(c *cli.Context) error { - level, err := zapcore.ParseLevel(c.String(flagLogLevel.Name)) + logOpts, err := flags.ParseLoggerFlags(c, "varlogsn.log") if err != nil { return err } - - logOpts := []log.Option{ - log.WithHumanFriendly(), - log.WithZapLoggerOptions(zap.AddStacktrace(zap.DPanicLevel)), - log.WithLogLevel(level), - } - if c.Bool(flagLogFileCompression.Name) { - logOpts = append(logOpts, log.WithCompression()) - } - if retention := c.Int(flagLogFileRetentionDays.Name); retention > 0 { - logOpts = append(logOpts, log.WithAgeDays(retention)) - } - if logDir := c.String(flagLogDir.Name); len(logDir) != 0 { - absDir, err := filepath.Abs(logDir) - if err != nil { - return err - } - logOpts = append(logOpts, log.WithPath(filepath.Join(absDir, "storagenode.log"))) - } + logOpts = append(logOpts, log.WithZapLoggerOptions(zap.AddStacktrace(zap.DPanicLevel))) logger, err := log.New(logOpts...) if err != nil { return err diff --git a/internal/flags/logger.go b/internal/flags/logger.go new file mode 100644 index 000000000..52cf17975 --- /dev/null +++ b/internal/flags/logger.go @@ -0,0 +1,156 @@ +package flags + +import ( + "fmt" + "path/filepath" + "strings" + + "github.com/urfave/cli/v2" + "go.uber.org/zap/zapcore" + + "github.com/kakao/varlog/pkg/util/log" +) + +const ( + CategoryLogger = "Logger:" + + DefaultLogFileMaxBackups = 100 + DefaultLogFileRetentionDays = 14 + DefaultLogFileMaxSizeMB = 100 + DefaultLogLevel = "INFO" +) + +var ( + // LogDir is a flag specifying the directory of the logs. + LogDir = &cli.StringFlag{ + Name: "logdir", + Category: CategoryLogger, + Aliases: []string{"log-dir"}, + EnvVars: []string{"LOGDIR", "LOG_DIR"}, + Usage: "Directory for the log files.", + } + // LogToStderr is a flag that decides whether the logs are printed to the stderr. + LogToStderr = &cli.BoolFlag{ + Name: "logtostderr", + Category: CategoryLogger, + Aliases: []string{"log-to-stderr"}, + EnvVars: []string{"LOGTOSTDERR"}, + Usage: "Print the logs to the stderr.", + } + // LogFileMaxBackups is a flag specifying the maximum number of backup log files. + LogFileMaxBackups = &cli.IntFlag{ + Name: "logfile-max-backups", + Category: CategoryLogger, + EnvVars: []string{"LOGFILE_MAX_BACKUPS"}, + Value: DefaultLogFileMaxBackups, + Usage: "Maximum number of backup log files. Retain all backup files if zero.", + Action: func(_ *cli.Context, value int) error { + if value < 0 { + return fmt.Errorf("invalid value \"%d\" for flag --logfile-max-backups", value) + } + return nil + }, + } + + // LogFileRetentionDays is a flag specifying the age of backup log files. + LogFileRetentionDays = &cli.IntFlag{ + Name: "logfile-retention-days", + Category: CategoryLogger, + EnvVars: []string{"LOGFILE_RETENTION_DAYS"}, + Value: DefaultLogFileRetentionDays, + Usage: "Age of backup log files. Unlimited age if zero, that is, retain all.", + Action: func(_ *cli.Context, value int) error { + if value < 0 { + return fmt.Errorf("invalid value \"%d\" for flag --logfile-retention-days", value) + } + return nil + }, + } + // LogFileMaxSizeMB is a flag specifying the maximum size for each log file. + LogFileMaxSizeMB = &cli.IntFlag{ + Name: "logfile-max-size-mb", + Category: CategoryLogger, + EnvVars: []string{"LOGFILE_MAX_SIZE_MB"}, + Value: DefaultLogFileMaxSizeMB, + Usage: "Maximum file size for each log file.", + Action: func(_ *cli.Context, value int) error { + if value <= 0 { + return fmt.Errorf("invalid value \"%d\" for flag --logfile-max-size-mb", value) + } + return nil + }, + } + // LogFileNameUTC is a flag that decides whether backup log files are named with timestamps in UTC. + LogFileNameUTC = &cli.BoolFlag{ + Name: "logfile-name-utc", + Category: CategoryLogger, + EnvVars: []string{"LOGFILE_NAME_UTC"}, + Usage: "Whether backup log files are named with timestamps in UTC or local time if not set. Log files are named 'example-1970-01-01T00-00-00.000.log' when the file is rotated.", + } + + // LogFIleCompression is a flag that decides whether backup log files are compressed. + LogFileCompression = &cli.BoolFlag{ + Name: "logfile-compression", + Category: CategoryLogger, + EnvVars: []string{"LOGFILE_COMPRESSION"}, + Usage: "Compress backup log files.", + } + + // LogHumanReadable is a flag that decides whether logs are human-readable. + LogHumanReadable = &cli.BoolFlag{ + Name: "log-human-readable", + Category: CategoryLogger, + EnvVars: []string{"LOG_HUMAN_READABLE"}, + Usage: "Human-readable output.", + } + + // LogLevel is a flag specifying log level. + LogLevel = &cli.StringFlag{ + Name: "loglevel", + Category: CategoryLogger, + Aliases: []string{"log-level"}, + EnvVars: []string{"LOGLEVEL", "LOG_LEVEL"}, + Value: DefaultLogLevel, + Usage: "Log levels, either debug, info, warn, or error case-insensitively.", + } +) + +func ParseLoggerFlags(c *cli.Context, maybeLogFileName string) (opts []log.Option, err error) { + opts = []log.Option{ + log.WithMaxBackups(c.Int(LogFileMaxBackups.Name)), + log.WithAgeDays(c.Int(LogFileRetentionDays.Name)), + log.WithMaxSizeMB(c.Int(LogFileMaxSizeMB.Name)), + } + + if logDir := c.String(LogDir.Name); len(logDir) != 0 { + logDir, err = filepath.Abs(logDir) + if err != nil { + return nil, err + } + opts = append(opts, log.WithPath(filepath.Join(logDir, maybeLogFileName))) + } + + if !c.Bool(LogToStderr.Name) { + opts = append(opts, log.WithoutLogToStderr()) + } + + if !c.Bool(LogFileNameUTC.Name) { + opts = append(opts, log.WithLocalTime()) + } + + if c.Bool(LogFileCompression.Name) { + opts = append(opts, log.WithCompression()) + } + + if c.Bool(LogHumanReadable.Name) { + opts = append(opts, log.WithHumanFriendly()) + } + + level, err := zapcore.ParseLevel(strings.ToLower(c.String(LogLevel.Name))) + if err != nil { + return nil, err + } + opts = append(opts, log.WithLogLevel(level)) + + return opts, nil +} diff --git a/internal/metarepos/config.go b/internal/metarepos/config.go index d83815202..767bbdbb8 100644 --- a/internal/metarepos/config.go +++ b/internal/metarepos/config.go @@ -125,7 +125,10 @@ func newConfig(opts []Option) (config, error) { return config{}, err } - cfg.logger = cfg.logger.Named("vmr").With(zap.Any("nodeid", cfg.nodeID)) + cfg.logger = cfg.logger.Named("mr").With( + zap.Uint32("cid", uint32(cfg.clusterID)), + zap.Uint64("nodeid", uint64(cfg.nodeID)), + ) return cfg, nil }