QueryService: Add feature toggles to better support testing (#86493)

2025-02-25 18:55:37 -06:00 · 2024-04-19 12:26:21 +03:00
parent 8a5c0cfdc0
commit 5a8384a245
14 changed files with 191 additions and 42 deletions
--- a/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md
+++ b/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md
@@ -150,6 +150,9 @@ Experimental features might be changed or removed without prior notice.
 | `idForwarding`                              | Generate signed id token for identity that can be forwarded to plugins and external services                                                                                                                                                                                      |
 | `enableNativeHTTPHistogram`                 | Enables native HTTP Histograms                                                                                                                                                                                                                                                    |
 | `kubernetesSnapshots`                       | Routes snapshot requests from /api to the /apis endpoint                                                                                                                                                                                                                          |
+| `queryService`                              | Register /apis/query.grafana.app/ -- will eventually replace /api/ds/query                                                                                                                                                                                                        |
+| `queryServiceRewrite`                       | Rewrite requests targeting /ds/query to the query service                                                                                                                                                                                                                         |
+| `queryServiceFromUI`                        | Routes requests to the new query service                                                                                                                                                                                                                                          |
 | `cachingOptimizeSerializationMemoryUsage`   | If enabled, the caching backend gradually serializes query responses for the cache, comparing against the configured `[caching]max_value_mb` value as it goes. This can can help prevent Grafana from running out of memory while attempting to cache very large query responses. |
 | `prometheusPromQAIL`                        | Prometheus and AI/ML to assist users in creating a query                                                                                                                                                                                                                          |
 | `prometheusCodeModeMetricNamesSearch`       | Enables search for metric names in Code Mode, to improve performance when working with an enormous number of metric names                                                                                                                                                         |
@@ -184,5 +187,4 @@ The following toggles require explicitly setting Grafana's [app mode]({{< relref
 | `unifiedStorage`                       | SQL-based k8s storage                                          |
 | `grafanaAPIServerWithExperimentalAPIs` | Register experimental APIs with the k8s API server             |
 | `grafanaAPIServerEnsureKubectlAccess`  | Start an additional https handler and write kubectl options    |
-| `kubernetesQueryServiceRewrite`        | Rewrite requests targeting /ds/query to the query service      |
 | `panelTitleSearchInV1`                 | Enable searching for dashboards using panel title in search v1 |
--- a/packages/grafana-data/src/types/featureToggles.gen.ts
+++ b/packages/grafana-data/src/types/featureToggles.gen.ts
@@ -121,7 +121,9 @@ export interface FeatureToggles {
  transformationsVariableSupport?: boolean;
  kubernetesPlaylists?: boolean;
  kubernetesSnapshots?: boolean;
-  kubernetesQueryServiceRewrite?: boolean;
+  queryService?: boolean;
+  queryServiceRewrite?: boolean;
+  queryServiceFromUI?: boolean;
  cloudWatchBatchQueries?: boolean;
  recoveryThreshold?: boolean;
  lokiStructuredMetadata?: boolean;
--- a/packages/grafana-runtime/src/config.ts
+++ b/packages/grafana-runtime/src/config.ts
@@ -214,9 +214,7 @@ export class GrafanaBootConfig implements GrafanaConfig {
      systemDateFormats.update(this.dateFormats);
    }

-    if (this.buildInfo.env === 'development') {
-      overrideFeatureTogglesFromUrl(this);
-    }
+    overrideFeatureTogglesFromUrl(this);
    overrideFeatureTogglesFromLocalStorage(this);

    if (this.featureToggles.disableAngular) {
@@ -253,15 +251,11 @@ function overrideFeatureTogglesFromUrl(config: GrafanaBootConfig) {
    return;
  }

-  const migrationFeatureFlags = new Set([
-    'autoMigrateOldPanels',
-    'autoMigrateGraphPanel',
-    'autoMigrateTablePanel',
-    'autoMigratePiechartPanel',
-    'autoMigrateWorldmapPanel',
-    'autoMigrateStatPanel',
-    'disableAngular',
-  ]);
+  const isDevelopment = config.buildInfo.env === 'development';
+
+  // Although most flags can not be changed from the URL in production,
+  // some of them are safe (and useful!) to change dynamically from the browser URL
+  const safeRuntimeFeatureFlags = new Set(['queryServiceFromUI']);

  const params = new URLSearchParams(window.location.search);
  params.forEach((value, key) => {
@@ -269,15 +263,14 @@ function overrideFeatureTogglesFromUrl(config: GrafanaBootConfig) {
      const featureToggles = config.featureToggles as Record<string, boolean>;
      const featureName = key.substring(10);

-      // skip the migration feature flags
-      if (migrationFeatureFlags.has(featureName)) {
-        return;
-      }
-
      const toggleState = value === 'true' || value === ''; // browser rewrites true as ''
      if (toggleState !== featureToggles[key]) {
-        featureToggles[featureName] = toggleState;
-        console.log(`Setting feature toggle ${featureName} = ${toggleState} via url`);
+        if (isDevelopment || safeRuntimeFeatureFlags.has(featureName)) {
+          featureToggles[featureName] = toggleState;
+          console.log(`Setting feature toggle ${featureName} = ${toggleState} via url`);
+        } else {
+          console.log(`Unable to change feature toggle ${featureName} via url in production.`);
+        }
      }
    }
  });
--- a/packages/grafana-runtime/src/utils/DataSourceWithBackend.ts
+++ b/packages/grafana-runtime/src/utils/DataSourceWithBackend.ts
@@ -207,6 +207,18 @@ class DataSourceWithBackend<

    let url = '/api/ds/query?ds_type=' + this.type;

+    // Use the new query service
+    if (config.featureToggles.queryServiceFromUI) {
+      if (!(config.featureToggles.queryService || config.featureToggles.grafanaAPIServerWithExperimentalAPIs)) {
+        console.warn('feature toggle queryServiceFromUI also requires the queryService to be running');
+      } else {
+        if (!hasExpr && dsUIDs.size === 1) {
+          // TODO? can we talk directly to the apiserver?
+        }
+        url = `/apis/query.grafana.app/v0alpha1/namespaces/${config.namespace}/query?ds_type=' + this.type`;
+      }
+    }
+
    if (hasExpr) {
      headers[PluginRequestHeaders.FromExpression] = 'true';
      url += '&expression=true';
--- a/pkg/api/ds_query.go
+++ b/pkg/api/ds_query.go
@@ -7,7 +7,6 @@ import (
 	"net/http"

 	"github.com/grafana/grafana-plugin-sdk-go/backend"
-	"github.com/grafana/grafana/pkg/util/proxyutil"

 	"github.com/grafana/grafana/pkg/api/dtos"
 	"github.com/grafana/grafana/pkg/api/response"
@@ -40,8 +39,7 @@ func (hs *HTTPServer) handleQueryMetricsError(err error) *response.NormalRespons

 // metrics.go
 func (hs *HTTPServer) getDSQueryEndpoint() web.Handler {
-	if hs.Features.IsEnabledGlobally(featuremgmt.FlagKubernetesQueryServiceRewrite) {
-		// DEV ONLY FEATURE FLAG!
+	if hs.Features.IsEnabledGlobally(featuremgmt.FlagQueryServiceRewrite) {
 		// rewrite requests from /ds/query to the new query service
 		namespaceMapper := request.GetNamespaceMapper(hs.Cfg)
 		return func(w http.ResponseWriter, r *http.Request) {
@@ -51,11 +49,9 @@ func (hs *HTTPServer) getDSQueryEndpoint() web.Handler {
 				return
 			}
 			r.URL.Path = "/apis/query.grafana.app/v0alpha1/namespaces/" + namespaceMapper(user.OrgID) + "/query"
-			r.Header.Add(proxyutil.IDHeaderName, user.GetIDToken())
 			hs.clientConfigProvider.DirectlyServeHTTP(w, r)
 		}
 	}
-
 	return routing.Wrap(hs.QueryMetricsV2)
 }

--- a/pkg/api/ds_query_test.go
+++ b/pkg/api/ds_query_test.go
--- a/pkg/registry/apis/query/README.md
+++ b/pkg/registry/apis/query/README.md
@@ -0,0 +1,78 @@
+# Query service
+
+This query service aims to replace the existing /api/ds/query.  
+
+The key differences are:
+1. This service has a stronger type system (not simplejson)
+2. Same workflow regardless if expressions exist
+3. Datasource settings+access is managed in each datasource, not at the beginning
+
+
+
+### Current /api/ds/query workflow
+
+```mermaid
+sequenceDiagram
+    autonumber
+    actor User as User or Process
+    participant api as /api/ds/query
+    participant db as Storage<br/>(SQL)
+    participant ds as Datasource<br/>Plugin
+    participant expr as Expression<br/>Engine
+
+    User->>api: POST Query
+    loop Each query
+        api->>api: Parse query
+        api->>db: Get ds config<br>and secrets
+        db->>api: 
+    end
+    alt No expressions
+      alt Single datasource
+          api->>ds: QueryData
+      else Multiple datasources
+        loop Each datasource (concurrently)
+          api->>ds: QueryData
+        end
+        api->>api: Wait for results
+      end
+    else Expressions exist
+        api->>expr: Calculate expressions graph
+        loop Each node (eg, refID)
+          alt Is query
+              expr->>ds: QueryData
+          else Is expression
+            expr->>expr: Process
+          end
+        end
+    end
+    api->>User: return results
+```
+
+
+
+### /apis/query.grafana.app (in single tenant grafana)
+
+```mermaid
+sequenceDiagram
+    autonumber
+    actor User as User or Process
+    participant api as /apis/query.grafana.app
+    participant ds as Datasource<br/>Handler/Plugin
+    participant db as Storage<br/>(SQL)
+    participant expr as Expression<br/>Engine
+
+    User->>api: POST Query
+    api->>api: Parse queries
+    api->>api: Calculate dependencies
+    loop Each datasource (concurrently)
+        api->>ds: QueryData
+        ds->>ds: Verify user access
+        ds->>db: Get settings <br> and secrets
+    end
+    loop Each expression
+        api->>expr: Execute
+    end
+    api->>api: Verify ResultExpectations
+    api->>User: return results
+```
+
--- a/pkg/registry/apis/query/client/plugin.go
+++ b/pkg/registry/apis/query/client/plugin.go
@@ -40,7 +40,7 @@ type pluginRegistry struct {
 var _ data.QueryDataClient = (*pluginClient)(nil)
 var _ query.DataSourceApiServerRegistry = (*pluginRegistry)(nil)

-// NewDummyTestRunner creates a runner that only works with testdata
+// NewQueryClientForPluginClient creates a client that delegates to the internal plugins.Client stack
 func NewQueryClientForPluginClient(p plugins.Client, ctx *plugincontext.Provider) data.QueryDataClient {
 	return &pluginClient{
 		pluginClient: p,
--- a/pkg/registry/apis/query/plugins.go
+++ b/pkg/registry/apis/query/plugins.go
@@ -9,7 +9,6 @@ import (
 	"k8s.io/apiserver/pkg/registry/rest"

 	common "github.com/grafana/grafana/pkg/apimachinery/apis/common/v0alpha1"
-	example "github.com/grafana/grafana/pkg/apis/example/v0alpha1"
 	query "github.com/grafana/grafana/pkg/apis/query/v0alpha1"
 )

@@ -24,6 +23,9 @@ type pluginsStorage struct {
 	resourceInfo   *common.ResourceInfo
 	tableConverter rest.TableConvertor
 	registry       query.DataSourceApiServerRegistry
+
+	// Always return an empty list regardless what we think exists
+	returnEmptyList bool
 }

 func newPluginsStorage(reg query.DataSourceApiServerRegistry) *pluginsStorage {
@@ -46,7 +48,7 @@ func (s *pluginsStorage) NamespaceScoped() bool {
 }

 func (s *pluginsStorage) GetSingularName() string {
-	return example.DummyResourceInfo.GetSingularName()
+	return s.resourceInfo.GetSingularName()
 }

 func (s *pluginsStorage) NewList() runtime.Object {
@@ -58,5 +60,8 @@ func (s *pluginsStorage) ConvertToTable(ctx context.Context, object runtime.Obje
 }

 func (s *pluginsStorage) List(ctx context.Context, options *internalversion.ListOptions) (runtime.Object, error) {
+	if s.returnEmptyList {
+		return s.NewList(), nil
+	}
 	return s.registry.GetDatasourceApiServers(ctx)
 }
--- a/pkg/registry/apis/query/register.go
+++ b/pkg/registry/apis/query/register.go
@@ -85,8 +85,9 @@ func RegisterAPIService(features featuremgmt.FeatureToggles,
 	tracer tracing.Tracer,
 	legacy service.LegacyDataSourceLookup,
 ) (*QueryAPIBuilder, error) {
-	if !features.IsEnabledGlobally(featuremgmt.FlagGrafanaAPIServerWithExperimentalAPIs) {
-		return nil, nil // skip registration unless opting into experimental apis
+	if !(features.IsEnabledGlobally(featuremgmt.FlagQueryService) ||
+		features.IsEnabledGlobally(featuremgmt.FlagGrafanaAPIServerWithExperimentalAPIs)) {
+		return nil, nil // skip registration unless explicitly added (or all experimental are added)
 	}

 	builder, err := NewQueryAPIBuilder(
@@ -132,10 +133,16 @@ func (b *QueryAPIBuilder) GetAPIGroupInfo(
 	gv := v0alpha1.SchemeGroupVersion
 	apiGroupInfo := genericapiserver.NewDefaultAPIGroupInfo(gv.Group, scheme, metav1.ParameterCodec, codecs)

-	plugins := newPluginsStorage(b.registry)
-
 	storage := map[string]rest.Storage{}
+
+	plugins := newPluginsStorage(b.registry)
 	storage[plugins.resourceInfo.StoragePath()] = plugins
+	if !b.features.IsEnabledGlobally(featuremgmt.FlagGrafanaAPIServerWithExperimentalAPIs) {
+		// The plugin registry is still experimental, and not yet accurate
+		// For standard k8s api discovery to work, at least one resource must be registered
+		// While this feature is under development, we can return an empty list for non-dev instances
+		plugins.returnEmptyList = true
+	}

 	apiGroupInfo.VersionedResourcesStorageMap[gv.Version] = storage
 	return &apiGroupInfo, nil
--- a/pkg/services/featuremgmt/registry.go
+++ b/pkg/services/featuremgmt/registry.go
@@ -769,12 +769,25 @@ var (
 			RequiresRestart: true, // changes the API routing
 		},
 		{
-			Name:            "kubernetesQueryServiceRewrite",
+			Name:            "queryService",
+			Description:     "Register /apis/query.grafana.app/ -- will eventually replace /api/ds/query",
+			Stage:           FeatureStageExperimental,
+			Owner:           grafanaAppPlatformSquad,
+			RequiresRestart: true, // Adds a route at startup
+		},
+		{
+			Name:            "queryServiceRewrite",
 			Description:     "Rewrite requests targeting /ds/query to the query service",
 			Stage:           FeatureStageExperimental,
 			Owner:           grafanaAppPlatformSquad,
 			RequiresRestart: true, // changes the API routing
-			RequiresDevMode: true,
+		},
+		{
+			Name:         "queryServiceFromUI",
+			Description:  "Routes requests to the new query service",
+			Stage:        FeatureStageExperimental,
+			Owner:        grafanaAppPlatformSquad,
+			FrontendOnly: true, // and can change at startup
 		},
 		{
 			Name:        "cloudWatchBatchQueries",
--- a/pkg/services/featuremgmt/toggles_gen.csv
+++ b/pkg/services/featuremgmt/toggles_gen.csv
@@ -102,7 +102,9 @@ formatString,preview,@grafana/dataviz-squad,false,false,true
 transformationsVariableSupport,preview,@grafana/dataviz-squad,false,false,true
 kubernetesPlaylists,GA,@grafana/grafana-app-platform-squad,false,true,false
 kubernetesSnapshots,experimental,@grafana/grafana-app-platform-squad,false,true,false
-kubernetesQueryServiceRewrite,experimental,@grafana/grafana-app-platform-squad,true,true,false
+queryService,experimental,@grafana/grafana-app-platform-squad,false,true,false
+queryServiceRewrite,experimental,@grafana/grafana-app-platform-squad,false,true,false
+queryServiceFromUI,experimental,@grafana/grafana-app-platform-squad,false,false,true
 cloudWatchBatchQueries,preview,@grafana/aws-datasources,false,false,false
 recoveryThreshold,GA,@grafana/alerting-squad,false,true,false
 lokiStructuredMetadata,GA,@grafana/observability-logs,false,false,false
--- a/pkg/services/featuremgmt/toggles_gen.go
+++ b/pkg/services/featuremgmt/toggles_gen.go
@@ -419,9 +419,17 @@ const (
 	// Routes snapshot requests from /api to the /apis endpoint
 	FlagKubernetesSnapshots = "kubernetesSnapshots"

-	// FlagKubernetesQueryServiceRewrite
+	// FlagQueryService
+	// Register /apis/query.grafana.app/ -- will eventually replace /api/ds/query
+	FlagQueryService = "queryService"
+
+	// FlagQueryServiceRewrite
 	// Rewrite requests targeting /ds/query to the query service
-	FlagKubernetesQueryServiceRewrite = "kubernetesQueryServiceRewrite"
+	FlagQueryServiceRewrite = "queryServiceRewrite"
+
+	// FlagQueryServiceFromUI
+	// Routes requests to the new query service
+	FlagQueryServiceFromUI = "queryServiceFromUI"

 	// FlagCloudWatchBatchQueries
 	// Runs CloudWatch metrics queries as separate batches
--- a/pkg/services/featuremgmt/toggles_gen.json
+++ b/pkg/services/featuremgmt/toggles_gen.json
@@ -278,15 +278,17 @@
    },
    {
      "metadata": {
-        "name": "kubernetesQueryServiceRewrite",
-        "resourceVersion": "1712639261786",
-        "creationTimestamp": "2024-04-09T05:07:41Z"
+        "name": "queryServiceRewrite",
+        "resourceVersion": "1713422970838",
+        "creationTimestamp": "2024-04-09T05:07:41Z",
+        "annotations": {
+          "grafana.app/updatedTimestamp": "2024-04-18 06:49:30.838977 +0000 UTC"
+        }
      },
      "spec": {
        "description": "Rewrite requests targeting /ds/query to the query service",
        "stage": "experimental",
        "codeowner": "@grafana/grafana-app-platform-squad",
-        "requiresDevMode": true,
        "requiresRestart": true
      }
    },
@@ -2134,6 +2136,35 @@
        "codeowner": "@grafana/observability-metrics",
        "requiresRestart": true
      }
+    },
+    {
+      "metadata": {
+        "name": "queryServiceFromUI",
+        "resourceVersion": "1713422970838",
+        "creationTimestamp": "2024-04-18T06:49:30Z"
+      },
+      "spec": {
+        "description": "Routes requests to the new query service",
+        "stage": "experimental",
+        "codeowner": "@grafana/grafana-app-platform-squad",
+        "frontend": true
+      }
+    },
+    {
+      "metadata": {
+        "name": "queryService",
+        "resourceVersion": "1713504737045",
+        "creationTimestamp": "2024-04-18T06:49:30Z",
+        "annotations": {
+          "grafana.app/updatedTimestamp": "2024-04-19 05:32:17.045343 +0000 UTC"
+        }
+      },
+      "spec": {
+        "description": "Register /apis/query.grafana.app/ -- will eventually replace /api/ds/query",
+        "stage": "experimental",
+        "codeowner": "@grafana/grafana-app-platform-squad",
+        "requiresRestart": true
+      }
    }
  ]
 }