From 1ec2de4a952c3855ea168f40a6233283b9f27a66 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Espino=20Garc=C3=ADa?= Date: Wed, 12 Jun 2024 11:33:04 +0200 Subject: [PATCH] Add mobile metrics (#27045) * Add mobile metrics * Fix mocks * Add tests * Fix lint * Address feedback * Fix lint * Fix test * Fix CI --------- Co-authored-by: Mattermost Build --- server/channels/app/helper_test.go | 12 +++ server/channels/app/metrics.go | 6 ++ server/channels/app/metrics_test.go | 102 +++++++++++++++++++ server/einterfaces/metrics.go | 3 + server/einterfaces/mocks/MetricsInterface.go | 15 +++ server/enterprise/metrics/metrics.go | 53 ++++++++++ server/enterprise/metrics/metrics_test.go | 55 ++++++++++ server/public/model/metrics.go | 4 + 8 files changed, 250 insertions(+) create mode 100644 server/channels/app/metrics_test.go diff --git a/server/channels/app/helper_test.go b/server/channels/app/helper_test.go index c494501f33..6d5dd00b98 100644 --- a/server/channels/app/helper_test.go +++ b/server/channels/app/helper_test.go @@ -166,6 +166,18 @@ func Setup(tb testing.TB, options ...Option) *TestHelper { return setupTestHelper(dbStore, false, true, nil, options, tb) } +func SetupEnterprise(tb testing.TB, options ...Option) *TestHelper { + if testing.Short() { + tb.SkipNow() + } + dbStore := mainHelper.GetStore() + dbStore.DropAllTables() + dbStore.MarkSystemRanUnitTests() + mainHelper.PreloadMigrations() + + return setupTestHelper(dbStore, true, true, nil, options, tb) +} + func SetupConfig(tb testing.TB, updateConfig func(cfg *model.Config)) *TestHelper { if testing.Short() { tb.SkipNow() diff --git a/server/channels/app/metrics.go b/server/channels/app/metrics.go index 172a4ba600..95a3cc5737 100644 --- a/server/channels/app/metrics.go +++ b/server/channels/app/metrics.go @@ -46,6 +46,12 @@ func (a *App) RegisterPerformanceReport(rctx request.CTX, report *model.Performa a.Metrics().ObserveClientRHSLoadDuration(commonLabels["platform"], commonLabels["agent"], h.Value/1000) case model.ClientGlobalThreadsLoadDuration: a.Metrics().ObserveGlobalThreadsLoadDuration(commonLabels["platform"], commonLabels["agent"], h.Value/1000) + case model.MobileClientLoadDuration: + a.Metrics().ObserveMobileClientLoadDuration(commonLabels["platform"], h.Value/1000) + case model.MobileClientChannelSwitchDuration: + a.Metrics().ObserveMobileClientChannelSwitchDuration(commonLabels["platform"], h.Value/1000) + case model.MobileClientTeamSwitchDuration: + a.Metrics().ObserveMobileClientTeamSwitchDuration(commonLabels["platform"], h.Value/1000) default: // we intentionally skip unknown metrics } diff --git a/server/channels/app/metrics_test.go b/server/channels/app/metrics_test.go new file mode 100644 index 0000000000..00eaa804ee --- /dev/null +++ b/server/channels/app/metrics_test.go @@ -0,0 +1,102 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +package app + +import ( + "fmt" + "testing" + + "github.com/mattermost/mattermost/server/public/model" + "github.com/mattermost/mattermost/server/v8/enterprise/metrics" + "github.com/prometheus/client_golang/prometheus" + prometheusModels "github.com/prometheus/client_model/go" + "github.com/stretchr/testify/require" +) + +func configureMetrics(th *TestHelper) { + th.App.Srv().SetLicense(nil) // clear license + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.MetricsSettings.Enable = true + *cfg.MetricsSettings.ListenAddress = ":0" + }) + th.App.Srv().SetLicense(model.NewTestLicense("metrics")) +} +func TestMobileMetrics(t *testing.T) { + th := SetupEnterprise(t, StartMetrics) + defer th.TearDown() + + configureMetrics(th) + mi := th.App.Metrics() + + miImpl, ok := mi.(*metrics.MetricsInterfaceImpl) + require.True(t, ok, fmt.Sprintf("App.Metrics is not *MetricsInterfaceImpl, but %T", mi)) + m := &prometheusModels.Metric{} + + for _, platform := range []string{"ios", "android"} { + ttcc := []struct { + name string + histogramVec *prometheus.HistogramVec + elapsed float64 + metricName model.MetricType + }{ + { + name: "load duration", + histogramVec: miImpl.MobileClientLoadDuration, + elapsed: 5001, + metricName: model.MobileClientLoadDuration, + }, + { + name: "channel switch duration", + histogramVec: miImpl.MobileClientChannelSwitchDuration, + elapsed: 501, + metricName: model.MobileClientChannelSwitchDuration, + }, + { + name: "team switch duration", + histogramVec: miImpl.MobileClientTeamSwitchDuration, + elapsed: 301, + metricName: model.MobileClientTeamSwitchDuration, + }, + } + + histograms := []*model.MetricSample{} + var start float64 = 125 + end := start + float64(len(ttcc)) + // Precheck + for i, tc := range ttcc { + actualMetric, err := tc.histogramVec.GetMetricWith(prometheus.Labels{"platform": platform}) + require.NoError(t, err) + require.NoError(t, actualMetric.(prometheus.Histogram).Write(m)) + require.Equal(t, uint64(0), m.Histogram.GetSampleCount()) + require.Equal(t, 0.0, m.Histogram.GetSampleSum()) + histograms = append(histograms, &model.MetricSample{ + Metric: tc.metricName, + Value: tc.elapsed, + Timestamp: start + float64(i), + }) + } + + appErr := th.App.RegisterPerformanceReport(th.Context, &model.PerformanceReport{ + Version: "0.1.0", + ClientID: "", + Labels: map[string]string{ + "platform": platform, + }, + Start: start, + End: end, + Counters: []*model.MetricSample{}, + Histograms: histograms, + }) + require.Nil(t, appErr) + + // After check + for _, tc := range ttcc { + actualMetric, err := tc.histogramVec.GetMetricWith(prometheus.Labels{"platform": platform}) + require.NoError(t, err) + require.NoError(t, actualMetric.(prometheus.Histogram).Write(m)) + require.Equal(t, uint64(1), m.Histogram.GetSampleCount()) + require.InDelta(t, tc.elapsed/1000, m.Histogram.GetSampleSum(), 0.001, "not equal value in %s", tc.name) + } + } +} diff --git a/server/einterfaces/metrics.go b/server/einterfaces/metrics.go index 87eea5efc9..ba2cdd859e 100644 --- a/server/einterfaces/metrics.go +++ b/server/einterfaces/metrics.go @@ -114,4 +114,7 @@ type MetricsInterface interface { ObserveClientTeamSwitchDuration(platform, agent string, elapsed float64) ObserveClientRHSLoadDuration(platform, agent string, elapsed float64) ObserveGlobalThreadsLoadDuration(platform, agent string, elapsed float64) + ObserveMobileClientLoadDuration(platform string, elapsed float64) + ObserveMobileClientChannelSwitchDuration(platform string, elapsed float64) + ObserveMobileClientTeamSwitchDuration(platform string, elapsed float64) } diff --git a/server/einterfaces/mocks/MetricsInterface.go b/server/einterfaces/mocks/MetricsInterface.go index 33414e1209..bcbac52bee 100644 --- a/server/einterfaces/mocks/MetricsInterface.go +++ b/server/einterfaces/mocks/MetricsInterface.go @@ -363,6 +363,21 @@ func (_m *MetricsInterface) ObserveGlobalThreadsLoadDuration(platform string, ag _m.Called(platform, agent, elapsed) } +// ObserveMobileClientChannelSwitchDuration provides a mock function with given fields: platform, elapsed +func (_m *MetricsInterface) ObserveMobileClientChannelSwitchDuration(platform string, elapsed float64) { + _m.Called(platform, elapsed) +} + +// ObserveMobileClientLoadDuration provides a mock function with given fields: platform, elapsed +func (_m *MetricsInterface) ObserveMobileClientLoadDuration(platform string, elapsed float64) { + _m.Called(platform, elapsed) +} + +// ObserveMobileClientTeamSwitchDuration provides a mock function with given fields: platform, elapsed +func (_m *MetricsInterface) ObserveMobileClientTeamSwitchDuration(platform string, elapsed float64) { + _m.Called(platform, elapsed) +} + // ObservePluginAPIDuration provides a mock function with given fields: pluginID, apiName, success, elapsed func (_m *MetricsInterface) ObservePluginAPIDuration(pluginID string, apiName string, success bool, elapsed float64) { _m.Called(pluginID, apiName, success, elapsed) diff --git a/server/enterprise/metrics/metrics.go b/server/enterprise/metrics/metrics.go index 11b7edf057..4037d34833 100644 --- a/server/enterprise/metrics/metrics.go +++ b/server/enterprise/metrics/metrics.go @@ -42,6 +42,7 @@ const ( MetricsSubsystemSystem = "system" MetricsSubsystemJobs = "jobs" MetricsSubsystemNotifications = "notifications" + MetricsSubsystemClientsMobileApp = "mobileapp" MetricsSubsystemClientsWeb = "webapp" MetricsCloudInstallationLabel = "installationId" MetricsCloudDatabaseClusterLabel = "databaseClusterName" @@ -222,6 +223,10 @@ type MetricsInterfaceImpl struct { ClientTeamSwitchDuration *prometheus.HistogramVec ClientRHSLoadDuration *prometheus.HistogramVec ClientGlobalThreadsLoadDuration *prometheus.HistogramVec + + MobileClientLoadDuration *prometheus.HistogramVec + MobileClientChannelSwitchDuration *prometheus.HistogramVec + MobileClientTeamSwitchDuration *prometheus.HistogramVec } func init() { @@ -1262,6 +1267,42 @@ func New(ps *platform.PlatformService, driver, dataSource string) *MetricsInterf ) m.Registry.MustRegister(m.ClientGlobalThreadsLoadDuration) + m.MobileClientLoadDuration = prometheus.NewHistogramVec( + prometheus.HistogramOpts{ + Namespace: MetricsNamespace, + Subsystem: MetricsSubsystemClientsMobileApp, + Name: "mobile_load", + Help: "Duration of the time taken from when a user opens the app and the app finally loads all relevant information (seconds)", + Buckets: []float64{1, 1.5, 2, 3, 4, 4.5, 5, 5.5, 6, 7.5, 10, 20, 25, 30}, + }, + []string{"platform"}, + ) + m.Registry.MustRegister(m.MobileClientLoadDuration) + + m.MobileClientChannelSwitchDuration = prometheus.NewHistogramVec( + prometheus.HistogramOpts{ + Namespace: MetricsNamespace, + Subsystem: MetricsSubsystemClientsMobileApp, + Name: "mobile_channel_switch", + Help: "Duration of the time taken from when a user clicks on a channel name, and the full channel sreen is loaded (seconds)", + Buckets: []float64{0.150, 0.200, 0.300, 0.400, 0.450, 0.500, 0.550, 0.600, 0.750, 1, 2, 3}, + }, + []string{"platform"}, + ) + m.Registry.MustRegister(m.MobileClientChannelSwitchDuration) + + m.MobileClientTeamSwitchDuration = prometheus.NewHistogramVec( + prometheus.HistogramOpts{ + Namespace: MetricsNamespace, + Subsystem: MetricsSubsystemClientsMobileApp, + Name: "mobile_team_switch", + Help: "Duration of the time taken from when a user clicks on a team, and the full categories screen is loaded (seconds)", + Buckets: []float64{0.150, 0.200, 0.250, 0.300, 0.350, 0.400, 0.500, 0.750, 1, 2, 3}, + }, + []string{"platform"}, + ) + m.Registry.MustRegister(m.MobileClientTeamSwitchDuration) + return m } @@ -1764,6 +1805,18 @@ func (mi *MetricsInterfaceImpl) ObserveGlobalThreadsLoadDuration(platform, agent mi.ClientGlobalThreadsLoadDuration.With(prometheus.Labels{"platform": platform, "agent": agent}).Observe(elapsed) } +func (mi *MetricsInterfaceImpl) ObserveMobileClientLoadDuration(platform string, elapsed float64) { + mi.MobileClientLoadDuration.With(prometheus.Labels{"platform": platform}).Observe(elapsed) +} + +func (mi *MetricsInterfaceImpl) ObserveMobileClientChannelSwitchDuration(platform string, elapsed float64) { + mi.MobileClientChannelSwitchDuration.With(prometheus.Labels{"platform": platform}).Observe(elapsed) +} + +func (mi *MetricsInterfaceImpl) ObserveMobileClientTeamSwitchDuration(platform string, elapsed float64) { + mi.MobileClientTeamSwitchDuration.With(prometheus.Labels{"platform": platform}).Observe(elapsed) +} + func extractDBCluster(driver, connectionString string) (string, error) { host, err := extractHost(driver, connectionString) if err != nil { diff --git a/server/enterprise/metrics/metrics_test.go b/server/enterprise/metrics/metrics_test.go index 95173490dd..4e2f79d344 100644 --- a/server/enterprise/metrics/metrics_test.go +++ b/server/enterprise/metrics/metrics_test.go @@ -176,6 +176,61 @@ func TestPluginMetrics(t *testing.T) { }) } +func TestMobileMetrics(t *testing.T) { + th := api4.SetupEnterprise(t, app.StartMetrics) + defer th.TearDown() + + configureMetrics(th) + mi := th.App.Metrics() + + miImpl, ok := mi.(*MetricsInterfaceImpl) + require.True(t, ok, fmt.Sprintf("App.Metrics is not *MetricsInterfaceImpl, but %T", mi)) + + ttcc := []struct { + name string + histogramVec *prometheus.HistogramVec + observeFunc func(string, float64) + }{ + { + name: "load duration", + histogramVec: miImpl.MobileClientLoadDuration, + observeFunc: mi.ObserveMobileClientLoadDuration, + }, + { + name: "channel switch duration", + histogramVec: miImpl.MobileClientChannelSwitchDuration, + observeFunc: mi.ObserveMobileClientChannelSwitchDuration, + }, + { + name: "team switch duration", + histogramVec: miImpl.MobileClientTeamSwitchDuration, + observeFunc: mi.ObserveMobileClientTeamSwitchDuration, + }, + } + + for _, tc := range ttcc { + t.Run(tc.name, func(t *testing.T) { + m := &prometheusModels.Metric{} + elapsed := 999.1 + + for _, platform := range []string{"ios", "android"} { + actualMetric, err := tc.histogramVec.GetMetricWith(prometheus.Labels{"platform": platform}) + require.NoError(t, err) + require.NoError(t, actualMetric.(prometheus.Histogram).Write(m)) + require.Equal(t, uint64(0), m.Histogram.GetSampleCount()) + require.Equal(t, 0.0, m.Histogram.GetSampleSum()) + + tc.observeFunc(platform, elapsed) + actualMetric, err = tc.histogramVec.GetMetricWith(prometheus.Labels{"platform": platform}) + require.NoError(t, err) + require.NoError(t, actualMetric.(prometheus.Histogram).Write(m)) + require.Equal(t, uint64(1), m.Histogram.GetSampleCount()) + require.InDelta(t, elapsed, m.Histogram.GetSampleSum(), 0.001) + } + }) + } +} + func TestExtractDBCluster(t *testing.T) { testCases := []struct { description string diff --git a/server/public/model/metrics.go b/server/public/model/metrics.go index beca80810c..a7f566923d 100644 --- a/server/public/model/metrics.go +++ b/server/public/model/metrics.go @@ -26,6 +26,10 @@ const ( ClientRHSLoadDuration MetricType = "rhs_load" ClientGlobalThreadsLoadDuration MetricType = "global_threads_load" + MobileClientLoadDuration MetricType = "mobile_load" + MobileClientChannelSwitchDuration MetricType = "mobile_channel_switch" + MobileClientTeamSwitchDuration MetricType = "mobile_team_switch" + performanceReportTTLMilliseconds = 300 * 1000 // 300 seconds/5 minutes )