From 377262c2838c0aed6518e58af3e6c8e98e78757c Mon Sep 17 00:00:00 2001 From: Kevin Yu Date: Tue, 12 Dec 2023 17:32:04 -0800 Subject: [PATCH] Cloudwatch: Fix errors while loading queries/datasource on Safari (#79417) * pass super.query to query runners * fix types in tests * clearer name for query function * fix test --- .../__mocks__/AnnotationQueryRunner.ts | 2 +- .../cloudwatch/__mocks__/LogsQueryRunner.ts | 2 +- .../__mocks__/MetricsQueryRunner.ts | 2 +- .../datasource/cloudwatch/datasource.ts | 28 +++---- .../CloudWatchAnnotationQueryRunner.test.ts | 2 +- .../CloudWatchAnnotationQueryRunner.ts | 13 ++- .../CloudWatchLogsQueryRunner.test.ts | 39 +++++---- .../query-runner/CloudWatchLogsQueryRunner.ts | 41 +++++---- .../CloudWatchMetricsQueryRunner.test.ts | 84 +++++++++++-------- .../CloudWatchMetricsQueryRunner.ts | 18 ++-- .../query-runner/CloudWatchRequest.ts | 18 +--- 11 files changed, 129 insertions(+), 120 deletions(-) diff --git a/public/app/plugins/datasource/cloudwatch/__mocks__/AnnotationQueryRunner.ts b/public/app/plugins/datasource/cloudwatch/__mocks__/AnnotationQueryRunner.ts index 4c65b952525..bae51a44dec 100644 --- a/public/app/plugins/datasource/cloudwatch/__mocks__/AnnotationQueryRunner.ts +++ b/public/app/plugins/datasource/cloudwatch/__mocks__/AnnotationQueryRunner.ts @@ -16,7 +16,7 @@ export function setupMockedAnnotationQueryRunner({ variables }: { variables?: Cu } const queryMock = jest.fn().mockReturnValue(of({})); - const runner = new CloudWatchAnnotationQueryRunner(CloudWatchSettings, templateService, queryMock); + const runner = new CloudWatchAnnotationQueryRunner(CloudWatchSettings, templateService); const request: DataQueryRequest = { range: TimeRangeMock, diff --git a/public/app/plugins/datasource/cloudwatch/__mocks__/LogsQueryRunner.ts b/public/app/plugins/datasource/cloudwatch/__mocks__/LogsQueryRunner.ts index 11f75e7b13e..94ff8a5f3ff 100644 --- a/public/app/plugins/datasource/cloudwatch/__mocks__/LogsQueryRunner.ts +++ b/public/app/plugins/datasource/cloudwatch/__mocks__/LogsQueryRunner.ts @@ -31,7 +31,7 @@ export function setupMockedLogsQueryRunner({ } const queryMock = jest.fn().mockReturnValue(of(toDataQueryResponse({ data }))); - const runner = new CloudWatchLogsQueryRunner(settings, templateService, queryMock); + const runner = new CloudWatchLogsQueryRunner(settings, templateService); return { runner, queryMock, templateService }; } diff --git a/public/app/plugins/datasource/cloudwatch/__mocks__/MetricsQueryRunner.ts b/public/app/plugins/datasource/cloudwatch/__mocks__/MetricsQueryRunner.ts index 186aa8df5e6..362983b0a2c 100644 --- a/public/app/plugins/datasource/cloudwatch/__mocks__/MetricsQueryRunner.ts +++ b/public/app/plugins/datasource/cloudwatch/__mocks__/MetricsQueryRunner.ts @@ -36,7 +36,7 @@ export function setupMockedMetricsQueryRunner({ const queryMock = errorResponse ? jest.fn().mockImplementation(() => throwError(errorResponse)) : jest.fn().mockReturnValue(of(toDataQueryResponse({ data }))); - const runner = new CloudWatchMetricsQueryRunner(instanceSettings, templateService, queryMock); + const runner = new CloudWatchMetricsQueryRunner(instanceSettings, templateService); const request: DataQueryRequest = { range: TimeRangeMock, diff --git a/public/app/plugins/datasource/cloudwatch/datasource.ts b/public/app/plugins/datasource/cloudwatch/datasource.ts index 3c736252ed6..01783555ebf 100644 --- a/public/app/plugins/datasource/cloudwatch/datasource.ts +++ b/public/app/plugins/datasource/cloudwatch/datasource.ts @@ -68,14 +68,10 @@ export class CloudWatchDatasource this.languageProvider = new CloudWatchLogsLanguageProvider(this); this.sqlCompletionItemProvider = new SQLCompletionItemProvider(this.resources, this.templateSrv); this.metricMathCompletionItemProvider = new MetricMathCompletionItemProvider(this.resources, this.templateSrv); - this.metricsQueryRunner = new CloudWatchMetricsQueryRunner(instanceSettings, templateSrv, super.query.bind(this)); + this.metricsQueryRunner = new CloudWatchMetricsQueryRunner(instanceSettings, templateSrv); this.logsCompletionItemProviderFunc = LogsCompletionItemProviderFunc(this.resources, this.templateSrv); - this.logsQueryRunner = new CloudWatchLogsQueryRunner(instanceSettings, templateSrv, super.query.bind(this)); - this.annotationQueryRunner = new CloudWatchAnnotationQueryRunner( - instanceSettings, - templateSrv, - super.query.bind(this) - ); + this.logsQueryRunner = new CloudWatchLogsQueryRunner(instanceSettings, templateSrv); + this.annotationQueryRunner = new CloudWatchAnnotationQueryRunner(instanceSettings, templateSrv); this.variables = new CloudWatchVariableSupport(this.resources); this.annotations = CloudWatchAnnotationSupport; this.defaultLogGroups = instanceSettings.jsonData.defaultLogGroups; @@ -106,15 +102,19 @@ export class CloudWatchDatasource const dataQueryResponses: Array> = []; if (logQueries.length) { - dataQueryResponses.push(this.logsQueryRunner.handleLogQueries(logQueries, options)); + dataQueryResponses.push(this.logsQueryRunner.handleLogQueries(logQueries, options, super.query.bind(this))); } if (metricsQueries.length) { - dataQueryResponses.push(this.metricsQueryRunner.handleMetricQueries(metricsQueries, options)); + dataQueryResponses.push( + this.metricsQueryRunner.handleMetricQueries(metricsQueries, options, super.query.bind(this)) + ); } if (annotationQueries.length) { - dataQueryResponses.push(this.annotationQueryRunner.handleAnnotationQuery(annotationQueries, options)); + dataQueryResponses.push( + this.annotationQueryRunner.handleAnnotationQuery(annotationQueries, options, super.query.bind(this)) + ); } // No valid targets, return the empty result to save a round trip. if (isEmpty(dataQueryResponses)) { @@ -143,13 +143,13 @@ export class CloudWatchDatasource })); } - getLogRowContext = async ( + getLogRowContext( row: LogRowModel, context?: LogRowContextOptions, query?: CloudWatchLogsQuery - ): Promise<{ data: DataFrame[] }> => { - return this.logsQueryRunner.getLogRowContext(row, context, query); - }; + ): Promise<{ data: DataFrame[] }> { + return this.logsQueryRunner.getLogRowContext(row, context, super.query.bind(this), query); + } targetContainsTemplate(target: any) { return ( diff --git a/public/app/plugins/datasource/cloudwatch/query-runner/CloudWatchAnnotationQueryRunner.test.ts b/public/app/plugins/datasource/cloudwatch/query-runner/CloudWatchAnnotationQueryRunner.test.ts index 535099abdfa..75df7ff069e 100644 --- a/public/app/plugins/datasource/cloudwatch/query-runner/CloudWatchAnnotationQueryRunner.test.ts +++ b/public/app/plugins/datasource/cloudwatch/query-runner/CloudWatchAnnotationQueryRunner.test.ts @@ -25,7 +25,7 @@ describe('CloudWatchAnnotationQueryRunner', () => { const { runner, queryMock, request } = setupMockedAnnotationQueryRunner({ variables: [namespaceVariable, regionVariable], }); - await expect(runner.handleAnnotationQuery(queries, request)).toEmitValuesWith(() => { + await expect(runner.handleAnnotationQuery(queries, request, queryMock)).toEmitValuesWith(() => { expect(queryMock.mock.calls[0][0].targets[0]).toMatchObject( expect.objectContaining({ region: regionVariable.current.value, diff --git a/public/app/plugins/datasource/cloudwatch/query-runner/CloudWatchAnnotationQueryRunner.ts b/public/app/plugins/datasource/cloudwatch/query-runner/CloudWatchAnnotationQueryRunner.ts index f048b9e53e8..ec01b0f3aa1 100644 --- a/public/app/plugins/datasource/cloudwatch/query-runner/CloudWatchAnnotationQueryRunner.ts +++ b/public/app/plugins/datasource/cloudwatch/query-runner/CloudWatchAnnotationQueryRunner.ts @@ -9,19 +9,16 @@ import { CloudWatchRequest } from './CloudWatchRequest'; // This class handles execution of CloudWatch annotation queries export class CloudWatchAnnotationQueryRunner extends CloudWatchRequest { - constructor( - instanceSettings: DataSourceInstanceSettings, - templateSrv: TemplateSrv, - queryFn: (request: DataQueryRequest) => Observable - ) { - super(instanceSettings, templateSrv, queryFn); + constructor(instanceSettings: DataSourceInstanceSettings, templateSrv: TemplateSrv) { + super(instanceSettings, templateSrv); } handleAnnotationQuery( queries: CloudWatchAnnotationQuery[], - options: DataQueryRequest + options: DataQueryRequest, + queryFn: (request: DataQueryRequest) => Observable ): Observable { - return this.query({ + return queryFn({ ...options, targets: queries.map((query) => ({ ...query, diff --git a/public/app/plugins/datasource/cloudwatch/query-runner/CloudWatchLogsQueryRunner.test.ts b/public/app/plugins/datasource/cloudwatch/query-runner/CloudWatchLogsQueryRunner.test.ts index 8f9b0b4b50a..dd098e59ddc 100644 --- a/public/app/plugins/datasource/cloudwatch/query-runner/CloudWatchLogsQueryRunner.test.ts +++ b/public/app/plugins/datasource/cloudwatch/query-runner/CloudWatchLogsQueryRunner.test.ts @@ -66,15 +66,14 @@ describe('CloudWatchLogsQueryRunner', () => { timeUtc: '', uid: '1', }; - await runner.getLogRowContext(row); + await runner.getLogRowContext(row, undefined, queryMock); expect(queryMock.mock.calls[0][0].targets[0].endTime).toBe(4); expect(queryMock.mock.calls[0][0].targets[0].region).toBe(''); - await runner.getLogRowContext( - row, - { direction: LogRowContextQueryDirection.Forward }, - { ...validLogsQuery, region: 'eu-east' } - ); + await runner.getLogRowContext(row, { direction: LogRowContextQueryDirection.Forward }, queryMock, { + ...validLogsQuery, + region: 'eu-east', + }); expect(queryMock.mock.calls[1][0].targets[0].startTime).toBe(4); expect(queryMock.mock.calls[1][0].targets[0].region).toBe('eu-east'); }); @@ -86,7 +85,7 @@ describe('CloudWatchLogsQueryRunner', () => { }); it('should stop querying when timed out', async () => { - const { runner } = setupMockedLogsQueryRunner(); + const { runner, queryMock } = setupMockedLogsQueryRunner(); const fakeFrames = genMockFrames(20); const initialRecordsMatched = fakeFrames[0].meta!.stats!.find((stat) => stat.displayName === 'Records scanned')! .value!; @@ -127,7 +126,7 @@ describe('CloudWatchLogsQueryRunner', () => { return i >= iterations; }; const myResponse = await lastValueFrom( - runner.logsQuery([{ queryId: 'fake-query-id', region: 'default', refId: 'A' }], timeoutFunc) + runner.logsQuery([{ queryId: 'fake-query-id', region: 'default', refId: 'A' }], timeoutFunc, queryMock) ); const expectedData = [ @@ -155,7 +154,7 @@ describe('CloudWatchLogsQueryRunner', () => { }); it('should continue querying as long as new data is being received', async () => { - const { runner } = setupMockedLogsQueryRunner(); + const { runner, queryMock } = setupMockedLogsQueryRunner(); const fakeFrames = genMockFrames(15); let i = 0; @@ -174,7 +173,7 @@ describe('CloudWatchLogsQueryRunner', () => { return Date.now() >= startTime.valueOf() + 6000; }; const myResponse = await lastValueFrom( - runner.logsQuery([{ queryId: 'fake-query-id', region: 'default', refId: 'A' }], timeoutFunc) + runner.logsQuery([{ queryId: 'fake-query-id', region: 'default', refId: 'A' }], timeoutFunc, queryMock) ); expect(myResponse).toEqual({ data: [fakeFrames[fakeFrames.length - 1]], @@ -185,7 +184,7 @@ describe('CloudWatchLogsQueryRunner', () => { }); it('should stop querying when results come back with status "Complete"', async () => { - const { runner } = setupMockedLogsQueryRunner(); + const { runner, queryMock } = setupMockedLogsQueryRunner(); const fakeFrames = genMockFrames(3); let i = 0; jest.spyOn(runner, 'makeLogActionRequest').mockImplementation((subtype: LogAction) => { @@ -203,7 +202,7 @@ describe('CloudWatchLogsQueryRunner', () => { return Date.now() >= startTime.valueOf() + 6000; }; const myResponse = await lastValueFrom( - runner.logsQuery([{ queryId: 'fake-query-id', region: 'default', refId: 'A' }], timeoutFunc) + runner.logsQuery([{ queryId: 'fake-query-id', region: 'default', refId: 'A' }], timeoutFunc, queryMock) ); expect(myResponse).toEqual({ @@ -250,7 +249,7 @@ describe('CloudWatchLogsQueryRunner', () => { describe('handleLogQueries', () => { it('should map log queries to start query requests correctly', async () => { - const { runner } = setupMockedLogsQueryRunner({ + const { runner, queryMock } = setupMockedLogsQueryRunner({ variables: [logGroupNamesVariable, regionVariable, limitVariable], settings: { ...CloudWatchSettings, @@ -263,7 +262,11 @@ describe('CloudWatchLogsQueryRunner', () => { }); const spy = jest.spyOn(runner, 'makeLogActionRequest'); await lastValueFrom( - runner.handleLogQueries([legacyLogGroupNamesQuery, logGroupNamesQuery, logsScopedVarQuery], LogsRequestMock) + runner.handleLogQueries( + [legacyLogGroupNamesQuery, logGroupNamesQuery, logsScopedVarQuery], + LogsRequestMock, + queryMock + ) ); const startQueryRequests: StartQueryRequest[] = [ { @@ -294,7 +297,7 @@ describe('CloudWatchLogsQueryRunner', () => { region: regionVariable.current.value as string, }, ]; - expect(spy).toHaveBeenNthCalledWith(1, 'StartQuery', startQueryRequests, LogsRequestMock); + expect(spy).toHaveBeenNthCalledWith(1, 'StartQuery', startQueryRequests, queryMock, LogsRequestMock); }); }); @@ -307,7 +310,9 @@ describe('CloudWatchLogsQueryRunner', () => { ...LogsRequestMock, range: { from, to, raw: { from, to } }, }; - await lastValueFrom(runner.makeLogActionRequest('StartQuery', [genMockCloudWatchLogsRequest()], options)); + await lastValueFrom( + runner.makeLogActionRequest('StartQuery', [genMockCloudWatchLogsRequest()], queryMock, options) + ); expect(queryMock.mock.calls[0][0].skipQueryCache).toBe(true); expect(queryMock.mock.calls[0][0]).toEqual(expect.objectContaining({ range: { from, to, raw: { from, to } } })); }); @@ -316,7 +321,7 @@ describe('CloudWatchLogsQueryRunner', () => { const from = dateTime(1111); const to = dateTime(2222); const { runner, queryMock } = setupMockedLogsQueryRunner(); - await lastValueFrom(runner.makeLogActionRequest('StartQuery', [genMockCloudWatchLogsRequest()])); + await lastValueFrom(runner.makeLogActionRequest('StartQuery', [genMockCloudWatchLogsRequest()], queryMock)); expect(queryMock.mock.calls[0][0].skipQueryCache).toBe(true); expect(queryMock.mock.calls[0][0]).toEqual(expect.objectContaining({ range: { from, to, raw: { from, to } } })); diff --git a/public/app/plugins/datasource/cloudwatch/query-runner/CloudWatchLogsQueryRunner.ts b/public/app/plugins/datasource/cloudwatch/query-runner/CloudWatchLogsQueryRunner.ts index b2cd2f28e99..a9b28b58765 100644 --- a/public/app/plugins/datasource/cloudwatch/query-runner/CloudWatchLogsQueryRunner.ts +++ b/public/app/plugins/datasource/cloudwatch/query-runner/CloudWatchLogsQueryRunner.ts @@ -61,12 +61,8 @@ export class CloudWatchLogsQueryRunner extends CloudWatchRequest { logQueries: Record = {}; tracingDataSourceUid?: string; - constructor( - instanceSettings: DataSourceInstanceSettings, - templateSrv: TemplateSrv, - queryFn: (request: DataQueryRequest) => Observable - ) { - super(instanceSettings, templateSrv, queryFn); + constructor(instanceSettings: DataSourceInstanceSettings, templateSrv: TemplateSrv) { + super(instanceSettings, templateSrv); this.tracingDataSourceUid = instanceSettings.jsonData.tracingDatasourceUid; this.logsTimeout = instanceSettings.jsonData.logsTimeout || '30m'; @@ -80,10 +76,12 @@ export class CloudWatchLogsQueryRunner extends CloudWatchRequest { error, logQueries, timeoutFunc, + queryFn, }: { frames: DataFrame[]; logQueries: CloudWatchLogsQuery[]; timeoutFunc: () => boolean; + queryFn: (request: DataQueryRequest) => Observable; error?: DataQueryError; }) => { // If every frame is already finished, we can return the result as the @@ -112,7 +110,8 @@ export class CloudWatchLogsQueryRunner extends CloudWatchRequest { refId: dataFrame.refId!, statsGroups: logQueries.find((target) => target.refId === dataFrame.refId)?.statsGroups, })), - timeoutFunc + timeoutFunc, + queryFn ).pipe( map((response: DataQueryResponse) => { if (!response.error && error) { @@ -131,7 +130,8 @@ export class CloudWatchLogsQueryRunner extends CloudWatchRequest { */ handleLogQueries = ( logQueries: CloudWatchLogsQuery[], - options: DataQueryRequest + options: DataQueryRequest, + queryFn: (request: DataQueryRequest) => Observable ): Observable => { const validLogQueries = logQueries.filter(this.filterQuery); @@ -171,13 +171,13 @@ export class CloudWatchLogsQueryRunner extends CloudWatchRequest { return runWithRetry( (targets) => { - return this.makeLogActionRequest('StartQuery', targets, options); + return this.makeLogActionRequest('StartQuery', targets, queryFn, options); }, startQueryRequests, timeoutFunc ).pipe( mergeMap(({ frames, error }: { frames: DataFrame[]; error?: DataQueryError }) => - this.getQueryResults({ frames, logQueries, timeoutFunc, error }) + this.getQueryResults({ frames, logQueries, timeoutFunc, error, queryFn }) ), mergeMap((dataQueryResponse) => { return from( @@ -202,7 +202,11 @@ export class CloudWatchLogsQueryRunner extends CloudWatchRequest { * Checks progress and polls data of a started logs query with some retry logic. * @param queryParams */ - logsQuery(queryParams: QueryParam[], timeoutFunc: () => boolean): Observable { + logsQuery( + queryParams: QueryParam[], + timeoutFunc: () => boolean, + queryFn: (request: DataQueryRequest) => Observable + ): Observable { this.logQueries = {}; queryParams.forEach((param) => { this.logQueries[param.refId] = { @@ -213,7 +217,7 @@ export class CloudWatchLogsQueryRunner extends CloudWatchRequest { }); const dataFrames = increasingInterval({ startPeriod: 100, endPeriod: 1000, step: 300 }).pipe( - concatMap((_) => this.makeLogActionRequest('GetQueryResults', queryParams)), + concatMap((_) => this.makeLogActionRequest('GetQueryResults', queryParams, queryFn)), repeat(), share() ); @@ -284,10 +288,10 @@ export class CloudWatchLogsQueryRunner extends CloudWatchRequest { takeWhile(({ state }) => state !== LoadingState.Error && state !== LoadingState.Done, true) ); - return withTeardown(queryResponse, () => this.stopQueries()); + return withTeardown(queryResponse, () => this.stopQueries(queryFn)); } - stopQueries() { + stopQueries(queryFn: (request: DataQueryRequest) => Observable) { if (Object.keys(this.logQueries).length > 0) { this.makeLogActionRequest( 'StopQuery', @@ -296,7 +300,8 @@ export class CloudWatchLogsQueryRunner extends CloudWatchRequest { region: logQuery.region, queryString: '', refId: '', - })) + })), + queryFn ).pipe( finalize(() => { this.logQueries = {}; @@ -308,6 +313,7 @@ export class CloudWatchLogsQueryRunner extends CloudWatchRequest { makeLogActionRequest( subtype: LogAction, queryParams: CloudWatchLogsRequest[], + queryFn: (request: DataQueryRequest) => Observable, options?: DataQueryRequest ): Observable { const range = options?.range || getDefaultTimeRange(); @@ -336,7 +342,7 @@ export class CloudWatchLogsQueryRunner extends CloudWatchRequest { })), }; - return this.query(requestParams).pipe( + return queryFn(requestParams).pipe( map((response) => response.data), catchError((err: FetchError) => { if (config.featureToggles.datasourceQueryMultiStatus && err.status === 207) { @@ -362,6 +368,7 @@ export class CloudWatchLogsQueryRunner extends CloudWatchRequest { getLogRowContext = async ( row: LogRowModel, { limit = 10, direction = LogRowContextQueryDirection.Backward }: LogRowContextOptions = {}, + queryFn: (request: DataQueryRequest) => Observable, query?: CloudWatchLogsQuery ): Promise<{ data: DataFrame[] }> => { let logStreamField = null; @@ -396,7 +403,7 @@ export class CloudWatchLogsQueryRunner extends CloudWatchRequest { requestParams.startTime = row.timeEpochMs; } - const dataFrames = await lastValueFrom(this.makeLogActionRequest('GetLogEvents', [requestParams])); + const dataFrames = await lastValueFrom(this.makeLogActionRequest('GetLogEvents', [requestParams], queryFn)); return { data: dataFrames, diff --git a/public/app/plugins/datasource/cloudwatch/query-runner/CloudWatchMetricsQueryRunner.test.ts b/public/app/plugins/datasource/cloudwatch/query-runner/CloudWatchMetricsQueryRunner.test.ts index 7bf94d093b5..fa0790b10f6 100644 --- a/public/app/plugins/datasource/cloudwatch/query-runner/CloudWatchMetricsQueryRunner.test.ts +++ b/public/app/plugins/datasource/cloudwatch/query-runner/CloudWatchMetricsQueryRunner.test.ts @@ -22,7 +22,7 @@ import { MetricQueryType, MetricEditorMode, CloudWatchMetricsQuery, DataQueryErr describe('CloudWatchMetricsQueryRunner', () => { describe('performTimeSeriesQuery', () => { it('should return the same length of data as result', async () => { - const { runner, timeRange, request } = setupMockedMetricsQueryRunner({ + const { runner, timeRange, request, queryMock } = setupMockedMetricsQueryRunner({ data: { results: { a: { refId: 'a', series: [{ target: 'cpu', datapoints: [[1, 1]] }] }, @@ -37,7 +37,7 @@ describe('CloudWatchMetricsQueryRunner', () => { targets: [validMetricSearchCodeQuery, validMetricSearchCodeQuery], range: timeRange, }, - timeRange + queryMock ); await expect(observable).toEmitValuesWith((received) => { @@ -47,7 +47,7 @@ describe('CloudWatchMetricsQueryRunner', () => { }); it('sets fields.config.interval based on period', async () => { - const { runner, timeRange, request } = setupMockedMetricsQueryRunner({ + const { runner, timeRange, request, queryMock } = setupMockedMetricsQueryRunner({ data: { results: { a: { @@ -68,7 +68,7 @@ describe('CloudWatchMetricsQueryRunner', () => { targets: [validMetricSearchCodeQuery, validMetricSearchCodeQuery], range: timeRange, }, - timeRange + queryMock ); await expect(observable).toEmitValuesWith((received) => { @@ -124,7 +124,7 @@ describe('CloudWatchMetricsQueryRunner', () => { it('should generate the correct query', async () => { const { runner, queryMock, request } = setupMockedMetricsQueryRunner({ data }); - await expect(runner.handleMetricQueries(queries, request)).toEmitValuesWith(() => { + await expect(runner.handleMetricQueries(queries, request, queryMock)).toEmitValuesWith(() => { expect(queryMock.mock.calls[0][0].targets).toMatchObject( expect.arrayContaining([ expect.objectContaining({ @@ -163,15 +163,15 @@ describe('CloudWatchMetricsQueryRunner', () => { variables: [periodIntervalVariable], }); - await expect(runner.handleMetricQueries(queries, request)).toEmitValuesWith(() => { + await expect(runner.handleMetricQueries(queries, request, queryMock)).toEmitValuesWith(() => { expect(queryMock.mock.calls[0][0].targets[0].period).toEqual('600'); }); }); it('should return series list', async () => { - const { runner, request } = setupMockedMetricsQueryRunner({ data }); + const { runner, request, queryMock } = setupMockedMetricsQueryRunner({ data }); - await expect(runner.handleMetricQueries(queries, request)).toEmitValuesWith((received) => { + await expect(runner.handleMetricQueries(queries, request, queryMock)).toEmitValuesWith((received) => { const result = received[0]; expect(getFrameDisplayName(result.data[0])).toBe( data.results.A.series?.length && data.results.A.series[0].target @@ -264,10 +264,10 @@ describe('CloudWatchMetricsQueryRunner', () => { }); it('should display one alert error message per region+datasource combination', async () => { - const { runner, request } = setupMockedMetricsQueryRunner({ errorResponse: backendErrorResponse }); + const { runner, request, queryMock } = setupMockedMetricsQueryRunner({ errorResponse: backendErrorResponse }); const memoizedDebounceSpy = jest.spyOn(runner, 'debouncedAlert'); - await expect(runner.handleMetricQueries(queries, request)).toEmitValuesWith(() => { + await expect(runner.handleMetricQueries(queries, request, queryMock)).toEmitValuesWith(() => { expect(memoizedDebounceSpy).toHaveBeenCalledWith('CloudWatch Test Datasource', 'us-east-1'); expect(memoizedDebounceSpy).toHaveBeenCalledWith('CloudWatch Test Datasource', 'us-east-2'); expect(memoizedDebounceSpy).toHaveBeenCalledWith('CloudWatch Test Datasource', 'eu-north-1'); @@ -323,9 +323,9 @@ describe('CloudWatchMetricsQueryRunner', () => { }; it('should return series list', async () => { - const { runner, request } = setupMockedMetricsQueryRunner({ data }); + const { runner, request, queryMock } = setupMockedMetricsQueryRunner({ data }); - await expect(runner.handleMetricQueries(queries, request)).toEmitValuesWith((received) => { + await expect(runner.handleMetricQueries(queries, request, queryMock)).toEmitValuesWith((received) => { const result = received[0]; expect(getFrameDisplayName(result.data[0])).toBe( data.results.A.series?.length && data.results.A.series[0].target @@ -379,7 +379,8 @@ describe('CloudWatchMetricsQueryRunner', () => { sqlExpression: 'SELECT SUM($metric) FROM "$namespace" GROUP BY ${labels:raw} LIMIT $limit', }, ], - request + request, + queryMock ); expect(queryMock).toHaveBeenCalledWith( expect.objectContaining({ @@ -477,7 +478,7 @@ describe('CloudWatchMetricsQueryRunner', () => { period: '300s', }, ]; - await expect(runner.handleMetricQueries(queries, request)).toEmitValuesWith(() => { + await expect(runner.handleMetricQueries(queries, request, queryMock)).toEmitValuesWith(() => { expect(queryMock.mock.calls[0][0].targets[0].dimensions['dim2']).toStrictEqual(['var2-foo']); }); }); @@ -505,13 +506,17 @@ describe('CloudWatchMetricsQueryRunner', () => { ]; await expect( - runner.handleMetricQueries(queries, { - ...request, - scopedVars: { - var1: { value: 'var1-foo', text: '' }, - var2: { value: 'var2-foo', text: '' }, + runner.handleMetricQueries( + queries, + { + ...request, + scopedVars: { + var1: { value: 'var1-foo', text: '' }, + var2: { value: 'var2-foo', text: '' }, + }, }, - }) + queryMock + ) ).toEmitValuesWith(() => { expect(queryMock.mock.calls[0][0].targets[0].dimensions['dim1']).toStrictEqual(['var1-foo']); expect(queryMock.mock.calls[0][0].targets[0].dimensions['dim2']).toStrictEqual(['var2-foo']); @@ -541,7 +546,7 @@ describe('CloudWatchMetricsQueryRunner', () => { }, ]; - await expect(runner.handleMetricQueries(queries, request)).toEmitValuesWith(() => { + await expect(runner.handleMetricQueries(queries, request, queryMock)).toEmitValuesWith(() => { expect(queryMock.mock.calls[0][0].targets[0].dimensions['dim1']).toStrictEqual(['var1-foo']); expect(queryMock.mock.calls[0][0].targets[0].dimensions['dim3']).toStrictEqual(['var3-foo', 'var3-baz']); expect(queryMock.mock.calls[0][0].targets[0].dimensions['dim4']).toStrictEqual(['var4-foo', 'var4-baz']); @@ -571,12 +576,16 @@ describe('CloudWatchMetricsQueryRunner', () => { ]; await expect( - runner.handleMetricQueries(queries, { - ...request, - scopedVars: { - var1: { value: 'var1-foo', text: '' }, + runner.handleMetricQueries( + queries, + { + ...request, + scopedVars: { + var1: { value: 'var1-foo', text: '' }, + }, }, - }) + queryMock + ) ).toEmitValuesWith(() => { expect(queryMock.mock.calls[0][0].targets[0].dimensions['dim1']).toStrictEqual(['var1-foo']); expect(queryMock.mock.calls[0][0].targets[0].dimensions['dim2']).toStrictEqual(['var2-foo']); @@ -618,11 +627,15 @@ describe('CloudWatchMetricsQueryRunner', () => { ]; test.each(testTable)('should use the right time zone offset', (ianaTimezone, expectedOffset) => { const { runner, queryMock, request } = setupMockedMetricsQueryRunner(); - runner.handleMetricQueries([testQuery], { - ...request, - range: { ...request.range, from: dateTime(), to: dateTime() }, - timezone: ianaTimezone, - }); + runner.handleMetricQueries( + [testQuery], + { + ...request, + range: { ...request.range, from: dateTime(), to: dateTime() }, + timezone: ianaTimezone, + }, + queryMock + ); expect(queryMock).toHaveBeenCalledWith( expect.objectContaining({ @@ -639,7 +652,7 @@ describe('CloudWatchMetricsQueryRunner', () => { describe('debouncedCustomAlert', () => { const debouncedAlert = jest.fn(); beforeEach(() => { - const { runner, request } = setupMockedMetricsQueryRunner({ + const { runner, request, queryMock } = setupMockedMetricsQueryRunner({ variables: [ { ...namespaceVariable, multi: true }, { ...metricVariable, multi: true }, @@ -666,7 +679,8 @@ describe('CloudWatchMetricsQueryRunner', () => { metricEditorMode: MetricEditorMode.Code, }, ], - request + request, + queryMock ); }); it('should show debounced alert for namespace and metric name', async () => { @@ -848,7 +862,7 @@ describe('CloudWatchMetricsQueryRunner', () => { }); it('should query for the datasource region if empty or "default"', async () => { - const { runner, instanceSettings, request } = setupMockedMetricsQueryRunner(); + const { runner, instanceSettings, request, queryMock } = setupMockedMetricsQueryRunner(); const performTimeSeriesQueryMock = jest .spyOn(runner, 'performTimeSeriesQuery') .mockReturnValue(of({ data: [], error: undefined })); @@ -871,7 +885,7 @@ describe('CloudWatchMetricsQueryRunner', () => { }, ]; - await expect(runner.handleMetricQueries(queries, request)).toEmitValuesWith(() => { + await expect(runner.handleMetricQueries(queries, request, queryMock)).toEmitValuesWith(() => { expect(performTimeSeriesQueryMock.mock.calls[0][0].targets[0].region).toBe( instanceSettings.jsonData.defaultRegion ); diff --git a/public/app/plugins/datasource/cloudwatch/query-runner/CloudWatchMetricsQueryRunner.ts b/public/app/plugins/datasource/cloudwatch/query-runner/CloudWatchMetricsQueryRunner.ts index eb40fb1d1e6..c6966eadc91 100644 --- a/public/app/plugins/datasource/cloudwatch/query-runner/CloudWatchMetricsQueryRunner.ts +++ b/public/app/plugins/datasource/cloudwatch/query-runner/CloudWatchMetricsQueryRunner.ts @@ -11,7 +11,6 @@ import { FieldType, rangeUtil, ScopedVars, - TimeRange, } from '@grafana/data'; import { notifyApp } from 'app/core/actions'; import { createErrorNotification } from 'app/core/copy/appNotification'; @@ -45,17 +44,14 @@ export class CloudWatchMetricsQueryRunner extends CloudWatchRequest { AppNotificationTimeout.Error ); - constructor( - instanceSettings: DataSourceInstanceSettings, - templateSrv: TemplateSrv, - queryFn: (request: DataQueryRequest) => Observable - ) { - super(instanceSettings, templateSrv, queryFn); + constructor(instanceSettings: DataSourceInstanceSettings, templateSrv: TemplateSrv) { + super(instanceSettings, templateSrv); } handleMetricQueries = ( metricQueries: CloudWatchMetricsQuery[], - options: DataQueryRequest + options: DataQueryRequest, + queryFn: (request: DataQueryRequest) => Observable ): Observable => { const timezoneUTCOffset = dateTimeFormat(Date.now(), { timeZone: options.timezone, @@ -86,7 +82,7 @@ export class CloudWatchMetricsQueryRunner extends CloudWatchRequest { targets: validMetricsQueries, }; - return this.performTimeSeriesQuery(request, options.range); + return this.performTimeSeriesQuery(request, queryFn); }; interpolateMetricsQueryVariables( @@ -109,9 +105,9 @@ export class CloudWatchMetricsQueryRunner extends CloudWatchRequest { performTimeSeriesQuery( request: DataQueryRequest, - { from, to }: TimeRange + queryFn: (request: DataQueryRequest) => Observable ): Observable { - return this.query(request).pipe( + return queryFn(request).pipe( map((res) => { const dataframes: DataFrame[] = res.data; if (!dataframes || dataframes.length <= 0) { diff --git a/public/app/plugins/datasource/cloudwatch/query-runner/CloudWatchRequest.ts b/public/app/plugins/datasource/cloudwatch/query-runner/CloudWatchRequest.ts index 63595dcdb92..8570f18393d 100644 --- a/public/app/plugins/datasource/cloudwatch/query-runner/CloudWatchRequest.ts +++ b/public/app/plugins/datasource/cloudwatch/query-runner/CloudWatchRequest.ts @@ -1,13 +1,6 @@ -import { Observable, of } from 'rxjs'; +import { Observable } from 'rxjs'; -import { - DataQueryRequest, - DataQueryResponse, - DataSourceInstanceSettings, - DataSourceRef, - getDataSourceRef, - ScopedVars, -} from '@grafana/data'; +import { DataSourceInstanceSettings, DataSourceRef, getDataSourceRef, ScopedVars } from '@grafana/data'; import { BackendDataSourceResponse, FetchResponse, getBackendSrv } from '@grafana/runtime'; import { notifyApp } from 'app/core/actions'; import { createErrorNotification } from 'app/core/copy/appNotification'; @@ -16,13 +9,12 @@ import { store } from 'app/store/store'; import { AppNotificationTimeout } from 'app/types'; import memoizedDebounce from '../memoizedDebounce'; -import { CloudWatchJsonData, CloudWatchQuery, Dimensions, MetricRequest, MultiFilters } from '../types'; +import { CloudWatchJsonData, Dimensions, MetricRequest, MultiFilters } from '../types'; export abstract class CloudWatchRequest { templateSrv: TemplateSrv; ref: DataSourceRef; dsQueryEndpoint = '/api/ds/query'; - query: (request: DataQueryRequest) => Observable; debouncedCustomAlert: (title: string, message: string) => void = memoizedDebounce( displayCustomError, AppNotificationTimeout.Error @@ -30,12 +22,10 @@ export abstract class CloudWatchRequest { constructor( public instanceSettings: DataSourceInstanceSettings, - templateSrv: TemplateSrv, - queryFn: (request: DataQueryRequest) => Observable = () => of({ data: [] }) + templateSrv: TemplateSrv ) { this.templateSrv = templateSrv; this.ref = getDataSourceRef(instanceSettings); - this.query = queryFn; } awsRequest(