From 4ee389676ed1bc752a0533f696369601e8478b87 Mon Sep 17 00:00:00 2001 From: Jo Date: Thu, 9 Mar 2023 08:26:13 +0000 Subject: [PATCH] RemoteCache: Cleanup infra remote cache (#64381) * add warning on Count * add usagestats service * fix typo * remove unused glog * remote cache stats collect * add encrypt usage stat * rename handler * Update pkg/infra/remotecache/remotecache.go Co-authored-by: Dan Cech * Update pkg/infra/remotecache/remotecache_test.go --------- Co-authored-by: Dan Cech --- pkg/infra/remotecache/memcached_storage.go | 2 +- pkg/infra/remotecache/remotecache.go | 26 ++++++++++++++----- pkg/infra/remotecache/remotecache_test.go | 21 ++++++++++++++- pkg/infra/remotecache/testing.go | 3 ++- .../usagestats/statscollector/service.go | 3 --- .../usagestats/statscollector/service_test.go | 1 - .../contexthandler/auth_proxy_test.go | 3 ++- 7 files changed, 45 insertions(+), 14 deletions(-) diff --git a/pkg/infra/remotecache/memcached_storage.go b/pkg/infra/remotecache/memcached_storage.go index 8fc3d0aa387..ef3ba900d12 100644 --- a/pkg/infra/remotecache/memcached_storage.go +++ b/pkg/infra/remotecache/memcached_storage.go @@ -12,7 +12,7 @@ import ( const memcachedCacheType = "memcached" -var ErrNotImplemented = errors.New("count not implemented") +var ErrNotImplemented = errors.New("not implemented") type memcachedStorage struct { c *memcache.Client diff --git a/pkg/infra/remotecache/remotecache.go b/pkg/infra/remotecache/remotecache.go index 0d9aa6f04fa..6ecf712e586 100644 --- a/pkg/infra/remotecache/remotecache.go +++ b/pkg/infra/remotecache/remotecache.go @@ -7,10 +7,8 @@ import ( "errors" "time" - "github.com/go-kit/log" - "github.com/grafana/grafana/pkg/infra/db" - glog "github.com/grafana/grafana/pkg/infra/log" + "github.com/grafana/grafana/pkg/infra/usagestats" "github.com/grafana/grafana/pkg/registry" "github.com/grafana/grafana/pkg/services/secrets" "github.com/grafana/grafana/pkg/setting" @@ -30,7 +28,8 @@ const ( ServiceName = "RemoteCache" ) -func ProvideService(cfg *setting.Cfg, sqlStore db.DB, secretsService secrets.Service) (*RemoteCache, error) { +func ProvideService(cfg *setting.Cfg, sqlStore db.DB, usageStats usagestats.Service, + secretsService secrets.Service) (*RemoteCache, error) { var codec codec if cfg.RemoteCacheOptions.Encryption { codec = &encryptionCodec{secretsService} @@ -44,12 +43,27 @@ func ProvideService(cfg *setting.Cfg, sqlStore db.DB, secretsService secrets.Ser s := &RemoteCache{ SQLStore: sqlStore, Cfg: cfg, - log: glog.New("cache.remote"), client: client, } + + usageStats.RegisterMetricsFunc(s.getUsageStats) + return s, nil } +func (ds *RemoteCache) getUsageStats(ctx context.Context) (map[string]interface{}, error) { + stats := map[string]interface{}{} + stats["stats.remote_cache."+ds.Cfg.RemoteCacheOptions.Name+".count"] = 1 + encryptVal := 0 + if ds.Cfg.RemoteCacheOptions.Encryption { + encryptVal = 1 + } + + stats["stats.remote_cache.encrypt_enabled.count"] = encryptVal + + return stats, nil +} + // CacheStorage allows the caller to set, get and delete items in the cache. // Cached items are stored as byte arrays and marshalled using "encoding/gob" // so any struct added to the cache needs to be registered with `remotecache.Register` @@ -66,12 +80,12 @@ type CacheStorage interface { // Count returns the number of items in the cache. // Optionaly a prefix can be provided to only count items with that prefix + // DO NOT USE. Not available for memcached. Count(ctx context.Context, prefix string) (int64, error) } // RemoteCache allows Grafana to cache data outside its own process type RemoteCache struct { - log log.Logger client CacheStorage SQLStore db.DB Cfg *setting.Cfg diff --git a/pkg/infra/remotecache/remotecache_test.go b/pkg/infra/remotecache/remotecache_test.go index 1c6bb6281f0..585154af647 100644 --- a/pkg/infra/remotecache/remotecache_test.go +++ b/pkg/infra/remotecache/remotecache_test.go @@ -10,6 +10,7 @@ import ( "github.com/grafana/grafana/pkg/infra/db" "github.com/grafana/grafana/pkg/infra/log" + "github.com/grafana/grafana/pkg/infra/usagestats" "github.com/grafana/grafana/pkg/services/secrets/fakes" "github.com/grafana/grafana/pkg/setting" ) @@ -29,7 +30,7 @@ func createTestClient(t *testing.T, opts *setting.RemoteCacheOptions, sqlstore d cfg := &setting.Cfg{ RemoteCacheOptions: opts, } - dc, err := ProvideService(cfg, sqlstore, fakes.NewFakeSecretsService()) + dc, err := ProvideService(cfg, sqlstore, &usagestats.UsageStatsMock{}, fakes.NewFakeSecretsService()) require.Nil(t, err, "Failed to init client for test") return dc @@ -120,6 +121,24 @@ func canNotFetchExpiredItems(t *testing.T, client CacheStorage) { assert.Equal(t, err, ErrCacheItemNotFound) } +func TestCollectUsageStats(t *testing.T) { + wantMap := map[string]interface{}{ + "stats.remote_cache.redis.count": 1, + "stats.remote_cache.encrypt_enabled.count": 1, + } + cfg := setting.NewCfg() + cfg.RemoteCacheOptions = &setting.RemoteCacheOptions{Name: redisCacheType, Encryption: true} + + remoteCache := &RemoteCache{ + Cfg: cfg, + } + + stats, err := remoteCache.getUsageStats(context.Background()) + require.NoError(t, err) + + assert.EqualValues(t, wantMap, stats) +} + func TestCachePrefix(t *testing.T) { db := db.InitTestDB(t) cache := &databaseCache{ diff --git a/pkg/infra/remotecache/testing.go b/pkg/infra/remotecache/testing.go index f42252aa7c1..e65c9054753 100644 --- a/pkg/infra/remotecache/testing.go +++ b/pkg/infra/remotecache/testing.go @@ -6,6 +6,7 @@ import ( "github.com/stretchr/testify/require" "github.com/grafana/grafana/pkg/infra/db" + "github.com/grafana/grafana/pkg/infra/usagestats" "github.com/grafana/grafana/pkg/services/secrets/fakes" "github.com/grafana/grafana/pkg/setting" ) @@ -23,7 +24,7 @@ func NewFakeStore(t *testing.T) *RemoteCache { dc, err := ProvideService(&setting.Cfg{ RemoteCacheOptions: opts, - }, sqlStore, fakes.NewFakeSecretsService()) + }, sqlStore, &usagestats.UsageStatsMock{}, fakes.NewFakeSecretsService()) require.NoError(t, err, "Failed to init remote cache for test") return dc diff --git a/pkg/infra/usagestats/statscollector/service.go b/pkg/infra/usagestats/statscollector/service.go index b14e1242374..feaf31bdd81 100644 --- a/pkg/infra/usagestats/statscollector/service.go +++ b/pkg/infra/usagestats/statscollector/service.go @@ -173,9 +173,6 @@ func (s *Service) collectSystemStats(ctx context.Context) (map[string]interface{ } m["stats.avg_auth_token_per_user.count"] = avgAuthTokensPerUser - if s.cfg.RemoteCacheOptions != nil && s.cfg.RemoteCacheOptions.Name != "" { - m["stats.remote_cache."+s.cfg.RemoteCacheOptions.Name+".count"] = 1 - } m["stats.packaging."+s.cfg.Packaging+".count"] = 1 m["stats.distributor."+s.cfg.ReportingDistributor+".count"] = 1 diff --git a/pkg/infra/usagestats/statscollector/service_test.go b/pkg/infra/usagestats/statscollector/service_test.go index 7f298de52e9..ca34d3c5496 100644 --- a/pkg/infra/usagestats/statscollector/service_test.go +++ b/pkg/infra/usagestats/statscollector/service_test.go @@ -182,7 +182,6 @@ func TestCollectingUsageStats(t *testing.T) { assert.EqualValues(t, 11, metrics["stats.data_keys.count"]) assert.EqualValues(t, 3, metrics["stats.active_data_keys.count"]) assert.EqualValues(t, 5, metrics["stats.public_dashboards.count"]) - assert.EqualValues(t, 1, metrics["stats.remote_cache.database.count"]) assert.InDelta(t, int64(65), metrics["stats.uptime"], 6) } diff --git a/pkg/services/contexthandler/auth_proxy_test.go b/pkg/services/contexthandler/auth_proxy_test.go index af9f1a13602..2a233b97b62 100644 --- a/pkg/services/contexthandler/auth_proxy_test.go +++ b/pkg/services/contexthandler/auth_proxy_test.go @@ -13,6 +13,7 @@ import ( "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/infra/remotecache" "github.com/grafana/grafana/pkg/infra/tracing" + "github.com/grafana/grafana/pkg/infra/usagestats" "github.com/grafana/grafana/pkg/services/anonymous/anontest" "github.com/grafana/grafana/pkg/services/auth/authtest" "github.com/grafana/grafana/pkg/services/auth/jwt" @@ -91,7 +92,7 @@ func getContextHandler(t *testing.T) *ContextHandler { cfg.AuthProxyHeaderName = "X-Killa" cfg.AuthProxyEnabled = true cfg.AuthProxyHeaderProperty = "username" - remoteCacheSvc, err := remotecache.ProvideService(cfg, sqlStore, fakes.NewFakeSecretsService()) + remoteCacheSvc, err := remotecache.ProvideService(cfg, sqlStore, &usagestats.UsageStatsMock{}, fakes.NewFakeSecretsService()) require.NoError(t, err) userAuthTokenSvc := authtest.NewFakeUserAuthTokenService() renderSvc := &fakeRenderService{}