Skip to content

Conversation

@joe4dev
Copy link
Member

@joe4dev joe4dev commented Nov 28, 2023

Motivation

Lambda records a CloudWatch (CW) error metric upon every failed Lambda invocation but LocalStack does not record the error metric anymore since the Lambda invocation loop rework. @steffyP recently added a test for this behavior in #9653 discovered in a Pro sample.

Errors – The number of invocations that result in a function error. Function errors include exceptions that your code throws and exceptions that the Lambda runtime throws. The runtime returns errors for issues such as timeouts and configuration errors. To calculate the error rate, divide the value of Errors by the value of Invocations. Note that the timestamp on an error metric reflects when the function was invoked, not when the error occurred.

see AWS CW metrics

Changes

  • Fix the regression and submit a CloudWatch metric upon failed Lambda invocation
  • Update some Lambda docs (nit)

Discussion

  • Exceptions that are not caught in localstack.services.lambda_.invocation.version_manager.LambdaVersionManager.invoke (i.e., all exceptions except StatusErrorException) won't be recorded as errors. For example, exceptions from the Lambda control plane (e.g., TooManyRequestsException) should be recorded in other metrics (e.g., Throttles).

Background

looks like a bug introduced in the Lambda invocation loop rework: https://github.com/localstack/localstack/pull/8970/files#diff-056f8f1dc9188bf87729941f6be9805c0caf16189dafa882f16fb259676f79daL727

similar to the record_invocation case here:

https://github.com/localstack/localstack/pull/8970/files#diff-056f8f1dc9188bf87729941f6be9805c0caf16189dafa882f16fb259676f79daR220

This should be fixed in the version_manager.py::invoke

@joe4dev joe4dev added the semver: patch Non-breaking changes which can be included in patch releases label Nov 28, 2023
@joe4dev joe4dev added this to the 3.1 milestone Nov 28, 2023
@joe4dev joe4dev self-assigned this Nov 28, 2023
@coveralls
Copy link

Coverage Status

coverage: 84.161% (+0.04%) from 84.126%
when pulling b5ef7ff on fix-lambda-cloudwatch-error-metric-recording
into 82fd17a on master.

@github-actions
Copy link

LocalStack Community integration with Pro

       2 files         2 suites   1h 8m 57s ⏱️
2 370 tests 2 056 ✔️ 314 💤 0
2 371 runs  2 056 ✔️ 315 💤 0

Results for commit b5ef7ff.

@joe4dev joe4dev marked this pull request as ready for review November 28, 2023 13:26
Copy link
Member

@dominikschubert dominikschubert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We definitely need to refactor this a bit at some point, especially to use a thread pool instead of spawning new threads every time 😬

otherwise LGTM!

@joe4dev joe4dev merged commit e13bb88 into master Nov 29, 2023
@joe4dev joe4dev deleted the fix-lambda-cloudwatch-error-metric-recording branch November 29, 2023 08:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

semver: patch Non-breaking changes which can be included in patch releases

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants