From fffff70d466b5f613d55e3481f2e55071a4b370d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Torkel=20=C3=96degaard?= Date: Thu, 13 May 2021 10:06:54 +0200 Subject: [PATCH] MinMax: Highlight issues with skipping field overrides (#33672) * MinMax: Highlight issues with skipping field overrides * check global range * more selective * basic test * moved into getFieldDisplayValues Co-authored-by: Ryan McKinley --- .../src/field/fieldDisplay.test.ts | 28 +++++++++++++ .../grafana-data/src/field/fieldDisplay.ts | 6 +++ packages/grafana-data/src/field/scale.test.ts | 26 +++++++++++- packages/grafana-data/src/field/scale.ts | 41 ++++++++++++++++++- .../features/query/state/PanelQueryRunner.ts | 7 +++- .../plugins/panel/bargauge/BarGaugePanel.tsx | 2 + public/app/plugins/panel/gauge/GaugePanel.tsx | 1 + public/app/plugins/panel/stat/StatPanel.tsx | 1 + 8 files changed, 109 insertions(+), 3 deletions(-) diff --git a/packages/grafana-data/src/field/fieldDisplay.test.ts b/packages/grafana-data/src/field/fieldDisplay.test.ts index 373f587d429..f66c47bb71b 100644 --- a/packages/grafana-data/src/field/fieldDisplay.test.ts +++ b/packages/grafana-data/src/field/fieldDisplay.test.ts @@ -72,6 +72,34 @@ describe('FieldDisplay', () => { expect(display.map((v) => v.display.numeric)).toEqual([1, 3]); // First 2 are from the first field }); + it('should not calculate min max if ensureGlobalRange is false', () => { + const options = createDisplayOptions({ + ensureGlobalRange: false, + reduceOptions: { + values: true, // + limit: 1000, + calcs: [], + }, + }); + const display = getFieldDisplayValues(options); + expect(display[0].field.min).toBeUndefined(); + expect(display[0].field.max).toBeUndefined(); + }); + + it('should ensure global min / max on numerical fields', () => { + const options = createDisplayOptions({ + ensureGlobalRange: true, + reduceOptions: { + values: true, // + limit: 1000, + calcs: [], + }, + }); + const display = getFieldDisplayValues(options); + expect(display[0].field.min).toEqual(1); + expect(display[0].field.max).toEqual(6); + }); + it('Should return field thresholds when there is no data', () => { const options = createEmptyDisplayOptions({ fieldConfig: { diff --git a/packages/grafana-data/src/field/fieldDisplay.ts b/packages/grafana-data/src/field/fieldDisplay.ts index 9f0b2bb4e25..1fea8363324 100644 --- a/packages/grafana-data/src/field/fieldDisplay.ts +++ b/packages/grafana-data/src/field/fieldDisplay.ts @@ -22,6 +22,7 @@ import { getTimeField } from '../dataframe/processDataFrame'; import { getFieldMatcher } from '../transformations'; import { FieldMatcherID } from '../transformations/matchers/ids'; import { getFieldDisplayName } from './fieldState'; +import { ensureGlobalRangeOnState } from './scale'; /** * Options for how to turn DataFrames into an array of display values @@ -73,6 +74,7 @@ export interface GetFieldDisplayValuesOptions { sparkline?: boolean; // Calculate the sparkline theme: GrafanaTheme2; timeZone?: TimeZone; + ensureGlobalRange?: boolean; } export const DEFAULT_FIELD_DISPLAY_VALUES_LIMIT = 25; @@ -99,6 +101,10 @@ export const getFieldDisplayValues = (options: GetFieldDisplayValuesOptions): Fi let hitLimit = false; + if (options.ensureGlobalRange) { + ensureGlobalRangeOnState(data); + } + for (let s = 0; s < data.length && !hitLimit; s++) { const dataFrame = data[s]; // Name is already set diff --git a/packages/grafana-data/src/field/scale.test.ts b/packages/grafana-data/src/field/scale.test.ts index 2e6924ffb8b..45cd98afd03 100644 --- a/packages/grafana-data/src/field/scale.test.ts +++ b/packages/grafana-data/src/field/scale.test.ts @@ -1,9 +1,10 @@ import { ThresholdsMode, Field, FieldType } from '../types'; import { sortThresholds } from './thresholds'; import { ArrayVector } from '../vector/ArrayVector'; -import { getScaleCalculator } from './scale'; +import { ensureGlobalRangeOnState, getScaleCalculator } from './scale'; import { createTheme } from '../themes'; import { getColorForTheme } from '../utils'; +import { toDataFrame } from '../dataframe'; describe('getScaleCalculator', () => { it('should return percent, threshold and color', () => { @@ -50,3 +51,26 @@ describe('getScaleCalculator', () => { }); }); }); + +describe('ensure global scales', () => { + it('should fill in all numeric values', () => { + const frame = toDataFrame({ + fields: [ + { type: FieldType.number, values: [1, 2, 3] }, + { type: FieldType.number, values: [7, 8, 9] }, + { type: FieldType.string, values: ['a', 'b', 'c'] }, + ], + }); + ensureGlobalRangeOnState([frame]); + + expect(frame.fields[0].state!.range).toMatchInlineSnapshot(` + Object { + "delta": 8, + "max": 9, + "min": 1, + } + `); + + expect(frame.fields[2].state?.range).toBeUndefined(); + }); +}); diff --git a/packages/grafana-data/src/field/scale.ts b/packages/grafana-data/src/field/scale.ts index c30fda912b5..cf94162c3ca 100644 --- a/packages/grafana-data/src/field/scale.ts +++ b/packages/grafana-data/src/field/scale.ts @@ -1,8 +1,9 @@ import { isNumber } from 'lodash'; import { GrafanaTheme2 } from '../themes/types'; import { reduceField, ReducerID } from '../transformations/fieldReducer'; -import { Field, FieldConfig, FieldType, NumericRange, Threshold } from '../types'; +import { DataFrame, Field, FieldConfig, FieldType, NumericRange, Threshold } from '../types'; import { getFieldColorModeForField } from './fieldColor'; +import { findNumericFieldMinMax } from './fieldOverrides'; import { getActiveThresholdForValue } from './thresholds'; export interface ColorScaleValue { @@ -112,3 +113,41 @@ export function getFieldConfigWithMinMax(field: Field, local?: boolean): FieldCo return { ...config, ...field.state.range }; } + +/** + * This will check that each field has a range value stored on state + * If the value is missing, the global range will be calculated and + * saved in the field state. + * + * The same process usually happens in `applyFieldOverrieds`, but + * when the process can be skipped the global range may be missing + * + * @internal + */ +export function ensureGlobalRangeOnState(frames?: DataFrame[]) { + if (!frames) { + return; + } + + let globalRange: NumericRange | undefined = undefined; + for (const frame of frames) { + for (const field of frame.fields) { + if (field.type === FieldType.number) { + if (field.state?.range) { + continue; // already set + } + const { config } = field; + if (!globalRange && (config.min == null || config.max == null)) { + globalRange = findNumericFieldMinMax(frames); + } + + const min = config.min ?? globalRange!.min; + const max = config.max ?? globalRange!.max; + if (!field.state) { + field.state = {}; + } + field.state.range = { min, max, delta: max! - min! }; + } + } + } +} diff --git a/public/app/features/query/state/PanelQueryRunner.ts b/public/app/features/query/state/PanelQueryRunner.ts index f22d287a6d6..b358650422e 100644 --- a/public/app/features/query/state/PanelQueryRunner.ts +++ b/public/app/features/query/state/PanelQueryRunner.ts @@ -114,7 +114,12 @@ export class PanelQueryRunner { fields: frame.fields.map((field, fieldIndex) => ({ ...field, values: data.series[frameIndex].fields[fieldIndex].values, - state: {}, + state: { + ...field.state, + calcs: undefined, + // add global range calculation here? (not optimal for streaming) + range: undefined, + }, })), })), }; diff --git a/public/app/plugins/panel/bargauge/BarGaugePanel.tsx b/public/app/plugins/panel/bargauge/BarGaugePanel.tsx index 2a0e2920869..6f203e7b951 100644 --- a/public/app/plugins/panel/bargauge/BarGaugePanel.tsx +++ b/public/app/plugins/panel/bargauge/BarGaugePanel.tsx @@ -69,9 +69,11 @@ export class BarGaugePanel extends PureComponent> { getValues = (): FieldDisplay[] => { const { data, options, replaceVariables, fieldConfig, timeZone } = this.props; + return getFieldDisplayValues({ fieldConfig, reduceOptions: options.reduceOptions, + ensureGlobalRange: true, replaceVariables, theme: config.theme2, data: data.series, diff --git a/public/app/plugins/panel/gauge/GaugePanel.tsx b/public/app/plugins/panel/gauge/GaugePanel.tsx index 54a09983a3c..9877756ebab 100644 --- a/public/app/plugins/panel/gauge/GaugePanel.tsx +++ b/public/app/plugins/panel/gauge/GaugePanel.tsx @@ -54,6 +54,7 @@ export class GaugePanel extends PureComponent> { const { data, options, replaceVariables, fieldConfig, timeZone } = this.props; return getFieldDisplayValues({ fieldConfig, + ensureGlobalRange: true, reduceOptions: options.reduceOptions, replaceVariables, theme: config.theme2, diff --git a/public/app/plugins/panel/stat/StatPanel.tsx b/public/app/plugins/panel/stat/StatPanel.tsx index 771a054397a..c33c563b82d 100644 --- a/public/app/plugins/panel/stat/StatPanel.tsx +++ b/public/app/plugins/panel/stat/StatPanel.tsx @@ -85,6 +85,7 @@ export class StatPanel extends PureComponent> { return getFieldDisplayValues({ fieldConfig, + ensureGlobalRange: true, reduceOptions: options.reduceOptions, replaceVariables, theme: config.theme2,