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 all 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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions .changeset/cool-coins-add.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
'@aws-amplify/cli-core': minor
'@aws-amplify/sandbox': minor
'@aws-amplify/backend-cli': minor
'create-amplify': patch
---

feat: add support for function logs streaming to sandbox
7,589 changes: 5,400 additions & 2,189 deletions package-lock.json

Large diffs are not rendered by default.

11 changes: 10 additions & 1 deletion packages/cli-core/API.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
/// <reference types="node" />

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

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

// @public (undocumented)
export type ColorName = (typeof colorNames)[number];

// @public (undocumented)
export const colorNames: readonly ["Green", "Yellow", "Blue", "Magenta", "Cyan"];

// @public
export class Format {
constructor(packageManagerRunnerName?: string);
// (undocumented)
bold: (message: string) => string;
// (undocumented)
color: (message: string, colorName: ColorName) => string;
// (undocumented)
command: (command: string) => string;
// (undocumented)
dim: (message: string) => string;
Expand Down Expand Up @@ -73,7 +82,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"
}
}
16 changes: 15 additions & 1 deletion 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 { after, before, beforeEach, describe, it } from 'node:test';
import { Format, 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, 'Green');
assert.strictEqual(actualOutput, expectedOutput);
assert.notStrictEqual(actualOutput, red(input)); // confirm that coloring actually works
});
});
31 changes: 30 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,26 @@ export class Format {
Object.entries(record)
.map(([key, value]) => `${key}: ${String(value)}`)
.join(os.EOL);
color = (message: string, colorName: ColorName) => colors[colorName](message);
}

// Map to connect colorName to kleur color
const colors: Record<ColorName, Colorize> = {
Green: green,
Yellow: yellow,
Blue: blue,
Magenta: magenta,
Cyan: cyan,
};

export const colorNames = [
'Green',
'Yellow',
'Blue',
'Magenta',
'Cyan',
edwardfoyle marked this conversation as resolved.
Show resolved Hide resolved
] as const;

export type ColorName = (typeof colorNames)[number];

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 { ColorName, colorNames, format, Format } from './format/format.js';
export * from './package-manager-controller/package_manager_controller_factory.js';
12 changes: 8 additions & 4 deletions packages/cli-core/src/printer/printer.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { WriteStream } from 'node:tty';
import { EOL } from 'os';

export type RecordValue = string | number | string[] | Date;

/**
Expand All @@ -19,8 +19,12 @@ 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
) {}

Expand Down Expand Up @@ -91,7 +95,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, /--logs-filter/);
assert.match(output, /--logs-out-file/);
sobolk marked this conversation as resolved.
Show resolved Hide resolved
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 --logs-out-file is provided without enabling --stream-function-logs', async () => {
assert.match(
await commandRunner.runCommand('sandbox --logs-out-file someFile'),
/Missing dependent arguments:\n logs-out-file -> stream-function-logs/
);
});

void it('starts sandbox with log watching options', async () => {
await commandRunner.runCommand(
'sandbox --stream-function-logs --logs-filter func1 --logs-filter func2 --logs-out-file someFile'
);
assert.equal(sandboxStartMock.mock.callCount(), 1);
assert.deepStrictEqual(
sandboxStartMock.mock.calls[0].arguments[0].functionStreamingOptions,
{
enabled: true,
logsFilters: ['func1', 'func2'],
logsOutFile: 'someFile',
} as SandboxFunctionStreamingOptions
);
});
});
57 changes: 55 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;
logsFilter: string[] | undefined;
logsOutFile: 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,
logsFilters: args.logsFilter,
logsOutFile: args.logsOutFile,
};

if (args.logsOutFile) {
watchExclusions.push(args.logsOutFile);
}
}
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,30 @@ export class SandboxCommand
boolean: true,
global: false,
})
.option('stream-function-logs', {
describe:
'Whether to stream function execution logs. Default: false. Use --logs-filter in addition to this flag to stream specific function logs',
boolean: true,
global: false,
group: 'Logs streaming',
})
.option('logs-filter', {
describe: `Regex pattern to filter logs from only matched functions. E.g. to stream logs for a function, specify it's name, and to stream logs from all functions starting with auth specify 'auth' Default: Stream all logs`,
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't a regex for "functions starting with auth" be ^auth.*? Also, why are we supporting regex and array input? You can make a regex to support multiple disjoint strings with (foo|bar|baz)

IMO if we are going to support regex, it should just be a single arg

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. For a full regex match yes, but I'm going for a partial string match as in my opinion it's easier to specify and also a default for pretty much all regex matchers. I don't see why we would prevent partial matching.
  2. Similarly providing array just makes it easier to provide input instead of creating complex regex pattern. @josefaidt to provide more input here.

Copy link
Contributor

Choose a reason for hiding this comment

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

👋 yes the important piece of regex is the wildcard match (e.g. auth* picks up all functions named with auth like auth-post-confirmation and auth-pre-signup) but it is much easier to specify multiple patterns than it is to craft complex regex when writing the command

--logs-filter auth* --logs-filter resolver*

I think regex can be a bit of a hurdle though compared to a glob pattern

Copy link
Member

Choose a reason for hiding this comment

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

Do we want to support glob patterns or regex then? The current impl is using regex which would require --logs-filter auth.* logs-filter resolver.*

Copy link
Contributor Author

Choose a reason for hiding this comment

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

glob patterns don't make sense for non pathlike strings. Globs are typically (or only?) used for pathname expansions.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah fair enough. Just want to make sure we're aligned that customers will have to specify auth.*, not auth*

Copy link
Contributor

Choose a reason for hiding this comment

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

There are use cases for glob patterns outside filenames
https://www.sqlitetutorial.net/sqlite-glob/
https://duckdb.org/docs/sql/functions/pattern_matching.html#glob

like minimatch without the globstar

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Minimatch is basically a glob implementation, still only relevant for pathlike string matching, not for any arbitrary matches.

In the SQL world, the glob is basically implemented as a regex, e.g. there is no such thing as **.

array: true,
type: 'string',
group: 'Logs streaming',
implies: ['stream-function-logs'],
requiresArg: true,
})
.option('logs-out-file', {
describe:
'File to append the streaming logs. The file is created if it does not exist. Default: stdout',
array: false,
type: 'string',
group: 'Logs streaming',
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 +257,13 @@ export class SandboxCommand
}
return true;
})
.conflicts('once', ['exclude', 'dir-to-watch'])
.conflicts('once', [
'exclude',
'dir-to-watch',
'stream-function-logs',
'logs-filter',
'logs-out-file',
])
sobolk marked this conversation as resolved.
Show resolved Hide resolved
.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;
logsFilters?: string[];
logsOutFile?: 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
5 changes: 4 additions & 1 deletion packages/sandbox/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,12 @@
"@aws-amplify/deployed-backend-client": "^1.0.2",
"@aws-amplify/platform-core": "^1.0.1",
"@aws-sdk/client-cloudformation": "^3.465.0",
"@aws-sdk/credential-providers": "^3.465.0",
"@aws-sdk/client-cloudwatch-logs": "^3.465.0",
"@aws-sdk/client-lambda": "^3.465.0",
"@aws-sdk/client-ssm": "^3.465.0",
"@aws-sdk/credential-providers": "^3.465.0",
"@aws-sdk/types": "^3.465.0",
"@aws-sdk/util-arn-parser": "^3.465.0",
"@parcel/watcher": "^2.4.1",
"debounce-promise": "^3.1.2",
"glob": "^10.2.7",
Expand Down
Loading
Loading