Plugins: Enable feature toggle pluginsDynamicAngularDetectionPatterns by default (#84723)

* Enable feature toggle pluginsDynamicAngularDetectionPatterns by default * Change backgroundJobInterval to 24h * re-generate feature toggles * Use different intervals for cloud vs on-prem * temporarily switch interval log to info level * debug level again * Simplify provideDynamic for tests * re-generated feature toggles files * PR review feedback * PR review feedback: removed dependency from plugin management config
2025-02-25 18:55:37 -06:00 · 2024-03-21 17:22:06 +01:00
parent 6c1de260a2
commit eb31f71f60
7 changed files with 106 additions and 66 deletions
--- a/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md
+++ b/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md
@@ -20,7 +20,7 @@ This page contains a list of available feature toggles. To learn how to turn on
 Some features are enabled by default. You can disable these feature by setting the feature flag to "false" in the configuration.

 | Feature toggle name                      | Description                                                                                                                                                                                                                  | Enabled by default |
-| ------------------------------------ | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------ |
+| ---------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------ |
 | `disableEnvelopeEncryption`              | Disable envelope encryption (emergency only)                                                                                                                                                                                 |                    |
 | `publicDashboards`                       | [Deprecated] Public dashboards are now enabled by default; to disable them, use the configuration setting. This feature toggle will be removed in the next major version.                                                    | Yes                |
 | `featureHighlights`                      | Highlight Grafana Enterprise features                                                                                                                                                                                        |                    |
@@ -37,6 +37,7 @@ Some features are enabled by default. You can disable these feature by setting t
 | `dataplaneFrontendFallback`              | Support dataplane contract field name change for transformations and field name matchers where the name is different                                                                                                         | Yes                |
 | `enableElasticsearchBackendQuerying`     | Enable the processing of queries and responses in the Elasticsearch data source through backend                                                                                                                              | Yes                |
 | `recordedQueriesMulti`                   | Enables writing multiple items from a single query within Recorded Queries                                                                                                                                                   | Yes                |
+| `pluginsDynamicAngularDetectionPatterns` | Enables fetching Angular detection patterns for plugins from GCOM and fallback to hardcoded ones                                                                                                                             | Yes                |
 | `logsExploreTableVisualisation`          | A table visualisation for logs in Explore                                                                                                                                                                                    | Yes                |
 | `transformationsRedesign`                | Enables the transformations redesign                                                                                                                                                                                         | Yes                |
 | `traceQLStreaming`                       | Enables response streaming of TraceQL queries of the Tempo data source                                                                                                                                                       |                    |
@@ -130,7 +131,6 @@ Experimental features might be changed or removed without prior notice.
 | `dashboardEmbed`                            | Allow embedding dashboard for external use in Code editors                                                                                                                                                                                                                        |
 | `frontendSandboxMonitorOnly`                | Enables monitor only in the plugin frontend sandbox (if enabled)                                                                                                                                                                                                                  |
 | `lokiFormatQuery`                           | Enables the ability to format Loki queries                                                                                                                                                                                                                                        |
-| `pluginsDynamicAngularDetectionPatterns`    | Enables fetching Angular detection patterns for plugins from GCOM and fallback to hardcoded ones                                                                                                                                                                                  |
 | `vizAndWidgetSplit`                         | Split panels between visualizations and widgets                                                                                                                                                                                                                                   |
 | `prometheusIncrementalQueryInstrumentation` | Adds RudderStack events to incremental queries                                                                                                                                                                                                                                    |
 | `awsDatasourcesTempCredentials`             | Support temporary security credentials in AWS plugins for Grafana Cloud customers                                                                                                                                                                                                 |
--- a/pkg/services/featuremgmt/registry.go
+++ b/pkg/services/featuremgmt/registry.go
@@ -510,9 +510,10 @@ var (
 		{
 			Name:         "pluginsDynamicAngularDetectionPatterns",
 			Description:  "Enables fetching Angular detection patterns for plugins from GCOM and fallback to hardcoded ones",
-			Stage:        FeatureStageExperimental,
+			Stage:        FeatureStageGeneralAvailability,
 			FrontendOnly: false,
 			Owner:        grafanaPluginsPlatformSquad,
+			Expression:   "true", // enabled by default
 		},
 		{
 			Name:         "vizAndWidgetSplit",
--- a/pkg/services/featuremgmt/toggles_gen.csv
+++ b/pkg/services/featuremgmt/toggles_gen.csv
@@ -67,7 +67,7 @@ frontendSandboxMonitorOnly,experimental,@grafana/plugins-platform-backend,false,
 sqlDatasourceDatabaseSelection,preview,@grafana/dataviz-squad,false,false,true
 lokiFormatQuery,experimental,@grafana/observability-logs,false,false,true
 recordedQueriesMulti,GA,@grafana/observability-metrics,false,false,false
-pluginsDynamicAngularDetectionPatterns,experimental,@grafana/plugins-platform-backend,false,false,false
+pluginsDynamicAngularDetectionPatterns,GA,@grafana/plugins-platform-backend,false,false,false
 vizAndWidgetSplit,experimental,@grafana/dashboards-squad,false,false,true
 prometheusIncrementalQueryInstrumentation,experimental,@grafana/observability-metrics,false,false,true
 logsExploreTableVisualisation,GA,@grafana/observability-logs,false,false,true
--- a/pkg/services/featuremgmt/toggles_gen.json
+++ b/pkg/services/featuremgmt/toggles_gen.json
@@ -441,12 +441,15 @@
    {
      "metadata": {
        "name": "pluginsDynamicAngularDetectionPatterns",
-        "resourceVersion": "1709648236447",
-        "creationTimestamp": "2024-03-05T14:17:16Z"
+        "resourceVersion": "1710847676897",
+        "creationTimestamp": "2024-03-05T14:17:16Z",
+        "annotations": {
+          "grafana.app/updatedTimestamp": "2024-03-19 11:27:56.897962695 +0000 UTC"
+        }
      },
      "spec": {
        "description": "Enables fetching Angular detection patterns for plugins from GCOM and fallback to hardcoded ones",
-        "stage": "experimental",
+        "stage": "GA",
        "codeowner": "@grafana/plugins-platform-backend"
      }
    },
--- a/pkg/services/pluginsintegration/angulardetectorsprovider/dynamic.go
+++ b/pkg/services/pluginsintegration/angulardetectorsprovider/dynamic.go
@@ -12,18 +12,21 @@ import (
 	"sync"
 	"time"

-	"github.com/grafana/grafana/pkg/plugins/config"
 	"github.com/grafana/grafana/pkg/plugins/log"
 	"github.com/grafana/grafana/pkg/plugins/manager/loader/angular/angulardetector"
 	"github.com/grafana/grafana/pkg/services/featuremgmt"
 	"github.com/grafana/grafana/pkg/services/pluginsintegration/angularpatternsstore"
+	"github.com/grafana/grafana/pkg/setting"
 )

 var errNotModified = errors.New("not modified")

-// backgroundJobInterval is the interval that passes between background job runs.
-// It can be overwritten in tests.
-var backgroundJobInterval = time.Hour * 1
+// Intervals that passes between background job runs.
+// Can be overwritten in tests.
+var (
+	backgroundJobIntervalOnPrem = time.Hour * 24
+	backgroundJobIntervalCloud  = time.Hour * 1
+)

 // Dynamic is an angulardetector.DetectorsProvider that calls GCOM to get Angular detection patterns,
 // converts them to detectors and caches them for all future calls.
@@ -45,20 +48,32 @@ type Dynamic struct {

 	// mux is the mutex used to read/write the cached detectors in a concurrency-safe way.
 	mux sync.RWMutex
+
+	// backgroundJobInterval is the interval that passes between background job runs.
+	backgroundJobInterval time.Duration
 }

-func ProvideDynamic(cfg *config.PluginManagementCfg, store angularpatternsstore.Service, features featuremgmt.FeatureToggles) (*Dynamic, error) {
+func ProvideDynamic(cfg *setting.Cfg, store angularpatternsstore.Service, features featuremgmt.FeatureToggles) (*Dynamic, error) {
+	backgroundJobInterval := backgroundJobIntervalOnPrem
+	if cfg.StackID != "" {
+		// Use a shorter interval for cloud.
+		// (in cloud, cfg.StackID is always set).
+		backgroundJobInterval = backgroundJobIntervalCloud
+	}
+
 	d := &Dynamic{
 		log:                   log.New("plugin.angulardetectorsprovider.dynamic"),
 		features:              features,
 		store:                 store,
 		httpClient:            makeHttpClient(),
 		baseURL:               cfg.GrafanaComURL,
+		backgroundJobInterval: backgroundJobInterval,
 	}
 	if d.IsDisabled() {
 		// Do not attempt to restore if the background service is disabled (no feature flag)
 		return d, nil
 	}
+	d.log.Debug("Providing dynamic angular detection patterns", "baseURL", d.baseURL, "interval", d.backgroundJobInterval)

 	// Perform the initial restore from db
 	st := time.Now()
@@ -224,7 +239,7 @@ func (d *Dynamic) IsDisabled() bool {
 }

 // randomSkew returns a random time.Duration between 0 and maxSkew.
-// This can be added to backgroundJobInterval to skew it by a random amount.
+// This can be added to d.backgroundJobInterval to skew it by a random amount.
 func (d *Dynamic) randomSkew(maxSkew time.Duration) time.Duration {
 	return time.Duration(rand.Float64() * float64(maxSkew))
 }
@@ -242,15 +257,15 @@ func (d *Dynamic) Run(ctx context.Context) error {
 	// Offset the background job interval a bit to skew GCOM calls from all instances,
 	// so GCOM is not overwhelmed with lots of requests all at the same time.
 	// Important when lots of HG instances restart at the same time.
-	skew := d.randomSkew(backgroundJobInterval / 4)
-	backgroundJobInterval += skew
+	skew := d.randomSkew(d.backgroundJobInterval / 4)
+	d.backgroundJobInterval += skew
 	d.log.Debug(
 		"Applied background job skew",
-		"skew", backgroundJobInterval, "interval", backgroundJobInterval,
+		"skew", d.backgroundJobInterval, "interval", d.backgroundJobInterval,
 	)

-	nextRunUntil := time.Until(lastUpdate.Add(backgroundJobInterval))
-	ticker := time.NewTicker(backgroundJobInterval)
+	nextRunUntil := time.Until(lastUpdate.Add(d.backgroundJobInterval))
+	ticker := time.NewTicker(d.backgroundJobInterval)
 	defer ticker.Stop()

 	var tick <-chan time.Time
@@ -286,7 +301,7 @@ func (d *Dynamic) Run(ctx context.Context) error {
 			d.log.Info("Patterns update finished", "duration", time.Since(st))

 			// Restore default ticker if we run with a shorter interval the first time
-			ticker.Reset(backgroundJobInterval)
+			ticker.Reset(d.backgroundJobInterval)
 			tick = ticker.C
 		case <-ctx.Done():
 			return ctx.Err()
--- a/pkg/services/pluginsintegration/angulardetectorsprovider/dynamic_test.go
+++ b/pkg/services/pluginsintegration/angulardetectorsprovider/dynamic_test.go
@@ -14,10 +14,10 @@ import (
 	"github.com/stretchr/testify/require"

 	"github.com/grafana/grafana/pkg/infra/kvstore"
-	"github.com/grafana/grafana/pkg/plugins/config"
 	"github.com/grafana/grafana/pkg/plugins/manager/loader/angular/angulardetector"
 	"github.com/grafana/grafana/pkg/services/featuremgmt"
 	"github.com/grafana/grafana/pkg/services/pluginsintegration/angularpatternsstore"
+	"github.com/grafana/grafana/pkg/setting"
 )

 func TestDynamicAngularDetectorsProvider(t *testing.T) {
@@ -314,6 +314,22 @@ func TestDynamicAngularDetectorsProvider(t *testing.T) {
 	})
 }

+func TestDynamicAngularDetectorsProviderCloudVsOnPrem(t *testing.T) {
+	gcom := newDefaultGCOMScenario()
+	srv := gcom.newHTTPTestServer()
+	t.Cleanup(srv.Close)
+
+	t.Run("should use cloud interval if stack_id is set", func(t *testing.T) {
+		svc := provideDynamic(t, srv.URL, provideDynamicOpts{cfg: &setting.Cfg{StackID: "1234"}})
+		require.Equal(t, backgroundJobIntervalCloud, svc.backgroundJobInterval)
+	})
+
+	t.Run("should use on-prem interval if stack_id is not set", func(t *testing.T) {
+		svc := provideDynamic(t, srv.URL, provideDynamicOpts{cfg: &setting.Cfg{StackID: ""}})
+		require.Equal(t, backgroundJobIntervalOnPrem, svc.backgroundJobInterval)
+	})
+}
+
 func TestDynamicAngularDetectorsProviderBackgroundService(t *testing.T) {
 	mockGCOMPatterns := newMockGCOMPatterns()
 	gcom := newDefaultGCOMScenario()
@@ -321,10 +337,10 @@ func TestDynamicAngularDetectorsProviderBackgroundService(t *testing.T) {
 	t.Cleanup(srv.Close)

 	t.Run("background service", func(t *testing.T) {
-		oldBackgroundJobInterval := backgroundJobInterval
-		backgroundJobInterval = time.Millisecond * 500
+		oldBackgroundJobInterval := backgroundJobIntervalOnPrem
+		backgroundJobIntervalOnPrem = time.Millisecond * 500
 		t.Cleanup(func() {
-			backgroundJobInterval = oldBackgroundJobInterval
+			backgroundJobIntervalOnPrem = oldBackgroundJobInterval
 		})

 		t.Run("is disabled if feature flag is not present", func(t *testing.T) {
@@ -563,6 +579,7 @@ func newError500GCOMScenario() *gcomScenario {

 type provideDynamicOpts struct {
 	store angularpatternsstore.Service
+	cfg   *setting.Cfg
 }

 func provideDynamic(t *testing.T, gcomURL string, opts ...provideDynamicOpts) *Dynamic {
@@ -573,8 +590,12 @@ func provideDynamic(t *testing.T, gcomURL string, opts ...provideDynamicOpts) *D
 	if opt.store == nil {
 		opt.store = angularpatternsstore.ProvideService(kvstore.NewFakeKVStore())
 	}
+	if opt.cfg == nil {
+		opt.cfg = setting.NewCfg()
+	}
+	opt.cfg.GrafanaComURL = gcomURL
 	d, err := ProvideDynamic(
-		&config.PluginManagementCfg{GrafanaComURL: gcomURL},
+		opt.cfg,
 		opt.store,
 		featuremgmt.WithFeatures(featuremgmt.FlagPluginsDynamicAngularDetectionPatterns),
 	)
--- a/pkg/services/pluginsintegration/angularinspector/angularinspector_test.go
+++ b/pkg/services/pluginsintegration/angularinspector/angularinspector_test.go
@@ -7,19 +7,19 @@ import (
 	"github.com/stretchr/testify/require"

 	"github.com/grafana/grafana/pkg/infra/kvstore"
-	"github.com/grafana/grafana/pkg/plugins/config"
 	"github.com/grafana/grafana/pkg/plugins/manager/loader/angular/angulardetector"
 	"github.com/grafana/grafana/pkg/plugins/manager/loader/angular/angularinspector"
 	"github.com/grafana/grafana/pkg/services/featuremgmt"
 	"github.com/grafana/grafana/pkg/services/pluginsintegration/angulardetectorsprovider"
 	"github.com/grafana/grafana/pkg/services/pluginsintegration/angularpatternsstore"
+	"github.com/grafana/grafana/pkg/setting"
 )

 func TestProvideService(t *testing.T) {
 	t.Run("uses hardcoded inspector if feature flag is not present", func(t *testing.T) {
 		features := featuremgmt.WithFeatures()
 		dynamic, err := angulardetectorsprovider.ProvideDynamic(
-			&config.PluginManagementCfg{},
+			setting.NewCfg(),
 			angularpatternsstore.ProvideService(kvstore.NewFakeKVStore()),
 			features,
 		)
@@ -37,7 +37,7 @@ func TestProvideService(t *testing.T) {
 			featuremgmt.FlagPluginsDynamicAngularDetectionPatterns,
 		)
 		dynamic, err := angulardetectorsprovider.ProvideDynamic(
-			&config.PluginManagementCfg{},
+			setting.NewCfg(),
 			angularpatternsstore.ProvideService(kvstore.NewFakeKVStore()),
 			features,
 		)