You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by GitBox <gi...@apache.org> on 2022/12/15 13:41:38 UTC
[GitHub] [nifi-minifi-cpp] martinzink commented on a diff in pull request #1434: MINIFICPP-1949 ConsumeWindowsEventLog precompiled regex
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-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org