From b7ff4d166146e5e7613704c42c5d055ea699ad04 Mon Sep 17 00:00:00 2001 From: "Grot (@grafanabot)" <43478413+grafanabot@users.noreply.github.com> Date: Fri, 10 Mar 2023 20:12:28 +0100 Subject: [PATCH] [v9.4.x] Cloudwatch Logs: Fix log group names not interpolating with multiple variables (#64631) 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 (cherry picked from commit 75f89e67affd7956f4a24d0f97916b682c3ba500) Co-authored-by: Kevin Yu --- .../CloudWatchLogsLanguageProvider.ts | 1 + .../CloudWatchLogsQueryRunner.test.ts | 7 +-- .../query-runner/CloudWatchLogsQueryRunner.ts | 4 +- .../utils/templateVariableUtils.test.ts | 49 ++++++++++++++++++- .../cloudwatch/utils/templateVariableUtils.ts | 10 ++-- 5 files changed, 62 insertions(+), 9 deletions(-) diff --git a/public/app/plugins/datasource/cloudwatch/language/cloudwatch-logs/CloudWatchLogsLanguageProvider.ts b/public/app/plugins/datasource/cloudwatch/language/cloudwatch-logs/CloudWatchLogsLanguageProvider.ts index 5d6c728f470..376aa817e5b 100644 --- a/public/app/plugins/datasource/cloudwatch/language/cloudwatch-logs/CloudWatchLogsLanguageProvider.ts +++ b/public/app/plugins/datasource/cloudwatch/language/cloudwatch-logs/CloudWatchLogsLanguageProvider.ts @@ -133,6 +133,7 @@ export class CloudWatchLogsLanguageProvider extends LanguageProvider { const interpolatedLogGroups = interpolateStringArrayUsingSingleOrMultiValuedVariable( getTemplateSrv(), logGroups.map((lg) => lg.name), + {}, 'text' ); const results = await Promise.all( diff --git a/public/app/plugins/datasource/cloudwatch/query-runner/CloudWatchLogsQueryRunner.test.ts b/public/app/plugins/datasource/cloudwatch/query-runner/CloudWatchLogsQueryRunner.test.ts index 79e379a79ad..f1770514826 100644 --- a/public/app/plugins/datasource/cloudwatch/query-runner/CloudWatchLogsQueryRunner.test.ts +++ b/public/app/plugins/datasource/cloudwatch/query-runner/CloudWatchLogsQueryRunner.test.ts @@ -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( diff --git a/public/app/plugins/datasource/cloudwatch/query-runner/CloudWatchLogsQueryRunner.ts b/public/app/plugins/datasource/cloudwatch/query-runner/CloudWatchLogsQueryRunner.ts index 3f5810fe607..3adc73ada4e 100644 --- a/public/app/plugins/datasource/cloudwatch/query-runner/CloudWatchLogsQueryRunner.ts +++ b/public/app/plugins/datasource/cloudwatch/query-runner/CloudWatchLogsQueryRunner.ts @@ -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' ); diff --git a/public/app/plugins/datasource/cloudwatch/utils/templateVariableUtils.test.ts b/public/app/plugins/datasource/cloudwatch/utils/templateVariableUtils.test.ts index f65ea9b2653..986cff080e3 100644 --- a/public/app/plugins/datasource/cloudwatch/utils/templateVariableUtils.test.ts +++ b/public/app/plugins/datasource/cloudwatch/utils/templateVariableUtils.test.ts @@ -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']); }); }); diff --git a/public/app/plugins/datasource/cloudwatch/utils/templateVariableUtils.ts b/public/app/plugins/datasource/cloudwatch/utils/templateVariableUtils.ts index c537d5f63b8..bf5458a793c 100644 --- a/public/app/plugins/datasource/cloudwatch/utils/templateVariableUtils.ts +++ b/public/app/plugins/datasource/cloudwatch/utils/templateVariableUtils.ts @@ -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