From 4a0b0aafbde8bd72ce4868c81eb6d69853ed829a Mon Sep 17 00:00:00 2001 From: Nityananda Gohain Date: Wed, 22 Jan 2025 13:12:21 +0530 Subject: [PATCH] fix: use common logging middleware (#6865) * feat: use common logging middleware * fix: update comment * fix: remove privatePort:true * fix: remove staticfields from logging * fix: remove staticfields from logging --- ee/query-service/app/server.go | 30 +++--------------------------- pkg/http/middleware/logging.go | 11 +++++------ pkg/query-service/app/server.go | 30 +++--------------------------- 3 files changed, 11 insertions(+), 60 deletions(-) diff --git a/ee/query-service/app/server.go b/ee/query-service/app/server.go index 48f78377856..7a25da0892b 100644 --- a/ee/query-service/app/server.go +++ b/ee/query-service/app/server.go @@ -29,6 +29,7 @@ import ( "go.signoz.io/signoz/ee/query-service/integrations/gateway" "go.signoz.io/signoz/ee/query-service/interfaces" "go.signoz.io/signoz/ee/query-service/rules" + "go.signoz.io/signoz/pkg/http/middleware" baseauth "go.signoz.io/signoz/pkg/query-service/auth" v3 "go.signoz.io/signoz/pkg/query-service/model/v3" "go.signoz.io/signoz/pkg/signoz" @@ -317,7 +318,7 @@ func (s *Server) createPrivateServer(apiHandler *api.APIHandler) (*http.Server, r.Use(setTimeoutMiddleware) r.Use(s.analyticsMiddleware) - r.Use(loggingMiddlewarePrivate) + r.Use(middleware.NewLogging(zap.L()).Wrap) r.Use(baseapp.LogCommentEnricher) apiHandler.RegisterPrivateRoutes(r) @@ -360,7 +361,7 @@ func (s *Server) createPublicServer(apiHandler *api.APIHandler, web web.Web) (*h r.Use(setTimeoutMiddleware) r.Use(s.analyticsMiddleware) - r.Use(loggingMiddleware) + r.Use(middleware.NewLogging(zap.L()).Wrap) r.Use(baseapp.LogCommentEnricher) apiHandler.RegisterRoutes(r, am) @@ -393,31 +394,6 @@ func (s *Server) createPublicServer(apiHandler *api.APIHandler, web web.Web) (*h }, nil } -// TODO(remove): Implemented at pkg/http/middleware/logging.go -// loggingMiddleware is used for logging public api calls -func loggingMiddleware(next http.Handler) http.Handler { - return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - route := mux.CurrentRoute(r) - path, _ := route.GetPathTemplate() - startTime := time.Now() - next.ServeHTTP(w, r) - zap.L().Info(path, zap.Duration("timeTaken", time.Since(startTime)), zap.String("path", path)) - }) -} - -// TODO(remove): Implemented at pkg/http/middleware/logging.go -// loggingMiddlewarePrivate is used for logging private api calls -// from internal services like alert manager -func loggingMiddlewarePrivate(next http.Handler) http.Handler { - return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - route := mux.CurrentRoute(r) - path, _ := route.GetPathTemplate() - startTime := time.Now() - next.ServeHTTP(w, r) - zap.L().Info(path, zap.Duration("timeTaken", time.Since(startTime)), zap.String("path", path), zap.Bool("tprivatePort", true)) - }) -} - // TODO(remove): Implemented at pkg/http/middleware/logging.go type loggingResponseWriter struct { http.ResponseWriter diff --git a/pkg/http/middleware/logging.go b/pkg/http/middleware/logging.go index ef755f66485..50fb4cd06b4 100644 --- a/pkg/http/middleware/logging.go +++ b/pkg/http/middleware/logging.go @@ -31,7 +31,6 @@ func NewLogging(logger *zap.Logger) *Logging { func (middleware *Logging) Wrap(next http.Handler) http.Handler { return http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { - ctx := req.Context() start := time.Now() host, port, _ := net.SplitHostPort(req.Host) path, err := mux.CurrentRoute(req).GetPathTemplate() @@ -40,7 +39,6 @@ func (middleware *Logging) Wrap(next http.Handler) http.Handler { } fields := []zap.Field{ - zap.Any("context", ctx), zap.String(string(semconv.ClientAddressKey), req.RemoteAddr), zap.String(string(semconv.UserAgentOriginalKey), req.UserAgent()), zap.String(string(semconv.ServerAddressKey), host), @@ -49,8 +47,8 @@ func (middleware *Logging) Wrap(next http.Handler) http.Handler { zap.String(string(semconv.HTTPRouteKey), path), } - buf := new(bytes.Buffer) - writer := newBadResponseLoggingWriter(rw, buf) + badResponseBuffer := new(bytes.Buffer) + writer := newBadResponseLoggingWriter(rw, badResponseBuffer) next.ServeHTTP(writer, req) statusCode, err := writer.StatusCode(), writer.WriteError() @@ -62,8 +60,9 @@ func (middleware *Logging) Wrap(next http.Handler) http.Handler { fields = append(fields, zap.Error(err)) middleware.logger.Error(logMessage, fields...) } else { - if buf.Len() != 0 { - fields = append(fields, zap.String("response.body", buf.String())) + // when the status code is 400 or >=500, and the response body is not empty. + if badResponseBuffer.Len() != 0 { + fields = append(fields, zap.String("response.body", badResponseBuffer.String())) } middleware.logger.Info(logMessage, fields...) diff --git a/pkg/query-service/app/server.go b/pkg/query-service/app/server.go index 4d4147f884a..9b265efcd9c 100644 --- a/pkg/query-service/app/server.go +++ b/pkg/query-service/app/server.go @@ -23,6 +23,7 @@ import ( "github.com/rs/cors" "github.com/soheilhy/cmux" + "go.signoz.io/signoz/pkg/http/middleware" "go.signoz.io/signoz/pkg/query-service/agentConf" "go.signoz.io/signoz/pkg/query-service/app/clickhouseReader" "go.signoz.io/signoz/pkg/query-service/app/cloudintegrations" @@ -263,7 +264,7 @@ func (s *Server) createPrivateServer(api *APIHandler) (*http.Server, error) { r.Use(setTimeoutMiddleware) r.Use(s.analyticsMiddleware) - r.Use(loggingMiddlewarePrivate) + r.Use(middleware.NewLogging(zap.L()).Wrap) api.RegisterPrivateRoutes(r) @@ -289,7 +290,7 @@ func (s *Server) createPublicServer(api *APIHandler, web web.Web) (*http.Server, r.Use(setTimeoutMiddleware) r.Use(s.analyticsMiddleware) - r.Use(loggingMiddleware) + r.Use(middleware.NewLogging(zap.L()).Wrap) r.Use(LogCommentEnricher) // add auth middleware @@ -338,18 +339,6 @@ func (s *Server) createPublicServer(api *APIHandler, web web.Web) (*http.Server, }, nil } -// TODO(remove): Implemented at pkg/http/middleware/logging.go -// loggingMiddleware is used for logging public api calls -func loggingMiddleware(next http.Handler) http.Handler { - return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - route := mux.CurrentRoute(r) - path, _ := route.GetPathTemplate() - startTime := time.Now() - next.ServeHTTP(w, r) - zap.L().Info(path, zap.Duration("timeTaken", time.Since(startTime)), zap.String("path", path)) - }) -} - func LogCommentEnricher(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { referrer := r.Header.Get("Referer") @@ -412,19 +401,6 @@ func LogCommentEnricher(next http.Handler) http.Handler { }) } -// TODO(remove): Implemented at pkg/http/middleware/logging.go -// loggingMiddlewarePrivate is used for logging private api calls -// from internal services like alert manager -func loggingMiddlewarePrivate(next http.Handler) http.Handler { - return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - route := mux.CurrentRoute(r) - path, _ := route.GetPathTemplate() - startTime := time.Now() - next.ServeHTTP(w, r) - zap.L().Info(path, zap.Duration("timeTaken", time.Since(startTime)), zap.String("path", path), zap.Bool("privatePort", true)) - }) -} - // TODO(remove): Implemented at pkg/http/middleware/logging.go type loggingResponseWriter struct { http.ResponseWriter