chore(profiling): improve native module loading (#9719)

The profiler bundles some native components which are not available on all platforms. The pattern we were using for detecting and handling this case was needlessly fiddly and incomplete. This patch refactors some aspects of this. - [x] The PR description includes an overview of the change - [x] The PR description articulates the motivation for the change - [x] The change includes tests OR the PR description describes a testing strategy - [x] The PR description notes risks associated with the change, if any - [x] Newly-added code is easy to change - [x] The change follows the [library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) - [x] The change includes or references documentation updates if necessary - [x] Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) - [x] Title is accurate - [x] All changes are related to the pull request's stated goal - [x] Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes - [x] Testing strategy adequately addresses listed risks - [x] Newly-added code is easy to change - [x] Release note makes sense to a user of the library - [x] If necessary, author has acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment - [x] 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) (cherry picked from commit 2057913)
DataDog · Aug 13, 2024 · e47f69c · e47f69c
1 parent 4d99cf0
commit e47f69c
Show file tree

Hide file tree

Showing 5 changed files with 142 additions and 218 deletions.
diff --git a/ddtrace/internal/datadog/profiling/ddup/__init__.py b/ddtrace/internal/datadog/profiling/ddup/__init__.py
@@ -1,4 +1,9 @@
-from .utils import sanitize_string  # noqa: F401
+# This module supports an optional feature.  It may not even load on all platforms or configurations.
+# In ddtrace/settings/profiling.py, this module is imported and the is_available attribute is checked to determine
+# whether the feature is available. If not, then the feature is disabled and all downstream consumption is
+# suppressed.
+is_available = False
+failure_msg = ""
 
 
 try:
@@ -7,89 +12,4 @@
     is_available = True
 
 except Exception as e:
-    from typing import Dict  # noqa:F401
-    from typing import Optional  # noqa:F401
-
-    from ddtrace.internal.logger import get_logger
-
-    LOG = get_logger(__name__)
-    LOG.debug("Failed to import _ddup: %s", e)
-
-    is_available = False
-
-    # Decorator for not-implemented
-    def not_implemented(func):
-        def wrapper(*args, **kwargs):
-            raise NotImplementedError("{} is not implemented on this platform".format(func.__name__))
-
-    @not_implemented
-    def init(
-        env,  # type: Optional[str]
-        service,  # type: Optional[str]
-        version,  # type: Optional[str]
-        tags,  # type: Optional[Dict[str, str]]
-        max_nframes,  # type: Optional[int]
-        url,  # type: Optional[str]
-    ):
-        pass
-
-    @not_implemented
-    def upload():  # type: () -> None
-        pass
-
-    class SampleHandle:
-        @not_implemented
-        def push_cputime(self, value, count):  # type: (int, int) -> None
-            pass
-
-        @not_implemented
-        def push_walltime(self, value, count):  # type: (int, int) -> None
-            pass
-
-        @not_implemented
-        def push_acquire(self, value, count):  # type: (int, int) -> None
-            pass
-
-        @not_implemented
-        def push_release(self, value, count):  # type: (int, int) -> None
-            pass
-
-        @not_implemented
-        def push_alloc(self, value, count):  # type: (int, int) -> None
-            pass
-
-        @not_implemented
-        def push_heap(self, value):  # type: (int) -> None
-            pass
-
-        @not_implemented
-        def push_lock_name(self, lock_name):  # type: (str) -> None
-            pass
-
-        @not_implemented
-        def push_frame(self, name, filename, address, line):  # type: (str, str, int, int) -> None
-            pass
-
-        @not_implemented
-        def push_threadinfo(self, thread_id, thread_native_id, thread_name):  # type: (int, int, Optional[str]) -> None
-            pass
-
-        @not_implemented
-        def push_taskinfo(self, task_id, task_name):  # type: (int, str) -> None
-            pass
-
-        @not_implemented
-        def push_exceptioninfo(self, exc_type, count):  # type: (type, int) -> None
-            pass
-
-        @not_implemented
-        def push_class_name(self, class_name):  # type: (str) -> None
-            pass
-
-        @not_implemented
-        def push_span(self, span, endpoint_collection_enabled):  # type: (Optional[Span], bool) -> None
-            pass
-
-        @not_implemented
-        def flush_sample(self):  # type: () -> None
-            pass
+    failure_msg = str(e)
diff --git a/ddtrace/internal/datadog/profiling/stack_v2/__init__.py b/ddtrace/internal/datadog/profiling/stack_v2/__init__.py
@@ -1,34 +1,13 @@
+# See ../ddup/__init__.py for some discussion on the is_available attribute.
+# This component is also loaded in ddtrace/settings/profiling.py
 is_available = False
-
-
-# Decorator for not-implemented
-def not_implemented(func):
-    def wrapper(*args, **kwargs):
-        raise NotImplementedError("{} is not implemented on this platform".format(func.__name__))
-
-
-@not_implemented
-def start(*args, **kwargs):
-    pass
-
-
-@not_implemented
-def stop(*args, **kwargs):
-    pass
-
-
-@not_implemented
-def set_interval(*args, **kwargs):
-    pass
+failure_msg = ""
 
 
 try:
-    from ._stack_v2 import *  # noqa: F401, F403
+    from ._stack_v2 import *  # noqa: F403, F401
 
     is_available = True
-except Exception as e:
-    from ddtrace.internal.logger import get_logger
 
-    LOG = get_logger(__name__)
-
-    LOG.debug("Failed to import _stack_v2: %s", e)
+except Exception as e:
+    failure_msg = str(e)
diff --git a/ddtrace/profiling/collector/stack.pyx b/ddtrace/profiling/collector/stack.pyx
@@ -478,7 +478,7 @@ class StackCollector(collector.PeriodicCollector):
     _thread_time = attr.ib(init=False, repr=False, eq=False)
     _last_wall_time = attr.ib(init=False, repr=False, eq=False, type=int)
     _thread_span_links = attr.ib(default=None, init=False, repr=False, eq=False)
-    _stack_collector_v2_enabled = attr.ib(type=bool, default=config.stack.v2.enabled)
+    _stack_collector_v2_enabled = attr.ib(type=bool, default=config.stack.v2_enabled)
 
     @max_time_usage_pct.validator
     def _check_max_time_usage(self, attribute, value):
@@ -497,7 +497,7 @@ class StackCollector(collector.PeriodicCollector):
         if config.export.libdd_enabled:
             set_use_libdd(True)
 
-        # If at the end of things, stack v2 is still enabled, then start the native thread running the v2 sampler
+        # If stack v2 is enabled, then use the v2 sampler
         if self._stack_collector_v2_enabled:
             LOG.debug("Starting the stack v2 sampler")
             stack_v2.start()

diff --git a/ddtrace/profiling/profiler.py b/ddtrace/profiling/profiler.py
@@ -116,6 +116,7 @@ class _ProfilerInstance(service.Service):
     agentless = attr.ib(type=bool, default=config.agentless)
     _memory_collector_enabled = attr.ib(type=bool, default=config.memory.enabled)
     _stack_collector_enabled = attr.ib(type=bool, default=config.stack.enabled)
+    _stack_v2_enabled = attr.ib(type=bool, default=config.stack.v2_enabled)
     _lock_collector_enabled = attr.ib(type=bool, default=config.lock.enabled)
     enable_code_provenance = attr.ib(type=bool, default=config.code_provenance)
     endpoint_collection_enabled = attr.ib(type=bool, default=config.endpoint_collection)
@@ -128,7 +129,6 @@ class _ProfilerInstance(service.Service):
         init=False, factory=lambda: os.environ.get("AWS_LAMBDA_FUNCTION_NAME"), type=Optional[str]
     )
     _export_libdd_enabled = attr.ib(type=bool, default=config.export.libdd_enabled)
-    _export_libdd_required = attr.ib(type=bool, default=config.export.libdd_required)
 
     ENDPOINT_TEMPLATE = "https://intake.profile.{}"
 
@@ -171,16 +171,10 @@ def _build_default_exporters(self):
         if self._lambda_function_name is not None:
             self.tags.update({"functionname": self._lambda_function_name})
 
-        # Did the user request the libdd collector?  Better log it.
-        if self._export_libdd_enabled:
-            LOG.debug("The libdd collector is enabled")
-        if self._export_libdd_required:
-            LOG.debug("The libdd collector is required")
-
         # Build the list of enabled Profiling features and send along as a tag
         configured_features = []
         if self._stack_collector_enabled:
-            if config.stack.v2.enabled:
+            if self._stack_v2_enabled:
                 configured_features.append("stack_v2")
             else:
                 configured_features.append("stack")
@@ -195,8 +189,6 @@ def _build_default_exporters(self):
             configured_features.append("exp_dd")
         else:
             configured_features.append("exp_py")
-        if self._export_libdd_required:
-            configured_features.append("req_dd")
         configured_features.append("CAP" + str(config.capture_pct))
         configured_features.append("MAXF" + str(config.max_frames))
         self.tags.update({"profiler_config": "_".join(configured_features)})
@@ -207,7 +199,6 @@ def _build_default_exporters(self):
 
         # If libdd is enabled, then
         # * If initialization fails, disable the libdd collector and fall back to the legacy exporter
-        # * If initialization fails and libdd is required, disable everything and return (error)
         if self._export_libdd_enabled:
             try:
                 ddup.init(
@@ -225,16 +216,11 @@ def _build_default_exporters(self):
                 self._export_libdd_enabled = False
                 config.export.libdd_enabled = False
 
-                # If we're here and libdd was required, then there's nothing else to do.  We don't have a
-                # collector.
-                if self._export_libdd_required:
-                    LOG.error("libdd collector is required but could not be initialized. Disabling profiling.")
-                    config.enabled = False
-                    config.export.libdd_required = False
-                    config.lock.enabled = False
-                    config.memory.enabled = False
-                    config.stack.enabled = False
-                    return []
+                # also disable other features that might be enabled
+                if self._stack_v2_enabled:
+                    LOG.error("Disabling stack_v2 as libdd collector failed to initialize")
+                    self._stack_v2_enabled = False
+                    config.stack.v2_enabled = False
 
         # DEV: Import this only if needed to avoid importing protobuf
         # unnecessarily