Cloudwatch Logs: Fix log group names not interpolating with multiple variables (#64567)

* Cloudwatch Logs: Fix log group names not interpolating with multiple variables

* fix test
This commit is contained in:
Kevin Yu 2023-03-10 11:00:35 -08:00 committed by GitHub
parent 72e50431df
commit 75f89e67af
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 62 additions and 9 deletions

View File

@ -133,6 +133,7 @@ export class CloudWatchLogsLanguageProvider extends LanguageProvider {
const interpolatedLogGroups = interpolateStringArrayUsingSingleOrMultiValuedVariable(
getTemplateSrv(),
logGroups.map((lg) => lg.name),
{},
'text'
);
const results = await Promise.all(

View File

@ -195,7 +195,7 @@ describe('CloudWatchLogsQueryRunner', () => {
const legacyLogGroupNamesQuery: CloudWatchLogsQuery = {
queryMode: 'Logs',
logGroupNames: ['group-A', 'templatedGroup-1', logGroupNamesVariable.name],
logGroupNames: ['group-A', 'templatedGroup-1', `$${logGroupNamesVariable.name}`],
hide: false,
id: '',
region: 'us-east-2',
@ -207,7 +207,7 @@ describe('CloudWatchLogsQueryRunner', () => {
queryMode: 'Logs',
logGroups: [
{ arn: 'arn:aws:logs:us-east-2:123456789012:log-group:group-A:*', name: 'group-A' },
{ arn: logGroupNamesVariable.name, name: logGroupNamesVariable.name },
{ arn: `$${logGroupNamesVariable.name}`, name: logGroupNamesVariable.name },
],
hide: false,
id: '',
@ -218,7 +218,7 @@ describe('CloudWatchLogsQueryRunner', () => {
const logsScopedVarQuery: CloudWatchLogsQuery = {
queryMode: 'Logs',
logGroups: [{ arn: logGroupNamesVariable.name, name: logGroupNamesVariable.name }],
logGroups: [{ arn: `$${logGroupNamesVariable.name}`, name: logGroupNamesVariable.name }],
hide: false,
id: '',
region: '$' + regionVariable.name,
@ -237,6 +237,7 @@ describe('CloudWatchLogsQueryRunner', () => {
logsTimeout: '500ms',
},
},
mockGetVariableName: false,
});
const spy = jest.spyOn(runner, 'makeLogActionRequest');
await lastValueFrom(

View File

@ -85,13 +85,15 @@ export class CloudWatchLogsQueryRunner extends CloudWatchRequest {
const startQueryRequests: StartQueryRequest[] = validLogQueries.map((target: CloudWatchLogsQuery) => {
const interpolatedLogGroupArns = interpolateStringArrayUsingSingleOrMultiValuedVariable(
this.templateSrv,
(target.logGroups || this.instanceSettings.jsonData.logGroups || []).map((lg) => lg.arn)
(target.logGroups || this.instanceSettings.jsonData.logGroups || []).map((lg) => lg.arn),
options.scopedVars
);
// need to support legacy format variables too
const interpolatedLogGroupNames = interpolateStringArrayUsingSingleOrMultiValuedVariable(
this.templateSrv,
target.logGroupNames || this.instanceSettings.jsonData.defaultLogGroups || [],
options.scopedVars,
'text'
);

View File

@ -11,6 +11,8 @@ interface TestCase {
key?: 'value' | 'text';
}
const suffix = 'suffix';
describe('templateVariableUtils', () => {
const multiValuedRepresentedAsArray = {
...logGroupNamesVariable,
@ -70,14 +72,57 @@ describe('templateVariableUtils', () => {
test.each(cases)('$name', async ({ variable, expected, key }) => {
const templateService = setupMockedTemplateService([variable]);
const strings = ['$' + variable.name, 'log-group'];
const result = interpolateStringArrayUsingSingleOrMultiValuedVariable(templateService, strings, key);
const result = interpolateStringArrayUsingSingleOrMultiValuedVariable(templateService, strings, {}, key);
expect(result).toEqual([...expected, 'log-group']);
});
const casesWithMultipleVariablesInString: TestCase[] = [
{
name: 'string with multiple variables should expand multi-valued variable with two values and use the metric find values',
variable: logGroupNamesVariable,
expected: [`${regionVariable.current.text}-${[...logGroupNamesVariable.current.value].join('|')}-${suffix}`],
},
{
name: 'string with multiple variables should expand multi-valued variable with two values and use the metric find texts',
variable: logGroupNamesVariable,
expected: [`${regionVariable.current.text}-${[...logGroupNamesVariable.current.text].join(' + ')}-${suffix}`],
key: 'text',
},
{
name: 'string with multiple variables should expand multi-valued variable with one selected value represented as array and use metric find values',
variable: multiValuedRepresentedAsArray,
expected: [`${regionVariable.current.text}-${multiValuedRepresentedAsArray.current.value}-${suffix}`],
},
{
name: 'should expand multi-valued variable with one selected value represented as array and use metric find texts',
variable: multiValuedRepresentedAsArray,
expected: [`${regionVariable.current.text}-${multiValuedRepresentedAsArray.current.text}-${suffix}`],
key: 'text',
},
{
name: 'string with multiple variables should expand multi-valued variable with one selected value represented as a string and use metric find value',
variable: multiValuedRepresentedAsString,
expected: [`${regionVariable.current.text}-${multiValuedRepresentedAsString.current.value}-${suffix}`],
},
{
name: 'string with multiple variables should expand multi-valued variable with one selected value represented as a string and use metric find text',
variable: multiValuedRepresentedAsString,
expected: [`${regionVariable.current.text}-${multiValuedRepresentedAsString.current.text}-${suffix}`],
key: 'text',
},
];
test.each(casesWithMultipleVariablesInString)('$name', async ({ variable, expected, key }) => {
const templateService = setupMockedTemplateService([regionVariable, variable]);
const strings = [`$${regionVariable.name}-$${variable.name}-${suffix}`, 'log-group'];
const result = interpolateStringArrayUsingSingleOrMultiValuedVariable(templateService, strings, {}, key);
expect(result).toEqual([...expected, 'log-group']);
});
it('should expand single-valued variable', () => {
const templateService = setupMockedTemplateService([regionVariable]);
const strings = ['$' + regionVariable.name, 'us-east-2'];
const result = interpolateStringArrayUsingSingleOrMultiValuedVariable(templateService, strings);
const result = interpolateStringArrayUsingSingleOrMultiValuedVariable(templateService, strings, {});
expect(result).toEqual([regionVariable.current.value, 'us-east-2']);
});
});

View File

@ -1,4 +1,4 @@
import { VariableOption, UserProps, OrgProps, DashboardProps } from '@grafana/data';
import { VariableOption, UserProps, OrgProps, DashboardProps, ScopedVars } from '@grafana/data';
import { TemplateSrv } from 'app/features/templating/template_srv';
/**
@ -9,14 +9,17 @@ import { TemplateSrv } from 'app/features/templating/template_srv';
* multi-valued variable + non-variable item. ['$multiValuedVariable', 'log-group'] => ['value1', 'value2', 'log-group']
* @param templateSrv - The template service
* @param strings - The array of strings to interpolate. May contain variables and non-variables.
* @pararm scopedVars - The scoped variables to use when interpolating the variables.
* @param key - Allows you to specify whether the variable MetricFindValue.text or MetricFindValue.value should be used when interpolating the variable. Optional, defaults to 'value'.
**/
export const interpolateStringArrayUsingSingleOrMultiValuedVariable = (
templateSrv: TemplateSrv,
strings: string[],
scopedVars: ScopedVars,
key?: 'value' | 'text'
) => {
key = key ?? 'value';
const format = key === 'value' ? 'pipe' : 'text';
let result: string[] = [];
for (const string of strings) {
const variableName = templateSrv.getVariableName(string);
@ -25,9 +28,10 @@ export const interpolateStringArrayUsingSingleOrMultiValuedVariable = (
if (valueVar && 'current' in valueVar && isVariableOption(valueVar.current)) {
const rawValue = valueVar.current[key];
if (Array.isArray(rawValue)) {
result = [...result, ...rawValue];
const separator = format === 'text' ? ' + ' : '|';
result.push(...templateSrv.replace(string, scopedVars, format).split(separator));
} else if (typeof rawValue === 'string') {
result.push(rawValue);
result.push(templateSrv.replace(string, scopedVars, format));
}
} else {
// if it's not a variable, just add the raw value