From a4f56446eee9f201cedb738ada1d2383a549ee04 Mon Sep 17 00:00:00 2001 From: Andres Martinez Gotor Date: Mon, 1 Aug 2022 17:48:49 +0200 Subject: [PATCH] Azure Monitor: Restore Metrics query parameters: subscription, resourceGroup, metricNamespace and resourceName (#52897) * Azure Monitor: (Components) deprecate ResourceURI (#52982) --- .../__mocks__/query.ts | 2 - .../azure_monitor_datasource.test.ts | 55 ++++- .../azure_monitor/azure_monitor_datasource.ts | 45 ++-- .../azure_monitor/url_builder.test.ts | 92 ++++++-- .../azure_monitor/url_builder.ts | 83 +++---- .../LogsQueryEditor/LogsQueryEditor.tsx | 4 +- .../LogsQueryEditor/setQueryValue.ts | 10 - .../MetricsQueryEditor/AggregationField.tsx | 2 +- .../MetricNamespaceField.tsx | 2 +- .../MetricsQueryEditor.test.tsx | 79 ++----- .../MetricsQueryEditor/MetricsQueryEditor.tsx | 12 +- .../MetricsQueryEditor/dataHooks.test.ts | 20 +- .../MetricsQueryEditor/dataHooks.ts | 38 ++-- .../MetricsQueryEditor/setQueryValue.test.ts | 27 --- .../MetricsQueryEditor/setQueryValue.ts | 33 --- .../components/QueryEditor/QueryEditor.tsx | 2 +- .../QueryEditor/usePreparedQuery.ts | 19 +- .../ResourceField/ResourceField.tsx | 70 ++---- .../ResourcePicker/Advanced.test.tsx | 35 +++ .../components/ResourcePicker/Advanced.tsx | 124 ++++++++++ .../ResourcePicker/ResourcePicker.test.tsx | 67 +++++- .../ResourcePicker/ResourcePicker.tsx | 94 +++----- .../components/ResourcePicker/utils.test.ts | 144 +++++++++++- .../components/ResourcePicker/utils.ts | 90 +++++++- .../datasource.ts | 4 +- .../resourcePicker/resourcePickerData.ts | 49 ++-- .../types/query.ts | 12 +- .../utils/migrateQuery.test.ts | 213 ++++-------------- .../utils/migrateQuery.ts | 152 +++---------- 29 files changed, 859 insertions(+), 720 deletions(-) delete mode 100644 public/app/plugins/datasource/grafana-azure-monitor-datasource/components/MetricsQueryEditor/setQueryValue.test.ts create mode 100644 public/app/plugins/datasource/grafana-azure-monitor-datasource/components/ResourcePicker/Advanced.test.tsx create mode 100644 public/app/plugins/datasource/grafana-azure-monitor-datasource/components/ResourcePicker/Advanced.tsx diff --git a/public/app/plugins/datasource/grafana-azure-monitor-datasource/__mocks__/query.ts b/public/app/plugins/datasource/grafana-azure-monitor-datasource/__mocks__/query.ts index e190a0c2168..b8088c13049 100644 --- a/public/app/plugins/datasource/grafana-azure-monitor-datasource/__mocks__/query.ts +++ b/public/app/plugins/datasource/grafana-azure-monitor-datasource/__mocks__/query.ts @@ -29,8 +29,6 @@ export default function createMockQuery(overrides?: Partial): azureMonitor: { // aggOptions: [], - resourceUri: - '/subscriptions/99999999-cccc-bbbb-aaaa-9106972f9572/resourceGroups/grafanastaging/providers/Microsoft.Compute/virtualMachines/grafana', aggregation: 'Average', allowedTimeGrainsMs: [60000, 300000, 900000, 1800000, 3600000, 21600000, 43200000, 86400000], // dimensionFilter: '*', diff --git a/public/app/plugins/datasource/grafana-azure-monitor-datasource/azure_monitor/azure_monitor_datasource.test.ts b/public/app/plugins/datasource/grafana-azure-monitor-datasource/azure_monitor/azure_monitor_datasource.test.ts index 0871335fae2..3b7ff7e5967 100644 --- a/public/app/plugins/datasource/grafana-azure-monitor-datasource/azure_monitor/azure_monitor_datasource.test.ts +++ b/public/app/plugins/datasource/grafana-azure-monitor-datasource/azure_monitor/azure_monitor_datasource.test.ts @@ -1,4 +1,4 @@ -import { startsWith, get, set } from 'lodash'; +import { get, set } from 'lodash'; import { DataSourceInstanceSettings } from '@grafana/data'; import { TemplateSrv } from 'app/features/templating/template_srv'; @@ -34,6 +34,50 @@ describe('AzureMonitorDatasource', () => { ctx.ds = new AzureMonitorDatasource(ctx.instanceSettings); }); + describe('filterQuery', () => { + [ + { + description: 'filter query all props', + query: createMockQuery(), + filtered: true, + }, + { + description: 'filter query with no resourceGroup', + query: createMockQuery({ azureMonitor: { resourceGroup: undefined } }), + filtered: false, + }, + { + description: 'filter query with no resourceName', + query: createMockQuery({ azureMonitor: { resourceName: undefined } }), + filtered: false, + }, + { + description: 'filter query with no metricNamespace', + query: createMockQuery({ azureMonitor: { metricNamespace: undefined } }), + filtered: false, + }, + { + description: 'filter query with no metricName', + query: createMockQuery({ azureMonitor: { metricName: undefined } }), + filtered: false, + }, + { + description: 'filter query with no aggregation', + query: createMockQuery({ azureMonitor: { aggregation: undefined } }), + filtered: false, + }, + { + description: 'filter hidden query', + query: createMockQuery({ hide: true }), + filtered: false, + }, + ].forEach((t) => { + it(t.description, () => { + expect(ctx.ds.filterQuery(t.query)).toEqual(t.filtered); + }); + }); + }); + describe('applyTemplateVariables', () => { it('should migrate metricDefinition to metricNamespace', () => { const query = createMockQuery({ @@ -245,7 +289,6 @@ describe('AzureMonitorDatasource', () => { it('should return a query with any template variables replaced', () => { const templateableProps = [ - 'resourceUri', 'resourceGroup', 'resourceName', 'metricNamespace', @@ -399,16 +442,14 @@ describe('AzureMonitorDatasource', () => { }, { name: 'storagetest', - type: 'Microsoft.Storage/storageAccounts', + type: 'microsoft.storage/storageaccounts', }, ], }; it('should return list of Resource Names', () => { - metricNamespace = 'Microsoft.Storage/storageAccounts/blobServices'; - const validMetricNamespace = startsWith(metricNamespace, 'Microsoft.Storage/storageAccounts/') - ? 'Microsoft.Storage/storageAccounts' - : metricNamespace; + metricNamespace = 'microsoft.storage/storageaccounts/blobservices'; + const validMetricNamespace = 'microsoft.storage/storageaccounts'; ctx.ds.azureMonitorDatasource.getResource = jest.fn().mockImplementation((path: string) => { const basePath = `azuremonitor/subscriptions/${subscription}/resourceGroups`; expect(path).toBe( diff --git a/public/app/plugins/datasource/grafana-azure-monitor-datasource/azure_monitor/azure_monitor_datasource.ts b/public/app/plugins/datasource/grafana-azure-monitor-datasource/azure_monitor/azure_monitor_datasource.ts index 14b67f0609a..256100ea845 100644 --- a/public/app/plugins/datasource/grafana-azure-monitor-datasource/azure_monitor/azure_monitor_datasource.ts +++ b/public/app/plugins/datasource/grafana-azure-monitor-datasource/azure_monitor/azure_monitor_datasource.ts @@ -62,15 +62,14 @@ export default class AzureMonitorDatasource extends DataSourceWithBackend> { @@ -158,8 +155,8 @@ export default class AzureMonitorDatasource extends DataSourceWithBackend { let list: Array<{ text: string; value: string }> = []; - if (startsWith(metricNamespace, 'Microsoft.Storage/storageAccounts/')) { - list = ResponseParser.parseResourceNames(result, 'Microsoft.Storage/storageAccounts'); + if (startsWith(metricNamespace?.toLowerCase(), 'microsoft.storage/storageaccounts/')) { + list = ResponseParser.parseResourceNames(result, 'microsoft.storage/storageaccounts'); for (let i = 0; i < list.length; i++) { list[i].text += '/default'; list[i].value += '/default'; @@ -215,13 +212,13 @@ export default class AzureMonitorDatasource extends DataSourceWithBackend { - if (url.includes('Microsoft.Storage/storageAccounts')) { + if (url.toLowerCase().includes('microsoft.storage/storageaccounts')) { const storageNamespaces = [ - 'Microsoft.Storage/storageAccounts', - 'Microsoft.Storage/storageAccounts/blobServices', - 'Microsoft.Storage/storageAccounts/fileServices', - 'Microsoft.Storage/storageAccounts/tableServices', - 'Microsoft.Storage/storageAccounts/queueServices', + 'microsoft.storage/storageaccounts', + 'microsoft.storage/storageaccounts/blobservices', + 'microsoft.storage/storageaccounts/fileservices', + 'microsoft.storage/storageaccounts/tableservices', + 'microsoft.storage/storageaccounts/queueservices', ]; for (const namespace of storageNamespaces) { if (!find(result, ['value', namespace.toLowerCase()])) { diff --git a/public/app/plugins/datasource/grafana-azure-monitor-datasource/azure_monitor/url_builder.test.ts b/public/app/plugins/datasource/grafana-azure-monitor-datasource/azure_monitor/url_builder.test.ts index 0a6cb613e16..55fe2fe61e5 100644 --- a/public/app/plugins/datasource/grafana-azure-monitor-datasource/azure_monitor/url_builder.test.ts +++ b/public/app/plugins/datasource/grafana-azure-monitor-datasource/azure_monitor/url_builder.test.ts @@ -19,25 +19,34 @@ describe('AzureMonitorUrlBuilder', () => { describe('buildResourceUri', () => { it('builds a resource uri when the required properties are provided', () => { expect( - UrlBuilder.buildResourceUri('sub', 'group', templateSrv, 'Microsoft.NetApp/netAppAccounts', 'name') + UrlBuilder.buildResourceUri(templateSrv, { + subscription: 'sub', + resourceGroup: 'group', + metricNamespace: 'Microsoft.NetApp/netAppAccounts', + resourceName: 'name', + }) ).toEqual('/subscriptions/sub/resourceGroups/group/providers/Microsoft.NetApp/netAppAccounts/name'); }); it('builds a resource uri correctly when a template variable is used as namespace', () => { - expect(UrlBuilder.buildResourceUri('sub', 'group', templateSrv, '$ns', 'name')).toEqual( - '/subscriptions/sub/resourceGroups/group/providers/$ns/name' - ); + expect( + UrlBuilder.buildResourceUri(templateSrv, { + subscription: 'sub', + resourceGroup: 'group', + metricNamespace: '$ns', + resourceName: 'name', + }) + ).toEqual('/subscriptions/sub/resourceGroups/group/providers/$ns/name'); }); it('builds a resource uri correctly when the namespace includes a storage sub-resource', () => { expect( - UrlBuilder.buildResourceUri( - 'sub', - 'group', - templateSrv, - 'Microsoft.Storage/storageAccounts/tableServices', - 'name' - ) + UrlBuilder.buildResourceUri(templateSrv, { + subscription: 'sub', + resourceGroup: 'group', + metricNamespace: 'Microsoft.Storage/storageAccounts/tableServices', + resourceName: 'name', + }) ).toEqual( '/subscriptions/sub/resourceGroups/group/providers/Microsoft.Storage/storageAccounts/name/tableServices/default' ); @@ -56,27 +65,64 @@ describe('AzureMonitorUrlBuilder', () => { templateSrv = getTemplateSrv(); it('builds a resource uri without specifying a subresource (default)', () => { - expect(UrlBuilder.buildResourceUri('sub', 'group', templateSrv, '$ns/tableServices', 'name')).toEqual( - '/subscriptions/sub/resourceGroups/group/providers/$ns/name/tableServices/default' - ); + expect( + UrlBuilder.buildResourceUri(templateSrv, { + subscription: 'sub', + resourceGroup: 'group', + metricNamespace: '$ns/tableServices', + resourceName: 'name', + }) + ).toEqual('/subscriptions/sub/resourceGroups/group/providers/$ns/name/tableServices/default'); }); it('builds a resource uri specifying a subresource (default)', () => { - expect(UrlBuilder.buildResourceUri('sub', 'group', templateSrv, '$ns/tableServices', 'name/default')).toEqual( - '/subscriptions/sub/resourceGroups/group/providers/$ns/name/tableServices/default' - ); + expect( + UrlBuilder.buildResourceUri(templateSrv, { + subscription: 'sub', + resourceGroup: 'group', + metricNamespace: '$ns/tableServices', + resourceName: 'name/default', + }) + ).toEqual('/subscriptions/sub/resourceGroups/group/providers/$ns/name/tableServices/default'); }); it('builds a resource uri specifying a resource template variable', () => { - expect(UrlBuilder.buildResourceUri('sub', 'group', templateSrv, '$ns/tableServices', '$rs/default')).toEqual( - '/subscriptions/sub/resourceGroups/group/providers/$ns/$rs/tableServices/default' - ); + expect( + UrlBuilder.buildResourceUri(templateSrv, { + subscription: 'sub', + resourceGroup: 'group', + metricNamespace: '$ns/tableServices', + resourceName: '$rs/default', + }) + ).toEqual('/subscriptions/sub/resourceGroups/group/providers/$ns/$rs/tableServices/default'); }); it('builds a resource uri specifying multiple template variables', () => { - expect(UrlBuilder.buildResourceUri('sub', 'group', templateSrv, '$ns/$ns2', '$rs/$rs2')).toEqual( - '/subscriptions/sub/resourceGroups/group/providers/$ns/$rs/$ns2/$rs2' - ); + expect( + UrlBuilder.buildResourceUri(templateSrv, { + subscription: 'sub', + resourceGroup: 'group', + metricNamespace: '$ns/$ns2', + resourceName: '$rs/$rs2', + }) + ).toEqual('/subscriptions/sub/resourceGroups/group/providers/$ns/$rs/$ns2/$rs2'); + }); + + it('builds a resource uri with only a subscription', () => { + expect( + UrlBuilder.buildResourceUri(templateSrv, { + subscription: 'sub', + }) + ).toEqual('/subscriptions/sub'); + }); + + it('builds a resource uri with a subscription and a resource group', () => { + expect( + UrlBuilder.buildResourceUri(templateSrv, { + subscription: 'sub', + resourceGroup: 'group', + }) + ).toEqual('/subscriptions/sub/resourceGroups/group'); }); }); }); diff --git a/public/app/plugins/datasource/grafana-azure-monitor-datasource/azure_monitor/url_builder.ts b/public/app/plugins/datasource/grafana-azure-monitor-datasource/azure_monitor/url_builder.ts index a4280935fa0..9b41f1e5030 100644 --- a/public/app/plugins/datasource/grafana-azure-monitor-datasource/azure_monitor/url_builder.ts +++ b/public/app/plugins/datasource/grafana-azure-monitor-datasource/azure_monitor/url_builder.ts @@ -1,45 +1,48 @@ import { TemplateSrv } from '@grafana/runtime'; -import { GetMetricNamespacesQuery, GetMetricNamesQuery } from '../types'; +import { AzureMetricResource, GetMetricNamespacesQuery, GetMetricNamesQuery } from '../types'; export default class UrlBuilder { - static buildResourceUri( - subscriptionId: string, - resourceGroup: string, - templateSrv: TemplateSrv, - metricNamespace?: string, - resourceName?: string - ) { - const urlArray = ['/subscriptions', subscriptionId, 'resourceGroups', resourceGroup]; + static buildResourceUri(templateSrv: TemplateSrv, resource: AzureMetricResource) { + const urlArray = []; + const { subscription, resourceGroup, metricNamespace, resourceName } = resource; - if (metricNamespace && resourceName) { - const metricNamespaceProcessed = templateSrv.replace(metricNamespace); - const metricNamespaceArray = metricNamespace.split('/'); - const resourceNameProcessed = templateSrv.replace(resourceName); - const resourceNameArray = resourceName.split('/'); - const provider = metricNamespaceArray.shift(); - if (provider) { - urlArray.push('providers', provider); - } + if (subscription) { + urlArray.push('/subscriptions', subscription); - if ( - metricNamespaceProcessed.startsWith('Microsoft.Storage/storageAccounts/') && - !resourceNameProcessed.endsWith('default') - ) { - resourceNameArray.push('default'); - } + if (resourceGroup) { + urlArray.push('resourceGroups', resourceGroup); - if (resourceNameArray.length > metricNamespaceArray.length) { - const parentResource = resourceNameArray.shift(); - if (parentResource) { - urlArray.push(parentResource); + if (metricNamespace && resourceName) { + const metricNamespaceProcessed = templateSrv.replace(metricNamespace); + const metricNamespaceArray = metricNamespace.split('/'); + const resourceNameProcessed = templateSrv.replace(resourceName); + const resourceNameArray = resourceName.split('/'); + const provider = metricNamespaceArray.shift(); + if (provider) { + urlArray.push('providers', provider); + } + + if ( + metricNamespaceProcessed.toLowerCase().startsWith('microsoft.storage/storageaccounts/') && + !resourceNameProcessed.endsWith('default') + ) { + resourceNameArray.push('default'); + } + + if (resourceNameArray.length > metricNamespaceArray.length) { + const parentResource = resourceNameArray.shift(); + if (parentResource) { + urlArray.push(parentResource); + } + } + + for (const i in metricNamespaceArray) { + urlArray.push(metricNamespaceArray[i]); + urlArray.push(resourceNameArray[i]); + } } } - - for (const i in metricNamespaceArray) { - urlArray.push(metricNamespaceArray[i]); - urlArray.push(resourceNameArray[i]); - } } return urlArray.join('/'); @@ -57,13 +60,12 @@ export default class UrlBuilder { resourceUri = query.resourceUri; } else { const { subscription, resourceGroup, metricNamespace, resourceName } = query; - resourceUri = UrlBuilder.buildResourceUri( + resourceUri = UrlBuilder.buildResourceUri(templateSrv, { subscription, resourceGroup, - templateSrv, metricNamespace, - resourceName - ); + resourceName, + }); } return `${baseUrl}${resourceUri}/providers/microsoft.insights/metricNamespaces?region=global&api-version=${apiVersion}`; @@ -82,13 +84,12 @@ export default class UrlBuilder { resourceUri = query.resourceUri; } else { const { subscription, resourceGroup, metricNamespace, resourceName } = query; - resourceUri = UrlBuilder.buildResourceUri( + resourceUri = UrlBuilder.buildResourceUri(templateSrv, { subscription, resourceGroup, - templateSrv, metricNamespace, - resourceName - ); + resourceName, + }); } let url = `${baseUrl}${resourceUri}/providers/microsoft.insights/metricdefinitions?api-version=${apiVersion}`; diff --git a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/LogsQueryEditor/LogsQueryEditor.tsx b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/LogsQueryEditor/LogsQueryEditor.tsx index 6c12084a6bc..6659b927bd7 100644 --- a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/LogsQueryEditor/LogsQueryEditor.tsx +++ b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/LogsQueryEditor/LogsQueryEditor.tsx @@ -9,7 +9,6 @@ import { ResourceRowType } from '../ResourcePicker/types'; import FormatAsField from './FormatAsField'; import QueryField from './QueryField'; -import { setResource } from './setQueryValue'; import useMigrations from './useMigrations'; interface LogsQueryEditorProps { @@ -53,8 +52,7 @@ const LogsQueryEditor: React.FC = ({ ResourceRowType.Resource, ResourceRowType.Variable, ]} - setResource={setResource} - resourceUri={query.azureLogAnalytics?.resource} + resource={query.azureLogAnalytics?.resource ?? ''} queryType="logs" /> diff --git a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/LogsQueryEditor/setQueryValue.ts b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/LogsQueryEditor/setQueryValue.ts index b8f870b3f5d..c406e028bba 100644 --- a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/LogsQueryEditor/setQueryValue.ts +++ b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/LogsQueryEditor/setQueryValue.ts @@ -19,13 +19,3 @@ export function setFormatAs(query: AzureMonitorQuery, formatAs: string): AzureMo }, }; } - -export function setResource(query: AzureMonitorQuery, resourceURI: string | undefined): AzureMonitorQuery { - return { - ...query, - azureLogAnalytics: { - ...query.azureLogAnalytics, - resource: resourceURI, - }, - }; -} diff --git a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/MetricsQueryEditor/AggregationField.tsx b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/MetricsQueryEditor/AggregationField.tsx index b71bdd152ed..854b7fe03af 100644 --- a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/MetricsQueryEditor/AggregationField.tsx +++ b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/MetricsQueryEditor/AggregationField.tsx @@ -41,7 +41,7 @@ const AggregationField: React.FC = ({ ({ + ...(jest.requireActual('@grafana/runtime') as unknown as object), + getTemplateSrv: () => ({ + replace: (val: string) => { + return val; + }, + }), +})); + const variableOptionGroup = { label: 'Template variables', options: [], @@ -66,7 +75,10 @@ describe('MetricsQueryEditor', () => { it('should change resource when a resource is selected in the ResourcePicker', async () => { const mockDatasource = createMockDatasource({ resourcePickerData: createMockResourcePickerData() }); const query = createMockQuery(); - delete query?.azureMonitor?.resourceUri; + delete query?.subscription; + delete query?.azureMonitor?.resourceGroup; + delete query?.azureMonitor?.resourceName; + delete query?.azureMonitor?.metricNamespace; const onChange = jest.fn(); render( @@ -105,68 +117,11 @@ describe('MetricsQueryEditor', () => { expect(onChange).toBeCalledTimes(1); expect(onChange).toBeCalledWith( expect.objectContaining({ + subscription: 'def-456', azureMonitor: expect.objectContaining({ - resourceUri: - '/subscriptions/def-456/resourceGroups/dev-3/providers/Microsoft.Compute/virtualMachines/web-server', - }), - }) - ); - }); - - it('should reset metric namespace, metric name, and aggregation fields after selecting a new resource when a valid query has already been set', async () => { - const mockDatasource = createMockDatasource({ resourcePickerData: createMockResourcePickerData() }); - const query = createMockQuery(); - const onChange = jest.fn(); - - render( - {}} - /> - ); - - const resourcePickerButton = await screen.findByRole('button', { name: /grafana/ }); - - expect(screen.getByText('Microsoft.Compute/virtualMachines')).toBeInTheDocument(); - expect(screen.getByText('Metric A')).toBeInTheDocument(); - expect(screen.getByText('Average')).toBeInTheDocument(); - - expect(resourcePickerButton).toBeInTheDocument(); - expect(screen.queryByRole('button', { name: 'Expand Primary Subscription' })).not.toBeInTheDocument(); - resourcePickerButton.click(); - - const subscriptionButton = await screen.findByRole('button', { name: 'Expand Dev Subscription' }); - expect(subscriptionButton).toBeInTheDocument(); - expect(screen.queryByRole('button', { name: 'Expand Development 3' })).not.toBeInTheDocument(); - subscriptionButton.click(); - - const resourceGroupButton = await screen.findByRole('button', { name: 'Expand Development 3' }); - expect(resourceGroupButton).toBeInTheDocument(); - expect(screen.queryByLabelText('db-server')).not.toBeInTheDocument(); - resourceGroupButton.click(); - - const checkbox = await screen.findByLabelText('db-server'); - expect(checkbox).toBeInTheDocument(); - expect(checkbox).not.toBeChecked(); - await userEvent.click(checkbox); - expect(checkbox).toBeChecked(); - await userEvent.click(await screen.findByRole('button', { name: 'Apply' })); - - expect(onChange).toBeCalledTimes(1); - expect(onChange).toBeCalledWith( - expect.objectContaining({ - azureMonitor: expect.objectContaining({ - resourceUri: - '/subscriptions/def-456/resourceGroups/dev-3/providers/Microsoft.Compute/virtualMachines/db-server', - metricNamespace: undefined, - metricName: undefined, - aggregation: undefined, - timeGrain: '', - dimensionFilters: [], + metricNamespace: 'microsoft.compute/virtualmachines', + resourceGroup: 'dev-3', + resourceName: 'web-server', }), }) ); diff --git a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/MetricsQueryEditor/MetricsQueryEditor.tsx b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/MetricsQueryEditor/MetricsQueryEditor.tsx index 44a71591e37..6d60a68ac26 100644 --- a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/MetricsQueryEditor/MetricsQueryEditor.tsx +++ b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/MetricsQueryEditor/MetricsQueryEditor.tsx @@ -4,7 +4,7 @@ import { PanelData } from '@grafana/data/src/types'; import { EditorRows, EditorRow, EditorFieldGroup } from '@grafana/ui'; import type Datasource from '../../datasource'; -import type { AzureMonitorQuery, AzureMonitorOption, AzureMonitorErrorish } from '../../types'; +import type { AzureMonitorQuery, AzureMonitorOption, AzureMonitorErrorish, AzureMetricResource } from '../../types'; import ResourceField from '../ResourceField'; import { ResourceRowType } from '../ResourcePicker/types'; @@ -16,7 +16,6 @@ import MetricNamespaceField from './MetricNamespaceField'; import TimeGrainField from './TimeGrainField'; import TopField from './TopField'; import { useMetricNames, useMetricNamespaces, useMetricMetadata } from './dataHooks'; -import { setResource } from './setQueryValue'; interface MetricsQueryEditorProps { data: PanelData | undefined; @@ -38,6 +37,12 @@ const MetricsQueryEditor: React.FC = ({ const metricsMetadata = useMetricMetadata(query, datasource, onChange); const metricNamespaces = useMetricNamespaces(query, datasource, onChange, setError); const metricNames = useMetricNames(query, datasource, onChange, setError); + const resource: AzureMetricResource = { + subscription: query.subscription, + resourceGroup: query.azureMonitor?.resourceGroup, + metricNamespace: query.azureMonitor?.metricNamespace, + resourceName: query.azureMonitor?.resourceName, + }; return ( @@ -50,8 +55,7 @@ const MetricsQueryEditor: React.FC = ({ onQueryChange={onChange} setError={setError} selectableEntryTypes={[ResourceRowType.Resource]} - setResource={setResource} - resourceUri={query.azureMonitor?.resourceUri} + resource={resource} queryType={'metrics'} /> { name: 'useMetricNames', hook: useMetricNames, emptyQueryPartial: { - resourceUri: - '/subscriptions/99999999-cccc-bbbb-aaaa-9106972f9572/resourceGroups/grafanastaging/providers/Microsoft.Compute/virtualMachines/grafana', metricNamespace: 'azure/vm', + resourceGroup: 'rg', + resourceName: 'rn', }, customProperties: { - resourceUri: - '/subscriptions/99999999-cccc-bbbb-aaaa-9106972f9572/resourceGroups/grafanastaging/providers/Microsoft.Compute/virtualMachines/grafana', metricNamespace: 'azure/vm', + resourceGroup: 'rg', + resourceName: 'rn', metricName: 'metric-$ENVIRONMENT', }, expectedOptions: [ @@ -74,14 +74,14 @@ describe('AzureMonitor: metrics dataHooks', () => { name: 'useMetricNamespaces', hook: useMetricNamespaces, emptyQueryPartial: { - resourceUri: - '/subscriptions/99999999-cccc-bbbb-aaaa-9106972f9572/resourceGroups/grafanastaging/providers/Microsoft.Compute/virtualMachines/grafana', metricNamespace: 'azure/vm', + resourceGroup: 'rg', + resourceName: 'rn', }, customProperties: { - resourceUri: - '/subscriptions/99999999-cccc-bbbb-aaaa-9106972f9572/resourceGroups/grafanastaging/providers/Microsoft.Compute/virtualMachines/grafana', metricNamespace: 'azure/vm-$ENVIRONMENT', + resourceGroup: 'rg', + resourceName: 'rn', metricName: 'metric-name', }, expectedOptions: [ @@ -188,8 +188,8 @@ describe('AzureMonitor: metrics dataHooks', () => { name: 'useMetricMetadata', hook: useMetricMetadata, emptyQueryPartial: { - resourceUri: - '/subscriptions/99999999-cccc-bbbb-aaaa-9106972f9572/resourceGroups/grafanastaging/providers/Microsoft.Compute/virtualMachines/grafana', + resourceGroup: 'rg', + resourceName: 'rn', metricNamespace: 'azure/vm', metricName: 'Average CPU', }, diff --git a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/MetricsQueryEditor/dataHooks.ts b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/MetricsQueryEditor/dataHooks.ts index 637de16d069..07d9dad3933 100644 --- a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/MetricsQueryEditor/dataHooks.ts +++ b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/MetricsQueryEditor/dataHooks.ts @@ -39,15 +39,21 @@ export interface MetricMetadata { type OnChangeFn = (newQuery: AzureMonitorQuery) => void; export const useMetricNamespaces: DataHook = (query, datasource, onChange, setError) => { - const { metricNamespace, resourceUri } = query.azureMonitor ?? {}; + const { subscription } = query; + const { metricNamespace, resourceGroup, resourceName } = query.azureMonitor ?? {}; const metricNamespaces = useAsyncState( async () => { - if (!resourceUri) { + if (!subscription || !resourceGroup || !resourceName) { return; } - const results = await datasource.azureMonitorDatasource.getMetricNamespaces({ resourceUri }); + const results = await datasource.azureMonitorDatasource.getMetricNamespaces({ + subscription, + metricNamespace, + resourceGroup, + resourceName, + }); const options = formatOptions(results, metricNamespace); // Do some cleanup of the query state if need be @@ -58,28 +64,34 @@ export const useMetricNamespaces: DataHook = (query, datasource, onChange, setEr return options; }, setError, - [resourceUri] + [subscription, metricNamespace, resourceGroup, resourceName] ); return metricNamespaces; }; export const useMetricNames: DataHook = (query, datasource, onChange, setError) => { - const { metricNamespace, metricName, resourceUri } = query.azureMonitor ?? {}; + const { subscription } = query; + const { metricNamespace, metricName, resourceGroup, resourceName } = query.azureMonitor ?? {}; return useAsyncState( async () => { - if (!(metricNamespace && resourceUri)) { + if (!subscription || !metricNamespace || !resourceGroup || !resourceName) { return; } - const results = await datasource.azureMonitorDatasource.getMetricNames({ resourceUri, metricNamespace }); + const results = await datasource.azureMonitorDatasource.getMetricNames({ + subscription, + resourceGroup, + resourceName, + metricNamespace, + }); const options = formatOptions(results, metricName); return options; }, setError, - [resourceUri, metricNamespace] + [subscription, resourceGroup, resourceName, metricNamespace] ); }; @@ -94,18 +106,18 @@ const defaultMetricMetadata: MetricMetadata = { export const useMetricMetadata = (query: AzureMonitorQuery, datasource: Datasource, onChange: OnChangeFn) => { const [metricMetadata, setMetricMetadata] = useState(defaultMetricMetadata); - - const { resourceUri, metricNamespace, metricName, aggregation, timeGrain } = query.azureMonitor ?? {}; + const { subscription } = query; + const { resourceGroup, resourceName, metricNamespace, metricName, aggregation, timeGrain } = query.azureMonitor ?? {}; // Fetch new metric metadata when the fields change useEffect(() => { - if (!(resourceUri && metricNamespace && metricName)) { + if (!subscription || !resourceGroup || !resourceName || !metricNamespace || !metricName) { setMetricMetadata(defaultMetricMetadata); return; } datasource.azureMonitorDatasource - .getMetricMetadata({ resourceUri, metricNamespace, metricName }) + .getMetricMetadata({ subscription, resourceGroup, resourceName, metricNamespace, metricName }) .then((metadata) => { // TODO: Move the aggregationTypes and timeGrain defaults into `getMetricMetadata` const aggregations = (metadata.supportedAggTypes || [metadata.primaryAggType]).map((v) => ({ @@ -122,7 +134,7 @@ export const useMetricMetadata = (query: AzureMonitorQuery, datasource: Datasour primaryAggType: metadata.primaryAggType, }); }); - }, [datasource, resourceUri, metricNamespace, metricName]); + }, [datasource, subscription, resourceGroup, resourceName, metricNamespace, metricName]); // Update the query state in response to the meta data changing useEffect(() => { diff --git a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/MetricsQueryEditor/setQueryValue.test.ts b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/MetricsQueryEditor/setQueryValue.test.ts deleted file mode 100644 index d1a4d4f9155..00000000000 --- a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/MetricsQueryEditor/setQueryValue.test.ts +++ /dev/null @@ -1,27 +0,0 @@ -import createMockQuery from '../../__mocks__/query'; - -import { setResource } from './setQueryValue'; - -describe('setResource', () => { - it('should set a resource URI', () => { - const q = setResource(createMockQuery(), '/new-uri'); - expect(q.azureMonitor?.resourceUri).toEqual('/new-uri'); - }); - - it('should remove clean up dependent fields', () => { - const q = createMockQuery(); - expect(q.azureMonitor?.metricNamespace).not.toEqual(undefined); - expect(q.azureMonitor?.metricName).not.toEqual(undefined); - expect(q.azureMonitor?.aggregation).not.toEqual(undefined); - expect(q.azureMonitor?.timeGrain).not.toEqual(''); - expect(q.azureMonitor?.timeGrain).not.toEqual([]); - const newQ = setResource(createMockQuery(), '/new-uri'); - expect(newQ.azureMonitor).toMatchObject({ - metricNamespace: undefined, - metricName: undefined, - aggregation: undefined, - timeGrain: '', - dimensionFilters: [], - }); - }); -}); diff --git a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/MetricsQueryEditor/setQueryValue.ts b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/MetricsQueryEditor/setQueryValue.ts index 5bec09a6203..140ffc746d7 100644 --- a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/MetricsQueryEditor/setQueryValue.ts +++ b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/MetricsQueryEditor/setQueryValue.ts @@ -1,42 +1,10 @@ import { AzureMetricDimension, AzureMonitorQuery } from '../../types'; -export function setResource(query: AzureMonitorQuery, resourceURI: string | undefined): AzureMonitorQuery { - return { - ...query, - azureMonitor: { - ...query.azureMonitor, - resourceUri: resourceURI, - metricNamespace: undefined, - metricName: undefined, - aggregation: undefined, - timeGrain: '', - dimensionFilters: [], - }, - }; -} - export function setMetricNamespace(query: AzureMonitorQuery, metricNamespace: string | undefined): AzureMonitorQuery { if (query.azureMonitor?.metricNamespace === metricNamespace) { return query; } - let resourceUri = query.azureMonitor?.resourceUri; - - // Storage Account URIs need to be handled differently due to the additional storage services (blob/queue/table/file). - // When one of these namespaces is selected it does not form a part of the URI for the storage account and so must be appended. - // The 'default' path must also be appended. Without these two paths any API call will fail. - if (resourceUri && metricNamespace?.includes('Microsoft.Storage/storageAccounts')) { - const splitUri = resourceUri.split('/'); - const accountNameIndex = splitUri.findIndex((item) => item === 'storageAccounts') + 1; - const baseUri = splitUri.slice(0, accountNameIndex + 1).join('/'); - if (metricNamespace === 'Microsoft.Storage/storageAccounts') { - resourceUri = baseUri; - } else { - const subNamespace = metricNamespace.split('/')[2]; - resourceUri = `${baseUri}/${subNamespace}/default`; - } - } - return { ...query, azureMonitor: { @@ -46,7 +14,6 @@ export function setMetricNamespace(query: AzureMonitorQuery, metricNamespace: st aggregation: undefined, timeGrain: '', dimensionFilters: [], - resourceUri, }, }; } diff --git a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/QueryEditor/QueryEditor.tsx b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/QueryEditor/QueryEditor.tsx index 675effc30ba..d7407e6335e 100644 --- a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/QueryEditor/QueryEditor.tsx +++ b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/QueryEditor/QueryEditor.tsx @@ -45,7 +45,7 @@ const QueryEditor: React.FC = ({ [onChange, onRunQuery] ); - const query = usePreparedQuery(baseQuery, onQueryChange, setError); + const query = usePreparedQuery(baseQuery, onQueryChange); const subscriptionId = query.subscription || datasource.azureMonitorDatasource.defaultSubscriptionId; const variableOptionGroup = { diff --git a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/QueryEditor/usePreparedQuery.ts b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/QueryEditor/usePreparedQuery.ts index a400102897c..560d734cbf6 100644 --- a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/QueryEditor/usePreparedQuery.ts +++ b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/QueryEditor/usePreparedQuery.ts @@ -2,22 +2,17 @@ import deepEqual from 'fast-deep-equal'; import { defaults } from 'lodash'; import { useEffect, useMemo } from 'react'; -import { getTemplateSrv } from '@grafana/runtime'; - -import { AzureMonitorErrorish, AzureMonitorQuery, AzureQueryType } from '../../types'; +import { AzureMonitorQuery, AzureQueryType } from '../../types'; import migrateQuery from '../../utils/migrateQuery'; const DEFAULT_QUERY = { queryType: AzureQueryType.AzureMonitor, }; -const prepareQuery = ( - query: AzureMonitorQuery, - setError: (errorSource: string, error: AzureMonitorErrorish) => void -) => { +const prepareQuery = (query: AzureMonitorQuery) => { // Note: _.defaults does not apply default values deeply. const withDefaults = defaults({}, query, DEFAULT_QUERY); - const migratedQuery = migrateQuery(withDefaults, getTemplateSrv(), setError); + const migratedQuery = migrateQuery(withDefaults); // If we didn't make any changes to the object, then return the original object to keep the // identity the same, and not trigger any other useEffects or anything. @@ -27,12 +22,8 @@ const prepareQuery = ( /** * Returns queries with some defaults + migrations, and calls onChange function to notify if it changes */ -const usePreparedQuery = ( - query: AzureMonitorQuery, - onChangeQuery: (newQuery: AzureMonitorQuery) => void, - setError: (errorSource: string, error: AzureMonitorErrorish) => void -) => { - const preparedQuery = useMemo(() => prepareQuery(query, setError), [query, setError]); +const usePreparedQuery = (query: AzureMonitorQuery, onChangeQuery: (newQuery: AzureMonitorQuery) => void) => { + const preparedQuery = useMemo(() => prepareQuery(query), [query]); useEffect(() => { if (preparedQuery !== query) { diff --git a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/ResourceField/ResourceField.tsx b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/ResourceField/ResourceField.tsx index a402bd0a2b8..6d9f8540e7f 100644 --- a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/ResourceField/ResourceField.tsx +++ b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/ResourceField/ResourceField.tsx @@ -5,44 +5,28 @@ import { Button, Icon, Modal, useStyles2 } from '@grafana/ui'; import Datasource from '../../datasource'; import { ResourcePickerQueryType } from '../../resourcePicker/resourcePickerData'; -import { AzureQueryEditorFieldProps, AzureMonitorQuery, AzureResourceSummaryItem } from '../../types'; +import { AzureQueryEditorFieldProps, AzureMetricResource } from '../../types'; import { Field } from '../Field'; import ResourcePicker from '../ResourcePicker'; import getStyles from '../ResourcePicker/styles'; import { ResourceRowType } from '../ResourcePicker/types'; -import { parseResourceURI } from '../ResourcePicker/utils'; +import { parseResourceDetails, setResource } from '../ResourcePicker/utils'; -function parseResourceDetails(resourceURI: string) { - const parsed = parseResourceURI(resourceURI); - - if (!parsed) { - return undefined; - } - - return { - subscriptionName: parsed.subscriptionID, - resourceGroupName: parsed.resourceGroup, - resourceName: parsed.resource, - }; -} - -interface ResourceFieldProps extends AzureQueryEditorFieldProps { - setResource: (query: AzureMonitorQuery, resourceURI?: string) => AzureMonitorQuery; +interface ResourceFieldProps extends AzureQueryEditorFieldProps { selectableEntryTypes: ResourceRowType[]; queryType: ResourcePickerQueryType; - resourceUri?: string; + resource: T; inlineField?: boolean; labelWidth?: number; } -const ResourceField: React.FC = ({ +const ResourceField: React.FC> = ({ query, datasource, onQueryChange, - setResource, selectableEntryTypes, queryType, - resourceUri, + resource, inlineField, labelWidth, }) => { @@ -58,11 +42,11 @@ const ResourceField: React.FC = ({ }, []); const handleApply = useCallback( - (resourceURI: string | undefined) => { - onQueryChange(setResource(query, resourceURI)); + (resource: string | AzureMetricResource | undefined) => { + onQueryChange(setResource(query, resource)); closePicker(); }, - [closePicker, onQueryChange, query, setResource] + [closePicker, onQueryChange, query] ); return ( @@ -78,7 +62,7 @@ const ResourceField: React.FC = ({ > = ({ ); }; -interface ResourceLabelProps { - resource: string | undefined; +interface ResourceLabelProps { + resource: T; datasource: Datasource; } -const ResourceLabel = ({ resource, datasource }: ResourceLabelProps) => { +const ResourceLabel = ({ resource, datasource }: ResourceLabelProps) => { const [resourceComponents, setResourceComponents] = useState(parseResourceDetails(resource ?? '')); useEffect(() => { if (resource && parseResourceDetails(resource)) { - datasource.resourcePickerData.getResourceURIDisplayProperties(resource).then(setResourceComponents); + typeof resource === 'string' + ? datasource.resourcePickerData.getResourceURIDisplayProperties(resource).then(setResourceComponents) + : setResourceComponents(resource); } else { - setResourceComponents(undefined); + setResourceComponents({}); } }, [datasource.resourcePickerData, resource]); - if (!resource) { + if (!resource || (typeof resource === 'object' && !resource.subscription)) { return <>Select a resource; } @@ -118,19 +104,11 @@ const ResourceLabel = ({ resource, datasource }: ResourceLabelProps) => { return ; } - if (resource.startsWith('$')) { - return ( - - {resource} - - ); - } - return <>{resource}; }; interface FormattedResourceProps { - resource: AzureResourceSummaryItem; + resource: AzureMetricResource; } const FormattedResource = ({ resource }: FormattedResourceProps) => { @@ -139,20 +117,20 @@ const FormattedResource = ({ resource }: FormattedResourceProps) => { if (resource.resourceName) { return ( - {resource.resourceName} + {resource.resourceName.split('/')[0]} ); } - if (resource.resourceGroupName) { + if (resource.resourceGroup) { return ( - {resource.resourceGroupName} + {resource.resourceGroup} ); } return ( - {resource.subscriptionName} + {resource.subscription} ); }; diff --git a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/ResourcePicker/Advanced.test.tsx b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/ResourcePicker/Advanced.test.tsx new file mode 100644 index 00000000000..e9efb20c08d --- /dev/null +++ b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/ResourcePicker/Advanced.test.tsx @@ -0,0 +1,35 @@ +import { render, screen } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import React from 'react'; + +import Advanced from './Advanced'; + +describe('AzureMonitor ResourcePicker', () => { + it('should set a parameter as an object', async () => { + const onChange = jest.fn(); + const { rerender } = render(); + const advancedSection = screen.getByText('Advanced'); + advancedSection.click(); + + const subsInput = await screen.findByLabelText('Subscription'); + await userEvent.type(subsInput, 'd'); + expect(onChange).toHaveBeenCalledWith({ subscription: 'd' }); + + rerender(); + expect(screen.getByLabelText('Subscription').outerHTML).toMatch('value="def-123"'); + }); + + it('should set a parameter as uri', async () => { + const onChange = jest.fn(); + const { rerender } = render(); + const advancedSection = screen.getByText('Advanced'); + advancedSection.click(); + + const subsInput = await screen.findByLabelText('Resource URI'); + await userEvent.type(subsInput, '/'); + expect(onChange).toHaveBeenCalledWith('/'); + + rerender(); + expect(screen.getByLabelText('Resource URI').outerHTML).toMatch('value="/subscriptions/sub"'); + }); +}); diff --git a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/ResourcePicker/Advanced.tsx b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/ResourcePicker/Advanced.tsx new file mode 100644 index 00000000000..933fdcdb42e --- /dev/null +++ b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/ResourcePicker/Advanced.tsx @@ -0,0 +1,124 @@ +import React, { useState } from 'react'; + +import { Icon, Input, Tooltip, Collapse, Label, InlineField } from '@grafana/ui'; + +import { AzureMetricResource } from '../../types'; +import { Space } from '../Space'; + +interface ResourcePickerProps { + resource: T; + onChange: (resource: T) => void; +} + +const Advanced = ({ resource, onChange }: ResourcePickerProps) => { + const [isAdvancedOpen, setIsAdvancedOpen] = useState(!!resource && JSON.stringify(resource).includes('$')); + + return ( +
+ setIsAdvancedOpen(!isAdvancedOpen)} + > + {typeof resource === 'string' ? ( + <> + {' '} + + onChange(event.currentTarget.value)} + placeholder="ex: /subscriptions/$subId" + /> + + ) : ( + <> + + onChange({ ...resource, subscription: event.currentTarget.value })} + placeholder="aaaaaaaa-bbbb-cccc-dddd-eeeeeeee" + /> + + + onChange({ ...resource, resourceGroup: event.currentTarget.value })} + placeholder="resource-group" + /> + + + onChange({ ...resource, metricNamespace: event.currentTarget.value })} + placeholder="Microsoft.Insights/metricNamespaces" + /> + + + onChange({ ...resource, resourceName: event.currentTarget.value })} + placeholder="name" + /> + + + )} + + +
+ ); +}; + +export default Advanced; diff --git a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/ResourcePicker/ResourcePicker.test.tsx b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/ResourcePicker/ResourcePicker.test.tsx index 2f8055ca82c..9b99c46bd2a 100644 --- a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/ResourcePicker/ResourcePicker.test.tsx +++ b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/ResourcePicker/ResourcePicker.test.tsx @@ -41,7 +41,7 @@ const queryType: ResourcePickerQueryType = 'logs'; const defaultProps = { templateVariables: [], - resourceURI: noResourceURI, + resource: noResourceURI, resourcePickerData: createMockResourcePickerData(), onCancel: noop, onApply: noop, @@ -59,7 +59,7 @@ describe('AzureMonitor ResourcePicker', () => { window.HTMLElement.prototype.scrollIntoView = jest.fn(); }); it('should pre-load subscriptions when there is no existing selection', async () => { - render(); + render(); const subscriptionCheckbox = await screen.findByLabelText('Primary Subscription'); expect(subscriptionCheckbox).toBeInTheDocument(); expect(subscriptionCheckbox).not.toBeChecked(); @@ -68,7 +68,7 @@ describe('AzureMonitor ResourcePicker', () => { }); it('should show a subscription as selected if there is one saved', async () => { - render(); + render(); const subscriptionCheckboxes = await screen.findAllByLabelText('Dev Subscription'); expect(subscriptionCheckboxes.length).toBe(2); expect(subscriptionCheckboxes[0]).toBeChecked(); @@ -76,7 +76,7 @@ describe('AzureMonitor ResourcePicker', () => { }); it('should show a resourceGroup as selected if there is one saved', async () => { - render(); + render(); const resourceGroupCheckboxes = await screen.findAllByLabelText('A Great Resource Group'); expect(resourceGroupCheckboxes.length).toBe(2); expect(resourceGroupCheckboxes[0]).toBeChecked(); @@ -84,7 +84,7 @@ describe('AzureMonitor ResourcePicker', () => { }); it('should show scroll down to a resource and mark it as selected if there is one saved', async () => { - render(); + render(); const resourceCheckboxes = await screen.findAllByLabelText('db-server'); expect(resourceCheckboxes.length).toBe(2); expect(resourceCheckboxes[0]).toBeChecked(); @@ -92,7 +92,7 @@ describe('AzureMonitor ResourcePicker', () => { }); it('opens the selected nested resources', async () => { - render(); + render(); const collapseSubscriptionBtn = await screen.findByLabelText('Collapse Dev Subscription'); expect(collapseSubscriptionBtn).toBeInTheDocument(); const collapseResourceGroupBtn = await screen.findByLabelText('Collapse A Great Resource Group'); @@ -100,7 +100,7 @@ describe('AzureMonitor ResourcePicker', () => { }); it('scrolls down to the selected resource', async () => { - render(); + render(); await screen.findByLabelText('Collapse A Great Resource Group'); expect(window.HTMLElement.prototype.scrollIntoView).toBeCalledTimes(1); }); @@ -127,6 +127,19 @@ describe('AzureMonitor ResourcePicker', () => { expect(onApply).toBeCalledWith('/subscriptions/def-123'); }); + it('should call onApply with a new subscription when a user clicks on the checkbox in the row', async () => { + const onApply = jest.fn(); + render(); + const subscriptionCheckbox = await screen.findByLabelText('Primary Subscription'); + expect(subscriptionCheckbox).toBeInTheDocument(); + expect(subscriptionCheckbox).not.toBeChecked(); + subscriptionCheckbox.click(); + const applyButton = screen.getByRole('button', { name: 'Apply' }); + applyButton.click(); + expect(onApply).toBeCalledTimes(1); + expect(onApply).toBeCalledWith({ subscription: 'def-123' }); + }); + it('should call onApply with a new subscription uri when a user types it in the selection box', async () => { const onApply = jest.fn(); render(); @@ -147,6 +160,44 @@ describe('AzureMonitor ResourcePicker', () => { expect(onApply).toBeCalledWith('/subscriptions/def-123'); }); + it('should call onApply with a new subscription when a user types it in the selection box', async () => { + const onApply = jest.fn(); + render(); + const subscriptionCheckbox = await screen.findByLabelText('Primary Subscription'); + expect(subscriptionCheckbox).toBeInTheDocument(); + expect(subscriptionCheckbox).not.toBeChecked(); + + const advancedSection = screen.getByText('Advanced'); + advancedSection.click(); + + const advancedInput = await screen.findByLabelText('Subscription'); + await userEvent.type(advancedInput, 'def-123'); + + const applyButton = screen.getByRole('button', { name: 'Apply' }); + applyButton.click(); + + expect(onApply).toBeCalledTimes(1); + expect(onApply).toBeCalledWith({ subscription: 'def-123' }); + }); + + it('should show unselect a subscription if the value is manually edited', async () => { + render(); + const subscriptionCheckboxes = await screen.findAllByLabelText('Dev Subscription'); + expect(subscriptionCheckboxes.length).toBe(2); + expect(subscriptionCheckboxes[0]).toBeChecked(); + expect(subscriptionCheckboxes[1]).toBeChecked(); + + const advancedSection = screen.getByText('Advanced'); + advancedSection.click(); + + const advancedInput = await screen.findByLabelText('Subscription'); + await userEvent.type(advancedInput, 'def-123'); + + const updatedCheckboxes = await screen.findAllByLabelText('Dev Subscription'); + expect(updatedCheckboxes.length).toBe(1); + expect(updatedCheckboxes[0]).not.toBeChecked(); + }); + it('renders a search field which show search results when there are results', async () => { render(); const searchRow1 = screen.queryByLabelText('search-result'); @@ -204,7 +255,7 @@ describe('AzureMonitor ResourcePicker', () => { }); it('resets result when the user clears their search', async () => { - render(); + render(); const subscriptionCheckboxBeforeSearch = await screen.findByLabelText('Primary Subscription'); expect(subscriptionCheckboxBeforeSearch).toBeInTheDocument(); diff --git a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/ResourcePicker/ResourcePicker.tsx b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/ResourcePicker/ResourcePicker.tsx index ad74846a93e..532d74110cb 100644 --- a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/ResourcePicker/ResourcePicker.tsx +++ b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/ResourcePicker/ResourcePicker.tsx @@ -2,63 +2,67 @@ import { cx } from '@emotion/css'; import React, { useCallback, useEffect, useState } from 'react'; import { useEffectOnce } from 'react-use'; -import { Alert, Button, Icon, Input, LoadingPlaceholder, Tooltip, useStyles2, Collapse, Label } from '@grafana/ui'; +import { Alert, Button, LoadingPlaceholder, useStyles2 } from '@grafana/ui'; import ResourcePickerData, { ResourcePickerQueryType } from '../../resourcePicker/resourcePickerData'; +import { AzureMetricResource } from '../../types'; import messageFromError from '../../utils/messageFromError'; import { Space } from '../Space'; +import Advanced from './Advanced'; import NestedRow from './NestedRow'; import Search from './Search'; import getStyles from './styles'; import { ResourceRow, ResourceRowGroup, ResourceRowType } from './types'; -import { findRow } from './utils'; +import { findRow, parseResourceDetails, resourceToString } from './utils'; -interface ResourcePickerProps { +interface ResourcePickerProps { resourcePickerData: ResourcePickerData; - resourceURI: string | undefined; + resource: T; selectableEntryTypes: ResourceRowType[]; queryType: ResourcePickerQueryType; - onApply: (resourceURI: string | undefined) => void; + onApply: (resource?: T) => void; onCancel: () => void; } const ResourcePicker = ({ resourcePickerData, - resourceURI, + resource, onApply, onCancel, selectableEntryTypes, queryType, -}: ResourcePickerProps) => { +}: ResourcePickerProps) => { const styles = useStyles2(getStyles); const [isLoading, setIsLoading] = useState(false); const [rows, setRows] = useState([]); const [selectedRows, setSelectedRows] = useState([]); - const [internalSelectedURI, setInternalSelectedURI] = useState(resourceURI); + const [internalSelected, setInternalSelected] = useState(resource); const [errorMessage, setErrorMessage] = useState(undefined); - const [isAdvancedOpen, setIsAdvancedOpen] = useState(resourceURI?.includes('$')); const [shouldShowLimitFlag, setShouldShowLimitFlag] = useState(false); // Sync the resourceURI prop to internal state useEffect(() => { - setInternalSelectedURI(resourceURI); - }, [resourceURI]); + setInternalSelected(resource); + }, [resource]); const loadInitialData = useCallback(async () => { if (!isLoading) { try { setIsLoading(true); - const resources = await resourcePickerData.fetchInitialRows(queryType, internalSelectedURI || ''); + const resources = await resourcePickerData.fetchInitialRows( + queryType, + parseResourceDetails(internalSelected ?? {}) + ); setRows(resources); } catch (error) { setErrorMessage(messageFromError(error)); } setIsLoading(false); } - }, [internalSelectedURI, isLoading, resourcePickerData, queryType]); + }, [internalSelected, isLoading, resourcePickerData, queryType]); useEffectOnce(() => { loadInitialData(); @@ -66,11 +70,11 @@ const ResourcePicker = ({ // set selected row data whenever row or selection changes useEffect(() => { - if (!internalSelectedURI) { + if (!internalSelected) { setSelectedRows([]); } - const found = internalSelectedURI && findRow(rows, internalSelectedURI); + const found = internalSelected && findRow(rows, resourceToString(internalSelected)); if (found) { return setSelectedRows([ { @@ -79,7 +83,8 @@ const ResourcePicker = ({ }, ]); } - }, [internalSelectedURI, rows]); + return setSelectedRows([]); + }, [internalSelected, rows]); // Request resources for an expanded resource group const requestNestedRows = useCallback( @@ -103,13 +108,21 @@ const ResourcePicker = ({ [resourcePickerData, rows, queryType] ); - const handleSelectionChanged = useCallback((row: ResourceRow, isSelected: boolean) => { - isSelected ? setInternalSelectedURI(row.uri) : setInternalSelectedURI(undefined); - }, []); + const resourceIsString = typeof resource === 'string'; + const handleSelectionChanged = useCallback( + (row: ResourceRow, isSelected: boolean) => { + isSelected + ? setInternalSelected(resourceIsString ? row.uri : parseResourceDetails(row.uri)) + : setInternalSelected(resourceIsString ? '' : {}); + }, + [resourceIsString] + ); const handleApply = useCallback(() => { - onApply(internalSelectedURI); - }, [internalSelectedURI, onApply]); + if (internalSelected) { + onApply(resourceIsString ? internalSelected : parseResourceDetails(internalSelected)); + } + }, [resourceIsString, internalSelected, onApply]); const handleSearch = useCallback( async (searchWord: string) => { @@ -216,44 +229,7 @@ const ResourcePicker = ({ )} - setIsAdvancedOpen(!isAdvancedOpen)} - > - - setInternalSelectedURI(event.currentTarget.value)} - placeholder="ex: /subscriptions/$subId" - /> - - + setInternalSelected(r)} />