diff --git a/cloudrequestlog/middleware.go b/cloudrequestlog/middleware.go index 7bf8b562..e082b994 100644 --- a/cloudrequestlog/middleware.go +++ b/cloudrequestlog/middleware.go @@ -8,6 +8,7 @@ import ( "runtime" "time" + "go.einride.tech/cloudrunner/cloudslog" "go.einride.tech/cloudrunner/cloudstream" ltype "google.golang.org/genproto/googleapis/logging/type" "google.golang.org/grpc" @@ -32,6 +33,7 @@ func (l *Middleware) GRPCUnaryServerInterceptor( ) (interface{}, error) { startTime := time.Now() ctx = WithAdditionalFields(ctx) + ctx = cloudslog.WithHTTPRequest(ctx, <ype.HttpRequest{Protocol: "gRPC"}) // Clone request to ensure not using a mutated one later requestClone := proto.Clone(request.(proto.Message)) response, err := handler(ctx, request) @@ -84,6 +86,7 @@ func (l *Middleware) GRPCStreamServerInterceptor( ) error { startTime := time.Now() ctx := WithAdditionalFields(ss.Context()) + ctx = cloudslog.WithHTTPRequest(ctx, <ype.HttpRequest{Protocol: "gRPC"}) ss = cloudstream.NewContextualServerStream(ctx, ss) err := handler(srv, ss) responseStatus := status.Convert(err) @@ -190,6 +193,17 @@ func (l *Middleware) HTTPServer(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { startTime := time.Now() ctx := WithAdditionalFields(r.Context()) + httpReq := <ype.HttpRequest{ + RequestMethod: r.Method, + UserAgent: r.UserAgent(), + RemoteIp: r.RemoteAddr, + Referer: r.Referer(), + Protocol: r.Proto, + } + if r.URL != nil { + httpReq.RequestUrl = r.URL.String() + } + ctx = cloudslog.WithHTTPRequest(ctx, httpReq) r = r.WithContext(ctx) responseWriter := &httpResponseWriter{ResponseWriter: w} next.ServeHTTP(responseWriter, r) diff --git a/cloudslog/handler.go b/cloudslog/handler.go index bfcba5c2..576d0001 100644 --- a/cloudslog/handler.go +++ b/cloudslog/handler.go @@ -85,17 +85,23 @@ func (t *handler) Handle(ctx context.Context, record slog.Record) error { } } + // Build context group with optional httpRequest and reportLocation. + var contextAttrs []any + if httpRequest := httpRequestFromContext(ctx); httpRequest != nil { + contextAttrs = append(contextAttrs, slog.Any("httpRequest", httpRequest)) + } if record.PC != 0 { fs := runtime.CallersFrames([]uintptr{record.PC}) f, _ := fs.Next() - record.AddAttrs(slog.Group("context", - slog.Group("reportLocation", - slog.String("filePath", f.File), - slog.Int("lineNumber", f.Line), - slog.String("functionName", f.Function), - ), + contextAttrs = append(contextAttrs, slog.Group("reportLocation", + slog.String("filePath", f.File), + slog.Int("lineNumber", f.Line), + slog.String("functionName", f.Function), )) } + if len(contextAttrs) > 0 { + record.AddAttrs(slog.Group("context", contextAttrs...)) + } } record.AddAttrs(attributesFromContext(ctx)...) return t.Handler.Handle(ctx, record) diff --git a/cloudslog/httprequest_context.go b/cloudslog/httprequest_context.go new file mode 100644 index 00000000..cd15d3ca --- /dev/null +++ b/cloudslog/httprequest_context.go @@ -0,0 +1,21 @@ +package cloudslog + +import ( + "context" + + ltype "google.golang.org/genproto/googleapis/logging/type" +) + +type httpRequestContextKey struct{} + +// WithHTTPRequest stores an [ltype.HttpRequest] on the context for use in error reporting. +// This should be called by request middleware before handling the request, so that +// error reports logged during request handling can include HTTP request context. +func WithHTTPRequest(parent context.Context, req *ltype.HttpRequest) context.Context { + return context.WithValue(parent, httpRequestContextKey{}, req) +} + +func httpRequestFromContext(ctx context.Context) *ltype.HttpRequest { + req, _ := ctx.Value(httpRequestContextKey{}).(*ltype.HttpRequest) + return req +} diff --git a/cloudslog/httprequest_context_test.go b/cloudslog/httprequest_context_test.go new file mode 100644 index 00000000..484d0a8f --- /dev/null +++ b/cloudslog/httprequest_context_test.go @@ -0,0 +1,49 @@ +package cloudslog + +import ( + "context" + "log/slog" + "strings" + "testing" + + ltype "google.golang.org/genproto/googleapis/logging/type" + "gotest.tools/v3/assert" +) + +func TestHandler_httpRequestFromContext(t *testing.T) { + t.Run("included in error report context", func(t *testing.T) { + var b strings.Builder + logger := slog.New(newHandler(&b, LoggerConfig{ReportErrors: true})) + ctx := WithHTTPRequest(context.Background(), <ype.HttpRequest{ + RequestMethod: "GET", + RequestUrl: "/test/path", + UserAgent: "test-agent", + }) + logger.ErrorContext(ctx, "something went wrong") + got := b.String() + assert.Assert(t, strings.Contains(got, `"requestMethod":"GET"`), got) + assert.Assert(t, strings.Contains(got, `"requestUrl":"/test/path"`), got) + assert.Assert(t, strings.Contains(got, `"userAgent":"test-agent"`), got) + }) + + t.Run("not included when no httpRequest on context", func(t *testing.T) { + var b strings.Builder + logger := slog.New(newHandler(&b, LoggerConfig{ReportErrors: true})) + logger.ErrorContext(context.Background(), "something went wrong") + got := b.String() + assert.Assert(t, !strings.Contains(got, `"httpRequest"`), got) + }) + + t.Run("not included for non-error levels", func(t *testing.T) { + var b strings.Builder + logger := slog.New(newHandler(&b, LoggerConfig{ReportErrors: true})) + ctx := WithHTTPRequest(context.Background(), <ype.HttpRequest{ + RequestMethod: "GET", + RequestUrl: "/test/path", + }) + logger.InfoContext(ctx, "just info") + got := b.String() + // httpRequest should not appear in the context group for non-error logs. + assert.Assert(t, !strings.Contains(got, `"httpRequest"`), got) + }) +}