Skip to content

fix: [M3-9805] - Revert Object Storage Size Conversions from Base10 to Base2 #12075

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jaalah-akamai merged 4 commits into from
Apr 23, 2025
Merged

fix: [M3-9805] - Revert Object Storage Size Conversions from Base10 to Base2 #12075

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
19c9f56
fix: [M3-9805] - Revert Object Storage Size Conversions from Base10 t…
jaalah Apr 21, 2025
daeb26a
More tests
jaalah Apr 21, 2025
dcaa320
Added changeset: Revert Object Storage Size Conversions from Base10 t…
jaalah Apr 21, 2025
fc7c246
Merge branch 'develop' into M3-9805
jaalah-akamai Apr 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions packages/manager/.changeset/pr-12075-fixed-1745266693581.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@linode/manager": Fixed
---

Revert Object Storage Size Conversions from Base10 to Base2 ([#12075](https://github.com/linode/manager/pull/12075))
3 changes: 1 addition & 2 deletions packages/manager/src/components/Uploaders/FileUpload.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,7 @@ export const FileUpload = React.memo((props: FileUploadProps) => {
})}
variant="body1"
>
{/* to convert from binary units (GiB) to decimal units (GB) we need to pass the base10 flag */}
{readableBytes(props.sizeInBytes, { base10: true }).formatted}
{readableBytes(props.sizeInBytes).formatted}
</StyledFileSizeTypography>
{props.percentCompleted === 100 ? (
<FileUploadComplete
Expand Down
25 changes: 24 additions & 1 deletion packages/manager/src/features/ObjectStorage/BucketDetail/ObjectDetailsDrawer.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ describe('ObjectDetailsDrawer', () => {
screen.findByText(/^Last modified: 2019-12-31/)
).resolves.toBeInTheDocument();

expect(screen.getByText('12.3 KB')).toBeVisible();
expect(screen.getByText(/^https:\/\/my-bucket/)).toBeVisible();
});

Expand All @@ -62,4 +61,28 @@ describe('ObjectDetailsDrawer', () => {
expect(() => screen.getByTestId('lastModified')).toThrow();
});
});

describe('readableBytes edge cases in Object Storage', () => {
it('displays bytes correctly for sizes under 1 KB threshold', () => {
renderWithTheme(<ObjectDetailsDrawer {...props} size={1000} />);

// Values under 1024 bytes should remain as bytes
expect(screen.getByText('1000 bytes')).toBeVisible();
});

it('uses base2 calculations (1024-based) for KB display', () => {
renderWithTheme(<ObjectDetailsDrawer {...props} size={1024} />);

// 1024 bytes = 1 KB in base2 (not 1.02 KB as it would be in base10)
expect(screen.getByText('1 KB')).toBeVisible();
});

it('converts base10 MB values correctly to base2 display', () => {
renderWithTheme(<ObjectDetailsDrawer {...props} size={1000000} />);

// 1,000,000 bytes / 1024 = 976.56 KB (rounded to 977 KB)
// Since this is less than 1 MB in base2, it displays as KB
expect(screen.getByText('977 KB')).toBeVisible();
});
});
});
3 changes: 1 addition & 2 deletions packages/manager/src/features/ObjectStorage/BucketDetail/ObjectDetailsDrawer.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,7 @@ export const ObjectDetailsDrawer = React.memo(
>
{size ? (
<Typography variant="subtitle2">
{/* to convert from binary units (GiB) to decimal units (GB) we need to pass the base10 flag */}
{readableBytes(size, { base10: true }).formatted}
{readableBytes(size).formatted}
</Typography>
) : null}
{formattedLastModified && Boolean(profile) ? (
Expand Down
7 changes: 2 additions & 5 deletions packages/manager/src/features/ObjectStorage/BucketDetail/ObjectTableRow.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,10 @@ export const ObjectTableRow = (props: Props) => {
<Grid
container
spacing={2}
wrap="nowrap"
sx={{
alignItems: 'center',
}}
wrap="nowrap"
>
<Grid className="py0">
<ObjectIcon size={20} />
Expand All @@ -56,10 +56,7 @@ export const ObjectTableRow = (props: Props) => {
</Grid>
</Grid>
</TableCell>
<TableCell noWrap>
{/* to convert from binary units (GiB) to decimal units (GB) we need to pass the base10 flag */}
{readableBytes(objectSize, { base10: true }).formatted}
</TableCell>
<TableCell noWrap>{readableBytes(objectSize).formatted}</TableCell>
<Hidden mdDown>
<TableCell noWrap>
<DateTimeDisplay value={objectLastModified} />
Expand Down
3 changes: 1 addition & 2 deletions packages/manager/src/features/ObjectStorage/BucketLanding/BucketDetailsDrawer.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,8 +117,7 @@ export const BucketDetailsDrawer = React.memo(
)}
{typeof size === 'number' && (
<Typography variant="subtitle2">
{/* to convert from binary units (GiB) to decimal units (GB) we need to pass the base10 flag */}
{readableBytes(size, { base10: true }).formatted}
{readableBytes(size).formatted}
</Typography>
)}
{/* @TODO OBJ Multicluster: use region instead of cluster if isObjMultiClusterEnabled. */}
Expand Down
4 changes: 2 additions & 2 deletions packages/manager/src/features/ObjectStorage/BucketLanding/BucketLanding.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,9 +179,9 @@ describe('ObjectStorageLanding', () => {
await screen.findByText(buckets[1].label);
});

it('renders a "Total usage" section if there is more than one Bucket', async () => {
it('renders a "Total usage" section using base2 calculations if there is more than one Bucket', async () => {
const buckets = objectStorageBucketFactory.buildList(2, {
size: 1e9 * 5,
size: 1024 * 1024 * 1024 * 5, // 5 GB in base2 (5 GiB)
});

// Mock Clusters
Expand Down
4 changes: 1 addition & 3 deletions packages/manager/src/features/ObjectStorage/BucketLanding/BucketLanding.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,9 +193,7 @@ export const BucketLanding = (props: Props) => {
style={{ marginTop: 18, textAlign: 'center', width: '100%' }}
variant="body1"
>
Total storage used:{' '}
{/* to convert from binary units (GiB) to decimal units (GB) we need to pass the base10 flag */}
{readableBytes(totalUsage, { base10: true }).formatted}
Total storage used: {readableBytes(totalUsage).formatted}
</Typography>
) : null}
<TransferDisplay
Expand Down
4 changes: 2 additions & 2 deletions packages/manager/src/features/ObjectStorage/BucketLanding/BucketTableRow.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,10 @@
getByText('test-bucket-001.alpha.linodeobjects.com');
});

it('should render a size with the correct size abbreviation', () => {
it('should render size with base2 calculations (displaying GB but representing GiB)', () => {
const { getByText } = renderWithTheme(
wrapWithTableBody(<BucketTableRow {...props} />)
);
getByText('5.42 GB');
getByText('5.05 GB');

Check warning on line 44 in packages/manager/src/features/ObjectStorage/BucketLanding/BucketTableRow.test.tsx

View workflow job for this annotation

GitHub Actions / ESLint Review (manager) 

[eslint] reported by reviewdog 🐶
Avoid destructuring queries from `render` result, use `screen.getByText` instead

Raw Output:
{"ruleId":"testing-library/prefer-screen-queries","severity":1,"message":"Avoid destructuring queries from `render` result, use `screen.getByText` instead","line":44,"column":5,"nodeType":"Identifier","messageId":"preferScreenQueries","endLine":44,"endColumn":14}

Check warning on line 44 in packages/manager/src/features/ObjectStorage/BucketLanding/BucketTableRow.test.tsx

View workflow job for this annotation

GitHub Actions / ESLint Review (manager) 

[eslint] reported by reviewdog 🐶
Wrap stand-alone `getBy*` query with `expect` function for better explicit assertion

Raw Output:
{"ruleId":"testing-library/prefer-explicit-assert","severity":1,"message":"Wrap stand-alone `getBy*` query with `expect` function for better explicit assertion","line":44,"column":5,"nodeType":"Identifier","messageId":"preferExplicitAssert","endLine":44,"endColumn":14}
});
});
5 changes: 2 additions & 3 deletions packages/manager/src/features/ObjectStorage/BucketLanding/BucketTableRow.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ export const BucketTableRow = (props: BucketTableRowProps) => {
<Typography data-qa-region variant="body1">
{isObjMultiClusterEnabled && regionsLookup && region
? regionsLookup[region].label
: clusterRegion?.label ?? cluster}
: (clusterRegion?.label ?? cluster)}
</Typography>
</StyledBucketRegionCell>
</Hidden>
Expand All @@ -109,8 +109,7 @@ export const BucketTableRow = (props: BucketTableRowProps) => {
</Hidden>
<StyledBucketSizeCell noWrap>
<Typography data-qa-size variant="body1">
{/* to convert from binary units (GiB) to decimal units (GB) we need to pass the base10 flag */}
{readableBytes(size, { base10: true }).formatted}
{readableBytes(size).formatted}
</Typography>
</StyledBucketSizeCell>

Expand Down
4 changes: 1 addition & 3 deletions packages/manager/src/features/ObjectStorage/BucketLanding/OMC_BucketLanding.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,9 +212,7 @@ export const OMC_BucketLanding = (props: Props) => {
style={{ marginTop: 18, textAlign: 'center', width: '100%' }}
variant="body1"
>
Total storage used:{' '}
{/* to convert from binary units (GiB) to decimal units (GB) we need to pass the base10 flag */}
{readableBytes(totalUsage, { base10: true }).formatted}
Total storage used: {readableBytes(totalUsage).formatted}
</Typography>
) : null}
<TransferDisplay spacingTop={buckets.length > 1 ? 8 : 18} />
Expand Down
36 changes: 27 additions & 9 deletions packages/utilities/src/helpers/unitConversions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,17 +42,35 @@ export interface ReadableBytesOptions {
base10?: boolean;
handleNegatives?: boolean;
maxUnit?: StorageSymbol;
round?: Partial<Record<StorageSymbol, number>> | number;
round?: number | Partial<Record<StorageSymbol, number>>;
unit?: StorageSymbol;
unitLabels?: Partial<Record<StorageSymbol, string>>;
}

export type StorageSymbol = 'GB' | 'KB' | 'MB' | 'TB' | 'byte' | 'bytes';
export type StorageSymbol = 'byte' | 'bytes' | 'GB' | 'KB' | 'MB' | 'TB';
Copy link
Contributor

@abailly-akamai abailly-akamai Apr 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couple questions here:

  1. are we not receiving/displaying actual base2 metric names (GiB, KiB, etc). I believe this was an important consideration
  2. could those types be moved to APIv4 package?

Copy link
Contributor Author

@jaalah-akamai jaalah-akamai Apr 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. That is correct - That's an intentional decision from Product which appears to be in alignment with other industry practices
  2. Potentially! Let me look

Copy link
Contributor Author

@jaalah-akamai jaalah-akamai Apr 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure it belongs in APIv4 - open to be convinced
On a side note, I think I'm going to follow-up on this PR and try to combine some redundancy from the Cloud Pulse utils packages/manager/src/features/CloudPulse/Utils/unitConversion.ts

Copy link
Contributor

@alioso alioso Apr 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fair enough, i suppose API usually sends everything in bytes so yeah, maybe not. It feels more right to import this type from there (which I need to do) but not a big deal.


// This code inspired by: https://ourcodeworld.com/articles/read/713/converting-bytes-to-human-readable-values-kb-mb-gb-tb-pb-eb-zb-yb-with-javascript

/**
* Converts raw bytes to human-readable format using base2 calculations (1024-based)
* while displaying using common units (KB/MB/GB/TB).
*
* IMPORTANT: We intentionally use base2 calculations (1024 bytes = 1 KB) even though
* we display using traditional storage units. This aligns with industry practice. Internally these represent GiB, TiB, PiB values
* despite the displayed labels.
*
* To use base10 calculations (1000-based), set the base10 option to true.
* By default, calculations use base2 (1024-based).
*
* See: https://techdocs.akamai.com/cloud-computing/docs/understanding-how-billing-works#storage-units
*
* @param num - The number of bytes to convert.
* @param options - Options for the conversion.
* @returns An object containing the formatted value, unit, and value.
*/
export const readableBytes = (
num: number,
options: ReadableBytesOptions = {}
options: ReadableBytesOptions = {},
) => {
// These are the units Classic uses. This can easily be extended –
// just keep adding to this array and the corresponding interface.
Expand Down Expand Up @@ -126,7 +144,7 @@ export const readableBytes = (
export const determinePower = (
num: number,
storageUnits: StorageSymbol[],
options: ReadableBytesOptions
options: ReadableBytesOptions,
) => {
// If maxUnit has been supplied, use that
if (options.unit) {
Expand All @@ -136,7 +154,7 @@ export const determinePower = (

// Otherwise, we need to do some magic, which I don't 100% understand
const magicallyCalculatedPower = Math.floor(
Math.log(num) / Math.log(multiplier)
Math.log(num) / Math.log(multiplier),
);

// If the magically calculated power/unit is higher than the
Expand All @@ -154,7 +172,7 @@ export const determinePower = (
const determineDecimalPlaces = (
num: number,
unit: StorageSymbol,
options: ReadableBytesOptions = {}
options: ReadableBytesOptions = {},
) => {
if (typeof options.round === 'number') {
return options.round;
Expand All @@ -175,12 +193,12 @@ const determineDecimalPlaces = (

export const convertBytesToTarget = (
unit: StorageSymbol | StorageUnitExponentKey,
value: number
value: number,
) => {
switch (unit) {
case 'B':
case 'byte':
case 'bytes':
case 'B':
return value;
default:
return convertStorageUnit('B', value, unit);
Expand All @@ -207,7 +225,7 @@ type StorageUnitExponentKey = keyof typeof StorageUnitExponents;
export const convertStorageUnit = (
sourceUnit: StorageUnitExponentKey,
sourceQuantity: number | undefined,
targetUnit: StorageUnitExponentKey
targetUnit: StorageUnitExponentKey,
) => {
if (sourceQuantity === undefined) {
return 0;
Expand Down