[Lldb-commits] [lldb] [llvm] [lldb][telemetry] Implement LLDB Telemetry (part 1) (PR #119716)

[Pavel Labath via lldb-commits]

================
@@ -0,0 +1,101 @@
+//===-- Telemetry.h ----------------------------------------------*- C++
+//-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLDB_CORE_TELEMETRY_H
+#define LLDB_CORE_TELEMETRY_H
+
+#include <chrono>
+#include <ctime>
+#include <memory>
+#include <optional>
+#include <string>
+#include <unordered_map>
+
+#include "lldb/Core/StructuredDataImpl.h"
+#include "lldb/Interpreter/CommandReturnObject.h"
+#include "lldb/Utility/StructuredData.h"
+#include "lldb/lldb-forward.h"
+#include "llvm/ADT/StringExtras.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/JSON.h"
+#include "llvm/Telemetry/Telemetry.h"
+
+namespace lldb_private {
+
+using llvm::telemetry::Destination;
+using llvm::telemetry::KindType;
+using llvm::telemetry::Serializer;
+using llvm::telemetry::TelemetryInfo;
+
+struct LldbEntryKind : public ::llvm::telemetry::EntryKind {
+  static const KindType BaseInfo = 0b11000;
+};
+
+/// Defines a convenient type for timestamp of various events.
+/// This is used by the EventStats below.
+using SteadyTimePoint = std::chrono::time_point<std::chrono::steady_clock,
+                                                std::chrono::nanoseconds>;
+
+/// Various time (and possibly memory) statistics of an event.
+struct EventStats {
+  // REQUIRED: Start time of an event
+  SteadyTimePoint start;
+  // OPTIONAL: End time of an event - may be empty if not meaningful.
+  std::optional<SteadyTimePoint> end;
+  // TBD: could add some memory stats here too?
+
+  EventStats() = default;
+  EventStats(SteadyTimePoint start) : start(start) {}
+  EventStats(SteadyTimePoint start, SteadyTimePoint end)
+      : start(start), end(end) {}
+};
+
+/// Describes the exit signal of an event.
+struct ExitDescription {
+  int exit_code;
+  std::string description;
+};
+
+struct LldbBaseTelemetryInfo : public TelemetryInfo {
+  EventStats stats;
+
+  std::optional<ExitDescription> exit_desc;
+
+  // For dyn_cast, isa, etc operations.
+  KindType getKind() const override { return LldbEntryKind::BaseInfo; }
+
+  static bool classof(const TelemetryInfo *t) {
+    // Subclasses of this is also acceptable.
+    return (t->getKind() & LldbEntryKind::BaseInfo) == LldbEntryKind::BaseInfo;
+  }
+
+  void serialize(Serializer &serializer) const override;
+};
+
+/// The base Telemetry manager instance in LLDB
+/// This class declares additional instrumentation points
+/// applicable to LLDB.
+class TelemetryManager : public llvm::telemetry::Manager {
+public:
+  TelemetryManager(std::unique_ptr<llvm::telemetry::Config> config,
+                   Debugger *debugger);
+
+  llvm::Error dispatch(TelemetryInfo *entry) override;
+
+  void addDestination(std::unique_ptr<Destination> destination) override;
+
+private:
+  std::unique_ptr<llvm::telemetry::Config> m_config;
+  Debugger *m_debugger;
----------------
labath wrote:

I thought that we've (in our conversation at the dev meeting) concluded that the telemetry manager should not be tied to a debugger (because some of the pieces of code -- e.g. everything inside a Symbol/ObjectFile class -- does not have access to debugger instance -- *by design*). Has that changed? If not, then this member shouldn't be here (but it could be attached to event types which *can* be tied to a specific debugger connection).

In the same breath, I want to make sure you're aware of the effort in [this RFC](https://discourse.llvm.org/t/rfc-lldb-dap-update-server-mode-to-allow-multiple-connections/82770), whose result will be that a single process can end up serving multiple DAP connections (with one Debugger instance for each connection) -- which means that some events may not be able to be tied to a specific DAP connection either.

I'm not really pushing for any particular solution, but I want to make sure you make a conscious decision about this, as it can have big implications for the rest of the implementation. It's basically a tradeoff. If you make the telemetry instance specific to a debugger object, then you cannot send telemetry from parts of the code which do not have access to a debugger. Or you have to do something similar to the Progress events, which send progress reports to *all* active debuggers. If you don't tie this to a debugger, then you can send telemetry from ~anywhere, but well.. that telemetry will not be tied to a debugger..

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