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

[lambda] expose log group and support metric filter #3838

Closed
2 tasks
alexarb opened this issue Aug 28, 2019 · 12 comments · Fixed by #5878
Closed
2 tasks

[lambda] expose log group and support metric filter #3838

alexarb opened this issue Aug 28, 2019 · 12 comments · Fixed by #5878
Assignees
Labels
@aws-cdk/aws-cloudwatch Related to Amazon CloudWatch @aws-cdk/aws-lambda Related to AWS Lambda feature-request A feature should be added or improved.

Comments

@alexarb
Copy link

alexarb commented Aug 28, 2019

🚀 Feature Request

General Information

  • 👋 I may be able to implement this feature request
  • ⚠️ This feature might incur a breaking change

Description

Currently it's impossible to add MetricFilter for Lambda logs, because Lambda does not return log group and log group will be created only after any invocations of function. Manually created log groups (in CDK) might not work because of possible race conditions.

Addition of MetricFilter to logs is similar to LogGroup retention setting as discussed here #667 (comment) and implemented in PR #2067

Proposed Solution

Environment

  • CDK CLI Version:
    all
  • Module Version:
    all
  • OS:
    all
  • Language:
    all

Other information

@alexarb alexarb added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Aug 28, 2019
@SomayaB SomayaB added @aws-cdk/aws-cloudwatch Related to Amazon CloudWatch @aws-cdk/aws-lambda Related to AWS Lambda labels Sep 5, 2019
@SomayaB
Copy link
Contributor

SomayaB commented Oct 10, 2019

Hi @alexarb, thanks for submitting a feature request! We will look into this and someone will update this issue when there is movement.

@SomayaB SomayaB removed the needs-triage This issue or PR still needs to be triaged. label Oct 10, 2019
@nija-at
Copy link
Contributor

nija-at commented Oct 18, 2019

Related #667

@nija-at nija-at changed the title Lambda - add support of metric filter [lambda] expose log group and support metric filter Oct 18, 2019
@AdonousTech
Copy link

Hello. Any update on this feature request? This is a major issue when attempting to create metric filters on lambda logs. Currently we must use ILogGroup for the logGroup property of the MetricFilter construct. This limits possible workarounds (e.g. referencing the name of existing log group). The only workaround seems to be manually creating the metric filter in the console. That is far from ideal.

@ivoanjo
Copy link
Contributor

ivoanjo commented Dec 30, 2019

+1 my team is also facing the issue @AdonousTech mentioned above.

@nija-at
Copy link
Contributor

nija-at commented Dec 30, 2019

Based on lambda's documentation, it seems that the log group name for the function will always be /aws/lambda/<function name>.

@alexarb, @AdonousTech & @ivoanjo - would using the LogGroup.fromLogGroupARN() API or the upcoming LogGroup.fromLogGroupName() API and manually specifying the LogGroup ARN or name unblock your use cases?

@Marty-Me
Copy link

We're facing the same issue. @nija-at It won't, as the Log Group is only created after the initial invocation of the Lambda. What worked for me was creating the Log Group ourselves /aws/lambda/<function name> and after this creating the metrics filter.

@AdonousTech
Copy link

@nija-at I was able to get it working last night by using the logs.LogGroup.fromLogGroupArn. However, I only tested it on a stack update. I need to test it on the full stack creation to ensure there are no race conditions (e.g. LogGroup.fromLogGroupArn does not work because log group is not yet created). I will update and let you know whether it worked.

Here's the syntax for my metric filter (I declared the actual metric and lambda function earlier in the stack):

      this.metricFilter = new logs.MetricFilter(this, 'MetricFilter', {
        filterPattern: {logPatternString: '{ $.eventType = "CriticalError" }'},
        logGroup: logs.LogGroup.fromLogGroupArn(this, 'LogGroup', 'arn:aws:logs:' + props?.env?.region + ':' + props?.env?.account + ':log-group:/aws/lambda/' + this.lambdaFunction.functionName),
        metricNamespace: this.metric.namespace,
        metricName: this.metric.metricName,
        defaultValue: 0,
        metricValue: '1'
      });

@AdonousTech
Copy link

I had a chance to test my workaround above, but as I suspected, it fails when creating a new stack.

The specified log group does not exist. (Service: AWSLogs; Status Code: 400; Error Code: ResourceNotFoundException; Request ID: 9f1672ef-e9a5-4d51-aa63-9786f5280fff)

Also, we cannot create the log group ahead of time because log groups starting with aws are restricted. Consequently, the solution above is better than nothing. Until the feature is added, I will need to comment out certain parts of the code, and uncomment after the lambda function and log group is created.

I will continue to watch this thread for updates.

@nija-at
Copy link
Contributor

nija-at commented Jan 2, 2020

Thanks for your responses.

What worked for me was creating the Log Group ourselves

@Marty-Me - did you do this through the CDK app or outside (CLI or console)?

we cannot create the log group ahead of time because log groups starting with aws are restricted

@AdonousTech - how do you mean? I was able to do the following -

⇒  aws logs create-log-group --log-group-name /aws/lambda/test-group
⇒  aws logs describe-log-groups --log-group-name-prefix /aws/lambda/test --query 'logGroups[].logGroupName'
[
    "/aws/lambda/test-group"
]

@Marty-Me
Copy link

Marty-Me commented Jan 2, 2020

@nija-at using the cdk, as I used the lambda's name, an implicit dependency was created between the log group and the lambda, which prevented 'order of creation'-conflicts.

@AdonousTech
Copy link

AdonousTech commented Jan 2, 2020

@nija-at I take that back, I can actually create the log group. I'm not sure what I did wrong last time when I received the naming error.

After working through this, it seems that I now have an acceptable flow. In my TS code I simply 1) declare the LogGroup after the lambda.Function and follow the standard naming convention /aws/lambda/' + this.functionName. 2) I reference the log group on the MetricFilter by simply using this.logGroup. This order seems to work (e.g. lambda function first, log group second).

Problem solved. Thanks! For anyone else struggling with this:

      // first declare lambda function
      this.lambdaFunction = new lambda.Function(this, Vars.lambda.functionName, {
        runtime: lambda.Runtime.NODEJS_12_X,
        handler: 'index.handler',
        tracing: lambda.Tracing.ACTIVE,
        role: this.executionRole,
        code: lambda.Code.fromAsset(path.join(__dirname, '../../lambda/code/skill_lambda.zip'))
      });

      // declare log group using standard naming convention
      this.logGroup = new LogGroup(this, 'LogGroup', {
        logGroupName: '/aws/lambda/' + this.lambdaFunction.functionName
      })

      // declare other dependencies on log group such as metric filter, alarms, etc
            this.metricFilter = new logs.MetricFilter(this, 'MetricFilter', {
        filterPattern: {logPatternString: '{ $.eventType = "CriticalError" }'},
        logGroup: this.logGroup,
        metricNamespace: this.metric.namespace,
        metricName: this.metric.metricName,
        defaultValue: 0,
        metricValue: '1'
      });

@mariusGundersen
Copy link

There seems to be two ways to do this:

  • create a new logGroup()
  • get an existing logGroup using LogGroup.fromLogGroupArn()

The first method only works if the log group doesn't already exist, but the second one only works if the log group already exists. In my scenario I had made the lambda (and the log group) in a previous deploy, and then I wanted to add a subscription to the log group in a second deploy, so I had to use the second variant. But then I tried to deploy this construct in a new stack, and now it failed, since the log group didn't exist yet. So Unless you know the state of the stack you are deploying to you can't really use these.

nija-at pushed a commit that referenced this issue Jan 20, 2020
The lambda function's log group is now available for use in the CDK app
so it can be used further to create subscription filters, metric
filters, etc. or in any other way that a regular log group may be used.

The implementation uses an underlying CustomResource to guarantee the
existence of this log group as part of the CloudFormation stack.
A new property called `exposeLogGroup` to enable this, so that existing
stacks that have a Lambda function don't change significantly by
automatically getting this CustomResource, as well as, users who are
not interested in this log group can opt out.

closes #3838
nija-at pushed a commit that referenced this issue Jan 20, 2020
The lambda function's log group is now available for use in the CDK app
so it can be used further to create subscription filters, metric
filters, etc. or in any other way that a regular log group may be used.

The implementation uses an underlying CustomResource to guarantee the
existence of this log group as part of the CloudFormation stack.
A new property called `exposeLogGroup` to enable this, so that existing
stacks that have a Lambda function don't change significantly by
automatically getting this CustomResource, as well as, users who are
not interested in this log group can opt out.

closes #3838
@mergify mergify bot closed this as completed in #5878 Jan 24, 2020
mergify bot pushed a commit that referenced this issue Jan 24, 2020
The lambda function's log group is now available for use in the CDK app
so it can be used further to create subscription filters, metric
filters, etc. or in any other way that a regular log group may be used.

The implementation uses an underlying CustomResource to guarantee the
existence of this log group as part of the CloudFormation stack.
However, the custom resource is only created either when the
`logRetention` property is set or when `logGroup` getter is called. This
prevents existing stacks that use Lambda function to get the custom
resource and can be opted into.

closes #3838

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

<!-- 
Please read the contribution guidelines and follow the pull-request checklist:
https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md
 -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-cloudwatch Related to Amazon CloudWatch @aws-cdk/aws-lambda Related to AWS Lambda feature-request A feature should be added or improved.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants