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

feat: add support for function logs streaming to sandbox #1492

Open
wants to merge 25 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changeset/cool-coins-add.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'@aws-amplify/cli-core': minor
'@aws-amplify/sandbox': minor
'@aws-amplify/backend-cli': minor
---

feat: add support for function logs streaming to sandbox
1,841 changes: 1,016 additions & 825 deletions package-lock.json

Large diffs are not rendered by default.

22 changes: 21 additions & 1 deletion packages/cli-core/API.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,10 @@

/// <reference types="node" />

import { Colorize } from 'kleur/colors';
import { PackageManagerController } from '@aws-amplify/plugin-types';
import { print } from 'kleur/colors';
import { WriteStream } from 'node:tty';

// @public
export class AmplifyPrompter {
Expand All @@ -21,12 +24,29 @@ export class AmplifyPrompter {
}) => Promise<boolean>;
}

// @public (undocumented)
export type Color = Colorize;

// @public (undocumented)
export type ColorName = keyof typeof colors;

// @public (undocumented)
export const colors: {
green: print;
yellow: print;
blue: print;
magenta: print;
cyan: print;
};

// @public
export class Format {
constructor(packageManagerRunnerName?: string);
// (undocumented)
bold: (message: string) => string;
// (undocumented)
color: (message: string, color: Color) => string;
// (undocumented)
command: (command: string) => string;
// (undocumented)
dim: (message: string) => string;
Expand Down Expand Up @@ -73,7 +93,7 @@ export class PackageManagerControllerFactory {

// @public
export class Printer {
constructor(minimumLogLevel: LogLevel, stdout?: NodeJS.WriteStream, stderr?: NodeJS.WriteStream, refreshRate?: number);
constructor(minimumLogLevel: LogLevel, stdout?: WriteStream | NodeJS.WritableStream, stderr?: WriteStream | NodeJS.WritableStream, refreshRate?: number);
indicateProgress(message: string, callback: () => Promise<void>): Promise<void>;
log(message: string, level?: LogLevel): void;
print: (message: string) => void;
Expand Down
4 changes: 4 additions & 0 deletions packages/cli-core/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,11 @@
"dependencies": {
"@aws-amplify/platform-core": "^1.0.0",
"@inquirer/prompts": "^3.0.0",
"color-support": "^1.1.3",
"execa": "^8.0.1",
"kleur": "^4.1.5"
},
"devDependencies": {
"@types/color-support": "^1.1.4"
}
}
18 changes: 16 additions & 2 deletions packages/cli-core/src/format/format.test.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
import * as os from 'node:os';
import * as assert from 'node:assert';
import { after, before, describe, it } from 'node:test';
import { Format, format } from './format.js';
import { after, before, beforeEach, describe, it } from 'node:test';
import { Format, colors, format } from './format.js';
import { $, blue, bold, cyan, green, red, underline } from 'kleur/colors';

void describe('format', () => {
beforeEach(() => {
$.enabled = true;
});

void it('should format ampx command with yarn', { concurrency: 1 }, () => {
const formatter = new Format('yarn');
assert.strictEqual(
Expand Down Expand Up @@ -157,3 +161,13 @@ void describe('format when terminal colors disabled', async () => {
assert.strictEqual(coloredMessage, 'npx ampx hello');
});
});

void describe('format.color', async () => {
void it('should format colors as requested', () => {
const input = 'something to color';
const expectedOutput = green(input);
const actualOutput = format.color(input, colors.green);
assert.strictEqual(actualOutput, expectedOutput);
assert.notStrictEqual(actualOutput, red(input)); // confirm that coloring actually works
});
});
23 changes: 22 additions & 1 deletion packages/cli-core/src/format/format.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,19 @@
import * as os from 'node:os';
import {
$,
Colorize,
blue,
bold,
cyan,
dim,
green,
grey,
magenta,
red,
underline,
yellow,
} from 'kleur/colors';
import supportColors from 'color-support';
import { AmplifyFault } from '@aws-amplify/platform-core';
import { getPackageManagerRunnerName } from '../package-manager-controller/get_package_manager_name.js';

Expand All @@ -21,7 +26,11 @@ export class Format {
*/
constructor(
private readonly packageManagerRunnerName = getPackageManagerRunnerName()
) {}
) {
if (!supportColors()) {
$.enabled = false;
}
}
normalizeAmpxCommand = (command: string) => {
if (command.length === 0) {
throw new AmplifyFault('InvalidFormatFault', {
Expand Down Expand Up @@ -71,6 +80,18 @@ export class Format {
Object.entries(record)
.map(([key, value]) => `${key}: ${String(value)}`)
.join(os.EOL);
color = (message: string, color: Color) => color(message);
}

export type Color = Colorize;
export type ColorName = keyof typeof colors;

export const colors = {
Amplifiyer marked this conversation as resolved.
Show resolved Hide resolved
green: green,
yellow: yellow,
blue: blue,
magenta: magenta,
cyan: cyan,
};

export const format = new Format();
2 changes: 1 addition & 1 deletion packages/cli-core/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
export * from './prompter/amplify_prompts.js';
export * from './printer/printer.js';
export * from './printer.js';
export { format, Format } from './format/format.js';
export { Color, ColorName, colors, format, Format } from './format/format.js';
export * from './package-manager-controller/package_manager_controller_factory.js';
20 changes: 15 additions & 5 deletions packages/cli-core/src/printer/printer.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { WriteStream } from 'node:tty';
import { EOL } from 'os';

import { $ } from 'kleur/colors';
export type RecordValue = string | number | string[] | Date;

/**
Expand All @@ -19,10 +20,19 @@ export class Printer {
*/
constructor(
private readonly minimumLogLevel: LogLevel,
private readonly stdout: NodeJS.WriteStream = process.stdout,
private readonly stderr: NodeJS.WriteStream = process.stderr,
private readonly stdout:
| WriteStream
| NodeJS.WritableStream = process.stdout,
private readonly stderr:
| WriteStream
| NodeJS.WritableStream = process.stderr,
private readonly refreshRate: number = 500
) {}
) {
// if not running in tty, turn off colors
Copy link
Member

Choose a reason for hiding this comment

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

why is this class manipulating colors?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since this is a central place for performing logging, it's better to turn off/on colors here rather than at the consumers place. Especially since this $ is a global variable, the colors will only change when one consumer changes it making the colors inconsistent.

Copy link
Member

Choose a reason for hiding this comment

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

Aren't the methods in format already a noop if colors are not enabled? Why do we need additional logic for it here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

format uses supportColors which only looks at the environment/terminal you are running in and decides whether to enable or disable colors. E.g. in CI/CD the colors might be disabled. See https://github.com/isaacs/color-support

In the printer, we might be piping the data in some other place as well (e.g. writing to a file) supportColors doesn't know that and will keep the colors enabled.

Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure this is right.

If $.enabled is global setting but there could be two Printer instances - one writing to console and the other to file - then how do we decide if colors should be enabled or not.

So either color control should not be in this class
OR color control should be implemented differently - for example given that node is single threaded try-finally blocks at each printX call could be used.

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 removed it from printer such that printer will print whatever is given to it. Instead of fiddling with kleur's $ the caller can then choose to not use format or colors.

if (!this.isTTY()) {
$.enabled = false;
}
}

/**
* Prints a given message to output stream followed by a newline.
Expand Down Expand Up @@ -91,7 +101,7 @@ export class Printer {
* Checks if the environment is TTY
*/
private isTTY() {
return this.stdout.isTTY;
return this.stdout instanceof WriteStream && this.stdout.isTTY;
}

/**
Expand Down
37 changes: 35 additions & 2 deletions packages/cli/src/commands/sandbox/sandbox_command.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,11 @@ import { AmplifyPrompter, format, printer } from '@aws-amplify/cli-core';
import { EventHandler, SandboxCommand } from './sandbox_command.js';
import { createSandboxCommand } from './sandbox_command_factory.js';
import { SandboxDeleteCommand } from './sandbox-delete/sandbox_delete_command.js';
import { Sandbox, SandboxSingletonFactory } from '@aws-amplify/sandbox';
import {
Sandbox,
SandboxFunctionStreamingOptions,
SandboxSingletonFactory,
} from '@aws-amplify/sandbox';
import { createSandboxSecretCommand } from './sandbox-secret/sandbox_secret_command_factory.js';
import { ClientConfigGeneratorAdapter } from '../../client-config/client_config_generator_adapter.js';
import { CommandMiddleware } from '../../command_middleware.js';
Expand Down Expand Up @@ -118,6 +122,9 @@ void describe('sandbox command', () => {
assert.match(output, /--outputs-format/);
assert.match(output, /--outputs-out-dir/);
assert.match(output, /--once/);
assert.match(output, /--stream-function-logs/);
assert.match(output, /--function-name/);
assert.match(output, /--stream-output/);
assert.equal(mockHandleProfile.mock.callCount(), 0);
});

Expand Down Expand Up @@ -341,7 +348,7 @@ void describe('sandbox command', () => {
);
});

void it('--once flag is mutually exclusive with dir-to-watch & exclude', async () => {
void it('--once flag is mutually exclusive with dir-to-watch, exclude and stream-function-logs', async () => {
assert.match(
await commandRunner.runCommand(
'sandbox --once --dir-to-watch nonExistentDir'
Expand All @@ -352,5 +359,31 @@ void describe('sandbox command', () => {
await commandRunner.runCommand('sandbox --once --exclude test'),
/Arguments once and exclude are mutually exclusive/
);
assert.match(
await commandRunner.runCommand('sandbox --once --stream-function-logs'),
/Arguments once and stream-function-logs are mutually exclusive/
);
});

void it('fails if --stream-output is provided without enabling --stream-function-logs', async () => {
assert.match(
await commandRunner.runCommand('sandbox --stream-output someFile'),
/Missing dependent arguments:\n stream-output -> stream-function-logs/
);
});

void it('starts sandbox with log watching options', async () => {
await commandRunner.runCommand(
'sandbox --stream-function-logs --function-name func1 --function-name func2 --stream-output someFile'
);
assert.equal(sandboxStartMock.mock.callCount(), 1);
assert.deepStrictEqual(
sandboxStartMock.mock.calls[0].arguments[0].functionStreamingOptions,
{
enabled: true,
functionNames: ['func1', 'func2'],
streamOutputLocation: 'someFile',
} as SandboxFunctionStreamingOptions
);
});
});
58 changes: 56 additions & 2 deletions packages/cli/src/commands/sandbox/sandbox_command.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@ import { ArgumentsCamelCase, Argv, CommandModule } from 'yargs';
import fs from 'fs';
import fsp from 'fs/promises';
import { AmplifyPrompter } from '@aws-amplify/cli-core';
import { SandboxSingletonFactory } from '@aws-amplify/sandbox';
import {
SandboxFunctionStreamingOptions,
SandboxSingletonFactory,
} from '@aws-amplify/sandbox';
import {
ClientConfigFormat,
ClientConfigVersion,
Expand All @@ -26,6 +29,9 @@ export type SandboxCommandOptionsKebabCase = ArgumentsKebabCase<
outputsOutDir: string | undefined;
outputsVersion: string;
once: boolean | undefined;
streamFunctionLogs: boolean | undefined;
functionName: string[] | undefined;
streamOutput: string | undefined;
} & SandboxCommandGlobalOptions
>;

Expand Down Expand Up @@ -123,12 +129,29 @@ export class SandboxCommand
}

watchExclusions.push(clientConfigWritePath);

let functionStreamingOptions: SandboxFunctionStreamingOptions = {
enabled: false,
};
if (args.streamFunctionLogs) {
// turn on function logs streaming
functionStreamingOptions = {
enabled: true,
functionNames: args.functionName,
streamOutputLocation: args.streamOutput,
};

if (args.streamOutput) {
watchExclusions.push(args.streamOutput);
}
}
await sandbox.start({
dir: args.dirToWatch,
exclude: watchExclusions,
identifier: args.identifier,
profile: args.profile,
watchForChanges: !args.once,
functionStreamingOptions,
});
process.once('SIGINT', () => void this.sigIntHandler());
};
Expand Down Expand Up @@ -196,6 +219,31 @@ export class SandboxCommand
boolean: true,
global: false,
})
.option('stream-function-logs', {
describe:
'Whether to stream function execution logs. Default: false. Use --function-names to filter for specific functions to stream logs from',
Amplifiyer marked this conversation as resolved.
Show resolved Hide resolved
boolean: true,
global: false,
group: 'Stream lambda function logs',
})
.option('function-name', {
describe:
'Name of functions to stream execution logs from. Usage --function-name func1 --function-name func2. Default: Stream all functions logs',
array: true,
type: 'string',
group: 'Stream lambda function logs',
implies: ['stream-function-logs'],
requiresArg: true,
})
.option('stream-output', {
Amplifiyer marked this conversation as resolved.
Show resolved Hide resolved
describe:
'File to append the function execution logs. The file is created if it does not exist. Default: stdout',
array: false,
type: 'string',
group: 'Stream lambda function logs',
implies: ['stream-function-logs'],
requiresArg: true,
})
.check(async (argv) => {
if (argv['dir-to-watch']) {
await this.validateDirectory('dir-to-watch', argv['dir-to-watch']);
Expand All @@ -210,7 +258,13 @@ export class SandboxCommand
}
return true;
})
.conflicts('once', ['exclude', 'dir-to-watch'])
.conflicts('once', [
'exclude',
'dir-to-watch',
'stream-function-logs',
'function-name',
'stream-output',
])
.middleware([this.commandMiddleware.ensureAwsCredentialAndRegion])
);
};
Expand Down
8 changes: 8 additions & 0 deletions packages/sandbox/API.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,13 @@ export type SandboxDeleteOptions = {
// @public (undocumented)
export type SandboxEvents = 'successfulDeployment' | 'failedDeployment' | 'successfulDeletion';

// @public (undocumented)
export type SandboxFunctionStreamingOptions = {
enabled: boolean;
functionNames?: string[];
streamOutputLocation?: string;
};

// @public (undocumented)
export type SandboxOptions = {
dir?: string;
Expand All @@ -38,6 +45,7 @@ export type SandboxOptions = {
format?: ClientConfigFormat;
profile?: string;
watchForChanges?: boolean;
functionStreamingOptions?: SandboxFunctionStreamingOptions;
};

// @public
Expand Down