-
Notifications
You must be signed in to change notification settings - Fork 59
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
fix: add cached info to timeline (#347)
* fix: add cached info to timeline * chore: update chart bar item tooltip text and position * chore: add loading state + simplify storybook * chore: add basic tests * fix: partial fix to avoid ChartBar crash for aborted tasks which are still wrongly marked as running Signed-off-by: Nastya Rusina <nastya@union.ai>
- Loading branch information
Showing
24 changed files
with
643 additions
and
279 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
25 changes: 25 additions & 0 deletions
25
src/components/Executions/ExecutionDetails/Timeline/BarChart/BarChart.stories.tsx
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
import { ComponentMeta, ComponentStory } from '@storybook/react'; | ||
import * as React from 'react'; | ||
import { NodeExecutionPhase } from 'models/Execution/enums'; | ||
import { BarChart } from '.'; | ||
|
||
const barItems = [ | ||
{ phase: NodeExecutionPhase.FAILED, startOffsetSec: 0, durationSec: 5, isFromCache: false }, | ||
{ phase: NodeExecutionPhase.SUCCEEDED, startOffsetSec: 10, durationSec: 2, isFromCache: true }, | ||
{ phase: NodeExecutionPhase.SUCCEEDED, startOffsetSec: 0, durationSec: 1, isFromCache: true }, | ||
{ phase: NodeExecutionPhase.RUNNING, startOffsetSec: 0, durationSec: 10, isFromCache: false }, | ||
{ phase: NodeExecutionPhase.UNDEFINED, startOffsetSec: 15, durationSec: 25, isFromCache: false }, | ||
{ phase: NodeExecutionPhase.SUCCEEDED, startOffsetSec: 7, durationSec: 20, isFromCache: false }, | ||
]; | ||
|
||
export default { | ||
title: 'Workflow/Timeline', | ||
component: BarChart, | ||
} as ComponentMeta<typeof BarChart>; | ||
|
||
const Template: ComponentStory<typeof BarChart> = (args) => <BarChart {...args} />; | ||
export const BarSection = Template.bind({}); | ||
BarSection.args = { | ||
items: barItems, | ||
chartTimeIntervalSec: 1, | ||
}; |
49 changes: 49 additions & 0 deletions
49
src/components/Executions/ExecutionDetails/Timeline/BarChart/BarChartSingleItem.stories.tsx
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,49 @@ | ||
import { ComponentMeta, ComponentStory } from '@storybook/react'; | ||
import * as React from 'react'; | ||
import { NodeExecutionPhase } from 'models/Execution/enums'; | ||
import { BarItemData } from './utils'; | ||
import { BarChart } from '.'; | ||
|
||
const phaseEnumTyping = { | ||
options: Object.values(NodeExecutionPhase), | ||
mapping: Object.values(NodeExecutionPhase), | ||
control: { | ||
type: 'select', | ||
labels: Object.keys(NodeExecutionPhase), | ||
}, | ||
}; | ||
|
||
interface SingleItemProps extends BarItemData { | ||
chartTimeIntervalSec: number; | ||
} | ||
|
||
/** | ||
* This is a fake storybook only component, to allow ease experimentation whith single bar item | ||
*/ | ||
const SingleBarItem = (props: SingleItemProps) => { | ||
const items = [props]; | ||
return <BarChart items={items} chartTimeIntervalSec={props.chartTimeIntervalSec} />; | ||
}; | ||
|
||
export default { | ||
title: 'Workflow/Timeline', | ||
component: SingleBarItem, | ||
// 👇 Creates specific argTypes | ||
argTypes: { | ||
phase: phaseEnumTyping, | ||
}, | ||
} as ComponentMeta<typeof SingleBarItem>; | ||
|
||
const TemplateSingleItem: ComponentStory<typeof SingleBarItem> = (args) => ( | ||
<SingleBarItem {...args} /> | ||
); | ||
|
||
export const BarChartSingleItem = TemplateSingleItem.bind({}); | ||
// const phaseDataSingle = generateChartData([barItems[0]]); | ||
BarChartSingleItem.args = { | ||
phase: NodeExecutionPhase.ABORTED, | ||
startOffsetSec: 15, | ||
durationSec: 30, | ||
isFromCache: false, | ||
chartTimeIntervalSec: 5, | ||
}; |
60 changes: 60 additions & 0 deletions
60
src/components/Executions/ExecutionDetails/Timeline/BarChart/barOptions.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,60 @@ | ||
import { Chart as ChartJS, registerables, Tooltip } from 'chart.js'; | ||
import ChartDataLabels from 'chartjs-plugin-datalabels'; | ||
|
||
ChartJS.register(...registerables, ChartDataLabels); | ||
|
||
// Create positioner to put tooltip at cursor position | ||
Tooltip.positioners.cursor = function (_chartElements, coordinates) { | ||
return coordinates; | ||
}; | ||
|
||
export const getBarOptions = (chartTimeIntervalSec: number, tooltipLabels: string[][]) => { | ||
return { | ||
animation: false as const, | ||
indexAxis: 'y' as const, | ||
elements: { | ||
bar: { | ||
borderWidth: 2, | ||
}, | ||
}, | ||
responsive: true, | ||
maintainAspectRatio: false, | ||
plugins: { | ||
legend: { | ||
display: false, | ||
}, | ||
title: { | ||
display: false, | ||
}, | ||
tooltip: { | ||
// Setting up tooltip: https://www.chartjs.org/docs/latest/configuration/tooltip.html | ||
position: 'cursor', | ||
filter: function (tooltipItem) { | ||
// no tooltip for offsets | ||
return tooltipItem.datasetIndex === 1; | ||
}, | ||
callbacks: { | ||
label: function (context) { | ||
const index = context.dataIndex; | ||
return tooltipLabels[index] ?? ''; | ||
}, | ||
}, | ||
}, | ||
}, | ||
scales: { | ||
x: { | ||
format: Intl.DateTimeFormat, | ||
position: 'top' as const, | ||
ticks: { | ||
display: false, | ||
autoSkip: false, | ||
stepSize: chartTimeIntervalSec, | ||
}, | ||
stacked: true, | ||
}, | ||
y: { | ||
stacked: true, | ||
}, | ||
}, | ||
}; | ||
}; |
75 changes: 75 additions & 0 deletions
75
src/components/Executions/ExecutionDetails/Timeline/BarChart/chartData.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,75 @@ | ||
import { timestampToDate } from 'common/utils'; | ||
import { CatalogCacheStatus, NodeExecutionPhase } from 'models/Execution/enums'; | ||
import { dNode } from 'models/Graph/types'; | ||
import { BarItemData } from './utils'; | ||
|
||
const WEEK_DURATION_SEC = 7 * 24 * 3600; | ||
|
||
const EMPTY_BAR_ITEM: BarItemData = { | ||
phase: NodeExecutionPhase.UNDEFINED, | ||
startOffsetSec: 0, | ||
durationSec: 0, | ||
isFromCache: false, | ||
}; | ||
|
||
export const getChartDurationData = ( | ||
nodes: dNode[], | ||
startedAt: Date, | ||
): { items: BarItemData[]; totalDurationSec: number } => { | ||
if (nodes.length === 0) return { items: [], totalDurationSec: 0 }; | ||
|
||
let totalDurationSec = 0; | ||
const initialStartTime = startedAt.getTime(); | ||
const result: BarItemData[] = nodes.map(({ execution }) => { | ||
if (!execution) { | ||
return EMPTY_BAR_ITEM; | ||
} | ||
|
||
let phase = execution.closure.phase; | ||
const isFromCache = | ||
execution.closure.taskNodeMetadata?.cacheStatus === CatalogCacheStatus.CACHE_HIT; | ||
|
||
// Offset values | ||
let startOffset = 0; | ||
const startedAt = execution.closure.startedAt; | ||
if (isFromCache) { | ||
if (execution.closure.createdAt) { | ||
startOffset = timestampToDate(execution.closure.createdAt).getTime() - initialStartTime; | ||
} | ||
} else if (startedAt) { | ||
startOffset = timestampToDate(startedAt).getTime() - initialStartTime; | ||
} | ||
|
||
// duration | ||
let durationSec = 0; | ||
if (isFromCache) { | ||
const updatedAt = execution.closure.updatedAt?.seconds?.toNumber() ?? 0; | ||
const createdAt = execution.closure.createdAt?.seconds?.toNumber() ?? 0; | ||
durationSec = updatedAt - createdAt; | ||
durationSec = durationSec === 0 ? 2 : durationSec; | ||
} else if (phase === NodeExecutionPhase.RUNNING) { | ||
if (startedAt) { | ||
const duration = Date.now() - timestampToDate(startedAt).getTime(); | ||
durationSec = duration / 1000; | ||
if (durationSec > WEEK_DURATION_SEC) { | ||
// TODO: https://github.com/flyteorg/flyteconsole/issues/332 | ||
// In some cases tasks which were needed to be ABORTED are stuck in running state, | ||
// In case if task is still running after a week - we assume it should have been aborted. | ||
// The proper fix should be covered by isue: flyteconsole#332 | ||
phase = NodeExecutionPhase.ABORTED; | ||
const allegedDurationSec = Math.trunc(totalDurationSec - startOffset / 1000); | ||
durationSec = allegedDurationSec > 0 ? allegedDurationSec : 10; | ||
} | ||
} | ||
} else { | ||
durationSec = execution.closure.duration?.seconds?.toNumber() ?? 0; | ||
} | ||
|
||
const startOffsetSec = Math.trunc(startOffset / 1000); | ||
totalDurationSec = Math.max(totalDurationSec, startOffsetSec + durationSec); | ||
return { phase, startOffsetSec, durationSec, isFromCache }; | ||
}); | ||
|
||
// Do we want to get initialStartTime from different place, to avoid recalculating it. | ||
return { items: result, totalDurationSec }; | ||
}; |
20 changes: 20 additions & 0 deletions
20
src/components/Executions/ExecutionDetails/Timeline/BarChart/index.tsx
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
import * as React from 'react'; | ||
import { Bar } from 'react-chartjs-2'; | ||
import { getBarOptions } from './barOptions'; | ||
import { BarItemData, generateChartData, getChartData } from './utils'; | ||
|
||
interface BarChartProps { | ||
items: BarItemData[]; | ||
chartTimeIntervalSec: number; | ||
} | ||
|
||
export const BarChart = (props: BarChartProps) => { | ||
const phaseData = generateChartData(props.items); | ||
|
||
return ( | ||
<Bar | ||
options={getBarOptions(props.chartTimeIntervalSec, phaseData.tooltipLabel) as any} | ||
data={getChartData(phaseData)} | ||
/> | ||
); | ||
}; |
139 changes: 139 additions & 0 deletions
139
src/components/Executions/ExecutionDetails/Timeline/BarChart/utils.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,139 @@ | ||
import { getNodeExecutionPhaseConstants } from 'components/Executions/utils'; | ||
import { primaryTextColor } from 'components/Theme/constants'; | ||
import { NodeExecutionPhase } from 'models/Execution/enums'; | ||
|
||
export const CASHED_GREEN = 'rgba(74,227,174,0.25)'; // statusColors.SUCCESS (Mint20) with 25% opacity | ||
export const TRANSPARENT = 'rgba(0, 0, 0, 0)'; | ||
|
||
export enum RelationToCache { | ||
None = 'none', | ||
ReadFromCaceh = 'Read from Cache', | ||
WroteToCache = 'Wrote to cache', | ||
} | ||
|
||
export interface BarItemData { | ||
phase: NodeExecutionPhase; | ||
startOffsetSec: number; | ||
durationSec: number; | ||
isFromCache: boolean; | ||
} | ||
|
||
interface ChartDataInput { | ||
elementsNumber: number; | ||
durations: number[]; | ||
startOffset: number[]; | ||
offsetColor: string[]; | ||
tooltipLabel: string[][]; | ||
barLabel: string[]; | ||
barColor: string[]; | ||
} | ||
|
||
/** | ||
* Depending on amounf of second provided shows data in | ||
* XhXmXs or XmXs or Xs format | ||
*/ | ||
export const formatSecondsToHmsFormat = (seconds: number) => { | ||
const hours = Math.floor(seconds / 3600); | ||
seconds %= 3600; | ||
const minutes = Math.floor(seconds / 60); | ||
seconds = seconds % 60; | ||
if (hours > 0) { | ||
return `${hours}h ${minutes}m ${seconds}s`; | ||
} else if (minutes > 0) { | ||
return `${minutes}m ${seconds}s`; | ||
} | ||
return `${seconds}s`; | ||
}; | ||
|
||
// narusina - check if exports are still needed | ||
export const getOffsetColor = (isCachedValue: boolean[]) => { | ||
const colors = isCachedValue.map((val) => (val === true ? CASHED_GREEN : TRANSPARENT)); | ||
return colors; | ||
}; | ||
|
||
/** | ||
* Generates chart data maps per each BarItemData ("node") section | ||
*/ | ||
export const generateChartData = (data: BarItemData[]): ChartDataInput => { | ||
const durations: number[] = []; | ||
const startOffset: number[] = []; | ||
const offsetColor: string[] = []; | ||
const tooltipLabel: string[][] = []; | ||
const barLabel: string[] = []; | ||
const barColor: string[] = []; | ||
|
||
data.forEach((element) => { | ||
const phaseConstant = getNodeExecutionPhaseConstants( | ||
element.phase ?? NodeExecutionPhase.UNDEFINED, | ||
); | ||
|
||
const durationString = formatSecondsToHmsFormat(element.durationSec); | ||
const tooltipString = `${phaseConstant.text}: ${durationString}`; | ||
// don't show Label if there is now duration yet. | ||
const labelString = element.durationSec > 0 ? durationString : ''; | ||
|
||
durations.push(element.durationSec); | ||
startOffset.push(element.startOffsetSec); | ||
offsetColor.push(element.isFromCache ? CASHED_GREEN : TRANSPARENT); | ||
tooltipLabel.push(element.isFromCache ? [tooltipString, 'Read from cache'] : [tooltipString]); | ||
barLabel.push(element.isFromCache ? '\u229A From cache' : labelString); | ||
barColor.push(phaseConstant.badgeColor); | ||
}); | ||
|
||
return { | ||
elementsNumber: data.length, | ||
durations, | ||
startOffset, | ||
offsetColor, | ||
tooltipLabel, | ||
barLabel, | ||
barColor, | ||
}; | ||
}; | ||
|
||
/** | ||
* Generates chart data format suitable for Chart.js Bar. Each bar consists of two data items: | ||
* |-----------|XXXXXXXXXXXXXXXX| | ||
* |-|XXXXXX| | ||
* |------|XXXXXXXXXXXXX| | ||
* Where |---| is offset - usually transparent part to give user a feeling that timeline wasn't started from ZERO time position | ||
* Where |XXX| is duration of the operation, colored per step Phase status. | ||
*/ | ||
export const getChartData = (data: ChartDataInput) => { | ||
const defaultStyle = { | ||
barPercentage: 1, | ||
borderWidth: 0, | ||
}; | ||
|
||
return { | ||
labels: Array(data.elementsNumber).fill(''), // clear up Chart Bar default labels | ||
datasets: [ | ||
// fill-in offsets | ||
{ | ||
...defaultStyle, | ||
data: data.startOffset, | ||
backgroundColor: data.offsetColor, | ||
datalabels: { | ||
labels: { | ||
title: null, | ||
}, | ||
}, | ||
}, | ||
// fill in duration bars | ||
{ | ||
...defaultStyle, | ||
data: data.durations, | ||
backgroundColor: data.barColor, | ||
datalabels: { | ||
// Positioning info - https://chartjs-plugin-datalabels.netlify.app/guide/positioning.html | ||
color: primaryTextColor, | ||
align: 'end' as const, // related to text | ||
anchor: 'start' as const, // related to bar | ||
formatter: function (value, context) { | ||
return data.barLabel[context.dataIndex] ?? ''; | ||
}, | ||
}, | ||
}, | ||
], | ||
}; | ||
}; |
Oops, something went wrong.