Skip to content

Commit c4644f0

Browse files
authored
chore(tracing): removes tracer._configure from products and tests (#12696)
## Background In ddtrace v3.0, most functionality previously handled by tracer.configure(...) was internalized. As a result, ddtrace >3.0 no longer provides a public API for configuring agent connections, custom samplers, UDS, or enabling tracing. To avoid refactoring all tests and products relying on tracer.configure(...), we introduced the private method tracer._configure. This method has been used across ddtrace products (e.g., ASM, MLObs, and CiVis), integrations (e.g., Tornado, Vertex AI), and hundreds of tests. Now, it’s time to fully migrate away from this API. 🫡 ## Motivation This refactor encourages the use of environment variables and/or remote configuration for setting up tracing features. Even though tracer._configure is internal, it can still be used to enable unsupported behavior. This PR aims to put an end to that. 🫡 ## Changes - Removes ddtrace.Tracer._configure from the library. - Updates tests requiring global tracer reconfiguration to run in a subprocess. - Introduces Tracer._reset. - Removes tests validating unsupported behavior (e.g., using tracer.configure to change the agent URL). - Removes tests validating unsupported behavior (e.g., using tracer.configure to change the agent URL). Note: This PR also introduces Tracer._recreate(...), a private method used in ASM and CIVis tests to recreate the agent writer and span processors. This was added to limit the scope of changes in this PR. Ideally, tests requiring tracer reconfiguration/restart should be run in a subprocess. This is a temporary workaround (hopefully). Follow up: - Deprecate `tracer.configure(...)`. - Provide a public method for setting trace processors (ex: Tracer.set_processor) - Encourage ASM users to enable features via environment variables - Remove `tracer.configure(...)` from all tests. ## 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)
1 parent 0289159 commit c4644f0

File tree

83 files changed

+431
-784
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

83 files changed

+431
-784
lines changed

benchmarks/threading/scenario.py

Lines changed: 6 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -2,28 +2,16 @@
22
import random
33
from typing import Callable
44
from typing import Generator
5-
from typing import List
6-
from typing import Optional
75

86
import bm
97

10-
from ddtrace._trace.span import Span
11-
from ddtrace._trace.tracer import Tracer
12-
from ddtrace.internal.writer import TraceWriter
8+
from ddtrace.trace import TraceFilter
9+
from ddtrace.trace import Tracer
1310

1411

15-
class NoopWriter(TraceWriter):
16-
def recreate(self) -> TraceWriter:
17-
return NoopWriter()
18-
19-
def stop(self, timeout: Optional[float] = None) -> None:
20-
pass
21-
22-
def write(self, spans: Optional[List[Span]] = None) -> None:
23-
pass
24-
25-
def flush_queue(self) -> None:
26-
pass
12+
class _DropTraces(TraceFilter):
13+
def process_trace(self, trace):
14+
return
2715

2816

2917
class Threading(bm.Scenario):
@@ -42,7 +30,7 @@ def run(self) -> Generator[Callable[[int], None], None, None]:
4230
from ddtrace.trace import tracer
4331

4432
# configure global tracer to drop traces rather
45-
tracer.configure(writer=NoopWriter())
33+
tracer.configure(trace_processors=[_DropTraces()])
4634

4735
def _(loops: int) -> None:
4836
for _ in range(loops):

ddtrace/_trace/tracer.py

Lines changed: 21 additions & 132 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414
from typing import Tuple
1515
from typing import TypeVar
1616
from typing import Union
17-
from urllib import parse
1817

1918
from ddtrace import _hooks
2019
from ddtrace import config
@@ -411,48 +410,6 @@ def configure(
411410
:param List[TraceProcessor] trace_processors: This parameter sets TraceProcessor (ex: TraceFilters).
412411
Trace processors are used to modify and filter traces based on certain criteria.
413412
"""
414-
return self._configure(
415-
context_provider=context_provider,
416-
trace_processors=trace_processors,
417-
compute_stats_enabled=compute_stats_enabled,
418-
appsec_enabled=appsec_enabled,
419-
iast_enabled=iast_enabled,
420-
apm_tracing_disabled=apm_tracing_disabled,
421-
)
422-
423-
def _configure(
424-
self,
425-
enabled: Optional[bool] = None,
426-
hostname: Optional[str] = None,
427-
port: Optional[int] = None,
428-
uds_path: Optional[str] = None,
429-
https: Optional[bool] = None,
430-
sampler: Optional[DatadogSampler] = None,
431-
context_provider: Optional[BaseContextProvider] = None,
432-
wrap_executor: Optional[Callable] = None,
433-
priority_sampling: Optional[bool] = None,
434-
trace_processors: Optional[List[TraceProcessor]] = None,
435-
dogstatsd_url: Optional[str] = None,
436-
writer: Optional[TraceWriter] = None,
437-
partial_flush_enabled: Optional[bool] = None,
438-
partial_flush_min_spans: Optional[int] = None,
439-
api_version: Optional[str] = None,
440-
compute_stats_enabled: Optional[bool] = None,
441-
appsec_enabled: Optional[bool] = None,
442-
iast_enabled: Optional[bool] = None,
443-
apm_tracing_disabled: Optional[bool] = None,
444-
) -> None:
445-
if enabled is not None:
446-
self.enabled = enabled
447-
448-
if trace_processors is not None:
449-
self._user_trace_processors = trace_processors
450-
451-
if partial_flush_enabled is not None:
452-
self._partial_flush_enabled = partial_flush_enabled
453-
454-
if partial_flush_min_spans is not None:
455-
self._partial_flush_min_spans = partial_flush_min_spans
456413

457414
if appsec_enabled is not None:
458415
asm_config._asm_enabled = appsec_enabled
@@ -468,114 +425,34 @@ def _configure(
468425
# Disable compute stats (neither agent or tracer should compute them)
469426
config._trace_compute_stats = False
470427
# Update the rate limiter to 1 trace per minute when tracing is disabled
471-
if sampler is not None:
472-
sampler._rate_limit_always_on = True
473-
sampler.limiter.rate_limit = 1
474-
sampler.limiter.time_window = 60e9
475-
else:
476-
sampler = DatadogSampler(rate_limit=1, rate_limit_window=60e9, rate_limit_always_on=True)
428+
self._sampler = DatadogSampler(rate_limit=1, rate_limit_window=60e9, rate_limit_always_on=True)
477429
log.debug("ASM standalone mode is enabled, traces will be rate limited at 1 trace per minute")
478430

479-
if sampler is not None:
480-
self._sampler = sampler
481-
482-
if dogstatsd_url is not None:
483-
self._dogstatsd_url = dogstatsd_url
484-
485-
if any(x is not None for x in [hostname, port, uds_path, https]):
486-
# If any of the parts of the URL have updated, merge them with
487-
# the previous writer values.
488-
prev_url_parsed = parse.urlparse(self._agent_url)
489-
490-
if uds_path is not None:
491-
if hostname is None and prev_url_parsed.scheme == "unix":
492-
hostname = prev_url_parsed.hostname
493-
new_url = "unix://%s%s" % (hostname or "", uds_path)
494-
else:
495-
if https is None:
496-
https = prev_url_parsed.scheme == "https"
497-
if hostname is None:
498-
hostname = prev_url_parsed.hostname or ""
499-
if port is None:
500-
port = prev_url_parsed.port
501-
scheme = "https" if https else "http"
502-
new_url = "%s://%s:%s" % (scheme, hostname, port)
503-
verify_url(new_url)
504-
self._agent_url = new_url
505-
else:
506-
new_url = None
507-
508431
if compute_stats_enabled is not None:
509432
self._compute_stats = compute_stats_enabled
510433

511-
try:
512-
self._writer.stop()
513-
except ServiceStatusError:
514-
# It's possible the writer never got started
515-
pass
516-
517-
if writer is not None:
518-
self._writer = writer
519-
elif any(x is not None for x in [new_url, api_version, sampler, dogstatsd_url, appsec_enabled]):
520-
if asm_config._asm_enabled:
521-
api_version = "v0.4"
522-
self._writer = AgentWriter(
523-
self._agent_url,
524-
dogstatsd=get_dogstatsd_client(self._dogstatsd_url),
525-
sync_mode=self._use_sync_mode(),
526-
api_version=api_version,
527-
# if apm opt out, neither agent or tracer should compute the stats
528-
headers=(
529-
{"Datadog-Client-Computed-Stats": "yes"}
530-
if (compute_stats_enabled or asm_config._apm_opt_out)
531-
else {}
532-
),
533-
report_metrics=not asm_config._apm_opt_out,
534-
response_callback=self._agent_response_callback,
535-
)
536-
elif writer is None and isinstance(self._writer, LogWriter):
537-
# No need to do anything for the LogWriter.
538-
pass
539434
if isinstance(self._writer, AgentWriter):
435+
if appsec_enabled:
436+
self._writer._api_version = "v0.4"
540437
self._writer.dogstatsd = get_dogstatsd_client(self._dogstatsd_url)
541438

439+
if trace_processors:
440+
self._user_trace_processors = trace_processors
441+
542442
if any(
543443
x is not None
544444
for x in [
545-
partial_flush_min_spans,
546-
partial_flush_enabled,
547-
writer,
548-
dogstatsd_url,
549-
hostname,
550-
port,
551-
https,
552-
uds_path,
553-
api_version,
554-
sampler,
555445
trace_processors,
556446
compute_stats_enabled,
557447
appsec_enabled,
558448
iast_enabled,
559449
]
560450
):
561-
self._span_processors, self._appsec_processor, self._deferred_processors = _default_span_processors_factory(
562-
self._user_trace_processors,
563-
self._writer,
564-
self._partial_flush_enabled,
565-
self._partial_flush_min_spans,
566-
self._compute_stats,
567-
self._single_span_sampling_rules,
568-
self._agent_url,
569-
self._sampler,
570-
self._endpoint_call_counter_span_processor,
571-
)
451+
self._recreate()
572452

573453
if context_provider is not None:
574454
self.context_provider = context_provider
575455

576-
if wrap_executor is not None:
577-
self._wrap_executor = wrap_executor
578-
579456
self._generate_diagnostic_logs()
580457

581458
def _agent_response_callback(self, resp: AgentResponse) -> None:
@@ -611,8 +488,22 @@ def _generate_diagnostic_logs(self):
611488

612489
def _child_after_fork(self):
613490
self._pid = getpid()
491+
self._recreate()
492+
self._new_process = True
493+
494+
def _recreate(self):
495+
"""Re-initialize the tracer's processors and trace writer. This method should only be used in tests."""
496+
# Stop the writer.
497+
# This will stop the periodic thread in HTTPWriters, preventing memory leaks and unnecessary I/O.
498+
try:
499+
self._writer.stop()
500+
except ServiceStatusError:
501+
# Some writers (ex: AgentWriter), start when the first trace chunk is encoded. Stopping
502+
# the writer before that point will raise a ServiceStatusError.
503+
pass
614504
# Re-create the background writer thread
615505
self._writer = self._writer.recreate()
506+
# Recreate the trace and span processors
616507
self._span_processors, self._appsec_processor, self._deferred_processors = _default_span_processors_factory(
617508
self._user_trace_processors,
618509
self._writer,
@@ -625,8 +516,6 @@ def _child_after_fork(self):
625516
self._endpoint_call_counter_span_processor,
626517
)
627518

628-
self._new_process = True
629-
630519
def _start_span_after_shutdown(
631520
self,
632521
name: str,

ddtrace/appsec/_remoteconfiguration.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -221,12 +221,12 @@ def _appsec_1click_activation(features: Mapping[str, Any], test_tracer: Optional
221221

222222
if rc_asm_enabled:
223223
if not asm_config._asm_enabled:
224-
tracer._configure(appsec_enabled=True)
224+
tracer.configure(appsec_enabled=True)
225225
else:
226226
asm_config._asm_enabled = True
227227
else:
228228
if asm_config._asm_enabled:
229-
tracer._configure(appsec_enabled=False)
229+
tracer.configure(appsec_enabled=False)
230230
else:
231231
asm_config._asm_enabled = False
232232

ddtrace/contrib/internal/tornado/application.py

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
from urllib.parse import urlparse
2+
13
from tornado import template
24

35
import ddtrace
@@ -34,21 +36,25 @@ def tracer_config(__init__, app, args, kwargs):
3436
service = settings["default_service"]
3537

3638
# extract extra settings
39+
# TODO: Remove `FILTERS` from supported settings
3740
trace_processors = settings.get("settings", {}).get("FILTERS")
3841

39-
# the tracer must use the right Context propagation and wrap executor;
40-
# this action is done twice because the patch() method uses the
41-
# global tracer while here we can have a different instance (even if
42-
# this is not usual).
43-
tracer._configure(
42+
tracer.configure(
4443
context_provider=context_provider,
45-
wrap_executor=decorators.wrap_executor,
46-
enabled=settings.get("enabled", None),
47-
hostname=settings.get("agent_hostname", None),
48-
port=settings.get("agent_port", None),
4944
trace_processors=trace_processors,
5045
)
51-
46+
tracer._wrap_executor = decorators.wrap_executor
47+
# TODO: Remove `enabled`, `hostname` and `port` settings in v4.0
48+
# Tracer should be configured via environment variables
49+
if settings.get("enabled") is not None:
50+
tracer.enabled = settings["enabled"]
51+
if settings.get("hostname") is not None or settings.get("port") is not None:
52+
curr_agent_url = urlparse(tracer._agent_url)
53+
hostname = settings.get("hostname", curr_agent_url.hostname)
54+
port = settings.get("port", curr_agent_url.port)
55+
tracer._agent_url = tracer._writer.intake_url = f"{curr_agent_url.scheme}://{hostname}:{port}"
56+
tracer._recreate()
57+
# TODO: Remove tags from settings, tags should be set via `DD_TAGS` environment variable
5258
# set global tags if any
5359
tags = settings.get("tags", None)
5460
if tags:

ddtrace/contrib/internal/tornado/patch.py

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -50,10 +50,8 @@ def patch():
5050
_w("tornado.template", "Template.generate", template.generate)
5151

5252
# configure the global tracer
53-
ddtrace.tracer._configure(
54-
context_provider=context_provider,
55-
wrap_executor=decorators.wrap_executor,
56-
)
53+
ddtrace.tracer.context_provider = context_provider
54+
ddtrace.tracer._wrap_executor = decorators.wrap_executor
5755

5856

5957
def unpatch():

ddtrace/internal/ci_visibility/recorder.py

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,9 @@ def __init__(
175175

176176
# Partial traces are required for ITR to work in suite-level skipping for long test sessions, but we
177177
# assume that a tracer is already configured if it's been passed in.
178-
self.tracer._configure(partial_flush_enabled=True, partial_flush_min_spans=TRACER_PARTIAL_FLUSH_MIN_SPANS)
178+
self.tracer._partial_flush_enabled = True
179+
self.tracer._partial_flush_min_spans = TRACER_PARTIAL_FLUSH_MIN_SPANS
180+
self.tracer._recreate()
179181

180182
self._api_client: Optional[_TestVisibilityAPIClientBase] = None
181183

@@ -395,7 +397,8 @@ def _configure_writer(
395397
itr_suite_skipping_mode=self._suite_skipping_mode,
396398
)
397399
if writer is not None:
398-
self.tracer._configure(writer=writer)
400+
self.tracer._writer = writer
401+
self.tracer._recreate()
399402

400403
def _agent_evp_proxy_is_available(self) -> bool:
401404
try:
@@ -602,7 +605,7 @@ def _start_service(self) -> None:
602605
tracer_filters = self.tracer._user_trace_processors
603606
if not any(isinstance(tracer_filter, TraceCiVisibilityFilter) for tracer_filter in tracer_filters):
604607
tracer_filters += [TraceCiVisibilityFilter(self._tags, self._service)] # type: ignore[arg-type]
605-
self.tracer._configure(trace_processors=tracer_filters)
608+
self.tracer.configure(trace_processors=tracer_filters)
606609

607610
if self.test_skipping_enabled():
608611
self._fetch_tests_to_skip()

ddtrace/internal/ci_visibility/writer.py

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,6 @@ def __init__(
108108
dogstatsd=None, # type: Optional[DogStatsd]
109109
sync_mode=False, # type: bool
110110
report_metrics=False, # type: bool
111-
api_version=None, # type: Optional[str]
112111
reuse_connections=None, # type: Optional[bool]
113112
headers=None, # type: Optional[Dict[str, str]]
114113
use_evp=False, # type: bool
@@ -129,10 +128,13 @@ def __init__(
129128
if not intake_url:
130129
intake_url = "%s.%s" % (AGENTLESS_BASE_URL, os.getenv("DD_SITE", AGENTLESS_DEFAULT_SITE))
131130

131+
self._use_evp = use_evp
132132
clients = (
133-
[CIVisibilityProxiedEventClient()] if use_evp else [CIVisibilityAgentlessEventClient()]
133+
[CIVisibilityProxiedEventClient()] if self._use_evp else [CIVisibilityAgentlessEventClient()]
134134
) # type: List[WriterClientBase]
135-
if coverage_enabled:
135+
self._coverage_enabled = coverage_enabled
136+
self._itr_suite_skipping_mode = itr_suite_skipping_mode
137+
if self._coverage_enabled:
136138
if not intake_cov_url:
137139
intake_cov_url = "%s.%s" % (AGENTLESS_COVERAGE_BASE_URL, os.getenv("DD_SITE", AGENTLESS_DEFAULT_SITE))
138140
clients.append(
@@ -154,6 +156,8 @@ def __init__(
154156
timeout=timeout,
155157
dogstatsd=dogstatsd,
156158
sync_mode=sync_mode,
159+
# FIXME(munir): report_metrics is not used in the CI Visibility Writers
160+
# self._report_metrics = report_metrics,
157161
reuse_connections=reuse_connections,
158162
headers=headers,
159163
)
@@ -170,6 +174,12 @@ def recreate(self):
170174
timeout=self._timeout,
171175
dogstatsd=self.dogstatsd,
172176
sync_mode=self._sync_mode,
177+
report_metrics=self._report_metrics,
178+
reuse_connections=self._reuse_connections,
179+
headers=self._headers,
180+
use_evp=self._use_evp,
181+
coverage_enabled=self._coverage_enabled,
182+
itr_suite_skipping_mode=self._itr_suite_skipping_mode,
173183
)
174184

175185
def _put(self, data, headers, client, no_trace):

0 commit comments

Comments
 (0)