From c05efc88da52e6b9d5da06464d1b3469e7322805 Mon Sep 17 00:00:00 2001 From: s3onghyun Date: Wed, 22 Jul 2026 08:36:34 +0900 Subject: [PATCH 1/3] [BUGFIX] StatChart: derive query mode from the spec instead of always using range MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit StatChart never sets queryOptions, so it always issues range queries. For a panel showing a single aggregate that is wasteful, and against backends that split range queries by interval it produces fragmented results on wide time ranges (perses#4277). #738 fixed that by declaring queryOptions: { mode: 'instant' } unconditionally and was reverted, because the sparkline draws the series itself and broke without range data. The same applies to the calculation: StatChartPanel computes it client-side over seriesData, so mean/sum/min/max/first collapse onto a single point under an instant query. So the mode is derived from the spec instead: range when a sparkline is configured or when the calculation aggregates over the series, instant otherwise. This mirrors how the table plugin already picks its mode via getTablePanelQueryOptions. Defaults are unaffected — createInitialStatChartOptions enables the sparkline, so a freshly created stat panel still uses range. Signed-off-by: s3onghyun --- statchart/src/StatChart.ts | 3 +- statchart/src/stat-chart-model.test.ts | 50 ++++++++++++++++++++++++++ statchart/src/stat-chart-model.ts | 29 +++++++++++++++ 3 files changed, 81 insertions(+), 1 deletion(-) create mode 100644 statchart/src/stat-chart-model.test.ts diff --git a/statchart/src/StatChart.ts b/statchart/src/StatChart.ts index efa3cf8c3..d33b76546 100644 --- a/statchart/src/StatChart.ts +++ b/statchart/src/StatChart.ts @@ -12,7 +12,7 @@ // limitations under the License. import { PanelPlugin } from '@perses-dev/plugin-system'; -import { createInitialStatChartOptions, StatChartOptions } from './stat-chart-model'; +import { createInitialStatChartOptions, getStatChartQueryOptions, StatChartOptions } from './stat-chart-model'; import { StatChartValueMappingEditor } from './StatChartValueMappingEditor'; import { StatChartOptionsEditorSettings } from './StatChartOptionsEditorSettings'; import { StatChartPanel, StatChartPanelProps } from './StatChartPanel'; @@ -23,6 +23,7 @@ import { StatChartPanel, StatChartPanelProps } from './StatChartPanel'; export const StatChart: PanelPlugin = { PanelComponent: StatChartPanel, supportedQueryTypes: ['TimeSeriesQuery'], + queryOptions: getStatChartQueryOptions, panelOptionsEditorComponents: [ { label: 'Settings', diff --git a/statchart/src/stat-chart-model.test.ts b/statchart/src/stat-chart-model.test.ts new file mode 100644 index 000000000..a4b4be4af --- /dev/null +++ b/statchart/src/stat-chart-model.test.ts @@ -0,0 +1,50 @@ +// Copyright The Perses Authors +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +import { CalculationType } from '@perses-dev/core'; +import { createInitialStatChartOptions, getStatChartQueryMode, StatChartOptions } from './stat-chart-model'; + +function options(overrides: Partial = {}): StatChartOptions { + return { + calculation: 'last-number', + format: { unit: 'decimal' }, + ...overrides, + }; +} + +describe('getStatChartQueryMode', () => { + it('uses instant when there is no sparkline and the calculation only needs the latest point', () => { + expect(getStatChartQueryMode(options({ calculation: 'last' }))).toBe('instant'); + expect(getStatChartQueryMode(options({ calculation: 'last-number' }))).toBe('instant'); + }); + + // Regression: requesting instant unconditionally broke the sparkline, which draws the + // series itself. See perses/plugins#738, which was reverted for this reason. + it('uses range whenever a sparkline is configured', () => { + expect(getStatChartQueryMode(options({ sparkline: {} }))).toBe('range'); + expect(getStatChartQueryMode(options({ sparkline: { color: '#fff', width: 2 } }))).toBe('range'); + // even for a calculation that would otherwise be instant + expect(getStatChartQueryMode(options({ calculation: 'last', sparkline: {} }))).toBe('range'); + }); + + it.each(['mean', 'sum', 'min', 'max', 'first', 'first-number'])( + 'uses range for %s, which aggregates over the whole series', + (calculation) => { + expect(getStatChartQueryMode(options({ calculation }))).toBe('range'); + } + ); + + it('uses range for the default options, which enable the sparkline', () => { + expect(getStatChartQueryMode(createInitialStatChartOptions())).toBe('range'); + }); +}); diff --git a/statchart/src/stat-chart-model.ts b/statchart/src/stat-chart-model.ts index b7ff3ff61..882c866fc 100644 --- a/statchart/src/stat-chart-model.ts +++ b/statchart/src/stat-chart-model.ts @@ -78,3 +78,32 @@ export function createInitialStatChartOptions(): StatChartOptions { legendMode: 'auto', }; } + +/** + * Calculations that only need the most recent point of a series. Everything else + * (`mean`, `sum`, `min`, `max`, `first`, `first-number`) is computed over the whole + * series in `StatChartPanel`, so it needs range data to be correct. + */ +const LATEST_POINT_CALCULATIONS: ReadonlySet = new Set(['last', 'last-number']); + +/** + * A stat panel usually shows a single aggregate value, for which an instant query is + * enough and much cheaper than a range query — especially against backends that split + * range queries into many sub-queries. + * + * It cannot always be instant though: + * - the sparkline draws the series itself, so it needs range data; + * - calculations other than `last`/`last-number` aggregate over the series, and with a + * single instant point they would silently collapse to that point. + * + * Requesting instant unconditionally breaks both (see perses/plugins#738, reverted for + * the sparkline case), so the mode is derived from the spec. + */ +export function getStatChartQueryMode(spec: StatChartOptions): 'instant' | 'range' { + if (spec.sparkline !== undefined) return 'range'; + return LATEST_POINT_CALCULATIONS.has(spec.calculation) ? 'instant' : 'range'; +} + +export function getStatChartQueryOptions(spec: StatChartOptions): { mode: 'instant' | 'range' } { + return { mode: getStatChartQueryMode(spec) }; +} From 3ab3dbe6c38249484318e211297d622aa11a96e3 Mon Sep 17 00:00:00 2001 From: s3onghyun Date: Wed, 22 Jul 2026 21:52:11 +0900 Subject: [PATCH 2/3] Import CalculationType from plugin-system instead of core Review feedback: the test was the only file in the plugin reaching into @perses-dev/core directly. Every other consumer of CalculationType, including stat-chart-model.ts next to it, imports it from @perses-dev/plugin-system. Signed-off-by: s3onghyun --- statchart/src/stat-chart-model.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/statchart/src/stat-chart-model.test.ts b/statchart/src/stat-chart-model.test.ts index a4b4be4af..fd5ddc4d4 100644 --- a/statchart/src/stat-chart-model.test.ts +++ b/statchart/src/stat-chart-model.test.ts @@ -11,7 +11,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -import { CalculationType } from '@perses-dev/core'; +import { CalculationType } from '@perses-dev/plugin-system'; import { createInitialStatChartOptions, getStatChartQueryMode, StatChartOptions } from './stat-chart-model'; function options(overrides: Partial = {}): StatChartOptions { From 5ec159a4c17561f69f9e8e984e859805463fe5f3 Mon Sep 17 00:00:00 2001 From: s3onghyun Date: Thu, 23 Jul 2026 22:53:33 +0900 Subject: [PATCH 3/3] Only treat 'last' as instant-safe, not 'last-number' MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review feedback from @celian-garcia: last-number is 'Last numeric value', i.e. the last *non-null* number. When the most recent point is null it must fall back to an earlier point, which an instant query cannot see — so instant and range would disagree. Only 'last' is literally the latest value and reproduces exactly under an instant query. Renamed the set to INSTANT_SAFE_CALCULATIONS = {last}, moved last-number to the range group with a test asserting it. Signed-off-by: s3onghyun --- statchart/src/stat-chart-model.test.ts | 9 +++++++-- statchart/src/stat-chart-model.ts | 16 +++++++++++----- 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/statchart/src/stat-chart-model.test.ts b/statchart/src/stat-chart-model.test.ts index fd5ddc4d4..5679b356b 100644 --- a/statchart/src/stat-chart-model.test.ts +++ b/statchart/src/stat-chart-model.test.ts @@ -23,9 +23,14 @@ function options(overrides: Partial = {}): StatChartOptions { } describe('getStatChartQueryMode', () => { - it('uses instant when there is no sparkline and the calculation only needs the latest point', () => { + it('uses instant for `last`, the one calculation an instant query reproduces exactly', () => { expect(getStatChartQueryMode(options({ calculation: 'last' }))).toBe('instant'); - expect(getStatChartQueryMode(options({ calculation: 'last-number' }))).toBe('instant'); + }); + + // `last-number` is the last *non-null* value, so with trailing nulls it must look + // back past the latest point — which an instant query cannot do. It needs range. + it('uses range for `last-number`', () => { + expect(getStatChartQueryMode(options({ calculation: 'last-number' }))).toBe('range'); }); // Regression: requesting instant unconditionally broke the sparkline, which draws the diff --git a/statchart/src/stat-chart-model.ts b/statchart/src/stat-chart-model.ts index 882c866fc..d017badd4 100644 --- a/statchart/src/stat-chart-model.ts +++ b/statchart/src/stat-chart-model.ts @@ -80,11 +80,17 @@ export function createInitialStatChartOptions(): StatChartOptions { } /** - * Calculations that only need the most recent point of a series. Everything else - * (`mean`, `sum`, `min`, `max`, `first`, `first-number`) is computed over the whole - * series in `StatChartPanel`, so it needs range data to be correct. + * Only `last` is safe to compute from a single instant point: it is literally the + * most recent value of the series, which is exactly what an instant query returns. + * + * Everything else needs the full series, so it must use a range query: + * - `mean`, `sum`, `min`, `max` aggregate over all points; + * - `first` / `first-number` need the *start* of the window, not the latest point; + * - `last-number` is the last *non-null* value (`Last numeric value`), so when the + * most recent point is null it must fall back to an earlier one — which an instant + * query cannot see, giving a different result than the range query. */ -const LATEST_POINT_CALCULATIONS: ReadonlySet = new Set(['last', 'last-number']); +const INSTANT_SAFE_CALCULATIONS: ReadonlySet = new Set(['last']); /** * A stat panel usually shows a single aggregate value, for which an instant query is @@ -101,7 +107,7 @@ const LATEST_POINT_CALCULATIONS: ReadonlySet = new Set