Expressions: Remove feature toggle (#30316)

* Expressions: remove feature toggle, add experimental badge
* Make button only show for backend and mixed data sources

Co-authored-by: Peter Holmberg <peter.hlmbrg@gmail.com>
Co-authored-by: Torkel Ödegaard <torkel@grafana.com>

packages/grafana-data/src/types/config.ts

@@ -33,7 +33,6 @@ export interface BuildInfo {
  */
 export interface FeatureToggles {
   live: boolean;
-  expressions: boolean;
   ngalert: boolean;
   panelLibrary: boolean;
 

packages/grafana-runtime/src/config.ts

@@ -53,7 +53,6 @@ export class GrafanaBootConfig implements GrafanaConfig {
   pluginsToPreload: string[] = [];
   featureToggles: FeatureToggles = {
     live: false,
-    expressions: false,
     meta: false,
     ngalert: false,
     panelLibrary: false,

packages/grafana-ui/src/components/InfoBox/FeatureInfoBox.tsx

@@ -40,11 +40,12 @@ const getFeatureInfoBoxStyles = stylesFactory((theme: GrafanaTheme) => {
 
 interface FeatureBadgeProps {
   featureState: FeatureState;
+  tooltip?: string;
 }
 
-export const FeatureBadge: React.FC<FeatureBadgeProps> = ({ featureState }) => {
+export const FeatureBadge: React.FC<FeatureBadgeProps> = ({ featureState, tooltip }) => {
   const display = getPanelStateBadgeDisplayModel(featureState);
-  return <Badge text={display.text} color={display.color} icon={display.icon} />;
+  return <Badge text={display.text} color={display.color} icon={display.icon} tooltip={tooltip} />;
 };
 
 function getPanelStateBadgeDisplayModel(featureState: FeatureState): BadgeProps {

packages/grafana-ui/src/components/index.ts

@@ -101,7 +101,7 @@ export { DataLinkInput } from './DataLinks/DataLinkInput';
 export { DataLinksContextMenu } from './DataLinks/DataLinksContextMenu';
 export { SeriesIcon } from './VizLegend/SeriesIcon';
 export { InfoBox } from './InfoBox/InfoBox';
-export { FeatureInfoBox } from './InfoBox/FeatureInfoBox';
+export { FeatureBadge, FeatureInfoBox } from './InfoBox/FeatureInfoBox';
 
 export { JSONFormatter } from './JSONFormatter/JSONFormatter';
 export { JsonExplorer } from './JSONFormatter/json_explorer/json_explorer';

pkg/api/metrics.go

@@ -28,16 +28,19 @@ func (hs *HTTPServer) QueryMetricsV2(c *models.ReqContext, reqDTO dtos.MetricReq
 		TimeRange: tsdb.NewTimeRange(reqDTO.From, reqDTO.To),
 		Debug:     reqDTO.Debug,
 		User:      c.SignedInUser,
+		Queries:   make([]*tsdb.Query, 0, len(reqDTO.Queries)),
+	}
+
+	// Loop to see if we have an expression.
+	for _, query := range reqDTO.Queries {
+		if query.Get("datasource").MustString("") == expr.DatasourceName {
+			return hs.handleExpressions(c, reqDTO)
+		}
 	}
 
-	hasExpr := false
 	var ds *models.DataSource
 	for i, query := range reqDTO.Queries {
 		hs.log.Debug("Processing metrics query", "query", query)
-		name := query.Get("datasource").MustString("")
-		if name == expr.DatasourceName {
-			hasExpr = true
-		}
 
 		datasourceID, err := query.Get("datasourceId").Int64()
 		if err != nil {
@@ -45,17 +48,13 @@ func (hs *HTTPServer) QueryMetricsV2(c *models.ReqContext, reqDTO dtos.MetricReq
 			return response.Error(400, "Query missing data source ID", nil)
 		}
 
-		if i == 0 && !hasExpr {
+		// For mixed datasource case, each data source is sent in a single request.
+		// So only the datasource from the first query is needed. As all requests
+		// should be the same data source.
+		if i == 0 {
 			ds, err = hs.DatasourceCache.GetDatasource(datasourceID, c.SignedInUser, c.SkipCache)
 			if err != nil {
-				hs.log.Debug("Encountered error getting data source", "err", err, "id", datasourceID)
-				if errors.Is(err, models.ErrDataSourceAccessDenied) {
-					return response.Error(403, "Access denied to data source", err)
-				}
-				if errors.Is(err, models.ErrDataSourceNotFound) {
-					return response.Error(400, "Invalid data source ID", err)
-				}
-				return response.Error(500, "Unable to load data source metadata", err)
+				return hs.handleGetDataSourceError(err, datasourceID)
 			}
 		}
 
@@ -69,22 +68,62 @@ func (hs *HTTPServer) QueryMetricsV2(c *models.ReqContext, reqDTO dtos.MetricReq
 		})
 	}
 
-	var resp *tsdb.Response
-	var err error
-	if !hasExpr {
-		resp, err = tsdb.HandleRequest(c.Req.Context(), ds, request)
-		if err != nil {
-			return response.Error(500, "Metric request error", err)
-		}
-	} else {
-		if !hs.Cfg.IsExpressionsEnabled() {
-			return response.Error(404, "Expressions feature toggle is not enabled", nil)
+	resp, err := tsdb.HandleRequest(c.Req.Context(), ds, request)
+	if err != nil {
+		return response.Error(500, "Metric request error", err)
+	}
+
+	statusCode := 200
+	for _, res := range resp.Results {
+		if res.Error != nil {
+			res.ErrorString = res.Error.Error()
+			resp.Message = res.ErrorString
+			statusCode = 400
 		}
+	}
 
-		resp, err = expr.WrapTransformData(c.Req.Context(), request)
+	return response.JSONStreaming(statusCode, resp)
+}
+
+// handleExpressions handles POST /api/ds/query when there is an expression.
+func (hs *HTTPServer) handleExpressions(c *models.ReqContext, reqDTO dtos.MetricRequest) response.Response {
+	request := &tsdb.TsdbQuery{
+		TimeRange: tsdb.NewTimeRange(reqDTO.From, reqDTO.To),
+		Debug:     reqDTO.Debug,
+		User:      c.SignedInUser,
+		Queries:   make([]*tsdb.Query, 0, len(reqDTO.Queries)),
+	}
+
+	for _, query := range reqDTO.Queries {
+		hs.log.Debug("Processing metrics query", "query", query)
+		name := query.Get("datasource").MustString("")
+
+		datasourceID, err := query.Get("datasourceId").Int64()
 		if err != nil {
-			return response.Error(500, "Transform request error", err)
+			hs.log.Debug("Can't process query since it's missing data source ID")
+			return response.Error(400, "Query missing data source ID", nil)
 		}
+
+		if name != expr.DatasourceName {
+			// Expression requests have everything in one request, so need to check
+			// all data source queries for possible permission / not found issues.
+			if _, err = hs.DatasourceCache.GetDatasource(datasourceID, c.SignedInUser, c.SkipCache); err != nil {
+				return hs.handleGetDataSourceError(err, datasourceID)
+			}
+		}
+
+		request.Queries = append(request.Queries, &tsdb.Query{
+			RefId:         query.Get("refId").MustString("A"),
+			MaxDataPoints: query.Get("maxDataPoints").MustInt64(100),
+			IntervalMs:    query.Get("intervalMs").MustInt64(1000),
+			QueryType:     query.Get("queryType").MustString(""),
+			Model:         query,
+		})
+	}
+
+	resp, err := expr.WrapTransformData(c.Req.Context(), request)
+	if err != nil {
+		return response.Error(500, "expression request error", err)
 	}
 
 	statusCode := 200
@@ -99,6 +138,17 @@ func (hs *HTTPServer) QueryMetricsV2(c *models.ReqContext, reqDTO dtos.MetricReq
 	return response.JSONStreaming(statusCode, resp)
 }
 
+func (hs *HTTPServer) handleGetDataSourceError(err error, datasourceID int64) *response.NormalResponse {
+	hs.log.Debug("Encountered error getting data source", "err", err, "id", datasourceID)
+	if errors.Is(err, models.ErrDataSourceAccessDenied) {
+		return response.Error(403, "Access denied to data source", err)
+	}
+	if errors.Is(err, models.ErrDataSourceNotFound) {
+		return response.Error(400, "Invalid data source ID", err)
+	}
+	return response.Error(500, "Unable to load data source metadata", err)
+}
+
 // QueryMetrics returns query metrics
 // POST /api/tsdb/query
 func (hs *HTTPServer) QueryMetrics(c *models.ReqContext, reqDto dtos.MetricRequest) response.Response {
@@ -115,10 +165,7 @@ func (hs *HTTPServer) QueryMetrics(c *models.ReqContext, reqDto dtos.MetricReque
 
 	ds, err := hs.DatasourceCache.GetDatasource(datasourceId, c.SignedInUser, c.SkipCache)
 	if err != nil {
-		if errors.Is(err, models.ErrDataSourceAccessDenied) {
-			return response.Error(403, "Access denied to datasource", err)
-		}
-		return response.Error(500, "Unable to load datasource meta data", err)
+		return hs.handleGetDataSourceError(err, datasourceId)
 	}
 
 	request := &tsdb.TsdbQuery{

pkg/setting/setting.go

@@ -341,11 +341,6 @@ type Cfg struct {
 	AutoAssignOrgRole string
 }
 
-// IsExpressionsEnabled returns whether the expressions feature is enabled.
-func (cfg Cfg) IsExpressionsEnabled() bool {
-	return cfg.FeatureToggles["expressions"]
-}
-
 // IsLiveEnabled returns if grafana live should be enabled
 func (cfg Cfg) IsLiveEnabled() bool {
 	return cfg.FeatureToggles["live"]

public/app/features/query/components/QueryGroup.tsx

@@ -2,7 +2,7 @@
 import React, { PureComponent } from 'react';
 // Components
 import { DataSourcePicker } from 'app/core/components/Select/DataSourcePicker';
-import { Button, CustomScrollbar, HorizontalGroup, Modal, stylesFactory } from '@grafana/ui';
+import { Button, CustomScrollbar, HorizontalGroup, Icon, Modal, stylesFactory, Tooltip } from '@grafana/ui';
 import { getDataSourceSrv } from '@grafana/runtime';
 import { QueryEditorRows } from './QueryEditorRows';
 // Services
@@ -173,7 +173,7 @@ export class QueryGroup extends PureComponent<Props, State> {
     this.props.onRunQueries();
   };
 
-  renderTopSection(styles: QueriesTabStyls) {
+  renderTopSection(styles: QueriesTabStyles) {
     const { onOpenQueryInspector, options } = this.props;
     const { dataSource, data } = this.state;
 
@@ -294,7 +294,11 @@ export class QueryGroup extends PureComponent<Props, State> {
     );
   }
 
-  renderAddQueryRow(dsSettings: DataSourceInstanceSettings) {
+  isExpressionsSupported(dsSettings: DataSourceInstanceSettings): boolean {
+    return (dsSettings.meta.alerting || dsSettings.meta.mixed) === true;
+  }
+
+  renderAddQueryRow(dsSettings: DataSourceInstanceSettings, styles: QueriesTabStyles) {
     const { isAddingMixed } = this.state;
     const showAddButton = !(isAddingMixed || isSharedDashboardQuery(dsSettings.name));
 
@@ -311,10 +315,17 @@ export class QueryGroup extends PureComponent<Props, State> {
           </Button>
         )}
         {isAddingMixed && this.renderMixedPicker()}
-        {config.featureToggles.expressions && (
-          <Button icon="plus" onClick={this.onAddExpressionClick} variant="secondary">
-            Expression
-          </Button>
+        {this.isExpressionsSupported(dsSettings) && (
+          <Tooltip content="Experimental feature, queries might break in next version">
+            <Button
+              icon="plus"
+              onClick={this.onAddExpressionClick}
+              variant="secondary"
+              className={styles.expressionButton}
+            >
+              Expression <Icon name="exclamation-triangle" className="muted" size="sm" />
+            </Button>
+          </Tooltip>
         )}
       </HorizontalGroup>
     );
@@ -337,7 +348,7 @@ export class QueryGroup extends PureComponent<Props, State> {
           {dsSettings && (
             <>
               <div className={styles.queriesWrapper}>{this.renderQueries(dsSettings)}</div>
-              {this.renderAddQueryRow(dsSettings)}
+              {this.renderAddQueryRow(dsSettings, styles)}
               {isHelpOpen && (
                 <Modal title="Data source help" isOpen={true} onDismiss={this.onCloseHelp}>
                   <PluginHelp plugin={dsSettings.meta} type="query_help" />
@@ -375,7 +386,11 @@ const getStyles = stylesFactory(() => {
     queriesWrapper: css`
       padding-bottom: 16px;
     `,
+    expressionWrapper: css``,
+    expressionButton: css`
+      margin-right: ${theme.spacing.sm};
+    `,
   };
 });
 
-type QueriesTabStyls = ReturnType<typeof getStyles>;
+type QueriesTabStyles = ReturnType<typeof getStyles>;