-
Notifications
You must be signed in to change notification settings - Fork 98
fix: Support provisioned concurrency in cold start detection #2124
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: Support provisioned concurrency in cold start detection #2124
Conversation
- Add AWS_LAMBDA_INITIALIZATION_TYPE environment variable check - Prevent false cold start reporting when using provisioned concurrency - Fixes issue aws-powertools#2113 across Logging, Metrics, and Tracing utilities Before: cold_start: true (incorrect with provisioned concurrency) After: cold_start: false (correct with provisioned concurrency)
@dcabib There's an issue in the SonarQube finding, can you look into that. |
- Refactor if-then-else logic to single return statement - Maintains same functionality for provisioned concurrency detection - Addresses SonarQube code smell flagged by @sdangol - All tests passing: 21 tests, 0 failures
@sdangol Issue fixed. |
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right now we're explicitly comparing the AWS_LAMBDA_INITIALIZATION_TYPE
to this value provisioned-concurrency
and returning false
in the isColdStart()
method.
As documented here, there are other types of initialization types during which isColdStart()
should return false
(hint snap-start
).
A better and more future proof approach would be to instead check if the initialization type is on-demand
and if it is, maintain a logic similar to the existing one, if it's not, always return false
.
- Remove .sonarcloud.properties changes (keep SonarCloud rules strict) - Remove LambdaHandlerProcessor.java changes (belong in PR aws-powertools#2124) - Remove LambdaConstants.java changes (belong in PR aws-powertools#2124) - Keep PR focused only on CRaC tracing functionality - Maintain GraalVM compatibility fixes for CRaC tracing SpotBugs exclusions are justified for CRaC beforeCheckpoint methods which intentionally ignore return values during priming operations.
Hey @dreamorosi, thanks for the feedback! I've cleaned up the PR - removed the SonarCloud changes, took out the unrelated provisioned concurrency stuff that belongs in #2124, fixed the license header, and kept it focused just on CRaC tracing. The SpotBugs exclusions are there because CRaC beforeCheckpoint methods intentionally ignore return values during priming. Should be good to go now! |
Hi, I see you've missed/ignored the review feedback that I left here and instead left a comment for a completely different PR. While we're open to contributors, we also need to mindful of our team's time and misusing reviewers time is not appropriate. Thank you, but I'll close this PR. |
Summary
Fixes #2113 - Provisioned concurrency not considered in Logging and Metrics
Problem
When Lambda functions use Provisioned Concurrency, AWS sets
AWS_LAMBDA_INITIALIZATION_TYPE=provisioned-concurrency
to indicate pre-warmed environments. However, Powertools incorrectly reported these as cold starts across all utilities:cold_start: true
in JSON logsColdStart
metric with value 1ColdStart: true
annotation in X-Ray segmentsSolution
Updated the centralized cold start detection logic in
LambdaHandlerProcessor.isColdStart()
to check for theAWS_LAMBDA_INITIALIZATION_TYPE
environment variable.Changes
isColdStart()
method to detect provisioned concurrencyEvidence
Before Fix (Bug):
ColdStart metric emitted:
"ColdStart":1.0
After Fix (Resolved):
No ColdStart metric emitted
Testing
Impact