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..5679b356b --- /dev/null +++ b/statchart/src/stat-chart-model.test.ts @@ -0,0 +1,55 @@ +// 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/plugin-system'; +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 for `last`, the one calculation an instant query reproduces exactly', () => { + expect(getStatChartQueryMode(options({ calculation: 'last' }))).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 + // 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..d017badd4 100644 --- a/statchart/src/stat-chart-model.ts +++ b/statchart/src/stat-chart-model.ts @@ -78,3 +78,38 @@ export function createInitialStatChartOptions(): StatChartOptions { legendMode: 'auto', }; } + +/** + * 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 INSTANT_SAFE_CALCULATIONS: ReadonlySet = new Set(['last']); + +/** + * 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 INSTANT_SAFE_CALCULATIONS.has(spec.calculation) ? 'instant' : 'range'; +} + +export function getStatChartQueryOptions(spec: StatChartOptions): { mode: 'instant' | 'range' } { + return { mode: getStatChartQueryMode(spec) }; +}