Skip to content

Commit

Permalink
chore(tracing): internalize most of tracer.configure(...) [3.0] (#11973)
Browse files Browse the repository at this point in the history
## Motivation

- Ensures tracing users can not modify agent, statsd, sampling,
api_version, partial flushing, and agent writer configurations after
application startup. This will allow us to better support remote
configuration, help us decouple ddtrace products from tracing, and
improve the overall architecture of the tracer.

## Description

- Deprecates the following parameters from
`ddtrace.tracer.configure(...)`
      - enabled
      - wrapped_context
      - hostname
      - port
      - uds_path
      - https
      - sampler
      - priority_sampling
      - settings
      - dogstatsd_url
      - writer
      - partial_flush_enabled
      - partial_flush_min_spans
      - api_version 
      - compute_stats_enabled
- Adds the `trace_proccessors` parameter to
`ddtrace.tracer.configure(...)`. This new parameter will replace the
deprecated `settings` parameter (ex:
`ddtrace.tracer.configure(settings=....)`).
- Renames `Tracer._filter` to `Tracer._user_proccessor`. This makes it
clear that `TraceProccessors/TraceFIlters` can be used for modifying
traces, this functionality is not limited to filtering spans.
- Creates an internal `ddtrace.Tracer._configure(...)` method. This
method will allow ddtrace products to configure the tracer after
startup. For now we only want to prevent users from doing this. We will
revisit this design decision after decoupling ddtrace products.

## Checklist
- [x] PR author has checked that all the criteria below are met
- The PR description includes an overview of the change
- The PR description articulates the motivation for the change
- The change includes tests OR the PR description describes a testing
strategy
- The PR description notes risks associated with the change, if any
- Newly-added code is easy to change
- The change follows the [library release note
guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html)
- The change includes or references documentation updates if necessary
- Backport labels are set (if
[applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting))

## Reviewer Checklist
- [x] Reviewer has checked that all the criteria below are met 
- Title is accurate
- All changes are related to the pull request's stated goal
- Avoids breaking
[API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces)
changes
- Testing strategy adequately addresses listed risks
- Newly-added code is easy to change
- Release note makes sense to a user of the library
- If necessary, author has acknowledged and discussed the performance
implications of this PR as reported in the benchmarks PR comment
- Backport labels are set in a manner that is consistent with the
[release branch maintenance
policy](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)
  • Loading branch information
mabdinur authored Jan 17, 2025
1 parent b614655 commit 7520c7a
Show file tree
Hide file tree
Showing 71 changed files with 415 additions and 306 deletions.
181 changes: 159 additions & 22 deletions ddtrace/_trace/tracer.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
from ddtrace import _hooks
from ddtrace import config
from ddtrace._trace.context import Context
from ddtrace._trace.filters import TraceFilter
from ddtrace._trace.processor import SpanAggregator
from ddtrace._trace.processor import SpanProcessor
from ddtrace._trace.processor import TopLevelSpanProcessor
Expand Down Expand Up @@ -103,7 +102,7 @@ def _start_appsec_processor() -> Optional[Any]:


def _default_span_processors_factory(
trace_filters: List[TraceFilter],
trace_filters: List[TraceProcessor],
trace_writer: TraceWriter,
partial_flush_enabled: bool,
partial_flush_min_spans: int,
Expand Down Expand Up @@ -210,6 +209,7 @@ 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:
Expand All @@ -227,7 +227,8 @@ def __init__(
category=DDTraceDeprecationWarning,
removal_version="3.0.0",
)
self._filters: List[TraceFilter] = []

self._user_trace_processors: List[TraceProcessor] = []

# globally set tags
self._tags = config.tags.copy()
Expand Down Expand Up @@ -281,7 +282,7 @@ def __init__(
# Direct link to the appsec processor
self._endpoint_call_counter_span_processor = EndpointCallCounterProcessor()
self._span_processors, self._appsec_processor, self._deferred_processors = _default_span_processors_factory(
self._filters,
self._user_trace_processors,
self._writer,
self._partial_flush_enabled,
self._partial_flush_min_spans,
Expand Down Expand Up @@ -438,7 +439,6 @@ def get_log_correlation_context(self, active: Optional[Union[Context, Span]] = N
"env": config.env or "",
}

# TODO: deprecate this method and make sure users create a new tracer if they need different parameters
def configure(
self,
enabled: Optional[bool] = None,
Expand All @@ -460,42 +460,135 @@ def configure(
appsec_enabled: Optional[bool] = None,
iast_enabled: Optional[bool] = None,
appsec_standalone_enabled: Optional[bool] = None,
trace_processors: Optional[List[TraceProcessor]] = None,
) -> None:
"""Configure a Tracer.
:param bool enabled: If True, finished traces will be submitted to the API, else they'll be dropped.
:param str hostname: Hostname running the Trace Agent
:param int port: Port of the Trace Agent
:param str uds_path: The Unix Domain Socket path of the agent.
:param bool https: Whether to use HTTPS or HTTP.
:param object sampler: A custom Sampler instance, locally deciding to totally drop the trace or not.
:param object context_provider: The ``ContextProvider`` that will be used to retrieve
automatically the current call context. This is an advanced option that usually
doesn't need to be changed from the default value
doesn't need to be changed from the default value.
:param bool appsec_enabled: Enables Application Security Monitoring (ASM) for the tracer.
:param bool iast_enabled: Enables IAST support for the tracer
:param bool appsec_standalone_enabled: When tracing is disabled ensures ASM support is still enabled.
:param List[TraceProcessor] trace_processors: This parameter sets TraceProcessor (ex: TraceFilters).
Trace processors are used to modify and filter traces based on certain criteria.
:param bool enabled: If True, finished traces will be submitted to the API, else they'll be dropped.
This parameter is deprecated and will be removed.
:param str hostname: Hostname running the Trace Agent. This parameter is deprecated and will be removed.
:param int port: Port of the Trace Agent. This parameter is deprecated and will be removed.
:param str uds_path: The Unix Domain Socket path of the agent. This parameter is deprecated and will be removed.
:param bool https: Whether to use HTTPS or HTTP. This parameter is deprecated and will be removed.
:param object sampler: A custom Sampler instance, locally deciding to totally drop the trace or not.
This parameter is deprecated and will be removed.
:param object wrap_executor: callable that is used when a function is decorated with
``Tracer.wrap()``. This is an advanced option that usually doesn't need to be changed
from the default value
:param priority_sampling: This argument is deprecated and will be removed in a future version.
from the default value. This parameter is deprecated and will be removed.
:param priority_sampling: This parameter is deprecated and will be removed in a future version.
:param bool settings: This parameter is deprecated and will be removed.
:param str dogstatsd_url: URL for UDP or Unix socket connection to DogStatsD
This parameter is deprecated and will be removed.
:param TraceWriter writer: This parameter is deprecated and will be removed.
:param bool partial_flush_enabled: This parameter is deprecated and will be removed.
:param bool partial_flush_min_spans: This parameter is deprecated and will be removed.
:param str api_version: This parameter is deprecated and will be removed.
:param bool compute_stats_enabled: This parameter is deprecated and will be removed.
"""
if settings is not None:
deprecate(
"Support for ``tracer.configure(...)`` with the settings parameter is deprecated",
message="Please use the trace_processors parameter instead of settings['FILTERS'].",
version="3.0.0",
category=DDTraceDeprecationWarning,
)
trace_processors = (trace_processors or []) + (settings.get("FILTERS") or [])

return self._configure(
enabled,
hostname,
port,
uds_path,
https,
sampler,
context_provider,
wrap_executor,
priority_sampling,
trace_processors,
dogstatsd_url,
writer,
partial_flush_enabled,
partial_flush_min_spans,
api_version,
compute_stats_enabled,
appsec_enabled,
iast_enabled,
appsec_standalone_enabled,
True,
)

def _configure(
self,
enabled: Optional[bool] = None,
hostname: Optional[str] = None,
port: Optional[int] = None,
uds_path: Optional[str] = None,
https: Optional[bool] = None,
sampler: Optional[BaseSampler] = None,
context_provider: Optional[DefaultContextProvider] = None,
wrap_executor: Optional[Callable] = None,
priority_sampling: Optional[bool] = None,
trace_processors: Optional[List[TraceProcessor]] = None,
dogstatsd_url: Optional[str] = None,
writer: Optional[TraceWriter] = None,
partial_flush_enabled: Optional[bool] = None,
partial_flush_min_spans: Optional[int] = None,
api_version: Optional[str] = None,
compute_stats_enabled: Optional[bool] = None,
appsec_enabled: Optional[bool] = None,
iast_enabled: Optional[bool] = None,
appsec_standalone_enabled: Optional[bool] = None,
log_deprecations: bool = False,
) -> None:
if enabled is not None:
self.enabled = enabled
if log_deprecations:
deprecate(
"Enabling/Disabling tracing after application start is deprecated",
message="Please use DD_TRACE_ENABLED instead.",
version="3.0.0",
category=DDTraceDeprecationWarning,
)

if priority_sampling is not None:
if priority_sampling is not None and log_deprecations:
deprecate(
"Configuring priority sampling on tracing clients is deprecated",
"Disabling priority sampling is deprecated",
message="Calling `tracer.configure(priority_sampling=....) has no effect",
version="3.0.0",
category=DDTraceDeprecationWarning,
)

if settings is not None:
self._filters = settings.get("FILTERS") or self._filters
if trace_processors is not None:
self._user_trace_processors = trace_processors

if partial_flush_enabled is not None:
self._partial_flush_enabled = partial_flush_enabled
if log_deprecations:
deprecate(
"Configuring partial flushing after application start is deprecated",
message="Please use DD_TRACE_PARTIAL_FLUSH_ENABLED to enable/disable the partial flushing instead.",
version="3.0.0",
category=DDTraceDeprecationWarning,
)

if partial_flush_min_spans is not None:
self._partial_flush_min_spans = partial_flush_min_spans
if log_deprecations:
deprecate(
"Configuring partial flushing after application start is deprecated",
message="Please use DD_TRACE_PARTIAL_FLUSH_MIN_SPANS to set the flushing threshold instead.",
version="3.0.0",
category=DDTraceDeprecationWarning,
)

if appsec_enabled is not None:
asm_config._asm_enabled = appsec_enabled
Expand Down Expand Up @@ -527,10 +620,33 @@ def configure(
if sampler is not None:
self._sampler = sampler
self._user_sampler = self._sampler
if log_deprecations:
deprecate(
"Configuring custom samplers is deprecated",
message="Please use DD_TRACE_SAMPLING_RULES to configure the sample rates instead",
category=DDTraceDeprecationWarning,
removal_version="3.0.0",
)

self._dogstatsd_url = dogstatsd_url or self._dogstatsd_url
if dogstatsd_url is not None:
if log_deprecations:
deprecate(
"Configuring dogstatsd_url after application start is deprecated",
message="Please use DD_DOGSTATSD_URL instead.",
version="3.0.0",
category=DDTraceDeprecationWarning,
)
self._dogstatsd_url = dogstatsd_url

if any(x is not None for x in [hostname, port, uds_path, https]):
if log_deprecations:
deprecate(
"Configuring tracer agent connection after application start is deprecated",
message="Please use DD_TRACE_AGENT_URL instead.",
version="3.0.0",
category=DDTraceDeprecationWarning,
)

# If any of the parts of the URL have updated, merge them with
# the previous writer values.
prev_url_parsed = compat.parse.urlparse(self._agent_url)
Expand All @@ -554,6 +670,13 @@ def configure(
new_url = None

if compute_stats_enabled is not None:
if log_deprecations:
deprecate(
"Configuring tracer stats computation after application start is deprecated",
message="Please use DD_TRACE_STATS_COMPUTATION_ENABLED instead.",
version="3.0.0",
category=DDTraceDeprecationWarning,
)
self._compute_stats = compute_stats_enabled

try:
Expand All @@ -562,6 +685,14 @@ def configure(
# It's possible the writer never got started
pass

if api_version is not None and log_deprecations:
deprecate(
"Configuring Tracer API version after application start is deprecated",
message="Please use DD_TRACE_API_VERSION instead.",
version="3.0.0",
category=DDTraceDeprecationWarning,
)

if writer is not None:
self._writer = writer
elif any(x is not None for x in [new_url, api_version, sampler, dogstatsd_url, appsec_enabled]):
Expand Down Expand Up @@ -600,14 +731,14 @@ def configure(
uds_path,
api_version,
sampler,
settings.get("FILTERS") if settings is not None else None,
trace_processors,
compute_stats_enabled,
appsec_enabled,
iast_enabled,
]
):
self._span_processors, self._appsec_processor, self._deferred_processors = _default_span_processors_factory(
self._filters,
self._user_trace_processors,
self._writer,
self._partial_flush_enabled,
self._partial_flush_min_spans,
Expand All @@ -623,6 +754,12 @@ def configure(

if wrap_executor is not None:
self._wrap_executor = wrap_executor
if log_deprecations:
deprecate(
"Support for tracer.configure(...) with the wrap_executor parameter is deprecated",
version="3.0.0",
category=DDTraceDeprecationWarning,
)

self._generate_diagnostic_logs()

Expand Down Expand Up @@ -670,7 +807,7 @@ def _child_after_fork(self):
# Re-create the background writer thread
self._writer = self._writer.recreate()
self._span_processors, self._appsec_processor, self._deferred_processors = _default_span_processors_factory(
self._filters,
self._user_trace_processors,
self._writer,
self._partial_flush_enabled,
self._partial_flush_min_spans,
Expand Down
4 changes: 2 additions & 2 deletions ddtrace/appsec/_remoteconfiguration.py
Original file line number Diff line number Diff line change
Expand Up @@ -221,12 +221,12 @@ def _appsec_1click_activation(features: Mapping[str, Any], test_tracer: Optional

if rc_asm_enabled:
if not asm_config._asm_enabled:
tracer.configure(appsec_enabled=True)
tracer._configure(appsec_enabled=True)
else:
asm_config._asm_enabled = True
else:
if asm_config._asm_enabled:
tracer.configure(appsec_enabled=False)
tracer._configure(appsec_enabled=False)
else:
asm_config._asm_enabled = False

Expand Down
2 changes: 1 addition & 1 deletion ddtrace/contrib/internal/aiohttp/middlewares.py
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ def trace_app(app, tracer, service="aiohttp-web"):
}

# the tracer must work with asynchronous Context propagation
tracer.configure(context_provider=context_provider)
tracer._configure(context_provider=context_provider)

# add the async tracer middleware as a first middleware
# and be sure that the on_prepare signal is the last one
Expand Down
6 changes: 3 additions & 3 deletions ddtrace/contrib/internal/tornado/application.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,19 +34,19 @@ def tracer_config(__init__, app, args, kwargs):
service = settings["default_service"]

# extract extra settings
extra_settings = settings.get("settings", {})
trace_processors = settings.get("settings", {}).get("FILTERS")

# the tracer must use the right Context propagation and wrap executor;
# this action is done twice because the patch() method uses the
# global tracer while here we can have a different instance (even if
# this is not usual).
tracer.configure(
tracer._configure(
context_provider=context_provider,
wrap_executor=decorators.wrap_executor,
enabled=settings.get("enabled", None),
hostname=settings.get("agent_hostname", None),
port=settings.get("agent_port", None),
settings=extra_settings,
trace_processors=trace_processors,
)

# set global tags if any
Expand Down
2 changes: 1 addition & 1 deletion ddtrace/contrib/internal/tornado/patch.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ def patch():
_w("tornado.template", "Template.generate", template.generate)

# configure the global tracer
ddtrace.tracer.configure(
ddtrace.tracer._configure(
context_provider=context_provider,
wrap_executor=decorators.wrap_executor,
)
Expand Down
8 changes: 4 additions & 4 deletions ddtrace/internal/ci_visibility/recorder.py
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ def __init__(self, tracer=None, config=None, service=None):

# Partial traces are required for ITR to work in suite-level skipping for long test sessions, but we
# assume that a tracer is already configured if it's been passed in.
self.tracer.configure(partial_flush_enabled=True, partial_flush_min_spans=TRACER_PARTIAL_FLUSH_MIN_SPANS)
self.tracer._configure(partial_flush_enabled=True, partial_flush_min_spans=TRACER_PARTIAL_FLUSH_MIN_SPANS)

self._api_client: Optional[_TestVisibilityAPIClientBase] = None

Expand Down Expand Up @@ -399,7 +399,7 @@ def _configure_writer(self, coverage_enabled=False, requests_mode=None, url=None
itr_suite_skipping_mode=self._suite_skipping_mode,
)
if writer is not None:
self.tracer.configure(writer=writer)
self.tracer._configure(writer=writer)

def _agent_evp_proxy_is_available(self):
# type: () -> bool
Expand Down Expand Up @@ -597,10 +597,10 @@ def disable(cls):

def _start_service(self):
# type: () -> None
tracer_filters = self.tracer._filters
tracer_filters = self.tracer._user_trace_processors
if not any(isinstance(tracer_filter, TraceCiVisibilityFilter) for tracer_filter in tracer_filters):
tracer_filters += [TraceCiVisibilityFilter(self._tags, self._service)] # type: ignore[arg-type]
self.tracer.configure(settings={"FILTERS": tracer_filters})
self.tracer._configure(trace_processors=tracer_filters)

if self.test_skipping_enabled():
self._fetch_tests_to_skip()
Expand Down
Loading

0 comments on commit 7520c7a

Please sign in to comment.