From 7e2f9709d25f298f264ec6b8ac5998c108cbcfd5 Mon Sep 17 00:00:00 2001 From: Manoj Vivek Date: Thu, 5 Dec 2024 12:16:46 +0530 Subject: [PATCH] ui: MetricsGraphStrips and TimelineGuide component improvements (#5362) * Extracting TimelineGuide from MetricGraphStrips and making it reusable * Added ticks count to the props * Removed the unwanted styles * MetricsGraphStrips props renamed * More refactoring and state integration * Storybook sotry fix * Type fix * Storybook mock data fix --- .../MetricsGraphStrips.stories.tsx | 16 +++---- .../profile/src/MetricsGraphStrips/index.tsx | 48 ++++++++++++++----- .../IcicleGraphArrow/index.tsx | 1 + .../profile/src/ProfileIcicleGraph/index.tsx | 10 ++-- .../components/DashboardItems/index.tsx | 3 -- .../context/ProfileViewContext.tsx | 12 +++++ .../shared/profile/src/ProfileView/index.tsx | 5 +- .../src/ProfileView/types/visualization.ts | 2 + .../profile/src/ProfileViewWithData.tsx | 16 +++++-- .../profile/src/TimelineGuide/index.tsx | 5 -- ui/packages/shared/profile/src/index.tsx | 1 + 11 files changed, 77 insertions(+), 42 deletions(-) diff --git a/ui/packages/shared/profile/src/MetricsGraphStrips/MetricsGraphStrips.stories.tsx b/ui/packages/shared/profile/src/MetricsGraphStrips/MetricsGraphStrips.stories.tsx index c541550aac1..13fa0185908 100644 --- a/ui/packages/shared/profile/src/MetricsGraphStrips/MetricsGraphStrips.stories.tsx +++ b/ui/packages/shared/profile/src/MetricsGraphStrips/MetricsGraphStrips.stories.tsx @@ -46,21 +46,21 @@ export default meta; export const ThreeCPUStrips = { args: { - cpus: Array.from(mockData, (_, i) => `CPU ${i + 1}`), + cpus: Array.from(mockData, (_, i) => ({labels: [{name: 'cpuid', value: i + 1}]})), data: mockData, - selectedTimeline: {index: 1, bounds: [mockData[0][25].timestamp, mockData[0][100].timestamp]}, - onSelectedTimeline: (index: number, bounds: NumberDuo): void => { - console.log('onSelectedTimeline', index, bounds); + selectedTimeframe: {index: 1, bounds: [mockData[0][25].timestamp, mockData[0][100].timestamp]}, + onSelectedTimeframe: (index: number, bounds: NumberDuo): void => { + console.log('onSelectedTimeframe', index, bounds); }, }, render: function Component(args: any): JSX.Element { const [, setArgs] = useArgs(); - const onSelectedTimeline = (index: number, bounds: NumberDuo): void => { - args.onSelectedTimeline(index, bounds); - setArgs({...args, selectedTimeline: {index, bounds}}); + const onSelectedTimeframe = (index: number, bounds: NumberDuo): void => { + args.onSelectedTimeframe(index, bounds); + setArgs({...args, selectedTimeframe: {index, bounds}}); }; - return ; + return ; }, }; diff --git a/ui/packages/shared/profile/src/MetricsGraphStrips/index.tsx b/ui/packages/shared/profile/src/MetricsGraphStrips/index.tsx index 0dacc8952ca..62e9b2f60e3 100644 --- a/ui/packages/shared/profile/src/MetricsGraphStrips/index.tsx +++ b/ui/packages/shared/profile/src/MetricsGraphStrips/index.tsx @@ -16,30 +16,53 @@ import {useMemo, useState} from 'react'; import {Icon} from '@iconify/react'; import * as d3 from 'd3'; +import {LabelSet} from '@parca/client'; + import {TimelineGuide} from '../TimelineGuide'; import {NumberDuo} from '../utils'; import {AreaGraph, DataPoint} from './AreaGraph'; interface Props { - cpus: string[]; + cpus: LabelSet[]; data: DataPoint[][]; - selectedTimeline?: { - index: number; + selectedTimeframe?: { + labels: LabelSet; bounds: NumberDuo; }; - onSelectedTimeline: (index: number, bounds: NumberDuo | undefined) => void; + onSelectedTimeframe: (labels: LabelSet, bounds: NumberDuo | undefined) => void; width?: number; } -const getTimelineGuideHeight = (cpus: string[], collapsedIndices: number[]): number => { +const labelSetToString = (labelSet?: LabelSet): string => { + if (labelSet === undefined) { + return '{}'; + } + + let str = '{'; + + let isFirst = true; + for (const label of labelSet.labels) { + if (!isFirst) { + str += ', '; + isFirst = false; + } + str += `${label.name}: ${label.value}`; + } + + str += '}'; + + return str; +}; + +const getTimelineGuideHeight = (cpus: LabelSet[], collapsedIndices: number[]): number => { return 56 * (cpus.length - collapsedIndices.length) + 20 * collapsedIndices.length + 24; }; export const MetricsGraphStrips = ({ cpus, data, - selectedTimeline, - onSelectedTimeline, + selectedTimeframe, + onSelectedTimeframe, width, }: Props): JSX.Element => { const [collapsedIndices, setCollapsedIndices] = useState([]); @@ -68,8 +91,9 @@ export const MetricsGraphStrips = ({ /> {cpus.map((cpu, i) => { const isCollapsed = collapsedIndices.includes(i); + const labelStr = labelSetToString(cpu); return ( -
+
{ @@ -83,19 +107,19 @@ export const MetricsGraphStrips = ({ }} > - {cpu} + {labelStr}
{!isCollapsed ? ( { - onSelectedTimeline(i, bounds); + onSelectedTimeframe(cpu, bounds); }} /> ) : null} diff --git a/ui/packages/shared/profile/src/ProfileIcicleGraph/IcicleGraphArrow/index.tsx b/ui/packages/shared/profile/src/ProfileIcicleGraph/IcicleGraphArrow/index.tsx index d39d58a8aed..0de487550bb 100644 --- a/ui/packages/shared/profile/src/ProfileIcicleGraph/IcicleGraphArrow/index.tsx +++ b/ui/packages/shared/profile/src/ProfileIcicleGraph/IcicleGraphArrow/index.tsx @@ -44,6 +44,7 @@ export const FIELD_MAPPING_BUILD_ID = 'mapping_build_id'; export const FIELD_LOCATION_ADDRESS = 'location_address'; export const FIELD_LOCATION_LINE = 'location_line'; export const FIELD_INLINED = 'inlined'; +export const FIELD_TIMESTAMP = 'timestamp'; export const FIELD_FUNCTION_NAME = 'function_name'; export const FIELD_FUNCTION_SYSTEM_NAME = 'function_system_name'; export const FIELD_FUNCTION_FILE_NAME = 'function_file_name'; diff --git a/ui/packages/shared/profile/src/ProfileIcicleGraph/index.tsx b/ui/packages/shared/profile/src/ProfileIcicleGraph/index.tsx index 4e81b029c3d..18ce880180c 100644 --- a/ui/packages/shared/profile/src/ProfileIcicleGraph/index.tsx +++ b/ui/packages/shared/profile/src/ProfileIcicleGraph/index.tsx @@ -46,7 +46,6 @@ interface ProfileIcicleGraphProps { isHalfScreen: boolean; metadataMappingFiles?: string[]; metadataLoading?: boolean; - showTimelineGuide?: boolean; } const ErrorContent = ({errorMessage}: {errorMessage: string}): JSX.Element => { @@ -66,10 +65,9 @@ const ProfileIcicleGraph = function ProfileIcicleGraphNonMemo({ width, isHalfScreen, metadataMappingFiles, - showTimelineGuide = false, }: ProfileIcicleGraphProps): JSX.Element { const {onError, authenticationErrorMessage, isDarkMode} = useParcaContext(); - const {compareMode} = useProfileViewContext(); + const {compareMode, timelineGuide} = useProfileViewContext(); const [isLoading, setIsLoading] = useState(true); const mappingsList = useMappingList(metadataMappingFiles); @@ -169,9 +167,9 @@ const ProfileIcicleGraph = function ProfileIcicleGraphNonMemo({ if (arrow !== undefined) return (
- {showTimelineGuide && ( + {timelineGuide?.show === true && ( { switch (type) { case 'icicle': @@ -100,7 +98,6 @@ export const getDashboardItem = ({ } metadataMappingFiles={flamegraphData.metadataMappingFiles} metadataLoading={flamegraphData.metadataLoading} - showTimelineGuide={showTimelineGuide} /> ); diff --git a/ui/packages/shared/profile/src/ProfileView/context/ProfileViewContext.tsx b/ui/packages/shared/profile/src/ProfileView/context/ProfileViewContext.tsx index 2354ae3b335..8fbc5c9f5d6 100644 --- a/ui/packages/shared/profile/src/ProfileView/context/ProfileViewContext.tsx +++ b/ui/packages/shared/profile/src/ProfileView/context/ProfileViewContext.tsx @@ -14,15 +14,27 @@ import {ReactNode, createContext, useContext} from 'react'; import {ProfileSource} from '../../ProfileSource'; +import {NumberDuo} from '../../utils'; + +export type TimelineGuideData = + | {show: false} + | { + show: true; + props: { + bounds: NumberDuo; + }; + }; interface Props { profileSource?: ProfileSource; compareMode: boolean; + timelineGuide?: TimelineGuideData; } export const defaultValue: Props = { profileSource: undefined, compareMode: false, + timelineGuide: {show: false}, }; const ProfileViewContext = createContext(defaultValue); diff --git a/ui/packages/shared/profile/src/ProfileView/index.tsx b/ui/packages/shared/profile/src/ProfileView/index.tsx index c8414131ddc..96bf25dd778 100644 --- a/ui/packages/shared/profile/src/ProfileView/index.tsx +++ b/ui/packages/shared/profile/src/ProfileView/index.tsx @@ -42,7 +42,7 @@ export const ProfileView = ({ pprofDownloading, compare, showVisualizationSelector, - showTimelineGuide, + timelineGuide, }: ProfileViewProps): JSX.Element => { const { timezone, @@ -110,7 +110,6 @@ export const ProfileView = ({ setSearchString, callgraphSVG, perf, - showTimelineGuide, }); }; @@ -131,7 +130,7 @@ export const ProfileView = ({ return ( - + { const metadata = useGrpcMetadata(); const [dashboardItems] = useURLState('dashboard_items', { @@ -44,7 +47,10 @@ export const ProfileViewWithData = ({ const [sourceBuildID] = useURLState('source_buildid'); const [sourceFilename] = useURLState('source_filename'); const [groupBy] = useURLState('group_by', { - defaultValue: [FIELD_FUNCTION_NAME], + defaultValue: [ + isGroupByTimestamp === true ? FIELD_TIMESTAMP : (null as unknown as string), + FIELD_FUNCTION_NAME, + ].filter(Boolean), alwaysReturnArray: true, }); @@ -244,7 +250,7 @@ export const ProfileViewWithData = ({ onDownloadPProf={() => void downloadPProfClick()} pprofDownloading={pprofDownloading} showVisualizationSelector={showVisualizationSelector} - showTimelineGuide={showTimelineGuide} + timelineGuide={timelineGuide} /> ); }; diff --git a/ui/packages/shared/profile/src/TimelineGuide/index.tsx b/ui/packages/shared/profile/src/TimelineGuide/index.tsx index 44403be73ae..2b9035004df 100644 --- a/ui/packages/shared/profile/src/TimelineGuide/index.tsx +++ b/ui/packages/shared/profile/src/TimelineGuide/index.tsx @@ -88,11 +88,6 @@ export const TimelineGuide = ({bounds, width, height, margin, ticks}: Props): JS y1={-height + 20} y2={-height + 20} /> - {/* - - Time - - */}
diff --git a/ui/packages/shared/profile/src/index.tsx b/ui/packages/shared/profile/src/index.tsx index a5a31b4279d..69460afc323 100644 --- a/ui/packages/shared/profile/src/index.tsx +++ b/ui/packages/shared/profile/src/index.tsx @@ -25,6 +25,7 @@ export * from './ProfileViewWithData'; export * from './utils'; export * from './ProfileTypeSelector'; export * from './SourceView'; +export * from './ProfileMetricsGraph'; export {default as Callgraph} from './Callgraph'; export const DEFAULT_PROFILE_EXPLORER_PARAM_VALUES = {