fix(tracing): add flag for aiohttp that disables potential for memory leak

[ZStriker19]

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

• PR author has checked that all the criteria below are met
• The PR description includes an overview of the change
• The PR description articulates the motivation for the change
• The change includes tests OR the PR description describes a testing strategy
• The PR description notes risks associated with the change, if any
• Newly-added code is easy to change
• The change follows the library release note guidelines
• The change includes or references documentation updates if necessary
• Backport labels are set (if applicable)

Reviewer Checklist

• Reviewer has checked that all the criteria below are met
• Title is accurate
• All changes are related to the pull request's stated goal
• Avoids breaking API changes
• Testing strategy adequately addresses listed risks
• Newly-added code is easy to change
• Release note makes sense to a user of the library
• If necessary, author has acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment
• Backport labels are set in a manner that is consistent with the release branch maintenance policy

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 12.43%. Comparing base (b90fa38) to head (426f466).

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.
📢 Have feedback on the report? Share it here.

[github-actions]

CODEOWNERS have been resolved as:

releasenotes/notes/add_aiohttp_memory_leak_flag-66005f987dbfbd47.yaml   @DataDog/apm-python
ddtrace/contrib/aiohttp.py                                              @DataDog/apm-core-python @DataDog/apm-idm-python
ddtrace/contrib/internal/aiohttp/middlewares.py                         @DataDog/apm-core-python @DataDog/apm-idm-python
ddtrace/contrib/internal/aiohttp/patch.py                               @DataDog/apm-core-python @DataDog/apm-idm-python
tests/contrib/aiohttp/test_request.py                                   @DataDog/apm-core-python @DataDog/apm-idm-python

[datadog-dd-trace-py-rkomorn]

Datadog Report

Branch report: zachg/add_aiohttp_flag_for_stream_response_time
Commit report: cf983ed
Test service: dd-trace-py

✅ 0 Failed, 98 Passed, 1378 Skipped, 3m 10.78s Total duration (30m 56.61s time saved)

[pr-commenter]

Benchmarks

Benchmark execution time: 2025-02-04 19:29:31

Comparing candidate commit 0bb5b4e in PR branch zachg/add_aiohttp_flag_for_stream_response_time with baseline commit e294f47 in branch 3.x-staging.

Found 0 performance improvements and 0 performance regressions! Performance is the same for 394 metrics, 2 unstable metrics.

[wantsui]

I don't know the best place for this, but is it possible to put this in the aiohttp configuration docs somewhere: https://ddtrace.readthedocs.io/en/stable/integrations.html#id14, or the general "configuration" docs to call out the existence of this setting?

[brettlangdon]

@ZStriker19

however it also caused us to sometimes never close spans

Do we know why this is?

Would weak refs for the finish callback be good enough?

[ZStriker19]

@brettlangdon no we don't know why and in my testing weak refs had the same effect of shortening the duration of the stream response span. This is essentially a mitigation for users. The next steps will be figuring out a way to not memory leak and also get span duration correct: https://github.com/DataDog/dd-trace-py/pull/12081/files#diff-dfa4245913c22bb0645a2e9a1b9ca2a0bf9db016b7920c4d07a3c112e66484afR139-R142

[chamini2]

looking forward to a version with this option!

[chamini2]

is there any way we can get this update in 2.X version? We have some stuff built with internals.