-
Notifications
You must be signed in to change notification settings - Fork 88
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
Cold start increased significantly by enabling powertools-logging #121
Comments
Hey @drissamri Thanks for opening the issue. I quickly looked at different branches, and its interesting to see this behaviour. While as far I can see difference between branches, you have not actually used any Powertools logging feature, but rather just changed to use log4j2 instead which you offcourse get as a dependency via logging module from powertools. We actually did a performance test before releasing the module and found log4j2 performing much faster compared to logback/slf4j instead. We in fact specifically tested via lambda-logging itself. Even if you look at this article on performance benchmark for different logging library we see the same results. Nevertheless, since you have concrete examples to show the difference, I will be diving a bit deep later this week to check what might be leading to the observation you have in the example above. |
After I posted this ticket I tested a few more things and saw a mistake in the implementation too (not too logging itself), if I didn't add the @powertools annotation I must've forgot it on that branch after my tests again. Going to circle back on the tests results later tonight. There was still a big difference after my last run, but will update the branches so you have a correct view. |
There was a bug in the lumigo-cli that made the invocation not complete succesfully, so the invocation timings weren't always correct. Thanks to @theburningmonk that's fixed now. @pankajagrawal16 I created two branches
In the powertoolslogging you do get to see these messages, so not sure if they cause for an extra delay:
If I'm still doing anything wrong or if I can help in any way, let me know! |
Aha, This makes sense, coz I tried replicating yesterday and found the same issue that function was failing in logs. I spent most of my time figuring out, what i might be doing wrong :D. I just ended up changing lambda function contract to work with String and just measure it based on that in both implementation. The findings for me were bit similar as well what you have posted above. I still have to dive a bit deep to see why this might be happening. When it come to warning, I too noticed that and also fixed it by adding:
I will look at your branches and dive a bit more into details some time this week. |
After a discussion with @theburningmonk today, I realized the lumigo-cli only prints out initDuration (Initialization in X-Ray) and not (execution) Duration. This will be changed by him later on. I don't think it matters for the statistics I posted before since the clear distinction is there in both in the Initialization alone already, but end to end should be measured to make sure one of both is not losing more time during execution. |
Yeah I have been looking at logic of cloudwatch log insights query that the cli executes and I could see just looks for Init time in lambda logs. I do agree, regardless it gives some indication anyways. |
FYI: I worked together with Yan and now the measure coldstarts CLI measures both initDuration and duration time. |
Hi @drissamri and @pankajagrawal16, may I ask if there is any update on this issue? Is it still a problem in v1.5.0? The reported version seems to be in a beta version a while ago. Many thanks! |
hi @qtdzz There was some optimizations done before going GA to the logging module, not specifically targeting this specific issue. |
I went ahead and tried it again with the latest version of the powertools (1.7.1) . I had to add Sadly, the results are worse than before :(
powertools:
|
Hi @drissamri Thanks for following up on this again. I have taken some time again today to understand what's going on between different implementations. Starting with your question on jackson-core and annotation missing. This has not changed. Reason why this is failing in your sample project is because Now for performance with slf4j and logback, we need to compare similar feature set. Which means powertools-loggging by default provides support for structured logging whereas comparison made above is not doing any structured logging. Therefore, I enabled json logging with logback with lambda-logging and results showed increased latency with cold start as well.
Other aspect is that powertools-logging provides several best practices on top of logging and that will definitely contribute to bit more processing and init time and thus contributing a bit more latency during init. Using I performed another test on latest powertools and results: (Also note difference in number of cold starts for metrics between powertools and lambda-logging)
That being said, I will spend some more time in this week to find the possibility of optimising jackson object mapper initialisation for structured logs |
Done minor tweaks to logging utility as well #484. Based on comparison and also difference in number of cold starts in metrics, I feel comfortable not doing anything more. I have had a look at default log4j2 config and it is optimal for the lambda workload already. Users always have ability to configure log4j2 the way they need like using asynchronous logging, etc. cc @drissamri I will be interested in hearing your thoughts on observations above. |
Some improvements shipped in https://github.com/awslabs/aws-lambda-powertools-java/releases/tag/v1.7.2 as well. I will wait for @drissamri thoughts before closing it. |
@drissamri Let me know if you have any thoughts on above findings ? Or else i will close this issue for now. |
Closing this issue for now. |
What were you trying to accomplish?
While testing in my sample (cold start optimized) application I can measure up to 20%+ increase by enable powertools-logging instead of lambda-logging (SLF4J + Logback)
The tests are executed by leveraging lumigo-cli measure-lambda-cold-starts.
Expected Behavior
Cold starts timings shouldn't become much worse by switching out another logging framework for powertools-logging
Current Behavior
lambda-logging (baseline)
powertools-logging
Possible Solution
/
Steps to Reproduce (for bugs)
This gives you the baseline results, now we enable powertools:
powertools
git branchEnvironment
The text was updated successfully, but these errors were encountered: