diff --git a/app/platform/config.go b/app/platform/config.go index fd5bb05f76..e8eb425b96 100644 --- a/app/platform/config.go +++ b/app/platform/config.go @@ -75,12 +75,6 @@ func (ps *PlatformService) SaveConfig(newCfg *model.Config, sendConfigChangeClus return nil, nil, model.NewAppError("saveConfig", "app.save_config.app_error", nil, "", http.StatusInternalServerError).Wrap(err) } - if ps.startMetrics && *ps.Config().MetricsSettings.Enable { - ps.RestartMetrics() - } else { - ps.ShutdownMetrics() - } - if ps.clusterIFace != nil { err := ps.clusterIFace.ConfigChanged(ps.configStore.RemoveEnvironmentOverrides(oldCfg), ps.configStore.RemoveEnvironmentOverrides(newCfg), sendConfigChangeClusterMessage) diff --git a/app/platform/config_test.go b/app/platform/config_test.go index af99b47d89..8268b32929 100644 --- a/app/platform/config_test.go +++ b/app/platform/config_test.go @@ -70,4 +70,31 @@ func TestConfigSave(t *testing.T) { updatedCfg := th.Service.Config() assert.Equal(t, "http://newhost.me", *updatedCfg.ServiceSettings.SiteURL) }) + + t.Run("do not restart the metrics server on a different type of config change", func(t *testing.T) { + th := Setup(t, StartMetrics()) + defer th.TearDown() + + metricsMock := &mocks.MetricsInterface{} + metricsMock.On("IncrementWebsocketEvent", mock.AnythingOfType("string")).Return() + metricsMock.On("IncrementWebSocketBroadcastBufferSize", mock.AnythingOfType("string"), mock.AnythingOfType("float64")).Return() + metricsMock.On("DecrementWebSocketBroadcastBufferSize", mock.AnythingOfType("string"), mock.AnythingOfType("float64")).Return() + metricsMock.On("Register").Return() + th.Service.metricsIFace = metricsMock + + // Change a random config setting + cfg := th.Service.Config().Clone() + cfg.ThemeSettings.EnableThemeSelection = model.NewBool(!*cfg.ThemeSettings.EnableThemeSelection) + th.Service.SaveConfig(cfg, false) + metricsMock.AssertNumberOfCalls(t, "Register", 0) + + // Disable metrics + cfg.MetricsSettings.Enable = model.NewBool(false) + th.Service.SaveConfig(cfg, false) + + // Change the metrics setting + cfg.MetricsSettings.Enable = model.NewBool(true) + th.Service.SaveConfig(cfg, false) + metricsMock.AssertNumberOfCalls(t, "Register", 1) + }) } diff --git a/app/platform/service.go b/app/platform/service.go index b2f02c2c77..857e14e3c8 100644 --- a/app/platform/service.go +++ b/app/platform/service.go @@ -268,6 +268,14 @@ func New(sc ServiceConfig, options ...Option) (*PlatformService, error) { if mErr := ps.resetMetrics(); mErr != nil { return nil, mErr } + + ps.configStore.AddListener(func(oldCfg, newCfg *model.Config) { + if *oldCfg.MetricsSettings.Enable != *newCfg.MetricsSettings.Enable || *oldCfg.MetricsSettings.ListenAddress != *newCfg.MetricsSettings.ListenAddress { + if mErr := ps.resetMetrics(); mErr != nil { + mlog.Warn("Failed to reset metrics", mlog.Err(mErr)) + } + } + }) } // Step 9: Init AsymmetricSigningKey depends on step 6 (store) diff --git a/app/platform/service_test.go b/app/platform/service_test.go index 11633b2d5b..8654b18685 100644 --- a/app/platform/service_test.go +++ b/app/platform/service_test.go @@ -105,10 +105,9 @@ func TestMetrics(t *testing.T) { // there is no config listener for the metrics // we handle it on config save step - th.Service.UpdateConfig(func(c *model.Config) { - c.MetricsSettings.Enable = model.NewBool(true) - }) - th.Service.SaveConfig(th.Service.Config(), false) + cfg := th.Service.Config().Clone() + cfg.MetricsSettings.Enable = model.NewBool(true) + th.Service.SaveConfig(cfg, false) require.NotNil(t, th.Service.metrics) metricsAddr := strings.Replace(th.Service.metrics.listenAddr, "[::]", "http://localhost", 1) @@ -117,17 +116,14 @@ func TestMetrics(t *testing.T) { require.NoError(t, err) require.Equal(t, http.StatusOK, resp.StatusCode) - th.Service.UpdateConfig(func(c *model.Config) { - c.MetricsSettings.Enable = model.NewBool(false) - }) - th.Service.SaveConfig(th.Service.Config(), false) + cfg.MetricsSettings.Enable = model.NewBool(false) + th.Service.SaveConfig(cfg, false) _, err = http.Get(metricsAddr) require.Error(t, err) }) t.Run("ensure the metrics server is started with advanced metrics", func(t *testing.T) { - t.Skip("MM-47635") th := Setup(t, StartMetrics()) defer th.TearDown()