diff --git a/docs/sources/datasources/azuremonitor/template-variables.md b/docs/sources/datasources/azuremonitor/template-variables.md index da6f53567d8..979c0a3b19a 100644 --- a/docs/sources/datasources/azuremonitor/template-variables.md +++ b/docs/sources/datasources/azuremonitor/template-variables.md @@ -62,3 +62,67 @@ Perf | summarize avg(CounterValue) by bin(TimeGenerated, $__interval), Computer | order by TimeGenerated asc ``` + +## Limitations + +As of Grafana 9.0, a resource URI is constructed to identify resources using the resource picker. On dashboards created prior to Grafana 9.0, Grafana automatically migrates any queries using the prior resource-picking mechanism to use this method. + +Some resource types use nested namespaces and resource names, such as `Microsoft.Storage/storageAccounts/tableServices` and `storageAccount/default`, or `Microsoft.Sql/servers/databases` and `serverName/databaseName`. Such template variables cannot be used because the result could be a malformed resource URI. + +### Supported cases + +#### Standard namespaces and resource names + +```kusto +metricDefinition = $ns +$ns = Microsoft.Compute/virtualMachines +resourceName = $rs +$rs = testvirtualmachine +``` + +#### Namespaces with a non-templated sub-namespace + +```kusto +metricDefinition = $ns/tableServices +$ns = Microsoft.Storage/storageAccounts +resourceName = $rs/default +$rs = storageaccount +``` + +#### Storage namespaces missing the `default` keyword + +```kusto +metricDefinition = $ns/tableServices +$ns = Microsoft.Storage/storageAccounts +resourceName = $rs +$rs = storageaccount +``` + +#### Namespaces with a templated sub-namespace + +```kusto +metricDefinition = $ns/$sns +$ns = Microsoft.Storage/storageAccounts +$sns = tableServices +resourceName = $rs +$rs = storageaccount +``` + +### Unsupported case + +If a dashboard uses this unsupported case, migrate it to one of the [supported cases](#supported-cases). + +If a namespace or resource name template variable contains multiple segments, Grafana will construct the resource URI incorrectly because the template variable cannot be appropriately split. + +For example: + +```kusto +metricDefinition = $ns +resourceName = $rs +$ns = 'Microsoft.Storage/storageAccounts/tableServices' +$rs = 'storageaccount/default' +``` + +This would result in an incorrect resource URI containing `Microsoft.Storage/storageAccounts/tableServices/storageaccount/default`. However, the correct URI would have the format `Microsoft.Storage/storageAccounts/storageaccount/tableServices/default`. + +An appropriate fix would be to update the template variable that does not match a supported case. If the namespace variable `$ns` is of the form `Microsoft.Storage/storageAccounts/tableServices` this could be split into two variables: `$ns1 = Microsoft.Storage/storageAccounts` and `$ns2 = tableServices`. The metric definition would then take the form `$ns1/$ns2` which leads to a correctly formatted URI. 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 a85e2c7320b..65db8e09727 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 @@ -47,7 +47,7 @@ const QueryEditor: React.FC = ({ [onChange, onRunQuery] ); - const query = usePreparedQuery(baseQuery, onQueryChange); + const query = usePreparedQuery(baseQuery, onQueryChange, setError); 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 9ed96bb1103..a400102897c 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 @@ -4,17 +4,20 @@ import { useEffect, useMemo } from 'react'; import { getTemplateSrv } from '@grafana/runtime'; -import { AzureMonitorQuery, AzureQueryType } from '../../types'; +import { AzureMonitorErrorish, AzureMonitorQuery, AzureQueryType } from '../../types'; import migrateQuery from '../../utils/migrateQuery'; const DEFAULT_QUERY = { queryType: AzureQueryType.AzureMonitor, }; -const prepareQuery = (query: AzureMonitorQuery) => { +const prepareQuery = ( + query: AzureMonitorQuery, + setError: (errorSource: string, error: AzureMonitorErrorish) => void +) => { // Note: _.defaults does not apply default values deeply. const withDefaults = defaults({}, query, DEFAULT_QUERY); - const migratedQuery = migrateQuery(withDefaults, getTemplateSrv()); + const migratedQuery = migrateQuery(withDefaults, getTemplateSrv(), setError); // 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. @@ -24,8 +27,12 @@ const prepareQuery = (query: AzureMonitorQuery) => { /** * Returns queries with some defaults + migrations, and calls onChange function to notify if it changes */ -const usePreparedQuery = (query: AzureMonitorQuery, onChangeQuery: (newQuery: AzureMonitorQuery) => void) => { - const preparedQuery = useMemo(() => prepareQuery(query), [query]); +const usePreparedQuery = ( + query: AzureMonitorQuery, + onChangeQuery: (newQuery: AzureMonitorQuery) => void, + setError: (errorSource: string, error: AzureMonitorErrorish) => void +) => { + const preparedQuery = useMemo(() => prepareQuery(query, setError), [query, setError]); useEffect(() => { if (preparedQuery !== query) { diff --git a/public/app/plugins/datasource/grafana-azure-monitor-datasource/types/types.ts b/public/app/plugins/datasource/grafana-azure-monitor-datasource/types/types.ts index 198060b8bbe..286ef80aeff 100644 --- a/public/app/plugins/datasource/grafana-azure-monitor-datasource/types/types.ts +++ b/public/app/plugins/datasource/grafana-azure-monitor-datasource/types/types.ts @@ -85,7 +85,7 @@ export interface AzureDataSourceSecureJsonData { // Represents an errors that come back from frontend requests. // Not totally sure how accurate this type is. -export type AzureMonitorErrorish = Error; +export type AzureMonitorErrorish = Error | string | React.ReactElement; // Azure Monitor API Types export interface AzureMonitorMetricsMetadataResponse { diff --git a/public/app/plugins/datasource/grafana-azure-monitor-datasource/utils/messageFromError.ts b/public/app/plugins/datasource/grafana-azure-monitor-datasource/utils/messageFromError.ts index aa0bb0a61ad..2a7236df003 100644 --- a/public/app/plugins/datasource/grafana-azure-monitor-datasource/utils/messageFromError.ts +++ b/public/app/plugins/datasource/grafana-azure-monitor-datasource/utils/messageFromError.ts @@ -1,3 +1,15 @@ +import { isValidElement } from 'react'; + +import { AzureMonitorErrorish } from '../types'; + +export function messageFromElement(error: AzureMonitorErrorish): AzureMonitorErrorish | undefined { + if (isValidElement(error)) { + return error; + } else { + return messageFromError(error); + } +} + export default function messageFromError(error: any): string | undefined { if (!error || typeof error !== 'object') { return undefined; diff --git a/public/app/plugins/datasource/grafana-azure-monitor-datasource/utils/migrateQuery.test.ts b/public/app/plugins/datasource/grafana-azure-monitor-datasource/utils/migrateQuery.test.ts index b9ef2a6f01f..9c047a84b71 100644 --- a/public/app/plugins/datasource/grafana-azure-monitor-datasource/utils/migrateQuery.test.ts +++ b/public/app/plugins/datasource/grafana-azure-monitor-datasource/utils/migrateQuery.test.ts @@ -1,6 +1,8 @@ +import React from 'react'; + import { getTemplateSrv } from '@grafana/runtime'; -import { AzureMetricDimension, AzureMonitorQuery, AzureQueryType } from '../types'; +import { AzureMetricDimension, AzureMonitorErrorish, AzureMonitorQuery, AzureQueryType } from '../types'; import migrateQuery from './migrateQuery'; @@ -17,6 +19,8 @@ jest.mock('@grafana/runtime', () => { let templateSrv = getTemplateSrv(); +let setErrorMock = jest.fn(); + const azureMonitorQueryV7 = { appInsights: { dimension: [], metricName: 'select', timeGrain: 'auto' }, azureLogAnalytics: { @@ -97,7 +101,7 @@ const modernMetricsQuery: AzureMonitorQuery = { describe('AzureMonitor: migrateQuery', () => { it('modern queries should not change', () => { - const result = migrateQuery(modernMetricsQuery, templateSrv); + const result = migrateQuery(modernMetricsQuery, templateSrv, setErrorMock); // MUST use .toBe because we want to assert that the identity of unmigrated queries remains the same expect(modernMetricsQuery).toBe(result); @@ -105,7 +109,7 @@ describe('AzureMonitor: migrateQuery', () => { describe('migrating from a v7 query to the latest query version', () => { it('should build a resource uri', () => { - const result = migrateQuery(azureMonitorQueryV7, templateSrv); + const result = migrateQuery(azureMonitorQueryV7, templateSrv, setErrorMock); expect(result).toMatchObject( expect.objectContaining({ azureMonitor: expect.objectContaining({ @@ -119,7 +123,7 @@ describe('AzureMonitor: migrateQuery', () => { describe('migrating from a v8 query to the latest query version', () => { it('should build a resource uri', () => { - const result = migrateQuery(azureMonitorQueryV8, templateSrv); + const result = migrateQuery(azureMonitorQueryV8, templateSrv, setErrorMock); expect(result).toMatchObject( expect.objectContaining({ azureMonitor: expect.objectContaining({ @@ -130,18 +134,66 @@ describe('AzureMonitor: migrateQuery', () => { ); }); - it('should not build a resource uri with an unsupported template variable', () => { - replaceMock = jest.fn().mockImplementation((s: string) => s.replace('$ns', 'Microsoft.Storage/storageAccounts')); + it('should not build a resource uri with an unsupported namespace template variable', () => { + replaceMock = jest + .fn() + .mockImplementation((s: string) => s.replace('$ns', 'Microsoft.Storage/storageAccounts/tableServices')); + setErrorMock = jest + .fn() + .mockImplementation((errorSource: string, error: AzureMonitorErrorish) => 'Template Var error'); + const errorElement = React.createElement( + 'div', + null, + `Failed to create resource URI. Validate the metric definition template variable against supported cases `, + React.createElement( + 'a', + { + href: 'https://grafana.com/docs/grafana/latest/datasources/azuremonitor/template-variables/', + }, + 'here.' + ) + ); templateSrv = getTemplateSrv(); const query = { ...azureMonitorQueryV8, azureMonitor: { - ...azureMonitorQueryV8, + ...azureMonitorQueryV8.azureMonitor, metricDefinition: '$ns', }, }; - const result = migrateQuery(query, templateSrv); + const result = migrateQuery(query, templateSrv, setErrorMock); expect(result.azureMonitor?.resourceUri).toBeUndefined(); + expect(setErrorMock).toHaveBeenCalledWith('Resource URI migration', errorElement); + }); + + it('should not build a resource uri with unsupported resource name template variable', () => { + replaceMock = jest.fn().mockImplementation((s: string) => s.replace('$resource', 'resource/default')); + setErrorMock = jest + .fn() + .mockImplementation((errorSource: string, error: AzureMonitorErrorish) => 'Template Var error'); + const errorElement = React.createElement( + 'div', + null, + `Failed to create resource URI. Validate the resource name template variable against supported cases `, + React.createElement( + 'a', + { + href: 'https://grafana.com/docs/grafana/latest/datasources/azuremonitor/template-variables/', + }, + 'here.' + ) + ); + templateSrv = getTemplateSrv(); + const query = { + ...azureMonitorQueryV8, + azureMonitor: { + ...azureMonitorQueryV8.azureMonitor, + resourceName: '$resource', + }, + }; + const result = migrateQuery(query, templateSrv, setErrorMock); + expect(result.azureMonitor?.resourceUri).toBeUndefined(); + expect(setErrorMock).toHaveBeenCalledWith('Resource URI migration', errorElement); }); }); @@ -150,7 +202,11 @@ describe('AzureMonitor: migrateQuery', () => { const dimensionFilters: AzureMetricDimension[] = [ { dimension: 'TestDimension', operator: 'eq', filters: ['testFilter'] }, ]; - const result = migrateQuery({ ...azureMonitorQueryV8, azureMonitor: { dimensionFilters } }, templateSrv); + const result = migrateQuery( + { ...azureMonitorQueryV8, azureMonitor: { dimensionFilters } }, + templateSrv, + setErrorMock + ); expect(result).toMatchObject( expect.objectContaining({ azureMonitor: expect.objectContaining({ @@ -161,7 +217,11 @@ describe('AzureMonitor: migrateQuery', () => { }); it('correctly updates old filter containing wildcard', () => { const dimensionFilters: AzureMetricDimension[] = [{ dimension: 'TestDimension', operator: 'eq', filter: '*' }]; - const result = migrateQuery({ ...azureMonitorQueryV8, azureMonitor: { dimensionFilters } }, templateSrv); + const result = migrateQuery( + { ...azureMonitorQueryV8, azureMonitor: { dimensionFilters } }, + templateSrv, + setErrorMock + ); expect(result).toMatchObject( expect.objectContaining({ azureMonitor: expect.objectContaining({ @@ -174,7 +234,11 @@ describe('AzureMonitor: migrateQuery', () => { }); it('correctly updates old filter containing value', () => { const dimensionFilters: AzureMetricDimension[] = [{ dimension: 'TestDimension', operator: 'eq', filter: 'test' }]; - const result = migrateQuery({ ...azureMonitorQueryV8, azureMonitor: { dimensionFilters } }, templateSrv); + const result = migrateQuery( + { ...azureMonitorQueryV8, azureMonitor: { dimensionFilters } }, + templateSrv, + setErrorMock + ); expect(result).toMatchObject( expect.objectContaining({ azureMonitor: expect.objectContaining({ @@ -189,7 +253,11 @@ describe('AzureMonitor: migrateQuery', () => { const dimensionFilters: AzureMetricDimension[] = [ { dimension: 'TestDimension', operator: 'eq', filter: '*', filters: ['testFilter'] }, ]; - const result = migrateQuery({ ...azureMonitorQueryV8, azureMonitor: { dimensionFilters } }, templateSrv); + const result = migrateQuery( + { ...azureMonitorQueryV8, azureMonitor: { dimensionFilters } }, + templateSrv, + setErrorMock + ); expect(result).toMatchObject( expect.objectContaining({ azureMonitor: expect.objectContaining({ @@ -208,7 +276,11 @@ describe('AzureMonitor: migrateQuery', () => { const dimensionFilters: AzureMetricDimension[] = [ { dimension: 'TestDimension', operator: 'eq', filter: 'testFilter', filters: ['testFilter'] }, ]; - const result = migrateQuery({ ...azureMonitorQueryV8, azureMonitor: { dimensionFilters } }, templateSrv); + const result = migrateQuery( + { ...azureMonitorQueryV8, azureMonitor: { dimensionFilters } }, + templateSrv, + setErrorMock + ); expect(result).toMatchObject( expect.objectContaining({ azureMonitor: expect.objectContaining({ diff --git a/public/app/plugins/datasource/grafana-azure-monitor-datasource/utils/migrateQuery.ts b/public/app/plugins/datasource/grafana-azure-monitor-datasource/utils/migrateQuery.ts index 54d569302b4..6492f204d27 100644 --- a/public/app/plugins/datasource/grafana-azure-monitor-datasource/utils/migrateQuery.ts +++ b/public/app/plugins/datasource/grafana-azure-monitor-datasource/utils/migrateQuery.ts @@ -1,3 +1,5 @@ +import React from 'react'; + import { TemplateSrv } from '@grafana/runtime'; import UrlBuilder from '../azure_monitor/url_builder'; @@ -7,11 +9,15 @@ import { setTimeGrain as setMetricsTimeGrain, } from '../components/MetricsQueryEditor/setQueryValue'; import TimegrainConverter from '../time_grain_converter'; -import { AzureMetricDimension, AzureMonitorQuery, AzureQueryType } from '../types'; +import { AzureMetricDimension, AzureMonitorErrorish, AzureMonitorQuery, AzureQueryType } from '../types'; const OLD_DEFAULT_DROPDOWN_VALUE = 'select'; -export default function migrateQuery(query: AzureMonitorQuery, templateSrv: TemplateSrv): AzureMonitorQuery { +export default function migrateQuery( + query: AzureMonitorQuery, + templateSrv: TemplateSrv, + setError: (errorSource: string, error: AzureMonitorErrorish) => void +): AzureMonitorQuery { let workingQuery = query; // The old angular controller also had a `migrateApplicationInsightsKeys` migraiton that @@ -23,7 +29,7 @@ export default function migrateQuery(query: AzureMonitorQuery, templateSrv: Temp workingQuery = migrateLogAnalyticsToFromTimes(workingQuery); workingQuery = migrateToDefaultNamespace(workingQuery); workingQuery = migrateDimensionToDimensionFilter(workingQuery); - workingQuery = migrateResourceUri(workingQuery, templateSrv); + workingQuery = migrateResourceUri(workingQuery, templateSrv, setError); workingQuery = migrateDimensionFilterToArray(workingQuery); return workingQuery; @@ -98,7 +104,11 @@ function migrateDimensionToDimensionFilter(query: AzureMonitorQuery): AzureMonit // Azure Monitor metric queries prior to Grafana version 9 did not include a `resourceUri`. // The resourceUri was previously constructed with the subscription id, resource group, // metric definition (a.k.a. resource type), and the resource name. -function migrateResourceUri(query: AzureMonitorQuery, templateSrv: TemplateSrv): AzureMonitorQuery { +function migrateResourceUri( + query: AzureMonitorQuery, + templateSrv: TemplateSrv, + setError?: (errorSource: string, error: AzureMonitorErrorish) => void +): AzureMonitorQuery { const azureMonitorQuery = query.azureMonitor; if (!azureMonitorQuery || azureMonitorQuery.resourceUri) { @@ -116,6 +126,23 @@ function migrateResourceUri(query: AzureMonitorQuery, templateSrv: TemplateSrv): // If a metric definition includes template variable with a subresource e.g. // Microsoft.Storage/storageAccounts/libraries, it's not possible to generate a valid // resource URI + if (setError) { + setError( + 'Resource URI migration', + React.createElement( + 'div', + null, + `Failed to create resource URI. Validate the metric definition template variable against supported cases `, + React.createElement( + 'a', + { + href: 'https://grafana.com/docs/grafana/latest/datasources/azuremonitor/template-variables/', + }, + 'here.' + ) + ) + ); + } return query; } @@ -123,6 +150,23 @@ function migrateResourceUri(query: AzureMonitorQuery, templateSrv: TemplateSrv): if (resourceNameArray.some((p) => templateSrv.replace(p).split('/').length > 1)) { // If a resource name includes template variable with a subresource e.g. // abc123/def456, it's not possible to generate a valid resource URI + if (setError) { + setError( + 'Resource URI migration', + React.createElement( + 'div', + null, + `Failed to create resource URI. Validate the resource name template variable against supported cases `, + React.createElement( + 'a', + { + href: 'https://grafana.com/docs/grafana/latest/datasources/azuremonitor/template-variables/', + }, + 'here.' + ) + ) + ); + } return query; } diff --git a/public/app/plugins/datasource/grafana-azure-monitor-datasource/utils/useLastError.ts b/public/app/plugins/datasource/grafana-azure-monitor-datasource/utils/useLastError.ts index 8fb0246e1d5..1c26117b2b2 100644 --- a/public/app/plugins/datasource/grafana-azure-monitor-datasource/utils/useLastError.ts +++ b/public/app/plugins/datasource/grafana-azure-monitor-datasource/utils/useLastError.ts @@ -2,7 +2,7 @@ import { useState, useCallback, useMemo } from 'react'; import { AzureMonitorErrorish } from '../types'; -import messageFromError from './messageFromError'; +import { messageFromElement } from './messageFromError'; type SourcedError = [string, AzureMonitorErrorish]; @@ -33,7 +33,7 @@ export default function useLastError() { const errorMessage = useMemo(() => { const recentError = errors[0]; - return recentError && messageFromError(recentError[1]); + return recentError && messageFromElement(recentError[1]); }, [errors]); return [errorMessage, addError] as const;