From edb750480f00a1852300dde823673f168e2766f0 Mon Sep 17 00:00:00 2001 From: Munir Abdinur Date: Fri, 20 Dec 2024 14:34:14 -0500 Subject: [PATCH 01/19] chore(tracer): make tracer a singleton --- ddtrace/_trace/tracer.py | 18 ++++++++++++++++++ ddtrace/contrib/grpc/__init__.py | 8 +++----- ddtrace/contrib/vertica/__init__.py | 3 +-- ddtrace/opentracer/tracer.py | 14 +++++++++++--- tests/utils.py | 5 +++++ 5 files changed, 38 insertions(+), 10 deletions(-) diff --git a/ddtrace/_trace/tracer.py b/ddtrace/_trace/tracer.py index 6027976d6dc..d7d6cca8f78 100644 --- a/ddtrace/_trace/tracer.py +++ b/ddtrace/_trace/tracer.py @@ -196,6 +196,24 @@ class Tracer(object): """ SHUTDOWN_TIMEOUT = 5 + _instance = None + + def ___new__(cls, *args, **kwargs): + if cls._instance is None: + cls._instance = super(Tracer, cls).__new__(cls) + 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( + "Creating multiple Tracer instances is deprecated", + "Use ddtrace.tracer to access the global tracer instance", + category=DDTraceDeprecationWarning, + removal_version="3.0.0", + ) + return cls._instance def __init__( self, diff --git a/ddtrace/contrib/grpc/__init__.py b/ddtrace/contrib/grpc/__init__.py index c329762316c..ce09c5fb324 100644 --- a/ddtrace/contrib/grpc/__init__.py +++ b/ddtrace/contrib/grpc/__init__.py @@ -45,13 +45,12 @@ ``Pin`` API:: import grpc - from ddtrace import Pin, patch, Tracer + from ddtrace import Pin, patch 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 @@ -64,10 +63,9 @@ from ddtrace import Pin, patch, 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) diff --git a/ddtrace/contrib/vertica/__init__.py b/ddtrace/contrib/vertica/__init__.py index dfafe39a79f..cae4e15f3d2 100644 --- a/ddtrace/contrib/vertica/__init__.py +++ b/ddtrace/contrib/vertica/__init__.py @@ -32,11 +32,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') """ from ddtrace.internal.utils.importlib import require_modules diff --git a/ddtrace/opentracer/tracer.py b/ddtrace/opentracer/tracer.py index 489c025037a..110fad2401c 100644 --- a/ddtrace/opentracer/tracer.py +++ b/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), diff --git a/tests/utils.py b/tests/utils.py index de0129f75a3..42d6787d401 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -609,6 +609,11 @@ class DummyTracer(Tracer): DummyTracer is a tracer which uses the DummyWriter by default """ + def ___new__(cls, *args, **kwargs): + # Override the __new__ method to ensure we can have multiple instances of the DummyTracer + cls._instance = super(object, cls).__new__(cls) + return cls._instance + def __init__(self, *args, **kwargs): super(DummyTracer, self).__init__() self._trace_flush_disabled_via_env = not asbool(os.getenv("_DD_TEST_TRACE_FLUSH_ENABLED", True)) From 728a816e08fd43b427cce831dd58ef9b934f73a3 Mon Sep 17 00:00:00 2001 From: Munir Abdinur Date: Fri, 20 Dec 2024 16:07:07 -0500 Subject: [PATCH 02/19] rn --- .../deprecate-multiple-tracer-instances-078b920081ba4a36.yaml | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 releasenotes/notes/deprecate-multiple-tracer-instances-078b920081ba4a36.yaml diff --git a/releasenotes/notes/deprecate-multiple-tracer-instances-078b920081ba4a36.yaml b/releasenotes/notes/deprecate-multiple-tracer-instances-078b920081ba4a36.yaml new file mode 100644 index 00000000000..7b96d366269 --- /dev/null +++ b/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. From a32d3f02989a7d95305d841e0fbd91a6c409aed7 Mon Sep 17 00:00:00 2001 From: Munir Abdinur Date: Mon, 23 Dec 2024 10:40:06 -0500 Subject: [PATCH 03/19] fix special method and add a regression test --- ddtrace/_trace/tracer.py | 4 ++-- tests/tracer/test_tracer.py | 20 ++++++++++++++++++++ tests/utils.py | 4 ++-- 3 files changed, 24 insertions(+), 4 deletions(-) diff --git a/ddtrace/_trace/tracer.py b/ddtrace/_trace/tracer.py index d7d6cca8f78..cd587afd4ed 100644 --- a/ddtrace/_trace/tracer.py +++ b/ddtrace/_trace/tracer.py @@ -198,7 +198,7 @@ class Tracer(object): SHUTDOWN_TIMEOUT = 5 _instance = None - def ___new__(cls, *args, **kwargs): + def __new__(cls, *args, **kwargs): if cls._instance is None: cls._instance = super(Tracer, cls).__new__(cls) else: @@ -209,7 +209,7 @@ def ___new__(cls, *args, **kwargs): # TODO(mabdinur): Convert this warning to an ValueError in 3.0.0 deprecate( "Creating multiple Tracer instances is deprecated", - "Use ddtrace.tracer to access the global tracer instance", + ". Use ddtrace.tracer instead.", category=DDTraceDeprecationWarning, removal_version="3.0.0", ) diff --git a/tests/tracer/test_tracer.py b/tests/tracer/test_tracer.py index 4cdcf876aba..f09aff96630 100644 --- a/tests/tracer/test_tracer.py +++ b/tests/tracer/test_tracer.py @@ -2095,3 +2095,23 @@ def test_gc_not_used_on_root_spans(): # print("referrers:", [f"object {objects.index(r)}" for r in gc.get_referrers(obj)[:-2]]) # print("referents:", [f"object {objects.index(r)}" if r in objects else r for r in gc.get_referents(obj)]) # print("--------------------") + + +@pytest.mark.subprocess() +def test_multiple_tracer_instances(): + import warnings + + with warnings.catch_warnings(record=True) as warns: + warnings.simplefilter("always") + import ddtrace + + assert ddtrace.tracer is not None + assert len(warns) == 0 + + t = ddtrace.Tracer() + assert t is ddtrace.tracer + assert len(warns) == 1 + assert ( + str(warns[0].message) == "Creating multiple Tracer instances is deprecated and will be " + "removed in version '3.0.0'. Use ddtrace.tracer instead." + ) diff --git a/tests/utils.py b/tests/utils.py index 42d6787d401..b1ccfc38892 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -609,9 +609,9 @@ class DummyTracer(Tracer): DummyTracer is a tracer which uses the DummyWriter by default """ - def ___new__(cls, *args, **kwargs): + def __new__(cls, *args, **kwargs): # Override the __new__ method to ensure we can have multiple instances of the DummyTracer - cls._instance = super(object, cls).__new__(cls) + cls._instance = object.__new__(cls) return cls._instance def __init__(self, *args, **kwargs): From a7c4581e13e517f07dbb6194c724e46826a35b35 Mon Sep 17 00:00:00 2001 From: Munir Abdinur Date: Mon, 23 Dec 2024 11:18:13 -0500 Subject: [PATCH 04/19] fix warning --- ddtrace/_trace/tracer.py | 2 +- tests/tracer/test_tracer.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ddtrace/_trace/tracer.py b/ddtrace/_trace/tracer.py index 2c58a4f3061..d0cf3570218 100644 --- a/ddtrace/_trace/tracer.py +++ b/ddtrace/_trace/tracer.py @@ -207,7 +207,7 @@ def __new__(cls, *args, **kwargs): # broken experience. # TODO(mabdinur): Convert this warning to an ValueError in 3.0.0 deprecate( - "Creating multiple Tracer instances is deprecated", + "Support for multiple Tracer instances is deprecated", ". Use ddtrace.tracer instead.", category=DDTraceDeprecationWarning, removal_version="3.0.0", diff --git a/tests/tracer/test_tracer.py b/tests/tracer/test_tracer.py index f09aff96630..0978e3ae92b 100644 --- a/tests/tracer/test_tracer.py +++ b/tests/tracer/test_tracer.py @@ -2112,6 +2112,6 @@ def test_multiple_tracer_instances(): assert t is ddtrace.tracer assert len(warns) == 1 assert ( - str(warns[0].message) == "Creating multiple Tracer instances is deprecated and will be " + str(warns[0].message) == "Support for multiple Tracer instances is deprecated and will be " "removed in version '3.0.0'. Use ddtrace.tracer instead." ) From fc4872244c4dce3dfdfd338b302c8a2672b7cb8c Mon Sep 17 00:00:00 2001 From: Munir Abdinur Date: Mon, 23 Dec 2024 13:25:35 -0500 Subject: [PATCH 05/19] make sure civis has it's own tracer instance --- ddtrace/internal/ci_visibility/recorder.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/ddtrace/internal/ci_visibility/recorder.py b/ddtrace/internal/ci_visibility/recorder.py index eb8f00d5405..809db6ac889 100644 --- a/ddtrace/internal/ci_visibility/recorder.py +++ b/ddtrace/internal/ci_visibility/recorder.py @@ -145,6 +145,15 @@ def _do_request(method, url, payload, headers, timeout=DEFAULT_TIMEOUT): return result +class CIVisibilityTracer(Tracer): + def __new__(cls, *args, **kwargs): + if cls._instance is None: + cls._instance = object.__new__(cls) + else: + log.debug("CIVisibilityTracer instance already exists, returning existing instance") + return cls._instance + + class CIVisibility(Service): _instance = None # type: Optional[CIVisibility] enabled = False @@ -166,7 +175,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 From cd27fc45402cb135b51961f980f751458fed6ce5 Mon Sep 17 00:00:00 2001 From: Munir Abdinur Date: Mon, 23 Dec 2024 14:15:42 -0500 Subject: [PATCH 06/19] do not make cisvis tracer a singleton --- ddtrace/internal/ci_visibility/recorder.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/ddtrace/internal/ci_visibility/recorder.py b/ddtrace/internal/ci_visibility/recorder.py index 809db6ac889..799f97c4b38 100644 --- a/ddtrace/internal/ci_visibility/recorder.py +++ b/ddtrace/internal/ci_visibility/recorder.py @@ -147,10 +147,8 @@ def _do_request(method, url, payload, headers, timeout=DEFAULT_TIMEOUT): class CIVisibilityTracer(Tracer): def __new__(cls, *args, **kwargs): - if cls._instance is None: - cls._instance = object.__new__(cls) - else: - log.debug("CIVisibilityTracer instance already exists, returning existing instance") + # Allows for multiple instances of the civis tracer to be created (unlike ddtrace.tracer) + cls._instance = object.__new__(cls) return cls._instance From 2325f7a458825ee3dd91eb62f5c3a99140bd4c32 Mon Sep 17 00:00:00 2001 From: Munir Abdinur Date: Mon, 23 Dec 2024 18:27:33 -0500 Subject: [PATCH 07/19] fix snapshot tests --- tests/utils.py | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/tests/utils.py b/tests/utils.py index b1ccfc38892..1c3d1fdc7fb 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -1160,11 +1160,6 @@ def wrapper(wrapped, instance, args, kwargs): else: clsname = "" - if include_tracer: - tracer = Tracer() - else: - tracer = ddtrace.tracer - module = inspect.getmodule(wrapped) # Use the fully qualified function name as a unique test token to @@ -1178,14 +1173,14 @@ def wrapper(wrapped, instance, args, kwargs): with snapshot_context( token, ignores=ignores, - tracer=tracer, + tracer=ddtrace.tracer, async_mode=async_mode, variants=variants, wait_for_num_traces=wait_for_num_traces, ): # Run the test. if include_tracer: - kwargs["tracer"] = tracer + kwargs["tracer"] = ddtrace.tracer return wrapped(*args, **kwargs) return wrapper From bad3790e49280ccac8979d2d36c047e3bcd6142b Mon Sep 17 00:00:00 2001 From: Munir Abdinur Date: Mon, 23 Dec 2024 20:09:38 -0500 Subject: [PATCH 08/19] fix failing tracer, profiling, stats, and integration tests --- tests/integration/test_debug.py | 31 ++++++++++++++----- tests/integration/test_sampling.py | 9 ++++-- tests/integration/test_trace_stats.py | 16 +++++----- tests/profiling/test_profiler.py | 43 ++++++++++++++++++++++----- tests/tracer/test_tracer.py | 5 +++- 5 files changed, 79 insertions(+), 25 deletions(-) diff --git a/tests/integration/test_debug.py b/tests/integration/test_debug.py index e699db36d6b..c27481c23ab 100644 --- a/tests/integration/test_debug.py +++ b/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 @@ -36,7 +34,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 +99,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 +115,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 +132,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) diff --git a/tests/integration/test_sampling.py b/tests/integration/test_sampling.py index 902b430bbc8..707999b74b1 100644 --- a/tests/integration/test_sampling.py +++ b/tests/integration/test_sampling.py @@ -19,6 +19,9 @@ def snapshot_parametrized_with_writers(f): def _patch(writer, tracer): + old_sampler = tracer._sampler + old_writer = tracer._writer + old_tags = tracer._tags if writer == "sync": writer = AgentWriter( tracer.agent_trace_url, @@ -29,11 +32,13 @@ def _patch(writer, tracer): writer._headers = tracer._writer._headers else: writer = tracer._writer - tracer.configure(writer=writer) try: return f(writer, tracer) finally: - tracer.shutdown() + tracer.flush() + # Reset tracer configurations to avoid leaking state between tests + tracer.configure(sampler=old_sampler, writer=old_writer) + tracer._tags = old_tags wrapped = snapshot(include_tracer=True, token_override=f.__name__)(_patch) return pytest.mark.parametrize( diff --git a/tests/integration/test_trace_stats.py b/tests/integration/test_trace_stats.py index 0fd7695fc23..e4ffde3d81b 100644 --- a/tests/integration/test_trace_stats.py +++ b/tests/integration/test_trace_stats.py @@ -5,12 +5,12 @@ import mock import pytest -from ddtrace import Tracer from ddtrace.constants import SPAN_MEASURED_KEY from ddtrace.ext import http from ddtrace.internal.processor.stats import SpanStatsProcessorV06 from ddtrace.sampler import DatadogSampler from ddtrace.sampler import SamplingRule +from tests.utils import DummyTracer from tests.utils import override_global_config from .test_integration import AGENT_VERSION @@ -21,9 +21,8 @@ @pytest.fixture def stats_tracer(): - # type: (float) -> Generator[Tracer, None, None] with override_global_config(dict(_trace_compute_stats=True)): - tracer = Tracer() + tracer = DummyTracer() yield tracer tracer.shutdown() @@ -70,7 +69,7 @@ def test_compute_stats_default_and_configure(run_python_code_in_subprocess, envv """Ensure stats computation can be enabled.""" # Test enabling via `configure` - t = Tracer() + t = DummyTracer() assert not t._compute_stats assert not any(isinstance(p, SpanStatsProcessorV06) for p in t._span_processors) t.configure(compute_stats_enabled=True) @@ -100,14 +99,16 @@ def test_compute_stats_default_and_configure(run_python_code_in_subprocess, envv assert status == 0, out + err -def test_apm_opt_out_compute_stats_and_configure(run_python_code_in_subprocess): +@pytest.mark.subprocess(err=b"WARNING:root:IAST not enabled but native module is being loaded\n") +def test_apm_opt_out_compute_stats_and_configure(): """ Ensure stats computation is disabled, but reported as enabled, if APM is opt-out. """ + from ddtrace import tracer as t + from ddtrace.internal.processor.stats import SpanStatsProcessorV06 # Test via `configure` - t = Tracer() assert not t._compute_stats assert not any(isinstance(p, SpanStatsProcessorV06) for p in t._span_processors) t.configure(appsec_enabled=True, appsec_standalone_enabled=True) @@ -116,8 +117,9 @@ def test_apm_opt_out_compute_stats_and_configure(run_python_code_in_subprocess): assert not t._compute_stats # but it's reported as enabled assert t._writer._headers.get("Datadog-Client-Computed-Stats") == "yes" - t.configure(appsec_enabled=False, appsec_standalone_enabled=False) + +def test_apm_opt_out_compute_stats_and_configure_env(run_python_code_in_subprocess): # Test via environment variable env = os.environ.copy() env.update({"DD_EXPERIMENTAL_APPSEC_STANDALONE_ENABLED": "true", "DD_APPSEC_ENABLED": "true"}) diff --git a/tests/profiling/test_profiler.py b/tests/profiling/test_profiler.py index 7f98bbf6aa8..879a50afd54 100644 --- a/tests/profiling/test_profiler.py +++ b/tests/profiling/test_profiler.py @@ -232,36 +232,66 @@ def _check_url(prof, url, api_key, endpoint_path="profiling/v1/input"): pytest.fail("Unable to find HTTP exporter") +@pytest.mark.subprocess() def test_tracer_url(): - t = ddtrace.Tracer() + import os + + from ddtrace import tracer as t + from ddtrace.profiling import profiler + from tests.profiling.test_profiler import _check_url + t.configure(hostname="foobar") prof = profiler.Profiler(tracer=t) _check_url(prof, "http://foobar:8126", os.environ.get("DD_API_KEY")) +@pytest.mark.subprocess() def test_tracer_url_https(): - t = ddtrace.Tracer() + import os + + from ddtrace import tracer as t + from ddtrace.profiling import profiler + from tests.profiling.test_profiler import _check_url + t.configure(hostname="foobar", https=True) prof = profiler.Profiler(tracer=t) _check_url(prof, "https://foobar:8126", os.environ.get("DD_API_KEY")) +@pytest.mark.subprocess() def test_tracer_url_uds_hostname(): - t = ddtrace.Tracer() + import os + + from ddtrace import tracer as t + from ddtrace.profiling import profiler + from tests.profiling.test_profiler import _check_url + t.configure(hostname="foobar", uds_path="/foobar") prof = profiler.Profiler(tracer=t) _check_url(prof, "unix://foobar/foobar", os.environ.get("DD_API_KEY")) +@pytest.mark.subprocess() def test_tracer_url_uds(): - t = ddtrace.Tracer() + import os + + from ddtrace import tracer as t + from ddtrace.profiling import profiler + from tests.profiling.test_profiler import _check_url + t.configure(uds_path="/foobar") prof = profiler.Profiler(tracer=t) _check_url(prof, "unix:///foobar", os.environ.get("DD_API_KEY")) +@pytest.mark.subprocess() def test_tracer_url_configure_after(): - t = ddtrace.Tracer() + import os + + from ddtrace import tracer as t + from ddtrace.profiling import profiler + from tests.profiling.test_profiler import _check_url + prof = profiler.Profiler(tracer=t) t.configure(hostname="foobar") _check_url(prof, "http://foobar:8126", os.environ.get("DD_API_KEY")) @@ -276,11 +306,10 @@ def test_env_no_api_key(): def test_env_endpoint_url(): import os - import ddtrace + from ddtrace import tracer as t from ddtrace.profiling import profiler from tests.profiling.test_profiler import _check_url - t = ddtrace.Tracer() prof = profiler.Profiler(tracer=t) _check_url(prof, "http://foobar:123", os.environ.get("DD_API_KEY")) diff --git a/tests/tracer/test_tracer.py b/tests/tracer/test_tracer.py index 0978e3ae92b..21972d44492 100644 --- a/tests/tracer/test_tracer.py +++ b/tests/tracer/test_tracer.py @@ -2106,8 +2106,11 @@ def test_multiple_tracer_instances(): import ddtrace assert ddtrace.tracer is not None - assert len(warns) == 0 + for w in warns: + # Ensure the warning is not about multiple tracer instances is not logged when importing ddtrace + assert "Support for multiple Tracer instances is deprecated" not in str(w.message) + warns.clear() t = ddtrace.Tracer() assert t is ddtrace.tracer assert len(warns) == 1 From f0a88daa1e2c6600c3308a176397cda397d97d60 Mon Sep 17 00:00:00 2001 From: Munir Abdinur Date: Tue, 24 Dec 2024 01:15:06 -0500 Subject: [PATCH 09/19] add missing snapshot --- ...propagation.test_trace_tags_multispan.json | 70 +++++++++++++++++++ 1 file changed, 70 insertions(+) create mode 100644 tests/snapshots/tests.integration.test_propagation.test_trace_tags_multispan.json diff --git a/tests/snapshots/tests.integration.test_propagation.test_trace_tags_multispan.json b/tests/snapshots/tests.integration.test_propagation.test_trace_tags_multispan.json new file mode 100644 index 00000000000..000f5f143c7 --- /dev/null +++ b/tests/snapshots/tests.integration.test_propagation.test_trace_tags_multispan.json @@ -0,0 +1,70 @@ +[[ + { + "name": "p", + "service": "tests.integration", + "resource": "p", + "trace_id": 0, + "span_id": 1, + "parent_id": 5678, + "type": "", + "error": 0, + "meta": { + "_dd.p.dm": "-1", + "_dd.p.test": "value", + "language": "python", + "runtime-id": "65e7346cd27a4fcbb1a2ccb98722fed3" + }, + "metrics": { + "_dd.top_level": 1, + "_dd.tracer_kr": 1.0, + "_sampling_priority_v1": 1, + "process_id": 4531 + }, + "duration": 48000, + "start": 1735013581776627000 + }, + { + "name": "c1", + "service": "tests.integration", + "resource": "c1", + "trace_id": 0, + "span_id": 2, + "parent_id": 1, + "type": "", + "error": 0, + "meta": { + "_dd.p.test": "value" + }, + "duration": 5000, + "start": 1735013581776649000 + }, + { + "name": "c2", + "service": "tests.integration", + "resource": "c2", + "trace_id": 0, + "span_id": 3, + "parent_id": 1, + "type": "", + "error": 0, + "meta": { + "_dd.p.test": "value" + }, + "duration": 7000, + "start": 1735013581776662000 + }, + { + "name": "gc", + "service": "tests.integration", + "resource": "gc", + "trace_id": 0, + "span_id": 4, + "parent_id": 3, + "type": "", + "error": 0, + "meta": { + "_dd.p.test": "value" + }, + "duration": 11000, + "start": 1735013581776667000 + }]] From 22fb7a8d1565127084b293f02d43cd440099aa69 Mon Sep 17 00:00:00 2001 From: Munir Abdinur Date: Tue, 24 Dec 2024 15:19:25 -0500 Subject: [PATCH 10/19] fix failing integration and civis tests --- tests/integration/test_context_snapshots.py | 3 +- tests/integration/test_debug.py | 3 +- tests/integration/test_encoding.py | 8 ++-- tests/integration/test_integration.py | 37 +++++++++++----- .../test_integration_civisibility.py | 2 +- .../integration/test_integration_snapshots.py | 43 +++++++++++-------- tests/integration/test_priority_sampling.py | 3 +- tests/integration/test_propagation.py | 7 ++- tests/integration/test_sampling.py | 3 +- tests/integration/test_settings.py | 2 +- tests/integration/test_trace_stats.py | 5 +-- tests/integration/test_tracemethods.py | 2 +- tests/integration/utils.py | 2 +- 13 files changed, 69 insertions(+), 51 deletions(-) diff --git a/tests/integration/test_context_snapshots.py b/tests/integration/test_context_snapshots.py index 4ed44bd558e..612422064a0 100644 --- a/tests/integration/test_context_snapshots.py +++ b/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") diff --git a/tests/integration/test_debug.py b/tests/integration/test_debug.py index c27481c23ab..f0912b7f6fc 100644 --- a/tests/integration/test_debug.py +++ b/tests/integration/test_debug.py @@ -15,11 +15,10 @@ from ddtrace.internal.writer import AgentWriter from ddtrace.internal.writer import TraceWriter import ddtrace.sampler +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.") diff --git a/tests/integration/test_encoding.py b/tests/integration/test_encoding.py index 43c47ac4840..7138ff94e00 100644 --- a/tests/integration/test_encoding.py +++ b/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() diff --git a/tests/integration/test_integration.py b/tests/integration/test_integration.py index 78606bbde14..529bdbcd40b 100644 --- a/tests/integration/test_integration.py +++ b/tests/integration/test_integration.py @@ -10,12 +10,8 @@ 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 @@ -23,8 +19,11 @@ 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() diff --git a/tests/integration/test_integration_civisibility.py b/tests/integration/test_integration_civisibility.py index 8a504f6a220..9de08770b00 100644 --- a/tests/integration/test_integration_civisibility.py +++ b/tests/integration/test_integration_civisibility.py @@ -3,7 +3,6 @@ import mock import pytest -from ddtrace._trace.tracer import Tracer from ddtrace.internal import agent from ddtrace.internal.ci_visibility import CIVisibility from ddtrace.internal.ci_visibility._api_client import TestVisibilityAPISettings @@ -12,6 +11,7 @@ from ddtrace.internal.ci_visibility.constants import EVP_PROXY_AGENT_ENDPOINT from ddtrace.internal.ci_visibility.constants import EVP_SUBDOMAIN_HEADER_EVENT_VALUE from ddtrace.internal.ci_visibility.constants import EVP_SUBDOMAIN_HEADER_NAME +from ddtrace.internal.ci_visibility.recorder import CIVisibilityTracer as Tracer from ddtrace.internal.ci_visibility.writer import CIVisibilityWriter from ddtrace.internal.utils.http import Response from tests.ci_visibility.util import _get_default_civisibility_ddconfig diff --git a/tests/integration/test_integration_snapshots.py b/tests/integration/test_integration_snapshots.py index bd48faa34a6..642e37f8293 100644 --- a/tests/integration/test_integration_snapshots.py +++ b/tests/integration/test_integration_snapshots.py @@ -11,13 +11,12 @@ from ddtrace.constants import SAMPLING_PRIORITY_KEY from ddtrace.constants import USER_KEEP from ddtrace.internal.writer import AgentWriter +from tests.integration.utils import AGENT_VERSION from tests.integration.utils import mark_snapshot from tests.integration.utils import parametrize_with_all_encodings from tests.utils import override_global_config 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") @@ -31,7 +30,7 @@ def test_single_trace_single_span(tracer): s.set_metric("float_metric", 12.34) s.set_metric("int_metric", 4321) s.finish() - tracer.shutdown() + tracer.flush() @snapshot(include_tracer=True) @@ -49,15 +48,22 @@ def test_multiple_traces(tracer): s.set_metric("float_metric", 12.34) s.set_metric("int_metric", 4321) tracer.trace("child").finish() - tracer.shutdown() + tracer.flush() -@pytest.mark.parametrize( - "writer", - ("default", "sync"), -) @snapshot(include_tracer=True) -def test_filters(writer, tracer): +@pytest.mark.subprocess( + parametrize={"DD_WRITER_MODE": ["default", "sync"]}, + token="tests.integration.test_integration_snapshots.test_filters", +) +def test_filters(): + import os + + from ddtrace import tracer + from ddtrace.internal.writer import AgentWriter + + writer = os.environ.get("DD_WRITER_MODE", "default") + if writer == "sync": writer = AgentWriter( tracer.agent_trace_url, @@ -89,15 +95,18 @@ def process_trace(self, trace): with tracer.trace("root"): with tracer.trace("child"): pass - tracer.shutdown() + tracer.flush() # Have to use sync mode snapshot so that the traces are associated to this # test case since we use a custom writer (that doesn't have the trace headers # injected). +@pytest.mark.subprocess() @snapshot(async_mode=False) def test_synchronous_writer(): - tracer = Tracer() + from ddtrace import tracer + from ddtrace.internal.writer import AgentWriter + writer = AgentWriter(tracer._writer.agent_url, sync_mode=True) tracer.configure(writer=writer) with tracer.trace("operation1", service="my-svc"): @@ -122,14 +131,14 @@ def test_tracer_trace_across_popen(): def task(tracer): with tracer.trace("child"): pass - tracer.shutdown() + tracer.flush() with tracer.trace("parent"): p = multiprocessing.Process(target=task, args=(tracer,)) p.start() p.join() - tracer.shutdown() + tracer.flush() @snapshot(async_mode=False) @@ -146,19 +155,19 @@ def task(tracer): def task2(tracer): with tracer.trace("child2"): pass - tracer.shutdown() + tracer.flush() with tracer.trace("child1"): p = multiprocessing.Process(target=task2, args=(tracer,)) p.start() p.join() - tracer.shutdown() + tracer.flush() with tracer.trace("parent"): p = multiprocessing.Process(target=task, args=(tracer,)) p.start() p.join() - tracer.shutdown() + tracer.flush() @snapshot() @@ -225,7 +234,7 @@ def test_tracetagsprocessor_only_adds_new_tags(): span.context.sampling_priority = AUTO_KEEP span.set_metric(SAMPLING_PRIORITY_KEY, USER_KEEP) - tracer.shutdown() + tracer.flush() # Override the token so that both parameterizations of the test use the same snapshot diff --git a/tests/integration/test_priority_sampling.py b/tests/integration/test_priority_sampling.py index 59177be57cb..653ef96d49e 100644 --- a/tests/integration/test_priority_sampling.py +++ b/tests/integration/test_priority_sampling.py @@ -9,12 +9,11 @@ from ddtrace.internal.encoding import MsgpackEncoderV04 as Encoder from ddtrace.internal.writer import AgentWriter from ddtrace.tracer import Tracer +from tests.integration.utils import AGENT_VERSION from tests.integration.utils import parametrize_with_all_encodings from tests.integration.utils import skip_if_testagent from tests.utils import override_global_config -from .test_integration import AGENT_VERSION - def _turn_tracer_into_dummy(tracer): """Override tracer's writer's write() method to keep traces instead of sending them away""" diff --git a/tests/integration/test_propagation.py b/tests/integration/test_propagation.py index 5bd0a122a8c..c2f8f6f684b 100644 --- a/tests/integration/test_propagation.py +++ b/tests/integration/test_propagation.py @@ -3,10 +3,9 @@ from ddtrace import Tracer from ddtrace.constants import MANUAL_DROP_KEY from ddtrace.propagation.http import HTTPPropagator +from tests.integration.utils import AGENT_VERSION from tests.utils import override_global_config -from .test_integration import AGENT_VERSION - pytestmark = pytest.mark.skipif(AGENT_VERSION != "testagent", reason="Tests only compatible with a testagent") @@ -33,7 +32,7 @@ def tracer(request): kwargs["partial_flush_min_spans"] = partial_flush_min_spans tracer.configure(**kwargs) yield tracer - tracer.shutdown() + tracer.flush() @pytest.mark.snapshot() @@ -65,7 +64,7 @@ def test_trace_tags_multispan(): def downstream_tracer(): tracer = Tracer() yield tracer - tracer.shutdown() + tracer.flush() @pytest.mark.snapshot() diff --git a/tests/integration/test_sampling.py b/tests/integration/test_sampling.py index 707999b74b1..9211657ce38 100644 --- a/tests/integration/test_sampling.py +++ b/tests/integration/test_sampling.py @@ -7,10 +7,9 @@ from ddtrace.sampler import DatadogSampler from ddtrace.sampler import RateSampler from ddtrace.sampler import SamplingRule +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") RESOURCE = "mycoolre$ource" # codespell:ignore diff --git a/tests/integration/test_settings.py b/tests/integration/test_settings.py index 55e8d1e76d8..249b0211bb4 100644 --- a/tests/integration/test_settings.py +++ b/tests/integration/test_settings.py @@ -2,7 +2,7 @@ import pytest -from .test_integration import AGENT_VERSION +from tests.integration.utils import AGENT_VERSION def _get_telemetry_config_items(events, item_name): diff --git a/tests/integration/test_trace_stats.py b/tests/integration/test_trace_stats.py index e4ffde3d81b..72a53af490a 100644 --- a/tests/integration/test_trace_stats.py +++ b/tests/integration/test_trace_stats.py @@ -10,11 +10,10 @@ from ddtrace.internal.processor.stats import SpanStatsProcessorV06 from ddtrace.sampler import DatadogSampler from ddtrace.sampler import SamplingRule +from tests.integration.utils import AGENT_VERSION from tests.utils import DummyTracer from tests.utils import override_global_config -from .test_integration import AGENT_VERSION - pytestmark = pytest.mark.skipif(AGENT_VERSION != "testagent", reason="Tests only compatible with a testagent") @@ -99,7 +98,7 @@ def test_compute_stats_default_and_configure(run_python_code_in_subprocess, envv assert status == 0, out + err -@pytest.mark.subprocess(err=b"WARNING:root:IAST not enabled but native module is being loaded\n") +@pytest.mark.subprocess(err=None) def test_apm_opt_out_compute_stats_and_configure(): """ Ensure stats computation is disabled, but reported as enabled, diff --git a/tests/integration/test_tracemethods.py b/tests/integration/test_tracemethods.py index 8568cbc3737..15129c56161 100644 --- a/tests/integration/test_tracemethods.py +++ b/tests/integration/test_tracemethods.py @@ -5,7 +5,7 @@ import pytest -from .test_integration import AGENT_VERSION +from tests.integration.utils import AGENT_VERSION pytestmark = pytest.mark.skipif(AGENT_VERSION != "testagent", reason="Tests only compatible with a testagent") diff --git a/tests/integration/utils.py b/tests/integration/utils.py index dea4a091ed4..21822ea6e59 100644 --- a/tests/integration/utils.py +++ b/tests/integration/utils.py @@ -33,7 +33,7 @@ def send_invalid_payload_and_get_logs(encoder_cls=BadEncoder): client.encoder = encoder_cls() with mock.patch("ddtrace.internal.writer.writer.log") as log: t.trace("asdf").finish() - t.shutdown() + t.flush() return log From ceb999d1131aa3a77d8b2697003f5885a6ebe7ea Mon Sep 17 00:00:00 2001 From: Munir Abdinur Date: Tue, 24 Dec 2024 16:57:20 -0500 Subject: [PATCH 11/19] lint and fix civis failures --- tests/integration/test_integration_civisibility.py | 13 +++++++++---- tests/integration/test_integration_snapshots.py | 1 - 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/tests/integration/test_integration_civisibility.py b/tests/integration/test_integration_civisibility.py index 9de08770b00..6cb284457f8 100644 --- a/tests/integration/test_integration_civisibility.py +++ b/tests/integration/test_integration_civisibility.py @@ -7,13 +7,10 @@ from ddtrace.internal.ci_visibility import CIVisibility from ddtrace.internal.ci_visibility._api_client import TestVisibilityAPISettings from ddtrace.internal.ci_visibility.constants import AGENTLESS_ENDPOINT -from ddtrace.internal.ci_visibility.constants import COVERAGE_TAG_NAME from ddtrace.internal.ci_visibility.constants import EVP_PROXY_AGENT_ENDPOINT from ddtrace.internal.ci_visibility.constants import EVP_SUBDOMAIN_HEADER_EVENT_VALUE from ddtrace.internal.ci_visibility.constants import EVP_SUBDOMAIN_HEADER_NAME from ddtrace.internal.ci_visibility.recorder import CIVisibilityTracer as Tracer -from ddtrace.internal.ci_visibility.writer import CIVisibilityWriter -from ddtrace.internal.utils.http import Response from tests.ci_visibility.util import _get_default_civisibility_ddconfig from tests.utils import override_env @@ -74,9 +71,17 @@ def test_civisibility_intake_with_apikey(): CIVisibility.disable() +@pytest.mark.subprocess() def test_civisibility_intake_payloads(): + import mock + + from ddtrace import tracer as t + from ddtrace.internal.ci_visibility.constants import COVERAGE_TAG_NAME + from ddtrace.internal.ci_visibility.recorder import CIVisibilityWriter + from ddtrace.internal.utils.http import Response + from tests.utils import override_env + with override_env(dict(DD_API_KEY="foobar.baz")): - t = Tracer() t.configure(writer=CIVisibilityWriter(reuse_connections=True, coverage_enabled=True)) t._writer._conn = mock.MagicMock() with mock.patch("ddtrace.internal.writer.Response.from_http_response") as from_http_response: diff --git a/tests/integration/test_integration_snapshots.py b/tests/integration/test_integration_snapshots.py index 642e37f8293..4d3cca4b9ca 100644 --- a/tests/integration/test_integration_snapshots.py +++ b/tests/integration/test_integration_snapshots.py @@ -10,7 +10,6 @@ from ddtrace.constants import AUTO_KEEP from ddtrace.constants import SAMPLING_PRIORITY_KEY from ddtrace.constants import USER_KEEP -from ddtrace.internal.writer import AgentWriter from tests.integration.utils import AGENT_VERSION from tests.integration.utils import mark_snapshot from tests.integration.utils import parametrize_with_all_encodings From cc6bd3ffcd3c9708d63400ecb50f372ebfd408dd Mon Sep 17 00:00:00 2001 From: Munir Abdinur Date: Thu, 2 Jan 2025 14:30:33 -0500 Subject: [PATCH 12/19] fix more testagent tests --- .../integration/test_integration_snapshots.py | 30 +++++++++++-------- tests/integration/test_sampling.py | 11 ++++++- ...ace_with_wrong_metrics_types_not_sent.json | 25 ---------------- ...tracetagsprocessor_only_adds_new_tags.json | 2 +- 4 files changed, 29 insertions(+), 39 deletions(-) delete mode 100644 tests/snapshots/tests.integration.test_integration_snapshots.test_trace_with_wrong_metrics_types_not_sent.json diff --git a/tests/integration/test_integration_snapshots.py b/tests/integration/test_integration_snapshots.py index 4d3cca4b9ca..d39e52e84dd 100644 --- a/tests/integration/test_integration_snapshots.py +++ b/tests/integration/test_integration_snapshots.py @@ -7,9 +7,6 @@ from ddtrace import Tracer from ddtrace import tracer -from ddtrace.constants import AUTO_KEEP -from ddtrace.constants import SAMPLING_PRIORITY_KEY -from ddtrace.constants import USER_KEEP from tests.integration.utils import AGENT_VERSION from tests.integration.utils import mark_snapshot from tests.integration.utils import parametrize_with_all_encodings @@ -21,7 +18,10 @@ @snapshot(include_tracer=True) +@pytest.mark.subprocess() def test_single_trace_single_span(tracer): + from ddtrace import tracer + s = tracer.trace("operation", service="my-svc") s.set_tag("k", "v") # numeric tag @@ -33,7 +33,10 @@ def test_single_trace_single_span(tracer): @snapshot(include_tracer=True) +@pytest.mark.subprocess() def test_multiple_traces(tracer): + from ddtrace import tracer + with tracer.trace("operation1", service="my-svc") as s: s.set_tag("k", "v") s.set_tag("num", 1234) @@ -125,7 +128,6 @@ def test_tracer_trace_across_popen(): the child span has does not have '_dd.p.dm' shows that sampling was run before fork automatically. """ - tracer = Tracer() def task(tracer): with tracer.trace("child"): @@ -148,7 +150,6 @@ def test_tracer_trace_across_multiple_popens(): the child span has does not have '_dd.p.dm' shows that sampling was run before fork automatically. """ - tracer = Tracer() def task(tracer): def task2(tracer): @@ -170,9 +171,13 @@ def task2(tracer): @snapshot() +@pytest.mark.subprocess() def test_wrong_span_name_type_not_sent(): """Span names should be a text type.""" - tracer = Tracer() + import mock + + from ddtrace import tracer + with mock.patch("ddtrace._trace.span.log") as log: with tracer.trace(123): pass @@ -188,11 +193,9 @@ def test_wrong_span_name_type_not_sent(): ], ) @pytest.mark.parametrize("encoding", ["v0.4", "v0.5"]) -@snapshot() def test_trace_with_wrong_meta_types_not_sent(encoding, meta, monkeypatch): """Wrong meta types should raise TypeErrors during encoding and fail to send to the agent.""" with override_global_config(dict(_trace_api=encoding)): - tracer = Tracer() with mock.patch("ddtrace._trace.span.log") as log: with tracer.trace("root") as root: root._meta = meta @@ -211,8 +214,6 @@ def test_trace_with_wrong_meta_types_not_sent(encoding, meta, monkeypatch): ], ) @pytest.mark.parametrize("encoding", ["v0.4", "v0.5"]) -@snapshot() -@pytest.mark.xfail def test_trace_with_wrong_metrics_types_not_sent(encoding, metrics, monkeypatch): """Wrong metric types should raise TypeErrors during encoding and fail to send to the agent.""" with override_global_config(dict(_trace_api=encoding)): @@ -226,9 +227,14 @@ def test_trace_with_wrong_metrics_types_not_sent(encoding, metrics, monkeypatch) log.exception.assert_called_once_with("error closing trace") -@snapshot() +@pytest.mark.subprocess() +@pytest.mark.snapshot() def test_tracetagsprocessor_only_adds_new_tags(): - tracer = Tracer() + from ddtrace import tracer + from ddtrace.constants import AUTO_KEEP + from ddtrace.constants import SAMPLING_PRIORITY_KEY + from ddtrace.constants import USER_KEEP + with tracer.trace(name="web.request") as span: span.context.sampling_priority = AUTO_KEEP span.set_metric(SAMPLING_PRIORITY_KEY, USER_KEEP) diff --git a/tests/integration/test_sampling.py b/tests/integration/test_sampling.py index 9211657ce38..9d124eb4c39 100644 --- a/tests/integration/test_sampling.py +++ b/tests/integration/test_sampling.py @@ -1,4 +1,3 @@ -import mock import pytest from ddtrace.constants import MANUAL_DROP_KEY @@ -302,10 +301,14 @@ def test_extended_sampling_float_special_case_match_star(writer, tracer): span.set_tag("tag", 20.1) +@pytest.mark.subprocess() def test_rate_limiter_on_spans(tracer): """ Ensure that the rate limiter is applied to spans """ + from ddtrace import tracer + from ddtrace.sampler import DatadogSampler + # Rate limit is only applied if a sample rate or trace sample rule is set tracer.configure(sampler=DatadogSampler(default_sample_rate=1, rate_limit=10)) spans = [] @@ -329,10 +332,16 @@ def test_rate_limiter_on_spans(tracer): assert dropped_span.context.sampling_priority < 0 +@pytest.mark.subprocess() def test_rate_limiter_on_long_running_spans(tracer): """ Ensure that the rate limiter is applied on increasing time intervals """ + import mock + + from ddtrace import tracer + from ddtrace.sampler import DatadogSampler + tracer.configure(sampler=DatadogSampler(rate_limit=5)) with mock.patch("ddtrace.internal.rate_limiter.time.monotonic_ns", return_value=1617333414): diff --git a/tests/snapshots/tests.integration.test_integration_snapshots.test_trace_with_wrong_metrics_types_not_sent.json b/tests/snapshots/tests.integration.test_integration_snapshots.test_trace_with_wrong_metrics_types_not_sent.json deleted file mode 100644 index a1a67aeefc8..00000000000 --- a/tests/snapshots/tests.integration.test_integration_snapshots.test_trace_with_wrong_metrics_types_not_sent.json +++ /dev/null @@ -1,25 +0,0 @@ -[[ - { - "name": "parent", - "service": "tests.integration", - "resource": "parent", - "trace_id": 0, - "span_id": 1, - "parent_id": 0, - "type": "", - "error": 0, - "meta": { - "_dd.p.dm": "-0", - "_dd.p.tid": "65f8a77100000000", - "language": "python", - "runtime-id": "005360373bf04c7fb732555994db4f78" - }, - "metrics": { - "_dd.top_level": 1, - "_dd.tracer_kr": 1.0, - "_sampling_priority_v1": 1, - "process_id": 5837 - }, - "duration": 1004386709, - "start": 1710794609240060721 - }]] diff --git a/tests/snapshots/tests.integration.test_integration_snapshots.test_tracetagsprocessor_only_adds_new_tags.json b/tests/snapshots/tests.integration.test_integration_snapshots.test_tracetagsprocessor_only_adds_new_tags.json index 9298b2342cd..08108bdeff9 100644 --- a/tests/snapshots/tests.integration.test_integration_snapshots.test_tracetagsprocessor_only_adds_new_tags.json +++ b/tests/snapshots/tests.integration.test_integration_snapshots.test_tracetagsprocessor_only_adds_new_tags.json @@ -1,7 +1,7 @@ [[ { "name": "web.request", - "service": "tests.integration", + "service": "ddtrace_subprocess_dir", "resource": "web.request", "trace_id": 0, "span_id": 1, From 491ca61d448db21f1d0199c7d71bb570dd3d302d Mon Sep 17 00:00:00 2001 From: Munir Abdinur Date: Fri, 3 Jan 2025 11:03:46 -0500 Subject: [PATCH 13/19] make this a deprecation not a feautre change --- ddtrace/_trace/tracer.py | 26 ++++++++-------- tests/integration/test_propagation.py | 43 +++------------------------ 2 files changed, 17 insertions(+), 52 deletions(-) diff --git a/ddtrace/_trace/tracer.py b/ddtrace/_trace/tracer.py index d0cf3570218..feb1267f5a9 100644 --- a/ddtrace/_trace/tracer.py +++ b/ddtrace/_trace/tracer.py @@ -200,19 +200,19 @@ class Tracer(object): def __new__(cls, *args, **kwargs): if cls._instance is None: cls._instance = super(Tracer, cls).__new__(cls) - 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", - ) - return cls._instance + return cls._instance + # 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", + ) + return super(Tracer, cls).__new__(cls) def __init__( self, diff --git a/tests/integration/test_propagation.py b/tests/integration/test_propagation.py index c2f8f6f684b..bcad0ed4432 100644 --- a/tests/integration/test_propagation.py +++ b/tests/integration/test_propagation.py @@ -1,43 +1,15 @@ import pytest -from ddtrace import Tracer +from ddtrace import tracer from ddtrace.constants import MANUAL_DROP_KEY from ddtrace.propagation.http import HTTPPropagator from tests.integration.utils import AGENT_VERSION -from tests.utils import override_global_config pytestmark = pytest.mark.skipif(AGENT_VERSION != "testagent", reason="Tests only compatible with a testagent") -@pytest.fixture( - params=[ - dict(global_config=dict()), - dict( - global_config=dict(_x_datadog_tags_max_length="0", _x_datadog_tags_enabled=False), - ), - dict(global_config=dict(), partial_flush_enabled=True, partial_flush_min_spans=2), - ] -) -def tracer(request): - global_config = request.param.get("global_config", dict()) - partial_flush_enabled = request.param.get("partial_flush_enabled") - partial_flush_min_spans = request.param.get("partial_flush_min_spans") - with override_global_config(global_config): - tracer = Tracer() - kwargs = dict() - if partial_flush_enabled: - kwargs["partial_flush_enabled"] = partial_flush_enabled - if partial_flush_min_spans: - kwargs["partial_flush_min_spans"] = partial_flush_min_spans - tracer.configure(**kwargs) - yield tracer - tracer.flush() - - -@pytest.mark.snapshot() def test_trace_tags_multispan(): - tracer = Tracer() headers = { "x-datadog-trace-id": "1234", "x-datadog-parent-id": "5678", @@ -60,15 +32,8 @@ def test_trace_tags_multispan(): gc.finish() -@pytest.fixture -def downstream_tracer(): - tracer = Tracer() - yield tracer - tracer.flush() - - @pytest.mark.snapshot() -def test_sampling_decision_downstream(downstream_tracer): +def test_sampling_decision_downstream(): """ Ensures that set_tag(MANUAL_DROP_KEY) on a span causes the sampling decision meta and sampling priority metric to be set appropriately indicating rejection @@ -80,7 +45,7 @@ def test_sampling_decision_downstream(downstream_tracer): "x-datadog-tags": "_dd.p.dm=-1", } kept_trace_context = HTTPPropagator.extract(headers_indicating_kept_trace) - downstream_tracer.context_provider.activate(kept_trace_context) + tracer.context_provider.activate(kept_trace_context) - with downstream_tracer.trace("p", service="downstream") as span_to_reject: + with tracer.trace("p", service="downstream") as span_to_reject: span_to_reject.set_tag(MANUAL_DROP_KEY) From da277f7bf8cc9d8c860fc89ad9e0be9b5432accb Mon Sep 17 00:00:00 2001 From: Munir Abdinur Date: Fri, 3 Jan 2025 12:29:54 -0500 Subject: [PATCH 14/19] update singleton test --- tests/tracer/test_tracer.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/tracer/test_tracer.py b/tests/tracer/test_tracer.py index 21972d44492..28754ffd64e 100644 --- a/tests/tracer/test_tracer.py +++ b/tests/tracer/test_tracer.py @@ -2112,7 +2112,8 @@ def test_multiple_tracer_instances(): warns.clear() t = ddtrace.Tracer() - assert t is ddtrace.tracer + # TODO: Update this assertion when the deprecation is removed and the tracer becomes a singleton + assert t is not ddtrace.tracer assert len(warns) == 1 assert ( str(warns[0].message) == "Support for multiple Tracer instances is deprecated and will be " From 81e5ca4fe8a4a1bc759b4cb523c877733714fdbb Mon Sep 17 00:00:00 2001 From: Munir Abdinur Date: Tue, 14 Jan 2025 15:56:16 -0500 Subject: [PATCH 15/19] avoid using new, use init --- ddtrace/_trace/tracer.py | 31 +++++++++------------- ddtrace/internal/ci_visibility/recorder.py | 8 +++--- tests/utils.py | 7 ++--- 3 files changed, 19 insertions(+), 27 deletions(-) diff --git a/ddtrace/_trace/tracer.py b/ddtrace/_trace/tracer.py index 745e4c2b760..bf9ebeca818 100644 --- a/ddtrace/_trace/tracer.py +++ b/ddtrace/_trace/tracer.py @@ -197,23 +197,6 @@ class Tracer(object): SHUTDOWN_TIMEOUT = 5 _instance = None - def __new__(cls, *args, **kwargs): - if cls._instance is None: - cls._instance = super(Tracer, cls).__new__(cls) - return cls._instance - # 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", - ) - return super(Tracer, cls).__new__(cls) - def __init__( self, url: Optional[str] = None, @@ -227,7 +210,19 @@ def __init__( :param url: The Datadog agent URL. :param dogstatsd_url: The DogStatsD URL. """ - + if self._instance is not None: + # 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._instance = self self._filters: List[TraceFilter] = [] # globally set tags diff --git a/ddtrace/internal/ci_visibility/recorder.py b/ddtrace/internal/ci_visibility/recorder.py index 5403fe5ea35..8d09dcd21bf 100644 --- a/ddtrace/internal/ci_visibility/recorder.py +++ b/ddtrace/internal/ci_visibility/recorder.py @@ -146,10 +146,10 @@ def _do_request(method, url, payload, headers, timeout=DEFAULT_TIMEOUT): class CIVisibilityTracer(Tracer): - def __new__(cls, *args, **kwargs): - # Allows for multiple instances of the civis tracer to be created (unlike ddtrace.tracer) - cls._instance = object.__new__(cls) - return cls._instance + def __init__(self, *args, **kwargs): + # Allows for multiple instances of the civis tracer to be created without logging a warning + self._instance = None + return super(CIVisibilityTracer, self).__init__(*args, **kwargs) class CIVisibility(Service): diff --git a/tests/utils.py b/tests/utils.py index 1c3d1fdc7fb..8eb610da1c1 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -609,12 +609,9 @@ class DummyTracer(Tracer): DummyTracer is a tracer which uses the DummyWriter by default """ - def __new__(cls, *args, **kwargs): - # Override the __new__ method to ensure we can have multiple instances of the DummyTracer - cls._instance = object.__new__(cls) - return cls._instance - def __init__(self, *args, **kwargs): + # Allows for multiple instances of the dummy tracer to be created without logging a warning + self._instance = None super(DummyTracer, self).__init__() self._trace_flush_disabled_via_env = not asbool(os.getenv("_DD_TEST_TRACE_FLUSH_ENABLED", True)) self._trace_flush_enabled = True From 64388c87f2c1ce74d53a8d9a1677755773beda37 Mon Sep 17 00:00:00 2001 From: Munir Abdinur Date: Tue, 14 Jan 2025 16:40:44 -0500 Subject: [PATCH 16/19] fix civis return --- ddtrace/internal/ci_visibility/recorder.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ddtrace/internal/ci_visibility/recorder.py b/ddtrace/internal/ci_visibility/recorder.py index 8d09dcd21bf..f57e51e03b1 100644 --- a/ddtrace/internal/ci_visibility/recorder.py +++ b/ddtrace/internal/ci_visibility/recorder.py @@ -149,7 +149,7 @@ class CIVisibilityTracer(Tracer): def __init__(self, *args, **kwargs): # Allows for multiple instances of the civis tracer to be created without logging a warning self._instance = None - return super(CIVisibilityTracer, self).__init__(*args, **kwargs) + super(CIVisibilityTracer, self).__init__(*args, **kwargs) class CIVisibility(Service): From d1b5c11b7074d8e8859f0f7afc30658057b4d390 Mon Sep 17 00:00:00 2001 From: Munir Abdinur Date: Tue, 14 Jan 2025 16:53:22 -0500 Subject: [PATCH 17/19] fix _instance reference in tracer --- ddtrace/_trace/tracer.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ddtrace/_trace/tracer.py b/ddtrace/_trace/tracer.py index bf9ebeca818..43d80ac91aa 100644 --- a/ddtrace/_trace/tracer.py +++ b/ddtrace/_trace/tracer.py @@ -222,7 +222,7 @@ def __init__( category=DDTraceDeprecationWarning, removal_version="3.0.0", ) - self._instance = self + Tracer._instance = self self._filters: List[TraceFilter] = [] # globally set tags From 2b0a186539a24126ef782e92f9b09fb6be453f22 Mon Sep 17 00:00:00 2001 From: Munir Abdinur Date: Tue, 14 Jan 2025 17:44:52 -0500 Subject: [PATCH 18/19] ensure Tracer._instance references the global tracer --- ddtrace/_trace/tracer.py | 30 ++++++++++++---------- ddtrace/internal/ci_visibility/recorder.py | 1 - tests/utils.py | 2 -- 3 files changed, 17 insertions(+), 16 deletions(-) diff --git a/ddtrace/_trace/tracer.py b/ddtrace/_trace/tracer.py index 43d80ac91aa..18a0d378811 100644 --- a/ddtrace/_trace/tracer.py +++ b/ddtrace/_trace/tracer.py @@ -210,19 +210,23 @@ def __init__( :param url: The Datadog agent URL. :param dogstatsd_url: The DogStatsD URL. """ - if self._instance is not None: - # 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", - ) - Tracer._instance = self + # 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 diff --git a/ddtrace/internal/ci_visibility/recorder.py b/ddtrace/internal/ci_visibility/recorder.py index f57e51e03b1..a69d1d72c6b 100644 --- a/ddtrace/internal/ci_visibility/recorder.py +++ b/ddtrace/internal/ci_visibility/recorder.py @@ -148,7 +148,6 @@ def _do_request(method, url, payload, headers, timeout=DEFAULT_TIMEOUT): class CIVisibilityTracer(Tracer): def __init__(self, *args, **kwargs): # Allows for multiple instances of the civis tracer to be created without logging a warning - self._instance = None super(CIVisibilityTracer, self).__init__(*args, **kwargs) diff --git a/tests/utils.py b/tests/utils.py index 8eb610da1c1..a2e8f55aa91 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -610,8 +610,6 @@ class DummyTracer(Tracer): """ def __init__(self, *args, **kwargs): - # Allows for multiple instances of the dummy tracer to be created without logging a warning - self._instance = None super(DummyTracer, self).__init__() self._trace_flush_disabled_via_env = not asbool(os.getenv("_DD_TEST_TRACE_FLUSH_ENABLED", True)) self._trace_flush_enabled = True From 2dca2a1b294f3034d7b103ef27a2a5d849ef0054 Mon Sep 17 00:00:00 2001 From: Munir Abdinur Date: Wed, 15 Jan 2025 15:18:33 -0500 Subject: [PATCH 19/19] add back snapshot test --- tests/integration/test_integration_snapshots.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/integration/test_integration_snapshots.py b/tests/integration/test_integration_snapshots.py index d39e52e84dd..6e656081eb1 100644 --- a/tests/integration/test_integration_snapshots.py +++ b/tests/integration/test_integration_snapshots.py @@ -214,6 +214,8 @@ def test_trace_with_wrong_meta_types_not_sent(encoding, meta, monkeypatch): ], ) @pytest.mark.parametrize("encoding", ["v0.4", "v0.5"]) +@snapshot() +@pytest.mark.xfail def test_trace_with_wrong_metrics_types_not_sent(encoding, metrics, monkeypatch): """Wrong metric types should raise TypeErrors during encoding and fail to send to the agent.""" with override_global_config(dict(_trace_api=encoding)):