From 23fa9a148415aec9acd8f7eeaa2434450d540a13 Mon Sep 17 00:00:00 2001 From: Drew Slobodnjak <60050885+drew08t@users.noreply.github.com> Date: Sat, 6 Sep 2025 18:24:42 -0700 Subject: [PATCH] TimeSeries: Use exported time shift and fix time comparison tooltip (#109947) * TimeSeries: Use exported time comparison function * Add alignTimeRangeCompareData to grafana/data * Simplify tooltip time text formatting * Bump scenes version * Add tests for alignTimeRangeCompareData * Add backwards compatibility for older scenes * Update shouldAlignTimeCompare for typical query * Fix tooltip for older versions of scenes * support for multiple shifts --------- Co-authored-by: Leon Sorokin --- package.json | 4 +- .../grafana-data/src/dataframe/utils.test.ts | 287 +++++++++++++++++- packages/grafana-data/src/dataframe/utils.ts | 77 +++++ packages/grafana-data/src/index.ts | 2 + .../panel/timeseries/TimeSeriesPanel.tsx | 44 ++- .../panel/timeseries/TimeSeriesTooltip.tsx | 13 +- yarn.lock | 22 +- 7 files changed, 431 insertions(+), 18 deletions(-) diff --git a/package.json b/package.json index b7a2952b7f3..d94b20510c3 100644 --- a/package.json +++ b/package.json @@ -286,8 +286,8 @@ "@grafana/plugin-ui": "^0.10.10", "@grafana/prometheus": "workspace:*", "@grafana/runtime": "workspace:*", - "@grafana/scenes": "6.33.0", - "@grafana/scenes-react": "6.33.0", + "@grafana/scenes": "6.34.0", + "@grafana/scenes-react": "6.34.0", "@grafana/schema": "workspace:*", "@grafana/sql": "workspace:*", "@grafana/ui": "workspace:*", diff --git a/packages/grafana-data/src/dataframe/utils.test.ts b/packages/grafana-data/src/dataframe/utils.test.ts index 5f7887a1c8c..a1b957f363c 100644 --- a/packages/grafana-data/src/dataframe/utils.test.ts +++ b/packages/grafana-data/src/dataframe/utils.test.ts @@ -1,7 +1,8 @@ import { FieldType } from '../types/dataFrame'; +import { TimeRange } from '../types/time'; import { createDataFrame, toDataFrame } from './processDataFrame'; -import { anySeriesWithTimeField, addRow } from './utils'; +import { anySeriesWithTimeField, addRow, alignTimeRangeCompareData, shouldAlignTimeCompare } from './utils'; describe('anySeriesWithTimeField', () => { describe('single frame', () => { @@ -104,3 +105,287 @@ describe('addRow', () => { expect(frame.length).toBe(2); }); }); + +describe('alignTimeRangeCompareData', () => { + const ONE_DAY_MS = 24 * 60 * 60 * 1000; // 86400000ms + const ONE_WEEK_MS = 7 * ONE_DAY_MS; // 604800000ms + + it('should align time field values with positive diff (1 day)', () => { + const frame = toDataFrame({ + fields: [ + { name: 'time', type: FieldType.time, values: [1000, 2000, 3000] }, + { name: 'value', type: FieldType.number, values: [10, 20, 30] }, + ], + }); + + alignTimeRangeCompareData(frame, ONE_DAY_MS); + + expect(frame.fields[0].values).toEqual([ONE_DAY_MS + 1000, ONE_DAY_MS + 2000, ONE_DAY_MS + 3000]); + expect(frame.fields[1].values).toEqual([10, 20, 30]); // non-time fields unchanged + }); + + it('should align time field values with negative diff (1 week)', () => { + const frame = toDataFrame({ + fields: [ + { name: 'time', type: FieldType.time, values: [1000, 2000, 3000] }, + { name: 'value', type: FieldType.number, values: [10, 20, 30] }, + ], + }); + + alignTimeRangeCompareData(frame, -ONE_WEEK_MS); + + // When diff is negative, function does v - diff, so v - (-ONE_WEEK_MS) = v + ONE_WEEK_MS + expect(frame.fields[0].values).toEqual([ONE_WEEK_MS + 1000, ONE_WEEK_MS + 2000, ONE_WEEK_MS + 3000]); + }); + + it('should apply default gray color and timeCompare config', () => { + const frame = toDataFrame({ + fields: [ + { name: 'time', type: FieldType.time, values: [1000, 2000] }, + { name: 'value', type: FieldType.number, values: [10, 20] }, + ], + }); + + alignTimeRangeCompareData(frame, ONE_DAY_MS); + + frame.fields.forEach((field) => { + expect(field.config.color?.fixedColor).toBe('gray'); + expect(field.config.custom?.timeCompare).toEqual({ + diffMs: ONE_DAY_MS, + isTimeShiftQuery: true, + }); + }); + }); + + it('should apply custom color when provided', () => { + const frame = toDataFrame({ + fields: [{ name: 'value', type: FieldType.number, values: [10, 20] }], + }); + + alignTimeRangeCompareData(frame, ONE_DAY_MS, 'red'); + + expect(frame.fields[0].config.color?.fixedColor).toBe('red'); + }); + + it('should preserve existing config when merging', () => { + const frame = toDataFrame({ + fields: [ + { + name: 'value', + type: FieldType.number, + values: [10, 20], + config: { + displayName: 'My Display Name', + custom: { existingProperty: 'existingValue' }, + }, + }, + ], + }); + + alignTimeRangeCompareData(frame, ONE_WEEK_MS); + + expect(frame.fields[0].config.displayName).toBe('My Display Name'); + expect(frame.fields[0].config.custom?.existingProperty).toBe('existingValue'); + expect(frame.fields[0].config.custom?.timeCompare?.diffMs).toBe(ONE_WEEK_MS); + }); +}); + +describe('shouldAlignTimeCompare', () => { + const TIME_VALUES_A = [1000, 2000, 3000]; + const TIME_VALUES_B = [5000, 6000, 7000]; + const ORIGINAL_VALUES = [10, 20, 30]; + const COMPARE_VALUES = [15, 25, 35]; + + const mockTimeRange: TimeRange = { + from: { valueOf: () => 4000 }, + to: { valueOf: () => 8000 }, + raw: { from: 'now-1h', to: 'now' }, + } as TimeRange; + + it('should return true when compare first time is before time range', () => { + const originalFrame = toDataFrame({ + refId: 'A', + fields: [ + { name: 'time', type: FieldType.time, values: TIME_VALUES_A }, + { name: 'value', type: FieldType.number, values: ORIGINAL_VALUES }, + ], + }); + + const compareFrame = toDataFrame({ + refId: 'A-compare', + fields: [ + { name: 'time', type: FieldType.time, values: TIME_VALUES_A }, + { name: 'value', type: FieldType.number, values: COMPARE_VALUES }, + ], + meta: { + timeCompare: { + isTimeShiftQuery: true, + diffMs: 86400000, + }, + }, + }); + + const allFrames = [originalFrame, compareFrame]; + expect(shouldAlignTimeCompare(compareFrame, allFrames, mockTimeRange)).toBe(true); + }); + + it('should return false when compare first time is after time range', () => { + const originalFrame = toDataFrame({ + refId: 'A', + fields: [ + { name: 'time', type: FieldType.time, values: TIME_VALUES_A }, + { name: 'value', type: FieldType.number, values: ORIGINAL_VALUES }, + ], + }); + + const compareFrame = toDataFrame({ + refId: 'A-compare', + fields: [ + { name: 'time', type: FieldType.time, values: TIME_VALUES_B }, + { name: 'value', type: FieldType.number, values: COMPARE_VALUES }, + ], + meta: { + timeCompare: { + isTimeShiftQuery: true, + diffMs: 86400000, + }, + }, + }); + + const allFrames = [originalFrame, compareFrame]; + expect(shouldAlignTimeCompare(compareFrame, allFrames, mockTimeRange)).toBe(false); + }); + + it('should return false when compare frame refId does not end with -compare', () => { + const compareFrame = toDataFrame({ + refId: 'A', + fields: [ + { name: 'time', type: FieldType.time, values: TIME_VALUES_A }, + { name: 'value', type: FieldType.number, values: ORIGINAL_VALUES }, + ], + }); + + const allFrames = [compareFrame]; + expect(shouldAlignTimeCompare(compareFrame, allFrames, mockTimeRange)).toBe(false); + }); + + it('should return false when original frame is not found', () => { + const compareFrame = toDataFrame({ + refId: 'A-compare', + fields: [ + { name: 'time', type: FieldType.time, values: TIME_VALUES_A }, + { name: 'value', type: FieldType.number, values: ORIGINAL_VALUES }, + ], + }); + + const allFrames = [compareFrame]; // No original frame with refId 'A' + expect(shouldAlignTimeCompare(compareFrame, allFrames, mockTimeRange)).toBe(false); + }); + + it('should return false when compare frame has no time field', () => { + const originalFrame = toDataFrame({ + refId: 'A', + fields: [ + { name: 'time', type: FieldType.time, values: TIME_VALUES_A }, + { name: 'value', type: FieldType.number, values: ORIGINAL_VALUES }, + ], + }); + + const compareFrame = toDataFrame({ + refId: 'A-compare', + fields: [{ name: 'value', type: FieldType.number, values: COMPARE_VALUES }], + }); + + const allFrames = [originalFrame, compareFrame]; + expect(shouldAlignTimeCompare(compareFrame, allFrames, mockTimeRange)).toBe(false); + }); + + it('should return false when original frame has no time field', () => { + const originalFrame = toDataFrame({ + refId: 'A', + fields: [{ name: 'value', type: FieldType.number, values: ORIGINAL_VALUES }], + }); + + const compareFrame = toDataFrame({ + refId: 'A-compare', + fields: [ + { name: 'time', type: FieldType.time, values: TIME_VALUES_A }, + { name: 'value', type: FieldType.number, values: COMPARE_VALUES }, + ], + }); + + const allFrames = [originalFrame, compareFrame]; + expect(shouldAlignTimeCompare(compareFrame, allFrames, mockTimeRange)).toBe(false); + }); + + it('should return false when time fields have empty values', () => { + const EMPTY_VALUES: number[] = []; + + const originalFrame = toDataFrame({ + refId: 'A', + fields: [ + { name: 'time', type: FieldType.time, values: EMPTY_VALUES }, + { name: 'value', type: FieldType.number, values: EMPTY_VALUES }, + ], + }); + + const compareFrame = toDataFrame({ + refId: 'A-compare', + fields: [ + { name: 'time', type: FieldType.time, values: EMPTY_VALUES }, + { name: 'value', type: FieldType.number, values: EMPTY_VALUES }, + ], + }); + + const allFrames = [originalFrame, compareFrame]; + expect(shouldAlignTimeCompare(compareFrame, allFrames, mockTimeRange)).toBe(false); + }); + + it('should handle null values and return true when first non-null time is before range', () => { + const TIME_WITH_NULLS = [null, ...TIME_VALUES_A]; + const ORIGINAL_WITH_NULLS = [null, ...ORIGINAL_VALUES]; + const COMPARE_WITH_NULLS = [null, ...COMPARE_VALUES]; + + const originalFrame = toDataFrame({ + refId: 'A', + fields: [ + { name: 'time', type: FieldType.time, values: TIME_WITH_NULLS }, + { name: 'value', type: FieldType.number, values: ORIGINAL_WITH_NULLS }, + ], + }); + + const compareFrame = toDataFrame({ + refId: 'A-compare', + fields: [ + { name: 'time', type: FieldType.time, values: TIME_WITH_NULLS }, + { name: 'value', type: FieldType.number, values: COMPARE_WITH_NULLS }, + ], + }); + + const allFrames = [originalFrame, compareFrame]; + expect(shouldAlignTimeCompare(compareFrame, allFrames, mockTimeRange)).toBe(true); + }); + + it('should return false when all time values are null', () => { + const ALL_NULL_TIMES = [null, null, null]; + + const originalFrame = toDataFrame({ + refId: 'A', + fields: [ + { name: 'time', type: FieldType.time, values: ALL_NULL_TIMES }, + { name: 'value', type: FieldType.number, values: ORIGINAL_VALUES }, + ], + }); + + const compareFrame = toDataFrame({ + refId: 'A-compare', + fields: [ + { name: 'time', type: FieldType.time, values: ALL_NULL_TIMES }, + { name: 'value', type: FieldType.number, values: COMPARE_VALUES }, + ], + }); + + const allFrames = [originalFrame, compareFrame]; + expect(shouldAlignTimeCompare(compareFrame, allFrames, mockTimeRange)).toBe(false); + }); +}); diff --git a/packages/grafana-data/src/dataframe/utils.ts b/packages/grafana-data/src/dataframe/utils.ts index 8f45ddd1b92..c9114e7e647 100644 --- a/packages/grafana-data/src/dataframe/utils.ts +++ b/packages/grafana-data/src/dataframe/utils.ts @@ -1,4 +1,5 @@ import { DataFrame, Field, FieldType } from '../types/dataFrame'; +import { TimeRange } from '../types/time'; import { getTimeField } from './processDataFrame'; @@ -123,3 +124,79 @@ export function addRow(dataFrame: DataFrame, row: Record | unkn // does not need any external updating. } } + +/** + * Aligns time range comparison data by adjusting timestamps and applying compare-specific styling + * @param series - The DataFrame containing the comparison data + * @param diff - The time difference in milliseconds to align the timestamps + * @param compareColor - Optional color to use for the comparison series (defaults to 'gray') + */ +export function alignTimeRangeCompareData(series: DataFrame, diff: number, compareColor = 'gray') { + series.fields.forEach((field: Field) => { + // Align compare series time stamps with reference series + if (field.type === FieldType.time) { + field.values = field.values.map((v: number) => { + return diff < 0 ? v - diff : v + diff; + }); + } + + field.config = { + ...(field.config ?? {}), + color: { + mode: 'fixed', + fixedColor: compareColor, + }, + custom: { + ...(field.config?.custom ?? {}), + timeCompare: { + diffMs: diff, + isTimeShiftQuery: true, + }, + }, + }; + }); +} + +/** + * Checks if a time comparison frame needs alignment based on whether its first time is before the current time range. + * Returns true if the first time in compare is before timeRange.from, indicating it needs shifting. + * @param compareFrame - The frame with time comparison data + * @param allFrames - Array of all frames to find the matching original frame + * @param timeRange - The current panel time range + * @returns true if alignment is needed + */ +export function shouldAlignTimeCompare(compareFrame: DataFrame, allFrames: DataFrame[], timeRange: TimeRange): boolean { + // Find the matching original frame by removing '-compare' from refId + const compareRefId = compareFrame.refId; + if (!compareRefId || !compareRefId.endsWith('-compare')) { + return false; + } + + const originalRefId = compareRefId.replace('-compare', ''); + const originalFrame = allFrames.find( + (frame) => frame.refId === originalRefId && !frame.meta?.timeCompare?.isTimeShiftQuery + ); + + if (!originalFrame) { + return false; + } + + // Find time fields + const compareTimeField = compareFrame.fields.find((field) => field.type === FieldType.time); + const originalTimeField = originalFrame.fields.find((field) => field.type === FieldType.time); + + if (!compareTimeField?.values.length || !originalTimeField?.values.length) { + return false; + } + + // Find first non-null time value from each frame + const compareFirstTime = compareTimeField.values.find((value) => value != null); + const originalFirstTime = originalTimeField.values.find((value) => value != null); + + if (compareFirstTime == null || originalFirstTime == null) { + return false; + } + + // Check if first non-null time value is before timeRange.from + return compareFirstTime < timeRange.from.valueOf(); +} diff --git a/packages/grafana-data/src/index.ts b/packages/grafana-data/src/index.ts index 23127b6c252..dec175674ce 100644 --- a/packages/grafana-data/src/index.ts +++ b/packages/grafana-data/src/index.ts @@ -50,6 +50,8 @@ export { isTimeSeriesField, getRowUniqueId, addRow, + alignTimeRangeCompareData, + shouldAlignTimeCompare, } from './dataframe/utils'; export { StreamingDataFrame, diff --git a/public/app/plugins/panel/timeseries/TimeSeriesPanel.tsx b/public/app/plugins/panel/timeseries/TimeSeriesPanel.tsx index 0ecb638099c..a80bc63ec0e 100644 --- a/public/app/plugins/panel/timeseries/TimeSeriesPanel.tsx +++ b/public/app/plugins/panel/timeseries/TimeSeriesPanel.tsx @@ -1,6 +1,14 @@ import { useMemo, useState } from 'react'; -import { PanelProps, DataFrameType, DashboardCursorSync } from '@grafana/data'; +import { + PanelProps, + DataFrameType, + DashboardCursorSync, + DataFrame, + alignTimeRangeCompareData, + shouldAlignTimeCompare, + FieldType, +} from '@grafana/data'; import { PanelDataErrorView } from '@grafana/runtime'; import { TooltipDisplayMode, VizOrientation } from '@grafana/schema'; import { EventBusPlugin, KeyboardPlugin, TooltipPlugin2, usePanelContext } from '@grafana/ui'; @@ -47,7 +55,38 @@ export const TimeSeriesPanel = ({ // Vertical orientation is not available for users through config. // It is simplified version of horizontal time series panel and it does not support all plugins. const isVerticallyOriented = options.orientation === VizOrientation.Vertical; - const frames = useMemo(() => prepareGraphableFields(data.series, config.theme2, timeRange), [data.series, timeRange]); + const { frames, compareDiffMs } = useMemo(() => { + let frames = prepareGraphableFields(data.series, config.theme2, timeRange); + + if (frames != null) { + let compareDiffMs: number[] = [0]; + + frames.forEach((frame: DataFrame) => { + const diffMs = frame.meta?.timeCompare?.diffMs ?? 0; + + frame.fields.forEach((field) => { + if (field.type !== FieldType.time) { + compareDiffMs.push(diffMs); + } + }); + + if (diffMs !== 0) { + // Check if the compared frame needs time alignment + // Apply alignment when time ranges match (no shift applied yet) + const needsAlignment = shouldAlignTimeCompare(frame, frames, timeRange); + + if (needsAlignment) { + alignTimeRangeCompareData(frame, diffMs, config.theme2.colors.text.disabled); + } + } + }); + + return { frames, compareDiffMs }; + } + + return { frames }; + }, [data.series, timeRange]); + const timezones = useMemo(() => getTimezones(options.timezone, timeZone), [options.timezone, timeZone]); const suggestions = useMemo(() => { if (frames?.length && frames.every((df) => df.meta?.type === DataFrameType.TimeSeriesLong)) { @@ -141,6 +180,7 @@ export const TimeSeriesPanel = ({ replaceVariables={replaceVariables} dataLinks={dataLinks} canExecuteActions={userCanExecuteActions} + compareDiffMs={compareDiffMs} /> ); }} diff --git a/public/app/plugins/panel/timeseries/TimeSeriesTooltip.tsx b/public/app/plugins/panel/timeseries/TimeSeriesTooltip.tsx index 8dd95160917..8c7125e6149 100644 --- a/public/app/plugins/panel/timeseries/TimeSeriesTooltip.tsx +++ b/public/app/plugins/panel/timeseries/TimeSeriesTooltip.tsx @@ -43,6 +43,7 @@ export interface TimeSeriesTooltipProps { hideZeros?: boolean; adHocFilters?: AdHocFilterModel[]; canExecuteActions?: boolean; + compareDiffMs?: number[]; } export const TimeSeriesTooltip = ({ @@ -60,9 +61,17 @@ export const TimeSeriesTooltip = ({ hideZeros, adHocFilters, canExecuteActions, + compareDiffMs, }: TimeSeriesTooltipProps) => { const xField = series.fields[0]; - const xVal = formattedValueToString(xField.display!(xField.values[dataIdxs[0]!])); + + let xVal = xField.values[dataIdxs[0]!]; + + if (compareDiffMs != null && xField.type === FieldType.time) { + xVal += compareDiffMs[seriesIdx ?? 1]; + } + + const xDisp = formattedValueToString(xField.display!(xVal)); const contentItems = getContentItems( series.fields, @@ -94,7 +103,7 @@ export const TimeSeriesTooltip = ({ const headerItem: VizTooltipItem = { label: xField.type === FieldType.time ? '' : (xField.state?.displayName ?? xField.name), - value: xVal, + value: xDisp, }; return ( diff --git a/yarn.lock b/yarn.lock index 78cfc86a961..e25761aad80 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3475,11 +3475,11 @@ __metadata: languageName: unknown linkType: soft -"@grafana/scenes-react@npm:6.33.0": - version: 6.33.0 - resolution: "@grafana/scenes-react@npm:6.33.0" +"@grafana/scenes-react@npm:6.34.0": + version: 6.34.0 + resolution: "@grafana/scenes-react@npm:6.34.0" dependencies: - "@grafana/scenes": "npm:6.33.0" + "@grafana/scenes": "npm:6.34.0" lru-cache: "npm:^10.2.2" react-use: "npm:^17.4.0" peerDependencies: @@ -3491,13 +3491,13 @@ __metadata: react: ^18.0.0 react-dom: ^18.0.0 react-router-dom: ^6.28.0 - checksum: 10/fbb6c2ee108496a6ba3dc704f902d9a88ae317ceba3ecb89be4bfb3c317cf107444c105d1a7c21836b73df36e93cabacc6c1eb131f137476f27df514493e226c + checksum: 10/a95e18c3e8e303c88ac307e37ce5795325593e134ce90e069675b223f82966d4fe27c8d09f8c4dbc259db46a572f669f3571adf4d967a36639fa55128f619f2e languageName: node linkType: hard -"@grafana/scenes@npm:6.33.0": - version: 6.33.0 - resolution: "@grafana/scenes@npm:6.33.0" +"@grafana/scenes@npm:6.34.0": + version: 6.34.0 + resolution: "@grafana/scenes@npm:6.34.0" dependencies: "@floating-ui/react": "npm:^0.26.16" "@leeoniya/ufuzzy": "npm:^1.0.16" @@ -3517,7 +3517,7 @@ __metadata: react: ^18.0.0 react-dom: ^18.0.0 react-router-dom: ^6.28.0 - checksum: 10/5fc020c210e8a1c8e629bbb2be84e30a08e58b2b53f97ebd3f770cd03878eb2c0760148d7fa1fe5e852c6857b8f9a43b14d1ededb7fb439f1de287648c870cf2 + checksum: 10/16e3c0309b2af0d655215ef7b73a254667bad7b2e3e40af9480a70b1610cd50fa48927bb81b477ea9f543791b7aed6e6e21189e608feadf64cc29b96d10d9de5 languageName: node linkType: hard @@ -18122,8 +18122,8 @@ __metadata: "@grafana/plugin-ui": "npm:^0.10.10" "@grafana/prometheus": "workspace:*" "@grafana/runtime": "workspace:*" - "@grafana/scenes": "npm:6.33.0" - "@grafana/scenes-react": "npm:6.33.0" + "@grafana/scenes": "npm:6.34.0" + "@grafana/scenes-react": "npm:6.34.0" "@grafana/schema": "workspace:*" "@grafana/sql": "workspace:*" "@grafana/test-utils": "workspace:*"