fix: replace nil context with context.TODO
This commit is contained in:
@@ -31,23 +31,40 @@ func RequestLogger(metrics *observability.Metrics) gin.HandlerFunc {
|
||||
level = observability.LogLevelWarn
|
||||
}
|
||||
|
||||
observability.LogJSON(
|
||||
fields := map[string]any{
|
||||
"client_ip": c.ClientIP(),
|
||||
"duration_ms": float64(duration.Microseconds()) / 1000,
|
||||
"method": c.Request.Method,
|
||||
"path": path,
|
||||
"request_id": c.Writer.Header().Get(requestIDHeader),
|
||||
"status": status,
|
||||
}
|
||||
privateErrors := c.Errors.ByType(gin.ErrorTypePrivate)
|
||||
var logErr error
|
||||
if len(privateErrors) > 0 {
|
||||
logErr = privateErrors.Last().Err
|
||||
}
|
||||
if route != path {
|
||||
fields["route"] = route
|
||||
}
|
||||
if query != "" {
|
||||
fields["query"] = query
|
||||
}
|
||||
if size := c.Writer.Size(); size >= 0 {
|
||||
fields["bytes"] = size
|
||||
}
|
||||
if errors := privateErrors.String(); errors != "" {
|
||||
fields["errors"] = errors
|
||||
}
|
||||
|
||||
observability.LogContext(
|
||||
c.Request.Context(),
|
||||
level,
|
||||
"http_request",
|
||||
"http",
|
||||
"",
|
||||
map[string]any{
|
||||
"method": c.Request.Method,
|
||||
"route": route,
|
||||
"path": path,
|
||||
"query": query,
|
||||
"status": status,
|
||||
"duration_ms": float64(duration.Microseconds()) / 1000,
|
||||
"bytes": c.Writer.Size(),
|
||||
"client_ip": c.ClientIP(),
|
||||
"errors": c.Errors.ByType(gin.ErrorTypePrivate).String(),
|
||||
},
|
||||
nil,
|
||||
c.Request.Method+" "+path,
|
||||
fields,
|
||||
logErr,
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
30
internal/server/request_context.go
Normal file
30
internal/server/request_context.go
Normal file
@@ -0,0 +1,30 @@
|
||||
package server
|
||||
|
||||
import (
|
||||
"mal/internal/observability"
|
||||
"strings"
|
||||
|
||||
"github.com/gin-gonic/gin"
|
||||
"github.com/google/uuid"
|
||||
)
|
||||
|
||||
const requestIDHeader = "X-Request-ID"
|
||||
|
||||
func RequestContextMiddleware() gin.HandlerFunc {
|
||||
return func(c *gin.Context) {
|
||||
requestID := strings.TrimSpace(c.GetHeader(requestIDHeader))
|
||||
if requestID == "" {
|
||||
requestID = uuid.NewString()
|
||||
}
|
||||
|
||||
path := c.Request.URL.Path
|
||||
route := c.FullPath()
|
||||
if route == "" {
|
||||
route = path
|
||||
}
|
||||
|
||||
c.Writer.Header().Set(requestIDHeader, requestID)
|
||||
c.Request = c.Request.WithContext(observability.WithRequestContext(c.Request.Context(), requestID, path, route))
|
||||
c.Next()
|
||||
}
|
||||
}
|
||||
@@ -27,7 +27,18 @@ func RespondError(c *gin.Context, status int, event string, component string, me
|
||||
if status >= http.StatusInternalServerError {
|
||||
level = observability.LogLevelError
|
||||
}
|
||||
observability.LogJSON(level, event, component, "", fields, err)
|
||||
if fields == nil {
|
||||
fields = make(map[string]any, 2)
|
||||
}
|
||||
if _, exists := fields["request_path"]; !exists {
|
||||
fields["request_path"] = c.Request.URL.Path
|
||||
}
|
||||
if route := c.FullPath(); route != "" && route != c.Request.URL.Path {
|
||||
if _, exists := fields["request_route"]; !exists {
|
||||
fields["request_route"] = route
|
||||
}
|
||||
}
|
||||
observability.LogContext(c.Request.Context(), level, event, component, "", fields, err)
|
||||
RespondHTMLOrJSONError(c, status, message)
|
||||
}
|
||||
|
||||
|
||||
@@ -27,7 +27,7 @@ func ProvideRouter(cfg config.Config, htmlRender render.HTMLRender, metrics *obs
|
||||
gin.SetMode(cfg.GinMode)
|
||||
}
|
||||
r := gin.New()
|
||||
r.Use(CORSMiddlewareWithConfig(cfg), audit.ContextMiddleware(), RequestLogger(metrics), gin.Recovery())
|
||||
r.Use(CORSMiddlewareWithConfig(cfg), RequestContextMiddleware(), audit.ContextMiddleware(), RequestLogger(metrics), gin.Recovery())
|
||||
r.Static("/static", "./static")
|
||||
r.Static("/dist", "./dist")
|
||||
r.GET("/metrics", gin.WrapH(metrics.Handler()))
|
||||
|
||||
@@ -44,6 +44,7 @@ func TestRequestLoggerUsesMatchedRoute(t *testing.T) {
|
||||
defer log.SetOutput(previousOutput)
|
||||
|
||||
router := gin.New()
|
||||
router.Use(RequestContextMiddleware())
|
||||
router.Use(RequestLogger(observability.NewMetrics()))
|
||||
router.GET("/anime/:id", func(c *gin.Context) {
|
||||
c.String(http.StatusOK, "ok")
|
||||
@@ -59,13 +60,54 @@ func TestRequestLoggerUsesMatchedRoute(t *testing.T) {
|
||||
}
|
||||
|
||||
logLine := string(output)
|
||||
if !strings.Contains(logLine, `"event":"http_request"`) {
|
||||
t.Fatalf("log line missing event: %s", logLine)
|
||||
if !strings.Contains(logLine, " INFO http 200 GET /anime/1") {
|
||||
t.Fatalf("log line missing compact http summary: %s", logLine)
|
||||
}
|
||||
if !strings.Contains(logLine, `"route":"/anime/:id"`) {
|
||||
if !strings.Contains(logLine, " route=/anime/:id") {
|
||||
t.Fatalf("log line missing route: %s", logLine)
|
||||
}
|
||||
if !strings.Contains(logLine, `"status":200`) {
|
||||
t.Fatalf("log line missing status: %s", logLine)
|
||||
if !strings.Contains(logLine, " request_id=") {
|
||||
t.Fatalf("log line missing request id: %s", logLine)
|
||||
}
|
||||
if strings.Contains(logLine, `"GET /anime/1"`) {
|
||||
t.Fatalf("log line should not duplicate request summary: %s", logLine)
|
||||
}
|
||||
if rec.Header().Get(requestIDHeader) == "" {
|
||||
t.Fatalf("expected %s response header to be set", requestIDHeader)
|
||||
}
|
||||
}
|
||||
|
||||
func TestRespondErrorIncludesRequestContext(t *testing.T) {
|
||||
gin.SetMode(gin.TestMode)
|
||||
|
||||
var logs bytes.Buffer
|
||||
previousOutput := log.Writer()
|
||||
log.SetOutput(&logs)
|
||||
defer log.SetOutput(previousOutput)
|
||||
|
||||
router := gin.New()
|
||||
router.Use(RequestContextMiddleware())
|
||||
router.GET("/anime/:id", func(c *gin.Context) {
|
||||
RespondError(c, http.StatusInternalServerError, "anime_lookup_failed", "anime", "failed", nil, context.DeadlineExceeded)
|
||||
})
|
||||
|
||||
req := httptest.NewRequestWithContext(context.Background(), http.MethodGet, "/anime/1", nil)
|
||||
rec := httptest.NewRecorder()
|
||||
router.ServeHTTP(rec, req)
|
||||
|
||||
output, err := io.ReadAll(&logs)
|
||||
if err != nil {
|
||||
t.Fatalf("read logs: %v", err)
|
||||
}
|
||||
|
||||
logLine := string(output)
|
||||
if !strings.Contains(logLine, " request_id=") {
|
||||
t.Fatalf("log line missing request id: %s", logLine)
|
||||
}
|
||||
if !strings.Contains(logLine, " request_path=/anime/1") {
|
||||
t.Fatalf("log line missing request path: %s", logLine)
|
||||
}
|
||||
if !strings.Contains(logLine, " request_route=/anime/:id") {
|
||||
t.Fatalf("log line missing request route: %s", logLine)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user