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): use CliIoHost as logger everywhere #32708

Open
wants to merge 17 commits into
base: main
Choose a base branch
from
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@
import type { CloudFormationStackArtifact } from '@aws-cdk/cx-api';
import * as chalk from 'chalk';
import { ResourceEvent, StackEventPoller } from './stack-event-poller';
import { error, LogLevel, setLogLevel } from '../../../logging';
import { error, setIoMessageThreshold } from '../../../logging';
import { IoMessageLevel } from '../../../toolkit/cli-io-host';
import type { ICloudFormationClient } from '../../aws-auth';
import { RewritableBlock } from '../display';

Expand Down Expand Up @@ -48,7 +49,7 @@
*
* @default - Use value from logging.logLevel
*/
readonly logLevel?: LogLevel;
readonly logLevel?: IoMessageLevel;

/**
* Whether to display all stack events or to display only the events for the
Expand Down Expand Up @@ -102,7 +103,7 @@
};

const isWindows = process.platform === 'win32';
const verbose = options.logLevel ?? LogLevel.INFO;
const verbose = options.logLevel ?? 'info';
// On some CI systems (such as CircleCI) output still reports as a TTY so we also
// need an individual check for whether we're running on CI.
// see: https://discuss.circleci.com/t/circleci-terminal-is-a-tty-but-term-is-not-set/9965
Expand Down Expand Up @@ -195,7 +196,7 @@

this.printer.print();
} catch (e) {
error('Error occurred while monitoring stack: %s', e);
error('Error occurred while monitoring stack: %s', String(e));

Check warning on line 199 in packages/aws-cdk/lib/api/util/cloudformation/stack-activity-monitor.ts

View check run for this annotation

Codecov / codecov/patch

packages/aws-cdk/lib/api/util/cloudformation/stack-activity-monitor.ts#L199

Added line #L199 was not covered by tests
}
this.scheduleNextTick();
}
Expand Down Expand Up @@ -626,7 +627,7 @@
*/
public readonly updateSleep: number = 2_000;

private oldLogLevel: LogLevel = LogLevel.INFO;
private oldLogLevel: IoMessageLevel = 'info';

Check warning on line 630 in packages/aws-cdk/lib/api/util/cloudformation/stack-activity-monitor.ts

View check run for this annotation

Codecov / codecov/patch

packages/aws-cdk/lib/api/util/cloudformation/stack-activity-monitor.ts#L630

Added line #L630 was not covered by tests
private block = new RewritableBlock(this.stream);

constructor(props: PrinterProps) {
Expand Down Expand Up @@ -674,11 +675,11 @@
public start() {
// Need to prevent the waiter from printing 'stack not stable' every 5 seconds, it messes
// with the output calculations.
setLogLevel(LogLevel.INFO);
setIoMessageThreshold('info');

Check warning on line 678 in packages/aws-cdk/lib/api/util/cloudformation/stack-activity-monitor.ts

View check run for this annotation

Codecov / codecov/patch

packages/aws-cdk/lib/api/util/cloudformation/stack-activity-monitor.ts#L678

Added line #L678 was not covered by tests
}

public stop() {
setLogLevel(this.oldLogLevel);
setIoMessageThreshold(this.oldLogLevel);

Check warning on line 682 in packages/aws-cdk/lib/api/util/cloudformation/stack-activity-monitor.ts

View check run for this annotation

Codecov / codecov/patch

packages/aws-cdk/lib/api/util/cloudformation/stack-activity-monitor.ts#L682

Added line #L682 was not covered by tests

// Print failures at the end
const lines = new Array<string>();
Expand Down
14 changes: 7 additions & 7 deletions packages/aws-cdk/lib/cdk-toolkit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@ export class CdkToolkit {
options.ignoreNoStacks,
);
const elapsedSynthTime = new Date().getTime() - startSynthTime;
print('\n✨ Synthesis time: %ss\n', formatTime(elapsedSynthTime));
print(`\n✨ Synthesis time: ${formatTime(elapsedSynthTime)}s\n`);

if (stackCollection.stackCount === 0) {
// eslint-disable-next-line no-console
Expand Down Expand Up @@ -387,7 +387,7 @@ export class CdkToolkit {
}

const stackIndex = stacks.indexOf(stack) + 1;
print('%s: deploying... [%s/%s]', chalk.bold(stack.displayName), stackIndex, stackCollection.stackCount);
print(`${chalk.bold(stack.displayName)}: deploying... [${stackIndex}/${stackCollection.stackCount}]`);
const startDeployTime = new Date().getTime();

let tags = options.tags;
Expand Down Expand Up @@ -491,7 +491,7 @@ export class CdkToolkit {

success('\n' + message, stack.displayName);
elapsedDeployTime = new Date().getTime() - startDeployTime;
print('\n✨ Deployment time: %ss\n', formatTime(elapsedDeployTime));
print(`\n✨ Deployment time: ${formatTime(elapsedDeployTime)}s\n`);

if (Object.keys(deployResult.outputs).length > 0) {
print('Outputs:');
Expand All @@ -501,7 +501,7 @@ export class CdkToolkit {

for (const name of Object.keys(deployResult.outputs).sort()) {
const value = deployResult.outputs[name];
print('%s.%s = %s', chalk.cyan(stack.id), chalk.cyan(name), chalk.underline(chalk.cyan(value)));
print(`${chalk.cyan(stack.id)}.${chalk.cyan(name)} = ${chalk.underline(chalk.cyan(value))}`);
kaizencc marked this conversation as resolved.
Show resolved Hide resolved
}

print('Stack ARN:');
Expand Down Expand Up @@ -533,7 +533,7 @@ export class CdkToolkit {
});
}
}
print('\n✨ Total time: %ss\n', formatTime(elapsedSynthTime + elapsedDeployTime));
print(`\n✨ Total time: ${formatTime(elapsedSynthTime + elapsedDeployTime)}s\n`);
};

const assetBuildTime = options.assetBuildTime ?? AssetBuildTime.ALL_BEFORE_DEPLOY;
Expand Down Expand Up @@ -575,7 +575,7 @@ export class CdkToolkit {
const startSynthTime = new Date().getTime();
const stackCollection = await this.selectStacksForDeploy(options.selector, true);
const elapsedSynthTime = new Date().getTime() - startSynthTime;
print('\n✨ Synthesis time: %ss\n', formatTime(elapsedSynthTime));
print(`\n✨ Synthesis time: ${formatTime(elapsedSynthTime)}s\n`);

if (stackCollection.stackCount === 0) {
// eslint-disable-next-line no-console
Expand All @@ -601,7 +601,7 @@ export class CdkToolkit {
anyRollbackable = true;
}
const elapsedRollbackTime = new Date().getTime() - startRollbackTime;
print('\n✨ Rollback time: %ss\n', formatTime(elapsedRollbackTime));
print('\n✨ Rollback time: %ss\n', formatTime(elapsedRollbackTime).toString());
Copy link
Contributor

Choose a reason for hiding this comment

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

If you're going away from the %ss, which i support, we should do that here too

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually people can still do %s. This is actually just a good catch that I don't need to do .toString anymore. It was previously more important.

} catch (e: any) {
error('\n ❌ %s failed: %s', chalk.bold(stack.displayName), e.message);
throw new ToolkitError('Rollback failed (use --force to orphan failing resources)');
Expand Down
11 changes: 6 additions & 5 deletions packages/aws-cdk/lib/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { HotswapMode } from './api/hotswap/common';
import { ILock } from './api/util/rwlock';
import { parseCommandLineArguments } from './parse-command-line-arguments';
import { checkForPlatformWarnings } from './platform-warnings';
import { IoMessageLevel } from './toolkit/cli-io-host';
import { enableTracing } from './util/tracing';
import { SdkProvider } from '../lib/api/aws-auth';
import { BootstrapSource, Bootstrapper } from '../lib/api/bootstrap';
Expand All @@ -22,7 +23,7 @@ import { docs } from '../lib/commands/docs';
import { doctor } from '../lib/commands/doctor';
import { getMigrateScanType } from '../lib/commands/migrate';
import { cliInit, printAvailableTemplates } from '../lib/init';
import { data, debug, error, print, setCI, setLogLevel, LogLevel } from '../lib/logging';
import { data, debug, error, print, setCI, setIoMessageThreshold } from '../lib/logging';
import { Notices } from '../lib/notices';
import { Command, Configuration, Settings } from '../lib/settings';
import * as version from '../lib/version';
Expand All @@ -43,17 +44,17 @@ export async function exec(args: string[], synthesizer?: Synthesizer): Promise<n
// if one -v, log at a DEBUG level
// if 2 -v, log at a TRACE level
if (argv.verbose) {
let logLevel: LogLevel;
let ioMessageLevel: IoMessageLevel;
switch (argv.verbose) {
case 1:
logLevel = LogLevel.DEBUG;
ioMessageLevel = 'debug';
break;
case 2:
default:
logLevel = LogLevel.TRACE;
ioMessageLevel = 'trace';
break;
}
setLogLevel(logLevel);
setIoMessageThreshold(ioMessageLevel);
}

// Debug should always imply tracing
Expand Down
Loading
Loading