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

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

Merged
merged 2 commits into from
Apr 11, 2019
Merged
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
13 changes: 10 additions & 3 deletions packages/@aws-cdk/aws-lambda/lib/log-retention-provider/index.ts
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

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
35 changes: 35 additions & 0 deletions packages/@aws-cdk/aws-lambda/test/test.log-retention-provider.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import AWSSDK = require('aws-sdk');
import AWS = require('aws-sdk-mock');
import nock = require('nock');
import { Test } from 'nodeunit';
Expand Down Expand Up @@ -227,4 +228,38 @@ export = {

test.done();
},

async 'does not fail when operations on provider log group fail'(test: Test) {
const createLogGroupFake = (params: AWSSDK.CloudWatchLogs.CreateLogGroupRequest) => {
if (params.logGroupName === '/aws/lambda/provider') {
return Promise.reject(new Error('OperationAbortedException'));
}
return Promise.resolve({});
};

const putRetentionPolicyFake = sinon.fake.resolves({});
const deleteRetentionPolicyFake = sinon.fake.resolves({});

AWS.mock('CloudWatchLogs', 'createLogGroup', createLogGroupFake);
AWS.mock('CloudWatchLogs', 'putRetentionPolicy', putRetentionPolicyFake);
AWS.mock('CloudWatchLogs', 'deleteRetentionPolicy', deleteRetentionPolicyFake);

const event = {
...eventCommon,
RequestType: 'Create',
ResourceProperties: {
ServiceToken: 'token',
RetentionInDays: '30',
LogGroupName: 'group'
}
};

const request = createRequest('SUCCESS');

await provider.handler(event as AWSLambda.CloudFormationCustomResourceCreateEvent, context);

test.equal(request.isDone(), true);

test.done();
}
};