diff --git a/pkg/infra/usagestats/mock.go b/pkg/infra/usagestats/mock.go index 0510233fbb8..f4c049c0010 100644 --- a/pkg/infra/usagestats/mock.go +++ b/pkg/infra/usagestats/mock.go @@ -2,7 +2,6 @@ package usagestats import ( "context" - "strings" "testing" "github.com/stretchr/testify/require" @@ -30,8 +29,4 @@ func (usm *UsageStatsMock) GetUsageReport(ctx context.Context) (Report, error) { return Report{Metrics: all}, nil } -func (usm *UsageStatsMock) ShouldBeReported(_ context.Context, s string) bool { - return !strings.HasPrefix(s, "unknown") -} - func (usm *UsageStatsMock) RegisterSendReportCallback(_ SendReportCallbackFunc) {} diff --git a/pkg/infra/usagestats/service.go b/pkg/infra/usagestats/service.go index 51d646d2278..9934cdf26e3 100644 --- a/pkg/infra/usagestats/service.go +++ b/pkg/infra/usagestats/service.go @@ -23,5 +23,4 @@ type Service interface { GetUsageReport(context.Context) (Report, error) RegisterMetricsFunc(MetricsFunc) RegisterSendReportCallback(SendReportCallbackFunc) - ShouldBeReported(context.Context, string) bool } diff --git a/pkg/infra/usagestats/service/service.go b/pkg/infra/usagestats/service/service.go index 4097eb20da1..9633d6001cc 100644 --- a/pkg/infra/usagestats/service/service.go +++ b/pkg/infra/usagestats/service/service.go @@ -10,7 +10,6 @@ import ( "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/infra/tracing" "github.com/grafana/grafana/pkg/infra/usagestats" - "github.com/grafana/grafana/pkg/plugins" ac "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/supportbundles" "github.com/grafana/grafana/pkg/setting" @@ -20,7 +19,6 @@ type UsageStats struct { Cfg *setting.Cfg kvStore *kvstore.NamespacedKVStore RouteRegister routing.RouteRegister - pluginStore plugins.Store accesscontrol ac.AccessControl log log.Logger @@ -31,7 +29,6 @@ type UsageStats struct { } func ProvideService(cfg *setting.Cfg, - pluginStore plugins.Store, kvStore kvstore.KVStore, routeRegister routing.RouteRegister, tracer tracing.Tracer, @@ -42,7 +39,6 @@ func ProvideService(cfg *setting.Cfg, s := &UsageStats{ Cfg: cfg, RouteRegister: routeRegister, - pluginStore: pluginStore, kvStore: kvstore.WithNamespace(kvStore, 0, "infra.usagestats"), log: log.New("infra.usagestats"), tracer: tracer, @@ -115,15 +111,6 @@ func (uss *UsageStats) RegisterSendReportCallback(c usagestats.SendReportCallbac uss.sendReportCallbacks = append(uss.sendReportCallbacks, c) } -func (uss *UsageStats) ShouldBeReported(ctx context.Context, dsType string) bool { - ds, exists := uss.pluginStore.Plugin(ctx, dsType) - if !exists { - return false - } - - return ds.Signature.IsValid() || ds.Signature.IsInternal() -} - func (uss *UsageStats) supportBundleCollector() supportbundles.Collector { return supportbundles.Collector{ UID: "usage-stats", diff --git a/pkg/infra/usagestats/service/usage_stats_test.go b/pkg/infra/usagestats/service/usage_stats_test.go index 0ec0264e76d..a5d1e30f496 100644 --- a/pkg/infra/usagestats/service/usage_stats_test.go +++ b/pkg/infra/usagestats/service/usage_stats_test.go @@ -21,7 +21,6 @@ import ( "github.com/grafana/grafana/pkg/infra/kvstore" "github.com/grafana/grafana/pkg/infra/tracing" "github.com/grafana/grafana/pkg/infra/usagestats" - "github.com/grafana/grafana/pkg/plugins" "github.com/grafana/grafana/pkg/services/accesscontrol/actest" "github.com/grafana/grafana/pkg/services/supportbundles/supportbundlestest" "github.com/grafana/grafana/pkg/setting" @@ -218,7 +217,6 @@ func createService(t *testing.T, cfg setting.Cfg, sqlStore db.DB, withDB bool) * service, _ := ProvideService( &cfg, - &plugins.FakePluginStore{}, kvstore.ProvideService(sqlStore), routing.NewRouteRegister(), tracing.InitializeTracerForTest(), diff --git a/pkg/infra/usagestats/statscollector/service.go b/pkg/infra/usagestats/statscollector/service.go index b2ac2af4494..d14b15fef10 100644 --- a/pkg/infra/usagestats/statscollector/service.go +++ b/pkg/infra/usagestats/statscollector/service.go @@ -13,6 +13,7 @@ import ( "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/infra/metrics" "github.com/grafana/grafana/pkg/infra/usagestats" + "github.com/grafana/grafana/pkg/infra/usagestats/validator" "github.com/grafana/grafana/pkg/login/social" "github.com/grafana/grafana/pkg/plugins" "github.com/grafana/grafana/pkg/registry" @@ -27,6 +28,7 @@ type Service struct { sqlstore db.DB plugins plugins.Store usageStats usagestats.Service + validator validator.Service statsService stats.Service features *featuremgmt.FeatureManager datasources datasources.DataSourceService @@ -42,6 +44,7 @@ type Service struct { func ProvideService( us usagestats.Service, + validator validator.Service, statsService stats.Service, cfg *setting.Cfg, store db.DB, @@ -56,6 +59,7 @@ func ProvideService( sqlstore: store, plugins: plugins, usageStats: us, + validator: validator, statsService: statsService, features: features, datasources: datasourceService, @@ -225,7 +229,7 @@ func (s *Service) collectDatasourceStats(ctx context.Context) (map[string]interf // as sending that name could be sensitive information dsOtherCount := 0 for _, dsStat := range dsStats.Result { - if s.usageStats.ShouldBeReported(ctx, dsStat.Type) { + if s.validator.ShouldBeReported(ctx, dsStat.Type) { m["stats.ds."+dsStat.Type+".count"] = dsStat.Count } else { dsOtherCount += dsStat.Count @@ -279,7 +283,7 @@ func (s *Service) collectDatasourceAccess(ctx context.Context) (map[string]inter access := strings.ToLower(dsAccessStat.Access) - if s.usageStats.ShouldBeReported(ctx, dsAccessStat.Type) { + if s.validator.ShouldBeReported(ctx, dsAccessStat.Type) { m["stats.ds_access."+dsAccessStat.Type+"."+access+".count"] = dsAccessStat.Count } else { old := dsAccessOtherCount[access] diff --git a/pkg/infra/usagestats/statscollector/service_test.go b/pkg/infra/usagestats/statscollector/service_test.go index f9118c8d748..2d6c49d9285 100644 --- a/pkg/infra/usagestats/statscollector/service_test.go +++ b/pkg/infra/usagestats/statscollector/service_test.go @@ -17,6 +17,7 @@ import ( "github.com/grafana/grafana/pkg/infra/db/dbtest" "github.com/grafana/grafana/pkg/infra/httpclient" "github.com/grafana/grafana/pkg/infra/usagestats" + "github.com/grafana/grafana/pkg/infra/usagestats/validator" "github.com/grafana/grafana/pkg/login/social" "github.com/grafana/grafana/pkg/plugins" "github.com/grafana/grafana/pkg/registry" @@ -436,6 +437,7 @@ func createService(t testing.TB, cfg *setting.Cfg, store db.DB, statsService sta return ProvideService( &usagestats.UsageStatsMock{}, + &validator.FakeUsageStatsValidator{}, statsService, cfg, store, diff --git a/pkg/infra/usagestats/validator/fake.go b/pkg/infra/usagestats/validator/fake.go new file mode 100644 index 00000000000..bfc9682ba5e --- /dev/null +++ b/pkg/infra/usagestats/validator/fake.go @@ -0,0 +1,12 @@ +package validator + +import ( + "context" + "strings" +) + +type FakeUsageStatsValidator struct{} + +func (uss *FakeUsageStatsValidator) ShouldBeReported(ctx context.Context, s string) bool { + return !strings.HasPrefix(s, "unknown") +} diff --git a/pkg/infra/usagestats/validator/impl.go b/pkg/infra/usagestats/validator/impl.go new file mode 100644 index 00000000000..48ab34ef324 --- /dev/null +++ b/pkg/infra/usagestats/validator/impl.go @@ -0,0 +1,28 @@ +package validator + +import ( + "context" + + "github.com/grafana/grafana/pkg/plugins" +) + +type UsageStatsValidator struct { + pluginStore plugins.Store +} + +func ProvideService(pluginStore plugins.Store) (Service, error) { + s := &UsageStatsValidator{ + pluginStore: pluginStore, + } + + return s, nil +} + +func (uss *UsageStatsValidator) ShouldBeReported(ctx context.Context, dsType string) bool { + ds, exists := uss.pluginStore.Plugin(ctx, dsType) + if !exists { + return false + } + + return ds.Signature.IsValid() || ds.Signature.IsInternal() +} diff --git a/pkg/infra/usagestats/validator/service.go b/pkg/infra/usagestats/validator/service.go new file mode 100644 index 00000000000..a9d338b026e --- /dev/null +++ b/pkg/infra/usagestats/validator/service.go @@ -0,0 +1,7 @@ +package validator + +import "context" + +type Service interface { + ShouldBeReported(context.Context, string) bool +} diff --git a/pkg/server/wire.go b/pkg/server/wire.go index d266c0776ef..340d0db3bf2 100644 --- a/pkg/server/wire.go +++ b/pkg/server/wire.go @@ -25,6 +25,7 @@ import ( "github.com/grafana/grafana/pkg/infra/usagestats" uss "github.com/grafana/grafana/pkg/infra/usagestats/service" "github.com/grafana/grafana/pkg/infra/usagestats/statscollector" + "github.com/grafana/grafana/pkg/infra/usagestats/validator" loginpkg "github.com/grafana/grafana/pkg/login" "github.com/grafana/grafana/pkg/login/social" "github.com/grafana/grafana/pkg/middleware/csrf" @@ -188,6 +189,7 @@ var wireBasicSet = wire.NewSet( updatechecker.ProvidePluginsService, uss.ProvideService, wire.Bind(new(usagestats.Service), new(*uss.UsageStats)), + validator.ProvideService, pluginsintegration.WireSet, pluginDashboards.ProvideFileStoreManager, wire.Bind(new(pluginDashboards.FileStore), new(*pluginDashboards.FileStoreManager)), diff --git a/pkg/services/alerting/engine.go b/pkg/services/alerting/engine.go index c20c821bb0d..0c42c7a9d85 100644 --- a/pkg/services/alerting/engine.go +++ b/pkg/services/alerting/engine.go @@ -15,6 +15,7 @@ import ( "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/infra/tracing" "github.com/grafana/grafana/pkg/infra/usagestats" + "github.com/grafana/grafana/pkg/infra/usagestats/validator" "github.com/grafana/grafana/pkg/services/annotations" "github.com/grafana/grafana/pkg/services/dashboards" "github.com/grafana/grafana/pkg/services/datasources" @@ -44,6 +45,7 @@ type AlertEngine struct { log log.Logger resultHandler resultHandler usageStatsService usagestats.Service + validator validator.Service tracer tracing.Tracer AlertStore AlertStore dashAlertExtractor DashAlertExtractor @@ -59,7 +61,7 @@ func (e *AlertEngine) IsDisabled() bool { // ProvideAlertEngine returns a new AlertEngine. func ProvideAlertEngine(renderer rendering.Service, requestValidator validations.PluginRequestValidator, - dataService legacydata.RequestHandler, usageStatsService usagestats.Service, encryptionService encryption.Internal, + dataService legacydata.RequestHandler, usageStatsService usagestats.Service, validator validator.Service, encryptionService encryption.Internal, notificationService *notifications.NotificationService, tracer tracing.Tracer, store AlertStore, cfg *setting.Cfg, dashAlertExtractor DashAlertExtractor, dashboardService dashboards.DashboardService, cacheService *localcache.CacheService, dsService datasources.DataSourceService, annotationsRepo annotations.Repository) *AlertEngine { e := &AlertEngine{ @@ -68,6 +70,7 @@ func ProvideAlertEngine(renderer rendering.Service, requestValidator validations RequestValidator: requestValidator, DataService: dataService, usageStatsService: usageStatsService, + validator: validator, tracer: tracer, AlertStore: store, dashAlertExtractor: dashAlertExtractor, @@ -278,7 +281,7 @@ func (e *AlertEngine) registerUsageMetrics() { metrics := map[string]interface{}{} for dsType, usageCount := range alertingUsageStats.DatasourceUsage { - if e.usageStatsService.ShouldBeReported(ctx, dsType) { + if e.validator.ShouldBeReported(ctx, dsType) { metrics[fmt.Sprintf("stats.alerting.ds.%s.count", dsType)] = usageCount } else { alertingOtherCount += usageCount diff --git a/pkg/services/alerting/engine_integration_test.go b/pkg/services/alerting/engine_integration_test.go index 519157b195e..0f0a48545e0 100644 --- a/pkg/services/alerting/engine_integration_test.go +++ b/pkg/services/alerting/engine_integration_test.go @@ -14,6 +14,7 @@ import ( "github.com/grafana/grafana/pkg/infra/localcache" "github.com/grafana/grafana/pkg/infra/tracing" "github.com/grafana/grafana/pkg/infra/usagestats" + "github.com/grafana/grafana/pkg/infra/usagestats/validator" "github.com/grafana/grafana/pkg/services/annotations/annotationstest" datasources "github.com/grafana/grafana/pkg/services/datasources/fakes" encryptionprovider "github.com/grafana/grafana/pkg/services/encryption/provider" @@ -27,6 +28,7 @@ func TestIntegrationEngineTimeouts(t *testing.T) { } usMock := &usagestats.UsageStatsMock{T: t} + usValidatorMock := &validator.FakeUsageStatsValidator{} encProvider := encryptionprovider.ProvideEncryptionProvider() cfg := setting.NewCfg() @@ -38,7 +40,7 @@ func TestIntegrationEngineTimeouts(t *testing.T) { tracer := tracing.InitializeTracerForTest() dsMock := &datasources.FakeDataSourceService{} annotationsRepo := annotationstest.NewFakeAnnotationsRepo() - engine := ProvideAlertEngine(nil, nil, nil, usMock, encService, nil, tracer, nil, setting.NewCfg(), nil, nil, localcache.New(time.Minute, time.Minute), dsMock, annotationsRepo) + engine := ProvideAlertEngine(nil, nil, nil, usMock, usValidatorMock, encService, nil, tracer, nil, setting.NewCfg(), nil, nil, localcache.New(time.Minute, time.Minute), dsMock, annotationsRepo) setting.AlertingNotificationTimeout = 30 * time.Second setting.AlertingMaxAttempts = 3 engine.resultHandler = &FakeResultHandler{} diff --git a/pkg/services/alerting/engine_test.go b/pkg/services/alerting/engine_test.go index 2f240bfc01d..fcad2e3f60d 100644 --- a/pkg/services/alerting/engine_test.go +++ b/pkg/services/alerting/engine_test.go @@ -13,6 +13,7 @@ import ( "github.com/grafana/grafana/pkg/infra/localcache" "github.com/grafana/grafana/pkg/infra/tracing" "github.com/grafana/grafana/pkg/infra/usagestats" + "github.com/grafana/grafana/pkg/infra/usagestats/validator" "github.com/grafana/grafana/pkg/services/alerting/models" "github.com/grafana/grafana/pkg/services/annotations/annotationstest" "github.com/grafana/grafana/pkg/services/dashboards" @@ -118,6 +119,7 @@ func (a *AlertStoreMock) PauseAllAlerts(context.Context, *models.PauseAllAlertCo func TestEngineProcessJob(t *testing.T) { usMock := &usagestats.UsageStatsMock{T: t} + usValidatorMock := &validator.FakeUsageStatsValidator{} encProvider := encryptionprovider.ProvideEncryptionProvider() cfg := setting.NewCfg() @@ -131,7 +133,7 @@ func TestEngineProcessJob(t *testing.T) { dsMock := &fd.FakeDataSourceService{ DataSources: []*datasources.DataSource{{ID: 1, Type: datasources.DS_PROMETHEUS}}, } - engine := ProvideAlertEngine(nil, nil, nil, usMock, encService, nil, tracer, store, setting.NewCfg(), nil, nil, localcache.New(time.Minute, time.Minute), dsMock, annotationstest.NewFakeAnnotationsRepo()) + engine := ProvideAlertEngine(nil, nil, nil, usMock, usValidatorMock, encService, nil, tracer, store, setting.NewCfg(), nil, nil, localcache.New(time.Minute, time.Minute), dsMock, annotationstest.NewFakeAnnotationsRepo()) setting.AlertingEvaluationTimeout = 30 * time.Second setting.AlertingNotificationTimeout = 30 * time.Second setting.AlertingMaxAttempts = 3