Skip to content
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.

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

chore(cli): log SDK calls when using -vvv #32308

Merged
merged 8 commits into from
Nov 28, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
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
144 changes: 144 additions & 0 deletions packages/aws-cdk/lib/api/aws-auth/sdk-logger.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,144 @@
import { inspect } from 'util';
import { Logger } from '@smithy/types';
import { trace } from '../../logging';

export class SdkToCliLogger implements Logger {
public trace(...content: any[]) {

Check warning on line 6 in packages/aws-cdk/lib/api/aws-auth/sdk-logger.ts

View check run for this annotation

Codecov / codecov/patch

packages/aws-cdk/lib/api/aws-auth/sdk-logger.ts#L6

Added line #L6 was not covered by tests
// This is too much detail for our logs
// trace('[SDK trace] %s', this.fmtContent(content));
Array.isArray(content);

Check warning on line 9 in packages/aws-cdk/lib/api/aws-auth/sdk-logger.ts

View check run for this annotation

Codecov / codecov/patch

packages/aws-cdk/lib/api/aws-auth/sdk-logger.ts#L9

Added line #L9 was not covered by tests
Copy link
Contributor

@otaviomacedo otaviomacedo Nov 28, 2024

Choose a reason for hiding this comment

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

I don't understand what this line is doing.

}

public debug(...content: any[]) {

Check warning on line 12 in packages/aws-cdk/lib/api/aws-auth/sdk-logger.ts

View check run for this annotation

Codecov / codecov/patch

packages/aws-cdk/lib/api/aws-auth/sdk-logger.ts#L12

Added line #L12 was not covered by tests
// This is too much detail for our logs
// trace('[SDK debug] %s', this.fmtContent(content));
Array.isArray(content);

Check warning on line 15 in packages/aws-cdk/lib/api/aws-auth/sdk-logger.ts

View check run for this annotation

Codecov / codecov/patch

packages/aws-cdk/lib/api/aws-auth/sdk-logger.ts#L15

Added line #L15 was not covered by tests
}

/**
* Info is called mostly (exclusively?) for successful API calls
*
* Payload:
*
* (Note the input contains entire CFN templates, for example)
*
* ```
* {
* clientName: 'S3Client',
* commandName: 'GetBucketLocationCommand',
* input: {
* Bucket: '.....',
* ExpectedBucketOwner: undefined
* },
* output: { LocationConstraint: 'eu-central-1' },
* metadata: {
* httpStatusCode: 200,
* requestId: '....',
* extendedRequestId: '...',
* cfId: undefined,
* attempts: 1,
* totalRetryDelay: 0
* }
* }
* ```
*/
public info(...content: any[]) {
trace('[sdk info] %s', this.fmtContent(content));

Check warning on line 46 in packages/aws-cdk/lib/api/aws-auth/sdk-logger.ts

View check run for this annotation

Codecov / codecov/patch

packages/aws-cdk/lib/api/aws-auth/sdk-logger.ts#L45-L46

Added lines #L45 - L46 were not covered by tests
}

public warn(...content: any[]) {
trace('[sdk warn] %s', this.fmtContent(content));

Check warning on line 50 in packages/aws-cdk/lib/api/aws-auth/sdk-logger.ts

View check run for this annotation

Codecov / codecov/patch

packages/aws-cdk/lib/api/aws-auth/sdk-logger.ts#L49-L50

Added lines #L49 - L50 were not covered by tests
}

/**
* Error is called mostly (exclusively?) for failing API calls
*
* Payload (input would be the entire API call arguments).
*
* ```
* {
* clientName: 'STSClient',
* commandName: 'GetCallerIdentityCommand',
* input: {},
* error: AggregateError [ECONNREFUSED]:
* at internalConnectMultiple (node:net:1121:18)
* at afterConnectMultiple (node:net:1688:7) {
* code: 'ECONNREFUSED',
* '$metadata': { attempts: 3, totalRetryDelay: 600 },
* [errors]: [ [Error], [Error] ]
* },
* metadata: { attempts: 3, totalRetryDelay: 600 }
* }
* ```
*/
public error(...content: any[]) {
trace('[sdk error] %s', this.fmtContent(content));

Check warning on line 75 in packages/aws-cdk/lib/api/aws-auth/sdk-logger.ts

View check run for this annotation

Codecov / codecov/patch

packages/aws-cdk/lib/api/aws-auth/sdk-logger.ts#L74-L75

Added lines #L74 - L75 were not covered by tests
}

/**
* This can be anything.
*
* For debug, it seems to be mostly strings.
* For info, it seems to be objects.
*
* Stringify and join without separator.
*/
private fmtContent(content: any[]) {

Check warning on line 86 in packages/aws-cdk/lib/api/aws-auth/sdk-logger.ts

View check run for this annotation

Codecov / codecov/patch

packages/aws-cdk/lib/api/aws-auth/sdk-logger.ts#L86

Added line #L86 was not covered by tests
if (content.length === 1) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can content not be an array with more than one error, for example?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If so we won't do the wrong thing, we just won't be doing a nice thing.

We'll see that in the logs at some point and fix it then.

const apiFmt = formatApiCall(content[0]);

Check warning on line 88 in packages/aws-cdk/lib/api/aws-auth/sdk-logger.ts

View check run for this annotation

Codecov / codecov/patch

packages/aws-cdk/lib/api/aws-auth/sdk-logger.ts#L88

Added line #L88 was not covered by tests
if (apiFmt) {
return apiFmt;

Check warning on line 90 in packages/aws-cdk/lib/api/aws-auth/sdk-logger.ts

View check run for this annotation

Codecov / codecov/patch

packages/aws-cdk/lib/api/aws-auth/sdk-logger.ts#L90

Added line #L90 was not covered by tests
}
}
return content.map((x) => typeof x === 'string' ? x : inspect(x)).join('');
}
}

export function formatApiCall(content: any): string | undefined {
if (!isSdkApiCallSuccess(content) && !isSdkApiCallError(content)) {
return undefined;

Check warning on line 99 in packages/aws-cdk/lib/api/aws-auth/sdk-logger.ts

View check run for this annotation

Codecov / codecov/patch

packages/aws-cdk/lib/api/aws-auth/sdk-logger.ts#L99

Added line #L99 was not covered by tests
}

const service = content.clientName.replace(/Client$/, '');
const api = content.commandName.replace(/Command$/, '');

Check warning on line 103 in packages/aws-cdk/lib/api/aws-auth/sdk-logger.ts

View check run for this annotation

Codecov / codecov/patch

packages/aws-cdk/lib/api/aws-auth/sdk-logger.ts#L102-L103

Added lines #L102 - L103 were not covered by tests

const parts = [];

Check warning on line 105 in packages/aws-cdk/lib/api/aws-auth/sdk-logger.ts

View check run for this annotation

Codecov / codecov/patch

packages/aws-cdk/lib/api/aws-auth/sdk-logger.ts#L105

Added line #L105 was not covered by tests
if (content.metadata.attempts > 1) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is metadata guaranteed to exist?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think so but who knows.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll code more defensively

parts.push(`[${content.metadata.attempts} attempts, ${content.metadata.totalRetryDelay}ms retry]`);

Check warning on line 107 in packages/aws-cdk/lib/api/aws-auth/sdk-logger.ts

View check run for this annotation

Codecov / codecov/patch

packages/aws-cdk/lib/api/aws-auth/sdk-logger.ts#L107

Added line #L107 was not covered by tests
}

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

Check warning on line 110 in packages/aws-cdk/lib/api/aws-auth/sdk-logger.ts

View check run for this annotation

Codecov / codecov/patch

packages/aws-cdk/lib/api/aws-auth/sdk-logger.ts#L110

Added line #L110 was not covered by tests

if (isSdkApiCallSuccess(content)) {
parts.push('-> OK');

Check warning on line 113 in packages/aws-cdk/lib/api/aws-auth/sdk-logger.ts

View check run for this annotation

Codecov / codecov/patch

packages/aws-cdk/lib/api/aws-auth/sdk-logger.ts#L113

Added line #L113 was not covered by tests
} else {
parts.push(`-> ${content.error}`);

Check warning on line 115 in packages/aws-cdk/lib/api/aws-auth/sdk-logger.ts

View check run for this annotation

Codecov / codecov/patch

packages/aws-cdk/lib/api/aws-auth/sdk-logger.ts#L115

Added line #L115 was not covered by tests
}

return parts.join(' ');

Check warning on line 118 in packages/aws-cdk/lib/api/aws-auth/sdk-logger.ts

View check run for this annotation

Codecov / codecov/patch

packages/aws-cdk/lib/api/aws-auth/sdk-logger.ts#L118

Added line #L118 was not covered by tests
}

interface SdkApiCallBase {
clientName: string;
commandName: string;
input: Record<string, unknown>;
metadata: {
httpStatusCode?: number;
requestId?: string;
extendedRequestId?: string;
cfId?: string;
attempts: number;
totalRetryDelay: number;
};
}

type SdkApiCallSuccess = SdkApiCallBase & { output: Record<string, unknown> };
type SdkApiCallError = SdkApiCallBase & { error: Error };

function isSdkApiCallSuccess(x: any): x is SdkApiCallSuccess {

Check warning on line 138 in packages/aws-cdk/lib/api/aws-auth/sdk-logger.ts

View check run for this annotation

Codecov / codecov/patch

packages/aws-cdk/lib/api/aws-auth/sdk-logger.ts#L138

Added line #L138 was not covered by tests
return x && typeof x === 'object' && x.commandName && x.output;
}

function isSdkApiCallError(x: any): x is SdkApiCallError {

Check warning on line 142 in packages/aws-cdk/lib/api/aws-auth/sdk-logger.ts

View check run for this annotation

Codecov / codecov/patch

packages/aws-cdk/lib/api/aws-auth/sdk-logger.ts#L142

Added line #L142 was not covered by tests
return x && typeof x === 'object' && x.commandName && x.error;
}
15 changes: 9 additions & 6 deletions packages/aws-cdk/lib/api/aws-auth/sdk-provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ export class SdkProvider {

const region = await AwsCliCompatible.region(options.profile);
const requestHandler = AwsCliCompatible.requestHandlerBuilder(options.httpOptions);
return new SdkProvider(credentialProvider, region, requestHandler);
return new SdkProvider(credentialProvider, region, requestHandler, options.logger);
}

private readonly plugins = new CredentialPlugins();
Expand All @@ -138,6 +138,7 @@ export class SdkProvider {
*/
public readonly defaultRegion: string,
private readonly requestHandler: NodeHttpHandlerOptions = {},
private readonly logger?: Logger,
) {}

/**
Expand Down Expand Up @@ -169,7 +170,7 @@ export class SdkProvider {

// Our current credentials must be valid and not expired. Confirm that before we get into doing
// actual CloudFormation calls, which might take a long time to hang.
const sdk = new SDK(baseCreds.credentials, env.region, this.requestHandler);
const sdk = new SDK(baseCreds.credentials, env.region, this.requestHandler, this.logger);
await sdk.validateCredentials();
return { sdk, didAssumeRole: false };
}
Expand Down Expand Up @@ -201,7 +202,7 @@ export class SdkProvider {
`${fmtObtainedCredentials(baseCreds)} could not be used to assume '${options.assumeRoleArn}', but are for the right account. Proceeding anyway.`,
);
return {
sdk: new SDK(baseCreds.credentials, env.region, this.requestHandler),
sdk: new SDK(baseCreds.credentials, env.region, this.requestHandler, this.logger),
didAssumeRole: false,
};
}
Expand All @@ -221,7 +222,7 @@ export class SdkProvider {
if (baseCreds.source === 'none') {
return undefined;
}
return (await new SDK(baseCreds.credentials, env.region, this.requestHandler).currentAccount()).partition;
return (await new SDK(baseCreds.credentials, env.region, this.requestHandler, this.logger).currentAccount()).partition;
}

/**
Expand Down Expand Up @@ -273,7 +274,7 @@ export class SdkProvider {
throw new Error('Unable to resolve AWS credentials (setup with "aws configure")');
}

return await new SDK(credentials, this.defaultRegion, this.requestHandler).currentAccount();
return await new SDK(credentials, this.defaultRegion, this.requestHandler, this.logger).currentAccount();
} catch (e: any) {
// Treat 'ExpiredToken' specially. This is a common situation that people may find themselves in, and
// they are complaining about if we fail 'cdk synth' on them. We loudly complain in order to show that
Expand Down Expand Up @@ -376,10 +377,12 @@ export class SdkProvider {
clientConfig: {
region,
requestHandler: this.requestHandler,
customUserAgent: 'aws-cdk',
logger: this.logger,
},
})();

return new SDK(credentials, region, this.requestHandler);
return new SDK(credentials, region, this.requestHandler, this.logger);
} catch (err: any) {
if (err.name === 'ExpiredToken') {
throw err;
Expand Down
2 changes: 2 additions & 0 deletions packages/aws-cdk/lib/api/aws-auth/sdk.ts
Original file line number Diff line number Diff line change
Expand Up @@ -545,13 +545,15 @@ export class SDK {
private readonly _credentials: AwsCredentialIdentity,
region: string,
requestHandler: NodeHttpHandlerOptions,
logger?: Logger,
) {
this.config = {
region,
credentials: _credentials,
requestHandler,
retryStrategy: new ConfiguredRetryStrategy(7, (attempt) => 300 * (2 ** attempt)),
customUserAgent: defaultCliUserAgent(),
logger,
};
this.currentRegion = region;
}
Expand Down
2 changes: 2 additions & 0 deletions packages/aws-cdk/lib/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import { data, debug, error, print, setLogLevel, setCI } from '../lib/logging';
import { Notices } from '../lib/notices';
import { Command, Configuration, Settings } from '../lib/settings';
import * as version from '../lib/version';
import { SdkToCliLogger } from './api/aws-auth/sdk-logger';

/* eslint-disable max-len */
/* eslint-disable @typescript-eslint/no-shadow */ // yargs
Expand Down Expand Up @@ -89,6 +90,7 @@ export async function exec(args: string[], synthesizer?: Synthesizer): Promise<n
proxyAddress: argv.proxy,
caBundlePath: argv['ca-bundle-path'],
},
logger: new SdkToCliLogger(),
});

let outDirLock: ILock | undefined;
Expand Down
7 changes: 6 additions & 1 deletion packages/aws-cdk/lib/util/directories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,12 @@
const tmpDir = fs.realpathSync(os.tmpdir());
let home;
try {
home = path.join((os.userInfo().homedir ?? os.homedir()).trim(), '.cdk');
let userInfoHome: string | undefined = os.userInfo().homedir;
// Node returns this if the user doesn't have a home directory
if (userInfoHome == '/var/empty') {
userInfoHome = undefined;

Check warning on line 23 in packages/aws-cdk/lib/util/directories.ts

View check run for this annotation

Codecov / codecov/patch

packages/aws-cdk/lib/util/directories.ts#L23

Added line #L23 was not covered by tests
}
home = path.join((userInfoHome ?? os.homedir()).trim(), '.cdk');
} catch {}
return process.env.CDK_HOME
? path.resolve(process.env.CDK_HOME)
Expand Down
2 changes: 1 addition & 1 deletion packages/aws-cdk/lib/util/npm.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { debug } from '../../lib/logging';
const exec = promisify(_exec);

export async function getLatestVersionFromNpm(): Promise<string> {
const { stdout, stderr } = await exec('npm view aws-cdk version');
const { stdout, stderr } = await exec('npm view aws-cdk version', { timeout: 3000 });
if (stderr && stderr.trim().length > 0) {
debug(`The 'npm view' command generated an error stream with content [${stderr.trim()}]`);
}
Expand Down
Loading