From 9216a3df7d03ed8004ef0efb96313dbc74232704 Mon Sep 17 00:00:00 2001 From: Giuseppe Guerra Date: Wed, 10 Jul 2024 11:15:10 +0200 Subject: [PATCH] Plugins: Remove datasourceQueryMultiStatus feature toggle (#90191) * Remove datasourceQueryMultiStatus feature toggle * PR review suggestion --- .../feature-toggles/index.md | 1 - .../src/types/featureToggles.gen.ts | 1 - pkg/api/ds_query.go | 10 +++----- pkg/api/ds_query_test.go | 24 ++++--------------- pkg/registry/apis/query/register.go | 2 -- pkg/services/featuremgmt/registry.go | 6 ----- pkg/services/featuremgmt/toggles_gen.csv | 1 - pkg/services/featuremgmt/toggles_gen.go | 4 ---- pkg/services/featuremgmt/toggles_gen.json | 3 ++- pkg/services/publicdashboards/api/api.go | 8 ++----- 10 files changed, 11 insertions(+), 49 deletions(-) diff --git a/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md b/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md index 4ca23b37fe5..10ae62a4035 100644 --- a/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md +++ b/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md @@ -114,7 +114,6 @@ Experimental features might be changed or removed without prior notice. | `publicDashboardsScene` | Enables public dashboard rendering using scenes | | `lokiExperimentalStreaming` | Support new streaming approach for loki (prototype, needs special loki build) | | `storage` | Configurable storage for dashboards, datasources, and resources | -| `datasourceQueryMultiStatus` | Introduce HTTP 207 Multi Status for api/ds/query | | `canvasPanelNesting` | Allow elements nesting | | `disableSecretsCompatibility` | Disable duplicated secret storage in legacy tables | | `logRequestsInstrumentedAsUnknown` | Logs the path for requests that are instrumented as unknown | diff --git a/packages/grafana-data/src/types/featureToggles.gen.ts b/packages/grafana-data/src/types/featureToggles.gen.ts index c25b96f7f9f..f4c1aeeeafb 100644 --- a/packages/grafana-data/src/types/featureToggles.gen.ts +++ b/packages/grafana-data/src/types/featureToggles.gen.ts @@ -29,7 +29,6 @@ export interface FeatureToggles { featureHighlights?: boolean; storage?: boolean; correlations?: boolean; - datasourceQueryMultiStatus?: boolean; autoMigrateOldPanels?: boolean; autoMigrateGraphPanel?: boolean; autoMigrateTablePanel?: boolean; diff --git a/pkg/api/ds_query.go b/pkg/api/ds_query.go index 65f58ee81be..2ff08a727b7 100644 --- a/pkg/api/ds_query.go +++ b/pkg/api/ds_query.go @@ -84,19 +84,15 @@ func (hs *HTTPServer) QueryMetricsV2(c *contextmodel.ReqContext) response.Respon } func (hs *HTTPServer) toJsonStreamingResponse(ctx context.Context, qdr *backend.QueryDataResponse) response.Response { - statusWhenError := http.StatusBadRequest - if hs.Features.IsEnabled(ctx, featuremgmt.FlagDatasourceQueryMultiStatus) { - statusWhenError = http.StatusMultiStatus - } - statusCode := http.StatusOK for _, res := range qdr.Responses { if res.Error != nil { - statusCode = statusWhenError + statusCode = http.StatusBadRequest + break } } - if statusCode == statusWhenError { + if statusCode == http.StatusBadRequest { // an error in the response we treat as downstream. requestmeta.WithDownstreamStatusSource(ctx) } diff --git a/pkg/api/ds_query_test.go b/pkg/api/ds_query_test.go index 6c0067c2460..7031ce542ea 100644 --- a/pkg/api/ds_query_test.go +++ b/pkg/api/ds_query_test.go @@ -23,7 +23,6 @@ import ( "github.com/grafana/grafana/pkg/plugins/manager/registry" "github.com/grafana/grafana/pkg/services/datasources" fakeDatasources "github.com/grafana/grafana/pkg/services/datasources/fakes" - "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/pluginsintegration/pluginconfig" "github.com/grafana/grafana/pkg/services/pluginsintegration/plugincontext" "github.com/grafana/grafana/pkg/services/pluginsintegration/pluginsettings" @@ -79,34 +78,19 @@ func TestAPIEndpoint_Metrics_QueryMetricsV2(t *testing.T) { }, &fakeDatasources.FakeCacheService{}, &fakeDatasources.FakeDataSourceService{}, pluginSettings.ProvideService(dbtest.NewFakeDB(), secretstest.NewFakeSecretsService()), pluginconfig.NewFakePluginRequestConfigProvider()), ) - serverFeatureEnabled := SetupAPITestServer(t, func(hs *HTTPServer) { + server := SetupAPITestServer(t, func(hs *HTTPServer) { hs.queryDataService = qds - hs.Features = featuremgmt.WithFeatures(featuremgmt.FlagDatasourceQueryMultiStatus, true) - hs.QuotaService = quotatest.New(false, nil) - }) - serverFeatureDisabled := SetupAPITestServer(t, func(hs *HTTPServer) { - hs.queryDataService = qds - hs.Features = featuremgmt.WithFeatures(featuremgmt.FlagDatasourceQueryMultiStatus, false) hs.QuotaService = quotatest.New(false, nil) }) - t.Run("Status code is 400 when data source response has an error and feature toggle is disabled", func(t *testing.T) { - req := serverFeatureDisabled.NewPostRequest("/api/ds/query", strings.NewReader(reqValid)) + t.Run("Status code is 400 when data source response has an error", func(t *testing.T) { + req := server.NewPostRequest("/api/ds/query", strings.NewReader(reqValid)) webtest.RequestWithSignedInUser(req, &user.SignedInUser{UserID: 1, OrgID: 1, Permissions: map[int64]map[string][]string{1: {datasources.ActionQuery: []string{datasources.ScopeAll}}}}) - resp, err := serverFeatureDisabled.SendJSON(req) + resp, err := server.SendJSON(req) require.NoError(t, err) require.NoError(t, resp.Body.Close()) require.Equal(t, http.StatusBadRequest, resp.StatusCode) }) - - t.Run("Status code is 207 when data source response has an error and feature toggle is enabled", func(t *testing.T) { - req := serverFeatureEnabled.NewPostRequest("/api/ds/query", strings.NewReader(reqValid)) - webtest.RequestWithSignedInUser(req, &user.SignedInUser{UserID: 1, OrgID: 1, Permissions: map[int64]map[string][]string{1: {datasources.ActionQuery: []string{datasources.ScopeAll}}}}) - resp, err := serverFeatureEnabled.SendJSON(req) - require.NoError(t, err) - require.NoError(t, resp.Body.Close()) - require.Equal(t, http.StatusMultiStatus, resp.StatusCode) - }) } func TestAPIEndpoint_Metrics_PluginDecryptionFailure(t *testing.T) { diff --git a/pkg/registry/apis/query/register.go b/pkg/registry/apis/query/register.go index 1c471bd5df0..57790a98fa1 100644 --- a/pkg/registry/apis/query/register.go +++ b/pkg/registry/apis/query/register.go @@ -38,7 +38,6 @@ type QueryAPIBuilder struct { log log.Logger concurrentQueryLimit int userFacingDefaultError string - returnMultiStatus bool // from feature toggle features featuremgmt.FeatureToggles tracer tracing.Tracer @@ -77,7 +76,6 @@ func NewQueryAPIBuilder(features featuremgmt.FeatureToggles, return &QueryAPIBuilder{ concurrentQueryLimit: 4, log: log.New("query_apiserver"), - returnMultiStatus: features.IsEnabledGlobally(featuremgmt.FlagDatasourceQueryMultiStatus), client: client, registry: registry, parser: newQueryParser(reader, legacy, tracer), diff --git a/pkg/services/featuremgmt/registry.go b/pkg/services/featuremgmt/registry.go index 4ac2ecc0d30..2dd1ee5c8d1 100644 --- a/pkg/services/featuremgmt/registry.go +++ b/pkg/services/featuremgmt/registry.go @@ -99,12 +99,6 @@ var ( Expression: "true", // enabled by default AllowSelfServe: true, }, - { - Name: "datasourceQueryMultiStatus", - Description: "Introduce HTTP 207 Multi Status for api/ds/query", - Stage: FeatureStageExperimental, - Owner: grafanaPluginsPlatformSquad, - }, { Name: "autoMigrateOldPanels", Description: "Migrate old angular panels to supported versions (graph, table-old, worldmap, etc)", diff --git a/pkg/services/featuremgmt/toggles_gen.csv b/pkg/services/featuremgmt/toggles_gen.csv index 582942e6b46..366c2b4eace 100644 --- a/pkg/services/featuremgmt/toggles_gen.csv +++ b/pkg/services/featuremgmt/toggles_gen.csv @@ -10,7 +10,6 @@ lokiExperimentalStreaming,experimental,@grafana/observability-logs,false,false,f featureHighlights,GA,@grafana/grafana-as-code,false,false,false storage,experimental,@grafana/grafana-app-platform-squad,false,false,false correlations,GA,@grafana/explore-squad,false,false,false -datasourceQueryMultiStatus,experimental,@grafana/plugins-platform-backend,false,false,false autoMigrateOldPanels,preview,@grafana/dataviz-squad,false,false,true autoMigrateGraphPanel,preview,@grafana/dataviz-squad,false,false,true autoMigrateTablePanel,preview,@grafana/dataviz-squad,false,false,true diff --git a/pkg/services/featuremgmt/toggles_gen.go b/pkg/services/featuremgmt/toggles_gen.go index 644b10c4473..a1076aa11d4 100644 --- a/pkg/services/featuremgmt/toggles_gen.go +++ b/pkg/services/featuremgmt/toggles_gen.go @@ -51,10 +51,6 @@ const ( // Correlations page FlagCorrelations = "correlations" - // FlagDatasourceQueryMultiStatus - // Introduce HTTP 207 Multi Status for api/ds/query - FlagDatasourceQueryMultiStatus = "datasourceQueryMultiStatus" - // FlagAutoMigrateOldPanels // Migrate old angular panels to supported versions (graph, table-old, worldmap, etc) FlagAutoMigrateOldPanels = "autoMigrateOldPanels" diff --git a/pkg/services/featuremgmt/toggles_gen.json b/pkg/services/featuremgmt/toggles_gen.json index a68a06f0350..98e7bc208c6 100644 --- a/pkg/services/featuremgmt/toggles_gen.json +++ b/pkg/services/featuremgmt/toggles_gen.json @@ -825,7 +825,8 @@ "metadata": { "name": "datasourceQueryMultiStatus", "resourceVersion": "1718727528075", - "creationTimestamp": "2022-05-03T16:02:20Z" + "creationTimestamp": "2022-05-03T16:02:20Z", + "deletionTimestamp": "2024-07-08T14:46:08Z" }, "spec": { "description": "Introduce HTTP 207 Multi Status for api/ds/query", diff --git a/pkg/services/publicdashboards/api/api.go b/pkg/services/publicdashboards/api/api.go index 84433fa6598..cbbbcf605f0 100644 --- a/pkg/services/publicdashboards/api/api.go +++ b/pkg/services/publicdashboards/api/api.go @@ -302,15 +302,11 @@ func (api *Api) DeletePublicDashboard(c *contextmodel.ReqContext) response.Respo // Copied from pkg/api/metrics.go func toJsonStreamingResponse(ctx context.Context, features featuremgmt.FeatureToggles, qdr *backend.QueryDataResponse) response.Response { - statusWhenError := http.StatusBadRequest - if features.IsEnabled(ctx, featuremgmt.FlagDatasourceQueryMultiStatus) { - statusWhenError = http.StatusMultiStatus - } - statusCode := http.StatusOK for _, res := range qdr.Responses { if res.Error != nil { - statusCode = statusWhenError + statusCode = http.StatusBadRequest + break } }