Skip to content

Functions logging #2245

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.

Sign up for GitHub

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

Merged
merged 17 commits into from
Nov 27, 2024
Merged

Functions logging #2245

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
17056f0
Functions logging
sobolk Nov 19, 2024
0cfba52
changeset
sobolk Nov 19, 2024
367ae90
more
sobolk Nov 19, 2024
49fe924
conversation handler
sobolk Nov 19, 2024
20615b0
conversation handler
sobolk Nov 19, 2024
3a90b89
Merge remote-tracking branch 'origin/main' into functions-logging
sobolk Nov 25, 2024
63f89d6
this works
sobolk Nov 26, 2024
1699eed
more tests
sobolk Nov 27, 2024
add274e
more tests
sobolk Nov 27, 2024
05291c7
more tests
sobolk Nov 27, 2024
b181192
Merge remote-tracking branch 'origin/main' into functions-logging
sobolk Nov 27, 2024
f735ed8
fix build
sobolk Nov 27, 2024
f56b52a
fix build
sobolk Nov 27, 2024
8bf43ce
pr feedback
sobolk Nov 27, 2024
0e3659e
pr feedback
sobolk Nov 27, 2024
6d6c3dd
api
sobolk Nov 27, 2024
23a7409
pr feedback
sobolk Nov 27, 2024
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
9 changes: 9 additions & 0 deletions .changeset/poor-moons-refuse.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
'@aws-amplify/ai-constructs': minor
'@aws-amplify/backend-ai': minor
'@aws-amplify/backend-function': minor
'@aws-amplify/backend': minor
'@aws-amplify/platform-core': minor
---

Add options to control log settings
4 changes: 4 additions & 0 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 6 additions & 0 deletions packages/ai-constructs/API.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,14 @@
/// <reference types="node" />

import { AIConversationOutput } from '@aws-amplify/backend-output-schemas';
import { ApplicationLogLevel } from 'aws-cdk-lib/aws-lambda';
import { BackendOutputStorageStrategy } from '@aws-amplify/plugin-types';
import * as bedrock from '@aws-sdk/client-bedrock-runtime';
import { Construct } from 'constructs';
import { FunctionResources } from '@aws-amplify/plugin-types';
import * as jsonSchemaToTypeScript from 'json-schema-to-ts';
import { ResourceProvider } from '@aws-amplify/plugin-types';
import { RetentionDays } from 'aws-cdk-lib/aws-logs';

declare namespace __export__conversation {
export {
Expand Down Expand Up @@ -55,6 +57,10 @@ type ConversationHandlerFunctionProps = {
region?: string;
}>;
memoryMB?: number;
logging?: {
level?: ApplicationLogLevel;
retention?: RetentionDays;
};
outputStorageStrategy?: BackendOutputStorageStrategy<AIConversationOutput>;
};

Expand Down
39 changes: 39 additions & 0 deletions packages/ai-constructs/src/conversation/conversation_handler_construct.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import { ConversationHandlerFunction } from './conversation_handler_construct';
import { Template } from 'aws-cdk-lib/assertions';
import path from 'path';
import { StackMetadataBackendOutputStorageStrategy } from '@aws-amplify/backend-output-storage';
import { ApplicationLogLevel } from 'aws-cdk-lib/aws-lambda';
import { RetentionDays } from 'aws-cdk-lib/aws-logs';

void describe('Conversation Handler Function construct', () => {
void it('creates handler with log group with JWT token redacting policy', () => {
Expand Down Expand Up @@ -284,4 +286,41 @@ void describe('Conversation Handler Function construct', () => {
}, new Error('memoryMB must be a whole number between 128 and 10240 inclusive'));
});
});

void describe('logging options', () => {
void it('sets log level', () => {
const app = new App();
const stack = new Stack(app);
new ConversationHandlerFunction(stack, 'conversationHandler', {
models: [],
logging: {
level: ApplicationLogLevel.DEBUG,
},
});
const template = Template.fromStack(stack);

template.hasResourceProperties('AWS::Lambda::Function', {
LoggingConfig: {
ApplicationLogLevel: 'DEBUG',
LogFormat: 'JSON',
},
});
});

void it('sets log retention', () => {
const app = new App();
const stack = new Stack(app);
new ConversationHandlerFunction(stack, 'conversationHandler', {
models: [],
logging: {
retention: RetentionDays.ONE_YEAR,
},
});
const template = Template.fromStack(stack);

template.hasResourceProperties('AWS::Logs::LogGroup', {
RetentionInDays: 365,
});
});
});
});
10 changes: 9 additions & 1 deletion packages/ai-constructs/src/conversation/conversation_handler_construct.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
import { Duration, Stack, Tags } from 'aws-cdk-lib';
import { Effect, PolicyStatement } from 'aws-cdk-lib/aws-iam';
import {
ApplicationLogLevel,
CfnFunction,
Runtime as LambdaRuntime,
LoggingFormat,
Expand Down Expand Up @@ -40,6 +41,12 @@ export type ConversationHandlerFunctionProps = {
* Default is 512MB.
*/
memoryMB?: number;

logging?: {
level?: ApplicationLogLevel;
retention?: RetentionDays;
};

/**
* @internal
*/
Expand Down Expand Up @@ -100,8 +107,9 @@ export class ConversationHandlerFunction
bundleAwsSDK: !!this.props.entry,
},
loggingFormat: LoggingFormat.JSON,
applicationLogLevelV2: this.props.logging?.level,
logGroup: new LogGroup(this, 'conversationHandlerFunctionLogGroup', {
retention: RetentionDays.INFINITE,
retention: this.props.logging?.retention ?? RetentionDays.INFINITE,
dataProtectionPolicy: new DataProtectionPolicy({
identifiers: [
new CustomDataIdentifier(
Expand Down
18 changes: 18 additions & 0 deletions packages/backend-ai/API.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,17 @@ import { AiModel } from '@aws-amplify/data-schema-types';
import { ConstructFactory } from '@aws-amplify/plugin-types';
import { ConversationTurnEventVersion } from '@aws-amplify/ai-constructs/conversation';
import { FunctionResources } from '@aws-amplify/plugin-types';
import { LogLevel } from '@aws-amplify/plugin-types';
import { LogRetention } from '@aws-amplify/plugin-types';
import { ResourceProvider } from '@aws-amplify/plugin-types';
import * as runtime from '@aws-amplify/ai-constructs/conversation/runtime';

declare namespace __export__conversation {
export {
ConversationHandlerFunctionFactory,
ConversationHandlerFunctionLogLevel,
ConversationHandlerFunctionLogRetention,
ConversationHandlerFunctionLoggingOptions,
DefineConversationHandlerFunctionProps,
defineConversationHandlerFunction
}
Expand All @@ -36,6 +41,18 @@ type ConversationHandlerFunctionFactory = ConstructFactory<ResourceProvider<Func
readonly eventVersion: ConversationTurnEventVersion;
};

// @public (undocumented)
type ConversationHandlerFunctionLoggingOptions = {
retention?: ConversationHandlerFunctionLogRetention;
level?: ConversationHandlerFunctionLogLevel;
};

// @public (undocumented)
type ConversationHandlerFunctionLogLevel = LogLevel;

// @public (undocumented)
type ConversationHandlerFunctionLogRetention = LogRetention;

// @public (undocumented)
type ConversationTurnEvent = runtime.ConversationTurnEvent;

Expand All @@ -54,6 +71,7 @@ type DefineConversationHandlerFunctionProps = {
region?: string;
}>;
memoryMB?: number;
logging?: ConversationHandlerFunctionLoggingOptions;
};

// @public (undocumented)
Expand Down
37 changes: 37 additions & 0 deletions packages/backend-ai/src/conversation/factory.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,4 +203,41 @@ void describe('ConversationHandlerFactory', () => {
MemorySize: 271,
});
});

void it('passes log level to construct', () => {
const factory = defineConversationHandlerFunction({
entry: './test-assets/with-default-entry/handler.ts',
name: 'testHandlerName',
models: [],
logging: {
level: 'debug',
},
});
const lambda = factory.getInstance(getInstanceProps);
const template = Template.fromStack(Stack.of(lambda.resources.lambda));
template.resourceCountIs('AWS::Lambda::Function', 1);
template.hasResourceProperties('AWS::Lambda::Function', {
LoggingConfig: {
ApplicationLogLevel: 'DEBUG',
LogFormat: 'JSON',
},
});
});

void it('passes log retention to construct', () => {
const factory = defineConversationHandlerFunction({
entry: './test-assets/with-default-entry/handler.ts',
name: 'testHandlerName',
models: [],
logging: {
retention: '1 day',
},
});
const lambda = factory.getInstance(getInstanceProps);
const template = Template.fromStack(Stack.of(lambda.resources.lambda));
template.resourceCountIs('AWS::Lambda::Function', 1);
template.hasResourceProperties('AWS::Logs::LogGroup', {
RetentionInDays: 1,
});
});
});
28 changes: 28 additions & 0 deletions packages/backend-ai/src/conversation/factory.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import {
ConstructFactoryGetInstanceProps,
FunctionResources,
GenerateContainerEntryProps,
LogLevel,
LogRetention,
ResourceProvider,
} from '@aws-amplify/plugin-types';
import {
Expand All @@ -17,6 +19,10 @@ import {
import path from 'path';
import { CallerDirectoryExtractor } from '@aws-amplify/platform-core';
import { AiModel } from '@aws-amplify/data-schema-types';
import {
LogLevelConverter,
LogRetentionConverter,
} from '@aws-amplify/platform-core/cdk';

class ConversationHandlerFunctionGenerator
implements ConstructContainerEntryGenerator
Expand Down Expand Up @@ -47,6 +53,18 @@ class ConversationHandlerFunctionGenerator
outputStorageStrategy: this.outputStorageStrategy,
memoryMB: this.props.memoryMB,
};
const logging: typeof constructProps.logging = {};
if (this.props.logging?.level) {
logging.level = new LogLevelConverter().toCDKLambdaApplicationLogLevel(
this.props.logging.level
);
}
if (this.props.logging?.retention) {
logging.retention = new LogRetentionConverter().toCDKRetentionDays(
this.props.logging.retention
);
}
constructProps.logging = logging;
const conversationHandlerFunction = new ConversationHandlerFunction(
scope,
this.props.name,
Expand Down Expand Up @@ -115,6 +133,15 @@ class DefaultConversationHandlerFunctionFactory
};
}

export type ConversationHandlerFunctionLogLevel = LogLevel;

export type ConversationHandlerFunctionLogRetention = LogRetention;

export type ConversationHandlerFunctionLoggingOptions = {
retention?: ConversationHandlerFunctionLogRetention;
level?: ConversationHandlerFunctionLogLevel;
};

export type DefineConversationHandlerFunctionProps = {
name: string;
entry?: string;
Expand All @@ -128,6 +155,7 @@ export type DefineConversationHandlerFunctionProps = {
* Default is 512MB.
*/
memoryMB?: number;
logging?: ConversationHandlerFunctionLoggingOptions;
};

/**
Expand Down
6 changes: 6 additions & 0 deletions packages/backend-ai/src/conversation/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,17 @@
import {
ConversationHandlerFunctionFactory,
ConversationHandlerFunctionLogLevel,
ConversationHandlerFunctionLogRetention,
ConversationHandlerFunctionLoggingOptions,
DefineConversationHandlerFunctionProps,
defineConversationHandlerFunction,
} from './factory.js';

export {
ConversationHandlerFunctionFactory,
ConversationHandlerFunctionLogLevel,
ConversationHandlerFunctionLogRetention,
ConversationHandlerFunctionLoggingOptions,
DefineConversationHandlerFunctionProps,
defineConversationHandlerFunction,
};
19 changes: 19 additions & 0 deletions packages/backend-function/API.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import { AmplifyResourceGroupName } from '@aws-amplify/plugin-types';
import { BackendSecret } from '@aws-amplify/plugin-types';
import { ConstructFactory } from '@aws-amplify/plugin-types';
import { FunctionResources } from '@aws-amplify/plugin-types';
import { LogLevel } from '@aws-amplify/plugin-types';
import { LogRetention } from '@aws-amplify/plugin-types';
import { ResourceAccessAcceptorFactory } from '@aws-amplify/plugin-types';
import { ResourceProvider } from '@aws-amplify/plugin-types';
import { StackProvider } from '@aws-amplify/plugin-types';
Expand All @@ -28,6 +30,22 @@ export type FunctionBundlingOptions = {
minify?: boolean;
};

// @public (undocumented)
export type FunctionLoggingOptions = ({
format: 'json';
level?: FunctionLogLevel;
} | {
format?: 'text';
}) & {
retention?: FunctionLogRetention;
};

// @public (undocumented)
export type FunctionLogLevel = LogLevel;

// @public (undocumented)
export type FunctionLogRetention = LogRetention;

// @public (undocumented)
export type FunctionProps = {
name?: string;
Expand All @@ -40,6 +58,7 @@ export type FunctionProps = {
layers?: Record<string, string>;
bundling?: FunctionBundlingOptions;
resourceGroupName?: AmplifyResourceGroupName;
logging?: FunctionLoggingOptions;
};

// @public (undocumented)
Expand Down
33 changes: 33 additions & 0 deletions packages/backend-function/src/factory.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -440,6 +440,39 @@ void describe('AmplifyFunctionFactory', () => {
});
});

void describe('logging options', () => {
void it('sets logging options', () => {
const lambda = defineFunction({
entry: './test-assets/default-lambda/handler.ts',
bundling: {
minify: false,
},
logging: {
format: 'json',
level: 'warn',
retention: '13 months',
},
}).getInstance(getInstanceProps);
const template = Template.fromStack(lambda.stack);
// Enabling log retention adds extra lambda.
template.resourceCountIs('AWS::Lambda::Function', 2);
const lambdas = template.findResources('AWS::Lambda::Function');
Comment on lines +457 to +459
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible to avoid creating this extra lambda by creating the logGroup ourselves https://repost.aws/questions/QUzCgY0gSIQNGHZc6frGcI7g/log-retention-lambda#ANwKiEC22QQ-uV0r5yi0JdBQ?

Copy link
Member Author

Choose a reason for hiding this comment

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

This has been tried and adopted in conversation handler.
However, it was easier there because it was a new thing.

Here the problem space is constrained in such a way that:

  1. In order to use custom log group (us or customer) we have to pass reference to function. So log group must be created before calling NodejsFunction ctor.
  2. In order to retain log group naming we need function name first. This is not available until NodejsFunction ctor is called, making it circlular dependency problem.
  3. When working on conversation handler I tried to compute function name before creating log group and nodejsfunction but I was not successful at producing function name that would match implicit cdk algorithm. That computation is internalized and I didn't find a way to implement it using available apis from cdk.
  4. Changing function name and/or log group name algorithm at this point would mean destructive changes.

I wanted to avoid destructive changes, so I pursued a solution based on this extra lambda.

I think the proper solution to expose log group knobs would be to add another alternative to LoggingOptions in the future where we {..existing...} | { logGroupKnobs } .

wdyt ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good, at least it's only one extra function for all customer defined lambda functions.

assert.ok(
Object.keys(lambdas).some((key) => key.startsWith('LogRetention'))
);
Comment on lines +460 to +462
Copy link
Contributor

Choose a reason for hiding this comment

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

no way to assert the actual retention period?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll double check.

Copy link
Member Author

Choose a reason for hiding this comment

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

I found something. added below.

template.hasResourceProperties('Custom::LogRetention', {
RetentionInDays: 400,
});
template.hasResourceProperties('AWS::Lambda::Function', {
Handler: 'index.handler',
LoggingConfig: {
ApplicationLogLevel: 'WARN',
LogFormat: 'JSON',
},
});
});
});

void describe('resourceAccessAcceptor', () => {
void it('attaches policy to execution role and configures ssm environment context', () => {
const functionFactory = defineFunction({
Expand Down
Loading