chore(tracer): deprecate multiple initializations of ddtrace.Tracer […

…3.0] (#11823)

- Logs a deprecation warning if the Tracer is initialized more than
once. The global tracer.
- Ensures that multiple instances of the DummyTracer and CIVisibility
Tracer can be created (maintain the current behavior). This will help
reset tracer configurations between tests.
- Update some tests that require multiple tracer instances. This will
make the 3.0 upgrade easier to do.

## 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)

ddtrace/_trace/tracer.py

@@ -195,6 +195,7 @@ class Tracer(object):
     """
 
     SHUTDOWN_TIMEOUT = 5
+    _instance = None
 
     def __init__(
         self,
@@ -209,7 +210,23 @@ def __init__(
         :param url: The Datadog agent URL.
         :param dogstatsd_url: The DogStatsD URL.
         """
-
+        # Do not set self._instance if this is a subclass of Tracer. Here we only want
+        # to reference the global instance.
+        if type(self) is Tracer:
+            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",
+                )
         self._filters: List[TraceFilter] = []
 
         # globally set tags

ddtrace/contrib/grpc/__init__.py

@@ -46,13 +46,12 @@
 
     import grpc
     from ddtrace import patch
-    from ddtrace.trace import Pin, Tracer
+    from ddtrace.trace import Pin
 
     patch(grpc=True)
-    custom_tracer = Tracer()
 
     # override the pin on the client
-    Pin.override(grpc.Channel, service='mygrpc', tracer=custom_tracer)
+    Pin.override(grpc.Channel, service='mygrpc')
     with grpc.insecure_channel('localhost:50051') as channel:
         # create stubs and send requests
         pass
@@ -66,10 +65,9 @@
     from ddtrace.trace import Pin, Tracer
 
     patch(grpc=True)
-    custom_tracer = Tracer()
 
     # override the pin on the server
-    Pin.override(grpc.Server, service='mygrpc', tracer=custom_tracer)
+    Pin.override(grpc.Server, service='mygrpc')
     server = grpc.server(logging_pool.pool(2))
     server.add_insecure_port('localhost:50051')
     add_MyServicer_to_server(MyServicer(), server)

ddtrace/contrib/vertica/__init__.py

@@ -33,11 +33,10 @@
 
     import vertica_python
 
-    custom_tracer = Tracer()
     conn = vertica_python.connect(**YOUR_VERTICA_CONFIG)
 
     # override the service and tracer to be used
-    Pin.override(conn, service='myverticaservice', tracer=custom_tracer)
+    Pin.override(conn, service='myverticaservice')
 """
 
 

ddtrace/internal/ci_visibility/recorder.py

@@ -145,6 +145,12 @@ def _do_request(method, url, payload, headers, timeout=DEFAULT_TIMEOUT):
     return result
 
 
+class CIVisibilityTracer(Tracer):
+    def __init__(self, *args, **kwargs):
+        # Allows for multiple instances of the civis tracer to be created without logging a warning
+        super(CIVisibilityTracer, self).__init__(*args, **kwargs)
+
+
 class CIVisibility(Service):
     _instance = None  # type: Optional[CIVisibility]
     enabled = False
@@ -166,7 +172,7 @@ def __init__(self, tracer=None, config=None, service=None):
                     log.debug("Using _CI_DD_AGENT_URL for CI Visibility tracer: %s", env_agent_url)
                     url = env_agent_url
 
-                self.tracer = Tracer(context_provider=CIContextProvider(), url=url)
+                self.tracer = CIVisibilityTracer(context_provider=CIContextProvider(), url=url)
             else:
                 self.tracer = ddtrace.tracer
 

ddtrace/opentracer/tracer.py

@@ -18,6 +18,7 @@
 from ddtrace.internal.constants import SPAN_API_OPENTRACING
 from ddtrace.internal.utils.config import get_application_name
 from ddtrace.settings import ConfigException
+from ddtrace.vendor.debtcollector import deprecate
 
 from ..internal.logger import get_logger
 from .propagation import HTTPPropagator
@@ -70,8 +71,8 @@ def __init__(
             If ``None`` is provided, defaults to
             :class:`opentracing.scope_managers.ThreadLocalScopeManager`.
         :param dd_tracer: (optional) the Datadog tracer for this tracer to use. This
-            should only be passed if a custom Datadog tracer is being used. Defaults
-            to the global ``ddtrace.tracer`` tracer.
+            parameter is deprecated and will be removed in v3.0.0. The
+            to the global tracer (``ddtrace.tracer``) should always be used.
         """
         # Merge the given config with the default into a new dict
         self._config = DEFAULT_CONFIG.copy()
@@ -99,7 +100,14 @@ def __init__(
         self._scope_manager = scope_manager or ThreadLocalScopeManager()
         dd_context_provider = get_context_provider_for_scope_manager(self._scope_manager)
 
-        self._dd_tracer = dd_tracer or ddtrace.tracer or DatadogTracer()
+        if dd_tracer is not None:
+            deprecate(
+                "The ``dd_tracer`` parameter is deprecated",
+                message="The global tracer (``ddtrace.tracer``) will be used instead.",
+                removal_version="3.0.0",
+            )
+
+        self._dd_tracer = dd_tracer or ddtrace.tracer
         self._dd_tracer.set_tags(self._config.get(keys.GLOBAL_TAGS))  # type: ignore[arg-type]
         self._dd_tracer.configure(
             enabled=self._config.get(keys.ENABLED),

releasenotes/notes/deprecate-multiple-tracer-instances-078b920081ba4a36.yaml

@@ -0,0 +1,4 @@
+---
+deprecations:
+  - |
+    tracing: Deprecates the use of multiple tracer instances in the same process. The global tracer (``ddtrace.tracer``) `should be used instead.

tests/integration/test_context_snapshots.py

@@ -1,9 +1,8 @@
 import pytest
 
+from tests.integration.utils import AGENT_VERSION
 from tests.utils import snapshot
 
-from .test_integration import AGENT_VERSION
-
 
 pytestmark = pytest.mark.skipif(AGENT_VERSION != "testagent", reason="Tests only compatible with a testagent")
 

tests/integration/test_debug.py

@@ -1,10 +1,8 @@
-from datetime import datetime
 import json
 import logging
 import os
 import re
 import subprocess
-import sys
 from typing import List
 from typing import Optional
 
@@ -17,11 +15,10 @@
 from ddtrace.internal import debug
 from ddtrace.internal.writer import AgentWriter
 from ddtrace.internal.writer import TraceWriter
+from tests.integration.utils import AGENT_VERSION
 from tests.subprocesstest import SubprocessTestCase
 from tests.subprocesstest import run_in_subprocess
 
-from .test_integration import AGENT_VERSION
-
 
 pytestmark = pytest.mark.skipif(AGENT_VERSION == "testagent", reason="The test agent doesn't support startup logs.")
 
@@ -36,7 +33,14 @@ def __eq__(self, other):
     return Match()
 
 
+@pytest.mark.subprocess()
 def test_standard_tags():
+    from datetime import datetime
+    import sys
+
+    import ddtrace
+    from ddtrace.internal import debug
+
     f = debug.collect(ddtrace.tracer)
 
     date = f.get("date")
@@ -94,7 +98,7 @@ def test_standard_tags():
     assert f.get("tracer_enabled") is True
     assert f.get("sampler_type") == "DatadogSampler"
     assert f.get("priority_sampler_type") == "N/A"
-    assert f.get("service") == "tests.integration"
+    assert f.get("service") == "ddtrace_subprocess_dir"
     assert f.get("dd_version") == ""
     assert f.get("debug") is False
     assert f.get("enabled_cli") is False
@@ -110,8 +114,13 @@ def test_standard_tags():
     assert icfg["flask"] == "N/A"
 
 
+@pytest.mark.subprocess()
 def test_debug_post_configure():
-    tracer = ddtrace.Tracer()
+    import re
+
+    from ddtrace import tracer
+    from ddtrace.internal import debug
+
     tracer.configure(
         hostname="0.0.0.0",
         port=1234,
@@ -122,16 +131,21 @@ def test_debug_post_configure():
     agent_url = f.get("agent_url")
     assert agent_url == "http://0.0.0.0:1234"
 
-    assert f.get("is_global_tracer") is False
+    assert f.get("is_global_tracer") is True
     assert f.get("tracer_enabled") is True
 
     agent_error = f.get("agent_error")
     # Error code can differ between Python version
     assert re.match("^Agent not reachable.*Connection refused", agent_error)
 
-    # Tracer doesn't support re-configure()-ing with a UDS after an initial
-    # configure with normal http settings. So we need a new tracer instance.
-    tracer = ddtrace.Tracer()
+
+@pytest.mark.subprocess()
+def test_debug_post_configure_uds():
+    import re
+
+    from ddtrace import tracer
+    from ddtrace.internal import debug
+
     tracer.configure(uds_path="/file.sock")
 
     f = debug.collect(tracer)

tests/integration/test_encoding.py

@@ -18,7 +18,7 @@ def test_simple_trace_accepted_by_agent(self):
                 for _ in range(999):
                     with tracer.trace("child"):
                         pass
-            tracer.shutdown()
+            tracer.flush()
         log.warning.assert_not_called()
         log.error.assert_not_called()
 
@@ -39,7 +39,7 @@ def test_trace_with_meta_accepted_by_agent(self, tags):
                 for _ in range(999):
                     with tracer.trace("child") as child:
                         child.set_tags(tags)
-            tracer.shutdown()
+            tracer.flush()
         log.warning.assert_not_called()
         log.error.assert_not_called()
 
@@ -60,7 +60,7 @@ def test_trace_with_metrics_accepted_by_agent(self, metrics):
                 for _ in range(999):
                     with tracer.trace("child") as child:
                         child.set_metrics(metrics)
-            tracer.shutdown()
+            tracer.flush()
         log.warning.assert_not_called()
         log.error.assert_not_called()
 
@@ -79,6 +79,6 @@ def test_trace_with_links_accepted_by_agent(self, span_links_kwargs):
                 for _ in range(10):
                     with tracer.trace("child") as child:
                         child.set_link(**span_links_kwargs)
-            tracer.shutdown()
+            tracer.flush()
         log.warning.assert_not_called()
         log.error.assert_not_called()

tests/integration/test_integration.py

@@ -10,21 +10,20 @@
 from ddtrace import Tracer
 from ddtrace.internal.atexit import register_on_exit_signal
 from ddtrace.internal.runtime import container
-from ddtrace.internal.writer import AgentWriter
-from tests.integration.utils import AGENT_VERSION
-from tests.integration.utils import BadEncoder
 from tests.integration.utils import import_ddtrace_in_subprocess
 from tests.integration.utils import parametrize_with_all_encodings
-from tests.integration.utils import send_invalid_payload_and_get_logs
 from tests.integration.utils import skip_if_testagent
 from tests.utils import call_program
 
 
 FOUR_KB = 1 << 12
 
 
+@pytest.mark.subprocess()
 def test_configure_keeps_api_hostname_and_port():
-    tracer = Tracer()
+    from ddtrace import tracer
+    from tests.integration.utils import AGENT_VERSION
+
     assert tracer._writer.agent_url == "http://localhost:{}".format("9126" if AGENT_VERSION == "testagent" else "8126")
     tracer.configure(hostname="127.0.0.1", port=8127)
     assert tracer._writer.agent_url == "http://127.0.0.1:8127"
@@ -506,8 +505,12 @@ def test_validate_headers_in_payload_to_intake_with_nested_spans():
     assert headers.get("X-Datadog-Trace-Count") == "10"
 
 
+@parametrize_with_all_encodings
 def test_trace_with_invalid_client_endpoint_generates_error_log():
-    t = Tracer()
+    import mock
+
+    from ddtrace import tracer as t
+
     for client in t._writer._clients:
         client.ENDPOINT = "/bad"
     with mock.patch("ddtrace.internal.writer.writer.log") as log:
@@ -526,7 +529,12 @@ def test_trace_with_invalid_client_endpoint_generates_error_log():
 
 
 @skip_if_testagent
+@pytest.mark.subprocess(err=None)
 def test_trace_with_invalid_payload_generates_error_log():
+    import mock
+
+    from tests.integration.utils import send_invalid_payload_and_get_logs
+
     log = send_invalid_payload_and_get_logs()
     log.error.assert_has_calls(
         [
@@ -541,11 +549,11 @@ def test_trace_with_invalid_payload_generates_error_log():
 
 
 @skip_if_testagent
-@pytest.mark.subprocess(env={"_DD_TRACE_WRITER_LOG_ERROR_PAYLOADS": "true", "DD_TRACE_API_VERSION": "v0.5"})
+@pytest.mark.subprocess(env={"_DD_TRACE_WRITER_LOG_ERROR_PAYLOADS": "true", "DD_TRACE_API_VERSION": "v0.5"}, err=None)
 def test_trace_with_invalid_payload_logs_payload_when_LOG_ERROR_PAYLOADS():
     import mock
 
-    from tests.integration.test_integration import send_invalid_payload_and_get_logs
+    from tests.integration.utils import send_invalid_payload_and_get_logs
 
     log = send_invalid_payload_and_get_logs()
     log.error.assert_has_calls(
@@ -562,12 +570,12 @@ def test_trace_with_invalid_payload_logs_payload_when_LOG_ERROR_PAYLOADS():
 
 
 @skip_if_testagent
-@pytest.mark.subprocess(env={"_DD_TRACE_WRITER_LOG_ERROR_PAYLOADS": "true", "DD_TRACE_API_VERSION": "v0.5"})
+@pytest.mark.subprocess(env={"_DD_TRACE_WRITER_LOG_ERROR_PAYLOADS": "true", "DD_TRACE_API_VERSION": "v0.5"}, err=None)
 def test_trace_with_non_bytes_payload_logs_payload_when_LOG_ERROR_PAYLOADS():
     import mock
 
-    from tests.integration.test_integration import send_invalid_payload_and_get_logs
     from tests.integration.utils import BadEncoder
+    from tests.integration.utils import send_invalid_payload_and_get_logs
 
     class NonBytesBadEncoder(BadEncoder):
         def encode(self):
@@ -590,7 +598,11 @@ def encode_traces(self, traces):
     )
 
 
+@pytest.mark.subprocess(err=None)
 def test_trace_with_failing_encoder_generates_error_log():
+    from tests.integration.utils import BadEncoder
+    from tests.integration.utils import send_invalid_payload_and_get_logs
+
     class ExceptionBadEncoder(BadEncoder):
         def encode(self):
             raise Exception()
@@ -620,8 +632,11 @@ def test_api_version_downgrade_generates_no_warning_logs():
     log.error.assert_not_called()
 
 
+@pytest.mark.subprocess()
 def test_synchronous_writer_shutdown_raises_no_exception():
-    tracer = Tracer()
+    from ddtrace import tracer
+    from ddtrace.internal.writer import AgentWriter
+
     tracer.configure(writer=AgentWriter(tracer._writer.agent_url, sync_mode=True))
     tracer.shutdown()