From d5f40b63bbafe0126086d42bde2caae59bed452b Mon Sep 17 00:00:00 2001 From: Sarah Zinger Date: Fri, 25 Mar 2022 14:12:52 -0400 Subject: [PATCH] Azure Monitor: Add support for multiple template variables in resource picker (#46215) --- .../azure_log_analytics_datasource.test.ts | 10 ++- .../azure_log_analytics_datasource.ts | 6 +- .../LogsQueryEditor/ResourceField.tsx | 3 - .../ResourcePicker/ResourcePicker.test.tsx | 19 ++-- .../ResourcePicker/ResourcePicker.tsx | 86 +++++++++++-------- .../components/ResourcePicker/utils.ts | 7 +- .../resourcePicker/resourcePickerData.test.ts | 13 +-- .../resourcePicker/resourcePickerData.ts | 17 ++-- 8 files changed, 90 insertions(+), 71 deletions(-) diff --git a/public/app/plugins/datasource/grafana-azure-monitor-datasource/azure_log_analytics/azure_log_analytics_datasource.test.ts b/public/app/plugins/datasource/grafana-azure-monitor-datasource/azure_log_analytics/azure_log_analytics_datasource.test.ts index d7031081f88..35a26d345c2 100644 --- a/public/app/plugins/datasource/grafana-azure-monitor-datasource/azure_log_analytics/azure_log_analytics_datasource.test.ts +++ b/public/app/plugins/datasource/grafana-azure-monitor-datasource/azure_log_analytics/azure_log_analytics_datasource.test.ts @@ -27,9 +27,11 @@ describe('AzureLogAnalyticsDatasource', () => { const ctx: any = {}; beforeEach(() => { + templateSrv.init([singleVariable]); ctx.instanceSettings = { jsonData: { subscriptionId: 'xxx' }, url: 'http://azureloganalyticsapi', + templateSrv: templateSrv, }; ctx.ds = new AzureMonitorDatasource(ctx.instanceSettings); @@ -76,10 +78,11 @@ describe('AzureLogAnalyticsDatasource', () => { describe('When performing getSchema', () => { beforeEach(() => { - ctx.ds.azureLogAnalyticsDatasource.getResource = jest.fn().mockImplementation((path: string) => { + ctx.mockGetResource = jest.fn().mockImplementation((path: string) => { expect(path).toContain('metadata'); return Promise.resolve(FakeSchemaData.getlogAnalyticsFakeMetadata()); }); + ctx.ds.azureLogAnalyticsDatasource.getResource = ctx.mockGetResource; }); it('should return a schema to use with monaco-kusto', async () => { @@ -112,6 +115,11 @@ describe('AzureLogAnalyticsDatasource', () => { }, ]); }); + + it('should interpolate variables when making a request for a schema with a uri that contains template variables', async () => { + await ctx.ds.azureLogAnalyticsDatasource.getKustoSchema('myWorkspace/$var1'); + expect(ctx.mockGetResource).lastCalledWith('loganalytics/v1myWorkspace/var1-foo/metadata'); + }); }); describe('When performing annotationQuery', () => { diff --git a/public/app/plugins/datasource/grafana-azure-monitor-datasource/azure_log_analytics/azure_log_analytics_datasource.ts b/public/app/plugins/datasource/grafana-azure-monitor-datasource/azure_log_analytics/azure_log_analytics_datasource.ts index b4458f8d640..c98aac12ece 100644 --- a/public/app/plugins/datasource/grafana-azure-monitor-datasource/azure_log_analytics/azure_log_analytics_datasource.ts +++ b/public/app/plugins/datasource/grafana-azure-monitor-datasource/azure_log_analytics/azure_log_analytics_datasource.ts @@ -109,8 +109,10 @@ export default class AzureLogAnalyticsDatasource extends DataSourceWithBackend< } async getKustoSchema(resourceUri: string) { - const metadata = await this.getMetadata(resourceUri); - return transformMetadataToKustoSchema(metadata, resourceUri); + const templateSrv = getTemplateSrv(); + const interpolatedUri = templateSrv.replace(resourceUri, {}, interpolateVariable); + const metadata = await this.getMetadata(interpolatedUri); + return transformMetadataToKustoSchema(metadata, interpolatedUri); } applyTemplateVariables(target: AzureMonitorQuery, scopedVars: ScopedVars): AzureMonitorQuery { diff --git a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/LogsQueryEditor/ResourceField.tsx b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/LogsQueryEditor/ResourceField.tsx index 11cb59d049c..4b8f4f9daab 100644 --- a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/LogsQueryEditor/ResourceField.tsx +++ b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/LogsQueryEditor/ResourceField.tsx @@ -47,8 +47,6 @@ const ResourceField: React.FC = ({ query, datasource [closePicker, onQueryChange, query] ); - const templateVariables = datasource.getVariables(); - return ( <> = ({ query, datasource { expect(onApply).toBeCalledTimes(1); expect(onApply).toBeCalledWith('/subscriptions/def-123'); }); - it('should call onApply with a template variable when a user selects it', async () => { + + it('should call onApply with a new subscription uri when a user types it', async () => { const onApply = jest.fn(); - render(); + render(); + const subscriptionCheckbox = await screen.findByLabelText('Primary Subscription'); + expect(subscriptionCheckbox).toBeInTheDocument(); + expect(subscriptionCheckbox).not.toBeChecked(); - const expandButton = await screen.findByLabelText('Expand Template variables'); - expandButton.click(); + const advancedSection = screen.getByText('Advanced'); + advancedSection.click(); - const workSpaceCheckbox = await screen.findByLabelText('$workspace'); - workSpaceCheckbox.click(); + const advancedInput = await screen.findByLabelText('Resource URI'); + userEvent.type(advancedInput, '/subscriptions/def-123'); const applyButton = screen.getByRole('button', { name: 'Apply' }); applyButton.click(); expect(onApply).toBeCalledTimes(1); - expect(onApply).toBeCalledWith('$workspace'); + expect(onApply).toBeCalledWith('/subscriptions/def-123'); }); describe('when rendering resource picker without any selectable entry types', () => { 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 5cea1a1a71e..03c335b6690 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 @@ -1,6 +1,6 @@ import { css } from '@emotion/css'; import { GrafanaTheme2 } from '@grafana/data'; -import { Alert, Button, LoadingPlaceholder, useStyles2 } from '@grafana/ui'; +import { Alert, Button, Icon, Input, LoadingPlaceholder, Tooltip, useStyles2, Collapse, Label } from '@grafana/ui'; import React, { useCallback, useEffect, useMemo, useState } from 'react'; import ResourcePickerData from '../../resourcePicker/resourcePickerData'; @@ -10,11 +10,9 @@ import NestedResourceTable from './NestedResourceTable'; import { ResourceRow, ResourceRowGroup, ResourceRowType } from './types'; import { addResources, findRow, parseResourceURI } from './utils'; -const TEMPLATE_VARIABLE_GROUP_ID = '$$grafana-templateVariables$$'; interface ResourcePickerProps { resourcePickerData: ResourcePickerData; resourceURI: string | undefined; - templateVariables: string[]; selectableEntryTypes: ResourceRowType[]; onApply: (resourceURI: string | undefined) => void; @@ -24,7 +22,6 @@ interface ResourcePickerProps { const ResourcePicker = ({ resourcePickerData, resourceURI, - templateVariables, onApply, onCancel, selectableEntryTypes, @@ -36,7 +33,7 @@ const ResourcePicker = ({ const [azureRows, setAzureRows] = useState([]); const [internalSelectedURI, setInternalSelectedURI] = useState(resourceURI); const [errorMessage, setErrorMessage] = useState(undefined); - + const [isAdvancedOpen, setIsAdvancedOpen] = useState(resourceURI?.includes('$')); // Sync the resourceURI prop to internal state useEffect(() => { setInternalSelectedURI(resourceURI); @@ -85,14 +82,10 @@ const ResourcePicker = ({ } }, [resourcePickerData, internalSelectedURI, azureRows, loadingStatus]); - const rows = useMemo(() => { - const templateVariableRow = transformVariablesToRow(templateVariables); - return templateVariables.length ? [...azureRows, templateVariableRow] : azureRows; - }, [azureRows, templateVariables]); - // Map the selected item into an array of rows const selectedResourceRows = useMemo(() => { - const found = internalSelectedURI && findRow(rows, internalSelectedURI); + const found = internalSelectedURI && findRow(azureRows, internalSelectedURI); + return found ? [ { @@ -101,7 +94,7 @@ const ResourcePicker = ({ }, ] : []; - }, [internalSelectedURI, rows]); + }, [internalSelectedURI, azureRows]); // Request resources for a expanded resource group const requestNestedRows = useCallback( @@ -109,12 +102,8 @@ const ResourcePicker = ({ // clear error message (also when loading cached resources) setErrorMessage(undefined); - // If we already have children, we don't need to re-fetch them. Also abort if we're expanding the special - // template variable group, though that shouldn't happen in practice - if ( - resourceGroupOrSubscription.children?.length || - resourceGroupOrSubscription.uri === TEMPLATE_VARIABLE_GROUP_ID - ) { + // If we already have children, we don't need to re-fetch them. + if (resourceGroupOrSubscription.children?.length) { return; } @@ -152,7 +141,7 @@ const ResourcePicker = ({ ) : ( <> {selectedResourceRows.length > 0 && ( <> -
Selection
+ )} - + setIsAdvancedOpen(!isAdvancedOpen)} + > + + setInternalSelectedURI(event.currentTarget.value)} + placeholder="ex: /subscriptions/$subId" + /> + + + @@ -215,20 +242,3 @@ const getStyles = (theme: GrafanaTheme2) => ({ color: theme.colors.text.secondary, }), }); - -function transformVariablesToRow(templateVariables: string[]): ResourceRow { - return { - id: TEMPLATE_VARIABLE_GROUP_ID, - uri: TEMPLATE_VARIABLE_GROUP_ID, - name: 'Template variables', - type: ResourceRowType.VariableGroup, - typeLabel: 'Variables', - children: templateVariables.map((v) => ({ - id: v, - uri: v, - name: v, - type: ResourceRowType.Variable, - typeLabel: 'Variable', - })), - }; -} diff --git a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/ResourcePicker/utils.ts b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/ResourcePicker/utils.ts index 3c3511d75e2..7595a0ff2ba 100644 --- a/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/ResourcePicker/utils.ts +++ b/public/app/plugins/datasource/grafana-azure-monitor-datasource/components/ResourcePicker/utils.ts @@ -48,9 +48,12 @@ export function addResources(rows: ResourceRowGroup, targetParentId: string, new return produce(rows, (draftState) => { const draftRow = findRow(draftState, targetParentId); + // we can't find the selected resource in our list of resources, + // probably means user has either mistyped in the input field + // or is using template variables. + // either way no need to throw, just show that none of the resources are checked if (!draftRow) { - // This case shouldn't happen often because we're usually coming here from a resource we already have - throw new Error('Unable to find resource'); + return; } draftRow.children = newResources; diff --git a/public/app/plugins/datasource/grafana-azure-monitor-datasource/resourcePicker/resourcePickerData.test.ts b/public/app/plugins/datasource/grafana-azure-monitor-datasource/resourcePicker/resourcePickerData.test.ts index 887db6140c1..4942b5089ca 100644 --- a/public/app/plugins/datasource/grafana-azure-monitor-datasource/resourcePicker/resourcePickerData.test.ts +++ b/public/app/plugins/datasource/grafana-azure-monitor-datasource/resourcePicker/resourcePickerData.test.ts @@ -92,7 +92,7 @@ describe('AzureMonitor resourcePickerData', () => { await resourcePickerData.getSubscriptions(); throw Error('expected getSubscriptions to fail but it succeeded'); } catch (err) { - expect(err.message).toEqual('unable to fetch subscriptions'); + expect(err.message).toEqual('No subscriptions were found'); } }); }); @@ -163,17 +163,6 @@ describe('AzureMonitor resourcePickerData', () => { }); }); - it('throws an error if it does not receive data', async () => { - const mockResponse = { data: [] }; - const { resourcePickerData } = createResourcePickerData([mockResponse]); - try { - await resourcePickerData.getResourceGroupsBySubscriptionId('123'); - throw Error('expected getSubscriptions to fail but it succeeded'); - } catch (err) { - expect(err.message).toEqual('unable to fetch resource groups'); - } - }); - it('throws an error if it recieves data with a malformed uri', async () => { const mockResponse = { data: [ diff --git a/public/app/plugins/datasource/grafana-azure-monitor-datasource/resourcePicker/resourcePickerData.ts b/public/app/plugins/datasource/grafana-azure-monitor-datasource/resourcePicker/resourcePickerData.ts index c2dee619835..e557a2d43f2 100644 --- a/public/app/plugins/datasource/grafana-azure-monitor-datasource/resourcePicker/resourcePickerData.ts +++ b/public/app/plugins/datasource/grafana-azure-monitor-datasource/resourcePicker/resourcePickerData.ts @@ -57,7 +57,7 @@ export default class ResourcePickerData extends DataSourceWithBackend(query, 1, options); if (!resourceResponse.data.length) { - throw new Error('unable to fetch subscriptions'); + throw new Error('No subscriptions were found'); } resources = resources.concat(resourceResponse.data); $skipToken = resourceResponse.$skipToken; @@ -100,9 +100,6 @@ export default class ResourcePickerData extends DataSourceWithBackend(query, 1, options); - if (!resourceResponse.data.length) { - throw new Error('unable to fetch resource groups'); - } resourceGroups = resourceGroups.concat(resourceResponse.data); $skipToken = resourceResponse.$skipToken; allFetched = !$skipToken; @@ -150,7 +147,7 @@ export default class ResourcePickerData extends DataSourceWithBackend { - const { subscriptionID, resourceGroup } = parseResourceURI(resourceURI) ?? {}; + const { subscriptionID, resourceGroup, resource } = parseResourceURI(resourceURI) ?? {}; if (!subscriptionID) { throw new Error('Invalid resource URI passed'); @@ -189,7 +186,15 @@ export default class ResourcePickerData extends DataSourceWithBackend