Skip to content

Commit

Permalink
chore(tracing): ensure ddtrace.trace.Tracer() is not reinitialized
Browse files Browse the repository at this point in the history
allow configuring dummy writers for civis

fix broken tests

update tracer

fix stuff

try to fix pytest

address failues
  • Loading branch information
mabdinur committed Feb 4, 2025
1 parent e294f47 commit dc30784
Show file tree
Hide file tree
Showing 27 changed files with 275 additions and 400 deletions.
12 changes: 2 additions & 10 deletions ddtrace/_trace/tracer.py
Original file line number Diff line number Diff line change
Expand Up @@ -216,16 +216,8 @@ def __init__(
if Tracer._instance is None:
Tracer._instance = self
else:
# ddtrace library does not support context propagation for multiple tracers.
# All instances of ddtrace ContextProviders share the same ContextVars. This means that
# if you create multiple instances of Tracer, spans will be shared between them creating a
# broken experience.
# TODO(mabdinur): Convert this warning to an ValueError in 3.0.0
deprecate(
"Support for multiple Tracer instances is deprecated",
". Use ddtrace.tracer instead.",
category=DDTraceDeprecationWarning,
removal_version="3.0.0",
raise ValueError(
"Multiple Tracer instances can not be initialized. Use ``ddtrace.trace.tracer`` instead.",
)

self._user_trace_processors: List[TraceProcessor] = []
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
upgrade:
- |
tracing: Drops support for multiple Tracer instances in the same process. Use ``ddtrace.trace.tracer`` to access the global tracer instance.
- |
pin: Removes the ``tracer`` parameter from ``ddtrace.trace.Pin`` methods. All Pin objects now use the global tracer instance.
7 changes: 4 additions & 3 deletions tests/ci_visibility/test_ci_visibility.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
from ddtrace.internal.ci_visibility.git_client import METADATA_UPLOAD_STATUS
from ddtrace.internal.ci_visibility.git_client import CIVisibilityGitClient
from ddtrace.internal.ci_visibility.git_client import CIVisibilityGitClientSerializerV1
from ddtrace.internal.ci_visibility.recorder import CIVisibilityTracer
from ddtrace.internal.ci_visibility.recorder import _extract_repository_name_from_url
import ddtrace.internal.test_visibility._internal_item_ids
from ddtrace.internal.utils.http import Response
Expand Down Expand Up @@ -685,7 +686,7 @@ def test_civisibilitywriter_evp_proxy_url(self):
), mock.patch(
"ddtrace.internal.agent.get_trace_url", return_value="http://evpproxy.bar:1234"
), mock.patch("ddtrace.settings._config.Config", _get_default_civisibility_ddconfig()), mock.patch(
"ddtrace.tracer", ddtrace.trace.Tracer()
"ddtrace.tracer", CIVisibilityTracer()
), mock.patch(
"ddtrace.internal.ci_visibility.recorder.CIVisibility._agent_evp_proxy_is_available", return_value=True
), _dummy_noop_git_client(), mock.patch(
Expand All @@ -705,7 +706,7 @@ def test_civisibilitywriter_only_traces(self):
)
), mock.patch(
"ddtrace.internal.agent.get_trace_url", return_value="http://onlytraces:1234"
), mock.patch("ddtrace.tracer", ddtrace.trace.Tracer()), mock.patch(
), mock.patch("ddtrace.tracer", CIVisibilityTracer()), mock.patch(
"ddtrace.internal.ci_visibility.recorder.CIVisibility._agent_evp_proxy_is_available", return_value=False
), mock.patch(
"ddtrace.internal.ci_visibility.writer.config", ddtrace.settings.Config()
Expand Down Expand Up @@ -1119,7 +1120,7 @@ def test_civisibility_enable_respects_passed_in_tracer():
), _dummy_noop_git_client(), mock.patch(
"ddtrace.internal.ci_visibility.recorder.ddconfig", _get_default_civisibility_ddconfig()
), mock.patch("ddtrace.internal.ci_visibility.writer.config", ddtrace.settings.Config()):
tracer = ddtrace.trace.Tracer()
tracer = CIVisibilityTracer()
tracer._configure(partial_flush_enabled=False, partial_flush_min_spans=100)
CIVisibility.enable(tracer=tracer)
assert CIVisibility._instance.tracer._partial_flush_enabled is False
Expand Down
3 changes: 2 additions & 1 deletion tests/ci_visibility/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
from ddtrace.internal.ci_visibility.git_client import METADATA_UPLOAD_STATUS
from ddtrace.internal.ci_visibility.git_client import CIVisibilityGitClient
from ddtrace.internal.ci_visibility.recorder import CIVisibility
from ddtrace.internal.ci_visibility.recorder import CIVisibilityTracer
from ddtrace.internal.test_visibility._internal_item_ids import InternalTestId
from tests.utils import DummyCIVisibilityWriter
from tests.utils import override_env
Expand Down Expand Up @@ -209,5 +210,5 @@ def _ci_override_env(
new_vars: t.Optional[t.Dict[str, str]] = None, mock_ci_env=False, replace_os_env=True, full_clear=False
):
env_vars = _get_default_ci_env_vars(new_vars, mock_ci_env, full_clear)
with override_env(env_vars, replace_os_env=replace_os_env), mock.patch("ddtrace.tracer", ddtrace.trace.Tracer()):
with override_env(env_vars, replace_os_env=replace_os_env), mock.patch("ddtrace.tracer", CIVisibilityTracer()):
yield
8 changes: 2 additions & 6 deletions tests/contrib/aiomysql/test_aiomysql.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
from ddtrace.contrib.internal.aiomysql.patch import unpatch
from ddtrace.internal.schema import DEFAULT_SPAN_SERVICE_NAME
from ddtrace.trace import Pin
from ddtrace.trace import Tracer
from tests.contrib import shared_tests_async as shared_tests
from tests.contrib.asyncio.utils import AsyncioTestCase
from tests.contrib.asyncio.utils import mark_asyncio
Expand All @@ -31,19 +30,16 @@ def patch_aiomysql():
@pytest.fixture
async def patched_conn(tracer):
conn = await aiomysql.connect(**AIOMYSQL_CONFIG)
Pin.get_from(conn).clone(tracer=tracer).onto(conn)
yield conn
conn.close()


@pytest.fixture()
async def snapshot_conn():
tracer = Tracer()
async def snapshot_conn(tracer):
conn = await aiomysql.connect(**AIOMYSQL_CONFIG)
Pin.get_from(conn).clone(tracer=tracer).onto(conn)
yield conn
conn.close()
tracer.shutdown()
tracer.flush()


@pytest.mark.asyncio
Expand Down
8 changes: 1 addition & 7 deletions tests/contrib/flask_cache/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
from ddtrace.contrib.internal.flask_cache.utils import _extract_client
from ddtrace.contrib.internal.flask_cache.utils import _extract_conn_tags
from ddtrace.contrib.internal.flask_cache.utils import _resource_from_cache_prefix
from ddtrace.trace import Tracer
from ddtrace.trace import tracer

from ..config import MEMCACHED_CONFIG
from ..config import REDIS_CONFIG
Expand All @@ -17,7 +17,6 @@ class FlaskCacheUtilsTest(unittest.TestCase):

def test_extract_redis_connection_metadata(self):
# create the TracedCache instance for a Flask app
tracer = Tracer()
Cache = get_traced_cache(tracer, service=self.SERVICE)
app = Flask(__name__)
config = {
Expand All @@ -37,7 +36,6 @@ def test_extract_redis_connection_metadata(self):

def test_extract_memcached_connection_metadata(self):
# create the TracedCache instance for a Flask app
tracer = Tracer()
Cache = get_traced_cache(tracer, service=self.SERVICE)
app = Flask(__name__)
config = {
Expand All @@ -56,7 +54,6 @@ def test_extract_memcached_connection_metadata(self):

def test_extract_memcached_multiple_connection_metadata(self):
# create the TracedCache instance for a Flask app
tracer = Tracer()
Cache = get_traced_cache(tracer, service=self.SERVICE)
app = Flask(__name__)
config = {
Expand All @@ -78,7 +75,6 @@ def test_extract_memcached_multiple_connection_metadata(self):

def test_resource_from_cache_with_prefix(self):
# create the TracedCache instance for a Flask app
tracer = Tracer()
Cache = get_traced_cache(tracer, service=self.SERVICE)
app = Flask(__name__)
config = {
Expand All @@ -94,7 +90,6 @@ def test_resource_from_cache_with_prefix(self):

def test_resource_from_cache_with_empty_prefix(self):
# create the TracedCache instance for a Flask app
tracer = Tracer()
Cache = get_traced_cache(tracer, service=self.SERVICE)
app = Flask(__name__)
config = {
Expand All @@ -110,7 +105,6 @@ def test_resource_from_cache_with_empty_prefix(self):

def test_resource_from_cache_without_prefix(self):
# create the TracedCache instance for a Flask app
tracer = Tracer()
Cache = get_traced_cache(tracer, service=self.SERVICE)
app = Flask(__name__)
traced_cache = Cache(app, config={"CACHE_TYPE": "redis"})
Expand Down
17 changes: 10 additions & 7 deletions tests/contrib/kafka/test_kafka.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
from ddtrace.internal.utils.retry import fibonacci_backoff_with_jitter
from ddtrace.trace import Pin
from ddtrace.trace import TraceFilter
from ddtrace.trace import Tracer
from ddtrace.trace import tracer as ddtracer
from tests.contrib.config import KAFKA_CONFIG
from tests.datastreams.test_public_api import MockedTracer
from tests.utils import DummyTracer
Expand Down Expand Up @@ -106,16 +106,16 @@ def should_filter_empty_polls():
@pytest.fixture
def tracer(should_filter_empty_polls):
patch()
t = Tracer()
if should_filter_empty_polls:
t._configure(trace_processors=[KafkaConsumerPollFilter()])
ddtracer.configure(trace_processors=[KafkaConsumerPollFilter()])
# disable backoff because it makes these tests less reliable
t._writer._send_payload_with_backoff = t._writer._send_payload
previous_backoff = ddtracer._writer._send_payload_with_backoff
ddtracer._writer._send_payload_with_backoff = ddtracer._writer._send_payload
try:
yield t
yield ddtracer
finally:
t.flush()
t.shutdown()
ddtracer.flush()
ddtracer._writer._send_payload_with_backoff = previous_backoff
unpatch()


Expand All @@ -124,6 +124,8 @@ def dsm_processor(tracer):
processor = tracer.data_streams_processor
with mock.patch("ddtrace.internal.datastreams.data_streams_processor", return_value=processor):
yield processor
# flush buckets for the next test run
processor.periodic()


@pytest.fixture
Expand Down Expand Up @@ -325,6 +327,7 @@ def test_commit_with_consume_with_multiple_messages(producer, consumer, kafka_to

@pytest.mark.snapshot(ignores=SNAPSHOT_IGNORES)
@pytest.mark.parametrize("should_filter_empty_polls", [False])
@pytest.mark.skip(reason="FIXME: This test requires the initialization of a new tracer. This is not supported")
def test_commit_with_consume_with_error(producer, consumer, kafka_topic):
producer.produce(kafka_topic, PAYLOAD, key=KEY)
producer.flush()
Expand Down
25 changes: 2 additions & 23 deletions tests/contrib/tornado/test_config.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
from ddtrace.trace import TraceFilter
from ddtrace.trace import Tracer
from tests.utils import DummyWriter
from tests.utils import DummyTracer

from .utils import TornadoTestCase

Expand All @@ -19,8 +18,7 @@ class TestTornadoSettings(TornadoTestCase):
"""

def get_app(self):
# Override with a real tracer
self.tracer = Tracer()
self.tracer = DummyTracer()
super(TestTornadoSettings, self).get_app()

def get_settings(self):
Expand All @@ -40,25 +38,6 @@ def get_settings(self):
},
}

def test_tracer_is_properly_configured(self):
# the tracer must be properly configured
assert self.tracer._tags.get("env") == "production"
assert self.tracer._tags.get("debug") == "false"
assert self.tracer.enabled is False
assert self.tracer.agent_trace_url == "http://dd-agent.service.consul:8126"

writer = DummyWriter()
self.tracer._configure(enabled=True, writer=writer)
with self.tracer.trace("keep"):
pass
spans = writer.pop()
assert len(spans) == 1

with self.tracer.trace("drop"):
pass
spans = writer.pop()
assert len(spans) == 0


class TestTornadoSettingsEnabled(TornadoTestCase):
def get_settings(self):
Expand Down
Loading

0 comments on commit dc30784

Please sign in to comment.