Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(tracer): deprecate multiple initializations of ddtrace.Tracer [3.0] #11823

Merged
merged 25 commits into from
Jan 15, 2025
Merged
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
edb7504
chore(tracer): make tracer a singleton
mabdinur Dec 20, 2024
4f93e15
Merge branch 'main' into munir/make-tracer-a-singleton
mabdinur Dec 20, 2024
728a816
rn
mabdinur Dec 20, 2024
a32d3f0
fix special method and add a regression test
mabdinur Dec 23, 2024
1ba2723
Merge branch 'main' into munir/make-tracer-a-singleton
mabdinur Dec 23, 2024
a7c4581
fix warning
mabdinur Dec 23, 2024
fc48722
make sure civis has it's own tracer instance
mabdinur Dec 23, 2024
cd27fc4
do not make cisvis tracer a singleton
mabdinur Dec 23, 2024
2325f7a
fix snapshot tests
mabdinur Dec 23, 2024
bad3790
fix failing tracer, profiling, stats, and integration tests
mabdinur Dec 24, 2024
f0a88da
add missing snapshot
mabdinur Dec 24, 2024
22fb7a8
fix failing integration and civis tests
mabdinur Dec 24, 2024
ceb999d
lint and fix civis failures
mabdinur Dec 24, 2024
cc6bd3f
fix more testagent tests
mabdinur Jan 2, 2025
491ca61
make this a deprecation not a feautre change
mabdinur Jan 3, 2025
da277f7
update singleton test
mabdinur Jan 3, 2025
2f768e1
Merge branch 'main' into munir/make-tracer-a-singleton
mabdinur Jan 13, 2025
59f74e3
Merge branch 'main' into munir/make-tracer-a-singleton
mabdinur Jan 14, 2025
81e5ca4
avoid using new, use init
mabdinur Jan 14, 2025
64388c8
fix civis return
mabdinur Jan 14, 2025
d1b5c11
fix _instance reference in tracer
mabdinur Jan 14, 2025
2b0a186
ensure Tracer._instance references the global tracer
mabdinur Jan 14, 2025
2dca2a1
add back snapshot test
mabdinur Jan 15, 2025
e6a5918
Merge branch 'main' into munir/make-tracer-a-singleton
mabdinur Jan 15, 2025
e6d9b45
Merge branch 'main' into munir/make-tracer-a-singleton
mabdinur Jan 15, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 18 additions & 1 deletion ddtrace/_trace/tracer.py
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,7 @@ class Tracer(object):
"""

SHUTDOWN_TIMEOUT = 5
_instance = None

def __init__(
self,
Expand All @@ -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:
mabdinur marked this conversation as resolved.
Show resolved Hide resolved
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
Expand Down
8 changes: 3 additions & 5 deletions ddtrace/contrib/grpc/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
Expand Down
3 changes: 1 addition & 2 deletions ddtrace/contrib/vertica/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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')
"""


Expand Down
8 changes: 7 additions & 1 deletion ddtrace/internal/ci_visibility/recorder.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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

Expand Down
14 changes: 11 additions & 3 deletions ddtrace/opentracer/tracer.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
mabdinur marked this conversation as resolved.
Show resolved Hide resolved
"""
# Merge the given config with the default into a new dict
self._config = DEFAULT_CONFIG.copy()
Expand Down Expand Up @@ -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),
Expand Down
Original file line number Diff line number Diff line change
@@ -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.
3 changes: 1 addition & 2 deletions tests/integration/test_context_snapshots.py
Original file line number Diff line number Diff line change
@@ -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")

Expand Down
34 changes: 24 additions & 10 deletions tests/integration/test_debug.py
Original file line number Diff line number Diff line change
@@ -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

Expand All @@ -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.")

Expand All @@ -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")
Expand Down Expand Up @@ -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
Expand All @@ -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,
Expand All @@ -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)
Expand Down
8 changes: 4 additions & 4 deletions tests/integration/test_encoding.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand All @@ -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()

Expand All @@ -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()

Expand All @@ -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()
37 changes: 26 additions & 11 deletions tests/integration/test_integration.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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:
Expand All @@ -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(
[
Expand All @@ -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(
Expand All @@ -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):
Expand All @@ -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()
Expand Down Expand Up @@ -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()

Expand Down
Loading
Loading