MM-56924: Connection invite metrics #675
Conversation
server/metrics/metrics.go
Outdated
| m.connectedUsersLimit = prometheus.NewGauge(prometheus.GaugeOpts{ | ||
| Namespace: MetricsNamespace, | ||
| Subsystem: MetricsSubsystemApp, | ||
| Name: "whitelist_limit", // really: connected_users_limit |
There was a problem hiding this comment.
metric name unchanged; should we rename to avoid confusion with new whitelist terminology?
There was a problem hiding this comment.
I'd be in favor of changing the name; we are not in production so nobody really depends on it, yet.
There was a problem hiding this comment.
Feel free to rename! Easy to update dashboards :)
webapp/src/index.tsx
Outdated
| msteams_invited_users: { | ||
| name: 'MS Teams: Invited Users', | ||
| id: 'msteams_invited_users', | ||
| icon: 'fa-users', // font-awesome-4.7.0 handler | ||
| value: siteStats?.pending_invited_users || 0, | ||
| }, | ||
| msteams_whitelisted_users: { | ||
| name: 'MS Teams: Whitelisted Users', | ||
| id: 'msteams_whitelisted_users', | ||
| icon: 'fa-users', // font-awesome-4.7.0 handler | ||
| value: siteStats?.current_whitelist_users || 0, | ||
| }, |
There was a problem hiding this comment.
I'm wondering if we should consider keeping this as a metric / Grafana dashboard only for now, vs. exposing to the enduser? It's the kind of data that's transiently useful (vs. connected count, which shows volume / adoption).
There was a problem hiding this comment.
I'm perfectly happy to remove them, but I do think they provide some value in monitoring or any potential troubleshooting during the initial rollout. 1/5
There was a problem hiding this comment.
We could leave them as "extra" stats in the api request, but then just not show them in the site statistics page.
server/metrics/metrics.go
Outdated
| m.connectedUsersLimit = prometheus.NewGauge(prometheus.GaugeOpts{ | ||
| Namespace: MetricsNamespace, | ||
| Subsystem: MetricsSubsystemApp, | ||
| Name: "whitelist_limit", // really: connected_users_limit |
There was a problem hiding this comment.
Feel free to rename! Easy to update dashboards :)
server/metrics/metrics.go
Outdated
| @@ -354,6 +353,7 @@ func NewMetrics(info InstanceInfo) Metrics { | |||
| }, []string{"action"}) | |||
| m.registry.MustRegister(m.syncMsgFileDelayTime) | |||
|
|
|||
| // connected, invited, whitelisted | |||
There was a problem hiding this comment.
Does this mean connected + invited + whitelisted? I'd like to make sure we can still measure actual connections separately.
There was a problem hiding this comment.
No, that's just a section header (ending on :403). Each metric is still separate.
calebroseland
added
4: Reviews Complete
Summary
Ticket Link
https://mattermost.atlassian.net/browse/MM-56924