chore(aap): appsec span processor without import cycles (#14244)

This PR introduces:
- remove of the `_appsec_processor` in the tracer. The appsec processor
is now part of the list of regular processors. This fixes several
problems.
- change the logic order of how appsec is enabled and loaded

This is removing 2 static import cycles, only 2 left (none with appsec).

Context: Before that PR, the appsec product was loaded through the
tracer, by creating an instance of AppSecSpanProcessor. After that PR,
the appsec product or the remote config layer will register the
AppSecSpanProcessor into the list of regular span processors of the
tracer.


**Bonus**:
As the appsec processor is not reset anymore at fork time by the respawn
of the tracer (because it does not depend on the tracer object anymore),
it looks like it's also fixing the gap time we could have where appsec
was back to factory setting at work respawn time.

APPSEC-57505

## 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)

Author: christophe-papazian

.github/workflows/system-tests.yml

@@ -40,7 +40,7 @@ jobs:
           persist-credentials: false
           repository: 'DataDog/system-tests'
           # Automatically managed, use scripts/update-system-tests-version to update
-          ref: '798937b84f21558f301e841cf88c81ee961d2f34'
+          ref: 'cc0f0a64853f98d0248b682d633d227fa41f4a6e'
 
       - name: Checkout dd-trace-py
         uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
@@ -90,7 +90,7 @@ jobs:
           persist-credentials: false
           repository: 'DataDog/system-tests'
           # Automatically managed, use scripts/update-system-tests-version to update
-          ref: '798937b84f21558f301e841cf88c81ee961d2f34'
+          ref: 'cc0f0a64853f98d0248b682d633d227fa41f4a6e'
 
       - name: Build runner
         uses: ./.github/actions/install_runner
@@ -273,7 +273,7 @@ jobs:
           persist-credentials: false
           repository: 'DataDog/system-tests'
           # Automatically managed, use scripts/update-system-tests-version to update
-          ref: '798937b84f21558f301e841cf88c81ee961d2f34'
+          ref: 'cc0f0a64853f98d0248b682d633d227fa41f4a6e'
       - name: Checkout dd-trace-py
         uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
         with:

ddtrace/_trace/tracer.py

@@ -7,12 +7,10 @@
 import os
 from os import getpid
 from threading import RLock
-from typing import Any
 from typing import Callable
 from typing import Dict
 from typing import List
 from typing import Optional
-from typing import Tuple
 from typing import TypeVar
 from typing import Union
 from typing import cast
@@ -69,42 +67,20 @@
 AnyCallable = TypeVar("AnyCallable", bound=Callable)
 
 
-def _start_appsec_processor():
-    try:
-        from ddtrace.appsec._processor import AppSecSpanProcessor
-
-        return AppSecSpanProcessor()
-    except Exception as e:
-        # DDAS-001-01
-        log.error(
-            "[DDAS-001-01] "
-            "AppSec could not start because of an unexpected error. No security activities will "
-            "be collected. "
-            "Please contact support at https://docs.datadoghq.com/help/ for help. Error details: "
-            "\n%s",
-            repr(e),
-        )
-        if config._raise:
-            raise
-    return None
-
-
 def _default_span_processors_factory(
     profiling_span_processor: EndpointCallCounterProcessor,
-) -> Tuple[List[SpanProcessor], Any]:
+) -> List[SpanProcessor]:
     """Construct the default list of span processors to use."""
     span_processors: List[SpanProcessor] = []
     span_processors += [TopLevelSpanProcessor()]
 
-    appsec_processor = None
     if asm_config._asm_libddwaf_available:
         if asm_config._asm_enabled:
             if asm_config._api_security_enabled:
                 from ddtrace.appsec._api_security.api_manager import APIManager
 
                 APIManager.enable()
 
-            appsec_processor = _start_appsec_processor()
         else:
             # api_security_active will keep track of the service status of APIManager
             # we don't want to import the module if it was not started before due to
@@ -131,7 +107,7 @@ def _default_span_processors_factory(
 
     span_processors.append(profiling_span_processor)
 
-    return span_processors, appsec_processor
+    return span_processors
 
 
 class Tracer(object):
@@ -181,9 +157,7 @@ def __init__(self) -> None:
             config._trace_compute_stats = False
         # Direct link to the appsec processor
         self._endpoint_call_counter_span_processor = EndpointCallCounterProcessor()
-        self._span_processors, self._appsec_processor = _default_span_processors_factory(
-            self._endpoint_call_counter_span_processor
-        )
+        self._span_processors = _default_span_processors_factory(self._endpoint_call_counter_span_processor)
         self._span_aggregator = SpanAggregator(
             partial_flush_enabled=config._partial_flush_enabled,
             partial_flush_min_spans=config._partial_flush_min_spans,
@@ -430,7 +404,7 @@ def _recreate(
             appsec_enabled=appsec_enabled,
             reset_buffer=reset_buffer,
         )
-        self._span_processors, self._appsec_processor = _default_span_processors_factory(
+        self._span_processors = _default_span_processors_factory(
             self._endpoint_call_counter_span_processor,
         )
 
@@ -604,9 +578,7 @@ def _start_span(
 
         # Only call span processors if the tracer is enabled (even if APM opted out)
         if self.enabled or asm_config._apm_opt_out:
-            for p in chain(
-                self._span_processors, SpanProcessor.__processors__, [self._appsec_processor, self._span_aggregator]
-            ):
+            for p in chain(self._span_processors, SpanProcessor.__processors__, [self._span_aggregator]):
                 if p:
                     p.on_span_start(span)
         core.dispatch("trace.span_start", (span,))
@@ -623,9 +595,7 @@ def _on_span_finish(self, span: Span) -> None:
 
         # Only call span processors if the tracer is enabled (even if APM opted out)
         if self.enabled or asm_config._apm_opt_out:
-            for p in chain(
-                self._span_processors, SpanProcessor.__processors__, [self._appsec_processor, self._span_aggregator]
-            ):
+            for p in chain(self._span_processors, SpanProcessor.__processors__, [self._span_aggregator]):
                 if p:
                     p.on_span_finish(span)
 
@@ -924,9 +894,7 @@ def shutdown(self, timeout: Optional[float] = None) -> None:
         """
         with self._shutdown_lock:
             # Thread safety: Ensures tracer is shutdown synchronously
-            for processor in chain(
-                self._span_processors, SpanProcessor.__processors__, [self._appsec_processor, self._span_aggregator]
-            ):
+            for processor in chain(self._span_processors, SpanProcessor.__processors__, [self._span_aggregator]):
                 if processor:
                     processor.shutdown(timeout)
             self.enabled = False

ddtrace/appsec/_api_security/api_manager.py

@@ -78,12 +78,10 @@ def __init__(self) -> None:
         log.debug("%s initialized", self.__class__.__name__)
         self._hashtable: collections.OrderedDict[int, float] = collections.OrderedDict()
 
-        from ddtrace.appsec import _processor as appsec_processor
         import ddtrace.appsec._asm_request_context as _asm_request_context
         import ddtrace.appsec._metrics as _metrics
 
         self._asm_context = _asm_request_context
-        self._appsec_processor = appsec_processor
         self._metrics = _metrics
 
     def _stop_service(self) -> None:

ddtrace/appsec/_capabilities.py

@@ -1,9 +1,6 @@
 import base64
 import enum
-from typing import Optional
 
-from ddtrace._trace import tracer
-from ddtrace.internal import core
 from ddtrace.settings._config import config
 from ddtrace.settings.asm import config as asm_config
 
@@ -65,13 +62,12 @@ def _asm_feature_is_required() -> bool:
     return (_FEATURE_REQUIRED & flags) != 0
 
 
-def _rc_capabilities(test_tracer: Optional[tracer.Tracer] = None) -> Flags:
-    tracer = core.tracer if test_tracer is None else test_tracer
+def _rc_capabilities() -> Flags:
     value = Flags(0)
     if config._remote_config_enabled:
         if asm_config._asm_can_be_enabled:
             value |= Flags.ASM_ACTIVATION
-        if tracer._appsec_processor and asm_config._asm_static_rule_file is None:  # type: ignore
+        if asm_config._asm_enabled and asm_config._asm_static_rule_file is None:
             value |= _ALL_ASM_BLOCKING
             if asm_config._ep_enabled:
                 value |= _ALL_RASP
@@ -80,7 +76,7 @@ def _rc_capabilities(test_tracer: Optional[tracer.Tracer] = None) -> Flags:
     return value
 
 
-def _appsec_rc_capabilities(test_tracer: Optional[tracer.Tracer] = None) -> str:
+def _appsec_rc_capabilities() -> str:
     r"""return the bit representation of the composed capabilities in base64
     bit 0: Reserved
     bit 1: ASM 1-click Activation
@@ -96,5 +92,5 @@ def _appsec_rc_capabilities(test_tracer: Optional[tracer.Tracer] = None) -> str:
     ...
     256         -> 100000000        -> b'\x01\x00'          -> b'AQA='
     """
-    value = _rc_capabilities(test_tracer=test_tracer)
+    value = _rc_capabilities()
     return base64.b64encode(value.to_bytes((value.bit_length() + 7) // 8, "big")).decode()

ddtrace/appsec/_common_module_patches.py

@@ -75,16 +75,27 @@ def _must_block(actions: Iterable[str]) -> bool:
     return any(action in (WAF_ACTIONS.BLOCK_ACTION, WAF_ACTIONS.REDIRECT_ACTION) for action in actions)
 
 
+def _get_rasp_capability(capability: str) -> bool:
+    """Check if the RASP capability is enabled."""
+    if asm_config._asm_enabled and asm_config._ep_enabled:
+        from ddtrace.appsec._asm_request_context import in_asm_context
+
+        if not in_asm_context():
+            return False
+
+        from ddtrace.appsec._processor import AppSecSpanProcessor
+
+        return AppSecSpanProcessor._instance is not None and getattr(
+            AppSecSpanProcessor._instance, f"rasp_{capability}_enabled", False
+        )
+    return False
+
+
 def wrapped_open_CFDDB7ABBA9081B6(original_open_callable, instance, args, kwargs):
     """
     wrapper for open file function
     """
-    if (
-        asm_config._asm_enabled
-        and asm_config._ep_enabled
-        and core.tracer._appsec_processor is not None
-        and core.tracer._appsec_processor.rasp_lfi_enabled
-    ):
+    if _get_rasp_capability("lfi"):
         try:
             from ddtrace.appsec._asm_request_context import call_waf_callback
             from ddtrace.appsec._asm_request_context import in_asm_context
@@ -131,12 +142,7 @@ def wrapped_open_ED4CF71136E15EBF(original_open_callable, instance, args, kwargs
         # TODO: IAST SSRF sink to be added
         pass
 
-    if (
-        asm_config._asm_enabled
-        and asm_config._ep_enabled
-        and core.tracer._appsec_processor is not None
-        and core.tracer._appsec_processor.rasp_ssrf_enabled
-    ):
+    if _get_rasp_capability("ssrf"):
         try:
             from ddtrace.appsec._asm_request_context import call_waf_callback
             from ddtrace.appsec._asm_request_context import in_asm_context
@@ -175,12 +181,7 @@ def wrapped_request_D8CB81E472AF98A2(original_request_callable, instance, args,
 
         _iast_report_ssrf(original_request_callable, *args, **kwargs)
 
-    if (
-        asm_config._asm_enabled
-        and asm_config._ep_enabled
-        and core.tracer._appsec_processor is not None
-        and core.tracer._appsec_processor.rasp_ssrf_enabled
-    ):
+    if _get_rasp_capability("ssrf"):
         try:
             from ddtrace.appsec._asm_request_context import call_waf_callback
             from ddtrace.appsec._asm_request_context import in_asm_context
@@ -211,12 +212,7 @@ def wrapped_system_5542593D237084A7(command: str) -> None:
     """
     wrapper for os.system function
     """
-    if (
-        asm_config._asm_enabled
-        and asm_config._ep_enabled
-        and core.tracer._appsec_processor is not None  # type: ignore
-        and core.tracer._appsec_processor.rasp_shi_enabled  # type: ignore
-    ):
+    if _get_rasp_capability("shi"):
         try:
             from ddtrace.appsec._asm_request_context import call_waf_callback
             from ddtrace.appsec._asm_request_context import in_asm_context
@@ -242,12 +238,7 @@ def popen_FD233052260D8B4D(arg_list: Union[List[str], str]) -> None:
     """
     listener for subprocess.Popen class
     """
-    if (
-        asm_config._asm_enabled
-        and asm_config._ep_enabled
-        and core.tracer._appsec_processor is not None  # type: ignore
-        and core.tracer._appsec_processor.rasp_cmdi_enabled  # type: ignore
-    ):
+    if _get_rasp_capability("cmdi"):
         try:
             from ddtrace.appsec._asm_request_context import call_waf_callback
             from ddtrace.appsec._asm_request_context import in_asm_context
@@ -287,12 +278,7 @@ def execute_4C9BAC8E228EB347(instrument_self, query, args, kwargs) -> None:
     parameters are ignored as they are properly handled by the dbapi without risk of injections
     """
 
-    if (
-        asm_config._asm_enabled
-        and asm_config._ep_enabled
-        and core.tracer._appsec_processor is not None  # type: ignore
-        and core.tracer._appsec_processor.rasp_sqli_enabled  # type: ignore
-    ):
+    if _get_rasp_capability("sqli"):
         try:
             from ddtrace.appsec._asm_request_context import call_waf_callback
             from ddtrace.appsec._asm_request_context import in_asm_context

ddtrace/appsec/_listeners.py

@@ -1,10 +1,24 @@
+import sys
+
 from ddtrace.internal import core
+from ddtrace.settings.asm import config as asm_config
 
 
 _APPSEC_TO_BE_LOADED = True
 
 
-def load_appsec():
+def _asm_switch_state() -> None:
+    if asm_config._asm_enabled:
+        from ddtrace.appsec._processor import AppSecSpanProcessor
+
+        AppSecSpanProcessor.enable()
+    elif "ddtrace.appsec._processor" in sys.modules:
+        from ddtrace.appsec._processor import AppSecSpanProcessor
+
+        AppSecSpanProcessor.disable()
+
+
+def load_appsec() -> None:
     """Lazily load the appsec module listeners."""
     from ddtrace.appsec._asm_request_context import asm_listen
     from ddtrace.appsec._handlers import listen
@@ -15,7 +29,12 @@ def load_appsec():
         listen()
         trace_listen()
         asm_listen()
+        core.on("asm.switch_state", _asm_switch_state)
         _APPSEC_TO_BE_LOADED = False
+    if asm_config._asm_enabled:
+        from ddtrace.appsec._processor import AppSecSpanProcessor
+
+        AppSecSpanProcessor.enable()
 
 
 def load_common_appsec_modules():