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.

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

Jump to bottom

fix(lambda): avoid OperationAbortedException when using log retention #2237

Merged
merged 2 commits into from
Apr 11, 2019
Merged

fix(lambda): avoid OperationAbortedException when using log retention #2237

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8410d48
fix(lambda): avoid OperationAbortedException when using log retention
jogold Apr 10, 2019
82762b7
Add unit test
jogold Apr 10, 2019
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
13 changes: 10 additions & 3 deletions packages/@aws-cdk/aws-lambda/lib/log-retention-provider/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,16 @@ export async function handler(event: AWSLambda.CloudFormationCustomResourceEvent
// group for this function should already exist at this stage because we
// already logged the event but due to the async nature of Lambda logging
// there could be a race condition. So we also try to create the log group
// of this function first.
await createLogGroupSafe(`/aws/lambda/${context.functionName}`);
await setRetentionPolicy(`/aws/lambda/${context.functionName}`, 1);
// of this function first. If multiple LogRetention constructs are present
// in the stack, they will try to act on this function's log group at the
// same time. This can sometime result in an OperationAbortedException. To
// avoid this and because this operation is not critical we catch all errors.
try {
await createLogGroupSafe(`/aws/lambda/${context.functionName}`);
Copy link
Contributor

Choose a reason for hiding this comment

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

What am I missing? Where is the part that sends the response to cloudformation?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is only to catch errors when setting the log retention on the lambda provider's log group of the custom resource not the log we are targeting, it's an internal try/catch. The external one still exists.

https://github.com/awslabs/aws-cdk/blob/8410d48ac570cd998b14569c5d16e08bac8408cc/packages/%40aws-cdk/aws-lambda/lib/log-retention-provider/index.ts#L66-L70

Copy link
Contributor Author

@jogold jogold Apr 10, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

We don't want to fail the whole process if setting the retention policy of 1 day on the provider's log group (singleton fn) fails.

Copy link
Contributor

Choose a reason for hiding this comment

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

ok. thanks for the clarification. missed that

await setRetentionPolicy(`/aws/lambda/${context.functionName}`, 1);
} catch (e) {
console.log(e);
}
}
}

Expand Down