-
Notifications
You must be signed in to change notification settings - Fork 3.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Windows events #3246
Windows events #3246
Conversation
I had to add couple of new go modules and hook the windows log in. Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
Codecov Report
@@ Coverage Diff @@
## master #3246 +/- ##
==========================================
+ Coverage 63.06% 63.08% +0.02%
==========================================
Files 198 198
Lines 16796 16796
==========================================
+ Hits 10592 10596 +4
+ Misses 5239 5236 -3
+ Partials 965 964 -1
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is exciting! 🎉
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Started review, left a few nits
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Finished review, there's a few nits and one []byte
mis-instantiation, then I'll approve.
Co-authored-by: Owen Diehl <ow.diehl@gmail.com>
Will check it out. |
Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
Co-authored-by: Owen Diehl <ow.diehl@gmail.com>
Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
// The bookmark will be saved at the given path. Use save to save the current position for a given event. | ||
func newBookMark(path string) (*bookMark, error) { | ||
// 4kb buffer for rendering bookmark | ||
buf := make([]byte, 4<<10) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
4kb will not be enough for complex event filters
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You have an example ? what would be a good value ? Thanks for the feedback
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By hitting some filters in Custom View of Windows Event Viewer and exporting resulting filter XML, I am easily getting a 8kb file. Bookmark with that filter takes a little bit more space.
Telegraf uses a 16kb buffer, and it's a better default value, enough for most cases.
The best way would be going full winapi: calling _EvtRender with zero as buf pointer value. It will return error, but also put the required length into bufferUsed. So then we can make buffer of required length and call _EvtRender again, this time with the newly created buffer pointer. It is a less performant way, and there can be a headache of memory allocating and freeing, but it is bulletproof.
Thank you very much for this PR btw! I made like 75% of porting Telegraf code to Loki, but got stuck at the polling loop, didn't find the satisfying method to do polls and got carried away by other duties. I like Loki, but extending it is such nontrivial work comparing to Telegraf, so many core files to change with custom targets, it is very unfriendly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like Loki, but extending it is such nontrivial work comparing to Telegraf, so many core files to change with custom targets, it is very unfriendly.
Sorry about that ! We'll try to get better at this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll increase the buffer :) before we cut a new release
includes grafana/loki#3284 and grafana/loki#3246
includes grafana/loki#3284 and grafana/loki#3246
See this discussion grafana#3246 (comment) Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
See this discussion #3246 (comment) Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
See this discussion grafana#3246 (comment) Signed-off-by: Cyril Tovena <cyril.tovena@gmail.com>
includes grafana/loki#3284 and grafana/loki#3246
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3246 +/- ##
==========================================
+ Coverage 62.44% 62.58% +0.14%
==========================================
Files 200 201 +1
Lines 17101 17113 +12
==========================================
+ Hits 10679 10711 +32
+ Misses 5457 5438 -19
+ Partials 965 964 -1
|
What this PR does / why we need it:
This PR introduces support for reading windows event logs with Promtail and send them to Loki.
It support saving the position of which events has been sent to avoid skipping logs during rollout or service interuption.
Which issue(s) this PR fixes:
Fixes #1395
Special notes for your reviewer:
I have forked telegraf windows event library in our repo to make it fit our scrape/push model. I'm using mostly syscall and event definition there. see https://github.com/influxdata/telegraf/tree/master/plugins/inputs/win_eventlog
Checklist