Skip to content

Commit

Permalink
Annotations: Fixes blank panels for queries with unknown data sources (
Browse files Browse the repository at this point in the history
…#39017)

* Annotations: Fixes blank panels for queries with unknown data sources

* Chore: fixes strict typescript error
  • Loading branch information
hugohaggmark committed Sep 9, 2021
1 parent 1133e56 commit 419ead9
Show file tree
Hide file tree
Showing 8 changed files with 63 additions and 14 deletions.
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
4 changes: 2 additions & 2 deletions public/app/features/query/state/DashboardQueryRunner/types.ts
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

0 comments on commit 419ead9

Please sign in to comment.