Unverified Commit b424e12a authored by gotjosh's avatar gotjosh Committed by GitHub
Browse files

Fix: Avoid glob of single-value array variables (#18420)

* Fix: Avoid glob of single-value array variables

Based on our current implementation of templates, when multi-select
variables are part of a dashboard query the default/fallback formatting option is `glob`.

Some data sources do not support glob (e.g. metrics.{a}.* instead of
metrics.a.*) for single variable queries. This behaviour breaks dashboards.

This commit introduces an alternative formatting option where globing is avoided if it's there is only one value as part of the query variable.

This means, queries previously formatted as `query=metrics.{a}.*.*`, are
now formatted as `query=metrics.a.*.*`. However, queries formatted as
`query=metrics.{a,b}.*.*` continue to be as is.
parent c3880421
......@@ -112,6 +112,23 @@ describe('templateSrv', () => {
expect(target).toBe('this.{value1,value2}.filters');
});
describe('when the globbed variable only has one value', () => {
beforeEach(() => {
initTemplateSrv([
{
type: 'query',
name: 'test',
current: { value: ['value1'] },
},
]);
});
it('should not glob the value', () => {
const target = _templateSrv.replace('this.$test.filters', {}, 'glob');
expect(target).toBe('this.value1.filters');
});
});
it('should replace ${test} with globbed value', () => {
const target = _templateSrv.replace('this.${test}.filters', {}, 'glob');
expect(target).toBe('this.{value1,value2}.filters');
......
......@@ -172,7 +172,7 @@ export class TemplateSrv {
return this.encodeURIComponentStrict(value);
}
default: {
if (_.isArray(value)) {
if (_.isArray(value) && value.length > 1) {
return '{' + value.join(',') + '}';
}
return value;
......
......@@ -2,7 +2,7 @@ import { GraphiteDatasource } from '../datasource';
import _ from 'lodash';
// @ts-ignore
import $q from 'q';
import { TemplateSrvStub } from 'test/specs/helpers';
import { TemplateSrv } from 'app/features/templating/template_srv';
import { dateTime } from '@grafana/data';
describe('graphiteDatasource', () => {
......@@ -10,7 +10,7 @@ describe('graphiteDatasource', () => {
backendSrv: {},
$q,
// @ts-ignore
templateSrv: new TemplateSrvStub(),
templateSrv: new TemplateSrv(),
instanceSettings: { url: 'url', name: 'graphiteProd', jsonData: {} },
};
......@@ -218,6 +218,38 @@ describe('graphiteDatasource', () => {
});
expect(results.length).toBe(2);
});
describe('when formatting targets', () => {
it('does not attempt to glob for one variable', () => {
ctx.ds.templateSrv.init([
{
type: 'query',
name: 'metric',
current: { value: ['b'] },
},
]);
const results = ctx.ds.buildGraphiteParams({
targets: [{ target: 'my.$metric.*' }],
});
expect(results).toStrictEqual(['target=my.b.*', 'format=json']);
});
it('globs for more than one variable', () => {
ctx.ds.templateSrv.init([
{
type: 'query',
name: 'metric',
current: { value: ['a', 'b'] },
},
]);
const results = ctx.ds.buildGraphiteParams({
targets: [{ target: 'my.[[metric]].*' }],
});
expect(results).toStrictEqual(['target=my.%7Ba%2Cb%7D.*', 'format=json']);
});
});
});
describe('querying for template variables', () => {
......@@ -308,7 +340,13 @@ describe('graphiteDatasource', () => {
});
it('/metrics/find should be POST', () => {
ctx.templateSrv.setGrafanaVariable('foo', 'bar');
ctx.ds.templateSrv.init([
{
type: 'query',
name: 'foo',
current: { value: ['bar'] },
},
]);
ctx.ds.metricFindQuery('[[foo]]').then((data: any) => {
results = data;
});
......@@ -327,7 +365,7 @@ function accessScenario(name: string, url: string, fn: any) {
backendSrv: {},
$q,
// @ts-ignore
templateSrv: new TemplateSrvStub(),
templateSrv: new TemplateSrv(),
instanceSettings: { url: 'url', name: 'graphiteProd', jsonData: {} },
};
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment