Logger API tweaks, add EventId support

benibus · Feb 29, 2024 · be71c78 · be71c78
1 parent 20dbb39
commit be71c78
Show file tree

Hide file tree

Showing 2 changed files with 84 additions and 24 deletions.
diff --git a/cpp/src/arrow/telemetry/logging.cc b/cpp/src/arrow/telemetry/logging.cc
@@ -66,7 +66,7 @@ namespace {
 
 class NoopLogger : public Logger {
  public:
-  void Log(LogLevel, std::string_view, const AttributeHolder&) override {}
+  void Log(const LogDescriptor&) override {}
 };
 
 }  // namespace
@@ -308,34 +308,46 @@ class OtelLogger : public Logger {
   OtelLogger(LoggingOptions options, otel_shared_ptr<otel::logs::Logger> ot_logger)
       : logger_(ot_logger), options_(std::move(options)) {}
 
-  void Log(LogLevel severity, std::string_view body,
-           const AttributeHolder& holder) override {
-    if (severity < options_.severity_threshold) {
+  void Log(const LogDescriptor& desc) override {
+    if (desc.severity < options_.severity_threshold) {
       return;
     }
 
+    const auto timestamp =
+        otel::common::SystemTimestamp(std::chrono::system_clock::now());
+
     auto log = logger_->CreateLogRecord();
     if (log == nullptr) {
       return;
     }
 
-    log->SetTimestamp(otel::common::SystemTimestamp(std::chrono::system_clock::now()));
-    log->SetSeverity(ToOtelSeverity(severity));
-    log->SetBody(otel_string_view(body.data(), body.length()));
-
-    auto span_ctx = otel::trace::Tracer::GetCurrentSpan()->GetContext();
-    log->SetSpanId(span_ctx.span_id());
-    log->SetTraceId(span_ctx.trace_id());
-    log->SetTraceFlags(span_ctx.trace_flags());
-
-    if (holder.num_attributes() > 0) {
+    if (desc.attributes && desc.attributes->num_attributes() > 0) {
       auto callback = [&log](std::string_view k, const AttributeValue& v) -> bool {
         AttributeConverter converter{};
         log->SetAttribute(otel_string_view(k.data(), k.length()),
                           std::visit(converter, v));
         return true;
       };
-      holder.ForEach(std::move(callback));
+      desc.attributes->ForEach(std::move(callback));
+    }
+
+    // We set the remaining attributes AFTER the custom attributes in AttributeHolder
+    // because, in the event of key collisions, these should take precedence.
+    log->SetTimestamp(timestamp);
+    log->SetSeverity(ToOtelSeverity(desc.severity));
+
+    auto span_ctx = otel::trace::Tracer::GetCurrentSpan()->GetContext();
+    log->SetSpanId(span_ctx.span_id());
+    log->SetTraceId(span_ctx.trace_id());
+    log->SetTraceFlags(span_ctx.trace_flags());
+
+    if (desc.body) {
+      auto body = *desc.body;
+      log->SetBody(otel_string_view(body.data(), body.length()));
+    }
+
+    if (const auto& event = desc.event_id; event.is_valid()) {
+      log->SetEventId(event.id, otel_string_view(event.name.data(), event.name.length()));
     }
 
     logger_->EmitLogRecord(std::move(log));

diff --git a/cpp/src/arrow/telemetry/logging.h b/cpp/src/arrow/telemetry/logging.h
@@ -51,6 +51,9 @@ struct ServiceAttributes {
   static ServiceAttributes Defaults() { return ServiceAttributes{}; }
 };
 
+constexpr LogLevel kDefaultSeverityThreshold = LogLevel::ARROW_WARNING;
+constexpr LogLevel kDefaultSeverity = LogLevel::ARROW_INFO;
+
 struct LoggingOptions {
   /// \brief Attributes to set for the LoggerProvider's Resource
   ServiceAttributes service_attributes = ServiceAttributes::Defaults();
@@ -61,29 +64,74 @@ struct LoggingOptions {
   std::ostream* default_export_stream = NULLPTR;
 
   /// \brief Minimum severity required to emit an OpenTelemetry log record
-  LogLevel severity_threshold = LogLevel::ARROW_INFO;
+  LogLevel severity_threshold = kDefaultSeverityThreshold;
 
   static LoggingOptions Defaults() { return LoggingOptions{}; }
 };
 
+/// \brief Represents an event as a name/integer id pair
+struct EventId {
+  constexpr EventId() = default;
+  constexpr EventId(int64_t id, std::string_view name) : name(name), id(id) {}
+
+  constexpr bool is_valid() const { return id >= 0; }
+  constexpr operator bool() const { return is_valid(); }
+
+  static constexpr EventId Invalid() { return EventId{}; }
+
+  std::string_view name;
+  int64_t id = -1;
+};
+
+struct LogDescriptor {
+  LogLevel severity = kDefaultSeverity;
+
+  std::optional<std::string_view> body = std::nullopt;
+
+  EventId event_id = EventId::Invalid();
+
+  const AttributeHolder* attributes = NULLPTR;
+};
+
 class ARROW_EXPORT Logger {
  public:
   virtual ~Logger() = default;
 
-  virtual void Log(LogLevel severity, std::string_view body, const AttributeHolder&) = 0;
+  virtual void Log(const LogDescriptor&) = 0;
 
-  void Log(LogLevel severity, const AttributeHolder& attributes) {
-    this->Log(severity, "", attributes);
+  void Log(LogLevel severity, std::string_view body, const AttributeHolder& attributes,
+           EventId event_id = EventId::Invalid()) {
+    LogDescriptor desc;
+    desc.severity = severity;
+    desc.body = body;
+    desc.attributes = &attributes;
+    desc.event_id = event_id;
+    this->Log(desc);
   }
 
-  void Log(LogLevel severity, std::string_view body) {
-    this->Log(severity, body, AttributeList{});
+  void Log(LogLevel severity, EventId event_id = EventId::Invalid()) {
+    LogDescriptor desc;
+    desc.severity = severity;
+    desc.event_id = event_id;
+    this->Log(desc);
   }
 
-  void Log(std::string_view message) { this->Log(LogLevel::ARROW_INFO, message); }
+  void Log(LogLevel severity, const AttributeHolder& attributes,
+           EventId event_id = EventId::Invalid()) {
+    LogDescriptor desc;
+    desc.severity = severity;
+    desc.attributes = &attributes;
+    desc.event_id = event_id;
+    this->Log(desc);
+  }
 
-  void Log(std::string_view body, const AttributeHolder& attributes) {
-    this->Log(LogLevel::ARROW_INFO, body, attributes);
+  void Log(LogLevel severity, std::string_view body,
+           EventId event_id = EventId::Invalid()) {
+    LogDescriptor desc;
+    desc.severity = severity;
+    desc.body = body;
+    desc.event_id = event_id;
+    this->Log(desc);
   }
 };