Skip to content

chore: leak uploader config to fix crashes on uwsgi tests #13520

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 67 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
67 commits
Select commit Hold shift + click to select a range
834108f
join on exit
taegyunkim May 7, 2025
0781f40
wait to join
taegyunkim May 7, 2025
6a8e056
join on linux too
taegyunkim May 7, 2025
f4d7395
join threads from restart_on_fork
taegyunkim May 8, 2025
75121af
Merge branch 'main' into taegyunkim/v2-join
taegyunkim May 22, 2025
0f1fd68
Discard changes to ddtrace/profiling/profiler.py
taegyunkim May 22, 2025
6c0fd61
main with uwsgi test
taegyunkim May 28, 2025
c533d77
debug print
taegyunkim May 28, 2025
f486c94
cpp debug print
taegyunkim May 28, 2025
373e87b
explicitly leak the strings
taegyunkim May 28, 2025
8b7c855
remove debug prints
taegyunkim May 28, 2025
5505361
static singleton for config
taegyunkim May 28, 2025
fc41a19
Discard changes to ddtrace/internal/datadog/profiling/dd_wrapper/src/…
taegyunkim May 28, 2025
095501c
remove prints
taegyunkim May 28, 2025
3c5462e
cformat
taegyunkim May 28, 2025
48f9da4
Merge branch 'main' into taegyunkim/uwsgi-test
taegyunkim May 28, 2025
f78baee
private constructor/destructor
taegyunkim May 28, 2025
80a608d
return reference
taegyunkim May 28, 2025
3df278e
make it less verbose
taegyunkim May 28, 2025
6a0ed1d
function local
taegyunkim May 28, 2025
41166e9
format
taegyunkim May 28, 2025
4eeb6e5
Merge branch 'main' into taegyunkim/uwsgi-test
taegyunkim May 28, 2025
b25efed
try splitting
taegyunkim May 28, 2025
bbece76
Merge branch 'taegyunkim/uwsgi-test' of github.com:DataDog/dd-trace-p…
taegyunkim May 28, 2025
c011f2c
debug print
taegyunkim May 28, 2025
11e2487
Merge branch 'main' into taegyunkim/v2-join
taegyunkim May 28, 2025
2a25aab
Merge branch 'taegyunkim/v2-join' into taegyunkim/uwsgi-test
taegyunkim May 28, 2025
8e5f020
compile with debug info
taegyunkim May 28, 2025
e79c478
bust
taegyunkim May 28, 2025
6354b01
bust again
taegyunkim May 28, 2025
33be897
dont pass output filename to config
taegyunkim May 29, 2025
19331bf
Revert "dont pass output filename to config"
taegyunkim May 29, 2025
ffbba45
what about this?
taegyunkim May 29, 2025
d35d2e8
double checking
taegyunkim May 29, 2025
1fbf968
thread safe function local static singleton
taegyunkim May 29, 2025
a6ccc20
more debug info
taegyunkim May 29, 2025
68f6365
lets check the order of prints
taegyunkim May 29, 2025
79dc2ac
leaky singleton
taegyunkim May 29, 2025
da92aa1
remove print
taegyunkim May 29, 2025
393ab98
print unloading
taegyunkim May 29, 2025
e2737e7
attribute destructor
taegyunkim May 29, 2025
e7c4acd
Discard changes to ddtrace/internal/datadog/profiling/stack_v2/includ…
taegyunkim May 29, 2025
00c7084
Discard changes to ddtrace/internal/datadog/profiling/stack_v2/src/sa…
taegyunkim May 29, 2025
f4fbed5
Discard changes to scripts/gen_gitlab_config.py
taegyunkim May 29, 2025
dd5a3e3
Discard changes to setup.py
taegyunkim May 29, 2025
48c6f80
print filename
taegyunkim May 29, 2025
cfe049d
Merge branch 'main' into taegyunkim/uwsgi-test
taegyunkim May 29, 2025
47f74bc
remove prints
taegyunkim May 29, 2025
c159958
Merge branch 'main' into taegyunkim/uwsgi-test
taegyunkim May 29, 2025
0f8a437
remove unistd.h
taegyunkim May 29, 2025
f3b81dd
does this give me stacktraces
taegyunkim May 29, 2025
78da342
edit native file to bust cache
taegyunkim May 29, 2025
ad2e2c5
sudo not found
taegyunkim May 29, 2025
ad92478
Discard changes to scripts/gen_gitlab_config.py
taegyunkim May 29, 2025
9b47c98
Discard changes to .gitlab/tests.yml
taegyunkim May 29, 2025
b0a1c46
Revert "Discard changes to scripts/gen_gitlab_config.py"
taegyunkim May 29, 2025
6d6a6e4
Revert "Discard changes to .gitlab/tests.yml"
taegyunkim May 29, 2025
40b96fd
seems like its already set
taegyunkim May 29, 2025
f405d42
upload on failure
taegyunkim May 30, 2025
4ca4fdf
use after_script
taegyunkim May 30, 2025
14c152d
ok jst get the core.* files
taegyunkim May 30, 2025
55eeedb
Discard changes to scripts/gen_gitlab_config.py
taegyunkim May 30, 2025
6ec6f20
Discard changes to .gitlab/tests.yml
taegyunkim May 30, 2025
26f69eb
Merge branch 'main' into taegyunkim/uwsgi-test
taegyunkim May 30, 2025
1bc72b4
Merge branch 'main' into taegyunkim/uwsgi-test
taegyunkim May 30, 2025
d409710
Merge branch 'main' into taegyunkim/v2-join
taegyunkim May 30, 2025
209e80a
Merge branch 'taegyunkim/v2-join' into taegyunkim/uwsgi-test
taegyunkim May 30, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,8 @@ class Uploader
std::string errmsg;
static inline ddog_CancellationToken cancel{ .inner = nullptr };
static inline std::atomic<uint64_t> upload_seq{ 0 };
std::string output_filename;
ddog_prof_ProfileExporter ddog_exporter{ .inner = nullptr };

bool export_to_file(ddog_prof_EncodedProfile* encoded);

public:
bool upload(ddog_prof_Profile& profile);
static void cancel_inflight();
Expand All @@ -35,7 +32,9 @@ class Uploader
static void postfork_parent();
static void postfork_child();

Uploader(std::string_view _url, ddog_prof_ProfileExporter ddog_exporter);
static bool export_to_file(ddog_prof_Profile& profile);

Uploader(ddog_prof_ProfileExporter ddog_exporter);
~Uploader()
{
// We need to call _drop() on the exporter and the cancellation token,
Expand Down Expand Up @@ -68,7 +67,6 @@ class Uploader
{
ddog_exporter = other.ddog_exporter;
other.ddog_exporter = { .inner = nullptr };
output_filename = std::move(other.output_filename);
errmsg = std::move(other.errmsg);
}

Expand All @@ -78,7 +76,6 @@ class Uploader
ddog_prof_Exporter_drop(&ddog_exporter);
ddog_exporter = other.ddog_exporter;
other.ddog_exporter = { .inner = nullptr };
output_filename = std::move(other.output_filename);
errmsg = std::move(other.errmsg);
}
return *this;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,48 +3,82 @@
#include "uploader.hpp"

#include <mutex>
#include <stdio.h>
#include <string>
#include <string_view>
#include <unordered_map>
#include <variant>

namespace Datadog {

class UploaderBuilder
class UploaderConfig
{
using ExporterTagset = std::unordered_map<std::string, std::string>;
static inline std::mutex tag_mutex{};

// Building parameters
static inline std::string dd_env;
static inline std::string service;
static inline std::string version;
static inline std::string runtime{ g_runtime_name };
static inline std::string runtime_id;
static inline std::string runtime_version;
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 };

private:
std::string dd_env;
std::string service;
std::string version;
std::string runtime{ g_runtime_name };
std::string runtime_id;
std::string runtime_version;
std::string profiler_version;
std::string url{ "http://localhost:8126" };
std::string output_filename;

std::string language{ g_language_name };
std::string family{ g_language_name };

std::mutex tag_mutex;
ExporterTagset user_tags;

// Private Constructor/Destructor
UploaderConfig() = default;
~UploaderConfig() = default;

public:
static void set_env(std::string_view _dd_env);
static void set_service(std::string_view _service);
static void set_version(std::string_view _version);
static void set_runtime(std::string_view _runtime);
static void set_runtime_id(std::string_view _runtime_id);
static void set_runtime_version(std::string_view _runtime_version);
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);
UploaderConfig(const UploaderConfig&) = delete;
UploaderConfig& operator=(const UploaderConfig&) = delete;
UploaderConfig(UploaderConfig&&) = delete;
UploaderConfig& operator=(UploaderConfig&&) = delete;

static std::variant<Uploader, std::string> build();
static UploaderConfig& get_instance()
{
static UploaderConfig* instance = new UploaderConfig();
return *instance;
}

void set_env(std::string_view _dd_env);
void set_service(std::string_view _service);
void set_version(std::string_view _version);
void set_runtime(std::string_view _runtime);
void set_runtime_id(std::string_view _runtime_id);
void set_runtime_version(std::string_view _runtime_version);
void set_profiler_version(std::string_view _profiler_version);
void set_url(std::string_view _url);
void set_tag(std::string_view _key, std::string_view _val);
void set_output_filename(std::string_view _output_filename);

std::string_view get_env() const;
std::string_view get_service() const;
std::string_view get_version() const;
std::string_view get_runtime() const;
std::string_view get_runtime_id() const;
std::string_view get_runtime_version() const;
std::string_view get_profiler_version() const;
std::string_view get_url() const;
std::string_view get_output_filename() const;
std::string_view get_language() const;
std::string_view get_family() const;
const ExporterTagset& get_user_tags() const;

static void postfork_child();
};

class UploaderBuilder
{
public:
static std::variant<Uploader, std::string> build();
};

} // namespace Datadog
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ ddup_postfork_child()
{
Datadog::Uploader::postfork_child();
Datadog::SampleManager::postfork_child();
Datadog::UploaderBuilder::postfork_child();
Datadog::UploaderConfig::postfork_child();
}

void
Expand All @@ -51,55 +51,55 @@ ddup_prefork()
void
ddup_config_env(std::string_view dd_env) // cppcheck-suppress unusedFunction
{
Datadog::UploaderBuilder::set_env(dd_env);
Datadog::UploaderConfig::get_instance().set_env(dd_env);
}

void
ddup_config_service(std::string_view service) // cppcheck-suppress unusedFunction
{
Datadog::UploaderBuilder::set_service(service);
Datadog::UploaderConfig::get_instance().set_service(service);
}

void
ddup_config_version(std::string_view version) // cppcheck-suppress unusedFunction
{
Datadog::UploaderBuilder::set_version(version);
Datadog::UploaderConfig::get_instance().set_version(version);
}

void
ddup_config_runtime(std::string_view runtime) // cppcheck-suppress unusedFunction
{
Datadog::UploaderBuilder::set_runtime(runtime);
Datadog::UploaderConfig::get_instance().set_runtime(runtime);
}

void
ddup_set_runtime_id(std::string_view runtime_id) // cppcheck-suppress unusedFunction
{
Datadog::UploaderBuilder::set_runtime_id(runtime_id);
Datadog::UploaderConfig::get_instance().set_runtime_id(runtime_id);
}

void
ddup_config_runtime_version(std::string_view runtime_version) // cppcheck-suppress unusedFunction
{
Datadog::UploaderBuilder::set_runtime_version(runtime_version);
Datadog::UploaderConfig::get_instance().set_runtime_version(runtime_version);
}

void
ddup_config_profiler_version(std::string_view profiler_version) // cppcheck-suppress unusedFunction
{
Datadog::UploaderBuilder::set_profiler_version(profiler_version);
Datadog::UploaderConfig::get_instance().set_profiler_version(profiler_version);
}

void
ddup_config_url(std::string_view url) // cppcheck-suppress unusedFunction
{
Datadog::UploaderBuilder::set_url(url);
Datadog::UploaderConfig::get_instance().set_url(url);
}

void
ddup_config_user_tag(std::string_view key, std::string_view val) // cppcheck-suppress unusedFunction
{
Datadog::UploaderBuilder::set_tag(key, val);
Datadog::UploaderConfig::get_instance().set_tag(key, val);
}

void
Expand All @@ -123,7 +123,7 @@ ddup_config_timeline(bool enabled) // cppcheck-suppress unusedFunction
void
ddup_config_output_filename(std::string_view output_filename) // cppcheck-suppress unusedFunction
{
Datadog::UploaderBuilder::set_output_filename(output_filename);
Datadog::UploaderConfig::get_instance().set_output_filename(output_filename);
}

void
Expand Down Expand Up @@ -337,6 +337,15 @@ ddup_upload() // cppcheck-suppress unusedFunction
return false;
}

const auto& config = Datadog::UploaderConfig::get_instance();

if (!config.get_output_filename().empty()) {
Datadog::Uploader::export_to_file(Datadog::Sample::profile_borrow());
Datadog::Sample::profile_release();
Datadog::Sample::profile_clear_state();
return true;
}

auto uploader_or_err = Datadog::UploaderBuilder::build();

if (std::holds_alternative<std::string>(uploader_or_err)) {
Expand Down
29 changes: 18 additions & 11 deletions ddtrace/internal/datadog/profiling/dd_wrapper/src/uploader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

#include "code_provenance.hpp"
#include "libdatadog_helpers.hpp"
#include "uploader_builder.hpp"

#include <errno.h> // errno
#include <fstream> // ofstream
Expand All @@ -17,40 +18,52 @@

using namespace Datadog;

Datadog::Uploader::Uploader(std::string_view _output_filename, ddog_prof_ProfileExporter _ddog_exporter)
: output_filename{ _output_filename }
, ddog_exporter{ _ddog_exporter }
Datadog::Uploader::Uploader(ddog_prof_ProfileExporter _ddog_exporter)
: 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)
Datadog::Uploader::export_to_file(ddog_prof_Profile& profile)
{
const auto& config = Datadog::UploaderConfig::get_instance();
// Write the profile to a file using the following format for filename:
// <output_filename>.<process_id>.<sequence_number>
std::ostringstream oss;
oss << output_filename << "." << getpid() << "." << upload_seq;
oss << config.get_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;
}

ddog_prof_Profile_SerializeResult serialize_result = ddog_prof_Profile_serialize(&profile, nullptr, nullptr);
if (serialize_result.tag != DDOG_PROF_PROFILE_SERIALIZE_RESULT_OK) {
auto err = serialize_result.err;
std::cerr << err_to_msg(&err, "Error serializing pprof") << std::endl;
ddog_Error_drop(&err);
return false;
}
ddog_prof_EncodedProfile* encoded = &serialize_result.ok;

auto bytes_res = ddog_prof_EncodedProfile_bytes(encoded);
if (bytes_res.tag == DDOG_PROF_RESULT_BYTE_SLICE_ERR_BYTE_SLICE) {
std::cerr << "Error getting bytes from encoded profile: "
<< err_to_msg(&bytes_res.err, "Error getting bytes from encoded profile") << std::endl;
ddog_Error_drop(&bytes_res.err);
return false;
}

out.write(reinterpret_cast<const char*>(bytes_res.ok.ptr), bytes_res.ok.len);
if (out.fail()) {
std::cerr << "Error writing to output file " << filename << ": " << strerror(errno) << std::endl;
return false;
}

return true;
}

Expand All @@ -69,12 +82,6 @@ Datadog::Uploader::upload(ddog_prof_Profile& profile)
}
ddog_prof_EncodedProfile* encoded = &serialize_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;
}

std::vector<ddog_prof_Exporter_File> to_compress_files;

std::string_view json_str = CodeProvenance::get_instance().get_json_str();
Expand Down
Loading
Loading