diff --git a/public/app/plugins/datasource/graphite/components/GraphiteQueryEditor.tsx b/public/app/plugins/datasource/graphite/components/GraphiteQueryEditor.tsx index 3d5413717a3..cfa65a29086 100644 --- a/public/app/plugins/datasource/graphite/components/GraphiteQueryEditor.tsx +++ b/public/app/plugins/datasource/graphite/components/GraphiteQueryEditor.tsx @@ -53,6 +53,7 @@ function GraphiteQueryEditorContent() { icon="pen" variant="secondary" aria-label="Toggle editor mode" + tooltip={state?.queryModel?.error} onClick={() => { dispatch(actions.toggleEditorMode()); }} diff --git a/public/app/plugins/datasource/graphite/datasource.test.ts b/public/app/plugins/datasource/graphite/datasource.test.ts index de993ff6102..548797d7342 100644 --- a/public/app/plugins/datasource/graphite/datasource.test.ts +++ b/public/app/plugins/datasource/graphite/datasource.test.ts @@ -744,6 +744,9 @@ describe('graphiteDatasource', () => { params: [{ multiple: true }], }, updateText: () => {}, + render: () => { + return ''; + }, })); }); diff --git a/public/app/plugins/datasource/graphite/datasource.ts b/public/app/plugins/datasource/graphite/datasource.ts index 9998fe8980e..e8a6487a275 100644 --- a/public/app/plugins/datasource/graphite/datasource.ts +++ b/public/app/plugins/datasource/graphite/datasource.ts @@ -143,7 +143,7 @@ export class GraphiteDatasource target: query.target || '', textEditor: false, }, - getTemplateSrv() + this.templateSrv ); graphiteQuery.parseTarget(); diff --git a/public/app/plugins/datasource/graphite/graphite_query.ts b/public/app/plugins/datasource/graphite/graphite_query.ts index d3009ecb71f..5c23e83f695 100644 --- a/public/app/plugins/datasource/graphite/graphite_query.ts +++ b/public/app/plugins/datasource/graphite/graphite_query.ts @@ -77,6 +77,21 @@ export default class GraphiteQuery { try { this.parseTargetRecursive(astNode, null); + if (this.target.target) { + const oldQuery = this.target.target; + const newQuery = this.generateQueryString(); + + // Spaces, quotes, and commas are used when rendering the AST back into a string. + // We are removing these for less false positives of query changes. + const sanitizeQuery = (o: string): string => o.replace(/\s|'|"|,/g, ''); + const oldSanitized = sanitizeQuery(oldQuery); + const newSanitized = sanitizeQuery(newQuery); + if (oldSanitized && newSanitized && oldSanitized !== newSanitized) { + throw new Error( + `Failed to make a visual query builder query that is equivalent to the query.\nOriginal query: ${oldQuery}\nQuery builder query: ${newQuery}` + ); + } + } } catch (err) { if (err instanceof Error) { console.error('error parsing target:', err.message); @@ -181,16 +196,19 @@ export default class GraphiteQuery { arrayMove(this.functions, index, index + offset); } - updateModelTarget(targets: any) { + generateQueryString(): string { const wrapFunction = (target: string, func: FuncInstance) => { return func.render(target, (value: string) => { return this.templateSrv ? this.templateSrv.replace(value, this.scopedVars) : value; }); }; + const metricPath = this.getSegmentPathUpTo(this.segments.length).replace(/\.?select metric$/, ''); + return reduce(this.functions, wrapFunction, metricPath); + } + updateModelTarget(targets: any) { if (!this.target.textEditor) { - const metricPath = this.getSegmentPathUpTo(this.segments.length).replace(/\.?select metric$/, ''); - this.target.target = reduce(this.functions, wrapFunction, metricPath); + this.target.target = this.generateQueryString(); } this.updateRenderedTarget(this.target, targets); diff --git a/public/app/plugins/datasource/graphite/specs/graphite_query.test.ts b/public/app/plugins/datasource/graphite/specs/graphite_query.test.ts index 5d5035c5dd6..e6e048e45de 100644 --- a/public/app/plugins/datasource/graphite/specs/graphite_query.test.ts +++ b/public/app/plugins/datasource/graphite/specs/graphite_query.test.ts @@ -258,6 +258,20 @@ describe('Graphite query model', () => { ctx.queryModel.updateModelTarget(targets); expect(ctx.queryModel.target.target).toContain(nestedFunctionAsParam); }); + + //This is not preferred behavior. The query builder cannot parse `maxSeries(sum(testSeries1), sum(testSeries2))` and when it can, remove this test + it('should return an error when visual query builder query does not match raw query', () => { + jest.spyOn(console, 'error').mockImplementation(); + ctx.target = { + refId: 'A', + target: 'maxSeries(sum(testSeries1), sum(testSeries2))', + }; + ctx.targets = [ctx.target]; + ctx.queryModel = new GraphiteQuery(ctx.datasource, ctx.target, ctx.templateSrv); + expect(ctx.queryModel.error).toBe( + 'Failed to make a visual query builder query that is equivalent to the query.\nOriginal query: maxSeries(sum(testSeries1), sum(testSeries2))\nQuery builder query: maxSeries(sumSeries(sumSeries(testSeries1), testSeries2))' + ); + }); }); }); }); diff --git a/public/app/plugins/datasource/graphite/state/store.ts b/public/app/plugins/datasource/graphite/state/store.ts index 0081c2bd0ea..6df365d4eee 100644 --- a/public/app/plugins/datasource/graphite/state/store.ts +++ b/public/app/plugins/datasource/graphite/state/store.ts @@ -2,7 +2,6 @@ import { AnyAction } from '@reduxjs/toolkit'; import { Action, Dispatch } from 'redux'; import { DataQuery, TimeRange } from '@grafana/data'; -import { getTemplateSrv } from '@grafana/runtime'; import { TemplateSrv } from '../../../../features/templating/template_srv'; import { GraphiteDatasource } from '../datasource'; @@ -57,7 +56,7 @@ const reducer = async (action: Action, state: GraphiteQueryEditorState): Promise state = { ...state, ...deps, - queryModel: new GraphiteQuery(deps.datasource, deps.target, getTemplateSrv()), + queryModel: new GraphiteQuery(deps.datasource, deps.target, state.templateSrv), supportsTags: deps.datasource.supportsTags, paused: false, removeTagValue: '-- remove tag --',