https://github.com/oontvoo updated https://github.com/llvm/llvm-project/pull/127696
>From 24e9f78744f98ecf3ac01f1f719f1eac9b3479f0 Mon Sep 17 00:00:00 2001 From: Vy Nguyen <v...@google.com> Date: Tue, 18 Feb 2025 15:58:08 -0500 Subject: [PATCH 01/16] [LLDB][Telemetry]Define DebuggerTelemetryInfo and related methods - This type of entry is used to collect data about the debugger startup/exit - Tests will be added (They may need to be shell test with a "test-only" TelemetryManager plugin defined. I'm trying to figure out how to get that linked only when tests are running and not to the LLDB binary all the time. --- lldb/include/lldb/Core/Telemetry.h | 78 +++++++++++++++++++ lldb/source/Core/Debugger.cpp | 40 ++++++++++ lldb/source/Core/Telemetry.cpp | 115 +++++++++++++++++++++++++---- 3 files changed, 220 insertions(+), 13 deletions(-) diff --git a/lldb/include/lldb/Core/Telemetry.h b/lldb/include/lldb/Core/Telemetry.h index b72556ecaf3c9..d6eec5dc687be 100644 --- a/lldb/include/lldb/Core/Telemetry.h +++ b/lldb/include/lldb/Core/Telemetry.h @@ -13,6 +13,7 @@ #include "lldb/Interpreter/CommandReturnObject.h" #include "lldb/Utility/StructuredData.h" #include "lldb/lldb-forward.h" +#include "llvm/ADT/DenseMap.h" #include "llvm/ADT/StringExtras.h" #include "llvm/ADT/StringRef.h" #include "llvm/Support/JSON.h" @@ -29,6 +30,9 @@ namespace telemetry { struct LLDBEntryKind : public ::llvm::telemetry::EntryKind { static const llvm::telemetry::KindType BaseInfo = 0b11000; + static const llvm::telemetry::KindType DebuggerInfo = 0b11001; + // There are other entries in between (added in separate PRs) + static const llvm::telemetry::KindType MiscInfo = 0b11110; }; /// Defines a convenient type for timestamp of various events. @@ -56,6 +60,71 @@ struct LLDBBaseTelemetryInfo : public llvm::telemetry::TelemetryInfo { void serialize(llvm::telemetry::Serializer &serializer) const override; }; +/// Describes the exit status of a debugger. +struct ExitDescription { + int exit_code; + std::string description; +}; + +struct DebuggerTelemetryInfo : public LLDBBaseTelemetryInfo { + std::string username; + std::string lldb_git_sha; + std::string lldb_path; + std::string cwd; + std::optional<ExitDescription> exit_desc; + + DebuggerTelemetryInfo() = default; + + // Provide a copy ctor because we may need to make a copy before + // sanitizing the data. + // (The sanitization might differ between different Destination classes). + DebuggerTelemetryInfo(const DebuggerTelemetryInfo &other) { + username = other.username; + lldb_git_sha = other.lldb_git_sha; + lldb_path = other.lldb_path; + cwd = other.cwd; + }; + + llvm::telemetry::KindType getKind() const override { + return LLDBEntryKind::DebuggerInfo; + } + + static bool classof(const llvm::telemetry::TelemetryInfo *T) { + return T->getKind() == LLDBEntryKind::DebuggerInfo; + } + + void serialize(llvm::telemetry::Serializer &serializer) const override; +}; + +/// The "catch-all" entry to store a set of non-standard data, such as +/// error-messages, etc. +struct MiscTelemetryInfo : public LLDBBaseTelemetryInfo { + /// If the event is/can be associated with a target entry, + /// this field contains that target's UUID. + /// <EMPTY> otherwise. + std::string target_uuid; + + /// Set of key-value pairs for any optional (or impl-specific) data + std::map<std::string, std::string> meta_data; + + MiscTelemetryInfo() = default; + + MiscTelemetryInfo(const MiscTelemetryInfo &other) { + target_uuid = other.target_uuid; + meta_data = other.meta_data; + } + + llvm::telemetry::KindType getKind() const override { + return LLDBEntryKind::MiscInfo; + } + + static bool classof(const llvm::telemetry::TelemetryInfo *T) { + return T->getKind() == LLDBEntryKind::MiscInfo; + } + + void serialize(llvm::telemetry::Serializer &serializer) const override; +}; + /// The base Telemetry manager instance in LLDB. /// This class declares additional instrumentation points /// applicable to LLDB. @@ -63,6 +132,11 @@ class TelemetryManager : public llvm::telemetry::Manager { public: llvm::Error preDispatch(llvm::telemetry::TelemetryInfo *entry) override; + const llvm::telemetry::Config *getConfig(); + + void atDebuggerStartup(DebuggerTelemetryInfo *entry); + void atDebuggerExit(DebuggerTelemetryInfo *entry); + virtual llvm::StringRef GetInstanceName() const = 0; static TelemetryManager *getInstance(); @@ -73,6 +147,10 @@ class TelemetryManager : public llvm::telemetry::Manager { private: std::unique_ptr<llvm::telemetry::Config> m_config; + // Each debugger is assigned a unique ID (session_id). + // All TelemetryInfo entries emitted for the same debugger instance + // will get the same session_id. + llvm::DenseMap<Debugger *, std::string> session_ids; static std::unique_ptr<TelemetryManager> g_instance; }; diff --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp index 18569e155b517..b458abc798a9e 100644 --- a/lldb/source/Core/Debugger.cpp +++ b/lldb/source/Core/Debugger.cpp @@ -62,6 +62,7 @@ #include "llvm/ADT/STLExtras.h" #include "llvm/ADT/StringRef.h" #include "llvm/ADT/iterator.h" +#include "llvm/Config/llvm-config.h" #include "llvm/Support/DynamicLibrary.h" #include "llvm/Support/FileSystem.h" #include "llvm/Support/Process.h" @@ -69,6 +70,11 @@ #include "llvm/Support/Threading.h" #include "llvm/Support/raw_ostream.h" +#ifdef LLVM_BUILD_TELEMETRY +#include "lldb/Core/Telemetry.h" +#include <chrono> +#endif + #include <cstdio> #include <cstdlib> #include <cstring> @@ -761,12 +767,29 @@ void Debugger::InstanceInitialize() { DebuggerSP Debugger::CreateInstance(lldb::LogOutputCallback log_callback, void *baton) { +#ifdef LLVM_BUILD_TELEMETRY + lldb_private::telemetry::SteadyTimePoint start_time = + std::chrono::steady_clock::now(); +#endif DebuggerSP debugger_sp(new Debugger(log_callback, baton)); if (g_debugger_list_ptr && g_debugger_list_mutex_ptr) { std::lock_guard<std::recursive_mutex> guard(*g_debugger_list_mutex_ptr); g_debugger_list_ptr->push_back(debugger_sp); } debugger_sp->InstanceInitialize(); + +#ifdef LLVM_BUILD_TELEMETRY + if (auto *telemetry_manager = telemetry::TelemetryManager::getInstance()) { + if (telemetry_manager->getConfig()->EnableTelemetry) { + lldb_private::telemetry::DebuggerTelemetryInfo entry; + entry.start_time = start_time; + entry.end_time = std::chrono::steady_clock::now(); + entry.debugger = debugger_sp.get(); + telemetry_manager->atDebuggerStartup(&entry); + } + } +#endif + return debugger_sp; } @@ -985,6 +1008,10 @@ void Debugger::Clear() { // static void Debugger::Destroy(lldb::DebuggerSP &debugger_sp); // static void Debugger::Terminate(); llvm::call_once(m_clear_once, [this]() { +#ifdef LLVM_BUILD_TELEMETRY + lldb_private::telemetry::SteadyTimePoint quit_start_time = + std::chrono::steady_clock::now(); +#endif ClearIOHandlers(); StopIOHandlerThread(); StopEventHandlerThread(); @@ -1007,6 +1034,19 @@ void Debugger::Clear() { if (Diagnostics::Enabled()) Diagnostics::Instance().RemoveCallback(m_diagnostics_callback_id); + +#ifdef LLVM_BUILD_TELEMETRY + if (auto telemetry_manager = telemetry::TelemetryManager::getInstance()) { + if (telemetry_manager->getConfig()->EnableTelemetry) { + // TBD: We *may* have to send off the log BEFORE the ClearIOHanders()? + lldb_private::telemetry::DebuggerTelemetryInfo entry; + entry.start_time = quit_start_time; + entry.end_time = std::chrono::steady_clock::now(); + entry.debugger = this; + telemetry_manager->atDebuggerExit(&entry); + } + } +#endif }); } diff --git a/lldb/source/Core/Telemetry.cpp b/lldb/source/Core/Telemetry.cpp index 5222f76704f91..ab7d8ae0b44df 100644 --- a/lldb/source/Core/Telemetry.cpp +++ b/lldb/source/Core/Telemetry.cpp @@ -10,14 +10,20 @@ #ifdef LLVM_BUILD_TELEMETRY -#include "lldb/Core/Telemetry.h" #include "lldb/Core/Debugger.h" +#include "lldb/Core/Telemetry.h" +#include "lldb/Host/FileSystem.h" +#include "lldb/Host/HostInfo.h" +#include "lldb/Target/Process.h" +#include "lldb/Target/Statistics.h" #include "lldb/Utility/LLDBLog.h" #include "lldb/Utility/UUID.h" +#include "lldb/Version/Version.h" #include "lldb/lldb-enumerations.h" #include "lldb/lldb-forward.h" #include "llvm/ADT/StringRef.h" #include "llvm/Support/Error.h" +#include "llvm/Support/Path.h" #include "llvm/Support/RandomNumberGenerator.h" #include "llvm/Telemetry/Telemetry.h" #include <chrono> @@ -35,15 +41,7 @@ static uint64_t ToNanosec(const SteadyTimePoint Point) { return std::chrono::nanoseconds(Point.time_since_epoch()).count(); } -void LLDBBaseTelemetryInfo::serialize(Serializer &serializer) const { - serializer.write("entry_kind", getKind()); - serializer.write("session_id", SessionId); - serializer.write("start_time", ToNanosec(start_time)); - if (end_time.has_value()) - serializer.write("end_time", ToNanosec(end_time.value())); -} - -[[maybe_unused]] static std::string MakeUUID(Debugger *debugger) { +static std::string MakeUUID(Debugger *debugger) { uint8_t random_bytes[16]; if (auto ec = llvm::getRandomBytes(random_bytes, 16)) { LLDB_LOG(GetLog(LLDBLog::Object), @@ -56,16 +54,107 @@ void LLDBBaseTelemetryInfo::serialize(Serializer &serializer) const { return UUID(random_bytes).GetAsString(); } +void LLDBBaseTelemetryInfo::serialize(Serializer &serializer) const { + serializer.write("entry_kind", getKind()); + serializer.write("session_id", SessionId); + serializer.write("start_time", ToNanosec(start_time)); + if (end_time.has_value()) + serializer.write("end_time", ToNanosec(end_time.value())); +} + +void DebuggerTelemetryInfo::serialize(Serializer &serializer) const { + LLDBBaseTelemetryInfo::serialize(serializer); + + serializer.write("username", username); + serializer.write("lldb_git_sha", lldb_git_sha); + serializer.write("lldb_path", lldb_path); + serializer.write("cwd", cwd); + if (exit_desc.has_value()) { + serializer.write("exit_code", exit_desc->exit_code); + serializer.write("exit_desc", exit_desc->description); + } +} + +void MiscTelemetryInfo::serialize(Serializer &serializer) const { + LLDBBaseTelemetryInfo::serialize(serializer); + serializer.write("target_uuid", target_uuid); + serializer.beginObject("meta_data"); + for (const auto &kv : meta_data) + serializer.write(kv.first, kv.second); + serializer.endObject(); +} + TelemetryManager::TelemetryManager(std::unique_ptr<Config> config) : m_config(std::move(config)) {} llvm::Error TelemetryManager::preDispatch(TelemetryInfo *entry) { - // Do nothing for now. - // In up-coming patch, this would be where the manager - // attach the session_uuid to the entry. + LLDBBaseTelemetryInfo *lldb_entry = + llvm::dyn_cast<LLDBBaseTelemetryInfo>(entry); + std::string session_id = ""; + if (Debugger *debugger = lldb_entry->debugger) { + auto session_id_pos = session_ids.find(debugger); + if (session_id_pos != session_ids.end()) + session_id = session_id_pos->second; + else + session_id_pos->second = session_id = MakeUUID(debugger); + } + lldb_entry->SessionId = session_id; + return llvm::Error::success(); } +const Config *getConfig() { return m_config.get(); } + +void TelemetryManager::atDebuggerStartup(DebuggerTelemetryInfo *entry) { + UserIDResolver &resolver = lldb_private::HostInfo::GetUserIDResolver(); + std::optional<llvm::StringRef> opt_username = + resolver.GetUserName(lldb_private::HostInfo::GetUserID()); + if (opt_username) + entry->username = *opt_username; + + entry->lldb_git_sha = + lldb_private::GetVersion(); // TODO: find the real git sha? + + entry->lldb_path = HostInfo::GetProgramFileSpec().GetPath(); + + llvm::SmallString<64> cwd; + if (!llvm::sys::fs::current_path(cwd)) { + entry->cwd = cwd.c_str(); + } else { + MiscTelemetryInfo misc_info; + misc_info.meta_data["internal_errors"] = "Cannot determine CWD"; + if (auto er = dispatch(&misc_info)) { + LLDB_LOG(GetLog(LLDBLog::Object), + "Failed to dispatch misc-info at startup"); + } + } + + if (auto er = dispatch(entry)) { + LLDB_LOG(GetLog(LLDBLog::Object), "Failed to dispatch entry at startup"); + } +} + +void TelemetryManager::atDebuggerExit(DebuggerTelemetryInfo *entry) { + // There must be a reference to the debugger at this point. + assert(entry->debugger != nullptr); + + if (auto *selected_target = + entry->debugger->GetSelectedExecutionContext().GetTargetPtr()) { + if (!selected_target->IsDummyTarget()) { + const lldb::ProcessSP proc = selected_target->GetProcessSP(); + if (proc == nullptr) { + // no process has been launched yet. + entry->exit_desc = {-1, "no process launched."}; + } else { + entry->exit_desc = {proc->GetExitStatus(), ""}; + if (const char *description = proc->GetExitDescription()) + entry->exit_desc->description = std::string(description); + } + } + } + dispatch(entry); +} + std::unique_ptr<TelemetryManager> TelemetryManager::g_instance = nullptr; TelemetryManager *TelemetryManager::getInstance() { return g_instance.get(); } >From 496c370bffd3a518eda4f8d34aabad13356831b3 Mon Sep 17 00:00:00 2001 From: Vy Nguyen <v...@google.com> Date: Tue, 18 Feb 2025 20:51:23 -0500 Subject: [PATCH 02/16] Update lldb/include/lldb/Core/Telemetry.h Co-authored-by: Jonas Devlieghere <jo...@devlieghere.com> --- lldb/include/lldb/Core/Telemetry.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lldb/include/lldb/Core/Telemetry.h b/lldb/include/lldb/Core/Telemetry.h index d6eec5dc687be..9d34bf205c607 100644 --- a/lldb/include/lldb/Core/Telemetry.h +++ b/lldb/include/lldb/Core/Telemetry.h @@ -105,7 +105,7 @@ struct MiscTelemetryInfo : public LLDBBaseTelemetryInfo { std::string target_uuid; /// Set of key-value pairs for any optional (or impl-specific) data - std::map<std::string, std::string> meta_data; + llvm::StringMap<std::string> meta_data; MiscTelemetryInfo() = default; >From 11efc09e1ab9ffcca8e965906530af995f5c8c83 Mon Sep 17 00:00:00 2001 From: Vy Nguyen <v...@google.com> Date: Wed, 19 Feb 2025 14:23:00 -0500 Subject: [PATCH 03/16] Update lldb/source/Core/Telemetry.cpp Co-authored-by: Pavel Labath <pa...@labath.sk> --- lldb/source/Core/Telemetry.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lldb/source/Core/Telemetry.cpp b/lldb/source/Core/Telemetry.cpp index ab7d8ae0b44df..2e86d793c5ab3 100644 --- a/lldb/source/Core/Telemetry.cpp +++ b/lldb/source/Core/Telemetry.cpp @@ -123,9 +123,9 @@ void TelemetryManager::atDebuggerStartup(DebuggerTelemetryInfo *entry) { } else { MiscTelemetryInfo misc_info; misc_info.meta_data["internal_errors"] = "Cannot determine CWD"; - if (auto er = dispatch(&misc_info)) { - LLDB_LOG(GetLog(LLDBLog::Object), - "Failed to dispatch misc-info at startup"); + if (llvm::Error er = dispatch(&misc_info)) { + LLDB_LOG_ERROR(GetLog(LLDBLog::Object), std::move(er), + "Failed to dispatch misc-info at startup: {0}"); } } >From 5aec2845cee08ada1a5ca7da7ff901ea9a1017d8 Mon Sep 17 00:00:00 2001 From: Vy Nguyen <v...@google.com> Date: Thu, 20 Feb 2025 15:51:06 -0500 Subject: [PATCH 04/16] adddressed review comments: - remove collection of username,cwd,lldb_path (that can be collected in vendor-specific impl) - use DebuggerID as key rather than the debugger pointer - renaming --- lldb/include/lldb/Core/Telemetry.h | 32 ++++++-------- lldb/source/Core/Telemetry.cpp | 67 ++++++++++++++---------------- 2 files changed, 44 insertions(+), 55 deletions(-) diff --git a/lldb/include/lldb/Core/Telemetry.h b/lldb/include/lldb/Core/Telemetry.h index 9d34bf205c607..1fb2c5b6b7eeb 100644 --- a/lldb/include/lldb/Core/Telemetry.h +++ b/lldb/include/lldb/Core/Telemetry.h @@ -66,24 +66,15 @@ struct ExitDescription { std::string description; }; -struct DebuggerTelemetryInfo : public LLDBBaseTelemetryInfo { - std::string username; - std::string lldb_git_sha; - std::string lldb_path; - std::string cwd; +struct DebuggerInfo : public LLDBBaseTelemetryInfo { + std::string lldb_version; std::optional<ExitDescription> exit_desc; - DebuggerTelemetryInfo() = default; + std::string lldb_path; + std::string cwd; + std::string username; - // Provide a copy ctor because we may need to make a copy before - // sanitizing the data. - // (The sanitization might differ between different Destination classes). - DebuggerTelemetryInfo(const DebuggerTelemetryInfo &other) { - username = other.username; - lldb_git_sha = other.lldb_git_sha; - lldb_path = other.lldb_path; - cwd = other.cwd; - }; + DebuggerInfo() = default; llvm::telemetry::KindType getKind() const override { return LLDBEntryKind::DebuggerInfo; @@ -134,8 +125,8 @@ class TelemetryManager : public llvm::telemetry::Manager { const llvm::telemetry::Config *getConfig(); - void atDebuggerStartup(DebuggerTelemetryInfo *entry); - void atDebuggerExit(DebuggerTelemetryInfo *entry); + virtual void AtDebuggerStartup(DebuggerInfo *entry); + virtual void AtDebuggerExit(DebuggerInfo *entry); virtual llvm::StringRef GetInstanceName() const = 0; static TelemetryManager *getInstance(); @@ -147,10 +138,13 @@ class TelemetryManager : public llvm::telemetry::Manager { private: std::unique_ptr<llvm::telemetry::Config> m_config; - // Each debugger is assigned a unique ID (session_id). + // Each instance of a TelemetryManager is assigned a unique ID. + const std::string m_id; + + // Map of debugger's ID to a unique session_id string. // All TelemetryInfo entries emitted for the same debugger instance // will get the same session_id. - llvm::DenseMap<Debugger *, std::string> session_ids; + llvm::DenseMap<lldb::user_id_t, std::string> session_ids; static std::unique_ptr<TelemetryManager> g_instance; }; diff --git a/lldb/source/Core/Telemetry.cpp b/lldb/source/Core/Telemetry.cpp index 2e86d793c5ab3..9ae5f9c4e8243 100644 --- a/lldb/source/Core/Telemetry.cpp +++ b/lldb/source/Core/Telemetry.cpp @@ -41,17 +41,32 @@ static uint64_t ToNanosec(const SteadyTimePoint Point) { return std::chrono::nanoseconds(Point.time_since_epoch()).count(); } -static std::string MakeUUID(Debugger *debugger) { +// Generate a unique string. This should be unique across different runs. +// We build such string by combining three parts: +// <16 random bytes>_<timestamp>_<hash of username> +// This reduces the chances of getting the same UUID, even when the same +// user runs the two copies of binary at the same time. +static std::string MakeUUID() { uint8_t random_bytes[16]; + std::string randomString = "_"; if (auto ec = llvm::getRandomBytes(random_bytes, 16)) { LLDB_LOG(GetLog(LLDBLog::Object), "Failed to generate random bytes for UUID: {0}", ec.message()); - // Fallback to using timestamp + debugger ID. - return llvm::formatv( - "{0}_{1}", std::chrono::steady_clock::now().time_since_epoch().count(), - debugger->GetID()); + } else { + randomString = UUID(random_bytes).GetAsString(); } - return UUID(random_bytes).GetAsString(); + + std::string username = "_"; + UserIDResolver &resolver = lldb_private::HostInfo::GetUserIDResolver(); + std::optional<llvm::StringRef> opt_username = + resolver.GetUserName(lldb_private::HostInfo::GetUserID()); + if (opt_username) + username = *opt_username; + + return llvm::formatv( + "{0}_{1}_{2}", randomString, + std::chrono::steady_clock::now().time_since_epoch().count(), + llvm::MD5Hash(username)); } void LLDBBaseTelemetryInfo::serialize(Serializer &serializer) const { @@ -62,7 +77,7 @@ void LLDBBaseTelemetryInfo::serialize(Serializer &serializer) const { serializer.write("end_time", ToNanosec(end_time.value())); } -void DebuggerTelemetryInfo::serialize(Serializer &serializer) const { +void DebuggerInfo::serialize(Serializer &serializer) const { LLDBBaseTelemetryInfo::serialize(serializer); serializer.write("username", username); @@ -85,18 +100,21 @@ void MiscTelemetryInfo::serialize(Serializer &serializer) const { } TelemetryManager::TelemetryManager(std::unique_ptr<Config> config) - : m_config(std::move(config)) {} + : m_config(std::move(config)), m_id(MakeUUID) {} llvm::Error TelemetryManager::preDispatch(TelemetryInfo *entry) { + // Look up the session_id to assign to this entry or make one + // if none had been computed for this debugger. LLDBBaseTelemetryInfo *lldb_entry = llvm::dyn_cast<LLDBBaseTelemetryInfo>(entry); - std::string session_id = ""; + std::string session_id = m_id; if (Debugger *debugger = lldb_entry->debugger) { - auto session_id_pos = session_ids.find(debugger); + auto session_id_pos = session_ids.find(debugger->getID()); if (session_id_pos != session_ids.end()) session_id = session_id_pos->second; else - session_id_pos->second = session_id = MakeUUID(debugger); + session_id_pos->second = session_id = + llvm::formatv("{0}_{1}", m_id, debugger->getID()); } lldb_entry->SessionId = session_id; @@ -105,36 +123,13 @@ llvm::Error TelemetryManager::preDispatch(TelemetryInfo *entry) { const Config *getConfig() { return m_config.get(); } -void TelemetryManager::atDebuggerStartup(DebuggerTelemetryInfo *entry) { - UserIDResolver &resolver = lldb_private::HostInfo::GetUserIDResolver(); - std::optional<llvm::StringRef> opt_username = - resolver.GetUserName(lldb_private::HostInfo::GetUserID()); - if (opt_username) - entry->username = *opt_username; - - entry->lldb_git_sha = - lldb_private::GetVersion(); // TODO: find the real git sha? - - entry->lldb_path = HostInfo::GetProgramFileSpec().GetPath(); - - llvm::SmallString<64> cwd; - if (!llvm::sys::fs::current_path(cwd)) { - entry->cwd = cwd.c_str(); - } else { - MiscTelemetryInfo misc_info; - misc_info.meta_data["internal_errors"] = "Cannot determine CWD"; - if (llvm::Error er = dispatch(&misc_info)) { - LLDB_LOG_ERROR(GetLog(LLDBLog::Object), std::move(er), - "Failed to dispatch misc-info at startup: {0}"); - } - } - +void TelemetryManager::AtDebuggerStartup(DebuggerInfo *entry) { if (auto er = dispatch(entry)) { LLDB_LOG(GetLog(LLDBLog::Object), "Failed to dispatch entry at startup"); } } -void TelemetryManager::atDebuggerExit(DebuggerTelemetryInfo *entry) { +void TelemetryManager::AtDebuggerExit(DebuggerInfo *entry) { // There must be a reference to the debugger at this point. assert(entry->debugger != nullptr); >From 4b6a4bdf158dd631baec5768fb44719b8e8bf100 Mon Sep 17 00:00:00 2001 From: Vy Nguyen <v...@google.com> Date: Mon, 24 Feb 2025 13:39:35 -0500 Subject: [PATCH 05/16] remove unused fields --- lldb/include/lldb/Core/Telemetry.h | 38 +----------- lldb/source/Core/Telemetry.cpp | 99 ++++++++++++------------------ 2 files changed, 41 insertions(+), 96 deletions(-) diff --git a/lldb/include/lldb/Core/Telemetry.h b/lldb/include/lldb/Core/Telemetry.h index 1fb2c5b6b7eeb..79351b8f8e37c 100644 --- a/lldb/include/lldb/Core/Telemetry.h +++ b/lldb/include/lldb/Core/Telemetry.h @@ -45,6 +45,7 @@ struct LLDBBaseTelemetryInfo : public llvm::telemetry::TelemetryInfo { std::optional<SteadyTimePoint> end_time; // TBD: could add some memory stats here too? + uint64_t debugger_id = 0; Debugger *debugger; // For dyn_cast, isa, etc operations. @@ -70,10 +71,6 @@ struct DebuggerInfo : public LLDBBaseTelemetryInfo { std::string lldb_version; std::optional<ExitDescription> exit_desc; - std::string lldb_path; - std::string cwd; - std::string username; - DebuggerInfo() = default; llvm::telemetry::KindType getKind() const override { @@ -87,35 +84,6 @@ struct DebuggerInfo : public LLDBBaseTelemetryInfo { void serialize(llvm::telemetry::Serializer &serializer) const override; }; -/// The "catch-all" entry to store a set of non-standard data, such as -/// error-messages, etc. -struct MiscTelemetryInfo : public LLDBBaseTelemetryInfo { - /// If the event is/can be associated with a target entry, - /// this field contains that target's UUID. - /// <EMPTY> otherwise. - std::string target_uuid; - - /// Set of key-value pairs for any optional (or impl-specific) data - llvm::StringMap<std::string> meta_data; - - MiscTelemetryInfo() = default; - - MiscTelemetryInfo(const MiscTelemetryInfo &other) { - target_uuid = other.target_uuid; - meta_data = other.meta_data; - } - - llvm::telemetry::KindType getKind() const override { - return LLDBEntryKind::MiscInfo; - } - - static bool classof(const llvm::telemetry::TelemetryInfo *T) { - return T->getKind() == LLDBEntryKind::MiscInfo; - } - - void serialize(llvm::telemetry::Serializer &serializer) const override; -}; - /// The base Telemetry manager instance in LLDB. /// This class declares additional instrumentation points /// applicable to LLDB. @@ -141,10 +109,6 @@ class TelemetryManager : public llvm::telemetry::Manager { // Each instance of a TelemetryManager is assigned a unique ID. const std::string m_id; - // Map of debugger's ID to a unique session_id string. - // All TelemetryInfo entries emitted for the same debugger instance - // will get the same session_id. - llvm::DenseMap<lldb::user_id_t, std::string> session_ids; static std::unique_ptr<TelemetryManager> g_instance; }; diff --git a/lldb/source/Core/Telemetry.cpp b/lldb/source/Core/Telemetry.cpp index 9ae5f9c4e8243..74dd8bf6f7e09 100644 --- a/lldb/source/Core/Telemetry.cpp +++ b/lldb/source/Core/Telemetry.cpp @@ -43,7 +43,7 @@ static uint64_t ToNanosec(const SteadyTimePoint Point) { // Generate a unique string. This should be unique across different runs. // We build such string by combining three parts: -// <16 random bytes>_<timestamp>_<hash of username> +// <16 random bytes>_<timestamp> // This reduces the chances of getting the same UUID, even when the same // user runs the two copies of binary at the same time. static std::string MakeUUID() { @@ -56,17 +56,9 @@ static std::string MakeUUID() { randomString = UUID(random_bytes).GetAsString(); } - std::string username = "_"; - UserIDResolver &resolver = lldb_private::HostInfo::GetUserIDResolver(); - std::optional<llvm::StringRef> opt_username = - resolver.GetUserName(lldb_private::HostInfo::GetUserID()); - if (opt_username) - username = *opt_username; - return llvm::formatv( - "{0}_{1}_{2}", randomString, - std::chrono::steady_clock::now().time_since_epoch().count(), - llvm::MD5Hash(username)); + "{0}_{1}", randomString, + std::chrono::steady_clock::now().time_since_epoch().count()); } void LLDBBaseTelemetryInfo::serialize(Serializer &serializer) const { @@ -90,33 +82,16 @@ void DebuggerInfo::serialize(Serializer &serializer) const { } } -void MiscTelemetryInfo::serialize(Serializer &serializer) const { - LLDBBaseTelemetryInfo::serialize(serializer); - serializer.write("target_uuid", target_uuid); - serializer.beginObject("meta_data"); - for (const auto &kv : meta_data) - serializer.write(kv.first, kv.second); - serializer.endObject(); -} - TelemetryManager::TelemetryManager(std::unique_ptr<Config> config) : m_config(std::move(config)), m_id(MakeUUID) {} llvm::Error TelemetryManager::preDispatch(TelemetryInfo *entry) { - // Look up the session_id to assign to this entry or make one - // if none had been computed for this debugger. + // Assign the manager_id, and debugger_id, if available, to this entry. LLDBBaseTelemetryInfo *lldb_entry = llvm::dyn_cast<LLDBBaseTelemetryInfo>(entry); - std::string session_id = m_id; - if (Debugger *debugger = lldb_entry->debugger) { - auto session_id_pos = session_ids.find(debugger->getID()); - if (session_id_pos != session_ids.end()) - session_id = session_id_pos->second; - else - session_id_pos->second = session_id = - llvm::formatv("{0}_{1}", m_id, debugger->getID()); - } - lldb_entry->SessionId = session_id; + lldb_entry->SessionId = m_id; + if (Debugger *debugger = lldb_entry->debugger) + lldb_entry->debugger_id = debugger->GetID(); return llvm::Error::success(); } @@ -124,40 +99,46 @@ llvm::Error TelemetryManager::preDispatch(TelemetryInfo *entry) { const Config *getConfig() { return m_config.get(); } void TelemetryManager::AtDebuggerStartup(DebuggerInfo *entry) { - if (auto er = dispatch(entry)) { - LLDB_LOG(GetLog(LLDBLog::Object), "Failed to dispatch entry at startup"); + if (llvm::Error er = dispatch(entry)) { + LLDB_LOG_ERROR(GetLog(LLDBLog::Object), std::move(er), + "Failed to dispatch entry at debugger startup: {0}"); } -} -void TelemetryManager::AtDebuggerExit(DebuggerInfo *entry) { - // There must be a reference to the debugger at this point. - assert(entry->debugger != nullptr); - - if (auto *selected_target = - entry->debugger->GetSelectedExecutionContext().GetTargetPtr()) { - if (!selected_target->IsDummyTarget()) { - const lldb::ProcessSP proc = selected_target->GetProcessSP(); - if (proc == nullptr) { - // no process has been launched yet. - entry->exit_desc = {-1, "no process launched."}; - } else { - entry->exit_desc = {proc->GetExitStatus(), ""}; - if (const char *description = proc->GetExitDescription()) - entry->exit_desc->description = std::string(description); + void TelemetryManager::AtDebuggerExit(DebuggerInfo * entry) { + // There must be a reference to the debugger at this point. + assert(entry->debugger != nullptr); + + if (auto *selected_target = + entry->debugger->GetSelectedExecutionContext().GetTargetPtr()) { + if (!selected_target->IsDummyTarget()) { + const lldb::ProcessSP proc = selected_target->GetProcessSP(); + if (proc == nullptr) { + // no process has been launched yet. + entry->exit_desc = {-1, "no process launched."}; + } else { + entry->exit_desc = {proc->GetExitStatus(), ""}; + if (const char *description = proc->GetExitDescription()) + entry->exit_desc->description = std::string(description); + } } } - } - dispatch(entry); -} -std::unique_ptr<TelemetryManager> TelemetryManager::g_instance = nullptr; -TelemetryManager *TelemetryManager::getInstance() { return g_instance.get(); } + if (llvm::Error er = dispatch(entry)) { + LLDB_LOG_ERROR(GetLog(LLDBLog::Object), std::move(er), + "Failed to dispatch entry at debugger exit: {0}"); + } -void TelemetryManager::setInstance(std::unique_ptr<TelemetryManager> manager) { - g_instance = std::move(manager); -} + std::unique_ptr<TelemetryManager> TelemetryManager::g_instance = nullptr; + TelemetryManager *TelemetryManager::getInstance() { + return g_instance.get(); + } + + void TelemetryManager::setInstance( + std::unique_ptr<TelemetryManager> manager) { + g_instance = std::move(manager); + } -} // namespace telemetry + } // namespace telemetry } // namespace lldb_private #endif // LLVM_BUILD_TELEMETRY >From 4dd0782e144c0b3943becf9ca2b62fc8bcfca039 Mon Sep 17 00:00:00 2001 From: Vy Nguyen <v...@google.com> Date: Wed, 26 Feb 2025 14:32:38 -0500 Subject: [PATCH 06/16] addressed review comments --- lldb/include/lldb/Core/Telemetry.h | 41 +++++++++++++++++- lldb/source/Core/Debugger.cpp | 67 +++++++++++++----------------- lldb/source/Core/Telemetry.cpp | 58 +++++++------------------- 3 files changed, 85 insertions(+), 81 deletions(-) diff --git a/lldb/include/lldb/Core/Telemetry.h b/lldb/include/lldb/Core/Telemetry.h index 62b3a3096d472..5c11004854c95 100644 --- a/lldb/include/lldb/Core/Telemetry.h +++ b/lldb/include/lldb/Core/Telemetry.h @@ -23,7 +23,7 @@ #include <memory> #include <optional> #include <string> -#include <unordered_map> +#include <stack> namespace lldb_private { namespace telemetry { @@ -91,11 +91,12 @@ class TelemetryManager : public llvm::telemetry::Manager { public: llvm::Error preDispatch(llvm::telemetry::TelemetryInfo *entry) override; - const llvm::telemetry::Config *getConfig(); virtual void AtDebuggerStartup(DebuggerInfo *entry); virtual void AtDebuggerExit(DebuggerInfo *entry); + const llvm::telemetry::Config *GetConfig(); + virtual llvm::StringRef GetInstanceName() const = 0; static TelemetryManager *GetInstance(); @@ -112,6 +113,42 @@ class TelemetryManager : public llvm::telemetry::Manager { static std::unique_ptr<TelemetryManager> g_instance; }; +/// Helper RAII class for collecting telemetry. +class ScopeTelemetryCollector { + public: + ScopeTelemetryCollector () { + if (TelemetryEnabled()) + m_start = std::chrono::steady_clock::now(); + } + + ~ScopeTelemetryCollector() { + while(! m_exit_funcs.empty()) { + (m_exit_funcs.top())(); + m_exit_funcs.pop(); + } + } + + bool TelemetryEnabled() { + TelemetryManager* instance = TelemetryManager::GetInstance(); + return instance != nullptr && instance->GetConfig()->EnableTelemetry; + } + + + SteadyTimePoint GetStartTime() {return m_start;} + SteadyTimePoint GetCurrentTime() { return std::chrono::steady_clock::now(); } + + template <typename Fp> + void RunAtScopeExit(Fp&& F){ + assert(llvm::telemetry::Config::BuildTimeEnableTelemetry && "Telemetry should have been enabled"); + m_exit_funcs.push(std::forward<Fp>(F)); + } + + private: + SteadyTimePoint m_start; + std::stack<std::function<void()>> m_exit_funcs; + +}; + } // namespace telemetry } // namespace lldb_private #endif // LLDB_CORE_TELEMETRY_H diff --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp index 1fd3b4a6f90c2..750ca3a91ba3d 100644 --- a/lldb/source/Core/Debugger.cpp +++ b/lldb/source/Core/Debugger.cpp @@ -52,6 +52,7 @@ #include "lldb/Utility/State.h" #include "lldb/Utility/Stream.h" #include "lldb/Utility/StreamString.h" +#include "lldb/Core/Telemetry.h" #include "lldb/lldb-enumerations.h" #if defined(_WIN32) @@ -70,11 +71,8 @@ #include "llvm/Support/Threading.h" #include "llvm/Support/raw_ostream.h" -#ifdef LLVM_BUILD_TELEMETRY -#include "lldb/Core/Telemetry.h" -#include <chrono> -#endif +#include <chrono> #include <cstdio> #include <cstdlib> #include <cstring> @@ -767,29 +765,27 @@ void Debugger::InstanceInitialize() { DebuggerSP Debugger::CreateInstance(lldb::LogOutputCallback log_callback, void *baton) { -#ifdef LLVM_BUILD_TELEMETRY - lldb_private::telemetry::SteadyTimePoint start_time = - std::chrono::steady_clock::now(); -#endif + lldb_private::telemetry::ScopeTelemetryCollector helper; DebuggerSP debugger_sp(new Debugger(log_callback, baton)); + + if (helper.TelemetryEnabled()) { + helper.RunAtScopeExit([&](){ + lldb_private::telemetry::TelemetryManager * manager = lldb_private::telemetry::TelemetryManager::GetInstance(); + lldb_private::telemetry::DebuggerInfo entry; + entry.debugger = debugger_sp.get(); + entry.start_time = helper.GetStartTime(); + entry.end_time = helper.GetCurrentTime(); + manager->AtDebuggerStartup(&entry); + }); + + } + if (g_debugger_list_ptr && g_debugger_list_mutex_ptr) { std::lock_guard<std::recursive_mutex> guard(*g_debugger_list_mutex_ptr); g_debugger_list_ptr->push_back(debugger_sp); } debugger_sp->InstanceInitialize(); -#ifdef LLVM_BUILD_TELEMETRY - if (auto *telemetry_manager = telemetry::TelemetryManager::getInstance()) { - if (telemetry_manager->getConfig()->EnableTelemetry) { - lldb_private::telemetry::DebuggerTelemetryInfo entry; - entry.start_time = start_time; - entry.end_time = std::chrono::steady_clock::now(); - entry.debugger = debugger_sp.get(); - telemetry_manager->atDebuggerStartup(&entry); - } - } -#endif - return debugger_sp; } @@ -1010,10 +1006,20 @@ void Debugger::Clear() { // static void Debugger::Destroy(lldb::DebuggerSP &debugger_sp); // static void Debugger::Terminate(); llvm::call_once(m_clear_once, [this]() { -#ifdef LLVM_BUILD_TELEMETRY - lldb_private::telemetry::SteadyTimePoint quit_start_time = - std::chrono::steady_clock::now(); -#endif + lldb_private::telemetry::ScopeTelemetryCollector helper; + if (helper.TelemetryEnabled()) { + // TBD: We *may* have to send off the log BEFORE the ClearIOHanders()? + helper.RunAtScopeExit([&helper, this](){ + lldb_private::telemetry::TelemetryManager* manager + = lldb_private::telemetry::TelemetryManager::GetInstance(); + lldb_private::telemetry::DebuggerInfo entry; + entry.debugger = this; + entry.exit_desc = {0, ""}; // If we are here, there was no error. + entry.start_time = helper.GetStartTime(); + entry.end_time = helper.GetCurrentTime(); + manager->AtDebuggerExit(&entry); + }); + } ClearIOHandlers(); StopIOHandlerThread(); StopEventHandlerThread(); @@ -1036,19 +1042,6 @@ void Debugger::Clear() { if (Diagnostics::Enabled()) Diagnostics::Instance().RemoveCallback(m_diagnostics_callback_id); - -#ifdef LLVM_BUILD_TELEMETRY - if (auto telemetry_manager = telemetry::TelemetryManager::getInstance()) { - if (telemetry_manager->getConfig()->EnableTelemetry) { - // TBD: We *may* have to send off the log BEFORE the ClearIOHanders()? - lldb_private::telemetry::DebuggerTelemetryInfo entry; - entry.start_time = quit_start_time; - entry.end_time = std::chrono::steady_clock::now(); - entry.debugger = this; - telemetry_manager->atDebuggerExit(&entry); - } - } -#endif }); } diff --git a/lldb/source/Core/Telemetry.cpp b/lldb/source/Core/Telemetry.cpp index 9870d10ad42eb..3ab1fcc8dfdf6 100644 --- a/lldb/source/Core/Telemetry.cpp +++ b/lldb/source/Core/Telemetry.cpp @@ -68,10 +68,7 @@ void LLDBBaseTelemetryInfo::serialize(Serializer &serializer) const { void DebuggerInfo::serialize(Serializer &serializer) const { LLDBBaseTelemetryInfo::serialize(serializer); - serializer.write("username", username); - serializer.write("lldb_git_sha", lldb_git_sha); - serializer.write("lldb_path", lldb_path); - serializer.write("cwd", cwd); + serializer.write("lldb_version", lldb_version); if (exit_desc.has_value()) { serializer.write("exit_code", exit_desc->exit_code); serializer.write("exit_desc", exit_desc->description); @@ -79,7 +76,7 @@ void DebuggerInfo::serialize(Serializer &serializer) const { } TelemetryManager::TelemetryManager(std::unique_ptr<Config> config) - : m_config(std::move(config)), m_id(MakeUUID) {} + : m_config(std::move(config)), m_id(MakeUUID()) {} llvm::Error TelemetryManager::preDispatch(TelemetryInfo *entry) { // Assign the manager_id, and debugger_id, if available, to this entry. @@ -97,44 +94,20 @@ void TelemetryManager::AtDebuggerStartup(DebuggerInfo *entry) { LLDB_LOG_ERROR(GetLog(LLDBLog::Object), std::move(er), "Failed to dispatch entry at debugger startup: {0}"); } +} + +void TelemetryManager::AtDebuggerExit(DebuggerInfo * entry) { + // There must be a reference to the debugger at this point. + assert(entry->debugger != nullptr); + + if (llvm::Error er = dispatch(entry)) { + LLDB_LOG_ERROR(GetLog(LLDBLog::Object), std::move(er), + "Failed to dispatch entry at debugger exit: {0}"); + } +} + +const Config *TelemetryManager::GetConfig() { return m_config.get(); } - void TelemetryManager::AtDebuggerExit(DebuggerInfo * entry) { - // There must be a reference to the debugger at this point. - assert(entry->debugger != nullptr); - - if (auto *selected_target = - entry->debugger->GetSelectedExecutionContext().GetTargetPtr()) { - if (!selected_target->IsDummyTarget()) { - const lldb::ProcessSP proc = selected_target->GetProcessSP(); - if (proc == nullptr) { - // no process has been launched yet. - entry->exit_desc = {-1, "no process launched."}; - } else { - entry->exit_desc = {proc->GetExitStatus(), ""}; - if (const char *description = proc->GetExitDescription()) - entry->exit_desc->description = std::string(description); - } - } - } - - if (llvm::Error er = dispatch(entry)) { - LLDB_LOG_ERROR(GetLog(LLDBLog::Object), std::move(er), - "Failed to dispatch entry at debugger exit: {0}"); - } - - std::unique_ptr<TelemetryManager> TelemetryManager::g_instance = nullptr; - TelemetryManager *TelemetryManager::getInstance() { - return g_instance.get(); - } - - void TelemetryManager::setInstance( - std::unique_ptr<TelemetryManager> manager) { - g_instance = std::move(manager); - } - - -const Config *getConfig() { return m_config.get(); } - std::unique_ptr<TelemetryManager> TelemetryManager::g_instance = nullptr; TelemetryManager *TelemetryManager::GetInstance() { if (!Config::BuildTimeEnableTelemetry) @@ -146,5 +119,6 @@ void TelemetryManager::SetInstance(std::unique_ptr<TelemetryManager> manager) { g_instance = std::move(manager); } + } // namespace telemetry } // namespace lldb_private >From 7ad8360c170f5cbe599af2e7153742306d74a663 Mon Sep 17 00:00:00 2001 From: Vy Nguyen <v...@google.com> Date: Wed, 26 Feb 2025 14:32:47 -0500 Subject: [PATCH 07/16] format --- lldb/include/lldb/Core/Telemetry.h | 31 ++++++++++++++---------------- lldb/source/Core/Debugger.cpp | 15 +++++++-------- lldb/source/Core/Telemetry.cpp | 3 +-- 3 files changed, 22 insertions(+), 27 deletions(-) diff --git a/lldb/include/lldb/Core/Telemetry.h b/lldb/include/lldb/Core/Telemetry.h index 5c11004854c95..3e9d76b37aba0 100644 --- a/lldb/include/lldb/Core/Telemetry.h +++ b/lldb/include/lldb/Core/Telemetry.h @@ -22,8 +22,8 @@ #include <ctime> #include <memory> #include <optional> -#include <string> #include <stack> +#include <string> namespace lldb_private { namespace telemetry { @@ -91,7 +91,6 @@ class TelemetryManager : public llvm::telemetry::Manager { public: llvm::Error preDispatch(llvm::telemetry::TelemetryInfo *entry) override; - virtual void AtDebuggerStartup(DebuggerInfo *entry); virtual void AtDebuggerExit(DebuggerInfo *entry); @@ -115,38 +114,36 @@ class TelemetryManager : public llvm::telemetry::Manager { /// Helper RAII class for collecting telemetry. class ScopeTelemetryCollector { - public: - ScopeTelemetryCollector () { +public: + ScopeTelemetryCollector() { if (TelemetryEnabled()) m_start = std::chrono::steady_clock::now(); } ~ScopeTelemetryCollector() { - while(! m_exit_funcs.empty()) { + while (!m_exit_funcs.empty()) { (m_exit_funcs.top())(); m_exit_funcs.pop(); } } bool TelemetryEnabled() { - TelemetryManager* instance = TelemetryManager::GetInstance(); - return instance != nullptr && instance->GetConfig()->EnableTelemetry; + TelemetryManager *instance = TelemetryManager::GetInstance(); + return instance != nullptr && instance->GetConfig()->EnableTelemetry; } + SteadyTimePoint GetStartTime() { return m_start; } + SteadyTimePoint GetCurrentTime() { return std::chrono::steady_clock::now(); } - SteadyTimePoint GetStartTime() {return m_start;} - SteadyTimePoint GetCurrentTime() { return std::chrono::steady_clock::now(); } - - template <typename Fp> - void RunAtScopeExit(Fp&& F){ - assert(llvm::telemetry::Config::BuildTimeEnableTelemetry && "Telemetry should have been enabled"); - m_exit_funcs.push(std::forward<Fp>(F)); - } + template <typename Fp> void RunAtScopeExit(Fp &&F) { + assert(llvm::telemetry::Config::BuildTimeEnableTelemetry && + "Telemetry should have been enabled"); + m_exit_funcs.push(std::forward<Fp>(F)); + } - private: +private: SteadyTimePoint m_start; std::stack<std::function<void()>> m_exit_funcs; - }; } // namespace telemetry diff --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp index 750ca3a91ba3d..6cafffe50cc22 100644 --- a/lldb/source/Core/Debugger.cpp +++ b/lldb/source/Core/Debugger.cpp @@ -17,6 +17,7 @@ #include "lldb/Core/PluginManager.h" #include "lldb/Core/Progress.h" #include "lldb/Core/StreamAsynchronousIO.h" +#include "lldb/Core/Telemetry.h" #include "lldb/DataFormatters/DataVisualization.h" #include "lldb/Expression/REPL.h" #include "lldb/Host/File.h" @@ -52,7 +53,6 @@ #include "lldb/Utility/State.h" #include "lldb/Utility/Stream.h" #include "lldb/Utility/StreamString.h" -#include "lldb/Core/Telemetry.h" #include "lldb/lldb-enumerations.h" #if defined(_WIN32) @@ -71,7 +71,6 @@ #include "llvm/Support/Threading.h" #include "llvm/Support/raw_ostream.h" - #include <chrono> #include <cstdio> #include <cstdlib> @@ -769,15 +768,15 @@ DebuggerSP Debugger::CreateInstance(lldb::LogOutputCallback log_callback, DebuggerSP debugger_sp(new Debugger(log_callback, baton)); if (helper.TelemetryEnabled()) { - helper.RunAtScopeExit([&](){ - lldb_private::telemetry::TelemetryManager * manager = lldb_private::telemetry::TelemetryManager::GetInstance(); + helper.RunAtScopeExit([&]() { + lldb_private::telemetry::TelemetryManager *manager = + lldb_private::telemetry::TelemetryManager::GetInstance(); lldb_private::telemetry::DebuggerInfo entry; entry.debugger = debugger_sp.get(); entry.start_time = helper.GetStartTime(); entry.end_time = helper.GetCurrentTime(); manager->AtDebuggerStartup(&entry); }); - } if (g_debugger_list_ptr && g_debugger_list_mutex_ptr) { @@ -1009,9 +1008,9 @@ void Debugger::Clear() { lldb_private::telemetry::ScopeTelemetryCollector helper; if (helper.TelemetryEnabled()) { // TBD: We *may* have to send off the log BEFORE the ClearIOHanders()? - helper.RunAtScopeExit([&helper, this](){ - lldb_private::telemetry::TelemetryManager* manager - = lldb_private::telemetry::TelemetryManager::GetInstance(); + helper.RunAtScopeExit([&helper, this]() { + lldb_private::telemetry::TelemetryManager *manager = + lldb_private::telemetry::TelemetryManager::GetInstance(); lldb_private::telemetry::DebuggerInfo entry; entry.debugger = this; entry.exit_desc = {0, ""}; // If we are here, there was no error. diff --git a/lldb/source/Core/Telemetry.cpp b/lldb/source/Core/Telemetry.cpp index 3ab1fcc8dfdf6..e83775c0c5855 100644 --- a/lldb/source/Core/Telemetry.cpp +++ b/lldb/source/Core/Telemetry.cpp @@ -96,7 +96,7 @@ void TelemetryManager::AtDebuggerStartup(DebuggerInfo *entry) { } } -void TelemetryManager::AtDebuggerExit(DebuggerInfo * entry) { +void TelemetryManager::AtDebuggerExit(DebuggerInfo *entry) { // There must be a reference to the debugger at this point. assert(entry->debugger != nullptr); @@ -119,6 +119,5 @@ void TelemetryManager::SetInstance(std::unique_ptr<TelemetryManager> manager) { g_instance = std::move(manager); } - } // namespace telemetry } // namespace lldb_private >From d7b1275f76e2141eb52b5b505c42797642990741 Mon Sep 17 00:00:00 2001 From: Vy Nguyen <v...@google.com> Date: Wed, 26 Feb 2025 15:28:31 -0500 Subject: [PATCH 08/16] try reporting the crash from a crash handler --- lldb/source/API/SBDebugger.cpp | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/lldb/source/API/SBDebugger.cpp b/lldb/source/API/SBDebugger.cpp index e646b09e05852..947df38bc716b 100644 --- a/lldb/source/API/SBDebugger.cpp +++ b/lldb/source/API/SBDebugger.cpp @@ -43,6 +43,7 @@ #include "lldb/DataFormatters/DataVisualization.h" #include "lldb/Host/Config.h" #include "lldb/Host/StreamFile.h" +#include "lldb/Core/Telemetry.h" #include "lldb/Host/XML.h" #include "lldb/Initialization/SystemLifetimeManager.h" #include "lldb/Interpreter/CommandInterpreter.h" @@ -226,6 +227,37 @@ lldb::SBError SBDebugger::InitializeWithErrorHandling() { return error; } +#if LLVM_ENABLE_TELEMETRY +#if ENABLE_BACKTRACES +static void TelemetryHandler(void *) { + // TODO: get the bt into the msg? + // Also need to pre-alloc the memory for this entry? + lldb_private::telemetry::DebuggerInfo entry; + entry.exit_desc = {-1, ""}; + if (auto* instance = lldb_private::telemetry::TelemeryManager::getInstance()) { + if (instance->GetConfig()->EnableTelemetry()) { + instance->AtDebuggerExit(&entry); + } + } +} + +static bool RegisterTelemetryHander() { + sys::AddSignalHandler(TelemetryHandler, nullptr); + return false; +} +#endif +#endif + +static void InstallCrashTelemetryReporter() { +#if LLVM_ENABLE_TELEMETRY + +#if ENABLE_BACKTRACES + static bool HandlerRegistered = RegisterTelemeryHandler(); + (void)HandlerRegistered; +#endif +#endif +} + void SBDebugger::PrintStackTraceOnError() { LLDB_INSTRUMENT(); @@ -233,6 +265,7 @@ void SBDebugger::PrintStackTraceOnError() { static std::string executable = llvm::sys::fs::getMainExecutable(nullptr, nullptr); llvm::sys::PrintStackTraceOnErrorSignal(executable); + InstallCrashTelemetryReporter(); } static void DumpDiagnostics(void *cookie) { >From 302552a0e8283476962056116f5aa3cd84b2af6a Mon Sep 17 00:00:00 2001 From: Vy Nguyen <v...@google.com> Date: Thu, 27 Feb 2025 09:56:16 -0500 Subject: [PATCH 09/16] revert crash handler changes --- lldb/source/API/SBDebugger.cpp | 33 --------------------------------- 1 file changed, 33 deletions(-) diff --git a/lldb/source/API/SBDebugger.cpp b/lldb/source/API/SBDebugger.cpp index 947df38bc716b..e646b09e05852 100644 --- a/lldb/source/API/SBDebugger.cpp +++ b/lldb/source/API/SBDebugger.cpp @@ -43,7 +43,6 @@ #include "lldb/DataFormatters/DataVisualization.h" #include "lldb/Host/Config.h" #include "lldb/Host/StreamFile.h" -#include "lldb/Core/Telemetry.h" #include "lldb/Host/XML.h" #include "lldb/Initialization/SystemLifetimeManager.h" #include "lldb/Interpreter/CommandInterpreter.h" @@ -227,37 +226,6 @@ lldb::SBError SBDebugger::InitializeWithErrorHandling() { return error; } -#if LLVM_ENABLE_TELEMETRY -#if ENABLE_BACKTRACES -static void TelemetryHandler(void *) { - // TODO: get the bt into the msg? - // Also need to pre-alloc the memory for this entry? - lldb_private::telemetry::DebuggerInfo entry; - entry.exit_desc = {-1, ""}; - if (auto* instance = lldb_private::telemetry::TelemeryManager::getInstance()) { - if (instance->GetConfig()->EnableTelemetry()) { - instance->AtDebuggerExit(&entry); - } - } -} - -static bool RegisterTelemetryHander() { - sys::AddSignalHandler(TelemetryHandler, nullptr); - return false; -} -#endif -#endif - -static void InstallCrashTelemetryReporter() { -#if LLVM_ENABLE_TELEMETRY - -#if ENABLE_BACKTRACES - static bool HandlerRegistered = RegisterTelemeryHandler(); - (void)HandlerRegistered; -#endif -#endif -} - void SBDebugger::PrintStackTraceOnError() { LLDB_INSTRUMENT(); @@ -265,7 +233,6 @@ void SBDebugger::PrintStackTraceOnError() { static std::string executable = llvm::sys::fs::getMainExecutable(nullptr, nullptr); llvm::sys::PrintStackTraceOnErrorSignal(executable); - InstallCrashTelemetryReporter(); } static void DumpDiagnostics(void *cookie) { >From c00cd54664dbbe489074f0588dcc5468786196cf Mon Sep 17 00:00:00 2001 From: Vy Nguyen <v...@google.com> Date: Thu, 27 Feb 2025 10:11:35 -0500 Subject: [PATCH 10/16] Update lldb/source/Core/Telemetry.cpp Co-authored-by: Pavel Labath <pa...@labath.sk> --- lldb/source/Core/Telemetry.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lldb/source/Core/Telemetry.cpp b/lldb/source/Core/Telemetry.cpp index e83775c0c5855..2921657395baa 100644 --- a/lldb/source/Core/Telemetry.cpp +++ b/lldb/source/Core/Telemetry.cpp @@ -81,7 +81,7 @@ TelemetryManager::TelemetryManager(std::unique_ptr<Config> config) llvm::Error TelemetryManager::preDispatch(TelemetryInfo *entry) { // Assign the manager_id, and debugger_id, if available, to this entry. LLDBBaseTelemetryInfo *lldb_entry = - llvm::dyn_cast<LLDBBaseTelemetryInfo>(entry); + llvm::cast<LLDBBaseTelemetryInfo>(entry); lldb_entry->SessionId = m_id; if (Debugger *debugger = lldb_entry->debugger) lldb_entry->debugger_id = debugger->GetID(); >From 597185f9a8a10c03100533dd29fbefd62dcb3ad9 Mon Sep 17 00:00:00 2001 From: Vy Nguyen <v...@google.com> Date: Thu, 27 Feb 2025 10:17:29 -0500 Subject: [PATCH 11/16] remove unused headers --- lldb/source/Core/Telemetry.cpp | 5 ----- 1 file changed, 5 deletions(-) diff --git a/lldb/source/Core/Telemetry.cpp b/lldb/source/Core/Telemetry.cpp index 2921657395baa..61b0300d496f9 100644 --- a/lldb/source/Core/Telemetry.cpp +++ b/lldb/source/Core/Telemetry.cpp @@ -8,10 +8,6 @@ #include "lldb/Core/Telemetry.h" #include "lldb/Core/Debugger.h" #include "lldb/Core/Telemetry.h" -#include "lldb/Host/FileSystem.h" -#include "lldb/Host/HostInfo.h" -#include "lldb/Target/Process.h" -#include "lldb/Target/Statistics.h" #include "lldb/Utility/LLDBLog.h" #include "lldb/Utility/UUID.h" #include "lldb/Version/Version.h" @@ -19,7 +15,6 @@ #include "lldb/lldb-forward.h" #include "llvm/ADT/StringRef.h" #include "llvm/Support/Error.h" -#include "llvm/Support/Path.h" #include "llvm/Support/RandomNumberGenerator.h" #include "llvm/Telemetry/Telemetry.h" #include <chrono> >From 41f664922a2da5788bce6e6db93c6738104765ef Mon Sep 17 00:00:00 2001 From: Vy Nguyen <v...@google.com> Date: Thu, 27 Feb 2025 10:21:01 -0500 Subject: [PATCH 12/16] remove miscinfo kindtype --- lldb/include/lldb/Core/Telemetry.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/lldb/include/lldb/Core/Telemetry.h b/lldb/include/lldb/Core/Telemetry.h index 3e9d76b37aba0..fb207931aa3d6 100644 --- a/lldb/include/lldb/Core/Telemetry.h +++ b/lldb/include/lldb/Core/Telemetry.h @@ -31,8 +31,6 @@ namespace telemetry { struct LLDBEntryKind : public ::llvm::telemetry::EntryKind { static const llvm::telemetry::KindType BaseInfo = 0b11000; static const llvm::telemetry::KindType DebuggerInfo = 0b11001; - // There are other entries in between (added in separate PRs) - static const llvm::telemetry::KindType MiscInfo = 0b11110; }; /// Defines a convenient type for timestamp of various events. >From 169449bbf47b5863175d0fcd0727ecc3e4652c95 Mon Sep 17 00:00:00 2001 From: Vy Nguyen <v...@google.com> Date: Thu, 27 Feb 2025 10:39:05 -0500 Subject: [PATCH 13/16] Init debugger_id to LLDB_INVALID_UID --- lldb/include/lldb/Core/Telemetry.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lldb/include/lldb/Core/Telemetry.h b/lldb/include/lldb/Core/Telemetry.h index fb207931aa3d6..5d663e0564428 100644 --- a/lldb/include/lldb/Core/Telemetry.h +++ b/lldb/include/lldb/Core/Telemetry.h @@ -43,7 +43,7 @@ struct LLDBBaseTelemetryInfo : public llvm::telemetry::TelemetryInfo { std::optional<SteadyTimePoint> end_time; // TBD: could add some memory stats here too? - uint64_t debugger_id = 0; + uint64_t debugger_id = LLDB_INVALID_UID; Debugger *debugger; // For dyn_cast, isa, etc operations. >From 74208119687fdcfcdba9011ccf1f4b990a40f321 Mon Sep 17 00:00:00 2001 From: Vy Nguyen <v...@google.com> Date: Thu, 27 Feb 2025 14:54:53 -0500 Subject: [PATCH 14/16] addressed review comment --- lldb/include/lldb/Core/Telemetry.h | 55 ++++++++++++++++-------------- lldb/source/Core/Debugger.cpp | 44 +++++++++--------------- 2 files changed, 47 insertions(+), 52 deletions(-) diff --git a/lldb/include/lldb/Core/Telemetry.h b/lldb/include/lldb/Core/Telemetry.h index fb207931aa3d6..725252fcb6db3 100644 --- a/lldb/include/lldb/Core/Telemetry.h +++ b/lldb/include/lldb/Core/Telemetry.h @@ -11,6 +11,7 @@ #include "lldb/Core/StructuredDataImpl.h" #include "lldb/Interpreter/CommandReturnObject.h" +#include "lldb/Utility/LLDBLog.h" #include "lldb/Utility/StructuredData.h" #include "lldb/lldb-forward.h" #include "llvm/ADT/DenseMap.h" @@ -111,37 +112,41 @@ class TelemetryManager : public llvm::telemetry::Manager { }; /// Helper RAII class for collecting telemetry. -class ScopeTelemetryCollector { -public: - ScopeTelemetryCollector() { - if (TelemetryEnabled()) - m_start = std::chrono::steady_clock::now(); +template <typename I> struct ScopedDispatcher { + // The debugger pointer is optional because we may not have a debugger yet. + // In that case, caller must set the debugger later. + ScopedDispatcher(std::function<void(I *info)> callback, + Debugger *debugger = nullptr) + : m_callback(callback) { + // Start the timer. + m_start_time = std::chrono::steady_clock::now(); + m_info.debugger = debugger; } - ~ScopeTelemetryCollector() { - while (!m_exit_funcs.empty()) { - (m_exit_funcs.top())(); - m_exit_funcs.pop(); + void SetDebugger(Debugger *debugger) { m_info.debugger = debugger; } + + ~ScopedDispatcher() { + TelemetryManager *manager = TelemetryManager::GetInstance(); + // If Telemetry is disabled (either at buildtime or runtime), + // then don't do anything. + if (!manager || !manager->GetConfig()->EnableTelemetry) + return; + m_info.start_time = m_start_time; + // Populate common fields that we can only set now. + m_info.end_time = std::chrono::steady_clock::now(); + // The callback will set the remaining fields. + m_callback(&m_info); + // And then we dispatch. + if (llvm::Error er = manager->dispatch(&m_info)) { + LLDB_LOG_ERROR(GetLog(LLDBLog::Object), std::move(er), + "Failed to dispatch entry of type: {0}", m_info.getKind()); } } - bool TelemetryEnabled() { - TelemetryManager *instance = TelemetryManager::GetInstance(); - return instance != nullptr && instance->GetConfig()->EnableTelemetry; - } - - SteadyTimePoint GetStartTime() { return m_start; } - SteadyTimePoint GetCurrentTime() { return std::chrono::steady_clock::now(); } - - template <typename Fp> void RunAtScopeExit(Fp &&F) { - assert(llvm::telemetry::Config::BuildTimeEnableTelemetry && - "Telemetry should have been enabled"); - m_exit_funcs.push(std::forward<Fp>(F)); - } - private: - SteadyTimePoint m_start; - std::stack<std::function<void()>> m_exit_funcs; + SteadyTimePoint m_start_time; + std::function<void(I *info)> m_callback; + I m_info; }; } // namespace telemetry diff --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp index 6cafffe50cc22..b31567ff2421d 100644 --- a/lldb/source/Core/Debugger.cpp +++ b/lldb/source/Core/Debugger.cpp @@ -53,6 +53,7 @@ #include "lldb/Utility/State.h" #include "lldb/Utility/Stream.h" #include "lldb/Utility/StreamString.h" +#include "lldb/Version/Version.h" #include "lldb/lldb-enumerations.h" #if defined(_WIN32) @@ -764,20 +765,13 @@ void Debugger::InstanceInitialize() { DebuggerSP Debugger::CreateInstance(lldb::LogOutputCallback log_callback, void *baton) { - lldb_private::telemetry::ScopeTelemetryCollector helper; + lldb_private::telemetry::ScopedDispatcher< + lldb_private::telemetry::DebuggerInfo> + helper([](lldb_private::telemetry::DebuggerInfo *entry) { + entry->lldb_version = lldb_private::GetVersion(); + }); DebuggerSP debugger_sp(new Debugger(log_callback, baton)); - - if (helper.TelemetryEnabled()) { - helper.RunAtScopeExit([&]() { - lldb_private::telemetry::TelemetryManager *manager = - lldb_private::telemetry::TelemetryManager::GetInstance(); - lldb_private::telemetry::DebuggerInfo entry; - entry.debugger = debugger_sp.get(); - entry.start_time = helper.GetStartTime(); - entry.end_time = helper.GetCurrentTime(); - manager->AtDebuggerStartup(&entry); - }); - } + helper.SetDebugger(debugger_sp.get()); if (g_debugger_list_ptr && g_debugger_list_mutex_ptr) { std::lock_guard<std::recursive_mutex> guard(*g_debugger_list_mutex_ptr); @@ -1005,20 +999,16 @@ void Debugger::Clear() { // static void Debugger::Destroy(lldb::DebuggerSP &debugger_sp); // static void Debugger::Terminate(); llvm::call_once(m_clear_once, [this]() { - lldb_private::telemetry::ScopeTelemetryCollector helper; - if (helper.TelemetryEnabled()) { - // TBD: We *may* have to send off the log BEFORE the ClearIOHanders()? - helper.RunAtScopeExit([&helper, this]() { - lldb_private::telemetry::TelemetryManager *manager = - lldb_private::telemetry::TelemetryManager::GetInstance(); - lldb_private::telemetry::DebuggerInfo entry; - entry.debugger = this; - entry.exit_desc = {0, ""}; // If we are here, there was no error. - entry.start_time = helper.GetStartTime(); - entry.end_time = helper.GetCurrentTime(); - manager->AtDebuggerExit(&entry); - }); - } + lldb_private::telemetry::ScopedDispatcher< + lldb_private::telemetry::DebuggerInfo> + helper( + [](lldb_private::telemetry::DebuggerInfo *info) { + // If we are here, then there was no error. + // Any abnormal exit will be reported by the crash-handler. + info->exit_desc = {0, ""}; + }, + this); + ClearIOHandlers(); StopIOHandlerThread(); StopEventHandlerThread(); >From 68f95ab119c998e86625a0d459ad074481a3016e Mon Sep 17 00:00:00 2001 From: Vy Nguyen <v...@google.com> Date: Thu, 27 Feb 2025 14:57:55 -0500 Subject: [PATCH 15/16] removed unused --- lldb/include/lldb/Core/Telemetry.h | 3 --- lldb/source/Core/Telemetry.cpp | 17 ----------------- 2 files changed, 20 deletions(-) diff --git a/lldb/include/lldb/Core/Telemetry.h b/lldb/include/lldb/Core/Telemetry.h index 1b1eee1c3248e..a04ae42a45a48 100644 --- a/lldb/include/lldb/Core/Telemetry.h +++ b/lldb/include/lldb/Core/Telemetry.h @@ -90,9 +90,6 @@ class TelemetryManager : public llvm::telemetry::Manager { public: llvm::Error preDispatch(llvm::telemetry::TelemetryInfo *entry) override; - virtual void AtDebuggerStartup(DebuggerInfo *entry); - virtual void AtDebuggerExit(DebuggerInfo *entry); - const llvm::telemetry::Config *GetConfig(); virtual llvm::StringRef GetInstanceName() const = 0; diff --git a/lldb/source/Core/Telemetry.cpp b/lldb/source/Core/Telemetry.cpp index 61b0300d496f9..0ef92995cbe40 100644 --- a/lldb/source/Core/Telemetry.cpp +++ b/lldb/source/Core/Telemetry.cpp @@ -84,23 +84,6 @@ llvm::Error TelemetryManager::preDispatch(TelemetryInfo *entry) { return llvm::Error::success(); } -void TelemetryManager::AtDebuggerStartup(DebuggerInfo *entry) { - if (llvm::Error er = dispatch(entry)) { - LLDB_LOG_ERROR(GetLog(LLDBLog::Object), std::move(er), - "Failed to dispatch entry at debugger startup: {0}"); - } -} - -void TelemetryManager::AtDebuggerExit(DebuggerInfo *entry) { - // There must be a reference to the debugger at this point. - assert(entry->debugger != nullptr); - - if (llvm::Error er = dispatch(entry)) { - LLDB_LOG_ERROR(GetLog(LLDBLog::Object), std::move(er), - "Failed to dispatch entry at debugger exit: {0}"); - } -} - const Config *TelemetryManager::GetConfig() { return m_config.get(); } std::unique_ptr<TelemetryManager> TelemetryManager::g_instance = nullptr; >From d586956edb741bb9f5da108bf21361d41297f068 Mon Sep 17 00:00:00 2001 From: Vy Nguyen <v...@google.com> Date: Thu, 27 Feb 2025 14:59:57 -0500 Subject: [PATCH 16/16] add smoke check --- lldb/source/Core/Debugger.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp index b31567ff2421d..2ebfb3d069fcb 100644 --- a/lldb/source/Core/Debugger.cpp +++ b/lldb/source/Core/Debugger.cpp @@ -1002,7 +1002,8 @@ void Debugger::Clear() { lldb_private::telemetry::ScopedDispatcher< lldb_private::telemetry::DebuggerInfo> helper( - [](lldb_private::telemetry::DebuggerInfo *info) { + [this](lldb_private::telemetry::DebuggerInfo *info) { + assert(this == info->debugger); // If we are here, then there was no error. // Any abnormal exit will be reported by the crash-handler. info->exit_desc = {0, ""}; _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits