-
Notifications
You must be signed in to change notification settings - Fork 420
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
fix(tracing): add flag for aiohttp that disables potential for memory leak #12081
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #12081 +/- ##
==========================================
- Coverage 12.98% 12.43% -0.56%
==========================================
Files 1607 1607
Lines 136979 136979
==========================================
- Hits 17782 17028 -754
- Misses 119197 119951 +754 ☔ View full report in Codecov by Sentry. |
|
# todo: this is a temporary fix for a memory leak in aiohttp. We should find a way to | ||
# consistently close spans with the correct timing. | ||
if not config.aiohttp["disable_stream_timing_for_mem_leak"]: | ||
if type(response) is web.StreamResponse and not response.task.done(): |
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.
Datadog ReportBranch report: ✅ 0 Failed, 130 Passed, 1378 Skipped, 4m 17.29s Total duration (35m 49.93s time saved) |
BenchmarksBenchmark execution time: 2025-01-24 23:15:39 Comparing candidate commit 52cad7f in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 394 metrics, 2 unstable metrics. |
This PR adds the environment variable
DD_AIOHTTP_CLIENT_DISABLE_STREAM_TIMING_FOR_MEM_LEAK
to address a potential memory leak in the aiohttp integration. When set to true, this flag may cause streamed response span timing to be inaccurate. The flag defaults to false.When this was merged #7551 we essentially added support for getting the correct timing of streamed responses, however it also caused us to sometimes never close spans, which lead to a memory leak. When set to true, this flag essentially reverts that behavior.
Checklist
Reviewer Checklist