feat(lambda): function log removal policy #34723
Conversation
| removalPolicy: props.logRemovalPolicy, | ||
| }); | ||
| this._logGroup = logs.LogGroup.fromLogGroupArn(this, 'LogGroup', logRetention.logGroupArn); | ||
| this._logRetention = logRetention; | ||
| } else if (!props.logGroup && (FeatureFlags.of(this).isEnabled(USE_CDK_MANAGED_LAMBDA_LOGGROUP))) { // logRetention:f/undef, logGroup:f/undef, FF.isEnabled() | ||
| this._logGroup = new logs.LogGroup(this, 'LogGroup', { | ||
| logGroupName: `/aws/lambda/${this.functionName}`, | ||
| removalPolicy: props.logRemovalPolicy, | ||
| }); | ||
| } |
There was a problem hiding this comment.
It seems that a log group is being generated within the getter. I thought to store logRemovalPolicy in a private instance variable and to pass it to the following properties too.
private _logRemovalPolicy?: RemovalPolicy;
// ...
// ...
this._logRemovalPolicy = props.logRemovalPolicy;
// ...
// ...
public get logGroup(): logs.ILogGroup {
if (!this._logGroup) {
const logRetention = new logs.LogRetention(this, 'LogRetention', {
logGroupName: `/aws/lambda/${this.functionName}`,
retention: logs.RetentionDays.INFINITE,
removalPolicy: this._logRemovalPolicy,
});
this._logGroup = logs.LogGroup.fromLogGroupArn(this, `${this.node.id}-LogGroup`, logRetention.logGroupArn);
}
return this._logGroup;
}However, other properties such as logRetentionRole and logRetentionRetryOptions are not passed to them in the getter. But logRetentionRole and logRetentionRetryOptions are only applied when props.logRetention is specified, as indicated by their prefixes.
if (props.logRetention) {
if (props.logGroup) {
throw new ValidationError('CDK does not support setting logRetention and logGroup', this);
}
const logRetention = new logs.LogRetention(this, 'LogRetention', {
logGroupName: `/aws/lambda/${this.functionName}`,
retention: props.logRetention,
role: props.logRetentionRole,
logRetentionRetryOptions: props.logRetentionRetryOptions as logs.LogRetentionRetryOptions,
});
this._logGroup = logs.LogGroup.fromLogGroupArn(this, 'LogGroup', logRetention.logGroupArn);On the other hand, the creation of a log group using this getter is performed when props.logRetention is NOT specified (otherwise this._logGroup will be true). In other words, unlike those properties, it seems natural that logRemovalPolicy is also passed in the getter.
How about it?
There was a problem hiding this comment.
Thank you so much for pointing this out. I totally missed this.
Added to this getter and include in tests.
7b61c0b to
76aa322
Compare
✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.
|
Thanks for your review @go-to-k |
cdf695a to
778d245
Compare
778d245 to
e41cda1
Compare
Co-authored-by: Matyas Boros <[email protected]>
fix documentation
|
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
|
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
|
Comments on closed issues and PRs are hard for our team to see. |
Issue # (if applicable)
Closes #34669
Reason for this change
Support configuring function log removal policy
Description of changes
Expose log group removal policy to function props
Describe any new or updated permissions being added
Description of how you validated changes
Unit + Integ
Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license