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

rix0rrr · 2024-11-27T12:03:58Z

(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.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

(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.

rix0rrr · 2024-11-27T12:04:57Z

Still a Draft because I'm not happy with how the messages are being formatted; there is too much detail in them as they include the entire API call payloads and responses (including somewhat-sensitive information).

But creating a PR already anyway because I want to store this code somewhere safe.

aws-cdk-automation

The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.

A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.

codecov · 2024-11-27T12:16:02Z

Codecov Report

Attention: Patch coverage is 95.55556% with 2 lines in your changes missing coverage. Please review.

Project coverage is 78.45%. Comparing base (559d676) to head (d94db0a).
Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #32308      +/-   ##
==========================================
+ Coverage   78.36%   78.45%   +0.08%     
==========================================
  Files         105      106       +1     
  Lines        7170     7208      +38     
  Branches     1315     1323       +8     
==========================================
+ Hits         5619     5655      +36     
- Misses       1364     1365       +1     
- Partials      187      188       +1

Flag	Coverage Δ
suite.unit	`78.45% <95.55%> (+0.08%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components	Coverage Δ
packages/aws-cdk	`78.45% <95.55%> (+0.08%)`	⬆️

otaviomacedo · 2024-11-28T13:37:01Z

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

+  public trace(...content: any[]) {
+    // This is too much detail for our logs
+    // trace('[SDK trace] %s', this.fmtContent(content));
+    Array.isArray(content);


I don't understand what this line is doing.

otaviomacedo · 2024-11-28T13:41:43Z

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

+  const api = content.commandName.replace(/Command$/, '');
+
+  const parts = [];
+  if (content.metadata.attempts > 1) {


Is metadata guaranteed to exist?

I think so but who knows.

I'll code more defensively

otaviomacedo · 2024-11-28T13:49:51Z

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

+   * Stringify and join without separator.
+   */
+  private fmtContent(content: any[]) {
+    if (content.length === 1) {


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

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.

✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.

mergify · 2024-11-28T17:03:03Z

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

mergify · 2024-11-28T17:07:39Z

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

aws-cdk-automation · 2024-11-28T17:58:00Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
Commit ID: d94db0a
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

mergify · 2024-11-28T17:58:14Z

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

github-actions · 2024-11-28T17:58:27Z

Comments on closed issues and PRs are hard for our team to see.
If you need help, please open a new issue that references this one.

github-actions bot added bug This issue is a bug. p2 labels Nov 27, 2024

aws-cdk-automation requested a review from a team November 27, 2024 12:04

mergify bot added the contribution/core This is a PR that came from AWS. label Nov 27, 2024

rix0rrr requested a deployment to test-pipeline November 27, 2024 12:04 — with GitHub Actions Waiting

rix0rrr changed the title ~~chore(cli): improve SDK logging~~ chore(cli): log SDK calls when using -vvv Nov 27, 2024

aws-cdk-automation previously requested changes Nov 27, 2024

View reviewed changes

aws-cdk-automation added the pr/needs-cli-test-run This PR needs CLI tests run against it. label Nov 27, 2024

Prettier format

751c525

rix0rrr marked this pull request as ready for review November 27, 2024 13:17

rix0rrr requested a review from a team as a code owner November 27, 2024 13:18

rix0rrr requested a deployment to test-pipeline November 27, 2024 13:18 — with GitHub Actions Waiting

rix0rrr added 2 commits November 27, 2024 14:18

opos

14de19c

Remove unused file

f3bdd3f

rix0rrr requested a deployment to test-pipeline November 27, 2024 13:20 — with GitHub Actions Waiting

Add missing file

7fc7c9a

rix0rrr requested a deployment to test-pipeline November 27, 2024 13:30 — with GitHub Actions Waiting

otaviomacedo reviewed Nov 28, 2024

View reviewed changes

rix0rrr added 2 commits November 28, 2024 15:09

Merge remote-tracking branch 'origin/main' into huijbers/cli-logging

d2c3b45

Add tests for the logger

146c489

rix0rrr requested a review from otaviomacedo November 28, 2024 14:28

rix0rrr added the pr-linter/cli-integ-tested Assert that any CLI changes have been integ tested label Nov 28, 2024

rix0rrr requested a deployment to test-pipeline November 28, 2024 14:29 — with GitHub Actions Waiting

aws-cdk-automation added pr/needs-maintainer-review This PR needs a review from a Core Team Member and removed pr/needs-cli-test-run This PR needs CLI tests run against it. labels Nov 28, 2024

otaviomacedo approved these changes Nov 28, 2024

View reviewed changes

aws-cdk-automation removed the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Nov 28, 2024

Merge branch 'main' into huijbers/cli-logging

d94db0a

mergify bot requested a deployment to test-pipeline November 28, 2024 17:28 Waiting

mergify bot merged commit d2e0b29 into main Nov 28, 2024
16 of 17 checks passed

mergify bot deleted the huijbers/cli-logging branch November 28, 2024 17:58

github-actions bot locked as resolved and limited conversation to collaborators Nov 28, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

rix0rrr commented Nov 27, 2024

rix0rrr commented Nov 27, 2024

aws-cdk-automation left a comment

codecov bot commented Nov 27, 2024 •

edited

Loading

otaviomacedo Nov 28, 2024 •

edited

Loading

otaviomacedo Nov 28, 2024

rix0rrr Nov 28, 2024

rix0rrr Nov 28, 2024

otaviomacedo Nov 28, 2024

rix0rrr Nov 28, 2024

mergify bot commented Nov 28, 2024

mergify bot commented Nov 28, 2024

aws-cdk-automation commented Nov 28, 2024

mergify bot commented Nov 28, 2024

github-actions bot commented Nov 28, 2024

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

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

Conversation

rix0rrr commented Nov 27, 2024

rix0rrr commented Nov 27, 2024

aws-cdk-automation left a comment

Choose a reason for hiding this comment

codecov bot commented Nov 27, 2024 • edited Loading

Codecov Report

otaviomacedo Nov 28, 2024 • edited Loading

Choose a reason for hiding this comment

otaviomacedo Nov 28, 2024

Choose a reason for hiding this comment

rix0rrr Nov 28, 2024

Choose a reason for hiding this comment

rix0rrr Nov 28, 2024

Choose a reason for hiding this comment

otaviomacedo Nov 28, 2024

Choose a reason for hiding this comment

rix0rrr Nov 28, 2024

Choose a reason for hiding this comment

mergify bot commented Nov 28, 2024

mergify bot commented Nov 28, 2024

aws-cdk-automation commented Nov 28, 2024

AWS CodeBuild CI Report

mergify bot commented Nov 28, 2024

github-actions bot commented Nov 28, 2024

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

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

codecov bot commented Nov 27, 2024 •

edited

Loading

otaviomacedo Nov 28, 2024 •

edited

Loading