DW: Use 1-2 decimal places for all GiB and memory values, or 3 places…

… for tooltips Signed-off-by: Mike Turley <[email protected]>
opendatahub-io · Apr 8, 2024 · c744083 · c744083
1 parent a6accd1
commit c744083
Show file tree

Hide file tree

Showing 6 changed files with 82 additions and 89 deletions.
diff --git a/frontend/src/pages/distributedWorkloads/components/WorkloadResourceUsageBar.tsx b/frontend/src/pages/distributedWorkloads/components/WorkloadResourceUsageBar.tsx
@@ -33,10 +33,11 @@ export const WorkloadResourceUsageBar: React.FC<WorkloadResourceUsageBarProps> =
       content={
         <Stack>
           <StackItem>
-            {getColorSwatch('used')} {metricLabel} usage: {used} {unitLabel}
+            {getColorSwatch('used')} {metricLabel} usage: {roundNumber(used, 3)} {unitLabel}
           </StackItem>
           <StackItem>
-            {getColorSwatch('requested')} {metricLabel} requested: {requested} {unitLabel}
+            {getColorSwatch('requested')} {metricLabel} requested: {roundNumber(requested, 3)}{' '}
+            {unitLabel}
           </StackItem>
         </Stack>
       }

diff --git a/...tend/src/pages/distributedWorkloads/global/projectMetrics/sections/RequestedResources.tsx b/...tend/src/pages/distributedWorkloads/global/projectMetrics/sections/RequestedResources.tsx
@@ -1,5 +1,5 @@
 import * as React from 'react';
-import { CardBody, Gallery, GalleryItem } from '@patternfly/react-core';
+import { CardBody, Gallery, GalleryItem, capitalize } from '@patternfly/react-core';
 import { ChartBullet, ChartLegend } from '@patternfly/react-charts';
 import {
   chart_color_blue_300 as chartColorBlue300,
@@ -16,16 +16,16 @@ import { bytesAsPreciseGiB, roundNumber } from '~/utilities/number';
 import { ErrorWorkloadState, LoadingWorkloadState } from './SharedStates';
 
 type RequestedResourcesBulletChartProps = {
-  title: string;
-  subTitle: string;
+  metricLabel: string;
+  unitLabel: string;
   numRequestedByThisProject: number;
   numRequestedByAllProjects: number;
   numTotalSharedQuota: number;
 };
 
 const RequestedResourcesBulletChart: React.FC<RequestedResourcesBulletChartProps> = ({
-  title,
-  subTitle,
+  metricLabel,
+  unitLabel,
   numRequestedByThisProject,
   numRequestedByAllProjects,
   numTotalSharedQuota,
@@ -43,14 +43,16 @@ const RequestedResourcesBulletChart: React.FC<RequestedResourcesBulletChartProps
     tooltip?: string; // Falls back to `name: preciseValue` if omitted
     hideValueInLegend?: boolean;
     preciseValue: number;
-    roundedValue: number;
+    legendValue: number;
+    tooltipValue: number;
     cappedValue: number;
   };
   const getDataItem = (
-    args: Omit<CappedBulletChartDataItem, 'roundedValue' | 'cappedValue'>,
+    args: Omit<CappedBulletChartDataItem, 'legendValue' | 'tooltipValue' | 'cappedValue'>,
   ): CappedBulletChartDataItem => ({
     ...args,
-    roundedValue: roundNumber(args.preciseValue),
+    legendValue: roundNumber(args.preciseValue),
+    tooltipValue: roundNumber(args.preciseValue, 3),
     cappedValue: roundNumber(Math.min(args.preciseValue, maxDomain)),
   });
 
@@ -88,17 +90,15 @@ const RequestedResourcesBulletChart: React.FC<RequestedResourcesBulletChartProps
   const allData = [...segmentedMeasureData, ...qualitativeRangeData, ...warningMeasureData];
   return (
     <ChartBullet
-      title={title}
-      subTitle={subTitle}
-      ariaTitle={`Requested ${title} ${subTitle}`}
+      title={metricLabel}
+      subTitle={capitalize(unitLabel)}
+      ariaTitle={`Requested ${metricLabel} ${unitLabel}`}
       ariaDesc="Bullet chart"
-      name={`requested-resources-chart-${title}`}
+      name={`requested-resources-chart-${metricLabel}`}
       labels={({ datum }) => {
         const matchingDataItem = allData.find(({ name }) => name === datum.name);
-        return (
-          matchingDataItem?.tooltip ||
-          `${matchingDataItem?.name}: ${matchingDataItem?.preciseValue}`
-        );
+        const { tooltip, name, tooltipValue } = matchingDataItem || {};
+        return tooltip || `${name}: ${tooltipValue} ${unitLabel}`;
       }}
       primarySegmentedMeasureData={segmentedMeasureData.map(({ name, cappedValue }) => ({
         name,
@@ -118,8 +118,8 @@ const RequestedResourcesBulletChart: React.FC<RequestedResourcesBulletChartProps
       legendOrientation="vertical"
       legendComponent={
         <ChartLegend
-          data={allData.map(({ name, hideValueInLegend, roundedValue }) => ({
-            name: hideValueInLegend ? name : `${name}: ${roundedValue}`,
+          data={allData.map(({ name, hideValueInLegend, legendValue }) => ({
+            name: hideValueInLegend ? name : `${name}: ${legendValue}`,
           }))}
           colorScale={allData.map(({ color }) => color)}
           gutter={30}
@@ -163,17 +163,17 @@ export const RequestedResources: React.FC = () => {
       <Gallery minWidths={{ default: '100%', md: '50%' }}>
         <GalleryItem data-testid="requested-resources-cpu-chart-container">
           <RequestedResourcesBulletChart
-            title="CPU"
-            subTitle="Cores"
+            metricLabel="CPU"
+            unitLabel="cores"
             numRequestedByThisProject={requestedByThisProject.cpuCoresRequested}
             numRequestedByAllProjects={requestedByAllProjects.cpuCoresRequested}
             numTotalSharedQuota={totalSharedQuota.cpuCoresRequested}
           />
         </GalleryItem>
         <GalleryItem data-testid="requested-resources-memory-chart-container">
           <RequestedResourcesBulletChart
-            title="Memory"
-            subTitle="GiB"
+            metricLabel="Memory"
+            unitLabel="GiB"
             numRequestedByThisProject={bytesAsPreciseGiB(
               requestedByThisProject.memoryBytesRequested,
             )}

diff --git a/...ges/distributedWorkloads/global/projectMetrics/sections/TopResourceConsumingWorkloads.tsx b/...ges/distributedWorkloads/global/projectMetrics/sections/TopResourceConsumingWorkloads.tsx
@@ -3,76 +3,58 @@ import { Card, CardBody, CardTitle, Gallery, GalleryItem } from '@patternfly/rea
 import { ChartLegend, ChartLabel, ChartDonut, ChartThemeColor } from '@patternfly/react-charts';
 import { DistributedWorkloadsContext } from '~/concepts/distributedWorkloads/DistributedWorkloadsContext';
 import { WorkloadStatusType, getStatusInfo } from '~/concepts/distributedWorkloads/utils';
+import { truncateString } from '~/utilities/string';
+import { bytesAsPreciseGiB, roundNumber } from '~/utilities/number';
+import { WorkloadWithUsage } from '~/api';
+import { WorkloadKind } from '~/k8sTypes';
 import { ErrorWorkloadState, LoadingWorkloadState, NoWorkloadState } from './SharedStates';
 
-//TODO: next 4 utility functions to be replaced or moved into a utility class
-const memoryBytesToGibStr = (bytes: number, excludeUnit = false): string => {
-  const gib = bytes / 1073741824;
-  return `${truncateNumberToStr(gib)}${excludeUnit ? '' : 'GiB'}`;
-};
-
-const numberToCoreStr = (num: number): string => `${truncateNumberToStr(num)} cores`;
-
-const truncateNumberToStr = (num: number): string => {
-  if (num > 0.001) {
-    return String(parseFloat(num.toFixed(3)));
-  }
-  if (num === 0) {
-    return '0';
-  }
-  return '< 0.001';
-};
-
-const truncateStr = (str: string, length: number): string => {
-  if (str.length <= length) {
-    return str;
-  }
-  return `${str.substring(0, length)}...`;
-};
-
 interface TopResourceConsumingWorkloadsChartProps {
-  label: string;
-  title: string;
-  subTitle?: string;
-  data: Array<{ name: string; usage: number }>;
-  dataStrConverter: (num: number) => string;
+  metricLabel: string;
+  unitLabel: string;
+  data: { totalUsage: number; topWorkloads: WorkloadWithUsage[] };
+  convertUnits?: (num?: number) => number;
 }
 
+const getWorkloadName = (workload: WorkloadKind | 'other') =>
+  workload === 'other' ? 'Other' : workload.metadata?.name || 'Unnamed';
+
 const TopResourceConsumingWorkloadsChart: React.FC<TopResourceConsumingWorkloadsChartProps> = ({
-  label,
-  title,
-  subTitle = '',
-  data = [],
-  dataStrConverter,
+  metricLabel,
+  unitLabel,
+  data,
+  convertUnits = (num) => num || 0,
 }) => (
   <ChartDonut
-    ariaTitle={`${label} chart`}
+    ariaTitle={`${metricLabel} chart`}
     constrainToVisibleArea
-    data={data.map((d: { name: string; usage: number }) => ({ x: d.name, y: d.usage }))}
+    data={data.topWorkloads.map(({ workload, usage }) => ({
+      x: getWorkloadName(workload),
+      y: roundNumber(convertUnits(usage), 3),
+    }))}
     height={150}
-    labels={({ datum }) => `${datum.x}: ${dataStrConverter(0 + datum.y)}`}
+    labels={({ datum }) => `${datum.x}: ${datum.y} ${unitLabel}`}
     legendComponent={
       <ChartLegend
-        data={data.map((d: { name: string; usage: number }) => ({
-          ...d,
-          name: truncateStr(d.name, 16),
+        data={data.topWorkloads.map(({ workload }) => ({
+          name: truncateString(getWorkloadName(workload), 16),
         }))}
         gutter={5}
         labelComponent={<ChartLabel style={{ fontSize: 10 }} />}
-        itemsPerRow={Math.ceil(data.length / 2)}
+        itemsPerRow={Math.ceil(data.topWorkloads.length / 2)}
       />
     }
     legendOrientation="vertical"
     legendPosition="right"
-    name={`topResourceConsuming${label}`}
+    name={`topResourceConsuming${metricLabel}`}
     padding={{
       bottom: 0,
       left: 0,
       right: 260, // Adjusted to accommodate legend
       top: 0,
     }}
-    subTitle={subTitle}
-    title={title}
+    subTitle={unitLabel}
+    title={String(roundNumber(convertUnits(data.totalUsage)))}
     themeColor={ChartThemeColor.multi}
     width={375}
   />
@@ -123,15 +105,9 @@ export const TopResourceConsumingWorkloads: React.FC = () => {
           <CardTitle>CPU</CardTitle>
           <CardBody>
             <TopResourceConsumingWorkloadsChart
-              label="CPU"
-              title={truncateNumberToStr(topWorkloadsByUsage.cpuCoresUsed.totalUsage)}
-              subTitle="cores"
-              data={topWorkloadsByUsage.cpuCoresUsed.topWorkloads.map((data) => ({
-                name:
-                  data.workload === 'other' ? 'other' : data.workload.metadata?.name || 'unnamed',
-                usage: data.usage || 0,
-              }))}
-              dataStrConverter={numberToCoreStr}
+              metricLabel="CPU"
+              unitLabel="cores"
+              data={topWorkloadsByUsage.cpuCoresUsed}
             />
           </CardBody>
         </Card>
@@ -141,15 +117,10 @@ export const TopResourceConsumingWorkloads: React.FC = () => {
           <CardTitle>Memory</CardTitle>
           <CardBody>
             <TopResourceConsumingWorkloadsChart
-              label="Memory"
-              title={memoryBytesToGibStr(topWorkloadsByUsage.memoryBytesUsed.totalUsage, true)}
-              subTitle="GiB"
-              data={topWorkloadsByUsage.memoryBytesUsed.topWorkloads.map((data) => ({
-                name:
-                  data.workload === 'other' ? 'other' : data.workload.metadata?.name || 'unnamed',
-                usage: data.usage || 0,
-              }))}
-              dataStrConverter={memoryBytesToGibStr}
+              metricLabel="Memory"
+              unitLabel="GiB"
+              data={topWorkloadsByUsage.memoryBytesUsed}
+              convertUnits={bytesAsPreciseGiB}
             />
           </CardBody>
         </Card>

diff --git a/frontend/src/utilities/__tests__/number.spec.ts b/frontend/src/utilities/__tests__/number.spec.ts
@@ -15,6 +15,13 @@ describe('roundNumber', () => {
     expect(roundNumber(0.09765625)).toBe(0.1);
     expect(roundNumber(0.048828125)).toBe(0.05);
   });
+
+  it('should round to custom precision if specified', () => {
+    expect(roundNumber(1, 3)).toBe(1);
+    expect(roundNumber(0.16, 3)).toBe(0.16);
+    expect(roundNumber(0.09765625, 3)).toBe(0.098);
+    expect(roundNumber(0.048828125, 3)).toBe(0.049);
+  });
 });
 
 describe('bytesAsPreciseGiB', () => {

diff --git a/frontend/src/utilities/number.ts b/frontend/src/utilities/number.ts
@@ -3,12 +3,16 @@ import { convertToUnit, MEMORY_UNITS_FOR_PARSING } from './valueUnits';
 /**
  * Returns the given number rounded to 1 decimal point, unless it is less than 0.1,
  * then it will return 2 decimal points so the value is not rounded to zero.
+ * If precision is passed, the above logic is ignored and the specified precision is used.
  */
-export const roundNumber = (value: number | typeof NaN): number => {
+export const roundNumber = (
+  value: number | typeof NaN,
+  precision = value < 0.1 ? 2 : 1,
+): number => {
   if (Number.isNaN(value)) {
     return 0;
   }
-  return parseFloat(value.toFixed(value < 0.1 ? 2 : 1));
+  return parseFloat(value.toFixed(precision));
 };
 
 /**

diff --git a/frontend/src/utilities/string.ts b/frontend/src/utilities/string.ts
@@ -93,3 +93,13 @@ export const isS3PathValid = (path: string): boolean => {
   const pattern = /^[a-zA-Z0-9\-_./]+$/;
   return pattern.test(path) && !containsMultipleSlashesPattern(path);
 };
+
+/*
+ * Truncates a string to a specified number of characters with ellipses
+ */
+export const truncateString = (str: string, length: number): string => {
+  if (str.length <= length) {
+    return str;
+  }
+  return `${str.substring(0, length)}…`;
+};