From d6a8ad0d55b0546d559331845afad3f37fa82f69 Mon Sep 17 00:00:00 2001 From: Devin Binnie <52460000+devinbinnie@users.noreply.github.com> Date: Thu, 9 May 2024 11:30:42 -0400 Subject: [PATCH] [MM-58159] Add admin setting for notification monitoring alongside feature flag (#26979) * [MM-58159] Add admin setting for notification monitoring alongside feature flag * Use helper function --- server/channels/app/notification.go | 30 +++++++++---------- server/channels/app/web_broadcast_hooks.go | 2 +- server/config/client.go | 1 + server/public/model/config.go | 13 +++++--- .../admin_console/admin_definition.tsx | 10 +++++++ webapp/channels/src/i18n/en.json | 2 ++ 6 files changed, 38 insertions(+), 20 deletions(-) diff --git a/server/channels/app/notification.go b/server/channels/app/notification.go index 8f11936b3f..9b243ca05f 100644 --- a/server/channels/app/notification.go +++ b/server/channels/app/notification.go @@ -1736,11 +1736,7 @@ func ShouldAckWebsocketNotification(channelType model.ChannelType, userNotificat } func (a *App) CountNotification(notificationType model.NotificationType) { - if a.Metrics() == nil { - return - } - - if !a.Config().FeatureFlags.NotificationMonitoring { + if a.notificationMetricsDisabled() { return } @@ -1748,11 +1744,7 @@ func (a *App) CountNotification(notificationType model.NotificationType) { } func (a *App) CountNotificationAck(notificationType model.NotificationType) { - if a.Metrics() == nil { - return - } - - if !a.Config().FeatureFlags.NotificationMonitoring { + if a.notificationMetricsDisabled() { return } @@ -1764,11 +1756,7 @@ func (a *App) CountNotificationReason( notificationType model.NotificationType, notificationReason model.NotificationReason, ) { - if a.Metrics() == nil { - return - } - - if !a.Config().FeatureFlags.NotificationMonitoring { + if a.notificationMetricsDisabled() { return } @@ -1783,3 +1771,15 @@ func (a *App) CountNotificationReason( a.Metrics().IncrementNotificationUnsupportedCounter(notificationType, notificationReason) } } + +func (a *App) notificationMetricsDisabled() bool { + if a.Metrics() == nil { + return true + } + + if a.Config().FeatureFlags.NotificationMonitoring && *a.Config().MetricsSettings.EnableNotificationMetrics { + return false + } + + return true +} diff --git a/server/channels/app/web_broadcast_hooks.go b/server/channels/app/web_broadcast_hooks.go index fbabcc9dcf..02a5913145 100644 --- a/server/channels/app/web_broadcast_hooks.go +++ b/server/channels/app/web_broadcast_hooks.go @@ -135,7 +135,7 @@ func incrementWebsocketCounter(wc *platform.WebConn) { return } - if !wc.Platform.Config().FeatureFlags.NotificationMonitoring { + if !(wc.Platform.Config().FeatureFlags.NotificationMonitoring && *wc.Platform.Config().MetricsSettings.EnableNotificationMetrics) { return } diff --git a/server/config/client.go b/server/config/client.go index fff0f3d5fe..70dd750d41 100644 --- a/server/config/client.go +++ b/server/config/client.go @@ -188,6 +188,7 @@ func GenerateClientConfig(c *model.Config, telemetryID string, license *model.Li if *license.Features.Cluster { props["EnableMetrics"] = strconv.FormatBool(*c.MetricsSettings.Enable) props["EnableClientMetrics"] = strconv.FormatBool(c.FeatureFlags.ClientMetrics && *c.MetricsSettings.EnableClientMetrics) + props["EnableNotificationMetrics"] = strconv.FormatBool(c.FeatureFlags.NotificationMonitoring && *c.MetricsSettings.EnableNotificationMetrics) } if *license.Features.Announcement { diff --git a/server/public/model/config.go b/server/public/model/config.go index df27045548..8be987db12 100644 --- a/server/public/model/config.go +++ b/server/public/model/config.go @@ -980,10 +980,11 @@ func (s *ClusterSettings) SetDefaults() { } type MetricsSettings struct { - Enable *bool `access:"environment_performance_monitoring,write_restrictable,cloud_restrictable"` - BlockProfileRate *int `access:"environment_performance_monitoring,write_restrictable,cloud_restrictable"` - ListenAddress *string `access:"environment_performance_monitoring,write_restrictable,cloud_restrictable"` // telemetry: none - EnableClientMetrics *bool `access:"environment_performance_monitoring,write_restrictable,cloud_restrictable"` + Enable *bool `access:"environment_performance_monitoring,write_restrictable,cloud_restrictable"` + BlockProfileRate *int `access:"environment_performance_monitoring,write_restrictable,cloud_restrictable"` + ListenAddress *string `access:"environment_performance_monitoring,write_restrictable,cloud_restrictable"` // telemetry: none + EnableClientMetrics *bool `access:"environment_performance_monitoring,write_restrictable,cloud_restrictable"` + EnableNotificationMetrics *bool `access:"environment_performance_monitoring,write_restrictable,cloud_restrictable"` } func (s *MetricsSettings) SetDefaults() { @@ -1002,6 +1003,10 @@ func (s *MetricsSettings) SetDefaults() { if s.EnableClientMetrics == nil { s.EnableClientMetrics = NewBool(true) } + + if s.EnableNotificationMetrics == nil { + s.EnableNotificationMetrics = NewBool(true) + } } type ExperimentalSettings struct { diff --git a/webapp/channels/src/components/admin_console/admin_definition.tsx b/webapp/channels/src/components/admin_console/admin_definition.tsx index 8ef006f27c..21e99e673f 100644 --- a/webapp/channels/src/components/admin_console/admin_definition.tsx +++ b/webapp/channels/src/components/admin_console/admin_definition.tsx @@ -2376,6 +2376,16 @@ const AdminDefinition: AdminDefinitionType = { isHidden: it.not(it.licensedForFeature('IDLoadedPushNotifications')), isDisabled: it.not(it.userHasWritePermissionOnResource(RESOURCE_KEYS.SITE.NOTIFICATIONS)), }, + { + type: 'bool', + key: 'MetricsSettings.EnableNotificationMetrics', + label: defineMessage({id: 'admin.metrics.enableNotificationMetricsTitle', defaultMessage: 'Enable Notification Monitoring:'}), + help_text: defineMessage({id: 'admin.metrics.enableNotificationMetricsDescription', defaultMessage: 'When true, Mattermost will enable notification data collection for web and Desktop App users.'}), + isDisabled: it.any( + it.configIsFalse('MetricsSettings', 'Enable'), + ), + isHidden: it.configIsFalse('FeatureFlags', 'NotificationMonitoring'), + }, ], }, }, diff --git a/webapp/channels/src/i18n/en.json b/webapp/channels/src/i18n/en.json index 7a63389835..9510ddd4a9 100644 --- a/webapp/channels/src/i18n/en.json +++ b/webapp/channels/src/i18n/en.json @@ -1489,6 +1489,8 @@ "admin.metrics.enableClientMetricsDescription": "When true, Mattermost will enable performance monitoring collection for web and desktop app users. Please see documentation to learn more about configuring performance monitoring for Mattermost.", "admin.metrics.enableClientMetricsTitle": "Enable Client Performance Monitoring:", "admin.metrics.enableDescription": "When true, Mattermost will enable performance monitoring collection and profiling. Please see documentation to learn more about configuring performance monitoring for Mattermost.", + "admin.metrics.enableNotificationMetricsDescription": "When true, Mattermost will enable notification data collection for web and Desktop App users.", + "admin.metrics.enableNotificationMetricsTitle": "Enable Notification Monitoring:", "admin.metrics.enableTitle": "Enable Performance Monitoring:", "admin.metrics.listenAddressDesc": "The address the server will listen on to expose performance metrics.", "admin.metrics.listenAddressEx": "E.g.: \":8067\"",