From 5c1f614d3c5e1d779b10a3096f0c2f16f68f6a2f Mon Sep 17 00:00:00 2001 From: Jo Date: Tue, 23 Aug 2022 14:24:55 +0200 Subject: [PATCH] Service Accounts: Separate metrics logic from store (#54085) * separate stats logic from store * remove in_teams unused stat * use init instead --- .../serviceaccounts/database/stats.go | 67 ++----------------- .../serviceaccounts/database/stats_test.go | 8 +-- .../serviceaccounts/manager/service.go | 8 +-- pkg/services/serviceaccounts/manager/stats.go | 57 ++++++++++++++++ .../serviceaccounts/manager/stats_test.go | 28 ++++++++ pkg/services/serviceaccounts/models.go | 5 ++ .../serviceaccounts/serviceaccounts.go | 2 +- pkg/services/serviceaccounts/tests/common.go | 9 ++- 8 files changed, 109 insertions(+), 75 deletions(-) create mode 100644 pkg/services/serviceaccounts/manager/stats.go create mode 100644 pkg/services/serviceaccounts/manager/stats_test.go diff --git a/pkg/services/serviceaccounts/database/stats.go b/pkg/services/serviceaccounts/database/stats.go index a279e46805a..8e99293aa05 100644 --- a/pkg/services/serviceaccounts/database/stats.go +++ b/pkg/services/serviceaccounts/database/stats.go @@ -2,70 +2,22 @@ package database import ( "context" - "sync" + "github.com/grafana/grafana/pkg/services/serviceaccounts" "github.com/grafana/grafana/pkg/services/sqlstore" - "github.com/prometheus/client_golang/prometheus" ) -const ( - ExporterName = "grafana" -) - -var ( - // MStatTotalServiceAccounts is a metric gauge for total number of service accounts - MStatTotalServiceAccounts prometheus.Gauge - - // MStatTotalServiceAccountTokens is a metric gauge for total number of service account tokens - MStatTotalServiceAccountTokens prometheus.Gauge - - once sync.Once - Initialised bool = false -) - -func InitMetrics() { - once.Do(func() { - MStatTotalServiceAccounts = prometheus.NewGauge(prometheus.GaugeOpts{ - Name: "stat_total_service_accounts", - Help: "total amount of service accounts", - Namespace: ExporterName, - }) - - MStatTotalServiceAccountTokens = prometheus.NewGauge(prometheus.GaugeOpts{ - Name: "stat_total_service_account_tokens", - Help: "total amount of service account tokens", - Namespace: ExporterName, - }) - - prometheus.MustRegister( - MStatTotalServiceAccounts, - MStatTotalServiceAccountTokens, - ) - }) -} - -func (s *ServiceAccountsStoreImpl) GetUsageMetrics(ctx context.Context) (map[string]interface{}, error) { - stats := map[string]interface{}{} +func (s *ServiceAccountsStoreImpl) GetUsageMetrics(ctx context.Context) (*serviceaccounts.Stats, error) { + dialect := s.sqlStore.Dialect sb := &sqlstore.SQLBuilder{} - dialect := s.sqlStore.Dialect sb.Write("SELECT ") sb.Write(`(SELECT COUNT(*) FROM ` + dialect.Quote("user") + ` WHERE is_service_account = ` + dialect.BooleanStr(true) + `) AS serviceaccounts,`) sb.Write(`(SELECT COUNT(*) FROM ` + dialect.Quote("api_key") + - ` WHERE service_account_id IS NOT NULL ) AS serviceaccount_tokens,`) - // Add count to how many service accounts are in teams - sb.Write(`(SELECT COUNT(*) FROM team_member - JOIN ` + dialect.Quote("user") + ` on team_member.user_id=` + dialect.Quote("user") + `.id - WHERE ` + dialect.Quote("user") + `.is_service_account=` + dialect.BooleanStr(true) + ` ) as serviceaccounts_in_teams`) + ` WHERE service_account_id IS NOT NULL ) AS serviceaccount_tokens`) - type saStats struct { - ServiceAccounts int64 `xorm:"serviceaccounts"` - Tokens int64 `xorm:"serviceaccount_tokens"` - InTeams int64 `xorm:"serviceaccounts_in_teams"` - } - - var sqlStats saStats + var sqlStats serviceaccounts.Stats if err := s.sqlStore.WithDbSession(ctx, func(sess *sqlstore.DBSession) error { _, err := sess.SQL(sb.GetSQLString(), sb.GetParams()...).Get(&sqlStats) return err @@ -73,12 +25,5 @@ func (s *ServiceAccountsStoreImpl) GetUsageMetrics(ctx context.Context) (map[str return nil, err } - stats["stats.serviceaccounts.count"] = sqlStats.ServiceAccounts - stats["stats.serviceaccounts.tokens.count"] = sqlStats.Tokens - stats["stats.serviceaccounts.in_teams.count"] = sqlStats.InTeams - - MStatTotalServiceAccountTokens.Set(float64(sqlStats.Tokens)) - MStatTotalServiceAccounts.Set(float64(sqlStats.ServiceAccounts)) - - return stats, nil + return &sqlStats, nil } diff --git a/pkg/services/serviceaccounts/database/stats_test.go b/pkg/services/serviceaccounts/database/stats_test.go index f6f5de36fc3..6050ff70154 100644 --- a/pkg/services/serviceaccounts/database/stats_test.go +++ b/pkg/services/serviceaccounts/database/stats_test.go @@ -5,7 +5,6 @@ import ( "testing" "github.com/grafana/grafana/pkg/components/apikeygen" - "github.com/grafana/grafana/pkg/services/apikey" "github.com/grafana/grafana/pkg/services/serviceaccounts" "github.com/grafana/grafana/pkg/services/serviceaccounts/tests" "github.com/stretchr/testify/assert" @@ -16,7 +15,6 @@ func TestStore_UsageStats(t *testing.T) { saToCreate := tests.TestUser{Login: "servicetestwithTeam@admin", IsServiceAccount: true} db, store := setupTestDatabase(t) sa := tests.SetupUserServiceAccount(t, db, saToCreate) - InitMetrics() keyName := t.Name() key, err := apikeygen.New(sa.OrgID, keyName) @@ -27,7 +25,6 @@ func TestStore_UsageStats(t *testing.T) { OrgId: sa.OrgID, Key: key.HashedKey, SecondsToLive: 0, - Result: &apikey.APIKey{}, } err = store.AddServiceAccountToken(context.Background(), sa.ID, &cmd) @@ -36,7 +33,6 @@ func TestStore_UsageStats(t *testing.T) { stats, err := store.GetUsageMetrics(context.Background()) require.NoError(t, err) - assert.Equal(t, int64(1), stats["stats.serviceaccounts.count"].(int64)) - assert.Equal(t, int64(1), stats["stats.serviceaccounts.tokens.count"].(int64)) - assert.Equal(t, int64(0), stats["stats.serviceaccounts.in_teams.count"].(int64)) + assert.Equal(t, int64(1), stats.ServiceAccounts) + assert.Equal(t, int64(1), stats.Tokens) } diff --git a/pkg/services/serviceaccounts/manager/service.go b/pkg/services/serviceaccounts/manager/service.go index 03808e1713c..9e1f362d203 100644 --- a/pkg/services/serviceaccounts/manager/service.go +++ b/pkg/services/serviceaccounts/manager/service.go @@ -11,7 +11,6 @@ import ( "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/serviceaccounts" "github.com/grafana/grafana/pkg/services/serviceaccounts/api" - "github.com/grafana/grafana/pkg/services/serviceaccounts/database" "github.com/grafana/grafana/pkg/setting" ) @@ -33,7 +32,6 @@ func ProvideServiceAccountsService( serviceAccountsStore serviceaccounts.Store, permissionService accesscontrol.ServiceAccountPermissionsService, ) (*ServiceAccountsService, error) { - database.InitMetrics() s := &ServiceAccountsService{ store: serviceAccountsStore, log: log.New("serviceaccounts"), @@ -44,7 +42,7 @@ func ProvideServiceAccountsService( s.log.Error("Failed to register roles", "error", err) } - usageStats.RegisterMetricsFunc(s.store.GetUsageMetrics) + usageStats.RegisterMetricsFunc(s.getUsageMetrics) serviceaccountsAPI := api.NewServiceAccountsAPI(cfg, s, ac, routeRegister, s.store, permissionService) serviceaccountsAPI.RegisterAPIEndpoints() @@ -55,7 +53,7 @@ func ProvideServiceAccountsService( func (sa *ServiceAccountsService) Run(ctx context.Context) error { sa.backgroundLog.Debug("service initialized") - if _, err := sa.store.GetUsageMetrics(ctx); err != nil { + if _, err := sa.getUsageMetrics(ctx); err != nil { sa.log.Warn("Failed to get usage metrics", "error", err.Error()) } @@ -75,7 +73,7 @@ func (sa *ServiceAccountsService) Run(ctx context.Context) error { case <-updateStatsTicker.C: sa.backgroundLog.Debug("updating usage metrics") - if _, err := sa.store.GetUsageMetrics(ctx); err != nil { + if _, err := sa.getUsageMetrics(ctx); err != nil { sa.backgroundLog.Warn("Failed to get usage metrics", "error", err.Error()) } } diff --git a/pkg/services/serviceaccounts/manager/stats.go b/pkg/services/serviceaccounts/manager/stats.go new file mode 100644 index 00000000000..684938f6e0a --- /dev/null +++ b/pkg/services/serviceaccounts/manager/stats.go @@ -0,0 +1,57 @@ +package manager + +import ( + "context" + + "github.com/prometheus/client_golang/prometheus" +) + +const ( + ExporterName = "grafana" +) + +var ( + // MStatTotalServiceAccounts is a metric gauge for total number of service accounts + MStatTotalServiceAccounts prometheus.Gauge + + // MStatTotalServiceAccountTokens is a metric gauge for total number of service account tokens + MStatTotalServiceAccountTokens prometheus.Gauge + + Initialised bool = false +) + +func init() { + MStatTotalServiceAccounts = prometheus.NewGauge(prometheus.GaugeOpts{ + Name: "stat_total_service_accounts", + Help: "total amount of service accounts", + Namespace: ExporterName, + }) + + MStatTotalServiceAccountTokens = prometheus.NewGauge(prometheus.GaugeOpts{ + Name: "stat_total_service_account_tokens", + Help: "total amount of service account tokens", + Namespace: ExporterName, + }) + + prometheus.MustRegister( + MStatTotalServiceAccounts, + MStatTotalServiceAccountTokens, + ) +} + +func (sa *ServiceAccountsService) getUsageMetrics(ctx context.Context) (map[string]interface{}, error) { + stats := map[string]interface{}{} + + sqlStats, err := sa.store.GetUsageMetrics(ctx) + if err != nil { + return nil, err + } + + stats["stats.serviceaccounts.count"] = sqlStats.ServiceAccounts + stats["stats.serviceaccounts.tokens.count"] = sqlStats.Tokens + + MStatTotalServiceAccountTokens.Set(float64(sqlStats.Tokens)) + MStatTotalServiceAccounts.Set(float64(sqlStats.ServiceAccounts)) + + return stats, nil +} diff --git a/pkg/services/serviceaccounts/manager/stats_test.go b/pkg/services/serviceaccounts/manager/stats_test.go new file mode 100644 index 00000000000..cc160744b46 --- /dev/null +++ b/pkg/services/serviceaccounts/manager/stats_test.go @@ -0,0 +1,28 @@ +package manager + +import ( + "context" + "testing" + + "github.com/grafana/grafana/pkg/services/serviceaccounts" + "github.com/grafana/grafana/pkg/services/serviceaccounts/tests" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func Test_UsageStats(t *testing.T) { + storeMock := &tests.ServiceAccountsStoreMock{Calls: tests.Calls{}, Stats: &serviceaccounts.Stats{ + ServiceAccounts: 1, + Tokens: 1, + }} + svc := ServiceAccountsService{store: storeMock} + err := svc.DeleteServiceAccount(context.Background(), 1, 1) + require.NoError(t, err) + assert.Len(t, storeMock.Calls.DeleteServiceAccount, 1) + + stats, err := svc.getUsageMetrics(context.Background()) + require.NoError(t, err) + + assert.Equal(t, int64(1), stats["stats.serviceaccounts.count"].(int64)) + assert.Equal(t, int64(1), stats["stats.serviceaccounts.tokens.count"].(int64)) +} diff --git a/pkg/services/serviceaccounts/models.go b/pkg/services/serviceaccounts/models.go index 59048ba8d00..0916a1d45ac 100644 --- a/pkg/services/serviceaccounts/models.go +++ b/pkg/services/serviceaccounts/models.go @@ -125,3 +125,8 @@ const ( FilterOnlyDisabled ServiceAccountFilter = "disabled" FilterIncludeAll ServiceAccountFilter = "all" ) + +type Stats struct { + ServiceAccounts int64 `xorm:"serviceaccounts"` + Tokens int64 `xorm:"serviceaccount_tokens"` +} diff --git a/pkg/services/serviceaccounts/serviceaccounts.go b/pkg/services/serviceaccounts/serviceaccounts.go index 4e825095288..63e3cd336a0 100644 --- a/pkg/services/serviceaccounts/serviceaccounts.go +++ b/pkg/services/serviceaccounts/serviceaccounts.go @@ -32,5 +32,5 @@ type Store interface { DeleteServiceAccountToken(ctx context.Context, orgID, serviceAccountID, tokenID int64) error RevokeServiceAccountToken(ctx context.Context, orgId, serviceAccountId, tokenId int64) error AddServiceAccountToken(ctx context.Context, serviceAccountID int64, cmd *AddServiceAccountTokenCommand) error - GetUsageMetrics(ctx context.Context) (map[string]interface{}, error) + GetUsageMetrics(ctx context.Context) (*Stats, error) } diff --git a/pkg/services/serviceaccounts/tests/common.go b/pkg/services/serviceaccounts/tests/common.go index 174e34605ba..788d6675629 100644 --- a/pkg/services/serviceaccounts/tests/common.go +++ b/pkg/services/serviceaccounts/tests/common.go @@ -138,6 +138,7 @@ type Calls struct { type ServiceAccountsStoreMock struct { serviceaccounts.Store + Stats *serviceaccounts.Stats Calls Calls } @@ -223,6 +224,10 @@ func (s *ServiceAccountsStoreMock) AddServiceAccountToken(ctx context.Context, s return nil } -func (s *ServiceAccountsStoreMock) GetUsageMetrics(ctx context.Context) (map[string]interface{}, error) { - return map[string]interface{}{}, nil +func (s *ServiceAccountsStoreMock) GetUsageMetrics(ctx context.Context) (*serviceaccounts.Stats, error) { + if s.Stats == nil { + return &serviceaccounts.Stats{}, nil + } + + return s.Stats, nil }