From 71fd926b24da0ae38075e82ea2df074c0ec33cb4 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Wed, 28 Aug 2024 14:19:25 +0000 Subject: [PATCH 1/2] chore(crashtracking): enable `wait_for_receiver` field to be configured via env var [backport 2.12] (#10381) Backport bb82a8e177cfb89a0b8e437e724fa0c06df59a61 from #10233 to 2.12. ### Change Overview This PR enables the `wait_for_receiver` field to be set via `DD_CRASHTRACKING_WAIT_FOR_RECEIVER` environment variable. If unset, the default is `true`. ### Motivation [System-tests for crashtracking](https://github.com/DataDog/system-tests/pull/2884) exposed that the Python crashtracker needs the `wait_for_receiver` field to be set to `true` to capture crashes and reliably send telemetry events to the agent. While set to `false`, telemetry events for crashes would work very rarely. When set to `true`, this would become reliably consistent. The value for `wait_for_receiver` is [unset in `libdatadog`](https://github.com/DataDog/libdatadog/blob/16528ffee456f7af5fe9ad80a6294fb5dcd38918/crashtracker/src/shared/configuration.rs#L28), which means it is set to `false` by default. This default needs to be updated on the `libdatadog` end. However, in order to unblock crashtracking functionality for `dd-trace-py`, we are also enabling this to be configured on the `dd-trace-py` side as well. ## 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: erikayasuda <153395705+erikayasuda@users.noreply.github.com> --- ddtrace/internal/core/crashtracking.py | 1 + .../datadog/profiling/crashtracker/_crashtracker.pyi | 1 + .../datadog/profiling/crashtracker/_crashtracker.pyx | 5 +++++ .../datadog/profiling/dd_wrapper/include/crashtracker.hpp | 2 ++ .../dd_wrapper/include/crashtracker_interface.hpp | 1 + .../datadog/profiling/dd_wrapper/src/crashtracker.cpp | 8 +++++++- .../profiling/dd_wrapper/src/crashtracker_interface.cpp | 6 ++++++ ddtrace/settings/crashtracker.py | 8 ++++++++ 8 files changed, 31 insertions(+), 1 deletion(-) diff --git a/ddtrace/internal/core/crashtracking.py b/ddtrace/internal/core/crashtracking.py index 9e96341af04..a36d9639fed 100644 --- a/ddtrace/internal/core/crashtracking.py +++ b/ddtrace/internal/core/crashtracking.py @@ -35,6 +35,7 @@ def start() -> bool: crashtracker.set_runtime_id(get_runtime_id()) crashtracker.set_library_version(version.get_version()) crashtracker.set_alt_stack(bool(crashtracker_config.alt_stack)) + crashtracker.set_wait_for_receiver(bool(crashtracker_config.wait_for_receiver)) if crashtracker_config.stacktrace_resolver == "fast": crashtracker.set_resolve_frames_fast() elif crashtracker_config.stacktrace_resolver == "full": diff --git a/ddtrace/internal/datadog/profiling/crashtracker/_crashtracker.pyi b/ddtrace/internal/datadog/profiling/crashtracker/_crashtracker.pyi index 0d181464961..5df95e5f859 100644 --- a/ddtrace/internal/datadog/profiling/crashtracker/_crashtracker.pyi +++ b/ddtrace/internal/datadog/profiling/crashtracker/_crashtracker.pyi @@ -11,6 +11,7 @@ def set_library_version(profiler_version: StringType) -> None: ... def set_stdout_filename(filename: StringType) -> None: ... def set_stderr_filename(filename: StringType) -> None: ... def set_alt_stack(alt_stack: bool) -> None: ... +def set_wait_for_receiver(wait: bool) -> None: ... def set_resolve_frames_disable() -> None: ... def set_resolve_frames_fast() -> None: ... def set_resolve_frames_full() -> None: ... diff --git a/ddtrace/internal/datadog/profiling/crashtracker/_crashtracker.pyx b/ddtrace/internal/datadog/profiling/crashtracker/_crashtracker.pyx index 9fbc43816a2..c7bb62da8be 100644 --- a/ddtrace/internal/datadog/profiling/crashtracker/_crashtracker.pyx +++ b/ddtrace/internal/datadog/profiling/crashtracker/_crashtracker.pyx @@ -28,6 +28,7 @@ cdef extern from "crashtracker_interface.hpp": void crashtracker_set_stdout_filename(string_view filename) void crashtracker_set_stderr_filename(string_view filename) void crashtracker_set_alt_stack(bint alt_stack) + void crashtracker_set_wait_for_receiver(bint wait) void crashtracker_set_resolve_frames_disable() void crashtracker_set_resolve_frames_fast() void crashtracker_set_resolve_frames_full() @@ -98,6 +99,10 @@ def set_alt_stack(alt_stack: bool) -> None: crashtracker_set_alt_stack(alt_stack) +def set_wait_for_receiver(wait: bool) -> None: + crashtracker_set_wait_for_receiver(wait) + + def set_resolve_frames_disable() -> None: crashtracker_set_resolve_frames_disable() diff --git a/ddtrace/internal/datadog/profiling/dd_wrapper/include/crashtracker.hpp b/ddtrace/internal/datadog/profiling/dd_wrapper/include/crashtracker.hpp index f340d926c00..02151d422ec 100644 --- a/ddtrace/internal/datadog/profiling/dd_wrapper/include/crashtracker.hpp +++ b/ddtrace/internal/datadog/profiling/dd_wrapper/include/crashtracker.hpp @@ -32,6 +32,7 @@ class Crashtracker { private: bool create_alt_stack = false; + bool wait_for_receiver = true; std::optional stderr_filename{ std::nullopt }; std::optional stdout_filename{ std::nullopt }; std::string path_to_receiver_binary; @@ -71,6 +72,7 @@ class Crashtracker void set_library_version(std::string_view _library_version); void set_url(std::string_view _url); void set_tag(std::string_view _key, std::string_view _value); + void set_wait_for_receiver(bool _wait); void set_create_alt_stack(bool _create_alt_stack); void set_stderr_filename(std::string_view _stderr_filename); diff --git a/ddtrace/internal/datadog/profiling/dd_wrapper/include/crashtracker_interface.hpp b/ddtrace/internal/datadog/profiling/dd_wrapper/include/crashtracker_interface.hpp index f3c747a2236..1cb5c294df0 100644 --- a/ddtrace/internal/datadog/profiling/dd_wrapper/include/crashtracker_interface.hpp +++ b/ddtrace/internal/datadog/profiling/dd_wrapper/include/crashtracker_interface.hpp @@ -19,6 +19,7 @@ extern "C" void crashtracker_set_stdout_filename(std::string_view filename); void crashtracker_set_stderr_filename(std::string_view filename); void crashtracker_set_alt_stack(bool alt_stack); + void crashtracker_set_wait_for_receiver(bool wait); void crashtracker_set_resolve_frames_disable(); void crashtracker_set_resolve_frames_fast(); void crashtracker_set_resolve_frames_full(); diff --git a/ddtrace/internal/datadog/profiling/dd_wrapper/src/crashtracker.cpp b/ddtrace/internal/datadog/profiling/dd_wrapper/src/crashtracker.cpp index f707573a65d..c8bda44eab3 100644 --- a/ddtrace/internal/datadog/profiling/dd_wrapper/src/crashtracker.cpp +++ b/ddtrace/internal/datadog/profiling/dd_wrapper/src/crashtracker.cpp @@ -12,6 +12,12 @@ Datadog::Crashtracker::set_create_alt_stack(bool _create_alt_stack) create_alt_stack = _create_alt_stack; } +void +Datadog::Crashtracker::set_wait_for_receiver(bool _wait) +{ + wait_for_receiver = _wait; +} + void Datadog::Crashtracker::set_env(std::string_view _env) { @@ -122,7 +128,7 @@ Datadog::Crashtracker::get_config() config.endpoint = ddog_endpoint_from_url(to_slice(url)); config.resolve_frames = resolve_frames; config.timeout_secs = timeout_secs; - + config.wait_for_receiver = wait_for_receiver; return config; } diff --git a/ddtrace/internal/datadog/profiling/dd_wrapper/src/crashtracker_interface.cpp b/ddtrace/internal/datadog/profiling/dd_wrapper/src/crashtracker_interface.cpp index 246af4f5159..bf3257225fd 100644 --- a/ddtrace/internal/datadog/profiling/dd_wrapper/src/crashtracker_interface.cpp +++ b/ddtrace/internal/datadog/profiling/dd_wrapper/src/crashtracker_interface.cpp @@ -82,6 +82,12 @@ crashtracker_set_alt_stack(bool alt_stack) // cppcheck-suppress unusedFunction crashtracker.set_create_alt_stack(alt_stack); } +void +crashtracker_set_wait_for_receiver(bool wait) // cppcheck-suppress unusedFunction +{ + crashtracker.set_wait_for_receiver(wait); +} + void crashtracker_set_resolve_frames_disable() // cppcheck-suppress unusedFunction { diff --git a/ddtrace/settings/crashtracker.py b/ddtrace/settings/crashtracker.py index b49b2ff5748..f22b363f56a 100644 --- a/ddtrace/settings/crashtracker.py +++ b/ddtrace/settings/crashtracker.py @@ -101,5 +101,13 @@ class CrashtrackingConfig(En): "This is generally useful only for dd-trace-py development.", ) + wait_for_receiver = En.v( + bool, + "wait_for_receiver", + default=True, + help_type="Boolean", + help="Whether to wait for the crashtracking receiver", + ) + config = CrashtrackingConfig() From 8d3373c43baab68270da2b2aacaa18f7f2e8a6b6 Mon Sep 17 00:00:00 2001 From: Christophe Papazian <114495376+christophe-papazian@users.noreply.github.com> Date: Thu, 29 Aug 2024 11:45:52 +0200 Subject: [PATCH 2/2] chore(asm): use byte pointer instead of c_char_p (#10438) Backport of [backport-10428-to-2.12](https://github.com/DataDog/dd-trace-py/pull/10428) ## 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) --- ddtrace/appsec/_ddwaf/ddwaf_types.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ddtrace/appsec/_ddwaf/ddwaf_types.py b/ddtrace/appsec/_ddwaf/ddwaf_types.py index ad5ce493121..d950a8eedbd 100644 --- a/ddtrace/appsec/_ddwaf/ddwaf_types.py +++ b/ddtrace/appsec/_ddwaf/ddwaf_types.py @@ -181,7 +181,7 @@ def create_without_limits(cls, struct: DDWafRulesType) -> "ddwaf_object": def struct(self) -> DDWafRulesType: """Generate a python structure from ddwaf_object""" if self.type == DDWAF_OBJ_TYPE.DDWAF_OBJ_STRING: - return self.value.stringValue.decode("UTF-8", errors="ignore") + return self.value.stringValue[: self.nbEntries].decode("UTF-8", errors="ignore") if self.type == DDWAF_OBJ_TYPE.DDWAF_OBJ_MAP: return { self.value.array[i].parameterName.decode("UTF-8", errors="ignore"): self.value.array[i].struct @@ -211,7 +211,7 @@ def __repr__(self): class ddwaf_value(ctypes.Union): _fields_ = [ - ("stringValue", ctypes.c_char_p), + ("stringValue", ctypes.POINTER(ctypes.c_char)), ("uintValue", ctypes.c_ulonglong), ("intValue", ctypes.c_longlong), ("array", ddwaf_object_p),