From da23addaa63fbbbf3d677e3cf9dce6dc5f96e3f8 Mon Sep 17 00:00:00 2001
From: Wael Nasreddine <wael.nasreddine@gmail.com>
Date: Sat, 21 Dec 2024 17:19:39 -0800
Subject: [PATCH] feat(cmd): Add global flag --log-level to change Logger level
 (#139)

### TL;DR

Moved logger initialization into the CLI command's `Before` hook and added log level configuration via flags.

### What changed?

- Relocated logger initialization from `main.go` to `cmd.cmd.go`'s `beforeFunc`
- Added a new `--log-level` flag with environment variable support
- Logger is now stored in the context and retrieved when needed
- Removed logger parameter from `New()` and `serveCommand()`
- Added validation for log level values

### How to test?

1. Run the application with different log levels:
```bash
go run . --log-level debug serve
go run . --log-level info serve
```

2. Set log level via environment variable:
```bash
LOG_LEVEL=warn go run . serve
```

3. Verify logger output format:
- In terminal: human-readable console output
- In non-terminal: JSON output

### Why make this change?

This change improves the application's logging configuration by:
- Allowing runtime log level configuration
- Following CLI best practices by using the context for dependency injection
- Maintaining proper separation of concerns with logging initialization
- Providing consistent logging format based on the execution environment
---
 cmd/cmd.go   | 46 ++++++++++++++++++++++++++++++++++++++++++++--
 cmd/serve.go | 10 +++++++---
 main.go      | 17 +----------------
 3 files changed, 52 insertions(+), 21 deletions(-)

diff --git a/cmd/cmd.go b/cmd/cmd.go
index c05dabf..eaac2b2 100644
--- a/cmd/cmd.go
+++ b/cmd/cmd.go
@@ -1,8 +1,15 @@
 package cmd
 
 import (
+	"context"
+	"fmt"
+	"io"
+	"os"
+	"time"
+
 	"github.com/rs/zerolog"
 	"github.com/urfave/cli/v3"
+	"golang.org/x/term"
 )
 
 // Version defines the version of the binary, and is meant to be set with ldflags at build time.
@@ -10,13 +17,48 @@ import (
 //nolint:gochecknoglobals
 var Version = "dev"
 
-func New(logger zerolog.Logger) *cli.Command {
+func New() *cli.Command {
 	return &cli.Command{
 		Name:    "ncps",
 		Usage:   "Nix Binary Cache Proxy Service",
 		Version: Version,
+		Before:  beforeFunc,
+		Flags: []cli.Flag{
+			&cli.StringFlag{
+				Name:    "log-level",
+				Usage:   "Set the log level",
+				Sources: cli.EnvVars("LOG_LEVEL"),
+				Value:   "info",
+				Validator: func(lvl string) error {
+					_, err := zerolog.ParseLevel(lvl)
+
+					return err
+				},
+			},
+		},
 		Commands: []*cli.Command{
-			serveCommand(logger),
+			serveCommand(),
 		},
 	}
 }
+
+func beforeFunc(ctx context.Context, cmd *cli.Command) (context.Context, error) {
+	logLvl := cmd.String("log-level")
+
+	lvl, err := zerolog.ParseLevel(logLvl)
+	if err != nil {
+		return ctx, fmt.Errorf("error parsing the log-level %q: %w", logLvl, err)
+	}
+
+	var output io.Writer = os.Stdout
+
+	if term.IsTerminal(int(os.Stdout.Fd())) {
+		output = zerolog.ConsoleWriter{Out: os.Stdout, TimeFormat: time.RFC3339}
+	}
+
+	log := zerolog.New(output).Level(lvl)
+
+	log.Info().Str("log-level", lvl.String()).Msg("logger created")
+
+	return log.WithContext(ctx), nil
+}
diff --git a/cmd/serve.go b/cmd/serve.go
index ebbca6d..3e8d31b 100644
--- a/cmd/serve.go
+++ b/cmd/serve.go
@@ -25,12 +25,12 @@ import (
 // ErrCacheMaxSizeRequired is returned if --cache-lru-schedule was given but not --cache-max-size.
 var ErrCacheMaxSizeRequired = errors.New("--cache-max-size is required when --cache-lru-schedule is specified")
 
-func serveCommand(logger zerolog.Logger) *cli.Command {
+func serveCommand() *cli.Command {
 	return &cli.Command{
 		Name:    "serve",
 		Aliases: []string{"s"},
 		Usage:   "serve the nix binary cache over http",
-		Action:  serveAction(logger.With().Str("cmd", "serve").Logger()),
+		Action:  serveAction(),
 		Flags: []cli.Flag{
 			&cli.BoolFlag{
 				Name:    "allow-delete",
@@ -109,8 +109,12 @@ func serveCommand(logger zerolog.Logger) *cli.Command {
 	}
 }
 
-func serveAction(logger zerolog.Logger) cli.ActionFunc {
+func serveAction() cli.ActionFunc {
 	return func(ctx context.Context, cmd *cli.Command) error {
+		logger := zerolog.Ctx(ctx).With().Str("cmd", "serve").Logger()
+
+		ctx = logger.WithContext(ctx)
+
 		ctx, cancel := context.WithCancel(ctx)
 
 		g, ctx := errgroup.WithContext(ctx)
diff --git a/main.go b/main.go
index 839c8c4..40dec7d 100644
--- a/main.go
+++ b/main.go
@@ -2,13 +2,8 @@ package main
 
 import (
 	"context"
-	"io"
 	"log"
 	"os"
-	"time"
-
-	"github.com/rs/zerolog"
-	"golang.org/x/term"
 
 	"github.com/kalbasit/ncps/cmd"
 )
@@ -18,7 +13,7 @@ func main() {
 }
 
 func realMain() int {
-	c := cmd.New(newLogger())
+	c := cmd.New()
 
 	if err := c.Run(context.Background(), os.Args); err != nil {
 		log.Printf("error running the application: %s", err)
@@ -28,13 +23,3 @@ func realMain() int {
 
 	return 0
 }
-
-func newLogger() zerolog.Logger {
-	var output io.Writer = os.Stdout
-
-	if term.IsTerminal(int(os.Stdout.Fd())) {
-		output = zerolog.ConsoleWriter{Out: os.Stdout, TimeFormat: time.RFC3339}
-	}
-
-	return zerolog.New(output)
-}