Azure Monitor: Bug fix for variable interpolations in metrics dropdowns (#43251)

* Azure Monitor: Bug fix for template variables.
- Adds back support for custom dropdown field names (such as those that use template variables)
- No longer clears a dropdown if selected value is an unknown option. (Hopefully makes debugging dynamically generated dashboards from json easier to debug).
- Add back support in UI for custom values
This commit is contained in:
Sarah Zinger 2021-12-20 10:11:10 -05:00 committed by GitHub
parent bd841edc38
commit b1df13e630
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 112 additions and 167 deletions

View File

@ -34,6 +34,7 @@ const MetricNameField: React.FC<MetricNameProps> = ({ metricNames, query, variab
onChange={handleChange}
options={options}
width={38}
allowCustomValue
/>
</Field>
);

View File

@ -44,6 +44,7 @@ const MetricNamespaceField: React.FC<MetricNamespaceFieldProps> = ({
onChange={handleChange}
options={options}
width={38}
allowCustomValue
/>
</Field>
);

View File

@ -36,6 +36,7 @@ const ResourceGroupsField: React.FC<ResourceGroupsFieldProps> = ({
onChange={handleChange}
options={options}
width={38}
allowCustomValue
/>
</Field>
);

View File

@ -36,6 +36,7 @@ const ResourceNameField: React.FC<ResourceNameFieldProps> = ({
onChange={handleChange}
options={options}
width={38}
allowCustomValue
/>
</Field>
);

View File

@ -40,6 +40,7 @@ const NamespaceField: React.FC<NamespaceFieldProps> = ({
onChange={handleChange}
options={options}
width={38}
allowCustomValue
/>
</Field>
);

View File

@ -9,8 +9,9 @@ import {
useResourceGroups,
useResourceNames,
useResourceTypes,
useSubscriptions,
} from './dataHooks';
import { AzureMetricQuery, AzureMonitorOption, AzureQueryType } from '../../types';
import { AzureMetricQuery, AzureMonitorOption, AzureMonitorQuery, AzureQueryType } from '../../types';
import createMockDatasource from '../../__mocks__/datasource';
import { MockedObjectDeep } from 'ts-jest/dist/utils/testing';
import Datasource from '../../datasource';
@ -88,13 +89,12 @@ interface TestScenario {
name: string;
hook: DataHook;
// For conviencence, only need to define the azureMonitor part of the query
// For convenience, only need to define the azureMonitor part of the query for some tests
emptyQueryPartial: AzureMetricQuery;
validQueryPartial: AzureMetricQuery;
invalidQueryPartial: AzureMetricQuery;
templateVariableQueryPartial: AzureMetricQuery;
customProperties: AzureMetricQuery;
topLevelCustomProperties?: Partial<AzureMonitorQuery>;
expectedClearedQueryPartial?: AzureMetricQuery;
expectedCustomPropertyResults?: Array<AzureMonitorOption<string>>;
expectedOptions: AzureMonitorOption[];
}
@ -106,18 +106,38 @@ describe('AzureMonitor: metrics dataHooks', () => {
};
const testTable: TestScenario[] = [
{
name: 'useSubscriptions',
hook: useSubscriptions,
emptyQueryPartial: {},
topLevelCustomProperties: {
subscription: 'subscription-$ENVIRONMENT',
},
customProperties: {},
expectedOptions: [
{
label: 'sub-abc-123',
value: 'sub-abc-123',
},
],
expectedCustomPropertyResults: [
{
label: 'sub-abc-123',
value: 'sub-abc-123',
},
{
label: 'subscription-$ENVIRONMENT',
value: 'subscription-$ENVIRONMENT',
},
],
},
{
name: 'useResourceGroups',
hook: useResourceGroups,
emptyQueryPartial: {},
validQueryPartial: {
resourceGroup: 'web-app-development',
},
invalidQueryPartial: {
resourceGroup: 'wrong-resource-group`',
},
templateVariableQueryPartial: {
resourceGroup: '$rg',
customProperties: {
resourceGroup: 'resource-group-$ENVIRONMENT',
},
expectedOptions: [
{
@ -129,9 +149,11 @@ describe('AzureMonitor: metrics dataHooks', () => {
value: 'web-app-development',
},
],
expectedClearedQueryPartial: {
resourceGroup: undefined,
},
expectedCustomPropertyResults: [
{ label: 'Web App - Production', value: 'web-app-production' },
{ label: 'Web App - Development', value: 'web-app-development' },
{ label: 'resource-group-$ENVIRONMENT', value: 'resource-group-$ENVIRONMENT' },
],
},
{
@ -140,17 +162,9 @@ describe('AzureMonitor: metrics dataHooks', () => {
emptyQueryPartial: {
resourceGroup: 'web-app-development',
},
validQueryPartial: {
customProperties: {
resourceGroup: 'web-app-development',
metricDefinition: 'azure/vm',
},
invalidQueryPartial: {
resourceGroup: 'web-app-development',
metricDefinition: 'azure/invalid-resource-type',
},
templateVariableQueryPartial: {
resourceGroup: 'web-app-development',
metricDefinition: '$rt',
metricDefinition: 'azure/resource-type-$ENVIRONMENT',
},
expectedOptions: [
{
@ -162,12 +176,12 @@ describe('AzureMonitor: metrics dataHooks', () => {
value: 'azure/db',
},
],
expectedClearedQueryPartial: {
resourceGroup: 'web-app-development',
metricDefinition: undefined,
expectedCustomPropertyResults: [
{ label: 'Virtual Machine', value: 'azure/vm' },
{ label: 'Database', value: 'azure/db' },
{ label: 'azure/resource-type-$ENVIRONMENT', value: 'azure/resource-type-$ENVIRONMENT' },
],
},
},
{
name: 'useResourceNames',
hook: useResourceNames,
@ -175,20 +189,10 @@ describe('AzureMonitor: metrics dataHooks', () => {
resourceGroup: 'web-app-development',
metricDefinition: 'azure/vm',
},
validQueryPartial: {
customProperties: {
resourceGroup: 'web-app-development',
metricDefinition: 'azure/vm',
resourceName: 'web-server',
},
invalidQueryPartial: {
resourceGroup: 'web-app-development',
metricDefinition: 'azure/vm',
resourceName: 'resource-that-doesnt-exist',
},
templateVariableQueryPartial: {
resourceGroup: 'web-app-development',
metricDefinition: 'azure/vm',
resourceName: '$variable',
resourceName: 'resource-name-$ENVIRONMENT',
},
expectedOptions: [
{
@ -200,11 +204,11 @@ describe('AzureMonitor: metrics dataHooks', () => {
value: 'job-server',
},
],
expectedClearedQueryPartial: {
resourceGroup: 'web-app-development',
metricDefinition: 'azure/vm',
resourceName: undefined,
},
expectedCustomPropertyResults: [
{ label: 'Web server', value: 'web-server' },
{ label: 'Job server', value: 'job-server' },
{ label: 'resource-name-$ENVIRONMENT', value: 'resource-name-$ENVIRONMENT' },
],
},
{
@ -216,25 +220,12 @@ describe('AzureMonitor: metrics dataHooks', () => {
resourceName: 'web-server',
metricNamespace: 'azure/vm',
},
validQueryPartial: {
customProperties: {
resourceGroup: 'web-app-development',
metricDefinition: 'azure/vm',
resourceName: 'web-server',
metricNamespace: 'azure/vm',
},
invalidQueryPartial: {
resourceGroup: 'web-app-development',
metricDefinition: 'azure/vm',
resourceName: 'web-server',
metricNamespace: 'azure/vm',
metricName: 'invalid-metric',
},
templateVariableQueryPartial: {
resourceGroup: 'web-app-development',
metricDefinition: 'azure/vm',
resourceName: 'web-server',
metricNamespace: 'azure/vm',
metricName: '$variable',
metricName: 'metric-$ENVIRONMENT',
},
expectedOptions: [
{
@ -246,13 +237,11 @@ describe('AzureMonitor: metrics dataHooks', () => {
value: 'free-memory',
},
],
expectedClearedQueryPartial: {
resourceGroup: 'web-app-development',
metricDefinition: 'azure/vm',
resourceName: 'web-server',
metricNamespace: 'azure/vm',
metricName: undefined,
},
expectedCustomPropertyResults: [
{ label: 'Percentage CPU', value: 'percentage-cpu' },
{ label: 'Free memory', value: 'free-memory' },
{ label: 'metric-$ENVIRONMENT', value: 'metric-$ENVIRONMENT' },
],
},
{
@ -264,25 +253,12 @@ describe('AzureMonitor: metrics dataHooks', () => {
resourceName: 'web-server',
metricNamespace: 'azure/vm',
},
validQueryPartial: {
customProperties: {
resourceGroup: 'web-app-development',
metricDefinition: 'azure/vm',
resourceName: 'web-server',
metricNamespace: 'azure/vm',
},
invalidQueryPartial: {
resourceGroup: 'web-app-development',
metricDefinition: 'azure/vm',
resourceName: 'web-server',
metricNamespace: 'azure/vm',
metricName: 'invalid-metric',
},
templateVariableQueryPartial: {
resourceGroup: 'web-app-development',
metricDefinition: 'azure/vm',
resourceName: 'web-server',
metricNamespace: 'azure/vm',
metricName: '$variable',
metricNamespace: 'azure/vm-$ENVIRONMENT',
metricName: 'metric-name',
},
expectedOptions: [
{
@ -293,6 +269,15 @@ describe('AzureMonitor: metrics dataHooks', () => {
label: 'Database NS',
value: 'azure/dbns',
},
{
label: 'azure/vm',
value: 'azure/vm',
},
],
expectedCustomPropertyResults: [
{ label: 'Compute Virtual Machine', value: 'azure/vmc' },
{ label: 'Database NS', value: 'azure/dbns' },
{ label: 'azure/vm-$ENVIRONMENT', value: 'azure/vm-$ENVIRONMENT' },
],
},
];
@ -308,6 +293,10 @@ describe('AzureMonitor: metrics dataHooks', () => {
datasource = createMockDatasource();
datasource.getVariables = jest.fn().mockReturnValue(['$sub', '$rg', '$rt', '$variable']);
datasource.azureMonitorDatasource.getSubscriptions = jest
.fn()
.mockResolvedValue([opt('sub-abc-123', 'sub-abc-123')]);
datasource.getResourceGroups = jest
.fn()
.mockResolvedValue([
@ -343,59 +332,16 @@ describe('AzureMonitor: metrics dataHooks', () => {
expect(result.current).toEqual(scenario.expectedOptions);
});
it('does not call onChange when the property has not been set', async () => {
it('adds custom properties as a valid option', async () => {
const query = {
...bareQuery,
azureMonitor: scenario.emptyQueryPartial,
azureMonitor: scenario.customProperties,
...scenario.topLevelCustomProperties,
};
const { waitForNextUpdate } = renderHook(() => scenario.hook(query, datasource, onChange, setError));
const { result, waitForNextUpdate } = renderHook(() => scenario.hook(query, datasource, onChange, setError));
await waitForNextUpdate(WAIT_OPTIONS);
expect(onChange).not.toHaveBeenCalled();
});
it('does not clear the property when it is a valid option', async () => {
const query = {
...bareQuery,
azureMonitor: scenario.validQueryPartial,
};
const { waitForNextUpdate } = renderHook(() => scenario.hook(query, datasource, onChange, setError));
await waitForNextUpdate(WAIT_OPTIONS);
expect(onChange).not.toHaveBeenCalled();
});
it('does not clear the property when it is a template variable', async () => {
const query = {
...bareQuery,
azureMonitor: scenario.templateVariableQueryPartial,
};
const { waitForNextUpdate } = renderHook(() => scenario.hook(query, datasource, onChange, setError));
await waitForNextUpdate(WAIT_OPTIONS);
expect(onChange).not.toHaveBeenCalled();
});
it('clears the property when it is not a valid option', async () => {
const query = {
...bareQuery,
azureMonitor: scenario.invalidQueryPartial,
};
const { waitForNextUpdate } = renderHook(() => scenario.hook(query, datasource, onChange, setError));
await waitForNextUpdate(WAIT_OPTIONS);
if (scenario.expectedClearedQueryPartial) {
expect(onChange).toHaveBeenCalledWith({
...query,
azureMonitor: {
...scenario.expectedClearedQueryPartial,
dimensionFilters: [],
timeGrain: '',
},
});
} else {
expect(onChange).not.toHaveBeenCalled();
}
expect(result.current).toEqual(scenario.expectedCustomPropertyResults);
});
});
});

View File

@ -2,14 +2,7 @@ import { useState, useEffect, useMemo } from 'react';
import Datasource from '../../datasource';
import { AzureMonitorErrorish, AzureMonitorOption, AzureMonitorQuery } from '../../types';
import { hasOption, toOption } from '../../utils/common';
import {
setMetricName,
setMetricNamespace,
setResourceGroup,
setResourceName,
setResourceType,
setSubscriptionID,
} from './setQueryValue';
import { setMetricNamespace, setSubscriptionID } from './setQueryValue';
export interface MetricMetadata {
aggOptions: AzureMonitorOption[];
@ -88,11 +81,14 @@ export const updateSubscriptions = (
export const useSubscriptions: DataHook = (query, datasource, onChange, setError) => {
const defaultSubscription = datasource.azureMonitorDatasource.defaultSubscriptionId;
const { subscription } = query;
const subscriptionOptions = useAsyncState(
async () => {
const results = await datasource.azureMonitorDatasource.getSubscriptions();
return results.map((v) => ({ label: v.text, value: v.value, description: v.value }));
const options = formatOptions(results, subscription);
return options;
},
setError,
[]
@ -116,11 +112,7 @@ export const useResourceGroups: DataHook = (query, datasource, onChange, setErro
}
const results = await datasource.getResourceGroups(subscription);
const options = results.map(toOption);
if (isInvalidOption(resourceGroup, options, datasource.getVariables())) {
onChange(setResourceGroup(query, undefined));
}
const options = formatOptions(results, resourceGroup);
return options;
},
@ -140,11 +132,7 @@ export const useResourceTypes: DataHook = (query, datasource, onChange, setError
}
const results = await datasource.getMetricDefinitions(subscription, resourceGroup);
const options = results.map(toOption);
if (isInvalidOption(metricDefinition, options, datasource.getVariables())) {
onChange(setResourceType(query, undefined));
}
const options = formatOptions(results, metricDefinition);
return options;
},
@ -164,11 +152,7 @@ export const useResourceNames: DataHook = (query, datasource, onChange, setError
}
const results = await datasource.getResourceNames(subscription, resourceGroup, metricDefinition);
const options = results.map(toOption);
if (isInvalidOption(resourceName, options, datasource.getVariables())) {
onChange(setResourceName(query, undefined));
}
const options = formatOptions(results, resourceName);
return options;
},
@ -188,7 +172,7 @@ export const useMetricNamespaces: DataHook = (query, datasource, onChange, setEr
}
const results = await datasource.getMetricNamespaces(subscription, resourceGroup, metricDefinition, resourceName);
const options = results.map(toOption);
const options = formatOptions(results, metricNamespace);
// Do some cleanup of the query state if need be
if (!metricNamespace && options.length) {
@ -222,11 +206,7 @@ export const useMetricNames: DataHook = (query, datasource, onChange, setError)
metricNamespace
);
const options = results.map(toOption);
if (isInvalidOption(metricName, options, datasource.getVariables())) {
onChange(setMetricName(query, undefined));
}
const options = formatOptions(results, metricName);
return options;
},
@ -297,6 +277,19 @@ export const useMetricMetadata = (query: AzureMonitorQuery, datasource: Datasour
return metricMetadata;
};
function isInvalidOption(value: string | undefined, options: AzureMonitorOption[], templateVariables: string[]) {
return value && !templateVariables.includes(value) && !hasOption(options, value);
function formatOptions(
rawResults: Array<{
text: string;
value: string;
}>,
selectedValue?: string
) {
const options = rawResults.map(toOption);
// account for custom values that might have been set in json file like ones crafted with a template variable (ex: "cloud-datasource-resource-$Environment")
if (selectedValue && !options.find((option) => option.value === selectedValue)) {
options.push({ label: selectedValue, value: selectedValue });
}
return options;
}

View File

@ -85,6 +85,7 @@ const SubscriptionField: React.FC<SubscriptionFieldProps> = ({
onChange={handleChange}
options={options}
width={38}
allowCustomValue
/>
</Field>
);