DataSourceWithBackend: Switch to new Observable fetch api (#26043)

* BackendSrv: Observable all the way POC

* starting to unify code paths

* tests pass

* Unified error handling

* Single request path and error handling

* Fixed ts issue

* another ts issu

* Added back old requestId cancellation

* Slow progress trying to grasp the full picture of cancellation

* Updates

* refactoring

* Remove a bunch of stuff from backendSrv

* Removed another function

* Do not show error alerts for data queries

* Muu

* Updated comment

* DataSourceWithBackend: Switch to new Observable fetch api

* fixed ts issue

* unify request options type

* Made query inspector subscribe to backendSrv stream instead of legacy app events

* Add back support for err.isHandled to limit scope

* never show success alerts

* Updated tests

* use ovservable in test

* remove processResponse

* remove processResponse

* trying to get tests to pass :(

* no need for the extra tests

* Fixed processsing

* Fixed tests

* Updated tests to mock fetch call

* lint fixes

Co-authored-by: Ryan McKinley <ryantxu@gmail.com>

packages/grafana-runtime/src/utils/DataSourceWithBackend.test.ts

@@ -1,6 +1,7 @@
-import { BackendSrv } from 'src/services';
+import { BackendSrv, BackendSrvRequest } from 'src/services';
 import { DataSourceWithBackend } from './DataSourceWithBackend';
 import { DataSourceJsonData, DataQuery, DataSourceInstanceSettings, DataQueryRequest } from '@grafana/data';
+import { of } from 'rxjs';
 
 class MyDataSource extends DataSourceWithBackend<DataQuery, DataSourceJsonData> {
   constructor(instanceSettings: DataSourceInstanceSettings<DataSourceJsonData>) {
@@ -11,7 +12,9 @@ class MyDataSource extends DataSourceWithBackend<DataQuery, DataSourceJsonData>
 const mockDatasourceRequest = jest.fn();
 
 const backendSrv = ({
-  datasourceRequest: mockDatasourceRequest,
+  fetch: (options: BackendSrvRequest) => {
+    return of(mockDatasourceRequest(options));
+  },
 } as unknown) as BackendSrv;
 
 jest.mock('../services', () => ({

packages/grafana-runtime/src/utils/DataSourceWithBackend.ts

@@ -7,7 +7,8 @@ import {
   DataSourceJsonData,
   ScopedVars,
 } from '@grafana/data';
-import { Observable, from, of } from 'rxjs';
+import { Observable, of } from 'rxjs';
+import { map, catchError } from 'rxjs/operators';
 import { config } from '..';
 import { getBackendSrv } from '../services';
 import { toDataQueryResponse } from './queryResponse';
@@ -101,42 +102,23 @@ export class DataSourceWithBackend<
       body.to = range.to.valueOf().toString();
     }
 
-    const req: Promise<DataQueryResponse> = getBackendSrv()
-      .datasourceRequest({
+    return getBackendSrv()
+      .fetch({
         url: '/api/ds/query',
         method: 'POST',
         data: body,
         requestId,
       })
-      .then((rsp: any) => {
-        const dqs = toDataQueryResponse(rsp);
-        if (this.processResponse) {
-          return this.processResponse(dqs);
-        }
-        return dqs;
-      })
-      .catch(err => {
-        err.isHandled = true; // Avoid extra popup warning
-        const dqs = toDataQueryResponse(err);
-        if (this.processResponse) {
-          return this.processResponse(dqs);
-        }
-        return dqs;
-      });
-
-    return from(req);
+      .pipe(
+        map((rsp: any) => {
+          return toDataQueryResponse(rsp);
+        }),
+        catchError(err => {
+          return of(toDataQueryResponse(err));
+        })
+      );
   }
 
-  /**
-   * Optionally augment the response before returning the results to the
-   *
-   * NOTE: this was added in 7.1 for azure, and will be removed in 7.2
-   * when the entire response pipeline is Observable
-   *
-   * @internal
-   */
-  processResponse?(res: DataQueryResponse): Promise<DataQueryResponse>;
-
   /**
    * Override to skip executing a query
    *

public/app/core/services/__mocks__/backend_srv.ts

@@ -1,3 +1,9 @@
+import 'whatwg-fetch'; // fetch polyfill needed for Headers
+
+import { BackendSrvRequest, FetchResponse } from '@grafana/runtime';
+import { of } from 'rxjs';
+import { BackendSrv } from '../backend_srv';
+
 /**
  * Creates a pretty bogus prom response. Definitelly needs more work but right now we do not test the contents of the
  * messages anyway.
@@ -20,14 +26,18 @@ function makePromResponse() {
   };
 }
 
-export const backendSrv = {
+export const backendSrv = ({
   get: jest.fn(),
-  getDashboard: jest.fn(),
   getDashboardByUid: jest.fn(),
   getFolderByUid: jest.fn(),
   post: jest.fn(),
   resolveCancelerIfExists: jest.fn(),
   datasourceRequest: jest.fn(() => Promise.resolve(makePromResponse())),
-};
+
+  // Observable support
+  fetch: (options: BackendSrvRequest) => {
+    return of(makePromResponse() as FetchResponse);
+  },
+} as unknown) as BackendSrv;
 
 export const getBackendSrv = jest.fn().mockReturnValue(backendSrv);

public/app/plugins/datasource/grafana-azure-monitor-datasource/app_insights/app_insights_datasource.test.ts

@@ -1,11 +1,13 @@
 import { DataFrame, getFrameDisplayName, toUtc } from '@grafana/data';
 import { TemplateSrv } from 'app/features/templating/template_srv';
-import { backendSrv } from 'app/core/services/backend_srv'; // will use the version in __mocks__
+import { backendSrv } from 'app/core/services/backend_srv';
 import { setBackendSrv } from '@grafana/runtime';
 import AppInsightsDatasource from './app_insights_datasource';
+import { of } from 'rxjs';
 
 const templateSrv = new TemplateSrv();
 
+jest.mock('app/core/services/backend_srv');
 jest.mock('@grafana/runtime', () => ({
   ...((jest.requireActual('@grafana/runtime') as unknown) as object),
   getBackendSrv: () => backendSrv,
@@ -14,6 +16,7 @@ jest.mock('@grafana/runtime', () => ({
 
 describe('AppInsightsDatasource', () => {
   const datasourceRequestMock = jest.spyOn(backendSrv, 'datasourceRequest');
+  const fetchMock = jest.spyOn(backendSrv, 'fetch');
 
   const ctx: any = {};
 
@@ -163,11 +166,11 @@ describe('AppInsightsDatasource', () => {
       };
 
       beforeEach(() => {
-        datasourceRequestMock.mockImplementation((options: any) => {
+        fetchMock.mockImplementation((options: any) => {
           expect(options.url).toContain('/api/ds/query');
           expect(options.data.queries.length).toBe(1);
           expect(options.data.queries[0].refId).toBe('A');
-          return Promise.resolve({ data: response, status: 200 });
+          return of({ data: response, status: 200 } as any);
         });
       });
 
@@ -205,11 +208,11 @@ describe('AppInsightsDatasource', () => {
 
       beforeEach(() => {
         options.targets[0].appInsights.segmentColumn = 'partition';
-        datasourceRequestMock.mockImplementation((options: any) => {
+        fetchMock.mockImplementation((options: any) => {
           expect(options.url).toContain('/api/ds/query');
           expect(options.data.queries.length).toBe(1);
           expect(options.data.queries[0].refId).toBe('A');
-          return Promise.resolve({ data: response, status: 200 });
+          return of({ data: response, status: 200 } as any);
         });
       });
 
@@ -267,13 +270,13 @@ describe('AppInsightsDatasource', () => {
       };
 
       beforeEach(() => {
-        datasourceRequestMock.mockImplementation((options: any) => {
+        fetchMock.mockImplementation((options: any) => {
           expect(options.url).toContain('/api/ds/query');
           expect(options.data.queries.length).toBe(1);
           expect(options.data.queries[0].refId).toBe('A');
           expect(options.data.queries[0].appInsights.rawQueryString).toBeUndefined();
           expect(options.data.queries[0].appInsights.metricName).toBe('exceptions/server');
-          return Promise.resolve({ data: response, status: 200 });
+          return of({ data: response, status: 200 } as any);
         });
       });
 
@@ -313,13 +316,13 @@ describe('AppInsightsDatasource', () => {
 
       beforeEach(() => {
         options.targets[0].appInsights.timeGrain = 'PT30M';
-        datasourceRequestMock.mockImplementation((options: any) => {
+        fetchMock.mockImplementation((options: any) => {
           expect(options.url).toContain('/api/ds/query');
           expect(options.data.queries[0].refId).toBe('A');
           expect(options.data.queries[0].appInsights.query).toBeUndefined();
           expect(options.data.queries[0].appInsights.metricName).toBe('exceptions/server');
           expect(options.data.queries[0].appInsights.timeGrain).toBe('PT30M');
-          return Promise.resolve({ data: response, status: 200 });
+          return of({ data: response, status: 200 } as any);
         });
       });
 
@@ -371,12 +374,12 @@ describe('AppInsightsDatasource', () => {
         beforeEach(() => {
           options.targets[0].appInsights.dimension = 'client/city';
 
-          datasourceRequestMock.mockImplementation((options: any) => {
+          fetchMock.mockImplementation((options: any) => {
             expect(options.url).toContain('/api/ds/query');
             expect(options.data.queries[0].appInsights.rawQueryString).toBeUndefined();
             expect(options.data.queries[0].appInsights.metricName).toBe('exceptions/server');
             expect([...options.data.queries[0].appInsights.dimension]).toMatchObject(['client/city']);
-            return Promise.resolve({ data: response, status: 200 });
+            return of({ data: response, status: 200 } as any);
           });
         });
 

public/app/plugins/datasource/grafana-azure-monitor-datasource/azure_log_analytics/azure_log_analytics_datasource.test.ts

@@ -3,10 +3,11 @@ import FakeSchemaData from './__mocks__/schema';
 import { TemplateSrv } from 'app/features/templating/template_srv';
 import { AzureLogsVariable, KustoSchema } from '../types';
 import { toUtc } from '@grafana/data';
-import { backendSrv } from 'app/core/services/backend_srv'; // will use the version in __mocks__
+import { backendSrv } from 'app/core/services/backend_srv';
 
 const templateSrv = new TemplateSrv();
 
+jest.mock('app/core/services/backend_srv');
 jest.mock('@grafana/runtime', () => ({
   ...((jest.requireActual('@grafana/runtime') as unknown) as object),
   getBackendSrv: () => backendSrv,

public/app/plugins/datasource/grafana-azure-monitor-datasource/azure_log_analytics/azure_log_analytics_datasource.ts

@@ -2,8 +2,16 @@ import _ from 'lodash';
 import LogAnalyticsQuerystringBuilder from '../log_analytics/querystring_builder';
 import ResponseParser from './response_parser';
 import { AzureMonitorQuery, AzureDataSourceJsonData, AzureLogsVariable, AzureQueryType } from '../types';
-import { DataQueryResponse, ScopedVars, DataSourceInstanceSettings, MetricFindValue } from '@grafana/data';
+import {
+  DataQueryRequest,
+  DataQueryResponse,
+  ScopedVars,
+  DataSourceInstanceSettings,
+  MetricFindValue,
+} from '@grafana/data';
 import { getBackendSrv, getTemplateSrv, DataSourceWithBackend } from '@grafana/runtime';
+import { Observable, from } from 'rxjs';
+import { mergeMap } from 'rxjs/operators';
 
 export default class AzureLogAnalyticsDatasource extends DataSourceWithBackend<
   AzureMonitorQuery,
@@ -129,6 +137,17 @@ export default class AzureLogAnalyticsDatasource extends DataSourceWithBackend<
     };
   }
 
+  /**
+   * Augment the results with links back to the azure console
+   */
+  query(request: DataQueryRequest<AzureMonitorQuery>): Observable<DataQueryResponse> {
+    return super.query(request).pipe(
+      mergeMap((res: DataQueryResponse) => {
+        return from(this.processResponse(res));
+      })
+    );
+  }
+
   async processResponse(res: DataQueryResponse): Promise<DataQueryResponse> {
     if (res.data) {
       for (const df of res.data) {