From 71a9c1c183850e5b1444dab25dd028a021cb5eed Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Thu, 16 May 2024 11:06:02 +0200 Subject: [PATCH] [winlogbeat] performance improvment; avoid rendering event message twice (#39544) (#39572) * wineventlog performance improvment; avoid rendering message twice * ignore missing or mismatched parameter values * add comment * changelog * actually increase the buffer (cherry picked from commit d2ebffe80cea3a872cb30a2e2b7434cc682b68d4) Co-authored-by: Leszek Kubik <39905449+intxgo@users.noreply.github.com> --- CHANGELOG.next.asciidoc | 1 + winlogbeat/sys/wineventlog/format_message.go | 40 ++++++++++----- .../sys/wineventlog/wineventlog_windows.go | 50 +++++++++++-------- 3 files changed, 59 insertions(+), 32 deletions(-) diff --git a/CHANGELOG.next.asciidoc b/CHANGELOG.next.asciidoc index 837de7b8025..8b3afec342c 100644 --- a/CHANGELOG.next.asciidoc +++ b/CHANGELOG.next.asciidoc @@ -167,6 +167,7 @@ https://github.com/elastic/beats/compare/v8.8.1\...main[Check the HEAD diff] *Winlogbeat* +- Use fixed size buffer at first pass for event parsing, improving throughput {issue}39530[39530] {pull}39544[39544] *Functionbeat* diff --git a/winlogbeat/sys/wineventlog/format_message.go b/winlogbeat/sys/wineventlog/format_message.go index e6502d384fa..9c1cf8254ac 100644 --- a/winlogbeat/sys/wineventlog/format_message.go +++ b/winlogbeat/sys/wineventlog/format_message.go @@ -75,23 +75,39 @@ func evtFormatMessage(metadataHandle EvtHandle, eventHandle EvtHandle, messageID valuesPtr = &values[0] } - // Determine the buffer size needed (given in WCHARs). - var bufferUsed uint32 - err := _EvtFormatMessage(metadataHandle, eventHandle, messageID, valuesCount, valuesPtr, messageFlag, 0, nil, &bufferUsed) - if err != windows.ERROR_INSUFFICIENT_BUFFER { //nolint:errorlint // This is an errno. - return "", fmt.Errorf("failed in EvtFormatMessage: %w", err) - } + // best guess render buffer size, 16KB, to avoid rendering message twice in most cases + const bestGuessRenderBufferSize = 1 << 14 + + // EvtFormatMessage operates with WCHAR buffer, assuming the size of the buffer in characters. + // https://docs.microsoft.com/en-us/windows/win32/api/winevt/nf-winevt-evtformatmessage + var bufferNeeded uint32 + bufferSize := uint32(bestGuessRenderBufferSize / 2) // Get a buffer from the pool and adjust its length. bb := sys.NewPooledByteBuffer() defer bb.Free() - // The documentation for EventFormatMessage specifies that the buffer is - // requested "in characters", and the buffer itself is LPWSTR, meaning the - // characters are WCHAR so double the value. - // https://docs.microsoft.com/en-us/windows/win32/api/winevt/nf-winevt-evtformatmessage - bb.Reserve(int(bufferUsed * 2)) + bb.Reserve(int(bufferSize * 2)) + + err := _EvtFormatMessage(metadataHandle, eventHandle, messageID, valuesCount, valuesPtr, messageFlag, bufferSize, bb.PtrAt(0), &bufferNeeded) + switch err { //nolint:errorlint // This is an errno or nil. + case nil: // OK + return sys.UTF16BytesToString(bb.Bytes()) + + // Ignore some errors so it can tolerate missing or mismatched parameter values. + case windows.ERROR_EVT_UNRESOLVED_VALUE_INSERT, + windows.ERROR_EVT_UNRESOLVED_PARAMETER_INSERT, + windows.ERROR_EVT_MAX_INSERTS_REACHED: + return sys.UTF16BytesToString(bb.Bytes()) + + case windows.ERROR_INSUFFICIENT_BUFFER: + bb.Reserve(int(bufferNeeded * 2)) + bufferSize = bufferNeeded + + default: + return "", fmt.Errorf("failed in EvtFormatMessage: %w", err) + } - err = _EvtFormatMessage(metadataHandle, eventHandle, messageID, valuesCount, valuesPtr, messageFlag, bufferUsed, bb.PtrAt(0), &bufferUsed) + err = _EvtFormatMessage(metadataHandle, eventHandle, messageID, valuesCount, valuesPtr, messageFlag, bufferSize, bb.PtrAt(0), &bufferNeeded) switch err { //nolint:errorlint // This is an errno or nil. case nil: // OK diff --git a/winlogbeat/sys/wineventlog/wineventlog_windows.go b/winlogbeat/sys/wineventlog/wineventlog_windows.go index 6b4abfaf5d1..22495f6bda2 100644 --- a/winlogbeat/sys/wineventlog/wineventlog_windows.go +++ b/winlogbeat/sys/wineventlog/wineventlog_windows.go @@ -239,15 +239,9 @@ func RenderEvent( // Only a single string is returned when rendering XML. err = FormatEventString(EvtFormatMessageXml, - eventHandle, providerName, EvtHandle(publisherHandle), lang, out) + eventHandle, providerName, EvtHandle(publisherHandle), lang, renderBuf, out) // Recover by rendering the XML without the RenderingInfo (message string). if err != nil { - // Do not try to recover from InsufficientBufferErrors because these - // can be retried with a larger buffer. - if errors.Is(err, sys.InsufficientBufferError{}) { - return err - } - err = RenderEventXML(eventHandle, renderBuf, out) } @@ -256,8 +250,8 @@ func RenderEvent( // Message reads the event data associated with the EvtHandle and renders // and returns the message only. -func Message(h EvtHandle, buf []byte, pubHandleProvider func(string) sys.MessageFiles) (message string, err error) { - providerName, err := evtRenderProviderName(buf, h) +func Message(h EvtHandle, renderBuf []byte, pubHandleProvider func(string) sys.MessageFiles) (message string, err error) { + providerName, err := evtRenderProviderName(renderBuf, h) if err != nil { return "", err } @@ -386,12 +380,15 @@ func Close(h EvtHandle) error { // publisherHandle is a handle to the publisher's metadata as provided by // EvtOpenPublisherMetadata. // lang is the language ID. +// renderBuf is a scratch buffer to render the message, if not provided or of +// insufficient size then a buffer from a system pool will be used func FormatEventString( messageFlag EvtFormatMessageFlag, eventHandle EvtHandle, publisher string, publisherHandle EvtHandle, lang uint32, + renderBuf []byte, out io.Writer, ) error { // Open a publisher handle if one was not provided. @@ -405,29 +402,42 @@ func FormatEventString( defer _EvtClose(ph) //nolint:errcheck // This is just a resource release. } - // Determine the buffer size needed (given in WCHARs). - var bufferUsed uint32 - err := _EvtFormatMessage(ph, eventHandle, 0, 0, nil, messageFlag, 0, nil, &bufferUsed) - if err != windows.ERROR_INSUFFICIENT_BUFFER { //nolint:errorlint // This is an errno. + var bufferPtr *byte + if renderBuf != nil { + bufferPtr = &renderBuf[0] + } + + // EvtFormatMessage operates with WCHAR buffer, assuming the size of the buffer in characters. + // https://docs.microsoft.com/en-us/windows/win32/api/winevt/nf-winevt-evtformatmessage + var bufferNeeded uint32 + bufferSize := uint32(len(renderBuf) / 2) + + err := _EvtFormatMessage(ph, eventHandle, 0, 0, nil, messageFlag, bufferSize, bufferPtr, &bufferNeeded) + if err != nil && err != windows.ERROR_INSUFFICIENT_BUFFER { //nolint:errorlint // This is an errno. return fmt.Errorf("failed in EvtFormatMessage: %w", err) + } else if err == nil { + // Windows API returns a null terminated WCHAR C-style string in the buffer. bufferNeeded applies + // only when ERROR_INSUFFICIENT_BUFFER is returned. Luckily the UTF16ToUTF8Bytes/UTF16ToString + // functions stop at null termination. Note, as signaled in a comment at the end of this function, + // this behavior is bad for EvtFormatMessageKeyword as then the API returns a list of null terminated + // strings in the buffer (it's fine for now as we don't use this parameter value). + return common.UTF16ToUTF8Bytes(renderBuf, out) } // Get a buffer from the pool and adjust its length. bb := sys.NewPooledByteBuffer() defer bb.Free() - // The documentation for EvtFormatMessage specifies that the buffer is - // requested "in characters", and the buffer itself is LPWSTR, meaning the - // characters are WCHAR so double the value. - // https://docs.microsoft.com/en-us/windows/win32/api/winevt/nf-winevt-evtformatmessage - bb.Reserve(int(bufferUsed * 2)) - err = _EvtFormatMessage(ph, eventHandle, 0, 0, nil, messageFlag, bufferUsed, bb.PtrAt(0), &bufferUsed) + bb.Reserve(int(bufferNeeded * 2)) + bufferSize = bufferNeeded + + err = _EvtFormatMessage(ph, eventHandle, 0, 0, nil, messageFlag, bufferSize, bb.PtrAt(0), &bufferNeeded) if err != nil { return fmt.Errorf("failed in EvtFormatMessage: %w", err) } // This assumes there is only a single string value to read. This will - // not work to read keys (when messageFlag == EvtFormatMessageKeyword). + // not work to read keys (when messageFlag == EvtFormatMessageKeyword) return common.UTF16ToUTF8Bytes(bb.Bytes(), out) }