[Lldb-commits] [lldb] [LLDB][Telemetry]Defind telemetry::CommandInfo (PR #129354)
Vy Nguyen via lldb-commits
lldb-commits at lists.llvm.org
Wed Mar 5 07:00:11 PST 2025
https://github.com/oontvoo updated https://github.com/llvm/llvm-project/pull/129354
>From 5a992aff351a93ff820d15ace3ebc2bea59dd5fc Mon Sep 17 00:00:00 2001
From: Vy Nguyen <vyng at google.com>
Date: Fri, 28 Feb 2025 23:03:35 -0500
Subject: [PATCH 01/17] [LLDB][Telemetry]Defind telemetry::CommandInfo and
collect telemetry about a command's execution.
---
lldb/include/lldb/Core/Telemetry.h | 127 +++++++++++++++++-
lldb/source/Core/Telemetry.cpp | 14 ++
.../source/Interpreter/CommandInterpreter.cpp | 20 +++
3 files changed, 158 insertions(+), 3 deletions(-)
diff --git a/lldb/include/lldb/Core/Telemetry.h b/lldb/include/lldb/Core/Telemetry.h
index b72556ecaf3c9..30b8474156124 100644
--- a/lldb/include/lldb/Core/Telemetry.h
+++ b/lldb/include/lldb/Core/Telemetry.h
@@ -12,11 +12,14 @@
#include "lldb/Core/StructuredDataImpl.h"
#include "lldb/Interpreter/CommandReturnObject.h"
#include "lldb/Utility/StructuredData.h"
+#include "lldb/Utility/LLDBLog.h"
#include "lldb/lldb-forward.h"
#include "llvm/ADT/StringExtras.h"
#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/FunctionExtras.h"
#include "llvm/Support/JSON.h"
#include "llvm/Telemetry/Telemetry.h"
+#include <atomic>
#include <chrono>
#include <ctime>
#include <memory>
@@ -27,8 +30,16 @@
namespace lldb_private {
namespace telemetry {
+struct LLDBConfig : public ::llvm::telemetry::Config {
+ const bool m_collect_original_command;
+
+ explicit LLDBConfig(bool enable_telemetry, bool collect_original_command)
+ : ::llvm::telemetry::Config(enable_telemetry), m_collect_original_command(collect_original_command) {}
+};
+
struct LLDBEntryKind : public ::llvm::telemetry::EntryKind {
- static const llvm::telemetry::KindType BaseInfo = 0b11000;
+ static const llvm::telemetry::KindType BaseInfo = 0b11000000;
+ static const llvm::telemetry::KindType CommandInfo = 0b11010000;
};
/// Defines a convenient type for timestamp of various events.
@@ -41,6 +52,7 @@ struct LLDBBaseTelemetryInfo : public llvm::telemetry::TelemetryInfo {
std::optional<SteadyTimePoint> end_time;
// TBD: could add some memory stats here too?
+ lldb::user_id_t debugger_id = LLDB_INVALID_UID;
Debugger *debugger;
// For dyn_cast, isa, etc operations.
@@ -56,6 +68,42 @@ struct LLDBBaseTelemetryInfo : public llvm::telemetry::TelemetryInfo {
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.
+ std::string target_uuid;
+ // A unique ID for a command so the manager can match the start entry with
+ // its end entry. These values only need to be unique within the same session.
+ // Necessary because we'd send off an entry right before a command's execution
+ // and another right after. This is to avoid losing telemetry if the command
+ // does not execute successfully.
+ int command_id;
+
+ // Eg., "breakpoint set"
+ std::string command_name;
+
+ // !!NOTE!! These two fields are not collected (upstream) due to PII risks.
+ // (Downstream impl may add them if needed).
+ // std::string original_command;
+ // std::string args;
+
+ lldb::ReturnStatus ret_status;
+ std::string error_data;
+
+
+ CommandInfo() = default;
+
+ llvm::telemetry::KindType getKind() const override { return LLDBEntryKind::CommandInfo; }
+
+ static bool classof(const llvm::telemetry::TelemetryInfo *T) {
+ return (T->getKind() & LLDBEntryKind::CommandInfo) == LLDBEntryKind::CommandInfo;
+ }
+
+ void serialize(Serializer &serializer) const override;
+};
+
/// The base Telemetry manager instance in LLDB.
/// This class declares additional instrumentation points
/// applicable to LLDB.
@@ -63,19 +111,92 @@ class TelemetryManager : public llvm::telemetry::Manager {
public:
llvm::Error preDispatch(llvm::telemetry::TelemetryInfo *entry) override;
+ int MakeNextCommandId();
+
+ LLDBConfig* GetConfig() { return m_config.get(); }
+
virtual llvm::StringRef GetInstanceName() const = 0;
static TelemetryManager *getInstance();
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;
+ const std::string m_id;
+ // We assign each command (in the same session) a unique id so that their
+ // "start" and "end" entries can be matched up.
+ // These values don't need to be unique across runs (because they are
+ // secondary-key), hence a simple counter is sufficent.
+ std::atomic<int> command_id_seed = 0;
static std::unique_ptr<TelemetryManager> g_instance;
};
+/// Helper RAII class for collecting telemetry.
+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();
+ debugger = debugger;
+ }
+ ScopedDispatcher(llvm::unique_function<void(Info *info)> final_callback,
+ Debugger *debugger = nullptr)
+ : m_final_callback(std::move(final_callback)) {
+ // Start the timer.
+ m_start_time = std::chrono::steady_clock::now();
+ debugger = debugger;
+ }
+
+
+ template typename<T>
+ T GetIfEnable(llvm::unique_function<T(TelemetryManager*)> callable,
+ T default_value) {
+ TelemetryManager *manager = TelemetryManager::GetInstanceIfEnabled();
+ if (!manager)
+ return default_value;
+ return callable(manager);
+ }
+
+ void SetDebugger(Debugger *debugger) { debugger = debugger; }
+
+ void SetFinalCallback(llvm::unique_function<void(Info *info)> final_callback) {
+ m_final_callback(std::move(final_callback));
+ }
+
+ void DispatchIfEnable(llvm::unique_function<void(Info *info)> populate_fields_cb) {
+ 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);
+ // And then we dispatch.
+ 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());
+ }
+
+ }
+
+ ~ScopedDispatcher() {
+ // TODO: check if there's a cb to call?
+ DispatchIfEnable(std::move(m_final_callback));
+ }
+
+private:
+ SteadyTimePoint m_start_time;
+ llvm::unique_function<void(Info *info)> m_final_callback;
+ Debugger * debugger;
+};
+
} // namespace telemetry
} // namespace lldb_private
#endif // LLDB_CORE_TELEMETRY_H
diff --git a/lldb/source/Core/Telemetry.cpp b/lldb/source/Core/Telemetry.cpp
index 5222f76704f91..7fb32f75f474e 100644
--- a/lldb/source/Core/Telemetry.cpp
+++ b/lldb/source/Core/Telemetry.cpp
@@ -43,6 +43,16 @@ void LLDBBaseTelemetryInfo::serialize(Serializer &serializer) const {
serializer.write("end_time", ToNanosec(end_time.value()));
}
+void CommandInfo::serialize(Serializer &serializer) const {
+ LLDBBaseTelemetryInfo::serializer(serializer);
+
+ serializer.write("target_uuid", target_uuid);
+ serializer.write("command_id", command_id);
+ serializer.write("command_name", command_name);
+ serializer.write("ret_status", ret_status);
+ serializer.write("error_data", error_data);
+}
+
[[maybe_unused]] static std::string MakeUUID(Debugger *debugger) {
uint8_t random_bytes[16];
if (auto ec = llvm::getRandomBytes(random_bytes, 16)) {
@@ -66,6 +76,10 @@ llvm::Error TelemetryManager::preDispatch(TelemetryInfo *entry) {
return llvm::Error::success();
}
+int TelemetryManager::MakeNextCommandId() {
+
+}
+
std::unique_ptr<TelemetryManager> TelemetryManager::g_instance = nullptr;
TelemetryManager *TelemetryManager::getInstance() { return g_instance.get(); }
diff --git a/lldb/source/Interpreter/CommandInterpreter.cpp b/lldb/source/Interpreter/CommandInterpreter.cpp
index c363f20081f9e..aab85145b4c3b 100644
--- a/lldb/source/Interpreter/CommandInterpreter.cpp
+++ b/lldb/source/Interpreter/CommandInterpreter.cpp
@@ -47,6 +47,7 @@
#include "lldb/Core/Debugger.h"
#include "lldb/Core/PluginManager.h"
+#include "lldb/Core/Telemetry.h"
#include "lldb/Host/StreamFile.h"
#include "lldb/Utility/ErrorMessages.h"
#include "lldb/Utility/LLDBLog.h"
@@ -88,6 +89,7 @@
#include "llvm/Support/Path.h"
#include "llvm/Support/PrettyStackTrace.h"
#include "llvm/Support/ScopedPrinter.h"
+#include "llvm/Telemetry/Telemetry.h"
#if defined(__APPLE__)
#include <TargetConditionals.h>
@@ -1883,10 +1885,28 @@ bool CommandInterpreter::HandleCommand(const char *command_line,
LazyBool lazy_add_to_history,
CommandReturnObject &result,
bool force_repeat_command) {
+ lldb_private::telemetry::ScopedDispatcher<
+ lldb_private::telemetry:CommandInfo> helper;
+ const int command_id = helper.GetIfEnable<int>([](lldb_private::telemetry::TelemetryManager* ins){
+ return ins->MakeNextCommandId(); }, 0);
+
std::string command_string(command_line);
std::string original_command_string(command_string);
std::string real_original_command_string(command_string);
+ helper.DispatchIfEnable([&](lldb_private::telemetry:CommandInfo* info,
+ lldb_private::telemetry::TelemetryManager* ins){
+ info.command_id = command_id;
+ if (Target* target = GetExecutionContext().GetTargetPtr()) {
+ // If we have a target attached to this command, then get the UUID.
+ info.target_uuid = target->GetExecutableModule() != nullptr
+ ? GetExecutableModule()->GetUUID().GetAsString()
+ : "";
+ }
+ if (ins->GetConfig()->m_collect_original_command)
+ info.original_command = original_command_string;
+ });
+
Log *log = GetLog(LLDBLog::Commands);
LLDB_LOGF(log, "Processing command: %s", command_line);
LLDB_SCOPED_TIMERF("Processing command: %s.", command_line);
>From ff1ce49d92821e5a83e66fec7f3dd95b44bcaea0 Mon Sep 17 00:00:00 2001
From: Vy Nguyen <vyng at google.com>
Date: Fri, 28 Feb 2025 23:48:20 -0500
Subject: [PATCH 02/17] more data
---
lldb/include/lldb/Core/Telemetry.h | 10 ----
.../source/Interpreter/CommandInterpreter.cpp | 49 +++++++++++++------
2 files changed, 35 insertions(+), 24 deletions(-)
diff --git a/lldb/include/lldb/Core/Telemetry.h b/lldb/include/lldb/Core/Telemetry.h
index 30b8474156124..8d51963fc3172 100644
--- a/lldb/include/lldb/Core/Telemetry.h
+++ b/lldb/include/lldb/Core/Telemetry.h
@@ -151,16 +151,6 @@ template <typename Info> struct ScopedDispatcher {
debugger = debugger;
}
-
- template typename<T>
- T GetIfEnable(llvm::unique_function<T(TelemetryManager*)> callable,
- T default_value) {
- TelemetryManager *manager = TelemetryManager::GetInstanceIfEnabled();
- if (!manager)
- return default_value;
- return callable(manager);
- }
-
void SetDebugger(Debugger *debugger) { debugger = debugger; }
void SetFinalCallback(llvm::unique_function<void(Info *info)> final_callback) {
diff --git a/lldb/source/Interpreter/CommandInterpreter.cpp b/lldb/source/Interpreter/CommandInterpreter.cpp
index aab85145b4c3b..6e069be26928a 100644
--- a/lldb/source/Interpreter/CommandInterpreter.cpp
+++ b/lldb/source/Interpreter/CommandInterpreter.cpp
@@ -1886,16 +1886,18 @@ bool CommandInterpreter::HandleCommand(const char *command_line,
CommandReturnObject &result,
bool force_repeat_command) {
lldb_private::telemetry::ScopedDispatcher<
- lldb_private::telemetry:CommandInfo> helper;
- const int command_id = helper.GetIfEnable<int>([](lldb_private::telemetry::TelemetryManager* ins){
- return ins->MakeNextCommandId(); }, 0);
+ lldb_private::telemetry:CommandInfo> helper(m_debugger);
+ lldb_private::telemetry::TelemetryManager *ins = lldb_private::telemetry::TelemetryManager::GetInstanceOrDummy();
+ const int command_id = ins->MakeNextCommandId();
+
std::string command_string(command_line);
std::string original_command_string(command_string);
std::string real_original_command_string(command_string);
+ std::string parsed_command_args;
+ CommandObject *cmd_obj = nullptr;
- helper.DispatchIfEnable([&](lldb_private::telemetry:CommandInfo* info,
- lldb_private::telemetry::TelemetryManager* ins){
+ helper.DispatchIfEnable([&](lldb_private::telemetry:CommandInfo* info){
info.command_id = command_id;
if (Target* target = GetExecutionContext().GetTargetPtr()) {
// If we have a target attached to this command, then get the UUID.
@@ -1905,6 +1907,26 @@ bool CommandInterpreter::HandleCommand(const char *command_line,
}
if (ins->GetConfig()->m_collect_original_command)
info.original_command = original_command_string;
+ // The rest (eg., command_name, args, etc) hasn't been parsed yet;
+ // Those will be collected by the on-exit-callback.
+ });
+
+ helper.DispatchOnExit([&](lldb_private::telemetry:CommandInfo* info) {
+ // TODO: this is logging the time the command-handler finishes.
+ // But we may want a finer-grain durations too?
+ // (ie., the execute_time recorded below?)
+
+ info.command_id = command_id;
+ llvm::StringRef command_name =
+ cmd_obj ? cmd_obj->GetCommandName() : "<not found>";
+ info.command_name = command_name.str();
+ info.ret_status = result.GetStatus();
+ if (llvm::StringRef error_data = result.GetErrorData();
+ !error_data.empty())
+ info.error_data = error_data.str();
+
+ if (ins->GetConfig()->m_collect_original_command)
+ info.args = parsed_command_args;
});
Log *log = GetLog(LLDBLog::Commands);
@@ -2011,7 +2033,7 @@ bool CommandInterpreter::HandleCommand(const char *command_line,
// From 1 above, we can determine whether the Execute function wants raw
// input or not.
- CommandObject *cmd_obj = ResolveCommandImpl(command_string, result);
+ cmd_obj = ResolveCommandImpl(command_string, result);
// We have to preprocess the whole command string for Raw commands, since we
// don't know the structure of the command. For parsed commands, we only
@@ -2073,37 +2095,36 @@ bool CommandInterpreter::HandleCommand(const char *command_line,
if (add_to_history)
m_command_history.AppendString(original_command_string);
- std::string remainder;
const std::size_t actual_cmd_name_len = cmd_obj->GetCommandName().size();
if (actual_cmd_name_len < command_string.length())
- remainder = command_string.substr(actual_cmd_name_len);
+ parsed_command_args = command_string.substr(actual_cmd_name_len);
// Remove any initial spaces
- size_t pos = remainder.find_first_not_of(k_white_space);
+ size_t pos = parsed_command_args.find_first_not_of(k_white_space);
if (pos != 0 && pos != std::string::npos)
- remainder.erase(0, pos);
+ parsed_command_args.erase(0, pos);
LLDB_LOGF(
log, "HandleCommand, command line after removing command name(s): '%s'",
- remainder.c_str());
+ parsed_command_args.c_str());
// To test whether or not transcript should be saved, `transcript_item` is
// used instead of `GetSaveTranscript()`. This is because the latter will
// fail when the command is "settings set interpreter.save-transcript true".
if (transcript_item) {
transcript_item->AddStringItem("commandName", cmd_obj->GetCommandName());
- transcript_item->AddStringItem("commandArguments", remainder);
+ transcript_item->AddStringItem("commandArguments", parsed_command_args);
}
ElapsedTime elapsed(execute_time);
cmd_obj->SetOriginalCommandString(real_original_command_string);
// Set the indent to the position of the command in the command line.
- pos = real_original_command_string.rfind(remainder);
+ pos = real_original_command_string.rfind(parsed_command_args);
std::optional<uint16_t> indent;
if (pos != std::string::npos)
indent = pos;
result.SetDiagnosticIndent(indent);
- cmd_obj->Execute(remainder.c_str(), result);
+ cmd_obj->Execute(parsed_command_args.c_str(), result);
}
LLDB_LOGF(log, "HandleCommand, command %s",
>From 4e9d65e1c3f9c93eacca526f35207eea3b718f4e Mon Sep 17 00:00:00 2001
From: Vy Nguyen <vyng at google.com>
Date: Mon, 3 Mar 2025 10:54:40 -0500
Subject: [PATCH 03/17] formatting + renaming
---
lldb/include/lldb/Core/Telemetry.h | 10 +++-------
lldb/source/Interpreter/CommandInterpreter.cpp | 6 ++----
2 files changed, 5 insertions(+), 11 deletions(-)
diff --git a/lldb/include/lldb/Core/Telemetry.h b/lldb/include/lldb/Core/Telemetry.h
index e0334be5fa7a8..42a62cae6ee37 100644
--- a/lldb/include/lldb/Core/Telemetry.h
+++ b/lldb/include/lldb/Core/Telemetry.h
@@ -91,20 +91,16 @@ struct CommandInfo : public LLDBBaseTelemetryInfo {
// and another right after. This is to avoid losing telemetry if the command
// does not execute successfully.
int command_id;
-
// Eg., "breakpoint set"
std::string command_name;
-
// !!NOTE!! These two fields are not collected by default due to PII risks.
// Vendor may allow them by setting the LLDBConfig::m_detailed_command_telemetry.
std::string original_command;
std::string args;
-
// Return status of a command and any error description in case of error.
lldb::ReturnStatus ret_status;
std::string error_data;
-
CommandInfo() = default;
llvm::telemetry::KindType getKind() const override { return LLDBEntryKind::CommandInfo; }
@@ -115,7 +111,7 @@ struct CommandInfo : public LLDBBaseTelemetryInfo {
void serialize(Serializer &serializer) const override;
};
-
+
struct DebuggerInfo : public LLDBBaseTelemetryInfo {
std::string lldb_version;
@@ -188,11 +184,11 @@ template <typename Info> struct ScopedDispatcher {
void SetDebugger(Debugger *debugger) { debugger = debugger; }
- void SetFinalCallback(llvm::unique_function<void(Info *info)> final_callback) {
+ void DispatchOnExit(llvm::unique_function<void(Info *info)> final_callback) {
m_final_callback(std::move(final_callback));
}
- void DispatchNowIfEnable(llvm::unique_function<void(Info *info)> populate_fields_cb) {
+ void DispatchNow(llvm::unique_function<void(Info *info)> populate_fields_cb) {
TelemetryManager *manager = TelemetryManager::GetInstanceIfEnabled();
if (!manager)
return;
diff --git a/lldb/source/Interpreter/CommandInterpreter.cpp b/lldb/source/Interpreter/CommandInterpreter.cpp
index 6e069be26928a..4bd78c10e9704 100644
--- a/lldb/source/Interpreter/CommandInterpreter.cpp
+++ b/lldb/source/Interpreter/CommandInterpreter.cpp
@@ -1887,17 +1887,16 @@ bool CommandInterpreter::HandleCommand(const char *command_line,
bool force_repeat_command) {
lldb_private::telemetry::ScopedDispatcher<
lldb_private::telemetry:CommandInfo> helper(m_debugger);
- lldb_private::telemetry::TelemetryManager *ins = lldb_private::telemetry::TelemetryManager::GetInstanceOrDummy();
+ lldb_private::telemetry::TelemetryManager *ins = lldb_private::telemetry::TelemetryManager::GetInstance();
const int command_id = ins->MakeNextCommandId();
-
std::string command_string(command_line);
std::string original_command_string(command_string);
std::string real_original_command_string(command_string);
std::string parsed_command_args;
CommandObject *cmd_obj = nullptr;
- helper.DispatchIfEnable([&](lldb_private::telemetry:CommandInfo* info){
+ helper.DispatchNow([&](lldb_private::telemetry:CommandInfo* info){
info.command_id = command_id;
if (Target* target = GetExecutionContext().GetTargetPtr()) {
// If we have a target attached to this command, then get the UUID.
@@ -1915,7 +1914,6 @@ bool CommandInterpreter::HandleCommand(const char *command_line,
// TODO: this is logging the time the command-handler finishes.
// But we may want a finer-grain durations too?
// (ie., the execute_time recorded below?)
-
info.command_id = command_id;
llvm::StringRef command_name =
cmd_obj ? cmd_obj->GetCommandName() : "<not found>";
>From 22d15d4137ed0c002ff2c8f8d6bb08cab434dfed Mon Sep 17 00:00:00 2001
From: Vy Nguyen <vyng at google.com>
Date: Mon, 3 Mar 2025 11:06:44 -0500
Subject: [PATCH 04/17] renaming for clarity
---
lldb/include/lldb/Core/Telemetry.h | 5 ++---
lldb/source/Core/Telemetry.cpp | 2 ++
lldb/source/Interpreter/CommandInterpreter.cpp | 7 ++++---
3 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/lldb/include/lldb/Core/Telemetry.h b/lldb/include/lldb/Core/Telemetry.h
index 42a62cae6ee37..67412c964190e 100644
--- a/lldb/include/lldb/Core/Telemetry.h
+++ b/lldb/include/lldb/Core/Telemetry.h
@@ -189,8 +189,8 @@ template <typename Info> struct ScopedDispatcher {
}
void DispatchNow(llvm::unique_function<void(Info *info)> populate_fields_cb) {
- TelemetryManager *manager = TelemetryManager::GetInstanceIfEnabled();
- if (!manager)
+ TelemetryManager *manager = TelemetryManager::GetInstance();
+ if (!manager->GetConfig()->EnableTelemetry)
return;
Info info;
// Populate the common fields we know aboutl
@@ -204,7 +204,6 @@ template <typename Info> struct ScopedDispatcher {
LLDB_LOG_ERROR(GetLog(LLDBLog::Object), std::move(er),
"Failed to dispatch entry of type: {0}", m_info.getKind());
}
-
}
~ScopedDispatcher() {
diff --git a/lldb/source/Core/Telemetry.cpp b/lldb/source/Core/Telemetry.cpp
index 77b50778fa180..00005a6b19c58 100644
--- a/lldb/source/Core/Telemetry.cpp
+++ b/lldb/source/Core/Telemetry.cpp
@@ -65,6 +65,8 @@ void CommandInfo::serialize(Serializer &serializer) const {
serializer.write("target_uuid", target_uuid);
serializer.write("command_id", command_id);
serializer.write("command_name", command_name);
+ serializer.write("original_command", original_command);
+ serializer.write("args", args);
serializer.write("ret_status", ret_status);
serializer.write("error_data", error_data);
}
diff --git a/lldb/source/Interpreter/CommandInterpreter.cpp b/lldb/source/Interpreter/CommandInterpreter.cpp
index 4bd78c10e9704..9bab0b3cadd60 100644
--- a/lldb/source/Interpreter/CommandInterpreter.cpp
+++ b/lldb/source/Interpreter/CommandInterpreter.cpp
@@ -1887,7 +1887,8 @@ bool CommandInterpreter::HandleCommand(const char *command_line,
bool force_repeat_command) {
lldb_private::telemetry::ScopedDispatcher<
lldb_private::telemetry:CommandInfo> helper(m_debugger);
- lldb_private::telemetry::TelemetryManager *ins = lldb_private::telemetry::TelemetryManager::GetInstance();
+ lldb_private::telemetry::TelemetryManager *ins =
+ lldb_private::telemetry::TelemetryManager::GetInstance();
const int command_id = ins->MakeNextCommandId();
std::string command_string(command_line);
@@ -1896,7 +1897,7 @@ bool CommandInterpreter::HandleCommand(const char *command_line,
std::string parsed_command_args;
CommandObject *cmd_obj = nullptr;
- helper.DispatchNow([&](lldb_private::telemetry:CommandInfo* info){
+ helper.DispatchNow([&](lldb_private::telemetry : CommandInfo *info) {
info.command_id = command_id;
if (Target* target = GetExecutionContext().GetTargetPtr()) {
// If we have a target attached to this command, then get the UUID.
@@ -1910,7 +1911,7 @@ bool CommandInterpreter::HandleCommand(const char *command_line,
// Those will be collected by the on-exit-callback.
});
- helper.DispatchOnExit([&](lldb_private::telemetry:CommandInfo* info) {
+ helper.DispatchOnExit([&](lldb_private::telemetry : CommandInfo *info) {
// TODO: this is logging the time the command-handler finishes.
// But we may want a finer-grain durations too?
// (ie., the execute_time recorded below?)
>From 291e923606bbf7fce128c0a4c48126881156c2e2 Mon Sep 17 00:00:00 2001
From: Vy Nguyen <vyng at google.com>
Date: Mon, 3 Mar 2025 11:11:04 -0500
Subject: [PATCH 05/17] remove empty line
---
lldb/source/Core/Telemetry.cpp | 1 -
1 file changed, 1 deletion(-)
diff --git a/lldb/source/Core/Telemetry.cpp b/lldb/source/Core/Telemetry.cpp
index 00005a6b19c58..d7e2f2ac604a4 100644
--- a/lldb/source/Core/Telemetry.cpp
+++ b/lldb/source/Core/Telemetry.cpp
@@ -118,7 +118,6 @@ class NoOpTelemetryManager : final public TelemetryManager {
}
};
-
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.
>From 2d7d713a24a31f71b97f12764f91979e3a85b49e Mon Sep 17 00:00:00 2001
From: Vy Nguyen <vyng at google.com>
Date: Mon, 3 Mar 2025 11:35:09 -0500
Subject: [PATCH 06/17] compile error
---
lldb/include/lldb/Core/Telemetry.h | 12 ++++---
.../source/Interpreter/CommandInterpreter.cpp | 32 ++++++++++---------
2 files changed, 24 insertions(+), 20 deletions(-)
diff --git a/lldb/include/lldb/Core/Telemetry.h b/lldb/include/lldb/Core/Telemetry.h
index 67412c964190e..5bcc242ad663c 100644
--- a/lldb/include/lldb/Core/Telemetry.h
+++ b/lldb/include/lldb/Core/Telemetry.h
@@ -50,7 +50,7 @@ struct LLDBConfig : public ::llvm::telemetry::Config {
// 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;
- tatic const llvm::telemetry::KindType CommandInfo = 0b11010000;
+ static const llvm::telemetry::KindType CommandInfo = 0b11010000;
static const llvm::telemetry::KindType DebuggerInfo = 0b11000100;
};
@@ -109,7 +109,7 @@ struct CommandInfo : public LLDBBaseTelemetryInfo {
return (T->getKind() & LLDBEntryKind::CommandInfo) == LLDBEntryKind::CommandInfo;
}
- void serialize(Serializer &serializer) const override;
+ void serialize(llvm::telemetry::Serializer &serializer) const override;
};
struct DebuggerInfo : public LLDBBaseTelemetryInfo {
@@ -185,7 +185,9 @@ template <typename Info> struct ScopedDispatcher {
void SetDebugger(Debugger *debugger) { debugger = debugger; }
void DispatchOnExit(llvm::unique_function<void(Info *info)> final_callback) {
- m_final_callback(std::move(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) {
@@ -202,13 +204,13 @@ template <typename Info> struct ScopedDispatcher {
// And then we dispatch.
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)
- DispatchIfEnable(std::move(m_final_callback));
+ DispatchNow(std::move(m_final_callback));
}
private:
diff --git a/lldb/source/Interpreter/CommandInterpreter.cpp b/lldb/source/Interpreter/CommandInterpreter.cpp
index 9bab0b3cadd60..07bfce8aa8eab 100644
--- a/lldb/source/Interpreter/CommandInterpreter.cpp
+++ b/lldb/source/Interpreter/CommandInterpreter.cpp
@@ -1886,7 +1886,8 @@ bool CommandInterpreter::HandleCommand(const char *command_line,
CommandReturnObject &result,
bool force_repeat_command) {
lldb_private::telemetry::ScopedDispatcher<
- lldb_private::telemetry:CommandInfo> helper(m_debugger);
+ lldb_private::telemetry::CommandInfo>
+ helper(m_debugger);
lldb_private::telemetry::TelemetryManager *ins =
lldb_private::telemetry::TelemetryManager::GetInstance();
const int command_id = ins->MakeNextCommandId();
@@ -1897,35 +1898,36 @@ bool CommandInterpreter::HandleCommand(const char *command_line,
std::string parsed_command_args;
CommandObject *cmd_obj = nullptr;
- helper.DispatchNow([&](lldb_private::telemetry : CommandInfo *info) {
- info.command_id = command_id;
+ helper.DispatchNow([&](lldb_private::telemetry::CommandInfo *info) {
+ info->command_id = command_id;
if (Target* target = GetExecutionContext().GetTargetPtr()) {
// If we have a target attached to this command, then get the UUID.
- info.target_uuid = target->GetExecutableModule() != nullptr
- ? GetExecutableModule()->GetUUID().GetAsString()
- : "";
+ info->target_uuid =
+ target->GetExecutableModule() != nullptr
+ ? target->GetExecutableModule()->GetUUID().GetAsString()
+ : "";
}
- if (ins->GetConfig()->m_collect_original_command)
- info.original_command = original_command_string;
+ if (ins->GetConfig()->m_detailed_command_telemetry)
+ info->original_command = original_command_string;
// The rest (eg., command_name, args, etc) hasn't been parsed yet;
// Those will be collected by the on-exit-callback.
});
- helper.DispatchOnExit([&](lldb_private::telemetry : CommandInfo *info) {
+ helper.DispatchOnExit([&](lldb_private::telemetry::CommandInfo *info) {
// TODO: this is logging the time the command-handler finishes.
// But we may want a finer-grain durations too?
// (ie., the execute_time recorded below?)
- info.command_id = command_id;
+ info->command_id = command_id;
llvm::StringRef command_name =
cmd_obj ? cmd_obj->GetCommandName() : "<not found>";
- info.command_name = command_name.str();
- info.ret_status = result.GetStatus();
+ info->command_name = command_name.str();
+ info->ret_status = result.GetStatus();
if (llvm::StringRef error_data = result.GetErrorData();
!error_data.empty())
- info.error_data = error_data.str();
+ info->error_data = error_data.str();
- if (ins->GetConfig()->m_collect_original_command)
- info.args = parsed_command_args;
+ if (ins->GetConfig()->m_detailed_command_telemetry)
+ info->args = parsed_command_args;
});
Log *log = GetLog(LLDBLog::Commands);
>From f875c85fa7c23538a46daa5903f1afee5ec85fe2 Mon Sep 17 00:00:00 2001
From: Vy Nguyen <vyng at google.com>
Date: Mon, 3 Mar 2025 13:18:52 -0500
Subject: [PATCH 07/17] fixed test
---
lldb/include/lldb/Core/Telemetry.h | 2 +-
lldb/source/Core/Telemetry.cpp | 18 ++++++++++--------
lldb/source/Interpreter/CommandInterpreter.cpp | 8 ++++----
lldb/unittests/Core/TelemetryTest.cpp | 2 +-
4 files changed, 16 insertions(+), 14 deletions(-)
diff --git a/lldb/include/lldb/Core/Telemetry.h b/lldb/include/lldb/Core/Telemetry.h
index 5bcc242ad663c..d5d285f6b2047 100644
--- a/lldb/include/lldb/Core/Telemetry.h
+++ b/lldb/include/lldb/Core/Telemetry.h
@@ -161,7 +161,7 @@ class TelemetryManager : public llvm::telemetry::Manager {
// "start" and "end" entries can be matched up.
// These values don't need to be unique across runs (because they are
// secondary-key), hence a simple counter is sufficent.
- std::atomic<int> command_id_seed = 0;
+ std::atomic<int> m_command_id_seed = 0;
static std::unique_ptr<TelemetryManager> g_instance;
};
diff --git a/lldb/source/Core/Telemetry.cpp b/lldb/source/Core/Telemetry.cpp
index d7e2f2ac604a4..3c546e94711d0 100644
--- a/lldb/source/Core/Telemetry.cpp
+++ b/lldb/source/Core/Telemetry.cpp
@@ -60,7 +60,7 @@ void LLDBBaseTelemetryInfo::serialize(Serializer &serializer) const {
serializer.write("end_time", ToNanosec(end_time.value()));
}
void CommandInfo::serialize(Serializer &serializer) const {
- LLDBBaseTelemetryInfo::serializer(serializer);
+ LLDBBaseTelemetryInfo::serialize(serializer);
serializer.write("target_uuid", target_uuid);
serializer.write("command_id", command_id);
@@ -78,7 +78,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) {
@@ -94,18 +94,20 @@ int TelemetryManager::MakeNextCommandId() {
return m_command_id_seed.fetch_add(1);
}
-const LLDBConfig *TelemetryManager::GetConfig() { return m_config.get(); }
-
-class NoOpTelemetryManager : final public TelemetryManager {
- public:
+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)>) {}
+ explicit NoOpTelemetryManager()
+ : TelemetryManager(std::make_unique<LLDBConfig>(
+ /*EnableTelemetry*/ false, /*DetailedCommand*/ false)) {}
- virtual llvm::StringRef GetInstanceName() const { return "NoOpTelemetryManager"; }
+ virtual llvm::StringRef GetInstanceName() const override {
+ return "NoOpTelemetryManager";
+ }
llvm::Error dispatch(llvm::telemetry::TelemetryInfo *entry) override {
// Does nothing.
diff --git a/lldb/source/Interpreter/CommandInterpreter.cpp b/lldb/source/Interpreter/CommandInterpreter.cpp
index 07bfce8aa8eab..f8b455c12f300 100644
--- a/lldb/source/Interpreter/CommandInterpreter.cpp
+++ b/lldb/source/Interpreter/CommandInterpreter.cpp
@@ -46,6 +46,7 @@
#include "Commands/CommandObjectWatchpoint.h"
#include "lldb/Core/Debugger.h"
+#include "lldb/Core/Module.h"
#include "lldb/Core/PluginManager.h"
#include "lldb/Core/Telemetry.h"
#include "lldb/Host/StreamFile.h"
@@ -1887,7 +1888,7 @@ bool CommandInterpreter::HandleCommand(const char *command_line,
bool force_repeat_command) {
lldb_private::telemetry::ScopedDispatcher<
lldb_private::telemetry::CommandInfo>
- helper(m_debugger);
+ helper(&m_debugger);
lldb_private::telemetry::TelemetryManager *ins =
lldb_private::telemetry::TelemetryManager::GetInstance();
const int command_id = ins->MakeNextCommandId();
@@ -1922,9 +1923,8 @@ bool CommandInterpreter::HandleCommand(const char *command_line,
cmd_obj ? cmd_obj->GetCommandName() : "<not found>";
info->command_name = command_name.str();
info->ret_status = result.GetStatus();
- if (llvm::StringRef error_data = result.GetErrorData();
- !error_data.empty())
- info->error_data = error_data.str();
+ if (std::string error_str = result.GetErrorString(); !error_str.empty())
+ info->error_data = std::move(error_str);
if (ins->GetConfig()->m_detailed_command_telemetry)
info->args = parsed_command_args;
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 f025bdb5e40de32e2223a333e8cb96d06673265d Mon Sep 17 00:00:00 2001
From: Vy Nguyen <vyng at google.com>
Date: Mon, 3 Mar 2025 13:24:43 -0500
Subject: [PATCH 08/17] formatting
---
lldb/include/lldb/Core/Telemetry.h | 30 ++++++++++---------
lldb/source/Core/Telemetry.cpp | 10 ++++---
.../source/Interpreter/CommandInterpreter.cpp | 2 +-
3 files changed, 23 insertions(+), 19 deletions(-)
diff --git a/lldb/include/lldb/Core/Telemetry.h b/lldb/include/lldb/Core/Telemetry.h
index d5d285f6b2047..bec75c8f57b9d 100644
--- a/lldb/include/lldb/Core/Telemetry.h
+++ b/lldb/include/lldb/Core/Telemetry.h
@@ -13,12 +13,10 @@
#include "lldb/Interpreter/CommandReturnObject.h"
#include "lldb/Utility/LLDBLog.h"
#include "lldb/Utility/StructuredData.h"
-#include "lldb/Utility/LLDBLog.h"
#include "lldb/lldb-forward.h"
#include "llvm/ADT/FunctionExtras.h"
#include "llvm/ADT/StringExtras.h"
#include "llvm/ADT/StringRef.h"
-#include "llvm/ADT/FunctionExtras.h"
#include "llvm/Support/JSON.h"
#include "llvm/Telemetry/Telemetry.h"
#include <atomic>
@@ -31,14 +29,15 @@
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.
+ // 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;
explicit LLDBConfig(bool enable_telemetry, bool detailed_command_telemetry)
- : ::llvm::telemetry::Config(enable_telemetry), m_detailed_command_telemetry(detailed_command_telemetry) {}
+ : ::llvm::telemetry::Config(enable_telemetry),
+ m_detailed_command_telemetry(detailed_command_telemetry) {}
};
// We expect each (direct) subclass of LLDBTelemetryInfo to
@@ -94,7 +93,8 @@ struct CommandInfo : public LLDBBaseTelemetryInfo {
// Eg., "breakpoint set"
std::string command_name;
// !!NOTE!! These two fields are not collected by default due to PII risks.
- // Vendor may allow them by setting the LLDBConfig::m_detailed_command_telemetry.
+ // Vendor may allow them by setting the
+ // LLDBConfig::m_detailed_command_telemetry.
std::string original_command;
std::string args;
// Return status of a command and any error description in case of error.
@@ -103,14 +103,17 @@ struct CommandInfo : public LLDBBaseTelemetryInfo {
CommandInfo() = default;
- llvm::telemetry::KindType getKind() const override { return LLDBEntryKind::CommandInfo; }
+ llvm::telemetry::KindType getKind() const override {
+ return LLDBEntryKind::CommandInfo;
+ }
static bool classof(const llvm::telemetry::TelemetryInfo *T) {
- return (T->getKind() & LLDBEntryKind::CommandInfo) == LLDBEntryKind::CommandInfo;
+ return (T->getKind() & LLDBEntryKind::CommandInfo) ==
+ LLDBEntryKind::CommandInfo;
}
void serialize(llvm::telemetry::Serializer &serializer) const override;
- };
+};
struct DebuggerInfo : public LLDBBaseTelemetryInfo {
std::string lldb_version;
@@ -142,7 +145,7 @@ class TelemetryManager : public llvm::telemetry::Manager {
/// Returns the next unique ID to assign to a command entry.
int MakeNextCommandId();
- const LLDBConfig* GetConfig() { return m_config.get(); }
+ const LLDBConfig *GetConfig() { return m_config.get(); }
virtual llvm::StringRef GetInstanceName() const = 0;
@@ -155,7 +158,7 @@ class TelemetryManager : public llvm::telemetry::Manager {
private:
std::unique_ptr<LLDBConfig> m_config;
- // Each instance of a TelemetryManager is assigned a unique ID.
+ // Each instance of a TelemetryManager is assigned a unique ID.
const std::string m_id;
// We assign each command (in the same session) a unique id so that their
// "start" and "end" entries can be matched up.
@@ -216,8 +219,7 @@ template <typename Info> struct ScopedDispatcher {
private:
SteadyTimePoint m_start_time;
llvm::unique_function<void(Info *info)> m_final_callback;
- Debugger * debugger;
-
+ Debugger *debugger;
};
} // namespace telemetry
diff --git a/lldb/source/Core/Telemetry.cpp b/lldb/source/Core/Telemetry.cpp
index 3c546e94711d0..8c601d3677b4a 100644
--- a/lldb/source/Core/Telemetry.cpp
+++ b/lldb/source/Core/Telemetry.cpp
@@ -111,19 +111,21 @@ class NoOpTelemetryManager : public TelemetryManager {
llvm::Error dispatch(llvm::telemetry::TelemetryInfo *entry) override {
// Does nothing.
- return llvm::Error::success();
+ return llvm::Error::success();
}
static NoOpTelemetryManager *GetInstance() {
- static std::unique_ptr<NoOpTelemetryManager> g_ins = std::make_unique<NoOpTelemetryManager>();
+ 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 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();
diff --git a/lldb/source/Interpreter/CommandInterpreter.cpp b/lldb/source/Interpreter/CommandInterpreter.cpp
index f8b455c12f300..d728f3e978285 100644
--- a/lldb/source/Interpreter/CommandInterpreter.cpp
+++ b/lldb/source/Interpreter/CommandInterpreter.cpp
@@ -1901,7 +1901,7 @@ bool CommandInterpreter::HandleCommand(const char *command_line,
helper.DispatchNow([&](lldb_private::telemetry::CommandInfo *info) {
info->command_id = command_id;
- if (Target* target = GetExecutionContext().GetTargetPtr()) {
+ if (Target *target = GetExecutionContext().GetTargetPtr()) {
// If we have a target attached to this command, then get the UUID.
info->target_uuid =
target->GetExecutableModule() != nullptr
>From b859e2d871e66ef7fc53d20c3a42dc99b2c9732f Mon Sep 17 00:00:00 2001
From: Vy Nguyen <vyng at google.com>
Date: Tue, 4 Mar 2025 09:59:52 -0500
Subject: [PATCH 09/17] qual
---
lldb/include/lldb/Core/Telemetry.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/lldb/include/lldb/Core/Telemetry.h b/lldb/include/lldb/Core/Telemetry.h
index bec75c8f57b9d..19d2b72241049 100644
--- a/lldb/include/lldb/Core/Telemetry.h
+++ b/lldb/include/lldb/Core/Telemetry.h
@@ -175,17 +175,17 @@ template <typename Info> struct ScopedDispatcher {
ScopedDispatcher(Debugger *debugger = nullptr) {
// Start the timer.
m_start_time = std::chrono::steady_clock::now();
- debugger = debugger;
+ this->debugger = debugger;
}
ScopedDispatcher(llvm::unique_function<void(Info *info)> final_callback,
Debugger *debugger = nullptr)
: m_final_callback(std::move(final_callback)) {
// Start the timer.
m_start_time = std::chrono::steady_clock::now();
- debugger = debugger;
+ this->debugger = debugger;
}
- void SetDebugger(Debugger *debugger) { debugger = debugger; }
+ void SetDebugger(Debugger *debugger) { this->debugger = debugger; }
void DispatchOnExit(llvm::unique_function<void(Info *info)> final_callback) {
// We probably should not be overriding previously set cb.
>From 017be757ca436d3ff9ebf958378c8e4c85d93f5d Mon Sep 17 00:00:00 2001
From: Vy Nguyen <vyng at google.com>
Date: Tue, 4 Mar 2025 15:46:49 -0500
Subject: [PATCH 10/17] Update lldb/unittests/Core/TelemetryTest.cpp
Co-authored-by: Jonas Devlieghere <jonas at devlieghere.com>
---
lldb/unittests/Core/TelemetryTest.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lldb/unittests/Core/TelemetryTest.cpp b/lldb/unittests/Core/TelemetryTest.cpp
index 065a57114181e..0522abd400626 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<telemetry::LLDBConfig>(true, true)) {}
+ std::make_unique<telemetry::LLDBConfig>(/*enable_telemetry=*/true, /*detailed_command_telemetry=*/true)) {}
// TelemetryManager interface
llvm::Error preDispatch(llvm::telemetry::TelemetryInfo *entry) override {
>From 9d6997b1483e27f2111b288e406e4d0cb18a05d0 Mon Sep 17 00:00:00 2001
From: Vy Nguyen <vyng at google.com>
Date: Tue, 4 Mar 2025 15:47:04 -0500
Subject: [PATCH 11/17] Update lldb/source/Interpreter/CommandInterpreter.cpp
Co-authored-by: Jonas Devlieghere <jonas at devlieghere.com>
---
lldb/source/Interpreter/CommandInterpreter.cpp | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/lldb/source/Interpreter/CommandInterpreter.cpp b/lldb/source/Interpreter/CommandInterpreter.cpp
index d728f3e978285..851b915b86941 100644
--- a/lldb/source/Interpreter/CommandInterpreter.cpp
+++ b/lldb/source/Interpreter/CommandInterpreter.cpp
@@ -1886,11 +1886,11 @@ bool CommandInterpreter::HandleCommand(const char *command_line,
LazyBool lazy_add_to_history,
CommandReturnObject &result,
bool force_repeat_command) {
- lldb_private::telemetry::ScopedDispatcher<
- lldb_private::telemetry::CommandInfo>
+ telemetry::ScopedDispatcher<
+ telemetry::CommandInfo>
helper(&m_debugger);
- lldb_private::telemetry::TelemetryManager *ins =
- lldb_private::telemetry::TelemetryManager::GetInstance();
+ telemetry::TelemetryManager *ins =
+ telemetry::TelemetryManager::GetInstance();
const int command_id = ins->MakeNextCommandId();
std::string command_string(command_line);
>From 714d8038821778c0b09dfb46560d12dfc51b0a5c Mon Sep 17 00:00:00 2001
From: Vy Nguyen <vyng at google.com>
Date: Tue, 4 Mar 2025 15:47:51 -0500
Subject: [PATCH 12/17] Update lldb/include/lldb/Core/Telemetry.h
Co-authored-by: Jonas Devlieghere <jonas at 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 19d2b72241049..6e7294fd86c0b 100644
--- a/lldb/include/lldb/Core/Telemetry.h
+++ b/lldb/include/lldb/Core/Telemetry.h
@@ -83,7 +83,7 @@ struct CommandInfo : public LLDBBaseTelemetryInfo {
// If the command is/can be associated with a target entry this field contains
// that target's UUID. <EMPTY> otherwise.
- std::string target_uuid;
+ UUID target_uuid;
// A unique ID for a command so the manager can match the start entry with
// its end entry. These values only need to be unique within the same session.
// Necessary because we'd send off an entry right before a command's execution
>From 481e1413f93a28b8ad2c9f2987946165a610dfc7 Mon Sep 17 00:00:00 2001
From: Vy Nguyen <vyng at google.com>
Date: Tue, 4 Mar 2025 15:48:04 -0500
Subject: [PATCH 13/17] Update lldb/include/lldb/Core/Telemetry.h
Co-authored-by: Jonas Devlieghere <jonas at 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 6e7294fd86c0b..bf4778846b8bb 100644
--- a/lldb/include/lldb/Core/Telemetry.h
+++ b/lldb/include/lldb/Core/Telemetry.h
@@ -89,7 +89,7 @@ struct CommandInfo : public LLDBBaseTelemetryInfo {
// Necessary because we'd send off an entry right before a command's execution
// and another right after. This is to avoid losing telemetry if the command
// does not execute successfully.
- int command_id;
+ uint64_t command_id;
// Eg., "breakpoint set"
std::string command_name;
// !!NOTE!! These two fields are not collected by default due to PII risks.
>From ec598c6525a3c3b77ffe01fac39a8f14b6fa0040 Mon Sep 17 00:00:00 2001
From: Vy Nguyen <vyng at google.com>
Date: Tue, 4 Mar 2025 16:02:35 -0500
Subject: [PATCH 14/17] addressed review commeht
---
lldb/include/lldb/Core/Telemetry.h | 16 ++++++++++------
lldb/source/Core/Telemetry.cpp | 7 +++----
lldb/source/Interpreter/CommandInterpreter.cpp | 2 +-
3 files changed, 14 insertions(+), 11 deletions(-)
diff --git a/lldb/include/lldb/Core/Telemetry.h b/lldb/include/lldb/Core/Telemetry.h
index bf4778846b8bb..03bf718837397 100644
--- a/lldb/include/lldb/Core/Telemetry.h
+++ b/lldb/include/lldb/Core/Telemetry.h
@@ -113,6 +113,15 @@ struct CommandInfo : public LLDBBaseTelemetryInfo {
}
void serialize(llvm::telemetry::Serializer &serializer) const override;
+
+ static uint64_t GetNextId() {}
+
+private:
+ // We assign each command (in the same session) a unique id so that their
+ // "start" and "end" entries can be matched up.
+ // These values don't need to be unique across runs (because they are
+ // secondary-key), hence a simple counter is sufficent.
+ static std::atomic<uint64_t> g_command_id_seed;
};
struct DebuggerInfo : public LLDBBaseTelemetryInfo {
@@ -160,11 +169,6 @@ 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;
- // We assign each command (in the same session) a unique id so that their
- // "start" and "end" entries can be matched up.
- // These values don't need to be unique across runs (because they are
- // secondary-key), hence a simple counter is sufficent.
- std::atomic<int> m_command_id_seed = 0;
static std::unique_ptr<TelemetryManager> g_instance;
};
@@ -198,7 +202,7 @@ template <typename Info> struct ScopedDispatcher {
if (!manager->GetConfig()->EnableTelemetry)
return;
Info info;
- // Populate the common fields we know aboutl
+ // Populate the common fields we know about.
info.start_time = m_start_time;
info.end_time = std::chrono::steady_clock::now();
info.debugger = debugger;
diff --git a/lldb/source/Core/Telemetry.cpp b/lldb/source/Core/Telemetry.cpp
index 8c601d3677b4a..52b7e32450805 100644
--- a/lldb/source/Core/Telemetry.cpp
+++ b/lldb/source/Core/Telemetry.cpp
@@ -78,6 +78,9 @@ void DebuggerInfo::serialize(Serializer &serializer) const {
serializer.write("is_exit_entry", is_exit_entry);
}
+std::atomic<uint64_t> CommandInfo::g_command_id_seed = 0;
+uint64_t DebuggerInfo::GetNextId() { return g_command_id_seed.fetch_add(1); }
+
TelemetryManager::TelemetryManager(std::unique_ptr<LLDBConfig> config)
: m_config(std::move(config)), m_id(MakeUUID()) {}
@@ -90,10 +93,6 @@ llvm::Error TelemetryManager::preDispatch(TelemetryInfo *entry) {
return llvm::Error::success();
}
-int TelemetryManager::MakeNextCommandId() {
- return m_command_id_seed.fetch_add(1);
-}
-
class NoOpTelemetryManager : public TelemetryManager {
public:
llvm::Error preDispatch(llvm::telemetry::TelemetryInfo *entry) override {
diff --git a/lldb/source/Interpreter/CommandInterpreter.cpp b/lldb/source/Interpreter/CommandInterpreter.cpp
index 851b915b86941..58d6716dbb987 100644
--- a/lldb/source/Interpreter/CommandInterpreter.cpp
+++ b/lldb/source/Interpreter/CommandInterpreter.cpp
@@ -1891,7 +1891,7 @@ bool CommandInterpreter::HandleCommand(const char *command_line,
helper(&m_debugger);
telemetry::TelemetryManager *ins =
telemetry::TelemetryManager::GetInstance();
- const int command_id = ins->MakeNextCommandId();
+ const int command_id = telemetry::CommandInfo::GetNextId();
std::string command_string(command_line);
std::string original_command_string(command_string);
>From 46225e8701348fecc55bea05603335a33a0120c2 Mon Sep 17 00:00:00 2001
From: Vy Nguyen <vyng at google.com>
Date: Tue, 4 Mar 2025 20:48:24 -0500
Subject: [PATCH 15/17] formatting
---
lldb/include/lldb/Core/Telemetry.h | 2 +-
lldb/source/Core/Telemetry.cpp | 5 +++--
lldb/source/Interpreter/CommandInterpreter.cpp | 7 +++----
3 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/lldb/include/lldb/Core/Telemetry.h b/lldb/include/lldb/Core/Telemetry.h
index 03bf718837397..c400a79aa217b 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/LLDBLog.h"
#include "lldb/Utility/StructuredData.h"
+#include "lldb/Utility/UUID.h"
#include "lldb/lldb-forward.h"
#include "llvm/ADT/FunctionExtras.h"
#include "llvm/ADT/StringExtras.h"
@@ -80,7 +81,6 @@ struct LLDBBaseTelemetryInfo : public llvm::telemetry::TelemetryInfo {
};
struct CommandInfo : public LLDBBaseTelemetryInfo {
-
// If the command is/can be associated with a target entry this field contains
// that target's UUID. <EMPTY> otherwise.
UUID target_uuid;
diff --git a/lldb/source/Core/Telemetry.cpp b/lldb/source/Core/Telemetry.cpp
index 52b7e32450805..856ed2ac72db3 100644
--- a/lldb/source/Core/Telemetry.cpp
+++ b/lldb/source/Core/Telemetry.cpp
@@ -59,10 +59,11 @@ void LLDBBaseTelemetryInfo::serialize(Serializer &serializer) const {
if (end_time.has_value())
serializer.write("end_time", ToNanosec(end_time.value()));
}
+
void CommandInfo::serialize(Serializer &serializer) const {
LLDBBaseTelemetryInfo::serialize(serializer);
- serializer.write("target_uuid", target_uuid);
+ serializer.write("target_uuid", target_uuid.GetAsString());
serializer.write("command_id", command_id);
serializer.write("command_name", command_name);
serializer.write("original_command", original_command);
@@ -79,7 +80,7 @@ void DebuggerInfo::serialize(Serializer &serializer) const {
}
std::atomic<uint64_t> CommandInfo::g_command_id_seed = 0;
-uint64_t DebuggerInfo::GetNextId() { return g_command_id_seed.fetch_add(1); }
+uint64_t CommandInfo::GetNextId() { return g_command_id_seed.fetch_add(1); }
TelemetryManager::TelemetryManager(std::unique_ptr<LLDBConfig> config)
: m_config(std::move(config)), m_id(MakeUUID()) {}
diff --git a/lldb/source/Interpreter/CommandInterpreter.cpp b/lldb/source/Interpreter/CommandInterpreter.cpp
index 58d6716dbb987..2e4c664a0e675 100644
--- a/lldb/source/Interpreter/CommandInterpreter.cpp
+++ b/lldb/source/Interpreter/CommandInterpreter.cpp
@@ -1903,10 +1903,9 @@ bool CommandInterpreter::HandleCommand(const char *command_line,
info->command_id = command_id;
if (Target *target = GetExecutionContext().GetTargetPtr()) {
// If we have a target attached to this command, then get the UUID.
- info->target_uuid =
- target->GetExecutableModule() != nullptr
- ? target->GetExecutableModule()->GetUUID().GetAsString()
- : "";
+ info->target_uuid = target->GetExecutableModule() != nullptr
+ ? target->GetExecutableModule()->GetUUID()
+ : UUID();
}
if (ins->GetConfig()->m_detailed_command_telemetry)
info->original_command = original_command_string;
>From a5c46a1a3ae09e5bf898ea2145ad5ba90864dd5f Mon Sep 17 00:00:00 2001
From: Vy Nguyen <vyng at google.com>
Date: Tue, 4 Mar 2025 20:53:08 -0500
Subject: [PATCH 16/17] formatting again
---
lldb/source/Interpreter/CommandInterpreter.cpp | 7 ++-----
lldb/unittests/Core/TelemetryTest.cpp | 4 ++--
2 files changed, 4 insertions(+), 7 deletions(-)
diff --git a/lldb/source/Interpreter/CommandInterpreter.cpp b/lldb/source/Interpreter/CommandInterpreter.cpp
index 2e4c664a0e675..7d2d5bbdbc773 100644
--- a/lldb/source/Interpreter/CommandInterpreter.cpp
+++ b/lldb/source/Interpreter/CommandInterpreter.cpp
@@ -1886,11 +1886,8 @@ bool CommandInterpreter::HandleCommand(const char *command_line,
LazyBool lazy_add_to_history,
CommandReturnObject &result,
bool force_repeat_command) {
- telemetry::ScopedDispatcher<
- telemetry::CommandInfo>
- helper(&m_debugger);
- telemetry::TelemetryManager *ins =
- telemetry::TelemetryManager::GetInstance();
+ telemetry::ScopedDispatcher<telemetry::CommandInfo> helper(&m_debugger);
+ telemetry::TelemetryManager *ins = telemetry::TelemetryManager::GetInstance();
const int command_id = telemetry::CommandInfo::GetNextId();
std::string command_string(command_line);
diff --git a/lldb/unittests/Core/TelemetryTest.cpp b/lldb/unittests/Core/TelemetryTest.cpp
index 0522abd400626..aac3032324461 100644
--- a/lldb/unittests/Core/TelemetryTest.cpp
+++ b/lldb/unittests/Core/TelemetryTest.cpp
@@ -51,8 +51,8 @@ class TestDestination : public llvm::telemetry::Destination {
class FakePlugin : public telemetry::TelemetryManager {
public:
FakePlugin()
- : telemetry::TelemetryManager(
- std::make_unique<telemetry::LLDBConfig>(/*enable_telemetry=*/true, /*detailed_command_telemetry=*/true)) {}
+ : telemetry::TelemetryManager(std::make_unique<telemetry::LLDBConfig>(
+ /*enable_telemetry=*/true, /*detailed_command_telemetry=*/true)) {}
// TelemetryManager interface
llvm::Error preDispatch(llvm::telemetry::TelemetryInfo *entry) override {
>From ac03cb0d67895e61606424c9c5f5a0f54a8dbb53 Mon Sep 17 00:00:00 2001
From: Vy Nguyen <vyng at google.com>
Date: Wed, 5 Mar 2025 09:59:49 -0500
Subject: [PATCH 17/17] fix dup definition
---
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 c400a79aa217b..e9c92734a5e85 100644
--- a/lldb/include/lldb/Core/Telemetry.h
+++ b/lldb/include/lldb/Core/Telemetry.h
@@ -114,7 +114,7 @@ struct CommandInfo : public LLDBBaseTelemetryInfo {
void serialize(llvm::telemetry::Serializer &serializer) const override;
- static uint64_t GetNextId() {}
+ static uint64_t GetNextId();
private:
// We assign each command (in the same session) a unique id so that their
More information about the lldb-commits
mailing list