From 53817b74295fe44249a8a9d8786158cac169acb0 Mon Sep 17 00:00:00 2001 From: David Kaltschmidt Date: Fri, 20 Apr 2018 15:28:04 +0200 Subject: [PATCH 1/3] Add silent option to backend requests * When set to `true`, the `silent` option for backend_srv requests suppresses all event emitters that are triggered when the response is received. * Added `helperRequest()` to the Prometheus datasource to support requests that are not triggered by the user, e.g., for tab completion. `helperRequest()` sets the `silent` option. * Migrated all non-timeseries queries of the Prometheus datasource to use `helperRequest()`. Fixes #11673 --- public/app/core/services/backend_srv.ts | 9 ++++++--- .../plugins/datasource/prometheus/datasource.ts | 16 +++++++++++++++- .../datasource/prometheus/metric_find_query.ts | 8 ++++---- 3 files changed, 25 insertions(+), 8 deletions(-) diff --git a/public/app/core/services/backend_srv.ts b/public/app/core/services/backend_srv.ts index 8b7ca518e8b..d582b6a3b18 100644 --- a/public/app/core/services/backend_srv.ts +++ b/public/app/core/services/backend_srv.ts @@ -170,7 +170,9 @@ export class BackendSrv { return this.$http(options) .then(response => { - appEvents.emit('ds-request-response', response); + if (!options.silent) { + appEvents.emit('ds-request-response', response); + } return response; }) .catch(err => { @@ -201,8 +203,9 @@ export class BackendSrv { if (err.data && !err.data.message && _.isString(err.data.error)) { err.data.message = err.data.error; } - - appEvents.emit('ds-request-error', err); + if (!options.silent) { + appEvents.emit('ds-request-error', err); + } throw err; }) .finally(() => { diff --git a/public/app/plugins/datasource/prometheus/datasource.ts b/public/app/plugins/datasource/prometheus/datasource.ts index 6cf6c713a90..3eceaf6c622 100644 --- a/public/app/plugins/datasource/prometheus/datasource.ts +++ b/public/app/plugins/datasource/prometheus/datasource.ts @@ -81,6 +81,20 @@ export class PrometheusDatasource { return this.backendSrv.datasourceRequest(options); } + // Use this for tab completion features, wont publish response to other components + helperRequest(url) { + const options: any = { + url: this.url + url, + silent: true, + }; + + if (this.basicAuth || this.withCredentials) { + options.withCredentials = true; + } + + return this.backendSrv.datasourceRequest(options); + } + interpolateQueryExpr(value, variable, defaultFormatFn) { // if no multi or include all do not regexEscape if (!variable.multi && !variable.includeAll) { @@ -229,7 +243,7 @@ export class PrometheusDatasource { ); } - return this._request('GET', url).then(result => { + return this.helperRequest(url).then(result => { this.metricsNameCache = { data: result.data.data, expire: Date.now() + 60 * 1000, diff --git a/public/app/plugins/datasource/prometheus/metric_find_query.ts b/public/app/plugins/datasource/prometheus/metric_find_query.ts index b27f1cd50af..c58f5c097b9 100644 --- a/public/app/plugins/datasource/prometheus/metric_find_query.ts +++ b/public/app/plugins/datasource/prometheus/metric_find_query.ts @@ -46,7 +46,7 @@ export default class PrometheusMetricFindQuery { // return label values globally url = '/api/v1/label/' + label + '/values'; - return this.datasource._request('GET', url).then(function(result) { + return this.datasource.helperRequest(url).then(function(result) { return _.map(result.data.data, function(value) { return { text: value }; }); @@ -56,7 +56,7 @@ export default class PrometheusMetricFindQuery { var end = this.datasource.getPrometheusTime(this.range.to, true); url = '/api/v1/series?match[]=' + encodeURIComponent(metric) + '&start=' + start + '&end=' + end; - return this.datasource._request('GET', url).then(function(result) { + return this.datasource.helperRequest(url).then(function(result) { var _labels = _.map(result.data.data, function(metric) { return metric[label] || ''; }).filter(function(label) { @@ -76,7 +76,7 @@ export default class PrometheusMetricFindQuery { metricNameQuery(metricFilterPattern) { var url = '/api/v1/label/__name__/values'; - return this.datasource._request('GET', url).then(function(result) { + return this.datasource.helperRequest(url).then(function(result) { return _.chain(result.data.data) .filter(function(metricName) { var r = new RegExp(metricFilterPattern); @@ -120,7 +120,7 @@ export default class PrometheusMetricFindQuery { var url = '/api/v1/series?match[]=' + encodeURIComponent(query) + '&start=' + start + '&end=' + end; var self = this; - return this.datasource._request('GET', url).then(function(result) { + return this.datasource.helperRequest(url).then(function(result) { return _.map(result.data.data, function(metric) { return { text: self.datasource.getOriginalMetricName(metric), From 006286ac05b7b74434bafaa0b4ecbd51e33bb0ac Mon Sep 17 00:00:00 2001 From: David Kaltschmidt Date: Tue, 24 Apr 2018 12:27:37 +0200 Subject: [PATCH 2/3] Renamed helperRequest and removed positional args From review feedback: * s/helper/metadata * combined positional args to _request into options dict * metadataRequest reuses _request() * moved consumption of this.httpMethod into _request, can be overwritten in options due to spread-after --- .../datasource/prometheus/datasource.ts | 30 +++++++------------ .../prometheus/metric_find_query.ts | 8 ++--- 2 files changed, 15 insertions(+), 23 deletions(-) diff --git a/public/app/plugins/datasource/prometheus/datasource.ts b/public/app/plugins/datasource/prometheus/datasource.ts index 3eceaf6c622..3a2c78dce2d 100644 --- a/public/app/plugins/datasource/prometheus/datasource.ts +++ b/public/app/plugins/datasource/prometheus/datasource.ts @@ -5,6 +5,7 @@ import kbn from 'app/core/utils/kbn'; import * as dateMath from 'app/core/utils/datemath'; import PrometheusMetricFindQuery from './metric_find_query'; import { ResultTransformer } from './result_transformer'; +import { BackendSrv } from 'app/core/services/backend_srv'; export function prometheusRegularEscape(value) { return value.replace(/'/g, "\\\\'"); @@ -29,7 +30,7 @@ export class PrometheusDatasource { resultTransformer: ResultTransformer; /** @ngInject */ - constructor(instanceSettings, private $q, private backendSrv, private templateSrv, private timeSrv) { + constructor(instanceSettings, private $q, private backendSrv: BackendSrv, private templateSrv, private timeSrv) { this.type = 'prometheus'; this.editorSrc = 'app/features/prometheus/partials/query.editor.html'; this.name = instanceSettings.name; @@ -43,13 +44,13 @@ export class PrometheusDatasource { this.resultTransformer = new ResultTransformer(templateSrv); } - _request(method, url, data?, requestId?) { + _request(url, data?, options?: any) { var options: any = { url: this.url + url, - method: method, - requestId: requestId, + method: this.httpMethod, + ...options, }; - if (method === 'GET') { + if (options.method === 'GET') { if (!_.isEmpty(data)) { options.url = options.url + @@ -82,17 +83,8 @@ export class PrometheusDatasource { } // Use this for tab completion features, wont publish response to other components - helperRequest(url) { - const options: any = { - url: this.url + url, - silent: true, - }; - - if (this.basicAuth || this.withCredentials) { - options.withCredentials = true; - } - - return this.backendSrv.datasourceRequest(options); + metadataRequest(url) { + return this._request(url, null, { silent: true }); } interpolateQueryExpr(value, variable, defaultFormatFn) { @@ -220,7 +212,7 @@ export class PrometheusDatasource { end: end, step: query.step, }; - return this._request(this.httpMethod, url, data, query.requestId); + return this._request(url, data, { requestId: query.requestId }); } performInstantQuery(query, time) { @@ -229,7 +221,7 @@ export class PrometheusDatasource { query: query.expr, time: time, }; - return this._request(this.httpMethod, url, data, query.requestId); + return this._request(url, data, { requestId: query.requestId }); } performSuggestQuery(query, cache = false) { @@ -243,7 +235,7 @@ export class PrometheusDatasource { ); } - return this.helperRequest(url).then(result => { + return this.metadataRequest(url).then(result => { this.metricsNameCache = { data: result.data.data, expire: Date.now() + 60 * 1000, diff --git a/public/app/plugins/datasource/prometheus/metric_find_query.ts b/public/app/plugins/datasource/prometheus/metric_find_query.ts index c58f5c097b9..337cd74c14c 100644 --- a/public/app/plugins/datasource/prometheus/metric_find_query.ts +++ b/public/app/plugins/datasource/prometheus/metric_find_query.ts @@ -46,7 +46,7 @@ export default class PrometheusMetricFindQuery { // return label values globally url = '/api/v1/label/' + label + '/values'; - return this.datasource.helperRequest(url).then(function(result) { + return this.datasource.metadataRequest(url).then(function(result) { return _.map(result.data.data, function(value) { return { text: value }; }); @@ -56,7 +56,7 @@ export default class PrometheusMetricFindQuery { var end = this.datasource.getPrometheusTime(this.range.to, true); url = '/api/v1/series?match[]=' + encodeURIComponent(metric) + '&start=' + start + '&end=' + end; - return this.datasource.helperRequest(url).then(function(result) { + return this.datasource.metadataRequest(url).then(function(result) { var _labels = _.map(result.data.data, function(metric) { return metric[label] || ''; }).filter(function(label) { @@ -76,7 +76,7 @@ export default class PrometheusMetricFindQuery { metricNameQuery(metricFilterPattern) { var url = '/api/v1/label/__name__/values'; - return this.datasource.helperRequest(url).then(function(result) { + return this.datasource.metadataRequest(url).then(function(result) { return _.chain(result.data.data) .filter(function(metricName) { var r = new RegExp(metricFilterPattern); @@ -120,7 +120,7 @@ export default class PrometheusMetricFindQuery { var url = '/api/v1/series?match[]=' + encodeURIComponent(query) + '&start=' + start + '&end=' + end; var self = this; - return this.datasource.helperRequest(url).then(function(result) { + return this.datasource.metadataRequest(url).then(function(result) { return _.map(result.data.data, function(metric) { return { text: self.datasource.getOriginalMetricName(metric), From 707700ac7dd1d938f281c110113d274907417692 Mon Sep 17 00:00:00 2001 From: David Kaltschmidt Date: Tue, 24 Apr 2018 16:26:46 +0200 Subject: [PATCH 3/3] force GET for metadataRequests, w/ test --- .../datasource/prometheus/datasource.ts | 2 +- .../prometheus/specs/datasource.jest.ts | 20 +++++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/public/app/plugins/datasource/prometheus/datasource.ts b/public/app/plugins/datasource/prometheus/datasource.ts index 3a2c78dce2d..cbf701a0abe 100644 --- a/public/app/plugins/datasource/prometheus/datasource.ts +++ b/public/app/plugins/datasource/prometheus/datasource.ts @@ -84,7 +84,7 @@ export class PrometheusDatasource { // Use this for tab completion features, wont publish response to other components metadataRequest(url) { - return this._request(url, null, { silent: true }); + return this._request(url, null, { method: 'GET', silent: true }); } interpolateQueryExpr(value, variable, defaultFormatFn) { diff --git a/public/app/plugins/datasource/prometheus/specs/datasource.jest.ts b/public/app/plugins/datasource/prometheus/specs/datasource.jest.ts index d2620b93bbc..a997a2d8233 100644 --- a/public/app/plugins/datasource/prometheus/specs/datasource.jest.ts +++ b/public/app/plugins/datasource/prometheus/specs/datasource.jest.ts @@ -14,6 +14,7 @@ describe('PrometheusDatasource', () => { }; ctx.backendSrvMock = {}; + ctx.templateSrvMock = { replace: a => a, }; @@ -23,6 +24,25 @@ describe('PrometheusDatasource', () => { ctx.ds = new PrometheusDatasource(instanceSettings, q, ctx.backendSrvMock, ctx.templateSrvMock, ctx.timeSrvMock); }); + describe('Datasource metadata requests', () => { + it('should perform a GET request with the default config', () => { + ctx.backendSrvMock.datasourceRequest = jest.fn(); + ctx.ds.metadataRequest('/foo'); + expect(ctx.backendSrvMock.datasourceRequest.mock.calls.length).toBe(1); + expect(ctx.backendSrvMock.datasourceRequest.mock.calls[0][0].method).toBe('GET'); + }); + + it('should still perform a GET request with the DS HTTP method set to POST', () => { + ctx.backendSrvMock.datasourceRequest = jest.fn(); + const postSettings = _.cloneDeep(instanceSettings); + postSettings.jsonData.httpMethod = 'POST'; + const ds = new PrometheusDatasource(postSettings, q, ctx.backendSrvMock, ctx.templateSrvMock, ctx.timeSrvMock); + ds.metadataRequest('/foo'); + expect(ctx.backendSrvMock.datasourceRequest.mock.calls.length).toBe(1); + expect(ctx.backendSrvMock.datasourceRequest.mock.calls[0][0].method).toBe('GET'); + }); + }); + describe('When converting prometheus histogram to heatmap format', () => { beforeEach(() => { ctx.query = {