Variables: Multi-select DataSource variables are inconsistently displayed in the Data source picker (#76039)

Always show multi-select DataSource(DS) variables in the DS picker, and display a warning in the panel when a DataSource variable holds multiple values and is not being repeated.

---------

Co-authored-by: Alexandra Vargas <alexa1866@gmail.com>
Co-authored-by: Alexa V <239999+axelavargas@users.noreply.github.com>
Co-authored-by: Torkel Ödegaard <torkel@grafana.com>

.betterer.results

@@ -4088,7 +4088,10 @@ exports[`better eslint`] = {
       [0, 0, 0, "Unexpected any. Specify a different type.", "0"]
     ],
     "public/app/features/templating/template_srv.mock.ts:5381": [
-      [0, 0, 0, "Do not use any type assertions.", "0"]
+      [0, 0, 0, "Do not use any type assertions.", "0"],
+      [0, 0, 0, "Do not use any type assertions.", "1"],
+      [0, 0, 0, "Do not use any type assertions.", "2"],
+      [0, 0, 0, "Do not use any type assertions.", "3"]
     ],
     "public/app/features/templating/template_srv.ts:5381": [
       [0, 0, 0, "Unexpected any. Specify a different type.", "0"],

public/app/features/plugins/datasource_srv.ts

@@ -251,8 +251,10 @@ export class DatasourceSrv implements DataSourceService {
           continue;
         }
         let dsValue = variable.current.value === 'default' ? this.defaultName : variable.current.value;
-        if (Array.isArray(dsValue) && dsValue.length === 1) {
-          // Support for multi-value variables with only one selected datasource
+        // Support for multi-value DataSource (ds) variables
+        if (Array.isArray(dsValue)) {
+          // If the ds variable have multiple selected datasources
+          // We will use the first one
           dsValue = dsValue[0];
         }
         const dsSettings =

public/app/features/query/state/PanelQueryRunner.test.ts

@@ -3,8 +3,9 @@ const applyFieldOverridesMock = jest.fn(); // needs to be first in this file
 import { Subject } from 'rxjs';
 
 // Importing this way to be able to spy on grafana/data
+
 import * as grafanaData from '@grafana/data';
-import { DataSourceApi } from '@grafana/data';
+import { DataSourceApi, TypedVariableModel } from '@grafana/data';
 import { DataSourceSrv, setDataSourceSrv, setEchoSrv } from '@grafana/runtime';
 import { TemplateSrvMock } from 'app/features/templating/template_srv.mock';
 
@@ -46,7 +47,27 @@ jest.mock('app/features/dashboard/services/DashboardSrv', () => ({
 }));
 
 jest.mock('app/features/templating/template_srv', () => ({
-  getTemplateSrv: () => new TemplateSrvMock({}),
+  ...jest.requireActual('app/features/templating/template_srv'),
+  getTemplateSrv: () =>
+    new TemplateSrvMock([
+      {
+        name: 'server',
+        type: 'datasource',
+        current: { text: 'Server1', value: 'server' },
+        options: [{ text: 'Server1', value: 'server1' }],
+      },
+      //multi value variable
+      {
+        name: 'multi',
+        type: 'datasource',
+        multi: true,
+        current: { text: 'Server1,Server2', value: ['server-1', 'server-2'] },
+        options: [
+          { text: 'Server1', value: 'server1' },
+          { text: 'Server2', value: 'server2' },
+        ],
+      },
+    ] as TypedVariableModel[]),
 }));
 
 interface ScenarioContext {
@@ -405,4 +426,53 @@ describe('PanelQueryRunner', () => {
       snapshotData,
     }
   );
+
+  describeQueryRunnerScenario(
+    'shouldAddErrorwhenDatasourceVariableIsMultiple',
+    (ctx) => {
+      it('should add error when datasource variable is multiple and not repeated', async () => {
+        // scopedVars is an object that represent the variables repeated in a panel
+        const scopedVars = {
+          server: { text: 'Server1', value: 'server-1' },
+        };
+
+        // We are spying on the replace method of the TemplateSrvMock to check if the custom format function is being called
+        const spyReplace = jest.spyOn(TemplateSrvMock.prototype, 'replace');
+
+        const response = {
+          data: [
+            {
+              target: 'hello',
+              datapoints: [
+                [1, 1000],
+                [2, 2000],
+              ],
+            },
+          ],
+        };
+
+        const datasource = {
+          name: '${multi}',
+          uid: '${multi}',
+          interval: ctx.dsInterval,
+          query: (options: grafanaData.DataQueryRequest) => {
+            ctx.queryCalledWith = options;
+            return Promise.resolve(response);
+          },
+          getRef: () => ({ type: 'test', uid: 'TestDB-uid' }),
+          testDatasource: jest.fn(),
+        } as unknown as DataSourceApi;
+
+        ctx.runner.shouldAddErrorWhenDatasourceVariableIsMultiple(datasource, scopedVars);
+
+        // the test is checking implementation details :(, but it is the only way to check if the error will be added
+        // if the getTemplateSrv.replace is called with the custom format function,it means we will check
+        // if the error should be added
+        expect(spyReplace.mock.calls[0][2]).toBeInstanceOf(Function);
+      });
+    },
+    {
+      ...defaultPanelConfig,
+    }
+  );
 });

public/app/features/query/state/PanelQueryRunner.ts

@@ -275,6 +275,9 @@ export class PanelQueryRunner {
       return;
     }
 
+    //check if datasource is a variable datasource and if that variable has multiple values
+    const addErroDSVariable = this.shouldAddErrorWhenDatasourceVariableIsMultiple(datasource, scopedVars);
+
     const request: DataQueryRequest = {
       app: app ?? CoreApp.Dashboard,
       requestId: getNextRequestId(),
@@ -327,7 +330,7 @@ export class PanelQueryRunner {
 
       this.lastRequest = request;
 
-      this.pipeToSubject(runRequest(ds, request), panelId);
+      this.pipeToSubject(runRequest(ds, request), panelId, false, addErroDSVariable);
     } catch (err) {
       this.pipeToSubject(
         of({
@@ -341,7 +344,12 @@ export class PanelQueryRunner {
     }
   }
 
-  private pipeToSubject(observable: Observable<PanelData>, panelId?: number, skipPreProcess = false) {
+  private pipeToSubject(
+    observable: Observable<PanelData>,
+    panelId?: number,
+    skipPreProcess = false,
+    addErroDSVariable = false
+  ) {
     if (this.subscription) {
       this.subscription.unsubscribe();
     }
@@ -379,6 +387,17 @@ export class PanelQueryRunner {
 
         this.lastResult = next;
 
+        //add error message if datasource is a variable and has multiple values
+        if (addErroDSVariable) {
+          next.errors = [
+            {
+              message:
+                'Panel is using a variable datasource with multiple values without repeat option. Please configure the panel to be repeated by the same datasource variable.',
+            },
+          ];
+          next.state = LoadingState.Error;
+        }
+
         // Store preprocessed query results for applying overrides later on in the pipeline
         this.subject.next(next);
       },
@@ -451,6 +470,28 @@ export class PanelQueryRunner {
   getLastRequest(): DataQueryRequest | undefined {
     return this.lastRequest;
   }
+
+  shouldAddErrorWhenDatasourceVariableIsMultiple(
+    datasource: DataSourceRef | DataSourceApi | null,
+    scopedVars: ScopedVars | undefined
+  ): boolean {
+    let addWarningMessageMultipleDatasourceVariable = false;
+
+    //If datasource is a variable
+    if (datasource?.uid?.startsWith('${')) {
+      // we can access the raw datasource variable values inside the replace function if we pass a custom format function
+      this.templateSrv.replace(datasource.uid, scopedVars, (value: string | string[]) => {
+        // if the variable has multiple values it means it's not being repeated
+        if (Array.isArray(value) && value.length > 1) {
+          addWarningMessageMultipleDatasourceVariable = true;
+        }
+        // return empty string to avoid replacing the variable
+        return '';
+      });
+    }
+
+    return addWarningMessageMultipleDatasourceVariable;
+  }
 }
 
 async function getDataSource(

public/app/features/templating/template_srv.mock.ts

@@ -1,4 +1,4 @@
-import { ScopedVars, TimeRange, TypedVariableModel } from '@grafana/data';
+import { ScopedVars, TimeRange, TypedVariableModel, VariableOption } from '@grafana/data';
 import { TemplateSrv } from '@grafana/runtime';
 
 import { variableRegex } from '../variables/utils';
@@ -13,18 +13,37 @@ import { variableRegex } from '../variables/utils';
  */
 export class TemplateSrvMock implements TemplateSrv {
   private regex = variableRegex;
-  constructor(private variables: Record<string, string>) {}
+  constructor(private variables: TypedVariableModel[]) {}
 
   getVariables(): TypedVariableModel[] {
-    return Object.keys(this.variables).map((key) => {
-      return {
-        type: 'custom',
-        name: key,
-        label: key,
+    if (!this.variables) {
+      return [];
+    }
+
+    return this.variables.reduce((acc: TypedVariableModel[], variable) => {
+      const commonProps = {
+        type: variable.type ?? 'custom',
+        name: variable.name ?? 'test',
+        label: variable.label ?? 'test',
       };
-      // TODO: we remove this type assertion in a later PR
-      // eslint-disable-next-line @typescript-eslint/consistent-type-assertions
-    }) as TypedVariableModel[];
+      if (variable.type === 'datasource') {
+        acc.push({
+          ...commonProps,
+          current: {
+            text: variable.current?.text,
+            value: variable.current?.value,
+          } as VariableOption,
+          options: variable.options ?? [],
+          multi: variable.multi ?? false,
+          includeAll: variable.includeAll ?? false,
+        } as TypedVariableModel);
+      } else {
+        acc.push({
+          ...commonProps,
+        } as TypedVariableModel);
+      }
+      return acc as TypedVariableModel[];
+    }, []);
   }
 
   replace(target?: string, scopedVars?: ScopedVars, format?: string | Function): string {
@@ -35,8 +54,7 @@ export class TemplateSrvMock implements TemplateSrv {
     this.regex.lastIndex = 0;
 
     return target.replace(this.regex, (match, var1, var2, fmt2, var3, fieldPath, fmt3) => {
-      const variableName = var1 || var2 || var3;
-      return this.variables[variableName];
+      return var1 || var2 || var3;
     });
   }