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 <v...@google.com>
Date: Fri, 28 Feb 2025 23:03:35 -0500
Subject: [PATCH 1/2] [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 <v...@google.com>
Date: Fri, 28 Feb 2025 23:48:20 -0500
Subject: [PATCH 2/2] 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",
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
