From 02a8f62021750b0af6dd8760ca2516e5e285a998 Mon Sep 17 00:00:00 2001 From: gotjosh Date: Fri, 17 Mar 2023 11:19:18 +0000 Subject: [PATCH] Alerting: Fix stats that display alert count when using unified alerting (#64852) * Alerting: Fix stats when using unified alerting --- .../statscollector/concurrent_users_test.go | 5 +- pkg/services/ngalert/ngalert.go | 2 +- pkg/services/stats/statsimpl/stats.go | 25 +++-- pkg/services/stats/statsimpl/stats_test.go | 3 +- pkg/tests/api/stats/admin_test.go | 92 +++++++++++++++++++ 5 files changed, 116 insertions(+), 11 deletions(-) create mode 100644 pkg/tests/api/stats/admin_test.go diff --git a/pkg/infra/usagestats/statscollector/concurrent_users_test.go b/pkg/infra/usagestats/statscollector/concurrent_users_test.go index da61dbba372..15c006441bd 100644 --- a/pkg/infra/usagestats/statscollector/concurrent_users_test.go +++ b/pkg/infra/usagestats/statscollector/concurrent_users_test.go @@ -13,12 +13,13 @@ import ( "github.com/grafana/grafana/pkg/infra/db" "github.com/grafana/grafana/pkg/services/stats/statsimpl" + "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/util" ) func TestConcurrentUsersMetrics(t *testing.T) { sqlStore, cfg := db.InitTestDBwithCfg(t) - statsService := statsimpl.ProvideService(sqlStore) + statsService := statsimpl.ProvideService(&setting.Cfg{}, sqlStore) s := createService(t, cfg, sqlStore, statsService) createConcurrentTokens(t, sqlStore) @@ -36,7 +37,7 @@ func TestConcurrentUsersMetrics(t *testing.T) { func TestConcurrentUsersStats(t *testing.T) { sqlStore, cfg := db.InitTestDBwithCfg(t) - statsService := statsimpl.ProvideService(sqlStore) + statsService := statsimpl.ProvideService(&setting.Cfg{}, sqlStore) s := createService(t, cfg, sqlStore, statsService) createConcurrentTokens(t, sqlStore) diff --git a/pkg/services/ngalert/ngalert.go b/pkg/services/ngalert/ngalert.go index eba15581956..5fc890af39e 100644 --- a/pkg/services/ngalert/ngalert.go +++ b/pkg/services/ngalert/ngalert.go @@ -338,7 +338,7 @@ func (ng *AlertNG) Run(ctx context.Context) error { return children.Wait() } -// IsDisabled returns true if the alerting service is disable for this instance. +// IsDisabled returns true if the alerting service is disabled for this instance. func (ng *AlertNG) IsDisabled() bool { if ng.Cfg == nil { return true diff --git a/pkg/services/stats/statsimpl/stats.go b/pkg/services/stats/statsimpl/stats.go index ed26afd847e..030c0878cbe 100644 --- a/pkg/services/stats/statsimpl/stats.go +++ b/pkg/services/stats/statsimpl/stats.go @@ -2,6 +2,7 @@ package statsimpl import ( "context" + "fmt" "strconv" "time" @@ -11,16 +12,20 @@ import ( "github.com/grafana/grafana/pkg/services/org" "github.com/grafana/grafana/pkg/services/sqlstore/migrator" "github.com/grafana/grafana/pkg/services/stats" + "github.com/grafana/grafana/pkg/setting" ) const activeUserTimeLimit = time.Hour * 24 * 30 const dailyActiveUserTimeLimit = time.Hour * 24 -func ProvideService(db db.DB) stats.Service { - return &sqlStatsService{db: db} +func ProvideService(cfg *setting.Cfg, db db.DB) stats.Service { + return &sqlStatsService{cfg: cfg, db: db} } -type sqlStatsService struct{ db db.DB } +type sqlStatsService struct { + db db.DB + cfg *setting.Cfg +} func (ss *sqlStatsService) GetAlertNotifiersUsageStats(ctx context.Context, query *stats.GetAlertNotifierUsageStatsQuery) error { return ss.db.WithDbSession(ctx, func(dbSession *db.Session) error { @@ -173,6 +178,11 @@ func (ss *sqlStatsService) GetAdminStats(ctx context.Context, query *stats.GetAd dailyActiveEndDate := now.Add(-dailyActiveUserTimeLimit) monthlyActiveEndDate := time.Date(now.Year(), now.Month(), 1, 0, 0, 0, 0, now.Location()) + alertsQuery := fmt.Sprintf("SELECT COUNT(*) FROM %s", dialect.Quote("alert")) + if ss.IsUnifiedAlertingEnabled() { + alertsQuery = fmt.Sprintf("SELECT COUNT(*) FROM %s", dialect.Quote("alert_rule")) + } + var rawSQL = `SELECT ( SELECT COUNT(*) @@ -202,10 +212,7 @@ func (ss *sqlStatsService) GetAdminStats(ctx context.Context, query *stats.GetAd SELECT COUNT(*) FROM ` + dialect.Quote("star") + ` ) AS stars, - ( - SELECT COUNT(*) - FROM ` + dialect.Quote("alert") + ` - ) AS alerts, + (` + alertsQuery + ` ) AS alerts, ( SELECT COUNT(*) FROM ` + dialect.Quote("user") + ` WHERE ` + notServiceAccount(dialect) + ` @@ -258,6 +265,10 @@ func (ss *sqlStatsService) GetSystemUserCountStats(ctx context.Context, query *s }) } +func (ss *sqlStatsService) IsUnifiedAlertingEnabled() bool { + return ss.cfg != nil && ss.cfg.UnifiedAlerting.IsEnabled() +} + func (ss *sqlStatsService) updateUserRoleCountsIfNecessary(ctx context.Context, forced bool) error { memoizationPeriod := time.Now().Add(-userStatsCacheLimetime) if forced || userStatsCache.memoized.Before(memoizationPeriod) { diff --git a/pkg/services/stats/statsimpl/stats_test.go b/pkg/services/stats/statsimpl/stats_test.go index c9787590a1c..678c1870d20 100644 --- a/pkg/services/stats/statsimpl/stats_test.go +++ b/pkg/services/stats/statsimpl/stats_test.go @@ -16,6 +16,7 @@ import ( "github.com/grafana/grafana/pkg/services/supportbundles/supportbundlestest" "github.com/grafana/grafana/pkg/services/user" "github.com/grafana/grafana/pkg/services/user/userimpl" + "github.com/grafana/grafana/pkg/setting" ) func TestIntegrationStatsDataAccess(t *testing.T) { @@ -122,7 +123,7 @@ func TestIntegration_GetAdminStats(t *testing.T) { t.Skip("skipping integration test") } db := sqlstore.InitTestDB(t) - statsService := ProvideService(db) + statsService := ProvideService(&setting.Cfg{}, db) query := stats.GetAdminStatsQuery{} err := statsService.GetAdminStats(context.Background(), &query) diff --git a/pkg/tests/api/stats/admin_test.go b/pkg/tests/api/stats/admin_test.go new file mode 100644 index 00000000000..5a9c9107115 --- /dev/null +++ b/pkg/tests/api/stats/admin_test.go @@ -0,0 +1,92 @@ +package stats + +import ( + "context" + "fmt" + "net/http" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/grafana/grafana/pkg/services/org" + "github.com/grafana/grafana/pkg/services/org/orgimpl" + "github.com/grafana/grafana/pkg/services/quota/quotaimpl" + "github.com/grafana/grafana/pkg/services/sqlstore" + "github.com/grafana/grafana/pkg/services/supportbundles/supportbundlestest" + "github.com/grafana/grafana/pkg/services/user" + "github.com/grafana/grafana/pkg/services/user/userimpl" + "github.com/grafana/grafana/pkg/tests/testinfra" +) + +func TestIntegrationAdminStats(t *testing.T) { + t.Run("with unified alerting enabled", func(t *testing.T) { + url := grafanaSetup(t, testinfra.GrafanaOpts{ + DisableLegacyAlerting: true, + EnableUnifiedAlerting: true, + AppModeProduction: true, + }) + + // nolint:gosec + resp, err := http.Get(url) + defer func() { + _ = resp.Body.Close() + }() + require.NoError(t, err) + require.Equal(t, http.StatusOK, resp.StatusCode) + }) + + t.Run("with legacy alerting enabled", func(t *testing.T) { + url := grafanaSetup(t, testinfra.GrafanaOpts{ + DisableLegacyAlerting: false, + EnableUnifiedAlerting: false, + AppModeProduction: true, + }) + + // nolint:gosec + resp, err := http.Get(url) + defer func() { + _ = resp.Body.Close() + }() + require.NoError(t, err) + require.Equal(t, http.StatusOK, resp.StatusCode) + }) +} + +// grafanaSetup creates the grafana server, user and returns the URL with credentials of the api/admin/stats endpoint. +func grafanaSetup(t *testing.T, opts testinfra.GrafanaOpts) string { + t.Helper() + + testinfra.SQLiteIntegrationTest(t) + + // Setup Grafana and its Database + dir, path := testinfra.CreateGrafDir(t, opts) + + grafanaListedAddr, store := testinfra.StartGrafana(t, dir, path) + + // Create a user to make authenticated requests + createUser(t, store, user.CreateUserCommand{ + DefaultOrgRole: string(org.RoleAdmin), + Login: "grafana", + Password: "password", + IsAdmin: true, + }) + + return fmt.Sprintf("http://%s:%s@%s/api/admin/stats", "grafana", "password", grafanaListedAddr) +} + +func createUser(t *testing.T, store *sqlstore.SQLStore, cmd user.CreateUserCommand) int64 { + t.Helper() + + store.Cfg.AutoAssignOrg = true + store.Cfg.AutoAssignOrgId = 1 + + quotaService := quotaimpl.ProvideService(store, store.Cfg) + orgService, err := orgimpl.ProvideService(store, store.Cfg, quotaService) + require.NoError(t, err) + usrSvc, err := userimpl.ProvideService(store, orgService, store.Cfg, nil, nil, quotaService, supportbundlestest.NewFakeBundleService()) + require.NoError(t, err) + + u, err := usrSvc.Create(context.Background(), &cmd) + require.NoError(t, err) + return u.ID +}