chore(tracer): remove asm properties from the tracer class (#11791)

## Description Removes the following tracer fields and replaces all usages with the corresponding `asm_config`: - Tracer._asm_enabled - Tracer._iast_enabled - Tracer._appsec_standalone_enabled Next steps: - Remove all asm, profiling, and serverless specific logic from `ddtrace/_trace/tracer.py`. - Define a product level configure methods - Introduce product level processors and then product specific logic. - Deprecate calling tracing.configure(....) for non tracing features. ## Motivation - Decouples asm configurations from tracing - Avoids storing duplicate references for global configurations. Ideally global configurations should have once source of truth. This will help simplify remote configuration. ## 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) --------- Co-authored-by: Alberto Vara <[email protected]>
DataDog · Jan 2, 2025 · 00cd9fd · 00cd9fd
1 parent e1eccbd
commit 00cd9fd
Show file tree

Hide file tree

Showing 19 changed files with 74 additions and 112 deletions.
diff --git a/ddtrace/_trace/tracer.py b/ddtrace/_trace/tracer.py
@@ -106,22 +106,21 @@ def _default_span_processors_factory(
     trace_writer: TraceWriter,
     partial_flush_enabled: bool,
     partial_flush_min_spans: int,
-    appsec_enabled: bool,
-    iast_enabled: bool,
     compute_stats_enabled: bool,
     single_span_sampling_rules: List[SpanSamplingRule],
     agent_url: str,
     trace_sampler: BaseSampler,
     profiling_span_processor: EndpointCallCounterProcessor,
-    apm_opt_out: bool = False,
 ) -> Tuple[List[SpanProcessor], Optional[Any], List[SpanProcessor]]:
     # FIXME: type should be AppsecSpanProcessor but we have a cyclic import here
     """Construct the default list of span processors to use."""
     trace_processors: List[TraceProcessor] = []
     trace_processors += [
         PeerServiceProcessor(_ps_config),
         BaseServiceProcessor(),
-        TraceSamplingProcessor(compute_stats_enabled, trace_sampler, single_span_sampling_rules, apm_opt_out),
+        TraceSamplingProcessor(
+            compute_stats_enabled, trace_sampler, single_span_sampling_rules, asm_config._apm_opt_out
+        ),
         TraceTagsProcessor(),
     ]
     trace_processors += trace_filters
@@ -130,7 +129,7 @@ def _default_span_processors_factory(
     span_processors += [TopLevelSpanProcessor()]
 
     if asm_config._asm_libddwaf_available:
-        if appsec_enabled:
+        if asm_config._asm_enabled:
             if asm_config._api_security_enabled:
                 from ddtrace.appsec._api_security.api_manager import APIManager
 
@@ -152,7 +151,7 @@ def _default_span_processors_factory(
     else:
         appsec_processor = None
 
-    if iast_enabled:
+    if asm_config._iast_enabled:
         from ddtrace.appsec._iast.processor import AppSecIastSpanProcessor
 
         span_processors.append(AppSecIastSpanProcessor())
@@ -229,14 +228,8 @@ def __init__(
         self.context_provider = context_provider or DefaultContextProvider()
         # _user_sampler is the backup in case we need to revert from remote config to local
         self._user_sampler: Optional[BaseSampler] = DatadogSampler()
-        self._asm_enabled = asm_config._asm_enabled
-        self._iast_enabled = asm_config._iast_enabled
-        self._appsec_standalone_enabled = asm_config._appsec_standalone_enabled
         self._dogstatsd_url = agent.get_stats_url() if dogstatsd_url is None else dogstatsd_url
-        self._apm_opt_out = self._appsec_standalone_enabled and (
-            self._asm_enabled or self._iast_enabled or config._sca_enabled
-        )
-        if self._apm_opt_out:
+        if asm_config._apm_opt_out:
             self.enabled = False
             # Disable compute stats (neither agent or tracer should compute them)
             config._trace_compute_stats = False
@@ -257,30 +250,28 @@ def __init__(
                 agent_url=self._agent_url,
                 dogstatsd=get_dogstatsd_client(self._dogstatsd_url),
                 sync_mode=self._use_sync_mode(),
-                headers={"Datadog-Client-Computed-Stats": "yes"} if (self._compute_stats or self._apm_opt_out) else {},
-                report_metrics=not self._apm_opt_out,
+                headers={"Datadog-Client-Computed-Stats": "yes"}
+                if (self._compute_stats or asm_config._apm_opt_out)
+                else {},
+                report_metrics=not asm_config._apm_opt_out,
                 response_callback=self._agent_response_callback,
             )
         self._single_span_sampling_rules: List[SpanSamplingRule] = get_span_sampling_rules()
         self._writer: TraceWriter = writer
         self._partial_flush_enabled = config._partial_flush_enabled
         self._partial_flush_min_spans = config._partial_flush_min_spans
         # Direct link to the appsec processor
-        self._appsec_processor = None
         self._endpoint_call_counter_span_processor = EndpointCallCounterProcessor()
         self._span_processors, self._appsec_processor, self._deferred_processors = _default_span_processors_factory(
             self._filters,
             self._writer,
             self._partial_flush_enabled,
             self._partial_flush_min_spans,
-            self._asm_enabled,
-            self._iast_enabled,
             self._compute_stats,
             self._single_span_sampling_rules,
             self._agent_url,
             self._sampler,
             self._endpoint_call_counter_span_processor,
-            self._apm_opt_out,
         )
         if config._data_streams_enabled:
             # Inline the import to avoid pulling in ddsketch or protobuf
@@ -335,7 +326,7 @@ def sampler(self, value):
     https://ddtrace.readthedocs.io/en/stable/configuration.html#DD_TRACE_SAMPLING_RULES""",
             category=DDTraceDeprecationWarning,
         )
-        if self._apm_opt_out:
+        if asm_config._apm_opt_out:
             log.warning("Cannot set a custom sampler with Standalone ASM mode")
             return
         self._sampler = value
@@ -407,7 +398,7 @@ def get_log_correlation_context(self, active: Optional[Union[Context, Span]] = N
         span id of the current active span, as well as the configured service, version, and environment names.
         If there is no active span, a dictionary with an empty string for each value will be returned.
         """
-        if active is None and (self.enabled or self._apm_opt_out):
+        if active is None and (self.enabled or asm_config._apm_opt_out):
             active = self.context_provider.active()
 
         if isinstance(active, Span) and active.service:
@@ -489,16 +480,15 @@ def configure(
             self._partial_flush_min_spans = partial_flush_min_spans
 
         if appsec_enabled is not None:
-            self._asm_enabled = asm_config._asm_enabled = appsec_enabled
+            asm_config._asm_enabled = appsec_enabled
 
         if iast_enabled is not None:
-            self._iast_enabled = asm_config._iast_enabled = iast_enabled
+            asm_config._iast_enabled = iast_enabled
 
         if appsec_standalone_enabled is not None:
-            self._appsec_standalone_enabled = asm_config._appsec_standalone_enabled = appsec_standalone_enabled
+            asm_config._appsec_standalone_enabled = appsec_standalone_enabled
 
-        if self._appsec_standalone_enabled and (self._asm_enabled or self._iast_enabled or config._sca_enabled):
-            self._apm_opt_out = True
+        if asm_config._apm_opt_out:
             self.enabled = False
             # Disable compute stats (neither agent or tracer should compute them)
             config._trace_compute_stats = False
@@ -557,7 +547,7 @@ def configure(
         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]):
-            if self._asm_enabled:
+            if asm_config._asm_enabled:
                 api_version = "v0.4"
             self._writer = AgentWriter(
                 self._agent_url,
@@ -566,9 +556,11 @@ def configure(
                 api_version=api_version,
                 # if apm opt out, neither agent or tracer should compute the stats
                 headers=(
-                    {"Datadog-Client-Computed-Stats": "yes"} if (compute_stats_enabled or self._apm_opt_out) else {}
+                    {"Datadog-Client-Computed-Stats": "yes"}
+                    if (compute_stats_enabled or asm_config._apm_opt_out)
+                    else {}
                 ),
-                report_metrics=not self._apm_opt_out,
+                report_metrics=not asm_config._apm_opt_out,
                 response_callback=self._agent_response_callback,
             )
         elif writer is None and isinstance(self._writer, LogWriter):
@@ -601,14 +593,11 @@ def configure(
                 self._writer,
                 self._partial_flush_enabled,
                 self._partial_flush_min_spans,
-                self._asm_enabled,
-                self._iast_enabled,
                 self._compute_stats,
                 self._single_span_sampling_rules,
                 self._agent_url,
                 self._sampler,
                 self._endpoint_call_counter_span_processor,
-                self._apm_opt_out,
             )
 
         if context_provider is not None:
@@ -667,14 +656,11 @@ def _child_after_fork(self):
             self._writer,
             self._partial_flush_enabled,
             self._partial_flush_min_spans,
-            self._asm_enabled,
-            self._iast_enabled,
             self._compute_stats,
             self._single_span_sampling_rules,
             self._agent_url,
             self._sampler,
             self._endpoint_call_counter_span_processor,
-            self._apm_opt_out,
         )
 
         self._new_process = True
@@ -859,7 +845,7 @@ def _start_span(
             self._services.add(service)
 
         # Only call span processors if the tracer is enabled (even if APM opted out)
-        if self.enabled or self._apm_opt_out:
+        if self.enabled or asm_config._apm_opt_out:
             for p in chain(self._span_processors, SpanProcessor.__processors__, self._deferred_processors):
                 p.on_span_start(span)
         self._hooks.emit(self.__class__.start_span, span)
@@ -876,7 +862,7 @@ def _on_span_finish(self, span: Span) -> None:
             log.debug("span %r closing after its parent %r, this is an error when not using async", span, span._parent)
 
         # Only call span processors if the tracer is enabled (even if APM opted out)
-        if self.enabled or self._apm_opt_out:
+        if self.enabled or asm_config._apm_opt_out:
             for p in chain(self._span_processors, SpanProcessor.__processors__, self._deferred_processors):
                 p.on_span_finish(span)
 

diff --git a/ddtrace/appsec/_remoteconfiguration.py b/ddtrace/appsec/_remoteconfiguration.py
@@ -52,12 +52,6 @@ def enable_appsec_rc(test_tracer: Optional[Tracer] = None) -> None:
 
     Parameters `test_tracer` and `start_subscribers` are needed for testing purposes
     """
-    # Import tracer here to avoid a circular import
-    if test_tracer is None:
-        from ddtrace import tracer
-    else:
-        tracer = test_tracer
-
     log.debug("[%s][P: %s] Register ASM Remote Config Callback", os.getpid(), os.getppid())
     asm_callback = (
         remoteconfig_poller.get_registered(PRODUCTS.ASM_FEATURES)
@@ -68,7 +62,7 @@ def enable_appsec_rc(test_tracer: Optional[Tracer] = None) -> None:
     if _asm_feature_is_required():
         remoteconfig_poller.register(PRODUCTS.ASM_FEATURES, asm_callback)
 
-    if tracer._asm_enabled and asm_config._asm_static_rule_file is None:
+    if asm_config._asm_enabled and asm_config._asm_static_rule_file is None:
         remoteconfig_poller.register(PRODUCTS.ASM_DATA, asm_callback)  # IP Blocking
         remoteconfig_poller.register(PRODUCTS.ASM, asm_callback)  # Exclusion Filters & Custom Rules
         remoteconfig_poller.register(PRODUCTS.ASM_DD, asm_callback)  # DD Rules
@@ -226,12 +220,12 @@ def _appsec_1click_activation(features: Mapping[str, Any], test_tracer: Optional
             )
 
             if rc_asm_enabled:
-                if not tracer._asm_enabled:
+                if not asm_config._asm_enabled:
                     tracer.configure(appsec_enabled=True)
                 else:
                     asm_config._asm_enabled = True
             else:
-                if tracer._asm_enabled:
+                if asm_config._asm_enabled:
                     tracer.configure(appsec_enabled=False)
                 else:
                     asm_config._asm_enabled = False

diff --git a/ddtrace/contrib/internal/requests/connection.py b/ddtrace/contrib/internal/requests/connection.py
@@ -15,6 +15,7 @@
 from ddtrace.internal.schema.span_attribute_schema import SpanDirection
 from ddtrace.internal.utils import get_argument_value
 from ddtrace.propagation.http import HTTPPropagator
+from ddtrace.settings.asm import config as asm_config
 
 
 log = get_logger(__name__)
@@ -59,7 +60,7 @@ def _wrap_send(func, instance, args, kwargs):
     tracer = getattr(instance, "datadog_tracer", ddtrace.tracer)
 
     # skip if tracing is not enabled
-    if not tracer.enabled and not tracer._apm_opt_out:
+    if not tracer.enabled and not asm_config._apm_opt_out:
         return func(*args, **kwargs)
 
     request = get_argument_value(args, kwargs, 0, "request")

diff --git a/ddtrace/pin.py b/ddtrace/pin.py
@@ -144,7 +144,10 @@ def override(
     def enabled(self):
         # type: () -> bool
         """Return true if this pin's tracer is enabled."""
-        return bool(self.tracer) and (self.tracer.enabled or self.tracer._apm_opt_out)
+        # inline to avoid circular imports
+        from ddtrace.settings.asm import config as asm_config
+
+        return bool(self.tracer) and (self.tracer.enabled or asm_config._apm_opt_out)
 
     def onto(self, obj, send=True):
         # type: (Any, bool) -> None

diff --git a/ddtrace/settings/asm.py b/ddtrace/settings/asm.py
@@ -269,6 +269,12 @@ def _eval_asm_can_be_enabled(self):
     def _api_security_feature_active(self) -> bool:
         return self._asm_libddwaf_available and self._asm_enabled and self._api_security_enabled
 
+    @property
+    def _apm_opt_out(self) -> bool:
+        return (
+            self._asm_enabled or self._iast_enabled or tracer_config._sca_enabled is True
+        ) and self._appsec_standalone_enabled
+
     @property
     def _user_event_mode(self) -> str:
         if self._asm_enabled and self._auto_user_instrumentation_enabled:

diff --git a/tests/appsec/appsec/test_asm_standalone.py b/tests/appsec/appsec/test_asm_standalone.py
@@ -138,8 +138,6 @@ def test_appsec_standalone_apm_enabled_metric(tracer_appsec_standalone):
         or args.get("iast_enabled", None)
         or args.get("DD_APPSEC_SCA_ENABLED", "0") == "1"
     ):
-        assert tracer._apm_opt_out is True
         assert span.get_metric("_dd.apm.enabled") == 0.0
     else:
-        assert tracer._apm_opt_out is False
         assert span.get_metric("_dd.apm.enabled") is None
diff --git a/tests/appsec/contrib_appsec/utils.py b/tests/appsec/contrib_appsec/utils.py
@@ -88,8 +88,6 @@ def check_rules_triggered(self, rule_id: List[str], root_span):
         assert result == rule_id, f"result={result}, expected={rule_id}"
 
     def update_tracer(self, interface):
-        interface.tracer._asm_enabled = asm_config._asm_enabled
-        interface.tracer._iast_enabled = asm_config._iast_enabled
         interface.tracer.configure(api_version="v0.4")
         assert asm_config._asm_libddwaf_available
         # Only for tests diagnostics

diff --git a/tests/appsec/utils.py b/tests/appsec/utils.py
@@ -6,7 +6,6 @@
 from ddtrace._trace.span import Span
 from ddtrace.ext import SpanTypes
 import ddtrace.internal.core as core
-from ddtrace.settings.asm import config as asm_config
 from tests.utils import override_global_config
 
 
@@ -35,8 +34,6 @@ def asm_context(
     with override_global_config(config) if config else contextlib.nullcontext():
         if tracer is None:
             tracer = default_tracer
-        if asm_config._asm_enabled:
-            tracer._asm_enabled = True
         if config:
             tracer.configure(api_version="v0.4")
 

diff --git a/tests/contrib/django/test_django_appsec.py b/tests/contrib/django/test_django_appsec.py
@@ -47,8 +47,6 @@ def _aux_appsec_get_root_span(
 ):
     if cookies is None:
         cookies = {}
-    tracer._asm_enabled = asm_config._asm_enabled
-    tracer._iast_enabled = asm_config._iast_enabled
     # Hack: need to pass an argument to configure so that the processors are recreated
     tracer.configure(api_version="v0.4")
     # Set cookies

diff --git a/tests/contrib/django/test_django_appsec_iast.py b/tests/contrib/django/test_django_appsec_iast.py
@@ -14,7 +14,6 @@
 from ddtrace.appsec._iast.constants import VULN_INSECURE_COOKIE
 from ddtrace.appsec._iast.constants import VULN_SQL_INJECTION
 from ddtrace.internal.compat import urlencode
-from ddtrace.settings.asm import config as asm_config
 from tests.appsec.iast.iast_utils import get_line_and_hash
 from tests.utils import override_env
 from tests.utils import override_global_config
@@ -66,8 +65,6 @@ def _aux_appsec_get_root_span(
 ):
     if cookies is None:
         cookies = {}
-    tracer._asm_enabled = asm_config._asm_enabled
-    tracer._iast_enabled = asm_config._iast_enabled
     # Hack: need to pass an argument to configure so that the processors are recreated
     tracer.configure(api_version="v0.4")
     # Set cookies

diff --git a/tests/contrib/djangorestframework/test_appsec.py b/tests/contrib/djangorestframework/test_appsec.py
@@ -11,7 +11,6 @@
 @pytest.mark.skipif(django.VERSION < (1, 10), reason="requires django version >= 1.10")
 def test_djangorest_request_body_urlencoded(client, test_spans, tracer):
     with override_global_config(dict(_asm_enabled=True)):
-        tracer._asm_enabled = True
         # Hack: need to pass an argument to configure so that the processors are recreated
         tracer.configure(api_version="v0.4")
         payload = urlencode({"mytestingbody_key": "mytestingbody_value"})

diff --git a/tests/contrib/fastapi/test_fastapi_appsec.py b/tests/contrib/fastapi/test_fastapi_appsec.py
@@ -9,7 +9,6 @@
 
 
 def _aux_appsec_prepare_tracer(tracer, asm_enabled=True):
-    tracer._asm_enabled = asm_enabled
     # Hack: need to pass an argument to configure so that the processors are recreated
     tracer.configure(api_version="v0.4")
 

diff --git a/tests/contrib/fastapi/test_fastapi_appsec_iast.py b/tests/contrib/fastapi/test_fastapi_appsec_iast.py
@@ -39,7 +39,6 @@ def _aux_appsec_prepare_tracer(tracer):
     patch_sqlite_sqli()
     oce.reconfigure()
 
-    tracer._iast_enabled = True
     # Hack: need to pass an argument to configure so that the processors are recreated
     tracer.configure(api_version="v0.4")
 

diff --git a/tests/contrib/flask/test_flask_appsec.py b/tests/contrib/flask/test_flask_appsec.py
@@ -30,7 +30,6 @@ def setUp(self):
         patch()
 
     def _aux_appsec_prepare_tracer(self, appsec_enabled=True):
-        self.tracer._asm_enabled = appsec_enabled
         # Hack: need to pass an argument to configure so that the processors are recreated
         self.tracer.configure(api_version="v0.4")
 

diff --git a/tests/contrib/flask/test_flask_appsec_iast.py b/tests/contrib/flask/test_flask_appsec_iast.py
@@ -48,9 +48,7 @@ def setUp(self):
             patch_header_injection()
             patch_json()
 
-            self.tracer._iast_enabled = True
-            self.tracer._asm_enabled = True
-            self.tracer.configure(api_version="v0.4")
+            self.tracer.configure(api_version="v0.4", appsec_enabled=True, iast_enabled=True)
             oce.reconfigure()
 
     @pytest.mark.skipif(not python_supported_by_iast(), reason="Python version not supported by IAST")
@@ -1381,8 +1379,6 @@ def setUp(self):
             )
         ):
             super(FlaskAppSecIASTDisabledTestCase, self).setUp()
-            self.tracer._iast_enabled = False
-            self.tracer._asm_enabled = False
             self.tracer.configure(api_version="v0.4")
 
     @pytest.mark.skipif(not python_supported_by_iast(), reason="Python version not supported by IAST")