Azure Monitor: Blank result formats are not overwritten (grafana#93493)

* if result format is null from previous query, it will be null and not correct to a different format * update to not have empty value in result format field * fix tests * add check to see if options are in list * reformat stuff so logic lives on the right layer * linterrrrrrrrrrrrrrrrrrRR * Apply suggestions from code review * if result format is null from previous query, it will be null and not correct to a different format * update to not have empty value in result format field * fix tests * add check to see if options are in list * reformat stuff so logic lives on the right layer * linterrrrrrrrrrrrrrrrrrRR * frontend linter * linter * feedback :)
Nortech-ai · Sep 19, 2024 · 8b6cec2 · 8b6cec2
1 parent f1ba7de
commit 8b6cec2
Show file tree

Hide file tree

Showing 10 changed files with 128 additions and 26 deletions.
diff --git a/pkg/tsdb/azuremonitor/loganalytics/utils.go b/pkg/tsdb/azuremonitor/loganalytics/utils.go
@@ -64,22 +64,20 @@ func meetsBasicLogsCriteria(resources []string, fromAlert bool, basicLogsEnabled
 	return true, nil
 }
 
+// This function should be part of migration function
 func ParseResultFormat(queryResultFormat *dataquery.ResultFormat, queryType dataquery.AzureQueryType) dataquery.ResultFormat {
-	var resultFormat dataquery.ResultFormat
-	if queryResultFormat != nil {
-		resultFormat = *queryResultFormat
+	if queryResultFormat != nil && *queryResultFormat != "" {
+		return *queryResultFormat
 	}
-	if resultFormat == "" {
-		if queryType == dataquery.AzureQueryTypeAzureLogAnalytics {
-			// Default to logs format for logs queries
-			resultFormat = dataquery.ResultFormatLogs
-		}
-		if queryType == dataquery.AzureQueryTypeAzureTraces {
-			// Default to table format for traces queries as many traces may be returned
-			resultFormat = dataquery.ResultFormatTable
-		}
+	if queryType == dataquery.AzureQueryTypeAzureLogAnalytics {
+		// Default to time series format for logs queries. It was time series before this change
+		return dataquery.ResultFormatTimeSeries
+	}
+	if queryType == dataquery.AzureQueryTypeAzureTraces {
+		// Default to table format for traces queries as many traces may be returned
+		return dataquery.ResultFormatTable
 	}
-	return resultFormat
+	return ""
 }
 
 func getApiURL(resourceOrWorkspace string, isAppInsightsQuery bool, basicLogsQuery bool) string {

diff --git a/pkg/tsdb/azuremonitor/loganalytics/utils_test.go b/pkg/tsdb/azuremonitor/loganalytics/utils_test.go
@@ -17,10 +17,10 @@ func TestParseResultFormat(t *testing.T) {
 		expectedResultFormat dataquery.ResultFormat
 	}{
 		{
-			name:                 "returns the logs format as default for logs queries if input format is nil",
+			name:                 "returns the time series format as default for logs queries if input format is nil",
 			queryResultFormat:    nil,
 			queryType:            dataquery.AzureQueryTypeAzureLogAnalytics,
-			expectedResultFormat: dataquery.ResultFormatLogs,
+			expectedResultFormat: dataquery.ResultFormatTimeSeries,
 		},
 		{
 			name:                 "returns the table format as default for traces queries if input format is nil",
@@ -32,7 +32,7 @@ func TestParseResultFormat(t *testing.T) {
 			name:                 "returns the logs format as default for logs queries if input format is empty",
 			queryResultFormat:    &emptyResultFormat,
 			queryType:            dataquery.AzureQueryTypeAzureLogAnalytics,
-			expectedResultFormat: dataquery.ResultFormatLogs,
+			expectedResultFormat: dataquery.ResultFormatTimeSeries,
 		},
 		{
 			name:                 "returns the table format as default for traces queries if input format is empty",

diff --git a/...c/app/plugins/datasource/azuremonitor/components/LogsQueryEditor/LogsQueryEditor.test.tsx b/...c/app/plugins/datasource/azuremonitor/components/LogsQueryEditor/LogsQueryEditor.test.tsx
@@ -5,6 +5,7 @@ import { dateTime, LoadingState } from '@grafana/data';
 
 import createMockDatasource from '../../__mocks__/datasource';
 import createMockQuery from '../../__mocks__/query';
+import { ResultFormat } from '../../dataquery.gen';
 import { createMockResourcePickerData } from '../MetricsQueryEditor/MetricsQueryEditor.test';
 
 import LogsQueryEditor from './LogsQueryEditor';
@@ -498,4 +499,57 @@ describe('LogsQueryEditor', () => {
       expect(await screen.queryByLabelText(/This query is processing 0.50 GiB when run./)).not.toBeInTheDocument();
     });
   });
+
+  describe('format as options', () => {
+    it('sets to time series if there is a query with empty result format', async () => {
+      const mockDatasource = createMockDatasource({ resourcePickerData: createMockResourcePickerData() });
+      const query = createMockQuery({
+        azureLogAnalytics: {
+          resultFormat: undefined,
+        },
+      });
+      const onChange = jest.fn();
+
+      await act(async () => {
+        render(
+          <LogsQueryEditor
+            query={query}
+            datasource={mockDatasource}
+            variableOptionGroup={variableOptionGroup}
+            onChange={onChange}
+            setError={() => {}}
+            basicLogsEnabled={false}
+          />
+        );
+      });
+      const newQuery = {
+        ...query,
+        azureLogAnalytics: { ...query.azureLogAnalytics, resultFormat: ResultFormat.TimeSeries },
+      };
+      expect(onChange).toHaveBeenCalledWith(newQuery);
+    });
+    it('sets to logs if the query is new', async () => {
+      const mockDatasource = createMockDatasource({ resourcePickerData: createMockResourcePickerData() });
+      const query = { ...createMockQuery(), azureLogAnalytics: undefined };
+      const onChange = jest.fn();
+
+      await act(async () => {
+        render(
+          <LogsQueryEditor
+            query={query}
+            datasource={mockDatasource}
+            variableOptionGroup={variableOptionGroup}
+            onChange={onChange}
+            setError={() => {}}
+            basicLogsEnabled={false}
+          />
+        );
+      });
+      const newQuery = {
+        ...query,
+        azureLogAnalytics: { resultFormat: ResultFormat.Logs },
+      };
+      expect(onChange).toHaveBeenCalledWith(newQuery);
+    });
+  });
 });
diff --git a/public/app/plugins/datasource/azuremonitor/components/LogsQueryEditor/LogsQueryEditor.tsx b/public/app/plugins/datasource/azuremonitor/components/LogsQueryEditor/LogsQueryEditor.tsx
@@ -17,7 +17,7 @@ import AdvancedResourcePicker from './AdvancedResourcePicker';
 import { LogsManagement } from './LogsManagement';
 import QueryField from './QueryField';
 import { TimeManagement } from './TimeManagement';
-import { setBasicLogsQuery, setFormatAs, setKustoQuery } from './setQueryValue';
+import { onLoad, setBasicLogsQuery, setFormatAs, setKustoQuery } from './setQueryValue';
 import useMigrations from './useMigrations';
 import { shouldShowBasicLogsToggle } from './utils';
 
@@ -226,6 +226,7 @@ const LogsQueryEditor = ({
                 defaultValue={ResultFormat.Logs}
                 setFormatAs={setFormatAs}
                 resultFormat={query.azureLogAnalytics?.resultFormat}
+                onLoad={onLoad}
               />
             )}
             {portalLinkButton}

diff --git a/public/app/plugins/datasource/azuremonitor/components/LogsQueryEditor/setQueryValue.ts b/public/app/plugins/datasource/azuremonitor/components/LogsQueryEditor/setQueryValue.ts
@@ -1,3 +1,5 @@
+import { SelectableValue } from '@grafana/data';
+
 import { AzureMonitorQuery, ResultFormat } from '../../types';
 
 export function setKustoQuery(query: AzureMonitorQuery, kustoQuery: string): AzureMonitorQuery {
@@ -49,3 +51,16 @@ export function setBasicLogsQuery(query: AzureMonitorQuery, basicLogsQuery: bool
     },
   };
 }
+export function onLoad(
+  query: AzureMonitorQuery,
+  defaultValue: ResultFormat,
+  handleChange: (change: SelectableValue<ResultFormat>) => void
+) {
+  if (!query.azureLogAnalytics) {
+    handleChange({ value: defaultValue });
+    return;
+  }
+  if (!query.azureLogAnalytics.resultFormat) {
+    handleChange({ value: ResultFormat.TimeSeries });
+  }
+}
diff --git a/...ic/app/plugins/datasource/azuremonitor/components/TracesQueryEditor/TracesQueryEditor.tsx b/...ic/app/plugins/datasource/azuremonitor/components/TracesQueryEditor/TracesQueryEditor.tsx
@@ -18,7 +18,7 @@ import FormatAsField from '../shared/FormatAsField';
 
 import Filters from './Filters';
 import TraceTypeField from './TraceTypeField';
-import { setDefaultTracesQuery, setFormatAs, setQueryOperationId } from './setQueryValue';
+import { onLoad, setDefaultTracesQuery, setFormatAs, setQueryOperationId } from './setQueryValue';
 
 interface TracesQueryEditorProps {
   query: AzureMonitorQuery;
@@ -169,6 +169,7 @@ const TracesQueryEditor = ({
               setFormatAs={setFormatAs}
               resultFormat={query.azureTraces?.resultFormat}
               range={range}
+              onLoad={onLoad}
             />
           </EditorFieldGroup>
         </EditorRow>

diff --git a/public/app/plugins/datasource/azuremonitor/components/TracesQueryEditor/setQueryValue.ts b/public/app/plugins/datasource/azuremonitor/components/TracesQueryEditor/setQueryValue.ts
@@ -1,3 +1,5 @@
+import { SelectableValue } from '@grafana/data';
+
 import { AzureMonitorQuery, AzureQueryType, AzureTracesFilter, ResultFormat } from '../../types';
 
 // Used when switching from a traces exemplar query to a standard Azure Traces query
@@ -49,3 +51,13 @@ export function setFilters(query: AzureMonitorQuery, filters: AzureTracesFilter[
     },
   };
 }
+
+export function onLoad(
+  query: AzureMonitorQuery,
+  defaultValue: ResultFormat,
+  handleChange: (change: SelectableValue<ResultFormat>) => void
+) {
+  if (!query.azureTraces?.resultFormat) {
+    handleChange({ value: defaultValue });
+  }
+}
diff --git a/public/app/plugins/datasource/azuremonitor/components/shared/FormatAsField.test.tsx b/public/app/plugins/datasource/azuremonitor/components/shared/FormatAsField.test.tsx
@@ -1,9 +1,10 @@
 import { render, screen } from '@testing-library/react';
 
+import { SelectableValue } from '@grafana/data';
+
 import createMockDatasource from '../../__mocks__/datasource';
 import createMockQuery from '../../__mocks__/query';
-import { ResultFormat } from '../../types';
-import { setFormatAs } from '../TracesQueryEditor/setQueryValue';
+import { AzureMonitorQuery, ResultFormat } from '../../types';
 
 import FormatAsField from './FormatAsField';
 
@@ -12,6 +13,21 @@ const options = [
   { label: 'Trace', value: ResultFormat.Trace },
   { label: 'Time Series', value: ResultFormat.TimeSeries },
 ];
+const setFormatAs = (query: AzureMonitorQuery, formatAs: ResultFormat): AzureMonitorQuery => {
+  return {
+    ...query,
+    azureTraces: {
+      ...query.azureTraces,
+      resultFormat: formatAs,
+    },
+  };
+};
+
+const onLoad = (
+  _query: AzureMonitorQuery,
+  _defaultValue: ResultFormat,
+  _handleChange: (change: SelectableValue<ResultFormat>) => void
+) => {};
 
 const props = {
   query: createMockQuery(),
@@ -25,6 +41,7 @@ const props = {
   defaultValue: ResultFormat.Table,
   setFormatAs,
   resultFormat: undefined,
+  onLoad: onLoad,
 };
 
 describe('FormatAsField', () => {

diff --git a/public/app/plugins/datasource/azuremonitor/components/shared/FormatAsField.tsx b/public/app/plugins/datasource/azuremonitor/components/shared/FormatAsField.tsx
@@ -16,6 +16,7 @@ const FormatAsField = ({
   inputId,
   options: formatOptions,
   defaultValue,
+  onLoad,
   setFormatAs,
   resultFormat,
 }: FormatAsFieldProps) => {
@@ -35,20 +36,18 @@ const FormatAsField = ({
   );
 
   useEffectOnce(() => {
-    if (!resultFormat) {
+    //sets to default if the value is not found in the list
+    if (!formatOptions.find((item) => item.value === resultFormat)) {
       handleChange({ value: defaultValue });
-    } else {
-      if (!formatOptions.find((item) => item.value === resultFormat)) {
-        handleChange({ value: defaultValue });
-      }
     }
+    onLoad(query, defaultValue, handleChange);
   });
 
   return (
     <Field label="Format as" data-testid={selectors.components.queryEditor.logsQueryEditor.formatSelection.input}>
       <Select
         inputId={`${inputId}-format-as-field`}
-        value={resultFormat ?? defaultValue}
+        value={resultFormat}
         onChange={handleChange}
         options={options}
         width={38}

diff --git a/public/app/plugins/datasource/azuremonitor/types/types.ts b/public/app/plugins/datasource/azuremonitor/types/types.ts
@@ -188,6 +188,11 @@ export interface FormatAsFieldProps extends AzureQueryEditorFieldProps {
   defaultValue: ResultFormat;
   setFormatAs: (query: AzureMonitorQuery, formatAs: ResultFormat) => AzureMonitorQuery;
   resultFormat?: ResultFormat;
+  onLoad: (
+    query: AzureMonitorQuery,
+    defaultValue: ResultFormat,
+    handleChange: (change: SelectableValue<ResultFormat>) => void
+  ) => void;
 }
 
 export interface AzureResourceSummaryItem {