mirror of
https://github.com/mattermost/mattermost.git
synced 2026-09-19 02:06:37 +08:00
[MM-69590] Remove NotificationMonitoring feature flag (#37386)
* [MM-69590] Remove NotificationMonitoring feature flag The NotificationMonitoring feature flag shipped and defaulted true in v9.9. Remove the flag and all conditional gating so notification delivery metrics collection is permanently enabled, gated only by the MetricsSettings.EnableNotificationMetrics admin setting. Co-authored-by: mattermost-code <matty-code@mattermost.com> * [MM-69590] Cover notification metrics client config Assert EnableNotificationMetrics client config prop tracks the MetricsSettings.EnableNotificationMetrics admin setting now that the NotificationMonitoring feature flag gate is removed. Co-authored-by: mattermost-code <matty-code@mattermost.com> * [MM-69590] Cover notification counter gating Add app-layer coverage asserting CountNotification increments the notification counter only when MetricsSettings.EnableNotificationMetrics is set, exercising the un-gated notificationMetricsDisabled path. Co-authored-by: mattermost-code <matty-code@mattermost.com> * [MM-69590] Cover websocket notification counter gating Assert the websocket notification counter increments via the posted-ack broadcast hook only when MetricsSettings.EnableNotificationMetrics is set, exercising the un-gated incrementWebsocketCounter path. Co-authored-by: mattermost-code <matty-code@mattermost.com> * Address PR feedback: 0 answered, 1 resolved, 0 declined Document that MetricsSettings.EnableNotificationMetrics must be set to true for notification monitoring, matching the code gating and the push-notification-health-targets doc. --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: mattermost-code <matty-code@mattermost.com>
This commit is contained in:
co-authored by
Cursor Agent
mattermost-code
parent
fe7fd4ebec
commit
7870605fb1
@@ -3426,7 +3426,7 @@ See the [performance monitoring](/administration-guide/scale/deploy-prometheus-g
|
||||
<Note>
|
||||
|
||||
- `MetricsSettings.Enable` must be set to `true`
|
||||
- The `NotificationMonitoring` feature flag must be set to `true`
|
||||
- `MetricsSettings.EnableNotificationMetrics` must be set to `true`
|
||||
|
||||
</Note>
|
||||
|
||||
|
||||
+1
-1
@@ -117,7 +117,7 @@ See [this Grafana guide](https://grafana.com/docs/grafana/v7.5/dashboards/export
|
||||
</Tip>
|
||||
|
||||
- [Mattermost Performance Monitoring v2](https://grafana.com/grafana/dashboards/15582-mattermost-performance-monitoring-v2/), which contains detailed charts for performance monitoring including application, cluster, job server, and system metrics.
|
||||
- [Mattermost Notification Health Monitoring](https://grafana.com/grafana/dashboards/21305-mattermost-notification-health/), which can be used to track different types of notifications sent from Mattermost. Accessing and enabling Mattermost Notification Health Monitoring requires the feature flag `NotificationMonitoring` to be set to `true`. System admins can [disable notification monitoring data collection](/administration-guide/configure/site-configuration-settings#enable-notification-monitoring) through the System Console.
|
||||
- [Mattermost Notification Health Monitoring](https://grafana.com/grafana/dashboards/21305-mattermost-notification-health/), which can be used to track different types of notifications sent from Mattermost. System admins can [disable notification monitoring data collection](/administration-guide/configure/site-configuration-settings#enable-notification-monitoring) through the System Console.
|
||||
- [Mattermost Web App Performance Metrics](https://grafana.com/grafana/dashboards/21460-web-app-metrics/), which contains detailed metrics for client-side performance, including web vitals and Mattermost-specifc metrics.
|
||||
- [Mattermost Desktop App Performance Metrics](https://grafana.com/grafana/dashboards/22736-desktop-app-metrics/), which contains detailed metrics for client-side desktop performance, including CPU and memory usage metrics.
|
||||
- [Mattermost Mobile App Performance Metrics](https://grafana.com/grafana/dashboards/21695-mobile-performance-metrics/), which contains detailed metrics for client-side mobile performance, including web vitals and Mattermost-specifc metrics.
|
||||
|
||||
@@ -7,7 +7,7 @@ When using the [Mattermost Notification Health](https://grafana.com/grafana/dash
|
||||
|
||||
<Note>
|
||||
|
||||
- Accessing and enabling Mattermost Notification Health Monitoring requires `MetricsSettings.Enable` set to `true`, and the feature flag `NotificationMonitoring` set to `true`.
|
||||
- Accessing and enabling Mattermost Notification Health Monitoring requires `MetricsSettings.Enable` set to `true`.
|
||||
- `MetricsSettings.EnableNotificationMetrics` must be enabled in the [Performance Monitoring](/administration-guide/configure/environment-configuration-settings#enable-notification-monitoring) configuration.
|
||||
- System admins can [disable notification monitoring data collection](/administration-guide/configure/site-configuration-settings#enable-notification-monitoring) through the System Console.
|
||||
|
||||
|
||||
@@ -802,7 +802,6 @@ const defaultServerConfig: AdminConfig = {
|
||||
StreamlinedMarketplace: true,
|
||||
CloudDedicatedExportUI: false,
|
||||
WebSocketEventScope: true,
|
||||
NotificationMonitoring: true,
|
||||
ExperimentalAuditSettingsSystemConsoleUI: true,
|
||||
CustomProfileAttributes: true,
|
||||
AttributeBasedAccessControl: true,
|
||||
|
||||
@@ -8,6 +8,7 @@ import (
|
||||
"testing"
|
||||
|
||||
"github.com/mattermost/mattermost/server/public/model"
|
||||
"github.com/mattermost/mattermost/server/v8/channels/app/platform"
|
||||
"github.com/mattermost/mattermost/server/v8/enterprise/metrics"
|
||||
"github.com/prometheus/client_golang/prometheus"
|
||||
prometheusModels "github.com/prometheus/client_model/go"
|
||||
@@ -100,3 +101,109 @@ func TestMobileMetrics(t *testing.T) {
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestCountNotificationMetrics(t *testing.T) {
|
||||
mainHelper.Parallel(t)
|
||||
th := SetupEnterprise(t, StartMetrics)
|
||||
|
||||
configureMetrics(th)
|
||||
mi := th.App.Metrics()
|
||||
|
||||
miImpl, ok := mi.(*metrics.MetricsInterfaceImpl)
|
||||
require.True(t, ok, fmt.Sprintf("App.Metrics is not *MetricsInterfaceImpl, but %T", mi))
|
||||
|
||||
counterValue := func() float64 {
|
||||
counter, err := miImpl.NotificationTotalCounters.GetMetricWith(prometheus.Labels{
|
||||
"type": string(model.NotificationTypePush),
|
||||
"platform": "ios",
|
||||
})
|
||||
require.NoError(t, err)
|
||||
m := &prometheusModels.Metric{}
|
||||
require.NoError(t, counter.Write(m))
|
||||
return m.Counter.GetValue()
|
||||
}
|
||||
|
||||
t.Run("counts when notification metrics are enabled", func(t *testing.T) {
|
||||
th.App.UpdateConfig(func(cfg *model.Config) {
|
||||
*cfg.MetricsSettings.EnableNotificationMetrics = true
|
||||
})
|
||||
|
||||
before := counterValue()
|
||||
th.App.CountNotification(model.NotificationTypePush, "ios")
|
||||
require.Equal(t, before+1, counterValue())
|
||||
})
|
||||
|
||||
t.Run("does not count when notification metrics are disabled", func(t *testing.T) {
|
||||
th.App.UpdateConfig(func(cfg *model.Config) {
|
||||
*cfg.MetricsSettings.EnableNotificationMetrics = false
|
||||
})
|
||||
|
||||
before := counterValue()
|
||||
th.App.CountNotification(model.NotificationTypePush, "ios")
|
||||
require.Equal(t, before, counterValue())
|
||||
})
|
||||
}
|
||||
|
||||
func TestWebsocketNotificationCounter(t *testing.T) {
|
||||
mainHelper.Parallel(t)
|
||||
th := SetupEnterprise(t, StartMetrics)
|
||||
|
||||
configureMetrics(th)
|
||||
mi := th.App.Metrics()
|
||||
|
||||
miImpl, ok := mi.(*metrics.MetricsInterfaceImpl)
|
||||
require.True(t, ok, fmt.Sprintf("App.Metrics is not *MetricsInterfaceImpl, but %T", mi))
|
||||
|
||||
counterValue := func() float64 {
|
||||
counter, err := miImpl.NotificationTotalCounters.GetMetricWith(prometheus.Labels{
|
||||
"type": string(model.NotificationTypeWebsocket),
|
||||
"platform": model.NotificationNoPlatform,
|
||||
})
|
||||
require.NoError(t, err)
|
||||
m := &prometheusModels.Metric{}
|
||||
require.NoError(t, counter.Write(m))
|
||||
return m.Counter.GetValue()
|
||||
}
|
||||
|
||||
hook := &postedAckBroadcastHook{}
|
||||
userID := model.NewId()
|
||||
webConn := &platform.WebConn{
|
||||
UserId: userID,
|
||||
Platform: th.Server.Platform(),
|
||||
PostedAck: true,
|
||||
}
|
||||
webConn.Active.Store(true)
|
||||
webConn.SetSession(&model.Session{})
|
||||
|
||||
// Process an acked broadcast that reaches incrementWebsocketCounter.
|
||||
ackPostedBroadcast := func() {
|
||||
msg := platform.MakeHookedWebSocketEvent(model.NewWebSocketEvent(model.WebsocketEventPosted, "", "", "", nil, ""))
|
||||
err := hook.Process(msg, webConn, map[string]any{
|
||||
"posted_user_id": model.NewId(),
|
||||
"channel_type": model.ChannelTypeOpen,
|
||||
"users": []string{userID},
|
||||
})
|
||||
require.NoError(t, err)
|
||||
require.True(t, msg.Event().GetData()["should_ack"].(bool))
|
||||
}
|
||||
|
||||
t.Run("counts when notification metrics are enabled", func(t *testing.T) {
|
||||
th.App.UpdateConfig(func(cfg *model.Config) {
|
||||
*cfg.MetricsSettings.EnableNotificationMetrics = true
|
||||
})
|
||||
|
||||
before := counterValue()
|
||||
ackPostedBroadcast()
|
||||
require.Equal(t, before+1, counterValue())
|
||||
})
|
||||
|
||||
t.Run("does not count when notification metrics are disabled", func(t *testing.T) {
|
||||
th.App.UpdateConfig(func(cfg *model.Config) {
|
||||
*cfg.MetricsSettings.EnableNotificationMetrics = false
|
||||
})
|
||||
|
||||
before := counterValue()
|
||||
ackPostedBroadcast()
|
||||
require.Equal(t, before, counterValue())
|
||||
})
|
||||
}
|
||||
|
||||
@@ -1812,7 +1812,7 @@ func (a *App) notificationMetricsDisabled() bool {
|
||||
return true
|
||||
}
|
||||
|
||||
if a.Config().FeatureFlags.NotificationMonitoring && *a.Config().MetricsSettings.EnableNotificationMetrics {
|
||||
if *a.Config().MetricsSettings.EnableNotificationMetrics {
|
||||
return false
|
||||
}
|
||||
|
||||
|
||||
@@ -539,7 +539,7 @@ func incrementWebsocketCounter(wc *platform.WebConn) {
|
||||
return
|
||||
}
|
||||
|
||||
if !(wc.Platform.Config().FeatureFlags.NotificationMonitoring && *wc.Platform.Config().MetricsSettings.EnableNotificationMetrics) {
|
||||
if !*wc.Platform.Config().MetricsSettings.EnableNotificationMetrics {
|
||||
return
|
||||
}
|
||||
|
||||
|
||||
@@ -201,7 +201,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.MetricsSettings.Enable && *c.MetricsSettings.EnableClientMetrics)
|
||||
props["EnableNotificationMetrics"] = strconv.FormatBool(c.FeatureFlags.NotificationMonitoring && *c.MetricsSettings.EnableNotificationMetrics)
|
||||
props["EnableNotificationMetrics"] = strconv.FormatBool(*c.MetricsSettings.EnableNotificationMetrics)
|
||||
}
|
||||
|
||||
if *license.Features.Announcement {
|
||||
|
||||
@@ -728,6 +728,46 @@ func TestGetClientConfig(t *testing.T) {
|
||||
map[string]string{},
|
||||
[]string{"MobileEphemeralModeEnabled", "MobileEphemeralModeDisconnectionTimeoutSeconds", "MobileEphemeralModeOfflinePersistenceTimerHours", "MobileEphemeralModeAutoCacheCleanupDays"},
|
||||
},
|
||||
{
|
||||
"notification metrics enabled follows the metrics setting",
|
||||
&model.Config{
|
||||
MetricsSettings: model.MetricsSettings{
|
||||
Enable: new(true),
|
||||
EnableNotificationMetrics: new(true),
|
||||
},
|
||||
},
|
||||
"",
|
||||
&model.License{
|
||||
Features: &model.Features{
|
||||
Cluster: new(true),
|
||||
},
|
||||
},
|
||||
map[string]string{
|
||||
"EnableMetrics": "true",
|
||||
"EnableNotificationMetrics": "true",
|
||||
},
|
||||
nil,
|
||||
},
|
||||
{
|
||||
"notification metrics disabled follows the metrics setting",
|
||||
&model.Config{
|
||||
MetricsSettings: model.MetricsSettings{
|
||||
Enable: new(true),
|
||||
EnableNotificationMetrics: new(false),
|
||||
},
|
||||
},
|
||||
"",
|
||||
&model.License{
|
||||
Features: &model.Features{
|
||||
Cluster: new(true),
|
||||
},
|
||||
},
|
||||
map[string]string{
|
||||
"EnableMetrics": "true",
|
||||
"EnableNotificationMetrics": "false",
|
||||
},
|
||||
nil,
|
||||
},
|
||||
}
|
||||
|
||||
for _, testCase := range testCases {
|
||||
|
||||
@@ -42,8 +42,6 @@ type FeatureFlags struct {
|
||||
|
||||
WebSocketEventScope bool
|
||||
|
||||
NotificationMonitoring bool
|
||||
|
||||
ExperimentalAuditSettingsSystemConsoleUI bool
|
||||
|
||||
CustomProfileAttributes bool
|
||||
@@ -161,7 +159,6 @@ func (f *FeatureFlags) SetDefaults() {
|
||||
f.StreamlinedMarketplace = true
|
||||
f.CloudDedicatedExportUI = false
|
||||
f.WebSocketEventScope = true
|
||||
f.NotificationMonitoring = true
|
||||
f.ExperimentalAuditSettingsSystemConsoleUI = true
|
||||
f.CustomProfileAttributes = true
|
||||
f.AttributeBasedAccessControl = true
|
||||
|
||||
@@ -3459,7 +3459,6 @@ const AdminDefinition: AdminDefinitionType = {
|
||||
isDisabled: it.any(
|
||||
it.configIsFalse('MetricsSettings', 'Enable'),
|
||||
),
|
||||
isHidden: it.configIsFalse('FeatureFlags', 'NotificationMonitoring'),
|
||||
},
|
||||
],
|
||||
},
|
||||
|
||||
Reference in New Issue
Block a user