update code as gabs suggestion

ddtrace/internal/datadog/profiling/stack_v2/include/constants.hpp

@@ -8,8 +8,3 @@ constexpr double g_default_sampling_period_s = g_default_sampling_period_us / 1e
 
 // Echion maintains a cache of frames--the size of this cache is specified up-front.
 constexpr unsigned int g_default_echion_frame_cache_size = 1024;
-
-// Default number of times we need to not see a thread id before we consider
-// it to be inactive and remove it from ThreadSpanLinks. This is to prevent
-// ThreadSpanLinks from clearing unseen threads that are still active.
-constexpr unsigned int g_default_unseen_count = 3;

ddtrace/internal/datadog/profiling/stack_v2/include/thread_span_links.hpp

@@ -1,7 +1,5 @@
 #pragma once
 
-#include "constants.hpp"
-
 #include <memory>
 #include <mutex>
 #include <optional>
@@ -17,14 +15,12 @@ struct Span
     uint64_t span_id;
     uint64_t local_root_span_id;
     std::string span_type;
-    unsigned int unseen_count;
 
     Span(uint64_t _span_id, uint64_t _local_root_span_id, std::string _span_type)
       : span_id(_span_id)
       , local_root_span_id(_local_root_span_id)
       , span_type(_span_type)
     {
-        unseen_count = g_default_unseen_count;
     }
 
     // for testing
@@ -50,7 +46,7 @@ class ThreadSpanLinks
 
     void link_span(uint64_t thread_id, uint64_t span_id, uint64_t local_root_span_id, std::string span_type);
     const std::optional<Span> get_active_span_from_thread_id(uint64_t thread_id);
-    void clear_unseen(const std::unordered_set<uint64_t>& seen_native_thread_ids);
+    void unlink_span(uint64_t thread_id);
     void reset();
 
     static void postfork_child();

ddtrace/internal/datadog/profiling/stack_v2/src/stack_v2.cpp

@@ -86,6 +86,7 @@ stack_v2_thread_unregister(PyObject* self, PyObject* args)
     }
 
     Sampler::get().unregister_thread(id);
+    ThreadSpanLinks::get_instance().unlink_span(id);
     Py_RETURN_NONE;
 }
 

ddtrace/internal/datadog/profiling/stack_v2/src/thread_span_links.cpp

@@ -19,7 +19,6 @@ ThreadSpanLinks::link_span(uint64_t thread_id, uint64_t span_id, uint64_t local_
         it->second->span_id = span_id;
         it->second->local_root_span_id = local_root_span_id;
         it->second->span_type = span_type;
-        it->second->unseen_count = g_default_unseen_count;
     }
 }
 
@@ -37,25 +36,11 @@ ThreadSpanLinks::get_active_span_from_thread_id(uint64_t thread_id)
 }
 
 void
-ThreadSpanLinks::clear_unseen(const std::unordered_set<uint64_t>& seen_native_thread_ids)
+ThreadSpanLinks::unlink_span(uint64_t thread_id)
 {
-    // DEV: Note that this way of clearing the unseen threads could lead to
-    // missing some threads that are still active but not seen in current set
-    // of samples.
     std::lock_guard<std::mutex> lock(mtx);
 
-    for (auto it = thread_id_to_span.begin(); it != thread_id_to_span.end();) {
-        if (seen_native_thread_ids.find(it->first) == seen_native_thread_ids.end()) {
-            it->second->unseen_count--;
-            if (it->second->unseen_count == 0) {
-                it = thread_id_to_span.erase(it);
-            } else {
-                ++it;
-            }
-        } else {
-            ++it;
-        }
-    }
+    thread_id_to_span.erase(thread_id); // This is a no-op if the key is not found
 }
 
 void

ddtrace/internal/datadog/profiling/stack_v2/test/CMakeLists.txt

@@ -14,7 +14,7 @@ include(AnalysisFunc)
 
 function(dd_wrapper_add_test name)
     add_executable(${name} ${ARGN})
-    target_include_directories(${name} PRIVATE ../include ../..)
+    target_include_directories(${name} PRIVATE ../include)
     target_link_libraries(${name} PRIVATE gmock gtest_main _stack_v2)
     add_ddup_config(${name})
 

ddtrace/internal/datadog/profiling/stack_v2/test/thread_span_links.cpp

@@ -1,7 +1,5 @@
 #include "thread_span_links.hpp"
 
-#include "constants.hpp"
-
 #include <gmock/gmock.h>
 #include <gtest/gtest.h>
 
@@ -45,7 +43,7 @@ TEST(ThreadSpanLinksConcurrency, GetSetRace)
     t2.join();
 }
 
-TEST(ThreadSpanLinks, ClearUnSeen)
+TEST(ThreadSpanLinks, ClearFinished)
 {
     unsigned int num_thread_ids = 100;
     std::unordered_set<uint64_t> thread_ids;
@@ -64,25 +62,21 @@ TEST(ThreadSpanLinks, ClearUnSeen)
         Datadog::ThreadSpanLinks::get_instance().link_span(thread_id, thread_id, thread_id, "test");
     }
 
-    // Randomly select some of the to be seen with p = 0.5
-    std::unordered_set<uint64_t> seen_thread_ids;
+    std::unordered_set<uint64_t> finished_threads;
     std::uniform_real_distribution<double> real_dis(0, 1);
 
     for (auto thread_id : thread_ids) {
         if (real_dis(gen) < 0.5) {
-            seen_thread_ids.insert(thread_id);
+            finished_threads.insert(thread_id);
+            Datadog::ThreadSpanLinks::get_instance().unlink_span(thread_id);
         }
     }
 
-    for (unsigned int i = 0; i < g_default_unseen_count; ++i) {
-        Datadog::ThreadSpanLinks::get_instance().clear_unseen(seen_thread_ids);
-    }
-
     // Check that the unseen ids are removed
     for (auto thread_id : thread_ids) {
         std::optional<Datadog::Span> span_opt =
           Datadog::ThreadSpanLinks::get_instance().get_active_span_from_thread_id(thread_id);
-        if (seen_thread_ids.find(thread_id) != seen_thread_ids.end()) {
+        if (finished_threads.find(thread_id) == finished_threads.end()) {
             EXPECT_EQ(span_opt, Datadog::Span(thread_id, thread_id, "test"));
 
         } else {