From d8693cb01047434362cf7650b06331627fa8a287 Mon Sep 17 00:00:00 2001 From: Manoj Vivek Date: Mon, 18 Nov 2024 12:26:33 +0530 Subject: [PATCH 1/8] Extracting TimelineGuide from MetricGraphStrips and making it reusable --- .../MetricsGraphStrips/AreaGraph/index.tsx | 4 ++-- .../profile/src/MetricsGraphStrips/index.tsx | 20 +++++++++++++++---- .../TimelineGuide/index.tsx | 19 ++++-------------- ui/packages/shared/profile/src/utils.ts | 2 ++ 4 files changed, 24 insertions(+), 21 deletions(-) rename ui/packages/shared/profile/src/{MetricsGraphStrips => }/TimelineGuide/index.tsx (84%) diff --git a/ui/packages/shared/profile/src/MetricsGraphStrips/AreaGraph/index.tsx b/ui/packages/shared/profile/src/MetricsGraphStrips/AreaGraph/index.tsx index aa16dbcb98b..44b915280d4 100644 --- a/ui/packages/shared/profile/src/MetricsGraphStrips/AreaGraph/index.tsx +++ b/ui/packages/shared/profile/src/MetricsGraphStrips/AreaGraph/index.tsx @@ -17,13 +17,13 @@ import {Icon} from '@iconify/react'; import cx from 'classnames'; import * as d3 from 'd3'; +import {NumberDuo} from '../../utils'; + export interface DataPoint { timestamp: number; value: number; } -export type NumberDuo = [number, number]; - interface Props { width: number; height: number; diff --git a/ui/packages/shared/profile/src/MetricsGraphStrips/index.tsx b/ui/packages/shared/profile/src/MetricsGraphStrips/index.tsx index e3423f01700..638e2b1ba61 100644 --- a/ui/packages/shared/profile/src/MetricsGraphStrips/index.tsx +++ b/ui/packages/shared/profile/src/MetricsGraphStrips/index.tsx @@ -11,13 +11,14 @@ // See the License for the specific language governing permissions and // limitations under the License. -import {useState} from 'react'; +import {useMemo, useState} from 'react'; import {Icon} from '@iconify/react'; import * as d3 from 'd3'; -import {AreaGraph, DataPoint, NumberDuo} from './AreaGraph'; -import {TimelineGuide} from './TimelineGuide'; +import {TimelineGuide} from '../TimelineGuide'; +import {NumberDuo} from '../utils'; +import {AreaGraph, DataPoint} from './AreaGraph'; interface Props { cpus: string[]; @@ -44,10 +45,21 @@ export const MetricsGraphStrips = ({ // @ts-expect-error const color = d3.scaleOrdinal(d3.schemeObservable10); + const bounds = useMemo(() => { + const bounds: NumberDuo = [Infinity, -Infinity]; + data.forEach(cpuData => { + cpuData.forEach(dataPoint => { + bounds[0] = Math.min(bounds[0], dataPoint.timestamp); + bounds[1] = Math.max(bounds[1], dataPoint.timestamp); + }); + }); + return [0, bounds[1] - bounds[0]] as NumberDuo; + }, [data]); + return (
{ @@ -35,18 +35,7 @@ const alignBeforeAxisCorrection = (val: number): number => { return 0; }; -export const TimelineGuide = ({data, width, height, margin}: Props): JSX.Element => { - const bounds = useMemo(() => { - const bounds: NumberDuo = [Infinity, -Infinity]; - data.forEach(cpuData => { - cpuData.forEach(dataPoint => { - bounds[0] = Math.min(bounds[0], dataPoint.timestamp); - bounds[1] = Math.max(bounds[1], dataPoint.timestamp); - }); - }); - return [0, bounds[1] - bounds[0]]; - }, [data]); - +export const TimelineGuide = ({bounds, width, height, margin}: Props): JSX.Element => { const xScale = d3.scaleLinear().domain(bounds).range([0, width]); return ( diff --git a/ui/packages/shared/profile/src/utils.ts b/ui/packages/shared/profile/src/utils.ts index f1a22d98c77..6c09f355bca 100644 --- a/ui/packages/shared/profile/src/utils.ts +++ b/ui/packages/shared/profile/src/utils.ts @@ -59,3 +59,5 @@ export const truncateStringReverse = (str: string, num: number): string => { return '...' + str.slice(str.length - num); }; + +export type NumberDuo = [number, number]; From e4d332a1054647310fafbfbb91742c747b663a94 Mon Sep 17 00:00:00 2001 From: Manoj Vivek Date: Wed, 27 Nov 2024 19:35:21 +0530 Subject: [PATCH 2/8] Added ticks count to the props --- ui/packages/shared/profile/src/TimelineGuide/index.tsx | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/ui/packages/shared/profile/src/TimelineGuide/index.tsx b/ui/packages/shared/profile/src/TimelineGuide/index.tsx index f9d8e73c64e..f63f1486eca 100644 --- a/ui/packages/shared/profile/src/TimelineGuide/index.tsx +++ b/ui/packages/shared/profile/src/TimelineGuide/index.tsx @@ -22,6 +22,7 @@ interface Props { height: number; margin: number; bounds: NumberDuo; + ticks?: number; } const alignBeforeAxisCorrection = (val: number): number => { @@ -35,7 +36,7 @@ const alignBeforeAxisCorrection = (val: number): number => { return 0; }; -export const TimelineGuide = ({bounds, width, height, margin}: Props): JSX.Element => { +export const TimelineGuide = ({bounds, width, height, margin, ticks}: Props): JSX.Element => { const xScale = d3.scaleLinear().domain(bounds).range([0, width]); return ( @@ -49,7 +50,7 @@ export const TimelineGuide = ({bounds, width, height, margin}: Props): JSX.Eleme textAnchor="middle" transform={`translate(0,${height - margin})`} > - {xScale.ticks().map((d, i) => ( + {xScale.ticks(ticks).map((d, i) => ( Date: Wed, 27 Nov 2024 19:36:31 +0530 Subject: [PATCH 3/8] Removed the unwanted styles --- ui/packages/shared/profile/src/TimelineGuide/index.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ui/packages/shared/profile/src/TimelineGuide/index.tsx b/ui/packages/shared/profile/src/TimelineGuide/index.tsx index f63f1486eca..6283cde2595 100644 --- a/ui/packages/shared/profile/src/TimelineGuide/index.tsx +++ b/ui/packages/shared/profile/src/TimelineGuide/index.tsx @@ -65,7 +65,7 @@ export const TimelineGuide = ({bounds, width, height, margin, ticks}: Props): JS Date: Wed, 4 Dec 2024 19:05:24 +0530 Subject: [PATCH 4/8] MetricsGraphStrips props renamed --- .../MetricsGraphStrips.stories.tsx | 14 +++--- .../profile/src/MetricsGraphStrips/index.tsx | 48 ++++++++++++++----- 2 files changed, 43 insertions(+), 19 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..027193e85ba 100644 --- a/ui/packages/shared/profile/src/MetricsGraphStrips/MetricsGraphStrips.stories.tsx +++ b/ui/packages/shared/profile/src/MetricsGraphStrips/MetricsGraphStrips.stories.tsx @@ -48,19 +48,19 @@ export const ThreeCPUStrips = { args: { cpus: Array.from(mockData, (_, i) => `CPU ${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} From 6b8e93f4e5bd26defc4cf813392c7b15f98766b9 Mon Sep 17 00:00:00 2001 From: Manoj Vivek Date: Thu, 5 Dec 2024 11:44:46 +0530 Subject: [PATCH 5/8] More refactoring and state integration --- .../IcicleGraphArrow/index.tsx | 1 + .../profile/src/ProfileIcicleGraph/index.tsx | 10 ++++------ .../components/DashboardItems/index.tsx | 3 --- .../ProfileView/context/ProfileViewContext.tsx | 13 +++++++++++++ .../shared/profile/src/ProfileView/index.tsx | 5 ++--- .../src/ProfileView/types/visualization.ts | 2 ++ .../shared/profile/src/ProfileViewWithData.tsx | 16 +++++++++++----- .../shared/profile/src/TimelineGuide/index.tsx | 5 ----- ui/packages/shared/profile/src/index.tsx | 1 + 9 files changed, 34 insertions(+), 22 deletions(-) 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..05518fa5022 100644 --- a/ui/packages/shared/profile/src/ProfileView/context/ProfileViewContext.tsx +++ b/ui/packages/shared/profile/src/ProfileView/context/ProfileViewContext.tsx @@ -13,16 +13,29 @@ import {ReactNode, createContext, useContext} from 'react'; +import {NumberDuo} from 'utils'; + import {ProfileSource} from '../../ProfileSource'; +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 = { From 097f21bbe93688582535f2beea9cc2cbec509cf7 Mon Sep 17 00:00:00 2001 From: Manoj Vivek Date: Thu, 5 Dec 2024 11:55:58 +0530 Subject: [PATCH 6/8] Storybook sotry fix --- .../src/MetricsGraphStrips/MetricsGraphStrips.stories.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ui/packages/shared/profile/src/MetricsGraphStrips/MetricsGraphStrips.stories.tsx b/ui/packages/shared/profile/src/MetricsGraphStrips/MetricsGraphStrips.stories.tsx index 027193e85ba..5627940b79c 100644 --- a/ui/packages/shared/profile/src/MetricsGraphStrips/MetricsGraphStrips.stories.tsx +++ b/ui/packages/shared/profile/src/MetricsGraphStrips/MetricsGraphStrips.stories.tsx @@ -46,7 +46,7 @@ export default meta; export const ThreeCPUStrips = { args: { - cpus: Array.from(mockData, (_, i) => `CPU ${i + 1}`), + cpus: Array.from(mockData, (_, i) => ({labels: {cpuid: i + 1}})), data: mockData, selectedTimeframe: {index: 1, bounds: [mockData[0][25].timestamp, mockData[0][100].timestamp]}, onSelectedTimeframe: (index: number, bounds: NumberDuo): void => { From 4abf0a549bde0a152a589bb63115a49728509cd0 Mon Sep 17 00:00:00 2001 From: Manoj Vivek Date: Thu, 5 Dec 2024 11:59:16 +0530 Subject: [PATCH 7/8] Type fix --- .../profile/src/ProfileView/context/ProfileViewContext.tsx | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/ui/packages/shared/profile/src/ProfileView/context/ProfileViewContext.tsx b/ui/packages/shared/profile/src/ProfileView/context/ProfileViewContext.tsx index 05518fa5022..8fbc5c9f5d6 100644 --- a/ui/packages/shared/profile/src/ProfileView/context/ProfileViewContext.tsx +++ b/ui/packages/shared/profile/src/ProfileView/context/ProfileViewContext.tsx @@ -13,9 +13,8 @@ import {ReactNode, createContext, useContext} from 'react'; -import {NumberDuo} from 'utils'; - import {ProfileSource} from '../../ProfileSource'; +import {NumberDuo} from '../../utils'; export type TimelineGuideData = | {show: false} From 5bd84b4b9117dd45cb08bbe126697bc2ce9f0acc Mon Sep 17 00:00:00 2001 From: Manoj Vivek Date: Thu, 5 Dec 2024 12:06:14 +0530 Subject: [PATCH 8/8] Storybook mock data fix --- .../src/MetricsGraphStrips/MetricsGraphStrips.stories.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ui/packages/shared/profile/src/MetricsGraphStrips/MetricsGraphStrips.stories.tsx b/ui/packages/shared/profile/src/MetricsGraphStrips/MetricsGraphStrips.stories.tsx index 5627940b79c..13fa0185908 100644 --- a/ui/packages/shared/profile/src/MetricsGraphStrips/MetricsGraphStrips.stories.tsx +++ b/ui/packages/shared/profile/src/MetricsGraphStrips/MetricsGraphStrips.stories.tsx @@ -46,7 +46,7 @@ export default meta; export const ThreeCPUStrips = { args: { - cpus: Array.from(mockData, (_, i) => ({labels: {cpuid: i + 1}})), + cpus: Array.from(mockData, (_, i) => ({labels: [{name: 'cpuid', value: i + 1}]})), data: mockData, selectedTimeframe: {index: 1, bounds: [mockData[0][25].timestamp, mockData[0][100].timestamp]}, onSelectedTimeframe: (index: number, bounds: NumberDuo): void => {