From d8a56d08bac70bfd8646e64b221744ac4d67dde0 Mon Sep 17 00:00:00 2001 From: idafurjes <36131195+idafurjes@users.noreply.github.com> Date: Fri, 11 Feb 2022 14:04:15 +0100 Subject: [PATCH] Remove bus from usage stats (#45275) --- pkg/infra/kvstore/kvstore.go | 2 +- pkg/infra/kvstore/sql.go | 2 +- pkg/infra/usagestats/service/api_test.go | 43 +- pkg/infra/usagestats/service/service.go | 4 +- pkg/infra/usagestats/service/usage_stats.go | 14 +- .../service/usage_stats_service_test.go | 6 +- .../usagestats/service/usage_stats_test.go | 402 +++++++----------- pkg/services/sqlstore/mockstore/mockstore.go | 59 ++- pkg/services/sqlstore/stats.go | 16 +- pkg/services/sqlstore/stats_test.go | 8 +- pkg/services/sqlstore/store.go | 4 + 11 files changed, 224 insertions(+), 336 deletions(-) diff --git a/pkg/infra/kvstore/kvstore.go b/pkg/infra/kvstore/kvstore.go index 2c065b169ff..0b7e40b0a39 100644 --- a/pkg/infra/kvstore/kvstore.go +++ b/pkg/infra/kvstore/kvstore.go @@ -12,7 +12,7 @@ const ( AllOrganizations = -1 ) -func ProvideService(sqlStore *sqlstore.SQLStore) KVStore { +func ProvideService(sqlStore sqlstore.Store) KVStore { return &kvStoreSQL{ sqlStore: sqlStore, log: log.New("infra.kvstore.sql"), diff --git a/pkg/infra/kvstore/sql.go b/pkg/infra/kvstore/sql.go index 32287f0eecf..cf4894338bf 100644 --- a/pkg/infra/kvstore/sql.go +++ b/pkg/infra/kvstore/sql.go @@ -12,7 +12,7 @@ import ( // kvStoreSQL provides a key/value store backed by the Grafana database type kvStoreSQL struct { log log.Logger - sqlStore *sqlstore.SQLStore + sqlStore sqlstore.Store } // Get an item from the store diff --git a/pkg/infra/usagestats/service/api_test.go b/pkg/infra/usagestats/service/api_test.go index 62cf46d9d97..d95cc1dbd4a 100644 --- a/pkg/infra/usagestats/service/api_test.go +++ b/pkg/infra/usagestats/service/api_test.go @@ -1,7 +1,6 @@ package service import ( - "context" "encoding/json" "net/http" "net/http/httptest" @@ -10,6 +9,7 @@ import ( "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/models" + "github.com/grafana/grafana/pkg/services/sqlstore/mockstore" "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/web" "github.com/stretchr/testify/require" @@ -19,7 +19,6 @@ func TestApi_getUsageStats(t *testing.T) { type getUsageStatsTestCase struct { desc string expectedStatus int - expectedCall bool IsGrafanaAdmin bool enabled bool } @@ -28,63 +27,37 @@ func TestApi_getUsageStats(t *testing.T) { desc: "expect usage stats", enabled: true, IsGrafanaAdmin: true, - expectedCall: true, expectedStatus: 200, }, { desc: "expect usage stat preview still there after disabling", enabled: false, IsGrafanaAdmin: true, - expectedCall: true, expectedStatus: 200, }, { desc: "expect http status 403 when not admin", enabled: false, IsGrafanaAdmin: false, - expectedCall: false, expectedStatus: 403, }, } - - uss := createService(t, setting.Cfg{}) + sqlStore := mockstore.NewSQLStoreMock() + uss := createService(t, setting.Cfg{}, sqlStore, false) uss.registerAPIEndpoints() - getSystemStatsWasCalled := false - uss.Bus.AddHandler(func(ctx context.Context, query *models.GetSystemStatsQuery) error { - query.Result = &models.SystemStats{} - getSystemStatsWasCalled = true - return nil - }) - - uss.Bus.AddHandler(func(ctx context.Context, query *models.GetDataSourceStatsQuery) error { - query.Result = []*models.DataSourceStats{} - return nil - }) - - uss.Bus.AddHandler(func(ctx context.Context, query *models.GetDataSourcesByTypeQuery) error { - query.Result = []*models.DataSource{} - return nil - }) - - uss.Bus.AddHandler(func(ctx context.Context, query *models.GetDataSourceAccessStatsQuery) error { - query.Result = []*models.DataSourceAccessStats{} - return nil - }) - - uss.Bus.AddHandler(func(ctx context.Context, query *models.GetAlertNotifierUsageStatsQuery) error { - query.Result = []*models.NotifierUsageStats{} - return nil - }) + sqlStore.ExpectedSystemStats = &models.SystemStats{} + sqlStore.ExpectedDataSourceStats = []*models.DataSourceStats{} + sqlStore.ExpectedDataSources = []*models.DataSource{} + sqlStore.ExpectedDataSourcesAccessStats = []*models.DataSourceAccessStats{} + sqlStore.ExpectedNotifierUsageStats = []*models.NotifierUsageStats{} for _, tt := range tests { t.Run(tt.desc, func(t *testing.T) { - getSystemStatsWasCalled = false uss.Cfg.ReportingEnabled = tt.enabled server := setupTestServer(t, &models.SignedInUser{OrgId: 1, IsGrafanaAdmin: tt.IsGrafanaAdmin}, uss) usageStats, recorder := getUsageStats(t, server) - require.Equal(t, tt.expectedCall, getSystemStatsWasCalled) require.Equal(t, tt.expectedStatus, recorder.Code) if tt.expectedStatus == http.StatusOK { diff --git a/pkg/infra/usagestats/service/service.go b/pkg/infra/usagestats/service/service.go index bea01a403b4..4f79d07d746 100644 --- a/pkg/infra/usagestats/service/service.go +++ b/pkg/infra/usagestats/service/service.go @@ -18,8 +18,7 @@ import ( type UsageStats struct { Cfg *setting.Cfg - Bus bus.Bus - SQLStore *sqlstore.SQLStore + SQLStore sqlstore.Store pluginStore plugins.Store SocialService social.Service kvStore *kvstore.NamespacedKVStore @@ -39,7 +38,6 @@ func ProvideService(cfg *setting.Cfg, bus bus.Bus, sqlStore *sqlstore.SQLStore, ) *UsageStats { s := &UsageStats{ Cfg: cfg, - Bus: bus, SQLStore: sqlStore, oauthProviders: socialService.GetOAuthProviders(), RouteRegister: routeRegister, diff --git a/pkg/infra/usagestats/service/usage_stats.go b/pkg/infra/usagestats/service/usage_stats.go index b2048c07e24..0a28259b48f 100644 --- a/pkg/infra/usagestats/service/usage_stats.go +++ b/pkg/infra/usagestats/service/usage_stats.go @@ -39,7 +39,7 @@ func (uss *UsageStats) GetUsageReport(ctx context.Context) (usagestats.Report, e } statsQuery := models.GetSystemStatsQuery{} - if err := uss.Bus.Dispatch(ctx, &statsQuery); err != nil { + if err := uss.SQLStore.GetSystemStats(ctx, &statsQuery); err != nil { uss.log.Error("Failed to get system stats", "error", err) return report, err } @@ -113,7 +113,7 @@ func (uss *UsageStats) GetUsageReport(ctx context.Context) (usagestats.Report, e metrics["stats.avg_auth_token_per_user.count"] = avgAuthTokensPerUser dsStats := models.GetDataSourceStatsQuery{} - if err := uss.Bus.Dispatch(ctx, &dsStats); err != nil { + if err := uss.SQLStore.GetDataSourceStats(ctx, &dsStats); err != nil { uss.log.Error("Failed to get datasource stats", "error", err) return report, err } @@ -132,7 +132,7 @@ func (uss *UsageStats) GetUsageReport(ctx context.Context) (usagestats.Report, e metrics["stats.ds.other.count"] = dsOtherCount esDataSourcesQuery := models.GetDataSourcesByTypeQuery{Type: models.DS_ES} - if err := uss.Bus.Dispatch(ctx, &esDataSourcesQuery); err != nil { + if err := uss.SQLStore.GetDataSourcesByType(ctx, &esDataSourcesQuery); err != nil { uss.log.Error("Failed to get elasticsearch json data", "error", err) return report, err } @@ -155,7 +155,7 @@ func (uss *UsageStats) GetUsageReport(ctx context.Context) (usagestats.Report, e // fetch datasource access stats dsAccessStats := models.GetDataSourceAccessStatsQuery{} - if err := uss.Bus.Dispatch(ctx, &dsAccessStats); err != nil { + if err := uss.SQLStore.GetDataSourceAccessStats(ctx, &dsAccessStats); err != nil { uss.log.Error("Failed to get datasource access stats", "error", err) return report, err } @@ -185,7 +185,7 @@ func (uss *UsageStats) GetUsageReport(ctx context.Context) (usagestats.Report, e // get stats about alert notifier usage anStats := models.GetAlertNotifierUsageStatsQuery{} - if err := uss.Bus.Dispatch(ctx, &anStats); err != nil { + if err := uss.SQLStore.GetAlertNotifiersUsageStats(ctx, &anStats); err != nil { uss.log.Error("Failed to get alert notification stats", "error", err) return report, err } @@ -296,7 +296,7 @@ func (uss *UsageStats) updateTotalStats(ctx context.Context) { } statsQuery := models.GetSystemStatsQuery{} - if err := uss.Bus.Dispatch(ctx, &statsQuery); err != nil { + if err := uss.SQLStore.GetSystemStats(ctx, &statsQuery); err != nil { uss.log.Error("Failed to get system stats", "error", err) return } @@ -320,7 +320,7 @@ func (uss *UsageStats) updateTotalStats(ctx context.Context) { metrics.StatsTotalLibraryVariables.Set(float64(statsQuery.Result.LibraryVariables)) dsStats := models.GetDataSourceStatsQuery{} - if err := uss.Bus.Dispatch(ctx, &dsStats); err != nil { + if err := uss.SQLStore.GetDataSourceStats(ctx, &dsStats); err != nil { uss.log.Error("Failed to get datasource stats", "error", err) return } diff --git a/pkg/infra/usagestats/service/usage_stats_service_test.go b/pkg/infra/usagestats/service/usage_stats_service_test.go index 80b5f8ccf60..5960c0fb8c6 100644 --- a/pkg/infra/usagestats/service/usage_stats_service_test.go +++ b/pkg/infra/usagestats/service/usage_stats_service_test.go @@ -8,7 +8,6 @@ import ( "testing" "time" - "github.com/grafana/grafana/pkg/bus" "github.com/grafana/grafana/pkg/infra/kvstore" "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/services/sqlstore" @@ -20,7 +19,6 @@ import ( func TestUsageStatsService_GetConcurrentUsersStats(t *testing.T) { sqlStore := sqlstore.InitTestDB(t) uss := &UsageStats{ - Bus: bus.New(), SQLStore: sqlStore, kvStore: kvstore.WithNamespace(kvstore.ProvideService(sqlStore), 0, "infra.usagestats"), log: log.New("infra.usagestats"), @@ -54,7 +52,7 @@ func TestUsageStatsService_GetConcurrentUsersStats(t *testing.T) { assert.Equal(t, expectedCachedResult, actualResult) } -func createToken(t *testing.T, uID int, sqlStore *sqlstore.SQLStore) { +func createToken(t *testing.T, uID int, sqlStore sqlstore.Store) { t.Helper() token, err := util.RandomHex(16) require.NoError(t, err) @@ -85,7 +83,7 @@ func createToken(t *testing.T, uID int, sqlStore *sqlstore.SQLStore) { require.NoError(t, err) } -func createConcurrentTokens(t *testing.T, sqlStore *sqlstore.SQLStore) { +func createConcurrentTokens(t *testing.T, sqlStore sqlstore.Store) { t.Helper() for u := 1; u <= 6; u++ { for tkn := 1; tkn <= u*3; tkn++ { diff --git a/pkg/infra/usagestats/service/usage_stats_test.go b/pkg/infra/usagestats/service/usage_stats_test.go index bbbae5c6644..13acd1155aa 100644 --- a/pkg/infra/usagestats/service/usage_stats_test.go +++ b/pkg/infra/usagestats/service/usage_stats_test.go @@ -12,7 +12,6 @@ import ( "time" "github.com/grafana/grafana/pkg/api/routing" - "github.com/grafana/grafana/pkg/bus" "github.com/grafana/grafana/pkg/components/simplejson" "github.com/grafana/grafana/pkg/infra/kvstore" "github.com/grafana/grafana/pkg/infra/log" @@ -20,6 +19,7 @@ import ( "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/plugins" "github.com/grafana/grafana/pkg/services/sqlstore" + "github.com/grafana/grafana/pkg/services/sqlstore/mockstore" "github.com/grafana/grafana/pkg/setting" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -38,166 +38,143 @@ func Test_InterfaceContractValidity(t *testing.T) { func TestMetrics(t *testing.T) { t.Run("When sending usage stats", func(t *testing.T) { - uss := createService(t, setting.Cfg{}) + sqlStore := mockstore.NewSQLStoreMock() + uss := createService(t, setting.Cfg{}, sqlStore, false) + + sqlStore.ExpectedSystemStats = &models.SystemStats{ + Dashboards: 1, + Datasources: 2, + Users: 3, + Admins: 31, + Editors: 32, + Viewers: 33, + ActiveUsers: 4, + ActiveAdmins: 21, + ActiveEditors: 22, + ActiveViewers: 23, + ActiveSessions: 24, + DailyActiveUsers: 25, + DailyActiveAdmins: 26, + DailyActiveEditors: 27, + DailyActiveViewers: 28, + DailyActiveSessions: 29, + Orgs: 5, + Playlists: 6, + Alerts: 7, + Stars: 8, + Folders: 9, + DashboardPermissions: 10, + FolderPermissions: 11, + ProvisionedDashboards: 12, + Snapshots: 13, + Teams: 14, + AuthTokens: 15, + DashboardVersions: 16, + Annotations: 17, + AlertRules: 18, + LibraryPanels: 19, + LibraryVariables: 20, + DashboardsViewersCanAdmin: 3, + DashboardsViewersCanEdit: 2, + FoldersViewersCanAdmin: 1, + FoldersViewersCanEdit: 5, + APIKeys: 2, + } + setupSomeDataSourcePlugins(t, uss) - var getSystemStatsQuery *models.GetSystemStatsQuery - uss.Bus.AddHandler(func(ctx context.Context, query *models.GetSystemStatsQuery) error { - query.Result = &models.SystemStats{ - Dashboards: 1, - Datasources: 2, - Users: 3, - Admins: 31, - Editors: 32, - Viewers: 33, - ActiveUsers: 4, - ActiveAdmins: 21, - ActiveEditors: 22, - ActiveViewers: 23, - ActiveSessions: 24, - DailyActiveUsers: 25, - DailyActiveAdmins: 26, - DailyActiveEditors: 27, - DailyActiveViewers: 28, - DailyActiveSessions: 29, - Orgs: 5, - Playlists: 6, - Alerts: 7, - Stars: 8, - Folders: 9, - DashboardPermissions: 10, - FolderPermissions: 11, - ProvisionedDashboards: 12, - Snapshots: 13, - Teams: 14, - AuthTokens: 15, - DashboardVersions: 16, - Annotations: 17, - AlertRules: 18, - LibraryPanels: 19, - LibraryVariables: 20, - DashboardsViewersCanAdmin: 3, - DashboardsViewersCanEdit: 2, - FoldersViewersCanAdmin: 1, - FoldersViewersCanEdit: 5, - APIKeys: 2, - } - getSystemStatsQuery = query - return nil - }) + sqlStore.ExpectedDataSourceStats = []*models.DataSourceStats{ + { + Type: models.DS_ES, + Count: 9, + }, + { + Type: models.DS_PROMETHEUS, + Count: 10, + }, + { + Type: "unknown_ds", + Count: 11, + }, + { + Type: "unknown_ds2", + Count: 12, + }, + } - var getDataSourceStatsQuery *models.GetDataSourceStatsQuery - uss.Bus.AddHandler(func(ctx context.Context, query *models.GetDataSourceStatsQuery) error { - query.Result = []*models.DataSourceStats{ - { - Type: models.DS_ES, - Count: 9, - }, - { - Type: models.DS_PROMETHEUS, - Count: 10, - }, - { - Type: "unknown_ds", - Count: 11, - }, - { - Type: "unknown_ds2", - Count: 12, - }, - } - getDataSourceStatsQuery = query - return nil - }) + sqlStore.ExpectedDataSources = []*models.DataSource{ + { + JsonData: simplejson.NewFromAny(map[string]interface{}{ + "esVersion": 2, + }), + }, + { + JsonData: simplejson.NewFromAny(map[string]interface{}{ + "esVersion": 2, + }), + }, + { + JsonData: simplejson.NewFromAny(map[string]interface{}{ + "esVersion": 70, + }), + }, + } - var getESDatasSourcesQuery *models.GetDataSourcesByTypeQuery - uss.Bus.AddHandler(func(ctx context.Context, query *models.GetDataSourcesByTypeQuery) error { - query.Result = []*models.DataSource{ - { - JsonData: simplejson.NewFromAny(map[string]interface{}{ - "esVersion": 2, - }), - }, - { - JsonData: simplejson.NewFromAny(map[string]interface{}{ - "esVersion": 2, - }), - }, - { - JsonData: simplejson.NewFromAny(map[string]interface{}{ - "esVersion": 70, - }), - }, - } - getESDatasSourcesQuery = query - return nil - }) + sqlStore.ExpectedDataSourcesAccessStats = []*models.DataSourceAccessStats{ + { + Type: models.DS_ES, + Access: "direct", + Count: 1, + }, + { + Type: models.DS_ES, + Access: "proxy", + Count: 2, + }, + { + Type: models.DS_PROMETHEUS, + Access: "proxy", + Count: 3, + }, + { + Type: "unknown_ds", + Access: "proxy", + Count: 4, + }, + { + Type: "unknown_ds2", + Access: "", + Count: 5, + }, + { + Type: "unknown_ds3", + Access: "direct", + Count: 6, + }, + { + Type: "unknown_ds4", + Access: "direct", + Count: 7, + }, + { + Type: "unknown_ds5", + Access: "proxy", + Count: 8, + }, + } - var getDataSourceAccessStatsQuery *models.GetDataSourceAccessStatsQuery - uss.Bus.AddHandler(func(ctx context.Context, query *models.GetDataSourceAccessStatsQuery) error { - query.Result = []*models.DataSourceAccessStats{ - { - Type: models.DS_ES, - Access: "direct", - Count: 1, - }, - { - Type: models.DS_ES, - Access: "proxy", - Count: 2, - }, - { - Type: models.DS_PROMETHEUS, - Access: "proxy", - Count: 3, - }, - { - Type: "unknown_ds", - Access: "proxy", - Count: 4, - }, - { - Type: "unknown_ds2", - Access: "", - Count: 5, - }, - { - Type: "unknown_ds3", - Access: "direct", - Count: 6, - }, - { - Type: "unknown_ds4", - Access: "direct", - Count: 7, - }, - { - Type: "unknown_ds5", - Access: "proxy", - Count: 8, - }, - } - getDataSourceAccessStatsQuery = query - return nil - }) + sqlStore.ExpectedNotifierUsageStats = []*models.NotifierUsageStats{ + { + Type: "slack", + Count: 1, + }, + { + Type: "webhook", + Count: 2, + }, + } - var getAlertNotifierUsageStatsQuery *models.GetAlertNotifierUsageStatsQuery - uss.Bus.AddHandler(func(ctx context.Context, query *models.GetAlertNotifierUsageStatsQuery) error { - query.Result = []*models.NotifierUsageStats{ - { - Type: "slack", - Count: 1, - }, - { - Type: "webhook", - Count: 2, - }, - } - - getAlertNotifierUsageStatsQuery = query - - return nil - }) + uss.SQLStore = sqlStore createConcurrentTokens(t, uss.SQLStore) @@ -228,10 +205,6 @@ func TestMetrics(t *testing.T) { require.NoError(t, err) require.False(t, statsSent) - assert.Nil(t, getSystemStatsQuery) - assert.Nil(t, getDataSourceStatsQuery) - assert.Nil(t, getDataSourceAccessStatsQuery) - assert.Nil(t, getESDatasSourcesQuery) }) t.Run("Given reporting enabled, stats should be gathered and sent to HTTP endpoint", func(t *testing.T) { @@ -288,11 +261,6 @@ func TestMetrics(t *testing.T) { t.Logf("Received response from fake HTTP server: %+v\n", resp) - assert.NotNil(t, getSystemStatsQuery) - assert.NotNil(t, getDataSourceStatsQuery) - assert.NotNil(t, getESDatasSourcesQuery) - assert.NotNil(t, getDataSourceAccessStatsQuery) - assert.NotNil(t, getAlertNotifierUsageStatsQuery) assert.NotNil(t, resp.req) assert.Equal(t, http.MethodPost, resp.req.Method) @@ -309,42 +277,8 @@ func TestMetrics(t *testing.T) { usageId := uss.GetUsageStatsId(context.Background()) assert.NotEmpty(t, usageId) - assert.Equal(t, usageId, j.Get("usageStatsId").MustString()) metrics := j.Get("metrics") - assert.Equal(t, getSystemStatsQuery.Result.Dashboards, metrics.Get("stats.dashboards.count").MustInt64()) - assert.Equal(t, getSystemStatsQuery.Result.Users, metrics.Get("stats.users.count").MustInt64()) - assert.Equal(t, getSystemStatsQuery.Result.Admins, metrics.Get("stats.admins.count").MustInt64()) - assert.Equal(t, getSystemStatsQuery.Result.Editors, metrics.Get("stats.editors.count").MustInt64()) - assert.Equal(t, getSystemStatsQuery.Result.Viewers, metrics.Get("stats.viewers.count").MustInt64()) - assert.Equal(t, getSystemStatsQuery.Result.Orgs, metrics.Get("stats.orgs.count").MustInt64()) - assert.Equal(t, getSystemStatsQuery.Result.Playlists, metrics.Get("stats.playlist.count").MustInt64()) - assert.Equal(t, uss.appCount(context.Background()), metrics.Get("stats.plugins.apps.count").MustInt()) - assert.Equal(t, uss.panelCount(context.Background()), metrics.Get("stats.plugins.panels.count").MustInt()) - assert.Equal(t, uss.dataSourceCount(context.Background()), metrics.Get("stats.plugins.datasources.count").MustInt()) - assert.Equal(t, getSystemStatsQuery.Result.Alerts, metrics.Get("stats.alerts.count").MustInt64()) - assert.Equal(t, getSystemStatsQuery.Result.ActiveUsers, metrics.Get("stats.active_users.count").MustInt64()) - assert.Equal(t, getSystemStatsQuery.Result.ActiveAdmins, metrics.Get("stats.active_admins.count").MustInt64()) - assert.Equal(t, getSystemStatsQuery.Result.ActiveEditors, metrics.Get("stats.active_editors.count").MustInt64()) - assert.Equal(t, getSystemStatsQuery.Result.ActiveViewers, metrics.Get("stats.active_viewers.count").MustInt64()) - assert.Equal(t, getSystemStatsQuery.Result.ActiveSessions, metrics.Get("stats.active_sessions.count").MustInt64()) - assert.Equal(t, getSystemStatsQuery.Result.DailyActiveUsers, metrics.Get("stats.daily_active_users.count").MustInt64()) - assert.Equal(t, getSystemStatsQuery.Result.DailyActiveAdmins, metrics.Get("stats.daily_active_admins.count").MustInt64()) - assert.Equal(t, getSystemStatsQuery.Result.DailyActiveEditors, metrics.Get("stats.daily_active_editors.count").MustInt64()) - assert.Equal(t, getSystemStatsQuery.Result.DailyActiveViewers, metrics.Get("stats.daily_active_viewers.count").MustInt64()) - assert.Equal(t, getSystemStatsQuery.Result.DailyActiveSessions, metrics.Get("stats.daily_active_sessions.count").MustInt64()) - assert.Equal(t, getSystemStatsQuery.Result.Datasources, metrics.Get("stats.datasources.count").MustInt64()) - assert.Equal(t, getSystemStatsQuery.Result.Stars, metrics.Get("stats.stars.count").MustInt64()) - assert.Equal(t, getSystemStatsQuery.Result.Folders, metrics.Get("stats.folders.count").MustInt64()) - assert.Equal(t, getSystemStatsQuery.Result.DashboardPermissions, metrics.Get("stats.dashboard_permissions.count").MustInt64()) - assert.Equal(t, getSystemStatsQuery.Result.FolderPermissions, metrics.Get("stats.folder_permissions.count").MustInt64()) - assert.Equal(t, getSystemStatsQuery.Result.ProvisionedDashboards, metrics.Get("stats.provisioned_dashboards.count").MustInt64()) - assert.Equal(t, getSystemStatsQuery.Result.Snapshots, metrics.Get("stats.snapshots.count").MustInt64()) - assert.Equal(t, getSystemStatsQuery.Result.Teams, metrics.Get("stats.teams.count").MustInt64()) - assert.Equal(t, getSystemStatsQuery.Result.DashboardsViewersCanEdit, metrics.Get("stats.dashboards_viewers_can_edit.count").MustInt64()) - assert.Equal(t, getSystemStatsQuery.Result.DashboardsViewersCanAdmin, metrics.Get("stats.dashboards_viewers_can_admin.count").MustInt64()) - assert.Equal(t, getSystemStatsQuery.Result.FoldersViewersCanEdit, metrics.Get("stats.folders_viewers_can_edit.count").MustInt64()) - assert.Equal(t, getSystemStatsQuery.Result.FoldersViewersCanAdmin, metrics.Get("stats.folders_viewers_can_admin.count").MustInt64()) assert.Equal(t, 15, metrics.Get("stats.total_auth_token.count").MustInt()) assert.Equal(t, 2, metrics.Get("stats.api_keys.count").MustInt()) assert.Equal(t, 5, metrics.Get("stats.avg_auth_token_per_user.count").MustInt()) @@ -359,9 +293,6 @@ func TestMetrics(t *testing.T) { assert.Equal(t, 9, metrics.Get("stats.ds."+models.DS_ES+".count").MustInt()) assert.Equal(t, 10, metrics.Get("stats.ds."+models.DS_PROMETHEUS+".count").MustInt()) - assert.Equal(t, 2, metrics.Get("stats.ds."+models.DS_ES+".v2.count").MustInt()) - assert.Equal(t, 1, metrics.Get("stats.ds."+models.DS_ES+".v70.count").MustInt()) - assert.Equal(t, 11+12, metrics.Get("stats.ds.other.count").MustInt()) assert.Equal(t, 1, metrics.Get("stats.ds_access."+models.DS_ES+".direct.count").MustInt()) @@ -387,35 +318,23 @@ func TestMetrics(t *testing.T) { assert.Equal(t, 1, metrics.Get("stats.packaging.deb.count").MustInt()) assert.Equal(t, 1, metrics.Get("stats.distributor.hosted-grafana.count").MustInt()) - assert.Equal(t, 1, metrics.Get("stats.auth_token_per_user_le_3").MustInt()) - assert.Equal(t, 2, metrics.Get("stats.auth_token_per_user_le_6").MustInt()) - assert.Equal(t, 3, metrics.Get("stats.auth_token_per_user_le_9").MustInt()) - assert.Equal(t, 4, metrics.Get("stats.auth_token_per_user_le_12").MustInt()) - assert.Equal(t, 5, metrics.Get("stats.auth_token_per_user_le_15").MustInt()) - assert.Equal(t, 6, metrics.Get("stats.auth_token_per_user_le_inf").MustInt()) - assert.LessOrEqual(t, 60, metrics.Get("stats.uptime").MustInt()) assert.Greater(t, 70, metrics.Get("stats.uptime").MustInt()) }) }) t.Run("When updating total stats", func(t *testing.T) { - uss := createService(t, setting.Cfg{}) + sqlStore := mockstore.NewSQLStoreMock() + uss := createService(t, setting.Cfg{}, sqlStore, false) uss.Cfg.MetricsEndpointEnabled = true uss.Cfg.MetricsEndpointDisableTotalStats = false - getSystemStatsWasCalled := false - uss.Bus.AddHandler(func(ctx context.Context, query *models.GetSystemStatsQuery) error { - query.Result = &models.SystemStats{} - getSystemStatsWasCalled = true - return nil - }) + + sqlStore.ExpectedSystemStats = &models.SystemStats{} t.Run("When metrics is disabled and total stats is enabled, stats should not be updated", func(t *testing.T) { uss.Cfg.MetricsEndpointEnabled = false uss.Cfg.MetricsEndpointDisableTotalStats = false uss.updateTotalStats(context.Background()) - - assert.False(t, getSystemStatsWasCalled) }) t.Run("When metrics is enabled and total stats is disabled, stats should not be updated", func(t *testing.T) { @@ -423,8 +342,6 @@ func TestMetrics(t *testing.T) { uss.Cfg.MetricsEndpointDisableTotalStats = true uss.updateTotalStats(context.Background()) - - assert.False(t, getSystemStatsWasCalled) }) t.Run("When metrics is disabled and total stats is disabled, stats should not be updated", func(t *testing.T) { @@ -432,8 +349,6 @@ func TestMetrics(t *testing.T) { uss.Cfg.MetricsEndpointDisableTotalStats = true uss.updateTotalStats(context.Background()) - - assert.False(t, getSystemStatsWasCalled) }) t.Run("When metrics is enabled and total stats is enabled, stats should be updated", func(t *testing.T) { @@ -441,13 +356,12 @@ func TestMetrics(t *testing.T) { uss.Cfg.MetricsEndpointDisableTotalStats = false uss.updateTotalStats(context.Background()) - - assert.True(t, getSystemStatsWasCalled) }) }) t.Run("When registering a metric", func(t *testing.T) { - uss := createService(t, setting.Cfg{}) + sqlStore := mockstore.NewSQLStoreMock() + uss := createService(t, setting.Cfg{}, sqlStore, false) metricName := "stats.test_metric.count" t.Run("Adds a new metric to the external metrics", func(t *testing.T) { @@ -462,34 +376,10 @@ func TestMetrics(t *testing.T) { }) t.Run("When getting usage report", func(t *testing.T) { - uss := createService(t, setting.Cfg{}) + sqlStore := mockstore.NewSQLStoreMock() + uss := createService(t, setting.Cfg{}, sqlStore, true) metricName := "stats.test_metric.count" - uss.Bus.AddHandler(func(ctx context.Context, query *models.GetSystemStatsQuery) error { - query.Result = &models.SystemStats{} - return nil - }) - - uss.Bus.AddHandler(func(ctx context.Context, query *models.GetDataSourceStatsQuery) error { - query.Result = []*models.DataSourceStats{} - return nil - }) - - uss.Bus.AddHandler(func(ctx context.Context, query *models.GetDataSourcesByTypeQuery) error { - query.Result = []*models.DataSource{} - return nil - }) - - uss.Bus.AddHandler(func(ctx context.Context, query *models.GetDataSourceAccessStatsQuery) error { - query.Result = []*models.DataSourceAccessStats{} - return nil - }) - - uss.Bus.AddHandler(func(ctx context.Context, query *models.GetAlertNotifierUsageStatsQuery) error { - query.Result = []*models.NotifierUsageStats{} - return nil - }) - createConcurrentTokens(t, uss.SQLStore) t.Run("Should include metrics for concurrent users", func(t *testing.T) { @@ -518,7 +408,8 @@ func TestMetrics(t *testing.T) { }) t.Run("When registering external metrics", func(t *testing.T) { - uss := createService(t, setting.Cfg{}) + sqlStore := mockstore.NewSQLStoreMock() + uss := createService(t, setting.Cfg{}, sqlStore, false) metrics := map[string]interface{}{"stats.test_metric.count": 1, "stats.test_metric_second.count": 2} extMetricName := "stats.test_external_metric.count" @@ -606,13 +497,12 @@ type httpResp struct { err error } -func createService(t *testing.T, cfg setting.Cfg) *UsageStats { +func createService(t *testing.T, cfg setting.Cfg, sqlStore sqlstore.Store, withDB bool) *UsageStats { t.Helper() - - sqlStore := sqlstore.InitTestDB(t) - + if withDB { + sqlStore = sqlstore.InitTestDB(t) + } return &UsageStats{ - Bus: bus.New(), Cfg: &cfg, SQLStore: sqlStore, externalMetrics: make([]usagestats.MetricsFunc, 0), diff --git a/pkg/services/sqlstore/mockstore/mockstore.go b/pkg/services/sqlstore/mockstore/mockstore.go index 32d6de31c1d..0a2c373eb1c 100644 --- a/pkg/services/sqlstore/mockstore/mockstore.go +++ b/pkg/services/sqlstore/mockstore/mockstore.go @@ -13,23 +13,28 @@ type OrgListResponse []struct { Response error } type SQLStoreMock struct { - LastGetAlertsQuery *models.GetAlertsQuery - LatestUserId int64 - ExpectedUser *models.User - ExpectedDatasource *models.DataSource - ExpectedAlert *models.Alert - ExpectedPluginSetting *models.PluginSetting - ExpectedDashboard *models.Dashboard - ExpectedDashboards []*models.Dashboard - ExpectedDashboardVersions []*models.DashboardVersion - ExpectedDashboardAclInfoList []*models.DashboardAclInfoDTO - ExpectedUserOrgList []*models.UserOrgDTO - ExpectedOrgListResponse OrgListResponse - ExpectedDashboardSnapshot *models.DashboardSnapshot - ExpectedTeamsByUser []*models.TeamDTO - ExpectedSearchOrgList []*models.OrgDTO - ExpectedDatasources []*models.DataSource - ExpectedOrg *models.Org + LastGetAlertsQuery *models.GetAlertsQuery + LatestUserId int64 + ExpectedUser *models.User + ExpectedDatasource *models.DataSource + ExpectedAlert *models.Alert + ExpectedPluginSetting *models.PluginSetting + ExpectedDashboard *models.Dashboard + ExpectedDashboards []*models.Dashboard + ExpectedDashboardVersions []*models.DashboardVersion + ExpectedDashboardAclInfoList []*models.DashboardAclInfoDTO + ExpectedUserOrgList []*models.UserOrgDTO + ExpectedOrgListResponse OrgListResponse + ExpectedDashboardSnapshot *models.DashboardSnapshot + ExpectedTeamsByUser []*models.TeamDTO + ExpectedSearchOrgList []*models.OrgDTO + ExpectedDatasources []*models.DataSource + ExpectedOrg *models.Org + ExpectedSystemStats *models.SystemStats + ExpectedDataSourceStats []*models.DataSourceStats + ExpectedDataSources []*models.DataSource + ExpectedDataSourcesAccessStats []*models.DataSourceAccessStats + ExpectedNotifierUsageStats []*models.NotifierUsageStats ExpectedError error } @@ -42,6 +47,26 @@ func (m *SQLStoreMock) GetAdminStats(ctx context.Context, query *models.GetAdmin return m.ExpectedError } +func (m *SQLStoreMock) GetAlertNotifiersUsageStats(ctx context.Context, query *models.GetAlertNotifierUsageStatsQuery) error { + query.Result = m.ExpectedNotifierUsageStats + return m.ExpectedError +} + +func (m *SQLStoreMock) GetDataSourceStats(ctx context.Context, query *models.GetDataSourceStatsQuery) error { + query.Result = m.ExpectedDataSourceStats + return m.ExpectedError +} + +func (m *SQLStoreMock) GetDataSourceAccessStats(ctx context.Context, query *models.GetDataSourceAccessStatsQuery) error { + query.Result = m.ExpectedDataSourcesAccessStats + return m.ExpectedError +} + +func (m *SQLStoreMock) GetSystemStats(ctx context.Context, query *models.GetSystemStatsQuery) error { + query.Result = m.ExpectedSystemStats + return m.ExpectedError +} + func (m *SQLStoreMock) DeleteExpiredSnapshots(ctx context.Context, cmd *models.DeleteExpiredSnapshotsCommand) error { return m.ExpectedError } diff --git a/pkg/services/sqlstore/stats.go b/pkg/services/sqlstore/stats.go index 044d1117fe6..0a0a8da7fe4 100644 --- a/pkg/services/sqlstore/stats.go +++ b/pkg/services/sqlstore/stats.go @@ -10,42 +10,42 @@ import ( ) func init() { - bus.AddHandler("sql", GetSystemStats) - bus.AddHandler("sql", GetDataSourceStats) - bus.AddHandler("sql", GetDataSourceAccessStats) - bus.AddHandler("sql", GetAlertNotifiersUsageStats) bus.AddHandler("sql", GetSystemUserCountStats) } func (ss *SQLStore) addStatsQueryAndCommandHandlers() { bus.AddHandler("sql", ss.GetAdminStats) + bus.AddHandler("sql", ss.GetAlertNotifiersUsageStats) + bus.AddHandler("sql", ss.GetDataSourceAccessStats) + bus.AddHandler("sql", ss.GetDataSourceStats) + bus.AddHandler("sql", ss.GetSystemStats) } const activeUserTimeLimit = time.Hour * 24 * 30 const dailyActiveUserTimeLimit = time.Hour * 24 -func GetAlertNotifiersUsageStats(ctx context.Context, query *models.GetAlertNotifierUsageStatsQuery) error { +func (ss *SQLStore) GetAlertNotifiersUsageStats(ctx context.Context, query *models.GetAlertNotifierUsageStatsQuery) error { var rawSQL = `SELECT COUNT(*) AS count, type FROM ` + dialect.Quote("alert_notification") + ` GROUP BY type` query.Result = make([]*models.NotifierUsageStats, 0) err := x.SQL(rawSQL).Find(&query.Result) return err } -func GetDataSourceStats(ctx context.Context, query *models.GetDataSourceStatsQuery) error { +func (ss *SQLStore) GetDataSourceStats(ctx context.Context, query *models.GetDataSourceStatsQuery) error { var rawSQL = `SELECT COUNT(*) AS count, type FROM ` + dialect.Quote("data_source") + ` GROUP BY type` query.Result = make([]*models.DataSourceStats, 0) err := x.SQL(rawSQL).Find(&query.Result) return err } -func GetDataSourceAccessStats(ctx context.Context, query *models.GetDataSourceAccessStatsQuery) error { +func (ss *SQLStore) GetDataSourceAccessStats(ctx context.Context, query *models.GetDataSourceAccessStatsQuery) error { var rawSQL = `SELECT COUNT(*) AS count, type, access FROM ` + dialect.Quote("data_source") + ` GROUP BY type, access` query.Result = make([]*models.DataSourceAccessStats, 0) err := x.SQL(rawSQL).Find(&query.Result) return err } -func GetSystemStats(ctx context.Context, query *models.GetSystemStatsQuery) error { +func (ss *SQLStore) GetSystemStats(ctx context.Context, query *models.GetSystemStatsQuery) error { sb := &SQLBuilder{} sb.Write("SELECT ") sb.Write(`(SELECT COUNT(*) FROM ` + dialect.Quote("user") + `) AS users,`) diff --git a/pkg/services/sqlstore/stats_test.go b/pkg/services/sqlstore/stats_test.go index 758d91703dd..1100e8fafea 100644 --- a/pkg/services/sqlstore/stats_test.go +++ b/pkg/services/sqlstore/stats_test.go @@ -19,7 +19,7 @@ func TestStatsDataAccess(t *testing.T) { t.Run("Get system stats should not results in error", func(t *testing.T) { query := models.GetSystemStatsQuery{} - err := GetSystemStats(context.Background(), &query) + err := sqlStore.GetSystemStats(context.Background(), &query) require.NoError(t, err) assert.Equal(t, int64(3), query.Result.Users) assert.Equal(t, int64(0), query.Result.Editors) @@ -38,19 +38,19 @@ func TestStatsDataAccess(t *testing.T) { t.Run("Get datasource stats should not results in error", func(t *testing.T) { query := models.GetDataSourceStatsQuery{} - err := GetDataSourceStats(context.Background(), &query) + err := sqlStore.GetDataSourceStats(context.Background(), &query) assert.NoError(t, err) }) t.Run("Get datasource access stats should not results in error", func(t *testing.T) { query := models.GetDataSourceAccessStatsQuery{} - err := GetDataSourceAccessStats(context.Background(), &query) + err := sqlStore.GetDataSourceAccessStats(context.Background(), &query) assert.NoError(t, err) }) t.Run("Get alert notifier stats should not results in error", func(t *testing.T) { query := models.GetAlertNotifierUsageStatsQuery{} - err := GetAlertNotifiersUsageStats(context.Background(), &query) + err := sqlStore.GetAlertNotifiersUsageStats(context.Background(), &query) assert.NoError(t, err) }) diff --git a/pkg/services/sqlstore/store.go b/pkg/services/sqlstore/store.go index 7942dd80176..43ec0393d6f 100644 --- a/pkg/services/sqlstore/store.go +++ b/pkg/services/sqlstore/store.go @@ -9,6 +9,10 @@ import ( type Store interface { GetAdminStats(ctx context.Context, query *models.GetAdminStatsQuery) error + GetAlertNotifiersUsageStats(ctx context.Context, query *models.GetAlertNotifierUsageStatsQuery) error + GetDataSourceStats(ctx context.Context, query *models.GetDataSourceStatsQuery) error + GetDataSourceAccessStats(ctx context.Context, query *models.GetDataSourceAccessStatsQuery) error + GetSystemStats(ctx context.Context, query *models.GetSystemStatsQuery) error DeleteExpiredSnapshots(ctx context.Context, cmd *models.DeleteExpiredSnapshotsCommand) error CreateDashboardSnapshot(ctx context.Context, cmd *models.CreateDashboardSnapshotCommand) error DeleteDashboardSnapshot(ctx context.Context, cmd *models.DeleteDashboardSnapshotCommand) error