[LLDB][Telemetry]Defind telemetry::CommandInfo #129354
Open
+178
−12
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…about a command's execution.
|
@llvm/pr-subscribers-lldb Author: Vy Nguyen (oontvoo) Changesand collect telemetry about a command's execution. *NOTE: Please consider this PR a DRAFT ( Waiting on PR/127696 to be submitted. ) Full diff: https://github.com/llvm/llvm-project/pull/129354.diff 3 Files Affected:
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);
|
You can test this locally with the following command:git-clang-format --diff 823a597d2ad0a76e8d5278a789f37a07b393cd2a ff1ce49d92821e5a83e66fec7f3dd95b44bcaea0 --extensions cpp,h -- lldb/include/lldb/Core/Telemetry.h lldb/source/Core/Telemetry.cpp lldb/source/Interpreter/CommandInterpreter.cppView the diff from clang-format here.diff --git a/lldb/include/lldb/Core/Telemetry.h b/lldb/include/lldb/Core/Telemetry.h
index 8d51963fc3..8e1cea00af 100644
--- a/lldb/include/lldb/Core/Telemetry.h
+++ b/lldb/include/lldb/Core/Telemetry.h
@@ -11,12 +11,12 @@
#include "lldb/Core/StructuredDataImpl.h"
#include "lldb/Interpreter/CommandReturnObject.h"
-#include "lldb/Utility/StructuredData.h"
#include "lldb/Utility/LLDBLog.h"
+#include "lldb/Utility/StructuredData.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>
@@ -34,7 +34,8 @@ 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) {}
+ : ::llvm::telemetry::Config(enable_telemetry),
+ m_collect_original_command(collect_original_command) {}
};
struct LLDBEntryKind : public ::llvm::telemetry::EntryKind {
@@ -68,7 +69,6 @@ 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
@@ -92,13 +92,15 @@ struct CommandInfo : public LLDBBaseTelemetryInfo {
lldb::ReturnStatus ret_status;
std::string error_data;
-
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(Serializer &serializer) const override;
@@ -113,7 +115,7 @@ public:
int MakeNextCommandId();
- LLDBConfig* GetConfig() { return m_config.get(); }
+ LLDBConfig *GetConfig() { return m_config.get(); }
virtual llvm::StringRef GetInstanceName() const = 0;
static TelemetryManager *getInstance();
@@ -153,11 +155,13 @@ template <typename Info> struct ScopedDispatcher {
void SetDebugger(Debugger *debugger) { debugger = debugger; }
- void SetFinalCallback(llvm::unique_function<void(Info *info)> final_callback) {
+ 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) {
+ void
+ DispatchIfEnable(llvm::unique_function<void(Info *info)> populate_fields_cb) {
TelemetryManager *manager = TelemetryManager::GetInstanceIfEnabled();
if (!manager)
return;
@@ -173,7 +177,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() {
@@ -184,7 +187,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 7fb32f75f4..7284ef3dc0 100644
--- a/lldb/source/Core/Telemetry.cpp
+++ b/lldb/source/Core/Telemetry.cpp
@@ -76,9 +76,7 @@ llvm::Error TelemetryManager::preDispatch(TelemetryInfo *entry) {
return llvm::Error::success();
}
-int TelemetryManager::MakeNextCommandId() {
-
-}
+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 6e069be269..f6fc4c9263 100644
--- a/lldb/source/Interpreter/CommandInterpreter.cpp
+++ b/lldb/source/Interpreter/CommandInterpreter.cpp
@@ -1886,24 +1886,25 @@ 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::TelemetryManager *ins = lldb_private::telemetry::TelemetryManager::GetInstanceOrDummy();
+ 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){
+ helper.DispatchIfEnable([&](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
- ? GetExecutableModule()->GetUUID().GetAsString()
- : "";
+ ? GetExecutableModule()->GetUUID().GetAsString()
+ : "";
}
if (ins->GetConfig()->m_collect_original_command)
info.original_command = original_command_string;
@@ -1911,7 +1912,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?)
@@ -1921,8 +1922,7 @@ 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())
+ if (llvm::StringRef error_data = result.GetErrorData(); !error_data.empty())
info.error_data = error_data.str();
if (ins->GetConfig()->m_collect_original_command)
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
and collect telemetry about a command's execution.
*NOTE: Please consider this PR a DRAFT ( Waiting on PR/127696 to be submitted. )