From 9601d2911754e65c4bf211e85d7d5e691852217b Mon Sep 17 00:00:00 2001
From: Taegyun Kim <taegyun.kim@datadoghq.com>
Date: Thu, 26 Sep 2024 22:00:13 +0000
Subject: [PATCH] address comment

---
 .../dd_wrapper/include/code_provenance.hpp    |  3 ++-
 .../dd_wrapper/src/code_provenance.cpp        | 12 ++++++------
 .../profiling/dd_wrapper/src/uploader.cpp     | 19 ++++++++-----------
 3 files changed, 16 insertions(+), 18 deletions(-)

diff --git a/ddtrace/internal/datadog/profiling/dd_wrapper/include/code_provenance.hpp b/ddtrace/internal/datadog/profiling/dd_wrapper/include/code_provenance.hpp
index f00c05ad17..29e0896c41 100644
--- a/ddtrace/internal/datadog/profiling/dd_wrapper/include/code_provenance.hpp
+++ b/ddtrace/internal/datadog/profiling/dd_wrapper/include/code_provenance.hpp
@@ -3,6 +3,7 @@
 #include <atomic>
 #include <memory>
 #include <mutex>
+#include <optional>
 #include <set>
 #include <string>
 #include <string_view>
@@ -41,7 +42,7 @@ class CodeProvenance
     void set_stdlib_path(std::string_view stdlib_path);
     void add_packages(std::unordered_map<std::string_view, std::string_view> packages);
     void add_filename(std::string_view filename);
-    std::string serialize_to_json_str();
+    std::optional<std::string> try_serialize_to_json_str();
 
   private:
     // Mutex to protect the state
diff --git a/ddtrace/internal/datadog/profiling/dd_wrapper/src/code_provenance.cpp b/ddtrace/internal/datadog/profiling/dd_wrapper/src/code_provenance.cpp
index 14ef947a0e..d304b0ae69 100644
--- a/ddtrace/internal/datadog/profiling/dd_wrapper/src/code_provenance.cpp
+++ b/ddtrace/internal/datadog/profiling/dd_wrapper/src/code_provenance.cpp
@@ -91,18 +91,17 @@ CodeProvenance::add_filename(std::string_view filename)
     }
 }
 
-std::string
-CodeProvenance::serialize_to_json_str()
+std::optional<std::string>
+CodeProvenance::try_serialize_to_json_str()
 {
     if (!is_enabled()) {
-        return "";
+        return std::nullopt;
     }
 
     std::lock_guard<std::mutex> lock(mtx);
 
     std::ostringstream out;
-
-    // TODO(taegyunkim): Use a JSON library to serialize this
+    // DEV: Simple JSON serialization, consider using a JSON library.
     out << "{\"v1\":["; // Start of the JSON array
     for (const auto& [package, paths] : packages_to_files) {
         out << "{"; // Start of the JSON object
@@ -128,8 +127,9 @@ CodeProvenance::serialize_to_json_str()
     out << "\"" << stdlib_path << "\"";
     out << "]";
     out << "}"; // End of stdlib JSON object
-
     out << "]}"; // End of the JSON array
+
+    // Clear the state
     packages_to_files.clear();
     return out.str();
 }
diff --git a/ddtrace/internal/datadog/profiling/dd_wrapper/src/uploader.cpp b/ddtrace/internal/datadog/profiling/dd_wrapper/src/uploader.cpp
index f6d4384575..3e63b97eb0 100644
--- a/ddtrace/internal/datadog/profiling/dd_wrapper/src/uploader.cpp
+++ b/ddtrace/internal/datadog/profiling/dd_wrapper/src/uploader.cpp
@@ -3,8 +3,9 @@
 #include "code_provenance.hpp"
 #include "libdatadog_helpers.hpp"
 
-#include <errno.h>  // errno
-#include <fstream>  // ofstream
+#include <errno.h> // errno
+#include <fstream> // ofstream
+#include <optional>
 #include <sstream>  // ostringstream
 #include <string.h> // strerror
 #include <unistd.h> // getpid
@@ -76,17 +77,13 @@ Datadog::Uploader::upload(ddog_prof_Profile& profile)
       .file = ddog_Vec_U8_as_slice(&encoded->buffer),
     } };
 
-    std::string json_str;
-
-    // DEV: re-consider this is ok to do so, as calling is_enabled/serialize will acquire
-    // a lock on the CodeProvenance instance
-    if (CodeProvenance::get_instance().is_enabled()) {
-        json_str = CodeProvenance::get_instance().serialize_to_json_str();
-    }
-    if (!json_str.empty()) {
+    // DEV: This function is called with the profile_lock held, and the following
+    // function call acquires lock on CodeProvenance.
+    std::optional<std::string> json_str_opt = CodeProvenance::get_instance().try_serialize_to_json_str();
+    if (json_str_opt.has_value() and !json_str_opt.value().empty()) {
         files_to_send.push_back({
           .name = to_slice("code-provenance.json"),
-          .file = to_byte_slice(json_str),
+          .file = to_byte_slice(json_str_opt.value()),
         });
     }