From ddf766567bdf6fd054f97adee8436f5a755db312 Mon Sep 17 00:00:00 2001 From: Jo Date: Mon, 4 Nov 2024 17:35:31 +0100 Subject: [PATCH] RemoteCache: Refactor remote cache settings (#95672) * refactor remote cache settings * fix cache error getting treating as application error * fix cache error getting treating as application error --- pkg/infra/remotecache/memcached_storage.go | 2 +- .../memcached_storage_integration_test.go | 2 +- pkg/infra/remotecache/redis_storage.go | 2 +- .../redis_storage_integration_test.go | 2 +- pkg/infra/remotecache/remotecache.go | 2 +- pkg/infra/remotecache/remotecache_test.go | 6 ++--- pkg/infra/remotecache/testing.go | 2 +- .../usagestats/statscollector/service_test.go | 2 +- pkg/services/login/authinfoimpl/service.go | 4 ++-- pkg/setting/setting.go | 23 ++----------------- pkg/setting/setting_remote_cache.go | 23 +++++++++++++++++++ 11 files changed, 37 insertions(+), 33 deletions(-) create mode 100644 pkg/setting/setting_remote_cache.go diff --git a/pkg/infra/remotecache/memcached_storage.go b/pkg/infra/remotecache/memcached_storage.go index e39dd971c2f..d1b940ae2d6 100644 --- a/pkg/infra/remotecache/memcached_storage.go +++ b/pkg/infra/remotecache/memcached_storage.go @@ -18,7 +18,7 @@ type memcachedStorage struct { c *memcache.Client } -func newMemcachedStorage(opts *setting.RemoteCacheOptions) *memcachedStorage { +func newMemcachedStorage(opts *setting.RemoteCacheSettings) *memcachedStorage { return &memcachedStorage{ c: memcache.New(opts.ConnStr), } diff --git a/pkg/infra/remotecache/memcached_storage_integration_test.go b/pkg/infra/remotecache/memcached_storage_integration_test.go index e19b0b2e02c..800c240b99e 100644 --- a/pkg/infra/remotecache/memcached_storage_integration_test.go +++ b/pkg/infra/remotecache/memcached_storage_integration_test.go @@ -17,7 +17,7 @@ func TestIntegrationMemcachedCacheStorage(t *testing.T) { t.Skip("No Memcached hosts provided") } - opts := &setting.RemoteCacheOptions{Name: memcachedCacheType, ConnStr: u} + opts := &setting.RemoteCacheSettings{Name: memcachedCacheType, ConnStr: u} client := createTestClient(t, opts, nil) runTestsForClient(t, client) } diff --git a/pkg/infra/remotecache/redis_storage.go b/pkg/infra/remotecache/redis_storage.go index 6f51754396d..84c9a081f81 100644 --- a/pkg/infra/remotecache/redis_storage.go +++ b/pkg/infra/remotecache/redis_storage.go @@ -78,7 +78,7 @@ func parseRedisConnStr(connStr string) (*redis.Options, error) { return options, nil } -func newRedisStorage(opts *setting.RemoteCacheOptions) (*redisStorage, error) { +func newRedisStorage(opts *setting.RemoteCacheSettings) (*redisStorage, error) { opt, err := parseRedisConnStr(opts.ConnStr) if err != nil { return nil, err diff --git a/pkg/infra/remotecache/redis_storage_integration_test.go b/pkg/infra/remotecache/redis_storage_integration_test.go index c6dd9ae5133..9d724884abe 100644 --- a/pkg/infra/remotecache/redis_storage_integration_test.go +++ b/pkg/infra/remotecache/redis_storage_integration_test.go @@ -34,7 +34,7 @@ func TestIntegrationRedisCacheStorage(t *testing.T) { b.WriteString(fmt.Sprintf(",db=%d", db)) } - opts := &setting.RemoteCacheOptions{Name: redisCacheType, ConnStr: b.String()} + opts := &setting.RemoteCacheSettings{Name: redisCacheType, ConnStr: b.String()} client := createTestClient(t, opts, nil) runTestsForClient(t, client) } diff --git a/pkg/infra/remotecache/remotecache.go b/pkg/infra/remotecache/remotecache.go index aa7ba2073f7..65f9fad61e1 100644 --- a/pkg/infra/remotecache/remotecache.go +++ b/pkg/infra/remotecache/remotecache.go @@ -109,7 +109,7 @@ func (ds *RemoteCache) Run(ctx context.Context) error { return ctx.Err() } -func createClient(opts *setting.RemoteCacheOptions, sqlstore db.DB, secretsService secrets.Service) (cache CacheStorage, err error) { +func createClient(opts *setting.RemoteCacheSettings, sqlstore db.DB, secretsService secrets.Service) (cache CacheStorage, err error) { switch opts.Name { case redisCacheType: cache, err = newRedisStorage(opts) diff --git a/pkg/infra/remotecache/remotecache_test.go b/pkg/infra/remotecache/remotecache_test.go index 7b7a64e8d38..239fc7b304c 100644 --- a/pkg/infra/remotecache/remotecache_test.go +++ b/pkg/infra/remotecache/remotecache_test.go @@ -21,7 +21,7 @@ func TestMain(m *testing.M) { testsuite.Run(m) } -func createTestClient(t *testing.T, opts *setting.RemoteCacheOptions, sqlstore db.DB) CacheStorage { +func createTestClient(t *testing.T, opts *setting.RemoteCacheSettings, sqlstore db.DB) CacheStorage { t.Helper() cfg := &setting.Cfg{ @@ -45,7 +45,7 @@ func TestCachedBasedOnConfig(t *testing.T) { } func TestInvalidCacheTypeReturnsError(t *testing.T) { - _, err := createClient(&setting.RemoteCacheOptions{Name: "invalid"}, nil, nil) + _, err := createClient(&setting.RemoteCacheSettings{Name: "invalid"}, nil, nil) assert.Equal(t, err, ErrInvalidCacheType) } @@ -94,7 +94,7 @@ func TestCollectUsageStats(t *testing.T) { "stats.remote_cache.encrypt_enabled.count": 1, } cfg := setting.NewCfg() - cfg.RemoteCacheOptions = &setting.RemoteCacheOptions{Name: redisCacheType, Encryption: true} + cfg.RemoteCacheOptions = &setting.RemoteCacheSettings{Name: redisCacheType, Encryption: true} remoteCache := &RemoteCache{ Cfg: cfg, diff --git a/pkg/infra/remotecache/testing.go b/pkg/infra/remotecache/testing.go index e65c9054753..898a8ffc3ee 100644 --- a/pkg/infra/remotecache/testing.go +++ b/pkg/infra/remotecache/testing.go @@ -15,7 +15,7 @@ import ( func NewFakeStore(t *testing.T) *RemoteCache { t.Helper() - opts := &setting.RemoteCacheOptions{ + opts := &setting.RemoteCacheSettings{ Name: "database", ConnStr: "", } diff --git a/pkg/infra/usagestats/statscollector/service_test.go b/pkg/infra/usagestats/statscollector/service_test.go index 600f39afac2..00d6ad621d0 100644 --- a/pkg/infra/usagestats/statscollector/service_test.go +++ b/pkg/infra/usagestats/statscollector/service_test.go @@ -145,7 +145,7 @@ func TestCollectingUsageStats(t *testing.T) { AuthProxy: setting.AuthProxySettings{Enabled: true}, Packaging: "deb", ReportingDistributor: "hosted-grafana", - RemoteCacheOptions: &setting.RemoteCacheOptions{ + RemoteCacheOptions: &setting.RemoteCacheSettings{ Name: "database", }, }, sqlStore, statsService, diff --git a/pkg/services/login/authinfoimpl/service.go b/pkg/services/login/authinfoimpl/service.go index b153b3fb58a..1d7736c2c54 100644 --- a/pkg/services/login/authinfoimpl/service.go +++ b/pkg/services/login/authinfoimpl/service.go @@ -47,7 +47,7 @@ func (s *Service) GetAuthInfo(ctx context.Context, query *login.GetAuthInfoQuery authInfo, err := s.getAuthInfoFromCache(ctx, query) if err != nil && !errors.Is(err, remotecache.ErrCacheItemNotFound) { - s.logger.Error("failed to retrieve auth info from cache", "error", err) + s.logger.Warn("failed to retrieve auth info from cache", "error", err) } else if authInfo != nil { return authInfo, nil } @@ -59,7 +59,7 @@ func (s *Service) GetAuthInfo(ctx context.Context, query *login.GetAuthInfoQuery err = s.setAuthInfoInCache(ctx, query, authInfo) if err != nil { - s.logger.Error("failed to set auth info in cache", "error", err) + s.logger.Warn("failed to set auth info in cache", "error", err) } else { s.logger.Debug("auth info set in cache", "cacheKey", generateCacheKey(query)) } diff --git a/pkg/setting/setting.go b/pkg/setting/setting.go index 558da79f409..e0a54e7ab6f 100644 --- a/pkg/setting/setting.go +++ b/pkg/setting/setting.go @@ -291,7 +291,7 @@ type Cfg struct { DataProxyUserAgent string // DistributedCache - RemoteCacheOptions *RemoteCacheOptions + RemoteCacheOptions *RemoteCacheSettings ViewersCanEdit bool EditorsCanAdmin bool @@ -1296,19 +1296,6 @@ func (cfg *Cfg) parseINIFile(iniFile *ini.File) error { enterprise := iniFile.Section("enterprise") cfg.EnterpriseLicensePath = valueAsString(enterprise, "license_path", filepath.Join(cfg.DataPath, "license.jwt")) - cacheServer := iniFile.Section("remote_cache") - dbName := valueAsString(cacheServer, "type", "database") - connStr := valueAsString(cacheServer, "connstr", "") - prefix := valueAsString(cacheServer, "prefix", "") - encryption := cacheServer.Key("encryption").MustBool(false) - - cfg.RemoteCacheOptions = &RemoteCacheOptions{ - Name: dbName, - ConnStr: connStr, - Prefix: prefix, - Encryption: encryption, - } - geomapSection := iniFile.Section("geomap") basemapJSON := valueAsString(geomapSection, "default_baselayer_config", "") if basemapJSON != "" { @@ -1322,6 +1309,7 @@ func (cfg *Cfg) parseINIFile(iniFile *ini.File) error { } cfg.GeomapEnableCustomBaseLayers = geomapSection.Key("enable_custom_baselayers").MustBool(true) + cfg.readRemoteCacheSettings() cfg.readDateFormats() cfg.readGrafanaJavascriptAgentConfig() @@ -1354,13 +1342,6 @@ func valueAsString(section *ini.Section, keyName string, defaultValue string) st return section.Key(keyName).MustString(defaultValue) } -type RemoteCacheOptions struct { - Name string - ConnStr string - Prefix string - Encryption bool -} - func (cfg *Cfg) readSAMLConfig() { samlSec := cfg.Raw.Section("auth.saml") cfg.SAMLAuthEnabled = samlSec.Key("enabled").MustBool(false) diff --git a/pkg/setting/setting_remote_cache.go b/pkg/setting/setting_remote_cache.go new file mode 100644 index 00000000000..87ef5ec9f6f --- /dev/null +++ b/pkg/setting/setting_remote_cache.go @@ -0,0 +1,23 @@ +package setting + +type RemoteCacheSettings struct { + Name string + ConnStr string + Prefix string + Encryption bool +} + +func (cfg *Cfg) readRemoteCacheSettings() { + cacheServer := cfg.Raw.Section("remote_cache") + dbName := valueAsString(cacheServer, "type", "database") + connStr := valueAsString(cacheServer, "connstr", "") + prefix := valueAsString(cacheServer, "prefix", "") + encryption := cacheServer.Key("encryption").MustBool(false) + + cfg.RemoteCacheOptions = &RemoteCacheSettings{ + Name: dbName, + ConnStr: connStr, + Prefix: prefix, + Encryption: encryption, + } +}