From 9c254c7e1efbfa688d81f347b0fdda631fb981f5 Mon Sep 17 00:00:00 2001 From: Andrej Ocenas <mr.ocenas@gmail.com> Date: Mon, 6 May 2024 12:28:57 +0200 Subject: [PATCH] Flamegraph: Add diff mode color legend (#87319) --- .../src/FlameGraph/FlameGraphMetadata.tsx | 2 +- .../src/FlameGraph/colors.ts | 4 +- .../src/FlameGraphContainer.tsx | 1 + .../src/FlameGraphHeader.tsx | 50 ++++++++++++++----- .../FlameGraphTopTableContainer.test.tsx | 2 + .../TopTable/FlameGraphTopTableContainer.tsx | 34 ++++++++++--- 6 files changed, 70 insertions(+), 23 deletions(-) diff --git a/packages/grafana-flamegraph/src/FlameGraph/FlameGraphMetadata.tsx b/packages/grafana-flamegraph/src/FlameGraph/FlameGraphMetadata.tsx index d385b7dd148..a24b96f7f19 100644 --- a/packages/grafana-flamegraph/src/FlameGraph/FlameGraphMetadata.tsx +++ b/packages/grafana-flamegraph/src/FlameGraph/FlameGraphMetadata.tsx @@ -82,7 +82,7 @@ const FlameGraphMetadata = React.memo( ); } - return <>{<div className={styles.metadata}>{parts}</div>}</>; + return <div className={styles.metadata}>{parts}</div>; } ); diff --git a/packages/grafana-flamegraph/src/FlameGraph/colors.ts b/packages/grafana-flamegraph/src/FlameGraph/colors.ts index 5e7a5c868e1..51757b2ca49 100644 --- a/packages/grafana-flamegraph/src/FlameGraph/colors.ts +++ b/packages/grafana-flamegraph/src/FlameGraph/colors.ts @@ -65,9 +65,9 @@ export function getBarColorByPackage(label: string, theme: GrafanaTheme2) { } // green to red -const diffDefaultColors = ['rgb(0, 170, 0)', 'rgb(148, 142, 142)', 'rgb(200, 0, 0)']; +export const diffDefaultColors = ['rgb(0, 170, 0)', 'rgb(148, 142, 142)', 'rgb(200, 0, 0)']; export const diffDefaultGradient = `linear-gradient(90deg, ${diffDefaultColors[0]} 0%, ${diffDefaultColors[1]} 50%, ${diffDefaultColors[2]} 100%)`; -const diffColorBlindColors = ['rgb(26, 133, 255)', 'rgb(148, 142, 142)', 'rgb(220, 50, 32)']; +export const diffColorBlindColors = ['rgb(26, 133, 255)', 'rgb(148, 142, 142)', 'rgb(220, 50, 32)']; export const diffColorBlindGradient = `linear-gradient(90deg, ${diffColorBlindColors[0]} 0%, ${diffColorBlindColors[1]} 50%, ${diffColorBlindColors[2]} 100%)`; export function getBarColorByDiff( diff --git a/packages/grafana-flamegraph/src/FlameGraphContainer.tsx b/packages/grafana-flamegraph/src/FlameGraphContainer.tsx index c4962e5b181..b2d9bbd5383 100644 --- a/packages/grafana-flamegraph/src/FlameGraphContainer.tsx +++ b/packages/grafana-flamegraph/src/FlameGraphContainer.tsx @@ -192,6 +192,7 @@ const FlameGraphContainer = ({ onSandwich={setSandwichItem} onSearch={setSearch} onTableSort={onTableSort} + colorScheme={colorScheme} /> ); diff --git a/packages/grafana-flamegraph/src/FlameGraphHeader.tsx b/packages/grafana-flamegraph/src/FlameGraphHeader.tsx index 20606e0637c..003035211f1 100644 --- a/packages/grafana-flamegraph/src/FlameGraphHeader.tsx +++ b/packages/grafana-flamegraph/src/FlameGraphHeader.tsx @@ -129,15 +129,6 @@ function ColorSchemeButton(props: ColorSchemeButtonProps) { </Menu> ); - if (props.isDiffMode) { - menu = ( - <Menu> - <Menu.Item label="Default (green to red)" onClick={() => props.onChange(ColorSchemeDiff.Default)} /> - <Menu.Item label="Color blind (blue to red)" onClick={() => props.onChange(ColorSchemeDiff.DiffColorBlind)} /> - </Menu> - ); - } - // Show a bit different gradient as a way to indicate selected value const colorDotStyle = { @@ -147,6 +138,25 @@ function ColorSchemeButton(props: ColorSchemeButtonProps) { [ColorSchemeDiff.Default]: styles.colorDotDiffDefault, }[props.value] || styles.colorDotByValue; + let contents = <span className={cx(styles.colorDot, colorDotStyle)} />; + + if (props.isDiffMode) { + menu = ( + <Menu> + <Menu.Item label="Default (green to red)" onClick={() => props.onChange(ColorSchemeDiff.Default)} /> + <Menu.Item label="Color blind (blue to red)" onClick={() => props.onChange(ColorSchemeDiff.DiffColorBlind)} /> + </Menu> + ); + + contents = ( + <div className={cx(styles.colorDotDiff, colorDotStyle)}> + <div>-100% (removed)</div> + <div>0%</div> + <div>+100% (added)</div> + </div> + ); + } + return ( <Dropdown overlay={menu}> <Button @@ -158,7 +168,7 @@ function ColorSchemeButton(props: ColorSchemeButtonProps) { className={styles.buttonSpacing} aria-label={'Change color scheme'} > - <span className={cx(styles.colorDot, colorDotStyle)} /> + {contents} </Button> </Dropdown> ); @@ -221,17 +231,16 @@ const getStyles = (theme: GrafanaTheme2) => ({ justifyContent: 'space-between', width: '100%', top: 0, + gap: theme.spacing(1), + marginTop: theme.spacing(1), }), stickyHeader: css({ zIndex: theme.zIndex.navbarFixed, position: 'sticky', - paddingBottom: theme.spacing(1), - paddingTop: theme.spacing(1), background: theme.colors.background.primary, }), inputContainer: css({ label: 'inputContainer', - marginRight: '20px', flexGrow: 1, minWidth: '150px', maxWidth: '350px', @@ -264,6 +273,21 @@ const getStyles = (theme: GrafanaTheme2) => ({ // eslint-disable-next-line @grafana/no-border-radius-literal borderRadius: '50%', }), + colorDotDiff: css({ + label: 'colorDotDiff', + display: 'flex', + width: '200px', + height: '12px', + color: 'white', + fontSize: 9, + lineHeight: 1.3, + fontWeight: 300, + justifyContent: 'space-between', + padding: '0 2px', + // We have a specific sizing for this so probably makes sense to use hardcoded value here + // eslint-disable-next-line @grafana/no-border-radius-literal + borderRadius: '2px', + }), colorDotByValue: css({ label: 'colorDotByValue', background: byValueGradient, diff --git a/packages/grafana-flamegraph/src/TopTable/FlameGraphTopTableContainer.test.tsx b/packages/grafana-flamegraph/src/TopTable/FlameGraphTopTableContainer.test.tsx index f693d057f9b..03d4ddb153b 100644 --- a/packages/grafana-flamegraph/src/TopTable/FlameGraphTopTableContainer.test.tsx +++ b/packages/grafana-flamegraph/src/TopTable/FlameGraphTopTableContainer.test.tsx @@ -6,6 +6,7 @@ import { createDataFrame } from '@grafana/data'; import { FlameGraphDataContainer } from '../FlameGraph/dataTransform'; import { data } from '../FlameGraph/testData/dataNestedSet'; +import { ColorScheme } from '../types'; import FlameGraphTopTableContainer from './FlameGraphTopTableContainer'; @@ -22,6 +23,7 @@ describe('FlameGraphTopTableContainer', () => { onSymbolClick={jest.fn()} onSearch={onSearch} onSandwich={onSandwich} + colorScheme={ColorScheme.ValueBased} /> ); diff --git a/packages/grafana-flamegraph/src/TopTable/FlameGraphTopTableContainer.tsx b/packages/grafana-flamegraph/src/TopTable/FlameGraphTopTableContainer.tsx index 2630e7dcf9d..491230334f1 100644 --- a/packages/grafana-flamegraph/src/TopTable/FlameGraphTopTableContainer.tsx +++ b/packages/grafana-flamegraph/src/TopTable/FlameGraphTopTableContainer.tsx @@ -22,9 +22,10 @@ import { useTheme2, } from '@grafana/ui'; +import { diffColorBlindColors, diffDefaultColors } from '../FlameGraph/colors'; import { FlameGraphDataContainer } from '../FlameGraph/dataTransform'; import { TOP_TABLE_COLUMN_WIDTH } from '../constants'; -import { TableData } from '../types'; +import { ColorScheme, ColorSchemeDiff, TableData } from '../types'; type Props = { data: FlameGraphDataContainer; @@ -37,10 +38,21 @@ type Props = { onSearch: (str: string) => void; onSandwich: (str?: string) => void; onTableSort?: (sort: string) => void; + colorScheme: ColorScheme | ColorSchemeDiff; }; const FlameGraphTopTableContainer = React.memo( - ({ data, onSymbolClick, search, matchedLabels, onSearch, sandwichItem, onSandwich, onTableSort }: Props) => { + ({ + data, + onSymbolClick, + search, + matchedLabels, + onSearch, + sandwichItem, + onSandwich, + onTableSort, + colorScheme, + }: Props) => { const table = useMemo(() => { // Group the data by label, we show only one row per label and sum the values // TODO: should be by filename + funcName + linenumber? @@ -51,7 +63,7 @@ const FlameGraphTopTableContainer = React.memo( const self = data.getSelf(i); const label = data.getLabel(i); - // If user is doing text search we filter out labels in the same way we highlight them in flamegraph. + // If user is doing text search we filter out labels in the same way we highlight them in flame graph. if (!matchedLabels || matchedLabels.has(label)) { filteredTable[label] = filteredTable[label] || {}; filteredTable[label].self = filteredTable[label].self ? filteredTable[label].self + self : self; @@ -85,6 +97,7 @@ const FlameGraphTopTableContainer = React.memo( onSearch, onSandwich, theme, + colorScheme, search, sandwichItem ); @@ -119,6 +132,7 @@ function buildTableDataFrame( onSearch: (str: string) => void, onSandwich: (str?: string) => void, theme: GrafanaTheme2, + colorScheme: ColorScheme | ColorSchemeDiff, search?: string, sandwichItem?: string ): DataFrame { @@ -153,11 +167,17 @@ function buildTableDataFrame( const comparisonField = createNumberField('Comparison', 'percent'); const diffField = createNumberField('Diff', 'percent'); diffField.config.custom.cellOptions.type = TableCellDisplayMode.ColorText; + + const [removeColor, addColor] = + colorScheme === ColorSchemeDiff.DiffColorBlind + ? [diffColorBlindColors[0], diffColorBlindColors[2]] + : [diffDefaultColors[0], diffDefaultColors[2]]; + diffField.config.mappings = [ - { type: MappingType.ValueToText, options: { [Infinity]: { text: 'new', color: 'red' } } }, - { type: MappingType.ValueToText, options: { [-100]: { text: 'removed', color: 'green' } } }, - { type: MappingType.RangeToText, options: { from: 0, to: Infinity, result: { color: 'red' } } }, - { type: MappingType.RangeToText, options: { from: -Infinity, to: 0, result: { color: 'green' } } }, + { type: MappingType.ValueToText, options: { [Infinity]: { text: 'new', color: addColor } } }, + { type: MappingType.ValueToText, options: { [-100]: { text: 'removed', color: removeColor } } }, + { type: MappingType.RangeToText, options: { from: 0, to: Infinity, result: { color: addColor } } }, + { type: MappingType.RangeToText, options: { from: -Infinity, to: 0, result: { color: removeColor } } }, ]; // For this we don't really consider sandwich view even though you can switch it on.