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

[James Henderson via llvm-commits]

================
@@ -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;
----------------
jh7370 wrote:

> This is implementation details, yes? if you want to not use this field, you can leave it empty and resort to your library's global field.

If it can be left empty, it shouldn't be here - it's presence here makes it look like it's a required field that always has to be populated. Really though, I think this is symptomatic of trying to represent a session without having an actual session class (more on that below).

> The reason it is here is because in some usages, there could be multiple sessions going on at once, and each of these TelemetryInfo entry have to be tagged with the correct session-id before they're sent off.

Yes, of course, so shouldn't there be a single class that telemetry entries are sent to, and then that class has the role of somehow attaching the additional data that is needed? In our internal implementation, the Session class is responsible for receiving events, attaching information that is more general (such as information about the session and triggering application) and then sending them to the desired protocols (roughly equivalent to the destinations mentioned).

> Sorry, not sure I understand your example here. Where exactly is the session class?
These session-id are conventionally collected from the tool being monitored.

Sorry, slight slip of the tongue as it were - ultimately a session "class" is whatever provides the single session ID for all related events. This doesn't have to be a specific class per se, but there will be something that knows the right session ID at least. In practice, I think a class that provides the coordination of the different components will need to exist. Indeed, is that not the intent of the Telemeter?

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