-
Notifications
You must be signed in to change notification settings - Fork 45
Aj/proactive init #343
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
Aj/proactive init #343
Conversation
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.
LGTM
if not _lambda_container_initialized: | ||
now = time.time_ns() | ||
if (now - init_timestamp_ns) // 1_000_000_000 > 10: | ||
_cold_start = False |
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.
Hey @astuyve @joeyzhao2018 should it also be _cold_start = True
? I'm getting use cold_start stats from apms and with this change I will need to take into account new proactive_initialization= true
to measure it like before.
Based on AWS docs I'm not sure if in this situation cold_start should be true or false. Any thoughts?
Thanks
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.
Hi @pvicente - We're actively working with AWS on documentation for this and I can't say more right now, but you should open a support ticket with them and tell them AJ Stuyvenberg sent you ;)
A bit more about what I can say:
With this change you'll still get cold_start:true
if the AWS Lambda function actually experienced a cold start, like so:
However if the function is proactively initialized, then it's not a cold start in the sense that there was no additional latency caused by sandbox initialization (as init occurred seconds, hours, or minutes before the first request was served by the sandbox).
We now tag those correctly, so when we use APM to select this python invocation:
We can see that initialization
occurred 3m59s before the function was executed:
I hope I've helped!
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.
Thanks for the explanation @astuyve 🥇
_cold_start = False | ||
_proactive_initialization = True | ||
else: | ||
_cold_start = not _lambda_container_initialized |
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.
@astuyve I think on this case _proactive_initialization = False
see my case from APM stats:
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.
We initialize _proactive_initialization
to False
on line 9 so we shouldn't need to redeclare that here.
In this case rolling up to top 10
is skewing the results:
I can see something similar in one of my orgs:
But when I remove the pie chart
grouping, I see many cold_start: true
spans:
However for clarity, line 27 should always be false
as we're already inside of a conditional checking for _lambda_container_initialized == false
What does this PR do?
This adds support for tagging proactively initialized lambda functions.
Motivation
Testing Guidelines
I built a pre-release and tested it, verifying we're correctly tagging proactively initialized lambda functions

Additional Notes
Types of Changes
Check all that apply