From a49daf5676bff0ed5651f7a5c419848c807391a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Torkel=20=C3=96degaard?= Date: Mon, 10 Aug 2020 16:08:11 +0200 Subject: [PATCH] BackendSrv: Improves logic for hiding requests from query inspector (#26877) * BackendSrv: Improves logic for hiding requests from query inspector * fixed tests * Update explore query inspector --- .../src/services/backendSrv.ts | 2 +- public/app/core/services/backend_srv.ts | 5 ++ public/app/core/specs/backend_srv.test.ts | 81 ++++--------------- .../components/Inspector/QueryInspector.tsx | 2 +- .../explore/ExploreQueryInspector.tsx | 2 +- .../plugins/datasource/jaeger/datasource.ts | 2 +- .../app/plugins/datasource/loki/datasource.ts | 2 +- .../datasource/prometheus/datasource.ts | 2 +- .../prometheus/metric_find_query.test.ts | 14 ++-- .../plugins/datasource/zipkin/datasource.ts | 2 +- 10 files changed, 34 insertions(+), 80 deletions(-) diff --git a/packages/grafana-runtime/src/services/backendSrv.ts b/packages/grafana-runtime/src/services/backendSrv.ts index 472c563db9c..60bd3eed1ee 100644 --- a/packages/grafana-runtime/src/services/backendSrv.ts +++ b/packages/grafana-runtime/src/services/backendSrv.ts @@ -49,7 +49,7 @@ export type BackendSrvRequest = { /** * Set to to true to not include call in query inspector */ - silent?: boolean; + hideFromInspector?: boolean; /** * The data to send diff --git a/public/app/core/services/backend_srv.ts b/public/app/core/services/backend_srv.ts index f873b399717..38df1149472 100644 --- a/public/app/core/services/backend_srv.ts +++ b/public/app/core/services/backend_srv.ts @@ -118,6 +118,11 @@ export class BackendSrv implements BackendService { } } + if (options.hideFromInspector === undefined) { + // Hide all local non data query calls + options.hideFromInspector = isLocalUrl(options.url) && !isDataQuery(options.url); + } + return options; } diff --git a/public/app/core/specs/backend_srv.test.ts b/public/app/core/specs/backend_srv.test.ts index 7aa9035f0d7..a3dbe4d737d 100644 --- a/public/app/core/specs/backend_srv.test.ts +++ b/public/app/core/specs/backend_srv.test.ts @@ -83,21 +83,22 @@ describe('backendSrv', () => { describe('parseRequestOptions', () => { it.each` retry | url | headers | orgId | noBackendCache | expected - ${undefined} | ${'http://localhost:3000/api/dashboard'} | ${undefined} | ${undefined} | ${undefined} | ${{ retry: 0, url: 'http://localhost:3000/api/dashboard' }} - ${1} | ${'http://localhost:3000/api/dashboard'} | ${{ Authorization: 'Some Auth' }} | ${1} | ${true} | ${{ retry: 1, url: 'http://localhost:3000/api/dashboard', headers: { Authorization: 'Some Auth' } }} - ${undefined} | ${'api/dashboard'} | ${undefined} | ${undefined} | ${undefined} | ${{ retry: 0, url: 'api/dashboard' }} - ${undefined} | ${'/api/dashboard'} | ${undefined} | ${undefined} | ${undefined} | ${{ retry: 0, url: 'api/dashboard' }} - ${undefined} | ${'/api/dashboard/'} | ${undefined} | ${undefined} | ${undefined} | ${{ retry: 0, url: 'api/dashboard/' }} - ${undefined} | ${'/api/dashboard/'} | ${{ Authorization: 'Some Auth' }} | ${undefined} | ${undefined} | ${{ retry: 0, url: 'api/dashboard/', headers: { 'X-DS-Authorization': 'Some Auth' } }} - ${undefined} | ${'/api/dashboard/'} | ${{ Authorization: 'Some Auth' }} | ${1} | ${undefined} | ${{ retry: 0, url: 'api/dashboard/', headers: { 'X-DS-Authorization': 'Some Auth', 'X-Grafana-Org-Id': 1 } }} - ${undefined} | ${'/api/dashboard/'} | ${{ Authorization: 'Some Auth' }} | ${1} | ${true} | ${{ retry: 0, url: 'api/dashboard/', headers: { 'X-DS-Authorization': 'Some Auth', 'X-Grafana-Org-Id': 1, 'X-Grafana-NoCache': 'true' } }} - ${1} | ${'/api/dashboard/'} | ${undefined} | ${undefined} | ${undefined} | ${{ retry: 1, url: 'api/dashboard/' }} - ${1} | ${'/api/dashboard/'} | ${{ Authorization: 'Some Auth' }} | ${undefined} | ${undefined} | ${{ retry: 1, url: 'api/dashboard/', headers: { 'X-DS-Authorization': 'Some Auth' } }} - ${1} | ${'/api/dashboard/'} | ${{ Authorization: 'Some Auth' }} | ${1} | ${undefined} | ${{ retry: 1, url: 'api/dashboard/', headers: { 'X-DS-Authorization': 'Some Auth', 'X-Grafana-Org-Id': 1 } }} - ${1} | ${'/api/dashboard/'} | ${{ Authorization: 'Some Auth' }} | ${1} | ${true} | ${{ retry: 1, url: 'api/dashboard/', headers: { 'X-DS-Authorization': 'Some Auth', 'X-Grafana-Org-Id': 1, 'X-Grafana-NoCache': 'true' } }} + ${undefined} | ${'http://localhost:3000/api/dashboard'} | ${undefined} | ${undefined} | ${undefined} | ${{ hideFromInspector: false, retry: 0, url: 'http://localhost:3000/api/dashboard' }} + ${1} | ${'http://localhost:3000/api/dashboard'} | ${{ Authorization: 'Some Auth' }} | ${1} | ${true} | ${{ hideFromInspector: false, retry: 1, url: 'http://localhost:3000/api/dashboard', headers: { Authorization: 'Some Auth' } }} + ${undefined} | ${'api/dashboard'} | ${undefined} | ${undefined} | ${undefined} | ${{ hideFromInspector: true, retry: 0, url: 'api/dashboard' }} + ${undefined} | ${'/api/dashboard'} | ${undefined} | ${undefined} | ${undefined} | ${{ hideFromInspector: true, retry: 0, url: 'api/dashboard' }} + ${undefined} | ${'/api/dashboard/'} | ${undefined} | ${undefined} | ${undefined} | ${{ hideFromInspector: true, retry: 0, url: 'api/dashboard/' }} + ${undefined} | ${'/api/dashboard/'} | ${{ Authorization: 'Some Auth' }} | ${undefined} | ${undefined} | ${{ hideFromInspector: true, retry: 0, url: 'api/dashboard/', headers: { 'X-DS-Authorization': 'Some Auth' } }} + ${undefined} | ${'/api/dashboard/'} | ${{ Authorization: 'Some Auth' }} | ${1} | ${undefined} | ${{ hideFromInspector: true, retry: 0, url: 'api/dashboard/', headers: { 'X-DS-Authorization': 'Some Auth', 'X-Grafana-Org-Id': 1 } }} + ${undefined} | ${'/api/dashboard/'} | ${{ Authorization: 'Some Auth' }} | ${1} | ${true} | ${{ hideFromInspector: true, retry: 0, url: 'api/dashboard/', headers: { 'X-DS-Authorization': 'Some Auth', 'X-Grafana-Org-Id': 1, 'X-Grafana-NoCache': 'true' } }} + ${1} | ${'/api/dashboard/'} | ${undefined} | ${undefined} | ${undefined} | ${{ hideFromInspector: true, retry: 1, url: 'api/dashboard/' }} + ${1} | ${'/api/dashboard/'} | ${{ Authorization: 'Some Auth' }} | ${undefined} | ${undefined} | ${{ hideFromInspector: true, retry: 1, url: 'api/dashboard/', headers: { 'X-DS-Authorization': 'Some Auth' } }} + ${1} | ${'/api/dashboard/'} | ${{ Authorization: 'Some Auth' }} | ${1} | ${undefined} | ${{ hideFromInspector: true, retry: 1, url: 'api/dashboard/', headers: { 'X-DS-Authorization': 'Some Auth', 'X-Grafana-Org-Id': 1 } }} + ${1} | ${'/api/dashboard/'} | ${{ Authorization: 'Some Auth' }} | ${1} | ${true} | ${{ hideFromInspector: true, retry: 1, url: 'api/dashboard/', headers: { 'X-DS-Authorization': 'Some Auth', 'X-Grafana-Org-Id': 1, 'X-Grafana-NoCache': 'true' } }} + ${undefined} | ${'api/datasources/proxy'} | ${undefined} | ${undefined} | ${undefined} | ${{ hideFromInspector: false, retry: 0, url: 'api/datasources/proxy' }} `( "when called with retry: '$retry', url: '$url' and orgId: '$orgId' then result should be '$expected'", - ({ retry, url, headers, orgId, noBackendCache, expected }) => { + async ({ retry, url, headers, orgId, noBackendCache, expected }) => { const srv = new BackendSrv({ contextSrv: { user: { @@ -107,7 +108,7 @@ describe('backendSrv', () => { } as any); if (noBackendCache) { - srv.withNoBackendCache(async () => { + await srv.withNoBackendCache(async () => { expect(srv['parseRequestOptions']({ retry, url, headers })).toEqual(expected); }); } else { @@ -298,58 +299,6 @@ describe('backendSrv', () => { }); describe('datasourceRequest', () => { - describe('when making a successful call and silent is true', () => { - it('then it should not emit message', async () => { - const url = 'http://localhost:3000/api/some-mock'; - const { backendSrv, appEventsMock, expectRequestCallChain } = getTestContext({ url }); - const options = { url, method: 'GET', silent: true }; - const result = await backendSrv.datasourceRequest(options); - - expect(result).toEqual({ - data: { test: 'hello world' }, - ok: true, - redirected: false, - status: 200, - statusText: 'Ok', - type: 'basic', - url, - config: options, - }); - - expect(appEventsMock.emit).not.toHaveBeenCalled(); - expectRequestCallChain({ url, method: 'GET', silent: true }); - }); - }); - - describe('when making a successful call and silent is not defined', () => { - it('then it should not emit message', async () => { - const url = 'http://localhost:3000/api/some-mock'; - const { backendSrv, expectRequestCallChain } = getTestContext({ url }); - const options = { url, method: 'GET' }; - - let inspectorPacket: any = null; - backendSrv.getInspectorStream().subscribe({ - next: rsp => (inspectorPacket = rsp), - }); - - const result = await backendSrv.datasourceRequest(options); - const expectedResult = { - data: { test: 'hello world' }, - ok: true, - redirected: false, - status: 200, - statusText: 'Ok', - type: 'basic', - url, - config: options, - }; - - expect(result).toEqual(expectedResult); - expect(inspectorPacket).toEqual(expectedResult); - expectRequestCallChain({ url, method: 'GET' }); - }); - }); - describe('when called with the same requestId twice', () => { it('then it should cancel the first call and the first call should be unsubscribed', async () => { const url = '/api/dashboard/'; diff --git a/public/app/features/dashboard/components/Inspector/QueryInspector.tsx b/public/app/features/dashboard/components/Inspector/QueryInspector.tsx index 5540a832f9d..e68a1e441b7 100644 --- a/public/app/features/dashboard/components/Inspector/QueryInspector.tsx +++ b/public/app/features/dashboard/components/Inspector/QueryInspector.tsx @@ -133,7 +133,7 @@ export class QueryInspector extends PureComponent { onDataSourceResponse(response: any) { // ignore silent requests - if (response.config?.silent) { + if (response.config?.hideFromInspector) { return; } diff --git a/public/app/features/explore/ExploreQueryInspector.tsx b/public/app/features/explore/ExploreQueryInspector.tsx index 912a0c954b3..52b932d6551 100644 --- a/public/app/features/explore/ExploreQueryInspector.tsx +++ b/public/app/features/explore/ExploreQueryInspector.tsx @@ -15,7 +15,7 @@ import { getPanelInspectorStyles } from '../dashboard/components/Inspector/style function stripPropsFromResponse(response: any) { // ignore silent requests - if (response.config?.silent) { + if (response.config?.hideFromInspector) { return {}; } diff --git a/public/app/plugins/datasource/jaeger/datasource.ts b/public/app/plugins/datasource/jaeger/datasource.ts index 52cf1ced4cf..7d2e38e239b 100644 --- a/public/app/plugins/datasource/jaeger/datasource.ts +++ b/public/app/plugins/datasource/jaeger/datasource.ts @@ -26,7 +26,7 @@ export class JaegerDatasource extends DataSourceApi { } async metadataRequest(url: string, params?: Record): Promise { - const res = await this._request(url, params, { silent: true }).toPromise(); + const res = await this._request(url, params, { hideFromInspector: true }).toPromise(); return res.data.data; } diff --git a/public/app/plugins/datasource/loki/datasource.ts b/public/app/plugins/datasource/loki/datasource.ts index 7e2a81931b0..b0ae8f88660 100644 --- a/public/app/plugins/datasource/loki/datasource.ts +++ b/public/app/plugins/datasource/loki/datasource.ts @@ -268,7 +268,7 @@ export class LokiDatasource extends DataSourceApi { } async metadataRequest(url: string, params?: Record) { - const res = await this._request(url, params, { silent: true }).toPromise(); + const res = await this._request(url, params, { hideFromInspector: true }).toPromise(); return res.data.data || res.data.values || []; } diff --git a/public/app/plugins/datasource/prometheus/datasource.ts b/public/app/plugins/datasource/prometheus/datasource.ts index 96606b510bb..4e02bd78088 100644 --- a/public/app/plugins/datasource/prometheus/datasource.ts +++ b/public/app/plugins/datasource/prometheus/datasource.ts @@ -145,7 +145,7 @@ export class PrometheusDatasource extends DataSourceApi // Use this for tab completion features, wont publish response to other components metadataRequest(url: string) { - return this._request(url, null, { method: 'GET', silent: true }); + return this._request(url, null, { method: 'GET', hideFromInspector: true }); } interpolateQueryExpr(value: string | string[] = [], variable: any) { diff --git a/public/app/plugins/datasource/prometheus/metric_find_query.test.ts b/public/app/plugins/datasource/prometheus/metric_find_query.test.ts index 098db3620ef..b4dd79cd5bb 100644 --- a/public/app/plugins/datasource/prometheus/metric_find_query.test.ts +++ b/public/app/plugins/datasource/prometheus/metric_find_query.test.ts @@ -73,7 +73,7 @@ describe('PrometheusMetricFindQuery', () => { expect(datasourceRequestMock).toHaveBeenCalledWith({ method: 'GET', url: 'proxied/api/v1/labels', - silent: true, + hideFromInspector: true, headers: {}, }); }); @@ -92,7 +92,7 @@ describe('PrometheusMetricFindQuery', () => { expect(datasourceRequestMock).toHaveBeenCalledWith({ method: 'GET', url: 'proxied/api/v1/label/resource/values', - silent: true, + hideFromInspector: true, headers: {}, }); }); @@ -117,7 +117,7 @@ describe('PrometheusMetricFindQuery', () => { url: `proxied/api/v1/series?match${encodeURIComponent( '[]' )}=metric&start=${raw.from.unix()}&end=${raw.to.unix()}`, - silent: true, + hideFromInspector: true, headers: {}, }); }); @@ -141,7 +141,7 @@ describe('PrometheusMetricFindQuery', () => { method: 'GET', url: 'proxied/api/v1/series?match%5B%5D=metric%7Blabel1%3D%22foo%22%2C+label2%3D%22bar%22%2C+label3%3D%22baz%22%7D&start=1524650400&end=1524654000', - silent: true, + hideFromInspector: true, headers: {}, }); }); @@ -168,7 +168,7 @@ describe('PrometheusMetricFindQuery', () => { url: `proxied/api/v1/series?match${encodeURIComponent( '[]' )}=metric&start=${raw.from.unix()}&end=${raw.to.unix()}`, - silent: true, + hideFromInspector: true, headers: {}, }); }); @@ -187,7 +187,7 @@ describe('PrometheusMetricFindQuery', () => { expect(datasourceRequestMock).toHaveBeenCalledWith({ method: 'GET', url: 'proxied/api/v1/label/__name__/values', - silent: true, + hideFromInspector: true, headers: {}, }); }); @@ -243,7 +243,7 @@ describe('PrometheusMetricFindQuery', () => { url: `proxied/api/v1/series?match${encodeURIComponent('[]')}=${encodeURIComponent( 'up{job="job1"}' )}&start=${raw.from.unix()}&end=${raw.to.unix()}`, - silent: true, + hideFromInspector: true, headers: {}, }); }); diff --git a/public/app/plugins/datasource/zipkin/datasource.ts b/public/app/plugins/datasource/zipkin/datasource.ts index cb019ae3855..aac1abf6ab9 100644 --- a/public/app/plugins/datasource/zipkin/datasource.ts +++ b/public/app/plugins/datasource/zipkin/datasource.ts @@ -36,7 +36,7 @@ export class ZipkinDatasource extends DataSourceApi { } async metadataRequest(url: string, params?: Record): Promise { - const res = await this.request(url, params, { silent: true }).toPromise(); + const res = await this.request(url, params, { hideFromInspector: true }).toPromise(); return res.data; }