From f8ffe0793d2cdaea7c6aa8f9152d0ecd05b6a78c Mon Sep 17 00:00:00 2001 From: "mayur.popli" Date: Wed, 26 Mar 2025 10:38:44 +0530 Subject: [PATCH] fix: resolved comments --- pkg/log/log.go | 45 +++++++++++++++++++-------------------------- pkg/log/log_test.go | 44 ++++++++++++++++++++++---------------------- 2 files changed, 41 insertions(+), 48 deletions(-) diff --git a/pkg/log/log.go b/pkg/log/log.go index dcf8d21..68beab4 100644 --- a/pkg/log/log.go +++ b/pkg/log/log.go @@ -17,7 +17,9 @@ import ( ) type Level string + type DestinationType string + type Destination struct { Type DestinationType `yaml:"type"` Config map[string]string `yaml:"config"` @@ -47,9 +49,9 @@ var logLevels = map[Level]zerolog.Level{ } type Config struct { - level Level `yaml:"level"` - destinations []Destination `yaml:"destinations"` - contextKeys []any `yaml:"contextKeys"` + Level Level `yaml:"level"` + Destinations []Destination `yaml:"destinations"` + ContextKeys []any `yaml:"contextKeys"` } var ( @@ -64,16 +66,16 @@ var ( ErrMissingFilePath = errors.New("file path missing in destination config for file logging") ) -func (config *Config) Validate() error { - if _, exists := logLevels[config.level]; !exists { +func (config *Config) validate() error { + if _, exists := logLevels[config.Level]; !exists { return ErrInvalidLogLevel } - if len(config.destinations) == 0 { + if len(config.Destinations) == 0 { return ErrLogDestinationNil } - for _, dest := range config.destinations { + for _, dest := range config.Destinations { switch dest.Type { case Stdout: case File: @@ -96,11 +98,10 @@ func (config *Config) Validate() error { } var defaultConfig = Config{ - level: InfoLevel, - destinations: []Destination{ + Level: InfoLevel, + Destinations: []Destination{ {Type: Stdout}, }, - contextKeys: []any{"userID", "requestID"}, } func init() { @@ -110,7 +111,7 @@ func init() { func getLogger(config Config) (zerolog.Logger, error) { var newLogger zerolog.Logger var writers []io.Writer - for _, dest := range config.destinations { + for _, dest := range config.Destinations { switch dest.Type { case Stdout: writers = append(writers, os.Stdout) @@ -120,20 +121,14 @@ func getLogger(config Config) (zerolog.Logger, error) { if err := os.MkdirAll(dir, os.ModePerm); err != nil { return newLogger, fmt.Errorf("failed to create log directory: %v", err) } - - fmt.Printf("writing test log to file: %v\n", config) lumberjackLogger := &lumberjack.Logger{ Filename: filePath, - MaxSize: 500, // Default size in MB if not overridden - MaxBackups: 15, // Number of backups - MaxAge: 30, // Days to retain Compress: false, } absPath, err := filepath.Abs(filePath) if err != nil { return newLogger, fmt.Errorf("failed to get absolute path: %v", err) } - fmt.Printf("Attempting to write logs to: %s\n", absPath) lumberjackLogger.Filename = absPath setConfigValue := func(key string, target *int) { @@ -159,30 +154,27 @@ func getLogger(config Config) (zerolog.Logger, error) { } }() newLogger = zerolog.New(multiwriter). - Level(logLevels[config.level]). + Level(logLevels[config.Level]). With(). Timestamp(). - Caller(). Logger() cfg = config return newLogger, nil } + func InitLogger(c Config) error { var initErr error once.Do(func() { - if err := c.Validate(); err != nil { + if initErr = c.validate(); initErr != nil { return } logger, initErr = getLogger(c) - if initErr != nil { - return - } }) return initErr - } + func Debug(ctx context.Context, msg string) { logEvent(ctx, zerolog.DebugLevel, msg, nil) } @@ -246,6 +238,7 @@ func logEvent(ctx context.Context, level zerolog.Level, msg string, err error) { addCtx(ctx, event) event.Msg(msg) } + func Request(ctx context.Context, r *http.Request, body []byte) { event := logger.Info() addCtx(ctx, event) @@ -257,13 +250,13 @@ func Request(ctx context.Context, r *http.Request, body []byte) { } func addCtx(ctx context.Context, event *zerolog.Event) { - for _, key := range cfg.contextKeys { + for _, key := range cfg.ContextKeys { val, ok := ctx.Value(key).(string) if !ok { continue } keyStr := key.(string) - event.Str(keyStr, val) + event.Any(keyStr, val) } } diff --git a/pkg/log/log_test.go b/pkg/log/log_test.go index 417f46f..a769db5 100644 --- a/pkg/log/log_test.go +++ b/pkg/log/log_test.go @@ -30,8 +30,8 @@ func setupLogger(t *testing.T, l Level) string { } config := Config{ - level: l, - destinations: []Destination{ + Level: l, + Destinations: []Destination{ { Type: File, Config: map[string]string{ @@ -43,7 +43,7 @@ func setupLogger(t *testing.T, l Level) string { }, }, }, - contextKeys: []any{"userID", "requestID"}, + ContextKeys: []any{"userID", "requestID"}, } err = InitLogger(config) if err != nil { @@ -403,8 +403,8 @@ func TestValidateConfig(t *testing.T) { { name: "Valid config with Stdout", config: Config{ - level: InfoLevel, - destinations: []Destination{ + Level: InfoLevel, + Destinations: []Destination{ {Type: Stdout}, }, }, @@ -413,8 +413,8 @@ func TestValidateConfig(t *testing.T) { { name: "Valid config with File destination and valid path", config: Config{ - level: InfoLevel, - destinations: []Destination{ + Level: InfoLevel, + Destinations: []Destination{ { Type: File, Config: map[string]string{ @@ -431,8 +431,8 @@ func TestValidateConfig(t *testing.T) { { name: "Error: Invalid log level", config: Config{ - level: "invalid", - destinations: []Destination{ + Level: "invalid", + Destinations: []Destination{ {Type: Stdout}, }, }, @@ -441,16 +441,16 @@ func TestValidateConfig(t *testing.T) { { name: "Error: No destinations provided", config: Config{ - level: InfoLevel, - destinations: []Destination{}, + Level: InfoLevel, + Destinations: []Destination{}, }, wantErr: ErrLogDestinationNil, }, { name: "Error: Invalid destination type", config: Config{ - level: InfoLevel, - destinations: []Destination{ + Level: InfoLevel, + Destinations: []Destination{ {Type: "unknown"}, }, }, @@ -459,8 +459,8 @@ func TestValidateConfig(t *testing.T) { { name: "Error: Missing file path for file destination", config: Config{ - level: InfoLevel, - destinations: []Destination{ + Level: InfoLevel, + Destinations: []Destination{ { Type: File, Config: map[string]string{ @@ -474,8 +474,8 @@ func TestValidateConfig(t *testing.T) { { name: "Error: Invalid maxSize value in file destination", config: Config{ - level: InfoLevel, - destinations: []Destination{ + Level: InfoLevel, + Destinations: []Destination{ { Type: File, Config: map[string]string{ @@ -490,8 +490,8 @@ func TestValidateConfig(t *testing.T) { { name: "Error: Invalid maxBackups value in file destination", config: Config{ - level: InfoLevel, - destinations: []Destination{ + Level: InfoLevel, + Destinations: []Destination{ { Type: File, Config: map[string]string{ @@ -506,8 +506,8 @@ func TestValidateConfig(t *testing.T) { { name: "Error: Invalid maxAge value in file destination", config: Config{ - level: InfoLevel, - destinations: []Destination{ + Level: InfoLevel, + Destinations: []Destination{ { Type: File, Config: map[string]string{ @@ -523,7 +523,7 @@ func TestValidateConfig(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - err := tt.config.Validate() + err := tt.config.validate() if (err == nil) != (tt.wantErr == nil) { t.Errorf("validate() error = %v, wantErr %v", err, tt.wantErr) }