From b019b5007f002d0105bc810ee44444274ddf29c8 Mon Sep 17 00:00:00 2001 From: Taegyun Kim Date: Mon, 12 Aug 2024 21:00:45 -0400 Subject: [PATCH] chore(profiling): output pprof file with `DD_PROFILING_{EXPORT_LIBDD_ENABLED, OUTPUT_PPROF}` (#10107) Minimal and short-term change to export to file when `DD_PROFILING_EXPORT_LIBDD_ENABLED` and `DD_PROFILING_OUTPUT_PPROF` are set, similar to existing [PprofFileExporter](https://github.com/DataDog/dd-trace-py/blob/322e0013bdf3a9e7db42fa6467815e9a7bfecd75/ddtrace/profiling/exporter/file.py#L10), which is used when `DD_PROFILING_EXPORT_LIBDD_ENABLED` is turned off and `DD_PROFILING_OUTPUT_PPROF` is set. #9739 was a previous attempt to implement this in a more robust way. Ideally, we'd want this logic to be in libdatadog which can then be shared among language libraries. Tested on https://github.com/DataDog/prof-correctness/pull/46. Also checked that the output profile is lz4 compressed as libdatadog does that. ## 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) --- .../dd_wrapper/include/ddup_interface.hpp | 1 + .../profiling/dd_wrapper/include/uploader.hpp | 6 ++- .../dd_wrapper/include/uploader_builder.hpp | 2 + .../dd_wrapper/src/ddup_interface.cpp | 6 +++ .../profiling/dd_wrapper/src/uploader.cpp | 40 ++++++++++++++++++- .../dd_wrapper/src/uploader_builder.cpp | 10 ++++- .../internal/datadog/profiling/ddup/_ddup.pyx | 7 ++++ ddtrace/profiling/profiler.py | 1 + 8 files changed, 69 insertions(+), 4 deletions(-) diff --git a/ddtrace/internal/datadog/profiling/dd_wrapper/include/ddup_interface.hpp b/ddtrace/internal/datadog/profiling/dd_wrapper/include/ddup_interface.hpp index ad9b91c5b98..9b689cade01 100644 --- a/ddtrace/internal/datadog/profiling/dd_wrapper/include/ddup_interface.hpp +++ b/ddtrace/internal/datadog/profiling/dd_wrapper/include/ddup_interface.hpp @@ -22,6 +22,7 @@ extern "C" void ddup_config_url(std::string_view url); void ddup_config_max_nframes(int max_nframes); void ddup_config_timeline(bool enable); + void ddup_config_output_filename(std::string_view filename); void ddup_config_sample_pool_capacity(uint64_t capacity); void ddup_config_user_tag(std::string_view key, std::string_view val); diff --git a/ddtrace/internal/datadog/profiling/dd_wrapper/include/uploader.hpp b/ddtrace/internal/datadog/profiling/dd_wrapper/include/uploader.hpp index 63ab57641e4..ed19f316fc3 100644 --- a/ddtrace/internal/datadog/profiling/dd_wrapper/include/uploader.hpp +++ b/ddtrace/internal/datadog/profiling/dd_wrapper/include/uploader.hpp @@ -3,6 +3,7 @@ #include "sample.hpp" #include "types.hpp" +#include #include #include @@ -24,9 +25,12 @@ class Uploader static inline std::mutex upload_lock{}; std::string errmsg; static inline std::unique_ptr cancel; - std::string url; + static inline std::atomic upload_seq{ 0 }; + std::string output_filename; std::unique_ptr ddog_exporter; + bool export_to_file(ddog_prof_EncodedProfile* encoded); + public: bool upload(ddog_prof_Profile& profile); static void cancel_inflight(); diff --git a/ddtrace/internal/datadog/profiling/dd_wrapper/include/uploader_builder.hpp b/ddtrace/internal/datadog/profiling/dd_wrapper/include/uploader_builder.hpp index 7fc9cb90438..62ee6aad853 100644 --- a/ddtrace/internal/datadog/profiling/dd_wrapper/include/uploader_builder.hpp +++ b/ddtrace/internal/datadog/profiling/dd_wrapper/include/uploader_builder.hpp @@ -25,6 +25,7 @@ class UploaderBuilder static inline std::string profiler_version; static inline std::string url{ "http://localhost:8126" }; static inline ExporterTagset user_tags{}; + static inline std::string output_filename{ "" }; static constexpr std::string_view language{ g_language_name }; static constexpr std::string_view family{ g_language_name }; @@ -39,6 +40,7 @@ class UploaderBuilder static void set_profiler_version(std::string_view _profiler_version); static void set_url(std::string_view _url); static void set_tag(std::string_view _key, std::string_view _val); + static void set_output_filename(std::string_view _output_filename); static std::variant build(); }; diff --git a/ddtrace/internal/datadog/profiling/dd_wrapper/src/ddup_interface.cpp b/ddtrace/internal/datadog/profiling/dd_wrapper/src/ddup_interface.cpp index db09cc83384..331038e011e 100644 --- a/ddtrace/internal/datadog/profiling/dd_wrapper/src/ddup_interface.cpp +++ b/ddtrace/internal/datadog/profiling/dd_wrapper/src/ddup_interface.cpp @@ -112,6 +112,12 @@ ddup_config_timeline(bool enabled) // cppcheck-suppress unusedFunction Datadog::SampleManager::set_timeline(enabled); } +void +ddup_config_output_filename(std::string_view output_filename) // cppcheck-suppress unusedFunction +{ + Datadog::UploaderBuilder::set_output_filename(output_filename); +} + void ddup_config_sample_pool_capacity(size_t capacity) // cppcheck-suppress unusedFunction { diff --git a/ddtrace/internal/datadog/profiling/dd_wrapper/src/uploader.cpp b/ddtrace/internal/datadog/profiling/dd_wrapper/src/uploader.cpp index fbe75ef733a..28005c29ea0 100644 --- a/ddtrace/internal/datadog/profiling/dd_wrapper/src/uploader.cpp +++ b/ddtrace/internal/datadog/profiling/dd_wrapper/src/uploader.cpp @@ -1,6 +1,12 @@ #include "uploader.hpp" #include "libdatadog_helpers.hpp" +#include // errno +#include // ofstream +#include // ostringstream +#include // strerror +#include // getpid + using namespace Datadog; void @@ -12,10 +18,34 @@ DdogCancellationTokenDeleter::operator()(ddog_CancellationToken* ptr) const } } -Datadog::Uploader::Uploader(std::string_view _url, ddog_prof_Exporter* _ddog_exporter) - : url{ _url } +Datadog::Uploader::Uploader(std::string_view _output_filename, ddog_prof_Exporter* _ddog_exporter) + : output_filename{ _output_filename } , ddog_exporter{ _ddog_exporter } { + // Increment the upload sequence number every time we build an uploader. + // Upoloaders are use-once-and-destroy. + upload_seq++; +} + +bool +Datadog::Uploader::export_to_file(ddog_prof_EncodedProfile* encoded) +{ + // Write the profile to a file using the following format for filename: + // .. + std::ostringstream oss; + oss << output_filename << "." << getpid() << "." << upload_seq; + std::string filename = oss.str(); + std::ofstream out(filename, std::ios::binary); + if (!out.is_open()) { + std::cerr << "Error opening output file " << filename << ": " << strerror(errno) << std::endl; + return false; + } + out.write(reinterpret_cast(encoded->buffer.ptr), encoded->buffer.len); + if (out.fail()) { + std::cerr << "Error writing to output file " << filename << ": " << strerror(errno) << std::endl; + return false; + } + return true; } bool @@ -32,6 +62,12 @@ Datadog::Uploader::upload(ddog_prof_Profile& profile) } ddog_prof_EncodedProfile* encoded = &result.ok; // NOLINT (cppcoreguidelines-pro-type-union-access) + if (!output_filename.empty()) { + bool ret = export_to_file(encoded); + ddog_prof_EncodedProfile_drop(encoded); + return ret; + } + // Build the request object const ddog_prof_Exporter_File file = { .name = to_slice("auto.pprof"), diff --git a/ddtrace/internal/datadog/profiling/dd_wrapper/src/uploader_builder.cpp b/ddtrace/internal/datadog/profiling/dd_wrapper/src/uploader_builder.cpp index e34db197643..0661b7f217f 100644 --- a/ddtrace/internal/datadog/profiling/dd_wrapper/src/uploader_builder.cpp +++ b/ddtrace/internal/datadog/profiling/dd_wrapper/src/uploader_builder.cpp @@ -82,6 +82,14 @@ Datadog::UploaderBuilder::set_tag(std::string_view _key, std::string_view _val) } } +void +Datadog::UploaderBuilder::set_output_filename(std::string_view _output_filename) +{ + if (!_output_filename.empty()) { + output_filename = _output_filename; + } +} + std::string join(const std::vector& vec, const std::string& delim) { @@ -176,5 +184,5 @@ Datadog::UploaderBuilder::build() return errmsg; } - return Datadog::Uploader{ url, ddog_exporter }; + return Datadog::Uploader{ output_filename, ddog_exporter }; } diff --git a/ddtrace/internal/datadog/profiling/ddup/_ddup.pyx b/ddtrace/internal/datadog/profiling/ddup/_ddup.pyx index bdadc38a5e3..0cbfbf4d3fd 100644 --- a/ddtrace/internal/datadog/profiling/ddup/_ddup.pyx +++ b/ddtrace/internal/datadog/profiling/ddup/_ddup.pyx @@ -39,6 +39,7 @@ cdef extern from "ddup_interface.hpp": void ddup_config_url(string_view url) void ddup_config_max_nframes(int max_nframes) void ddup_config_timeline(bint enable) + void ddup_config_output_filename(string_view output_filename) void ddup_config_sample_pool_capacity(uint64_t sample_pool_capacity) void ddup_config_user_tag(string_view key, string_view val) @@ -95,6 +96,9 @@ cdef call_ddup_config_profiler_version(bytes profiler_version): cdef call_ddup_config_user_tag(bytes key, bytes val): ddup_config_user_tag(string_view(key, len(key)), string_view(val, len(val))) +cdef call_ddup_config_output_filename(bytes output_filename): + ddup_config_output_filename(string_view(output_filename, len(output_filename))) + # Conversion functions cdef uint64_t clamp_to_uint64_unsigned(value): @@ -125,6 +129,7 @@ def config( max_nframes: Optional[int] = None, url: StringType = None, timeline_enabled: Optional[bool] = None, + output_filename: StringType = None, sample_pool_capacity: Optional[int] = None) -> None: # Try to provide a ddtrace-specific default service if one is not given @@ -138,6 +143,8 @@ def config( call_ddup_config_version(ensure_binary_or_empty(version)) if url: call_ddup_config_url(ensure_binary_or_empty(url)) + if output_filename: + call_ddup_config_output_filename(ensure_binary_or_empty(output_filename)) # Inherited call_ddup_config_runtime(ensure_binary_or_empty(platform.python_implementation())) diff --git a/ddtrace/profiling/profiler.py b/ddtrace/profiling/profiler.py index a402f64924f..90394c48a45 100644 --- a/ddtrace/profiling/profiler.py +++ b/ddtrace/profiling/profiler.py @@ -234,6 +234,7 @@ def _build_default_exporters(self): max_nframes=config.max_frames, url=endpoint, timeline_enabled=config.timeline_enabled, + output_filename=config.output_pprof, sample_pool_capacity=config.sample_pool_capacity, ) ddup.start()