Graphite: Compare query builder query to raw query (#101104)
* compare queries to insure query isnt changed * comment * removed calls to getTemplateSrv because it makes things impossible to test. added a check for empty raw queries * prettier * Update public/app/plugins/datasource/graphite/graphite_query.ts Co-authored-by: Adam Yeats <16296989+adamyeats@users.noreply.github.com> --------- Co-authored-by: Adam Yeats <16296989+adamyeats@users.noreply.github.com>
This commit is contained in:
co-authored by
Adam Yeats
parent
31e92ba720
commit
3bdc9d1e19
@@ -53,6 +53,7 @@ function GraphiteQueryEditorContent() {
|
||||
icon="pen"
|
||||
variant="secondary"
|
||||
aria-label="Toggle editor mode"
|
||||
tooltip={state?.queryModel?.error}
|
||||
onClick={() => {
|
||||
dispatch(actions.toggleEditorMode());
|
||||
}}
|
||||
|
||||
@@ -744,6 +744,9 @@ describe('graphiteDatasource', () => {
|
||||
params: [{ multiple: true }],
|
||||
},
|
||||
updateText: () => {},
|
||||
render: () => {
|
||||
return '';
|
||||
},
|
||||
}));
|
||||
});
|
||||
|
||||
|
||||
@@ -143,7 +143,7 @@ export class GraphiteDatasource
|
||||
target: query.target || '',
|
||||
textEditor: false,
|
||||
},
|
||||
getTemplateSrv()
|
||||
this.templateSrv
|
||||
);
|
||||
graphiteQuery.parseTarget();
|
||||
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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))'
|
||||
);
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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 --',
|
||||
|
||||
Reference in New Issue
Block a user