[llvm] [llvm]Add a simple Telemetry framework (PR #102323)

James Henderson via llvm-commits llvm-commits at lists.llvm.org
Tue Sep 3 01:09:38 PDT 2024


================
@@ -23,82 +67,94 @@
 namespace llvm {
 namespace telemetry {
 
-using SteadyTimePoint = std::chrono::time_point<std::chrono::steady_clock>;
-
 struct TelemetryConfig {
   // If true, telemetry will be enabled.
-  bool enable_telemetry;
-
-  // Additional destinations to send the logged entries.
-  // Could be stdout, stderr, or some local paths.
-  // Note: these are destinations are __in addition to__ whatever the default
-  // destination(s) are, as implemented by vendors.
-  std::vector<std::string> additional_destinations;
+  bool EnableTelemetry;
+
+  // Implementation-defined names of additional destinations to send
+  // telemetry data (Could be stdout, stderr, or some local paths, etc).
+  //
+  // These strings will be interpreted by the vendor's code.
+  // So the users must pick the  from their vendor's pre-defined
+  // set of Destinations.
+  std::vector<std::string> AdditionalDestinations;
 };
 
+using SteadyTimePoint = std::chrono::time_point<std::chrono::steady_clock>;
+
 struct TelemetryEventStats {
-  // REQUIRED: Start time of event
-  SteadyTimePoint m_start;
-  // OPTIONAL: End time of event - may be empty if not meaningful.
-  std::optional<SteadyTimePoint> m_end;
+  // 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?
 
   TelemetryEventStats() = default;
-  TelemetryEventStats(SteadyTimePoint start) : m_start(start) {}
-  TelemetryEventStats(SteadyTimePoint start, SteadyTimePoint end)
-      : m_start(start), m_end(end) {}
-
-  std::string ToString() const;
+  TelemetryEventStats(SteadyTimePoint Start) : Start(Start) {}
+  TelemetryEventStats(SteadyTimePoint Start, SteadyTimePoint End)
+      : Start(Start), End(End) {}
 };
 
 struct ExitDescription {
-  int exit_code;
-  std::string description;
+  int ExitCode;
+  std::string Description;
+};
 
-  std::string ToString() const;
+// For isa, dyn_cast, etc operations on TelemetryInfo.
+typedef unsigned KindType;
+struct EntryKind {
+  static const KindType Base = 0;
 };
 
-// The base class contains the basic set of data.
+// TelemetryInfo is the data courier, used to forward data from
+// the tool being monitored and the Telemery framework.
+//
+// This base class contains only the basic set of telemetry data.
 // Downstream implementations can add more fields as needed.
 struct TelemetryInfo {
   // A "session" corresponds to every time the tool starts.
   // All entries emitted for the same session will have
   // the same session_uuid
-  std::string session_uuid;
+  std::string SessionUuid;
 
-  TelemetryEventStats stats;
+  TelemetryEventStats Stats;
 
-  std::optional<ExitDescription> exit_description;
+  std::optional<ExitDescription> ExitDesc;
 
   // Counting number of entries.
   // (For each set of entries with the same session_uuid, this value should
   // be unique for each entry)
-  size_t counter;
+  size_t Counter;
 
   TelemetryInfo() = default;
   ~TelemetryInfo() = default;
-  virtual std::string ToString() const;
+
+  virtual json::Object serializeToJson() const;
+
+  // For isa, dyn_cast, etc, operations.
+  virtual KindType getEntryKind() const { return EntryKind::Base; }
+  static bool classof(const TelemetryInfo *T) {
+    return T->getEntryKind() == EntryKind::Base;
+  }
 };
 
 // Where/how to send the telemetry entries.
----------------
jh7370 wrote:

The mixing of the "Where" and the "How" is a red-flag to me, because it suggests that the class is trying to do two things at once. It's related to a comment I made previously: these two concepts are somewhat orthogonal. It might be that we want to send data to a file or a string or a pipe or ... , and we might want to send it as JSON/YAML/some kind of structured messages etc. It's certainly true as noted before that mixing some "hows" with some "wheres" doesn't make sense, but there are enough cases where it does make sense that I don't think we should risk having to have a combinatorial explosion of classes e.g. `JSONFileDestination`, `JSONStringDestination`, `YAMLFileDestination`, `YAMLStringDestination` etc. (Yes, strings and files might both be modelled as streams, but that's not really the point).

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


More information about the llvm-commits mailing list