Skip to content
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): avoid assigning span's local root to self, so that the python GC doesn't kick in [backport 2.12] #10836

Open
wants to merge 2 commits into
base: 2.12
Choose a base branch
from

Commits on Sep 26, 2024

  1. fix(tracing): avoid assigning span's local root to self, so that the …

    …python GC doesn't kick in (#10809)
    
    _<<Description copied from the [original
    PR](https://github.com/DataDog/dd-trace-py/pull/10624)>>_
    
    We (Modal Labs) noticed that enabling tracing in parts of our code
    caused the Python GC to kick in a lot and add large delays to our event
    loop. A simple repro (see test) showed us that there was a self
    reference cycle in span when a top-level span's local root was assigned
    to the span itself, preventing Python from cleaning up the objects by
    reference counting.
    
    The change itself is pretty straightforward - we use None to represent
    when a span's local root is itself. The span still exposes _local_root
    with the same behavior via a property, and internally stores a
    _local_root_value that holds a non-self-referential span. This also
    means that the tracer no longer needs to explicitly set the local root
    for top-level spans. This should be fairly non-risky as it doesn't
    change any code behavior.
    
    ## Checklist
    - [x] 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](https://ddtrace.readthedocs.io/en/stable/releasenotes.html)
    - The change includes or references documentation updates if necessary
    - Backport labels are set (if
    [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting))
    
    ## Reviewer Checklist
    - [x] 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](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces)
    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](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)
    
    ---------
    
    Co-authored-by: Daniel Shaar <[email protected]>
    Co-authored-by: Daniel Shaar <[email protected]>
    (cherry picked from commit e160b36)
    erikayasuda authored and github-actions[bot] committed Sep 26, 2024
    Configuration menu
    Copy the full SHA
    4d7ec35 View commit details
    Browse the repository at this point in the history

Commits on Sep 27, 2024

  1. Configuration menu
    Copy the full SHA
    d89e748 View commit details
    Browse the repository at this point in the history