From d466fcac1ca9346c06baa80a4d33f6637ea2ab97 Mon Sep 17 00:00:00 2001 From: "Grot (@grafanabot)" <43478413+grafanabot@users.noreply.github.com> Date: Mon, 2 May 2022 13:19:47 -0400 Subject: [PATCH] Graphite Datasource: add responseType: 'text' to http options to return full list of functions (#47663) (#48594) * add response type text to graphite datasource http options to return full list of functions * add comment for adding response type text to call to graphite /functions endpoint * Add tests for invalid and valid JSON mocking backendSrv fromFetch * remove unnecessary code from tests * remove extra logic for graphite /functions endpoint returning {} #46681 * add graphite functions list logic back in to see why alert test broke * fix conflict message * fix conflicts * fix issues with rebase, add responseType text back in, remove extra graphite functions list logic checks * add email for license/cla check (cherry picked from commit 4867a6b15f5a8c8516f12b6afe1443de9932ef07) Co-authored-by: Brendan O'Handley --- .../datasource/graphite/datasource.test.ts | 8 - .../plugins/datasource/graphite/datasource.ts | 37 ++--- .../graphite/datasource_integration.test.ts | 147 ++++++++++++++++++ 3 files changed, 159 insertions(+), 33 deletions(-) create mode 100644 public/app/plugins/datasource/graphite/datasource_integration.test.ts diff --git a/public/app/plugins/datasource/graphite/datasource.test.ts b/public/app/plugins/datasource/graphite/datasource.test.ts index 23c5695a8ff..54ce71f387a 100644 --- a/public/app/plugins/datasource/graphite/datasource.test.ts +++ b/public/app/plugins/datasource/graphite/datasource.test.ts @@ -303,14 +303,6 @@ describe('graphiteDatasource', () => { }, }); }); - - it('should use hardcoded list of functions when no functions are returned', async () => { - fetchMock.mockImplementation(() => { - return of(createFetchResponse('{}')); - }); - const funcDefs = await ctx.ds.getFuncDefs(); - expect(Object.keys(funcDefs)).not.toHaveLength(0); - }); }); describe('building graphite params', () => { diff --git a/public/app/plugins/datasource/graphite/datasource.ts b/public/app/plugins/datasource/graphite/datasource.ts index 4a464822859..a83b90b2380 100644 --- a/public/app/plugins/datasource/graphite/datasource.ts +++ b/public/app/plugins/datasource/graphite/datasource.ts @@ -26,7 +26,8 @@ import { getRollupNotice, getRuntimeConsolidationNotice } from 'app/plugins/data import { getSearchFilterScopedVar } from '../../../features/variables/utils'; import gfunc, { FuncDefs, FuncInstance } from './gfunc'; -import { default as GraphiteQueryModel } from './graphite_query'; +import GraphiteQueryModel from './graphite_query'; +// Types import { GraphiteLokiMapping, GraphiteMetricLokiMatcher, @@ -750,35 +751,21 @@ export class GraphiteDatasource const httpOptions = { method: 'GET', url: '/functions', + // add responseType because if this is not defined, + // backend_srv defaults to json + responseType: 'text', }; return lastValueFrom( this.doGraphiteRequest(httpOptions).pipe( map((results: any) => { - if (results.status !== 200 || typeof results.data !== 'object') { - if (typeof results.data === 'string') { - // Fix for a Graphite bug: https://github.com/graphite-project/graphite-web/issues/2609 - // There is a fix for it https://github.com/graphite-project/graphite-web/pull/2612 but - // it was merged to master in July 2020 but it has never been released (the last Graphite - // release was 1.1.7 - March 2020). The bug was introduced in Graphite 1.1.7, in versions - // 1.1.0 - 1.1.6 /functions endpoint returns a valid JSON - const fixedData = JSON.parse(results.data.replace(/"default": ?Infinity/g, '"default": 1e9999')); - this.funcDefs = gfunc.parseFuncDefs(fixedData); - } else { - this.funcDefs = gfunc.getFuncDefs(this.graphiteVersion); - } - } else { - this.funcDefs = gfunc.parseFuncDefs(results.data); - } - - // When /functions endpoint returns application/json response but containing invalid JSON the fix above - // wont' be triggered due to the changes in https://github.com/grafana/grafana/pull/45598 (parsing happens - // in fetch and Graphite receives an empty object and no error). In such cases, when the provided JSON - // seems empty we fallback to the hardcoded list of functions. - // See also: https://github.com/grafana/grafana/issues/45948 - if (Object.keys(this.funcDefs).length === 0) { - this.funcDefs = gfunc.getFuncDefs(this.graphiteVersion); - } + // Fix for a Graphite bug: https://github.com/graphite-project/graphite-web/issues/2609 + // There is a fix for it https://github.com/graphite-project/graphite-web/pull/2612 but + // it was merged to master in July 2020 but it has never been released (the last Graphite + // release was 1.1.7 - March 2020). The bug was introduced in Graphite 1.1.7, in versions + // 1.1.0 - 1.1.6 /functions endpoint returns a valid JSON + const fixedData = JSON.parse(results.data.replace(/"default": ?Infinity/g, '"default": 1e9999')); + this.funcDefs = gfunc.parseFuncDefs(fixedData); return this.funcDefs; }), catchError((error: any) => { diff --git a/public/app/plugins/datasource/graphite/datasource_integration.test.ts b/public/app/plugins/datasource/graphite/datasource_integration.test.ts new file mode 100644 index 00000000000..f962383f441 --- /dev/null +++ b/public/app/plugins/datasource/graphite/datasource_integration.test.ts @@ -0,0 +1,147 @@ +import { of } from 'rxjs'; + +import { setBackendSrv } from '@grafana/runtime'; +import { BackendSrv } from 'app/core/services/backend_srv'; + +import { ContextSrv, User } from '../../../core/services/context_srv'; + +import { GraphiteDatasource } from './datasource'; + +interface Context { + ds: GraphiteDatasource; +} + +describe('graphiteDatasource integration with backendSrv and fetch', () => { + let ctx = {} as Context; + + beforeEach(() => { + jest.clearAllMocks(); + + const instanceSettings = { + url: '/api/datasources/proxy/1', + name: 'graphiteProd', + jsonData: { + rollupIndicatorEnabled: true, + }, + }; + const ds = new GraphiteDatasource(instanceSettings); + ctx = { ds }; + }); + + describe('returns a list of functions', () => { + it('should return a list of functions with invalid JSON', async () => { + const INVALID_JSON = + '{"testFunction":{"name":"function","description":"description","module":"graphite.render.functions","group":"Transform","params":[{"name":"param","type":"intOrInf","required":true,"default":Infinity}]}}'; + + mockBackendSrv(INVALID_JSON); + + const funcDefs = await ctx.ds.getFuncDefs(); + + expect(funcDefs).toEqual({ + testFunction: { + category: 'Transform', + defaultParams: ['inf'], + description: 'description', + fake: true, + name: 'function', + params: [ + { + multiple: false, + name: 'param', + optional: false, + options: undefined, + type: 'int_or_infinity', + }, + ], + }, + }); + }); + + it('should return a list of functions with valid JSON', async () => { + const VALID_JSON = + '{"testFunction":{"name":"function","description":"description","module":"graphite.render.functions","group":"Transform","params":[{"name":"param","type":"intOrInf","required":true,"default":1e9999}]}}'; + + mockBackendSrv(VALID_JSON); + + const funcDefs = await ctx.ds.getFuncDefs(); + + expect(funcDefs).toEqual({ + testFunction: { + category: 'Transform', + defaultParams: ['inf'], + description: 'description', + fake: true, + name: 'function', + params: [ + { + multiple: false, + name: 'param', + optional: false, + options: undefined, + type: 'int_or_infinity', + }, + ], + }, + }); + }); + }); +}); + +function mockBackendSrv(data: string) { + const defaults = { + data: '', + ok: true, + status: 200, + statusText: 'Ok', + isSignedIn: true, + orgId: 1337, + redirected: false, + type: 'basic', + url: 'http://localhost:3000/api/some-mock', + }; + + const props = { ...defaults }; + + props.data = data; + + const textMock = jest.fn().mockResolvedValue(props.data); + + const fromFetchMock = jest.fn().mockImplementation(() => { + const mockedResponse = { + ok: props.ok, + status: props.status, + statusText: props.statusText, + text: textMock, + redirected: false, + type: 'basic', + url: 'http://localhost:3000/api/some-mock', + headers: { + method: 'GET', + url: '/functions', + // to work around Graphite returning invalid JSON + responseType: 'text', + }, + }; + return of(mockedResponse); + }); + + const appEventsMock = {} as any; + + const user: User = { + isSignedIn: props.isSignedIn, + orgId: props.orgId, + } as any as User; + const contextSrvMock: ContextSrv = { + user, + } as any as ContextSrv; + const logoutMock = jest.fn(); + + const mockedBackendSrv = new BackendSrv({ + fromFetch: fromFetchMock, + appEvents: appEventsMock, + contextSrv: contextSrvMock, + logout: logoutMock, + }); + + setBackendSrv(mockedBackendSrv); +}