Refactoring to allow lazy loading of datadog.api #163
Conversation
not that I am aware of and I don't see any trace of boto3 loading in the flamegraph (except in aws-xray-sdk but I've opened a PR here aws/aws-xray-sdk-python#312 🤞 ) |
|
Should we include something we discussed about #147 here? Seems touching similar code path. |
datadog_lambda/metric.py
Outdated
| def stop(self): | ||
| self.thread_stats.stop() | ||
|
|
||
|
|
||
| def init_lambda_stats(): |
There was a problem hiding this comment.
As we discussed before, might make sense to call init_lambda_stats() in the __init__ method of the wrapper? So it only creates the writer on cold start?
There was a problem hiding this comment.
Alternatively you could check lambda_stats is None before creating a new writer? But I think it's probably the best that we have both?
There was a problem hiding this comment.
I've reverted the PR #147 as it had no effect on /hello route
datadog_lambda/metric.py
Outdated
| ) | ||
| logger.debug("Setting DATADOG_HOST to %s", api._api_host) | ||
| # Set API Key only if extension is not here | ||
| if not should_use_extension: |
There was a problem hiding this comment.
Perhaps we should put them into a function? and call it in the wrapper's __init__ method? I think we can further reduce the scope here, the API key is ONLY needed when DD_FLUSH_TO_LOG is false (we send metrics synchronously from the library)
There was a problem hiding this comment.
I've updated the condition
| write_metric_point_to_stdout(metric_name, value, timestamp=timestamp, tags=tags) | ||
| else: | ||
| logger.debug("Sending metric %s to Datadog via lambda layer", metric_name) | ||
| if should_use_extension: | ||
| lambda_stats.distribution(metric_name, value, tags=tags, timestamp=timestamp) |
There was a problem hiding this comment.
I am usually not a big fan of having two if branches which execute the same code but I think for this case it makes the code easier to read
What does this PR do?
This PR
Motivation
Cold start improvements
Testing Guidelines
Unit + Integration tests
Types of Changes
Check all that apply