From 000c00aee9dca0bc1100a41f1955a4e6544de558 Mon Sep 17 00:00:00 2001 From: Paul Marbach Date: Wed, 10 Dec 2025 16:54:29 -0500 Subject: [PATCH] Sparkline: Improve min/max logic to avoid issues for very narrow deltas (#115030) * Sparkline: Prevent infinite loop when rendering a sparkline with a single value * some tests for this case * refactor out utils, experiment with getting highlightIndex working * add comments throughout for #112977 * remove unused import * Update Sparkline.test.tsx * fix points mode rendering * Sparkline: Improve min/max logic to avoid issues for very narrow deltas * spread all config * defaults deep * delete unused import * remove go.work.sum delta * line break at end of file --- .../src/field/fieldOverrides.test.ts | 37 +++++++++++- .../grafana-data/src/field/fieldOverrides.ts | 15 +++-- .../src/components/Sparkline/utils.test.ts | 18 +++++- .../src/components/Sparkline/utils.ts | 56 ++++++++++++------- 4 files changed, 96 insertions(+), 30 deletions(-) diff --git a/packages/grafana-data/src/field/fieldOverrides.test.ts b/packages/grafana-data/src/field/fieldOverrides.test.ts index 901fc6d8cc2..767da439543 100644 --- a/packages/grafana-data/src/field/fieldOverrides.test.ts +++ b/packages/grafana-data/src/field/fieldOverrides.test.ts @@ -253,7 +253,7 @@ describe('applyFieldOverrides', () => { ], }); - it('will apply field overrides to the fields within the frame', () => { + it('will apply default field overrides to the fields within the frame', () => { const f0 = createDataFrame({ name: 'A', fields: [ @@ -285,6 +285,41 @@ describe('applyFieldOverrides', () => { expect(withOverrides[0].fields[1].values[0].fields[1].state.range.max).toBe(30); }); + it('will apply targeted field overrides to the fields within the frame', () => { + const f0 = createDataFrame({ + name: 'A', + fields: [ + { + name: 'message', + type: FieldType.string, + values: ['foo'], + }, + { + name: 'frame', + type: FieldType.frame, + values: [f0Internal], + }, + ], + }); + const withOverrides = applyFieldOverrides({ + data: [f0], + fieldConfig: { + defaults: {}, + overrides: [ + { + matcher: { id: FieldMatcherID.byName, options: 'frame' }, + properties: [{ id: 'max', value: 30 }], + }, + ], + }, + replaceVariables: (value) => value, + theme: createTheme(), + fieldConfigRegistry: customFieldRegistry, + }); + + expect(withOverrides[0].fields[1].values[0].fields[1].config.max).toBe(30); + }); + it('will not crash when some of the nested frames are undefined', () => { const f0 = createDataFrame({ name: 'A', diff --git a/packages/grafana-data/src/field/fieldOverrides.ts b/packages/grafana-data/src/field/fieldOverrides.ts index 2aac477cd2b..8b345036c64 100644 --- a/packages/grafana-data/src/field/fieldOverrides.ts +++ b/packages/grafana-data/src/field/fieldOverrides.ts @@ -1,4 +1,4 @@ -import { isNumber, set, unset, get, cloneDeep } from 'lodash'; +import { isNumber, set, unset, get, cloneDeep, defaultsDeep } from 'lodash'; import { createContext, useContext, useMemo, useRef } from 'react'; import { usePrevious } from 'react-use'; @@ -240,9 +240,14 @@ export function applyFieldOverrides(options: ApplyFieldOverrideOptions): DataFra ...options, // nested frames can be `undefined` in certain situations, like after `merge` transform due to padding the value array. // let's replace them with empty frames to avoid errors applying overrides - data: field.values.map( - (nestedFrame: DataFrame | undefined): DataFrame => nestedFrame ?? createDataFrame({ fields: [] }) - ), + data: field.values.map((nestedFrame: DataFrame | undefined): DataFrame => { + const result = nestedFrame ?? createDataFrame({ fields: [] }); + result.fields = result.fields.map((newField) => { + newField.config = defaultsDeep(newField.config || {}, config); + return newField; + }); + return result; + }), }); } } @@ -256,7 +261,7 @@ function calculateRange( field: Field, globalRange: NumericRange | undefined, data: DataFrame[] -): { range?: { min?: number | null; max?: number | null; delta: number }; newGlobalRange: NumericRange | undefined } { +): { range?: NumericRange; newGlobalRange?: NumericRange } { // If range is defined with min/max, use it if (isNumber(config.min) && isNumber(config.max)) { const range = { min: config.min, max: config.max, delta: config.max - config.min }; diff --git a/packages/grafana-ui/src/components/Sparkline/utils.test.ts b/packages/grafana-ui/src/components/Sparkline/utils.test.ts index 9eb914af438..2a77677d998 100644 --- a/packages/grafana-ui/src/components/Sparkline/utils.test.ts +++ b/packages/grafana-ui/src/components/Sparkline/utils.test.ts @@ -114,6 +114,13 @@ describe('Get y range', () => { config: {}, state: { range: { min: -2, max: -2, delta: 0 } }, }; + const decimalsCloseYField: Field = { + name: 'y', + values: [2, 1.999999999999999, 2.000000000000001, 2, 2], + type: FieldType.number, + config: {}, + state: { range: { min: 1.999999999999999, max: 2.000000000000001, delta: 0 } }, + }; const xField: Field = { name: 'x', values: [1000, 2000, 3000, 4000, 5000], @@ -154,12 +161,12 @@ describe('Get y range', () => { { description: 'straight line', field: straightLineYField, - expected: [0, 4], + expected: [2, 4], }, { description: 'straight line, negative values', field: straightLineNegYField, - expected: [-4, 0], + expected: [-4, -2], }, { description: 'straight line with config min and max', @@ -171,8 +178,13 @@ describe('Get y range', () => { field: { ...straightLineYField, config: { noValue: '0' } }, expected: [0, 2], }, + { + description: 'long decimals which are nearly equal and result in a functional delta of 0', + field: decimalsCloseYField, + expected: [2, 4], + }, ])(`should return correct range for $description`, ({ field, expected }) => { - const actual = getYRange(field, getAlignedFrame(field)); + const actual = getYRange(getAlignedFrame(field)); expect(actual).toEqual(expected); expect(actual[0]).toBeLessThan(actual[1]!); }); diff --git a/packages/grafana-ui/src/components/Sparkline/utils.ts b/packages/grafana-ui/src/components/Sparkline/utils.ts index fa07cafb604..e4d17a85c15 100644 --- a/packages/grafana-ui/src/components/Sparkline/utils.ts +++ b/packages/grafana-ui/src/components/Sparkline/utils.ts @@ -3,7 +3,6 @@ import { Range } from 'uplot'; import { applyNullInsertThreshold, DataFrame, - Field, FieldConfig, FieldSparkline, FieldType, @@ -11,6 +10,7 @@ import { GrafanaTheme2, isLikelyAscendingVector, nullToValue, + roundDecimals, sortDataFrame, } from '@grafana/data'; import { t } from '@grafana/i18n'; @@ -68,35 +68,49 @@ export function preparePlotFrame(sparkline: FieldSparkline, config?: FieldConfig /** * apply configuration defaults and ensure that the range is never two equal values. */ -export function getYRange(field: Field, alignedFrame: DataFrame): Range.MinMax { - let { min, max } = alignedFrame.fields[1].state?.range!; +export function getYRange(alignedFrame: DataFrame): Range.MinMax { + const field = alignedFrame.fields[1]; + let { min, max } = field.state?.range!; - // enure that the min/max from the field config are respected - min = Math.max(min!, field.config.min ?? -Infinity); - max = Math.min(max!, field.config.max ?? Infinity); + // enure that the min/max from the field config are respected. + min = Math.min(min!, field.config.min ?? Infinity); + max = Math.max(max!, field.config.max ?? -Infinity); + + // console.log({ min, max }); // if noValue is set, ensure that it is included in the range as well - const noValue = +alignedFrame.fields[1].config?.noValue!; + const noValue = +field.config?.noValue!; if (!Number.isNaN(noValue)) { min = Math.min(min, noValue); max = Math.max(max, noValue); } - // if min and max are equal after all of that, create a range - // that allows the sparkline to be visible in the center of the viz - if (min === max) { - if (min === 0) { - max = 100; - } else if (min < 0) { - max = 0; - min *= 2; - } else { - min = 0; - max *= 2; - } + // call roundDecimals to mirror what is going to eventually happen in uplot + let roundedMin = roundDecimals(min, field.config.decimals ?? 0); + let roundedMax = roundDecimals(max, field.config.decimals ?? 0); + + // if the rounded min and max are different, + // we can return the real min and max. + if (roundedMin !== roundedMax) { + return [min, max]; } - return [min, max]; + // we are forced to tweak the min and max since they + // will be treated as equal after rounding by uPlot. + if (roundedMin === 0) { + // both are zero + roundedMax = 1; + } else if (roundedMin < 0) { + // both are negative + // max = 0; + roundedMin *= 2; + } else { + // both are positive + // min = 0; + roundedMax *= 2; + } + + return [roundedMin, roundedMax]; } // TODO: #112977 enable highlight index @@ -182,7 +196,7 @@ export const prepareConfig = ( scaleKey, orientation: ScaleOrientation.Vertical, direction: ScaleDirection.Up, - range: () => getYRange(field, dataFrame), + range: () => getYRange(dataFrame), }); builder.addAxis({