Skip to content

Commit

Permalink
fix(cli): trace output (-vv) is useless when files are uploaded (#33104)
Browse files Browse the repository at this point in the history
### Reason for this change

When running the CDK CLI in trace mode (with `-vv`), e.g. `cdk deploy -vv` we log all SDK calls and their inputs. For file uploads, the input contains the buffer of the file to be uploaded. This results in really bad output:

```
[20:14:31] [sdk info] S3.PutObject({"Bucket":"cdk-hnb659fds-assets-us-east-1","Key":"220c435f80d2dd2cfc5dd0e8e29bfbb6ec58892bd813f5d6a008fdda38f6c02f.zip","Body":{"type":"Buffer","data":[80,75,3,4,20,0,8,0,8,0,0,0,33,0,0,0,0,0,0,0,0,0,0,0,0,0,8,0,0,0,100,97,116,97,46,116,120,116,75,73,44,73,52,227,2,0,80,75,7,8,165,92,116,66,8,0,0,0,6,0,0,0,80,75,1,2,45,3,20,0,8,0,8,0,0,0,33,0,165,92,116,66,8,0,0,0,6,0,0,0,8,0,0,0,0,0,0,0,0,0,32,0,164,129,0,0,0,0,100,97,116,97,46,116,120,116,80,75,5,6,0,0,0,0,1,0,1,0,54,0,0,0,62,0,0,0,0,0]},"ContentType":"application/zip","ChecksumAlgorithm":"SHA256","ServerSideEncryption":"aws:kms"}) -> OK
```

Now in this case we are only uploading 138 bytes. Imagine what this log looks like for files of 10MiB or 100MiB. Reportedly this also crashed every terminal.

### Description of changes

There's no need to log the buffer bytes. Instead replace it with a string indicating its a buffer and the buffer length:

```
[20:14:31] [sdk info] S3.PutObject({"Body":"<Buffer 138 Bytes>"}) -> OK
```

### Describe any new or updated permissions being added

n/a

### Description of how you validated changes

Tests

### Checklist
- [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md)

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
mrgrain authored Jan 24, 2025
1 parent c0a2cbf commit d95add3
Show file tree
Hide file tree
Showing 5 changed files with 81 additions and 1 deletion.
3 changes: 2 additions & 1 deletion packages/aws-cdk/lib/api/aws-auth/sdk-logger.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { inspect } from 'util';
import { Logger } from '@smithy/types';
import { trace } from '../../logging';
import { replacerBufferWithInfo } from '../../serialize';

export class SdkToCliLogger implements Logger {
public trace(..._content: any[]) {
Expand Down Expand Up @@ -105,7 +106,7 @@ function formatApiCall(content: any): string | undefined {
parts.push(`[${content.metadata?.attempts} attempts, ${content.metadata?.totalRetryDelay}ms retry]`);
}

parts.push(`${service}.${api}(${JSON.stringify(content.input)})`);
parts.push(`${service}.${api}(${JSON.stringify(content.input, replacerBufferWithInfo)})`);

if (isSdkApiCallSuccess(content)) {
parts.push('-> OK');
Expand Down
33 changes: 33 additions & 0 deletions packages/aws-cdk/lib/serialize.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import * as fs from 'fs-extra';
import { formatBytes } from './util/bytes';
import * as yaml_cfn from './util/yaml-cfn';

/**
Expand Down Expand Up @@ -51,3 +52,35 @@ export function obscureTemplate(template: any = {}) {

return template;
}

/**
* Detects a buffer that has been converted to a JSON-like object
* In Node, `Buffer`s have `toJSON()` method that converts the buffer
* into a JS object that can be JSON stringified.
* Unfortunately this conversion happens before the replacer is called,
* so normal means of detecting a `Buffer` objet don't work anymore.
* @see https://github.com/nodejs/node-v0.x-archive/issues/5110
*/
function isJsonBuffer(value: any): value is {
type: 'Buffer';
data: number[];
} {
return typeof value === 'object'
&& 'type' in value
&& value.type === 'Buffer'
&& 'data' in value
&& Array.isArray(value.data);
}

/**
* A JSON.stringify() replacer that converts Buffers into a string with information
* Use this if you plan to print out JSON stringified objects that may contain a Buffer.
* Without this, large buffers (think: Megabytes) will completely fill up the output
* and even crash the system.
*/
export function replacerBufferWithInfo(_key: any, value: any): any {
if (isJsonBuffer(value)) {
return `<Buffer: ${formatBytes(value.data.length)}>`;
}
return value;
}
21 changes: 21 additions & 0 deletions packages/aws-cdk/lib/util/bytes.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
/**
* Format bytes as a human readable string
*
* @param bytes Number of bytes to format
* @param decimals Number of decimal places to show (default 2)
* @returns Formatted string with appropriate unit suffix
*/
export function formatBytes(bytes: number, decimals: number = 2): string {
decimals = decimals < 0 ? 0 : decimals;

if (bytes === 0) {
return '0 Bytes';
}

const k = 1024;
const sizes = ['Bytes', 'KiB', 'MiB', 'GiB', 'TiB', 'PiB', 'EiB', 'ZiB', 'YiB'];

const i = Math.floor(Math.log(bytes) / Math.log(k));

return `${parseFloat((bytes / Math.pow(k, i)).toFixed(decimals))} ${sizes[i]}`;
}
18 changes: 18 additions & 0 deletions packages/aws-cdk/test/util/bytes.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import { formatBytes } from '../../lib/util/bytes';

test.each([
[0, '0 Bytes'],
[10, '10 Bytes'],
[1024, '1 KiB'],
[10.5 * 1024 * 1024, '10.5 MiB'],
])('converts %s bytes to %s', (bytes: number, expected: string) => {
expect(formatBytes(bytes)).toEqual(expected);
});

test('can format many decimals', () => {
expect(formatBytes(10.51234 * 1024 * 1024, 5)).toEqual('10.51234 MiB');
});

test('can deal with negative decimals', () => {
expect(formatBytes(10.5 * 1024 * 1024, -1)).toEqual('11 MiB');
});
7 changes: 7 additions & 0 deletions packages/aws-cdk/test/util/serialize.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import { replacerBufferWithInfo } from '../../lib/serialize';

test('converts buffer to information', () => {
const res = JSON.stringify({ data: Buffer.from('test data') }, replacerBufferWithInfo);

expect(res).toEqual('{"data":"<Buffer: 9 Bytes>"}');
});

0 comments on commit d95add3

Please sign in to comment.