From 86361e87d5a3acda182c3a1184e8c7bc31c558fb Mon Sep 17 00:00:00 2001 From: Matthew Gabeler-Lee Date: Thu, 7 Dec 2023 18:08:27 -0500 Subject: [PATCH] fix: don't race poking logger variable, just make events from it fixes: #68 --- logger.go | 30 ++++++++++++------------------ logger_test.go | 38 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 18 deletions(-) diff --git a/logger.go b/logger.go index dff00a0..3a57264 100644 --- a/logger.go +++ b/logger.go @@ -104,34 +104,28 @@ func SetLogger(opts ...Option) gin.HandlerFunc { } latency := end.Sub(start) - l = l.With(). - Int("status", c.Writer.Status()). - Str("method", c.Request.Method). - Str("path", path). - Str("ip", c.ClientIP()). - Dur("latency", latency). - Str("user_agent", c.Request.UserAgent()).Logger() - msg := "Request" if len(c.Errors) > 0 { msg = c.Errors.String() } + var evt *zerolog.Event switch { case c.Writer.Status() >= http.StatusBadRequest && c.Writer.Status() < http.StatusInternalServerError: - { - l.WithLevel(cfg.clientErrorLevel). - Msg(msg) - } + evt = l.WithLevel(cfg.clientErrorLevel) case c.Writer.Status() >= http.StatusInternalServerError: - { - l.WithLevel(cfg.serverErrorLevel). - Msg(msg) - } + evt = l.WithLevel(cfg.serverErrorLevel) default: - l.WithLevel(cfg.defaultLevel). - Msg(msg) + evt = l.WithLevel(cfg.defaultLevel) } + evt. + Int("status", c.Writer.Status()). + Str("method", c.Request.Method). + Str("path", path). + Str("ip", c.ClientIP()). + Dur("latency", latency). + Str("user_agent", c.Request.UserAgent()). + Msg(msg) } } } diff --git a/logger_test.go b/logger_test.go index 0bf0e35..7dda2f7 100644 --- a/logger_test.go +++ b/logger_test.go @@ -3,9 +3,12 @@ package logger import ( "bytes" "context" + "fmt" "net/http" "net/http/httptest" "regexp" + "strings" + "sync" "testing" "github.com/gin-gonic/gin" @@ -151,6 +154,41 @@ func TestLoggerWithLevels(t *testing.T) { assert.Contains(t, buffer.String(), "FTL") } +func TestCustomLoggerIssue68(t *testing.T) { + buffer := new(bytes.Buffer) + gin.SetMode(gin.ReleaseMode) + r := gin.New() + l := zerolog.New(buffer) + r.Use(SetLogger( + WithLogger(func(*gin.Context, zerolog.Logger) zerolog.Logger { return l }), + WithDefaultLevel(zerolog.DebugLevel), + WithClientErrorLevel(zerolog.ErrorLevel), + WithServerErrorLevel(zerolog.FatalLevel), + )) + r.GET("/example", func(c *gin.Context) {}) + + // concurrent requests should only have their info logged once + var wg sync.WaitGroup + for i := 0; i < 10; i++ { + wg.Add(1) + req := fmt.Sprintf("/example?a=%d", i) + go func() { + defer wg.Done() + performRequest(r, "GET", req) + }() + } + wg.Wait() + + bs := buffer.String() + for i := 0; i < 10; i++ { + // should contain each request log exactly once + msg := fmt.Sprintf("/example?a=%d", i) + if assert.Contains(t, bs, msg) { + assert.Equal(t, strings.Index(bs, msg), strings.LastIndex(bs, msg)) + } + } +} + func TestLoggerParseLevel(t *testing.T) { type args struct { levelStr string