diff --git a/server/channels/api4/metrics_test.go b/server/channels/api4/metrics_test.go index 73026b8c55..810326b645 100644 --- a/server/channels/api4/metrics_test.go +++ b/server/channels/api4/metrics_test.go @@ -155,4 +155,67 @@ func TestSubmitMetrics(t *testing.T) { require.NoError(t, err) require.Equal(t, http.StatusOK, resp.StatusCode) }) + + t.Run("metrics recorded for API errors", func(t *testing.T) { + metricsMock := setupMetricsMock() + metricsMock.On("IncrementClientLongTasks", mock.AnythingOfType("string"), mock.AnythingOfType("string"), mock.AnythingOfType("float64")).Return() + + platform.RegisterMetricsInterface(func(_ *platform.PlatformService, _, _ string) einterfaces.MetricsInterface { + return metricsMock + }) + t.Cleanup(func() { + platform.RegisterMetricsInterface(func(_ *platform.PlatformService, _, _ string) einterfaces.MetricsInterface { + return nil + }) + }) + + th := SetupEnterpriseWithServerOptions(t, []app.Option{app.StartMetrics}) + defer th.TearDown() + + // enable metrics and add the license + th.App.Srv().SetLicense(model.NewTestLicense()) + th.App.UpdateConfig(func(cfg *model.Config) { *cfg.MetricsSettings.Enable = true }) + th.App.UpdateConfig(func(cfg *model.Config) { *cfg.MetricsSettings.ListenAddress = ":0" }) + + _, resp, err := th.Client.CreatePost(th.Context.Context(), &model.Post{ + ChannelId: model.NewId(), + }) + + require.Error(t, err) + require.Equal(t, http.StatusForbidden, resp.StatusCode) + metricsMock.AssertCalled(t, "IncrementHTTPRequest") + metricsMock.AssertCalled(t, "IncrementHTTPError") + }) + + t.Run("metrics recorded for URL length limit errors", func(t *testing.T) { + metricsMock := setupMetricsMock() + metricsMock.On("IncrementClientLongTasks", mock.AnythingOfType("string"), mock.AnythingOfType("string"), mock.AnythingOfType("float64")).Return() + + platform.RegisterMetricsInterface(func(_ *platform.PlatformService, _, _ string) einterfaces.MetricsInterface { + return metricsMock + }) + t.Cleanup(func() { + platform.RegisterMetricsInterface(func(_ *platform.PlatformService, _, _ string) einterfaces.MetricsInterface { + return nil + }) + }) + + th := SetupEnterpriseWithServerOptions(t, []app.Option{app.StartMetrics}) + defer th.TearDown() + + // enable metrics and add the license + th.App.Srv().SetLicense(model.NewTestLicense()) + th.App.UpdateConfig(func(cfg *model.Config) { *cfg.MetricsSettings.Enable = true }) + th.App.UpdateConfig(func(cfg *model.Config) { *cfg.MetricsSettings.ListenAddress = ":0" }) + th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.MaximumURLLength = 1 }) + + _, resp, err := th.Client.CreatePost(th.Context.Context(), &model.Post{ + ChannelId: model.NewId(), + }) + + require.Error(t, err) + require.Equal(t, http.StatusRequestURITooLong, resp.StatusCode) + metricsMock.AssertCalled(t, "IncrementHTTPRequest") + metricsMock.AssertCalled(t, "IncrementHTTPError") + }) } diff --git a/server/channels/web/handlers.go b/server/channels/web/handlers.go index c4f13cfc86..f257232160 100644 --- a/server/channels/web/handlers.go +++ b/server/channels/web/handlers.go @@ -164,7 +164,7 @@ func (h Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { } requestID := model.NewId() - var statusCode string + var rateLimitExceeded bool defer func() { responseLogFields := []mlog.Field{ mlog.String("method", r.Method), @@ -175,11 +175,18 @@ func (h Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { if c.AppContext.Session() != nil { responseLogFields = append(responseLogFields, mlog.String("user_id", c.AppContext.Session().UserId)) } + + statusCode := strconv.Itoa(w.(*responseWriterWrapper).StatusCode()) + // Websockets are returning status code 0 to requests after closing the socket if statusCode != "0" { responseLogFields = append(responseLogFields, mlog.String("status_code", statusCode)) } mlog.Debug("Received HTTP request", responseLogFields...) + + if !rateLimitExceeded { + h.recordMetrics(c, r, now, statusCode) + } }() t, _ := i18n.GetTranslationsAndLocaleFromRequest(r) @@ -302,8 +309,11 @@ func (h Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { } // Rate limit by UserID - if c.App.Srv().RateLimiter != nil && c.App.Srv().RateLimiter.UserIdRateLimit(c.AppContext.Session().UserId, w) { - return + if c.App.Srv().RateLimiter != nil { + rateLimitExceeded = c.App.Srv().RateLimiter.UserIdRateLimit(c.AppContext.Session().UserId, w) + if rateLimitExceeded { + return + } } h.checkCSRFToken(c, r, token, tokenLocation, session) @@ -382,8 +392,9 @@ func (h Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { h.handleContextError(c, w, r) return } +} - statusCode = strconv.Itoa(w.(*responseWriterWrapper).StatusCode()) +func (h Handler) recordMetrics(c *Context, r *http.Request, now time.Time, statusCode string) { if c.App.Metrics() != nil { c.App.Metrics().IncrementHTTPRequest()