From 2191fe1285e7667db7760040cee10f4ae3868ac7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Torkel=20=C3=96degaard?= Date: Thu, 9 Jul 2020 07:32:39 +0200 Subject: [PATCH] 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 --- .../src/utils/DataSourceWithBackend.test.ts | 7 +++- .../src/utils/DataSourceWithBackend.ts | 42 ++++++------------- .../core/services/__mocks__/backend_srv.ts | 16 +++++-- .../app_insights_datasource.test.ts | 25 ++++++----- .../azure_log_analytics_datasource.test.ts | 3 +- .../azure_log_analytics_datasource.ts | 21 +++++++++- 6 files changed, 66 insertions(+), 48 deletions(-) diff --git a/packages/grafana-runtime/src/utils/DataSourceWithBackend.test.ts b/packages/grafana-runtime/src/utils/DataSourceWithBackend.test.ts index f8a20e4b388..622fcd4bd93 100644 --- a/packages/grafana-runtime/src/utils/DataSourceWithBackend.test.ts +++ b/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 { constructor(instanceSettings: DataSourceInstanceSettings) { @@ -11,7 +12,9 @@ class MyDataSource extends DataSourceWithBackend const mockDatasourceRequest = jest.fn(); const backendSrv = ({ - datasourceRequest: mockDatasourceRequest, + fetch: (options: BackendSrvRequest) => { + return of(mockDatasourceRequest(options)); + }, } as unknown) as BackendSrv; jest.mock('../services', () => ({ diff --git a/packages/grafana-runtime/src/utils/DataSourceWithBackend.ts b/packages/grafana-runtime/src/utils/DataSourceWithBackend.ts index 558a36d65b4..aeaf063b9cb 100644 --- a/packages/grafana-runtime/src/utils/DataSourceWithBackend.ts +++ b/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 = 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; - /** * Override to skip executing a query * diff --git a/public/app/core/services/__mocks__/backend_srv.ts b/public/app/core/services/__mocks__/backend_srv.ts index cbbd475c206..85b62afafac 100644 --- a/public/app/core/services/__mocks__/backend_srv.ts +++ b/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); diff --git a/public/app/plugins/datasource/grafana-azure-monitor-datasource/app_insights/app_insights_datasource.test.ts b/public/app/plugins/datasource/grafana-azure-monitor-datasource/app_insights/app_insights_datasource.test.ts index c176638a77e..02608d1b976 100644 --- a/public/app/plugins/datasource/grafana-azure-monitor-datasource/app_insights/app_insights_datasource.test.ts +++ b/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); }); }); diff --git a/public/app/plugins/datasource/grafana-azure-monitor-datasource/azure_log_analytics/azure_log_analytics_datasource.test.ts b/public/app/plugins/datasource/grafana-azure-monitor-datasource/azure_log_analytics/azure_log_analytics_datasource.test.ts index 8a999a88872..609f11f113e 100644 --- a/public/app/plugins/datasource/grafana-azure-monitor-datasource/azure_log_analytics/azure_log_analytics_datasource.test.ts +++ b/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, diff --git a/public/app/plugins/datasource/grafana-azure-monitor-datasource/azure_log_analytics/azure_log_analytics_datasource.ts b/public/app/plugins/datasource/grafana-azure-monitor-datasource/azure_log_analytics/azure_log_analytics_datasource.ts index 90a17335a97..54e7767f666 100644 --- a/public/app/plugins/datasource/grafana-azure-monitor-datasource/azure_log_analytics/azure_log_analytics_datasource.ts +++ b/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): Observable { + return super.query(request).pipe( + mergeMap((res: DataQueryResponse) => { + return from(this.processResponse(res)); + }) + ); + } + async processResponse(res: DataQueryResponse): Promise { if (res.data) { for (const df of res.data) {