Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 5 additions & 0 deletions .changeset/silver-turkeys-argue.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@aws-amplify/backend-function': patch
---

Update lambda logging configuration to use non-deprecated AWS CDK properties
41 changes: 33 additions & 8 deletions packages/backend-function/src/factory.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -578,7 +578,7 @@ void describe('AmplifyFunctionFactory', () => {
});

void describe('logging options', () => {
void it('sets logging options', () => {
void it('sets logging options with log group', () => {
const lambda = defineFunction({
entry: './test-assets/default-lambda/handler.ts',
bundling: {
Expand All @@ -591,15 +591,14 @@ void describe('AmplifyFunctionFactory', () => {
},
}).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');
assert.ok(
Object.keys(lambdas).some((key) => key.startsWith('LogRetention')),
);
template.hasResourceProperties('Custom::LogRetention', {

// We now create a LogGroup directly rather than using the deprecated logRetention
template.resourceCountIs('AWS::Logs::LogGroup', 1);
template.hasResourceProperties('AWS::Logs::LogGroup', {
RetentionInDays: 400,
});

// The function should use the applicationLogLevelV2 and loggingFormat properties
template.hasResourceProperties('AWS::Lambda::Function', {
Handler: 'index.handler',
LoggingConfig: {
Expand All @@ -608,6 +607,32 @@ void describe('AmplifyFunctionFactory', () => {
},
});
});

void it('sets logging options without retention', () => {
const lambda = defineFunction({
entry: './test-assets/default-lambda/handler.ts',
bundling: {
minify: false,
},
logging: {
format: 'json',
level: 'info',
},
}).getInstance(getInstanceProps);
const template = Template.fromStack(lambda.stack);

// No log group should be created when retention is not specified
template.resourceCountIs('AWS::Logs::LogGroup', 0);

// The function should still have logging config
template.hasResourceProperties('AWS::Lambda::Function', {
Handler: 'index.handler',
LoggingConfig: {
ApplicationLogLevel: 'INFO',
LogFormat: 'JSON',
},
});
});
});

void describe('resourceAccessAcceptor', () => {
Expand Down
14 changes: 12 additions & 2 deletions packages/backend-function/src/factory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,10 @@ import * as path from 'path';
import { FunctionEnvironmentTranslator } from './function_env_translator.js';
import { FunctionEnvironmentTypeGenerator } from './function_env_type_generator.js';
import { FunctionLayerArnParser } from './layer_parser.js';
import { convertLoggingOptionsToCDK } from './logging_options_parser.js';
import {
convertLoggingOptionsToCDK,
createLogGroup,
} from './logging_options_parser.js';
import { convertFunctionSchedulesToRuleSchedules } from './schedule_parser.js';
import {
ProvidedFunctionFactory,
Expand Down Expand Up @@ -583,6 +586,13 @@ class AmplifyFunction

let functionLambda: NodejsFunction;
const cdkLoggingOptions = convertLoggingOptionsToCDK(props.logging);

// Create a log group if retention is specified (replacing deprecated logRetention property)
let logGroup = cdkLoggingOptions.logGroup;
if (props.logging.retention !== undefined && !logGroup) {
logGroup = createLogGroup(scope, id, props.logging);
}

try {
functionLambda = new NodejsFunction(scope, `${id}-lambda`, {
entry: props.entry,
Expand All @@ -599,7 +609,7 @@ class AmplifyFunction
externalModules: Object.keys(props.layers),
logLevel: EsBuildLogLevel.ERROR,
},
logRetention: cdkLoggingOptions.retention,
logGroup: logGroup,
applicationLogLevelV2: cdkLoggingOptions.level,
loggingFormat: cdkLoggingOptions.format,
});
Expand Down
45 changes: 38 additions & 7 deletions packages/backend-function/src/logging_options_parser.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,22 +4,23 @@ import { FunctionLoggingOptions } from './factory.js';
import {
CDKLoggingOptions,
convertLoggingOptionsToCDK,
createLogGroup,
} from './logging_options_parser.js';
import { ApplicationLogLevel, LoggingFormat } from 'aws-cdk-lib/aws-lambda';
import { RetentionDays } from 'aws-cdk-lib/aws-logs';
import { LogGroup } from 'aws-cdk-lib/aws-logs';
import { App, Stack } from 'aws-cdk-lib';

type TestCase = {
type ConversionTestCase = {
input: FunctionLoggingOptions;
expectedOutput: CDKLoggingOptions;
};

const testCases: Array<TestCase> = [
const conversionTestCases: Array<ConversionTestCase> = [
{
input: {},
expectedOutput: {
format: undefined,
level: undefined,
retention: undefined,
},
},
{
Expand All @@ -29,7 +30,6 @@ const testCases: Array<TestCase> = [
},
expectedOutput: {
format: LoggingFormat.TEXT,
retention: RetentionDays.THIRTEEN_MONTHS,
level: undefined,
},
},
Expand All @@ -40,17 +40,48 @@ const testCases: Array<TestCase> = [
},
expectedOutput: {
format: LoggingFormat.JSON,
retention: undefined,
level: ApplicationLogLevel.DEBUG,
},
},
];

void describe('LoggingOptions converter', () => {
testCases.forEach((testCase, index) => {
conversionTestCases.forEach((testCase, index) => {
void it(`converts to cdk options[${index}]`, () => {
const convertedOptions = convertLoggingOptionsToCDK(testCase.input);
assert.deepStrictEqual(convertedOptions, testCase.expectedOutput);
});
});

void it('createLogGroup creates a log group with proper retention', () => {
const app = new App();
const stack = new Stack(app, 'TestStack');
const logGroup = createLogGroup(stack, 'test-function', {
retention: '13 months',
});

// Check that the log group was created
assert.ok(logGroup instanceof LogGroup);

// We can't easily check the CDK properties from the instance
// So we verify the synthesized CloudFormation template
const template = JSON.parse(
JSON.stringify(app.synth().getStackArtifact('TestStack').template),
);

// Find the LogGroup resource
const logGroupResources = Object.values(template.Resources).filter(
// eslint-disable-next-line @typescript-eslint/no-explicit-any
(resource: any) => resource.Type === 'AWS::Logs::LogGroup',
);
Comment on lines +66 to +76
Copy link
Author

Choose a reason for hiding this comment

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

I'm not too proud of this, however, I can see that template is also defined ultimate as any. If someone has a better idea how to implement this, feel free to let me know.

Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like you might be able to get the same functionality out of template.resourceCountIs() (which is a part of the aws-cdk-lib/assertions library that is already used in this file). See: https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.assertions-readme.html#counting-resources -- you were doing this in the factory test file as well


// Ensure we found exactly one log group
assert.strictEqual(logGroupResources.length, 1);

const logGroupResource = logGroupResources[0] as {
// eslint-disable-next-line @typescript-eslint/naming-convention
Properties: { RetentionInDays: number };
};
Comment on lines +81 to +84
Copy link
Author

Choose a reason for hiding this comment

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

Also, I hate using eslint-disable commands. However, the resource names are not following the same naming convention as the amplify-backend repo. Open for other recommendations.

Copy link
Contributor

Choose a reason for hiding this comment

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

You should also be able to use aws-cdk-lib/assertions for this with template.hasResourceProperties -- https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.assertions-readme.html#resource-matching--retrieval

assert.strictEqual(logGroupResource.Properties.RetentionInDays, 400); // 13 months = 400 days
});
});
44 changes: 37 additions & 7 deletions packages/backend-function/src/logging_options_parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,46 @@ import {
LogLevelConverter,
LogRetentionConverter,
} from '@aws-amplify/platform-core/cdk';
import { RetentionDays } from 'aws-cdk-lib/aws-logs';
import { ILogGroup, LogGroup } from 'aws-cdk-lib/aws-logs';
import { Construct } from 'constructs';
import { RemovalPolicy } from 'aws-cdk-lib';

/**
* CDK Lambda logging options using non-deprecated properties
* - level: ApplicationLogLevel for the lambda function (maps to applicationLogLevelV2)
* - format: Logging format (JSON or TEXT)
* - logGroup: Custom log group for the function (preferred over using logRetention directly)
*/
export type CDKLoggingOptions = {
level?: ApplicationLogLevel;
retention?: RetentionDays;
format?: LoggingFormat;
logGroup?: ILogGroup;
};

/**
* Converts logging options to CDK.
* Creates a LogGroup with the specified retention settings
* This replaces the deprecated logRetention property on NodejsFunction
*/
export const createLogGroup = (
scope: Construct,
id: string,
loggingOptions: FunctionLoggingOptions,
): ILogGroup => {
const retentionDays = new LogRetentionConverter().toCDKRetentionDays(
loggingOptions.retention,
);

return new LogGroup(scope, `${id}-log-group`, {
retention: retentionDays,
removalPolicy: RemovalPolicy.DESTROY, // This matches Lambda's default for auto-created log groups
});
};

/**
* Converts logging options to CDK format using non-deprecated properties
* Note: This function no longer includes 'retention' in the return object
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you remove the note about retention being removed from here and references to the deprecated logRetention property in the other comments in this file, this will not be useful for people in the future who don't need to know this was previously implemented with logRetention.

* as that property is deprecated. Instead, create a LogGroup with
* createLogGroup() when needed.
*/
export const convertLoggingOptionsToCDK = (
loggingOptions: FunctionLoggingOptions,
Expand All @@ -24,18 +54,18 @@ export const convertLoggingOptionsToCDK = (
loggingOptions.level,
);
}
const retention = new LogRetentionConverter().toCDKRetentionDays(
loggingOptions.retention,
);

const format = convertFormat(loggingOptions.format);

return {
level,
retention,
format,
};
};

/**
* Converts format string to LoggingFormat enum
*/
const convertFormat = (format: 'json' | 'text' | undefined) => {
switch (format) {
case undefined:
Expand Down
Loading