feat: Support CRaC priming for powertools-tracing and powertools-serialization#2104
feat: Support CRaC priming for powertools-tracing and powertools-serialization#2104dcabib wants to merge 2 commits intoaws-powertools:mainfrom
Conversation
…alization - Add CRaC dependency and generate-classesloaded-file profile to both modules - Implement Resource interface in TracingUtils and JsonConfig classes - Add classesloaded.txt files for automatic class preloading - Add comprehensive CRaC tests for both modules - Update documentation with SnapStart priming guidance - Update spotbugs-exclude.xml for beforeCheckpoint methods Addresses issues aws-powertools#2004 and aws-powertools#2003
phipag
left a comment
There was a problem hiding this comment.
Hey @dcabib,
thanks for sending this PR. I added a couple of comments.
After implemented, I will test this in my AWS account to see if the runtime hooks get correctly executed at runtime when publishing a SnapStart enabled Lambda version using these dependencies.
There was a problem hiding this comment.
This is not needed since we load it below anyway.
There was a problem hiding this comment.
This doesn't have any side-effects and we should aim at not catching an exception at all.
- Can you generate realistic events that pass Jackson loading?
- Why are you loading the event classes reflectively using
Class<?> eventClass = Class.forName(className);? I don't think this is necessary – let's import them directly and parse the generated fake events usingmapper.readValue(fakeEvent, SQSEvent.class).
I think you can use sam cli to generate fake events https://docs.aws.amazon.com/serverless-application-model/latest/developerguide/sam-cli-command-reference-sam-local-generate-event.html
| @Test | ||
| void testBeforeCheckpointDoesNotThrowException() { | ||
| assertThatNoException().isThrownBy(() -> config.beforeCheckpoint(context)); | ||
| } | ||
|
|
||
| @Test | ||
| void testAfterRestoreDoesNotThrowException() { | ||
| assertThatNoException().isThrownBy(() -> config.afterRestore(context)); | ||
| } |
There was a problem hiding this comment.
After removing the exception wildcard catch these tests should still pass. Right now, they have no effect.
See comment: https://github.com/aws-powertools/powertools-lambda-java/pull/2104/files#r2314166695
There was a problem hiding this comment.
Can you check if we can remove this exception catching as well?
There was a problem hiding this comment.
Let's move pre-load classes to the top of beforeCheckpoint everywhere? This makes sure that this always runs even if something else throws an exception at runtime. It has dedicated unit tests making sure that this is reliable.
| <And> | ||
| <Class name="software.amazon.lambda.powertools.utilities.JsonConfig"/> | ||
| <Method name="beforeCheckpoint"/> | ||
| </And> |
There was a problem hiding this comment.
I believe we can remove this again after implementing my comment about JsonConfig priming. If it is still needed let's keep it there.
| === "Constructor" | ||
|
|
||
| ```java hl_lines="7" | ||
| import software.amazon.lambda.powertools.tracing.Tracing; | ||
| import software.amazon.lambda.powertools.tracing.TracingUtils; | ||
|
|
||
| public class MyFunctionHandler implements RequestHandler<APIGatewayProxyRequestEvent, APIGatewayProxyResponseEvent> { | ||
|
|
||
| public MyFunctionHandler() { | ||
| TracingUtils.putAnnotation("init", "priming"); // Ensure TracingUtils is loaded for SnapStart | ||
| } | ||
|
|
||
| @Override | ||
| @Tracing | ||
| public APIGatewayProxyResponseEvent handleRequest(APIGatewayProxyRequestEvent input, Context context) { | ||
| // ... | ||
| return something; | ||
| } | ||
| } | ||
| ``` | ||
|
|
||
| === "Static Initializer" | ||
|
|
||
| ```java hl_lines="7" | ||
| import software.amazon.lambda.powertools.tracing.Tracing; | ||
| import software.amazon.lambda.powertools.tracing.TracingUtils; | ||
|
|
||
| public class MyFunctionHandler implements RequestHandler<APIGatewayProxyRequestEvent, APIGatewayProxyResponseEvent> { | ||
|
|
||
| static { | ||
| TracingUtils.putAnnotation("init", "priming"); // Ensure TracingUtils is loaded for SnapStart | ||
| } | ||
|
|
||
| @Override | ||
| @Tracing | ||
| public APIGatewayProxyResponseEvent handleRequest(APIGatewayProxyRequestEvent input, Context context) { | ||
| // ... | ||
| return something; | ||
| } | ||
| } | ||
| ``` |
There was a problem hiding this comment.
I think we should expand TracingUtils with a method TracingUtils.prime() that has no side-effects unlike the example given here. As a user, I don't want to be forced to add e.g. an annotation to my traces just to trigger SnapStart priming.
- Add TracingUtils.prime() method with no side-effects for public API - Move ClassPreLoader.preloadClasses() to top of beforeCheckpoint methods - Remove unnecessary exception catching in CRaC hooks - Update JsonConfig to use direct imports instead of reflection for AWS Lambda events - Fix CRaC tests to not use reflection for accessing private fields - Update documentation examples to use TracingUtils.prime() - Consolidate SpotBugs exclusions into single Or structure All CRaC tests passing (4 tests, 0 failures)
|
|
Hey @dcabib, if possible, let's split this PR in two PRs please since it addresses two separate issues. It will be easier to address comments on a smaller scale since I see some of my comments were ignored in the recent push and all of them were left unanswered. |
|
Closing this PR as requested by @phipag to split into separate PRs. This has been addressed with:
Both separate PRs are now ready for review with all SonarQube issues resolved and all tests passing. |
Summary
Adds support for CRaC priming in both the powertools-tracing and powertools-serialization modules to improve AWS Lambda SnapStart restore durations.
Changes
Implementation Details
Tracing Module:
Serialization Module:
Both implementations follow the exact same patterns established in PR #2081 for the validation module.
Testing
Issue numbers: #2004 and #2003
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.