From 7c7b222ae732969fcda26a99289fb0f5a1d9a5d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Torkel=20=C3=96degaard?= Date: Thu, 13 Apr 2023 14:16:46 +0200 Subject: [PATCH] FrameGraphTopTable: Use standard Table component (#66411) * FrameGraphTopTable: Use standard Table component * Simplify * Fix test * Update tests * Fixing test --- .../components/FlameGraphContainer.test.tsx | 6 +- .../components/FlameGraphHeader.tsx | 26 +- .../TopTable/FlameGraphTopTable.tsx | 265 ------------------ .../FlameGraphTopTableContainer.test.tsx | 4 +- .../TopTable/FlameGraphTopTableContainer.tsx | 168 +++++++---- 5 files changed, 124 insertions(+), 345 deletions(-) delete mode 100644 public/app/plugins/panel/flamegraph/components/TopTable/FlameGraphTopTable.tsx diff --git a/public/app/plugins/panel/flamegraph/components/FlameGraphContainer.test.tsx b/public/app/plugins/panel/flamegraph/components/FlameGraphContainer.test.tsx index f39ff0c5fb3..c3f5dc8f397 100644 --- a/public/app/plugins/panel/flamegraph/components/FlameGraphContainer.test.tsx +++ b/public/app/plugins/panel/flamegraph/components/FlameGraphContainer.test.tsx @@ -39,11 +39,11 @@ describe('FlameGraphContainer', () => { it('should update search when row selected in top table', async () => { render(); - await userEvent.click((await screen.findAllByRole('row'))[1]); + await userEvent.click((await screen.findAllByTitle('Highlight symbol'))[0]); expect(screen.getByDisplayValue('net/http.HandlerFunc.ServeHTTP')).toBeInTheDocument(); - await userEvent.click(screen.getAllByRole('row')[2]); + await userEvent.click((await screen.findAllByTitle('Highlight symbol'))[1]); expect(screen.getByDisplayValue('total')).toBeInTheDocument(); - await userEvent.click(screen.getAllByRole('row')[2]); + await userEvent.click((await screen.findAllByTitle('Highlight symbol'))[1]); expect(screen.queryByDisplayValue('total')).not.toBeInTheDocument(); }); diff --git a/public/app/plugins/panel/flamegraph/components/FlameGraphHeader.tsx b/public/app/plugins/panel/flamegraph/components/FlameGraphHeader.tsx index c539e8f1ecf..8b062c10bef 100644 --- a/public/app/plugins/panel/flamegraph/components/FlameGraphHeader.tsx +++ b/public/app/plugins/panel/flamegraph/components/FlameGraphHeader.tsx @@ -39,6 +39,7 @@ const FlameGraphHeader = ({ { value: SelectedView.TopTable, label: 'Top Table', description: 'Only show top table' }, { value: SelectedView.FlameGraph, label: 'Flame Graph', description: 'Only show flame graph' }, ]; + if (containerWidth >= MIN_WIDTH_TO_SHOW_BOTH_TOPTABLE_AND_FLAMEGRAPH) { viewOptions.push({ value: SelectedView.Both, @@ -47,6 +48,14 @@ const FlameGraphHeader = ({ }); } + const onResetView = () => { + setTopLevelIndex(0); + setSelectedBarIndex(0); + setRangeMin(0); + setRangeMax(1); + setSearch(''); + }; + return (
@@ -57,22 +66,11 @@ const FlameGraphHeader = ({ setSearch(v.currentTarget.value); }} placeholder={'Search..'} - width={24} + width={44} />
-
diff --git a/public/app/plugins/panel/flamegraph/components/TopTable/FlameGraphTopTable.tsx b/public/app/plugins/panel/flamegraph/components/TopTable/FlameGraphTopTable.tsx deleted file mode 100644 index ddbfb006c1d..00000000000 --- a/public/app/plugins/panel/flamegraph/components/TopTable/FlameGraphTopTable.tsx +++ /dev/null @@ -1,265 +0,0 @@ -import { css, cx } from '@emotion/css'; -import React, { CSSProperties, useCallback, useMemo } from 'react'; -import { SortByFn, useSortBy, useAbsoluteLayout, useTable, CellProps } from 'react-table'; -import { FixedSizeList } from 'react-window'; - -import { GrafanaTheme2 } from '@grafana/data'; -import { Icon, useStyles2, CustomScrollbar } from '@grafana/ui'; - -import { TOP_TABLE_COLUMN_WIDTH } from '../../constants'; -import { ColumnTypes, TopTableData, TopTableValue } from '../types'; - -type Props = { - width: number; - height: number; - data: TopTableData[]; - search: string; - setSearch: (search: string) => void; - setTopLevelIndex: (level: number) => void; - setSelectedBarIndex: (bar: number) => void; - setRangeMin: (range: number) => void; - setRangeMax: (range: number) => void; -}; - -const FlameGraphTopTable = ({ - width, - height, - data, - search, - setSearch, - setTopLevelIndex, - setSelectedBarIndex, - setRangeMin, - setRangeMax, -}: Props) => { - const styles = useStyles2((theme) => getStyles(theme)); - - const sortSymbols: SortByFn = (a, b, column) => { - return a.values[column].localeCompare(b.values[column]); - }; - - const sortUnits: SortByFn = (a, b, column) => { - return a.values[column].value.toString().localeCompare(b.values[column].value.toString(), 'en', { numeric: true }); - }; - - const columns = useMemo( - () => [ - { - accessor: ColumnTypes.Symbol.toLowerCase(), - header: ColumnTypes.Symbol, - cell: SymbolCell, - sortType: sortSymbols, - width: width - TOP_TABLE_COLUMN_WIDTH * 2, - }, - { - accessor: ColumnTypes.Self.toLowerCase(), - header: ColumnTypes.Self, - cell: UnitCell, - sortType: sortUnits, - width: TOP_TABLE_COLUMN_WIDTH, - }, - { - accessor: ColumnTypes.Total.toLowerCase(), - header: ColumnTypes.Total, - cell: UnitCell, - sortType: sortUnits, - width: TOP_TABLE_COLUMN_WIDTH, - }, - ], - [width] - ); - - const options = useMemo( - () => ({ - columns, - data, - initialState: { - sortBy: [ - { - id: ColumnTypes.Self.toLowerCase(), - desc: true, - }, - ], - }, - }), - [columns, data] - ); - - const rowClicked = useCallback( - (symbol: string) => { - if (search === symbol) { - setSearch(''); - } else { - setSearch(symbol); - // Reset selected level in flamegraph when selecting row in top table - setTopLevelIndex(0); - setSelectedBarIndex(0); - setRangeMin(0); - setRangeMax(1); - } - }, - [search, setRangeMax, setRangeMin, setSearch, setTopLevelIndex, setSelectedBarIndex] - ); - - const { headerGroups, rows, prepareRow } = useTable(options, useSortBy, useAbsoluteLayout); - - const renderRow = React.useCallback( - ({ index, style }: { index: number; style: CSSProperties }) => { - let row = rows[index]; - prepareRow(row); - - const symbol = row.values[ColumnTypes.Symbol.toLowerCase()]; - const classNames = cx(symbol === search && styles.matchedRow, styles.row); - - return ( -
{ - rowClicked(symbol); - }} - > - {row.cells.map((cell) => { - const { key, ...cellProps } = cell.getCellProps(); - if (cellProps.style) { - cellProps.style.minWidth = cellProps.style.width; - } - return ( -
- {cell.render('cell')} -
- ); - })} -
- ); - }, - [rows, prepareRow, search, styles.matchedRow, styles.row, styles.cell, rowClicked] - ); - - return ( -
- {headerGroups.map((headerGroup) => { - const { key, ...headerGroupProps } = headerGroup.getHeaderGroupProps(); - - return ( -
- {headerGroup.headers.map((column) => { - const { key, ...headerProps } = column.getHeaderProps( - column.canSort ? column.getSortByToggleProps() : undefined - ); - - return ( -
- {column.render('header')} - {column.isSorted && } -
- ); - })} -
- ); - })} - - {rows.length > 0 ? ( - - - {renderRow} - - - ) : ( -
- No data -
- )} -
- ); -}; - -const SymbolCell = ({ cell: { value } }: CellProps) => { - return
{value}
; -}; - -const UnitCell = ({ cell: { value } }: CellProps) => { - return
{value.unitValue}
; -}; - -const getStyles = (theme: GrafanaTheme2) => ({ - table: (height: number) => { - return css` - background-color: ${theme.colors.background.primary}; - height: ${height}px; - overflow: scroll; - display: flex; - flex-direction: column; - width: 100%; - `; - }, - header: css` - height: 38px; - - & > :nth-child(2), - & > :nth-child(3) { - text-align: right; - } - - // needed to keep header row height fixed so header row does not resize with browser - & > :nth-child(3) { - position: relative !important; - } - `, - headerCell: css` - background-color: ${theme.colors.background.secondary}; - color: ${theme.colors.primary.text}; - padding: ${theme.spacing(1)}; - `, - matchedRow: css` - & > :nth-child(1), - & > :nth-child(2), - & > :nth-child(3) { - background-color: ${theme.colors.background.secondary} !important; - } - `, - row: css` - border-top: 1px solid ${theme.components.panel.borderColor}; - - &:hover { - background-color: ${theme.colors.emphasize(theme.colors.background.primary, 0.03)}; - } - & > :nth-child(2), - & > :nth-child(3) { - text-align: right; - } - & > :nth-child(3) { - border-right: none; - } - `, - cell: css` - border-right: 1px solid ${theme.components.panel.borderColor}; - padding: ${theme.spacing(1)}; - - div { - overflow: hidden; - text-overflow: ellipsis; - white-space: nowrap; - } - &:hover { - overflow: visible; - width: auto !important; - box-shadow: 0 0 2px ${theme.colors.primary.main}; - background-color: ${theme.colors.emphasize(theme.colors.background.primary, 0.03)}; - z-index: 1; - } - `, - noData: css` - align-items: center; - display: flex; - justify-content: center; - `, -}); - -export default FlameGraphTopTable; 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 d9390cb681a..1e823537ec4 100644 --- a/public/app/plugins/panel/flamegraph/components/TopTable/FlameGraphTopTableContainer.test.tsx +++ b/public/app/plugins/panel/flamegraph/components/TopTable/FlameGraphTopTableContainer.test.tsx @@ -45,7 +45,7 @@ describe('FlameGraphTopTableContainer', () => { render(); const rows = screen.getAllByRole('row'); - expect(rows).toHaveLength(17); // + 1 for the columnHeaders + expect(rows).toHaveLength(16); const columnHeaders = screen.getAllByRole('columnheader'); expect(columnHeaders).toHaveLength(3); @@ -54,7 +54,7 @@ describe('FlameGraphTopTableContainer', () => { expect(columnHeaders[2].textContent).toEqual('Total'); const cells = screen.getAllByRole('cell'); - expect(cells).toHaveLength(48); // 16 rows + expect(cells).toHaveLength(45); // 16 rows expect(cells[0].textContent).toEqual('net/http.HandlerFunc.ServeHTTP'); expect(cells[1].textContent).toEqual('31.7 K'); expect(cells[2].textContent).toEqual('31.7 Bil'); diff --git a/public/app/plugins/panel/flamegraph/components/TopTable/FlameGraphTopTableContainer.tsx b/public/app/plugins/panel/flamegraph/components/TopTable/FlameGraphTopTableContainer.tsx index cd80cf0f22b..3ef85d5e51b 100644 --- a/public/app/plugins/panel/flamegraph/components/TopTable/FlameGraphTopTableContainer.tsx +++ b/public/app/plugins/panel/flamegraph/components/TopTable/FlameGraphTopTableContainer.tsx @@ -1,15 +1,22 @@ import { css } from '@emotion/css'; -import React, { useMemo } from 'react'; +import React from 'react'; import AutoSizer from 'react-virtualized-auto-sizer'; -import { CoreApp, DisplayValue } from '@grafana/data'; -import { useStyles2 } from '@grafana/ui'; +import { + applyFieldOverrides, + ArrayVector, + CoreApp, + DataFrame, + DataLinkClickEvent, + Field, + FieldType, +} from '@grafana/data'; +import { config } from '@grafana/runtime'; +import { Table, useStyles2 } from '@grafana/ui'; -import { PIXELS_PER_LEVEL } from '../../constants'; +import { PIXELS_PER_LEVEL, TOP_TABLE_COLUMN_WIDTH } from '../../constants'; import { FlameGraphDataContainer } from '../FlameGraph/dataTransform'; -import { SelectedView, TableData, TopTableData } from '../types'; - -import FlameGraphTopTable from './FlameGraphTopTable'; +import { SelectedView, TableData } from '../types'; type Props = { data: FlameGraphDataContainer; @@ -38,68 +45,107 @@ const FlameGraphTopTableContainer = ({ }: Props) => { const styles = useStyles2(() => getStyles(selectedView, app)); - const topTable = useMemo(() => { - // Group the data by label - // TODO: should be by filename + funcName + linenumber? - let table: { [key: string]: TableData } = {}; - 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; + const onSymbolClick = (symbol: string) => { + if (search === symbol) { + setSearch(''); + } else { + setSearch(symbol); + // Reset selected level in flamegraph when selecting row in top table + setTopLevelIndex(0); + setSelectedBarIndex(0); + setRangeMin(0); + setRangeMax(1); } + }; - let topTable: TopTableData[] = []; - for (let key in table) { - const selfUnit = handleUnits(data.valueDisplayProcessor(table[key].self), data.getUnitTitle()); - const valueUnit = handleUnits(data.valueDisplayProcessor(table[key].total), data.getUnitTitle()); - - topTable.push({ - symbol: key, - self: { value: table[key].self, unitValue: selfUnit }, - total: { value: table[key].total, unitValue: valueUnit }, - }); - } - - return topTable; - }, [data]); + const initialSortBy = [{ displayName: 'Self', desc: true }]; return ( - <> - {topTable && ( -
- - {({ width, height }) => ( - - )} - -
- )} - +
+ + {({ width, height }) => { + if (width < 3 || height < 3) { + return null; + } + + const frame = buildTableDataFrame(data, width, onSymbolClick); + return ; + }} + + ); }; -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; - } +function buildTableDataFrame( + data: FlameGraphDataContainer, + width: number, + onSymbolClick: (str: string) => void +): DataFrame { + // Group the data by label + // TODO: should be by filename + funcName + linenumber? + let table: { [key: string]: TableData } = {}; + 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 unitValue; + + const symbolField = { + type: FieldType.string, + name: 'Symbol', + values: new ArrayVector(), + config: { + custom: { width: width - TOP_TABLE_COLUMN_WIDTH * 2 }, + links: [ + { + title: 'Highlight symbol', + url: '', + onClick: (e: DataLinkClickEvent) => { + const field: Field = e.origin.field; + const value = field.values.get(e.origin.rowIndex); + onSymbolClick(value); + }, + }, + ], + }, + }; + + const selfField = { + type: FieldType.number, + name: 'Self', + values: new ArrayVector(), + config: { unit: data.selfField.config.unit, custom: { width: TOP_TABLE_COLUMN_WIDTH } }, + }; + + const totalField = { + type: FieldType.number, + name: 'Total', + values: new ArrayVector(), + config: { unit: data.valueField.config.unit, custom: { width: TOP_TABLE_COLUMN_WIDTH } }, + }; + + for (let key in table) { + symbolField.values.add(key); + selfField.values.add(table[key].self); + totalField.values.add(table[key].total); + } + + const frame = { fields: [symbolField, selfField, totalField], length: symbolField.values.length }; + + const dataFrames = applyFieldOverrides({ + data: [frame], + fieldConfig: { + defaults: {}, + overrides: [], + }, + replaceVariables: (value: string) => value, + theme: config.theme2, + }); + + return dataFrames[0]; } const getStyles = (selectedView: SelectedView, app: CoreApp) => {