[MI-3651] Add nil checks missing for some metrics in the code (#369)

* [MI-3624] Add some additional metrics

* [MI-3624] Remove extra comments

* [MI-3624] Fix testcases

* [MI-3642] Add metrics to count upstream users

* [MI-3643] Add metrics to show messages confirmed count

* [MI-3644] Add metrics of subscription count for different actions

* [MI-3644] Revert extra changes

* [MI-3648] Add metrics for message sync time

* [MI-3648] Update logic of calculating time for the messages duration

* [MI-3651] Add nil checks missing for some metrics in the code

* [MI-3651] Add comment for better understanding

* [MI-3651] Add more nil checks

* [MI-3651] Fix test cases

* [MI-3651] Remove unwanted constants

server/api.go

@@ -114,7 +114,9 @@ func (a *API) processActivity(w http.ResponseWriter, req *http.Request) {
 			continue
 		}
 
-		a.p.metricsService.ObserveChangeEventTotal(activity.ChangeType)
+		if a.p.metricsService != nil {
+			a.p.metricsService.ObserveChangeEventTotal(activity.ChangeType)
+		}
 		if err := a.p.activityHandler.Handle(activity); err != nil {
 			a.p.API.LogError("Unable to process created activity", "activity", activity, "error", err.Error())
 			errors += err.Error() + "\n"
@@ -153,7 +155,9 @@ func (a *API) processLifecycle(w http.ResponseWriter, req *http.Request) {
 			a.p.API.LogError("Invalid webhook secret received in lifecycle event")
 			continue
 		}
-		a.p.metricsService.ObserveLifecycleEventTotal(event.LifecycleEvent)
+		if a.p.metricsService != nil {
+			a.p.metricsService.ObserveLifecycleEventTotal(event.LifecycleEvent)
+		}
 		a.p.activityHandler.HandleLifecycleEvent(event)
 	}
 

server/command.go

@@ -203,7 +203,9 @@ func (p *Plugin) executeLinkCommand(args *model.CommandArgs, parameters []string
 		return p.cmdError(args.UserId, args.ChannelId, "Unable to subscribe to the channel")
 	}
 
-	p.metricsService.ObserveSubscriptionsCount(metrics.SubscriptionConnected)
+	if p.metricsService != nil {
+		p.metricsService.ObserveSubscriptionsCount(metrics.SubscriptionConnected)
+	}
 	if err = p.store.StoreChannelLink(&channelLink); err != nil {
 		p.API.LogDebug("Unable to create the new link", "error", err.Error())
 		return p.cmdError(args.UserId, args.ChannelId, "Unable to create new link.")

server/handlers/attachments.go

@@ -11,7 +11,7 @@ import (
 	"strings"
 	"time"
 
-	"github.com/mattermost/mattermost-plugin-msteams-sync/server/metrics"
+	m "github.com/mattermost/mattermost-plugin-msteams-sync/server/metrics"
 	"github.com/mattermost/mattermost-plugin-msteams-sync/server/msteams"
 	"github.com/mattermost/mattermost-server/v6/app/imaging"
 	"github.com/mattermost/mattermost-server/v6/model"
@@ -121,6 +121,7 @@ func (ah *ActivityHandler) handleAttachments(channelID, userID, text string, msg
 		isDirectMessage = true
 	}
 
+	metrics := ah.plugin.GetMetrics()
 	for _, a := range msg.Attachments {
 		// remove the attachment tags from the text
 		newText = attachRE.ReplaceAllString(newText, "")
@@ -152,22 +153,28 @@ func (ah *ActivityHandler) handleAttachments(channelID, userID, text string, msg
 			attachmentData, err = ah.handleDownloadFile(a.ContentURL, client)
 			if err != nil {
 				ah.plugin.GetAPI().LogError("failed to download the file", "filename", a.Name, "error", err.Error())
-				ah.plugin.GetMetrics().ObserveFileCount(metrics.ActionCreated, metrics.ActionSourceMSTeams, discardedReasonUnableToGetTeamsData, isDirectMessage)
+				if metrics != nil {
+					metrics.ObserveFileCount(m.ActionCreated, m.ActionSourceMSTeams, discardedReasonUnableToGetTeamsData, isDirectMessage)
+				}
 				continue
 			}
 		} else {
 			fileSize, downloadURL, err = client.GetFileSizeAndDownloadURL(a.ContentURL)
 			if err != nil {
 				ah.plugin.GetAPI().LogError("failed to get file size and download URL", "error", err.Error())
-				ah.plugin.GetMetrics().ObserveFileCount(metrics.ActionCreated, metrics.ActionSourceMSTeams, discardedReasonUnableToGetTeamsData, isDirectMessage)
+				if metrics != nil {
+					metrics.ObserveFileCount(m.ActionCreated, m.ActionSourceMSTeams, discardedReasonUnableToGetTeamsData, isDirectMessage)
+				}
 				continue
 			}
 
 			fileSizeAllowed := *ah.plugin.GetAPI().GetConfig().FileSettings.MaxFileSize
 			if fileSize > fileSizeAllowed {
 				ah.plugin.GetAPI().LogError("skipping file download from MS Teams because the file size is greater than the allowed size")
 				errorFound = true
-				ah.plugin.GetMetrics().ObserveFileCount(metrics.ActionCreated, metrics.ActionSourceMSTeams, discardedReasonMaxFileSizeExceeded, isDirectMessage)
+				if metrics != nil {
+					metrics.ObserveFileCount(m.ActionCreated, m.ActionSourceMSTeams, discardedReasonMaxFileSizeExceeded, isDirectMessage)
+				}
 				continue
 			}
 
@@ -176,7 +183,9 @@ func (ah *ActivityHandler) handleAttachments(channelID, userID, text string, msg
 				attachmentData, err = client.GetFileContent(downloadURL)
 				if err != nil {
 					ah.plugin.GetAPI().LogError("failed to get file content", "error", err.Error())
-					ah.plugin.GetMetrics().ObserveFileCount(metrics.ActionCreated, metrics.ActionSourceMSTeams, discardedReasonUnableToGetTeamsData, isDirectMessage)
+					if metrics != nil {
+						metrics.ObserveFileCount(m.ActionCreated, m.ActionSourceMSTeams, discardedReasonUnableToGetTeamsData, isDirectMessage)
+					}
 					continue
 				}
 			}
@@ -197,15 +206,23 @@ func (ah *ActivityHandler) handleAttachments(channelID, userID, text string, msg
 		}
 
 		if fileInfoID == "" {
-			ah.plugin.GetMetrics().ObserveFileCount(metrics.ActionCreated, metrics.ActionSourceMSTeams, discardedReasonEmptyFileID, isDirectMessage)
+			if metrics != nil {
+				metrics.ObserveFileCount(m.ActionCreated, m.ActionSourceMSTeams, discardedReasonEmptyFileID, isDirectMessage)
+			}
 			continue
 		}
 		attachments = append(attachments, fileInfoID)
-		ah.plugin.GetMetrics().ObserveFileCount(metrics.ActionCreated, metrics.ActionSourceMSTeams, "", isDirectMessage)
+		if metrics != nil {
+			metrics.ObserveFileCount(m.ActionCreated, m.ActionSourceMSTeams, "", isDirectMessage)
+		}
 		countFileAttachments++
 		if countFileAttachments == maxFileAttachmentsSupported {
 			ah.plugin.GetAPI().LogDebug("discarding the rest of the attachments as Mattermost supports only 10 attachments per post")
-			ah.plugin.GetMetrics().ObserveFilesCount(metrics.ActionCreated, metrics.ActionSourceMSTeams, discardedReasonFileLimitReached, isDirectMessage, int64(len(msg.Attachments)-countNonFileAttachments-countFileAttachments))
+			if metrics != nil {
+				// Calculate the count of file attachments discarded by subtracting handled file attachments and other attachments from total message attachments.
+				fileAttachmentsDiscarded := len(msg.Attachments) - countNonFileAttachments - countFileAttachments
+				metrics.ObserveFilesCount(m.ActionCreated, m.ActionSourceMSTeams, discardedReasonFileLimitReached, isDirectMessage, int64(fileAttachmentsDiscarded))
+			}
 			break
 		}
 	}

server/handlers/attachments_test.go

@@ -355,7 +355,7 @@ func TestHandleAttachments(t *testing.T) {
 				p.On("GetClientForApp").Return(client).Once()
 				p.On("GetAPI").Return(mockAPI)
 				p.On("GetMaxSizeForCompleteDownload").Return(1).Times(10)
-				p.On("GetMetrics").Return(mockmetrics).Times(11)
+				p.On("GetMetrics").Return(mockmetrics).Times(1)
 			},
 			setupAPI: func(mockAPI *plugintest.API) {
 				mockAPI.On("GetConfig").Return(&model.Config{
@@ -384,6 +384,7 @@ func TestHandleAttachments(t *testing.T) {
 			description: "Attachment type code snippet",
 			setupPlugin: func(p *mocksPlugin.PluginIface, mockAPI *plugintest.API, client *mocksClient.Client, store *mocksStore.Store, mockmetrics *mocksMetrics.Metrics) {
 				p.On("GetClientForApp").Return(client)
+				p.On("GetMetrics").Return(mockmetrics).Times(1)
 			},
 			setupAPI: func(mockAPI *plugintest.API) {
 				mockAPI.On("GetConfig").Return(&model.Config{
@@ -409,6 +410,7 @@ func TestHandleAttachments(t *testing.T) {
 		{
 			description: "Attachment type message reference",
 			setupPlugin: func(p *mocksPlugin.PluginIface, mockAPI *plugintest.API, client *mocksClient.Client, store *mocksStore.Store, mockmetrics *mocksMetrics.Metrics) {
+				p.On("GetMetrics").Return(mockmetrics).Times(1)
 				p.On("GetClientForApp").Return(client)
 				p.On("GetStore").Return(store, nil)
 				p.On("GetAPI").Return(mockAPI)

server/handlers/converters_test.go

@@ -7,6 +7,7 @@ import (
 	"time"
 
 	mocksPlugin "github.com/mattermost/mattermost-plugin-msteams-sync/server/handlers/mocks"
+	mocksMetrics "github.com/mattermost/mattermost-plugin-msteams-sync/server/metrics/mocks"
 	"github.com/mattermost/mattermost-plugin-msteams-sync/server/msteams"
 	mocksClient "github.com/mattermost/mattermost-plugin-msteams-sync/server/msteams/mocks"
 	mocksStore "github.com/mattermost/mattermost-plugin-msteams-sync/server/store/mocks"
@@ -33,7 +34,7 @@ func TestMsgToPost(t *testing.T) {
 		senderID    string
 		message     *msteams.Message
 		post        *model.Post
-		setupPlugin func(plugin *mocksPlugin.PluginIface, mockAPI *plugintest.API, client *mocksClient.Client)
+		setupPlugin func(plugin *mocksPlugin.PluginIface, mockAPI *plugintest.API, client *mocksClient.Client, mockmetrics *mocksMetrics.Metrics)
 		setupAPI    func(*plugintest.API)
 	}{
 		{
@@ -47,10 +48,11 @@ func TestMsgToPost(t *testing.T) {
 				UserID:          testutils.GetUserID(),
 				CreateAt:        msteamsCreateAtTime,
 			},
-			setupPlugin: func(p *mocksPlugin.PluginIface, mockAPI *plugintest.API, client *mocksClient.Client) {
+			setupPlugin: func(p *mocksPlugin.PluginIface, mockAPI *plugintest.API, client *mocksClient.Client, mockmetrics *mocksMetrics.Metrics) {
 				p.On("GetBotUserID").Return(testutils.GetSenderID())
 				p.On("GetURL").Return("https://example.com/")
 				p.On("GetClientForApp").Return(client)
+				p.On("GetMetrics").Return(mockmetrics).Times(1)
 			},
 			setupAPI: func(api *plugintest.API) {
 				api.On("LogDebug", "Unable to get user avatar", "Error", mock.Anything).Once()
@@ -75,8 +77,9 @@ func TestMsgToPost(t *testing.T) {
 			ah := ActivityHandler{}
 			client := mocksClient.NewClient(t)
 			mockAPI := &plugintest.API{}
+			mockMetrics := mocksMetrics.NewMetrics(t)
 			testCase.setupAPI(mockAPI)
-			testCase.setupPlugin(p, mockAPI, client)
+			testCase.setupPlugin(p, mockAPI, client, mockMetrics)
 
 			ah.plugin = p
 

server/handlers/handlers.go

@@ -11,7 +11,7 @@ import (
 	"time"
 
 	"github.com/enescakir/emoji"
-	"github.com/mattermost/mattermost-plugin-msteams-sync/server/metrics"
+	m "github.com/mattermost/mattermost-plugin-msteams-sync/server/metrics"
 	"github.com/mattermost/mattermost-plugin-msteams-sync/server/msteams"
 	"github.com/mattermost/mattermost-plugin-msteams-sync/server/store"
 	"github.com/mattermost/mattermost-plugin-msteams-sync/server/store/storemodels"
@@ -51,7 +51,7 @@ const (
 type PluginIface interface {
 	GetAPI() plugin.API
 	GetStore() store.Store
-	GetMetrics() metrics.Metrics
+	GetMetrics() m.Metrics
 	GetSyncDirectMessages() bool
 	GetSyncGuestUsers() bool
 	GetMaxSizeForCompleteDownload() int
@@ -93,15 +93,20 @@ func New(plugin PluginIface) *ActivityHandler {
 }
 
 func (ah *ActivityHandler) Start() {
-	// This is constant for now, but report it as a metric to future proof dashboards.
-	ah.plugin.GetMetrics().ObserveChangeEventQueueCapacity(activityQueueSize)
+	metrics := ah.plugin.GetMetrics()
+	if metrics != nil {
+		// This is constant for now, but report it as a metric to future proof dashboards.
+		metrics.ObserveChangeEventQueueCapacity(activityQueueSize)
+	}
 
 	for i := 0; i < numberOfWorkers; i++ {
 		go func() {
 			for {
 				select {
 				case activity := <-ah.queue:
-					ah.plugin.GetMetrics().DecrementChangeEventQueueLength(activity.ChangeType)
+					if metrics != nil {
+						metrics.DecrementChangeEventQueueLength(activity.ChangeType)
+					}
 					ah.handleActivity(activity)
 				case <-ah.quit:
 					// we have received a signal to stop
@@ -122,7 +127,10 @@ func (ah *ActivityHandler) Stop() {
 
 func (ah *ActivityHandler) Handle(activity msteams.Activity) error {
 	ah.queue <- activity
-	ah.plugin.GetMetrics().IncrementChangeEventQueueLength(activity.ChangeType)
+	metrics := ah.plugin.GetMetrics()
+	if metrics != nil {
+		metrics.IncrementChangeEventQueueLength(activity.ChangeType)
+	}
 
 	return nil
 }
@@ -137,7 +145,10 @@ func (ah *ActivityHandler) HandleLifecycleEvent(event msteams.Activity) {
 		if err != nil {
 			ah.plugin.GetAPI().LogError("Unable to refresh the subscription", "error", err.Error())
 		} else {
-			ah.plugin.GetMetrics().ObserveSubscriptionsCount(metrics.SubscriptionRefreshed)
+			metrics := ah.plugin.GetMetrics()
+			if metrics != nil {
+				metrics.ObserveSubscriptionsCount(m.SubscriptionRefreshed)
+			}
 			if err = ah.plugin.GetStore().UpdateSubscriptionExpiresOn(event.SubscriptionID, *expiresOn); err != nil {
 				ah.plugin.GetAPI().LogError("Unable to store the subscription new expiry date", "subscriptionID", event.SubscriptionID, "error", err.Error())
 			}
@@ -169,7 +180,9 @@ func (ah *ActivityHandler) handleActivity(activity msteams.Activity) {
 
 	if activityIds.ChatID == "" {
 		if !ah.checkSubscription(activity.SubscriptionID) {
-			metrics.ObserveProcessedChangeEventTotal(activity.ChangeType, discardedReasonExpiredSubscription)
+			if metrics != nil {
+				metrics.ObserveProcessedChangeEventTotal(activity.ChangeType, discardedReasonExpiredSubscription)
+			}
 			return
 		}
 	}
@@ -187,7 +200,9 @@ func (ah *ActivityHandler) handleActivity(activity msteams.Activity) {
 		ah.plugin.GetAPI().LogError("Unsupported change type", "change_type", activity.ChangeType)
 	}
 
-	metrics.ObserveProcessedChangeEventTotal(activity.ChangeType, discardedReason)
+	if metrics != nil {
+		metrics.ObserveProcessedChangeEventTotal(activity.ChangeType, discardedReason)
+	}
 }
 
 func (ah *ActivityHandler) handleCreatedActivity(activityIds msteams.ActivityIds) string {
@@ -207,12 +222,17 @@ func (ah *ActivityHandler) handleCreatedActivity(activityIds msteams.ActivityIds
 		return discardedReasonNotUserEvent
 	}
 
+	isDirectMessage := IsDirectMessage(activityIds.ChatID)
+
+	metrics := ah.plugin.GetMetrics()
 	// Avoid possible duplication
 	postInfo, _ := ah.plugin.GetStore().GetPostInfoByMSTeamsID(msg.ChatID+msg.ChannelID, msg.ID)
 	if postInfo != nil {
 		ah.plugin.GetAPI().LogDebug("duplicate post")
 		ah.updateLastReceivedChangeDate(msg.LastUpdateAt)
-		ah.plugin.GetMetrics().ObserveMessagesConfirmedCount(metrics.ActionSourceMattermost, IsDirectMessage(activityIds.ChatID))
+		if metrics != nil {
+			metrics.ObserveMessagesConfirmedCount(m.ActionSourceMattermost, isDirectMessage)
+		}
 		return discardedReasonDuplicatedPost
 	}
 
@@ -284,8 +304,11 @@ func (ah *ActivityHandler) handleCreatedActivity(activityIds msteams.ActivityIds
 		return discardedReasonOther
 	}
 
+	if metrics != nil {
+		metrics.ObserveMessagesCount(m.ActionCreated, m.ActionSourceMSTeams, isDirectMessage)
+	}
+
 	ah.plugin.GetAPI().LogDebug("Post created")
-	ah.plugin.GetMetrics().ObserveMessagesCount(metrics.ActionCreated, metrics.ActionSourceMSTeams, IsDirectMessage(activityIds.ChatID))
 	if errorFound {
 		_ = ah.plugin.GetAPI().SendEphemeralPost(senderID, &model.Post{
 			ChannelId: channelID,
@@ -400,7 +423,10 @@ func (ah *ActivityHandler) handleUpdatedActivity(activityIds msteams.ActivityIds
 	}
 
 	isDirectMessage := IsDirectMessage(activityIds.ChatID)
-	ah.plugin.GetMetrics().ObserveMessagesCount(metrics.ActionUpdated, metrics.ActionSourceMSTeams, isDirectMessage)
+	metrics := ah.plugin.GetMetrics()
+	if metrics != nil {
+		metrics.ObserveMessagesCount(m.ActionUpdated, m.ActionSourceMSTeams, isDirectMessage)
+	}
 	ah.updateLastReceivedChangeDate(msg.LastUpdateAt)
 	ah.handleReactions(postInfo.MattermostID, channelID, isDirectMessage, msg.Reactions)
 	return discardedReasonNone
@@ -438,13 +464,16 @@ func (ah *ActivityHandler) handleReactions(postID, channelID string, isDirectMes
 		allReactions[reactionUserID+emojiName] = true
 	}
 
+	metrics := ah.plugin.GetMetrics()
 	for _, r := range postReactions {
 		if !allReactions[r.UserId+r.EmojiName] {
 			r.ChannelId = "removedfromplugin"
 			if appErr = ah.plugin.GetAPI().RemoveReaction(r); appErr != nil {
 				ah.plugin.GetAPI().LogError("Unable to remove reaction", "error", appErr.Error())
 			}
-			ah.plugin.GetMetrics().ObserveReactionsCount(metrics.ReactionUnsetAction, metrics.ActionSourceMSTeams, isDirectMessage)
+			if metrics != nil {
+				metrics.ObserveReactionsCount(m.ReactionUnsetAction, m.ActionSourceMSTeams, isDirectMessage)
+			}
 		}
 	}
 
@@ -474,7 +503,9 @@ func (ah *ActivityHandler) handleReactions(postID, channelID string, isDirectMes
 				continue
 			}
 			ah.plugin.GetAPI().LogDebug("Added reaction", "reaction", r)
-			ah.plugin.GetMetrics().ObserveReactionsCount(metrics.ReactionSetAction, metrics.ActionSourceMSTeams, isDirectMessage)
+			if metrics != nil {
+				metrics.ObserveReactionsCount(m.ReactionSetAction, m.ActionSourceMSTeams, isDirectMessage)
+			}
 		}
 	}
 }
@@ -495,7 +526,10 @@ func (ah *ActivityHandler) handleDeletedActivity(activityIds msteams.ActivityIds
 		return discardedReasonOther
 	}
 
-	ah.plugin.GetMetrics().ObserveMessagesCount(metrics.ActionDeleted, metrics.ActionSourceMSTeams, IsDirectMessage(activityIds.ChatID))
+	metrics := ah.plugin.GetMetrics()
+	if metrics != nil {
+		metrics.ObserveMessagesCount(m.ActionDeleted, m.ActionSourceMSTeams, IsDirectMessage(activityIds.ChatID))
+	}
 	return discardedReasonNone
 }