martinzink commented on code in PR #1434:
URL: https://github.com/apache/nifi-minifi-cpp/pull/1434#discussion_r1049653119


##########
extensions/windows-event-log/wel/WindowsEventLog.h:
##########
@@ -146,20 +147,19 @@ class WindowsEventLogMetadata {
 
 class WindowsEventLogMetadataImpl : public WindowsEventLogMetadata {
  public:
-  WindowsEventLogMetadataImpl(EVT_HANDLE metadataProvider, EVT_HANDLE 
event_ptr) : metadata_ptr_(metadataProvider), event_timestamp_(0), 
event_ptr_(event_ptr) {
+  WindowsEventLogMetadataImpl(EVT_HANDLE metadataProvider, EVT_HANDLE 
event_ptr) : metadata_ptr_(metadataProvider), event_ptr_(event_ptr) {
     renderMetadata();
   }
 
-  std::string getEventData(EVT_FORMAT_MESSAGE_FLAGS flags) const override;
+  [[nodiscard]] std::string getEventData(EVT_FORMAT_MESSAGE_FLAGS flags) const 
override;
 
-  std::string getEventTimestamp() const override { return 
event_timestamp_str_; }
+  [[nodiscard]] std::string getEventTimestamp() const override { return 
event_timestamp_str_; }
 
   short getEventTypeIndex() const override { return event_type_index_; }  // 
NOLINT short comes from WINDOWS API
 
  private:
   void renderMetadata();
 
-  uint64_t event_timestamp_;
   std::string event_type_;
   short event_type_index_;  // NOLINT short comes from WINDOWS API

Review Comment:
   Added initaliziation in 
https://github.com/apache/nifi-minifi-cpp/pull/1434/commits/de87bde523da4625cd966052b2fd7884734f745c#diff-72a907e167b376aa07b9e4ee412b95e974d9fa89c80477a735f4c23e966038b3R164



##########
extensions/windows-event-log/wel/WindowsEventLog.cpp:
##########
@@ -25,64 +25,67 @@
 #include "UnicodeConversion.h"
 #include "utils/Deleters.h"
 #include "utils/gsl.h"
+#include "UniqueEvtHandle.h"
 
 namespace org::apache::nifi::minifi::wel {
 
+namespace {
+std::string GetEventTimestampStr(uint64_t event_timestamp) {
+  SYSTEMTIME st;
+  FILETIME ft;
+
+  ft.dwHighDateTime = (DWORD)((event_timestamp >> 32) & 0xFFFFFFFF);
+  ft.dwLowDateTime = (DWORD)(event_timestamp & 0xFFFFFFFF);

Review Comment:
   I like this idea, I've changed it in 
https://github.com/apache/nifi-minifi-cpp/pull/1434/commits/de87bde523da4625cd966052b2fd7884734f745c#diff-d148f5f0fe3bb6a877f5754ff0567450d155c4e21d65e28292966f40bb0b77a4R37-R38



##########
extensions/windows-event-log/ConsumeWindowsEventLog.cpp:
##########
@@ -537,79 +548,71 @@ void 
ConsumeWindowsEventLog::substituteXMLPercentageItems(pugi::xml_document& do
   doc.traverse(treeWalker);
 }
 
-bool ConsumeWindowsEventLog::createEventRender(EVT_HANDLE hEvent, EventRender& 
eventRender) {
+nonstd::expected<std::string, std::string> 
ConsumeWindowsEventLog::renderEventAsXml(EVT_HANDLE event_handle) {
   logger_->log_trace("Rendering an event");
   WCHAR stackBuffer[4096];
   DWORD size = sizeof(stackBuffer);
   using Deleter = utils::StackAwareDeleter<WCHAR, utils::FreeDeleter>;
-  std::unique_ptr<WCHAR, Deleter> buf{stackBuffer, Deleter{ stackBuffer }};
+  std::unique_ptr<WCHAR, Deleter> buf{stackBuffer, Deleter{stackBuffer}};
 
   DWORD used = 0;
   DWORD propertyCount = 0;
-  if (!EvtRender(NULL, hEvent, EvtRenderEventXml, size, buf.get(), &used, 
&propertyCount)) {
-    if (ERROR_INSUFFICIENT_BUFFER != GetLastError()) {
-      LOG_LAST_ERROR(EvtRender);
-      return false;
+  if (!EvtRender(nullptr, event_handle, EvtRenderEventXml, size, buf.get(), 
&used, &propertyCount)) {
+    DWORD last_error = GetLastError();
+    if (ERROR_INSUFFICIENT_BUFFER != last_error) {
+      std::string error_message = std::format("EvtRender failed due to %s", 
utils::OsUtils::getWindowsErrorAsString(last_error));
+      return nonstd::make_unexpected(error_message);
     }
-    if (used > maxBufferSize_) {
-      logger_->log_error("Dropping event because it couldn't be rendered 
within %" PRIu64 " bytes.", maxBufferSize_);
-      return false;
+    if (used > max_buffer_size_) {
+      std::string error_message = std::format("Dropping event because it 
couldn't be rendered within %" PRIu64 " bytes.", max_buffer_size_);
+      return nonstd::make_unexpected(error_message);
     }
     size = used;
-    buf.reset((LPWSTR)malloc(size));
-    if (!buf) {
-      return false;
-    }
-    if (!EvtRender(NULL, hEvent, EvtRenderEventXml, size, buf.get(), &used, 
&propertyCount)) {
-      LOG_LAST_ERROR(EvtRender);
-      return false;
+    buf.reset((LPWSTR) malloc(size));
+    if (!EvtRender(nullptr, event_handle, EvtRenderEventXml, size, buf.get(), 
&used, &propertyCount)) {
+      std::string error_message = std::format("EvtRender failed due to %s", 
utils::OsUtils::getWindowsErrorAsString(GetLastError()));
+      return nonstd::make_unexpected(error_message);
     }
   }
+  logger_->log_trace("Event rendered with size %" PRIu32, used);
+  return wel::to_string(buf.get());
+}
 
-  logger_->log_debug("Event rendered with size %" PRIu32 ". Performing doc 
traversing...", used);
-
-  std::string xml = wel::to_string(buf.get());
+nonstd::expected<EventRender, std::string> 
ConsumeWindowsEventLog::createEventRender(EVT_HANDLE hEvent) {
+  auto event_as_xml = renderEventAsXml(hEvent);
+  if (!event_as_xml)
+    return nonstd::make_unexpected(event_as_xml.error());
 
   pugi::xml_document doc;
-  pugi::xml_parse_result result = doc.load_string(xml.c_str());
+  if (!doc.load_string(event_as_xml->c_str()))
+    return nonstd::make_unexpected("Invalid XML produced");
 
-  if (!result) {
-    logger_->log_error("Invalid XML produced");
-    return false;
-  }
+  EventRender result;
 
   // this is a well known path.
-  std::string providerName = 
doc.child("Event").child("System").child("Provider").attribute("Name").value();
-  wel::WindowsEventLogMetadataImpl 
metadata{getEventLogHandler(providerName).getMetadata(), hEvent};
+  std::string provider_name = 
doc.child("Event").child("System").child("Provider").attribute("Name").value();
+  wel::WindowsEventLogMetadataImpl 
metadata{getEventLogHandler(provider_name).getMetadata(), hEvent};
   wel::MetadataWalker walker{metadata, channel_, !resolve_as_attributes_, 
apply_identifier_function_, regex_};
 
   // resolve the event metadata
   doc.traverse(walker);
 
-  logger_->log_debug("Finish doc traversing, performing writing...");
+  logger_->log_trace("Finish doc traversing, performing writing...");
 
   if (output_.plaintext) {
     logger_->log_trace("Writing event in plain text");
 
-    auto& handler = getEventLogHandler(providerName);
-    auto message = handler.getEventMessage(hEvent);
+    auto& handler = getEventLogHandler(provider_name);
+    auto event_message = handler.getEventMessage(hEvent);
+
+    std::string_view payload_name = event_message ? "Message" : "Error";
+
+    wel::WindowsEventLogHeader log_header(header_names_, header_delimiter_, 
payload_name.size());
+    result.plaintext = log_header.getEventHeader([&walker](wel::METADATA 
metadata) { return walker.getMetadata(metadata); });
+    result.plaintext += payload_name;
+    result.plaintext += log_header.getDelimiterFor(payload_name.size());
+    result.plaintext += event_message.has_value() ? *event_message : 
utils::OsUtils::getWindowsErrorAsString(event_message.error());
 
-    if (!message.empty()) {
-      for (const auto &mapEntry : walker.getIdentifiers()) {
-        // replace the identifiers with their translated strings.
-        if (mapEntry.first.empty() || mapEntry.second.empty()) {
-          continue;  // This is most probably a result of a failed ID 
resolution
-        }
-        utils::StringUtils::replaceAll(message, mapEntry.first, 
mapEntry.second);
-      }

Review Comment:
   seems like this got lost in the latest rebase good catch, I've readded it in 
https://github.com/apache/nifi-minifi-cpp/pull/1434/commits/de87bde523da4625cd966052b2fd7884734f745c#diff-0769252d38a0bd8b7b412439a306a061cd5da7b84419220a1aec6e100a853cecR609-R617
 
   I will also try to write some unit tests that could verify this feature so 
it won't get removed accidentally or otherwise in the future.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@nifi.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to