Merge branch 'main' into client

lldb/include/lldb/Core/Telemetry.h

@@ -31,14 +31,20 @@ namespace lldb_private {
 namespace telemetry {
 
 struct LLDBConfig : public ::llvm::telemetry::Config {
-  // If true, we will collect telemetry from LLDB's clients (eg., lldb-dap) via
+  // 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 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 enable_client_telemetry)
+  const bool enable_client_telemetry;
+  
+  explicit LLDBConfig(bool enable_telemetry, bool detailed_command_telemetry, bool enable_client_telemetry)
       : ::llvm::telemetry::Config(enable_telemetry),
-        m_enable_client_telemetry(enable_client_telemetry) {}
+        detailed_command_telemetry(detailed_command_telemetry),
+        enable_client_telemetry(enable_client_telemetry){}
 };
 
 // We expect each (direct) subclass of LLDBTelemetryInfo to
@@ -84,6 +90,52 @@ struct ClientInfo : public LLDBBaseTelemetryInfo {
   std::string request_name;
   std::optional<std::string> error_msg;
 };
+
+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;
+  /// 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.
+  uint64_t command_id;
+  /// The command name(eg., "breakpoint set")
+  std::string command_name;
+  /// These two fields are not collected by default due to PII risks.
+  /// Vendor may allow them by setting the
+  /// LLDBConfig::detailed_command_telemetry.
+  /// @{
+  std::optional<std::string> original_command;
+  std::optional<std::string> args;
+  /// @}
+  /// Return status of a command and any error description in case of error.
+  std::optional<lldb::ReturnStatus> ret_status;
+  std::optional<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(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 {
   std::string lldb_version;
@@ -117,7 +169,6 @@ class TelemetryManager : public llvm::telemetry::Manager {
   virtual void
   DispatchClientTelemery(const lldb_private::StructuredDataImpl &entry,
                          Debugger *debugger);
-
   virtual llvm::StringRef GetInstanceName() const = 0;
 
   static TelemetryManager *GetInstance();

lldb/source/Core/Telemetry.cpp

@@ -89,6 +89,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 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()) {}
 
@@ -145,6 +148,7 @@ void TelemetryManager::DispatchClientTelemetry(
                    "Failed to dispatch client telemetry");
 }
 
+
 class NoOpTelemetryManager : public TelemetryManager {
 public:
   llvm::Error preDispatch(llvm::telemetry::TelemetryInfo *entry) override {
@@ -156,18 +160,18 @@ class NoOpTelemetryManager : public TelemetryManager {
       : TelemetryManager(std::make_unique<LLDBConfig>(
             /*EnableTelemetry*/ false, /*DetailedCommand*/ false)) {}
 
-  llvm::StringRef GetInstanceName() const override {
+  virtual llvm::StringRef GetInstanceName() const override {
     return "NoOpTelemetryManager";
   }
 
-  llvm::Error dispatch(llvm::telemetry::TelemetryInfo *entry) override {
+  DispatchClientTelemetry(
+    const lldb_private::StructuredDataImpl &entry, Debugger *debugger) override {
     // Does nothing.
-    return llvm::Error::success();
   }
-
-  void DispatchClientTelemetry(const lldb_private::StructuredDataImpl &entry,
-                               Debugger *debugger) override {
+
+  llvm::Error dispatch(llvm::telemetry::TelemetryInfo *entry) override {
     // Does nothing.
+    return llvm::Error::success();
   }
 
   static NoOpTelemetryManager *GetInstance() {

lldb/tools/lldb-dap/DAP.cpp

@@ -672,56 +672,19 @@ void DAP::SetTarget(const lldb::SBTarget target) {
   }
 }
 
-PacketStatus DAP::GetNextObject(llvm::json::Object &object) {
-  std::string json = ReadJSON();
-  if (json.empty())
-    return PacketStatus::EndOfFile;
-
-  llvm::StringRef json_sref(json);
-  llvm::Expected<llvm::json::Value> json_value = llvm::json::parse(json_sref);
-  if (!json_value) {
-    auto error = json_value.takeError();
-    if (log) {
-      std::string error_str;
-      llvm::raw_string_ostream strm(error_str);
-      strm << error;
-      *log << "error: failed to parse JSON: " << error_str << std::endl
-           << json << std::endl;
-    }
-    return PacketStatus::JSONMalformed;
-  }
-
-  if (log) {
-    *log << llvm::formatv("{0:2}", *json_value).str() << std::endl;
-  }
-
-  llvm::json::Object *object_ptr = json_value->getAsObject();
-  if (!object_ptr) {
-    if (log)
-      *log << "error: json packet isn't a object" << std::endl;
-    return PacketStatus::JSONNotObject;
-  }
-  object = *object_ptr;
-  return PacketStatus::Success;
-}
-
-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);
+bool DAP::HandleObject(const protocol::Message &M) {
+ TelemetryDispatcher dispatcher;
+  if (const auto *req = std::get_if<protocol::Request>(&M)) {
+    auto handler_pos = request_handlers.find(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:" + command).str());
-    if (log)
-      *log << "error: unhandled command \"" << command.data() << "\""
-           << std::endl;
+    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
   }
 

lldb/tools/lldb-dap/LLDBUtils.h

@@ -190,6 +190,9 @@ class TelemetryDispatcher {
   SBDebugger *debugger;
 };
 
+/// Take ownership of the stored error.
+llvm::Error ToError(const lldb::SBError &error);
+
 } // namespace lldb_dap
 
 #endif

lldb/unittests/Core/TelemetryTest.cpp

@@ -52,8 +52,8 @@ class TestDestination : public llvm::telemetry::Destination {
 class FakePlugin : public telemetry::TelemetryManager {
 public:
   FakePlugin()
-      : telemetry::TelemetryManager(
-            std::make_unique<telemetry::LLDBConfig>(true, true)) {}
+      : telemetry::TelemetryManager(std::make_unique<telemetry::LLDBConfig>(
+            /*enable_telemetry=*/true, /*detailed_command_telemetry=*/true, /*enable_client_telemetry*/true)) {}
 
   // TelemetryManager interface
   llvm::Error preDispatch(llvm::telemetry::TelemetryInfo *entry) override {