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]>

ddtrace/_trace/span.py

@@ -113,7 +113,7 @@ class Span(object):
         "duration_ns",
         # Internal attributes
         "_context",
-        "_local_root",
+        "_local_root_value",
         "_parent",
         "_ignored_exceptions",
         "_on_finish_callbacks",
@@ -206,7 +206,7 @@ def __init__(
         self._events: List[SpanEvent] = []
         self._parent: Optional["Span"] = None
         self._ignored_exceptions: Optional[List[Type[Exception]]] = None
-        self._local_root: Optional["Span"] = None
+        self._local_root_value: Optional["Span"] = None  # None means this is the root span.
         self._store: Optional[Dict[str, Any]] = None
 
     def _ignore_exception(self, exc: Type[Exception]) -> None:
@@ -595,6 +595,23 @@ def context(self) -> Context:
             self._context = Context(trace_id=self.trace_id, span_id=self.span_id, is_remote=False)
         return self._context
 
+    @property
+    def _local_root(self) -> "Span":
+        if self._local_root_value is None:
+            return self
+        return self._local_root_value
+
+    @_local_root.setter
+    def _local_root(self, value: "Span") -> None:
+        if value is not self:
+            self._local_root_value = value
+        else:
+            self._local_root_value = None
+
+    @_local_root.deleter
+    def _local_root(self) -> None:
+        del self._local_root_value
+
     def link_span(self, context: Context, attributes: Optional[Dict[str, Any]] = None) -> None:
         """Defines a causal relationship between two spans"""
         if not context.trace_id or not context.span_id:

ddtrace/_trace/tracer.py

@@ -797,8 +797,6 @@ def _start_span(
                 span._parent = parent
                 span._local_root = parent._local_root
 
-            if span._local_root is None:
-                span._local_root = span
             for k, v in _get_metas_to_propagate(context):
                 # We do not want to propagate AppSec propagation headers
                 # to children spans, only across distributed spans
@@ -815,7 +813,6 @@ def _start_span(
                 span_api=span_api,
                 on_finish=[self._on_span_finish],
             )
-            span._local_root = span
             if config.report_hostname:
                 span.set_tag_str(HOSTNAME_KEY, hostname.get_hostname())
 

ddtrace/propagation/http.py

@@ -988,6 +988,7 @@ def parent_call():
             span_context = non_active_span.context
 
         if hasattr(ddtrace, "tracer") and hasattr(ddtrace.tracer, "sample"):
+            root_span: Optional[Span] = None
             if non_active_span is not None:
                 root_span = non_active_span._local_root
             else:

releasenotes/notes/fix-span-unnecessary-gc.yaml

@@ -0,0 +1,4 @@
+---
+fixes:
+  - |
+    tracing: Removes a reference cycle that caused unnecessary garbage collection for top-level spans.

tests/tracer/test_tracer.py

@@ -2057,3 +2057,23 @@ def test_asm_standalone_configuration():
     assert tracer._compute_stats is False
     # reset tracer values
     tracer.configure(appsec_enabled=False, appsec_standalone_enabled=False)
+
+
+def test_gc_not_used_on_root_spans():
+    tracer = ddtrace.Tracer()
+    gc.freeze()
+
+    with tracer.trace("test-event"):
+        pass
+
+    # There should be no more span objects lingering around.
+    assert not any(str(obj).startswith("<Span") for obj in gc.get_objects())
+
+    # To check the exact nature of the objects and their references, use the following:
+
+    # for i, obj in enumerate(objects):
+    #     print("--------------------")
+    #     print(f"object {i}:", obj)
+    #     print("referrers:", [f"object {objects.index(r)}" for r in gc.get_referrers(obj)[:-2]])
+    #     print("referents:", [f"object {objects.index(r)}" if r in objects else r for r in gc.get_referents(obj)])
+    #     print("--------------------")