-
Notifications
You must be signed in to change notification settings - Fork 47
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
Changes from all commits
09fe4e1
4fa11f3
e3f7153
62dee4c
1025a94
2c87b1a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,17 +6,28 @@ | |
| logger = logging.getLogger(__name__) | ||
|
|
||
| _cold_start = True | ||
| _proactive_initialization = False | ||
| _lambda_container_initialized = False | ||
|
|
||
|
|
||
| def set_cold_start(): | ||
| def set_cold_start(init_timestamp_ns): | ||
| """Set the value of the cold start global | ||
|
|
||
| This should be executed once per Lambda execution before the execution | ||
| """ | ||
| global _cold_start | ||
| global _lambda_container_initialized | ||
| _cold_start = not _lambda_container_initialized | ||
| global _proactive_initialization | ||
| if not _lambda_container_initialized: | ||
| now = time.time_ns() | ||
| if (now - init_timestamp_ns) // 1_000_000_000 > 10: | ||
| _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 commentThe reason will be displayed to describe this comment to others. Learn more. @astuyve I think on this case
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We initialize In this case rolling up to However for clarity, line 27 should always be |
||
| else: | ||
| _cold_start = False | ||
| _proactive_initialization = False | ||
| _lambda_container_initialized = True | ||
|
|
||
|
|
||
|
|
@@ -25,11 +36,25 @@ def is_cold_start(): | |
| return _cold_start | ||
|
|
||
|
|
||
| def is_proactive_init(): | ||
| """Returns the value of the global proactive_initialization""" | ||
| return _proactive_initialization | ||
|
|
||
|
|
||
| def is_new_sandbox(): | ||
| return is_cold_start() or is_proactive_init() | ||
|
|
||
|
|
||
| def get_cold_start_tag(): | ||
| """Returns the cold start tag to be used in metrics""" | ||
| return "cold_start:{}".format(str(is_cold_start()).lower()) | ||
|
|
||
|
|
||
| def get_proactive_init_tag(): | ||
| """Returns the proactive init tag to be used in metrics""" | ||
| return "proactive_initialization:{}".format(str(is_proactive_init()).lower()) | ||
|
|
||
|
|
||
| class ImportNode(object): | ||
| def __init__(self, module_name, full_file_path, start_time_ns, end_time_ns=None): | ||
| self.module_name = module_name | ||
|
|
@@ -115,7 +140,7 @@ def wrapped_find_spec(*args, **kwargs): | |
|
|
||
| def initialize_cold_start_tracing(): | ||
| if ( | ||
| is_cold_start() | ||
| is_new_sandbox() | ||
| and os.environ.get("DD_TRACE_ENABLED", "true").lower() == "true" | ||
| and os.environ.get("DD_COLD_START_TRACING", "true").lower() == "true" | ||
| ): | ||
|
|
||




Uh oh!
There was an error while loading. Please reload this page.
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 newproactive_initialization= trueto 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:trueif 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
initializationoccurred 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 🥇