[Lldb-commits] [lldb] [LLDB][Telemetry] Collect telemetry from client when allowed. (PR #129728)
Vy Nguyen via lldb-commits
lldb-commits at lists.llvm.org
Mon Mar 24 07:17:30 PDT 2025
https://github.com/oontvoo updated https://github.com/llvm/llvm-project/pull/129728
>From 21103adacdf9c08cee4065f8a6b90ff812fefbb3 Mon Sep 17 00:00:00 2001
From: Vy Nguyen <vyng at google.com>
Date: Tue, 4 Mar 2025 11:01:46 -0500
Subject: [PATCH 1/3] [LLDB][Telemetry] Collect telemetry from client when
allowed.
This patch is slightly different from other impl in that we dispatch client-telemetry via a different helper method.
This is to make it easier for vendor to opt-out (simply by overriding the method to do nothing).
There is also a configuration option to disallow collecting client telemetry.
---
lldb/include/lldb/API/SBDebugger.h | 3 +
lldb/include/lldb/Core/Debugger.h | 5 ++
lldb/include/lldb/Core/Telemetry.h | 89 +++++++++++++++++-------
lldb/source/API/SBDebugger.cpp | 11 +++
lldb/source/Core/Debugger.cpp | 6 ++
lldb/source/Core/Telemetry.cpp | 99 +++++++++++++++++++++++----
lldb/tools/lldb-dap/DAP.cpp | 5 +-
lldb/tools/lldb-dap/LLDBUtils.h | 34 +++++++++
lldb/unittests/Core/TelemetryTest.cpp | 2 +-
9 files changed, 214 insertions(+), 40 deletions(-)
diff --git a/lldb/include/lldb/API/SBDebugger.h b/lldb/include/lldb/API/SBDebugger.h
index e0819f1684f8b..28f92f2095951 100644
--- a/lldb/include/lldb/API/SBDebugger.h
+++ b/lldb/include/lldb/API/SBDebugger.h
@@ -13,6 +13,7 @@
#include "lldb/API/SBDefines.h"
#include "lldb/API/SBPlatform.h"
+#include "lldb/API/SBStructuredData.h"
namespace lldb_private {
class CommandPluginInterfaceImplementation;
@@ -249,6 +250,8 @@ class LLDB_API SBDebugger {
lldb::SBTarget GetDummyTarget();
+ void DispatchClientTelemetry(const lldb::SBStructuredData &data);
+
// Return true if target is deleted from the target list of the debugger.
bool DeleteTarget(lldb::SBTarget &target);
diff --git a/lldb/include/lldb/Core/Debugger.h b/lldb/include/lldb/Core/Debugger.h
index 6ebc6147800e1..e40666d5ceec7 100644
--- a/lldb/include/lldb/Core/Debugger.h
+++ b/lldb/include/lldb/Core/Debugger.h
@@ -19,6 +19,8 @@
#include "lldb/Core/FormatEntity.h"
#include "lldb/Core/IOHandler.h"
#include "lldb/Core/SourceManager.h"
+#include "lldb/Core/StructuredDataImpl.h"
+#include "lldb/Core/Telemetry.h"
#include "lldb/Core/UserSettingsController.h"
#include "lldb/Host/HostThread.h"
#include "lldb/Host/StreamFile.h"
@@ -31,6 +33,7 @@
#include "lldb/Utility/Diagnostics.h"
#include "lldb/Utility/FileSpec.h"
#include "lldb/Utility/Status.h"
+#include "lldb/Utility/StructuredData.h"
#include "lldb/Utility/UserID.h"
#include "lldb/lldb-defines.h"
#include "lldb/lldb-enumerations.h"
@@ -127,6 +130,8 @@ class Debugger : public std::enable_shared_from_this<Debugger>,
void Clear();
+ void DispatchClientTelemetry(const lldb_private::StructuredDataImpl &entry);
+
bool GetAsyncExecution();
void SetAsyncExecution(bool async);
diff --git a/lldb/include/lldb/Core/Telemetry.h b/lldb/include/lldb/Core/Telemetry.h
index 7d8716f1659b5..cad4a4a6c9048 100644
--- a/lldb/include/lldb/Core/Telemetry.h
+++ b/lldb/include/lldb/Core/Telemetry.h
@@ -19,6 +19,7 @@
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/JSON.h"
#include "llvm/Telemetry/Telemetry.h"
+#include <atomic>
#include <chrono>
#include <ctime>
#include <memory>
@@ -28,6 +29,23 @@
namespace lldb_private {
namespace telemetry {
+struct LLDBConfig : public ::llvm::telemetry::Config {
+ // If true, we will collect full details about a debug command (eg., args and
+ // original command). Note: This may contain PII, hence can only be enabled by
+ // the vendor while creating the Manager.
+ const bool m_detailed_command_telemetry;
+ // If true, we will collect telemetry from LLDB's clients (eg., lldb-dap) via
+ // the SB interface. Must also be enabled by the vendor while creating the
+ // manager.
+ const bool m_enable_client_telemetry;
+
+ explicit LLDBConfig(bool enable_telemetry, bool detailed_command_telemetry,
+ bool enable_client_telemetry)
+ : ::llvm::telemetry::Config(enable_telemetry),
+ m_detailed_command_telemetry(detailed_command_telemetry),
+ m_enable_client_telemetry(enable_client_telemetry) {}
+};
+
// We expect each (direct) subclass of LLDBTelemetryInfo to
// have an LLDBEntryKind in the form 0b11xxxxxxxx
// Specifically:
@@ -37,6 +55,7 @@ namespace telemetry {
// must have their LLDBEntryKind in the similar form (ie., share common prefix)
struct LLDBEntryKind : public ::llvm::telemetry::EntryKind {
static const llvm::telemetry::KindType BaseInfo = 0b11000000;
+ static const llvm::telemetry::KindType ClientInfo = 0b11100000;
static const llvm::telemetry::KindType DebuggerInfo = 0b11000100;
};
@@ -86,6 +105,11 @@ struct DebuggerInfo : public LLDBBaseTelemetryInfo {
void serialize(llvm::telemetry::Serializer &serializer) const override;
};
+struct ClientInfo : public LLDBBaseTelemetryInfo {
+ std::string request_name;
+ std::optional<std::string> error_msg;
+};
+
/// The base Telemetry manager instance in LLDB.
/// This class declares additional instrumentation points
/// applicable to LLDB.
@@ -93,24 +117,24 @@ class TelemetryManager : public llvm::telemetry::Manager {
public:
llvm::Error preDispatch(llvm::telemetry::TelemetryInfo *entry) override;
- const llvm::telemetry::Config *GetConfig();
+ const LLDBConfig *GetConfig() { return m_config.get(); }
+
+ void DispatchClientTelemery(const lldb_private::StructuredDataImpl &entry,
+ Debugger *debugger);
virtual llvm::StringRef GetInstanceName() const = 0;
static TelemetryManager *GetInstance();
- static TelemetryManager *GetInstanceIfEnabled();
-
protected:
- TelemetryManager(std::unique_ptr<llvm::telemetry::Config> config);
+ TelemetryManager(std::unique_ptr<LLDBConfig> config);
static void SetInstance(std::unique_ptr<TelemetryManager> manger);
private:
- std::unique_ptr<llvm::telemetry::Config> m_config;
+ std::unique_ptr<LLDBConfig> m_config;
// Each instance of a TelemetryManager is assigned a unique ID.
const std::string m_id;
-
static std::unique_ptr<TelemetryManager> g_instance;
};
@@ -118,39 +142,54 @@ class TelemetryManager : public llvm::telemetry::Manager {
template <typename Info> 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(llvm::unique_function<void(Info *info)> callback,
+ ScopedDispatcher(Debugger *debugger = nullptr) {
+ // Start the timer.
+ m_start_time = std::chrono::steady_clock::now();
+ this->debugger = debugger;
+ }
+ ScopedDispatcher(llvm::unique_function<void(Info *info)> final_callback,
Debugger *debugger = nullptr)
- : m_callback(std::move(callback)) {
+ : m_final_callback(std::move(final_callback)) {
// Start the timer.
m_start_time = std::chrono::steady_clock::now();
- m_info.debugger = debugger;
+ this->debugger = debugger;
}
- void SetDebugger(Debugger *debugger) { m_info.debugger = debugger; }
+ void SetDebugger(Debugger *debugger) { this->debugger = debugger; }
- ~ScopedDispatcher() {
- // If Telemetry is disabled (either at buildtime or runtime),
- // then don't do anything.
- TelemetryManager *manager = TelemetryManager::GetInstanceIfEnabled();
- if (!manager)
- return;
+ void DispatchOnExit(llvm::unique_function<void(Info *info)> final_callback) {
+ // We probably should not be overriding previously set cb.
+ assert(!m_final_callback);
+ m_final_callback = std::move(final_callback);
+ }
- 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);
+ void DispatchNow(llvm::unique_function<void(Info *info)> populate_fields_cb) {
+ TelemetryManager *manager = TelemetryManager::GetInstance();
+ if (!manager->GetConfig()->EnableTelemetry)
+ return;
+ Info info;
+ // Populate the common fields we know aboutl
+ info.start_time = m_start_time;
+ info.end_time = std::chrono::steady_clock::now();
+ info.debugger = debugger;
+ // The callback will set the rest.
+ populate_fields_cb(&info);
// And then we dispatch.
- if (llvm::Error er = manager->dispatch(&m_info)) {
+ if (llvm::Error er = manager->dispatch(&info)) {
LLDB_LOG_ERROR(GetLog(LLDBLog::Object), std::move(er),
- "Failed to dispatch entry of type: {0}", m_info.getKind());
+ "Failed to dispatch entry of type: {0}", info.getKind());
}
}
+ ~ScopedDispatcher() {
+ if (m_final_callback)
+ DispatchNow(std::move(m_final_callback));
+ }
+
private:
SteadyTimePoint m_start_time;
- llvm::unique_function<void(Info *info)> m_callback;
- Info m_info;
+ llvm::unique_function<void(Info *info)> m_final_callback;
+ Debugger *debugger;
};
} // namespace telemetry
diff --git a/lldb/source/API/SBDebugger.cpp b/lldb/source/API/SBDebugger.cpp
index e646b09e05852..7cd2beae94189 100644
--- a/lldb/source/API/SBDebugger.cpp
+++ b/lldb/source/API/SBDebugger.cpp
@@ -965,6 +965,17 @@ SBTarget SBDebugger::GetDummyTarget() {
return sb_target;
}
+void SBDebugger::DispatchClientTelemetry(const lldb::SBStructuredData &entry) {
+ LLDB_INSTRUMENT_VA(this);
+ if (lldb_private::Debugger *debugger = this->get()) {
+ debugger->DispatchClientTelemetry(*(entry.m_impl_up.get()));
+ } else {
+ Log *log = GetLog(LLDBLog::API);
+ LLDB_LOGF(log,
+ "Could not send telemetry from SBDebugger - debugger was null.");
+ }
+}
+
bool SBDebugger::DeleteTarget(lldb::SBTarget &target) {
LLDB_INSTRUMENT_VA(this, target);
diff --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp
index dbe3d72e5efa4..5a04eb876a815 100644
--- a/lldb/source/Core/Debugger.cpp
+++ b/lldb/source/Core/Debugger.cpp
@@ -781,6 +781,12 @@ DebuggerSP Debugger::CreateInstance(lldb::LogOutputCallback log_callback,
return debugger_sp;
}
+void Debugger::DispatchClientTelemetry(
+ const lldb_private::StructuredDataImpl &entry) {
+ lldb_private::telemetry::TelemeryManager::GetInstance()
+ ->DispatchClientTelemetry(entry);
+}
+
void Debugger::HandleDestroyCallback() {
const lldb::user_id_t user_id = GetID();
// Invoke and remove all the callbacks in an FIFO order. Callbacks which are
diff --git a/lldb/source/Core/Telemetry.cpp b/lldb/source/Core/Telemetry.cpp
index 367e94d6566de..3edb27e535da9 100644
--- a/lldb/source/Core/Telemetry.cpp
+++ b/lldb/source/Core/Telemetry.cpp
@@ -59,6 +59,12 @@ void LLDBBaseTelemetryInfo::serialize(Serializer &serializer) const {
if (end_time.has_value())
serializer.write("end_time", ToNanosec(end_time.value()));
}
+void ClientInfo::serialize(Serializer &serializer) const {
+ LLDBBaseTelemetryInfo::serialize(serializer);
+ serializer.write("request_name", request_name);
+ if (error_msg.has_value())
+ serializer.write("error_msg", error_msg.value());
+}
void DebuggerInfo::serialize(Serializer &serializer) const {
LLDBBaseTelemetryInfo::serialize(serializer);
@@ -67,7 +73,7 @@ void DebuggerInfo::serialize(Serializer &serializer) const {
serializer.write("is_exit_entry", is_exit_entry);
}
-TelemetryManager::TelemetryManager(std::unique_ptr<Config> config)
+TelemetryManager::TelemetryManager(std::unique_ptr<LLDBConfig> config)
: m_config(std::move(config)), m_id(MakeUUID()) {}
llvm::Error TelemetryManager::preDispatch(TelemetryInfo *entry) {
@@ -79,23 +85,90 @@ llvm::Error TelemetryManager::preDispatch(TelemetryInfo *entry) {
return llvm::Error::success();
}
-const Config *TelemetryManager::GetConfig() { return m_config.get(); }
+void TelemetryManager::DispatchClientTelemetry(
+ const lldb_private::StructuredDataImpl &entry, Debugger *debugger) {
+ if (!m_config->m_enable_client_telemetry)
+ return;
-std::unique_ptr<TelemetryManager> TelemetryManager::g_instance = nullptr;
-TelemetryManager *TelemetryManager::GetInstance() {
- if (!Config::BuildTimeEnableTelemetry)
- return nullptr;
- return g_instance.get();
+ ClientInfo client_info;
+ client_info.debugger = debugger;
+
+ std::optional<llvm::StringRef> request_name = entry.getString("request_name");
+ if (!request_name.has_value())
+ LLDB_LOG(GetLog(LLDBLog::Object),
+ "Cannot determine request name from client-telemetry entry");
+ else
+ client_info.request_name = request_name->str();
+
+ std::optional<int64_t> start_time = entry.getInteger("start_time");
+ std::optional<int64_t> end_time = entry.getInteger("end_time");
+ SteadyTimePoint epoch;
+ if (!start_time.has_value()) {
+ LLDB_LOG(GetLog(LLDBLog::Object),
+ "Cannot determine start-time from client-telemetry entry");
+ client_info.start_time = 0;
+ } else {
+ client_info.start_time =
+ epoch + std::chrono::nanoseconds(static_cast<size_t>(*start_time));
+ }
+
+ if (!end_time.has_value()) {
+ LLDB_LOG(GetLog(LLDBLog::Object),
+ "Cannot determine end-time from client-telemetry entry");
+ } else {
+ client_info.end_time =
+ epoch + std::chrono::nanoseconds(static_cast<size_t>(*end_time));
+ }
+
+ std::optional<llvm::StringRef> error_msg = entry.getString("error");
+ if (error_msg.has_value())
+ client_info.error_msg = error_msg->str();
+
+ if (llvm::Error er = dispatch(&client_info))
+ LLDB_LOG_ERROR(GetLog(LLDBLog::Object), std::move(er),
+ "Failed to dispatch client telemetry");
}
-TelemetryManager *TelemetryManager::GetInstanceIfEnabled() {
- // Telemetry may be enabled at build time but disabled at runtime.
- if (TelemetryManager *ins = TelemetryManager::GetInstance()) {
- if (ins->GetConfig()->EnableTelemetry)
- return ins;
+class NoOpTelemetryManager : public TelemetryManager {
+public:
+ llvm::Error preDispatch(llvm::telemetry::TelemetryInfo *entry) override {
+ // Does nothing.
+ return llvm::Error::success();
+ }
+
+ explicit NoOpTelemetryManager()
+ : TelemetryManager(std::make_unique<LLDBConfig>(
+ /*EnableTelemetry*/ false, /*DetailedCommand*/ false)) {}
+
+ llvm::StringRef GetInstanceName() const override {
+ return "NoOpTelemetryManager";
+ }
+
+ llvm::Error dispatch(llvm::telemetry::TelemetryInfo *entry) override {
+ // Does nothing.
+ return llvm::Error::success();
}
- return nullptr;
+ void DispatchClientTelemetry(const lldb_private::StructuredDataImpl &entry,
+ Debugger *debugger) override {
+ // Does nothing.
+ }
+
+ static NoOpTelemetryManager *GetInstance() {
+ static std::unique_ptr<NoOpTelemetryManager> g_ins =
+ std::make_unique<NoOpTelemetryManager>();
+ return g_ins.get();
+ }
+};
+
+std::unique_ptr<TelemetryManager> TelemetryManager::g_instance = nullptr;
+TelemetryManager *TelemetryManager::GetInstance() {
+ // If Telemetry is disabled or if there is no default instance, then use the
+ // NoOp manager. We use a dummy instance to avoid having to do nullchecks in
+ // various places.
+ if (!Config::BuildTimeEnableTelemetry || !g_instance)
+ return NoOpTelemetryManager::GetInstance();
+ return g_instance.get();
}
void TelemetryManager::SetInstance(std::unique_ptr<TelemetryManager> manager) {
diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp
index 53c514b790f38..257d6cc09dcc6 100644
--- a/lldb/tools/lldb-dap/DAP.cpp
+++ b/lldb/tools/lldb-dap/DAP.cpp
@@ -754,16 +754,19 @@ PacketStatus DAP::GetNextObject(llvm::json::Object &object) {
}
bool DAP::HandleObject(const llvm::json::Object &object) {
+ TelemetryDispatcher dispatcher;
+
const auto packet_type = GetString(object, "type");
if (packet_type == "request") {
const auto command = GetString(object, "command");
-
+ dispatcher.set("request_name", command);
auto new_handler_pos = request_handlers.find(command);
if (new_handler_pos != request_handlers.end()) {
(*new_handler_pos->second)(object);
return true; // Success
}
+ dispatcher.set("error", llvm::Twine("unhandled-command:" + command).str());
if (log)
*log << "error: unhandled command \"" << command.data() << "\""
<< std::endl;
diff --git a/lldb/tools/lldb-dap/LLDBUtils.h b/lldb/tools/lldb-dap/LLDBUtils.h
index a9e13bb3678da..a52f8637a9bb4 100644
--- a/lldb/tools/lldb-dap/LLDBUtils.h
+++ b/lldb/tools/lldb-dap/LLDBUtils.h
@@ -16,6 +16,7 @@
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/JSON.h"
#include "llvm/Support/raw_ostream.h"
+#include <chrono>
#include <string>
namespace lldb_dap {
@@ -154,6 +155,39 @@ uint32_t GetLLDBFrameID(uint64_t dap_frame_id);
lldb::SBEnvironment
GetEnvironmentFromArguments(const llvm::json::Object &arguments);
+class TelemetryDispatcher {
+public:
+ TelemetryDispatcher(SBDebugger *debugger) {
+ m_telemetry_array =
+ ({"start_time",
+ std::chrono::steady_clock::now().time_since_epoch().count()});
+ this->debugger = debugger;
+ }
+
+ void Set(std::string key, std::string value) {
+ m_telemetry_array.push_back(llvm::json::Value{key, value})
+ }
+
+ void Set(std::string key, int64_t value) {
+ m_telemetry_array.push_back(llvm::json::Value{key, value})
+ }
+
+ ~TelemetryDispatcher() {
+ m_telemetry_array.push_back(
+ {"end_time",
+ std::chrono::steady_clock::now().time_since_epoch().count()});
+ lldb::SBStructuredData telemetry_entry;
+ llvm::json::Value val(std::move(telemetry_array));
+ std::string string_rep = lldb_dap::JSONToString(val);
+ telemetry_entry.SetFromJSON(string_rep.c_str());
+ debugger->DispatchClientTelemetry(telemetry_entry);
+ }
+
+private:
+ llvm::json::Array m_telemetry_array;
+ SBDebugger *debugger;
+};
+
} // namespace lldb_dap
#endif
diff --git a/lldb/unittests/Core/TelemetryTest.cpp b/lldb/unittests/Core/TelemetryTest.cpp
index 0e9f329110872..065a57114181e 100644
--- a/lldb/unittests/Core/TelemetryTest.cpp
+++ b/lldb/unittests/Core/TelemetryTest.cpp
@@ -52,7 +52,7 @@ class FakePlugin : public telemetry::TelemetryManager {
public:
FakePlugin()
: telemetry::TelemetryManager(
- std::make_unique<llvm::telemetry::Config>(true)) {}
+ std::make_unique<telemetry::LLDBConfig>(true, true)) {}
// TelemetryManager interface
llvm::Error preDispatch(llvm::telemetry::TelemetryInfo *entry) override {
>From c0cf1c0938f9886c281936e8f84affbae7ead0a4 Mon Sep 17 00:00:00 2001
From: Vy Nguyen <vyng at google.com>
Date: Tue, 4 Mar 2025 11:10:33 -0500
Subject: [PATCH 2/3] formatting
---
lldb/include/lldb/Core/Telemetry.h | 78 +++++++++++-------------------
1 file changed, 29 insertions(+), 49 deletions(-)
diff --git a/lldb/include/lldb/Core/Telemetry.h b/lldb/include/lldb/Core/Telemetry.h
index cad4a4a6c9048..e2c45fd0dd715 100644
--- a/lldb/include/lldb/Core/Telemetry.h
+++ b/lldb/include/lldb/Core/Telemetry.h
@@ -19,7 +19,6 @@
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/JSON.h"
#include "llvm/Telemetry/Telemetry.h"
-#include <atomic>
#include <chrono>
#include <ctime>
#include <memory>
@@ -30,19 +29,13 @@ namespace lldb_private {
namespace telemetry {
struct LLDBConfig : public ::llvm::telemetry::Config {
- // If true, we will collect full details about a debug command (eg., args and
- // original command). Note: This may contain PII, hence can only be enabled by
- // the vendor while creating the Manager.
- const bool m_detailed_command_telemetry;
// If true, we will collect telemetry from LLDB's clients (eg., lldb-dap) via
// the SB interface. Must also be enabled by the vendor while creating the
// manager.
const bool m_enable_client_telemetry;
- explicit LLDBConfig(bool enable_telemetry, bool detailed_command_telemetry,
- bool enable_client_telemetry)
+ explicit LLDBConfig(bool enable_telemetry, bool enable_client_telemetry)
: ::llvm::telemetry::Config(enable_telemetry),
- m_detailed_command_telemetry(detailed_command_telemetry),
m_enable_client_telemetry(enable_client_telemetry) {}
};
@@ -85,6 +78,11 @@ struct LLDBBaseTelemetryInfo : public llvm::telemetry::TelemetryInfo {
void serialize(llvm::telemetry::Serializer &serializer) const override;
};
+struct ClientInfo : public LLDBBaseTelemetryInfo {
+ std::string request_name;
+ std::optional<std::string> error_msg;
+};
+
struct DebuggerInfo : public LLDBBaseTelemetryInfo {
std::string lldb_version;
@@ -105,11 +103,6 @@ struct DebuggerInfo : public LLDBBaseTelemetryInfo {
void serialize(llvm::telemetry::Serializer &serializer) const override;
};
-struct ClientInfo : public LLDBBaseTelemetryInfo {
- std::string request_name;
- std::optional<std::string> error_msg;
-};
-
/// The base Telemetry manager instance in LLDB.
/// This class declares additional instrumentation points
/// applicable to LLDB.
@@ -119,8 +112,9 @@ class TelemetryManager : public llvm::telemetry::Manager {
const LLDBConfig *GetConfig() { return m_config.get(); }
- void DispatchClientTelemery(const lldb_private::StructuredDataImpl &entry,
- Debugger *debugger);
+ virtual void
+ DispatchClientTelemery(const lldb_private::StructuredDataImpl &entry,
+ Debugger *debugger);
virtual llvm::StringRef GetInstanceName() const = 0;
@@ -135,6 +129,7 @@ class TelemetryManager : public llvm::telemetry::Manager {
std::unique_ptr<LLDBConfig> m_config;
// Each instance of a TelemetryManager is assigned a unique ID.
const std::string m_id;
+
static std::unique_ptr<TelemetryManager> g_instance;
};
@@ -142,54 +137,39 @@ class TelemetryManager : public llvm::telemetry::Manager {
template <typename Info> 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(Debugger *debugger = nullptr) {
- // Start the timer.
- m_start_time = std::chrono::steady_clock::now();
- this->debugger = debugger;
- }
- ScopedDispatcher(llvm::unique_function<void(Info *info)> final_callback,
+ ScopedDispatcher(llvm::unique_function<void(Info *info)> callback,
Debugger *debugger = nullptr)
- : m_final_callback(std::move(final_callback)) {
+ : m_callback(std::move(callback)) {
// Start the timer.
m_start_time = std::chrono::steady_clock::now();
- this->debugger = debugger;
+ m_info.debugger = debugger;
}
- void SetDebugger(Debugger *debugger) { this->debugger = debugger; }
+ void SetDebugger(Debugger *debugger) { m_info.debugger = debugger; }
- void DispatchOnExit(llvm::unique_function<void(Info *info)> final_callback) {
- // We probably should not be overriding previously set cb.
- assert(!m_final_callback);
- m_final_callback = std::move(final_callback);
- }
-
- void DispatchNow(llvm::unique_function<void(Info *info)> populate_fields_cb) {
- TelemetryManager *manager = TelemetryManager::GetInstance();
- if (!manager->GetConfig()->EnableTelemetry)
+ ~ScopedDispatcher() {
+ // If Telemetry is disabled (either at buildtime or runtime),
+ // then don't do anything.
+ TelemetryManager *manager = TelemetryManager::GetInstanceIfEnabled();
+ if (!manager)
return;
- Info info;
- // Populate the common fields we know aboutl
- info.start_time = m_start_time;
- info.end_time = std::chrono::steady_clock::now();
- info.debugger = debugger;
- // The callback will set the rest.
- populate_fields_cb(&info);
+
+ 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(&info)) {
+ if (llvm::Error er = manager->dispatch(&m_info)) {
LLDB_LOG_ERROR(GetLog(LLDBLog::Object), std::move(er),
- "Failed to dispatch entry of type: {0}", info.getKind());
+ "Failed to dispatch entry of type: {0}", m_info.getKind());
}
}
- ~ScopedDispatcher() {
- if (m_final_callback)
- DispatchNow(std::move(m_final_callback));
- }
-
private:
SteadyTimePoint m_start_time;
- llvm::unique_function<void(Info *info)> m_final_callback;
- Debugger *debugger;
+ llvm::unique_function<void(Info *info)> m_callback;
+ Info m_info;
};
} // namespace telemetry
>From 11fd330f933c0ea3f1e9d27b6d0c319b2c506d55 Mon Sep 17 00:00:00 2001
From: Vy Nguyen <vyng at google.com>
Date: Wed, 19 Mar 2025 16:17:26 -0400
Subject: [PATCH 3/3] reconcile with new api change
---
lldb/include/lldb/Core/Telemetry.h | 16 +++++----
lldb/source/Core/Debugger.cpp | 4 +--
lldb/source/Core/Telemetry.cpp | 51 ++++++++++++++-------------
lldb/tools/lldb-dap/DAP.cpp | 7 ++--
lldb/tools/lldb-dap/LLDBUtils.h | 36 ++++++++++++-------
lldb/unittests/Core/TelemetryTest.cpp | 3 +-
6 files changed, 68 insertions(+), 49 deletions(-)
diff --git a/lldb/include/lldb/Core/Telemetry.h b/lldb/include/lldb/Core/Telemetry.h
index aadf15bd01abe..2cf2c0bd85adc 100644
--- a/lldb/include/lldb/Core/Telemetry.h
+++ b/lldb/include/lldb/Core/Telemetry.h
@@ -36,15 +36,16 @@ struct LLDBConfig : public ::llvm::telemetry::Config {
// the vendor while creating the Manager.
const bool detailed_command_telemetry;
- // If true, we will collect telemetry from LLDB's clients (eg., lldb-dap) via
+ // If true, we will collect telemetry from LLDB's clients (eg., lldb-dap) via
// the SB interface. Must also be enabled by the vendor while creating the
// manager.
const bool enable_client_telemetry;
-
- explicit LLDBConfig(bool enable_telemetry, bool detailed_command_telemetry, bool enable_client_telemetry)
+
+ explicit LLDBConfig(bool enable_telemetry, bool detailed_command_telemetry,
+ bool enable_client_telemetry)
: ::llvm::telemetry::Config(enable_telemetry),
detailed_command_telemetry(detailed_command_telemetry),
- enable_client_telemetry(enable_client_telemetry){}
+ enable_client_telemetry(enable_client_telemetry) {}
};
// We expect each (direct) subclass of LLDBTelemetryInfo to
@@ -57,6 +58,7 @@ struct LLDBConfig : public ::llvm::telemetry::Config {
struct LLDBEntryKind : public ::llvm::telemetry::EntryKind {
static const llvm::telemetry::KindType BaseInfo = 0b11000000;
static const llvm::telemetry::KindType ClientInfo = 0b11100000;
+ static const llvm::telemetry::KindType CommandInfo = 0b11010000;
static const llvm::telemetry::KindType DebuggerInfo = 0b11000100;
};
@@ -89,8 +91,10 @@ struct LLDBBaseTelemetryInfo : public llvm::telemetry::TelemetryInfo {
struct ClientInfo : public LLDBBaseTelemetryInfo {
std::string request_name;
std::optional<std::string> error_msg;
+
+ void serialize(llvm::telemetry::Serializer &serializer) const override;
};
-
+
struct CommandInfo : public LLDBBaseTelemetryInfo {
/// If the command is/can be associated with a target entry this field
/// contains that target's UUID. <EMPTY> otherwise.
@@ -167,7 +171,7 @@ class TelemetryManager : public llvm::telemetry::Manager {
const LLDBConfig *GetConfig() { return m_config.get(); }
virtual void
- DispatchClientTelemery(const lldb_private::StructuredDataImpl &entry,
+ DispatchClientTelemetry(const lldb_private::StructuredDataImpl &entry,
Debugger *debugger);
virtual llvm::StringRef GetInstanceName() const = 0;
diff --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp
index 3a070b6252a72..a19a3d940934d 100644
--- a/lldb/source/Core/Debugger.cpp
+++ b/lldb/source/Core/Debugger.cpp
@@ -783,8 +783,8 @@ DebuggerSP Debugger::CreateInstance(lldb::LogOutputCallback log_callback,
void Debugger::DispatchClientTelemetry(
const lldb_private::StructuredDataImpl &entry) {
- lldb_private::telemetry::TelemeryManager::GetInstance()
- ->DispatchClientTelemetry(entry);
+ lldb_private::telemetry::TelemetryManager::GetInstance()
+ ->DispatchClientTelemetry(entry, this);
}
void Debugger::HandleDestroyCallback() {
diff --git a/lldb/source/Core/Telemetry.cpp b/lldb/source/Core/Telemetry.cpp
index 51df8826e701e..251b065654921 100644
--- a/lldb/source/Core/Telemetry.cpp
+++ b/lldb/source/Core/Telemetry.cpp
@@ -106,49 +106,52 @@ llvm::Error TelemetryManager::preDispatch(TelemetryInfo *entry) {
void TelemetryManager::DispatchClientTelemetry(
const lldb_private::StructuredDataImpl &entry, Debugger *debugger) {
- if (!m_config->m_enable_client_telemetry)
+ if (!m_config->enable_client_telemetry)
return;
ClientInfo client_info;
client_info.debugger = debugger;
+ auto* dict = entry.GetObjectSP()->GetAsDictionary();
- std::optional<llvm::StringRef> request_name = entry.getString("request_name");
- if (!request_name.has_value())
+ llvm::StringRef request_name;
+ if (dict->GetValueForKeyAsString("request_name", request_name))
+ client_info.request_name = request_name.str();
+ else
LLDB_LOG(GetLog(LLDBLog::Object),
"Cannot determine request name from client-telemetry entry");
- else
- client_info.request_name = request_name->str();
- std::optional<int64_t> start_time = entry.getInteger("start_time");
- std::optional<int64_t> end_time = entry.getInteger("end_time");
SteadyTimePoint epoch;
- if (!start_time.has_value()) {
+ int64_t start_time;
+ if (dict->GetValueForKeyAsInteger("start_time", start_time)) {
+ client_info.start_time =
+ epoch + std::chrono::nanoseconds(static_cast<size_t>(start_time));
+ } else {
LLDB_LOG(GetLog(LLDBLog::Object),
"Cannot determine start-time from client-telemetry entry");
- client_info.start_time = 0;
- } else {
- client_info.start_time =
- epoch + std::chrono::nanoseconds(static_cast<size_t>(*start_time));
+ client_info.start_time = epoch;
+
}
- if (!end_time.has_value()) {
+ int64_t end_time;
+ if (dict->GetValueForKeyAsInteger("end_time", end_time)) {
+ client_info.end_time =
+ epoch + std::chrono::nanoseconds(static_cast<size_t>(end_time));
+ } else {
LLDB_LOG(GetLog(LLDBLog::Object),
"Cannot determine end-time from client-telemetry entry");
- } else {
- client_info.end_time =
- epoch + std::chrono::nanoseconds(static_cast<size_t>(*end_time));
+ client_info.end_time = epoch;
}
- std::optional<llvm::StringRef> error_msg = entry.getString("error");
- if (error_msg.has_value())
- client_info.error_msg = error_msg->str();
+ llvm::StringRef error_msg;
+ if (dict->GetValueForKeyAsString("error", error_msg)) {
+ client_info.error_msg = error_msg.str();
+ }
if (llvm::Error er = dispatch(&client_info))
LLDB_LOG_ERROR(GetLog(LLDBLog::Object), std::move(er),
"Failed to dispatch client telemetry");
}
-
class NoOpTelemetryManager : public TelemetryManager {
public:
llvm::Error preDispatch(llvm::telemetry::TelemetryInfo *entry) override {
@@ -158,17 +161,17 @@ class NoOpTelemetryManager : public TelemetryManager {
explicit NoOpTelemetryManager()
: TelemetryManager(std::make_unique<LLDBConfig>(
- /*EnableTelemetry*/ false, /*DetailedCommand*/ false)) {}
+ /*EnableTelemetry*/ false, /*DetailedCommand*/ false, /*ClientTelemery*/ false)) {}
virtual llvm::StringRef GetInstanceName() const override {
return "NoOpTelemetryManager";
}
- DispatchClientTelemetry(
- const lldb_private::StructuredDataImpl &entry, Debugger *debugger) override {
+ void DispatchClientTelemetry(const lldb_private::StructuredDataImpl &entry,
+ Debugger *debugger) override {
// Does nothing.
}
-
+
llvm::Error dispatch(llvm::telemetry::TelemetryInfo *entry) override {
// Does nothing.
return llvm::Error::success();
diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp
index 0e7a1a1d08fc6..9bfc27b691196 100644
--- a/lldb/tools/lldb-dap/DAP.cpp
+++ b/lldb/tools/lldb-dap/DAP.cpp
@@ -673,16 +673,17 @@ void DAP::SetTarget(const lldb::SBTarget target) {
}
bool DAP::HandleObject(const protocol::Message &M) {
- TelemetryDispatcher dispatcher;
+ TelemetryDispatcher dispatcher(&debugger);
if (const auto *req = std::get_if<protocol::Request>(&M)) {
auto handler_pos = request_handlers.find(req->command);
- dispatcher.set("request_name", req->command);
+ dispatcher.Set("request_name", req->command);
if (handler_pos != request_handlers.end()) {
(*handler_pos->second)(*req);
return true; // Success
}
- dispatcher.set("error", llvm::Twine("unhandled-command:" + req->command).str());
+ dispatcher.Set("error",
+ llvm::Twine("unhandled-command:" + req->command).str());
DAP_LOG(log, "({0}) error: unhandled command '{1}'",
transport.GetClientName(), req->command);
return false; // Fail
diff --git a/lldb/tools/lldb-dap/LLDBUtils.h b/lldb/tools/lldb-dap/LLDBUtils.h
index dddd6ef10b84b..71df8a924044c 100644
--- a/lldb/tools/lldb-dap/LLDBUtils.h
+++ b/lldb/tools/lldb-dap/LLDBUtils.h
@@ -157,37 +157,47 @@ uint32_t GetLLDBFrameID(uint64_t dap_frame_id);
lldb::SBEnvironment
GetEnvironmentFromArguments(const llvm::json::Object &arguments);
+/// Helper for sending telemetry to lldb server.
class TelemetryDispatcher {
public:
- TelemetryDispatcher(SBDebugger *debugger) {
- m_telemetry_array =
- ({"start_time",
- std::chrono::steady_clock::now().time_since_epoch().count()});
+ TelemetryDispatcher(lldb::SBDebugger *debugger) {
+ m_telemetry_json = llvm::json::Object();
+ m_telemetry_json.try_emplace(
+ "start_time",
+ std::chrono::steady_clock::now().time_since_epoch().count());
this->debugger = debugger;
}
void Set(std::string key, std::string value) {
- m_telemetry_array.push_back(llvm::json::Value{key, value})
+ m_telemetry_json.try_emplace(key, value);
}
void Set(std::string key, int64_t value) {
- m_telemetry_array.push_back(llvm::json::Value{key, value})
+ m_telemetry_json.try_emplace(key, value);
}
~TelemetryDispatcher() {
- m_telemetry_array.push_back(
- {"end_time",
- std::chrono::steady_clock::now().time_since_epoch().count()});
+ m_telemetry_json.try_emplace(
+ "end_time",
+ std::chrono::steady_clock::now().time_since_epoch().count());
+
lldb::SBStructuredData telemetry_entry;
- llvm::json::Value val(std::move(telemetry_array));
- std::string string_rep = lldb_dap::JSONToString(val);
+ llvm::json::Value val(std::move(m_telemetry_json));
+
+ std::string string_rep = JSONToString(val);
telemetry_entry.SetFromJSON(string_rep.c_str());
debugger->DispatchClientTelemetry(telemetry_entry);
}
private:
- llvm::json::Array m_telemetry_array;
- SBDebugger *debugger;
+ static std::string JSONToString(const llvm::json::Value &json) {
+ std::string data;
+ llvm::raw_string_ostream os(data);
+ os << json;
+ return data;
+}
+ llvm::json::Object m_telemetry_json;
+ lldb::SBDebugger *debugger;
};
/// Take ownership of the stored error.
diff --git a/lldb/unittests/Core/TelemetryTest.cpp b/lldb/unittests/Core/TelemetryTest.cpp
index 2c5e0a8c82328..2d9c30b63cdff 100644
--- a/lldb/unittests/Core/TelemetryTest.cpp
+++ b/lldb/unittests/Core/TelemetryTest.cpp
@@ -53,7 +53,8 @@ class FakePlugin : public telemetry::TelemetryManager {
public:
FakePlugin()
: telemetry::TelemetryManager(std::make_unique<telemetry::LLDBConfig>(
- /*enable_telemetry=*/true, /*detailed_command_telemetry=*/true, /*enable_client_telemetry*/true)) {}
+ /*enable_telemetry=*/true, /*detailed_command_telemetry=*/true,
+ /*enable_client_telemetry*/ true)) {}
// TelemetryManager interface
llvm::Error preDispatch(llvm::telemetry::TelemetryInfo *entry) override {
More information about the lldb-commits
mailing list