Plugins: Remove pluginsInstrumentationStatusSource feature toggle (#83067)

* Plugins: Remove pluginsInstrumentationStatusSource feature toggle

* update tests

* Inline pluginRequestDurationWithLabels, pluginRequestCounterWithLabels, pluginRequestDurationSecondsWithLabels

docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md

@@ -157,7 +157,6 @@ Experimental features might be changed or removed without prior notice.
 | `kubernetesSnapshots`                       | Routes snapshot requests from /api to the /apis endpoint                                                                                                                                                                                                                          |
 | `teamHttpHeaders`                           | Enables datasources to apply team headers to the client requests                                                                                                                                                                                                                  |
 | `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. |
-| `pluginsInstrumentationStatusSource`        | Include a status source label for plugin request metrics and logs                                                                                                                                                                                                                 |
 | `prometheusPromQAIL`                        | Prometheus and AI/ML to assist users in creating a query                                                                                                                                                                                                                          |
 | `alertmanagerRemoteSecondary`               | Enable Grafana to sync configuration and state with a remote Alertmanager.                                                                                                                                                                                                        |
 | `alertmanagerRemotePrimary`                 | Enable Grafana to have a remote Alertmanager instance as the primary Alertmanager.                                                                                                                                                                                                |

packages/grafana-data/src/types/featureToggles.gen.ts

@@ -139,7 +139,6 @@ export interface FeatureToggles {
   awsDatasourcesNewFormStyling?: boolean;
   cachingOptimizeSerializationMemoryUsage?: boolean;
   panelTitleSearchInV1?: boolean;
-  pluginsInstrumentationStatusSource?: boolean;
   managedPluginsInstall?: boolean;
   prometheusPromQAIL?: boolean;
   addFieldFromCalculationStatFunctions?: boolean;

pkg/services/featuremgmt/registry.go

@@ -905,13 +905,6 @@ var (
 			Stage:           FeatureStageExperimental,
 			Owner:           grafanaBackendPlatformSquad,
 		},
-		{
-			Name:         "pluginsInstrumentationStatusSource",
-			Description:  "Include a status source label for plugin request metrics and logs",
-			FrontendOnly: false,
-			Stage:        FeatureStageExperimental,
-			Owner:        grafanaPluginsPlatformSquad,
-		},
 		{
 			Name:            "managedPluginsInstall",
 			Description:     "Install managed plugins directly from plugins catalog",

pkg/services/featuremgmt/toggles_gen.csv

@@ -120,7 +120,6 @@ teamHttpHeaders,experimental,@grafana/identity-access-team,false,false,false
 awsDatasourcesNewFormStyling,preview,@grafana/aws-datasources,false,false,true
 cachingOptimizeSerializationMemoryUsage,experimental,@grafana/grafana-operator-experience-squad,false,false,false
 panelTitleSearchInV1,experimental,@grafana/backend-platform,true,false,false
-pluginsInstrumentationStatusSource,experimental,@grafana/plugins-platform-backend,false,false,false
 managedPluginsInstall,preview,@grafana/plugins-platform-backend,false,false,false
 prometheusPromQAIL,experimental,@grafana/observability-metrics,false,false,true
 addFieldFromCalculationStatFunctions,preview,@grafana/dataviz-squad,false,false,true

pkg/services/featuremgmt/toggles_gen.go

@@ -491,10 +491,6 @@ const (
 	// Enable searching for dashboards using panel title in search v1
 	FlagPanelTitleSearchInV1 = "panelTitleSearchInV1"
 
-	// FlagPluginsInstrumentationStatusSource
-	// Include a status source label for plugin request metrics and logs
-	FlagPluginsInstrumentationStatusSource = "pluginsInstrumentationStatusSource"
-
 	// FlagManagedPluginsInstall
 	// Install managed plugins directly from plugins catalog
 	FlagManagedPluginsInstall = "managedPluginsInstall"

pkg/services/featuremgmt/toggles_gen.json

@@ -82,7 +82,8 @@
       "metadata": {
         "name": "pluginsInstrumentationStatusSource",
         "resourceVersion": "1708108588074",
-        "creationTimestamp": "2024-02-16T18:36:28Z"
+        "creationTimestamp": "2024-02-16T18:36:28Z",
+        "deletionTimestamp": "2024-02-19T14:18:02Z"
       },
       "spec": {
         "description": "Include a status source label for plugin request metrics and logs",

pkg/services/pluginsintegration/clientmiddleware/logger_middleware.go

@@ -50,9 +50,7 @@ func (m *LoggerMiddleware) logRequest(ctx context.Context, fn func(ctx context.C
 	if err != nil {
 		logParams = append(logParams, "error", err)
 	}
-	if m.features.IsEnabled(ctx, featuremgmt.FlagPluginsInstrumentationStatusSource) {
+	logParams = append(logParams, "statusSource", pluginrequestmeta.StatusSourceFromContext(ctx))
-		logParams = append(logParams, "statusSource", pluginrequestmeta.StatusSourceFromContext(ctx))
-	}
 
 	ctxLogger := m.logger.FromContext(ctx)
 	logFunc := ctxLogger.Info
@@ -81,9 +79,11 @@ func (m *LoggerMiddleware) QueryData(ctx context.Context, req *backend.QueryData
 		ctxLogger := m.logger.FromContext(ctx)
 		for refID, dr := range resp.Responses {
 			if dr.Error != nil {
-				logParams := []any{"refID", refID, "status", int(dr.Status), "error", dr.Error}
+				logParams := []any{
-				if m.features.IsEnabled(ctx, featuremgmt.FlagPluginsInstrumentationStatusSource) {
+					"refID", refID,
-					logParams = append(logParams, "statusSource", pluginrequestmeta.StatusSourceFromPluginErrorSource(dr.ErrorSource))
+					"status", int(dr.Status),
+					"error", dr.Error,
+					"statusSource", pluginrequestmeta.StatusSourceFromPluginErrorSource(dr.ErrorSource),
 				}
 				ctxLogger.Error("Partial data response error", logParams...)
 			}

pkg/services/pluginsintegration/clientmiddleware/metrics_middleware.go

@@ -32,10 +32,7 @@ type MetricsMiddleware struct {
 }
 
 func newMetricsMiddleware(promRegisterer prometheus.Registerer, pluginRegistry registry.Service, features featuremgmt.FeatureToggles) *MetricsMiddleware {
-	var additionalLabels []string
+	additionalLabels := []string{"status_source"}
-	if features.IsEnabledGlobally(featuremgmt.FlagPluginsInstrumentationStatusSource) {
-		additionalLabels = []string{"status_source"}
-	}
 	pluginRequestCounter := prometheus.NewCounterVec(prometheus.CounterOpts{
 		Namespace: "grafana",
 		Name:      "plugin_request_total",
@@ -119,19 +116,11 @@ func (m *MetricsMiddleware) instrumentPluginRequest(ctx context.Context, pluginC
 	status, err := fn(ctx)
 	elapsed := time.Since(start)
 
-	pluginRequestDurationLabels := []string{pluginCtx.PluginID, endpoint, target}
+	statusSource := pluginrequestmeta.StatusSourceFromContext(ctx)
-	pluginRequestCounterLabels := []string{pluginCtx.PluginID, endpoint, status.String(), target}
-	pluginRequestDurationSecondsLabels := []string{"grafana-backend", pluginCtx.PluginID, endpoint, status.String(), target}
-	if m.features.IsEnabled(ctx, featuremgmt.FlagPluginsInstrumentationStatusSource) {
-		statusSource := pluginrequestmeta.StatusSourceFromContext(ctx)
-		pluginRequestDurationLabels = append(pluginRequestDurationLabels, string(statusSource))
-		pluginRequestCounterLabels = append(pluginRequestCounterLabels, string(statusSource))
-		pluginRequestDurationSecondsLabels = append(pluginRequestDurationSecondsLabels, string(statusSource))
-	}
 
-	pluginRequestDurationWithLabels := m.pluginRequestDuration.WithLabelValues(pluginRequestDurationLabels...)
+	pluginRequestDurationWithLabels := m.pluginRequestDuration.WithLabelValues(pluginCtx.PluginID, endpoint, target, string(statusSource))
-	pluginRequestCounterWithLabels := m.pluginRequestCounter.WithLabelValues(pluginRequestCounterLabels...)
+	pluginRequestCounterWithLabels := m.pluginRequestCounter.WithLabelValues(pluginCtx.PluginID, endpoint, status.String(), target, string(statusSource))
-	pluginRequestDurationSecondsWithLabels := m.pluginRequestDurationSeconds.WithLabelValues(pluginRequestDurationSecondsLabels...)
+	pluginRequestDurationSecondsWithLabels := m.pluginRequestDurationSeconds.WithLabelValues("grafana-backend", pluginCtx.PluginID, endpoint, status.String(), target, string(statusSource))
 
 	if traceID := tracing.TraceIDFromContext(ctx, true); traceID != "" {
 		pluginRequestDurationWithLabels.(prometheus.ExemplarObserver).ObserveWithExemplar(

pkg/services/pluginsintegration/clientmiddleware/metrics_middleware_test.go

@@ -90,7 +90,7 @@ func TestInstrumentationMiddleware(t *testing.T) {
 				require.Equal(t, 1, testutil.CollectAndCount(promRegistry, metricRequestDurationMs))
 				require.Equal(t, 1, testutil.CollectAndCount(promRegistry, metricRequestDurationS))
 
-				counter := mw.pluginMetrics.pluginRequestCounter.WithLabelValues(pluginID, tc.expEndpoint, requestStatusOK.String(), string(backendplugin.TargetUnknown))
+				counter := mw.pluginMetrics.pluginRequestCounter.WithLabelValues(pluginID, tc.expEndpoint, requestStatusOK.String(), string(backendplugin.TargetUnknown), string(pluginrequestmeta.DefaultStatusSource))
 				require.Equal(t, 1.0, testutil.ToFloat64(counter))
 				for _, m := range []string{metricRequestDurationMs, metricRequestDurationS} {
 					require.NoError(t, checkHistogram(promRegistry, m, map[string]string{
@@ -115,12 +115,6 @@ func TestInstrumentationMiddleware(t *testing.T) {
 
 func TestInstrumentationMiddlewareStatusSource(t *testing.T) {
 	const labelStatusSource = "status_source"
-	queryDataOKCounterLabels := prometheus.Labels{
-		"plugin_id": pluginID,
-		"endpoint":  endpointQueryData,
-		"status":    requestStatusOK.String(),
-		"target":    string(backendplugin.TargetUnknown),
-	}
 	queryDataErrorCounterLabels := prometheus.Labels{
 		"plugin_id": pluginID,
 		"endpoint":  endpointQueryData,
@@ -159,8 +153,7 @@ func TestInstrumentationMiddlewareStatusSource(t *testing.T) {
 	require.NoError(t, pluginsRegistry.Add(context.Background(), &plugins.Plugin{
 		JSONData: plugins.JSONData{ID: pluginID, Backend: true},
 	}))
-	features := featuremgmt.WithFeatures(featuremgmt.FlagPluginsInstrumentationStatusSource)
+	metricsMw := newMetricsMiddleware(promRegistry, pluginsRegistry, featuremgmt.WithFeatures())
-	metricsMw := newMetricsMiddleware(promRegistry, pluginsRegistry, features)
 	cdt := clienttest.NewClientDecoratorTest(t, clienttest.WithMiddlewares(
 		NewPluginRequestMetaMiddleware(),
 		plugins.ClientMiddlewareFunc(func(next plugins.Client) plugins.Client {
@@ -171,53 +164,21 @@ func TestInstrumentationMiddlewareStatusSource(t *testing.T) {
 	))
 
 	t.Run("Metrics", func(t *testing.T) {
-		t.Run("Should ignore ErrorSource if feature flag is disabled", func(t *testing.T) {
+		metricsMw.pluginMetrics.pluginRequestCounter.Reset()
-			// Use different middleware without feature flag
-			metricsMw := newMetricsMiddleware(prometheus.NewRegistry(), pluginsRegistry, featuremgmt.WithFeatures())
-			cdt := clienttest.NewClientDecoratorTest(t, clienttest.WithMiddlewares(
-				plugins.ClientMiddlewareFunc(func(next plugins.Client) plugins.Client {
-					metricsMw.next = next
-					return metricsMw
-				}),
-			))
 
-			cdt.TestClient.QueryDataFunc = func(ctx context.Context, req *backend.QueryDataRequest) (*backend.QueryDataResponse, error) {
+		cdt.TestClient.QueryDataFunc = func(ctx context.Context, req *backend.QueryDataRequest) (*backend.QueryDataResponse, error) {
-				return &backend.QueryDataResponse{Responses: map[string]backend.DataResponse{"A": downstreamErrorResponse}}, nil
+			return &backend.QueryDataResponse{Responses: map[string]backend.DataResponse{"A": downstreamErrorResponse}}, nil
-			}
+		}
-			_, err := cdt.Decorator.QueryData(context.Background(), &backend.QueryDataRequest{PluginContext: pCtx})
+		_, err := cdt.Decorator.QueryData(context.Background(), &backend.QueryDataRequest{PluginContext: pCtx})
-			require.NoError(t, err)
+		require.NoError(t, err)
-			counter, err := metricsMw.pluginMetrics.pluginRequestCounter.GetMetricWith(newLabels(queryDataErrorCounterLabels, nil))
+		counter, err := metricsMw.pluginMetrics.pluginRequestCounter.GetMetricWith(newLabels(
-			require.NoError(t, err)
+			queryDataErrorCounterLabels,
-			require.Equal(t, 1.0, testutil.ToFloat64(counter))
+			prometheus.Labels{
-
+				labelStatusSource: string(backend.ErrorSourceDownstream),
-			// error_source should not be defined at all
+			}),
-			_, err = metricsMw.pluginMetrics.pluginRequestCounter.GetMetricWith(newLabels(
+		)
-				queryDataOKCounterLabels,
+		require.NoError(t, err)
-				prometheus.Labels{
+		require.Equal(t, 1.0, testutil.ToFloat64(counter))
-					labelStatusSource: string(backend.ErrorSourceDownstream),
-				}),
-			)
-			require.Error(t, err)
-			require.ErrorContains(t, err, "inconsistent label cardinality")
-		})
-
-		t.Run("Should add error_source label if feature flag is enabled", func(t *testing.T) {
-			metricsMw.pluginMetrics.pluginRequestCounter.Reset()
-
-			cdt.TestClient.QueryDataFunc = func(ctx context.Context, req *backend.QueryDataRequest) (*backend.QueryDataResponse, error) {
-				return &backend.QueryDataResponse{Responses: map[string]backend.DataResponse{"A": downstreamErrorResponse}}, nil
-			}
-			_, err := cdt.Decorator.QueryData(context.Background(), &backend.QueryDataRequest{PluginContext: pCtx})
-			require.NoError(t, err)
-			counter, err := metricsMw.pluginMetrics.pluginRequestCounter.GetMetricWith(newLabels(
-				queryDataErrorCounterLabels,
-				prometheus.Labels{
-					labelStatusSource: string(backend.ErrorSourceDownstream),
-				}),
-			)
-			require.NoError(t, err)
-			require.Equal(t, 1.0, testutil.ToFloat64(counter))
-		})
 	})
 
 	t.Run("Priority", func(t *testing.T) {

pkg/services/pluginsintegration/pluginsintegration.go

@@ -153,12 +153,8 @@ func NewClientDecorator(
 }
 
 func CreateMiddlewares(cfg *setting.Cfg, oAuthTokenService oauthtoken.OAuthTokenService, tracer tracing.Tracer, cachingService caching.CachingService, features *featuremgmt.FeatureManager, promRegisterer prometheus.Registerer, registry registry.Service) []plugins.ClientMiddleware {
-	var middlewares []plugins.ClientMiddleware
+	middlewares := []plugins.ClientMiddleware{
-
+		clientmiddleware.NewPluginRequestMetaMiddleware(),
-	if features.IsEnabledGlobally(featuremgmt.FlagPluginsInstrumentationStatusSource) {
-		middlewares = []plugins.ClientMiddleware{
-			clientmiddleware.NewPluginRequestMetaMiddleware(),
-		}
 	}
 
 	skipCookiesNames := []string{cfg.LoginCookieName}
@@ -189,11 +185,9 @@ func CreateMiddlewares(cfg *setting.Cfg, oAuthTokenService oauthtoken.OAuthToken
 
 	middlewares = append(middlewares, clientmiddleware.NewHTTPClientMiddleware())
 
-	if features.IsEnabledGlobally(featuremgmt.FlagPluginsInstrumentationStatusSource) {
+	// StatusSourceMiddleware should be at the very bottom, or any middlewares below it won't see the
-		// StatusSourceMiddleware should be at the very bottom, or any middlewares below it won't see the
+	// correct status source in their context.Context
-		// correct status source in their context.Context
+	middlewares = append(middlewares, clientmiddleware.NewStatusSourceMiddleware())
-		middlewares = append(middlewares, clientmiddleware.NewStatusSourceMiddleware())
-	}
 
 	return middlewares
 }