Skip to content

Commit

Permalink
Logging refactoring
Browse files Browse the repository at this point in the history
  • Loading branch information
mantzas committed Oct 25, 2024
1 parent d86e1d5 commit 8d126f9
Show file tree
Hide file tree
Showing 5 changed files with 88 additions and 16 deletions.
23 changes: 23 additions & 0 deletions component/http/observability.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import (
"fmt"
"net/http"
"net/http/pprof"

"github.com/beatlabs/patron/observability/log"
)

func ProfilingRoutes(enableExpVar bool) []*Route {
Expand Down Expand Up @@ -47,3 +49,24 @@ func expVars(w http.ResponseWriter, _ *http.Request) {
})
_, _ = fmt.Fprintf(w, "\n}\n")
}

// LoggingRoutes returns a routes relates to logs.
func LoggingRoutes() []*Route {
handler := func(w http.ResponseWriter, r *http.Request) {
lvl := r.PathValue("level")
if lvl == "" {
http.Error(w, "missing log level", http.StatusBadRequest)
return
}

err := log.SetLevel(lvl)
if err != nil {
http.Error(w, err.Error(), http.StatusBadRequest)
return
}
w.WriteHeader(http.StatusOK)
}

route, _ := NewRoute("POST /debug/log/{level}", handler)
return []*Route{route}
}
46 changes: 43 additions & 3 deletions component/http/observability_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"net/http/httptest"
"testing"

"github.com/beatlabs/patron/observability/log"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
Expand All @@ -18,7 +19,7 @@ type profilingTestCase struct {

func TestProfilingRoutes(t *testing.T) {
t.Run("without vars", func(t *testing.T) {
server := createProfilingServer(false)
server := createServer(false)
defer server.Close()

for name, tt := range createProfilingTestCases(false) {
Expand All @@ -34,7 +35,7 @@ func TestProfilingRoutes(t *testing.T) {
})

t.Run("with vars", func(t *testing.T) {
server := createProfilingServer(true)
server := createServer(true)
defer server.Close()

for name, tt := range createProfilingTestCases(true) {
Expand All @@ -50,11 +51,14 @@ func TestProfilingRoutes(t *testing.T) {
})
}

func createProfilingServer(enableExpVar bool) *httptest.Server {
func createServer(enableExpVar bool) *httptest.Server {
mux := http.NewServeMux()
for _, route := range ProfilingRoutes(enableExpVar) {
mux.HandleFunc(route.path, route.handler)
}
for _, route := range LoggingRoutes() {
mux.HandleFunc(route.path, route.handler)
}

return httptest.NewServer(mux)
}
Expand All @@ -80,3 +84,39 @@ func createProfilingTestCases(enableExpVar bool) map[string]profilingTestCase {
"vars": {"/debug/vars/", expVarWant},
}
}

func TestLoggingRoutes(t *testing.T) {
log.Setup(&log.Config{

Check failure on line 89 in component/http/observability_test.go

View workflow job for this annotation

GitHub Actions / Lint and fmt check

Error return value of `log.Setup` is not checked (errcheck)
IsJSON: true,
Level: "info",
})
server := createServer(true)
defer server.Close()

t.Run("change log level to debug", func(t *testing.T) {
req, err := http.NewRequestWithContext(context.Background(), http.MethodPost, fmt.Sprintf("%s/debug/log/debug", server.URL), nil)

Check failure on line 97 in component/http/observability_test.go

View workflow job for this annotation

GitHub Actions / Lint and fmt check

fmt.Sprintf can be replaced with string concatenation (perfsprint)
require.NoError(t, err)
resp, err := http.DefaultClient.Do(req)
require.NoError(t, err)
assert.Equal(t, http.StatusOK, resp.StatusCode)
require.NoError(t, resp.Body.Close())
})

t.Run("wrong log level", func(t *testing.T) {
req, err := http.NewRequestWithContext(context.Background(), http.MethodPost, fmt.Sprintf("%s/debug/log/xxx", server.URL), nil)

Check failure on line 106 in component/http/observability_test.go

View workflow job for this annotation

GitHub Actions / Lint and fmt check

fmt.Sprintf can be replaced with string concatenation (perfsprint)
require.NoError(t, err)
resp, err := http.DefaultClient.Do(req)
require.NoError(t, err)
assert.Equal(t, http.StatusBadRequest, resp.StatusCode)
require.NoError(t, resp.Body.Close())
})

t.Run("empty log level", func(t *testing.T) {
req, err := http.NewRequestWithContext(context.Background(), http.MethodPost, fmt.Sprintf("%s/debug/log/", server.URL), nil)

Check failure on line 115 in component/http/observability_test.go

View workflow job for this annotation

GitHub Actions / Lint and fmt check

fmt.Sprintf can be replaced with string concatenation (perfsprint)
require.NoError(t, err)
resp, err := http.DefaultClient.Do(req)
require.NoError(t, err)
assert.Equal(t, http.StatusNotFound, resp.StatusCode)
require.NoError(t, resp.Body.Close())
})
}
24 changes: 15 additions & 9 deletions observability/log/log.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,21 +19,26 @@ type ctxKey struct{}
var logCfg *Config

// Setup sets up the logger with the given configuration.
func Setup(cfg *Config) {
func Setup(cfg *Config) error {
logCfg = cfg
setDefaultLogger(cfg)
return setDefaultLogger(cfg)
}

// SetLevel sets the logger level.
func SetLevel(lvl string) {
func SetLevel(lvl string) error {
logCfg.Level = lvl
setDefaultLogger(logCfg)
return setDefaultLogger(logCfg)
}

func setDefaultLogger(cfg *Config) {
func setDefaultLogger(cfg *Config) error {
lvl, err := level(cfg.Level)
if err != nil {
return err
}

ho := &slog.HandlerOptions{
AddSource: true,
Level: level(cfg.Level),
Level: lvl,
}

var hnd slog.Handler
Expand All @@ -45,15 +50,16 @@ func setDefaultLogger(cfg *Config) {
}

slog.SetDefault(slog.New(hnd.WithAttrs(cfg.Attributes)))
return nil
}

func level(lvl string) slog.Level {
func level(lvl string) (slog.Level, error) {
lv := slog.LevelVar{}
if err := lv.UnmarshalText([]byte(lvl)); err != nil {
return slog.LevelInfo
return slog.LevelInfo, err
}

return lv.Level()
return lv.Level(), nil
}

// FromContext returns the logger, if it exists in the context, or nil.
Expand Down
6 changes: 3 additions & 3 deletions observability/log/log_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ func TestSetup(t *testing.T) {
IsJSON: true,
Level: "debug",
}
Setup(cfg)
assert.NoError(t, Setup(cfg))

Check failure on line 19 in observability/log/log_test.go

View workflow job for this annotation

GitHub Actions / Lint and fmt check

require-error: for error assertions use require (testifylint)
assert.NotNil(t, slog.Default())
})

Expand All @@ -26,7 +26,7 @@ func TestSetup(t *testing.T) {
IsJSON: false,
Level: "debug",
}
Setup(cfg)
assert.NoError(t, Setup(cfg))

Check failure on line 29 in observability/log/log_test.go

View workflow job for this annotation

GitHub Actions / Lint and fmt check

require-error: for error assertions use require (testifylint)
assert.NotNil(t, slog.Default())
})
}
Expand Down Expand Up @@ -55,7 +55,7 @@ func TestSetLevelAndCheckEnable(t *testing.T) {
assert.True(t, Enabled(slog.LevelInfo))
assert.False(t, Enabled(slog.LevelDebug))

SetLevel("debug")
assert.NoError(t, SetLevel("debug"))

Check failure on line 58 in observability/log/log_test.go

View workflow job for this annotation

GitHub Actions / Lint and fmt check

require-error: for error assertions use require (testifylint)

assert.True(t, Enabled(slog.LevelDebug))
}
Expand Down
5 changes: 4 additions & 1 deletion observability/observability.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,10 @@ type Config struct {
// It creates a resource with the given name and version, sets up the metric and trace providers,
// and returns a Provider containing the initialized providers.
func Setup(ctx context.Context, cfg Config) (*Provider, error) {
log.Setup(&cfg.LogConfig)
err := log.Setup(&cfg.LogConfig)
if err != nil {
return nil, err
}

res, err := createResource(cfg.Name, cfg.Version)
if err != nil {
Expand Down

0 comments on commit 8d126f9

Please sign in to comment.