feat(profiling): support dynamic agent url (#11319)

https://ddtrace.readthedocs.io/en/stable/advanced_usage.html#agent-configuration Customers can use `tracer.configure()` to set the agent url on the fly, and profiler doesn't use the url passed in. Without this change, the following test fails with the Python exporter having `http://localhost:8126` as endpoint url. ``` def test_tracer_url_configure_after(): t = ddtrace.Tracer() prof = profiler.Profiler(tracer=t) t.configure(hostname="foobar") _check_url(prof, "http://foobar:8126", os.environ.get("DD_API_KEY")) ``` Addressing customer issue [PROF-10698](https://datadoghq.atlassian.net/browse/PROF-10698) ## 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) [PROF-10698]: https://datadoghq.atlassian.net/browse/PROF-10698?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
DataDog · Nov 22, 2024 · defea4e · defea4e
1 parent 9f7551f
commit defea4e
Show file tree

Hide file tree

Showing 9 changed files with 138 additions and 117 deletions.
diff --git a/ddtrace/internal/datadog/profiling/ddup/_ddup.pyi b/ddtrace/internal/datadog/profiling/ddup/_ddup.pyi
@@ -10,7 +10,6 @@ def config(
     version: StringType,
     tags: Optional[Dict[Union[str, bytes], Union[str, bytes]]],
     max_nframes: Optional[int],
-    url: Optional[str],
     timeline_enabled: Optional[bool],
     output_filename: Optional[str],
     sample_pool_capacity: Optional[int],

diff --git a/ddtrace/internal/datadog/profiling/ddup/_ddup.pyx b/ddtrace/internal/datadog/profiling/ddup/_ddup.pyx
@@ -15,6 +15,7 @@ import ddtrace
 import platform
 from .._types import StringType
 from ..util import sanitize_string
+from ddtrace.internal import agent
 from ddtrace.internal.constants import DEFAULT_SERVICE_NAME
 from ddtrace.internal.packages import get_distributions
 from ddtrace.internal.runtime import get_runtime_id
@@ -339,7 +340,6 @@ def config(
         version: StringType = None,
         tags: Optional[Dict[Union[str, bytes], Union[str, bytes]]] = None,
         max_nframes: Optional[int] = None,
-        url: StringType = None,
         timeline_enabled: Optional[bool] = None,
         output_filename: StringType = None,
         sample_pool_capacity: Optional[int] = None,
@@ -354,8 +354,6 @@ def config(
         call_func_with_str(ddup_config_env, env)
     if version:
         call_func_with_str(ddup_config_version, version)
-    if url:
-        call_func_with_str(ddup_config_url, url)
     if output_filename:
         call_func_with_str(ddup_config_output_filename, output_filename)
 
@@ -388,6 +386,16 @@ def start() -> None:
     ddup_start()
 
 
+def _get_endpoint(tracer)-> str:
+    # DEV: ddtrace.profiling.utils has _get_endpoint but importing that function
+    # leads to a circular import, so re-implementing it here.
+    # TODO(taegyunkim): support agentless mode by modifying uploader_builder to
+    # build exporter for agentless mode too.
+    tracer_agent_url = tracer.agent_trace_url
+    endpoint = tracer_agent_url if tracer_agent_url else agent.get_trace_url()
+    return endpoint
+
+
 def upload() -> None:
     call_func_with_str(ddup_set_runtime_id, get_runtime_id())
 
@@ -397,6 +405,9 @@ def upload() -> None:
     call_ddup_profile_set_endpoints(endpoint_to_span_ids)
     call_ddup_profile_add_endpoint_counts(endpoint_counts)
 
+    endpoint = _get_endpoint(ddtrace.tracer)
+    call_func_with_str(ddup_config_url, endpoint)
+
     with nogil:
         ddup_upload()
 

diff --git a/ddtrace/profiling/exporter/http.py b/ddtrace/profiling/exporter/http.py
@@ -20,6 +20,8 @@
 from ddtrace.profiling import exporter
 from ddtrace.profiling import recorder  # noqa:F401
 from ddtrace.profiling.exporter import pprof
+from ddtrace.profiling.utils import _get_endpoint
+from ddtrace.profiling.utils import _get_endpoint_path
 from ddtrace.settings.profiling import config
 
 
@@ -39,24 +41,23 @@ class PprofHTTPExporter(pprof.PprofExporter):
 
     def __init__(
         self,
+        tracer: ddtrace.Tracer = ddtrace.tracer,
         enable_code_provenance: bool = True,
-        endpoint: typing.Optional[str] = None,
         api_key: typing.Optional[str] = None,
         timeout: float = config.api_timeout,
         service: typing.Optional[str] = None,
         env: typing.Optional[str] = None,
         version: typing.Optional[str] = None,
         tags: typing.Optional[typing.Dict[str, str]] = None,
         max_retry_delay: typing.Optional[float] = None,
-        endpoint_path: str = "/profiling/v1/input",
         endpoint_call_counter_span_processor: typing.Optional[EndpointCallCounterProcessor] = None,
         *args,
         **kwargs,
     ):
         super().__init__(*args, **kwargs)
         # repeat this to please mypy
+        self.tracer = tracer
         self.enable_code_provenance: bool = enable_code_provenance
-        self.endpoint: str = endpoint if endpoint is not None else agent.get_trace_url()
         self.api_key: typing.Optional[str] = api_key
         # Do not use the default agent timeout: it is too short, the agent is just a unbuffered proxy and the profiling
         # backend is not as fast as the tracer one.
@@ -67,13 +68,20 @@ def __init__(
         self.tags: typing.Dict[str, str] = tags if tags is not None else {}
         self.max_retry_delay: typing.Optional[float] = max_retry_delay
         self._container_info: typing.Optional[container.CGroupInfo] = container.get_container_info()
-        self.endpoint_path: str = endpoint_path
         self.endpoint_call_counter_span_processor: typing.Optional[
             EndpointCallCounterProcessor
         ] = endpoint_call_counter_span_processor
 
         self.__post_init__()
 
+    @property
+    def endpoint(self):
+        return _get_endpoint(self.tracer, agentless=config.agentless)
+
+    @property
+    def endpoint_path(self):
+        return _get_endpoint_path(agentless=config.agentless)
+
     def __eq__(self, other):
         # Exporter class used to be decorated with @attr.s which implements __eq__, using only the attributes defined
         # in the class. However, the _upload attribute is added dynamically to the class, so we need to ignore it when

diff --git a/ddtrace/profiling/profiler.py b/ddtrace/profiling/profiler.py
@@ -10,12 +10,10 @@
 
 import ddtrace
 from ddtrace import config
-from ddtrace.internal import agent
 from ddtrace.internal import atexit
 from ddtrace.internal import forksafe
 from ddtrace.internal import service
 from ddtrace.internal import uwsgi
-from ddtrace.internal import writer
 from ddtrace.internal.datadog.profiling import ddup
 from ddtrace.internal.module import ModuleWatchdog
 from ddtrace.internal.telemetry import telemetry_writer
@@ -110,18 +108,14 @@ class _ProfilerInstance(service.Service):
 
     """
 
-    ENDPOINT_TEMPLATE = "https://intake.profile.{}"
-
     def __init__(
         self,
-        url: Optional[str] = None,
         service: Optional[str] = None,
         tags: Optional[Dict[str, str]] = None,
         env: Optional[str] = None,
         version: Optional[str] = None,
         tracer: Any = ddtrace.tracer,
         api_key: Optional[str] = None,
-        agentless: bool = profiling_config.agentless,
         _memory_collector_enabled: bool = profiling_config.memory.enabled,
         _stack_collector_enabled: bool = profiling_config.stack.enabled,
         _stack_v2_enabled: bool = profiling_config.stack.v2_enabled,
@@ -131,14 +125,12 @@ def __init__(
     ):
         super().__init__()
         # User-supplied values
-        self.url: Optional[str] = url
         self.service: Optional[str] = service if service is not None else config.service
         self.tags: Dict[str, str] = tags if tags is not None else profiling_config.tags
         self.env: Optional[str] = env if env is not None else config.env
         self.version: Optional[str] = version if version is not None else config.version
         self.tracer: Any = tracer
         self.api_key: Optional[str] = api_key if api_key is not None else config._dd_api_key
-        self.agentless: bool = agentless
         self._memory_collector_enabled: bool = _memory_collector_enabled
         self._stack_collector_enabled: bool = _stack_collector_enabled
         self._stack_v2_enabled: bool = _stack_v2_enabled
@@ -178,28 +170,6 @@ def _build_default_exporters(self):
                     file.PprofFileExporter(prefix=_OUTPUT_PPROF),
                 ]
 
-        if self.url is not None:
-            endpoint = self.url
-        elif self.agentless:
-            LOG.warning(
-                "Agentless uploading is currently for internal usage only and not officially supported. "
-                "You should not enable it unless somebody at Datadog instructed you to do so."
-            )
-            endpoint = self.ENDPOINT_TEMPLATE.format(os.environ.get("DD_SITE", "datadoghq.com"))
-        else:
-            if isinstance(self.tracer._writer, writer.AgentWriter):
-                endpoint = self.tracer._writer.agent_url
-            else:
-                endpoint = agent.get_trace_url()
-
-        if self.agentless:
-            endpoint_path = "/api/v2/profile"
-        else:
-            # Agent mode
-            # path is relative because it is appended
-            # to the agent base path.
-            endpoint_path = "profiling/v1/input"
-
         if self._lambda_function_name is not None:
             self.tags.update({"functionname": self._lambda_function_name})
 
@@ -221,7 +191,6 @@ def _build_default_exporters(self):
                     version=self.version,
                     tags=self.tags,  # type: ignore
                     max_nframes=profiling_config.max_frames,
-                    url=endpoint,
                     timeline_enabled=profiling_config.timeline_enabled,
                     output_filename=profiling_config.output_pprof,
                     sample_pool_capacity=profiling_config.sample_pool_capacity,
@@ -256,13 +225,12 @@ def _build_default_exporters(self):
 
         return [
             http.PprofHTTPExporter(
+                tracer=self.tracer,
                 service=self.service,
                 env=self.env,
                 tags=self.tags,
                 version=self.version,
                 api_key=self.api_key,
-                endpoint=endpoint,
-                endpoint_path=endpoint_path,
                 enable_code_provenance=self.enable_code_provenance,
                 endpoint_call_counter_span_processor=endpoint_call_counter_span_processor,
             )

diff --git a/ddtrace/profiling/utils.py b/ddtrace/profiling/utils.py
@@ -0,0 +1,29 @@
+import logging
+import os
+
+from ddtrace.internal import agent
+
+
+LOG = logging.getLogger(__name__)
+
+
+def _get_endpoint(tracer, agentless=False) -> str:
+    if agentless:
+        LOG.warning(
+            "Agentless uploading is currently for internal usage only and not officially supported. "
+            "You should not enable it unless somebody at Datadog instructed you to do so."
+        )
+        endpoint = "https://intake.profile.{}".format(os.environ.get("DD_SITE", "datadoghq.com"))
+    else:
+        tracer_agent_url = tracer.agent_trace_url
+        endpoint = tracer_agent_url if tracer_agent_url else agent.get_trace_url()
+    return endpoint
+
+
+def _get_endpoint_path(agentless=False) -> str:
+    if agentless:
+        endpoint_path = "/api/v2/profile"
+    else:
+        # path is relative because it is appended to the agent base path
+        endpoint_path = "profiling/v1/input"
+    return endpoint_path
diff --git a/releasenotes/notes/profiling-dynamic-agent-url-1f285029201620b8.yaml b/releasenotes/notes/profiling-dynamic-agent-url-1f285029201620b8.yaml
@@ -0,0 +1,4 @@
+---
+features:
+  - |
+    profiling: profiler uses agent url configured via ``tracer.configure()``