From b544a54c40264f9e9ada7debf380cb3d205ffb06 Mon Sep 17 00:00:00 2001 From: Nityananda Gohain Date: Fri, 24 Jan 2025 14:43:28 +0530 Subject: [PATCH] fix: logging middleware add excluded routes (#6917) * fix: logging middleware add excluded routes * fix: consistant name and update example * fix: consistant name --- conf/example.yaml | 5 ++++- ee/query-service/app/server.go | 4 ++-- pkg/apiserver/config.go | 11 +++++++++++ pkg/apiserver/config_test.go | 6 ++++++ pkg/http/middleware/logging.go | 18 +++++++++++++++--- pkg/query-service/app/server.go | 4 ++-- 6 files changed, 40 insertions(+), 8 deletions(-) diff --git a/conf/example.yaml b/conf/example.yaml index d57cd7f91e..be7731976e 100644 --- a/conf/example.yaml +++ b/conf/example.yaml @@ -77,4 +77,7 @@ apiserver: max: 600s excluded_routes: - /api/v1/logs/tail - - /api/v3/logs/livetail \ No newline at end of file + - /api/v3/logs/livetail + logging: + excluded_routes: + - /api/v1/health \ No newline at end of file diff --git a/ee/query-service/app/server.go b/ee/query-service/app/server.go index 666be0e940..61c081cb79 100644 --- a/ee/query-service/app/server.go +++ b/ee/query-service/app/server.go @@ -330,7 +330,7 @@ func (s *Server) createPrivateServer(apiHandler *api.APIHandler) (*http.Server, s.serverOptions.Config.APIServer.Timeout.Max, ).Wrap) r.Use(middleware.NewAnalytics(zap.L()).Wrap) - r.Use(middleware.NewLogging(zap.L()).Wrap) + r.Use(middleware.NewLogging(zap.L(), s.serverOptions.Config.APIServer.Logging.ExcludedRoutes).Wrap) apiHandler.RegisterPrivateRoutes(r) @@ -376,7 +376,7 @@ func (s *Server) createPublicServer(apiHandler *api.APIHandler, web web.Web) (*h s.serverOptions.Config.APIServer.Timeout.Max, ).Wrap) r.Use(middleware.NewAnalytics(zap.L()).Wrap) - r.Use(middleware.NewLogging(zap.L()).Wrap) + r.Use(middleware.NewLogging(zap.L(), s.serverOptions.Config.APIServer.Logging.ExcludedRoutes).Wrap) apiHandler.RegisterRoutes(r, am) apiHandler.RegisterLogsRoutes(r, am) diff --git a/pkg/apiserver/config.go b/pkg/apiserver/config.go index bc0e0745fe..79a4259589 100644 --- a/pkg/apiserver/config.go +++ b/pkg/apiserver/config.go @@ -9,6 +9,7 @@ import ( // Config holds the configuration for config. type Config struct { Timeout Timeout `mapstructure:"timeout"` + Logging Logging `mapstructure:"logging"` } type Timeout struct { @@ -20,6 +21,11 @@ type Timeout struct { ExcludedRoutes []string `mapstructure:"excluded_routes"` } +type Logging struct { + // The list of routes that are excluded from the logging + ExcludedRoutes []string `mapstructure:"excluded_routes"` +} + func NewConfigFactory() factory.ConfigFactory { return factory.NewConfigFactory(factory.MustNewName("apiserver"), newConfig) } @@ -34,6 +40,11 @@ func newConfig() factory.Config { "/api/v3/logs/livetail", }, }, + Logging: Logging{ + ExcludedRoutes: []string{ + "/api/v1/health", + }, + }, } } diff --git a/pkg/apiserver/config_test.go b/pkg/apiserver/config_test.go index 728fd2ac0f..373deaae28 100644 --- a/pkg/apiserver/config_test.go +++ b/pkg/apiserver/config_test.go @@ -16,6 +16,7 @@ func TestNewWithEnvProvider(t *testing.T) { t.Setenv("SIGNOZ_APISERVER_TIMEOUT_DEFAULT", "70s") t.Setenv("SIGNOZ_APISERVER_TIMEOUT_MAX", "700s") t.Setenv("SIGNOZ_APISERVER_TIMEOUT_EXCLUDED__ROUTES", "/excluded1,/excluded2") + t.Setenv("SIGNOZ_APISERVER_LOGGING_EXCLUDED__ROUTES", "/api/v1/health1") conf, err := config.New( context.Background(), @@ -45,6 +46,11 @@ func TestNewWithEnvProvider(t *testing.T) { "/excluded2", }, }, + Logging: Logging{ + ExcludedRoutes: []string{ + "/api/v1/health1", + }, + }, } assert.Equal(t, expected, actual) diff --git a/pkg/http/middleware/logging.go b/pkg/http/middleware/logging.go index 278a170406..10b608d94c 100644 --- a/pkg/http/middleware/logging.go +++ b/pkg/http/middleware/logging.go @@ -21,16 +21,23 @@ const ( ) type Logging struct { - logger *zap.Logger + logger *zap.Logger + excludedRoutes map[string]struct{} } -func NewLogging(logger *zap.Logger) *Logging { +func NewLogging(logger *zap.Logger, excludedRoutes []string) *Logging { if logger == nil { panic("cannot build logging, logger is empty") } + excludedRoutesMap := make(map[string]struct{}) + for _, route := range excludedRoutes { + excludedRoutesMap[route] = struct{}{} + } + return &Logging{ - logger: logger.Named(pkgname), + logger: logger.Named(pkgname), + excludedRoutes: excludedRoutesMap, } } @@ -59,6 +66,11 @@ func (middleware *Logging) Wrap(next http.Handler) http.Handler { writer := newBadResponseLoggingWriter(rw, badResponseBuffer) next.ServeHTTP(writer, req) + // if the path is in the excludedRoutes map, don't log + if _, ok := middleware.excludedRoutes[path]; ok { + return + } + statusCode, err := writer.StatusCode(), writer.WriteError() fields = append(fields, zap.Int(string(semconv.HTTPResponseStatusCodeKey), statusCode), diff --git a/pkg/query-service/app/server.go b/pkg/query-service/app/server.go index d5e54ba9d7..8a1cfb0b74 100644 --- a/pkg/query-service/app/server.go +++ b/pkg/query-service/app/server.go @@ -275,7 +275,7 @@ func (s *Server) createPrivateServer(api *APIHandler) (*http.Server, error) { s.serverOptions.Config.APIServer.Timeout.Max, ).Wrap) r.Use(middleware.NewAnalytics(zap.L()).Wrap) - r.Use(middleware.NewLogging(zap.L()).Wrap) + r.Use(middleware.NewLogging(zap.L(), s.serverOptions.Config.APIServer.Logging.ExcludedRoutes).Wrap) api.RegisterPrivateRoutes(r) @@ -305,7 +305,7 @@ func (s *Server) createPublicServer(api *APIHandler, web web.Web) (*http.Server, s.serverOptions.Config.APIServer.Timeout.Max, ).Wrap) r.Use(middleware.NewAnalytics(zap.L()).Wrap) - r.Use(middleware.NewLogging(zap.L()).Wrap) + r.Use(middleware.NewLogging(zap.L(), s.serverOptions.Config.APIServer.Logging.ExcludedRoutes).Wrap) // add auth middleware getUserFromRequest := func(r *http.Request) (*model.UserPayload, error) {