From 02e23a32755cd38cf73e22da971620b93ad318e2 Mon Sep 17 00:00:00 2001 From: Devin Binnie <52460000+devinbinnie@users.noreply.github.com> Date: Thu, 18 Apr 2024 10:30:08 -0400 Subject: [PATCH] [MM-57066][MM-57329] Added metrics for all notification stopping points, consolidated categories between metrics and logging (#26799) * [MM-57066] Add metric counters for notification events * Some small changes * Account for Metrics() sometimes being nil * Fix test (again) * Fix more tests * A few changes from testing - added success counter * Missed a mock * Lint * Add feature flag for notification monitoring --- server/channels/api4/system.go | 12 +- server/channels/app/app_iface.go | 3 + server/channels/app/expirynotify.go | 16 +- server/channels/app/notification.go | 234 +++++++++++------- server/channels/app/notification_push.go | 100 ++++---- server/channels/app/notification_push_test.go | 70 +++--- .../app/opentracing/opentracing_layer.go | 45 ++++ server/channels/app/post.go | 46 +++- server/channels/app/post_acknowledgements.go | 5 +- server/channels/app/reaction.go | 5 +- server/channels/app/web_broadcast_hooks.go | 15 ++ .../channels/app/web_broadcast_hooks_test.go | 3 +- server/channels/wsapi/system.go | 28 ++- server/einterfaces/metrics.go | 6 + server/einterfaces/mocks/MetricsInterface.go | 25 ++ server/enterprise/metrics/metrics.go | 88 +++++++ server/public/model/feature_flags.go | 3 + server/public/model/notification.go | 42 +++- webapp/channels/package.json | 2 +- webapp/channels/src/actions/new_post.ts | 4 +- .../src/actions/notification_actions.jsx | 32 +-- webapp/channels/src/utils/desktop_api.ts | 4 +- webapp/channels/src/utils/notifications.ts | 6 +- webapp/package-lock.json | 8 +- webapp/platform/client/src/websocket.ts | 4 +- webapp/platform/types/src/posts.ts | 7 + 26 files changed, 572 insertions(+), 241 deletions(-) diff --git a/server/channels/api4/system.go b/server/channels/api4/system.go index 860086a86c0..9f665505a57 100644 --- a/server/channels/api4/system.go +++ b/server/channels/api4/system.go @@ -641,13 +641,16 @@ func pushNotificationAck(c *Context, w http.ResponseWriter, r *http.Request) { return } + c.App.CountNotificationAck(model.NotificationTypePush) + err := c.App.SendAckToPushProxy(&ack) if ack.IsIdLoaded { if err != nil { + c.App.CountNotificationReason(model.NotificationStatusError, model.NotificationTypePush, model.NotificationReasonPushProxySendError) c.App.NotificationsLog().Error("Notification ack not sent to push proxy", - mlog.String("type", model.TypePush), - mlog.String("status", model.StatusServerError), - mlog.String("reason", model.ReasonServerError), + mlog.String("type", model.NotificationTypePush), + mlog.String("status", model.NotificationStatusError), + mlog.String("reason", model.NotificationReasonPushProxySendError), mlog.String("ack_id", ack.Id), mlog.String("push_type", ack.NotificationType), mlog.String("post_id", ack.PostId), @@ -656,6 +659,8 @@ func pushNotificationAck(c *Context, w http.ResponseWriter, r *http.Request) { mlog.Int("received_at", ack.ClientReceivedAt), mlog.Err(err), ) + } else { + c.App.CountNotificationReason(model.NotificationStatusSuccess, model.NotificationTypePush, model.NotificationReason("")) } // Return post data only when PostId is passed. if ack.PostId != "" && ack.NotificationType == model.PushTypeMessage { @@ -687,6 +692,7 @@ func pushNotificationAck(c *Context, w http.ResponseWriter, r *http.Request) { return } + c.App.CountNotificationReason(model.NotificationStatusSuccess, model.NotificationTypePush, model.NotificationReason("")) ReturnStatusOK(w) } diff --git a/server/channels/app/app_iface.go b/server/channels/app/app_iface.go index 4e3c5df79cd..8a8d1aa1fc4 100644 --- a/server/channels/app/app_iface.go +++ b/server/channels/app/app_iface.go @@ -513,6 +513,9 @@ type AppIface interface { ConvertGroupMessageToChannel(c request.CTX, convertedByUserId string, gmConversionRequest *model.GroupMessageConversionRequestBody) (*model.Channel, *model.AppError) CopyFileInfos(rctx request.CTX, userID string, fileIDs []string) ([]string, *model.AppError) CopyWranglerPostlist(c request.CTX, wpl *model.WranglerPostList, targetChannel *model.Channel) (*model.Post, *model.AppError) + CountNotification(notificationType model.NotificationType) + CountNotificationAck(notificationType model.NotificationType) + CountNotificationReason(notificationStatus model.NotificationStatus, notificationType model.NotificationType, notificationReason model.NotificationReason) CreateChannel(c request.CTX, channel *model.Channel, addMember bool) (*model.Channel, *model.AppError) CreateChannelBookmark(c request.CTX, newBookmark *model.ChannelBookmark, connectionId string) (*model.ChannelBookmarkWithFileInfo, *model.AppError) CreateChannelWithUser(c request.CTX, channel *model.Channel, userID string) (*model.Channel, *model.AppError) diff --git a/server/channels/app/expirynotify.go b/server/channels/app/expirynotify.go index a93b7468825..81f04ed46f3 100644 --- a/server/channels/app/expirynotify.go +++ b/server/channels/app/expirynotify.go @@ -24,10 +24,11 @@ func (a *App) NotifySessionsExpired() error { // Get all mobile sessions that expired within the last hour. sessions, err := a.ch.srv.Store().Session().GetSessionsExpired(OneHourMillis, true, true) if err != nil { + a.CountNotificationReason(model.NotificationStatusError, model.NotificationTypePush, model.NotificationReasonFetchError) a.NotificationsLog().Error("Cannot get sessions expired", - mlog.String("type", model.TypePush), - mlog.String("status", model.StatusServerError), - mlog.String("reason", model.ReasonFetchError), + mlog.String("type", model.NotificationTypePush), + mlog.String("status", model.NotificationStatusError), + mlog.String("reason", model.NotificationReasonFetchError), mlog.Err(err), ) return model.NewAppError("NotifySessionsExpired", "app.session.analytics_session_count.app_error", nil, "", http.StatusInternalServerError).Wrap(err) @@ -46,10 +47,11 @@ func (a *App) NotifySessionsExpired() error { errPush := a.sendToPushProxy(tmpMessage, session) if errPush != nil { + a.CountNotificationReason(model.NotificationStatusError, model.NotificationTypePush, model.NotificationReasonPushProxySendError) a.NotificationsLog().Error("Failed to send to push proxy", - mlog.String("type", model.TypePush), - mlog.String("status", model.StatusNotSent), - mlog.String("reason", model.ReasonPushProxyError), + mlog.String("type", model.NotificationTypePush), + mlog.String("status", model.NotificationStatusNotSent), + mlog.String("reason", model.NotificationReasonPushProxySendError), mlog.String("ack_id", tmpMessage.AckId), mlog.String("push_type", tmpMessage.Type), mlog.String("user_id", session.UserId), @@ -60,7 +62,7 @@ func (a *App) NotifySessionsExpired() error { } a.NotificationsLog().Trace("Notification sent to push proxy", - mlog.String("type", model.TypePush), + mlog.String("type", model.NotificationTypePush), mlog.String("ack_id", tmpMessage.AckId), mlog.String("push_type", tmpMessage.Type), mlog.String("user_id", session.UserId), diff --git a/server/channels/app/notification.go b/server/channels/app/notification.go index 6f79f1e62d2..79580c7e7c6 100644 --- a/server/channels/app/notification.go +++ b/server/channels/app/notification.go @@ -24,17 +24,17 @@ import ( func (a *App) canSendPushNotifications() bool { if !*a.Config().EmailSettings.SendPushNotifications { a.NotificationsLog().Debug("Push notifications are disabled - server config", - mlog.String("status", model.StatusBlocked), - mlog.String("reason", model.ReasonServerConfig), + mlog.String("status", model.NotificationStatusNotSent), + mlog.String("reason", "push_disabled"), ) return false } pushServer := *a.Config().EmailSettings.PushNotificationServer if license := a.Srv().License(); pushServer == model.MHPNS && (license == nil || !*license.Features.MHPNS) { - a.NotificationsLog().Info("Push notifications are disabled - license missing", - mlog.String("status", model.StatusBlocked), - mlog.String("reason", model.ReasonServerConfig), + a.NotificationsLog().Warn("Push notifications are disabled - license missing", + mlog.String("status", model.NotificationStatusNotSent), + mlog.String("reason", "push_disabled_license"), ) mlog.Warn("Push notifications have been disabled. Update your license or go to System Console > Environment > Push Notification Server to use a different server") return false @@ -97,11 +97,12 @@ func (a *App) SendNotifications(c request.CTX, post *model.Post, team *model.Tea pResult := <-pchan if pResult.NErr != nil { + a.CountNotificationReason(model.NotificationStatusError, model.NotificationTypeAll, model.NotificationReasonFetchError) a.NotificationsLog().Error("Error fetching profiles", mlog.String("sender_id", sender.Id), mlog.String("post_id", post.Id), - mlog.String("status", model.StatusServerError), - mlog.String("reason", model.ReasonFetchError), + mlog.String("status", model.NotificationStatusError), + mlog.String("reason", model.NotificationReasonFetchError), mlog.Err(pResult.NErr), ) return nil, pResult.NErr @@ -110,11 +111,12 @@ func (a *App) SendNotifications(c request.CTX, post *model.Post, team *model.Tea cmnResult := <-cmnchan if cmnResult.NErr != nil { + a.CountNotificationReason(model.NotificationStatusError, model.NotificationTypeAll, model.NotificationReasonFetchError) a.NotificationsLog().Error("Error fetching notify props", mlog.String("sender_id", sender.Id), mlog.String("post_id", post.Id), - mlog.String("status", model.StatusServerError), - mlog.String("reason", model.ReasonFetchError), + mlog.String("status", model.NotificationStatusError), + mlog.String("reason", model.NotificationReasonFetchError), mlog.Err(cmnResult.NErr), ) return nil, cmnResult.NErr @@ -125,11 +127,12 @@ func (a *App) SendNotifications(c request.CTX, post *model.Post, team *model.Tea if tchan != nil { tResult := <-tchan if tResult.NErr != nil { + a.CountNotificationReason(model.NotificationStatusError, model.NotificationTypeAll, model.NotificationReasonFetchError) a.NotificationsLog().Error("Error fetching thread followers", mlog.String("sender_id", sender.Id), mlog.String("post_id", post.Id), - mlog.String("status", model.StatusServerError), - mlog.String("reason", model.ReasonFetchError), + mlog.String("status", model.NotificationStatusError), + mlog.String("reason", model.NotificationReasonFetchError), mlog.Err(tResult.NErr), ) return nil, tResult.NErr @@ -143,11 +146,12 @@ func (a *App) SendNotifications(c request.CTX, post *model.Post, team *model.Tea if gchan != nil { gResult := <-gchan if gResult.NErr != nil { + a.CountNotificationReason(model.NotificationStatusError, model.NotificationTypeAll, model.NotificationReasonFetchError) a.NotificationsLog().Error("Error fetching group mentions", mlog.String("sender_id", sender.Id), mlog.String("post_id", post.Id), - mlog.String("status", model.StatusServerError), - mlog.String("reason", model.ReasonFetchError), + mlog.String("status", model.NotificationStatusError), + mlog.String("reason", model.NotificationReasonFetchError), mlog.Err(gResult.NErr), ) return nil, gResult.NErr @@ -169,11 +173,12 @@ func (a *App) SendNotifications(c request.CTX, post *model.Post, team *model.Tea group := groups[groupID] anyUsersMentionedByGroup, err := a.insertGroupMentions(sender.Id, group, channel, profileMap, mentions) if err != nil { + a.CountNotificationReason(model.NotificationStatusError, model.NotificationTypeAll, model.NotificationReasonFetchError) a.NotificationsLog().Error("Failed to populate group mentions", mlog.String("sender_id", sender.Id), mlog.String("post_id", post.Id), - mlog.String("status", model.StatusServerError), - mlog.String("reason", model.ReasonFetchError), + mlog.String("status", model.NotificationStatusError), + mlog.String("reason", model.NotificationReasonFetchError), mlog.Err(err), ) return nil, err @@ -190,8 +195,8 @@ func (a *App) SendNotifications(c request.CTX, post *model.Post, team *model.Tea a.NotificationsLog().Warn("Failed to send warning for out of channel mentions", mlog.String("sender_id", sender.Id), mlog.String("post_id", post.Id), - mlog.String("status", model.StatusServerError), - mlog.String("reason", model.ReasonServerError), + mlog.String("status", model.NotificationStatusError), + mlog.String("reason", "failed_to_send_out_of_channel"), mlog.Err(err), ) c.Logger().Error("Failed to send warning for out of channel mentions", mlog.String("user_id", sender.Id), mlog.String("post_id", post.Id), mlog.Err(err)) @@ -368,7 +373,7 @@ func (a *App) SendNotifications(c request.CTX, post *model.Post, team *model.Tea if *a.Config().EmailSettings.SendEmailNotifications { a.NotificationsLog().Trace("Begin sending email notifications", - mlog.String("type", model.TypeEmail), + mlog.String("type", model.NotificationTypeEmail), mlog.String("sender_id", sender.Id), mlog.String("post_id", post.Id), ) @@ -377,11 +382,12 @@ func (a *App) SendNotifications(c request.CTX, post *model.Post, team *model.Tea for _, id := range emailRecipients { if profileMap[id] == nil { - a.NotificationsLog().Warn("Missing profile", - mlog.String("type", model.TypeEmail), + a.CountNotificationReason(model.NotificationStatusError, model.NotificationTypeEmail, model.NotificationReasonMissingProfile) + a.NotificationsLog().Error("Missing profile", + mlog.String("type", model.NotificationTypeEmail), mlog.String("post_id", post.Id), - mlog.String("status", model.StatusNotSent), - mlog.String("reason", model.ReasonMissingProfile), + mlog.String("status", model.NotificationStatusNotSent), + mlog.String("reason", model.NotificationReasonMissingProfile), mlog.String("sender_id", sender.Id), mlog.String("receiver_id", id), ) @@ -390,11 +396,12 @@ func (a *App) SendNotifications(c request.CTX, post *model.Post, team *model.Tea //If email verification is required and user email is not verified don't send email. if *a.Config().EmailSettings.RequireEmailVerification && !profileMap[id].EmailVerified { + a.CountNotificationReason(model.NotificationStatusNotSent, model.NotificationTypeEmail, model.NotificationReasonEmailNotVerified) a.NotificationsLog().Debug("Email not verified", - mlog.String("type", model.TypeEmail), + mlog.String("type", model.NotificationTypeEmail), mlog.String("post_id", post.Id), - mlog.String("status", model.StatusNotSent), - mlog.String("reason", model.ReasonUserConfig), + mlog.String("status", model.NotificationStatusNotSent), + mlog.String("reason", model.NotificationReasonEmailNotVerified), mlog.String("sender_id", sender.Id), mlog.String("receiver_id", id), ) @@ -408,11 +415,12 @@ func (a *App) SendNotifications(c request.CTX, post *model.Post, team *model.Tea a.Log().Warn("Unable to get the sender user profile image.", mlog.String("user_id", sender.Id), mlog.Err(err)) } if err := a.sendNotificationEmail(c, notification, profileMap[id], team, senderProfileImage); err != nil { - a.NotificationsLog().Warn("Error sending email notification", - mlog.String("type", model.TypeEmail), + a.CountNotificationReason(model.NotificationStatusError, model.NotificationTypeEmail, model.NotificationReasonEmailSendError) + a.NotificationsLog().Error("Error sending email notification", + mlog.String("type", model.NotificationTypeEmail), mlog.String("post_id", post.Id), - mlog.String("status", model.StatusNotSent), - mlog.String("reason", model.ReasonServerError), + mlog.String("status", model.NotificationStatusError), + mlog.String("reason", model.NotificationReasonEmailSendError), mlog.String("sender_id", sender.Id), mlog.String("receiver_id", id), mlog.Err(err), @@ -421,10 +429,10 @@ func (a *App) SendNotifications(c request.CTX, post *model.Post, team *model.Tea } } else { a.NotificationsLog().Debug("Email disallowed by user", - mlog.String("type", model.TypeEmail), + mlog.String("type", model.NotificationTypeEmail), mlog.String("post_id", post.Id), - mlog.String("status", model.StatusNotSent), - mlog.String("reason", model.ReasonUserConfig), + mlog.String("status", model.NotificationStatusNotSent), + mlog.String("reason", "email_disallowed_by_user"), mlog.String("sender_id", sender.Id), mlog.String("receiver_id", id), ) @@ -432,7 +440,7 @@ func (a *App) SendNotifications(c request.CTX, post *model.Post, team *model.Tea } a.NotificationsLog().Trace("Finished sending email notifications", - mlog.String("type", model.TypeEmail), + mlog.String("type", model.NotificationTypeEmail), mlog.String("sender_id", sender.Id), mlog.String("post_id", post.Id), ) @@ -440,11 +448,12 @@ func (a *App) SendNotifications(c request.CTX, post *model.Post, team *model.Tea // Check for channel-wide mentions in channels that have too many members for those to work if int64(len(profileMap)) > *a.Config().TeamSettings.MaxNotificationsPerChannel { + a.CountNotificationReason(model.NotificationStatusNotSent, model.NotificationTypeAll, model.NotificationReasonTooManyUsersInChannel) a.NotificationsLog().Debug("Too many users to notify - will send ephemeral message", mlog.String("sender_id", sender.Id), mlog.String("post_id", post.Id), - mlog.String("status", model.StatusNotSent), - mlog.String("reason", model.ReasonServerConfig), + mlog.String("status", model.NotificationStatusNotSent), + mlog.String("reason", model.NotificationReasonTooManyUsersInChannel), ) T := i18n.GetUserTranslations(sender.Locale) @@ -488,18 +497,19 @@ func (a *App) SendNotifications(c request.CTX, post *model.Post, team *model.Tea if a.canSendPushNotifications() { a.NotificationsLog().Trace("Begin sending push notifications", - mlog.String("type", model.TypePush), + mlog.String("type", model.NotificationTypePush), mlog.String("sender_id", sender.Id), mlog.String("post_id", post.Id), ) for _, id := range mentionedUsersList { if profileMap[id] == nil { - a.NotificationsLog().Warn("Missing profile", - mlog.String("type", model.TypePush), + a.CountNotificationReason(model.NotificationStatusError, model.NotificationTypePush, model.NotificationReasonMissingProfile) + a.NotificationsLog().Error("Missing profile", + mlog.String("type", model.NotificationTypePush), mlog.String("post_id", post.Id), - mlog.String("status", model.StatusNotSent), - mlog.String("reason", model.ReasonMissingProfile), + mlog.String("status", model.NotificationStatusNotSent), + mlog.String("reason", model.NotificationReasonMissingProfile), mlog.String("sender_id", sender.Id), mlog.String("receiver_id", id), ) @@ -508,9 +518,9 @@ func (a *App) SendNotifications(c request.CTX, post *model.Post, team *model.Tea if notificationsForCRT.Push.Contains(id) { a.NotificationsLog().Trace("Skipped direct push notification - will send as CRT notification", - mlog.String("type", model.TypePush), + mlog.String("type", model.NotificationTypePush), mlog.String("post_id", post.Id), - mlog.String("status", model.StatusNotSent), + mlog.String("status", model.NotificationStatusNotSent), mlog.String("sender_id", sender.Id), ) continue @@ -546,11 +556,12 @@ func (a *App) SendNotifications(c request.CTX, post *model.Post, team *model.Tea for _, id := range allActivityPushUserIds { if profileMap[id] == nil { - a.NotificationsLog().Warn("Missing profile", - mlog.String("type", model.TypePush), + a.CountNotificationReason(model.NotificationStatusError, model.NotificationTypePush, model.NotificationReasonMissingProfile) + a.NotificationsLog().Error("Missing profile", + mlog.String("type", model.NotificationTypePush), mlog.String("post_id", post.Id), - mlog.String("status", model.StatusNotSent), - mlog.String("reason", model.ReasonMissingProfile), + mlog.String("status", model.NotificationStatusError), + mlog.String("reason", model.NotificationReasonMissingProfile), mlog.String("sender_id", sender.Id), mlog.String("receiver_id", id), ) @@ -559,9 +570,9 @@ func (a *App) SendNotifications(c request.CTX, post *model.Post, team *model.Tea if notificationsForCRT.Push.Contains(id) { a.NotificationsLog().Trace("Skipped direct push notification - will send as CRT notification", - mlog.String("type", model.TypePush), + mlog.String("type", model.NotificationTypePush), mlog.String("post_id", post.Id), - mlog.String("status", model.StatusNotSent), + mlog.String("status", model.NotificationStatusNotSent), mlog.String("sender_id", sender.Id), ) continue @@ -589,11 +600,12 @@ func (a *App) SendNotifications(c request.CTX, post *model.Post, team *model.Tea for _, id := range notificationsForCRT.Push { if profileMap[id] == nil { - a.NotificationsLog().Warn("Missing profile", - mlog.String("type", model.TypePush), + a.CountNotificationReason(model.NotificationStatusError, model.NotificationTypePush, model.NotificationReasonMissingProfile) + a.NotificationsLog().Error("Missing profile", + mlog.String("type", model.NotificationTypePush), mlog.String("post_id", post.Id), - mlog.String("status", model.StatusNotSent), - mlog.String("reason", model.ReasonMissingProfile), + mlog.String("status", model.NotificationStatusError), + mlog.String("reason", model.NotificationReasonMissingProfile), mlog.String("sender_id", sender.Id), mlog.String("receiver_id", id), ) @@ -615,11 +627,12 @@ func (a *App) SendNotifications(c request.CTX, post *model.Post, team *model.Tea model.CommentsNotifyCRT, ) } else { + a.CountNotificationReason(model.NotificationStatusNotSent, model.NotificationTypePush, statusReason) a.NotificationsLog().Debug("Notification not sent - status", - mlog.String("type", model.TypePush), + mlog.String("type", model.NotificationTypePush), mlog.String("post_id", post.Id), - mlog.String("status", model.StatusNotSent), - mlog.String("reason", model.ReasonUserConfig), + mlog.String("status", model.NotificationStatusNotSent), + mlog.String("reason", statusReason), mlog.String("status_reason", statusReason), mlog.String("sender_id", post.UserId), mlog.String("receiver_id", id), @@ -629,14 +642,14 @@ func (a *App) SendNotifications(c request.CTX, post *model.Post, team *model.Tea } a.NotificationsLog().Trace("Finished sending push notifications", - mlog.String("type", model.TypePush), + mlog.String("type", model.NotificationTypePush), mlog.String("sender_id", sender.Id), mlog.String("post_id", post.Id), ) } a.NotificationsLog().Trace("Begin sending websocket notifications", - mlog.String("type", model.TypeWebsocket), + mlog.String("type", model.NotificationTypeWebsocket), mlog.String("sender_id", sender.Id), mlog.String("post_id", post.Id), ) @@ -690,11 +703,12 @@ func (a *App) SendNotifications(c request.CTX, post *model.Post, team *model.Tea published, err := a.publishWebsocketEventForPermalinkPost(c, post, message) if err != nil { + a.CountNotificationReason(model.NotificationStatusError, model.NotificationTypeWebsocket, model.NotificationReasonFetchError) a.NotificationsLog().Error("Couldn't send websocket notification for permalink post", - mlog.String("type", model.TypeWebsocket), + mlog.String("type", model.NotificationTypeWebsocket), mlog.String("post_id", post.Id), - mlog.String("status", model.StatusServerError), - mlog.String("reason", model.ReasonFetchError), + mlog.String("status", model.NotificationStatusError), + mlog.String("reason", model.NotificationReasonFetchError), mlog.String("sender_id", sender.Id), mlog.Err(err), ) @@ -704,11 +718,12 @@ func (a *App) SendNotifications(c request.CTX, post *model.Post, team *model.Tea removePermalinkMetadataFromPost(post) postJSON, jsonErr := post.ToJSON() if jsonErr != nil { + a.CountNotificationReason(model.NotificationStatusError, model.NotificationTypeWebsocket, model.NotificationReasonParseError) a.NotificationsLog().Error("JSON parse error", - mlog.String("type", model.TypeWebsocket), + mlog.String("type", model.NotificationTypeWebsocket), mlog.String("post_id", post.Id), - mlog.String("status", model.StatusServerError), - mlog.String("reason", model.ReasonServerError), + mlog.String("status", model.NotificationStatusError), + mlog.String("reason", model.NotificationReasonParseError), mlog.String("sender_id", sender.Id), mlog.Err(err), ) @@ -725,11 +740,12 @@ func (a *App) SendNotifications(c request.CTX, post *model.Post, team *model.Tea // A user following a thread but had left the channel won't get a notification // https://mattermost.atlassian.net/browse/MM-36769 if profileMap[uid] == nil { - a.NotificationsLog().Warn("Missing profile", - mlog.String("type", model.TypeWebsocket), + a.CountNotificationReason(model.NotificationStatusError, model.NotificationTypeWebsocket, model.NotificationReasonMissingProfile) + a.NotificationsLog().Error("Missing profile", + mlog.String("type", model.NotificationTypeWebsocket), mlog.String("post_id", post.Id), - mlog.String("status", model.StatusNotSent), - mlog.String("reason", model.ReasonMissingProfile), + mlog.String("status", model.NotificationStatusError), + mlog.String("reason", model.NotificationReasonMissingProfile), mlog.String("sender_id", sender.Id), mlog.String("receiver_id", uid), ) @@ -741,11 +757,12 @@ func (a *App) SendNotifications(c request.CTX, post *model.Post, team *model.Tea if threadMembership == nil { tm, err := a.Srv().Store().Thread().GetMembershipForUser(uid, post.RootId) if err != nil { + a.CountNotificationReason(model.NotificationStatusError, model.NotificationTypeWebsocket, model.NotificationReasonFetchError) a.NotificationsLog().Error("Missing thread membership", - mlog.String("type", model.TypeWebsocket), + mlog.String("type", model.NotificationTypeWebsocket), mlog.String("post_id", post.Id), - mlog.String("status", model.StatusServerError), - mlog.String("reason", model.ReasonFetchError), + mlog.String("status", model.NotificationStatusError), + mlog.String("reason", model.NotificationReasonFetchError), mlog.String("sender_id", sender.Id), mlog.String("receiver_id", uid), mlog.Err(err), @@ -753,11 +770,12 @@ func (a *App) SendNotifications(c request.CTX, post *model.Post, team *model.Tea return nil, errors.Wrapf(err, "Missing thread membership for participant in notifications. user_id=%q thread_id=%q", uid, post.RootId) } if tm == nil { + a.CountNotificationReason(model.NotificationStatusNotSent, model.NotificationTypeWebsocket, model.NotificationReasonMissingThreadMembership) a.NotificationsLog().Warn("Missing thread membership", - mlog.String("type", model.TypeWebsocket), + mlog.String("type", model.NotificationTypeWebsocket), mlog.String("post_id", post.Id), - mlog.String("status", model.StatusNotSent), - mlog.String("reason", model.ReasonServerError), + mlog.String("status", model.NotificationStatusNotSent), + mlog.String("reason", model.NotificationReasonMissingThreadMembership), mlog.String("sender_id", sender.Id), mlog.String("receiver_id", uid), ) @@ -767,11 +785,12 @@ func (a *App) SendNotifications(c request.CTX, post *model.Post, team *model.Tea } userThread, err := a.Srv().Store().Thread().GetThreadForUser(threadMembership, true, a.IsPostPriorityEnabled()) if err != nil { + a.CountNotificationReason(model.NotificationStatusError, model.NotificationTypeWebsocket, model.NotificationReasonFetchError) a.NotificationsLog().Error("Missing thread", - mlog.String("type", model.TypeWebsocket), + mlog.String("type", model.NotificationTypeWebsocket), mlog.String("post_id", post.Id), - mlog.String("status", model.StatusServerError), - mlog.String("reason", model.ReasonFetchError), + mlog.String("status", model.NotificationStatusError), + mlog.String("reason", model.NotificationReasonFetchError), mlog.String("sender_id", sender.Id), mlog.String("receiver_id", uid), mlog.Err(err), @@ -800,11 +819,12 @@ func (a *App) SendNotifications(c request.CTX, post *model.Post, team *model.Tea // should set unread mentions, and unread replies to 0 _, err = a.Srv().Store().Thread().MaintainMembership(uid, post.RootId, opts) if err != nil { + a.CountNotificationReason(model.NotificationStatusError, model.NotificationTypeWebsocket, model.NotificationReasonFetchError) a.NotificationsLog().Error("Failed to update thread membership", - mlog.String("type", model.TypeWebsocket), + mlog.String("type", model.NotificationTypeWebsocket), mlog.String("post_id", post.Id), - mlog.String("status", model.StatusServerError), - mlog.String("reason", model.ReasonServerError), + mlog.String("status", model.NotificationStatusError), + mlog.String("reason", model.NotificationReasonFetchError), mlog.String("sender_id", sender.Id), mlog.String("receiver_id", uid), mlog.Err(err), @@ -819,11 +839,12 @@ func (a *App) SendNotifications(c request.CTX, post *model.Post, team *model.Tea sanitizedPost, err := a.SanitizePostMetadataForUser(c, userThread.Post, uid) if err != nil { + a.CountNotificationReason(model.NotificationStatusError, model.NotificationTypeWebsocket, model.NotificationReasonParseError) a.NotificationsLog().Error("Failed to sanitize metadata", - mlog.String("type", model.TypeWebsocket), + mlog.String("type", model.NotificationTypeWebsocket), mlog.String("post_id", post.Id), - mlog.String("status", model.StatusServerError), - mlog.String("reason", model.ReasonServerError), + mlog.String("status", model.NotificationStatusError), + mlog.String("reason", model.NotificationReasonParseError), mlog.String("sender_id", sender.Id), mlog.String("receiver_id", uid), mlog.Err(err), @@ -847,7 +868,7 @@ func (a *App) SendNotifications(c request.CTX, post *model.Post, team *model.Tea } a.NotificationsLog().Trace("Finish sending websocket notifications", - mlog.String("type", model.TypeWebsocket), + mlog.String("type", model.NotificationTypeWebsocket), mlog.String("sender_id", sender.Id), mlog.String("post_id", post.Id), ) @@ -1709,3 +1730,50 @@ func ShouldAckWebsocketNotification(channelType model.ChannelType, userNotificat return false } + +func (a *App) CountNotification(notificationType model.NotificationType) { + if a.Metrics() == nil { + return + } + + if !a.Config().FeatureFlags.NotificationMonitoring { + return + } + + a.Metrics().IncrementNotificationCounter(notificationType) +} + +func (a *App) CountNotificationAck(notificationType model.NotificationType) { + if a.Metrics() == nil { + return + } + + if !a.Config().FeatureFlags.NotificationMonitoring { + return + } + + a.Metrics().IncrementNotificationAckCounter(notificationType) +} + +func (a *App) CountNotificationReason( + notificationStatus model.NotificationStatus, + notificationType model.NotificationType, + notificationReason model.NotificationReason, +) { + if a.Metrics() == nil { + return + } + + if !a.Config().FeatureFlags.NotificationMonitoring { + return + } + + switch notificationStatus { + case model.NotificationStatusSuccess: + a.Metrics().IncrementNotificationSuccessCounter(notificationType) + case model.NotificationStatusError: + a.Metrics().IncrementNotificationErrorCounter(notificationType, notificationReason) + case model.NotificationStatusNotSent: + a.Metrics().IncrementNotificationNotSentCounter(notificationType, notificationReason) + } +} diff --git a/server/channels/app/notification_push.go b/server/channels/app/notification_push.go index 9a4a8fb4477..e77af5618a1 100644 --- a/server/channels/app/notification_push.go +++ b/server/channels/app/notification_push.go @@ -24,22 +24,12 @@ import ( ) type notificationType string -type notifyPropsReason string -type statusReason string const ( notificationTypeClear notificationType = "clear" notificationTypeMessage notificationType = "message" notificationTypeUpdateBadge notificationType = "update_badge" notificationTypeDummy notificationType = "dummy" - - NotifyPropsReasonChannelMuted notifyPropsReason = "channel_muted" - NotifyPropsReasonSystemMessage notifyPropsReason = "system_message" - NotifyPropsReasonSetToNone notifyPropsReason = "notify_props_set_to_note" - NotifyPropsReasonSetToMention notifyPropsReason = "notify_props_set_to_mention_and_was_not_mentioned" - - StatusReasonDNDOrOOO statusReason = "status_is_dnd_or_ooo" - StatusReasonIsActive statusReason = "user_is_active_on_channel" ) type PushNotificationsHub struct { @@ -107,10 +97,12 @@ func (a *App) sendPushNotificationToAllSessions(msg *model.PushNotification, use if rejectionReason != "" { // Notifications rejected by a plugin should not be considered errors - a.NotificationsLog().Info("Notification rejected by plugin", - mlog.String("type", model.TypePush), - mlog.String("status", model.StatusNotSent), - mlog.String("reason", rejectionReason), + // This is likely normal operation so no need for metrics here + a.NotificationsLog().Debug("Notification rejected by plugin", + mlog.String("type", model.NotificationTypePush), + mlog.String("status", model.NotificationStatusNotSent), + mlog.String("reason", model.NotificationReasonRejectedByPlugin), + mlog.String("rejection_reason", rejectionReason), mlog.String("user_id", userID), ) return nil @@ -118,10 +110,11 @@ func (a *App) sendPushNotificationToAllSessions(msg *model.PushNotification, use sessions, err := a.getMobileAppSessions(userID) if err != nil { + a.CountNotificationReason(model.NotificationStatusError, model.NotificationTypePush, model.NotificationReasonFetchError) a.NotificationsLog().Error("Failed to send mobile app sessions", - mlog.String("type", model.TypePush), - mlog.String("status", model.StatusServerError), - mlog.String("reason", model.ReasonFetchError), + mlog.String("type", model.NotificationTypePush), + mlog.String("status", model.NotificationStatusError), + mlog.String("reason", model.NotificationReasonFetchError), mlog.String("user_id", userID), mlog.Err(err), ) @@ -129,10 +122,11 @@ func (a *App) sendPushNotificationToAllSessions(msg *model.PushNotification, use } if msg == nil { + a.CountNotificationReason(model.NotificationStatusError, model.NotificationTypePush, model.NotificationReasonParseError) a.NotificationsLog().Error("Failed to parse push notification", - mlog.String("type", model.TypePush), - mlog.String("status", model.StatusServerError), - mlog.String("reason", model.ReasonServerError), + mlog.String("type", model.NotificationTypePush), + mlog.String("status", model.NotificationStatusError), + mlog.String("reason", model.NotificationReasonParseError), mlog.String("user_id", userID), ) return model.NewAppError( @@ -147,10 +141,11 @@ func (a *App) sendPushNotificationToAllSessions(msg *model.PushNotification, use for _, session := range sessions { // Don't send notifications to this session if it's expired or we want to skip it if session.IsExpired() || (skipSessionId != "" && skipSessionId == session.Id) { + a.CountNotificationReason(model.NotificationStatusNotSent, model.NotificationTypePush, model.NotificationReasonSessionExpired) a.NotificationsLog().Debug("Session expired or skipped", - mlog.String("type", model.TypePush), - mlog.String("status", model.StatusNotSent), - mlog.String("reason", model.ReasonUserStatus), + mlog.String("type", model.NotificationTypePush), + mlog.String("status", model.NotificationStatusNotSent), + mlog.String("reason", model.NotificationReasonSessionExpired), mlog.String("user_id", session.UserId), mlog.String("session_id", session.Id), ) @@ -164,10 +159,11 @@ func (a *App) sendPushNotificationToAllSessions(msg *model.PushNotification, use err := a.sendToPushProxy(tmpMessage, session) if err != nil { + a.CountNotificationReason(model.NotificationStatusError, model.NotificationTypePush, model.NotificationReasonPushProxySendError) a.NotificationsLog().Error("Failed to send to push proxy", - mlog.String("type", model.TypePush), - mlog.String("status", model.StatusNotSent), - mlog.String("reason", model.ReasonPushProxyError), + mlog.String("type", model.NotificationTypePush), + mlog.String("status", model.NotificationStatusNotSent), + mlog.String("reason", model.NotificationReasonPushProxySendError), mlog.String("ack_id", tmpMessage.AckId), mlog.String("push_type", tmpMessage.Type), mlog.String("user_id", session.UserId), @@ -178,7 +174,7 @@ func (a *App) sendPushNotificationToAllSessions(msg *model.PushNotification, use } a.NotificationsLog().Trace("Notification sent to push proxy", - mlog.String("type", model.TypePush), + mlog.String("type", model.NotificationTypePush), mlog.String("ack_id", tmpMessage.AckId), mlog.String("push_type", tmpMessage.Type), mlog.String("user_id", session.UserId), @@ -477,7 +473,7 @@ func (a *App) sendToPushProxy(msg *model.PushNotification, session *model.Sessio msg.ServerId = a.TelemetryId() a.NotificationsLog().Trace("Notification will be sent", - mlog.String("type", model.TypePush), + mlog.String("type", model.NotificationTypePush), mlog.String("ack_id", msg.AckId), mlog.String("push_type", msg.Type), mlog.String("user_id", session.UserId), @@ -507,7 +503,7 @@ func (a *App) SendAckToPushProxy(ack *model.PushNotificationAck) error { } a.NotificationsLog().Trace("Notification successfully received", - mlog.String("type", model.TypePush), + mlog.String("type", model.NotificationTypePush), mlog.String("ack_id", ack.Id), mlog.String("push_type", ack.NotificationType), mlog.String("post_id", ack.PostId), @@ -553,12 +549,12 @@ func (a *App) getMobileAppSessions(userID string) ([]*model.Session, *model.AppE func (a *App) ShouldSendPushNotification(user *model.User, channelNotifyProps model.StringMap, wasMentioned bool, status *model.Status, post *model.Post, isGM bool) bool { if notifyPropsAllowedReason := DoesNotifyPropsAllowPushNotification(user, channelNotifyProps, post, wasMentioned, isGM); notifyPropsAllowedReason != "" { + a.CountNotificationReason(model.NotificationStatusNotSent, model.NotificationTypePush, notifyPropsAllowedReason) a.NotificationsLog().Debug("Notification not sent - notify props", - mlog.String("type", model.TypePush), + mlog.String("type", model.NotificationTypePush), mlog.String("post_id", post.Id), - mlog.String("status", model.StatusNotSent), - mlog.String("reason", model.ReasonUserConfig), - mlog.String("notify_props_reason", notifyPropsAllowedReason), + mlog.String("status", model.NotificationStatusNotSent), + mlog.String("reason", notifyPropsAllowedReason), mlog.String("sender_id", post.UserId), mlog.String("receiver_id", user.Id), ) @@ -566,12 +562,12 @@ func (a *App) ShouldSendPushNotification(user *model.User, channelNotifyProps mo } if statusAllowedReason := DoesStatusAllowPushNotification(user.NotifyProps, status, post.ChannelId); statusAllowedReason != "" { + a.CountNotificationReason(model.NotificationStatusNotSent, model.NotificationTypePush, statusAllowedReason) a.NotificationsLog().Debug("Notification not sent - status", - mlog.String("type", model.TypePush), + mlog.String("type", model.NotificationTypePush), mlog.String("post_id", post.Id), - mlog.String("status", model.StatusNotSent), - mlog.String("reason", model.ReasonUserConfig), - mlog.String("status_reason", statusAllowedReason), + mlog.String("status", model.NotificationStatusNotSent), + mlog.String("reason", statusAllowedReason), mlog.String("sender_id", post.UserId), mlog.String("receiver_id", user.Id), mlog.String("receiver_status", status.Status), @@ -582,7 +578,7 @@ func (a *App) ShouldSendPushNotification(user *model.User, channelNotifyProps mo return true } -func DoesNotifyPropsAllowPushNotification(user *model.User, channelNotifyProps model.StringMap, post *model.Post, wasMentioned, isGM bool) notifyPropsReason { +func DoesNotifyPropsAllowPushNotification(user *model.User, channelNotifyProps model.StringMap, post *model.Post, wasMentioned, isGM bool) model.NotificationReason { userNotifyProps := user.NotifyProps userNotify := userNotifyProps[model.PushNotifyProp] channelNotify, ok := channelNotifyProps[model.PushNotifyProp] @@ -600,19 +596,19 @@ func DoesNotifyPropsAllowPushNotification(user *model.User, channelNotifyProps m // If the channel is muted do not send push notifications if channelNotifyProps[model.MarkUnreadNotifyProp] == model.ChannelMarkUnreadMention { - return NotifyPropsReasonChannelMuted + return model.NotificationReasonChannelMuted } if post.IsSystemMessage() { - return NotifyPropsReasonSystemMessage + return model.NotificationReasonSystemMessage } if notify == model.ChannelNotifyNone { - return NotifyPropsReasonSetToNone + return model.NotificationReasonLevelSetToNone } if notify == model.ChannelNotifyMention && !wasMentioned { - return NotifyPropsReasonSetToMention + return model.NotificationReasonNotMentioned } if (notify == model.ChannelNotifyAll) && @@ -623,10 +619,10 @@ func DoesNotifyPropsAllowPushNotification(user *model.User, channelNotifyProps m return "" } -func DoesStatusAllowPushNotification(userNotifyProps model.StringMap, status *model.Status, channelID string) statusReason { +func DoesStatusAllowPushNotification(userNotifyProps model.StringMap, status *model.Status, channelID string) model.NotificationReason { // If User status is DND or OOO return false right away if status.Status == model.StatusDnd || status.Status == model.StatusOutOfOffice { - return StatusReasonDNDOrOOO + return model.NotificationReasonUserStatus } pushStatus, ok := userNotifyProps[model.PushStatusNotifyProp] @@ -642,7 +638,7 @@ func DoesStatusAllowPushNotification(userNotifyProps model.StringMap, status *mo return "" } - return StatusReasonIsActive + return model.NotificationReasonUserIsActive } func (a *App) BuildPushNotificationMessage(c request.CTX, contentsConfig string, post *model.Post, user *model.User, channel *model.Channel, channelName string, senderName string, @@ -689,11 +685,12 @@ func (a *App) SendTestPushNotification(deviceID string) string { pushResponse, err := a.rawSendToPushProxy(msg) if err != nil { + a.CountNotificationReason(model.NotificationStatusError, model.NotificationTypePush, model.NotificationReasonPushProxySendError) a.NotificationsLog().Error("Failed to send test notification to push proxy", - mlog.String("type", model.TypePush), + mlog.String("type", model.NotificationTypePush), mlog.String("push_type", msg.Type), - mlog.String("status", model.StatusServerError), - mlog.String("reason", model.ReasonPushProxyError), + mlog.String("status", model.NotificationStatusError), + mlog.String("reason", model.NotificationReasonPushProxySendError), mlog.String("device_id", msg.DeviceId), mlog.Err(err), ) @@ -704,11 +701,12 @@ func (a *App) SendTestPushNotification(deviceID string) string { case model.PushStatusRemove: return "false" case model.PushStatusFail: + a.CountNotificationReason(model.NotificationStatusError, model.NotificationTypePush, model.NotificationReasonPushProxyError) a.NotificationsLog().Error("Push proxy failed to send test notification", - mlog.String("type", model.TypePush), + mlog.String("type", model.NotificationTypePush), mlog.String("push_type", msg.Type), - mlog.String("status", model.StatusServerError), - mlog.String("reason", model.ReasonPushProxyError), + mlog.String("status", model.NotificationStatusError), + mlog.String("reason", model.NotificationReasonPushProxyError), mlog.String("device_id", msg.DeviceId), mlog.Err(errors.New(pushResponse[model.PushStatusErrorMsg])), ) diff --git a/server/channels/app/notification_push_test.go b/server/channels/app/notification_push_test.go index aa34c5e8e90..bce213904a4 100644 --- a/server/channels/app/notification_push_test.go +++ b/server/channels/app/notification_push_test.go @@ -34,7 +34,7 @@ func TestDoesNotifyPropsAllowPushNotification(t *testing.T) { withSystemPost bool wasMentioned bool isMuted bool - expected notifyPropsReason + expected model.NotificationReason isGM bool }{ { @@ -44,7 +44,7 @@ func TestDoesNotifyPropsAllowPushNotification(t *testing.T) { withSystemPost: true, wasMentioned: false, isMuted: false, - expected: NotifyPropsReasonSystemMessage, + expected: model.NotificationReasonSystemMessage, isGM: false, }, { @@ -54,7 +54,7 @@ func TestDoesNotifyPropsAllowPushNotification(t *testing.T) { withSystemPost: true, wasMentioned: true, isMuted: false, - expected: NotifyPropsReasonSystemMessage, + expected: model.NotificationReasonSystemMessage, isGM: false, }, { @@ -84,7 +84,7 @@ func TestDoesNotifyPropsAllowPushNotification(t *testing.T) { withSystemPost: false, wasMentioned: false, isMuted: false, - expected: NotifyPropsReasonSetToMention, + expected: model.NotificationReasonNotMentioned, isGM: false, }, { @@ -104,7 +104,7 @@ func TestDoesNotifyPropsAllowPushNotification(t *testing.T) { withSystemPost: false, wasMentioned: false, isMuted: false, - expected: NotifyPropsReasonSetToNone, + expected: model.NotificationReasonLevelSetToNone, isGM: false, }, { @@ -114,7 +114,7 @@ func TestDoesNotifyPropsAllowPushNotification(t *testing.T) { withSystemPost: false, wasMentioned: true, isMuted: false, - expected: NotifyPropsReasonSetToNone, + expected: model.NotificationReasonLevelSetToNone, isGM: false, }, { @@ -144,7 +144,7 @@ func TestDoesNotifyPropsAllowPushNotification(t *testing.T) { withSystemPost: false, wasMentioned: false, isMuted: false, - expected: NotifyPropsReasonSetToMention, + expected: model.NotificationReasonNotMentioned, isGM: false, }, { @@ -164,7 +164,7 @@ func TestDoesNotifyPropsAllowPushNotification(t *testing.T) { withSystemPost: false, wasMentioned: false, isMuted: false, - expected: NotifyPropsReasonSetToNone, + expected: model.NotificationReasonLevelSetToNone, isGM: false, }, { @@ -174,7 +174,7 @@ func TestDoesNotifyPropsAllowPushNotification(t *testing.T) { withSystemPost: false, wasMentioned: true, isMuted: false, - expected: NotifyPropsReasonSetToNone, + expected: model.NotificationReasonLevelSetToNone, isGM: false, }, { @@ -244,7 +244,7 @@ func TestDoesNotifyPropsAllowPushNotification(t *testing.T) { withSystemPost: false, wasMentioned: false, isMuted: false, - expected: NotifyPropsReasonSetToMention, + expected: model.NotificationReasonNotMentioned, isGM: false, }, { @@ -264,7 +264,7 @@ func TestDoesNotifyPropsAllowPushNotification(t *testing.T) { withSystemPost: false, wasMentioned: false, isMuted: false, - expected: NotifyPropsReasonSetToMention, + expected: model.NotificationReasonNotMentioned, isGM: false, }, { @@ -284,7 +284,7 @@ func TestDoesNotifyPropsAllowPushNotification(t *testing.T) { withSystemPost: false, wasMentioned: false, isMuted: false, - expected: NotifyPropsReasonSetToMention, + expected: model.NotificationReasonNotMentioned, isGM: false, }, { @@ -304,7 +304,7 @@ func TestDoesNotifyPropsAllowPushNotification(t *testing.T) { withSystemPost: false, wasMentioned: false, isMuted: false, - expected: NotifyPropsReasonSetToNone, + expected: model.NotificationReasonLevelSetToNone, isGM: false, }, { @@ -314,7 +314,7 @@ func TestDoesNotifyPropsAllowPushNotification(t *testing.T) { withSystemPost: false, wasMentioned: true, isMuted: false, - expected: NotifyPropsReasonSetToNone, + expected: model.NotificationReasonLevelSetToNone, isGM: false, }, { @@ -324,7 +324,7 @@ func TestDoesNotifyPropsAllowPushNotification(t *testing.T) { withSystemPost: false, wasMentioned: false, isMuted: false, - expected: NotifyPropsReasonSetToNone, + expected: model.NotificationReasonLevelSetToNone, isGM: false, }, { @@ -334,7 +334,7 @@ func TestDoesNotifyPropsAllowPushNotification(t *testing.T) { withSystemPost: false, wasMentioned: true, isMuted: false, - expected: NotifyPropsReasonSetToNone, + expected: model.NotificationReasonLevelSetToNone, isGM: false, }, { @@ -344,7 +344,7 @@ func TestDoesNotifyPropsAllowPushNotification(t *testing.T) { withSystemPost: false, wasMentioned: false, isMuted: false, - expected: NotifyPropsReasonSetToNone, + expected: model.NotificationReasonLevelSetToNone, isGM: false, }, { @@ -354,7 +354,7 @@ func TestDoesNotifyPropsAllowPushNotification(t *testing.T) { withSystemPost: false, wasMentioned: true, isMuted: false, - expected: NotifyPropsReasonSetToNone, + expected: model.NotificationReasonLevelSetToNone, isGM: false, }, { @@ -364,7 +364,7 @@ func TestDoesNotifyPropsAllowPushNotification(t *testing.T) { withSystemPost: false, wasMentioned: false, isMuted: true, - expected: NotifyPropsReasonChannelMuted, + expected: model.NotificationReasonChannelMuted, isGM: false, }, { @@ -374,7 +374,7 @@ func TestDoesNotifyPropsAllowPushNotification(t *testing.T) { withSystemPost: false, wasMentioned: false, isMuted: false, - expected: NotifyPropsReasonSetToNone, + expected: model.NotificationReasonLevelSetToNone, isGM: true, }, { @@ -404,7 +404,7 @@ func TestDoesNotifyPropsAllowPushNotification(t *testing.T) { withSystemPost: false, wasMentioned: false, isMuted: false, - expected: NotifyPropsReasonSetToMention, + expected: model.NotificationReasonNotMentioned, isGM: true, }, } @@ -447,7 +447,7 @@ func TestDoesStatusAllowPushNotification(t *testing.T) { userNotifySetting string status *model.Status channelID string - expected statusReason + expected model.NotificationReason }{ { name: "WHEN props is ONLINE and user is offline with channel", @@ -489,21 +489,21 @@ func TestDoesStatusAllowPushNotification(t *testing.T) { userNotifySetting: model.StatusOnline, status: online, channelID: "", - expected: StatusReasonIsActive, + expected: model.NotificationReasonUserIsActive, }, { name: "WHEN props is ONLINE and user is dnd with channel", userNotifySetting: model.StatusOnline, status: dnd, channelID: channelID, - expected: StatusReasonDNDOrOOO, + expected: model.NotificationReasonUserStatus, }, { name: "WHEN props is ONLINE and user is dnd without channel", userNotifySetting: model.StatusOnline, status: dnd, channelID: "", - expected: StatusReasonDNDOrOOO, + expected: model.NotificationReasonUserStatus, }, { name: "WHEN props is AWAY and user is offline with channel", @@ -538,28 +538,28 @@ func TestDoesStatusAllowPushNotification(t *testing.T) { userNotifySetting: model.StatusAway, status: online, channelID: channelID, - expected: StatusReasonIsActive, + expected: model.NotificationReasonUserIsActive, }, { name: "WHEN props is AWAY and user is online without channel", userNotifySetting: model.StatusAway, status: online, channelID: "", - expected: StatusReasonIsActive, + expected: model.NotificationReasonUserIsActive, }, { name: "WHEN props is AWAY and user is dnd with channel", userNotifySetting: model.StatusAway, status: dnd, channelID: channelID, - expected: StatusReasonDNDOrOOO, + expected: model.NotificationReasonUserStatus, }, { name: "WHEN props is AWAY and user is dnd without channel", userNotifySetting: model.StatusAway, status: dnd, channelID: "", - expected: StatusReasonDNDOrOOO, + expected: model.NotificationReasonUserStatus, }, { name: "WHEN props is OFFLINE and user is offline with channel", @@ -580,42 +580,42 @@ func TestDoesStatusAllowPushNotification(t *testing.T) { userNotifySetting: model.StatusOffline, status: away, channelID: channelID, - expected: StatusReasonIsActive, + expected: model.NotificationReasonUserIsActive, }, { name: "WHEN props is OFFLINE and user is away without channel", userNotifySetting: model.StatusOffline, status: away, channelID: "", - expected: StatusReasonIsActive, + expected: model.NotificationReasonUserIsActive, }, { name: "WHEN props is OFFLINE and user is online with channel", userNotifySetting: model.StatusOffline, status: online, channelID: channelID, - expected: StatusReasonIsActive, + expected: model.NotificationReasonUserIsActive, }, { name: "WHEN props is OFFLINE and user is online without channel", userNotifySetting: model.StatusOffline, status: online, channelID: "", - expected: StatusReasonIsActive, + expected: model.NotificationReasonUserIsActive, }, { name: "WHEN props is OFFLINE and user is dnd with channel", userNotifySetting: model.StatusOffline, status: dnd, channelID: channelID, - expected: StatusReasonDNDOrOOO, + expected: model.NotificationReasonUserStatus, }, { name: "WHEN props is OFFLINE and user is dnd without channel", userNotifySetting: model.StatusOffline, status: dnd, channelID: "", - expected: StatusReasonDNDOrOOO, + expected: model.NotificationReasonUserStatus, }, } diff --git a/server/channels/app/opentracing/opentracing_layer.go b/server/channels/app/opentracing/opentracing_layer.go index bd46c5e179d..73e9d11ec23 100644 --- a/server/channels/app/opentracing/opentracing_layer.go +++ b/server/channels/app/opentracing/opentracing_layer.go @@ -1967,6 +1967,51 @@ func (a *OpenTracingAppLayer) CopyWranglerPostlist(c request.CTX, wpl *model.Wra return resultVar0, resultVar1 } +func (a *OpenTracingAppLayer) CountNotification(notificationType model.NotificationType) { + origCtx := a.ctx + span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.CountNotification") + + a.ctx = newCtx + a.app.Srv().Store().SetContext(newCtx) + defer func() { + a.app.Srv().Store().SetContext(origCtx) + a.ctx = origCtx + }() + + defer span.Finish() + a.app.CountNotification(notificationType) +} + +func (a *OpenTracingAppLayer) CountNotificationAck(notificationType model.NotificationType) { + origCtx := a.ctx + span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.CountNotificationAck") + + a.ctx = newCtx + a.app.Srv().Store().SetContext(newCtx) + defer func() { + a.app.Srv().Store().SetContext(origCtx) + a.ctx = origCtx + }() + + defer span.Finish() + a.app.CountNotificationAck(notificationType) +} + +func (a *OpenTracingAppLayer) CountNotificationReason(notificationStatus model.NotificationStatus, notificationType model.NotificationType, notificationReason model.NotificationReason) { + origCtx := a.ctx + span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.CountNotificationReason") + + a.ctx = newCtx + a.app.Srv().Store().SetContext(newCtx) + defer func() { + a.app.Srv().Store().SetContext(origCtx) + a.ctx = origCtx + }() + + defer span.Finish() + a.app.CountNotificationReason(notificationStatus, notificationType, notificationReason) +} + func (a *OpenTracingAppLayer) CreateBot(rctx request.CTX, bot *model.Bot) (*model.Bot, *model.AppError) { origCtx := a.ctx span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.CreateBot") diff --git a/server/channels/app/post.go b/server/channels/app/post.go index ab06ed95399..f1ddbdfbdd7 100644 --- a/server/channels/app/post.go +++ b/server/channels/app/post.go @@ -357,11 +357,12 @@ func (a *App) CreatePost(c request.CTX, post *model.Post, channel *model.Channel if rpost.RootId != "" { if appErr := a.ResolvePersistentNotification(c, parentPostList.Posts[post.RootId], rpost.UserId); appErr != nil { + a.CountNotificationReason(model.NotificationStatusError, model.NotificationTypeWebsocket, model.NotificationReasonResolvePersistentNotificationError) a.NotificationsLog().Error("Error resolving persistent notification", mlog.String("sender_id", rpost.UserId), mlog.String("post_id", post.RootId), - mlog.String("status", model.StatusServerError), - mlog.String("reason", model.ReasonFetchError), + mlog.String("status", model.NotificationStatusError), + mlog.String("reason", model.NotificationReasonResolvePersistentNotificationError), mlog.Err(appErr), ) return nil, appErr @@ -487,10 +488,11 @@ func (a *App) handlePostEvents(c request.CTX, post *model.Post, user *model.User if channel.TeamId != "" { t, err := a.Srv().Store().Team().Get(channel.TeamId) if err != nil { + a.CountNotificationReason(model.NotificationStatusError, model.NotificationTypeAll, model.NotificationReasonFetchError) a.NotificationsLog().Error("Missing team", mlog.String("post_id", post.Id), - mlog.String("status", model.StatusServerError), - mlog.String("reason", model.ReasonFetchError), + mlog.String("status", model.NotificationStatusError), + mlog.String("reason", model.NotificationReasonFetchError), mlog.Err(err), ) return err @@ -781,11 +783,12 @@ func (a *App) publishWebsocketEventForPermalinkPost(c request.CTX, post *model.P } if !model.IsValidId(previewedPostID) { - a.NotificationsLog().Warn("Invalid post prop id for permalink post", - mlog.String("type", model.TypeWebsocket), + a.CountNotificationReason(model.NotificationStatusError, model.NotificationTypeAll, model.NotificationReasonParseError) + a.NotificationsLog().Error("Invalid post prop id for permalink post", + mlog.String("type", model.NotificationTypeWebsocket), mlog.String("post_id", post.Id), - mlog.String("status", model.StatusServerError), - mlog.String("reason", model.ReasonServerError), + mlog.String("status", model.NotificationStatusError), + mlog.String("reason", model.NotificationReasonParseError), mlog.String("prop_value", previewedPostID), ) c.Logger().Warn("invalid post prop value", mlog.String("prop_key", model.PostPropsPreviewedPost), mlog.String("prop_value", previewedPostID)) @@ -795,12 +798,14 @@ func (a *App) publishWebsocketEventForPermalinkPost(c request.CTX, post *model.P previewedPost, err := a.GetSinglePost(previewedPostID, false) if err != nil { if err.StatusCode == http.StatusNotFound { - a.NotificationsLog().Warn("permalink post not found", - mlog.String("type", model.TypeWebsocket), + a.CountNotificationReason(model.NotificationStatusError, model.NotificationTypeAll, model.NotificationReasonFetchError) + a.NotificationsLog().Error("permalink post not found", + mlog.String("type", model.NotificationTypeWebsocket), mlog.String("post_id", post.Id), - mlog.String("status", model.StatusServerError), - mlog.String("reason", model.ReasonServerError), + mlog.String("status", model.NotificationStatusError), + mlog.String("reason", model.NotificationReasonFetchError), mlog.String("referenced_post_id", previewedPostID), + mlog.Err(err), ) c.Logger().Warn("permalinked post not found", mlog.String("referenced_post_id", previewedPostID)) return false, nil @@ -810,12 +815,29 @@ func (a *App) publishWebsocketEventForPermalinkPost(c request.CTX, post *model.P userIDs, nErr := a.Srv().Store().Channel().GetAllChannelMemberIdsByChannelId(post.ChannelId) if nErr != nil { + a.CountNotificationReason(model.NotificationStatusError, model.NotificationTypeAll, model.NotificationReasonFetchError) + a.NotificationsLog().Error("Cannot get channel members", + mlog.String("type", model.NotificationTypeWebsocket), + mlog.String("post_id", post.Id), + mlog.String("status", model.NotificationStatusError), + mlog.String("reason", model.NotificationReasonFetchError), + mlog.String("referenced_post_id", previewedPostID), + mlog.Err(nErr), + ) return false, model.NewAppError("publishWebsocketEventForPermalinkPost", "app.channel.get_members.app_error", nil, "", http.StatusInternalServerError).Wrap(err) } permalinkPreviewedChannel, err := a.GetChannel(c, previewedPost.ChannelId) if err != nil { if err.StatusCode == http.StatusNotFound { + a.CountNotificationReason(model.NotificationStatusError, model.NotificationTypeAll, model.NotificationReasonFetchError) + a.NotificationsLog().Error("Cannot get channel", + mlog.String("type", model.NotificationTypeWebsocket), + mlog.String("post_id", post.Id), + mlog.String("status", model.NotificationStatusError), + mlog.String("reason", model.NotificationReasonFetchError), + mlog.String("referenced_post_id", previewedPostID), + ) c.Logger().Warn("channel containing permalinked post not found", mlog.String("referenced_channel_id", previewedPost.ChannelId)) return false, nil } diff --git a/server/channels/app/post_acknowledgements.go b/server/channels/app/post_acknowledgements.go index d03b1f95816..92d74b88a2d 100644 --- a/server/channels/app/post_acknowledgements.go +++ b/server/channels/app/post_acknowledgements.go @@ -42,11 +42,12 @@ func (a *App) SaveAcknowledgementForPost(c request.CTX, postID, userID string) ( } if appErr := a.ResolvePersistentNotification(c, post, userID); appErr != nil { + a.CountNotificationReason(model.NotificationStatusError, model.NotificationTypeWebsocket, model.NotificationReasonResolvePersistentNotificationError) a.NotificationsLog().Error("Error resolving persistent notification", mlog.String("sender_id", userID), mlog.String("post_id", post.RootId), - mlog.String("status", model.StatusServerError), - mlog.String("reason", model.ReasonFetchError), + mlog.String("status", model.NotificationStatusError), + mlog.String("reason", model.NotificationReasonResolvePersistentNotificationError), mlog.Err(appErr), ) return nil, appErr diff --git a/server/channels/app/reaction.go b/server/channels/app/reaction.go index 6239832b0a3..3a37e6ad957 100644 --- a/server/channels/app/reaction.go +++ b/server/channels/app/reaction.go @@ -67,11 +67,12 @@ func (a *App) SaveReactionForPost(c request.CTX, reaction *model.Reaction) (*mod if post.RootId == "" { if appErr := a.ResolvePersistentNotification(c, post, reaction.UserId); appErr != nil { + a.CountNotificationReason(model.NotificationStatusError, model.NotificationTypeAll, model.NotificationReasonResolvePersistentNotificationError) a.NotificationsLog().Error("Error resolving persistent notification", mlog.String("sender_id", reaction.UserId), mlog.String("post_id", post.RootId), - mlog.String("status", model.StatusServerError), - mlog.String("reason", model.ReasonFetchError), + mlog.String("status", model.NotificationStatusError), + mlog.String("reason", model.NotificationReasonResolvePersistentNotificationError), mlog.Err(appErr), ) return nil, appErr diff --git a/server/channels/app/web_broadcast_hooks.go b/server/channels/app/web_broadcast_hooks.go index 944ebc93c08..d13ffce8234 100644 --- a/server/channels/app/web_broadcast_hooks.go +++ b/server/channels/app/web_broadcast_hooks.go @@ -86,6 +86,7 @@ func (h *postedAckBroadcastHook) Process(msg *platform.HookedWebSocketEvent, web // This works since we currently do have an order for broadcast hooks, but this probably should be reworked going forward if msg.Get("followers") != nil || msg.Get("mentions") != nil { msg.Add("should_ack", true) + incrementWebsocketCounter(webConn) return nil } @@ -107,6 +108,7 @@ func (h *postedAckBroadcastHook) Process(msg *platform.HookedWebSocketEvent, web // Always ACK direct channels if channelType == model.ChannelTypeDirect { msg.Add("should_ack", true) + incrementWebsocketCounter(webConn) return nil } @@ -117,11 +119,24 @@ func (h *postedAckBroadcastHook) Process(msg *platform.HookedWebSocketEvent, web if len(users) > 0 && slices.Contains(users, webConn.UserId) { msg.Add("should_ack", true) + incrementWebsocketCounter(webConn) } return nil } +func incrementWebsocketCounter(wc *platform.WebConn) { + if wc.Platform.Metrics() == nil { + return + } + + if !wc.Platform.Config().FeatureFlags.NotificationMonitoring { + return + } + + wc.Platform.Metrics().IncrementNotificationCounter(model.NotificationTypeWebsocket) +} + // getTypedArg returns a correctly typed hook argument with the given key, reinterpreting the type using JSON encoding // if necessary. This is needed because broadcast hook args are JSON encoded in a multi-server environment, and any // type information is lost because those types aren't known at decode time. diff --git a/server/channels/app/web_broadcast_hooks_test.go b/server/channels/app/web_broadcast_hooks_test.go index cbfa71963c4..305bbb5cad4 100644 --- a/server/channels/app/web_broadcast_hooks_test.go +++ b/server/channels/app/web_broadcast_hooks_test.go @@ -87,7 +87,8 @@ func TestPostedAckHook_Process(t *testing.T) { hook := &postedAckBroadcastHook{} userID := model.NewId() webConn := &platform.WebConn{ - UserId: userID, + UserId: userID, + Platform: &platform.PlatformService{}, } t.Run("should ack if user is in the list of users to notify", func(t *testing.T) { diff --git a/server/channels/wsapi/system.go b/server/channels/wsapi/system.go index 8efb1923bb6..d65fd726e92 100644 --- a/server/channels/wsapi/system.go +++ b/server/channels/wsapi/system.go @@ -26,14 +26,38 @@ func ping(req *model.WebSocketRequest) (map[string]any, *model.AppError) { func (api *API) websocketNotificationAck(req *model.WebSocketRequest) (map[string]any, *model.AppError) { // Log the ACKs if necessary api.App.NotificationsLog().Debug("Websocket notification acknowledgment", - mlog.String("type", model.TypeWebsocket), + mlog.String("type", model.NotificationTypeWebsocket), mlog.String("user_id", req.Session.UserId), mlog.Any("user_agent", req.Data["user_agent"]), mlog.Any("post_id", req.Data["post_id"]), - mlog.Any("result", req.Data["result"]), + mlog.Any("status", req.Data["status"]), mlog.Any("reason", req.Data["reason"]), mlog.Any("data", req.Data["data"]), ) + // Count metrics for websocket acks + api.App.CountNotificationAck(model.NotificationTypeWebsocket) + + status := req.Data["status"] + reason := req.Data["reason"] + if status == nil { + return nil, nil + } + + notificationStatus := model.NotificationStatus(status.(string)) + if reason == nil && notificationStatus != model.NotificationStatusSuccess { + return nil, nil + } + var notificationReason model.NotificationReason + if reason != nil { + notificationReason = model.NotificationReason(reason.(string)) + } + + api.App.CountNotificationReason( + notificationStatus, + model.NotificationTypeWebsocket, + notificationReason, + ) + return nil, nil } diff --git a/server/einterfaces/metrics.go b/server/einterfaces/metrics.go index dac9ca016dd..2483451461b 100644 --- a/server/einterfaces/metrics.go +++ b/server/einterfaces/metrics.go @@ -95,4 +95,10 @@ type MetricsInterface interface { SetReplicaLagAbsolute(node string, value float64) SetReplicaLagTime(node string, value float64) + + IncrementNotificationCounter(notificationType model.NotificationType) + IncrementNotificationAckCounter(notificationType model.NotificationType) + IncrementNotificationSuccessCounter(notificationType model.NotificationType) + IncrementNotificationErrorCounter(notificationType model.NotificationType, errorReason model.NotificationReason) + IncrementNotificationNotSentCounter(notificationType model.NotificationType, notSentReason model.NotificationReason) } diff --git a/server/einterfaces/mocks/MetricsInterface.go b/server/einterfaces/mocks/MetricsInterface.go index 27ecc9ca0fc..f7440510fdd 100644 --- a/server/einterfaces/mocks/MetricsInterface.go +++ b/server/einterfaces/mocks/MetricsInterface.go @@ -163,6 +163,31 @@ func (_m *MetricsInterface) IncrementMemCacheMissCounterSession() { _m.Called() } +// IncrementNotificationAckCounter provides a mock function with given fields: notificationType +func (_m *MetricsInterface) IncrementNotificationAckCounter(notificationType model.NotificationType) { + _m.Called(notificationType) +} + +// IncrementNotificationCounter provides a mock function with given fields: notificationType +func (_m *MetricsInterface) IncrementNotificationCounter(notificationType model.NotificationType) { + _m.Called(notificationType) +} + +// IncrementNotificationErrorCounter provides a mock function with given fields: notificationType, errorReason +func (_m *MetricsInterface) IncrementNotificationErrorCounter(notificationType model.NotificationType, errorReason model.NotificationReason) { + _m.Called(notificationType, errorReason) +} + +// IncrementNotificationNotSentCounter provides a mock function with given fields: notificationType, notSentReason +func (_m *MetricsInterface) IncrementNotificationNotSentCounter(notificationType model.NotificationType, notSentReason model.NotificationReason) { + _m.Called(notificationType, notSentReason) +} + +// IncrementNotificationSuccessCounter provides a mock function with given fields: notificationType +func (_m *MetricsInterface) IncrementNotificationSuccessCounter(notificationType model.NotificationType) { + _m.Called(notificationType) +} + // IncrementPostBroadcast provides a mock function with given fields: func (_m *MetricsInterface) IncrementPostBroadcast() { _m.Called() diff --git a/server/enterprise/metrics/metrics.go b/server/enterprise/metrics/metrics.go index 06caeac358f..06d0a5e9f36 100644 --- a/server/enterprise/metrics/metrics.go +++ b/server/enterprise/metrics/metrics.go @@ -41,6 +41,7 @@ const ( MetricsSubsystemSharedChannels = "shared_channels" MetricsSubsystemSystem = "system" MetricsSubsystemJobs = "jobs" + MetricsSubsystemNotifications = "notifications" MetricsCloudInstallationLabel = "installationId" MetricsCloudDatabaseClusterLabel = "databaseClusterName" MetricsCloudInstallationGroupLabel = "installationGroupId" @@ -201,6 +202,12 @@ type MetricsInterfaceImpl struct { ServerStartTime prometheus.Gauge JobsActive *prometheus.GaugeVec + + NotificationTotalCounters *prometheus.CounterVec + NotificationAckCounters *prometheus.CounterVec + NotificationSuccessCounters *prometheus.CounterVec + NotificationErrorCounters *prometheus.CounterVec + NotificationNotSentCounters *prometheus.CounterVec } func init() { @@ -1041,6 +1048,67 @@ func New(ps *platform.PlatformService, driver, dataSource string) *MetricsInterf []string{"type"}, ) m.Registry.MustRegister(m.JobsActive) + + m.NotificationTotalCounters = prometheus.NewCounterVec( + prometheus.CounterOpts{ + Namespace: MetricsNamespace, + Subsystem: MetricsSubsystemNotifications, + Name: "total", + Help: "Total number of notification events", + ConstLabels: additionalLabels, + }, + []string{"type"}, + ) + m.Registry.MustRegister(m.NotificationTotalCounters) + + m.NotificationAckCounters = prometheus.NewCounterVec( + prometheus.CounterOpts{ + Namespace: MetricsNamespace, + Subsystem: MetricsSubsystemNotifications, + Name: "total_ack", + Help: "Total number of notification events acknowledged", + ConstLabels: additionalLabels, + }, + []string{"type"}, + ) + m.Registry.MustRegister(m.NotificationAckCounters) + + m.NotificationSuccessCounters = prometheus.NewCounterVec( + prometheus.CounterOpts{ + Namespace: MetricsNamespace, + Subsystem: MetricsSubsystemNotifications, + Name: "success", + Help: "Total number of successfully sent notifications", + ConstLabels: additionalLabels, + }, + []string{"type"}, + ) + m.Registry.MustRegister(m.NotificationSuccessCounters) + + m.NotificationErrorCounters = prometheus.NewCounterVec( + prometheus.CounterOpts{ + Namespace: MetricsNamespace, + Subsystem: MetricsSubsystemNotifications, + Name: "error", + Help: "Total number of errors that stop the notification flow", + ConstLabels: additionalLabels, + }, + []string{"type", "reason"}, + ) + m.Registry.MustRegister(m.NotificationErrorCounters) + + m.NotificationNotSentCounters = prometheus.NewCounterVec( + prometheus.CounterOpts{ + Namespace: MetricsNamespace, + Subsystem: MetricsSubsystemNotifications, + Name: "not_sent", + Help: "Total number of notifications the system deliberately did not send", + ConstLabels: additionalLabels, + }, + []string{"type", "reason"}, + ) + m.Registry.MustRegister(m.NotificationNotSentCounters) + return m } @@ -1467,6 +1535,26 @@ func (mi *MetricsInterfaceImpl) SetReplicaLagTime(node string, value float64) { mi.DbReplicaLagGaugeTime.With(prometheus.Labels{"node": node}).Set(value) } +func (mi *MetricsInterfaceImpl) IncrementNotificationCounter(notificationType model.NotificationType) { + mi.NotificationTotalCounters.With(prometheus.Labels{"type": string(notificationType)}).Inc() +} + +func (mi *MetricsInterfaceImpl) IncrementNotificationAckCounter(notificationType model.NotificationType) { + mi.NotificationAckCounters.With(prometheus.Labels{"type": string(notificationType)}).Inc() +} + +func (mi *MetricsInterfaceImpl) IncrementNotificationSuccessCounter(notificationType model.NotificationType) { + mi.NotificationSuccessCounters.With(prometheus.Labels{"type": string(notificationType)}).Inc() +} + +func (mi *MetricsInterfaceImpl) IncrementNotificationErrorCounter(notificationType model.NotificationType, errorReason model.NotificationReason) { + mi.NotificationErrorCounters.With(prometheus.Labels{"type": string(notificationType), "reason": string(errorReason)}).Inc() +} + +func (mi *MetricsInterfaceImpl) IncrementNotificationNotSentCounter(notificationType model.NotificationType, notSentReason model.NotificationReason) { + mi.NotificationNotSentCounters.With(prometheus.Labels{"type": string(notificationType), "reason": string(notSentReason)}).Inc() +} + func (mi *MetricsInterfaceImpl) IncrementHTTPWebSockets(originClient string) { mi.HTTPWebsocketsGauge.With(prometheus.Labels{"origin_client": originClient}).Inc() } diff --git a/server/public/model/feature_flags.go b/server/public/model/feature_flags.go index 717205e84aa..260cf34cd80 100644 --- a/server/public/model/feature_flags.go +++ b/server/public/model/feature_flags.go @@ -55,6 +55,8 @@ type FeatureFlags struct { ChannelBookmarks bool WebSocketEventScope bool + + NotificationMonitoring bool } func (f *FeatureFlags) SetDefaults() { @@ -77,6 +79,7 @@ func (f *FeatureFlags) SetDefaults() { f.CloudDedicatedExportUI = false f.ChannelBookmarks = false f.WebSocketEventScope = false + f.NotificationMonitoring = true } // ToMap returns the feature flags as a map[string]string diff --git a/server/public/model/notification.go b/server/public/model/notification.go index 8a61b796704..eb511fcac50 100644 --- a/server/public/model/notification.go +++ b/server/public/model/notification.go @@ -3,20 +3,36 @@ package model +type NotificationStatus string +type NotificationType string +type NotificationReason string + const ( - StatusBlocked = "blocked" - StatusServerError = "server_error" - StatusNotSent = "not_sent" + NotificationStatusSuccess NotificationStatus = "success" + NotificationStatusError NotificationStatus = "error" + NotificationStatusNotSent NotificationStatus = "not_sent" - TypeEmail = "email" - TypeWebsocket = "websocket" - TypePush = "push" + NotificationTypeAll NotificationType = "all" + NotificationTypeEmail NotificationType = "email" + NotificationTypeWebsocket NotificationType = "websocket" + NotificationTypePush NotificationType = "push" - ReasonServerConfig = "server_config" - ReasonUserConfig = "user_config" - ReasonUserStatus = "user_status" - ReasonFetchError = "error_fetching" - ReasonServerError = "server_error" - ReasonMissingProfile = "missing_profile" - ReasonPushProxyError = "push_proxy_error" + NotificationReasonFetchError NotificationReason = "fetch_error" + NotificationReasonParseError NotificationReason = "json_parse_error" + NotificationReasonPushProxyError NotificationReason = "push_proxy_error" + NotificationReasonPushProxySendError NotificationReason = "push_proxy_send_error" + NotificationReasonRejectedByPlugin NotificationReason = "rejected_by_plugin" + NotificationReasonSessionExpired NotificationReason = "session_expired" + NotificationReasonChannelMuted NotificationReason = "channel_muted" + NotificationReasonSystemMessage NotificationReason = "system_message" + NotificationReasonLevelSetToNone NotificationReason = "notify_level_none" + NotificationReasonNotMentioned NotificationReason = "not_mentioned" + NotificationReasonUserStatus NotificationReason = "user_status" + NotificationReasonUserIsActive NotificationReason = "user_is_active" + NotificationReasonMissingProfile NotificationReason = "missing_profile" + NotificationReasonEmailNotVerified NotificationReason = "email_not_verified" + NotificationReasonEmailSendError NotificationReason = "email_send_error" + NotificationReasonTooManyUsersInChannel NotificationReason = "too_many_users_in_channel" + NotificationReasonResolvePersistentNotificationError NotificationReason = "resolve_persistent_notification_error" + NotificationReasonMissingThreadMembership NotificationReason = "missing_thread_membership" ) diff --git a/webapp/channels/package.json b/webapp/channels/package.json index c3d1c319559..1f1d82d095f 100644 --- a/webapp/channels/package.json +++ b/webapp/channels/package.json @@ -13,7 +13,7 @@ "@mattermost/client": "*", "@mattermost/compass-components": "^0.2.12", "@mattermost/compass-icons": "0.1.39", - "@mattermost/desktop-api": "5.8.0-4", + "@mattermost/desktop-api": "5.8.0-5", "@mattermost/types": "*", "@mui/base": "5.0.0-alpha.127", "@mui/material": "5.11.16", diff --git a/webapp/channels/src/actions/new_post.ts b/webapp/channels/src/actions/new_post.ts index 4ce59a6bd01..76162ec30a3 100644 --- a/webapp/channels/src/actions/new_post.ts +++ b/webapp/channels/src/actions/new_post.ts @@ -80,11 +80,11 @@ export function completePostReceive(post: Post, websocketMessageProps: NewPostMe dispatch(setThreadRead(post)); } - const {result, reason, data} = await dispatch(sendDesktopNotification(post, websocketMessageProps)); + const {status, reason, data} = await dispatch(sendDesktopNotification(post, websocketMessageProps)); // Only ACK for posts that require it if (websocketMessageProps.should_ack) { - WebSocketClient.acknowledgePostedNotification(post.id, result, reason, data); + WebSocketClient.acknowledgePostedNotification(post.id, status, reason, data); } return {data: true}; diff --git a/webapp/channels/src/actions/notification_actions.jsx b/webapp/channels/src/actions/notification_actions.jsx index e6abadf0e33..446cf87493f 100644 --- a/webapp/channels/src/actions/notification_actions.jsx +++ b/webapp/channels/src/actions/notification_actions.jsx @@ -57,11 +57,11 @@ export function sendDesktopNotification(post, msgProps) { const currentUserId = getCurrentUserId(state); if ((currentUserId === post.user_id && post.props.from_webhook !== 'true')) { - return {result: 'not_sent', reason: 'own_post'}; + return {status: 'not_sent', reason: 'own_post'}; } if (isSystemMessage(post) && !isUserAddedInChannel(post, currentUserId)) { - return {result: 'not_sent', reason: 'system_message'}; + return {status: 'not_sent', reason: 'system_message'}; } let userFromPost = getUser(state, post.user_id); @@ -92,15 +92,15 @@ export function sendDesktopNotification(post, msgProps) { const isCrtReply = isCollapsedThreadsEnabled(state) && post.root_id !== ''; if (!member) { - return {result: 'not_sent', reason: 'no_member'}; + return {status: 'error', reason: 'no_member'}; } if (isChannelMuted(member)) { - return {result: 'not_sent', reason: 'channel_muted'}; + return {status: 'not_sent', reason: 'channel_muted'}; } if (userStatus === UserStatuses.DND || userStatus === UserStatuses.OUT_OF_OFFICE) { - return {result: 'not_sent', reason: 'user_status', data: userStatus}; + return {status: 'not_sent', reason: 'user_status', data: userStatus}; } const channelNotifyProp = member?.notify_props?.desktop || NotificationLevels.DEFAULT; @@ -115,7 +115,7 @@ export function sendDesktopNotification(post, msgProps) { } if (notifyLevel === NotificationLevels.NONE) { - return {result: 'not_sent', reason: 'notify_level', data: notifyLevel}; + return {status: 'not_sent', reason: 'notify_level_none'}; } else if (channel?.type === 'G' && notifyLevel === NotificationLevels.MENTION) { // Compose the whole text in the message, including interactive messages. let text = post.message; @@ -189,13 +189,13 @@ export function sendDesktopNotification(post, msgProps) { } if (!isExplicitlyMentioned) { - return {result: 'not_sent', reason: 'not_explicitly_mentioned', data: mentionableText}; + return {status: 'not_sent', reason: 'not_explicitly_mentioned', data: mentionableText}; } } else if (notifyLevel === NotificationLevels.MENTION && mentions.indexOf(user.id) === -1 && msgProps.channel_type !== Constants.DM_CHANNEL) { - return {result: 'not_sent', reason: 'not_mentioned'}; + return {status: 'not_sent', reason: 'not_mentioned'}; } else if (isCrtReply && notifyLevel === NotificationLevels.ALL && followers.indexOf(currentUserId) === -1) { // if user is not following the thread don't notify - return {result: 'not_sent', reason: 'not_following_thread'}; + return {status: 'not_sent', reason: 'not_following_thread'}; } const config = getConfig(state); @@ -273,18 +273,18 @@ export function sendDesktopNotification(post, msgProps) { const channelId = channel ? channel.id : null; let notify = false; - let notifyResult = {result: 'not_sent', reason: 'unknown'}; + let notifyResult = {status: 'not_sent', reason: 'unknown'}; if (state.views.browser.focused) { - notifyResult = {result: 'not_sent', reason: 'window_is_focused'}; + notifyResult = {status: 'not_sent', reason: 'window_is_focused'}; if (isCrtReply) { notify = !isThreadOpen(state, post.root_id); if (!notify) { - notifyResult = {result: 'not_sent', reason: 'thread_is_open', data: post.root_id}; + notifyResult = {status: 'not_sent', reason: 'thread_is_open', data: post.root_id}; } } else { notify = activeChannel && activeChannel.id !== channelId; if (!notify) { - notifyResult = {result: 'not_sent', reason: 'channel_is_open', data: activeChannel?.id}; + notifyResult = {status: 'not_sent', reason: 'channel_is_open', data: activeChannel?.id}; } } } else { @@ -305,7 +305,7 @@ export function sendDesktopNotification(post, msgProps) { const hookResult = await dispatch(runDesktopNotificationHooks(post, msgProps, channel, teamId, args)); if (hookResult.error) { dispatch(logError(hookResult.error)); - return {result: 'error', reason: 'desktop_notification_hook', data: String(hookResult.error)}; + return {status: 'error', reason: 'desktop_notification_hook', data: String(hookResult.error)}; } let silent = false; @@ -323,7 +323,7 @@ export function sendDesktopNotification(post, msgProps) { } if (args.notify && !notify) { - notifyResult = {result: 'not_sent', reason: 'desktop_notification_hook', data: String(hookResult)}; + notifyResult = {status: 'not_sent', reason: 'desktop_notification_hook', data: String(hookResult)}; } return notifyResult; @@ -349,6 +349,6 @@ export const notifyMe = (title, body, channel, teamId, silent, soundName, url) = }); } catch (error) { dispatch(logError(error)); - return {result: 'error', reason: 'notification_api', data: String(error)}; + return {status: 'error', reason: 'notification_api', data: String(error)}; } }; diff --git a/webapp/channels/src/utils/desktop_api.ts b/webapp/channels/src/utils/desktop_api.ts index 5ef2c477773..392740e3fd1 100644 --- a/webapp/channels/src/utils/desktop_api.ts +++ b/webapp/channels/src/utils/desktop_api.ts @@ -167,7 +167,7 @@ class DesktopAppAPI { ) => { if (window.desktopAPI?.sendNotification) { const result = await window.desktopAPI.sendNotification(title, body, channelId, teamId, url, silent, soundName); - return result ?? {result: 'unsupported', reason: 'desktop_app_unsupported'}; + return result ?? {status: 'unsupported', reason: 'desktop_app_unsupported'}; } // get the desktop app to trigger the notification @@ -186,7 +186,7 @@ class DesktopAppAPI { }, window.location.origin, ); - return {result: 'unsupported', reason: 'desktop_app_unsupported'}; + return {status: 'unsupported', reason: 'desktop_app_unsupported'}; }; doBrowserHistoryPush = (path: string) => { diff --git a/webapp/channels/src/utils/notifications.ts b/webapp/channels/src/utils/notifications.ts index 7373bae0d0d..e85f62470dc 100644 --- a/webapp/channels/src/utils/notifications.ts +++ b/webapp/channels/src/utils/notifications.ts @@ -53,7 +53,7 @@ export async function showNotification( if (Notification.permission !== 'granted' && requestedNotificationPermission) { // User didn't allow notifications - return {result: 'not_sent', reason: 'notifications_permission_previously_denied', data: Notification.permission, callback: () => {}}; + return {status: 'not_sent', reason: 'notifications_permission_previously_denied', data: Notification.permission, callback: () => {}}; } requestedNotificationPermission = true; @@ -68,7 +68,7 @@ export async function showNotification( if (permission !== 'granted') { // User has denied notification for the site - return {result: 'not_sent', reason: 'notifications_permission_denied', data: permission, callback: () => {}}; + return {status: 'not_sent', reason: 'notifications_permission_denied', data: permission, callback: () => {}}; } const notification = new Notification(title, { @@ -95,7 +95,7 @@ export async function showNotification( } return { - result: 'success', + status: 'success', callback: () => { notification.close(); }, diff --git a/webapp/package-lock.json b/webapp/package-lock.json index d6ef3df7661..631d84c42b9 100644 --- a/webapp/package-lock.json +++ b/webapp/package-lock.json @@ -62,7 +62,7 @@ "@mattermost/client": "*", "@mattermost/compass-components": "^0.2.12", "@mattermost/compass-icons": "0.1.39", - "@mattermost/desktop-api": "5.8.0-4", + "@mattermost/desktop-api": "5.8.0-5", "@mattermost/types": "*", "@mui/base": "5.0.0-alpha.127", "@mui/material": "5.11.16", @@ -4290,9 +4290,9 @@ "link": true }, "node_modules/@mattermost/desktop-api": { - "version": "5.8.0-4", - "resolved": "https://registry.npmjs.org/@mattermost/desktop-api/-/desktop-api-5.8.0-4.tgz", - "integrity": "sha512-oEbh3ByDgM432LrjO9JsRnIwqBIAkF6bJaQkHjYeQAYwkI4/s4CSQ4kZcfMiO9hE0MjfF1VeXnGTBv9wyWsbAw==", + "version": "5.8.0-5", + "resolved": "https://registry.npmjs.org/@mattermost/desktop-api/-/desktop-api-5.8.0-5.tgz", + "integrity": "sha512-YPtFRnduVFOXyK25GedJA+PkAKmFpLDKqfxvV/IIS+SMypv6BD17LJ8AkdBsguFFa6ZWLlZU40M5grKYBbjOuA==", "peerDependencies": { "typescript": "^4.3.0 || ^5.0.0" }, diff --git a/webapp/platform/client/src/websocket.ts b/webapp/platform/client/src/websocket.ts index 237c1d6c56b..73e15072922 100644 --- a/webapp/platform/client/src/websocket.ts +++ b/webapp/platform/client/src/websocket.ts @@ -412,11 +412,11 @@ export default class WebSocketClient { this.sendMessage('user_update_active_status', data, callback); } - acknowledgePostedNotification(postId: string, result: 'error' | 'not_sent' | 'unsupported' | 'success', reason?: string, postedData?: string) { + acknowledgePostedNotification(postId: string, status: string, reason?: string, postedData?: string) { const data = { post_id: postId, user_agent: window.navigator.userAgent, - result, + status, reason, data: postedData, }; diff --git a/webapp/platform/types/src/posts.ts b/webapp/platform/types/src/posts.ts index d12723a1f14..3634febc50d 100644 --- a/webapp/platform/types/src/posts.ts +++ b/webapp/platform/types/src/posts.ts @@ -221,3 +221,10 @@ export type PostInfo = { team_display_name: string; has_joined_team: boolean; } + +export type NotificationStatus = 'error' | 'not_sent' | 'unsupported' | 'success'; +export type NotificationResult = { + status: NotificationStatus; + reason?: string; + data?: string; +}