From 8dd8c50dc41c5b10e655ad00076377523c19e5e1 Mon Sep 17 00:00:00 2001 From: Isabella Siu Date: Thu, 7 Jul 2022 13:03:02 -0400 Subject: [PATCH] CloudWatch: Add default log groups to config page (#49286) Co-authored-by: Shirley Leu <4163034+fridgepoet@users.noreply.github.com> --- .betterer.results | 18 +-- .../__mocks__/CloudWatchDataSource.ts | 1 + .../components/ConfigEditor.test.tsx | 132 +++++++++++------- .../cloudwatch/components/ConfigEditor.tsx | 60 +++++++- .../components/LogGroupSelector.test.tsx | 1 - .../components/LogsQueryField.test.tsx | 23 ++- .../cloudwatch/components/LogsQueryField.tsx | 6 +- .../__snapshots__/ConfigEditor.test.tsx.snap | 80 ++++++++++- .../datasource/cloudwatch/datasource.ts | 18 +-- .../plugins/datasource/cloudwatch/types.ts | 1 + 10 files changed, 259 insertions(+), 81 deletions(-) diff --git a/.betterer.results b/.betterer.results index 18fc7a23a54..50682b218d8 100644 --- a/.betterer.results +++ b/.betterer.results @@ -86,8 +86,8 @@ exports[`no enzyme tests`] = { "public/app/features/folders/FolderSettingsPage.test.tsx:1109052730": [ [0, 19, 13, "RegExp match", "2409514259"] ], - "public/app/plugins/datasource/cloudwatch/components/ConfigEditor.test.tsx:227258837": [ - [0, 19, 13, "RegExp match", "2409514259"] + "public/app/plugins/datasource/cloudwatch/components/ConfigEditor.test.tsx:4057721851": [ + [1, 19, 13, "RegExp match", "2409514259"] ], "public/app/plugins/datasource/elasticsearch/configuration/ConfigEditor.test.tsx:3481855642": [ [0, 26, 13, "RegExp match", "2409514259"] @@ -6659,8 +6659,12 @@ exports[`better eslint`] = { [0, 0, 0, "Do not use any type assertions.", "1"], [0, 0, 0, "Unexpected any. Specify a different type.", "2"] ], + "public/app/plugins/datasource/cloudwatch/components/ConfigEditor.test.tsx:5381": [ + [0, 0, 0, "Unexpected any. Specify a different type.", "0"] + ], "public/app/plugins/datasource/cloudwatch/components/ConfigEditor.tsx:5381": [ - [0, 0, 0, "Do not use any type assertions.", "0"] + [0, 0, 0, "Unexpected any. Specify a different type.", "0"], + [0, 0, 0, "Do not use any type assertions.", "1"] ], "public/app/plugins/datasource/cloudwatch/components/LogsQueryEditor.tsx:5381": [ [0, 0, 0, "Do not use any type assertions.", "0"] @@ -6668,7 +6672,8 @@ exports[`better eslint`] = { "public/app/plugins/datasource/cloudwatch/components/LogsQueryField.test.tsx:5381": [ [0, 0, 0, "Unexpected any. Specify a different type.", "0"], [0, 0, 0, "Unexpected any. Specify a different type.", "1"], - [0, 0, 0, "Unexpected any. Specify a different type.", "2"] + [0, 0, 0, "Unexpected any. Specify a different type.", "2"], + [0, 0, 0, "Unexpected any. Specify a different type.", "3"] ], "public/app/plugins/datasource/cloudwatch/components/LogsQueryField.tsx:5381": [ [0, 0, 0, "Unexpected any. Specify a different type.", "0"], @@ -7568,10 +7573,7 @@ exports[`better eslint`] = { [0, 0, 0, "Unexpected any. Specify a different type.", "13"], [0, 0, 0, "Unexpected any. Specify a different type.", "14"], [0, 0, 0, "Unexpected any. Specify a different type.", "15"], - [0, 0, 0, "Unexpected any. Specify a different type.", "16"], - [0, 0, 0, "Unexpected any. Specify a different type.", "17"], - [0, 0, 0, "Unexpected any. Specify a different type.", "18"], - [0, 0, 0, "Unexpected any. Specify a different type.", "19"] + [0, 0, 0, "Unexpected any. Specify a different type.", "16"] ], "public/app/plugins/datasource/influxdb/specs/influx_query_model.test.ts:5381": [ [0, 0, 0, "Unexpected any. Specify a different type.", "0"], diff --git a/public/app/plugins/datasource/cloudwatch/__mocks__/CloudWatchDataSource.ts b/public/app/plugins/datasource/cloudwatch/__mocks__/CloudWatchDataSource.ts index bcb8d3a7584..d662e226a76 100644 --- a/public/app/plugins/datasource/cloudwatch/__mocks__/CloudWatchDataSource.ts +++ b/public/app/plugins/datasource/cloudwatch/__mocks__/CloudWatchDataSource.ts @@ -52,6 +52,7 @@ export function setupMockedDataSource({ datasource.getNamespaces = jest.fn().mockResolvedValue([]); datasource.getRegions = jest.fn().mockResolvedValue([]); + datasource.defaultLogGroups = []; const fetchMock = jest.fn().mockReturnValue(of({ data })); setBackendSrv({ fetch: fetchMock } as any); diff --git a/public/app/plugins/datasource/cloudwatch/components/ConfigEditor.test.tsx b/public/app/plugins/datasource/cloudwatch/components/ConfigEditor.test.tsx index ac4ca34f1e1..26b998f6514 100644 --- a/public/app/plugins/datasource/cloudwatch/components/ConfigEditor.test.tsx +++ b/public/app/plugins/datasource/cloudwatch/components/ConfigEditor.test.tsx @@ -1,71 +1,92 @@ +import { render, screen } from '@testing-library/react'; import { shallow } from 'enzyme'; import React from 'react'; +import selectEvent from 'react-select-event'; import { AwsAuthType } from '@grafana/aws-sdk'; +import { setupMockedDataSource } from '../__mocks__/CloudWatchDataSource'; + import { ConfigEditor, Props } from './ConfigEditor'; +const ds = setupMockedDataSource(); + jest.mock('app/features/plugins/datasource_srv', () => ({ getDatasourceSrv: () => ({ - loadDatasource: jest.fn().mockImplementation(() => - Promise.resolve({ - getRegions: jest.fn().mockReturnValue([ - { - label: 'ap-east-1', - value: 'ap-east-1', - }, - ]), - }) - ), + loadDatasource: jest.fn().mockResolvedValue({ + getRegions: jest.fn().mockResolvedValue([ + { + label: 'ap-east-1', + value: 'ap-east-1', + }, + ]), + describeLogGroups: jest.fn().mockResolvedValue(['logGroup-foo', 'logGroup-bar']), + getActualRegion: jest.fn().mockReturnValue('ap-east-1'), + getVariables: jest.fn().mockReturnValue([]), + }), }), })); -const setup = (propOverrides?: object) => { - const props: Props = { - options: { - id: 1, - uid: 'z', - orgId: 1, - typeLogoUrl: '', - name: 'CloudWatch', - access: 'proxy', - url: '', - database: '', - type: 'cloudwatch', - typeName: 'Cloudwatch', - user: '', - basicAuth: false, - basicAuthUser: '', - isDefault: true, - readOnly: false, - withCredentials: false, - secureJsonFields: { - accessKey: false, - secretKey: false, - }, - jsonData: { - assumeRoleArn: '', - externalId: '', - database: '', - customMetricsNamespaces: '', - authType: AwsAuthType.Keys, - defaultRegion: 'us-east-2', - timeField: '@timestamp', - }, - secureJsonData: { - secretKey: '', - accessKey: '', - }, +jest.mock('./XrayLinkConfig', () => ({ + XrayLinkConfig: () => <>, +})); + +jest.mock('@grafana/runtime', () => ({ + ...jest.requireActual('@grafana/runtime'), + getBackendSrv: () => ({ + put: jest.fn().mockResolvedValue({ datasource: ds.datasource }), + }), +})); + +const props: Props = { + options: { + id: 1, + uid: 'z', + orgId: 1, + typeLogoUrl: '', + name: 'CloudWatch', + access: 'proxy', + url: '', + database: '', + type: 'cloudwatch', + typeName: 'Cloudwatch', + user: '', + basicAuth: false, + basicAuthUser: '', + isDefault: true, + readOnly: false, + withCredentials: false, + secureJsonFields: { + accessKey: false, + secretKey: false, }, - onOptionsChange: jest.fn(), - }; + jsonData: { + assumeRoleArn: '', + externalId: '', + database: '', + customMetricsNamespaces: '', + authType: AwsAuthType.Keys, + defaultRegion: 'us-east-2', + timeField: '@timestamp', + }, + secureJsonData: { + secretKey: '', + accessKey: '', + }, + }, + onOptionsChange: jest.fn(), +}; - Object.assign(props, propOverrides); +const setup = (propOverrides?: object) => { + const newProps = { ...props, ...propOverrides }; - return shallow(); + return shallow(); }; describe('Render', () => { + beforeEach(() => { + jest.resetAllMocks(); + }); it('should render component', () => { const wrapper = setup(); @@ -107,4 +128,15 @@ describe('Render', () => { }); expect(wrapper).toMatchSnapshot(); }); + + it('should load log groups when multiselect is opened', async () => { + (window as any).grafanaBootData = { + settings: {}, + }; + + render(); + const multiselect = await screen.findByLabelText('Log Groups'); + selectEvent.openMenu(multiselect); + expect(await screen.findByText('logGroup-foo')).toBeInTheDocument(); + }); }); diff --git a/public/app/plugins/datasource/cloudwatch/components/ConfigEditor.tsx b/public/app/plugins/datasource/cloudwatch/components/ConfigEditor.tsx index 7d54e625375..7eb6a4567cd 100644 --- a/public/app/plugins/datasource/cloudwatch/components/ConfigEditor.tsx +++ b/public/app/plugins/datasource/cloudwatch/components/ConfigEditor.tsx @@ -7,7 +7,9 @@ import { DataSourcePluginOptionsEditorProps, onUpdateDatasourceJsonDataOption, updateDatasourcePluginJsonDataOption, + updateDatasourcePluginOption, } from '@grafana/data'; +import { getBackendSrv } from '@grafana/runtime'; import { Input, InlineField } from '@grafana/ui'; import { notifyApp } from 'app/core/actions'; import { createWarningNotification } from 'app/core/copy/appNotification'; @@ -17,16 +19,43 @@ import { store } from 'app/store/store'; import { CloudWatchDatasource } from '../datasource'; import { CloudWatchJsonData, CloudWatchSecureJsonData } from '../types'; +import { LogGroupSelector } from './LogGroupSelector'; import { XrayLinkConfig } from './XrayLinkConfig'; export type Props = DataSourcePluginOptionsEditorProps; export const ConfigEditor: FC = (props: Props) => { const { options } = props; + const { defaultLogGroups, logsTimeout, defaultRegion } = options.jsonData; + const [saved, setSaved] = useState(!!options.version && options.version > 1); - const datasource = useDatasource(options.name); + const datasource = useDatasource(options.name, saved); useAuthenticationWarning(options.jsonData); - const logsTimeoutError = useTimoutValidation(props.options.jsonData.logsTimeout); + const logsTimeoutError = useTimoutValidation(logsTimeout); + useEffect(() => { + setSaved(false); + }, [ + props.options.jsonData.assumeRoleArn, + props.options.jsonData.authType, + props.options.jsonData.defaultRegion, + props.options.jsonData.endpoint, + props.options.jsonData.externalId, + props.options.jsonData.profile, + props.options.secureJsonData?.accessKey, + props.options.secureJsonData?.secretKey, + ]); + + const saveOptions = async (): Promise => { + if (saved) { + return; + } + await getBackendSrv() + .put(`/api/datasources/${options.id}`, options) + .then((result: { datasource: any }) => { + updateDatasourcePluginOption(props, 'version', result.datasource.version); + }); + setSaved(true); + }; return ( <> @@ -52,7 +81,7 @@ export const ConfigEditor: FC = (props: Props) => { = (props: Props) => { title={'The timeout must be a valid duration string, such as "15m" "30s" "2000ms" etc.'} /> + + { + updateDatasourcePluginJsonDataOption(props, 'defaultLogGroups', logGroups); + }} + onOpenMenu={saveOptions} + width={60} + saved={saved} + /> + (); useEffect(() => { + // reload the datasource when it's saved + if (!saved) { + return; + } getDatasourceSrv() .loadDatasource(datasourceName) .then((datasource) => { @@ -102,7 +152,7 @@ function useDatasource(datasourceName: string) { // So a "as" type assertion here is a necessary evil. setDatasource(datasource as CloudWatchDatasource); }); - }, [datasourceName]); + }, [datasourceName, saved]); return datasource; } diff --git a/public/app/plugins/datasource/cloudwatch/components/LogGroupSelector.test.tsx b/public/app/plugins/datasource/cloudwatch/components/LogGroupSelector.test.tsx index 519b4ebed3c..91ebf4d32eb 100644 --- a/public/app/plugins/datasource/cloudwatch/components/LogGroupSelector.test.tsx +++ b/public/app/plugins/datasource/cloudwatch/components/LogGroupSelector.test.tsx @@ -85,7 +85,6 @@ describe('LogGroupSelector', () => { 'DeliciousGroup', 'DeliciousGroup2', 'DeliciousGroup3', - 'VelvetGroup', 'VelvetGroup2', 'VelvetGroup3', diff --git a/public/app/plugins/datasource/cloudwatch/components/LogsQueryField.test.tsx b/public/app/plugins/datasource/cloudwatch/components/LogsQueryField.test.tsx index b3dc5a0654a..5ad1dcbdad7 100644 --- a/public/app/plugins/datasource/cloudwatch/components/LogsQueryField.test.tsx +++ b/public/app/plugins/datasource/cloudwatch/components/LogsQueryField.test.tsx @@ -1,4 +1,4 @@ -import { render, screen, fireEvent } from '@testing-library/react'; +import { render, screen, fireEvent, waitFor } from '@testing-library/react'; import _, { DebouncedFunc } from 'lodash'; // eslint-disable-line lodash/import-scope import React from 'react'; import { act } from 'react-dom/test-utils'; @@ -34,4 +34,25 @@ describe('CloudWatchLogsQueryField', () => { }); expect(onRunQuery).toHaveBeenCalled(); }); + + it('loads defaultLogGroups', async () => { + const onRunQuery = jest.fn(); + const ds = setupMockedDataSource(); + ds.datasource.defaultLogGroups = ['foo']; + + render( + {}} + /> + ); + + await waitFor(() => { + expect(screen.getByText('foo')).toBeInTheDocument(); + }); + }); }); diff --git a/public/app/plugins/datasource/cloudwatch/components/LogsQueryField.tsx b/public/app/plugins/datasource/cloudwatch/components/LogsQueryField.tsx index ab1815eda45..259b9aeb6b5 100644 --- a/public/app/plugins/datasource/cloudwatch/components/LogsQueryField.tsx +++ b/public/app/plugins/datasource/cloudwatch/components/LogsQueryField.tsx @@ -66,10 +66,10 @@ export class CloudWatchLogsQueryField extends React.PureComponent { - const { query, onChange } = this.props; + const { query, datasource, onChange } = this.props; if (onChange) { - onChange({ ...query, logGroupNames: query.logGroupNames ?? [] }); + onChange({ ...query, logGroupNames: query.logGroupNames ?? datasource.defaultLogGroups }); } }; @@ -135,7 +135,7 @@ export class CloudWatchLogsQueryField extends React.PureComponent + + + + + + + + + + + + + + + ): Observable => { - const validLogQueries = logQueries.filter((item) => item.logGroupNames?.length); + const queryParams = logQueries.map((target: CloudWatchLogsQuery) => ({ + queryString: target.expression || '', + refId: target.refId, + logGroupNames: target.logGroupNames || this.defaultLogGroups, + region: this.replace(this.getActualRegion(target.region), options.scopedVars, true, 'region'), + })); + + const validLogQueries = queryParams.filter((item) => item.logGroupNames?.length); if (logQueries.length > validLogQueries.length) { return of({ data: [], error: { message: 'Log group is required' } }); } @@ -186,13 +195,6 @@ export class CloudWatchDatasource return of({ data: [], state: LoadingState.Done }); } - const queryParams = logQueries.map((target: CloudWatchLogsQuery) => ({ - queryString: target.expression || '', - refId: target.refId, - logGroupNames: target.logGroupNames, - region: this.replace(this.getActualRegion(target.region), options.scopedVars, true, 'region'), - })); - const startTime = new Date(); const timeoutFunc = () => { return Date.now() >= startTime.valueOf() + rangeUtil.intervalToMs(this.logsTimeout); diff --git a/public/app/plugins/datasource/cloudwatch/types.ts b/public/app/plugins/datasource/cloudwatch/types.ts index d9428d8581a..668dad3e861 100644 --- a/public/app/plugins/datasource/cloudwatch/types.ts +++ b/public/app/plugins/datasource/cloudwatch/types.ts @@ -121,6 +121,7 @@ export interface CloudWatchJsonData extends AwsAuthDataSourceJsonData { logsTimeout?: string; // Used to create links if logs contain traceId. tracingDatasourceUid?: string; + defaultLogGroups?: string[]; } export interface CloudWatchSecureJsonData extends AwsAuthDataSourceSecureJsonData {