wip

Author: KowalskiThomas

ddtrace/internal/datadog/profiling/dd_wrapper/CMakeLists.txt

@@ -56,6 +56,7 @@ add_library(
     src/profile.cpp
     src/profile_borrow.cpp
     src/profiler_stats.cpp
+    src/profiler_upload_batch.cpp
     src/sample.cpp
     src/sample_manager.cpp
     src/static_sample_pool.cpp

ddtrace/internal/datadog/profiling/dd_wrapper/include/profile.hpp

@@ -16,11 +16,13 @@ extern "C"
 namespace Datadog {
 
 class ProfileBorrow;
+class ProfilerUploadBatch;
 
 // Serves to collect individual samples, as well as lengthen the scope of string data
 class Profile
 {
     friend class ProfileBorrow;
+    friend class ProfilerUploadBatch;
 
   private:
     // Serialization for static state
@@ -46,6 +48,7 @@ class Profile
     // The profile object is initialized here as a skeleton object, but it
     // cannot be used until it's initialized by libdatadog
     ddog_prof_Profile cur_profile{};
+    ddog_prof_Profile last_profile{};
 
     Datadog::ProfilerStats cur_profiler_stats{};
 

ddtrace/internal/datadog/profiling/dd_wrapper/include/profile_borrow.hpp

@@ -12,6 +12,7 @@ class ProfileBorrow
 {
   private:
     Profile* profile_ptr;
+    ProfilerStats profiler_stats;
 
   public:
     explicit ProfileBorrow(Profile& profile);

ddtrace/internal/datadog/profiling/dd_wrapper/include/profiler_stats.hpp

@@ -3,7 +3,6 @@
 #include <cstddef>
 
 #include <string>
-#include <string_view>
 
 namespace Datadog {
 
@@ -17,8 +16,6 @@ a mutex to protect access to the data.
 class ProfilerStats
 {
   private:
-    std::string internal_metadata_json;
-
     // Number of samples collected (one per thread)
     size_t sample_count = 0;
 
@@ -37,9 +34,7 @@ class ProfilerStats
 
     // Returns a JSON string containing relevant Profiler Stats to be included
     // in the libdatadog payload.
-    // The function returned a string_view to a statically allocated string that
-    // is updated every time the function is called.
-    std::string_view get_internal_metadata_json();
+    std::string get_internal_metadata_json();
 
     void reset_state();
 };

ddtrace/internal/datadog/profiling/dd_wrapper/include/profiler_upload_batch.hpp

@@ -0,0 +1,52 @@
+#pragma once
+
+#include "profile.hpp"
+#include "profiler_stats.hpp"
+
+extern "C"
+{
+#include "datadog/profiling.h"
+}
+
+namespace Datadog {
+
+// Forward declaration
+class Profile;
+
+// RAII wrapper specifically for uploading profiles with minimal lock time
+// This class:
+// 1. Locks briefly to swap profiles
+// 2. Serializes the profile (which resets it)
+// 3. Unlocks before doing the actual upload
+class ProfilerUploadBatch
+{
+  private:
+    ddog_prof_Profile profile_to_upload{};
+    ProfilerStats stats_to_upload{};
+    ddog_prof_EncodedProfile encoded_profile{};
+    bool serialized{ false };
+
+  public:
+    explicit ProfilerUploadBatch(Profile& profile);
+    ~ProfilerUploadBatch();
+
+    // Disable copy
+    ProfilerUploadBatch(const ProfilerUploadBatch&) = delete;
+    ProfilerUploadBatch& operator=(const ProfilerUploadBatch&) = delete;
+
+    // Enable move
+    ProfilerUploadBatch(ProfilerUploadBatch&& other) noexcept;
+    ProfilerUploadBatch& operator=(ProfilerUploadBatch&& other) noexcept;
+
+    // Serialize the profile (must be called before accessing encoded profile)
+    bool serialize();
+
+    // Accessors (only valid after serialize() returns true)
+    ddog_prof_EncodedProfile* get_encoded_profile();
+    ProfilerStats& stats();
+
+    // For operations that need access to the profile before serialization
+    ddog_prof_Profile& profile();
+};
+
+} // namespace Datadog

ddtrace/internal/datadog/profiling/dd_wrapper/include/sample.hpp

@@ -3,6 +3,7 @@
 #include "libdatadog_helpers.hpp"
 #include "profile.hpp"
 #include "profile_borrow.hpp"
+#include "profiler_upload_batch.hpp"
 #include "types.hpp"
 
 #include <string>
@@ -136,6 +137,7 @@ class Sample
     bool flush_sample(bool reverse_locations = false);
 
     static ProfileBorrow profile_borrow();
+    static ProfilerUploadBatch profile_upload_batch();
     static void postfork_child();
     Sample(SampleType _type_mask, unsigned int _max_nframes);
 

ddtrace/internal/datadog/profiling/dd_wrapper/include/uploader.hpp

@@ -27,6 +27,7 @@ class Uploader
 
   public:
     bool upload(ddog_prof_Profile& profile, Datadog::ProfilerStats& profiler_stats);
+    bool upload(ddog_prof_EncodedProfile* encoded, Datadog::ProfilerStats& profiler_stats);
     static void cancel_inflight();
     static void lock();
     static void unlock();

ddtrace/internal/datadog/profiling/dd_wrapper/src/ddup_interface.cpp

@@ -359,14 +359,18 @@ ddup_upload() // cppcheck-suppress unusedFunction
 
     // Get the reference to the uploader
     auto& uploader = std::get<Datadog::Uploader>(uploader_or_err);
-    // There are a few things going on here.
-    // * profile_borrow() takes a reference in a way that locks the areas where the profile might
-    //  be modified.  It gets released and cleared after uploading.
-    // * Uploading cancels inflight uploads. There are better ways to do this, but this is what
-    //   we have for now.
-    auto borrowed = Datadog::Sample::profile_borrow();
-    bool success = uploader.upload(borrowed.profile(), borrowed.stats());
-    borrowed.stats().reset_state();
+
+    // Create the upload batch - this swaps profiles and unlocks immediately
+    auto upload_batch = Datadog::Sample::profile_upload_batch();
+
+    // Serialize the profile (still outside of any locks)
+    if (!upload_batch.serialize()) {
+        return false;
+    }
+
+    // Upload without holding any locks
+    bool success = uploader.upload(upload_batch.get_encoded_profile(), upload_batch.stats());
+    upload_batch.stats().reset_state();
     return success;
 }
 

ddtrace/internal/datadog/profiling/dd_wrapper/src/profile.cpp

@@ -58,6 +58,18 @@ Datadog::Profile::reset_profile()
         return false;
     }
 
+    res = ddog_prof_Profile_reset(&last_profile);
+    if (!res.ok) {          // NOLINT (cppcoreguidelines-pro-type-union-access)
+        auto err = res.err; // NOLINT (cppcoreguidelines-pro-type-union-access)
+        if (!already_warned) {
+            already_warned = true;
+            const std::string errmsg = err_to_msg(&err, "Error resetting last_profile");
+            std::cerr << "Could not drop last_profile:" << errmsg << std::endl;
+        }
+        ddog_Error_drop(&err);
+        return false;
+    }
+
     cur_profiler_stats.reset_state();
     return true;
 }
@@ -189,7 +201,15 @@ Datadog::Profile::one_time_init(SampleType type, unsigned int _max_nframes)
     if (!make_profile(sample_types, &default_period, cur_profile)) {
         if (!already_warned) {
             already_warned = true;
-            std::cerr << "Error initializing profile" << std::endl;
+            std::cerr << "Error initializing cur_profile" << std::endl;
+        }
+        return;
+    }
+
+    if (!make_profile(sample_types, &default_period, last_profile)) {
+        if (!already_warned) {
+            already_warned = true;
+            std::cerr << "Error initializing last_profile" << std::endl;
         }
         return;
     }

ddtrace/internal/datadog/profiling/dd_wrapper/src/profile_borrow.cpp

@@ -6,6 +6,9 @@ Datadog::ProfileBorrow::ProfileBorrow(Profile& profile)
 {
     // Lock the mutex on construction
     profile_ptr->profile_borrow_internal();
+
+    std::swap(profile_ptr->cur_profile, profile_ptr->last_profile);
+    std::swap(profile_ptr->cur_profiler_stats, profiler_stats);
 }
 
 Datadog::ProfileBorrow::~ProfileBorrow()
@@ -17,6 +20,7 @@ Datadog::ProfileBorrow::~ProfileBorrow()
 
 Datadog::ProfileBorrow::ProfileBorrow(ProfileBorrow&& other) noexcept
   : profile_ptr(other.profile_ptr)
+  , profiler_stats(std::move(other.profiler_stats))
 {
     other.profile_ptr = nullptr;
 }
@@ -32,6 +36,7 @@ Datadog::ProfileBorrow::operator=(ProfileBorrow&& other) noexcept
 
         // Take ownership from other
         profile_ptr = other.profile_ptr;
+        profiler_stats = std::move(other.profiler_stats);
         other.profile_ptr = nullptr;
     }
     return *this;
@@ -40,11 +45,11 @@ Datadog::ProfileBorrow::operator=(ProfileBorrow&& other) noexcept
 ddog_prof_Profile&
 Datadog::ProfileBorrow::profile()
 {
-    return profile_ptr->cur_profile;
+    return profile_ptr->last_profile;
 }
 
 Datadog::ProfilerStats&
 Datadog::ProfileBorrow::stats()
 {
-    return profile_ptr->cur_profiler_stats;
+    return profiler_stats;
 }