Skip to content

Commit

Permalink
[MI-3644] Add metrics of subscription count for different actions (#365)
Browse files Browse the repository at this point in the history
* [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-3624] Update variable names and add new

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

* [MI-3644] Revert extra changes

* [MI-3624] Update function, added util and minor review fixes

* [MI-3644] Update help text

* [MI-3624] Reuse constants from other packages

* [MI-3642] Add comments in metrics interface

* [MI-3624] Update function arguments type

* [MI-3642] Remove extra debug log and re-organize interface methods

* [MI-3624] Fix lint

* [MI-3642] Reverting removal of extra information from debug log

* [MI-3643] Move constants from handlers to metrics package and update metrics name"

* Update server/metrics/metrics.go

* [MI-3642] Removed the term "Total" from some of the metrics

---------

Co-authored-by: Jesse Hallam <jesse.hallam@gmail.com>
Co-authored-by: Manoj <manoj@brightscout.com>
  • Loading branch information
3 people authored Oct 25, 2023
1 parent 17373d2 commit 5482e5d
Show file tree
Hide file tree
Showing 10 changed files with 183 additions and 37 deletions.
14 changes: 12 additions & 2 deletions server/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -381,6 +381,7 @@ func TestProcessLifecycle(t *testing.T) {
SetupAPI func(*plugintest.API)
SetupClient func(client *clientmocks.Client, uclient *clientmocks.Client)
SetupStore func(*storemocks.Store)
SetupMetrics func(mockmetrics *metricsmocks.Metrics)
RequestBody string
ExpectedStatusCode int
ExpectedResult string
Expand All @@ -390,6 +391,7 @@ func TestProcessLifecycle(t *testing.T) {
SetupAPI: func(api *plugintest.API) {},
SetupClient: func(client *clientmocks.Client, uclient *clientmocks.Client) {},
SetupStore: func(store *storemocks.Store) {},
SetupMetrics: func(mockmetrics *metricsmocks.Metrics) {},
ValidationToken: "mockValidationToken",
RequestBody: `{
"Value": [{
Expand All @@ -406,6 +408,7 @@ func TestProcessLifecycle(t *testing.T) {
SetupAPI: func(api *plugintest.API) {},
SetupClient: func(client *clientmocks.Client, uclient *clientmocks.Client) {},
SetupStore: func(store *storemocks.Store) {},
SetupMetrics: func(mockmetrics *metricsmocks.Metrics) {},
RequestBody: `{`,
ExpectedStatusCode: http.StatusBadRequest,
ExpectedResult: "unable to get the lifecycle events from the message\n",
Expand All @@ -415,8 +418,10 @@ func TestProcessLifecycle(t *testing.T) {
SetupAPI: func(api *plugintest.API) {
api.On("LogError", "Invalid webhook secret received in lifecycle event").Times(1)
},
SetupClient: func(client *clientmocks.Client, uclient *clientmocks.Client) {},
SetupStore: func(store *storemocks.Store) {},
SetupClient: func(client *clientmocks.Client, uclient *clientmocks.Client) {},
SetupStore: func(store *storemocks.Store) {},
SetupMetrics: func(mockmetrics *metricsmocks.Metrics) {},

RequestBody: `{
"Value": [{
"Resource": "mockResource",
Expand All @@ -437,6 +442,7 @@ func TestProcessLifecycle(t *testing.T) {
}, nil).Once()
store.On("GetLinkByMSTeamsChannelID", testutils.GetTeamsTeamID(), testutils.GetMSTeamsChannelID()).Return(nil, nil).Once()
},
SetupMetrics: func(mockmetrics *metricsmocks.Metrics) {},
RequestBody: `{
"Value": [{
"SubscriptionID": "mockID",
Expand All @@ -461,6 +467,9 @@ func TestProcessLifecycle(t *testing.T) {
store.On("GetLinkByMSTeamsChannelID", testutils.GetTeamsTeamID(), testutils.GetMSTeamsChannelID()).Return(nil, nil).Once()
store.On("UpdateSubscriptionExpiresOn", "mockID", newTime).Return(nil)
},
SetupMetrics: func(mockmetrics *metricsmocks.Metrics) {
mockmetrics.On("ObserveSubscriptionsCount", metrics.SubscriptionRefreshed).Times(1)
},
RequestBody: `{
"Value": [{
"SubscriptionID": "mockID",
Expand All @@ -485,6 +494,7 @@ func TestProcessLifecycle(t *testing.T) {
test.SetupStore(plugin.store.(*storemocks.Store))
test.SetupAPI(plugin.API.(*plugintest.API))
test.SetupClient(plugin.msteamsAppClient.(*clientmocks.Client), plugin.clientBuilderWithToken("", "", "", "", nil, nil).(*clientmocks.Client))
test.SetupMetrics(plugin.metricsService.(*metricsmocks.Metrics))
w := httptest.NewRecorder()
r := httptest.NewRequest(http.MethodPost, "/lifecycle", bytes.NewBufferString(test.RequestBody))
if test.ValidationToken != "" {
Expand Down
2 changes: 2 additions & 0 deletions server/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"strings"

"github.com/mattermost/mattermost-plugin-api/experimental/command"
"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/storemodels"
"github.com/mattermost/mattermost-server/v6/model"
Expand Down Expand Up @@ -202,6 +203,7 @@ 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 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.")
Expand Down
44 changes: 32 additions & 12 deletions server/command_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import (
"testing"
"time"

"github.com/mattermost/mattermost-plugin-msteams-sync/server/metrics"
mockMetrics "github.com/mattermost/mattermost-plugin-msteams-sync/server/metrics/mocks"
"github.com/mattermost/mattermost-plugin-msteams-sync/server/msteams"
mockClient "github.com/mattermost/mattermost-plugin-msteams-sync/server/msteams/mocks"
mockStore "github.com/mattermost/mattermost-plugin-msteams-sync/server/store/mocks"
Expand Down Expand Up @@ -544,12 +546,13 @@ func TestExecuteLinkCommand(t *testing.T) {
mockAPI := &plugintest.API{}

for _, testCase := range []struct {
description string
parameters []string
args *model.CommandArgs
setupAPI func(*plugintest.API)
setupStore func(*mockStore.Store)
setupClient func(*mockClient.Client, *mockClient.Client)
description string
parameters []string
args *model.CommandArgs
setupAPI func(*plugintest.API)
setupStore func(*mockStore.Store)
setupClient func(*mockClient.Client, *mockClient.Client)
setupMetrics func(mockmetrics *mockMetrics.Metrics)
}{
{
description: "Successfully executed link command",
Expand Down Expand Up @@ -585,6 +588,9 @@ func TestExecuteLinkCommand(t *testing.T) {
setupClient: func(c *mockClient.Client, uc *mockClient.Client) {
uc.On("GetChannelInTeam", testutils.GetTeamsUserID(), testutils.GetChannelID()).Return(&msteams.Channel{}, nil)
},
setupMetrics: func(mockmetrics *mockMetrics.Metrics) {
mockmetrics.On("ObserveSubscriptionsCount", metrics.SubscriptionConnected).Times(1)
},
},
{
description: "Error in beginning the database transaction",
Expand Down Expand Up @@ -619,6 +625,9 @@ func TestExecuteLinkCommand(t *testing.T) {
setupClient: func(c *mockClient.Client, uc *mockClient.Client) {
uc.On("GetChannelInTeam", testutils.GetTeamsUserID(), testutils.GetChannelID()).Return(&msteams.Channel{}, nil)
},
setupMetrics: func(mockmetrics *mockMetrics.Metrics) {
mockmetrics.On("ObserveSubscriptionsCount", metrics.SubscriptionConnected).Times(1)
},
},
{
description: "Unable to commit the database transaction",
Expand Down Expand Up @@ -655,6 +664,9 @@ func TestExecuteLinkCommand(t *testing.T) {
setupClient: func(c *mockClient.Client, uc *mockClient.Client) {
uc.On("GetChannelInTeam", testutils.GetTeamsUserID(), testutils.GetChannelID()).Return(&msteams.Channel{}, nil)
},
setupMetrics: func(mockmetrics *mockMetrics.Metrics) {
mockmetrics.On("ObserveSubscriptionsCount", metrics.SubscriptionConnected).Times(1)
},
},
{
description: "Unable to link a MS Teams channel to multiple channels",
Expand All @@ -681,6 +693,7 @@ func TestExecuteLinkCommand(t *testing.T) {
setupClient: func(c *mockClient.Client, uc *mockClient.Client) {
uc.On("GetChannelInTeam", testutils.GetTeamsUserID(), testutils.GetChannelID()).Return(&msteams.Channel{}, nil)
},
setupMetrics: func(mockmetrics *mockMetrics.Metrics) {},
},
{
description: "Invalid link command",
Expand All @@ -696,8 +709,9 @@ func TestExecuteLinkCommand(t *testing.T) {
api.On("HasPermissionToChannel", testutils.GetUserID(), testutils.GetChannelID(), model.PermissionManageChannelRoles).Return(true).Times(1)
api.On("SendEphemeralPost", testutils.GetUserID(), testutils.GetEphemeralPost("bot-user-id", testutils.GetChannelID(), "Invalid link command, please pass the MS Teams team id and channel id as parameters.")).Return(testutils.GetPost(testutils.GetChannelID(), testutils.GetUserID(), time.Now().UnixMicro())).Times(1)
},
setupStore: func(s *mockStore.Store) {},
setupClient: func(c *mockClient.Client, uc *mockClient.Client) {},
setupStore: func(s *mockStore.Store) {},
setupClient: func(c *mockClient.Client, uc *mockClient.Client) {},
setupMetrics: func(mockmetrics *mockMetrics.Metrics) {},
},
{
description: "Team is not enabled for MS Teams sync",
Expand All @@ -713,7 +727,8 @@ func TestExecuteLinkCommand(t *testing.T) {
setupStore: func(s *mockStore.Store) {
s.On("CheckEnabledTeamByTeamID", "").Return(false).Times(1)
},
setupClient: func(c *mockClient.Client, uc *mockClient.Client) {},
setupClient: func(c *mockClient.Client, uc *mockClient.Client) {},
setupMetrics: func(mockmetrics *mockMetrics.Metrics) {},
},
{
description: "Unable to get the current channel information",
Expand All @@ -728,7 +743,8 @@ func TestExecuteLinkCommand(t *testing.T) {
setupStore: func(s *mockStore.Store) {
s.On("CheckEnabledTeamByTeamID", testutils.GetTeamsUserID()).Return(true).Times(1)
},
setupClient: func(c *mockClient.Client, uc *mockClient.Client) {},
setupClient: func(c *mockClient.Client, uc *mockClient.Client) {},
setupMetrics: func(mockmetrics *mockMetrics.Metrics) {},
},
{
description: "Unable to link the channel as only channel admin can link it",
Expand All @@ -747,7 +763,8 @@ func TestExecuteLinkCommand(t *testing.T) {
setupStore: func(s *mockStore.Store) {
s.On("CheckEnabledTeamByTeamID", testutils.GetTeamsUserID()).Return(true).Times(1)
},
setupClient: func(c *mockClient.Client, uc *mockClient.Client) {},
setupClient: func(c *mockClient.Client, uc *mockClient.Client) {},
setupMetrics: func(mockmetrics *mockMetrics.Metrics) {},
},
{
description: "Unable to link channel as channel is either a direct or group message",
Expand All @@ -765,7 +782,8 @@ func TestExecuteLinkCommand(t *testing.T) {
setupStore: func(s *mockStore.Store) {
s.On("CheckEnabledTeamByTeamID", testutils.GetTeamsUserID()).Return(true).Times(1)
},
setupClient: func(c *mockClient.Client, uc *mockClient.Client) {},
setupClient: func(c *mockClient.Client, uc *mockClient.Client) {},
setupMetrics: func(mockmetrics *mockMetrics.Metrics) {},
},
{
description: "Unable to find MS Teams channel as user don't have the permissions to access it",
Expand Down Expand Up @@ -796,6 +814,7 @@ func TestExecuteLinkCommand(t *testing.T) {
setupClient: func(c *mockClient.Client, uc *mockClient.Client) {
uc.On("GetChannelInTeam", testutils.GetTeamsUserID(), "").Return(nil, errors.New("Error while getting the channel"))
},
setupMetrics: func(mockmetrics *mockMetrics.Metrics) {},
},
} {
t.Run(testCase.description, func(t *testing.T) {
Expand All @@ -804,6 +823,7 @@ func TestExecuteLinkCommand(t *testing.T) {

testCase.setupStore(p.store.(*mockStore.Store))
testCase.setupClient(p.msteamsAppClient.(*mockClient.Client), p.clientBuilderWithToken("", "", "", "", nil, nil).(*mockClient.Client))
testCase.setupMetrics(p.metricsService.(*mockMetrics.Metrics))
_, _ = p.executeLinkCommand(testCase.args, testCase.parameters)
})
}
Expand Down
1 change: 1 addition & 0 deletions server/handlers/handlers.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ 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)
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())
}
Expand Down
34 changes: 27 additions & 7 deletions server/metrics/metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,16 @@ const (

MetricsCloudInstallationLabel = "installationId"

ActionSourceMSTeams = "msteams"
ActionSourceMattermost = "mattermost"
ActionCreated = "created"
ActionUpdated = "updated"
ActionDeleted = "deleted"
ReactionSetAction = "set"
ReactionUnsetAction = "unset"
ActionSourceMSTeams = "msteams"
ActionSourceMattermost = "mattermost"
ActionCreated = "created"
ActionUpdated = "updated"
ActionDeleted = "deleted"
ReactionSetAction = "set"
ReactionUnsetAction = "unset"
SubscriptionRefreshed = "refreshed"
SubscriptionConnected = "connected"
SubscriptionReconnected = "reconnected"
)

type Metrics interface {
Expand All @@ -42,6 +45,7 @@ type Metrics interface {
ObserveFilesCount(action, source, discardedReason string, isDirectMessage bool, count int64)
ObserveFileCount(action, source, discardedReason string, isDirectMessage bool)
ObserveMessagesConfirmedCount(source string, isDirectMessage bool)
ObserveSubscriptionsCount(action string)

ObserveConnectedUsers(count int64)
ObserveSyntheticUsers(count int64)
Expand Down Expand Up @@ -78,6 +82,7 @@ type metrics struct {
reactionsCount *prometheus.CounterVec
filesCount *prometheus.CounterVec
messagesConfirmedCount *prometheus.CounterVec
subscriptionsCount *prometheus.CounterVec

connectedUsers prometheus.Gauge
syntheticUsers prometheus.Gauge
Expand Down Expand Up @@ -209,6 +214,15 @@ func NewMetrics(info InstanceInfo) Metrics {
}, []string{"source", "is_direct"})
m.registry.MustRegister(m.messagesConfirmedCount)

m.subscriptionsCount = prometheus.NewCounterVec(prometheus.CounterOpts{
Namespace: MetricsNamespace,
Subsystem: MetricsSubsystemEvents,
Name: "subscriptions_count",
Help: "The total number of connected, reconnected and refreshed subscriptions.",
ConstLabels: additionalLabels,
}, []string{"action"})
m.registry.MustRegister(m.subscriptionsCount)

m.connectedUsers = prometheus.NewGauge(prometheus.GaugeOpts{
Namespace: MetricsNamespace,
Subsystem: MetricsSubsystemApp,
Expand Down Expand Up @@ -339,6 +353,12 @@ func (m *metrics) ObserveMessagesConfirmedCount(source string, isDirectMessage b
}
}

func (m *metrics) ObserveSubscriptionsCount(action string) {
if m != nil {
m.subscriptionsCount.With(prometheus.Labels{"action": action}).Inc()
}
}

func (m *metrics) ObserveSyntheticUsers(count int64) {
if m != nil {
m.syntheticUsers.Set(float64(count))
Expand Down
5 changes: 5 additions & 0 deletions server/metrics/mocks/Metrics.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 4 additions & 1 deletion server/monitor/monitor.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"time"

"github.com/mattermost/mattermost-plugin-api/cluster"
"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-server/v6/plugin"
Expand All @@ -16,17 +17,19 @@ type Monitor struct {
client msteams.Client
store store.Store
api plugin.API
metrics metrics.Metrics
job *cluster.Job
baseURL string
webhookSecret string
useEvaluationAPI bool
}

func New(client msteams.Client, store store.Store, api plugin.API, baseURL string, webhookSecret string, useEvaluationAPI bool) *Monitor {
func New(client msteams.Client, store store.Store, api plugin.API, metrics metrics.Metrics, baseURL string, webhookSecret string, useEvaluationAPI bool) *Monitor {
return &Monitor{
client: client,
store: store,
api: api,
metrics: metrics,
baseURL: baseURL,
webhookSecret: webhookSecret,
useEvaluationAPI: useEvaluationAPI,
Expand Down
6 changes: 6 additions & 0 deletions server/monitor/subscriptions.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"sync"
"time"

"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/storemodels"
)
Expand Down Expand Up @@ -134,6 +135,7 @@ func (m *Monitor) CreateAndSaveChatSubscription(mmSubscription *storemodels.Glob
return
}

m.metrics.ObserveSubscriptionsCount(metrics.SubscriptionConnected)
if mmSubscription != nil {
if err := m.store.DeleteSubscription(mmSubscription.SubscriptionID); err != nil {
m.api.LogError("Unable to delete the old all chats subscription", "error", err.Error())
Expand Down Expand Up @@ -172,6 +174,7 @@ func (m *Monitor) recreateChannelSubscription(subscriptionID, teamID, channelID,
return
}

m.metrics.ObserveSubscriptionsCount(metrics.SubscriptionReconnected)
if subscriptionID != "" {
if err = m.store.DeleteSubscription(subscriptionID); err != nil {
m.api.LogDebug("Unable to delete old channel subscription from DB", "subscriptionID", subscriptionID, "error", err.Error())
Expand Down Expand Up @@ -214,6 +217,7 @@ func (m *Monitor) recreateGlobalSubscription(subscriptionID, secret string) erro
return err
}

m.metrics.ObserveSubscriptionsCount(metrics.SubscriptionReconnected)
if err = m.store.DeleteSubscription(subscriptionID); err != nil {
m.api.LogDebug("Unable to delete old global subscription from DB", "subscriptionID", subscriptionID, "error", err.Error())
}
Expand All @@ -225,6 +229,8 @@ func (m *Monitor) refreshSubscription(subscriptionID string) error {
if err != nil {
return err
}

m.metrics.ObserveSubscriptionsCount(metrics.SubscriptionRefreshed)
return m.store.UpdateSubscriptionExpiresOn(subscriptionID, *newSubscriptionTime)
}

Expand Down
Loading

0 comments on commit 5482e5d

Please sign in to comment.