From 67f0d0474497eb0caf8abf6675ea4f9eb3592308 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ida=20=C5=A0tambuk?= Date: Fri, 26 Jul 2024 14:20:39 +0200 Subject: [PATCH] Cloudwatch: Add account Id in groupBy options for Metric Insights cross-account (#90906) --- .../SQLBuilderEditor/SQLGroupBy.test.tsx | 44 +++++++++++++++++++ .../SQLBuilderEditor/SQLGroupBy.tsx | 20 +++++++-- .../language/cloudwatch-sql/SQLGenerator.ts | 7 ++- 3 files changed, 66 insertions(+), 5 deletions(-) diff --git a/public/app/plugins/datasource/cloudwatch/components/QueryEditor/MetricsQueryEditor/SQLBuilderEditor/SQLGroupBy.test.tsx b/public/app/plugins/datasource/cloudwatch/components/QueryEditor/MetricsQueryEditor/SQLBuilderEditor/SQLGroupBy.test.tsx index 9f98569e3e7..c8a55ab74ae 100644 --- a/public/app/plugins/datasource/cloudwatch/components/QueryEditor/MetricsQueryEditor/SQLBuilderEditor/SQLGroupBy.test.tsx +++ b/public/app/plugins/datasource/cloudwatch/components/QueryEditor/MetricsQueryEditor/SQLBuilderEditor/SQLGroupBy.test.tsx @@ -2,6 +2,8 @@ import { render, screen, waitFor } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import selectEvent from 'react-select-event'; +import { config } from '@grafana/runtime'; + import { setupMockedDataSource } from '../../../../__mocks__/CloudWatchDataSource'; import { createArray, createGroupBy } from '../../../../__mocks__/sqlUtils'; import { CloudWatchMetricsQuery, MetricEditorMode, MetricQueryType, SQLExpression } from '../../../../types'; @@ -25,6 +27,10 @@ const makeSQLQuery = (sql?: SQLExpression): CloudWatchMetricsQuery => ({ datasource.resources.getDimensionKeys = jest.fn().mockResolvedValue([]); describe('Cloudwatch SQLGroupBy', () => { + afterEach(() => { + baseProps.datasource.resources.isMonitoringAccount = jest.fn().mockResolvedValue(false); + }); + const baseProps = { query: makeSQLQuery(), datasource, @@ -55,6 +61,44 @@ describe('Cloudwatch SQLGroupBy', () => { }); }); + it('should show Account ID in groupBy options if feature flag is enabled', async () => { + config.featureToggles.cloudWatchCrossAccountQuerying = true; + config.featureToggles.cloudwatchMetricInsightsCrossAccount = true; + baseProps.datasource.resources.isMonitoringAccount = jest.fn().mockResolvedValue(true); + const query = makeSQLQuery(); + + render(); + const addButton = screen.getByRole('button', { name: 'Add' }); + await userEvent.click(addButton); + selectEvent.openMenu(screen.getByLabelText(/Group by/)); + expect(screen.getByText('Account ID')).toBeInTheDocument(); + }); + + it('should not show Account ID in groupBy options if not using a monitoring account', async () => { + config.featureToggles.cloudWatchCrossAccountQuerying = true; + config.featureToggles.cloudwatchMetricInsightsCrossAccount = true; + baseProps.datasource.resources.isMonitoringAccount = jest.fn().mockResolvedValue(false); + + const query = makeSQLQuery(); + + render(); + const addButton = screen.getByRole('button', { name: 'Add' }); + await userEvent.click(addButton); + selectEvent.openMenu(screen.getByLabelText(/Group by/)); + expect(screen.queryByText('Account ID')).not.toBeInTheDocument(); + }); + + it('should not show Account ID in groupBy options if feature flag is disabled', async () => { + config.featureToggles.cloudwatchMetricInsightsCrossAccount = false; + const query = makeSQLQuery(); + + render(); + const addButton = screen.getByRole('button', { name: 'Add' }); + await userEvent.click(addButton); + selectEvent.openMenu(screen.getByLabelText(/Group by/)); + expect(screen.queryByText('Account ID')).not.toBeInTheDocument(); + }); + it('should allow adding a new dimension filter', async () => { const query = makeSQLQuery({ groupBy: undefined, diff --git a/public/app/plugins/datasource/cloudwatch/components/QueryEditor/MetricsQueryEditor/SQLBuilderEditor/SQLGroupBy.tsx b/public/app/plugins/datasource/cloudwatch/components/QueryEditor/MetricsQueryEditor/SQLBuilderEditor/SQLGroupBy.tsx index 359b896ad88..fa4fe294ad8 100644 --- a/public/app/plugins/datasource/cloudwatch/components/QueryEditor/MetricsQueryEditor/SQLBuilderEditor/SQLGroupBy.tsx +++ b/public/app/plugins/datasource/cloudwatch/components/QueryEditor/MetricsQueryEditor/SQLBuilderEditor/SQLGroupBy.tsx @@ -2,6 +2,7 @@ import { useMemo, useState } from 'react'; import { SelectableValue, toOption } from '@grafana/data'; import { AccessoryButton, EditorList, InputGroup } from '@grafana/experimental'; +import { config } from '@grafana/runtime'; import { Select } from '@grafana/ui'; import { CloudWatchDatasource } from '../../../../datasource'; @@ -10,7 +11,7 @@ import { QueryEditorGroupByExpression, QueryEditorPropertyType, } from '../../../../expressions'; -import { useDimensionKeys } from '../../../../hooks'; +import { useDimensionKeys, useIsMonitoringAccount } from '../../../../hooks'; import { CloudWatchMetricsQuery } from '../../../../types'; import { @@ -31,6 +32,7 @@ const SQLGroupBy = ({ query, datasource, onQueryChange }: SQLGroupByProps) => { const sql = query.sql ?? {}; const groupBysFromQuery = useMemo(() => getFlattenedGroupBys(query.sql ?? {}), [query.sql]); const [items, setItems] = useState(groupBysFromQuery); + const isMonitoringAccount = useIsMonitoringAccount(datasource.resources, query.region); const namespace = getNamespaceFromExpression(sql.from); const metricName = getMetricNameFromExpression(sql.select); @@ -38,8 +40,20 @@ const SQLGroupBy = ({ query, datasource, onQueryChange }: SQLGroupByProps) => { const baseOptions = useDimensionKeys(datasource, { region: query.region, namespace, metricName }); const options = useMemo( // Exclude options we've already selected - () => baseOptions.filter((option) => !groupBysFromQuery.some((v) => v.property.name === option.value)), - [baseOptions, groupBysFromQuery] + () => { + const isCrossAccountEnabled = + config.featureToggles.cloudWatchCrossAccountQuerying && + config.featureToggles.cloudwatchMetricInsightsCrossAccount; + + const baseOptionsWithAccountId = + isCrossAccountEnabled && isMonitoringAccount + ? [{ label: 'Account ID', value: 'AWS.AccountId' }, ...baseOptions] + : baseOptions; + return baseOptionsWithAccountId.filter( + (option) => !groupBysFromQuery.some((v) => v.property.name === option.value) + ); + }, + [baseOptions, groupBysFromQuery, isMonitoringAccount] ); const onChange = (newItems: Array>) => { diff --git a/public/app/plugins/datasource/cloudwatch/language/cloudwatch-sql/SQLGenerator.ts b/public/app/plugins/datasource/cloudwatch/language/cloudwatch-sql/SQLGenerator.ts index a6b7f2fb291..e9a41bac30b 100644 --- a/public/app/plugins/datasource/cloudwatch/language/cloudwatch-sql/SQLGenerator.ts +++ b/public/app/plugins/datasource/cloudwatch/language/cloudwatch-sql/SQLGenerator.ts @@ -160,8 +160,11 @@ export default class SQLGenerator { const startsWithNumber = /^\d/; const interpolated = this.templateSrv.replace(label, {}, 'raw'); - if (specialCharacters.test(interpolated) || startsWithNumber.test(interpolated)) { - return `"${label}"`; + if (interpolated !== 'AWS.AccountId') { + // AWS.AccountId should never be in quotes + if (specialCharacters.test(interpolated) || startsWithNumber.test(interpolated)) { + return `"${label}"`; + } } return label;