Python frames

r1viollet · r1viollet · commit f8d102f03e2c · 2023-06-14T11:00:37.000+02:00
- Avoid ending profiling when non utf8 strings are found
- Other fixups to investigate libaustin issue
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -267,7 +267,7 @@ target_link_libraries(dd_profiling-embedded PUBLIC dl pthread rt)
 # add libaustin
 add_library(austin SHARED IMPORTED)
 set_target_properties(
-  austin PROPERTIES IMPORTED_LOCATION "${CMAKE_SOURCE_DIR}/lib/libaustin.a"
+  austin PROPERTIES IMPORTED_LOCATION "${CMAKE_SOURCE_DIR}/lib_debug/libaustin.a"
                     INTERFACE_INCLUDE_DIRECTORIES "${CMAKE_SOURCE_DIR}/include/libaustin")
 
 set(LIBDD_PROFILING_EMBEDDED_OBJECT "${CMAKE_BINARY_DIR}/libdd_profiling-embedded.o")
diff --git a/app/base-env/Dockerfile b/app/base-env/Dockerfile
@@ -165,12 +165,17 @@ RUN pip3 install cmake_format
 
 RUN apt-get update \
   && DEBIAN_FRONTEND=noninteractive apt-get install -y \
-    libtool
+    libtool \
+    libiberty-dev
 
-RUN git clone --branch r1viollet/libaustin https://github.com/r1viollet/austin.git && \
+
+# possibly useful command
+# CFLAGS="-g -O0 -DTRACE" ./configure
+# Valid commit : git checkout a7a292b3f5a1058051b017c0d339a678efdec704
+RUN git clone --branch r1viollet/libaustin_v2 https://github.com/r1viollet/austin.git && \
     cd austin && \
     autoreconf --install && \
-    ./configure && \
+    CFLAGS="-g -O0 -DTRACE" ./configure && \
     make && \
     make install
 
diff --git a/include/ddprof_process.hpp b/include/ddprof_process.hpp
@@ -36,7 +36,7 @@ class Process {
   // lazy read of container id
   const ContainerId &get_container_id(std::string_view path_to_proc = "");
 
-  austin_handle_t get_austin_handle();
+  austin_handle_t get_austin_handle(pid_t tid);
 
   uint64_t _sample_counter = {};
 
diff --git a/include/symbol.hpp b/include/symbol.hpp
@@ -21,7 +21,7 @@ class Symbol {
   Symbol(std::string symname, std::string demangle_name, uint32_t lineno,
          std::string srcpath)
       : _symname(std::move(symname)), _demangle_name(std::move(demangle_name)),
-        _lineno(lineno), _srcpath(std::move(srcpath)) {}
+        _lineno(lineno), _srcpath(std::move(srcpath)), _is_python_frame(false) {}
 
   // OUTPUT OF ADDRINFO
   std::string _symname;
diff --git a/include/unwind.hpp b/include/unwind.hpp
@@ -16,7 +16,7 @@ void unwind_init(void);
 
 // Fill sample info to prepare for unwinding
 void unwind_init_sample(UnwindState *us, uint64_t *sample_regs,
-                        pid_t sample_pid, uint64_t sample_size_stack,
+                        pid_t sample_pid, pid_t sample_tid, uint64_t sample_size_stack,
                         char *sample_data_stack);
 
 // Main unwind API
diff --git a/include/unwind_state.hpp b/include/unwind_state.hpp
@@ -54,6 +54,7 @@ struct UnwindState {
   ddprof::ProcessHdr process_hdr;
 
   pid_t pid;
+  pid_t tid;
   char *stack;
   size_t stack_sz;
 
diff --git a/lib/libaustin.so b/lib/libaustin.so
diff --git a/lib_debug/libaustin.a b/lib_debug/libaustin.a
diff --git a/src/ddprof_process.cc b/src/ddprof_process.cc
@@ -27,9 +27,11 @@ Process::~Process() {
   }
 }
 
-austin_handle_t Process::get_austin_handle() {
+austin_handle_t Process::get_austin_handle(pid_t tid) {
   if (!_austin_handle) {
+    LG_NTC("Attaching to PID %d/%d", _pid, tid);
     _austin_handle = austin_attach(_pid);
+    LG_NTC("Result of attach %p", _austin_handle);
   }
   return _austin_handle;
 }
diff --git a/src/ddprof_worker.cc b/src/ddprof_worker.cc
@@ -233,7 +233,7 @@ static DDRes ddprof_unwind_sample(DDProfContext *ctx, perf_event_sample *sample,
   ddprof_stats_add(STATS_UNWIND_AVG_STACK_SIZE, sample->size_stack, nullptr);
 
   // copy the sample context into the unwind structure
-  unwind_init_sample(us, sample->regs, sample->pid, sample->size_stack,
+  unwind_init_sample(us, sample->regs, sample->pid, sample->tid, sample->size_stack,
                      sample->data_stack);
 
   // If a sample has a PID, it has a TID.  Include it for downstream labels
diff --git a/src/pprof/ddprof_pprof.cc b/src/pprof/ddprof_pprof.cc
@@ -268,8 +268,9 @@ DDRes pprof_aggregate(const UnwindOutput *uw_output,
   ddog_prof_Profile_AddResult add_res = ddog_prof_Profile_add(profile, sample);
   if (add_res.tag == DDOG_PROF_PROFILE_ADD_RESULT_ERR) {
     defer { ddog_Error_drop(&add_res.err); };
-    DDRES_RETURN_ERROR_LOG(DD_WHAT_PPROF, "Unable to add profile: %s",
-                           add_res.err.message.ptr);
+    LG_NTC("Unable to add profile - continue execution.");
+//    DDRES_RETURN_ERROR_LOG(DD_WHAT_PPROF, "Unable to add profile: %s",
+//                           add_res.err.message.ptr);
   }
   return ddres_init();
 }
diff --git a/src/unwind.cc b/src/unwind.cc
@@ -44,13 +44,14 @@ static void add_container_id(UnwindState *us) {
 }
 
 void unwind_init_sample(UnwindState *us, uint64_t *sample_regs,
-                        pid_t sample_pid, uint64_t sample_size_stack,
+                        pid_t sample_pid, pid_t sample_tid, uint64_t sample_size_stack,
                         char *sample_data_stack) {
   us->output.clear();
   memcpy(&us->initial_regs.regs[0], sample_regs,
          K_NB_REGS_UNWIND * sizeof(uint64_t));
   us->current_ip = us->initial_regs.regs[REGNAME(PC)];
   us->pid = sample_pid;
+  us->tid = sample_tid;
   us->stack_sz = sample_size_stack;
   us->stack = sample_data_stack;
 }
diff --git a/src/unwind_dwfl.cc b/src/unwind_dwfl.cc
@@ -406,10 +406,14 @@ static DDRes add_python_frame(UnwindState *us, SymbolIdx_t symbol_idx,
                               ElfAddress_t pc, Dwfl_Frame *dwfl_frame) {
   SymbolHdr &unwind_symbol_hdr = us->symbol_hdr;
   SymbolTable &symbol_table = unwind_symbol_hdr._symbol_table;
-  Process &p = us->process_hdr.get_process(us->pid);
-  austin_handle_t handle = p.get_austin_handle();
 
-  if (handle && (symbol_table.at(symbol_idx)._is_python_frame)) {
+  if (symbol_table.at(symbol_idx)._is_python_frame) {
+    // only attempt to attach on python frames
+    Process &p = us->process_hdr.get_process(us->pid);
+    austin_handle_t handle = p.get_austin_handle(us->tid);
+    if (!handle) {
+      return ddres_error(1);
+    }
     AustinSymbolLookup &austin_symbol_lookup =
         unwind_symbol_hdr._austin_symbol_lookup;
     // The register we are interested in is RSI, but it doesn't seem to be
diff --git a/test/allocation_tracker-ut.cc b/test/allocation_tracker-ut.cc
@@ -73,7 +73,7 @@ TEST(allocation_tracker, start_stop) {
     ASSERT_EQ(sample->addr, 0xdeadbeef);
 
     UnwindState state;
-    ddprof::unwind_init_sample(&state, sample->regs, sample->pid,
+    ddprof::unwind_init_sample(&state, sample->regs, sample->pid, sample->tid,
                                sample->size_stack, sample->data_stack);
     ddprof::unwindstate__unwind(&state);
 
diff --git a/test/savecontext-ut.cc b/test/savecontext-ut.cc
@@ -29,7 +29,7 @@ void funcB() {
   uint64_t regs[K_NB_REGS_UNWIND];
   size_t stack_size = save_context(retrieve_stack_bounds(), regs, stack);
 
-  ddprof::unwind_init_sample(&state, regs, getpid(), stack_size,
+  ddprof::unwind_init_sample(&state, regs, getpid(), 0, stack_size,
                              reinterpret_cast<char *>(stack));
   ddprof::unwindstate__unwind(&state);
 
@@ -98,7 +98,7 @@ TEST(getcontext, unwind_from_sighandler) {
   t.join();
 
   UnwindState state;
-  ddprof::unwind_init_sample(&state, regs, getpid(), stack_size,
+  ddprof::unwind_init_sample(&state, regs, getpid(), 0, stack_size,
                              reinterpret_cast<char *>(stack));
   ddprof::unwindstate__unwind(&state);
 

Original file line number	Diff line number	Diff line change
`@@ -27,9 +27,11 @@ Process::~Process() {`
`27`	`27`	`}`
`28`	`28`	`}`
`29`	`29`
`30`		`-austin_handle_t Process::get_austin_handle() {`
	`30`	`+austin_handle_t Process::get_austin_handle(pid_t tid) {`
`31`	`31`	`if (!_austin_handle) {`
	`32`	`+ LG_NTC("Attaching to PID %d/%d", _pid, tid);`
`32`	`33`	`_austin_handle = austin_attach(_pid);`
	`34`	`+ LG_NTC("Result of attach %p", _austin_handle);`
`33`	`35`	`}`
`34`	`36`	`return _austin_handle;`
`35`	`37`	`}`