From edc7321f8dba9c8b46fe6fe3eaa5550a9106c095 Mon Sep 17 00:00:00 2001 From: Dan Kortschak <90160302+efd6@users.noreply.github.com> Date: Wed, 6 Sep 2023 07:18:48 +0930 Subject: [PATCH] winlogbeat/eventlog: ensure event loggers retain metric collection when handling recoverable errors (#36483) When winlogbeat's event loggers encounter recoverable errors they handle this by closing and reopening the channel. This causes the metric collection for the beat and dependent winlog filebeat input to lose metric collection as metric registration only occurs on configuration. So add a soft-close method, Reset, that only closes the channel and leaves the event logger valid, leaving the metrics in tact, and use this for recovering from errors. --- CHANGELOG.next.asciidoc | 2 ++ filebeat/input/winlog/input.go | 8 ++++---- winlogbeat/beater/eventlogger.go | 8 ++++---- winlogbeat/eventlog/eventlog.go | 5 +++++ winlogbeat/eventlog/wineventlog.go | 5 +++++ winlogbeat/eventlog/wineventlog_experimental.go | 9 +++++++++ 6 files changed, 29 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.next.asciidoc b/CHANGELOG.next.asciidoc index d9418e40c70..8b5204dd33c 100644 --- a/CHANGELOG.next.asciidoc +++ b/CHANGELOG.next.asciidoc @@ -103,6 +103,7 @@ https://github.com/elastic/beats/compare/v8.8.1\...main[Check the HEAD diff] - Fix panic when redact option is not provided to CEL input. {issue}36387[36387] {pull}36388[36388] - Remove 'onFilteredOut' and 'onDroppedOnPublish' callback logs {issue}36299[36299] {pull}36399[36399] - Added a fix for Crowdstrike pipeline handling process arrays {pull}36496[36496] +- Ensure winlog input retains metric collection when handling recoverable errors. {issue}36479[36479] {pull}36483[36483] *Heartbeat* @@ -139,6 +140,7 @@ https://github.com/elastic/beats/compare/v8.8.1\...main[Check the HEAD diff] *Winlogbeat* +- Ensure event loggers retains metric collection when handling recoverable errors. {issue}36479[36479] {pull}36483[36483] *Elastic Logging Plugin* diff --git a/filebeat/input/winlog/input.go b/filebeat/input/winlog/input.go index 617d9b26f35..ab925cbdd3c 100644 --- a/filebeat/input/winlog/input.go +++ b/filebeat/input/winlog/input.go @@ -136,15 +136,15 @@ runLoop: records, err := api.Read() if eventlog.IsRecoverable(err) { log.Errorw("Encountered recoverable error when reading from Windows Event Log", "error", err) - if closeErr := api.Close(); closeErr != nil { - log.Errorw("Error closing Windows Event Log handle", "error", closeErr) + if resetErr := api.Reset(); resetErr != nil { + log.Errorw("Error resetting Windows Event Log handle", "error", resetErr) } continue runLoop } if !api.IsFile() && eventlog.IsChannelNotFound(err) { log.Errorw("Encountered channel not found error when reading from Windows Event Log", "error", err) - if closeErr := api.Close(); closeErr != nil { - log.Errorw("Error closing Windows Event Log handle", "error", closeErr) + if resetErr := api.Reset(); resetErr != nil { + log.Errorw("Error resetting Windows Event Log handle", "error", resetErr) } continue runLoop } diff --git a/winlogbeat/beater/eventlogger.go b/winlogbeat/beater/eventlogger.go index 3176ecd7c36..2a04f39df72 100644 --- a/winlogbeat/beater/eventlogger.go +++ b/winlogbeat/beater/eventlogger.go @@ -177,15 +177,15 @@ runLoop: records, err := api.Read() if eventlog.IsRecoverable(err) { e.log.Warnw("Read() encountered recoverable error. Reopening handle...", "error", err, "channel", api.Channel()) - if closeErr := api.Close(); closeErr != nil { - e.log.Warnw("Close() error.", "error", err) + if resetErr := api.Reset(); resetErr != nil { + e.log.Warnw("Reset() error.", "error", err) } continue runLoop } if !api.IsFile() && eventlog.IsChannelNotFound(err) { e.log.Warnw("Read() encountered channel not found error for channel %q. Reopening handle...", "error", err, "channel", api.Channel()) - if closeErr := api.Close(); closeErr != nil { - e.log.Warnw("Close() error.", "error", err) + if resetErr := api.Reset(); resetErr != nil { + e.log.Warnw("Reset() error.", "error", err) } continue runLoop } diff --git a/winlogbeat/eventlog/eventlog.go b/winlogbeat/eventlog/eventlog.go index 538d622ead9..0a06bf13ce9 100644 --- a/winlogbeat/eventlog/eventlog.go +++ b/winlogbeat/eventlog/eventlog.go @@ -48,6 +48,11 @@ type EventLog interface { // reading and close the log. Read() ([]Record, error) + // Reset closes the event log channel to allow recovering from recoverable + // errors. Open must be successfully called after a Reset before Read may + // be called. + Reset() error + // Close the event log. It should not be re-opened after closing. Close() error diff --git a/winlogbeat/eventlog/wineventlog.go b/winlogbeat/eventlog/wineventlog.go index a69774fa52f..ddb703dbbb0 100644 --- a/winlogbeat/eventlog/wineventlog.go +++ b/winlogbeat/eventlog/wineventlog.go @@ -580,6 +580,11 @@ func (l *winEventLog) createBookmarkFromEvent(evtHandle win.EvtHandle) (string, return string(l.outputBuf.Bytes()), err } +func (l *winEventLog) Reset() error { + debugf("%s Closing handle for reset", l.logPrefix) + return win.Close(l.subscription) +} + func (l *winEventLog) Close() error { debugf("%s Closing handle", l.logPrefix) l.metrics.close() diff --git a/winlogbeat/eventlog/wineventlog_experimental.go b/winlogbeat/eventlog/wineventlog_experimental.go index dc1d8997109..5e323858f22 100644 --- a/winlogbeat/eventlog/wineventlog_experimental.go +++ b/winlogbeat/eventlog/wineventlog_experimental.go @@ -348,9 +348,18 @@ func (l *winEventLogExp) createBookmarkFromEvent(evtHandle win.EvtHandle) (strin return bookmark.XML() } +func (l *winEventLogExp) Reset() error { + l.log.Debug("Closing event log reader handles for reset.") + return l.close() +} + func (l *winEventLogExp) Close() error { l.log.Debug("Closing event log reader handles.") l.metrics.close() + return l.close() +} + +func (l *winEventLogExp) close() error { if l.iterator == nil { return l.renderer.Close() }