Skip to content

Commit d2e0b29

Browse files
authored
chore(cli): log SDK calls when using -vvv (#32308)
(This work came out of the proxy issue research) The logging of what SDK calls were being performed was lost during the migration of SDKv2 -> SDKv3. Add it back. Also in this PR: - Set a timeout on `npm view`; if for network isolation reasons NPM can't connect to the server, it will make the CLI hang for a minute. - Work around an issue where the CLI entirely fails if it is run as a user that doesn't have a home directory. Closes #32306. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent 559d676 commit d2e0b29

File tree

8 files changed

+249
-8
lines changed

8 files changed

+249
-8
lines changed
Lines changed: 142 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,142 @@
1+
import { inspect } from 'util';
2+
import { Logger } from '@smithy/types';
3+
import { trace } from '../../logging';
4+
5+
export class SdkToCliLogger implements Logger {
6+
public trace(..._content: any[]) {
7+
// This is too much detail for our logs
8+
// trace('[SDK trace] %s', fmtContent(content));
9+
}
10+
11+
public debug(..._content: any[]) {
12+
// This is too much detail for our logs
13+
// trace('[SDK debug] %s', fmtContent(content));
14+
}
15+
16+
/**
17+
* Info is called mostly (exclusively?) for successful API calls
18+
*
19+
* Payload:
20+
*
21+
* (Note the input contains entire CFN templates, for example)
22+
*
23+
* ```
24+
* {
25+
* clientName: 'S3Client',
26+
* commandName: 'GetBucketLocationCommand',
27+
* input: {
28+
* Bucket: '.....',
29+
* ExpectedBucketOwner: undefined
30+
* },
31+
* output: { LocationConstraint: 'eu-central-1' },
32+
* metadata: {
33+
* httpStatusCode: 200,
34+
* requestId: '....',
35+
* extendedRequestId: '...',
36+
* cfId: undefined,
37+
* attempts: 1,
38+
* totalRetryDelay: 0
39+
* }
40+
* }
41+
* ```
42+
*/
43+
public info(...content: any[]) {
44+
trace('[sdk info] %s', formatSdkLoggerContent(content));
45+
}
46+
47+
public warn(...content: any[]) {
48+
trace('[sdk warn] %s', formatSdkLoggerContent(content));
49+
}
50+
51+
/**
52+
* Error is called mostly (exclusively?) for failing API calls
53+
*
54+
* Payload (input would be the entire API call arguments).
55+
*
56+
* ```
57+
* {
58+
* clientName: 'STSClient',
59+
* commandName: 'GetCallerIdentityCommand',
60+
* input: {},
61+
* error: AggregateError [ECONNREFUSED]:
62+
* at internalConnectMultiple (node:net:1121:18)
63+
* at afterConnectMultiple (node:net:1688:7) {
64+
* code: 'ECONNREFUSED',
65+
* '$metadata': { attempts: 3, totalRetryDelay: 600 },
66+
* [errors]: [ [Error], [Error] ]
67+
* },
68+
* metadata: { attempts: 3, totalRetryDelay: 600 }
69+
* }
70+
* ```
71+
*/
72+
public error(...content: any[]) {
73+
trace('[sdk error] %s', formatSdkLoggerContent(content));
74+
}
75+
}
76+
77+
/**
78+
* This can be anything.
79+
*
80+
* For debug, it seems to be mostly strings.
81+
* For info, it seems to be objects.
82+
*
83+
* Stringify and join without separator.
84+
*/
85+
export function formatSdkLoggerContent(content: any[]) {
86+
if (content.length === 1) {
87+
const apiFmt = formatApiCall(content[0]);
88+
if (apiFmt) {
89+
return apiFmt;
90+
}
91+
}
92+
return content.map((x) => typeof x === 'string' ? x : inspect(x)).join('');
93+
}
94+
95+
function formatApiCall(content: any): string | undefined {
96+
if (!isSdkApiCallSuccess(content) && !isSdkApiCallError(content)) {
97+
return undefined;
98+
}
99+
100+
const service = content.clientName.replace(/Client$/, '');
101+
const api = content.commandName.replace(/Command$/, '');
102+
103+
const parts = [];
104+
if ((content.metadata?.attempts ?? 0) > 1) {
105+
parts.push(`[${content.metadata?.attempts} attempts, ${content.metadata?.totalRetryDelay}ms retry]`);
106+
}
107+
108+
parts.push(`${service}.${api}(${JSON.stringify(content.input)})`);
109+
110+
if (isSdkApiCallSuccess(content)) {
111+
parts.push('-> OK');
112+
} else {
113+
parts.push(`-> ${content.error}`);
114+
}
115+
116+
return parts.join(' ');
117+
}
118+
119+
interface SdkApiCallBase {
120+
clientName: string;
121+
commandName: string;
122+
input: Record<string, unknown>;
123+
metadata?: {
124+
httpStatusCode?: number;
125+
requestId?: string;
126+
extendedRequestId?: string;
127+
cfId?: string;
128+
attempts?: number;
129+
totalRetryDelay?: number;
130+
};
131+
}
132+
133+
type SdkApiCallSuccess = SdkApiCallBase & { output: Record<string, unknown> };
134+
type SdkApiCallError = SdkApiCallBase & { error: Error };
135+
136+
function isSdkApiCallSuccess(x: any): x is SdkApiCallSuccess {
137+
return x && typeof x === 'object' && x.commandName && x.output;
138+
}
139+
140+
function isSdkApiCallError(x: any): x is SdkApiCallError {
141+
return x && typeof x === 'object' && x.commandName && x.error;
142+
}

packages/aws-cdk/lib/api/aws-auth/sdk-provider.ts

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ export class SdkProvider {
126126

127127
const region = await AwsCliCompatible.region(options.profile);
128128
const requestHandler = AwsCliCompatible.requestHandlerBuilder(options.httpOptions);
129-
return new SdkProvider(credentialProvider, region, requestHandler);
129+
return new SdkProvider(credentialProvider, region, requestHandler, options.logger);
130130
}
131131

132132
private readonly plugins = new CredentialPlugins();
@@ -138,6 +138,7 @@ export class SdkProvider {
138138
*/
139139
public readonly defaultRegion: string,
140140
private readonly requestHandler: NodeHttpHandlerOptions = {},
141+
private readonly logger?: Logger,
141142
) {}
142143

143144
/**
@@ -169,7 +170,7 @@ export class SdkProvider {
169170

170171
// Our current credentials must be valid and not expired. Confirm that before we get into doing
171172
// actual CloudFormation calls, which might take a long time to hang.
172-
const sdk = new SDK(baseCreds.credentials, env.region, this.requestHandler);
173+
const sdk = new SDK(baseCreds.credentials, env.region, this.requestHandler, this.logger);
173174
await sdk.validateCredentials();
174175
return { sdk, didAssumeRole: false };
175176
}
@@ -201,7 +202,7 @@ export class SdkProvider {
201202
`${fmtObtainedCredentials(baseCreds)} could not be used to assume '${options.assumeRoleArn}', but are for the right account. Proceeding anyway.`,
202203
);
203204
return {
204-
sdk: new SDK(baseCreds.credentials, env.region, this.requestHandler),
205+
sdk: new SDK(baseCreds.credentials, env.region, this.requestHandler, this.logger),
205206
didAssumeRole: false,
206207
};
207208
}
@@ -221,7 +222,7 @@ export class SdkProvider {
221222
if (baseCreds.source === 'none') {
222223
return undefined;
223224
}
224-
return (await new SDK(baseCreds.credentials, env.region, this.requestHandler).currentAccount()).partition;
225+
return (await new SDK(baseCreds.credentials, env.region, this.requestHandler, this.logger).currentAccount()).partition;
225226
}
226227

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

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

382-
return new SDK(credentials, region, this.requestHandler);
385+
return new SDK(credentials, region, this.requestHandler, this.logger);
383386
} catch (err: any) {
384387
if (err.name === 'ExpiredToken') {
385388
throw err;

packages/aws-cdk/lib/api/aws-auth/sdk.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -545,13 +545,15 @@ export class SDK {
545545
private readonly _credentials: AwsCredentialIdentity,
546546
region: string,
547547
requestHandler: NodeHttpHandlerOptions,
548+
logger?: Logger,
548549
) {
549550
this.config = {
550551
region,
551552
credentials: _credentials,
552553
requestHandler,
553554
retryStrategy: new ConfiguredRetryStrategy(7, (attempt) => 300 * (2 ** attempt)),
554555
customUserAgent: defaultCliUserAgent(),
556+
logger,
555557
};
556558
this.currentRegion = region;
557559
}

packages/aws-cdk/lib/cli.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import { data, debug, error, print, setCI, setLogLevel, LogLevel } from '../lib/
2626
import { Notices } from '../lib/notices';
2727
import { Command, Configuration, Settings } from '../lib/settings';
2828
import * as version from '../lib/version';
29+
import { SdkToCliLogger } from './api/aws-auth/sdk-logger';
2930

3031
/* eslint-disable max-len */
3132
/* eslint-disable @typescript-eslint/no-shadow */ // yargs
@@ -101,6 +102,7 @@ export async function exec(args: string[], synthesizer?: Synthesizer): Promise<n
101102
proxyAddress: argv.proxy,
102103
caBundlePath: argv['ca-bundle-path'],
103104
},
105+
logger: new SdkToCliLogger(),
104106
});
105107

106108
let outDirLock: ILock | undefined;

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

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,13 @@ export function cdkHomeDir() {
1717
const tmpDir = fs.realpathSync(os.tmpdir());
1818
let home;
1919
try {
20-
home = path.join((os.userInfo().homedir ?? os.homedir()).trim(), '.cdk');
20+
let userInfoHome: string | undefined = os.userInfo().homedir;
21+
// Node returns this if the user doesn't have a home directory
22+
/* istanbul ignore if: will not happen in normal setups */
23+
if (userInfoHome == '/var/empty') {
24+
userInfoHome = undefined;
25+
}
26+
home = path.join((userInfoHome ?? os.homedir()).trim(), '.cdk');
2127
} catch {}
2228
return process.env.CDK_HOME
2329
? path.resolve(process.env.CDK_HOME)

packages/aws-cdk/lib/util/npm.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import { debug } from '../../lib/logging';
66
const exec = promisify(_exec);
77

88
export async function getLatestVersionFromNpm(): Promise<string> {
9-
const { stdout, stderr } = await exec('npm view aws-cdk version');
9+
const { stdout, stderr } = await exec('npm view aws-cdk version', { timeout: 3000 });
1010
if (stderr && stderr.trim().length > 0) {
1111
debug(`The 'npm view' command generated an error stream with content [${stderr.trim()}]`);
1212
}
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
// Jest Snapshot v1, https://goo.gl/fbAQLP
2+
3+
exports[`formatting a failing SDK call looks broadly reasonable 1`] = `"[2 attempts, 30ms retry] S3.GetBucketLocation({"Bucket":"....."}) -> Error: it failed"`;
4+
5+
exports[`formatting a successful SDK call looks broadly reasonable 1`] = `"[2 attempts, 30ms retry] S3.GetBucketLocation({"Bucket":"....."}) -> OK"`;
Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
import { formatSdkLoggerContent, SdkToCliLogger } from '../../../lib/api/aws-auth/sdk-logger';
2+
import * as logging from '../../../lib/logging';
3+
4+
describe(SdkToCliLogger, () => {
5+
const logger = new SdkToCliLogger();
6+
const trace = jest.spyOn(logging, 'trace');
7+
8+
beforeEach(() => {
9+
trace.mockReset();
10+
});
11+
12+
test.each(['trace', 'debug'] as Array<keyof SdkToCliLogger>)('%s method does not call trace', (meth) => {
13+
logger[meth]('test');
14+
expect(trace).not.toHaveBeenCalled();
15+
});
16+
17+
test.each(['info', 'warn', 'error'] as Array<keyof SdkToCliLogger>)('%s method logs to trace', (meth) => {
18+
logger[meth]('test');
19+
expect(trace).toHaveBeenCalled();
20+
});
21+
});
22+
23+
const SUCCESS_CALL = {
24+
clientName: 'S3Client',
25+
commandName: 'GetBucketLocationCommand',
26+
input: {
27+
Bucket: '.....',
28+
ExpectedBucketOwner: undefined,
29+
},
30+
output: { LocationConstraint: 'eu-central-1' },
31+
metadata: {
32+
httpStatusCode: 200,
33+
requestId: '....',
34+
extendedRequestId: '...',
35+
cfId: undefined,
36+
attempts: 2,
37+
totalRetryDelay: 30,
38+
},
39+
};
40+
41+
const ERROR_CALL = {
42+
clientName: 'S3Client',
43+
commandName: 'GetBucketLocationCommand',
44+
input: {
45+
Bucket: '.....',
46+
ExpectedBucketOwner: undefined,
47+
},
48+
error: new Error('it failed'),
49+
metadata: {
50+
httpStatusCode: 200,
51+
attempts: 2,
52+
totalRetryDelay: 30,
53+
},
54+
};
55+
56+
test('formatting a successful SDK call looks broadly reasonable', () => {
57+
const output = formatSdkLoggerContent([SUCCESS_CALL]);
58+
expect(output).toMatchSnapshot();
59+
});
60+
61+
test('formatting a failing SDK call looks broadly reasonable', () => {
62+
const output = formatSdkLoggerContent([ERROR_CALL]);
63+
expect(output).toMatchSnapshot();
64+
});
65+
66+
test('formatting a successful SDK call includes attempts and retries if greater than 1', () => {
67+
const output = formatSdkLoggerContent([SUCCESS_CALL]);
68+
expect(output).toContain('2 attempts');
69+
expect(output).toContain('30ms');
70+
});
71+
72+
test('formatting a failing SDK call includes attempts and retries if greater than 1', () => {
73+
const output = formatSdkLoggerContent([ERROR_CALL]);
74+
expect(output).toContain('2 attempts');
75+
expect(output).toContain('30ms');
76+
});
77+
78+
test('formatting a failing SDK call includes the error', () => {
79+
const output = formatSdkLoggerContent([ERROR_CALL]);
80+
expect(output).toContain('it failed');
81+
});

0 commit comments

Comments
 (0)