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

Make log groups for Lambda/CodeBuild accessible and/or manipulable #667

Closed
rix0rrr opened this issue Sep 5, 2018 · 4 comments · Fixed by #2067 · May be fixed by MechanicalRock/account-reaper#6
Closed

Make log groups for Lambda/CodeBuild accessible and/or manipulable #667

rix0rrr opened this issue Sep 5, 2018 · 4 comments · Fixed by #2067 · May be fixed by MechanicalRock/account-reaper#6
Labels
feature-request A feature should be added or improved.

Comments

@rix0rrr
Copy link
Contributor

rix0rrr commented Sep 5, 2018

Those Log Groups are implicitly created by the services upon first execution, which makes it impossible to attach things like MetricFilters to them.

We need a Custom Resource that will make the LG if it doesn't exist yet, do nothing if it already exists.

The same LG can be used to set the retention policy on the LG.

@rix0rrr
Copy link
Contributor Author

rix0rrr commented Mar 18, 2019

Would be better if we can do this in CloudFormation, but we currently cannot. There are two issues if we were to create a LogGroup object as part of the deployment, as in this example:

class LoggedFunction extends lambda.Function {
  constructor(scope: cdk.Construct, id: string, props: lambda.FunctionProps) {
    super(scope, id, props);

    const log = new logs.LogGroup(this, `${id}-LogGroup`, {
      retentionDays: 30,
      logGroupName: `/aws/lambda/${this.functionName}`,
    });
    const logResource = log.node.findChild('Resource') as logs.CfnLogGroup;
    logResource.options.deletionPolicy = cdk.DeletionPolicy.Delete;
  }
}
  1. There is a race condition, if your Lambda happens to get invoked while CloudFormation is still deploying, the creation of the LogGroup will fail and your CFN deployment will roll back.
  2. If you do this on a hard-named Lambda functions (i.e., not with an autogenerated name), then the Log Group can't be created and your CFN deployment will also fail

jogold added a commit to jogold/aws-cdk that referenced this issue Mar 21, 2019
Adds a new property `logRetentionDays` on `Function` to control the log
retention policy of the function logs in CloudWatch Logs.

The implementation uses a Custom Resource to create the log group if it doesn't
exist yet and to set the retention policy as discussed in aws#667.

A retention policy of 1 day is set on the logs of the Lambda provider.

The different retention days supported by CloudWatch Logs have been centralized
in @aws-cdk/aws-logs. Some have been renamed to better match the console
experience.

Closes aws#667

BREAKING CHANGE: `cloudWatchLogsRetentionTimeDays` in @aws-cdk/aws-cloudtrail
now uses a `logs.RetentionDays` instead of a `LogRetention`.
jogold added a commit to jogold/aws-cdk that referenced this issue Mar 21, 2019
Adds a new property `logRetentionDays` on `Function` to control the log
retention policy of the function logs in CloudWatch Logs.

The implementation uses a Custom Resource to create the log group if it doesn't
exist yet and to set the retention policy as discussed in aws#667.

A retention policy of 1 day is set on the logs of the Lambda provider.

The different retention days supported by CloudWatch Logs have been centralized
in `@aws-cdk/aws-logs`. Some have been renamed to better match the console
experience.

Closes aws#667

BREAKING CHANGE: `cloudWatchLogsRetentionTimeDays` in `@aws-cdk/aws-cloudtrail`
now uses a `logs.RetentionDays` instead of a `LogRetention`.
@eladb
Copy link
Contributor

eladb commented Mar 24, 2019

Do you guys think #2067 will practically resolve this?

eladb pushed a commit that referenced this issue Mar 28, 2019
Adds a new property `logRetentionDays` on `Function` to control the log
retention policy of the function logs in CloudWatch Logs.

The implementation uses a Custom Resource to create the log group if it doesn't
exist yet and to set the retention policy as discussed in #667.

A retention policy of 1 day is set on the logs of the Lambda provider.

The different retention days supported by CloudWatch Logs have been centralized
in `@aws-cdk/aws-logs`. Some have been renamed to better match the console
experience.

Closes #667

BREAKING CHANGE: `cloudWatchLogsRetentionTimeDays` in `@aws-cdk/aws-cloudtrail`
now uses a `logs.RetentionDays` instead of a `LogRetention`.
@SleeperSmith
Copy link

@rix0rrr Actually, this is possible by using some "depends on" statement straight in cloudformation.

  1. Lambda role with no permission.
  2. Lambda function that ref resource 1
  3. CloudWatch log group that ref resource 2
  4. Lambda Role permission policy that grant Lambda function permission to write to 3. This depends on 2 and 3.

This way, CFN creates role, lambda, log, policy in that order and doesn't race. Yeah you'll lose log if the invocation happens before the policy is created. Pick your poison for the time being I guess.

I feel the problem is Lambda should accept the name of log group instead of hard defaulting to a magical name. Urghh....

@mgjam
Copy link

mgjam commented Nov 1, 2019

@eladb when using #2067 on my existing lambda resource, it tried to create entirely new lambda function, not sure whether this is intended behavior. Looks like the issue can be solved by #2240 (comment). I will stick with creating log group relying on auto-generated log group name and configuring it - https://forums.aws.amazon.com/thread.jspa?threadID=260775

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request A feature should be added or improved.
Projects
None yet
5 participants