Cloudwatch: Refactor log group fields request (#60909)

* cloudwatch/log-group-fields-refactor

* remove not used code

* remove empty line

* fix broken test

* add tests

* Update pkg/tsdb/cloudwatch/routes/log_group_fields_test.go

Co-authored-by: Isabella Siu <Isabella.siu@grafana.com>

* pr feedback

Co-authored-by: Isabella Siu <Isabella.siu@grafana.com>

public/app/plugins/datasource/cloudwatch/api.ts

@@ -17,6 +17,8 @@ import {
   Account,
   ResourceRequest,
   ResourceResponse,
+  GetLogGroupFieldsRequest,
+  LogGroupField,
 } from './types';
 
 export interface SelectableResourceValue extends SelectableValue<string> {
@@ -70,6 +72,18 @@ export class CloudWatchAPI extends CloudWatchRequest {
     });
   }
 
+  async getLogGroupFields({
+    region,
+    arn,
+    logGroupName,
+  }: GetLogGroupFieldsRequest): Promise<Array<ResourceResponse<LogGroupField>>> {
+    return this.memoizedGetRequest<Array<ResourceResponse<LogGroupField>>>('log-group-fields', {
+      region: this.templateSrv.replace(this.getActualRegion(region)),
+      logGroupName: this.templateSrv.replace(logGroupName, {}),
+      logGroupArn: this.templateSrv.replace(arn),
+    });
+  }
+
   async describeAllLogGroups(params: DescribeLogGroupsRequest) {
     return this.memoizedGetRequest<SelectableResourceValue[]>('all-log-groups', {
       ...params,

public/app/plugins/datasource/cloudwatch/components/LogsQueryField.tsx

@@ -61,7 +61,8 @@ export const CloudWatchLogsQueryField = (props: CloudWatchLogsQueryFieldProps) =
   };
 
   const onTypeahead = async (typeahead: TypeaheadInput): Promise<TypeaheadOutput> => {
-    const { logGroupNames } = query;
+    const { datasource, query } = props;
+    const { logGroups } = query;
 
     if (!datasource.languageProvider) {
       return { suggestions: [] };
@@ -76,7 +77,7 @@ export const CloudWatchLogsQueryField = (props: CloudWatchLogsQueryFieldProps) =
       {
         history,
         absoluteRange,
-        logGroupNames,
+        logGroups: logGroups,
         region: query.region,
       }
     );

public/app/plugins/datasource/cloudwatch/language_provider.test.ts

@@ -15,7 +15,7 @@ import {
   STRING_FUNCTIONS,
   FIELD_AND_FILTER_FUNCTIONS,
 } from './syntax';
-import { GetLogGroupFieldsResponse } from './types';
+import { LogGroupField, ResourceResponse } from './types';
 
 const fields = ['field1', '@message'];
 
@@ -109,9 +109,9 @@ async function runSuggestionTest(query: string, expectedItems: string[][]) {
 
 function makeDatasource(): CloudWatchDatasource {
   return {
-    logsQueryRunner: {
-      getLogGroupFields(): Promise<GetLogGroupFieldsResponse> {
-        return Promise.resolve({ logGroupFields: [{ name: 'field1' }, { name: '@message' }] });
+    api: {
+      getLogGroupFields(): Promise<Array<ResourceResponse<LogGroupField>>> {
+        return Promise.resolve([{ value: { name: 'field1' } }, { value: { name: '@message' } }]);
       },
     },
   } as any;
@@ -131,7 +131,7 @@ function getProvideCompletionItems(query: string): Promise<TypeaheadOutput> {
     {
       value,
     } as any,
-    { logGroupNames: ['logGroup1'], region: 'custom' }
+    { logGroups: [{ name: 'logGroup1', arn: 'logGroup1' }], region: 'custom' }
   );
 }
 

public/app/plugins/datasource/cloudwatch/language_provider.ts

@@ -1,4 +1,3 @@
-import { sortedUniq } from 'lodash';
 import Prism, { Grammar } from 'prismjs';
 import { lastValueFrom } from 'rxjs';
 
@@ -16,14 +15,14 @@ import syntax, {
   QUERY_COMMANDS,
   STRING_FUNCTIONS,
 } from './syntax';
-import { CloudWatchQuery, TSDBResponse } from './types';
+import { CloudWatchQuery, LogGroup, TSDBResponse } from './types';
 
 export type CloudWatchHistoryItem = HistoryItem<CloudWatchQuery>;
 
 type TypeaheadContext = {
   history?: CloudWatchHistoryItem[];
   absoluteRange?: AbsoluteTimeRange;
-  logGroupNames?: string[];
+  logGroups?: LogGroup[];
   region: string;
 };
 
@@ -106,7 +105,7 @@ export class CloudWatchLanguageProvider extends LanguageProvider {
     }
 
     if (isInsideFunctionParenthesis(curToken)) {
-      return await this.getFieldCompletionItems(context?.logGroupNames ?? [], context?.region || 'default');
+      return await this.getFieldCompletionItems(context?.logGroups, context?.region || 'default');
     }
 
     if (isAfterKeyword('by', curToken)) {
@@ -127,44 +126,20 @@ export class CloudWatchLanguageProvider extends LanguageProvider {
     };
   }
 
-  private fetchedFieldsCache:
-    | {
-        time: number;
-        logGroups: string[];
-        fields: string[];
-      }
-    | undefined;
-
-  private fetchFields = async (logGroups: string[], region: string): Promise<string[]> => {
-    if (
-      this.fetchedFieldsCache &&
-      Date.now() - this.fetchedFieldsCache.time < 30 * 1000 &&
-      sortedUniq(this.fetchedFieldsCache.logGroups).join('|') === sortedUniq(logGroups).join('|')
-    ) {
-      return this.fetchedFieldsCache.fields;
-    }
-
+  private fetchFields = async (logGroups: LogGroup[], region: string): Promise<string[]> => {
     const results = await Promise.all(
-      logGroups.map((logGroup) => this.datasource.logsQueryRunner.getLogGroupFields({ logGroupName: logGroup, region }))
+      logGroups.map((logGroup) =>
+        this.datasource.api
+          .getLogGroupFields({ logGroupName: logGroup.name, arn: logGroup.arn, region })
+          .then((fields) => fields.filter((f) => f).map((f) => f.value.name ?? ''))
+      )
     );
 
-    const fields = [
-      ...new Set<string>(
-        results.reduce((acc: string[], cur) => acc.concat(cur.logGroupFields?.map((f) => f.name) as string[]), [])
-      ).values(),
-    ];
-
-    this.fetchedFieldsCache = {
-      time: Date.now(),
-      logGroups,
-      fields,
-    };
-
-    return fields;
+    return results.flat();
   };
 
   private handleKeyword = async (context?: TypeaheadContext): Promise<TypeaheadOutput> => {
-    const suggs = await this.getFieldCompletionItems(context?.logGroupNames ?? [], context?.region || 'default');
+    const suggs = await this.getFieldCompletionItems(context?.logGroups, context?.region || 'default');
     const functionSuggestions: CompletionItemGroup[] = [
       {
         searchFunctionType: SearchFunctionType.Prefix,
@@ -192,7 +167,7 @@ export class CloudWatchLanguageProvider extends LanguageProvider {
 
     if (queryCommand === 'parse') {
       if (currentTokenIsFirstArg) {
-        return await this.getFieldCompletionItems(context?.logGroupNames ?? [], context?.region || 'default');
+        return await this.getFieldCompletionItems(context?.logGroups ?? [], context?.region || 'default');
       }
     }
 
@@ -210,7 +185,7 @@ export class CloudWatchLanguageProvider extends LanguageProvider {
 
     if (['display', 'fields'].includes(queryCommand)) {
       const typeaheadOutput = await this.getFieldCompletionItems(
-        context?.logGroupNames ?? [],
+        context?.logGroups ?? [],
         context?.region || 'default'
       );
       typeaheadOutput.suggestions.push(...this.getFieldAndFilterFunctionCompletionItems().suggestions);
@@ -229,7 +204,7 @@ export class CloudWatchLanguageProvider extends LanguageProvider {
     }
 
     if (queryCommand === 'filter' && currentTokenIsFirstArg) {
-      const sugg = await this.getFieldCompletionItems(context?.logGroupNames ?? [], context?.region || 'default');
+      const sugg = await this.getFieldCompletionItems(context?.logGroups, context?.region || 'default');
       const boolFuncs = this.getBoolFuncCompletionItems();
       sugg.suggestions.push(...boolFuncs.suggestions);
       return sugg;
@@ -243,7 +218,7 @@ export class CloudWatchLanguageProvider extends LanguageProvider {
     context?: TypeaheadContext
   ): Promise<TypeaheadOutput> {
     if (isFirstArgument) {
-      return await this.getFieldCompletionItems(context?.logGroupNames ?? [], context?.region || 'default');
+      return await this.getFieldCompletionItems(context?.logGroups, context?.region || 'default');
     } else if (isTokenType(prevNonWhitespaceToken(curToken), 'field-name')) {
       // suggest sort options
       return {
@@ -266,10 +241,7 @@ export class CloudWatchLanguageProvider extends LanguageProvider {
   }
 
   private handleComparison = async (context?: TypeaheadContext) => {
-    const fieldsSuggestions = await this.getFieldCompletionItems(
-      context?.logGroupNames ?? [],
-      context?.region || 'default'
-    );
+    const fieldsSuggestions = await this.getFieldCompletionItems(context?.logGroups, context?.region || 'default');
     const comparisonSuggestions = this.getComparisonCompletionItems();
     fieldsSuggestions.suggestions.push(...comparisonSuggestions.suggestions);
     return fieldsSuggestions;
@@ -321,9 +293,15 @@ export class CloudWatchLanguageProvider extends LanguageProvider {
     };
   };
 
-  private getFieldCompletionItems = async (logGroups: string[], region: string): Promise<TypeaheadOutput> => {
-    const fields = await this.fetchFields(logGroups, region);
+  private getFieldCompletionItems = async (
+    logGroups: LogGroup[] | undefined,
+    region: string
+  ): Promise<TypeaheadOutput> => {
+    if (!logGroups) {
+      return { suggestions: [] };
+    }
 
+    const fields = await this.fetchFields(logGroups, region);
     return {
       suggestions: [
         {

public/app/plugins/datasource/cloudwatch/query-runner/CloudWatchLogsQueryRunner.test.ts

@@ -1,6 +1,6 @@
 import { interval, lastValueFrom, of } from 'rxjs';
 
-import { dataFrameToJSON, DataQueryErrorType, FieldType, LogLevel, LogRowModel, MutableDataFrame } from '@grafana/data';
+import { DataQueryErrorType, FieldType, LogLevel, LogRowModel, MutableDataFrame } from '@grafana/data';
 
 import { genMockFrames, setupMockedLogsQueryRunner } from '../__mocks__/LogsQueryRunner';
 import { validLogsQuery } from '../__mocks__/queries';
@@ -51,34 +51,6 @@ describe('CloudWatchLogsQueryRunner', () => {
     });
   });
 
-  describe('getLogGroupFields', () => {
-    it('passes region correctly', async () => {
-      const { runner, fetchMock } = setupMockedLogsQueryRunner();
-      fetchMock.mockReturnValueOnce(
-        of({
-          data: {
-            results: {
-              A: {
-                frames: [
-                  dataFrameToJSON(
-                    new MutableDataFrame({
-                      fields: [
-                        { name: 'key', values: [] },
-                        { name: 'val', values: [] },
-                      ],
-                    })
-                  ),
-                ],
-              },
-            },
-          },
-        })
-      );
-      await runner.getLogGroupFields({ region: 'us-west-1', logGroupName: 'test' });
-      expect(fetchMock.mock.calls[0][0].data.queries[0].region).toBe('us-west-1');
-    });
-  });
-
   describe('logs query', () => {
     beforeEach(() => {
       jest.spyOn(rxjsUtils, 'increasingInterval').mockImplementation(() => interval(100));

public/app/plugins/datasource/cloudwatch/query-runner/CloudWatchLogsQueryRunner.ts

@@ -42,7 +42,6 @@ import {
   DescribeLogGroupsRequest,
   GetLogEventsRequest,
   GetLogGroupFieldsRequest,
-  GetLogGroupFieldsResponse,
   LogAction,
   StartQueryRequest,
 } from '../types';
@@ -415,18 +414,6 @@ export class CloudWatchLogsQueryRunner extends CloudWatchRequest {
     };
   };
 
-  async getLogGroupFields(params: GetLogGroupFieldsRequest): Promise<GetLogGroupFieldsResponse> {
-    const dataFrames = await lastValueFrom(this.makeLogActionRequest('GetLogGroupFields', [params]));
-
-    const fieldNames = dataFrames[0].fields[0].values.toArray();
-    const fieldPercentages = dataFrames[0].fields[1].values.toArray();
-    const getLogGroupFieldsResponse = {
-      logGroupFields: fieldNames.map((val, i) => ({ name: val, percent: fieldPercentages[i] })) ?? [],
-    };
-
-    return getLogGroupFieldsResponse;
-  }
-
   private filterQuery(query: CloudWatchLogsQuery) {
     const hasMissingLegacyLogGroupNames = !query.logGroupNames?.length;
     const hasMissingLogGroups = !query.logGroups?.length;

public/app/plugins/datasource/cloudwatch/types.ts

@@ -267,36 +267,17 @@ export interface TSDBTimeSeries {
 }
 export type TSDBTimePoint = [number, number];
 
-export interface GetLogGroupFieldsRequest {
-  /**
-   * The name of the log group to search.
-   */
-  logGroupName: string;
-  /**
-   * The time to set as the center of the query. If you specify time, the 8 minutes before and 8 minutes after this time are searched. If you omit time, the past 15 minutes are queried. The time value is specified as epoch time, the number of seconds since January 1, 1970, 00:00:00 UTC.
-   */
-  time?: number;
-  region: string;
-}
-
 export interface LogGroupField {
   /**
    * The name of a log field.
    */
-  name?: string;
+  name: string;
   /**
    * The percentage of log events queried that contained the field.
    */
   percent?: number;
 }
 
-export interface GetLogGroupFieldsResponse {
-  /**
-   * The array of fields found in the query. Each object in the array contains the name of the field, along with the percentage of time it appeared in the log events that were queried.
-   */
-  logGroupFields?: LogGroupField[];
-}
-
 export interface StartQueryRequest {
   /**
    * The log group on which to perform the query. A StartQuery operation must include a logGroupNames or a logGroupName parameter, but not both.
@@ -423,6 +404,17 @@ export interface ResourceRequest {
   accountId?: string;
 }
 
+export interface GetLogGroupFieldsRequest extends ResourceRequest {
+  /**
+   * The log group identifier
+   */
+  arn?: string;
+  /**
+   * The name of the log group to search.
+   */
+  logGroupName: string;
+}
+
 export interface GetDimensionKeysRequest extends ResourceRequest {
   metricName?: string;
   namespace?: string;