-
Notifications
You must be signed in to change notification settings - Fork 421
feat(general): make logger, tracer and metrics utilities aware of provisioned concurrency #6324
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
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #6324 +/- ##
===========================================
- Coverage 96.32% 96.31% -0.02%
===========================================
Files 242 242
Lines 11689 11706 +17
Branches 867 870 +3
===========================================
+ Hits 11260 11275 +15
- Misses 336 337 +1
- Partials 93 94 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Hey @dreamorosi! I'd like your review here. Please read the PR body. |
|
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.
Agree with the overall strategy of postponing the major refactor, I only have a question about the actual logic before we move forward.
After we've clarified that I can approve
Already replied that. Please let me know if you have any additional question. |
Issue number: #6321
Summary
Changes
This PR improves cold start detection in AWS Powertools for Python by leveraging the
AWS_LAMBDA_INITIALIZATION_TYPE
environment variable to accurately identify function initialization states across Provisioned Concurrency (PC) environments.There are some considerations in this PR:
We currently rely on a global variable when importing a specific module, which creates challenges for cold start detection across modules. To address this properly, we would need to refactor the code to use a single function that can control cold start logic across all modules. However, this would involve significant work and I'd need to invest too much time seeing the impact. To avoid this risk, I will temporarily duplicate code across modules to ensure compatibility and prevent disruptions.
The cold start detection logic for Metrics is even more problematic because it relies on legacy code that has been in place since Powertools was first released. In the [feat(metrics): support to bring your own metrics provider #2194], we attempted to refactor this logic but realized that customers are directly importing certain variables, and changing them could break existing implementations. As a result, adding tests for Metrics (specific for PC) would require a major refactor, which I don't want to undertake at this time. For now, I will maintain the current implementation to avoid introducing risks while ensuring stability for customers.
I'm happy to move forward with this PR, even with these limitations.
I'm adding a new item to the v4 discussion to track these items
User experience
Doesn't change the customer experience.
Checklist
If your change doesn't seem to apply, please leave them unchecked.
Is this a breaking change?
RFC issue number:
Checklist:
Acknowledgment
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.