apply PR feedback

Author: wbrowne

backend/app/instance_provider.go

@@ -16,10 +16,10 @@ type InstanceFactoryFunc func(ctx context.Context, settings backend.AppInstanceS
 // NewInstanceManager creates a new app instance manager.
 //
 // This is a helper method for calling NewInstanceProvider and creating a new instancemgmt.InstanceProvider,
-// and providing that to instancemgmt.NewContextAwareInstanceManager.
+// and providing that to instancemgmt.NewInstanceManagerWrapper.
 func NewInstanceManager(fn InstanceFactoryFunc) instancemgmt.InstanceManager {
 	ip := NewInstanceProvider(fn)
-	return instancemgmt.NewContextAwareInstanceManager(ip)
+	return instancemgmt.NewInstanceManagerWrapper(ip)
 }
 
 // NewInstanceProvider create a new app instance provider,

backend/datasource/instance_provider.go

@@ -30,7 +30,7 @@ type InstanceFactoryFunc func(ctx context.Context, settings backend.DataSourceIn
 // and providing that to instancemgmt.New.
 func NewInstanceManager(fn InstanceFactoryFunc) instancemgmt.InstanceManager {
 	ip := NewInstanceProvider(fn)
-	return instancemgmt.NewContextAwareInstanceManager(ip)
+	return instancemgmt.NewInstanceManagerWrapper(ip)
 }
 
 // NewInstanceProvider create a new data source instance provuder,

backend/instancemgmt/context_aware_manager.go

@@ -7,27 +7,27 @@ import (
 	"github.com/grafana/grafana-plugin-sdk-go/experimental/featuretoggles"
 )
 
-// NewContextAwareInstanceManager creates a new instance manager that dynamically selects
+// NewInstanceManagerWrapper creates a new instance manager that dynamically selects
 // between standard and TTL instance managers based on feature toggles from the Grafana config.
-func NewContextAwareInstanceManager(provider InstanceProvider) InstanceManager {
-	return &contextAwareInstanceManager{
+func NewInstanceManagerWrapper(provider InstanceProvider) InstanceManager {
+	return &instanceManagerWrapper{
 		provider:        provider,
 		standardManager: New(provider),
 		ttlManager:      NewTTLInstanceManager(provider),
 	}
 }
 
-// contextAwareInstanceManager is a wrapper that dynamically selects the appropriate
+// instanceManagerWrapper is a wrapper that dynamically selects the appropriate
 // instance manager implementation based on feature toggles in the context.
-type contextAwareInstanceManager struct {
+type instanceManagerWrapper struct {
 	provider        InstanceProvider
 	standardManager InstanceManager
 	ttlManager      InstanceManager
 }
 
 // selectManager returns the appropriate instance manager based on the feature toggle
 // from the Grafana config in the plugin context.
-func (c *contextAwareInstanceManager) selectManager(_ context.Context, pluginContext backend.PluginContext) InstanceManager {
+func (c *instanceManagerWrapper) selectManager(_ context.Context, pluginContext backend.PluginContext) InstanceManager {
 	// Check if TTL instance manager feature toggle is enabled
 	if pluginContext.GrafanaConfig != nil {
 		featureToggles := pluginContext.GrafanaConfig.FeatureToggles()
@@ -41,13 +41,13 @@ func (c *contextAwareInstanceManager) selectManager(_ context.Context, pluginCon
 }
 
 // Get returns an Instance using the appropriate manager based on feature toggles.
-func (c *contextAwareInstanceManager) Get(ctx context.Context, pluginContext backend.PluginContext) (Instance, error) {
+func (c *instanceManagerWrapper) Get(ctx context.Context, pluginContext backend.PluginContext) (Instance, error) {
 	manager := c.selectManager(ctx, pluginContext)
 	return manager.Get(ctx, pluginContext)
 }
 
 // Do provides an Instance as argument to fn using the appropriate manager based on feature toggles.
-func (c *contextAwareInstanceManager) Do(ctx context.Context, pluginContext backend.PluginContext, fn InstanceCallbackFunc) error {
+func (c *instanceManagerWrapper) Do(ctx context.Context, pluginContext backend.PluginContext, fn InstanceCallbackFunc) error {
 	manager := c.selectManager(ctx, pluginContext)
 	return manager.Do(ctx, pluginContext, fn)
 }

backend/instancemgmt/context_aware_manager_test.go

@@ -11,10 +11,10 @@ import (
 	"github.com/grafana/grafana-plugin-sdk-go/experimental/featuretoggles"
 )
 
-func TestContextAwareInstanceManager(t *testing.T) {
+func TestInstanceManagerWrapper(t *testing.T) {
 	ctx := context.Background()
 	tip := &testInstanceProvider{}
-	im := NewContextAwareInstanceManager(tip)
+	im := NewInstanceManagerWrapper(tip)
 
 	t.Run("Should use standard manager when feature toggle is disabled", func(t *testing.T) {
 		pCtx := backend.PluginContext{
@@ -27,7 +27,7 @@ func TestContextAwareInstanceManager(t *testing.T) {
 			}),
 		}
 
-		manager := im.(*contextAwareInstanceManager).selectManager(ctx, pCtx)
+		manager := im.(*instanceManagerWrapper).selectManager(ctx, pCtx)
 		require.IsType(t, &instanceManager{}, manager)
 	})
 
@@ -42,7 +42,7 @@ func TestContextAwareInstanceManager(t *testing.T) {
 			}),
 		}
 
-		manager := im.(*contextAwareInstanceManager).selectManager(ctx, pCtx)
+		manager := im.(*instanceManagerWrapper).selectManager(ctx, pCtx)
 		require.IsType(t, &instanceManagerWithTTL{}, manager)
 	})
 
@@ -55,7 +55,7 @@ func TestContextAwareInstanceManager(t *testing.T) {
 			GrafanaConfig: nil,
 		}
 
-		manager := im.(*contextAwareInstanceManager).selectManager(ctx, pCtx)
+		manager := im.(*instanceManagerWrapper).selectManager(ctx, pCtx)
 		require.IsType(t, &instanceManager{}, manager)
 	})
 
@@ -70,7 +70,7 @@ func TestContextAwareInstanceManager(t *testing.T) {
 			}),
 		}
 
-		manager := im.(*contextAwareInstanceManager).selectManager(ctx, pCtx)
+		manager := im.(*instanceManagerWrapper).selectManager(ctx, pCtx)
 		require.IsType(t, &instanceManagerWithTTL{}, manager)
 	})
 

backend/instancemgmt/doc.go

@@ -8,7 +8,7 @@
 //  2. TTL Instance Manager (NewTTLInstanceManager): Uses TTL-based caching
 //     that automatically evicts instances after a configurable time period.
 //
-//  3. Context-Aware Instance Manager (NewContextAwareInstanceManager):
+//  3. Instance Manager Wrapper (NewInstanceManagerWrapper):
 //     Dynamically selects between standard and TTL managers based on
 //     feature toggles from the Grafana config in the context.
 //

backend/instancemgmt/intance_manager_ttl.go

@@ -62,15 +62,17 @@ func (im *instanceManagerWithTTL) Get(ctx context.Context, pluginContext backend
 	}
 	// Double-checked locking for update/create criteria
 	cacheKey := fmt.Sprintf("%v", providerKey)
+	im.locker.RLock(cacheKey)
 	item, ok := im.cache.Get(cacheKey)
-
+	im.locker.RUnlock(cacheKey)
 	if ok {
 		ci := item.(CachedInstance)
 		needsUpdate := im.provider.NeedsUpdate(ctx, pluginContext, ci)
 
 		if !needsUpdate {
-			// SetDefault() creates a new cache entry with fresh TTL, effectively extending the instance's lifetime.
-			im.cache.SetDefault(cacheKey, ci)
+			im.locker.Lock(cacheKey)
+			im.refreshTTL(cacheKey, ci)
+			im.locker.Unlock(cacheKey)
 			return ci.instance, nil
 		}
 	}
@@ -83,8 +85,7 @@ func (im *instanceManagerWithTTL) Get(ctx context.Context, pluginContext backend
 		needsUpdate := im.provider.NeedsUpdate(ctx, pluginContext, ci)
 
 		if !needsUpdate {
-			// SetDefault() creates a new cache entry with fresh TTL, effectively extending the instance's lifetime.
-			im.cache.SetDefault(cacheKey, ci)
+			im.refreshTTL(cacheKey, ci)
 			return ci.instance, nil
 		}
 
@@ -117,3 +118,9 @@ func (im *instanceManagerWithTTL) Do(ctx context.Context, pluginContext backend.
 	callInstanceHandlerFunc(fn, instance)
 	return nil
 }
+
+// refreshTTL updates the TTL of the cached instance by resetting its expiration time.
+func (im *instanceManagerWithTTL) refreshTTL(cacheKey string, ci CachedInstance) {
+	// SetDefault() technically creates a new cache entry with fresh TTL, effectively extending the instance's lifetime.
+	im.cache.SetDefault(cacheKey, ci)
+}