Chore: Split usagestats service (#65191)

pull/65301/head
Andres Martinez Gotor 2 years ago committed by GitHub
parent 9ba38b760a
commit 10adb1ff66
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 5
      pkg/infra/usagestats/mock.go
  2. 1
      pkg/infra/usagestats/service.go
  3. 13
      pkg/infra/usagestats/service/service.go
  4. 2
      pkg/infra/usagestats/service/usage_stats_test.go
  5. 8
      pkg/infra/usagestats/statscollector/service.go
  6. 2
      pkg/infra/usagestats/statscollector/service_test.go
  7. 12
      pkg/infra/usagestats/validator/fake.go
  8. 28
      pkg/infra/usagestats/validator/impl.go
  9. 7
      pkg/infra/usagestats/validator/service.go
  10. 2
      pkg/server/wire.go
  11. 7
      pkg/services/alerting/engine.go
  12. 4
      pkg/services/alerting/engine_integration_test.go
  13. 4
      pkg/services/alerting/engine_test.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) {}

@ -23,5 +23,4 @@ type Service interface {
GetUsageReport(context.Context) (Report, error)
RegisterMetricsFunc(MetricsFunc)
RegisterSendReportCallback(SendReportCallbackFunc)
ShouldBeReported(context.Context, string) bool
}

@ -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",

@ -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(),

@ -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]

@ -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,

@ -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")
}

@ -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()
}

@ -0,0 +1,7 @@
package validator
import "context"
type Service interface {
ShouldBeReported(context.Context, string) bool
}

@ -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)),

@ -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

@ -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{}

@ -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

Loading…
Cancel
Save