From f2327baf66e725b3fe8b0ebc98955b83a0ca8389 Mon Sep 17 00:00:00 2001 From: Ryan McKinley Date: Fri, 22 Jan 2021 10:18:46 -0800 Subject: [PATCH] DataFrame: cache frame/field index in field state (#30529) --- packages/grafana-data/src/types/dataFrame.ts | 10 +- .../src/components/GraphNG/GraphNG.tsx | 29 +- .../src/components/GraphNG/utils.test.ts | 291 ++++++++++++------ .../src/components/GraphNG/utils.ts | 46 ++- .../src/components/Sparkline/Sparkline.tsx | 5 +- .../src/components/uPlot/Plot.test.tsx | 32 +- .../grafana-ui/src/components/uPlot/Plot.tsx | 8 +- .../src/components/uPlot/context.ts | 12 +- .../grafana-ui/src/components/uPlot/types.ts | 9 +- 9 files changed, 254 insertions(+), 188 deletions(-) diff --git a/packages/grafana-data/src/types/dataFrame.ts b/packages/grafana-data/src/types/dataFrame.ts index a4cd0c889f5..154d64a9888 100644 --- a/packages/grafana-data/src/types/dataFrame.ts +++ b/packages/grafana-data/src/types/dataFrame.ts @@ -162,6 +162,13 @@ export interface FieldState { * Useful for assigning color to series by looking up a color in a palette using this index */ seriesIndex?: number; + + /** + * Location of this field within the context frames results + * + * @internal -- we will try to make this unnecessary + */ + origin?: DataFrameFieldIndex; } export interface NumericRange { @@ -206,7 +213,8 @@ export const TIME_SERIES_METRIC_FIELD_NAME = 'Metric'; /** * Describes where a specific data frame field is located within a * dataset of type DataFrame[] - * @public + * + * @internal -- we will try to make this unnecessary */ export interface DataFrameFieldIndex { frameIndex: number; diff --git a/packages/grafana-ui/src/components/GraphNG/GraphNG.tsx b/packages/grafana-ui/src/components/GraphNG/GraphNG.tsx index 8e60b824529..a2abda34649 100755 --- a/packages/grafana-ui/src/components/GraphNG/GraphNG.tsx +++ b/packages/grafana-ui/src/components/GraphNG/GraphNG.tsx @@ -12,7 +12,7 @@ import { reduceField, TimeRange, } from '@grafana/data'; -import { alignDataFrames } from './utils'; +import { joinDataFrames } from './utils'; import { useTheme } from '../../themes'; import { UPlotChart } from '../uPlot/Plot'; import { PlotProps } from '../uPlot/types'; @@ -64,9 +64,7 @@ export const GraphNG: React.FC = ({ const theme = useTheme(); const hasLegend = useRef(legend && legend.displayMode !== LegendDisplayMode.Hidden); - const alignedFrameWithGapTest = useMemo(() => alignDataFrames(data, fields), [data, fields]); - const alignedFrame = alignedFrameWithGapTest?.frame; - const getDataFrameFieldIndex = alignedFrameWithGapTest?.getDataFrameFieldIndex; + const frame = useMemo(() => joinDataFrames(data, fields), [data, fields]); const compareFrames = useCallback((a?: DataFrame | null, b?: DataFrame | null) => { if (a && b) { @@ -98,17 +96,17 @@ export const GraphNG: React.FC = ({ currentTimeRange.current = timeRange; }, [timeRange]); - const configRev = useRevision(alignedFrame, compareFrames); + const configRev = useRevision(frame, compareFrames); const configBuilder = useMemo(() => { const builder = new UPlotConfigBuilder(); - if (!alignedFrame) { + if (!frame) { return builder; } // X is the first field in the aligned frame - const xField = alignedFrame.fields[0]; + const xField = frame.fields[0]; if (xField.type === FieldType.time) { builder.addScale({ @@ -141,8 +139,8 @@ export const GraphNG: React.FC = ({ } let indexByName: Map | undefined = undefined; - for (let i = 0; i < alignedFrame.fields.length; i++) { - const field = alignedFrame.fields[i]; + for (let i = 0; i < frame.fields.length; i++) { + const field = frame.fields[i]; const config = field.config as FieldConfig; const customConfig: GraphFieldConfig = { ...defaultConfig, @@ -182,14 +180,13 @@ export const GraphNG: React.FC = ({ } const showPoints = customConfig.drawStyle === DrawStyle.Points ? PointVisibility.Always : customConfig.showPoints; - const dataFrameFieldIndex = getDataFrameFieldIndex ? getDataFrameFieldIndex(i) : undefined; let { fillOpacity } = customConfig; if (customConfig.fillBelowTo) { if (!indexByName) { - indexByName = getNamesToFieldIndex(alignedFrame); + indexByName = getNamesToFieldIndex(frame); } - const t = indexByName.get(getFieldDisplayName(field, alignedFrame)); + const t = indexByName.get(getFieldDisplayName(field, frame)); const b = indexByName.get(customConfig.fillBelowTo); if (isNumber(b) && isNumber(t)) { builder.addBand({ @@ -221,15 +218,15 @@ export const GraphNG: React.FC = ({ thresholds: config.thresholds, // The following properties are not used in the uPlot config, but are utilized as transport for legend config - dataFrameFieldIndex, - fieldName: getFieldDisplayName(field, alignedFrame), + dataFrameFieldIndex: field.state?.origin, + fieldName: getFieldDisplayName(field, frame), hideInLegend: customConfig.hideFrom?.legend, }); } return builder; }, [configRev, timeZone]); - if (alignedFrameWithGapTest == null) { + if (!frame) { return (

No data found in response

@@ -299,7 +296,7 @@ export const GraphNG: React.FC = ({ {(vizWidth: number, vizHeight: number) => ( { - describe('aligned frame', () => { +describe('joinDataFrames', () => { + describe('joined frame', () => { it('should align multiple data frames into one data frame', () => { const data: DataFrame[] = [ toDataFrame({ @@ -20,37 +19,64 @@ describe('alignDataFrames', () => { }), ]; - const aligned = alignDataFrames(data); + const joined = joinDataFrames(data); - expect(aligned?.frame.fields).toEqual([ - { - config: {}, - state: {}, - name: 'time', - type: FieldType.time, - values: new ArrayVector([1000, 2000, 3000, 4000]), - }, - { - config: {}, - state: { - displayName: 'temperature A', - seriesIndex: 0, + expect(joined?.fields).toMatchInlineSnapshot(` + Array [ + Object { + "config": Object {}, + "name": "time", + "state": Object { + "origin": undefined, + }, + "type": "time", + "values": Array [ + 1000, + 2000, + 3000, + 4000, + ], }, - name: 'temperature A', - type: FieldType.number, - values: new ArrayVector([1, 3, 5, 7]), - }, - { - config: {}, - state: { - displayName: 'temperature B', - seriesIndex: 1, + Object { + "config": Object {}, + "name": "temperature A", + "state": Object { + "displayName": "temperature A", + "origin": Object { + "fieldIndex": 1, + "frameIndex": 0, + }, + "seriesIndex": 0, + }, + "type": "number", + "values": Array [ + 1, + 3, + 5, + 7, + ], }, - name: 'temperature B', - type: FieldType.number, - values: new ArrayVector([0, 2, 6, 7]), - }, - ]); + Object { + "config": Object {}, + "name": "temperature B", + "state": Object { + "displayName": "temperature B", + "origin": Object { + "fieldIndex": 1, + "frameIndex": 1, + }, + "seriesIndex": 1, + }, + "type": "number", + "values": Array [ + 0, + 2, + 6, + 7, + ], + }, + ] + `); }); it('should align multiple data frames into one data frame but only keep first time field', () => { @@ -69,37 +95,64 @@ describe('alignDataFrames', () => { }), ]; - const aligned = alignDataFrames(data); + const aligned = joinDataFrames(data); - expect(aligned?.frame.fields).toEqual([ - { - config: {}, - state: {}, - name: 'time', - type: FieldType.time, - values: new ArrayVector([1000, 2000, 3000, 4000]), - }, - { - config: {}, - state: { - displayName: 'temperature', - seriesIndex: 0, + expect(aligned?.fields).toMatchInlineSnapshot(` + Array [ + Object { + "config": Object {}, + "name": "time", + "state": Object { + "origin": undefined, + }, + "type": "time", + "values": Array [ + 1000, + 2000, + 3000, + 4000, + ], }, - name: 'temperature', - type: FieldType.number, - values: new ArrayVector([1, 3, 5, 7]), - }, - { - config: {}, - state: { - displayName: 'temperature B', - seriesIndex: 1, + Object { + "config": Object {}, + "name": "temperature", + "state": Object { + "displayName": "temperature", + "origin": Object { + "fieldIndex": 1, + "frameIndex": 0, + }, + "seriesIndex": 0, + }, + "type": "number", + "values": Array [ + 1, + 3, + 5, + 7, + ], }, - name: 'temperature B', - type: FieldType.number, - values: new ArrayVector([0, 2, 6, 7]), - }, - ]); + Object { + "config": Object {}, + "name": "temperature B", + "state": Object { + "displayName": "temperature B", + "origin": Object { + "fieldIndex": 1, + "frameIndex": 1, + }, + "seriesIndex": 1, + }, + "type": "number", + "values": Array [ + 0, + 2, + 6, + 7, + ], + }, + ] + `); }); it('should align multiple data frames into one data frame and skip non-numeric fields', () => { @@ -113,27 +166,45 @@ describe('alignDataFrames', () => { }), ]; - const aligned = alignDataFrames(data); + const aligned = joinDataFrames(data); - expect(aligned?.frame.fields).toEqual([ - { - config: {}, - state: {}, - name: 'time', - type: FieldType.time, - values: new ArrayVector([1000, 2000, 3000, 4000]), - }, - { - config: {}, - state: { - displayName: 'temperature', - seriesIndex: 0, + expect(aligned?.fields).toMatchInlineSnapshot(` + Array [ + Object { + "config": Object {}, + "name": "time", + "state": Object { + "origin": undefined, + }, + "type": "time", + "values": Array [ + 1000, + 2000, + 3000, + 4000, + ], }, - name: 'temperature', - type: FieldType.number, - values: new ArrayVector([1, 3, 5, 7]), - }, - ]); + Object { + "config": Object {}, + "name": "temperature", + "state": Object { + "displayName": "temperature", + "origin": Object { + "fieldIndex": 1, + "frameIndex": 0, + }, + "seriesIndex": 0, + }, + "type": "number", + "values": Array [ + 1, + 3, + 5, + 7, + ], + }, + ] + `); }); it('should align multiple data frames into one data frame and skip non-numeric fields', () => { @@ -147,32 +218,50 @@ describe('alignDataFrames', () => { }), ]; - const aligned = alignDataFrames(data); + const aligned = joinDataFrames(data); - expect(aligned?.frame.fields).toEqual([ - { - config: {}, - state: {}, - name: 'time', - type: FieldType.time, - values: new ArrayVector([1000, 2000, 3000, 4000]), - }, - { - config: {}, - state: { - displayName: 'temperature', - seriesIndex: 0, + expect(aligned?.fields).toMatchInlineSnapshot(` + Array [ + Object { + "config": Object {}, + "name": "time", + "state": Object { + "origin": undefined, + }, + "type": "time", + "values": Array [ + 1000, + 2000, + 3000, + 4000, + ], }, - name: 'temperature', - type: FieldType.number, - values: new ArrayVector([1, 3, 5, 7]), - }, - ]); + Object { + "config": Object {}, + "name": "temperature", + "state": Object { + "displayName": "temperature", + "origin": Object { + "fieldIndex": 1, + "frameIndex": 0, + }, + "seriesIndex": 0, + }, + "type": "number", + "values": Array [ + 1, + 3, + 5, + 7, + ], + }, + ] + `); }); }); describe('getDataFrameFieldIndex', () => { - let aligned: AlignedFrameWithGapTest | null; + let aligned: DataFrame | null; beforeAll(() => { const data: DataFrame[] = [ @@ -197,7 +286,7 @@ describe('alignDataFrames', () => { }), ]; - aligned = alignDataFrames(data); + aligned = joinDataFrames(data); }); it.each` @@ -209,7 +298,7 @@ describe('alignDataFrames', () => { `('should return correct index for yDim', ({ yDim, index }) => { const [frameIndex, fieldIndex] = index; - expect(aligned?.getDataFrameFieldIndex(yDim)).toEqual({ + expect(aligned?.fields[yDim].state?.origin).toEqual({ frameIndex, fieldIndex, }); diff --git a/packages/grafana-ui/src/components/GraphNG/utils.ts b/packages/grafana-ui/src/components/GraphNG/utils.ts index 3e2416a00cc..e0602a543f9 100755 --- a/packages/grafana-ui/src/components/GraphNG/utils.ts +++ b/packages/grafana-ui/src/components/GraphNG/utils.ts @@ -12,7 +12,6 @@ import { sortDataFrame, Vector, } from '@grafana/data'; -import { AlignedFrameWithGapTest } from '../uPlot/types'; import uPlot, { AlignedData, JoinNullMode } from 'uplot'; import { XYFieldMatchers } from './GraphNG'; @@ -44,7 +43,7 @@ export function mapDimesions(match: XYFieldMatchers, frame: DataFrame, frames?: * * @alpha */ -export function alignDataFrames(frames: DataFrame[], fields?: XYFieldMatchers): AlignedFrameWithGapTest | null { +export function joinDataFrames(frames: DataFrame[], fields?: XYFieldMatchers): DataFrame | null { const valuesFromFrames: AlignedData[] = []; const sourceFields: Field[] = []; const sourceFieldsRefs: Record = {}; @@ -118,39 +117,34 @@ export function alignDataFrames(frames: DataFrame[], fields?: XYFieldMatchers): } // do the actual alignment (outerJoin on the first arrays) - let alignedData = uPlot.join(valuesFromFrames, nullModes); + let joinedData = uPlot.join(valuesFromFrames, nullModes); - if (alignedData!.length !== sourceFields.length) { + if (joinedData!.length !== sourceFields.length) { throw new Error('outerJoinValues lost a field?'); } let seriesIdx = 0; // Replace the values from the outer-join field return { - frame: { - length: alignedData![0].length, - fields: alignedData!.map((vals, idx) => { - let state: FieldState = { ...sourceFields[idx].state }; + ...frames[0], + length: joinedData![0].length, + fields: joinedData!.map((vals, idx) => { + let state: FieldState = { + ...sourceFields[idx].state, + origin: sourceFieldsRefs[idx], + }; - if (sourceFields[idx].type !== FieldType.time) { - state.seriesIndex = seriesIdx; - seriesIdx++; - } - - return { - ...sourceFields[idx], - state, - values: new ArrayVector(vals), - }; - }), - }, - getDataFrameFieldIndex: (alignedFieldIndex: number) => { - const index = sourceFieldsRefs[alignedFieldIndex]; - if (!index) { - throw new Error(`Could not find index for ${alignedFieldIndex}`); + if (sourceFields[idx].type !== FieldType.time) { + state.seriesIndex = seriesIdx; + seriesIdx++; } - return index; - }, + + return { + ...sourceFields[idx], + state, + values: new ArrayVector(vals), + }; + }), }; } diff --git a/packages/grafana-ui/src/components/Sparkline/Sparkline.tsx b/packages/grafana-ui/src/components/Sparkline/Sparkline.tsx index cd4e1c6c78d..e4388e09083 100755 --- a/packages/grafana-ui/src/components/Sparkline/Sparkline.tsx +++ b/packages/grafana-ui/src/components/Sparkline/Sparkline.tsx @@ -160,10 +160,7 @@ export class Sparkline extends PureComponent { return ( undefined, - }} + data={data} config={configBuilder} width={width} height={height} diff --git a/packages/grafana-ui/src/components/uPlot/Plot.test.tsx b/packages/grafana-ui/src/components/uPlot/Plot.test.tsx index 03f5dc2df94..327b3b8e0d4 100644 --- a/packages/grafana-ui/src/components/uPlot/Plot.test.tsx +++ b/packages/grafana-ui/src/components/uPlot/Plot.test.tsx @@ -1,12 +1,11 @@ import React from 'react'; import { UPlotChart } from './Plot'; import { act, render } from '@testing-library/react'; -import { ArrayVector, DataFrame, dateTime, FieldConfig, FieldType, MutableDataFrame } from '@grafana/data'; +import { ArrayVector, dateTime, FieldConfig, FieldType, MutableDataFrame } from '@grafana/data'; import { GraphFieldConfig, DrawStyle } from '../uPlot/config'; import uPlot from 'uplot'; import createMockRaf from 'mock-raf'; import { UPlotConfigBuilder } from './config/UPlotConfigBuilder'; -import { AlignedFrameWithGapTest } from './types'; const mockRaf = createMockRaf(); const setDataMock = jest.fn(); @@ -69,11 +68,10 @@ describe('UPlotChart', () => { it('destroys uPlot instance when component unmounts', () => { const { data, timeRange, config } = mockData(); - const uPlotData = createPlotData(data); const { unmount } = render( { describe('data update', () => { it('skips uPlot reinitialization when there are no field config changes', () => { const { data, timeRange, config } = mockData(); - const uPlotData = createPlotData(data); const { rerender } = render( { expect(uPlot).toBeCalledTimes(1); data.fields[1].values.set(0, 1); - uPlotData.frame = data; rerender( { describe('config update', () => { it('skips uPlot intialization for width and height equal 0', async () => { const { data, timeRange, config } = mockData(); - const uPlotData = createPlotData(data); const { queryAllByTestId } = render( - + ); expect(queryAllByTestId('uplot-main-div')).toHaveLength(1); @@ -148,11 +143,10 @@ describe('UPlotChart', () => { it('reinitializes uPlot when config changes', () => { const { data, timeRange, config } = mockData(); - const uPlotData = createPlotData(data); const { rerender } = render( { rerender( { it('skips uPlot reinitialization when only dimensions change', () => { const { data, timeRange, config } = mockData(); - const uPlotData = createPlotData(data); const { rerender } = render( { rerender( { }); }); }); - -const createPlotData = (frame: DataFrame): AlignedFrameWithGapTest => { - return { - frame, - getDataFrameFieldIndex: () => undefined, - }; -}; diff --git a/packages/grafana-ui/src/components/uPlot/Plot.tsx b/packages/grafana-ui/src/components/uPlot/Plot.tsx index 7edd5dd5ac0..831458f808e 100755 --- a/packages/grafana-ui/src/components/uPlot/Plot.tsx +++ b/packages/grafana-ui/src/components/uPlot/Plot.tsx @@ -39,7 +39,7 @@ export const UPlotChart: React.FC = (props) => { // 1. When config is ready and there is no uPlot instance, create new uPlot and return if (isConfigReady && !plotInstance.current) { - plotInstance.current = initializePlot(prepareData(props.data.frame), currentConfig.current, canvasRef.current); + plotInstance.current = initializePlot(prepareData(props.data), currentConfig.current, canvasRef.current); setIsPlotReady(true); return; } @@ -60,12 +60,12 @@ export const UPlotChart: React.FC = (props) => { pluginLog('uPlot core', false, 'destroying instance'); plotInstance.current.destroy(); } - plotInstance.current = initializePlot(prepareData(props.data.frame), currentConfig.current, canvasRef.current); + plotInstance.current = initializePlot(prepareData(props.data), currentConfig.current, canvasRef.current); return; } // 4. Otherwise, assume only data has changed and update uPlot data - updateData(props.data.frame, props.config, plotInstance.current, prepareData(props.data.frame)); + updateData(props.data, props.config, plotInstance.current, prepareData(props.data)); }, [props, isConfigReady]); // When component unmounts, clean the existing uPlot instance @@ -86,7 +86,7 @@ export const UPlotChart: React.FC = (props) => { ); }; -function prepareData(frame: DataFrame) { +function prepareData(frame: DataFrame): AlignedData { return frame.fields.map((f) => f.values.toArray()) as AlignedData; } diff --git a/packages/grafana-ui/src/components/uPlot/context.ts b/packages/grafana-ui/src/components/uPlot/context.ts index d36b08825de..13956dac779 100644 --- a/packages/grafana-ui/src/components/uPlot/context.ts +++ b/packages/grafana-ui/src/components/uPlot/context.ts @@ -1,6 +1,6 @@ import React, { useCallback, useContext } from 'react'; import uPlot, { Series } from 'uplot'; -import { PlotPlugin, AlignedFrameWithGapTest } from './types'; +import { PlotPlugin } from './types'; import { DataFrame, Field, FieldConfig } from '@grafana/data'; interface PlotCanvasContextType { @@ -26,7 +26,7 @@ interface PlotContextType extends PlotPluginsContextType { getSeries: () => Series[]; getCanvas: () => PlotCanvasContextType; canvasRef: any; - data: AlignedFrameWithGapTest; + data: DataFrame; } export const PlotContext = React.createContext({} as PlotContextType); @@ -76,7 +76,7 @@ export const usePlotData = (): PlotDataAPI => { if (!ctx) { throwWhenNoContext('usePlotData'); } - return ctx!.data.frame.fields[idx]; + return ctx!.data.fields[idx]; }, [ctx] ); @@ -109,7 +109,7 @@ export const usePlotData = (): PlotDataAPI => { } // by uPlot convention x-axis is always first field // this may change when we introduce non-time x-axis and multiple x-axes (https://leeoniya.github.io/uPlot/demos/time-periods.html) - return ctx!.data.frame.fields.slice(1); + return ctx!.data.fields.slice(1); }, [ctx]); if (!ctx) { @@ -117,7 +117,7 @@ export const usePlotData = (): PlotDataAPI => { } return { - data: ctx.data.frame, + data: ctx.data, getField, getFieldValue, getFieldConfig, @@ -129,7 +129,7 @@ export const usePlotData = (): PlotDataAPI => { export const buildPlotContext = ( isPlotReady: boolean, canvasRef: any, - data: AlignedFrameWithGapTest, + data: DataFrame, registerPlugin: any, getPlotInstance: () => uPlot | undefined ): PlotContextType => { diff --git a/packages/grafana-ui/src/components/uPlot/types.ts b/packages/grafana-ui/src/components/uPlot/types.ts index 1985ecade1a..5c5e11a99d3 100755 --- a/packages/grafana-ui/src/components/uPlot/types.ts +++ b/packages/grafana-ui/src/components/uPlot/types.ts @@ -1,6 +1,6 @@ import React from 'react'; import uPlot, { Options, Hooks } from 'uplot'; -import { DataFrame, DataFrameFieldIndex, TimeRange, TimeZone } from '@grafana/data'; +import { DataFrame, TimeRange, TimeZone } from '@grafana/data'; import { UPlotConfigBuilder } from './config/UPlotConfigBuilder'; export type PlotSeriesConfig = Pick; @@ -16,7 +16,7 @@ export interface PlotPluginProps { } export interface PlotProps { - data: AlignedFrameWithGapTest; + data: DataFrame; timeRange: TimeRange; timeZone: TimeZone; width: number; @@ -29,8 +29,3 @@ export abstract class PlotConfigBuilder { constructor(public props: P) {} abstract getConfig(): T; } - -export interface AlignedFrameWithGapTest { - frame: DataFrame; - getDataFrameFieldIndex: (alignedFieldIndex: number) => DataFrameFieldIndex | undefined; -}