Skip to content

Commit 06b8494

Browse files
fix(tracing) lazy importing asyncio should not reset existing trace context (#9272)
This PR introduces two main changes: 1. Do not reset `_DD_CONTEXTVAR` when initializing a `DefaultContextProvider` 2. Explicitly clear `_DD_CONTEXTVAR` after every test in dd-trace-py CI 1. We also emit a warning, so pytest will show them to us ## Motivation Currently, when 1. asyncio auto instrumentation is patched 5. and there's an existing trace context 6. and `asyncio` is imported Then the existing trace context will be reset to None. The expected behavior is that the existing trace context should not be reset for this kind of lazy imports ## Checklist - [x] Change(s) are motivated and described in the PR description - [x] Testing strategy is described if automated tests are not included in the PR - [x] Risks are described (performance impact, potential for breakage, maintainability) - [x] Change is maintainable (easy to change, telemetry, documentation) - [x] [Library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) are followed or label `changelog/no-changelog` is set - [x] Documentation is included (in-code, generated user docs, [public corp docs](https://github.com/DataDog/documentation/)) - [x] Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) - [x] If this PR changes the public interface, I've notified `@DataDog/apm-tees`. ## Reviewer Checklist - [ ] Title is accurate - [ ] All changes are related to the pull request's stated goal - [ ] Description motivates each change - [ ] Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes - [ ] Testing strategy adequately addresses listed risks - [ ] Change is maintainable (easy to change, telemetry, documentation) - [ ] Release note makes sense to a user of the library - [ ] 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: Brett Langdon <[email protected]>
1 parent 4b40fce commit 06b8494

File tree

4 files changed

+35
-1
lines changed

4 files changed

+35
-1
lines changed

ddtrace/_trace/provider.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,6 @@ class DefaultContextProvider(BaseContextProvider, DatadogContextMixin):
113113
def __init__(self):
114114
# type: () -> None
115115
super(DefaultContextProvider, self).__init__()
116-
_DD_CONTEXTVAR.set(None)
117116

118117
def _has_active_context(self):
119118
# type: () -> bool
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
fixes:
3+
- |
4+
This fix resolves an issue where importing ``asyncio`` after a trace has already been started will reset the currently active span.

tests/conftest.py

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,14 @@
1616
from typing import Generator # noqa:F401
1717
from typing import Tuple # noqa:F401
1818
from unittest import mock
19+
import warnings
1920

2021
from _pytest.runner import call_and_report
2122
from _pytest.runner import pytest_runtest_protocol as default_pytest_runtest_protocol
2223
import pytest
2324

2425
import ddtrace
26+
from ddtrace._trace.provider import _DD_CONTEXTVAR
2527
from ddtrace.internal.compat import httplib
2628
from ddtrace.internal.compat import parse
2729
from ddtrace.internal.remoteconfig.client import RemoteConfigClient
@@ -67,6 +69,17 @@ def test_spans(tracer):
6769
container.reset()
6870

6971

72+
@pytest.fixture(autouse=True)
73+
def clear_context_after_every_test():
74+
try:
75+
yield
76+
finally:
77+
ctx = _DD_CONTEXTVAR.get()
78+
if ctx is not None:
79+
warnings.warn(f"Context was not cleared after test, expected None, got {ctx}")
80+
_DD_CONTEXTVAR.set(None)
81+
82+
7083
@pytest.fixture
7184
def run_python_code_in_subprocess(tmpdir):
7285
def _run(code, **kwargs):
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
import pytest # noqa: I001
2+
3+
4+
@pytest.mark.subprocess()
5+
def test_lazy_import():
6+
import ddtrace.auto # noqa: F401,I001
7+
from ddtrace import tracer # noqa: I001
8+
9+
assert tracer.context_provider.active() is None
10+
span = tracer.trace("itsatest", service="test", resource="resource", span_type="http")
11+
assert tracer.context_provider.active() is span
12+
13+
# Importing asyncio after starting a trace does not remove the current active span
14+
import asyncio # noqa: F401
15+
16+
assert tracer.context_provider.active() is span
17+
span.finish()
18+
assert tracer.context_provider.active() is None

0 commit comments

Comments
 (0)