[Lldb-commits] [lldb] [LLDB][Telemetry]Define DebuggerTelemetryInfo and related methods (PR #127696)

[Pavel Labath via lldb-commits]

================
@@ -56,16 +54,107 @@ void LLDBBaseTelemetryInfo::serialize(Serializer &serializer) const {
   return UUID(random_bytes).GetAsString();
 }
 
+void LLDBBaseTelemetryInfo::serialize(Serializer &serializer) const {
+  serializer.write("entry_kind", getKind());
+  serializer.write("session_id", SessionId);
+  serializer.write("start_time", ToNanosec(start_time));
+  if (end_time.has_value())
+    serializer.write("end_time", ToNanosec(end_time.value()));
+}
+
+void DebuggerTelemetryInfo::serialize(Serializer &serializer) const {
+  LLDBBaseTelemetryInfo::serialize(serializer);
+
+  serializer.write("username", username);
+  serializer.write("lldb_git_sha", lldb_git_sha);
+  serializer.write("lldb_path", lldb_path);
+  serializer.write("cwd", cwd);
+  if (exit_desc.has_value()) {
+    serializer.write("exit_code", exit_desc->exit_code);
+    serializer.write("exit_desc", exit_desc->description);
+  }
+}
+
+void MiscTelemetryInfo::serialize(Serializer &serializer) const {
+  LLDBBaseTelemetryInfo::serialize(serializer);
+  serializer.write("target_uuid", target_uuid);
+  serializer.beginObject("meta_data");
+  for (const auto &kv : meta_data)
+    serializer.write(kv.first, kv.second);
+  serializer.endObject();
+}
+
 TelemetryManager::TelemetryManager(std::unique_ptr<Config> config)
     : m_config(std::move(config)) {}
 
 llvm::Error TelemetryManager::preDispatch(TelemetryInfo *entry) {
-  // Do nothing for now.
-  // In up-coming patch, this would be where the manager
-  // attach the session_uuid to the entry.
+  LLDBBaseTelemetryInfo *lldb_entry =
+      llvm::dyn_cast<LLDBBaseTelemetryInfo>(entry);
+  std::string session_id = "";
+  if (Debugger *debugger = lldb_entry->debugger) {
+    auto session_id_pos = session_ids.find(debugger);
+    if (session_id_pos != session_ids.end())
+      session_id = session_id_pos->second;
+    else
+      session_id_pos->second = session_id = MakeUUID(debugger);
+  }
+  lldb_entry->SessionId = session_id;
+
   return llvm::Error::success();
 }
 
+const Config *getConfig() { return m_config.get(); }
+
+void TelemetryManager::atDebuggerStartup(DebuggerTelemetryInfo *entry) {
+  UserIDResolver &resolver = lldb_private::HostInfo::GetUserIDResolver();
+  std::optional<llvm::StringRef> opt_username =
+      resolver.GetUserName(lldb_private::HostInfo::GetUserID());
+  if (opt_username)
+    entry->username = *opt_username;
+
+  entry->lldb_git_sha =
+      lldb_private::GetVersion(); // TODO: find the real git sha?
+
+  entry->lldb_path = HostInfo::GetProgramFileSpec().GetPath();
+
+  llvm::SmallString<64> cwd;
+  if (!llvm::sys::fs::current_path(cwd)) {
+    entry->cwd = cwd.c_str();
+  } else {
+    MiscTelemetryInfo misc_info;
+    misc_info.meta_data["internal_errors"] = "Cannot determine CWD";
+    if (auto er = dispatch(&misc_info)) {
+      LLDB_LOG(GetLog(LLDBLog::Object),
+               "Failed to dispatch misc-info at startup");
+    }
+  }
+
+  if (auto er = dispatch(entry)) {
+    LLDB_LOG(GetLog(LLDBLog::Object), "Failed to dispatch entry at startup");
+  }
+}
+
+void TelemetryManager::atDebuggerExit(DebuggerTelemetryInfo *entry) {
+  // There must be a reference to the debugger at this point.
+  assert(entry->debugger != nullptr);
+
+  if (auto *selected_target =
+          entry->debugger->GetSelectedExecutionContext().GetTargetPtr()) {
+    if (!selected_target->IsDummyTarget()) {
+      const lldb::ProcessSP proc = selected_target->GetProcessSP();
+      if (proc == nullptr) {
+        // no process has been launched yet.
+        entry->exit_desc = {-1, "no process launched."};
+      } else {
+        entry->exit_desc = {proc->GetExitStatus(), ""};
+        if (const char *description = proc->GetExitDescription())
+          entry->exit_desc->description = std::string(description);
+      }
+    }
+  }
----------------
labath wrote:

> > There's no guarantee that Process object will  exist at the time the debugger terminates, or that it will have ran exactly once, etc.
> 
> Yes, that's why there' s a check - if there was no process when the debugger terminates (ie `proc == nullptr`), then we reports taht no process has been launched.

And if the process was launched twice this will get the last exit code -- is that what you want? What about the situation where the process is launched and then its target is deleted (`run; target delete; exit`)? Or when the process is launched, but then the user switches to debugging a different target (`run; target create/select; exit`)?

> 
> > If you're interesting in collecting the exit status, why not hook into a place that deals with that directly
> 
> This code is more interested in the exit of the _debugger_ (ie., did the debugger exit normally or because some process crashed? or waht ...), which is why the telemetry-callback is done from Debugger's cleanup code.

I don't see how one is relevant for the other. LLDB doesn't exit because the debugged process crashed. Any debuggee exit code is WAI as far as LLDB is concerned. I can see how you might be able to gather some very rough trends based on this information, but I think there may be better ways to achieve that (for example, I think that a better proxy for "was the user able to do something useful in this session" would be "whether the process exited without hitting any breakpoints". I also don't see why this would have to be gathered here, and I suspect its because you see lldb through the eyes of the DAP protocol, where there's a mostly one-to-one correspondence between a dap connection, a debugger object, and a debugged process. LLDB is much more general than that. It's true that a lot of command line users will use LLDB in a dap-like manner, but we actually want to discourage that as longer-lived sessions give you the benefit of all having all the information cached already.

https://github.com/llvm/llvm-project/pull/127696