From df3b68e6599c0212c7160051a5925e91ad7cf80b Mon Sep 17 00:00:00 2001 From: Ivana Huckova <30407135+ivanahuckova@users.noreply.github.com> Date: Tue, 25 May 2021 20:29:59 +0200 Subject: [PATCH] Prometheus: Update default HTTP method to POST for existing data sources (#34599) (#34676) * Change default method to POST * Update comment * Add tests * Update Whats new * Update docs/sources/whatsnew/whats-new-in-v8-0.md Co-authored-by: Julien Pivotto * Update docs/sources/whatsnew/whats-new-in-v8-0.md Co-authored-by: achatterjee-grafana <70489351+achatterjee-grafana@users.noreply.github.com> * Update docs/sources/whatsnew/whats-new-in-v8-0.md Co-authored-by: Julien Pivotto Co-authored-by: achatterjee-grafana <70489351+achatterjee-grafana@users.noreply.github.com> (cherry picked from commit 93c5c8345c4d301ee6c6a816751f29504fdc6fe0) --- docs/sources/administration/provisioning.md | 2 +- docs/sources/whatsnew/whats-new-in-v8-0.md | 11 +- .../configuration/PromSettings.test.tsx | 14 +-- .../prometheus/configuration/PromSettings.tsx | 9 +- .../datasource/prometheus/datasource.test.ts | 104 +++++++++++++----- .../datasource/prometheus/datasource.ts | 2 +- 6 files changed, 90 insertions(+), 52 deletions(-) diff --git a/docs/sources/administration/provisioning.md b/docs/sources/administration/provisioning.md index a65976dc3f3..d3be3fd37eb 100644 --- a/docs/sources/administration/provisioning.md +++ b/docs/sources/administration/provisioning.md @@ -152,7 +152,7 @@ Since not all datasources have the same configuration settings we only have the | timeInterval | string | Prometheus, Elasticsearch, InfluxDB, MySQL, PostgreSQL and MSSQL | Lowest interval/step value that should be used for this data source. | | httpMode | string | Influxdb | HTTP Method. 'GET', 'POST', defaults to GET | | maxSeries | number | Influxdb | Max number of series/tables that Grafana processes | -| httpMethod | string | Prometheus | HTTP Method. 'GET', 'POST', defaults to GET | +| httpMethod | string | Prometheus | HTTP Method. 'GET', 'POST', defaults to POST | | customQueryParameters | string | Prometheus | Query parameters to add, as a URL-encoded string. | | esVersion | string | Elasticsearch | Elasticsearch version (E.g. `7.0.0`, `7.6.1`) | | timeField | string | Elasticsearch | Which field that should be used as timestamp | diff --git a/docs/sources/whatsnew/whats-new-in-v8-0.md b/docs/sources/whatsnew/whats-new-in-v8-0.md index 993616fd9a8..5fe6acac5ca 100644 --- a/docs/sources/whatsnew/whats-new-in-v8-0.md +++ b/docs/sources/whatsnew/whats-new-in-v8-0.md @@ -32,7 +32,8 @@ New visualization that allows categorical data display. Following the new panel ### Time series panel updates -The Time series is out of beta! We are removing the `Beta`tag and graduating the Time series panel to a stable state. +The Time series is out of beta! We are removing the `Beta`tag and graduating the Time series panel to a stable state. + - **Time series** is now the default visualization option, replacing the **Graph (old)**. - The Time series panel now supports stacking. For more information, refer to [Graph stacked time series]({{< relref "../panels/visualizations/time-series/graph-time-series-stacking.md" >}}). - You can now add alerts in the Time series panel, just like the old Graph panel. @@ -88,7 +89,7 @@ Grafana 8.0 includes many performance enhancements. #### Initial startup and load performance -We reduced the Grafana initial download size massively, approximately 40%. This means that on slower or mobile connections, the initial login page or home dashboard will load much faster. +We reduced the Grafana initial download size massively, approximately 40%. This means that on slower or mobile connections, the initial login page or home dashboard will load much faster. All panels that have migrated from Flot to uPlot will also render two to three times faster because the library is much more efficient. Right now, this includes the Time series, Stat, Timeline, Histogram, and Barchart panel visualizations. @@ -179,7 +180,7 @@ You can now configure generic OAuth with empty scopes. This allows OAuth Identit ##### Added OAuth support for strict parsing of role_attribute_path -You can now configure generic OAuth with strict parsing of the `role_attribute_path`. By default, if th `role_attribute_path` property does not return a role, then the user is assigned the `Viewer` role. You can disable the role assignment by setting `role_attribute_strict = true`. It denies user access if no role or an invalid role is returned. +You can now configure generic OAuth with strict parsing of the `role_attribute_path`. By default, if the `role_attribute_path` property does not return a role, then the user is assigned the `Viewer` role. You can disable the role assignment by setting `role_attribute_strict = true`. It denies user access if no role or an invalid role is returned. ## Enterprise features @@ -211,3 +212,7 @@ Documentation was updated to reflect these changes. ### Elasticsearch: Use application/x-ndjson content type for multi-search requests For multi-search requests, we now use the correct application/x-ndjson content type instead of the incorrect application/json. Although this should be transparent to most of the users, if you are running Elasticsearch behind a proxy, then be sure that your proxy correctly handles requests with this content type. + +### Prometheus: Update default HTTP method to POST for existing data sources + +The default HTTP method for Prometheus data source is now POST, previously it was GET. The POST APIs are there since January 2018 (Prometheus 2.1.0) and they have fewer limitations than the GET APIs. Users with Prometheus instance with version < 2.1.0 that use the default HTTP method should update their HTTP method to GET. diff --git a/public/app/plugins/datasource/prometheus/configuration/PromSettings.test.tsx b/public/app/plugins/datasource/prometheus/configuration/PromSettings.test.tsx index ae426979d3d..fd4525b09de 100644 --- a/public/app/plugins/datasource/prometheus/configuration/PromSettings.test.tsx +++ b/public/app/plugins/datasource/prometheus/configuration/PromSettings.test.tsx @@ -88,7 +88,7 @@ describe('PromSettings', () => { describe('PromSettings component', () => { const defaultProps = createDefaultConfigOptions(); - it('should show POST httpMethod if no httpMethod and no url', () => { + it('should show POST httpMethod if no httpMethod', () => { const options = defaultProps; options.url = ''; options.jsonData.httpMethod = ''; @@ -100,18 +100,6 @@ describe('PromSettings', () => { ); expect(screen.getByText('POST')).toBeInTheDocument(); }); - it('should show GET httpMethod if no httpMethod and url', () => { - const options = defaultProps; - options.url = 'test_url'; - options.jsonData.httpMethod = ''; - - render( -
- {}} options={options} /> -
- ); - expect(screen.getByText('GET')).toBeInTheDocument(); - }); it('should show POST httpMethod if POST httpMethod is configured', () => { const options = defaultProps; options.url = 'test_url'; diff --git a/public/app/plugins/datasource/prometheus/configuration/PromSettings.tsx b/public/app/plugins/datasource/prometheus/configuration/PromSettings.tsx index 6a0386d6364..fab61014329 100644 --- a/public/app/plugins/datasource/prometheus/configuration/PromSettings.tsx +++ b/public/app/plugins/datasource/prometheus/configuration/PromSettings.tsx @@ -20,15 +20,10 @@ type Props = Pick, 'options' | ' export const PromSettings = (props: Props) => { const { options, onOptionsChange } = props; - /** - * We want to change the default httpMethod to 'POST' for all of the new Prometheus data sources instances (no url) added in 7.5+. - * We are explicitly adding httpMethod, as previously it could be undefined and defaulted to 'GET'. - * Undefined httpMethod is still going to be considered 'GET' for backward compatibility reasons, but if users open data - * source settings it is going to be set to 'GET' explicitly and it will be selected in httpMethod dropdown as 'GET'. - * */ + // We are explicitly adding httpMethod so it is correctly displayed in dropdown. This way, it is more predictable for users. if (!options.jsonData.httpMethod) { - options.url ? (options.jsonData.httpMethod = 'GET') : (options.jsonData.httpMethod = 'POST'); + options.jsonData.httpMethod = 'POST'; } return ( diff --git a/public/app/plugins/datasource/prometheus/datasource.test.ts b/public/app/plugins/datasource/prometheus/datasource.test.ts index d4878c5ce87..3a90c7dc976 100644 --- a/public/app/plugins/datasource/prometheus/datasource.test.ts +++ b/public/app/plugins/datasource/prometheus/datasource.test.ts @@ -127,12 +127,6 @@ describe('PrometheusDatasource', () => { }); describe('customQueryParams', () => { - const promDs = new PrometheusDatasource( - { ...instanceSettings, jsonData: { customQueryParameters: 'customQuery=123' } as any }, - templateSrvStub as any, - timeSrvStub as any - ); - const target = { expr: 'test{job="testjob"}', format: 'time_series', refId: '' }; function makeQuery(target: PromQuery) { return { @@ -141,31 +135,87 @@ describe('PrometheusDatasource', () => { interval: '60s', } as any; } - it('added to metadata request', () => { - promDs.metadataRequest('/foo'); - expect(fetchMock.mock.calls.length).toBe(1); - expect(fetchMock.mock.calls[0][0].url).toBe('proxied/foo?customQuery=123'); - }); - it('adds params to timeseries query', () => { - promDs.query(makeQuery(target)); - expect(fetchMock.mock.calls.length).toBe(1); - expect(fetchMock.mock.calls[0][0].url).toBe( - 'proxied/api/v1/query_range?query=test%7Bjob%3D%22testjob%22%7D&start=60&end=180&step=60&customQuery=123' + describe('with GET http method', () => { + const promDs = new PrometheusDatasource( + { ...instanceSettings, jsonData: { customQueryParameters: 'customQuery=123', httpMethod: 'GET' } as any }, + templateSrvStub as any, + timeSrvStub as any ); - }); - it('adds params to exemplars query', () => { - promDs.query(makeQuery({ ...target, exemplar: true })); - // We do also range query for single exemplars target - expect(fetchMock.mock.calls.length).toBe(2); - expect(fetchMock.mock.calls[0][0].url).toContain('&customQuery=123'); - expect(fetchMock.mock.calls[1][0].url).toContain('&customQuery=123'); + + it('added to metadata request', () => { + promDs.metadataRequest('/foo'); + expect(fetchMock.mock.calls.length).toBe(1); + expect(fetchMock.mock.calls[0][0].url).toBe('proxied/foo?customQuery=123'); + }); + + it('adds params to timeseries query', () => { + promDs.query(makeQuery(target)); + expect(fetchMock.mock.calls.length).toBe(1); + expect(fetchMock.mock.calls[0][0].url).toBe( + 'proxied/api/v1/query_range?query=test%7Bjob%3D%22testjob%22%7D&start=60&end=180&step=60&customQuery=123' + ); + }); + it('adds params to exemplars query', () => { + promDs.query(makeQuery({ ...target, exemplar: true })); + // We do also range query for single exemplars target + expect(fetchMock.mock.calls.length).toBe(2); + expect(fetchMock.mock.calls[0][0].url).toContain('&customQuery=123'); + expect(fetchMock.mock.calls[1][0].url).toContain('&customQuery=123'); + }); + + it('adds params to instant query', () => { + promDs.query(makeQuery({ ...target, instant: true })); + expect(fetchMock.mock.calls.length).toBe(1); + expect(fetchMock.mock.calls[0][0].url).toContain('&customQuery=123'); + }); }); - it('adds params to instant query', () => { - promDs.query(makeQuery({ ...target, instant: true })); - expect(fetchMock.mock.calls.length).toBe(1); - expect(fetchMock.mock.calls[0][0].url).toContain('&customQuery=123'); + describe('with POST http method', () => { + const promDs = new PrometheusDatasource( + { ...instanceSettings, jsonData: { customQueryParameters: 'customQuery=123', httpMethod: 'POST' } as any }, + templateSrvStub as any, + timeSrvStub as any + ); + + it('added to metadata request with non-POST endpoint', () => { + promDs.metadataRequest('/foo'); + expect(fetchMock.mock.calls.length).toBe(1); + expect(fetchMock.mock.calls[0][0].url).toBe('proxied/foo?customQuery=123'); + }); + + it('added to metadata request with POST endpoint', () => { + promDs.metadataRequest('/api/v1/labels'); + expect(fetchMock.mock.calls.length).toBe(1); + expect(fetchMock.mock.calls[0][0].url).toBe('proxied/api/v1/labels'); + expect(fetchMock.mock.calls[0][0].data.customQuery).toBe('123'); + }); + + it('adds params to timeseries query', () => { + promDs.query(makeQuery(target)); + expect(fetchMock.mock.calls.length).toBe(1); + expect(fetchMock.mock.calls[0][0].url).toBe('proxied/api/v1/query_range'); + expect(fetchMock.mock.calls[0][0].data).toEqual({ + customQuery: '123', + query: 'test{job="testjob"}', + step: 60, + end: 180, + start: 60, + }); + }); + it('adds params to exemplars query', () => { + promDs.query(makeQuery({ ...target, exemplar: true })); + // We do also range query for single exemplars target + expect(fetchMock.mock.calls.length).toBe(2); + expect(fetchMock.mock.calls[0][0].data.customQuery).toBe('123'); + expect(fetchMock.mock.calls[1][0].data.customQuery).toBe('123'); + }); + + it('adds params to instant query', () => { + promDs.query(makeQuery({ ...target, instant: true })); + expect(fetchMock.mock.calls.length).toBe(1); + expect(fetchMock.mock.calls[0][0].data.customQuery).toBe('123'); + }); }); }); diff --git a/public/app/plugins/datasource/prometheus/datasource.ts b/public/app/plugins/datasource/prometheus/datasource.ts index bfcbcec3fb5..91ccc0ff170 100644 --- a/public/app/plugins/datasource/prometheus/datasource.ts +++ b/public/app/plugins/datasource/prometheus/datasource.ts @@ -78,7 +78,7 @@ export class PrometheusDatasource extends DataSourceApi this.withCredentials = instanceSettings.withCredentials; this.interval = instanceSettings.jsonData.timeInterval || '15s'; this.queryTimeout = instanceSettings.jsonData.queryTimeout; - this.httpMethod = instanceSettings.jsonData.httpMethod || 'GET'; + this.httpMethod = instanceSettings.jsonData.httpMethod || 'POST'; this.directUrl = instanceSettings.jsonData.directUrl; this.exemplarTraceIdDestinations = instanceSettings.jsonData.exemplarTraceIdDestinations; this.ruleMappings = {};