Skip to content

Commit 8bd9113

Browse files
duhminickduhminick
committed
Changes to accomodate inclusve end offset
Fix assertion in wineventlog test Fix mocks
1 parent 42ab528 commit 8bd9113

5 files changed

Lines changed: 34 additions & 29 deletions

File tree

‎plugins/inputs/windows_event_log/wineventlog/mock_windows_event_api.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -531,13 +531,13 @@ func (m *MockWindowsEventAPI) EvtOpenPublisherMetadata(session EvtHandle, publis
531531
// Helper methods
532532
func (m *MockWindowsEventAPI) extractRangeFromQuery(query string) state.Range {
533533
// Parse the XML query to extract EventRecordID constraints using a single regex
534-
// Look for pattern like "EventRecordID >= 2 and EventRecordID < 4"
534+
// Look for pattern like "EventRecordID > 2 and EventRecordID < 4"
535535

536536
var start, end uint64 = 0, 1000 // Default range
537537

538538
if query != "" {
539539
// Extract both start and end in one regex
540-
rangeRegex := regexp.MustCompile(`EventRecordID >= (\d+) and EventRecordID < (\d+)`)
540+
rangeRegex := regexp.MustCompile(`EventRecordID > (\d+) and EventRecordID < (\d+)`)
541541
if matches := rangeRegex.FindStringSubmatch(query); len(matches) > 2 {
542542
if parsedStart, err := strconv.ParseUint(matches[1], 10, 64); err == nil {
543543
start = parsedStart
@@ -559,7 +559,7 @@ func (m *MockWindowsEventAPI) findOrCreateHandleForRange(r state.Range) EvtHandl
559559
filteredEvents := []*MockEventRecord{}
560560
for _, event := range events {
561561
eventID, _ := strconv.ParseUint(event.EventRecordID, 10, 64)
562-
inRange := eventID >= r.StartOffset() && eventID < r.EndOffset()
562+
inRange := eventID > r.StartOffset() && eventID < r.EndOffset()
563563
if inRange {
564564
filteredEvents = append(filteredEvents, event)
565565
}

‎plugins/inputs/windows_event_log/wineventlog/utils.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ const (
2828
eventLogQueryTemplate = `<QueryList><Query Id="0"><Select Path="%s">*[System[%s]]</Select></Query></QueryList>`
2929
eventLogLevelFilter = "Level='%s'"
3030
eventIgnoreOldFilter = "TimeCreated[timediff(@SystemTime) &lt;= %d]"
31-
eventRangeFilter = "EventRecordID &gt;= %d and EventRecordID &lt; %d"
31+
eventRangeFilter = "EventRecordID &gt; %d and EventRecordID &lt; %d"
3232
emptySpaceScanLength = 100
3333
UnknownBytesPerCharacter = 0
3434

‎plugins/inputs/windows_event_log/wineventlog/utils_test.go‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -193,49 +193,49 @@ func TestCreateRangeQuery(t *testing.T) {
193193
path: "Application",
194194
levels: []string{"2"},
195195
r: state.NewRange(100, 200),
196-
expected: `<QueryList><Query Id="0"><Select Path="Application">*[System[(Level='2') and TimeCreated[timediff(@SystemTime) &lt;= 1209600000] and EventRecordID &gt;= 100 and EventRecordID &lt; 200]]</Select></Query></QueryList>`,
196+
expected: `<QueryList><Query Id="0"><Select Path="Application">*[System[(Level='2') and TimeCreated[timediff(@SystemTime) &lt;= 1209600000] and EventRecordID &gt; 100 and EventRecordID &lt; 200]]</Select></Query></QueryList>`,
197197
},
198198
{
199199
name: "Multiple levels with range",
200200
path: "System",
201201
levels: []string{"2", "3"},
202202
r: state.NewRange(1000, 2000),
203-
expected: `<QueryList><Query Id="0"><Select Path="System">*[System[(Level='2' or Level='3') and TimeCreated[timediff(@SystemTime) &lt;= 1209600000] and EventRecordID &gt;= 1000 and EventRecordID &lt; 2000]]</Select></Query></QueryList>`,
203+
expected: `<QueryList><Query Id="0"><Select Path="System">*[System[(Level='2' or Level='3') and TimeCreated[timediff(@SystemTime) &lt;= 1209600000] and EventRecordID &gt; 1000 and EventRecordID &lt; 2000]]</Select></Query></QueryList>`,
204204
},
205205
{
206206
name: "No levels with range",
207207
path: "Security",
208208
levels: []string{},
209209
r: state.NewRange(50, 150),
210-
expected: `<QueryList><Query Id="0"><Select Path="Security">*[System[TimeCreated[timediff(@SystemTime) &lt;= 1209600000] and EventRecordID &gt;= 50 and EventRecordID &lt; 150]]</Select></Query></QueryList>`,
210+
expected: `<QueryList><Query Id="0"><Select Path="Security">*[System[TimeCreated[timediff(@SystemTime) &lt;= 1209600000] and EventRecordID &gt; 50 and EventRecordID &lt; 150]]</Select></Query></QueryList>`,
211211
},
212212
{
213213
name: "Empty levels with range",
214214
path: "Application",
215215
levels: nil,
216216
r: state.NewRange(0, 100),
217-
expected: `<QueryList><Query Id="0"><Select Path="Application">*[System[TimeCreated[timediff(@SystemTime) &lt;= 1209600000] and EventRecordID &gt;= 0 and EventRecordID &lt; 100]]</Select></Query></QueryList>`,
217+
expected: `<QueryList><Query Id="0"><Select Path="Application">*[System[TimeCreated[timediff(@SystemTime) &lt;= 1209600000] and EventRecordID &gt; 0 and EventRecordID &lt; 100]]</Select></Query></QueryList>`,
218218
},
219219
{
220220
name: "Large range values",
221221
path: "System",
222222
levels: []string{"2", "3", "4"},
223223
r: state.NewRange(999999, 1000000),
224-
expected: `<QueryList><Query Id="0"><Select Path="System">*[System[(Level='2' or Level='3' or Level='4') and TimeCreated[timediff(@SystemTime) &lt;= 1209600000] and EventRecordID &gt;= 999999 and EventRecordID &lt; 1000000]]</Select></Query></QueryList>`,
224+
expected: `<QueryList><Query Id="0"><Select Path="System">*[System[(Level='2' or Level='3' or Level='4') and TimeCreated[timediff(@SystemTime) &lt;= 1209600000] and EventRecordID &gt; 999999 and EventRecordID &lt; 1000000]]</Select></Query></QueryList>`,
225225
},
226226
{
227227
name: "Zero start range",
228228
path: "Test",
229229
levels: []string{"2"},
230230
r: state.NewRange(0, 1),
231-
expected: `<QueryList><Query Id="0"><Select Path="Test">*[System[(Level='2') and TimeCreated[timediff(@SystemTime) &lt;= 1209600000] and EventRecordID &gt;= 0 and EventRecordID &lt; 1]]</Select></Query></QueryList>`,
231+
expected: `<QueryList><Query Id="0"><Select Path="Test">*[System[(Level='2') and TimeCreated[timediff(@SystemTime) &lt;= 1209600000] and EventRecordID &gt; 0 and EventRecordID &lt; 1]]</Select></Query></QueryList>`,
232232
},
233233
{
234234
name: "Path with special characters and range",
235235
path: "Microsoft-Windows-Kernel-General",
236236
levels: []string{"2"},
237237
r: state.NewRange(12345, 67890),
238-
expected: `<QueryList><Query Id="0"><Select Path="Microsoft-Windows-Kernel-General">*[System[(Level='2') and TimeCreated[timediff(@SystemTime) &lt;= 1209600000] and EventRecordID &gt;= 12345 and EventRecordID &lt; 67890]]</Select></Query></QueryList>`,
238+
expected: `<QueryList><Query Id="0"><Select Path="Microsoft-Windows-Kernel-General">*[System[(Level='2') and TimeCreated[timediff(@SystemTime) &lt;= 1209600000] and EventRecordID &gt; 12345 and EventRecordID &lt; 67890]]</Select></Query></QueryList>`,
239239
},
240240
}
241241

‎plugins/inputs/windows_event_log/wineventlog/wineventlog.go‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,9 @@ func NewEventLog(name string, levels []string, logGroupName, logStreamName, rend
9696
func (w *windowsEventLog) Init() error {
9797
go w.stateManager.Run(state.Notification{Done: w.done})
9898
restored, _ := w.stateManager.Restore()
99+
// Do note that the end offset is inclusive here as opposed to exclusive like done
100+
// in logfile. This is because we use the EvtSubscribeStartAfterBookmark flag in
101+
// EvtSubscribe.
99102
w.eventOffset = restored.Last().EndOffset()
100103
if !restored.OnlyUseMaxOffset() {
101104
w.gapsToRead = state.InvertRanges(restored)
@@ -182,8 +185,8 @@ func (w *windowsEventLog) run() {
182185
var records []*windowsEventLogRecord
183186
readingFromGap := false
184187
if len(w.gapsToRead) > 0 {
185-
readingFromGap = true
186188
records = w.readGaps()
189+
readingFromGap = true
187190
} else {
188191
records = w.read()
189192
}
@@ -194,9 +197,7 @@ func (w *windowsEventLog) run() {
194197
continue
195198
}
196199
recordNumber, _ := strconv.ParseUint(record.System.EventRecordID, 10, 64)
197-
// On gap records, we need to shift by 1 because Shift sets the end. If this does not happen
198-
// then the final state file will still have a gap despite the event being processed.
199-
// Windows record IDs also begin at 1 instead of 0.
200+
// Need to shift by 1 because the range logic assumes [start, end) whereas Windows events is [start, end]
200201
if readingFromGap {
201202
r.Shift(recordNumber + 1)
202203
} else {

‎plugins/inputs/windows_event_log/wineventlog/wineventlog_test.go‎

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -116,14 +116,18 @@ func TestReadGaps(t *testing.T) {
116116
t.Parallel()
117117

118118
rl := state.RangeList{
119-
state.NewRange(0, 100),
120-
state.NewRange(105, 107),
119+
state.NewRange(0, 1),
120+
state.NewRange(4, 5),
121121
}
122122
elog, stateFileName := newTestEventLogWithState(t, NAME, LEVELS, rl)
123123
mockAPI := NewMockWindowsEventAPI()
124124
elog.SetEventAPI(mockAPI)
125125

126-
mockAPI.AddMockEventsForQuery(createMockEventRecordsRange(100, 105))
126+
// This is per EvtHandle hence the necessity to break up these calls
127+
// 0, 1, 4 were "sent" previously (should be skipped)
128+
mockAPI.AddMockEventsForQuery(createMockEventRecords(0, 1, 4, 5))
129+
// Gap records (should be read by gap reading)
130+
mockAPI.AddMockEventsForQuery(createMockEventRecords(2, 3))
127131

128132
elog.Init()
129133

@@ -133,23 +137,23 @@ func TestReadGaps(t *testing.T) {
133137
records = append(records, e)
134138
e.Done()
135139
})
136-
time.Sleep(5 * time.Second)
140+
time.Sleep(8 * time.Second)
137141
elog.Stop()
138142

139143
assert.Empty(t, elog.gapsToRead, "Gaps should be cleared after reading")
140-
assert.Len(t, records, 5, "Should return 5 mock events")
144+
assert.Len(t, records, 2, "Should return 2 mock events")
141145
assert.Len(t, mockAPI.QueryCalls, 1, "Should make one query call")
142146
assert.Equal(t, NAME, mockAPI.QueryCalls[0].Path, "Should query correct path")
143-
assert.Contains(t, mockAPI.QueryCalls[0].Query, "EventRecordID &gt;= 100", "Query should contain start range")
144-
assert.Contains(t, mockAPI.QueryCalls[0].Query, "EventRecordID &lt; 105", "Query should contain end range")
147+
assert.Contains(t, mockAPI.QueryCalls[0].Query, "EventRecordID &gt; 1", "Query should contain start range")
148+
assert.Contains(t, mockAPI.QueryCalls[0].Query, "EventRecordID &lt; 4", "Query should contain end range")
145149
assert.Greater(t, len(mockAPI.CloseCalls), 0, "Should make close calls")
146150

147151
for i, record := range records {
148-
assert.Contains(t, record.Message(), fmt.Sprintf("Event %d", 100+i))
152+
assert.Contains(t, record.Message(), fmt.Sprintf("Event %d", 2+i))
149153
}
150154

151155
assertStateFileRange(t, stateFileName, state.RangeList{
152-
state.NewRange(0, 107),
156+
state.NewRange(0, 5),
153157
})
154158
})
155159
t.Run("ReadGapThenSubscribe", func(t *testing.T) {
@@ -165,9 +169,9 @@ func TestReadGaps(t *testing.T) {
165169

166170
// This is per EvtHandle hence the necessity to break up these calls
167171
// 0, 1, 4 were "sent" previously (should be skipped)
168-
mockAPI.AddMockEventsForQuery(createMockEventRecords(0, 1, 4))
172+
mockAPI.AddMockEventsForQuery(createMockEventRecords(0, 2, 4, 5))
169173
// Gap records (should be read by gap reading)
170-
mockAPI.AddMockEventsForQuery(createMockEventRecords(2, 3))
174+
mockAPI.AddMockEventsForQuery(createMockEventRecords(3, 4))
171175

172176
elog.Init()
173177

@@ -184,16 +188,16 @@ func TestReadGaps(t *testing.T) {
184188
elog.Stop()
185189

186190
assert.Empty(t, elog.gapsToRead, "Gaps should be cleared after reading")
187-
assert.Len(t, records, 6, "Should return 6 mock events")
191+
assert.Len(t, records, 5, "Should return 5 mock events")
188192
assert.Len(t, mockAPI.QueryCalls, 1, "Should make one query call")
189193
assert.Equal(t, NAME, mockAPI.QueryCalls[0].Path, "Should query correct path")
190-
assert.Contains(t, mockAPI.QueryCalls[0].Query, "EventRecordID &gt;= 2", "Query should contain start range")
194+
assert.Contains(t, mockAPI.QueryCalls[0].Query, "EventRecordID &gt; 2", "Query should contain start range")
191195
assert.Contains(t, mockAPI.QueryCalls[0].Query, "EventRecordID &lt; 4", "Query should contain end range")
192196
assert.Len(t, mockAPI.SubscribeCalls, 1, "Should make one subscribe call")
193197
assert.Greater(t, len(mockAPI.CloseCalls), 0, "Should make close calls")
194198

195199
expectedRecords := []int{
196-
2, 3, 5, 6, 7, 8,
200+
3, 5, 6, 7, 8,
197201
}
198202
for i, record := range records {
199203
assert.Contains(t, record.Message(), fmt.Sprintf("Event %d", expectedRecords[i]))

0 commit comments

Comments
 (0)