Skip to content

Commit

Permalink
address comment
Browse files Browse the repository at this point in the history
  • Loading branch information
taegyunkim committed Sep 26, 2024
1 parent de60f4e commit 9601d29
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
#include <atomic>
#include <memory>
#include <mutex>
#include <optional>
#include <set>
#include <string>
#include <string_view>
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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();
}
Expand Down
19 changes: 8 additions & 11 deletions ddtrace/internal/datadog/profiling/dd_wrapper/src/uploader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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()),
});
}

Expand Down

0 comments on commit 9601d29

Please sign in to comment.