From db6694994f3dcae9ff4b80dd72e015780e8967d8 Mon Sep 17 00:00:00 2001 From: Andrej Ocenas Date: Thu, 30 Mar 2023 11:32:44 +0200 Subject: [PATCH] FlameGraph: Refactor handling of the labels (#65491) --- .../components/FlameGraph/FlameGraph.test.tsx | 11 +- .../components/FlameGraph/FlameGraph.tsx | 101 +++++--------- .../FlameGraph/FlameGraphContextMenu.tsx | 21 ++- .../FlameGraph/FlameGraphMetadata.test.ts | 60 +++++---- .../FlameGraph/FlameGraphMetadata.tsx | 41 +++--- ...tip.test.ts => FlameGraphTooltip.test.tsx} | 44 +++--- .../FlameGraph/FlameGraphTooltip.tsx | 123 ++++++++--------- .../FlameGraph/dataTransform.test.ts | 36 ++--- .../components/FlameGraph/dataTransform.ts | 126 +++++++++++++++--- .../components/FlameGraph/rendering.test.ts | 64 ++++----- .../components/FlameGraph/rendering.ts | 20 ++- .../components/FlameGraphContainer.tsx | 51 +++---- .../FlameGraphTopTableContainer.test.tsx | 11 +- .../TopTable/FlameGraphTopTableContainer.tsx | 95 +++++-------- .../panel/flamegraph/components/types.ts | 10 -- 15 files changed, 395 insertions(+), 419 deletions(-) rename public/app/plugins/panel/flamegraph/components/FlameGraph/{FlameGraphTooltip.test.ts => FlameGraphTooltip.test.tsx} (54%) diff --git a/public/app/plugins/panel/flamegraph/components/FlameGraph/FlameGraph.test.tsx b/public/app/plugins/panel/flamegraph/components/FlameGraph/FlameGraph.test.tsx index 4fb7d0234fe..b49738c4f7f 100644 --- a/public/app/plugins/panel/flamegraph/components/FlameGraph/FlameGraph.test.tsx +++ b/public/app/plugins/panel/flamegraph/components/FlameGraph/FlameGraph.test.tsx @@ -2,12 +2,12 @@ import { fireEvent, screen } from '@testing-library/dom'; import { render } from '@testing-library/react'; import React, { useState } from 'react'; -import { CoreApp, DataFrameView, MutableDataFrame } from '@grafana/data'; +import { CoreApp, MutableDataFrame } from '@grafana/data'; import { SelectedView } from '../types'; import FlameGraph from './FlameGraph'; -import { Item, nestedSetToLevels } from './dataTransform'; +import { FlameGraphDataContainer, nestedSetToLevels } from './dataTransform'; import { data } from './testData/dataNestedSet'; import 'jest-canvas-mock'; @@ -29,12 +29,12 @@ describe('FlameGraph', () => { const [selectedView, _] = useState(SelectedView.Both); const flameGraphData = new MutableDataFrame(data); - const dataView = new DataFrameView(flameGraphData); - const levels = nestedSetToLevels(dataView); + const container = new FlameGraphDataContainer(flameGraphData); + const levels = nestedSetToLevels(container); return ( { setRangeMin={setRangeMin} setRangeMax={setRangeMax} selectedView={selectedView} - getLabelValue={(val) => val.toString()} /> ); }; diff --git a/public/app/plugins/panel/flamegraph/components/FlameGraph/FlameGraph.tsx b/public/app/plugins/panel/flamegraph/components/FlameGraph/FlameGraph.tsx index 5fc11b97a15..ed381659a5f 100644 --- a/public/app/plugins/panel/flamegraph/components/FlameGraph/FlameGraph.tsx +++ b/public/app/plugins/panel/flamegraph/components/FlameGraph/FlameGraph.tsx @@ -21,22 +21,22 @@ import uFuzzy from '@leeoniya/ufuzzy'; import React, { useEffect, useMemo, useRef, useState } from 'react'; import { useMeasure } from 'react-use'; -import { CoreApp, createTheme, DataFrame, FieldType, getDisplayProcessor } from '@grafana/data'; +import { CoreApp } from '@grafana/data'; import { PIXELS_PER_LEVEL } from '../../constants'; -import { TooltipData, SelectedView, ContextMenuData } from '../types'; +import { SelectedView, ContextMenuData } from '../types'; import FlameGraphContextMenu from './FlameGraphContextMenu'; import FlameGraphMetadata from './FlameGraphMetadata'; -import FlameGraphTooltip, { getTooltipData } from './FlameGraphTooltip'; -import { ItemWithStart } from './dataTransform'; +import FlameGraphTooltip from './FlameGraphTooltip'; +import { FlameGraphDataContainer, LevelItem } from './dataTransform'; import { getBarX, getRectDimensionsForLevel, renderRect } from './rendering'; type Props = { - data: DataFrame; + data: FlameGraphDataContainer; app: CoreApp; flameGraphHeight?: number; - levels: ItemWithStart[][]; + levels: LevelItem[][]; topLevelIndex: number; selectedBarIndex: number; rangeMin: number; @@ -48,7 +48,6 @@ type Props = { setRangeMax: (range: number) => void; selectedView: SelectedView; style?: React.CSSProperties; - getLabelValue: (label: string | number) => string; }; const FlameGraph = ({ @@ -66,52 +65,35 @@ const FlameGraph = ({ setRangeMin, setRangeMax, selectedView, - getLabelValue, }: Props) => { const styles = getStyles(selectedView, app, flameGraphHeight); - const totalTicks = data.fields[1].values.get(0); - const valueField = - data.fields.find((f) => f.name === 'value') ?? data.fields.find((f) => f.type === FieldType.number); - - if (!valueField) { - throw new Error('Malformed dataFrame: value field of type number is not in the query response'); - } + const totalTicks = data.getValue(0); const [sizeRef, { width: wrapperWidth }] = useMeasure(); const graphRef = useRef(null); const tooltipRef = useRef(null); - const [tooltipData, setTooltipData] = useState(); + const [tooltipItem, setTooltipItem] = useState(); const [contextMenuData, setContextMenuData] = useState(); const [ufuzzy] = useState(() => { return new uFuzzy(); }); - const uniqueLabels = useMemo(() => { - const labelField = data.fields.find((f) => f.name === 'label'); - const enumConfig = labelField?.config?.type?.enum; - if (enumConfig) { - return enumConfig.text || []; - } else { - return [...new Set(labelField?.values.toArray())]; - } - }, [data]); - const foundLabels = useMemo(() => { const foundLabels = new Set(); if (search) { - let idxs = ufuzzy.filter(uniqueLabels, search); + let idxs = ufuzzy.filter(data.getUniqueLabels(), search); if (idxs) { for (let idx of idxs) { - foundLabels.add(uniqueLabels[idx]); + foundLabels.add(data.getUniqueLabels()[idx]); } } } return foundLabels; - }, [ufuzzy, search, uniqueLabels]); + }, [ufuzzy, search, data]); useEffect(() => { if (!levels.length) { @@ -131,48 +113,25 @@ const FlameGraph = ({ ctx.font = 12 * window.devicePixelRatio + 'px monospace'; ctx.strokeStyle = 'white'; - const processor = getDisplayProcessor({ - field: valueField, - theme: createTheme() /* theme does not matter for us here */, - }); - for (let levelIndex = 0; levelIndex < levels.length; levelIndex++) { const level = levels[levelIndex]; // Get all the dimensions of the rectangles for the level. We do this by level instead of per rectangle, because // sometimes we collapse multiple bars into single rect. - const dimensions = getRectDimensionsForLevel( - level, - levelIndex, - totalTicks, - rangeMin, - pixelsPerTick, - processor, - getLabelValue - ); + const dimensions = getRectDimensionsForLevel(data, level, levelIndex, totalTicks, rangeMin, pixelsPerTick); for (const rect of dimensions) { // Render each rectangle based on the computed dimensions renderRect(ctx, rect, totalTicks, rangeMin, rangeMax, search, levelIndex, topLevelIndex, foundLabels); } } - }, [ - levels, - wrapperWidth, - valueField, - totalTicks, - rangeMin, - rangeMax, - search, - topLevelIndex, - foundLabels, - getLabelValue, - ]); + }, [data, levels, wrapperWidth, totalTicks, rangeMin, rangeMax, search, topLevelIndex, foundLabels]); useEffect(() => { if (graphRef.current) { graphRef.current.onclick = (e) => { - setTooltipData(undefined); + setTooltipItem(undefined); const pixelsPerTick = graphRef.current!.clientWidth / totalTicks / (rangeMax - rangeMin); const { levelIndex, barIndex } = convertPixelCoordinatesToBarCoordinates( + data, e, pixelsPerTick, levels, @@ -191,9 +150,10 @@ const FlameGraph = ({ graphRef.current!.onmousemove = (e) => { if (tooltipRef.current && contextMenuData === undefined) { - setTooltipData(undefined); + setTooltipItem(undefined); const pixelsPerTick = graphRef.current!.clientWidth / totalTicks / (rangeMax - rangeMin); const { levelIndex, barIndex } = convertPixelCoordinatesToBarCoordinates( + data, e, pixelsPerTick, levels, @@ -204,19 +164,17 @@ const FlameGraph = ({ if (barIndex !== -1 && !isNaN(levelIndex) && !isNaN(barIndex)) { tooltipRef.current.style.left = e.clientX + 10 + 'px'; tooltipRef.current.style.top = e.clientY + 'px'; - - const bar = levels[levelIndex][barIndex]; - const tooltipData = getTooltipData(valueField, bar.label, bar.value, bar.self, totalTicks); - setTooltipData(tooltipData); + setTooltipItem(levels[levelIndex][barIndex]); } } }; graphRef.current!.onmouseleave = () => { - setTooltipData(undefined); + setTooltipItem(undefined); }; } }, [ + data, levels, rangeMin, rangeMax, @@ -227,13 +185,12 @@ const FlameGraph = ({ setRangeMin, setRangeMax, selectedView, - valueField, setSelectedBarIndex, setContextMenuData, contextMenuData, ]); - // hide context menu if outside of the flame graph canvas is clicked + // hide context menu if outside the flame graph canvas is clicked useEffect(() => { const handleOnClick = (e: MouseEvent) => { // eslint-disable-next-line @typescript-eslint/consistent-type-assertions @@ -248,18 +205,19 @@ const FlameGraph = ({ return (
- + {contextMenuData && ( )}
@@ -293,14 +250,15 @@ const getStyles = (selectedView: SelectedView, app: CoreApp, flameGraphHeight: n // Convert pixel coordinates to bar coordinates in the levels array so that we can add mouse events like clicks to // the canvas. const convertPixelCoordinatesToBarCoordinates = ( + data: FlameGraphDataContainer, e: MouseEvent, pixelsPerTick: number, - levels: ItemWithStart[][], + levels: LevelItem[][], totalTicks: number, rangeMin: number ) => { const levelIndex = Math.floor(e.offsetY / (PIXELS_PER_LEVEL / window.devicePixelRatio)); - const barIndex = getBarIndex(e.offsetX, levels[levelIndex], pixelsPerTick, totalTicks, rangeMin); + const barIndex = getBarIndex(e.offsetX, data, levels[levelIndex], pixelsPerTick, totalTicks, rangeMin); return { levelIndex, barIndex }; }; @@ -310,7 +268,8 @@ const convertPixelCoordinatesToBarCoordinates = ( */ const getBarIndex = ( x: number, - level: ItemWithStart[], + data: FlameGraphDataContainer, + level: LevelItem[], pixelsPerTick: number, totalTicks: number, rangeMin: number @@ -323,7 +282,7 @@ const getBarIndex = ( const midIndex = (start + end) >> 1; const startOfBar = getBarX(level[midIndex].start, totalTicks, rangeMin, pixelsPerTick); const startOfNextBar = getBarX( - level[midIndex].start + level[midIndex].value, + level[midIndex].start + data.getValue(level[midIndex].itemIndex), totalTicks, rangeMin, pixelsPerTick diff --git a/public/app/plugins/panel/flamegraph/components/FlameGraph/FlameGraphContextMenu.tsx b/public/app/plugins/panel/flamegraph/components/FlameGraph/FlameGraphContextMenu.tsx index c61a5f3fb28..886803bcac2 100644 --- a/public/app/plugins/panel/flamegraph/components/FlameGraph/FlameGraphContextMenu.tsx +++ b/public/app/plugins/panel/flamegraph/components/FlameGraph/FlameGraphContextMenu.tsx @@ -4,11 +4,12 @@ import { MenuItem, ContextMenu } from '@grafana/ui'; import { ContextMenuData } from '../types'; -import { ItemWithStart } from './dataTransform'; +import { FlameGraphDataContainer, LevelItem } from './dataTransform'; type Props = { contextMenuData: ContextMenuData; - levels: ItemWithStart[][]; + data: FlameGraphDataContainer; + levels: LevelItem[][]; totalTicks: number; graphRef: React.RefObject; setContextMenuData: (event: ContextMenuData | undefined) => void; @@ -16,7 +17,6 @@ type Props = { setSelectedBarIndex: (bar: number) => void; setRangeMin: (range: number) => void; setRangeMax: (range: number) => void; - getLabelValue: (label: string | number) => string; }; const FlameGraphContextMenu = ({ @@ -29,8 +29,10 @@ const FlameGraphContextMenu = ({ setSelectedBarIndex, setRangeMin, setRangeMax, - getLabelValue, + data, }: Props) => { + const clickedItem = levels[contextMenuData.levelIndex][contextMenuData.barIndex]; + const renderMenuItems = () => { return ( <> @@ -41,12 +43,8 @@ const FlameGraphContextMenu = ({ if (graphRef.current && contextMenuData) { setTopLevelIndex(contextMenuData.levelIndex); setSelectedBarIndex(contextMenuData.barIndex); - setRangeMin(levels[contextMenuData.levelIndex][contextMenuData.barIndex].start / totalTicks); - setRangeMax( - (levels[contextMenuData.levelIndex][contextMenuData.barIndex].start + - levels[contextMenuData.levelIndex][contextMenuData.barIndex].value) / - totalTicks - ); + setRangeMin(clickedItem.start / totalTicks); + setRangeMax((clickedItem.start + data.getValue(clickedItem.itemIndex)) / totalTicks); setContextMenuData(undefined); } }} @@ -56,8 +54,7 @@ const FlameGraphContextMenu = ({ icon={'copy'} onClick={() => { if (graphRef.current && contextMenuData) { - const bar = levels[contextMenuData.levelIndex][contextMenuData.barIndex]; - navigator.clipboard.writeText(getLabelValue(bar.label)).then(() => { + navigator.clipboard.writeText(data.getLabel(clickedItem.itemIndex)).then(() => { setContextMenuData(undefined); }); } diff --git a/public/app/plugins/panel/flamegraph/components/FlameGraph/FlameGraphMetadata.test.ts b/public/app/plugins/panel/flamegraph/components/FlameGraph/FlameGraphMetadata.test.ts index 9c38490df90..2c1c2dcde10 100644 --- a/public/app/plugins/panel/flamegraph/components/FlameGraph/FlameGraphMetadata.test.ts +++ b/public/app/plugins/panel/flamegraph/components/FlameGraph/FlameGraphMetadata.test.ts @@ -1,10 +1,28 @@ -import { ArrayVector, Field, FieldType } from '@grafana/data'; +import { MutableDataFrame } from '@grafana/data'; import { getMetadata } from './FlameGraphMetadata'; +import { FlameGraphDataContainer } from './dataTransform'; + +function makeDataFrame(fields: Record>, unit?: string) { + return new MutableDataFrame({ + fields: Object.keys(fields).map((key) => ({ + name: key, + values: fields[key], + config: unit + ? { + unit, + } + : {}, + })), + }); +} describe('should get metadata correctly', () => { it('for bytes', () => { - const metadata = getMetadata(makeField('bytes'), 1_624_078_250, 8_624_078_250); + const container = new FlameGraphDataContainer( + makeDataFrame({ value: [1_624_078_250], level: [1], label: ['1'], self: [0] }, 'bytes') + ); + const metadata = getMetadata(container, { itemIndex: 0, start: 0 }, 8_624_078_250); expect(metadata).toEqual({ percentValue: 18.83, unitTitle: 'RAM', @@ -14,7 +32,10 @@ describe('should get metadata correctly', () => { }); it('with default unit', () => { - const metadata = getMetadata(makeField('none'), 1_624_078_250, 8_624_078_250); + const container = new FlameGraphDataContainer( + makeDataFrame({ value: [1_624_078_250], level: [1], label: ['1'], self: [0] }, 'none') + ); + const metadata = getMetadata(container, { itemIndex: 0, start: 0 }, 8_624_078_250); expect(metadata).toEqual({ percentValue: 18.83, unitTitle: 'Count', @@ -24,16 +45,10 @@ describe('should get metadata correctly', () => { }); it('without unit', () => { - const metadata = getMetadata( - { - name: 'test', - type: FieldType.number, - values: new ArrayVector(), - config: {}, - }, - 1_624_078_250, - 8_624_078_250 + const container = new FlameGraphDataContainer( + makeDataFrame({ value: [1_624_078_250], level: [1], label: ['1'], self: [0] }) ); + const metadata = getMetadata(container, { itemIndex: 0, start: 0 }, 8_624_078_250); expect(metadata).toEqual({ percentValue: 18.83, unitTitle: 'Count', @@ -43,7 +58,10 @@ describe('should get metadata correctly', () => { }); it('for objects', () => { - const metadata = getMetadata(makeField('short'), 1_624_078_250, 8_624_078_250); + const container = new FlameGraphDataContainer( + makeDataFrame({ value: [1_624_078_250], level: [1], label: ['1'], self: [0] }, 'short') + ); + const metadata = getMetadata(container, { itemIndex: 0, start: 0 }, 8_624_078_250); expect(metadata).toEqual({ percentValue: 18.83, unitTitle: 'Count', @@ -53,7 +71,10 @@ describe('should get metadata correctly', () => { }); it('for nanoseconds', () => { - const metadata = getMetadata(makeField('ns'), 1_624_078_250, 8_624_078_250); + const container = new FlameGraphDataContainer( + makeDataFrame({ value: [1_624_078_250], level: [1], label: ['1'], self: [0] }, 'ns') + ); + const metadata = getMetadata(container, { itemIndex: 0, start: 0 }, 8_624_078_250); expect(metadata).toEqual({ percentValue: 18.83, unitTitle: 'Time', @@ -62,14 +83,3 @@ describe('should get metadata correctly', () => { }); }); }); - -function makeField(unit: string): Field { - return { - name: 'test', - type: FieldType.number, - config: { - unit, - }, - values: new ArrayVector(), - }; -} diff --git a/public/app/plugins/panel/flamegraph/components/FlameGraph/FlameGraphMetadata.tsx b/public/app/plugins/panel/flamegraph/components/FlameGraph/FlameGraphMetadata.tsx index ca0b6dab9db..6569db110ae 100644 --- a/public/app/plugins/panel/flamegraph/components/FlameGraph/FlameGraphMetadata.tsx +++ b/public/app/plugins/panel/flamegraph/components/FlameGraph/FlameGraphMetadata.tsx @@ -1,53 +1,42 @@ import { css } from '@emotion/css'; import React from 'react'; -import { createTheme, Field, getDisplayProcessor, Vector } from '@grafana/data'; import { useStyles2 } from '@grafana/ui'; -import { Metadata, SampleUnit } from '../types'; +import { Metadata } from '../types'; -import { ItemWithStart } from './dataTransform'; +import { FlameGraphDataContainer, LevelItem } from './dataTransform'; type Props = { - levels: ItemWithStart[][]; + data: FlameGraphDataContainer; + levels: LevelItem[][]; topLevelIndex: number; selectedBarIndex: number; - valueField: Field>; totalTicks: number; }; -const FlameGraphMetadata = React.memo(({ levels, topLevelIndex, selectedBarIndex, valueField, totalTicks }: Props) => { +const FlameGraphMetadata = React.memo(({ data, levels, topLevelIndex, selectedBarIndex, totalTicks }: Props) => { const styles = useStyles2(getStyles); if (levels[topLevelIndex] && levels[topLevelIndex][selectedBarIndex]) { const bar = levels[topLevelIndex][selectedBarIndex]; - const metadata = getMetadata(valueField, bar.value, totalTicks); + const metadata = getMetadata(data, bar, totalTicks); const metadataText = `${metadata?.unitValue} (${metadata?.percentValue}%) of ${metadata?.samples} total samples (${metadata?.unitTitle})`; return <>{
{metadataText}
}; } return <>; }); -export const getMetadata = (field: Field, value: number, totalTicks: number): Metadata => { - let unitTitle; - const processor = getDisplayProcessor({ field, theme: createTheme() /* theme does not matter for us here */ }); - const displayValue = processor(value); - const percentValue = Math.round(10000 * (value / totalTicks)) / 100; +export const getMetadata = (data: FlameGraphDataContainer, bar: LevelItem, totalTicks: number): Metadata => { + const displayValue = data.getValueDisplay(bar.itemIndex); + const percentValue = Math.round(10000 * (displayValue.numeric / totalTicks)) / 100; let unitValue = displayValue.text + displayValue.suffix; - switch (field.config.unit) { - case SampleUnit.Bytes: - unitTitle = 'RAM'; - break; - case SampleUnit.Nanoseconds: - unitTitle = 'Time'; - break; - default: - unitTitle = 'Count'; - if (!displayValue.suffix) { - // Makes sure we don't show 123undefined or something like that if suffix isn't defined - unitValue = displayValue.text; - } - break; + const unitTitle = data.getUnitTitle(); + if (unitTitle === 'Count') { + if (!displayValue.suffix) { + // Makes sure we don't show 123undefined or something like that if suffix isn't defined + unitValue = displayValue.text; + } } return { diff --git a/public/app/plugins/panel/flamegraph/components/FlameGraph/FlameGraphTooltip.test.ts b/public/app/plugins/panel/flamegraph/components/FlameGraph/FlameGraphTooltip.test.tsx similarity index 54% rename from public/app/plugins/panel/flamegraph/components/FlameGraph/FlameGraphTooltip.test.ts rename to public/app/plugins/panel/flamegraph/components/FlameGraph/FlameGraphTooltip.test.tsx index e21266b6ba5..b511d3af2bc 100644 --- a/public/app/plugins/panel/flamegraph/components/FlameGraph/FlameGraphTooltip.test.ts +++ b/public/app/plugins/panel/flamegraph/components/FlameGraph/FlameGraphTooltip.test.tsx @@ -1,10 +1,23 @@ -import { ArrayVector, Field, FieldType } from '@grafana/data'; +import { ArrayVector, Field, FieldType, MutableDataFrame } from '@grafana/data'; import { getTooltipData } from './FlameGraphTooltip'; +import { FlameGraphDataContainer } from './dataTransform'; -describe('should get tooltip data correctly', () => { +function setupData(unit?: string) { + const flameGraphData = new MutableDataFrame({ + fields: [ + { name: 'level', values: [0] }, + unit ? makeField('value', unit, [8_624_078_250]) : { name: 'value', values: [8_624_078_250] }, + { name: 'self', values: [978_250] }, + { name: 'label', values: ['total'] }, + ], + }); + return new FlameGraphDataContainer(flameGraphData); +} + +describe('FlameGraphTooltip', () => { it('for bytes', () => { - const tooltipData = getTooltipData(makeField('bytes'), 'total', 8_624_078_250, 978_250, 8_624_078_250); + const tooltipData = getTooltipData(setupData('bytes'), { start: 0, itemIndex: 0 }, 8_624_078_250); expect(tooltipData).toEqual({ name: 'total', percentSelf: 0.01, @@ -17,7 +30,7 @@ describe('should get tooltip data correctly', () => { }); it('with default unit', () => { - const tooltipData = getTooltipData(makeField('none'), 'total', 8_624_078_250, 978_250, 8_624_078_250); + const tooltipData = getTooltipData(setupData('none'), { start: 0, itemIndex: 0 }, 8_624_078_250); expect(tooltipData).toEqual({ name: 'total', percentSelf: 0.01, @@ -30,18 +43,7 @@ describe('should get tooltip data correctly', () => { }); it('without unit', () => { - const tooltipData = getTooltipData( - { - name: 'test', - type: FieldType.number, - values: new ArrayVector(), - config: {}, - }, - 'total', - 8_624_078_250, - 978_250, - 8_624_078_250 - ); + const tooltipData = getTooltipData(setupData('none'), { start: 0, itemIndex: 0 }, 8_624_078_250); expect(tooltipData).toEqual({ name: 'total', percentSelf: 0.01, @@ -54,7 +56,7 @@ describe('should get tooltip data correctly', () => { }); it('for objects', () => { - const tooltipData = getTooltipData(makeField('short'), 'total', 8_624_078_250, 978_250, 8_624_078_250); + const tooltipData = getTooltipData(setupData('short'), { start: 0, itemIndex: 0 }, 8_624_078_250); expect(tooltipData).toEqual({ name: 'total', percentSelf: 0.01, @@ -67,7 +69,7 @@ describe('should get tooltip data correctly', () => { }); it('for nanoseconds', () => { - const tooltipData = getTooltipData(makeField('ns'), 'total', 8_624_078_250, 978_250, 8_624_078_250); + const tooltipData = getTooltipData(setupData('ns'), { start: 0, itemIndex: 0 }, 8_624_078_250); expect(tooltipData).toEqual({ name: 'total', percentSelf: 0.01, @@ -80,13 +82,13 @@ describe('should get tooltip data correctly', () => { }); }); -function makeField(unit: string): Field { +function makeField(name: string, unit: string, values: number[]): Field { return { - name: 'test', + name, type: FieldType.number, config: { unit, }, - values: new ArrayVector(), + values: new ArrayVector(values), }; } diff --git a/public/app/plugins/panel/flamegraph/components/FlameGraph/FlameGraphTooltip.tsx b/public/app/plugins/panel/flamegraph/components/FlameGraph/FlameGraphTooltip.tsx index 3b92818f16e..55811867d10 100644 --- a/public/app/plugins/panel/flamegraph/components/FlameGraph/FlameGraphTooltip.tsx +++ b/public/app/plugins/panel/flamegraph/components/FlameGraph/FlameGraphTooltip.tsx @@ -1,94 +1,95 @@ import { css } from '@emotion/css'; import React, { LegacyRef } from 'react'; -import { createTheme, Field, getDisplayProcessor } from '@grafana/data'; import { useStyles2, Tooltip } from '@grafana/ui'; -import { TooltipData, SampleUnit } from '../types'; +import { FlameGraphDataContainer, LevelItem } from './dataTransform'; type Props = { - tooltipRef: LegacyRef; - tooltipData: TooltipData; - getLabelValue: (label: string | number) => string; + data: FlameGraphDataContainer; + totalTicks: number; + item?: LevelItem; + tooltipRef?: LegacyRef; }; -const FlameGraphTooltip = ({ tooltipRef, tooltipData, getLabelValue }: Props) => { +const FlameGraphTooltip = ({ data, tooltipRef, item, totalTicks }: Props) => { const styles = useStyles2(getStyles); + let content = null; + if (item) { + const tooltipData = getTooltipData(data, item, totalTicks); + content = ( + +

{data.getLabel(item.itemIndex)}

+

+ {tooltipData.unitTitle} +
+ Total: {tooltipData.unitValue} ({tooltipData.percentValue}%) +
+ Self: {tooltipData.unitSelf} ({tooltipData.percentSelf}%) +
+ Samples: {tooltipData.samples} +

+ + } + placement={'right'} + show={true} + > + + + ); + } + + // Even if we don't show tooltip we need this div so the ref is consistently attached. Would need some refactor in + // FlameGraph.tsx to make it work without it. return (
- {tooltipData && ( - -

{getLabelValue(tooltipData.name)}

-

- {tooltipData.unitTitle} -
- Total: {tooltipData.unitValue} ({tooltipData.percentValue}%) -
- Self: {tooltipData.unitSelf} ({tooltipData.percentSelf}%) -
- Samples: {tooltipData.samples} -

-
- } - placement={'right'} - show={true} - > - - - )} + {content} ); }; -export const getTooltipData = ( - field: Field, - label: string, - value: number, - self: number, - totalTicks: number -): TooltipData => { - let unitTitle; +type TooltipData = { + name: string; + percentValue: number; + percentSelf: number; + unitTitle: string; + unitValue: string; + unitSelf: string; + samples: string; +}; - const processor = getDisplayProcessor({ field, theme: createTheme() /* theme does not matter for us here */ }); - const displayValue = processor(value); - const displaySelf = processor(self); +export const getTooltipData = (data: FlameGraphDataContainer, item: LevelItem, totalTicks: number): TooltipData => { + const displayValue = data.getValueDisplay(item.itemIndex); + const displaySelf = data.getSelfDisplay(item.itemIndex); - const percentValue = Math.round(10000 * (value / totalTicks)) / 100; - const percentSelf = Math.round(10000 * (self / totalTicks)) / 100; + const percentValue = Math.round(10000 * (displayValue.numeric / totalTicks)) / 100; + const percentSelf = Math.round(10000 * (displaySelf.numeric / totalTicks)) / 100; let unitValue = displayValue.text + displayValue.suffix; let unitSelf = displaySelf.text + displaySelf.suffix; - switch (field.config.unit) { - case SampleUnit.Bytes: - unitTitle = 'RAM'; - break; - case SampleUnit.Nanoseconds: - unitTitle = 'Time'; - break; - default: - unitTitle = 'Count'; - if (!displayValue.suffix) { - // Makes sure we don't show 123undefined or something like that if suffix isn't defined - unitValue = displayValue.text; - } - if (!displaySelf.suffix) { - // Makes sure we don't show 123undefined or something like that if suffix isn't defined - unitSelf = displaySelf.text; - } - break; + const unitTitle = data.getUnitTitle(); + if (unitTitle === 'Count') { + if (!displayValue.suffix) { + // Makes sure we don't show 123undefined or something like that if suffix isn't defined + unitValue = displayValue.text; + } + if (!displaySelf.suffix) { + // Makes sure we don't show 123undefined or something like that if suffix isn't defined + unitSelf = displaySelf.text; + } } return { - name: label, + name: data.getLabel(item.itemIndex), percentValue, percentSelf, unitTitle, unitValue, unitSelf, - samples: value.toLocaleString(), + samples: displayValue.numeric.toLocaleString(), }; }; diff --git a/public/app/plugins/panel/flamegraph/components/FlameGraph/dataTransform.test.ts b/public/app/plugins/panel/flamegraph/components/FlameGraph/dataTransform.test.ts index 56948df7b78..f6588c98de5 100644 --- a/public/app/plugins/panel/flamegraph/components/FlameGraph/dataTransform.test.ts +++ b/public/app/plugins/panel/flamegraph/components/FlameGraph/dataTransform.test.ts @@ -1,6 +1,6 @@ -import { DataFrameView, MutableDataFrame } from '@grafana/data'; +import { MutableDataFrame } from '@grafana/data'; -import { Item, nestedSetToLevels } from './dataTransform'; +import { FlameGraphDataContainer, nestedSetToLevels } from './dataTransform'; describe('nestedSetToLevels', () => { it('converts nested set data frame to levels', () => { @@ -9,25 +9,26 @@ describe('nestedSetToLevels', () => { { name: 'level', values: [0, 1, 2, 3, 2, 1, 2, 3, 4] }, { name: 'value', values: [10, 5, 3, 1, 1, 4, 3, 2, 1] }, { name: 'label', values: ['1', '2', '3', '4', '5', '6', '7', '8', '9'] }, + { name: 'self', values: [0, 0, 0, 0, 0, 0, 0, 0, 0] }, ], }); - const levels = nestedSetToLevels(new DataFrameView(frame)); + const levels = nestedSetToLevels(new FlameGraphDataContainer(frame)); expect(levels).toEqual([ - [{ level: 0, value: 10, start: 0, label: '1' }], + [{ start: 0, itemIndex: 0 }], [ - { level: 1, value: 5, start: 0, label: '2' }, - { level: 1, value: 4, start: 5, label: '6' }, + { start: 0, itemIndex: 1 }, + { start: 5, itemIndex: 5 }, ], [ - { level: 2, value: 3, start: 0, label: '3' }, - { level: 2, value: 1, start: 3, label: '5' }, - { level: 2, value: 3, start: 5, label: '7' }, + { start: 0, itemIndex: 2 }, + { start: 3, itemIndex: 4 }, + { start: 5, itemIndex: 6 }, ], [ - { level: 3, value: 1, start: 0, label: '4' }, - { level: 3, value: 2, start: 5, label: '8' }, + { start: 0, itemIndex: 3 }, + { start: 5, itemIndex: 7 }, ], - [{ level: 4, value: 1, start: 5, label: '9' }], + [{ start: 5, itemIndex: 8 }], ]); }); @@ -37,15 +38,16 @@ describe('nestedSetToLevels', () => { { name: 'level', values: [0, 1, 1, 1] }, { name: 'value', values: [10, 5, 3, 1] }, { name: 'label', values: ['1', '2', '3', '4'] }, + { name: 'self', values: [10, 5, 3, 1] }, ], }); - const levels = nestedSetToLevels(new DataFrameView(frame)); + const levels = nestedSetToLevels(new FlameGraphDataContainer(frame)); expect(levels).toEqual([ - [{ level: 0, value: 10, start: 0, label: '1' }], + [{ start: 0, itemIndex: 0 }], [ - { level: 1, value: 5, start: 0, label: '2' }, - { level: 1, value: 3, start: 5, label: '3' }, - { level: 1, value: 1, start: 8, label: '4' }, + { start: 0, itemIndex: 1 }, + { start: 5, itemIndex: 2 }, + { start: 8, itemIndex: 3 }, ], ]); }); diff --git a/public/app/plugins/panel/flamegraph/components/FlameGraph/dataTransform.ts b/public/app/plugins/panel/flamegraph/components/FlameGraph/dataTransform.ts index fc21ecbe7a3..8bd8e84f0e7 100644 --- a/public/app/plugins/panel/flamegraph/components/FlameGraph/dataTransform.ts +++ b/public/app/plugins/panel/flamegraph/components/FlameGraph/dataTransform.ts @@ -1,35 +1,125 @@ -import { DataFrameView } from '@grafana/data'; +import { + createTheme, + DataFrame, + DisplayProcessor, + Field, + getDisplayProcessor, + getEnumDisplayProcessor, + GrafanaTheme2, +} from '@grafana/data'; -export type Item = { level: number; value: number; label: string; self: number }; -export type ItemWithStart = Item & { start: number }; +import { SampleUnit } from '../types'; + +export type LevelItem = { start: number; itemIndex: number }; /** * Convert data frame with nested set format into array of level. This is mainly done for compatibility with current * rendering code. - * @param dataView */ -export function nestedSetToLevels(dataView: DataFrameView): ItemWithStart[][] { - const levels: ItemWithStart[][] = []; +export function nestedSetToLevels(container: FlameGraphDataContainer): LevelItem[][] { + const levels: LevelItem[][] = []; let offset = 0; - for (let i = 0; i < dataView.length; i++) { - // We have to clone the items as .get(i) returns a changing pointer not the data themselves. - const item = { ...dataView.get(i) }; - const prevItem = i > 0 ? { ...dataView.get(i - 1) } : undefined; + for (let i = 0; i < container.data.length; i++) { + const currentLevel = container.getLevel(i); + const prevLevel = i > 0 ? container.getLevel(i - 1) : undefined; - levels[item.level] = levels[item.level] || []; - if (prevItem && prevItem.level >= item.level) { - // We are going down a level or staying at the same level so we are adding a sibling to the last item in a level. + levels[currentLevel] = levels[currentLevel] || []; + if (prevLevel && prevLevel >= currentLevel) { + // We are going down a level or staying at the same level, so we are adding a sibling to the last item in a level. // So we have to compute the correct offset based on the last sibling. - const lastItem = levels[item.level][levels[item.level].length - 1]; - offset = lastItem.start + lastItem.value; + const lastItem = levels[currentLevel][levels[currentLevel].length - 1]; + offset = lastItem.start + container.getValue(lastItem.itemIndex); } - const newItem: ItemWithStart = { - ...item, + const newItem: LevelItem = { + itemIndex: i, start: offset, }; - levels[item.level].push(newItem); + levels[currentLevel].push(newItem); } return levels; } + +export class FlameGraphDataContainer { + data: DataFrame; + labelField: Field; + levelField: Field; + valueField: Field; + selfField: Field; + + labelDisplayProcessor: DisplayProcessor; + valueDisplayProcessor: DisplayProcessor; + uniqueLabels: string[]; + + constructor(data: DataFrame, theme: GrafanaTheme2 = createTheme()) { + this.data = data; + this.labelField = data.fields.find((f) => f.name === 'label')!; + this.levelField = data.fields.find((f) => f.name === 'level')!; + this.valueField = data.fields.find((f) => f.name === 'value')!; + this.selfField = data.fields.find((f) => f.name === 'self')!; + + if (!(this.labelField && this.levelField && this.valueField && this.selfField)) { + throw new Error('Malformed dataFrame: value, level and label and self fields are required.'); + } + + const enumConfig = this.labelField?.config?.type?.enum; + // Label can actually be an enum field so depending on that we have to access it through display processor. This is + // both a backward compatibility but also to allow using a simple dataFrame without enum config. This would allow + // users to use this panel with correct query from data sources that do not return profiles natively. + if (enumConfig) { + this.labelDisplayProcessor = getEnumDisplayProcessor(theme, enumConfig); + this.uniqueLabels = enumConfig.text || []; + } else { + this.labelDisplayProcessor = (value) => ({ + text: value + '', + numeric: 0, + }); + this.uniqueLabels = [...new Set(this.labelField.values.toArray())]; + } + + this.valueDisplayProcessor = getDisplayProcessor({ + field: this.valueField, + theme, + }); + } + + getLabel(index: number) { + return this.labelDisplayProcessor(this.labelField.values.get(index)).text; + } + + getLevel(index: number) { + return this.levelField.values.get(index); + } + + getValue(index: number) { + return this.valueField.values.get(index); + } + + getValueDisplay(index: number) { + return this.valueDisplayProcessor(this.valueField.values.get(index)); + } + + getSelf(index: number) { + return this.selfField.values.get(index); + } + + getSelfDisplay(index: number) { + return this.valueDisplayProcessor(this.selfField.values.get(index)); + } + + getUniqueLabels() { + return this.uniqueLabels; + } + + getUnitTitle() { + switch (this.valueField.config.unit) { + case SampleUnit.Bytes: + return 'RAM'; + case SampleUnit.Nanoseconds: + return 'Time'; + } + + return 'Count'; + } +} diff --git a/public/app/plugins/panel/flamegraph/components/FlameGraph/rendering.test.ts b/public/app/plugins/panel/flamegraph/components/FlameGraph/rendering.test.ts index 6b476e0f3f0..8a86199f505 100644 --- a/public/app/plugins/panel/flamegraph/components/FlameGraph/rendering.test.ts +++ b/public/app/plugins/panel/flamegraph/components/FlameGraph/rendering.test.ts @@ -1,20 +1,22 @@ -import { createTheme, getDisplayProcessor } from '@grafana/data'; +import { MutableDataFrame } from '@grafana/data'; -import { ItemWithStart } from './dataTransform'; +import { FlameGraphDataContainer, LevelItem } from './dataTransform'; import { getRectDimensionsForLevel } from './rendering'; +function makeDataFrame(fields: Record>) { + return new MutableDataFrame({ + fields: Object.keys(fields).map((key) => ({ + name: key, + values: fields[key], + })), + }); +} + describe('getRectDimensionsForLevel', () => { it('should render a single item', () => { - const level: ItemWithStart[] = [{ level: 1, start: 0, value: 100, label: '1', self: 0 }]; - const result = getRectDimensionsForLevel( - level, - 1, - 100, - 0, - 10, - getDisplayProcessor({ field: { config: {} }, theme: createTheme() }), - (val) => val.toString() - ); + const level: LevelItem[] = [{ start: 0, itemIndex: 0 }]; + const container = new FlameGraphDataContainer(makeDataFrame({ value: [100], level: [1], label: ['1'], self: [0] })); + const result = getRectDimensionsForLevel(container, level, 1, 100, 0, 10); expect(result).toEqual([ { width: 999, @@ -30,20 +32,15 @@ describe('getRectDimensionsForLevel', () => { }); it('should render a multiple items', () => { - const level: ItemWithStart[] = [ - { level: 2, start: 0, value: 100, label: '1', self: 0 }, - { level: 2, start: 100, value: 50, label: '2', self: 0 }, - { level: 2, start: 150, value: 50, label: '3', self: 0 }, + const level: LevelItem[] = [ + { start: 0, itemIndex: 0 }, + { start: 100, itemIndex: 1 }, + { start: 150, itemIndex: 2 }, ]; - const result = getRectDimensionsForLevel( - level, - 2, - 100, - 0, - 10, - getDisplayProcessor({ field: { config: {} }, theme: createTheme() }), - (val) => val.toString() + const container = new FlameGraphDataContainer( + makeDataFrame({ value: [100, 50, 50], level: [2, 2, 2], label: ['1', '2', '3'], self: [0, 0, 0] }) ); + const result = getRectDimensionsForLevel(container, level, 2, 100, 0, 10); expect(result).toEqual([ { width: 999, height: 22, x: 0, y: 44, collapsed: false, ticks: 100, label: '1', unitLabel: '100' }, { width: 499, height: 22, x: 1000, y: 44, collapsed: false, ticks: 50, label: '2', unitLabel: '50' }, @@ -52,20 +49,15 @@ describe('getRectDimensionsForLevel', () => { }); it('should render a collapsed items', () => { - const level: ItemWithStart[] = [ - { level: 2, start: 0, value: 100, label: '1', self: 0 }, - { level: 2, start: 100, value: 2, label: '2', self: 0 }, - { level: 2, start: 102, value: 1, label: '3', self: 0 }, + const level: LevelItem[] = [ + { start: 0, itemIndex: 0 }, + { start: 100, itemIndex: 1 }, + { start: 102, itemIndex: 2 }, ]; - const result = getRectDimensionsForLevel( - level, - 2, - 100, - 0, - 1, - getDisplayProcessor({ field: { config: {} }, theme: createTheme() }), - (val) => val.toString() + const container = new FlameGraphDataContainer( + makeDataFrame({ value: [100, 2, 1], level: [2, 2, 2], label: ['1', '2', '3'], self: [0, 0, 0] }) ); + const result = getRectDimensionsForLevel(container, level, 2, 100, 0, 1); expect(result).toEqual([ { width: 99, height: 22, x: 0, y: 44, collapsed: false, ticks: 100, label: '1', unitLabel: '100' }, { width: 3, height: 22, x: 100, y: 44, collapsed: true, ticks: 3, label: '2', unitLabel: '2' }, diff --git a/public/app/plugins/panel/flamegraph/components/FlameGraph/rendering.ts b/public/app/plugins/panel/flamegraph/components/FlameGraph/rendering.ts index bf528d42609..32793cc0f01 100644 --- a/public/app/plugins/panel/flamegraph/components/FlameGraph/rendering.ts +++ b/public/app/plugins/panel/flamegraph/components/FlameGraph/rendering.ts @@ -1,4 +1,3 @@ -import { DisplayProcessor } from '@grafana/data'; import { colors } from '@grafana/ui'; import { @@ -10,7 +9,7 @@ import { PIXELS_PER_LEVEL, } from '../../constants'; -import { ItemWithStart } from './dataTransform'; +import { FlameGraphDataContainer, LevelItem } from './dataTransform'; type RectData = { width: number; @@ -28,19 +27,18 @@ type RectData = { * into bigger rects. */ export function getRectDimensionsForLevel( - level: ItemWithStart[], + data: FlameGraphDataContainer, + level: LevelItem[], levelIndex: number, totalTicks: number, rangeMin: number, - pixelsPerTick: number, - processor: DisplayProcessor, - getLabelValue: (value: number | string) => string + pixelsPerTick: number ): RectData[] { const coordinatesLevel = []; for (let barIndex = 0; barIndex < level.length; barIndex += 1) { const item = level[barIndex]; const barX = getBarX(item.start, totalTicks, rangeMin, pixelsPerTick); - let curBarTicks = item.value; + let curBarTicks = data.getValue(item.itemIndex); // merge very small blocks into big "collapsed" ones for performance const collapsed = curBarTicks * pixelsPerTick <= COLLAPSE_THRESHOLD; @@ -48,14 +46,14 @@ export function getRectDimensionsForLevel( while ( barIndex < level.length - 1 && item.start + curBarTicks === level[barIndex + 1].start && - level[barIndex + 1].value * pixelsPerTick <= COLLAPSE_THRESHOLD + data.getValue(level[barIndex + 1].itemIndex) * pixelsPerTick <= COLLAPSE_THRESHOLD ) { barIndex += 1; - curBarTicks += level[barIndex].value; + curBarTicks += data.getValue(level[barIndex].itemIndex); } } - const displayValue = processor(item.value); + const displayValue = data.getValueDisplay(item.itemIndex); let unit = displayValue.suffix ? displayValue.text + displayValue.suffix : displayValue.text; const width = curBarTicks * pixelsPerTick - (collapsed ? 0 : BAR_BORDER_WIDTH * 2); @@ -66,7 +64,7 @@ export function getRectDimensionsForLevel( y: levelIndex * PIXELS_PER_LEVEL, collapsed, ticks: curBarTicks, - label: getLabelValue(item.label), + label: data.getLabel(item.itemIndex), unitLabel: unit, }); } diff --git a/public/app/plugins/panel/flamegraph/components/FlameGraphContainer.tsx b/public/app/plugins/panel/flamegraph/components/FlameGraphContainer.tsx index d2ce0f3d3e2..df4368b782f 100644 --- a/public/app/plugins/panel/flamegraph/components/FlameGraphContainer.tsx +++ b/public/app/plugins/panel/flamegraph/components/FlameGraphContainer.tsx @@ -1,14 +1,14 @@ import { css } from '@emotion/css'; -import React, { useCallback, useEffect, useMemo, useState } from 'react'; +import React, { useEffect, useMemo, useState } from 'react'; import { useMeasure } from 'react-use'; -import { DataFrame, DataFrameView, CoreApp, getEnumDisplayProcessor } from '@grafana/data'; +import { DataFrame, CoreApp } from '@grafana/data'; import { useStyles2, useTheme2 } from '@grafana/ui'; import { MIN_WIDTH_TO_SHOW_BOTH_TOPTABLE_AND_FLAMEGRAPH, PIXELS_PER_LEVEL } from '../constants'; import FlameGraph from './FlameGraph/FlameGraph'; -import { Item, nestedSetToLevels } from './FlameGraph/dataTransform'; +import { FlameGraphDataContainer, LevelItem, nestedSetToLevels } from './FlameGraph/dataTransform'; import FlameGraphHeader from './FlameGraphHeader'; import FlameGraphTopTableContainer from './TopTable/FlameGraphTopTableContainer'; import { SelectedView } from './types'; @@ -30,38 +30,21 @@ const FlameGraphContainer = (props: Props) => { const [search, setSearch] = useState(''); const [selectedView, setSelectedView] = useState(SelectedView.Both); const [sizeRef, { width: containerWidth }] = useMeasure(); - - const labelField = props.data?.fields.find((f) => f.name === 'label'); - const theme = useTheme2(); - // Label can actually be an enum field so depending on that we have to access it through display processor. This is - // both a backward compatibility but also to allow using a simple dataFrame without enum config. This would allow - // users to use this panel with correct query from data sources that do not return profiles natively. - const getLabelValue = useCallback( - (label: string | number) => { - const enumConfig = labelField?.config?.type?.enum; - if (enumConfig) { - return getEnumDisplayProcessor(theme, enumConfig)(label).text; - } else { - return label.toString(); - } - }, - [labelField, theme] - ); - - // Transform dataFrame with nested set format to array of levels. Each level contains all the bars for a particular - // level of the flame graph. We do this temporary as in the end we should be able to render directly by iterating - // over the dataFrame rows. - const levels = useMemo(() => { + const [dataContainer, levels] = useMemo((): [FlameGraphDataContainer, LevelItem[][]] | [undefined, undefined] => { if (!props.data) { - return []; + return [undefined, undefined]; } - const dataView = new DataFrameView(props.data); - return nestedSetToLevels(dataView); - }, [props.data]); + const container = new FlameGraphDataContainer(props.data, theme); - const styles = useStyles2(() => getStyles(props.app, PIXELS_PER_LEVEL * levels.length)); + // Transform dataFrame with nested set format to array of levels. Each level contains all the bars for a particular + // level of the flame graph. We do this temporary as in the end we should be able to render directly by iterating + // over the dataFrame rows. + return [container, nestedSetToLevels(container)]; + }, [props.data, theme]); + + const styles = useStyles2(() => getStyles(props.app, PIXELS_PER_LEVEL * (levels?.length ?? 0))); // If user resizes window with both as the selected view useEffect(() => { @@ -83,7 +66,7 @@ const FlameGraphContainer = (props: Props) => { return ( <> - {props.data && ( + {dataContainer && (
{ {selectedView !== SelectedView.FlameGraph && ( { setSelectedBarIndex={setSelectedBarIndex} setRangeMin={setRangeMin} setRangeMax={setRangeMax} - getLabelValue={getLabelValue} /> )} {selectedView !== SelectedView.TopTable && ( { setRangeMin={setRangeMin} setRangeMax={setRangeMax} selectedView={selectedView} - getLabelValue={getLabelValue} /> )}
diff --git a/public/app/plugins/panel/flamegraph/components/TopTable/FlameGraphTopTableContainer.test.tsx b/public/app/plugins/panel/flamegraph/components/TopTable/FlameGraphTopTableContainer.test.tsx index 4bb58c62063..d9390cb681a 100644 --- a/public/app/plugins/panel/flamegraph/components/TopTable/FlameGraphTopTableContainer.test.tsx +++ b/public/app/plugins/panel/flamegraph/components/TopTable/FlameGraphTopTableContainer.test.tsx @@ -1,9 +1,9 @@ import { render, screen } from '@testing-library/react'; import React, { useState } from 'react'; -import { CoreApp, DataFrameView, MutableDataFrame } from '@grafana/data'; +import { CoreApp, MutableDataFrame } from '@grafana/data'; -import { Item, nestedSetToLevels } from '../FlameGraph/dataTransform'; +import { FlameGraphDataContainer, nestedSetToLevels } from '../FlameGraph/dataTransform'; import { data } from '../FlameGraph/testData/dataNestedSet'; import { SelectedView } from '../types'; @@ -15,12 +15,12 @@ describe('FlameGraphTopTableContainer', () => { const [selectedView, _] = useState(SelectedView.Both); const flameGraphData = new MutableDataFrame(data); - const dataView = new DataFrameView(flameGraphData); - const levels = nestedSetToLevels(dataView); + const container = new FlameGraphDataContainer(flameGraphData); + const levels = nestedSetToLevels(container); return ( { setSelectedBarIndex={jest.fn()} setRangeMin={jest.fn()} setRangeMax={jest.fn()} - getLabelValue={(val) => val.toString()} /> ); }; diff --git a/public/app/plugins/panel/flamegraph/components/TopTable/FlameGraphTopTableContainer.tsx b/public/app/plugins/panel/flamegraph/components/TopTable/FlameGraphTopTableContainer.tsx index 08c04d317cb..cd80cf0f22b 100644 --- a/public/app/plugins/panel/flamegraph/components/TopTable/FlameGraphTopTableContainer.tsx +++ b/public/app/plugins/panel/flamegraph/components/TopTable/FlameGraphTopTableContainer.tsx @@ -1,17 +1,18 @@ import { css } from '@emotion/css'; -import React, { useCallback, useEffect, useState } from 'react'; +import React, { useMemo } from 'react'; import AutoSizer from 'react-virtualized-auto-sizer'; -import { CoreApp, DataFrame, Field, FieldType, getDisplayProcessor } from '@grafana/data'; -import { useStyles2, useTheme2 } from '@grafana/ui'; +import { CoreApp, DisplayValue } from '@grafana/data'; +import { useStyles2 } from '@grafana/ui'; import { PIXELS_PER_LEVEL } from '../../constants'; -import { SampleUnit, SelectedView, TableData, TopTableData } from '../types'; +import { FlameGraphDataContainer } from '../FlameGraph/dataTransform'; +import { SelectedView, TableData, TopTableData } from '../types'; import FlameGraphTopTable from './FlameGraphTopTable'; type Props = { - data: DataFrame; + data: FlameGraphDataContainer; app: CoreApp; totalLevels: number; selectedView: SelectedView; @@ -21,7 +22,6 @@ type Props = { setSelectedBarIndex: (bar: number) => void; setRangeMin: (range: number) => void; setRangeMax: (range: number) => void; - getLabelValue: (label: string | number) => string; }; const FlameGraphTopTableContainer = ({ @@ -35,70 +35,26 @@ const FlameGraphTopTableContainer = ({ setSelectedBarIndex, setRangeMin, setRangeMax, - getLabelValue, }: Props) => { const styles = useStyles2(() => getStyles(selectedView, app)); - const theme = useTheme2(); - const [topTable, setTopTable] = useState(); - const valueField = - data.fields.find((f) => f.name === 'value') ?? data.fields.find((f) => f.type === FieldType.number); - const selfField = data.fields.find((f) => f.name === 'self') ?? data.fields.find((f) => f.type === FieldType.number); - const labelsField = data.fields.find((f) => f.name === 'label'); - - const sortLevelsIntoTable = useCallback(() => { - let label, self, value; + const topTable = useMemo(() => { + // Group the data by label + // TODO: should be by filename + funcName + linenumber? let table: { [key: string]: TableData } = {}; - - if (valueField && selfField && labelsField) { - const valueValues = valueField.values; - const selfValues = selfField.values; - const labelValues = labelsField.values; - - for (let i = 0; i < valueValues.length; i++) { - value = valueValues.get(i); - self = selfValues.get(i); - label = getLabelValue(labelValues.get(i)); - table[label] = table[label] || {}; - table[label].self = table[label].self ? table[label].self + self : self; - table[label].total = table[label].total ? table[label].total + value : value; - } + for (let i = 0; i < data.data.length; i++) { + const value = data.getValue(i); + const self = data.getSelf(i); + const label = data.getLabel(i); + table[label] = table[label] || {}; + table[label].self = table[label].self ? table[label].self + self : self; + table[label].total = table[label].total ? table[label].total + value : value; } - return table; - }, [getLabelValue, selfField, valueField, labelsField]); - - const getTopTableData = useCallback( - (field: Field, value: number) => { - const processor = getDisplayProcessor({ field, theme }); - const displayValue = processor(value); - let unitValue = displayValue.text + displayValue.suffix; - - switch (field.config.unit) { - case SampleUnit.Bytes: - break; - case SampleUnit.Nanoseconds: - break; - default: - if (!displayValue.suffix) { - // Makes sure we don't show 123undefined or something like that if suffix isn't defined - unitValue = displayValue.text; - } - break; - } - - return unitValue; - }, - [theme] - ); - - useEffect(() => { - const table = sortLevelsIntoTable(); - let topTable: TopTableData[] = []; for (let key in table) { - const selfUnit = getTopTableData(selfField!, table[key].self); - const valueUnit = getTopTableData(valueField!, table[key].total); + const selfUnit = handleUnits(data.valueDisplayProcessor(table[key].self), data.getUnitTitle()); + const valueUnit = handleUnits(data.valueDisplayProcessor(table[key].total), data.getUnitTitle()); topTable.push({ symbol: key, @@ -107,8 +63,8 @@ const FlameGraphTopTableContainer = ({ }); } - setTopTable(topTable); - }, [data.fields, selfField, sortLevelsIntoTable, valueField, getTopTableData]); + return topTable; + }, [data]); return ( <> @@ -135,6 +91,17 @@ const FlameGraphTopTableContainer = ({ ); }; +function handleUnits(displayValue: DisplayValue, unit: string) { + let unitValue = displayValue.text + displayValue.suffix; + if (unit === 'Count') { + if (!displayValue.suffix) { + // Makes sure we don't show 123undefined or something like that if suffix isn't defined + unitValue = displayValue.text; + } + } + return unitValue; +} + const getStyles = (selectedView: SelectedView, app: CoreApp) => { const marginRight = '20px'; diff --git a/public/app/plugins/panel/flamegraph/components/types.ts b/public/app/plugins/panel/flamegraph/components/types.ts index e1bc75d32ef..a84b66b2fac 100644 --- a/public/app/plugins/panel/flamegraph/components/types.ts +++ b/public/app/plugins/panel/flamegraph/components/types.ts @@ -1,13 +1,3 @@ -export type TooltipData = { - name: string; - percentValue: number; - percentSelf: number; - unitTitle: string; - unitValue: string; - unitSelf: string; - samples: string; -}; - export type ContextMenuData = { e: MouseEvent; levelIndex: number;