From 013fe5c37f8fbfaad06060c5cc20f9b27be859b2 Mon Sep 17 00:00:00 2001 From: Alexander Zobnin Date: Fri, 28 Aug 2020 14:40:39 +0300 Subject: [PATCH] Refactor: use InstanceManager for managing ds instances --- pkg/datasource/datasource.go | 61 ++++++--------- pkg/datasource/datasource_cache.go | 11 --- pkg/datasource/datasource_cache_test.go | 100 ------------------------ pkg/datasource/datasource_test.go | 47 ++++------- pkg/datasource/resource_handler.go | 2 +- 5 files changed, 39 insertions(+), 182 deletions(-) delete mode 100644 pkg/datasource/datasource_cache_test.go diff --git a/pkg/datasource/datasource.go b/pkg/datasource/datasource.go index f277f35..b104c85 100644 --- a/pkg/datasource/datasource.go +++ b/pkg/datasource/datasource.go @@ -4,14 +4,14 @@ import ( "context" "encoding/json" "errors" - "fmt" "time" - "github.com/alexanderzobnin/grafana-zabbix/pkg/cache" "github.com/alexanderzobnin/grafana-zabbix/pkg/gtime" "github.com/alexanderzobnin/grafana-zabbix/pkg/zabbixapi" "github.com/grafana/grafana-plugin-sdk-go/backend" + "github.com/grafana/grafana-plugin-sdk-go/backend/datasource" + "github.com/grafana/grafana-plugin-sdk-go/backend/instancemgmt" "github.com/grafana/grafana-plugin-sdk-go/backend/log" "github.com/grafana/grafana-plugin-sdk-go/data" ) @@ -22,8 +22,8 @@ var ( ) type ZabbixDatasource struct { - datasourceCache *cache.Cache - logger log.Logger + im instancemgmt.InstanceManager + logger log.Logger } // ZabbixDatasourceInstance stores state about a specific datasource @@ -37,30 +37,36 @@ type ZabbixDatasourceInstance struct { } func NewZabbixDatasource() *ZabbixDatasource { + im := datasource.NewInstanceManager(newZabbixDatasourceInstance) return &ZabbixDatasource{ - datasourceCache: cache.NewCache(10*time.Minute, 10*time.Minute), - logger: log.New(), + im: im, + logger: log.New(), } } -// NewZabbixDatasourceInstance returns an initialized zabbix datasource instance -func NewZabbixDatasourceInstance(dsInfo *backend.DataSourceInstanceSettings) (*ZabbixDatasourceInstance, error) { - zabbixAPI, err := zabbixapi.New(dsInfo.URL) +// newZabbixDatasourceInstance returns an initialized zabbix datasource instance +func newZabbixDatasourceInstance(settings backend.DataSourceInstanceSettings) (instancemgmt.Instance, error) { + logger := log.New() + logger.Debug("Initializing new data source instance") + + zabbixAPI, err := zabbixapi.New(settings.URL) if err != nil { + logger.Error("Error initializing Zabbix API", "error", err) return nil, err } - zabbixSettings, err := readZabbixSettings(dsInfo) + zabbixSettings, err := readZabbixSettings(&settings) if err != nil { + logger.Error("Error parsing Zabbix settings", "error", err) return nil, err } return &ZabbixDatasourceInstance{ - dsInfo: dsInfo, + dsInfo: &settings, zabbixAPI: zabbixAPI, Settings: zabbixSettings, queryCache: NewDatasourceCache(zabbixSettings.CacheTTL, 10*time.Minute), - logger: log.New(), + logger: logger, }, nil } @@ -68,7 +74,7 @@ func NewZabbixDatasourceInstance(dsInfo *backend.DataSourceInstanceSettings) (*Z func (ds *ZabbixDatasource) CheckHealth(ctx context.Context, req *backend.CheckHealthRequest) (*backend.CheckHealthResult, error) { res := &backend.CheckHealthResult{} - dsInstance, err := ds.GetDatasource(req.PluginContext) + dsInstance, err := ds.getDSInstance(req.PluginContext) if err != nil { res.Status = backend.HealthStatusError res.Message = "Error getting datasource instance" @@ -92,7 +98,7 @@ func (ds *ZabbixDatasource) CheckHealth(ctx context.Context, req *backend.CheckH func (ds *ZabbixDatasource) QueryData(ctx context.Context, req *backend.QueryDataRequest) (*backend.QueryDataResponse, error) { qdr := backend.NewQueryDataResponse() - zabbixDS, err := ds.GetDatasource(req.PluginContext) + zabbixDS, err := ds.getDSInstance(req.PluginContext) if err != nil { return nil, err } @@ -121,32 +127,13 @@ func (ds *ZabbixDatasource) QueryData(ctx context.Context, req *backend.QueryDat return qdr, nil } -// GetDatasource Returns cached datasource or creates new one -func (ds *ZabbixDatasource) GetDatasource(pluginContext backend.PluginContext) (*ZabbixDatasourceInstance, error) { - dsSettings := pluginContext.DataSourceInstanceSettings - dsKey := fmt.Sprintf("%d-%d", pluginContext.OrgID, dsSettings.ID) - // Get hash to check if settings changed - dsInfoHash := HashDatasourceInfo(dsSettings) - - if cachedData, ok := ds.datasourceCache.Get(dsKey); ok { - if cachedDS, ok := cachedData.(*ZabbixDatasourceInstance); ok { - cachedDSHash := HashDatasourceInfo(cachedDS.dsInfo) - if cachedDSHash == dsInfoHash { - return cachedDS, nil - } - ds.logger.Debug("Data source settings changed", "org", pluginContext.OrgID, "id", dsSettings.ID, "name", dsSettings.Name) - } - } - - ds.logger.Debug("Initializing data source", "org", pluginContext.OrgID, "id", dsSettings.ID, "name", dsSettings.Name) - dsInstance, err := NewZabbixDatasourceInstance(pluginContext.DataSourceInstanceSettings) +// getDSInstance Returns cached datasource or creates new one +func (ds *ZabbixDatasource) getDSInstance(pluginContext backend.PluginContext) (*ZabbixDatasourceInstance, error) { + instance, err := ds.im.Get(pluginContext) if err != nil { - ds.logger.Error("Error initializing datasource", "error", err) return nil, err } - - ds.datasourceCache.Set(dsKey, dsInstance) - return dsInstance, nil + return instance.(*ZabbixDatasourceInstance), nil } func readZabbixSettings(dsInstanceSettings *backend.DataSourceInstanceSettings) (*ZabbixDatasourceSettings, error) { diff --git a/pkg/datasource/datasource_cache.go b/pkg/datasource/datasource_cache.go index 1e17285..86567c4 100644 --- a/pkg/datasource/datasource_cache.go +++ b/pkg/datasource/datasource_cache.go @@ -3,11 +3,9 @@ package datasource import ( "crypto/sha1" "encoding/hex" - "encoding/json" "time" "github.com/alexanderzobnin/grafana-zabbix/pkg/cache" - "github.com/grafana/grafana-plugin-sdk-go/backend" ) // DatasourceCache is a cache for datasource instance. @@ -40,12 +38,3 @@ func HashString(text string) string { hash.Write([]byte(text)) return hex.EncodeToString(hash.Sum(nil)) } - -// HashDatasourceInfo converts the given datasource info to hash string -func HashDatasourceInfo(dsInfo *backend.DataSourceInstanceSettings) string { - digester := sha1.New() - if err := json.NewEncoder(digester).Encode(dsInfo); err != nil { - panic(err) // This shouldn't be possible but just in case DatasourceInfo changes - } - return hex.EncodeToString(digester.Sum(nil)) -} diff --git a/pkg/datasource/datasource_cache_test.go b/pkg/datasource/datasource_cache_test.go deleted file mode 100644 index dfdd37a..0000000 --- a/pkg/datasource/datasource_cache_test.go +++ /dev/null @@ -1,100 +0,0 @@ -package datasource - -import ( - "testing" - - "github.com/grafana/grafana-plugin-sdk-go/backend" - "gotest.tools/assert" -) - -func TestHashDatasourceInfo(t *testing.T) { - tests := []struct { - name string - dsInfoBefore *backend.DataSourceInstanceSettings - dsInfoAfter *backend.DataSourceInstanceSettings - equal bool - }{ - { - name: "Same datasource settings", - dsInfoBefore: &backend.DataSourceInstanceSettings{ - ID: 1, - Name: "Zabbix", - URL: "https://localhost:3306/zabbix/api_jsonrpc.php", - JSONData: []byte("{}"), - DecryptedSecureJSONData: map[string]string{ - "username": "grafanaZabbixUser", - "password": "$uper$ecr3t!!!", - }, - }, - dsInfoAfter: &backend.DataSourceInstanceSettings{ - ID: 1, - Name: "Zabbix", - URL: "https://localhost:3306/zabbix/api_jsonrpc.php", - JSONData: []byte("{}"), - DecryptedSecureJSONData: map[string]string{ - "username": "grafanaZabbixUser", - "password": "$uper$ecr3t!!!", - }, - }, - equal: true, - }, - { - name: "Password changed", - dsInfoBefore: &backend.DataSourceInstanceSettings{ - ID: 1, - Name: "Zabbix", - URL: "https://localhost:3306/zabbix/api_jsonrpc.php", - JSONData: []byte("{}"), - DecryptedSecureJSONData: map[string]string{ - "username": "grafanaZabbixUser", - "password": "$uper$ecr3t!!!", - }, - }, - dsInfoAfter: &backend.DataSourceInstanceSettings{ - ID: 1, - Name: "Zabbix", - URL: "https://localhost:3306/zabbix/api_jsonrpc.php", - JSONData: []byte("{}"), - DecryptedSecureJSONData: map[string]string{ - "username": "grafanaZabbixUser", - "password": "new$uper$ecr3t!!!", - }, - }, - equal: false, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - before := HashDatasourceInfo(tt.dsInfoBefore) - after := HashDatasourceInfo(tt.dsInfoAfter) - got := before == after - assert.Equal(t, tt.equal, got) - }) - } -} - -func BenchmarkHashDatasourceInfo(b *testing.B) { - benches := []struct { - name string - dsInfo *backend.DataSourceInstanceSettings - }{ - { - "Normal Datasource Info", - &backend.DataSourceInstanceSettings{ - ID: 4, - Name: "MyZabbixDatasource", - URL: "https://localhost:3306/zabbix/api_jsonrpc.php", - JSONData: []byte(`{ "addThresholds": true, "disableReadOnlyUsersAck": true }`), - DecryptedSecureJSONData: map[string]string{ - "username": "grafanaZabbixUser", - "password": "$uper$ecr3t!!!", - }, - }, - }, - } - for _, bt := range benches { - b.Run(bt.name, func(b *testing.B) { - HashDatasourceInfo(bt.dsInfo) - }) - } -} diff --git a/pkg/datasource/datasource_test.go b/pkg/datasource/datasource_test.go index 48d4e3b..2046730 100644 --- a/pkg/datasource/datasource_test.go +++ b/pkg/datasource/datasource_test.go @@ -1,38 +1,32 @@ package datasource import ( - "fmt" "testing" - "github.com/alexanderzobnin/grafana-zabbix/pkg/cache" "github.com/grafana/grafana-plugin-sdk-go/backend" - "github.com/grafana/grafana-plugin-sdk-go/backend/log" "gotest.tools/assert" ) func TestZabbixBackend_getCachedDatasource(t *testing.T) { - basicDsSettings := &backend.DataSourceInstanceSettings{ + basicDsSettings := backend.DataSourceInstanceSettings{ ID: 1, Name: "TestDatasource", URL: "http://zabbix.org/zabbix", JSONData: []byte("{}"), } - modifiedDatasourceSettings := &backend.DataSourceInstanceSettings{ + modifiedDatasourceSettings := backend.DataSourceInstanceSettings{ ID: 1, Name: "TestDatasource", URL: "http://another.zabbix.org/zabbix", JSONData: []byte("{}"), } - modifiedDatasource, _ := NewZabbixDatasourceInstance(modifiedDatasourceSettings) + modifiedDatasource, _ := newZabbixDatasourceInstance(modifiedDatasourceSettings) - basicDS, _ := NewZabbixDatasourceInstance(basicDsSettings) - dsCache := cache.NewCache(cache.NoExpiration, cache.NoExpiration) - dsCache.Set("1-1", basicDS) + basicDS, _ := newZabbixDatasourceInstance(basicDsSettings) tests := []struct { name string - cache *cache.Cache pluginContext backend.PluginContext want *ZabbixDatasourceInstance }{ @@ -40,47 +34,34 @@ func TestZabbixBackend_getCachedDatasource(t *testing.T) { name: "Uncached Datasource (nothing in cache)", pluginContext: backend.PluginContext{ OrgID: 1, - DataSourceInstanceSettings: basicDsSettings, + DataSourceInstanceSettings: &basicDsSettings, }, - want: basicDS, + want: basicDS.(*ZabbixDatasourceInstance), }, { - name: "Cached Datasource", - cache: dsCache, + name: "Cached Datasource", pluginContext: backend.PluginContext{ OrgID: 1, - DataSourceInstanceSettings: basicDsSettings, + DataSourceInstanceSettings: &basicDsSettings, }, - want: basicDS, + want: basicDS.(*ZabbixDatasourceInstance), }, { - name: "Cached then modified", - cache: dsCache, + name: "Cached then modified", pluginContext: backend.PluginContext{ OrgID: 1, - DataSourceInstanceSettings: modifiedDatasourceSettings, + DataSourceInstanceSettings: &modifiedDatasourceSettings, }, - want: modifiedDatasource, + want: modifiedDatasource.(*ZabbixDatasourceInstance), }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if tt.cache == nil { - tt.cache = cache.NewCache(cache.NoExpiration, cache.NoExpiration) - } - ds := &ZabbixDatasource{ - datasourceCache: tt.cache, - logger: log.New(), - } - got, _ := ds.GetDatasource(tt.pluginContext) + ds := NewZabbixDatasource() + got, _ := ds.getDSInstance(tt.pluginContext) // Only checking the URL, being the easiest value to, and guarantee equality for assert.Equal(t, tt.want.zabbixAPI.GetUrl().String(), got.zabbixAPI.GetUrl().String()) - - // Ensure the datasource is in the cache - cacheds, ok := tt.cache.Get(fmt.Sprint("1-", tt.pluginContext.DataSourceInstanceSettings.ID)) - assert.Equal(t, true, ok) - assert.Equal(t, got, cacheds) }) } } diff --git a/pkg/datasource/resource_handler.go b/pkg/datasource/resource_handler.go index 3cb20dc..81f5ef4 100644 --- a/pkg/datasource/resource_handler.go +++ b/pkg/datasource/resource_handler.go @@ -40,7 +40,7 @@ func (ds *ZabbixDatasource) ZabbixAPIHandler(rw http.ResponseWriter, req *http.R } pluginCxt := httpadapter.PluginConfigFromContext(req.Context()) - dsInstance, err := ds.GetDatasource(pluginCxt) + dsInstance, err := ds.getDSInstance(pluginCxt) if err != nil { ds.logger.Error("Error loading datasource", "error", err) writeError(rw, http.StatusInternalServerError, err)