From 5f474c632820c877b188a5527e19e427f03213f5 Mon Sep 17 00:00:00 2001 From: Ryan McKinley Date: Thu, 18 Apr 2019 14:10:18 -0700 Subject: [PATCH] Refactor: move getQueryRunner() to PanelModel (#16679) * move queryRunner to panelModel * Added interval back as prop, not used yet * PanelQueryRunner: Refactoring, added getQueryRunner to PanelModel * PanelQueryRunner: interpolatel min interval --- .../dashboard/dashgrid/PanelChrome.tsx | 17 +-- .../dashboard/panel_editor/QueriesTab.tsx | 9 +- .../features/dashboard/state/PanelModel.ts | 9 +- .../dashboard/state/PanelQueryRunner.test.ts | 136 +++++++++++++++++- .../dashboard/state/PanelQueryRunner.ts | 34 +++-- .../panel/specs/metrics_panel_ctrl.test.ts | 4 +- 6 files changed, 175 insertions(+), 34 deletions(-) diff --git a/public/app/features/dashboard/dashgrid/PanelChrome.tsx b/public/app/features/dashboard/dashgrid/PanelChrome.tsx index 93a5ffc0c5e..d4f6c622948 100644 --- a/public/app/features/dashboard/dashgrid/PanelChrome.tsx +++ b/public/app/features/dashboard/dashgrid/PanelChrome.tsx @@ -22,7 +22,7 @@ import { ScopedVars } from '@grafana/ui'; import templateSrv from 'app/features/templating/template_srv'; -import { PanelQueryRunner, getProcessedSeriesData } from '../state/PanelQueryRunner'; +import { getProcessedSeriesData } from '../state/PanelQueryRunner'; import { Unsubscribable } from 'rxjs'; const DEFAULT_PLUGIN_ERROR = 'Error in plugin'; @@ -134,16 +134,17 @@ export class PanelChrome extends PureComponent { // Issue Query if (this.wantsQueryExecution) { if (width < 0) { - console.log('No width yet... wait till we know'); + console.log('Refresh skippted, no width yet... wait till we know'); return; } - if (!panel.queryRunner) { - panel.queryRunner = new PanelQueryRunner(); - } + + const queryRunner = panel.getQueryRunner(); + if (!this.querySubscription) { - this.querySubscription = panel.queryRunner.subscribe(this.panelDataObserver); + this.querySubscription = queryRunner.subscribe(this.panelDataObserver); } - panel.queryRunner.run({ + + queryRunner.run({ datasource: panel.datasource, queries: panel.targets, panelId: panel.id, @@ -152,8 +153,8 @@ export class PanelChrome extends PureComponent { timeRange: timeData.timeRange, timeInfo: timeData.timeInfo, widthPixels: width, - minInterval: undefined, // Currently not passed in DataPanel? maxDataPoints: panel.maxDataPoints, + minInterval: panel.interval, scopedVars: panel.scopedVars, cacheTimeout: panel.cacheTimeout, }); diff --git a/public/app/features/dashboard/panel_editor/QueriesTab.tsx b/public/app/features/dashboard/panel_editor/QueriesTab.tsx index 3cfc0bb1d07..75f4ded618e 100644 --- a/public/app/features/dashboard/panel_editor/QueriesTab.tsx +++ b/public/app/features/dashboard/panel_editor/QueriesTab.tsx @@ -20,7 +20,7 @@ import { PanelModel } from '../state/PanelModel'; import { DashboardModel } from '../state/DashboardModel'; import { DataQuery, DataSourceSelectItem, PanelData, LoadingState } from '@grafana/ui/src/types'; import { PluginHelp } from 'app/core/components/PluginHelp/PluginHelp'; -import { PanelQueryRunner, PanelQueryRunnerFormat } from '../state/PanelQueryRunner'; +import { PanelQueryRunnerFormat } from '../state/PanelQueryRunner'; import { Unsubscribable } from 'rxjs'; interface Props { @@ -58,12 +58,9 @@ export class QueriesTab extends PureComponent { componentDidMount() { const { panel } = this.props; + const queryRunner = panel.getQueryRunner(); - if (!panel.queryRunner) { - panel.queryRunner = new PanelQueryRunner(); - } - - this.querySubscription = panel.queryRunner.subscribe(this.panelDataObserver, PanelQueryRunnerFormat.both); + this.querySubscription = queryRunner.subscribe(this.panelDataObserver, PanelQueryRunnerFormat.both); } componentWillUnmount() { diff --git a/public/app/features/dashboard/state/PanelModel.ts b/public/app/features/dashboard/state/PanelModel.ts index aa0a43dddfa..7492e5f32f5 100644 --- a/public/app/features/dashboard/state/PanelModel.ts +++ b/public/app/features/dashboard/state/PanelModel.ts @@ -118,7 +118,7 @@ export class PanelModel { cachedPluginOptions?: any; legend?: { show: boolean }; plugin?: PanelPlugin; - queryRunner?: PanelQueryRunner; + private queryRunner?: PanelQueryRunner; constructor(model: any) { this.events = new Emitter(); @@ -326,6 +326,13 @@ export class PanelModel { }); } + getQueryRunner(): PanelQueryRunner { + if (!this.queryRunner) { + this.queryRunner = new PanelQueryRunner(); + } + return this.queryRunner; + } + destroy() { this.events.emit('panel-teardown'); this.events.removeAllListeners(); diff --git a/public/app/features/dashboard/state/PanelQueryRunner.test.ts b/public/app/features/dashboard/state/PanelQueryRunner.test.ts index 511f9978b93..6e341ce5da1 100644 --- a/public/app/features/dashboard/state/PanelQueryRunner.test.ts +++ b/public/app/features/dashboard/state/PanelQueryRunner.test.ts @@ -1,6 +1,8 @@ -import { getProcessedSeriesData } from './PanelQueryRunner'; +import { getProcessedSeriesData, PanelQueryRunner } from './PanelQueryRunner'; +import { PanelData, DataQueryOptions } from '@grafana/ui/src/types'; +import moment from 'moment'; -describe('QueryRunner', () => { +describe('PanelQueryRunner', () => { it('converts timeseries to table skipping nulls', () => { const input1 = { target: 'Field Name', @@ -35,3 +37,133 @@ describe('QueryRunner', () => { expect(getProcessedSeriesData([])).toEqual([]); }); }); + +interface ScenarioContext { + setup: (fn: () => void) => void; + maxDataPoints?: number | null; + widthPixels: number; + dsInterval?: string; + minInterval?: string; + events?: PanelData[]; + res?: PanelData; + queryCalledWith?: DataQueryOptions; +} + +type ScenarioFn = (ctx: ScenarioContext) => void; + +function describeQueryRunnerScenario(description: string, scenarioFn: ScenarioFn) { + describe(description, () => { + let setupFn = () => {}; + + const ctx: ScenarioContext = { + widthPixels: 200, + setup: (fn: () => void) => { + setupFn = fn; + }, + }; + + let runner: PanelQueryRunner; + const response: any = { + data: [{ target: 'hello', datapoints: [] }], + }; + + beforeEach(async () => { + setupFn(); + + const ds: any = { + interval: ctx.dsInterval, + query: (options: DataQueryOptions) => { + ctx.queryCalledWith = options; + return Promise.resolve(response); + }, + testDatasource: jest.fn(), + }; + + const args: any = { + ds: ds as any, + datasource: '', + minInterval: ctx.minInterval, + widthPixels: ctx.widthPixels, + maxDataPoints: ctx.maxDataPoints, + timeRange: { + from: moment().subtract(1, 'days'), + to: moment(), + raw: { from: '1h', to: 'now' }, + }, + panelId: 0, + queries: [{ refId: 'A', test: 1 }], + }; + + runner = new PanelQueryRunner(); + runner.subscribe({ + next: (data: PanelData) => { + ctx.events.push(data); + }, + }); + + ctx.events = []; + ctx.res = await runner.run(args); + }); + + scenarioFn(ctx); + }); +} + +describe('PanelQueryRunner', () => { + describeQueryRunnerScenario('with no maxDataPoints or minInterval', ctx => { + ctx.setup(() => { + ctx.maxDataPoints = null; + ctx.widthPixels = 200; + }); + + it('should return data', async () => { + expect(ctx.res.error).toBeUndefined(); + expect(ctx.res.series.length).toBe(1); + }); + + it('should use widthPixels as maxDataPoints', async () => { + expect(ctx.queryCalledWith.maxDataPoints).toBe(200); + }); + + it('should calculate interval based on width', async () => { + expect(ctx.queryCalledWith.interval).toBe('5m'); + }); + + it('fast query should only publish 1 data events', async () => { + expect(ctx.events.length).toBe(1); + }); + }); + + describeQueryRunnerScenario('with no panel min interval but datasource min interval', ctx => { + ctx.setup(() => { + ctx.widthPixels = 20000; + ctx.dsInterval = '15s'; + }); + + it('should limit interval to data source min interval', async () => { + expect(ctx.queryCalledWith.interval).toBe('15s'); + }); + }); + + describeQueryRunnerScenario('with panel min interval and data source min interval', ctx => { + ctx.setup(() => { + ctx.widthPixels = 20000; + ctx.dsInterval = '15s'; + ctx.minInterval = '30s'; + }); + + it('should limit interval to panel min interval', async () => { + expect(ctx.queryCalledWith.interval).toBe('30s'); + }); + }); + + describeQueryRunnerScenario('with maxDataPoints', ctx => { + ctx.setup(() => { + ctx.maxDataPoints = 10; + }); + + it('should pass maxDataPoints if specified', async () => { + expect(ctx.queryCalledWith.maxDataPoints).toBe(10); + }); + }); +}); diff --git a/public/app/features/dashboard/state/PanelQueryRunner.ts b/public/app/features/dashboard/state/PanelQueryRunner.ts index 02ff5b9259e..c5240cb1005 100644 --- a/public/app/features/dashboard/state/PanelQueryRunner.ts +++ b/public/app/features/dashboard/state/PanelQueryRunner.ts @@ -1,5 +1,13 @@ -import { getDatasourceSrv } from 'app/features/plugins/datasource_srv'; +// Libraries +import cloneDeep from 'lodash/cloneDeep'; import { Subject, Unsubscribable, PartialObserver } from 'rxjs'; + +// Services & Utils +import { getDatasourceSrv } from 'app/features/plugins/datasource_srv'; +import kbn from 'app/core/utils/kbn'; +import templateSrv from 'app/features/templating/template_srv'; + +// Components & Types import { guessFieldTypes, toSeriesData, @@ -16,10 +24,6 @@ import { DataSourceApi, } from '@grafana/ui'; -import cloneDeep from 'lodash/cloneDeep'; - -import kbn from 'app/core/utils/kbn'; - export interface QueryRunnerOptions { ds?: DataSourceApi; // if they already have the datasource, don't look it up datasource: string | null; @@ -27,11 +31,11 @@ export interface QueryRunnerOptions { panelId: number; dashboardId?: number; timezone?: string; - timeRange?: TimeRange; + timeRange: TimeRange; timeInfo?: string; // String description of time range for display widthPixels: number; - minInterval?: string; - maxDataPoints?: number; + maxDataPoints: number | undefined | null; + minInterval: string | undefined | null; scopedVars?: ScopedVars; cacheTimeout?: string; delayStateNotification?: number; // default 100ms. @@ -98,6 +102,7 @@ export class PanelQueryRunner { widthPixels, maxDataPoints, scopedVars, + minInterval, delayStateNotification, } = options; @@ -118,14 +123,12 @@ export class PanelQueryRunner { }; if (!queries) { - this.data = { + return this.publishUpdate({ state: LoadingState.Done, series: [], // Clear the data legacy: [], request, - }; - this.subject.next(this.data); - return this.data; + }); } let loadingStateTimeoutId = 0; @@ -133,8 +136,8 @@ export class PanelQueryRunner { try { const ds = options.ds ? options.ds : await getDatasourceSrv().get(datasource, request.scopedVars); - const minInterval = options.minInterval || ds.interval; - const norm = kbn.calculateInterval(timeRange, widthPixels, minInterval); + const lowerIntervalLimit = minInterval ? templateSrv.replace(minInterval, request.scopedVars) : ds.interval; + const norm = kbn.calculateInterval(timeRange, widthPixels, lowerIntervalLimit); // make shallow copy of scoped vars, // and add built in variables interval and interval_ms @@ -142,6 +145,7 @@ export class PanelQueryRunner { __interval: { text: norm.interval, value: norm.interval }, __interval_ms: { text: norm.intervalMs, value: norm.intervalMs }, }); + request.interval = norm.interval; request.intervalMs = norm.intervalMs; @@ -151,7 +155,6 @@ export class PanelQueryRunner { }, delayStateNotification || 500); const resp = await ds.query(request); - request.endTime = Date.now(); // Make sure the response is in a supported format @@ -229,5 +232,6 @@ export function getProcessedSeriesData(results?: any[]): SeriesData[] { series.push(guessFieldTypes(toSeriesData(r))); } } + return series; } diff --git a/public/app/features/panel/specs/metrics_panel_ctrl.test.ts b/public/app/features/panel/specs/metrics_panel_ctrl.test.ts index 3ee4c5165cb..0590cb7adce 100644 --- a/public/app/features/panel/specs/metrics_panel_ctrl.test.ts +++ b/public/app/features/panel/specs/metrics_panel_ctrl.test.ts @@ -30,7 +30,7 @@ describe('MetricsPanelCtrl', () => { describe('and has datasource set that supports explore and user does not have access to explore', () => { it('should not return any items', () => { const ctrl = setupController({ hasAccessToExplore: false }); - ctrl.datasource = { meta: { explore: true } }; + ctrl.datasource = { meta: { explore: true } } as any; expect(ctrl.getAdditionalMenuItems().length).toBe(0); }); @@ -39,7 +39,7 @@ describe('MetricsPanelCtrl', () => { describe('and has datasource set that supports explore and user has access to explore', () => { it('should return one item', () => { const ctrl = setupController({ hasAccessToExplore: true }); - ctrl.datasource = { meta: { explore: true } }; + ctrl.datasource = { meta: { explore: true } } as any; expect(ctrl.getAdditionalMenuItems().length).toBe(1); });