Graphite: fix autocomplete when tags are not available (#31680)

* Return empty list of tags when tags are not available

In some configurations graphite-web fail to return the list of tags. It shouldn't however prevent displaying list of metrics (which is concatenated with tags).

* Populate jsonData with default version of Graphite

The version of Graphite is preselected in the dropdown but was not saved in jsonData initially.

* Fix a typo

* Show a popup with an error message

* Always use the latest Graphite value as the default one when creating a datasource

* Move autocomplete error handling to GraphiteQueryCtrl

* Test error handing in Graphite autocomplete

* Test default Graphite version fallback

* Rename graphite_versions.ts to versions.ts

* Remove redundant import

* Code formatting, minor renaming

* Remove redundant error info

public/app/plugins/datasource/graphite/configuration/ConfigEditor.tsx

@@ -3,16 +3,14 @@ import { DataSourceHttpSettings, InlineFormLabel, LegacyForms } from '@grafana/u
 const { Select, Switch } = LegacyForms;
 import {
   DataSourcePluginOptionsEditorProps,
+  updateDatasourcePluginJsonDataOption,
   onUpdateDatasourceJsonDataOptionSelect,
   onUpdateDatasourceJsonDataOptionChecked,
 } from '@grafana/data';
 import { GraphiteOptions, GraphiteType } from '../types';
+import { DEFAULT_GRAPHITE_VERSION, GRAPHITE_VERSIONS } from '../versions';
 
-const graphiteVersions = [
-  { label: '0.9.x', value: '0.9' },
-  { label: '1.0.x', value: '1.0' },
-  { label: '1.1.x', value: '1.1' },
-];
+const graphiteVersions = GRAPHITE_VERSIONS.map((version) => ({ label: `${version}.x`, value: version }));
 
 const graphiteTypes = Object.entries(GraphiteType).map(([label, value]) => ({
   label,
@@ -40,11 +38,14 @@ export class ConfigEditor extends PureComponent<Props> {
     );
   };
 
+  componentDidMount() {
+    updateDatasourcePluginJsonDataOption(this.props, 'graphiteVersion', this.currentGraphiteVersion);
+  }
+
   render() {
     const { options, onOptionsChange } = this.props;
 
-    const currentVersion =
-      graphiteVersions.find((item) => item.value === options.jsonData.graphiteVersion) ?? graphiteVersions[2];
+    const currentVersion = graphiteVersions.find((item) => item.value === this.currentGraphiteVersion);
 
     return (
       <>
@@ -96,4 +97,8 @@ export class ConfigEditor extends PureComponent<Props> {
       </>
     );
   }
+
+  private get currentGraphiteVersion() {
+    return this.props.options.jsonData.graphiteVersion || DEFAULT_GRAPHITE_VERSION;
+  }
 }

public/app/plugins/datasource/graphite/datasource.ts

@@ -18,6 +18,7 @@ import { getTemplateSrv, TemplateSrv } from 'app/features/templating/template_sr
 import { GraphiteOptions, GraphiteQuery, GraphiteType, MetricTankRequestMeta } from './types';
 import { getRollupNotice, getRuntimeConsolidationNotice } from 'app/plugins/datasource/graphite/meta';
 import { getSearchFilterScopedVar } from '../../../features/variables/utils';
+import { DEFAULT_GRAPHITE_VERSION } from './versions';
 
 export class GraphiteDatasource extends DataSourceApi<GraphiteQuery, GraphiteOptions> {
   basicAuth: string;
@@ -38,7 +39,9 @@ export class GraphiteDatasource extends DataSourceApi<GraphiteQuery, GraphiteOpt
     this.basicAuth = instanceSettings.basicAuth;
     this.url = instanceSettings.url;
     this.name = instanceSettings.name;
-    this.graphiteVersion = instanceSettings.jsonData.graphiteVersion || '1.1';
+    // graphiteVersion is set when a datasource is created but it hadn't been set in the past so we're
+    // still falling back to the default behavior here for backwards compatibility (see also #17429)
+    this.graphiteVersion = instanceSettings.jsonData.graphiteVersion || DEFAULT_GRAPHITE_VERSION;
     this.isMetricTank = instanceSettings.jsonData.graphiteType === GraphiteType.Metrictank;
     this.supportsTags = supportsTags(this.graphiteVersion);
     this.cacheTimeout = instanceSettings.cacheTimeout;
@@ -449,7 +452,7 @@ export class GraphiteDatasource extends DataSourceApi<GraphiteQuery, GraphiteOpt
     });
   }
 
-  getTagsAutoComplete(expressions: any[], tagPrefix: any, optionalOptions: any) {
+  getTagsAutoComplete(expressions: any[], tagPrefix: any, optionalOptions?: any) {
     const options = optionalOptions || {};
 
     const httpOptions: any = {

public/app/plugins/datasource/graphite/query_ctrl.ts

@@ -4,11 +4,12 @@ import './func_editor';
 import _ from 'lodash';
 import GraphiteQuery from './graphite_query';
 import { QueryCtrl } from 'app/plugins/sdk';
-import appEvents from 'app/core/app_events';
 import { promiseToDigest } from 'app/core/utils/promiseToDigest';
 import { auto } from 'angular';
 import { TemplateSrv } from '@grafana/runtime';
-import { AppEvents } from '@grafana/data';
+import { dispatch } from 'app/store/store';
+import { notifyApp } from 'app/core/actions';
+import { createErrorNotification } from 'app/core/copy/appNotification';
 
 const GRAPHITE_TAG_OPERATORS = ['=', '!=', '=~', '!=~'];
 const TAG_PREFIX = 'tag: ';
@@ -23,6 +24,9 @@ export class GraphiteQueryCtrl extends QueryCtrl {
   supportsTags: boolean;
   paused: boolean;
 
+  // to avoid error flooding, it's shown only once per session
+  private _tagsAutoCompleteErrorShown = false;
+
   /** @ngInject */
   constructor(
     $scope: any,
@@ -106,7 +110,7 @@ export class GraphiteQueryCtrl extends QueryCtrl {
         }
       })
       .catch((err: any) => {
-        appEvents.emit(AppEvents.alertError, ['Error', err]);
+        dispatch(notifyApp(createErrorNotification('Error', err)));
       });
   }
 
@@ -331,24 +335,34 @@ export class GraphiteQueryCtrl extends QueryCtrl {
 
   getTags(index: number, tagPrefix: any) {
     const tagExpressions = this.queryModel.renderTagExpressions(index);
-    return this.datasource.getTagsAutoComplete(tagExpressions, tagPrefix).then((values: any) => {
-      const altTags = _.map(values, 'text');
-      altTags.splice(0, 0, this.removeTagValue);
-      return mapToDropdownOptions(altTags);
-    });
+    return this.datasource
+      .getTagsAutoComplete(tagExpressions, tagPrefix)
+      .then((values: any) => {
+        const altTags = _.map(values, 'text');
+        altTags.splice(0, 0, this.removeTagValue);
+        return mapToDropdownOptions(altTags);
+      })
+      .catch((err: any) => {
+        this.handleTagsAutoCompleteError(err);
+      });
   }
 
   getTagsAsSegments(tagPrefix: string) {
     const tagExpressions = this.queryModel.renderTagExpressions();
-    return this.datasource.getTagsAutoComplete(tagExpressions, tagPrefix).then((values: any) => {
-      return _.map(values, (val) => {
-        return this.uiSegmentSrv.newSegment({
-          value: val.text,
-          type: 'tag',
-          expandable: false,
+    return this.datasource
+      .getTagsAutoComplete(tagExpressions, tagPrefix)
+      .then((values: any) => {
+        return _.map(values, (val) => {
+          return this.uiSegmentSrv.newSegment({
+            value: val.text,
+            type: 'tag',
+            expandable: false,
+          });
         });
+      })
+      .catch((err: any) => {
+        this.handleTagsAutoCompleteError(err);
       });
-    });
   }
 
   getTagOperators() {
@@ -415,6 +429,14 @@ export class GraphiteQueryCtrl extends QueryCtrl {
   getCollapsedText() {
     return this.target.target;
   }
+
+  private handleTagsAutoCompleteError(error: Error): void {
+    console.log(error);
+    if (!this._tagsAutoCompleteErrorShown) {
+      this._tagsAutoCompleteErrorShown = true;
+      dispatch(notifyApp(createErrorNotification(`Fetching tags failed: ${error.message}.`)));
+    }
+  }
 }
 
 function mapToDropdownOptions(results: any[]) {

public/app/plugins/datasource/graphite/specs/datasource.test.ts

@@ -4,6 +4,7 @@ import _ from 'lodash';
 import { TemplateSrv } from 'app/features/templating/template_srv';
 import { dateTime, getFrameDisplayName } from '@grafana/data';
 import { backendSrv } from 'app/core/services/backend_srv'; // will use the version in __mocks__
+import { DEFAULT_GRAPHITE_VERSION } from '../versions';
 
 jest.mock('@grafana/runtime', () => ({
   ...((jest.requireActual('@grafana/runtime') as unknown) as object),
@@ -35,6 +36,10 @@ describe('graphiteDatasource', () => {
     ctx = { templateSrv, ds };
   });
 
+  it('uses default Graphite version when no graphiteVersion is provided', () => {
+    expect(ctx.ds.graphiteVersion).toBe(DEFAULT_GRAPHITE_VERSION);
+  });
+
   describe('convertResponseToDataFrames', () => {
     it('should transform regular result', () => {
       const result = ctx.ds.convertResponseToDataFrames({

public/app/plugins/datasource/graphite/specs/query_ctrl.test.ts

@@ -1,7 +1,9 @@
+import { dispatch } from 'app/store/store';
 import { uiSegmentSrv } from 'app/core/services/segment_srv';
 import gfunc from '../gfunc';
 import { GraphiteQueryCtrl } from '../query_ctrl';
 import { TemplateSrvStub } from 'test/specs/helpers';
+import { silenceConsoleOutput } from 'test/core/utils/silenceConsoleOutput';
 
 jest.mock('app/core/utils/promiseToDigest', () => ({
   promiseToDigest: (scope: any) => {
@@ -9,6 +11,11 @@ jest.mock('app/core/utils/promiseToDigest', () => ({
   },
 }));
 
+jest.mock('app/store/store', () => ({
+  dispatch: jest.fn(),
+}));
+const mockDispatch = dispatch as jest.Mock;
+
 describe('GraphiteQueryCtrl', () => {
   const ctx = {
     datasource: {
@@ -17,6 +24,7 @@ describe('GraphiteQueryCtrl', () => {
       getFuncDef: gfunc.getFuncDef,
       waitForFuncDefsLoaded: jest.fn(() => Promise.resolve(null)),
       createFuncInstance: gfunc.createFuncInstance,
+      getTagsAutoComplete: jest.fn().mockReturnValue(Promise.resolve([])),
     },
     target: { target: 'aliasByNode(scaleToSeconds(test.prod.*,1),2)' },
     panelCtrl: {
@@ -29,6 +37,7 @@ describe('GraphiteQueryCtrl', () => {
   };
 
   beforeEach(() => {
+    jest.clearAllMocks();
     GraphiteQueryCtrl.prototype.target = ctx.target;
     GraphiteQueryCtrl.prototype.datasource = ctx.datasource;
     GraphiteQueryCtrl.prototype.panelCtrl = ctx.panelCtrl;
@@ -184,6 +193,55 @@ describe('GraphiteQueryCtrl', () => {
     });
   });
 
+  describe('when autocomplete for tags is not available', () => {
+    silenceConsoleOutput();
+    beforeEach(() => {
+      ctx.ctrl.datasource.getTagsAutoComplete.mockReturnValue(
+        new Promise(() => {
+          throw new Error();
+        })
+      );
+    });
+
+    it('getTags should handle autocomplete errors', async () => {
+      await expect(async () => {
+        await ctx.ctrl.getTags(0, 'any');
+        expect(mockDispatch).toBeCalledWith(
+          expect.objectContaining({
+            type: 'appNotifications/notifyApp',
+          })
+        );
+      }).not.toThrow();
+    });
+
+    it('getTags should display the error message only once', async () => {
+      await ctx.ctrl.getTags(0, 'any');
+      expect(mockDispatch.mock.calls.length).toBe(1);
+
+      await ctx.ctrl.getTags(0, 'any');
+      expect(mockDispatch.mock.calls.length).toBe(1);
+    });
+
+    it('getTagsAsSegments should handle autocomplete errors', async () => {
+      await expect(async () => {
+        await ctx.ctrl.getTagsAsSegments('any');
+        expect(mockDispatch).toBeCalledWith(
+          expect.objectContaining({
+            type: 'appNotifications/notifyApp',
+          })
+        );
+      }).not.toThrow();
+    });
+
+    it('getTagsAsSegments should display the error message only once', async () => {
+      await ctx.ctrl.getTagsAsSegments('any');
+      expect(mockDispatch.mock.calls.length).toBe(1);
+
+      await ctx.ctrl.getTagsAsSegments('any');
+      expect(mockDispatch.mock.calls.length).toBe(1);
+    });
+  });
+
   describe('targetChanged', () => {
     beforeEach(() => {
       ctx.ctrl.target.target = 'aliasByNode(scaleToSeconds(test.prod.*, 1), 2)';

public/app/plugins/datasource/graphite/versions.ts

@@ -0,0 +1,5 @@
+import { last } from 'lodash';
+
+export const GRAPHITE_VERSIONS = ['0.9', '1.0', '1.1'];
+
+export const DEFAULT_GRAPHITE_VERSION = last(GRAPHITE_VERSIONS)!;