From d23031d85cee8cbb996ec0813358511215485839 Mon Sep 17 00:00:00 2001 From: Anton Gilgur Date: Sun, 10 Sep 2023 14:28:25 -0400 Subject: [PATCH 1/3] refactor(ui): migrate reports to functional component - follow a very similar process to 25e1939e25551cd15d89bd47e4232c8073b40a9c - `state` -> `useState` - `this.props` -> arguments - `BasePage` + `this.queryParams` -> `location` argument + `URLSearchParams` - `componentDidMount` -> `useEffect` - `services.info.collectEvent` -> `useCollectEvent` - `saveHistory` -> `useEffect` + `historyUrl` - `Consumer` -> `useContext` - merge `renderReport` into the main component - will move filters into a separate component in the next commit, following existing convention and the commit - Promise chains -> async/await - getters and setters to regular functions - split out a pure function - can move this into a util file in the next commit - also modify some comments in `cron-workflows-list` - convert variable assigned to anonymous function to a named function - better practice, better tracing, less memory, etc Signed-off-by: Anton Gilgur --- .../cron-workflow-list/cron-workflow-list.tsx | 10 +- ui/src/app/reports/components/reports.tsx | 499 +++++++++--------- 2 files changed, 250 insertions(+), 259 deletions(-) diff --git a/ui/src/app/cron-workflows/components/cron-workflow-list/cron-workflow-list.tsx b/ui/src/app/cron-workflows/components/cron-workflow-list/cron-workflow-list.tsx index e5c7b0b08e48..5c1f60a3b288 100644 --- a/ui/src/app/cron-workflows/components/cron-workflow-list/cron-workflow-list.tsx +++ b/ui/src/app/cron-workflows/components/cron-workflow-list/cron-workflow-list.tsx @@ -26,13 +26,12 @@ require('./cron-workflow-list.scss'); const learnMore = Learn more; -export const CronWorkflowList = ({match, location, history}: RouteComponentProps) => { - // boiler-plate +export function CronWorkflowList ({match, location, history}: RouteComponentProps) { const queryParams = new URLSearchParams(location.search); const {navigation} = useContext(Context); - // state for URL, query and label parameters - const [namespace, setNamespace] = useState(Utils.getNamespace(match.params.namespace) || ''); + // state for URL, query, and label parameters + const [namespace, setNamespace] = useState(Utils.getNamespace(match.params.namespace) || ''); const [sidePanel, setSidePanel] = useState(queryParams.get('sidePanel') === 'true'); const [labels, setLabels] = useState([]); const [states, setStates] = useState(['Running', 'Suspended']); // check all by default @@ -44,6 +43,7 @@ export const CronWorkflowList = ({match, location, history}: RouteComponentProps [history] ); + // save history useEffect( () => history.push( @@ -176,4 +176,4 @@ export const CronWorkflowList = ({match, location, history}: RouteComponentProps ); -}; +} diff --git a/ui/src/app/reports/components/reports.tsx b/ui/src/app/reports/components/reports.tsx index a4feac527964..c2d3e0bb77f4 100644 --- a/ui/src/app/reports/components/reports.tsx +++ b/ui/src/app/reports/components/reports.tsx @@ -2,20 +2,22 @@ import {Page} from 'argo-ui/src/index'; import {ChartOptions} from 'chart.js'; import 'chartjs-plugin-annotation'; import * as React from 'react'; +import {useContext, useEffect, useState} from 'react'; import {Bar, ChartData} from 'react-chartjs-2'; import {RouteComponentProps} from 'react-router-dom'; import {getColorForNodePhase, NODE_PHASE, Workflow} from '../../../models'; import {uiUrl} from '../../shared/base'; -import {BasePage} from '../../shared/components/base-page'; import {DataLoaderDropdown} from '../../shared/components/data-loader-dropdown'; import {ErrorNotice} from '../../shared/components/error-notice'; import {InfoIcon} from '../../shared/components/fa-icons'; import {NamespaceFilter} from '../../shared/components/namespace-filter'; import {TagsInput} from '../../shared/components/tags-input/tags-input'; +import {useCollectEvent} from '../../shared/components/use-collect-event'; import {ZeroState} from '../../shared/components/zero-state'; -import {Consumer, ContextApis} from '../../shared/context'; +import {Context} from '../../shared/context'; import {denominator} from '../../shared/duration'; import {Footnote} from '../../shared/footnote'; +import {historyUrl} from '../../shared/history'; import {services} from '../../shared/services'; import {Utils} from '../../shared/utils'; @@ -24,238 +26,150 @@ interface Chart { options: ChartOptions; } -interface State { - namespace: string; - labels: string[]; - autocompleteLabels: string[]; - error?: Error; - charts?: Chart[]; -} - const limit = 100; const labelKeyPhase = 'workflows.argoproj.io/phase'; const labelKeyWorkflowTemplate = 'workflows.argoproj.io/workflow-template'; const labelKeyCronWorkflow = 'workflows.argoproj.io/cron-workflow'; -export class Reports extends BasePage, State> { - private get phase() { - return this.getLabel(labelKeyPhase); - } +export function Reports ({match, location, history}: RouteComponentProps) { + const queryParams = new URLSearchParams(location.search); + const {navigation} = useContext(Context); - private set phase(value: string) { - this.setLabel(labelKeyPhase, value); - } + // state for URL, query, and label parameters + const [namespace, setNamespace] = useState(Utils.getNamespace(match.params.namespace) || ''); + const [labels, setLabels] = useState((queryParams.get('labels') || '').split(',').filter(v => v !== '')); + const [autocompleteLabels, setAutocompleteLabels] = useState(['']); + const [charts, setCharts] = useState(); + const [error, setError] = useState(); - private set workflowTemplate(value: string) { - this.setLabel(labelKeyWorkflowTemplate, value); - } + // save history + useEffect(() => { + history.push(historyUrl('reports' + (Utils.managedNamespace ? '' : '/{namespace}'), {namespace, labels: labels.join(',')})) + }, [namespace, labels]); - private set cronWorkflow(value: string) { - this.setLabel(labelKeyCronWorkflow, value); - } + async function fetchReport(newNamespace: string, newLabels: string[]) { + if (newNamespace === '' || newLabels.length === 0) { + setNamespace(newNamespace); + setLabels(newLabels); + setCharts(null); + return; + } - constructor(props: RouteComponentProps, context: any) { - super(props, context); - this.state = { - namespace: Utils.getNamespace(this.props.match.params.namespace) || '', - labels: (this.queryParam('labels') || '').split(',').filter(v => v !== ''), - autocompleteLabels: [''] - }; + try { + const list = await services.workflows.list(newNamespace, [], newLabels, {limit}, [ + 'items.metadata.name', + 'items.status.phase', + 'items.status.startedAt', + 'items.status.finishedAt', + 'items.status.resourcesDuration' + ]); + const newCharts = getExtractDatasets(list.items || []); + setNamespace(newNamespace); + setLabels(newLabels); + setCharts(newCharts); + setError(null); + } catch (newError) { + setError(newError); + } } - public componentDidMount() { - this.fetchReport(this.state.namespace, this.state.labels); - services.info.collectEvent('openedReports').then(); + function getLabel(name: string) { + return (labels.find(label => label.startsWith(name)) || '').replace(name + '=', ''); } - public render() { - return ( - - {ctx => ( - -
-
{this.renderFilters()}
- -
{this.renderReport(ctx)}
-
-
- )} -
- ); + function setLabel(name: string, value: string) { + fetchReport(namespace, labels.filter(label => !label.startsWith(name)).concat(name + '=' + value)); } - private getLabel(name: string) { - return (this.state.labels.find(label => label.startsWith(name)) || '').replace(name + '=', ''); + function getPhase() { + return getLabel(labelKeyPhase); } - private setLabel(name: string, value: string) { - this.fetchReport(this.state.namespace, this.state.labels.filter(label => !label.startsWith(name)).concat(name + '=' + value)); + function setPhase(value: string) { + setLabel(labelKeyPhase, value); } - private fetchReport(namespace: string, labels: string[]) { - if (namespace === '' || labels.length === 0) { - this.setState({namespace, labels, charts: null}); - return; - } - services.workflows - .list(namespace, [], labels, {limit}, [ - 'items.metadata.name', - 'items.status.phase', - 'items.status.startedAt', - 'items.status.finishedAt', - 'items.status.resourcesDuration' - ]) - .then(list => this.getExtractDatasets(list.items || [])) - .then(charts => this.setState({error: null, charts, namespace, labels}, this.saveHistory)) - .catch(error => this.setState({error})); + function setWorkflowTemplate(value: string) { + setLabel(labelKeyWorkflowTemplate, value); } - private saveHistory() { - const newNamespace = Utils.managedNamespace ? '' : this.state.namespace; - this.url = uiUrl('reports' + (newNamespace ? '/' + newNamespace : '') + '?labels=' + this.state.labels.join(',')); - Utils.currentNamespace = this.state.namespace; + function setCronWorkflow(value: string) { + setLabel(labelKeyCronWorkflow, value); } - private getExtractDatasets(workflows: Workflow[]) { - const filteredWorkflows = workflows - .filter(wf => !!wf.status.finishedAt) - .map(wf => ({ - name: wf.metadata.name, - finishedAt: new Date(wf.status.finishedAt), - startedAt: new Date(wf.status.startedAt), - phase: wf.status.phase, - resourcesDuration: wf.status.resourcesDuration - })) - .sort((a, b) => b.finishedAt.getTime() - a.finishedAt.getTime()) - .slice(0, limit) - .reverse(); + useEffect(() => { + fetchReport(namespace, labels); + }, [namespace, labels]); - const labels: string[] = new Array(filteredWorkflows.length); - const backgroundColors: string[] = new Array(filteredWorkflows.length); - const durationData: number[] = new Array(filteredWorkflows.length); - const resourceData = {} as any; + useCollectEvent('openedReports'); - filteredWorkflows.forEach((wf, i) => { - labels[i] = wf.name; - backgroundColors[i] = getColorForNodePhase(wf.phase); - durationData[i] = (wf.finishedAt.getTime() - wf.startedAt.getTime()) / 1000; - Object.entries(wf.resourcesDuration || {}).forEach(([resource, value]) => { - if (!resourceData[resource]) { - resourceData[resource] = new Array(filteredWorkflows.length); - } - resourceData[resource][i] = value; - }); - }); - const resourceColors = { - 'cpu': 'teal', - 'memory': 'blue', - 'storage': 'purple', - 'ephemeral-storage': 'purple' - } as any; + return ( + +
+
{renderFilters()}
- return [ - { - data: { - name: 'duration', - labels, - datasets: [ - { - data: durationData, - backgroundColor: backgroundColors - } - ] - }, - options: { - title: { - display: true, - text: 'Duration' - }, - legend: {display: false}, - scales: { - xAxes: [{}], - yAxes: [ - { - id: 'duration', - ticks: { - beginAtZero: true - }, - scaleLabel: { - display: true, - labelString: 'Duration (seconds)' - } - } - ] - }, - annotation: { - annotations: [ - { - type: 'line', - mode: 'horizontal', - scaleID: 'duration', - value: durationData.length > 0 ? durationData.reduce((a, b) => a + b, 0) / durationData.length : 0, - borderColor: 'gray', - borderWidth: 1, - label: { - enabled: true, - position: 'left', - content: 'Average' - } - } - ] - } - } - }, - { - data: { - name: 'resources', - labels, - datasets: Object.entries(resourceData).map(([resource, data]) => ({ - yAxesID: resource, - label: resource, - data, - backgroundColor: resourceColors[resource] || 'black' - })) - }, - options: { - title: { - display: true, - text: 'Resources (not available for archived workflows)' - }, - scales: { - xAxes: [{}], - yAxes: Object.keys(resourceData).map(resource => ({ - id: resource, - ticks: { - beginAtZero: true - }, - scaleLabel: { - display: true, - labelString: resource + ' (' + denominator(resource) + ')' - } - })) - } - } - } - ]; - } +
+ ; + {!charts ? ( + +

+ Use this page to find costly or time consuming workflows. You must label workflows you want to report on. If you use workflow templates or{' '} + cron workflows, your workflows will be automatically labelled. You'll probably need to enable the{' '} + workflow archive to get long term data. Only the {limit} most recent workflows are + shown. +

+

Select a namespace and at least one label to get a report.

+

+ Learn more about cost optimization +

+
+ ) : ( + <> + {charts.map(chart => ( +
+
+ { + const activePoint = e[0]; + if (activePoint === undefined) { + return; + } + const workflowName = chart.data.labels[activePoint._index]; + navigation.goto(uiUrl('workflows/' + namespace + '/' + workflowName)); + }} + /> +
+
+ ))} + + {charts[0].data.labels.length} records. + + + )} +
+
+
+ ); - private renderFilters() { + function renderFilters() { return (

Namespace

{ - this.fetchReport(namespace, this.state.labels); + value={namespace} + onChange={newNamespace => { + fetchReport(newNamespace, labels); }} />
@@ -263,9 +177,9 @@ export class Reports extends BasePage, State> {

Labels

this.fetchReport(this.state.namespace, labels)} + tags={labels} + autocomplete={autocompleteLabels} + onChange={newLabels => fetchReport(namespace, newLabels)} />
@@ -273,18 +187,18 @@ export class Reports extends BasePage, State> { services.workflowTemplate - .list(this.state.namespace, []) + .list(namespace, []) .then(list => list.items || []) .then(list => list.map(x => x.metadata.name)) } - onChange={value => (this.workflowTemplate = value)} + onChange={value => (setWorkflowTemplate(value))} />

Cron Workflow

services.cronWorkflows.list(this.state.namespace).then(list => list.map(x => x.metadata.name))} - onChange={value => (this.cronWorkflow = value)} + load={() => services.cronWorkflows.list(namespace).then(list => list.map(x => x.metadata.name))} + onChange={value => (setCronWorkflow(value))} />
@@ -292,7 +206,7 @@ export class Reports extends BasePage, State> { {[NODE_PHASE.SUCCEEDED, NODE_PHASE.ERROR, NODE_PHASE.FAILED].map(phase => (
@@ -302,51 +216,128 @@ export class Reports extends BasePage, State> {
); } +} - private renderReport(ctx: ContextApis) { - if (this.state.error) { - return ; - } - if (!this.state.charts) { - return ( - -

- Use this page to find costly or time consuming workflows. You must label workflows you want to report on. If you use workflow templates or{' '} - cron workflows, your workflows will be automatically labelled. You'll probably need to enable the{' '} - workflow archive to get long term data. Only the {limit} most recent workflows are - shown. -

-

Select a namespace and at least one label to get a report.

-

- Learn more about cost optimization -

-
- ); +// pure function on the workflows (no requests, state, props, context, etc) +function getExtractDatasets(workflows: Workflow[]) { + const filteredWorkflows = workflows + .filter(wf => !!wf.status.finishedAt) + .map(wf => ({ + name: wf.metadata.name, + finishedAt: new Date(wf.status.finishedAt), + startedAt: new Date(wf.status.startedAt), + phase: wf.status.phase, + resourcesDuration: wf.status.resourcesDuration + })) + .sort((a, b) => b.finishedAt.getTime() - a.finishedAt.getTime()) + .slice(0, limit) + .reverse(); + + const labels: string[] = new Array(filteredWorkflows.length); + const backgroundColors: string[] = new Array(filteredWorkflows.length); + const durationData: number[] = new Array(filteredWorkflows.length); + const resourceData = {} as any; + + filteredWorkflows.forEach((wf, i) => { + labels[i] = wf.name; + backgroundColors[i] = getColorForNodePhase(wf.phase); + durationData[i] = (wf.finishedAt.getTime() - wf.startedAt.getTime()) / 1000; + Object.entries(wf.resourcesDuration || {}).forEach(([resource, value]) => { + if (!resourceData[resource]) { + resourceData[resource] = new Array(filteredWorkflows.length); + } + resourceData[resource][i] = value; + }); + }); + const resourceColors = { + 'cpu': 'teal', + 'memory': 'blue', + 'storage': 'purple', + 'ephemeral-storage': 'purple' + } as any; + + return [ + { + data: { + name: 'duration', + labels, + datasets: [ + { + data: durationData, + backgroundColor: backgroundColors + } + ] + }, + options: { + title: { + display: true, + text: 'Duration' + }, + legend: {display: false}, + scales: { + xAxes: [{}], + yAxes: [ + { + id: 'duration', + ticks: { + beginAtZero: true + }, + scaleLabel: { + display: true, + labelString: 'Duration (seconds)' + } + } + ] + }, + annotation: { + annotations: [ + { + type: 'line', + mode: 'horizontal', + scaleID: 'duration', + value: durationData.length > 0 ? durationData.reduce((a, b) => a + b, 0) / durationData.length : 0, + borderColor: 'gray', + borderWidth: 1, + label: { + enabled: true, + position: 'left', + content: 'Average' + } + } + ] + } + } + }, + { + data: { + name: 'resources', + labels, + datasets: Object.entries(resourceData).map(([resource, data]) => ({ + yAxesID: resource, + label: resource, + data, + backgroundColor: resourceColors[resource] || 'black' + })) + }, + options: { + title: { + display: true, + text: 'Resources (not available for archived workflows)' + }, + scales: { + xAxes: [{}], + yAxes: Object.keys(resourceData).map(resource => ({ + id: resource, + ticks: { + beginAtZero: true + }, + scaleLabel: { + display: true, + labelString: resource + ' (' + denominator(resource) + ')' + } + })) + } + } } - return ( - <> - {this.state.charts.map(chart => ( -
-
- { - const activePoint = e[0]; - if (activePoint === undefined) { - return; - } - const workflowName = chart.data.labels[activePoint._index]; - ctx.navigation.goto(uiUrl('workflows/' + this.state.namespace + '/' + workflowName)); - }} - /> -
-
- ))} - - {this.state.charts[0].data.labels.length} records. - - - ); - } + ]; } From a1f9b0ef7e5e8176ee150c6d8b9f12036304f054 Mon Sep 17 00:00:00 2001 From: Anton Gilgur Date: Sun, 10 Sep 2023 15:29:52 -0400 Subject: [PATCH 2/3] refactor(ui): split `reports` into more files - split out pure function into its own file - to make clear that it is pure and unrelated to React state, context, etc - split out `ReportsFilters` component similar to `CronWorkflowFilters`, `WorkflowFilters`, etc - be consistent with how all these work - similar to 25e1939e25551cd15d89bd47e4232c8073b40a9c, refactor logic to have an `onChange` function and retrieve Workflows in a top-level effect (after all state has been set) - and similarly add an SCSS file with the appropriate styles - note: this may be worth refactoring as nearly all the filters have the same styles - refactor `ReportsFilters` logic to use async/await syntax - also refactor `CronWorkflowList` to use newer async/await syntax consistent with future state of codebase Signed-off-by: Anton Gilgur --- .../cron-workflow-filters.scss | 2 +- .../cron-workflow-filters.tsx | 4 +- .../cron-workflow-list/cron-workflow-list.tsx | 24 +- .../reports/components/reports-filters.scss | 28 ++ .../reports/components/reports-filters.tsx | 95 +++++++ ui/src/app/reports/components/reports.tsx | 268 ++---------------- .../components/workflows-to-chart-data.ts | 125 ++++++++ 7 files changed, 293 insertions(+), 253 deletions(-) create mode 100644 ui/src/app/reports/components/reports-filters.scss create mode 100644 ui/src/app/reports/components/reports-filters.tsx create mode 100644 ui/src/app/reports/components/workflows-to-chart-data.ts diff --git a/ui/src/app/cron-workflows/components/cron-workflow-filters/cron-workflow-filters.scss b/ui/src/app/cron-workflows/components/cron-workflow-filters/cron-workflow-filters.scss index f77489f4d762..65f18b9410ca 100644 --- a/ui/src/app/cron-workflows/components/cron-workflow-filters/cron-workflow-filters.scss +++ b/ui/src/app/cron-workflows/components/cron-workflow-filters/cron-workflow-filters.scss @@ -25,4 +25,4 @@ font-size: 15px; background-color: transparent; border: 0; -} \ No newline at end of file +} diff --git a/ui/src/app/cron-workflows/components/cron-workflow-filters/cron-workflow-filters.tsx b/ui/src/app/cron-workflows/components/cron-workflow-filters/cron-workflow-filters.tsx index d36c670ee5fd..ae67fe603cad 100644 --- a/ui/src/app/cron-workflows/components/cron-workflow-filters/cron-workflow-filters.tsx +++ b/ui/src/app/cron-workflows/components/cron-workflow-filters/cron-workflow-filters.tsx @@ -15,7 +15,7 @@ interface WorkflowFilterProps { onChange: (namespace: string, labels: string[], states: string[]) => void; } -export const CronWorkflowFilters = ({cronWorkflows, namespace, labels, states, onChange}: WorkflowFilterProps) => { +export function CronWorkflowFilters({cronWorkflows, namespace, labels, states, onChange}: WorkflowFilterProps) { const [labelSuggestion, setLabelSuggestion] = useState([]); useEffect(() => { @@ -74,4 +74,4 @@ export const CronWorkflowFilters = ({cronWorkflows, namespace, labels, states, o
); -}; +} diff --git a/ui/src/app/cron-workflows/components/cron-workflow-list/cron-workflow-list.tsx b/ui/src/app/cron-workflows/components/cron-workflow-list/cron-workflow-list.tsx index 5c1f60a3b288..b783e33e2481 100644 --- a/ui/src/app/cron-workflows/components/cron-workflow-list/cron-workflow-list.tsx +++ b/ui/src/app/cron-workflows/components/cron-workflow-list/cron-workflow-list.tsx @@ -26,7 +26,7 @@ require('./cron-workflow-list.scss'); const learnMore = Learn more; -export function CronWorkflowList ({match, location, history}: RouteComponentProps) { +export function CronWorkflowList({match, location, history}: RouteComponentProps) { const queryParams = new URLSearchParams(location.search); const {navigation} = useContext(Context); @@ -60,21 +60,23 @@ export function CronWorkflowList ({match, location, history}: RouteComponentProp const [cronWorkflows, setCronWorkflows] = useState(); useEffect(() => { - services.cronWorkflows - .list(namespace, labels) - .then(l => { + (async () => { + try { + const list = await services.cronWorkflows.list(namespace, labels); if (states.length === 1) { if (states.includes('Suspended')) { - return l.filter(el => el.spec.suspend === true); + setCronWorkflows(list.filter(el => el.spec.suspend === true)); } else { - return l.filter(el => el.spec.suspend !== true); + setCronWorkflows(list.filter(el => el.spec.suspend !== true)); } + } else { + setCronWorkflows(list); } - return l; - }) - .then(setCronWorkflows) - .then(() => setError(null)) - .catch(setError); + setError(null); + } catch (newError) { + setError(newError); + } + })(); }, [namespace, labels, states]); useCollectEvent('openedCronWorkflowList'); diff --git a/ui/src/app/reports/components/reports-filters.scss b/ui/src/app/reports/components/reports-filters.scss new file mode 100644 index 000000000000..65f18b9410ca --- /dev/null +++ b/ui/src/app/reports/components/reports-filters.scss @@ -0,0 +1,28 @@ +@import 'node_modules/argo-ui/src/styles/config'; + +.wf-filters-container { + overflow: visible; + position: relative; + border-radius: 5px; + box-shadow: 1px 1px 3px #8fa4b1; + padding: 0 1em 0.75em 1em; + margin: 12px 0; + background-color: white; +} + +.wf-filters-container p { + margin: 0; + margin-top: 1em; + color: #6d7f8b; + text-transform: uppercase; +} + +.wf-filters-container__title { + position: relative; + width: 100%; + max-width: 100%; + padding: 8px 0; + font-size: 15px; + background-color: transparent; + border: 0; +} diff --git a/ui/src/app/reports/components/reports-filters.tsx b/ui/src/app/reports/components/reports-filters.tsx new file mode 100644 index 000000000000..62e0319e230e --- /dev/null +++ b/ui/src/app/reports/components/reports-filters.tsx @@ -0,0 +1,95 @@ +import * as React from 'react'; +import {NODE_PHASE} from '../../../models'; +import {DataLoaderDropdown} from '../../shared/components/data-loader-dropdown'; +import {NamespaceFilter} from '../../shared/components/namespace-filter'; +import {TagsInput} from '../../shared/components/tags-input/tags-input'; +import {services} from '../../shared/services'; + +require('./reports-filters.scss'); + +const labelKeyPhase = 'workflows.argoproj.io/phase'; +const labelKeyWorkflowTemplate = 'workflows.argoproj.io/workflow-template'; +const labelKeyCronWorkflow = 'workflows.argoproj.io/cron-workflow'; + +interface ReportFiltersProps { + namespace: string; + labels: string[]; + onChange: (newNamespace: string, newLabels: string[]) => void; +} + +export function ReportFilters({namespace, labels, onChange}: ReportFiltersProps) { + function getLabel(name: string) { + return (labels.find(label => label.startsWith(name)) || '').replace(name + '=', ''); + } + + function setLabel(name: string, value: string) { + onChange(namespace, labels.filter(label => !label.startsWith(name)).concat(name + '=' + value)); + } + + function getPhase() { + return getLabel(labelKeyPhase); + } + + function setPhase(value: string) { + setLabel(labelKeyPhase, value); + } + + function setWorkflowTemplate(value: string) { + setLabel(labelKeyWorkflowTemplate, value); + } + + function setCronWorkflow(value: string) { + setLabel(labelKeyCronWorkflow, value); + } + + return ( +
+
+
+

Namespace

+ { + onChange(newNamespace, labels); + }} + /> +
+
+

Labels

+ onChange(namespace, newLabels)} /> +
+
+

Workflow Template

+ { + const list = await services.workflowTemplate.list(namespace, []); + return (list.items || []).map(x => x.metadata.name); + }} + onChange={value => setWorkflowTemplate(value)} + /> +
+
+

Cron Workflow

+ { + const list = await services.cronWorkflows.list(namespace); + return list.map(x => x.metadata.name); + }} + onChange={value => setCronWorkflow(value)} + /> +
+
+

Phase

+ {[NODE_PHASE.SUCCEEDED, NODE_PHASE.ERROR, NODE_PHASE.FAILED].map(phase => ( +
+ +
+ ))} +
+
+
+ ); +} diff --git a/ui/src/app/reports/components/reports.tsx b/ui/src/app/reports/components/reports.tsx index c2d3e0bb77f4..613f72e32133 100644 --- a/ui/src/app/reports/components/reports.tsx +++ b/ui/src/app/reports/components/reports.tsx @@ -5,21 +5,18 @@ import * as React from 'react'; import {useContext, useEffect, useState} from 'react'; import {Bar, ChartData} from 'react-chartjs-2'; import {RouteComponentProps} from 'react-router-dom'; -import {getColorForNodePhase, NODE_PHASE, Workflow} from '../../../models'; import {uiUrl} from '../../shared/base'; -import {DataLoaderDropdown} from '../../shared/components/data-loader-dropdown'; import {ErrorNotice} from '../../shared/components/error-notice'; import {InfoIcon} from '../../shared/components/fa-icons'; -import {NamespaceFilter} from '../../shared/components/namespace-filter'; -import {TagsInput} from '../../shared/components/tags-input/tags-input'; import {useCollectEvent} from '../../shared/components/use-collect-event'; import {ZeroState} from '../../shared/components/zero-state'; import {Context} from '../../shared/context'; -import {denominator} from '../../shared/duration'; import {Footnote} from '../../shared/footnote'; import {historyUrl} from '../../shared/history'; import {services} from '../../shared/services'; import {Utils} from '../../shared/utils'; +import {ReportFilters} from './reports-filters'; +import {workflowsToChartData} from './workflows-to-chart-data'; interface Chart { data: ChartData; @@ -27,78 +24,51 @@ interface Chart { } const limit = 100; -const labelKeyPhase = 'workflows.argoproj.io/phase'; -const labelKeyWorkflowTemplate = 'workflows.argoproj.io/workflow-template'; -const labelKeyCronWorkflow = 'workflows.argoproj.io/cron-workflow'; -export function Reports ({match, location, history}: RouteComponentProps) { +export function Reports({match, location, history}: RouteComponentProps) { const queryParams = new URLSearchParams(location.search); const {navigation} = useContext(Context); // state for URL, query, and label parameters const [namespace, setNamespace] = useState(Utils.getNamespace(match.params.namespace) || ''); const [labels, setLabels] = useState((queryParams.get('labels') || '').split(',').filter(v => v !== '')); - const [autocompleteLabels, setAutocompleteLabels] = useState(['']); + // internal state const [charts, setCharts] = useState(); const [error, setError] = useState(); // save history useEffect(() => { - history.push(historyUrl('reports' + (Utils.managedNamespace ? '' : '/{namespace}'), {namespace, labels: labels.join(',')})) + history.push(historyUrl('reports' + (Utils.managedNamespace ? '' : '/{namespace}'), {namespace, labels: labels.join(',')})); }, [namespace, labels]); - async function fetchReport(newNamespace: string, newLabels: string[]) { + async function onChange(newNamespace: string, newLabels: string[]) { if (newNamespace === '' || newLabels.length === 0) { setNamespace(newNamespace); setLabels(newLabels); setCharts(null); return; } - - try { - const list = await services.workflows.list(newNamespace, [], newLabels, {limit}, [ - 'items.metadata.name', - 'items.status.phase', - 'items.status.startedAt', - 'items.status.finishedAt', - 'items.status.resourcesDuration' - ]); - const newCharts = getExtractDatasets(list.items || []); - setNamespace(newNamespace); - setLabels(newLabels); - setCharts(newCharts); - setError(null); - } catch (newError) { - setError(newError); - } - } - - function getLabel(name: string) { - return (labels.find(label => label.startsWith(name)) || '').replace(name + '=', ''); - } - - function setLabel(name: string, value: string) { - fetchReport(namespace, labels.filter(label => !label.startsWith(name)).concat(name + '=' + value)); - } - - function getPhase() { - return getLabel(labelKeyPhase); - } - - function setPhase(value: string) { - setLabel(labelKeyPhase, value); - } - - function setWorkflowTemplate(value: string) { - setLabel(labelKeyWorkflowTemplate, value); - } - - function setCronWorkflow(value: string) { - setLabel(labelKeyCronWorkflow, value); + setNamespace(newNamespace); + setLabels(newLabels); } useEffect(() => { - fetchReport(namespace, labels); + (async () => { + try { + const list = await services.workflows.list(namespace, [], labels, {limit}, [ + 'items.metadata.name', + 'items.status.phase', + 'items.status.startedAt', + 'items.status.finishedAt', + 'items.status.resourcesDuration' + ]); + const newCharts = workflowsToChartData(list.items || [], limit); + setCharts(newCharts); + setError(null); + } catch (newError) { + setError(newError); + } + })(); }, [namespace, labels]); useCollectEvent('openedReports'); @@ -113,8 +83,9 @@ export function Reports ({match, location, history}: RouteComponentProps) { ] }}>
-
{renderFilters()}
- +
+ +
; {!charts ? ( @@ -122,8 +93,8 @@ export function Reports ({match, location, history}: RouteComponentProps) {

Use this page to find costly or time consuming workflows. You must label workflows you want to report on. If you use workflow templates or{' '} cron workflows, your workflows will be automatically labelled. You'll probably need to enable the{' '} - workflow archive to get long term data. Only the {limit} most recent workflows are - shown. + workflow archive to get long term data. Only the {limit} most recent + workflows are shown.

Select a namespace and at least one label to get a report.

@@ -159,185 +130,4 @@ export function Reports ({match, location, history}: RouteComponentProps) {

); - - function renderFilters() { - return ( -
-
-
-

Namespace

- { - fetchReport(newNamespace, labels); - }} - /> -
-
-

Labels

- fetchReport(namespace, newLabels)} - /> -
-
-

Workflow Template

- - services.workflowTemplate - .list(namespace, []) - .then(list => list.items || []) - .then(list => list.map(x => x.metadata.name)) - } - onChange={value => (setWorkflowTemplate(value))} - /> -
-
-

Cron Workflow

- services.cronWorkflows.list(namespace).then(list => list.map(x => x.metadata.name))} - onChange={value => (setCronWorkflow(value))} - /> -
-
-

Phase

- {[NODE_PHASE.SUCCEEDED, NODE_PHASE.ERROR, NODE_PHASE.FAILED].map(phase => ( -
- -
- ))} -
-
-
- ); - } -} - -// pure function on the workflows (no requests, state, props, context, etc) -function getExtractDatasets(workflows: Workflow[]) { - const filteredWorkflows = workflows - .filter(wf => !!wf.status.finishedAt) - .map(wf => ({ - name: wf.metadata.name, - finishedAt: new Date(wf.status.finishedAt), - startedAt: new Date(wf.status.startedAt), - phase: wf.status.phase, - resourcesDuration: wf.status.resourcesDuration - })) - .sort((a, b) => b.finishedAt.getTime() - a.finishedAt.getTime()) - .slice(0, limit) - .reverse(); - - const labels: string[] = new Array(filteredWorkflows.length); - const backgroundColors: string[] = new Array(filteredWorkflows.length); - const durationData: number[] = new Array(filteredWorkflows.length); - const resourceData = {} as any; - - filteredWorkflows.forEach((wf, i) => { - labels[i] = wf.name; - backgroundColors[i] = getColorForNodePhase(wf.phase); - durationData[i] = (wf.finishedAt.getTime() - wf.startedAt.getTime()) / 1000; - Object.entries(wf.resourcesDuration || {}).forEach(([resource, value]) => { - if (!resourceData[resource]) { - resourceData[resource] = new Array(filteredWorkflows.length); - } - resourceData[resource][i] = value; - }); - }); - const resourceColors = { - 'cpu': 'teal', - 'memory': 'blue', - 'storage': 'purple', - 'ephemeral-storage': 'purple' - } as any; - - return [ - { - data: { - name: 'duration', - labels, - datasets: [ - { - data: durationData, - backgroundColor: backgroundColors - } - ] - }, - options: { - title: { - display: true, - text: 'Duration' - }, - legend: {display: false}, - scales: { - xAxes: [{}], - yAxes: [ - { - id: 'duration', - ticks: { - beginAtZero: true - }, - scaleLabel: { - display: true, - labelString: 'Duration (seconds)' - } - } - ] - }, - annotation: { - annotations: [ - { - type: 'line', - mode: 'horizontal', - scaleID: 'duration', - value: durationData.length > 0 ? durationData.reduce((a, b) => a + b, 0) / durationData.length : 0, - borderColor: 'gray', - borderWidth: 1, - label: { - enabled: true, - position: 'left', - content: 'Average' - } - } - ] - } - } - }, - { - data: { - name: 'resources', - labels, - datasets: Object.entries(resourceData).map(([resource, data]) => ({ - yAxesID: resource, - label: resource, - data, - backgroundColor: resourceColors[resource] || 'black' - })) - }, - options: { - title: { - display: true, - text: 'Resources (not available for archived workflows)' - }, - scales: { - xAxes: [{}], - yAxes: Object.keys(resourceData).map(resource => ({ - id: resource, - ticks: { - beginAtZero: true - }, - scaleLabel: { - display: true, - labelString: resource + ' (' + denominator(resource) + ')' - } - })) - } - } - } - ]; } diff --git a/ui/src/app/reports/components/workflows-to-chart-data.ts b/ui/src/app/reports/components/workflows-to-chart-data.ts new file mode 100644 index 000000000000..a8c2efae9479 --- /dev/null +++ b/ui/src/app/reports/components/workflows-to-chart-data.ts @@ -0,0 +1,125 @@ +import {getColorForNodePhase, Workflow} from '../../../models'; +import {denominator} from '../../shared/duration'; + +export function workflowsToChartData(workflows: Workflow[], limit: number) { + const filteredWorkflows = workflows + .filter(wf => !!wf.status.finishedAt) + .map(wf => ({ + name: wf.metadata.name, + finishedAt: new Date(wf.status.finishedAt), + startedAt: new Date(wf.status.startedAt), + phase: wf.status.phase, + resourcesDuration: wf.status.resourcesDuration + })) + .sort((a, b) => b.finishedAt.getTime() - a.finishedAt.getTime()) + .slice(0, limit) + .reverse(); + + const labels: string[] = new Array(filteredWorkflows.length); + const backgroundColors: string[] = new Array(filteredWorkflows.length); + const durationData: number[] = new Array(filteredWorkflows.length); + const resourceData = {} as any; + + filteredWorkflows.forEach((wf, i) => { + labels[i] = wf.name; + backgroundColors[i] = getColorForNodePhase(wf.phase); + durationData[i] = (wf.finishedAt.getTime() - wf.startedAt.getTime()) / 1000; + Object.entries(wf.resourcesDuration || {}).forEach(([resource, value]) => { + if (!resourceData[resource]) { + resourceData[resource] = new Array(filteredWorkflows.length); + } + resourceData[resource][i] = value; + }); + }); + const resourceColors = { + 'cpu': 'teal', + 'memory': 'blue', + 'storage': 'purple', + 'ephemeral-storage': 'purple' + } as any; + + return [ + { + data: { + name: 'duration', + labels, + datasets: [ + { + data: durationData, + backgroundColor: backgroundColors + } + ] + }, + options: { + title: { + display: true, + text: 'Duration' + }, + legend: {display: false}, + scales: { + xAxes: [{}], + yAxes: [ + { + id: 'duration', + ticks: { + beginAtZero: true + }, + scaleLabel: { + display: true, + labelString: 'Duration (seconds)' + } + } + ] + }, + annotation: { + annotations: [ + { + type: 'line', + mode: 'horizontal', + scaleID: 'duration', + value: durationData.length > 0 ? durationData.reduce((a, b) => a + b, 0) / durationData.length : 0, + borderColor: 'gray', + borderWidth: 1, + label: { + enabled: true, + position: 'left', + content: 'Average' + } + } + ] + } + } + }, + { + data: { + name: 'resources', + labels, + datasets: Object.entries(resourceData).map(([resource, data]) => ({ + yAxesID: resource, + label: resource, + data, + backgroundColor: resourceColors[resource] || 'black' + })) + }, + options: { + title: { + display: true, + text: 'Resources (not available for archived workflows)' + }, + scales: { + xAxes: [{}], + yAxes: Object.keys(resourceData).map(resource => ({ + id: resource, + ticks: { + beginAtZero: true + }, + scaleLabel: { + display: true, + labelString: resource + ' (' + denominator(resource) + ')' + } + })) + } + } + } + ]; +} From 1972369f0335f512d72a8854ec40cfeea22e9448 Mon Sep 17 00:00:00 2001 From: Anton Gilgur Date: Sun, 10 Sep 2023 17:24:59 -0400 Subject: [PATCH 3/3] fix leftover this.state usage Signed-off-by: Anton Gilgur --- ui/src/app/reports/components/reports.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ui/src/app/reports/components/reports.tsx b/ui/src/app/reports/components/reports.tsx index 613f72e32133..527ea83228fd 100644 --- a/ui/src/app/reports/components/reports.tsx +++ b/ui/src/app/reports/components/reports.tsx @@ -79,7 +79,7 @@ export function Reports({match, location, history}: RouteComponentProps) { toolbar={{ breadcrumbs: [ {title: 'Reports', path: uiUrl('reports')}, - {title: this.state.namespace, path: uiUrl('reports/' + this.state.namespace)} + {title: namespace, path: uiUrl('reports/' + namespace)} ] }}>