From 4371c45dffdcded6aea66efd16db9e6b06670039 Mon Sep 17 00:00:00 2001 From: "Grot (@grafanabot)" <43478413+grafanabot@users.noreply.github.com> Date: Wed, 1 Jun 2022 06:43:02 -0400 Subject: [PATCH] Usage stats: Divide collection into multiple functions to isolate failures (#49928) (#49989) (cherry picked from commit d3ffb9e24541a791c54f8039149a37e1e5ac6339) Co-authored-by: Emil Tullstedt --- pkg/infra/usagestats/service/usage_stats.go | 5 + .../usagestats/service/usage_stats_test.go | 4 +- .../statscollector/concurrent_users.go | 21 +++ .../statscollector/concurrent_users_test.go | 2 +- .../statscollector/prometheus_flavor.go | 15 +- .../usagestats/statscollector/service.go | 162 ++++++++++-------- .../usagestats/statscollector/service_test.go | 136 +++++++++------ 7 files changed, 210 insertions(+), 135 deletions(-) diff --git a/pkg/infra/usagestats/service/usage_stats.go b/pkg/infra/usagestats/service/usage_stats.go index 95df118a6c0..15dc764d2cb 100644 --- a/pkg/infra/usagestats/service/usage_stats.go +++ b/pkg/infra/usagestats/service/usage_stats.go @@ -48,10 +48,13 @@ func (uss *UsageStats) GetUsageReport(ctx context.Context) (usagestats.Report, e } func (uss *UsageStats) gatherMetrics(ctx context.Context, metrics map[string]interface{}) { + totC, errC := 0, 0 for _, fn := range uss.externalMetrics { fnMetrics, err := fn(ctx) + totC++ if err != nil { uss.log.Error("Failed to fetch external metrics", "error", err) + errC++ continue } @@ -59,6 +62,8 @@ func (uss *UsageStats) gatherMetrics(ctx context.Context, metrics map[string]int metrics[name] = value } } + metrics["stats.usagestats.debug.collect.total.count"] = totC + metrics["stats.usagestats.debug.collect.error.count"] = errC } func (uss *UsageStats) RegisterMetricsFunc(fn usagestats.MetricsFunc) { diff --git a/pkg/infra/usagestats/service/usage_stats_test.go b/pkg/infra/usagestats/service/usage_stats_test.go index 7c36eead42e..f68cd2c8c22 100644 --- a/pkg/infra/usagestats/service/usage_stats_test.go +++ b/pkg/infra/usagestats/service/usage_stats_test.go @@ -178,6 +178,7 @@ func TestRegisterMetrics(t *testing.T) { uss.gatherMetrics(context.Background(), metrics) assert.Equal(t, 1, metrics[goodMetricName]) + metricsCount := len(metrics) t.Run("do not add metrics that return an error when fetched", func(t *testing.T) { const badMetricName = "stats.test_external_metric_error.count" @@ -192,7 +193,8 @@ func TestRegisterMetrics(t *testing.T) { require.Nil(t, extErrorMetric, "Invalid metric should not be added") assert.Equal(t, 1, extMetric) - assert.Len(t, metrics, 3, "Expected only one available metric") + assert.Len(t, metrics, metricsCount, "Expected same number of metrics before and after collecting bad metric") + assert.EqualValues(t, 1, metrics["stats.usagestats.debug.collect.error.count"]) }) } diff --git a/pkg/infra/usagestats/statscollector/concurrent_users.go b/pkg/infra/usagestats/statscollector/concurrent_users.go index 55d23881ebc..23ae1e16874 100644 --- a/pkg/infra/usagestats/statscollector/concurrent_users.go +++ b/pkg/infra/usagestats/statscollector/concurrent_users.go @@ -56,3 +56,24 @@ FROM (select count(1) as tokens from user_auth_token group by user_id) uat;` s.concurrentUserStatsCache.memoized = time.Now() return s.concurrentUserStatsCache.stats, nil } + +func (s *Service) collectConcurrentUsers(ctx context.Context) (map[string]interface{}, error) { + m := map[string]interface{}{} + + // Get concurrent users stats as histogram + concurrentUsersStats, err := s.concurrentUsers(ctx) + if err != nil { + s.log.Error("Failed to get concurrent users stats", "error", err) + return nil, err + } + + // Histogram is cumulative and metric name has a postfix of le_"" + m["stats.auth_token_per_user_le_3"] = concurrentUsersStats.BucketLE3 + m["stats.auth_token_per_user_le_6"] = concurrentUsersStats.BucketLE6 + m["stats.auth_token_per_user_le_9"] = concurrentUsersStats.BucketLE9 + m["stats.auth_token_per_user_le_12"] = concurrentUsersStats.BucketLE12 + m["stats.auth_token_per_user_le_15"] = concurrentUsersStats.BucketLE15 + m["stats.auth_token_per_user_le_inf"] = concurrentUsersStats.BucketLEInf + + return m, nil +} diff --git a/pkg/infra/usagestats/statscollector/concurrent_users_test.go b/pkg/infra/usagestats/statscollector/concurrent_users_test.go index dfa027d0b85..c74eb0e11cf 100644 --- a/pkg/infra/usagestats/statscollector/concurrent_users_test.go +++ b/pkg/infra/usagestats/statscollector/concurrent_users_test.go @@ -23,7 +23,7 @@ func TestConcurrentUsersMetrics(t *testing.T) { createConcurrentTokens(t, sqlStore) - stats, err := s.collect(context.Background()) + stats, err := s.collectConcurrentUsers(context.Background()) require.NoError(t, err) assert.Equal(t, int32(1), stats["stats.auth_token_per_user_le_3"]) diff --git a/pkg/infra/usagestats/statscollector/prometheus_flavor.go b/pkg/infra/usagestats/statscollector/prometheus_flavor.go index 652b312ea6a..09ba3a2171c 100644 --- a/pkg/infra/usagestats/statscollector/prometheus_flavor.go +++ b/pkg/infra/usagestats/statscollector/prometheus_flavor.go @@ -18,6 +18,19 @@ type memoPrometheusFlavor struct { memoized time.Time } +func (s *Service) collectPrometheusFlavors(ctx context.Context) (map[string]interface{}, error) { + m := map[string]interface{}{} + variants, err := s.detectPrometheusVariants(ctx) + if err != nil { + return nil, err + } + + for variant, count := range variants { + m["stats.ds.prometheus.flavor."+variant+".count"] = count + } + return m, nil +} + func (s *Service) detectPrometheusVariants(ctx context.Context) (map[string]int64, error) { if s.promFlavorCache.memoized.Add(promFlavorCacheLifetime).After(time.Now()) && s.promFlavorCache.variants != nil { @@ -77,7 +90,7 @@ func (s *Service) detectPrometheusVariant(ctx context.Context, ds *models.DataSo // Possibly configuration error, the risk of a false positive is // too high. s.log.Debug("Failed to send Prometheus build info request", "error", err) - return "", nil + return "unreachable", nil } defer func() { err := resp.Body.Close() diff --git a/pkg/infra/usagestats/statscollector/service.go b/pkg/infra/usagestats/statscollector/service.go index b2c930c2107..c5326b1491a 100644 --- a/pkg/infra/usagestats/statscollector/service.go +++ b/pkg/infra/usagestats/statscollector/service.go @@ -6,6 +6,8 @@ import ( "strings" "time" + "github.com/prometheus/client_golang/prometheus" + "github.com/grafana/grafana/pkg/infra/httpclient" "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/infra/metrics" @@ -18,7 +20,6 @@ import ( "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/sqlstore" "github.com/grafana/grafana/pkg/setting" - "github.com/prometheus/client_golang/prometheus" ) type Service struct { @@ -40,7 +41,7 @@ type Service struct { } func ProvideService( - usagestats usagestats.Service, + us usagestats.Service, cfg *setting.Cfg, store sqlstore.Store, social social.Service, @@ -54,7 +55,7 @@ func ProvideService( sqlstore: store, plugins: plugins, social: social, - usageStats: usagestats, + usageStats: us, features: features, datasources: datasourceService, httpClientProvider: httpClientProvider, @@ -63,7 +64,19 @@ func ProvideService( log: log.New("infra.usagestats.collector"), } - usagestats.RegisterMetricsFunc(s.collect) + collectors := []usagestats.MetricsFunc{ + s.collectSystemStats, + s.collectConcurrentUsers, + s.collectDatasourceStats, + s.collectDatasourceAccess, + s.collectElasticStats, + s.collectAlertNotifierStats, + s.collectPrometheusFlavors, + s.collectAdditionalMetrics, + } + for _, c := range collectors { + us.RegisterMetricsFunc(c) + } return s } @@ -89,7 +102,7 @@ func (s *Service) Run(ctx context.Context) error { } } -func (s *Service) collect(ctx context.Context) (map[string]interface{}, error) { +func (s *Service) collectSystemStats(ctx context.Context) (map[string]interface{}, error) { m := map[string]interface{}{} statsQuery := models.GetSystemStatsQuery{} @@ -158,7 +171,66 @@ func (s *Service) collect(ctx context.Context) (map[string]interface{}, error) { } m["stats.avg_auth_token_per_user.count"] = avgAuthTokensPerUser + m["stats.packaging."+s.cfg.Packaging+".count"] = 1 + m["stats.distributor."+s.cfg.ReportingDistributor+".count"] = 1 + // Add stats about auth configuration + authTypes := map[string]bool{} + authTypes["anonymous"] = s.cfg.AnonymousEnabled + authTypes["basic_auth"] = s.cfg.BasicAuthEnabled + authTypes["ldap"] = s.cfg.LDAPEnabled + authTypes["auth_proxy"] = s.cfg.AuthProxyEnabled + + for provider, enabled := range s.social.GetOAuthProviders() { + authTypes["oauth_"+provider] = enabled + } + + for authType, enabled := range authTypes { + enabledValue := 0 + if enabled { + enabledValue = 1 + } + m["stats.auth_enabled."+authType+".count"] = enabledValue + } + + m["stats.uptime"] = int64(time.Since(s.startTime).Seconds()) + + featureUsageStats := s.features.GetUsageStats(ctx) + for k, v := range featureUsageStats { + m[k] = v + } + + return m, nil +} + +func (s *Service) collectAdditionalMetrics(ctx context.Context) (map[string]interface{}, error) { + m := map[string]interface{}{} + for _, usageStatProvider := range s.usageStatProviders { + stats := usageStatProvider.GetUsageStats(ctx) + for k, v := range stats { + m[k] = v + } + } + return m, nil +} + +func (s *Service) collectAlertNotifierStats(ctx context.Context) (map[string]interface{}, error) { + m := map[string]interface{}{} + // get stats about alert notifier usage + anStats := models.GetAlertNotifierUsageStatsQuery{} + if err := s.sqlstore.GetAlertNotifiersUsageStats(ctx, &anStats); err != nil { + s.log.Error("Failed to get alert notification stats", "error", err) + return nil, err + } + + for _, stats := range anStats.Result { + m["stats.alert_notifiers."+stats.Type+".count"] = stats.Count + } + return m, nil +} + +func (s *Service) collectDatasourceStats(ctx context.Context) (map[string]interface{}, error) { + m := map[string]interface{}{} dsStats := models.GetDataSourceStatsQuery{} if err := s.sqlstore.GetDataSourceStats(ctx, &dsStats); err != nil { s.log.Error("Failed to get datasource stats", "error", err) @@ -178,6 +250,11 @@ func (s *Service) collect(ctx context.Context) (map[string]interface{}, error) { } m["stats.ds.other.count"] = dsOtherCount + return m, nil +} + +func (s *Service) collectElasticStats(ctx context.Context) (map[string]interface{}, error) { + m := map[string]interface{}{} esDataSourcesQuery := models.GetDataSourcesByTypeQuery{Type: models.DS_ES} if err := s.sqlstore.GetDataSourcesByType(ctx, &esDataSourcesQuery); err != nil { s.log.Error("Failed to get elasticsearch json data", "error", err) @@ -196,9 +273,11 @@ func (s *Service) collect(ctx context.Context) (map[string]interface{}, error) { m[statName] = count + 1 } + return m, nil +} - m["stats.packaging."+s.cfg.Packaging+".count"] = 1 - m["stats.distributor."+s.cfg.ReportingDistributor+".count"] = 1 +func (s *Service) collectDatasourceAccess(ctx context.Context) (map[string]interface{}, error) { + m := map[string]interface{}{} // fetch datasource access stats dsAccessStats := models.GetDataSourceAccessStatsQuery{} @@ -207,15 +286,6 @@ func (s *Service) collect(ctx context.Context) (map[string]interface{}, error) { return nil, err } - variants, err := s.detectPrometheusVariants(ctx) - if err != nil { - return nil, err - } - - for variant, count := range variants { - m["stats.ds.prometheus.flavor."+variant+".count"] = count - } - // send access counters for each data source // but ignore any custom data sources // as sending that name could be sensitive information @@ -238,66 +308,6 @@ func (s *Service) collect(ctx context.Context) (map[string]interface{}, error) { for access, count := range dsAccessOtherCount { m["stats.ds_access.other."+access+".count"] = count } - - // get stats about alert notifier usage - anStats := models.GetAlertNotifierUsageStatsQuery{} - if err := s.sqlstore.GetAlertNotifiersUsageStats(ctx, &anStats); err != nil { - s.log.Error("Failed to get alert notification stats", "error", err) - return nil, err - } - - for _, stats := range anStats.Result { - m["stats.alert_notifiers."+stats.Type+".count"] = stats.Count - } - - // Add stats about auth configuration - authTypes := map[string]bool{} - authTypes["anonymous"] = s.cfg.AnonymousEnabled - authTypes["basic_auth"] = s.cfg.BasicAuthEnabled - authTypes["ldap"] = s.cfg.LDAPEnabled - authTypes["auth_proxy"] = s.cfg.AuthProxyEnabled - - for provider, enabled := range s.social.GetOAuthProviders() { - authTypes["oauth_"+provider] = enabled - } - - for authType, enabled := range authTypes { - enabledValue := 0 - if enabled { - enabledValue = 1 - } - m["stats.auth_enabled."+authType+".count"] = enabledValue - } - - // Get concurrent users stats as histogram - concurrentUsersStats, err := s.concurrentUsers(ctx) - if err != nil { - s.log.Error("Failed to get concurrent users stats", "error", err) - return nil, err - } - - // Histogram is cumulative and metric name has a postfix of le_"" - m["stats.auth_token_per_user_le_3"] = concurrentUsersStats.BucketLE3 - m["stats.auth_token_per_user_le_6"] = concurrentUsersStats.BucketLE6 - m["stats.auth_token_per_user_le_9"] = concurrentUsersStats.BucketLE9 - m["stats.auth_token_per_user_le_12"] = concurrentUsersStats.BucketLE12 - m["stats.auth_token_per_user_le_15"] = concurrentUsersStats.BucketLE15 - m["stats.auth_token_per_user_le_inf"] = concurrentUsersStats.BucketLEInf - - m["stats.uptime"] = int64(time.Since(s.startTime).Seconds()) - - featureUsageStats := s.features.GetUsageStats(ctx) - for k, v := range featureUsageStats { - m[k] = v - } - - for _, usageStatProvider := range s.usageStatProviders { - stats := usageStatProvider.GetUsageStats(ctx) - for k, v := range stats { - m[k] = v - } - } - return m, nil } diff --git a/pkg/infra/usagestats/statscollector/service_test.go b/pkg/infra/usagestats/statscollector/service_test.go index 987d417978d..1a3d2c3f8e8 100644 --- a/pkg/infra/usagestats/statscollector/service_test.go +++ b/pkg/infra/usagestats/statscollector/service_test.go @@ -7,12 +7,13 @@ import ( "testing" "time" + "github.com/grafana/grafana/pkg/components/simplejson" + sdkhttpclient "github.com/grafana/grafana-plugin-sdk-go/backend/httpclient" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "github.com/grafana/grafana/pkg/components/simplejson" "github.com/grafana/grafana/pkg/infra/httpclient" "github.com/grafana/grafana/pkg/infra/usagestats" "github.com/grafana/grafana/pkg/login/social" @@ -97,7 +98,7 @@ func TestUsageStatsProviders(t *testing.T) { s := createService(t, setting.NewCfg(), store) s.RegisterProviders([]registry.ProvidesUsageStats{provider1, provider2}) - m, err := s.collect(context.Background()) + m, err := s.collectAdditionalMetrics(context.Background()) require.NoError(t, err, "Expected no error") assert.Equal(t, "val1", m["my_stat_1"]) @@ -111,7 +112,7 @@ func TestFeatureUsageStats(t *testing.T) { mockSystemStats(store) s := createService(t, setting.NewCfg(), store) - m, err := s.collect(context.Background()) + m, err := s.collectSystemStats(context.Background()) require.NoError(t, err, "Expected no error") assert.Equal(t, 1, m["stats.features.feature_1.count"]) @@ -134,6 +135,56 @@ func TestCollectingUsageStats(t *testing.T) { s.startTime = time.Now().Add(-1 * time.Minute) mockSystemStats(sqlStore) + + createConcurrentTokens(t, sqlStore) + + s.social = &mockSocial{ + OAuthProviders: map[string]bool{ + "github": true, + "gitlab": true, + "azuread": true, + "google": true, + "generic_oauth": true, + "grafana_com": true, + }, + } + + metrics, err := s.collectSystemStats(context.Background()) + require.NoError(t, err) + + assert.EqualValues(t, 15, metrics["stats.total_auth_token.count"]) + assert.EqualValues(t, 2, metrics["stats.api_keys.count"]) + assert.EqualValues(t, 5, metrics["stats.avg_auth_token_per_user.count"]) + assert.EqualValues(t, 16, metrics["stats.dashboard_versions.count"]) + assert.EqualValues(t, 17, metrics["stats.annotations.count"]) + assert.EqualValues(t, 18, metrics["stats.alert_rules.count"]) + assert.EqualValues(t, 19, metrics["stats.library_panels.count"]) + assert.EqualValues(t, 20, metrics["stats.library_variables.count"]) + + assert.EqualValues(t, 1, metrics["stats.auth_enabled.anonymous.count"]) + assert.EqualValues(t, 1, metrics["stats.auth_enabled.basic_auth.count"]) + assert.EqualValues(t, 1, metrics["stats.auth_enabled.ldap.count"]) + assert.EqualValues(t, 1, metrics["stats.auth_enabled.auth_proxy.count"]) + assert.EqualValues(t, 1, metrics["stats.auth_enabled.oauth_github.count"]) + assert.EqualValues(t, 1, metrics["stats.auth_enabled.oauth_gitlab.count"]) + assert.EqualValues(t, 1, metrics["stats.auth_enabled.oauth_google.count"]) + assert.EqualValues(t, 1, metrics["stats.auth_enabled.oauth_azuread.count"]) + assert.EqualValues(t, 1, metrics["stats.auth_enabled.oauth_generic_oauth.count"]) + assert.EqualValues(t, 1, metrics["stats.auth_enabled.oauth_grafana_com.count"]) + + assert.EqualValues(t, 1, metrics["stats.packaging.deb.count"]) + assert.EqualValues(t, 1, metrics["stats.distributor.hosted-grafana.count"]) + + assert.EqualValues(t, 11, metrics["stats.data_keys.count"]) + assert.EqualValues(t, 3, metrics["stats.active_data_keys.count"]) + + assert.InDelta(t, int64(65), metrics["stats.uptime"], 6) +} + +func TestDatasourceStats(t *testing.T) { + sqlStore := mockstore.NewSQLStoreMock() + s := createService(t, &setting.Cfg{}, sqlStore) + setupSomeDataSourcePlugins(t, s) sqlStore.ExpectedDataSourceStats = []*models.DataSourceStats{ @@ -216,6 +267,31 @@ func TestCollectingUsageStats(t *testing.T) { }, } + { + db, err := s.collectDatasourceStats(context.Background()) + require.NoError(t, err) + + assert.EqualValues(t, 9, db["stats.ds."+models.DS_ES+".count"]) + assert.EqualValues(t, 10, db["stats.ds."+models.DS_PROMETHEUS+".count"]) + assert.EqualValues(t, 11+12, db["stats.ds.other.count"]) + } + + { + dba, err := s.collectDatasourceAccess(context.Background()) + require.NoError(t, err) + + assert.EqualValues(t, 1, dba["stats.ds_access."+models.DS_ES+".direct.count"]) + assert.EqualValues(t, 2, dba["stats.ds_access."+models.DS_ES+".proxy.count"]) + assert.EqualValues(t, 3, dba["stats.ds_access."+models.DS_PROMETHEUS+".proxy.count"]) + assert.EqualValues(t, 6+7, dba["stats.ds_access.other.direct.count"]) + assert.EqualValues(t, 4+8, dba["stats.ds_access.other.proxy.count"]) + } +} + +func TestAlertNotifiersStats(t *testing.T) { + sqlStore := mockstore.NewSQLStoreMock() + s := createService(t, &setting.Cfg{}, sqlStore) + sqlStore.ExpectedNotifierUsageStats = []*models.NotifierUsageStats{ { Type: "slack", @@ -227,63 +303,11 @@ func TestCollectingUsageStats(t *testing.T) { }, } - createConcurrentTokens(t, sqlStore) - - s.social = &mockSocial{ - OAuthProviders: map[string]bool{ - "github": true, - "gitlab": true, - "azuread": true, - "google": true, - "generic_oauth": true, - "grafana_com": true, - }, - } - - metrics, err := s.collect(context.Background()) + metrics, err := s.collectAlertNotifierStats(context.Background()) require.NoError(t, err) - assert.EqualValues(t, 15, metrics["stats.total_auth_token.count"]) - assert.EqualValues(t, 2, metrics["stats.api_keys.count"]) - assert.EqualValues(t, 5, metrics["stats.avg_auth_token_per_user.count"]) - assert.EqualValues(t, 16, metrics["stats.dashboard_versions.count"]) - assert.EqualValues(t, 17, metrics["stats.annotations.count"]) - assert.EqualValues(t, 18, metrics["stats.alert_rules.count"]) - assert.EqualValues(t, 19, metrics["stats.library_panels.count"]) - assert.EqualValues(t, 20, metrics["stats.library_variables.count"]) - - assert.EqualValues(t, 9, metrics["stats.ds."+models.DS_ES+".count"]) - assert.EqualValues(t, 10, metrics["stats.ds."+models.DS_PROMETHEUS+".count"]) - - assert.EqualValues(t, 11+12, metrics["stats.ds.other.count"]) - - assert.EqualValues(t, 1, metrics["stats.ds_access."+models.DS_ES+".direct.count"]) - assert.EqualValues(t, 2, metrics["stats.ds_access."+models.DS_ES+".proxy.count"]) - assert.EqualValues(t, 3, metrics["stats.ds_access."+models.DS_PROMETHEUS+".proxy.count"]) - assert.EqualValues(t, 6+7, metrics["stats.ds_access.other.direct.count"]) - assert.EqualValues(t, 4+8, metrics["stats.ds_access.other.proxy.count"]) - assert.EqualValues(t, 1, metrics["stats.alert_notifiers.slack.count"]) assert.EqualValues(t, 2, metrics["stats.alert_notifiers.webhook.count"]) - - assert.EqualValues(t, 1, metrics["stats.auth_enabled.anonymous.count"]) - assert.EqualValues(t, 1, metrics["stats.auth_enabled.basic_auth.count"]) - assert.EqualValues(t, 1, metrics["stats.auth_enabled.ldap.count"]) - assert.EqualValues(t, 1, metrics["stats.auth_enabled.auth_proxy.count"]) - assert.EqualValues(t, 1, metrics["stats.auth_enabled.oauth_github.count"]) - assert.EqualValues(t, 1, metrics["stats.auth_enabled.oauth_gitlab.count"]) - assert.EqualValues(t, 1, metrics["stats.auth_enabled.oauth_google.count"]) - assert.EqualValues(t, 1, metrics["stats.auth_enabled.oauth_azuread.count"]) - assert.EqualValues(t, 1, metrics["stats.auth_enabled.oauth_generic_oauth.count"]) - assert.EqualValues(t, 1, metrics["stats.auth_enabled.oauth_grafana_com.count"]) - - assert.EqualValues(t, 1, metrics["stats.packaging.deb.count"]) - assert.EqualValues(t, 1, metrics["stats.distributor.hosted-grafana.count"]) - - assert.EqualValues(t, 11, metrics["stats.data_keys.count"]) - assert.EqualValues(t, 3, metrics["stats.active_data_keys.count"]) - - assert.InDelta(t, int64(65), metrics["stats.uptime"], 6) } func mockSystemStats(sqlStore *mockstore.SQLStoreMock) {