From 88884502bbcb378ee72a9e45622883541bd4270e Mon Sep 17 00:00:00 2001 From: datadog-bits <263423550+datadog-bits@users.noreply.github.com> Date: Tue, 29 Sep 2026 08:05:37 +0000 Subject: [PATCH 1/7] fix(ci_visibility): guard coverage stack exits Co-authored-by: gnufede <412857+gnufede@users.noreply.github.com> --- ddtrace/internal/coverage/code.py | 42 ++++++++++------ ...lity-copied-context-coverage-f8c173bd.yaml | 7 +++ tests/coverage/test_coverage.py | 48 +++++++++++++++++++ 3 files changed, 82 insertions(+), 15 deletions(-) create mode 100644 releasenotes/notes/fix-ci-visibility-copied-context-coverage-f8c173bd.yaml diff --git a/ddtrace/internal/coverage/code.py b/ddtrace/internal/coverage/code.py index 496cb5dd5c5..8b9ca14ff96 100644 --- a/ddtrace/internal/coverage/code.py +++ b/ddtrace/internal/coverage/code.py @@ -49,7 +49,7 @@ def _is_site_packages_path(path: Path) -> bool: # NOTE: A mutable ContextVar default would be shared across threads until set() is called. -# Keep None so CollectInContext initializes a separate coverage stack in each context. +# Keep None so CollectInContext starts a separate coverage stack in each context. ctx_covered: ContextVar[t.Optional[list[defaultdict[str, CoverageLines]]]] = ContextVar("ctx_covered", default=None) ctx_covered_files: ContextVar[t.Optional[list[set[str]]]] = ContextVar("ctx_covered_files", default=None) ctx_is_import_coverage = ContextVar("ctx_is_import_coverage", default=False) @@ -373,14 +373,14 @@ def _get_covered_file_paths_with_imports(self, covered_file_paths: set[str]) -> class CollectInContext: def __init__(self, is_import_coverage: bool = False): self.is_import_coverage = is_import_coverage - if ctx_covered.get() is None: - ctx_covered.set([]) - if ctx_covered_files.get() is None: - ctx_covered_files.set([]) def __enter__(self): - ctx_covered.get().append(defaultdict(CoverageLines)) - ctx_covered_files.get().append(set()) + # ContextVar values are copied by reference into new execution contexts. + # Replace the stacks so a nested collector cannot mutate its parent's stack. + self._covered_lines = defaultdict(CoverageLines) + self._covered_files = set() + ctx_covered.set((ctx_covered.get() or []) + [self._covered_lines]) + ctx_covered_files.set((ctx_covered_files.get() or []) + [self._covered_files]) ctx_coverage_enabled.set(True) if self.is_import_coverage: @@ -412,13 +412,25 @@ def __enter__(self): return self def __exit__(self, *args, **kwargs): - covered_lines_stack = ctx_covered.get() - covered_files_stack = ctx_covered_files.get() - covered_lines_stack.pop() - covered_files_stack.pop() + covered_lines_stack = ctx_covered.get() or [] + covered_files_stack = ctx_covered_files.get() or [] + if ( + covered_lines_stack + and covered_files_stack + and covered_lines_stack[-1] is self._covered_lines + and covered_files_stack[-1] is self._covered_files + ): + covered_lines_stack = covered_lines_stack[:-1] + covered_files_stack = covered_files_stack[:-1] + ctx_covered.set(covered_lines_stack) + ctx_covered_files.set(covered_files_stack) + else: + # A copied context may finish a collector that was entered elsewhere. + # Leave this context's collector intact instead of popping the wrong one. + return # Stop coverage if we're exiting the last context - if len(covered_lines_stack) == 0: + if not covered_lines_stack: ctx_coverage_enabled.set(False) if _PY_GE_314: _tls_coverage.covered = None @@ -428,7 +440,7 @@ def __exit__(self, *args, **kwargs): _tls_coverage.covered_files = covered_files_stack[-1] def get_covered_lines(self) -> dict[str, CoverageLines]: - covered_lines = _get_ctx_covered_lines() + covered_lines = self._covered_lines if global_instance := ModuleCodeCollector._instance: global_instance._add_import_time_lines(covered_lines) return covered_lines @@ -437,8 +449,8 @@ def get_covered_file_paths(self) -> t.AbstractSet[str]: # Python < 3.12 and injected child-process coverage may only update the line-oriented # context data. Merge those keys into the file set so file-level uploads still include # every file that would have been emitted by get_covered_lines(). - covered_file_paths = set(_get_ctx_covered_files()) - covered_file_paths.update(_get_ctx_covered_lines()) + covered_file_paths = set(self._covered_files) + covered_file_paths.update(self._covered_lines) if global_instance := ModuleCodeCollector._instance: return global_instance._get_covered_file_paths_with_imports(covered_file_paths) return covered_file_paths diff --git a/releasenotes/notes/fix-ci-visibility-copied-context-coverage-f8c173bd.yaml b/releasenotes/notes/fix-ci-visibility-copied-context-coverage-f8c173bd.yaml new file mode 100644 index 00000000000..74503a63a0d --- /dev/null +++ b/releasenotes/notes/fix-ci-visibility-copied-context-coverage-f8c173bd.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + CI Visibility: Fixes an issue where pytest runs using copied or restored execution + contexts (e.g. Django async tests via asgiref's ``async_to_sync``) could end with an + internal coverage error, or silently lose the coverage data of the tests that use + them. diff --git a/tests/coverage/test_coverage.py b/tests/coverage/test_coverage.py index 638980d1bac..2bc9a3c4ce8 100644 --- a/tests/coverage/test_coverage.py +++ b/tests/coverage/test_coverage.py @@ -12,6 +12,54 @@ import pytest +def test_coverage_stacks_are_isolated_across_copied_contexts(): + from contextvars import copy_context + + from ddtrace.internal.coverage.code import ModuleCodeCollector + from ddtrace.internal.coverage.code import ctx_covered + from ddtrace.internal.coverage.code import ctx_covered_files + + with ModuleCodeCollector.CollectInContext(): + parent_lines_stack = ctx_covered.get() + parent_files_stack = ctx_covered_files.get() + parent_depth = len(parent_lines_stack) + child_context = copy_context() + + def collect_in_child_context(): + with ModuleCodeCollector.CollectInContext(): + assert len(ctx_covered.get()) == len(ctx_covered_files.get()) == parent_depth + 1 + assert ctx_covered.get()[-1] is not parent_lines_stack[-1] + assert ctx_covered_files.get()[-1] is not parent_files_stack[-1] + + child_context.run(collect_in_child_context) + assert ctx_covered.get() is parent_lines_stack + assert ctx_covered_files.get() is parent_files_stack + assert len(parent_lines_stack) == len(parent_files_stack) == parent_depth + + +def test_exiting_collector_in_another_context_preserves_active_coverage(): + from contextvars import copy_context + + from ddtrace.internal.coverage.code import ModuleCodeCollector + from ddtrace.internal.coverage.code import ctx_covered + from ddtrace.internal.coverage.code import ctx_covered_files + + with ModuleCodeCollector.CollectInContext(): + parent_depth = len(ctx_covered.get()) + parent_lines = ctx_covered.get()[-1] + parent_files = ctx_covered_files.get()[-1] + child_context = copy_context() + child = ModuleCodeCollector.CollectInContext() + child_context.run(child.__enter__) + + child.__exit__() + assert ctx_covered.get()[-1] is parent_lines + assert ctx_covered_files.get()[-1] is parent_files + + child_context.run(child.__exit__) + assert len(ctx_covered.get()) == len(ctx_covered_files.get()) == parent_depth + + @pytest.mark.skipif(sys.version_info < (3, 12), reason="Test specific to Python 3.12+ monitoring API") @pytest.mark.subprocess() def test_coverage_defaults_to_file_level_when_env_unset(): From a89ce3907f115266a36bd9784a53ed4ae493c9ed Mon Sep 17 00:00:00 2001 From: "federico.mon" Date: Tue, 6 Oct 2026 16:58:08 +0000 Subject: [PATCH 2/7] fix(ci_visibility): compare coverage stacks by identity for context restores The guarded stack exits from the previous commit prevent the crash caused by stack ownership moving between execution contexts, but a second failure mode remains: value-based context restoration (e.g. asgiref's ``_restore_context``, used by Django's async test support via ``async_to_sync``) compares ContextVar values with ``!=``. Plain list stacks compare by value, so two distinct stacks holding equal entries compare equal and the restore is masked: the asyncio task keeps the loop-thread wrapper's stack, the test body's coverage data is recorded into the wrapper's entry, and the per-test collector reads its own (now empty) entry. The final restore, running before the executor thread join, then replaces the caller's stack with the wrapper's, stranding the data: async tests silently report no coverage (and can never be selected for skipping under ITR). Make the per-context stacks compare by identity so value-based restores propagate the correct stack: distinct stacks are never equal (restores are no longer masked, so the task starts with the caller's stack and coverage is attributed to the right test), and a context already holding the exact stack object ignores further restores of it (no forced swap, push/pop pairing stays balanced). Verified against Django's test suite (pytest + CI Visibility coverage, asgiref ``async_to_sync``/``sync_to_async``): both line-level and file-level coverage now complete the full run (2035 tests) with no internal errors, and async tests report non-empty coverage again. Co-authored-by: Claude --- ddtrace/internal/coverage/code.py | 32 +++- ...st_coverage_asgiref_context_propagation.py | 153 ++++++++++++++++++ 2 files changed, 181 insertions(+), 4 deletions(-) create mode 100644 tests/coverage/test_coverage_asgiref_context_propagation.py diff --git a/ddtrace/internal/coverage/code.py b/ddtrace/internal/coverage/code.py index 8b9ca14ff96..b1ce7e5d844 100644 --- a/ddtrace/internal/coverage/code.py +++ b/ddtrace/internal/coverage/code.py @@ -370,6 +370,28 @@ def _get_covered_file_paths_with_imports(self, covered_file_paths: set[str]) -> self._file_level_covered_paths_cache.popitem(last=False) return paths + class _ContextStack(list): + """Per-context stack of coverage data that compares by identity, not value. + + Context-propagation helpers (e.g. asgiref's ``_restore_context``, used by Django's + async test support via ``async_to_sync``/``sync_to_async``) restore context + variables by comparing the current value with the incoming one using ``!=``. + A plain ``list`` compares by value, which both silently masks legitimate stack + swaps (when two distinct stacks happen to contain equal entries) and allows one + context's stack to be replaced by another context's stack object. Comparing + stacks by identity makes such propagation respect stack ownership: restores only + propagate a stack reference into a context that does not already hold that exact + stack object, keeping the coverage data attributed to the right context. + """ + + __slots__ = () + + def __eq__(self, other: object) -> bool: # noqa: D105 + return self is other + + def __ne__(self, other: object) -> bool: # noqa: D105 + return self is not other + class CollectInContext: def __init__(self, is_import_coverage: bool = False): self.is_import_coverage = is_import_coverage @@ -379,8 +401,10 @@ def __enter__(self): # Replace the stacks so a nested collector cannot mutate its parent's stack. self._covered_lines = defaultdict(CoverageLines) self._covered_files = set() - ctx_covered.set((ctx_covered.get() or []) + [self._covered_lines]) - ctx_covered_files.set((ctx_covered_files.get() or []) + [self._covered_files]) + ctx_covered.set(ModuleCodeCollector._ContextStack((ctx_covered.get() or []) + [self._covered_lines])) + ctx_covered_files.set( + ModuleCodeCollector._ContextStack((ctx_covered_files.get() or []) + [self._covered_files]) + ) ctx_coverage_enabled.set(True) if self.is_import_coverage: @@ -420,8 +444,8 @@ def __exit__(self, *args, **kwargs): and covered_lines_stack[-1] is self._covered_lines and covered_files_stack[-1] is self._covered_files ): - covered_lines_stack = covered_lines_stack[:-1] - covered_files_stack = covered_files_stack[:-1] + covered_lines_stack = ModuleCodeCollector._ContextStack(covered_lines_stack[:-1]) + covered_files_stack = ModuleCodeCollector._ContextStack(covered_files_stack[:-1]) ctx_covered.set(covered_lines_stack) ctx_covered_files.set(covered_files_stack) else: diff --git a/tests/coverage/test_coverage_asgiref_context_propagation.py b/tests/coverage/test_coverage_asgiref_context_propagation.py new file mode 100644 index 00000000000..ac3447fdf1a --- /dev/null +++ b/tests/coverage/test_coverage_asgiref_context_propagation.py @@ -0,0 +1,153 @@ +"""Regression tests for per-context coverage stack corruption caused by +value-based context restoration. + +Django's async test support (asgiref ``async_to_sync``/``sync_to_async``) +runs a new event loop in a worker thread, which ddtrace's threading +integration wraps in its own coverage context. The framework then restores +context variable values between the caller, the worker thread and the task +using value-based comparisons (``cvar.get() != cvalue`` in asgiref's +``_restore_context``). + +Because the per-context coverage stacks used to be plain lists (compared by +value), such restores could replace one context's stack with another +context's stack object. That corrupted the push/pop pairing of +``CollectInContext``, crashing the caller with ``IndexError: pop from empty +list`` and mis-attributing coverage data between contexts. +""" + +import importlib.util + +import pytest + + +# The first test hand-rolls the context-propagation mechanism and has no +# dependency on asgiref. The second exercises the real library and is skipped +# when it is not installed. +HAS_ASGIREF = importlib.util.find_spec("asgiref") is not None + + +@pytest.mark.subprocess(env={"_DD_COVERAGE_FILE_LEVEL": "false"}) +def test_coverage_context_thread_value_based_context_restore(): + import contextvars + import os + from pathlib import Path + import threading + + from ddtrace.internal.coverage.code import ModuleCodeCollector + from ddtrace.internal.coverage.installer import install + from tests.coverage.utils import _get_relpath_dict + + cwd = os.getcwd() + + include_paths = [Path(cwd) / "tests/coverage/included_path/"] + install(include_paths=include_paths) + + # Import before entering the context so module-level lines are not included + from tests.coverage.included_path.callee import called_in_context_main + + def restore_context_values(context): + # Mirrors asgiref.sync._restore_context (and similar value-based + # ContextVar propagation helpers): a restore is skipped whenever the + # current and incoming values compare equal, and applied otherwise. + for cvar in context: + cvalue = context.get(cvar) + try: + if cvar.get() != cvalue: + cvar.set(cvalue) + except LookupError: + cvar.set(cvalue) + + context_collector = ModuleCodeCollector.CollectInContext() + context_collector.__enter__() + try: + # The caller's context, captured while the per-test coverage context + # is active (fresh empty coverage entries, as at the start of a test). + caller_context = contextvars.copy_context() + + task_context_holder = {} + + def thread_body(): + # ddtrace's threading integration wraps this thread in its own + # coverage context, entered in the thread's fresh base context. + # Simulate the framework task inheriting the thread's context and + # the caller's context values being restored into it. + task_context = contextvars.copy_context() + + def task_body(): + restore_context_values(caller_context) + called_in_context_main(1, 2) + + task_context.run(task_body) + task_context_holder["task"] = task_context + + thread = threading.Thread(target=thread_body) + thread.start() + thread.join() + + # Simulate the framework restoring the task context back into the + # caller (as asgiref's AsyncToSync does at the end of an async call). + restore_context_values(task_context_holder["task"]) + + context_covered = _get_relpath_dict(cwd, context_collector.get_covered_lines()) + finally: + # Regression: this used to raise ``IndexError: pop from empty list`` + # (or ``list index out of range``) after the value-based restore + # replaced this context's stack with the worker thread's (already + # popped) stack. + context_collector.__exit__() + + expected_lines = { + "tests/coverage/included_path/callee.py": {10, 11, 13, 14}, + "tests/coverage/included_path/in_context_lib.py": {1, 2, 5}, + } + + assert expected_lines == context_covered, f"Mismatched lines: {expected_lines} vs {context_covered}" + + +@pytest.mark.skipif(not HAS_ASGIREF, reason="asgiref is not installed") +@pytest.mark.subprocess(env={"_DD_COVERAGE_FILE_LEVEL": "false"}) +def test_coverage_context_thread_async_to_sync(): + import os + from pathlib import Path + + from asgiref.sync import async_to_sync + from asgiref.sync import sync_to_async + + from ddtrace.internal.coverage.code import ModuleCodeCollector + from ddtrace.internal.coverage.installer import install + from tests.coverage.utils import _get_relpath_dict + + cwd = os.getcwd() + + include_paths = [Path(cwd) / "tests/coverage/included_path/"] + install(include_paths=include_paths) + + # Import before entering the context so module-level lines are not included + from tests.coverage.included_path.callee import called_in_context_main + + def sync_fn(a, b): + called_in_context_main(a, b) + + async def async_fn(a, b): + # Simulate a Django async test body performing sync work via + # sync_to_async (thread-sensitive mode uses the CurrentThreadExecutor + # in the calling thread). + await sync_to_async(sync_fn)(a, b) + + context_collector = ModuleCodeCollector.CollectInContext() + context_collector.__enter__() + try: + async_to_sync(async_fn)(1, 2) + context_covered = _get_relpath_dict(cwd, context_collector.get_covered_lines()) + finally: + # Regression: this used to raise ``IndexError: pop from empty list`` + # after asgiref restored a value-equal (but different) coverage stack + # into this context. + context_collector.__exit__() + + expected_lines = { + "tests/coverage/included_path/callee.py": {10, 11, 13, 14}, + "tests/coverage/included_path/in_context_lib.py": {1, 2, 5}, + } + + assert expected_lines == context_covered, f"Mismatched lines: {expected_lines} vs {context_covered}" From d77e0b134a7ea74983a76da4b33cdc149bd18e96 Mon Sep 17 00:00:00 2001 From: "federico.mon" Date: Tue, 6 Oct 2026 17:24:08 +0000 Subject: [PATCH 3/7] fix(ci_visibility): annotate _ContextStack as list[Any] The stacks hold different element types (defaultdict[str, CoverageLines] for lines, set[str] for files), so parameterize the generic base with Any to satisfy mypy's type-arg check without duplicating the class. --- ddtrace/internal/coverage/code.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ddtrace/internal/coverage/code.py b/ddtrace/internal/coverage/code.py index b1ce7e5d844..f2da569f139 100644 --- a/ddtrace/internal/coverage/code.py +++ b/ddtrace/internal/coverage/code.py @@ -370,7 +370,7 @@ def _get_covered_file_paths_with_imports(self, covered_file_paths: set[str]) -> self._file_level_covered_paths_cache.popitem(last=False) return paths - class _ContextStack(list): + class _ContextStack(list[t.Any]): """Per-context stack of coverage data that compares by identity, not value. Context-propagation helpers (e.g. asgiref's ``_restore_context``, used by Django's From c084f7c30a4e92b48d33f455cc067a3ef11e5594 Mon Sep 17 00:00:00 2001 From: "federico.mon" Date: Tue, 6 Oct 2026 18:26:39 +0000 Subject: [PATCH 4/7] fix(ci_visibility): re-sync TLS coverage fallback on mismatched exits On Python 3.14+, sys.monitoring callbacks run in a snapshot context and fall back to the thread-local coverage state when they cannot observe ContextVar changes. When a collector entered in a copied context is exited from a context whose stack top is a different collector, the early return left _tls_coverage pointing at the completed collector, so subsequent instrumented code in that thread was recorded into the stale entry instead of the active collector. Re-sync the fallback to the current stack top (or clear it) before returning, mirroring what the pop path already does. --- ddtrace/internal/coverage/code.py | 9 +++++++ tests/coverage/test_coverage.py | 40 +++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+) diff --git a/ddtrace/internal/coverage/code.py b/ddtrace/internal/coverage/code.py index f2da569f139..3500ae99b81 100644 --- a/ddtrace/internal/coverage/code.py +++ b/ddtrace/internal/coverage/code.py @@ -451,6 +451,15 @@ def __exit__(self, *args, **kwargs): else: # A copied context may finish a collector that was entered elsewhere. # Leave this context's collector intact instead of popping the wrong one. + if _PY_GE_314: + # The exited collector may still be this thread's TLS fallback: the + # sys.monitoring callbacks fall back to this thread-local state when + # their snapshot context cannot observe ContextVar changes. Re-sync + # the fallback to the collector that is actually active in this + # context (or clear it) so coverage keeps being attributed to the + # right entry instead of the completed collector. + _tls_coverage.covered = covered_lines_stack[-1] if covered_lines_stack else None + _tls_coverage.covered_files = covered_files_stack[-1] if covered_files_stack else None return # Stop coverage if we're exiting the last context diff --git a/tests/coverage/test_coverage.py b/tests/coverage/test_coverage.py index 2bc9a3c4ce8..e2f049b9646 100644 --- a/tests/coverage/test_coverage.py +++ b/tests/coverage/test_coverage.py @@ -60,6 +60,46 @@ def test_exiting_collector_in_another_context_preserves_active_coverage(): assert len(ctx_covered.get()) == len(ctx_covered_files.get()) == parent_depth +def test_mismatched_exit_resyncs_tls_fallback(): + """A mismatched exit must not leave the TLS fallback on the completed collector. + + On Python 3.14+, sys.monitoring callbacks run in a snapshot context and fall back + to the thread-local coverage state when they cannot observe ContextVar changes. + When a collector entered in a copied context is exited from a different context + (a mismatched exit), that thread-local fallback must be re-synced to the active + collector instead of pointing at the collector that just completed. + """ + from contextvars import copy_context + + import ddtrace.internal.coverage.code as coverage_code + from ddtrace.internal.coverage.code import ModuleCodeCollector + from ddtrace.internal.coverage.code import ctx_covered + + original_flag = coverage_code._PY_GE_314 + coverage_code._PY_GE_314 = True + try: + with ModuleCodeCollector.CollectInContext(): + parent_lines = ctx_covered.get()[-1] + parent_files = coverage_code.ctx_covered_files.get()[-1] + child_context = copy_context() + child = ModuleCodeCollector.CollectInContext() + child_context.run(child.__enter__) + + # The thread-local fallback tracks the most recent collector entered in + # this thread, which is the child's. + assert coverage_code._tls_coverage.covered is child._covered_lines + + # Exiting the child from the parent context is a mismatched exit. + child.__exit__() + + # The fallback must be re-synced to the parent's active entries rather + # than left pointing at the completed child collector. + assert coverage_code._tls_coverage.covered is parent_lines + assert coverage_code._tls_coverage.covered_files is parent_files + finally: + coverage_code._PY_GE_314 = original_flag + + @pytest.mark.skipif(sys.version_info < (3, 12), reason="Test specific to Python 3.12+ monitoring API") @pytest.mark.subprocess() def test_coverage_defaults_to_file_level_when_env_unset(): From 34ea199d80e986c7fd4eb39a3b3a98f396ec1ac0 Mon Sep 17 00:00:00 2001 From: "federico.mon" Date: Tue, 6 Oct 2026 19:40:09 +0000 Subject: [PATCH 5/7] fix(ci_visibility): skip completed collectors when resolving coverage entries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A context copied while a collector was active (eg. a task scheduled by a module imported inside a test) keeps a reference to a stack that still contains that collector's entry after it completes. Coverage recorded in such a context was written into the completed collector's entry — one that after_import() had already consumed — instead of the enclosing active collector, so the data was silently lost. Mark each collector's entries as closed when it exits, and make the context resolvers (and the Python 3.14+ TLS fallback) walk down the stack to the nearest entry that is still open, attributing new coverage to the closest active collector instead of an orphaned entry. --- ddtrace/internal/coverage/code.py | 56 +++++++++++++++++++++++++------ tests/coverage/test_coverage.py | 43 ++++++++++++++++++++++++ 2 files changed, 89 insertions(+), 10 deletions(-) diff --git a/ddtrace/internal/coverage/code.py b/ddtrace/internal/coverage/code.py index 3500ae99b81..f36d31c0423 100644 --- a/ddtrace/internal/coverage/code.py +++ b/ddtrace/internal/coverage/code.py @@ -48,10 +48,29 @@ def _is_site_packages_path(path: Path) -> bool: return not _SITE_PACKAGES_DIRNAMES.isdisjoint(path.parts) +class _ContextLinesEntry(defaultdict[str, CoverageLines]): + """Coverage lines container for one collector, tracking whether it completed. + + Execution contexts copied while a collector is active keep a reference to a + stack that still contains its entry (eg. a task scheduled by a module that is + imported inside a test). Marking the entry ``closed`` when the collector + finishes lets the context resolvers skip completed collectors and attribute new + coverage to the nearest collector that is still active. + """ + + closed: bool = False + + +class _ContextFilesEntry(set[str]): + """File-level counterpart of ``_ContextLinesEntry``.""" + + closed: bool = False + + # NOTE: A mutable ContextVar default would be shared across threads until set() is called. # Keep None so CollectInContext starts a separate coverage stack in each context. -ctx_covered: ContextVar[t.Optional[list[defaultdict[str, CoverageLines]]]] = ContextVar("ctx_covered", default=None) -ctx_covered_files: ContextVar[t.Optional[list[set[str]]]] = ContextVar("ctx_covered_files", default=None) +ctx_covered: ContextVar[t.Optional[list[_ContextLinesEntry]]] = ContextVar("ctx_covered", default=None) +ctx_covered_files: ContextVar[t.Optional[list[_ContextFilesEntry]]] = ContextVar("ctx_covered_files", default=None) ctx_is_import_coverage = ContextVar("ctx_is_import_coverage", default=False) ctx_coverage_enabled = ContextVar("ctx_coverage_enabled", default=False) @@ -63,13 +82,20 @@ def _is_site_packages_path(path: Path) -> bool: def _get_ctx_covered_lines() -> defaultdict[str, CoverageLines]: if ctx_coverage_enabled.get(): if context_stack := ctx_covered.get(): - return context_stack[-1] - log.debug("_get_ctx_covered_lines() called but ctx_covered stack is empty") + for entry in reversed(context_stack): + if not entry.closed: + return entry + # Every entry on this stack belongs to a collector that has completed: + # this context inherited the stack before they exited. Fall through to + # the TLS fallback / an empty container instead of attributing new + # coverage to a completed collector. + else: + log.debug("_get_ctx_covered_lines() called but ctx_covered stack is empty") # Fallback for Python 3.14+ where sys.monitoring callbacks can't see ContextVars if _PY_GE_314: tls_covered = getattr(_tls_coverage, "covered", None) - if tls_covered is not None: + if tls_covered is not None and not tls_covered.closed: return tls_covered return defaultdict(CoverageLines) @@ -78,12 +104,15 @@ def _get_ctx_covered_lines() -> defaultdict[str, CoverageLines]: def _get_ctx_covered_files() -> set[str]: if ctx_coverage_enabled.get(): if context_stack := ctx_covered_files.get(): - return context_stack[-1] - log.debug("_get_ctx_covered_files() called but ctx_covered_files stack is empty") + for entry in reversed(context_stack): + if not entry.closed: + return entry + else: + log.debug("_get_ctx_covered_files() called but ctx_covered_files stack is empty") if _PY_GE_314: tls_covered_files = getattr(_tls_coverage, "covered_files", None) - if tls_covered_files is not None: + if tls_covered_files is not None and not tls_covered_files.closed: return tls_covered_files return set() @@ -399,8 +428,8 @@ def __init__(self, is_import_coverage: bool = False): def __enter__(self): # ContextVar values are copied by reference into new execution contexts. # Replace the stacks so a nested collector cannot mutate its parent's stack. - self._covered_lines = defaultdict(CoverageLines) - self._covered_files = set() + self._covered_lines = _ContextLinesEntry(CoverageLines) + self._covered_files = _ContextFilesEntry() ctx_covered.set(ModuleCodeCollector._ContextStack((ctx_covered.get() or []) + [self._covered_lines])) ctx_covered_files.set( ModuleCodeCollector._ContextStack((ctx_covered_files.get() or []) + [self._covered_files]) @@ -436,6 +465,13 @@ def __enter__(self): return self def __exit__(self, *args, **kwargs): + # The collector is completing. Stacks inherited by other contexts (copied + # while this collector was active) still reference its entry: mark it + # closed so the resolvers skip it and attribute new coverage to the + # nearest active collector instead of this completed one. + self._covered_lines.closed = True + self._covered_files.closed = True + covered_lines_stack = ctx_covered.get() or [] covered_files_stack = ctx_covered_files.get() or [] if ( diff --git a/tests/coverage/test_coverage.py b/tests/coverage/test_coverage.py index e2f049b9646..ec5ccb3313c 100644 --- a/tests/coverage/test_coverage.py +++ b/tests/coverage/test_coverage.py @@ -60,6 +60,49 @@ def test_exiting_collector_in_another_context_preserves_active_coverage(): assert len(ctx_covered.get()) == len(ctx_covered_files.get()) == parent_depth +def test_completed_collector_entries_do_not_capture_inherited_context_coverage(): + """Contexts that inherit a stack still holding a completed entry must not write to it. + + A module imported inside a test may create an asyncio task before finishing its + import collector. The task inherits the test's context, whose stack still + references the (now completed) import entry. New coverage in the task must be + attributed to the enclosing live collector instead of the orphaned entry. + """ + from contextvars import copy_context + + import ddtrace.internal.coverage.code as coverage_code + from ddtrace.internal.coverage.code import ModuleCodeCollector + + with ModuleCodeCollector.CollectInContext() as test_collector: + with ModuleCodeCollector.CollectInContext() as import_collector: + task_context = copy_context() + + assert import_collector._covered_lines.closed + + # The task context still sees the completed import entry atop its stack. + task_stack = task_context.run(coverage_code.ctx_covered.get) + assert task_stack[-1] is import_collector._covered_lines + + # Resolution inside the task context must skip the completed entry and + # attribute coverage to the still-active test collector. + assert task_context.run(coverage_code._get_ctx_covered_lines) is test_collector._covered_lines + assert task_context.run(coverage_code._get_ctx_covered_files) is test_collector._covered_files + + # A live collector entered in the task context takes precedence even though + # the completed import entry remains buried beneath it on the stack. + nested = ModuleCodeCollector.CollectInContext() + task_context.run(nested.__enter__) + assert task_context.run(coverage_code._get_ctx_covered_lines) is nested._covered_lines + task_context.run(nested.__exit__) + assert task_context.run(coverage_code._get_ctx_covered_lines) is test_collector._covered_lines + + # Once every collector the task inherited has completed, new coverage lands in + # a fresh container rather than in any of the completed entries. + stale = task_context.run(coverage_code._get_ctx_covered_lines) + assert stale is not import_collector._covered_lines + assert stale is not test_collector._covered_lines + + def test_mismatched_exit_resyncs_tls_fallback(): """A mismatched exit must not leave the TLS fallback on the completed collector. From d69c01c26040a71d84473eef9412dd9dcb1fad7f Mon Sep 17 00:00:00 2001 From: "federico.mon" Date: Tue, 6 Oct 2026 19:53:32 +0000 Subject: [PATCH 6/7] chore(ci_visibility): use plain prose in internal coverage docstrings The coverage module is private (nothing under docs/ renders it), so docstrings and comments should read as plain text per the repository convention in AGENTS.md: drop the rST double-backtick literals and write the names as-is. --- ddtrace/internal/coverage/code.py | 24 +++++++++---------- ...st_coverage_asgiref_context_propagation.py | 20 ++++++++-------- 2 files changed, 22 insertions(+), 22 deletions(-) diff --git a/ddtrace/internal/coverage/code.py b/ddtrace/internal/coverage/code.py index f36d31c0423..cb4722d91c7 100644 --- a/ddtrace/internal/coverage/code.py +++ b/ddtrace/internal/coverage/code.py @@ -53,8 +53,8 @@ class _ContextLinesEntry(defaultdict[str, CoverageLines]): Execution contexts copied while a collector is active keep a reference to a stack that still contains its entry (eg. a task scheduled by a module that is - imported inside a test). Marking the entry ``closed`` when the collector - finishes lets the context resolvers skip completed collectors and attribute new + imported inside a test). Marking the entry closed when the collector finishes + lets the context resolvers skip completed collectors and attribute new coverage to the nearest collector that is still active. """ @@ -62,7 +62,7 @@ class _ContextLinesEntry(defaultdict[str, CoverageLines]): class _ContextFilesEntry(set[str]): - """File-level counterpart of ``_ContextLinesEntry``.""" + """File-level counterpart of _ContextLinesEntry.""" closed: bool = False @@ -402,15 +402,15 @@ def _get_covered_file_paths_with_imports(self, covered_file_paths: set[str]) -> class _ContextStack(list[t.Any]): """Per-context stack of coverage data that compares by identity, not value. - Context-propagation helpers (e.g. asgiref's ``_restore_context``, used by Django's - async test support via ``async_to_sync``/``sync_to_async``) restore context - variables by comparing the current value with the incoming one using ``!=``. - A plain ``list`` compares by value, which both silently masks legitimate stack - swaps (when two distinct stacks happen to contain equal entries) and allows one - context's stack to be replaced by another context's stack object. Comparing - stacks by identity makes such propagation respect stack ownership: restores only - propagate a stack reference into a context that does not already hold that exact - stack object, keeping the coverage data attributed to the right context. + Context-propagation helpers (e.g. asgiref's _restore_context, used by Django's + async test support via async_to_sync/sync_to_async) restore context variables + by comparing the current value with the incoming one using !=. A plain list + compares by value, which both silently masks legitimate stack swaps (when + two distinct stacks happen to contain equal entries) and allows one context's + stack to be replaced by another context's stack object. Comparing stacks by + identity makes such propagation respect stack ownership: restores only + propagate a stack reference into a context that does not already hold that + exact stack object, keeping the coverage data attributed to the right context. """ __slots__ = () diff --git a/tests/coverage/test_coverage_asgiref_context_propagation.py b/tests/coverage/test_coverage_asgiref_context_propagation.py index ac3447fdf1a..1ef74d68a7b 100644 --- a/tests/coverage/test_coverage_asgiref_context_propagation.py +++ b/tests/coverage/test_coverage_asgiref_context_propagation.py @@ -1,18 +1,18 @@ """Regression tests for per-context coverage stack corruption caused by value-based context restoration. -Django's async test support (asgiref ``async_to_sync``/``sync_to_async``) +Django's async test support (asgiref async_to_sync/sync_to_async) runs a new event loop in a worker thread, which ddtrace's threading integration wraps in its own coverage context. The framework then restores context variable values between the caller, the worker thread and the task -using value-based comparisons (``cvar.get() != cvalue`` in asgiref's -``_restore_context``). +using value-based comparisons (cvar.get() != cvalue in asgiref's +_restore_context). Because the per-context coverage stacks used to be plain lists (compared by -value), such restores could replace one context's stack with another -context's stack object. That corrupted the push/pop pairing of -``CollectInContext``, crashing the caller with ``IndexError: pop from empty -list`` and mis-attributing coverage data between contexts. +value), such restores could replace one context's stack with another context's +stack object. That corrupted the push/pop pairing of CollectInContext, crashing +the caller with IndexError: pop from empty list and mis-attributing coverage +data between contexts. """ import importlib.util @@ -90,8 +90,8 @@ def task_body(): context_covered = _get_relpath_dict(cwd, context_collector.get_covered_lines()) finally: - # Regression: this used to raise ``IndexError: pop from empty list`` - # (or ``list index out of range``) after the value-based restore + # Regression: this used to raise IndexError: pop from empty list + # (or list index out of range) after the value-based restore # replaced this context's stack with the worker thread's (already # popped) stack. context_collector.__exit__() @@ -140,7 +140,7 @@ async def async_fn(a, b): async_to_sync(async_fn)(1, 2) context_covered = _get_relpath_dict(cwd, context_collector.get_covered_lines()) finally: - # Regression: this used to raise ``IndexError: pop from empty list`` + # Regression: this used to raise IndexError: pop from empty list # after asgiref restored a value-equal (but different) coverage stack # into this context. context_collector.__exit__() From 3cf0cb7648b853c7fe828484415682d65878e526 Mon Sep 17 00:00:00 2001 From: Federico Mon Date: Wed, 7 Oct 2026 17:55:29 +0200 Subject: [PATCH 7/7] fix(ci_visibility): fix coverage crash and data loss with copied/restored execution contexts (#20864) ## Description CI Visibility coverage can abort pytest sessions or silently lose test coverage when async bridges such as asgiref copy and restore execution contexts. Mutable coverage stacks share their structure across context copies, and value-based restoration can confuse distinct collectors whose coverage data happens to be equal. Coverage scopes now own their data and appear in one immutable context stack. This preserves attribution across copied and restored contexts while making collector ownership and completion explicit. Monitoring callbacks on Python 3.14 also skip completed collectors when using their thread-local fallback. ## Changes - Replace the parallel line/file stacks and custom container subclasses with a tuple of collector objects. Ordinary object identity distinguishes different scopes during value-based context restoration. - Keep line data, file data, and one completion flag on each collector. Context and TLS lookups use the same resolver to find the nearest active scope, including inherited stacks containing completed import collectors. - Exercise both coverage modes with real asgiref installed in the coverage test environments, and document the context stack and TLS fallback. ## Testing - The new snapshot-callback regression failed on the previous PR implementation for both normal and mismatched exits, then passed with the unified stack. It verifies line writes, file-set writes, and exclusion of completed collectors. - Full coverage suite on Python 3.12 and 3.14: 130 passed, one skipped, and one expected failure on each version. This includes real async_to_sync/sync_to_async propagation in both line-level and file-level modes. - Final targeted rerun on Python 3.12: all seven snapshot and context-restoration cases passed. - All repository lint checks and concrete dependency-lock validation passed. Performance benchmarks were not executed. ## Risks The per-line lookup implementation changes; its performance impact has not been measured locally. Context propagation and collector lifetime behavior are covered by the cross-version regression tests. ## Additional Notes TLS means thread-local storage. It remains the fallback for Python 3.14 monitoring callbacks that observe a snapshot without current ContextVar values. --- PR by Bits - [View session in Datadog](https://app.datadoghq.com/code/6145ef01-bebb-4079-bd76-18d5f20470c9) Comment @datadog to request changes Co-authored-by: datadog-bits <263423550+datadog-bits@users.noreply.github.com> Co-authored-by: gnufede <412857+gnufede@users.noreply.github.com> --- .../requirements/{1127dcb.txt => 17bc84a.txt} | 7 +- .../requirements/{18da66a.txt => 1bab40e.txt} | 7 +- .../requirements/{175a6ba.txt => 1c13793.txt} | 7 +- .../requirements/{6dcdfb3.txt => 4ef3ac0.txt} | 8 +- .../requirements/{ae7e800.txt => 78c2bdf.txt} | 7 +- .../requirements/{1edb5f0.txt => ce0db6f.txt} | 7 +- ddtrace/internal/README.md | 14 ++ ddtrace/internal/coverage/code.py | 176 +++++------------- tests/ci_visibility/suitespec.yml | 2 +- tests/coverage/test_coverage.py | 128 +++++++------ ...st_coverage_asgiref_context_propagation.py | 21 +-- tests/coverage/test_coverage_threading.py | 21 +-- 12 files changed, 161 insertions(+), 244 deletions(-) rename .riot/requirements/{1127dcb.txt => 17bc84a.txt} (57%) rename .riot/requirements/{18da66a.txt => 1bab40e.txt} (62%) rename .riot/requirements/{175a6ba.txt => 1c13793.txt} (59%) rename .riot/requirements/{6dcdfb3.txt => 4ef3ac0.txt} (56%) rename .riot/requirements/{ae7e800.txt => 78c2bdf.txt} (56%) rename .riot/requirements/{1edb5f0.txt => ce0db6f.txt} (57%) diff --git a/.riot/requirements/1127dcb.txt b/.riot/requirements/17bc84a.txt similarity index 57% rename from .riot/requirements/1127dcb.txt rename to .riot/requirements/17bc84a.txt index 81163aa9f71..138609f6f5d 100644 --- a/.riot/requirements/1127dcb.txt +++ b/.riot/requirements/17bc84a.txt @@ -1,9 +1,4 @@ -# -# This file is autogenerated by pip-compile with Python 3.13 -# by the following command: -# -# pip-compile --allow-unsafe --no-annotate .riot/requirements/1127dcb.in -# +asgiref==3.12.1 attrs==26.1.0 coverage[toml]==7.13.5 hypothesis==6.45.0 diff --git a/.riot/requirements/18da66a.txt b/.riot/requirements/1bab40e.txt similarity index 62% rename from .riot/requirements/18da66a.txt rename to .riot/requirements/1bab40e.txt index 0ba5d1c4f4d..bde50691d1e 100644 --- a/.riot/requirements/18da66a.txt +++ b/.riot/requirements/1bab40e.txt @@ -1,9 +1,4 @@ -# -# This file is autogenerated by pip-compile with Python 3.10 -# by the following command: -# -# pip-compile --allow-unsafe --no-annotate .riot/requirements/18da66a.in -# +asgiref==3.12.1 attrs==25.3.0 coverage[toml]==7.8.2 exceptiongroup==1.3.0 diff --git a/.riot/requirements/175a6ba.txt b/.riot/requirements/1c13793.txt similarity index 59% rename from .riot/requirements/175a6ba.txt rename to .riot/requirements/1c13793.txt index 100ecdb0d4a..2730ba7ee9f 100644 --- a/.riot/requirements/175a6ba.txt +++ b/.riot/requirements/1c13793.txt @@ -1,9 +1,4 @@ -# -# This file is autogenerated by pip-compile with Python 3.9 -# by the following command: -# -# pip-compile --allow-unsafe --no-annotate --resolver=backtracking .riot/requirements/175a6ba.in -# +asgiref==3.11.1 attrs==25.3.0 coverage[toml]==7.8.2 exceptiongroup==1.3.0 diff --git a/.riot/requirements/6dcdfb3.txt b/.riot/requirements/4ef3ac0.txt similarity index 56% rename from .riot/requirements/6dcdfb3.txt rename to .riot/requirements/4ef3ac0.txt index 75d1884f637..2076c94bb53 100644 --- a/.riot/requirements/6dcdfb3.txt +++ b/.riot/requirements/4ef3ac0.txt @@ -1,9 +1,4 @@ -# -# This file is autogenerated by pip-compile with Python 3.12 -# by the following command: -# -# pip-compile --allow-unsafe --no-annotate .riot/requirements/6dcdfb3.in -# +asgiref==3.12.1 attrs==25.3.0 coverage[toml]==7.8.2 hypothesis==6.45.0 @@ -17,3 +12,4 @@ pytest==8.4.0 pytest-cov==6.1.1 pytest-mock==3.14.1 sortedcontainers==2.4.0 +tomli==2.4.1 diff --git a/.riot/requirements/ae7e800.txt b/.riot/requirements/78c2bdf.txt similarity index 56% rename from .riot/requirements/ae7e800.txt rename to .riot/requirements/78c2bdf.txt index 03903bc8f3d..9d2ebc923f3 100644 --- a/.riot/requirements/ae7e800.txt +++ b/.riot/requirements/78c2bdf.txt @@ -1,9 +1,4 @@ -# -# This file is autogenerated by pip-compile with Python 3.11 -# by the following command: -# -# pip-compile --allow-unsafe --no-annotate .riot/requirements/ae7e800.in -# +asgiref==3.12.1 attrs==25.3.0 coverage[toml]==7.8.2 hypothesis==6.45.0 diff --git a/.riot/requirements/1edb5f0.txt b/.riot/requirements/ce0db6f.txt similarity index 57% rename from .riot/requirements/1edb5f0.txt rename to .riot/requirements/ce0db6f.txt index 9034f604daa..138609f6f5d 100644 --- a/.riot/requirements/1edb5f0.txt +++ b/.riot/requirements/ce0db6f.txt @@ -1,9 +1,4 @@ -# -# This file is autogenerated by pip-compile with Python 3.14 -# by the following command: -# -# pip-compile --allow-unsafe --no-annotate .riot/requirements/1edb5f0.in -# +asgiref==3.12.1 attrs==26.1.0 coverage[toml]==7.13.5 hypothesis==6.45.0 diff --git a/ddtrace/internal/README.md b/ddtrace/internal/README.md index 3de3b928640..9cb1ac73dbc 100644 --- a/ddtrace/internal/README.md +++ b/ddtrace/internal/README.md @@ -6,6 +6,20 @@ These modules are not intended to be used outside of `ddtrace`. The APIs found within `ddtrace.internal` are subject to breaking changes at any time and do not follow the semver versioning scheme of the `ddtrace` package. +## Coverage collection contexts + +`ModuleCodeCollector.CollectInContext` owns the line and file coverage for one +collection scope, such as a test or an import. A `ContextVar` holds an immutable +tuple of these collectors. Copying an execution context shares the collectors +and their data, while entering or exiting a scope replaces only that context's +stack. Collectors compare by identity so context restoration can distinguish +scopes even when both have empty coverage. Exiting a collector marks it closed +in all inherited stacks; writes resolve to the nearest collector still active. + +On Python 3.14+, monitoring callbacks can observe a snapshot that does not see +current `ContextVar` values. Thread-local storage (TLS) provides a fallback stack +for these callbacks, using the same rules to skip completed collectors. + ## The Product Protocol diff --git a/ddtrace/internal/coverage/code.py b/ddtrace/internal/coverage/code.py index cb4722d91c7..c9eba2bdeb4 100644 --- a/ddtrace/internal/coverage/code.py +++ b/ddtrace/internal/coverage/code.py @@ -48,29 +48,11 @@ def _is_site_packages_path(path: Path) -> bool: return not _SITE_PACKAGES_DIRNAMES.isdisjoint(path.parts) -class _ContextLinesEntry(defaultdict[str, CoverageLines]): - """Coverage lines container for one collector, tracking whether it completed. - - Execution contexts copied while a collector is active keep a reference to a - stack that still contains its entry (eg. a task scheduled by a module that is - imported inside a test). Marking the entry closed when the collector finishes - lets the context resolvers skip completed collectors and attribute new - coverage to the nearest collector that is still active. - """ - - closed: bool = False - - -class _ContextFilesEntry(set[str]): - """File-level counterpart of _ContextLinesEntry.""" - - closed: bool = False - - # NOTE: A mutable ContextVar default would be shared across threads until set() is called. -# Keep None so CollectInContext starts a separate coverage stack in each context. -ctx_covered: ContextVar[t.Optional[list[_ContextLinesEntry]]] = ContextVar("ctx_covered", default=None) -ctx_covered_files: ContextVar[t.Optional[list[_ContextFilesEntry]]] = ContextVar("ctx_covered_files", default=None) +# Use an immutable tuple so contexts can share collectors without sharing stack mutations. +ctx_collectors: ContextVar[tuple["ModuleCodeCollector.CollectInContext", ...]] = ContextVar( + "ctx_collectors", default=() +) ctx_is_import_coverage = ContextVar("ctx_is_import_coverage", default=False) ctx_coverage_enabled = ContextVar("ctx_coverage_enabled", default=False) @@ -79,42 +61,36 @@ class _ContextFilesEntry(set[str]): _tls_coverage = _threading.local() -def _get_ctx_covered_lines() -> defaultdict[str, CoverageLines]: +def _get_active_collector( + stack: tuple["ModuleCodeCollector.CollectInContext", ...], +) -> t.Optional["ModuleCodeCollector.CollectInContext"]: + # Contexts copied during an import may still contain its completed collector. + for collector in reversed(stack): + if not collector.closed: + return collector + return None + + +def _get_ctx_collector() -> t.Optional["ModuleCodeCollector.CollectInContext"]: if ctx_coverage_enabled.get(): - if context_stack := ctx_covered.get(): - for entry in reversed(context_stack): - if not entry.closed: - return entry - # Every entry on this stack belongs to a collector that has completed: - # this context inherited the stack before they exited. Fall through to - # the TLS fallback / an empty container instead of attributing new - # coverage to a completed collector. - else: - log.debug("_get_ctx_covered_lines() called but ctx_covered stack is empty") + if collector := _get_active_collector(ctx_collectors.get()): + return collector - # Fallback for Python 3.14+ where sys.monitoring callbacks can't see ContextVars + # The same lifetime rules apply when monitoring callbacks need the TLS fallback. if _PY_GE_314: - tls_covered = getattr(_tls_coverage, "covered", None) - if tls_covered is not None and not tls_covered.closed: - return tls_covered + return _get_active_collector(getattr(_tls_coverage, "stack", ())) + return None + +def _get_ctx_covered_lines() -> defaultdict[str, CoverageLines]: + if collector := _get_ctx_collector(): + return collector._covered_lines return defaultdict(CoverageLines) def _get_ctx_covered_files() -> set[str]: - if ctx_coverage_enabled.get(): - if context_stack := ctx_covered_files.get(): - for entry in reversed(context_stack): - if not entry.closed: - return entry - else: - log.debug("_get_ctx_covered_files() called but ctx_covered_files stack is empty") - - if _PY_GE_314: - tls_covered_files = getattr(_tls_coverage, "covered_files", None) - if tls_covered_files is not None and not tls_covered_files.closed: - return tls_covered_files - + if collector := _get_ctx_collector(): + return collector._covered_files return set() @@ -203,7 +179,7 @@ def hook_file(self, path: str) -> None: self._covered_files.add(path) self.covered[path].add(0) - if ctx_coverage_enabled.get() or (_PY_GE_314 and getattr(_tls_coverage, "covered", None) is not None): + if ctx_coverage_enabled.get() or (_PY_GE_314 and bool(getattr(_tls_coverage, "stack", ()))): ctx_covered_file_paths = _get_ctx_covered_files() if path not in ctx_covered_file_paths: ctx_covered_file_paths.add(path) @@ -214,7 +190,7 @@ def hook_line(self, path: str, line: int) -> None: lines = self.covered[path] lines.add(line) - if ctx_coverage_enabled.get() or (_PY_GE_314 and getattr(_tls_coverage, "covered", None) is not None): + if ctx_coverage_enabled.get() or (_PY_GE_314 and bool(getattr(_tls_coverage, "stack", ()))): # Import-time contexts store their lines in a non-context variable to be aggregated on request when # reporting coverage ctx_lines = _get_ctx_covered_lines()[path] @@ -399,41 +375,24 @@ def _get_covered_file_paths_with_imports(self, covered_file_paths: set[str]) -> self._file_level_covered_paths_cache.popitem(last=False) return paths - class _ContextStack(list[t.Any]): - """Per-context stack of coverage data that compares by identity, not value. - - Context-propagation helpers (e.g. asgiref's _restore_context, used by Django's - async test support via async_to_sync/sync_to_async) restore context variables - by comparing the current value with the incoming one using !=. A plain list - compares by value, which both silently masks legitimate stack swaps (when - two distinct stacks happen to contain equal entries) and allows one context's - stack to be replaced by another context's stack object. Comparing stacks by - identity makes such propagation respect stack ownership: restores only - propagate a stack reference into a context that does not already hold that - exact stack object, keeping the coverage data attributed to the right context. - """ - - __slots__ = () - - def __eq__(self, other: object) -> bool: # noqa: D105 - return self is other + class CollectInContext: + """Own coverage data for one collection scope. - def __ne__(self, other: object) -> bool: # noqa: D105 - return self is not other + Context copies share collector objects but have independent immutable stacks. + Object identity distinguishes scopes during value-based context restoration; + the shared closed flag prevents inherited contexts from writing to finished scopes. + """ - class CollectInContext: def __init__(self, is_import_coverage: bool = False): self.is_import_coverage = is_import_coverage def __enter__(self): - # ContextVar values are copied by reference into new execution contexts. - # Replace the stacks so a nested collector cannot mutate its parent's stack. - self._covered_lines = _ContextLinesEntry(CoverageLines) - self._covered_files = _ContextFilesEntry() - ctx_covered.set(ModuleCodeCollector._ContextStack((ctx_covered.get() or []) + [self._covered_lines])) - ctx_covered_files.set( - ModuleCodeCollector._ContextStack((ctx_covered_files.get() or []) + [self._covered_files]) - ) + # Collector objects compare by identity, so value-based context restores + # distinguish different collectors even when their coverage data is empty. + self._covered_lines: defaultdict[str, CoverageLines] = defaultdict(CoverageLines) + self._covered_files: set[str] = set() + self.closed = False + ctx_collectors.set(ctx_collectors.get() + (self,)) ctx_coverage_enabled.set(True) if self.is_import_coverage: @@ -442,8 +401,7 @@ def __enter__(self): # Python 3.14+ sys.monitoring callbacks can't see ContextVar changes, # so also store in thread-local as a fallback for the hook. if _PY_GE_314: - _tls_coverage.covered = ctx_covered.get()[-1] - _tls_coverage.covered_files = ctx_covered_files.get()[-1] + _tls_coverage.stack = ctx_collectors.get() # For Python 3.12+, dynamically detect whether other sys.monitoring tools are # active and update the DISABLE optimisation flag accordingly. Then re-enable @@ -465,48 +423,16 @@ def __enter__(self): return self def __exit__(self, *args, **kwargs): - # The collector is completing. Stacks inherited by other contexts (copied - # while this collector was active) still reference its entry: mark it - # closed so the resolvers skip it and attribute new coverage to the - # nearest active collector instead of this completed one. - self._covered_lines.closed = True - self._covered_files.closed = True - - covered_lines_stack = ctx_covered.get() or [] - covered_files_stack = ctx_covered_files.get() or [] - if ( - covered_lines_stack - and covered_files_stack - and covered_lines_stack[-1] is self._covered_lines - and covered_files_stack[-1] is self._covered_files - ): - covered_lines_stack = ModuleCodeCollector._ContextStack(covered_lines_stack[:-1]) - covered_files_stack = ModuleCodeCollector._ContextStack(covered_files_stack[:-1]) - ctx_covered.set(covered_lines_stack) - ctx_covered_files.set(covered_files_stack) - else: - # A copied context may finish a collector that was entered elsewhere. - # Leave this context's collector intact instead of popping the wrong one. - if _PY_GE_314: - # The exited collector may still be this thread's TLS fallback: the - # sys.monitoring callbacks fall back to this thread-local state when - # their snapshot context cannot observe ContextVar changes. Re-sync - # the fallback to the collector that is actually active in this - # context (or clear it) so coverage keeps being attributed to the - # right entry instead of the completed collector. - _tls_coverage.covered = covered_lines_stack[-1] if covered_lines_stack else None - _tls_coverage.covered_files = covered_files_stack[-1] if covered_files_stack else None - return - - # Stop coverage if we're exiting the last context - if not covered_lines_stack: - ctx_coverage_enabled.set(False) - if _PY_GE_314: - _tls_coverage.covered = None - _tls_coverage.covered_files = None - elif _PY_GE_314: - _tls_coverage.covered = covered_lines_stack[-1] - _tls_coverage.covered_files = covered_files_stack[-1] + # Closing the shared collector expires it in every inherited stack. + self.closed = True + stack = ctx_collectors.get() + if stack and stack[-1] is self: + stack = stack[:-1] + ctx_collectors.set(stack) + # An exit in a different context must preserve that context's collectors. + ctx_coverage_enabled.set(_get_active_collector(stack) is not None) + if _PY_GE_314: + _tls_coverage.stack = stack def get_covered_lines(self) -> dict[str, CoverageLines]: covered_lines = self._covered_lines diff --git a/tests/ci_visibility/suitespec.yml b/tests/ci_visibility/suitespec.yml index 354ee94504d..fcec1dad070 100644 --- a/tests/ci_visibility/suitespec.yml +++ b/tests/ci_visibility/suitespec.yml @@ -74,7 +74,7 @@ suites: snapshot: true matrix: command: pytest --no-cov {cmdargs} tests/coverage -s - variants: [{name: dd_coverage}] + variants: [{name: dd_coverage, dependencies: [asgiref]}] pytest: venvs_per_job: 9 paths: diff --git a/tests/coverage/test_coverage.py b/tests/coverage/test_coverage.py index ec5ccb3313c..c29a4a643eb 100644 --- a/tests/coverage/test_coverage.py +++ b/tests/coverage/test_coverage.py @@ -12,52 +12,85 @@ import pytest +@pytest.mark.parametrize("mismatched_exit", [False, True]) +def test_tls_fallback_skips_completed_inherited_collectors(monkeypatch, mismatched_exit): + from contextvars import Context + from contextvars import copy_context + + import ddtrace.internal.coverage.code as coverage_code + + monkeypatch.setattr(coverage_code, "_PY_GE_314", True) + snapshot = Context() + collector = object.__new__(coverage_code.ModuleCodeCollector) + collector._coverage_enabled = False + with coverage_code.ModuleCodeCollector.CollectInContext() as test_collector: + with coverage_code.ModuleCodeCollector.CollectInContext() as import_collector: + task_context = copy_context() + + nested = coverage_code.ModuleCodeCollector.CollectInContext() + if mismatched_exit: + child_context = task_context.copy() + child_context.run(nested.__enter__) + # This inherited stack does not contain the nested collector and + # its top collector has already completed. + task_context.run(nested.__exit__) + else: + task_context.run(nested.__enter__) + task_context.run(nested.__exit__) + + # Monitoring callbacks see a snapshot without the task's ContextVars. + snapshot.run(collector.hook_line, "/repo/active.py", 42) + snapshot.run(collector.hook_file, "/repo/file.py") + assert 42 in test_collector.get_covered_lines()["/repo/active.py"].to_sorted_list() + assert "/repo/file.py" in test_collector._covered_files + assert "/repo/active.py" not in import_collector.get_covered_lines() + assert "/repo/active.py" not in nested.get_covered_lines() + assert "/repo/file.py" not in import_collector.get_covered_file_paths() + assert "/repo/file.py" not in nested.get_covered_file_paths() + + snapshot.run(collector.hook_line, "/repo/late.py", 7) + assert "/repo/late.py" not in test_collector.get_covered_lines() + + def test_coverage_stacks_are_isolated_across_copied_contexts(): from contextvars import copy_context from ddtrace.internal.coverage.code import ModuleCodeCollector - from ddtrace.internal.coverage.code import ctx_covered - from ddtrace.internal.coverage.code import ctx_covered_files + from ddtrace.internal.coverage.code import ctx_collectors with ModuleCodeCollector.CollectInContext(): - parent_lines_stack = ctx_covered.get() - parent_files_stack = ctx_covered_files.get() - parent_depth = len(parent_lines_stack) + parent_stack = ctx_collectors.get() + parent_depth = len(parent_stack) child_context = copy_context() def collect_in_child_context(): - with ModuleCodeCollector.CollectInContext(): - assert len(ctx_covered.get()) == len(ctx_covered_files.get()) == parent_depth + 1 - assert ctx_covered.get()[-1] is not parent_lines_stack[-1] - assert ctx_covered_files.get()[-1] is not parent_files_stack[-1] + with ModuleCodeCollector.CollectInContext() as child: + assert len(ctx_collectors.get()) == parent_depth + 1 + assert ctx_collectors.get()[-1] is child + assert ctx_collectors.get()[-2] is parent_stack[-1] child_context.run(collect_in_child_context) - assert ctx_covered.get() is parent_lines_stack - assert ctx_covered_files.get() is parent_files_stack - assert len(parent_lines_stack) == len(parent_files_stack) == parent_depth + assert ctx_collectors.get() is parent_stack + assert len(parent_stack) == parent_depth def test_exiting_collector_in_another_context_preserves_active_coverage(): from contextvars import copy_context from ddtrace.internal.coverage.code import ModuleCodeCollector - from ddtrace.internal.coverage.code import ctx_covered - from ddtrace.internal.coverage.code import ctx_covered_files + from ddtrace.internal.coverage.code import ctx_collectors with ModuleCodeCollector.CollectInContext(): - parent_depth = len(ctx_covered.get()) - parent_lines = ctx_covered.get()[-1] - parent_files = ctx_covered_files.get()[-1] + parent_stack = ctx_collectors.get() child_context = copy_context() child = ModuleCodeCollector.CollectInContext() child_context.run(child.__enter__) child.__exit__() - assert ctx_covered.get()[-1] is parent_lines - assert ctx_covered_files.get()[-1] is parent_files + assert ctx_collectors.get() is parent_stack child_context.run(child.__exit__) - assert len(ctx_covered.get()) == len(ctx_covered_files.get()) == parent_depth + assert ctx_collectors.get() is parent_stack def test_completed_collector_entries_do_not_capture_inherited_context_coverage(): @@ -77,11 +110,11 @@ def test_completed_collector_entries_do_not_capture_inherited_context_coverage() with ModuleCodeCollector.CollectInContext() as import_collector: task_context = copy_context() - assert import_collector._covered_lines.closed + assert import_collector.closed # The task context still sees the completed import entry atop its stack. - task_stack = task_context.run(coverage_code.ctx_covered.get) - assert task_stack[-1] is import_collector._covered_lines + task_stack = task_context.run(coverage_code.ctx_collectors.get) + assert task_stack[-1] is import_collector # Resolution inside the task context must skip the completed entry and # attribute coverage to the still-active test collector. @@ -103,44 +136,25 @@ def test_completed_collector_entries_do_not_capture_inherited_context_coverage() assert stale is not test_collector._covered_lines -def test_mismatched_exit_resyncs_tls_fallback(): - """A mismatched exit must not leave the TLS fallback on the completed collector. - - On Python 3.14+, sys.monitoring callbacks run in a snapshot context and fall back - to the thread-local coverage state when they cannot observe ContextVar changes. - When a collector entered in a copied context is exited from a different context - (a mismatched exit), that thread-local fallback must be re-synced to the active - collector instead of pointing at the collector that just completed. - """ +def test_mismatched_exit_resyncs_tls_fallback(monkeypatch): + """A mismatched exit must leave snapshot callbacks recording in the active collector.""" + from contextvars import Context from contextvars import copy_context import ddtrace.internal.coverage.code as coverage_code from ddtrace.internal.coverage.code import ModuleCodeCollector - from ddtrace.internal.coverage.code import ctx_covered - - original_flag = coverage_code._PY_GE_314 - coverage_code._PY_GE_314 = True - try: - with ModuleCodeCollector.CollectInContext(): - parent_lines = ctx_covered.get()[-1] - parent_files = coverage_code.ctx_covered_files.get()[-1] - child_context = copy_context() - child = ModuleCodeCollector.CollectInContext() - child_context.run(child.__enter__) - - # The thread-local fallback tracks the most recent collector entered in - # this thread, which is the child's. - assert coverage_code._tls_coverage.covered is child._covered_lines - - # Exiting the child from the parent context is a mismatched exit. - child.__exit__() - - # The fallback must be re-synced to the parent's active entries rather - # than left pointing at the completed child collector. - assert coverage_code._tls_coverage.covered is parent_lines - assert coverage_code._tls_coverage.covered_files is parent_files - finally: - coverage_code._PY_GE_314 = original_flag + + monkeypatch.setattr(coverage_code, "_PY_GE_314", True) + snapshot = Context() + with ModuleCodeCollector.CollectInContext() as parent: + child_context = copy_context() + child = ModuleCodeCollector.CollectInContext() + child_context.run(child.__enter__) + assert snapshot.run(coverage_code._get_ctx_covered_lines) is child._covered_lines + + child.__exit__() + assert snapshot.run(coverage_code._get_ctx_covered_lines) is parent._covered_lines + assert snapshot.run(coverage_code._get_ctx_covered_files) is parent._covered_files @pytest.mark.skipif(sys.version_info < (3, 12), reason="Test specific to Python 3.12+ monitoring API") diff --git a/tests/coverage/test_coverage_asgiref_context_propagation.py b/tests/coverage/test_coverage_asgiref_context_propagation.py index 1ef74d68a7b..6990c867ce9 100644 --- a/tests/coverage/test_coverage_asgiref_context_propagation.py +++ b/tests/coverage/test_coverage_asgiref_context_propagation.py @@ -15,22 +15,15 @@ data between contexts. """ -import importlib.util - import pytest -# The first test hand-rolls the context-propagation mechanism and has no -# dependency on asgiref. The second exercises the real library and is skipped -# when it is not installed. -HAS_ASGIREF = importlib.util.find_spec("asgiref") is not None - - -@pytest.mark.subprocess(env={"_DD_COVERAGE_FILE_LEVEL": "false"}) +@pytest.mark.subprocess(parametrize={"_DD_COVERAGE_FILE_LEVEL": ["true", "false"]}) def test_coverage_context_thread_value_based_context_restore(): import contextvars import os from pathlib import Path + import sys import threading from ddtrace.internal.coverage.code import ModuleCodeCollector @@ -101,14 +94,17 @@ def task_body(): "tests/coverage/included_path/in_context_lib.py": {1, 2, 5}, } + if ModuleCodeCollector.file_level_coverage_enabled() and sys.version_info >= (3, 12): + expected_lines = {path: {0} for path in expected_lines} + assert expected_lines == context_covered, f"Mismatched lines: {expected_lines} vs {context_covered}" -@pytest.mark.skipif(not HAS_ASGIREF, reason="asgiref is not installed") -@pytest.mark.subprocess(env={"_DD_COVERAGE_FILE_LEVEL": "false"}) +@pytest.mark.subprocess(parametrize={"_DD_COVERAGE_FILE_LEVEL": ["true", "false"]}) def test_coverage_context_thread_async_to_sync(): import os from pathlib import Path + import sys from asgiref.sync import async_to_sync from asgiref.sync import sync_to_async @@ -150,4 +146,7 @@ async def async_fn(a, b): "tests/coverage/included_path/in_context_lib.py": {1, 2, 5}, } + if ModuleCodeCollector.file_level_coverage_enabled() and sys.version_info >= (3, 12): + expected_lines = {path: {0} for path in expected_lines} + assert expected_lines == context_covered, f"Mismatched lines: {expected_lines} vs {context_covered}" diff --git a/tests/coverage/test_coverage_threading.py b/tests/coverage/test_coverage_threading.py index 4ebb80c6c3b..2f147b1d9bf 100644 --- a/tests/coverage/test_coverage_threading.py +++ b/tests/coverage/test_coverage_threading.py @@ -120,8 +120,7 @@ def test_coverage_context_isolated_across_threads(): import threading from ddtrace.internal.coverage.code import ModuleCodeCollector - from ddtrace.internal.coverage.code import ctx_covered - from ddtrace.internal.coverage.code import ctx_covered_files + from ddtrace.internal.coverage.code import ctx_collectors from ddtrace.internal.coverage.installer import install cwd = os.getcwd() @@ -129,11 +128,8 @@ def test_coverage_context_isolated_across_threads(): thread_entered = threading.Event() thread_can_exit = threading.Event() - with ModuleCodeCollector.CollectInContext(): - main_lines_stack = ctx_covered.get() - main_files_stack = ctx_covered_files.get() - main_lines = main_lines_stack[-1] - main_files = main_files_stack[-1] + with ModuleCodeCollector.CollectInContext() as main_collector: + main_stack = ctx_collectors.get() def worker(): # The patched _bootstrap_inner enters a coverage context before calling the target. @@ -150,18 +146,15 @@ def worker(): # On Python 3.14 the target runs in a snapshot context, so inspect the # parent's stacks while the patched bootstrap's context is still active. assert hasattr(thread, "_coverage_context") - assert ctx_covered.get() is main_lines_stack - assert ctx_covered_files.get() is main_files_stack - assert len(main_lines_stack) == len(main_files_stack) == 1 - assert main_lines_stack[-1] is main_lines - assert main_files_stack[-1] is main_files + assert ctx_collectors.get() is main_stack + assert len(main_stack) == 1 + assert main_stack[-1] is main_collector finally: thread_can_exit.set() thread.join(timeout=5) assert not thread.is_alive(), "Worker did not exit its coverage context" - assert main_lines_stack[-1] is main_lines - assert main_files_stack[-1] is main_files + assert main_stack[-1] is main_collector @pytest.mark.subprocess(env={"_DD_COVERAGE_FILE_LEVEL": "false"})