Skip to content
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

feat(profiling): use upstream echion #12517

Draft
wants to merge 9 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 6 additions & 4 deletions ddtrace/internal/datadog/profiling/stack_v2/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ endif()

# Add echion
set(ECHION_COMMIT
"ed744987f224fae3f93c842b2b5ffb083984ff8b"
"merge-downstream"
CACHE STRING "Commit hash of echion to use")
FetchContent_Declare(
echion
Expand All @@ -51,7 +51,8 @@ if(NOT echion_POPULATED)
endif()

# Specify the target C-extension that we want to build
add_library(${EXTENSION_NAME} SHARED src/sampler.cpp src/stack_renderer.cpp src/stack_v2.cpp src/thread_span_links.cpp)
add_library(${EXTENSION_NAME} SHARED ${echion_SOURCE_DIR}/echion/frame.cc ${echion_SOURCE_DIR}/echion/render.cc
src/sampler.cpp src/stack_renderer.cpp src/stack_v2.cpp src/thread_span_links.cpp)

# Add common config
add_ddup_config(${EXTENSION_NAME})
Expand All @@ -68,8 +69,9 @@ add_cppcheck_target(
SRC
${CMAKE_CURRENT_SOURCE_DIR}/src)

# Never build with native unwinding, since this is not currently used
target_compile_definitions(${EXTENSION_NAME} PRIVATE UNWIND_NATIVE_DISABLE)
# Never build with native unwinding, since this is not currently used. Echion is by default configured to ignore idle
# threads.
target_compile_definitions(${EXTENSION_NAME} PRIVATE UNWIND_NATIVE_DISABLE ECHION_OBSERVE_IDLE_THREADS)

# Includes; echion and python are marked "system" to suppress warnings, but note in MSVC we'll have to #pragma
# warning(push, 0 then pop for the same effect.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
#include "python_headers.hpp"

#include "dd_wrapper/include/ddup_interface.hpp"

#include "echion/frame.h"
#include "echion/render.h"

namespace Datadog {
Expand All @@ -38,16 +40,28 @@ class StackRenderer : public RendererInterface
// the sample is created, this has to be reset.
bool pushed_task_name = false;

void open() override {}
void close() override {}
void header() override {}
void metadata(const std::string&, const std::string&) override {}
void stack(mojo_int_t, mojo_int_t, const std::string&) override {};
void frame(mojo_ref_t, mojo_ref_t, mojo_ref_t, mojo_int_t, mojo_int_t, mojo_int_t, mojo_int_t) override{};
void frame_ref(mojo_ref_t) override{};
void frame_kernel(const std::string&) override {};
void metric_time(mojo_int_t) override{};
void metric_memory(mojo_int_t) override{};
void string(mojo_ref_t, const std::string&) override {};
void string_ref(mojo_ref_t) override{};

virtual void render_message(std::string_view msg) override;
virtual void render_thread_begin(PyThreadState* tstate,
std::string_view name,
microsecond_t wall_time_us,
uintptr_t thread_id,
unsigned long native_id) override;
virtual void render_task_begin(std::string_view name) override;
virtual void render_task_begin() override;
virtual void render_stack_begin() override;
virtual void render_python_frame(std::string_view name, std::string_view file, uint64_t line) override;
virtual void render_native_frame(std::string_view name, std::string_view file, uint64_t line) override;
virtual void render_frame(Frame& frame) override;
virtual void render_cpu_time(uint64_t cpu_time_us) override;
virtual void render_stack_end() override;
virtual bool is_valid() override;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
#include "thread_span_links.hpp"
#include "utf8_validate.hpp"

#include "echion/strings.h"

using namespace Datadog;

void
Expand Down Expand Up @@ -66,7 +68,7 @@ StackRenderer::render_thread_begin(PyThreadState* tstate,
}

void
StackRenderer::render_task_begin(std::string_view)
StackRenderer::render_task_begin()
{
static bool failed = false;
if (failed) {
Expand Down Expand Up @@ -112,43 +114,57 @@ StackRenderer::render_stack_begin()
}

void
StackRenderer::render_python_frame(std::string_view name, std::string_view file, uint64_t line)
StackRenderer::render_frame(Frame& frame)
{
if (sample == nullptr) {
std::cerr << "Received a new frame without sample storage. Some profiling data has been lost." << std::endl;
return;
}

// Ordinarily we could just call frame_cache->lookup() here, but our
// underlying frame is owned by the LRUCache, which may have cleaned it up,
// causing the table keys to be garbage. Since individual frames in
// the stack may be bad, this isn't a failable condition. Instead, populate
// some defaults.
static constexpr std::string_view missing_filename = "<unknown file>";
static constexpr std::string_view missing_name = "<unknown function>";
std::string_view filename_str;
std::string_view name_str;
try {
filename_str = string_table.lookup(frame.filename);
} catch (StringTable::Error&) {
filename_str = missing_filename;
}

try {
name_str = string_table.lookup(frame.name);
} catch (StringTable::Error&) {
name_str = missing_name;
}

auto line = frame.location.line;

// Normally, further utf-8 validation would be pointless here, but we may be reading data where the
// string pointer was valid, but the string is actually garbage data at the exact time of the read.
// This is rare, but blowing some cycles on early validation allows the sample to be retained by
// libdatadog, so we can evaluate the actual impact of this scenario in live scenarios.
static const std::string_view invalid = "<invalid_utf8>";
if (!utf8_check_is_valid(name.data(), name.size())) {
name = invalid;
if (!utf8_check_is_valid(name_str.data(), name_str.size())) {
name_str = invalid;
}
if (!utf8_check_is_valid(file.data(), file.size())) {
file = invalid;
if (!utf8_check_is_valid(filename_str.data(), filename_str.size())) {
filename_str = invalid;
}
// DEV: Echion pushes a dummy frame containing task name, and its line
// number is set to 0.
if (!pushed_task_name and line == 0) {
ddup_push_task_name(sample, name);
ddup_push_task_name(sample, name_str);
pushed_task_name = true;
// And return early to avoid pushing task name as a frame
return;
}

ddup_push_frame(sample, name, file, 0, line);
}

void
StackRenderer::render_native_frame(std::string_view name, std::string_view file, uint64_t line)
{
// This function is part of the necessary API, but it is unused by the Datadog profiler for now.
(void)name;
(void)file;
(void)line;
ddup_push_frame(sample, name_str, filename_str, 0, line);
}

void
Expand Down
Loading