-
Notifications
You must be signed in to change notification settings - Fork 4k
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
fix(lambda): avoid OperationAbortedException when using log retention #2237
Conversation
If multiple LogRetention constructs are present in the stack, they will try to act on the provider's log group at the same time. This can sometime result in an OperationAbortedException. To avoid this and because this operation is not critical it is better to catch all errors when acting on the provider's log group
// 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}`); |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please add a unit test?
If multiple LogRetention constructs are present in the stack, they will try to
act on the provider's log group at the same time. This can sometime result in
an OperationAbortedException. To avoid this and because this operation is not
critical it is better to catch all errors when acting on the provider's log
group
Pull Request Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license.