Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[v8.1.x] Annotations: Fixes blank panels for queries with unknown data sources #39034

Merged
merged 1 commit into from
Sep 9, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,14 @@ describe('AnnotationsQueryRunner', () => {
});
});

describe('when canWork is called without datasource', () => {
it('then it should return false', () => {
const datasource: any = undefined;

expect(runner.canRun(datasource)).toBe(false);
});
});

describe('when canWork is called with incorrect props', () => {
it('then it should return false', () => {
const datasource: any = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,11 @@ import { executeAnnotationQuery } from '../../../annotations/annotations_srv';
import { handleAnnotationQueryRunnerError } from './utils';

export class AnnotationsQueryRunner implements AnnotationQueryRunner {
canRun(datasource: DataSourceApi): boolean {
canRun(datasource?: DataSourceApi): boolean {
if (!datasource) {
return false;
}

return !Boolean(datasource.annotationQuery && !datasource.annotations);
}

Expand All @@ -19,7 +23,7 @@ export class AnnotationsQueryRunner implements AnnotationQueryRunner {

const panel: PanelModel = ({} as unknown) as PanelModel; // deliberate setting panel to empty object because executeAnnotationQuery shouldn't depend on panelModel

return executeAnnotationQuery({ dashboard, range, panel }, datasource, annotation).pipe(
return executeAnnotationQuery({ dashboard, range, panel }, datasource!, annotation).pipe(
map((result) => {
return result.events ?? [];
}),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { DashboardQueryRunnerOptions, DashboardQueryRunnerWorkerResult } from '.
import { AnnotationQuery } from '@grafana/data';
import { delay } from 'rxjs/operators';

function getTestContext() {
function getTestContext(dataSourceSrvRejects = false) {
jest.clearAllMocks();
const cancellations = new Subject<AnnotationQuery>();
setDashboardQueryRunnerFactory(() => ({
Expand All @@ -28,6 +28,9 @@ function getTestContext() {
const annotationQueryMock = jest.fn().mockResolvedValue([{ id: 'Legacy' }]);
const dataSourceSrvMock: any = {
get: async (name: string) => {
if (dataSourceSrvRejects) {
return Promise.reject(`Could not find datasource with name: ${name}`);
}
if (name === LEGACY_DS_NAME) {
return {
annotationQuery: annotationQueryMock,
Expand Down Expand Up @@ -272,4 +275,19 @@ describe('AnnotationsWorker', () => {
});
});
});

describe('when run is called with correct props and call to datasourceSrv fails', () => {
silenceConsoleOutput();
it('then it should return the correct results', async () => {
const { options, executeAnnotationQueryMock, annotationQueryMock } = getTestContext(true);

await expect(worker.work(options)).toEmitValuesWith((received) => {
expect(received).toHaveLength(1);
const result = received[0];
expect(result).toEqual({ alertStates: [], annotations: [] });
expect(executeAnnotationQueryMock).not.toHaveBeenCalled();
expect(annotationQueryMock).not.toHaveBeenCalled();
});
});
});
});
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { cloneDeep } from 'lodash';
import { from, merge, Observable, of } from 'rxjs';
import { filter, finalize, map, mergeAll, mergeMap, reduce, takeUntil } from 'rxjs/operators';
import { catchError, filter, finalize, map, mergeAll, mergeMap, reduce, takeUntil } from 'rxjs/operators';
import { getDataSourceSrv } from '@grafana/runtime';
import { AnnotationQuery, DataSourceApi } from '@grafana/data';

Expand All @@ -10,7 +10,7 @@ import {
DashboardQueryRunnerWorker,
DashboardQueryRunnerWorkerResult,
} from './types';
import { emptyResult, translateQueryResult } from './utils';
import { emptyResult, handleDatasourceSrvError, translateQueryResult } from './utils';
import { LegacyAnnotationQueryRunner } from './LegacyAnnotationQueryRunner';
import { AnnotationsQueryRunner } from './AnnotationsQueryRunner';
import { AnnotationQueryFinished, AnnotationQueryStarted } from '../../../../types/events';
Expand All @@ -37,9 +37,11 @@ export class AnnotationsWorker implements DashboardQueryRunnerWorker {
const { dashboard, range } = options;
const annotations = dashboard.annotations.list.filter(AnnotationsWorker.getAnnotationsToProcessFilter);
const observables = annotations.map((annotation) => {
const datasourcePromise = getDataSourceSrv().get(annotation.datasource);
return from(datasourcePromise).pipe(
mergeMap((datasource: DataSourceApi) => {
const datasourceObservable = from(getDataSourceSrv().get(annotation.datasource)).pipe(
catchError(handleDatasourceSrvError) // because of the reduce all observables need to be completed, so an erroneous observable wont do
);
return datasourceObservable.pipe(
mergeMap((datasource?: DataSourceApi) => {
const runner = this.runners.find((r) => r.canRun(datasource));
if (!runner) {
return of([]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ function getTestContext(annotationQuery?: jest.Mock) {
jest.clearAllMocks();
const dispatchMock = jest.spyOn(store, 'dispatch');
const options = getDefaultOptions(annotationQuery);
const annotationQueryMock = options.datasource.annotationQuery;
const annotationQueryMock = options.datasource!.annotationQuery;

return { options, dispatchMock, annotationQueryMock };
}
Expand All @@ -38,6 +38,14 @@ describe('LegacyAnnotationQueryRunner', () => {
});
});

describe('when canWork is called without datasource', () => {
it('then it should return false', () => {
const datasource: any = undefined;

expect(runner.canRun(datasource)).toBe(false);
});
});

describe('when canWork is called with incorrect props', () => {
it('then it should return false', () => {
const datasource: any = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,11 @@ import { AnnotationQueryRunner, AnnotationQueryRunnerOptions } from './types';
import { handleAnnotationQueryRunnerError } from './utils';

export class LegacyAnnotationQueryRunner implements AnnotationQueryRunner {
canRun(datasource: DataSourceApi): boolean {
canRun(datasource?: DataSourceApi): boolean {
if (!datasource) {
return false;
}

return Boolean(datasource.annotationQuery && !datasource.annotations);
}

Expand All @@ -15,7 +19,7 @@ export class LegacyAnnotationQueryRunner implements AnnotationQueryRunner {
return of([]);
}

return from(datasource.annotationQuery!({ range, rangeRaw: range.raw, annotation, dashboard })).pipe(
return from(datasource!.annotationQuery!({ range, rangeRaw: range.raw, annotation, dashboard })).pipe(
catchError(handleAnnotationQueryRunnerError)
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,11 @@ export interface DashboardQueryRunnerWorker {
}

export interface AnnotationQueryRunnerOptions extends DashboardQueryRunnerOptions {
datasource: DataSourceApi;
datasource?: DataSourceApi;
annotation: AnnotationQuery;
}

export interface AnnotationQueryRunner {
canRun: (datasource: DataSourceApi) => boolean;
canRun: (datasource?: DataSourceApi) => boolean;
run: (options: AnnotationQueryRunnerOptions) => Observable<AnnotationEvent[]>;
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { cloneDeep } from 'lodash';
import { Observable, of } from 'rxjs';
import { AnnotationEvent, AnnotationQuery, DataFrame, DataFrameView } from '@grafana/data';
import { AnnotationEvent, AnnotationQuery, DataFrame, DataFrameView, DataSourceApi } from '@grafana/data';
import { config, toDataQueryError } from '@grafana/runtime';

import { dispatch } from 'app/store/store';
Expand All @@ -17,6 +17,11 @@ export function handleAnnotationQueryRunnerError(err: any): Observable<Annotatio
return of([]);
}

export function handleDatasourceSrvError(err: any): Observable<DataSourceApi | undefined> {
notifyWithError('Failed to retrieve datasource', err);
return of(undefined);
}

export const emptyResult: () => Observable<DashboardQueryRunnerWorkerResult> = () =>
of({ annotations: [], alertStates: [] });

Expand Down