Skip to content

Commit

Permalink
chore(tracing): make ddtrace.trace.Tracer a singleton (#12209)
Browse files Browse the repository at this point in the history
This PR ensures an error is logged when more than one instance of the
Tracer is initialized in a process. In all scenarios the global tracer
must be used to trace an application.

## 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)
  • Loading branch information
mabdinur authored Feb 5, 2025
1 parent b17990b commit 5b42292
Show file tree
Hide file tree
Showing 24 changed files with 267 additions and 391 deletions.
12 changes: 2 additions & 10 deletions ddtrace/_trace/tracer.py
Original file line number Diff line number Diff line change
Expand Up @@ -217,16 +217,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",
log.error(
"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,4 @@
---
upgrade:
- |
tracing: Drops support for multiple Tracer instances in the same process. Use ``ddtrace.trace.tracer`` to access 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
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
50 changes: 29 additions & 21 deletions tests/integration/test_debug.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@
import os
import re
import subprocess
from typing import List
from typing import Optional

import mock
import pytest
Expand All @@ -13,11 +11,10 @@
import ddtrace._trace.sampler
from ddtrace.internal import debug
from ddtrace.internal.writer import AgentWriter
from ddtrace.internal.writer import TraceWriter
from ddtrace.trace import Span
from tests.integration.utils import AGENT_VERSION
from tests.subprocesstest import SubprocessTestCase
from tests.subprocesstest import run_in_subprocess
from tests.utils import DummyTracer


pytestmark = pytest.mark.skipif(AGENT_VERSION == "testagent", reason="The test agent doesn't support startup logs.")
Expand Down Expand Up @@ -189,13 +186,12 @@ def test_trace_agent_url(self):
)
)
def test_tracer_loglevel_info_connection(self):
tracer = ddtrace.trace.Tracer()
logging.basicConfig(level=logging.INFO)
with mock.patch.object(logging.Logger, "log") as mock_logger:
# shove an unserializable object into the config log output
# regression: this used to cause an exception to be raised
ddtrace.config.version = AgentWriter(agent_url="foobar")
tracer._configure()
ddtrace.trace.tracer.configure()
assert mock.call(logging.INFO, re_matcher("- DATADOG TRACER CONFIGURATION - ")) in mock_logger.mock_calls

@run_in_subprocess(
Expand All @@ -205,10 +201,9 @@ def test_tracer_loglevel_info_connection(self):
)
)
def test_tracer_loglevel_info_no_connection(self):
tracer = ddtrace.trace.Tracer()
logging.basicConfig(level=logging.INFO)
with mock.patch.object(logging.Logger, "log") as mock_logger:
tracer._configure()
ddtrace.trace.tracer.configure()
assert mock.call(logging.INFO, re_matcher("- DATADOG TRACER CONFIGURATION - ")) in mock_logger.mock_calls
assert mock.call(logging.WARNING, re_matcher("- DATADOG TRACER DIAGNOSTIC - ")) in mock_logger.mock_calls

Expand All @@ -219,9 +214,8 @@ def test_tracer_loglevel_info_no_connection(self):
)
)
def test_tracer_log_disabled_error(self):
tracer = ddtrace.trace.Tracer()
with mock.patch.object(logging.Logger, "log") as mock_logger:
tracer._configure()
ddtrace.trace.tracer._configure()
assert mock_logger.mock_calls == []

@run_in_subprocess(
Expand All @@ -231,9 +225,8 @@ def test_tracer_log_disabled_error(self):
)
)
def test_tracer_log_disabled(self):
tracer = ddtrace.trace.Tracer()
with mock.patch.object(logging.Logger, "log") as mock_logger:
tracer._configure()
ddtrace.trace.tracer._configure()
assert mock_logger.mock_calls == []

@run_in_subprocess(
Expand All @@ -243,9 +236,8 @@ def test_tracer_log_disabled(self):
)
def test_tracer_info_level_log(self):
logging.basicConfig(level=logging.INFO)
tracer = ddtrace.trace.Tracer()
with mock.patch.object(logging.Logger, "log") as mock_logger:
tracer._configure()
ddtrace.trace.tracer._configure()
assert mock_logger.mock_calls == []


Expand Down Expand Up @@ -287,16 +279,24 @@ def test_to_json():
json.dumps(info)


@pytest.mark.subprocess(env={"AWS_LAMBDA_FUNCTION_NAME": "something"})
def test_agentless(monkeypatch):
monkeypatch.setenv("AWS_LAMBDA_FUNCTION_NAME", "something")
tracer = ddtrace.trace.Tracer()
info = debug.collect(tracer)
from ddtrace.internal import debug
from ddtrace.trace import tracer

info = debug.collect(tracer)
assert info.get("agent_url") == "AGENTLESS"


@pytest.mark.subprocess()
def test_custom_writer():
tracer = ddtrace.trace.Tracer()
from typing import List
from typing import Optional

from ddtrace.internal import debug
from ddtrace.internal.writer import TraceWriter
from ddtrace.trace import Span
from ddtrace.trace import tracer

class CustomWriter(TraceWriter):
def recreate(self) -> TraceWriter:
Expand All @@ -317,16 +317,24 @@ def flush_queue(self) -> None:
assert info.get("agent_url") == "CUSTOM"


@pytest.mark.subprocess()
def test_different_samplers():
tracer = ddtrace.trace.Tracer()
import ddtrace
from ddtrace.internal import debug
from ddtrace.trace import tracer

tracer._configure(sampler=ddtrace._trace.sampler.RateSampler())
info = debug.collect(tracer)

assert info.get("sampler_type") == "RateSampler"


@pytest.mark.subprocess()
def test_startup_logs_sampling_rules():
tracer = ddtrace.trace.Tracer()
import ddtrace
from ddtrace.internal import debug
from ddtrace.trace import tracer

sampler = ddtrace._trace.sampler.DatadogSampler(rules=[ddtrace._trace.sampler.SamplingRule(sample_rate=1.0)])
tracer._configure(sampler=sampler)
f = debug.collect(tracer)
Expand Down Expand Up @@ -415,7 +423,7 @@ def test_debug_span_log():


def test_partial_flush_log():
tracer = ddtrace.trace.Tracer()
tracer = DummyTracer()

tracer._configure(
partial_flush_enabled=True,
Expand Down
6 changes: 1 addition & 5 deletions tests/integration/test_encoding.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,14 @@
import mock
import pytest

from ddtrace.trace import Tracer
from ddtrace.trace import tracer


AGENT_VERSION = os.environ.get("AGENT_VERSION")


class TestTraceAcceptedByAgent:
def test_simple_trace_accepted_by_agent(self):
tracer = Tracer()
with mock.patch("ddtrace.internal.writer.writer.log") as log:
with tracer.trace("root"):
for _ in range(999):
Expand All @@ -32,7 +31,6 @@ def test_simple_trace_accepted_by_agent(self):
)
def test_trace_with_meta_accepted_by_agent(self, tags):
"""Meta tags should be text types."""
tracer = Tracer()
with mock.patch("ddtrace.internal.writer.writer.log") as log:
with tracer.trace("root", service="test_encoding", resource="test_resource") as root:
root.set_tags(tags)
Expand All @@ -53,7 +51,6 @@ def test_trace_with_meta_accepted_by_agent(self, tags):
)
def test_trace_with_metrics_accepted_by_agent(self, metrics):
"""Metric tags should be numeric types - i.e. int, float, long (py3), and str numbers."""
tracer = Tracer()
with mock.patch("ddtrace.internal.writer.writer.log") as log:
with tracer.trace("root") as root:
root.set_metrics(metrics)
Expand All @@ -72,7 +69,6 @@ def test_trace_with_metrics_accepted_by_agent(self, metrics):
)
def test_trace_with_links_accepted_by_agent(self, span_links_kwargs):
"""Links should not break things."""
tracer = Tracer()
with mock.patch("ddtrace.internal.writer.writer.log") as log:
with tracer.trace("root", service="test_encoding", resource="test_resource") as root:
root.set_link(**span_links_kwargs)
Expand Down
Loading

0 comments on commit 5b42292

Please sign in to comment.