in_winevtlog: Make configurable for the size of collecting threshold per a cycle #8677

cosmo0920 · 2024-04-05T07:54:20Z

Previously, I had chosen to restrict with the threshold to prevent unlimited subscribing per a cycle.
This should cause flood of memory comsumptions but it depends on the amound of the channels of Windows EventLog.
Nowadays, this plugin is getting widely used.
Also, in the fluent community slack, there is a request to make to be able to configure for this threshold per a cycle.

To prevent slowing down for the subscribe the channels, it won't de able to set up below 512KiB with the newly introduced parameter.

Enter [N/A] in the box, if an item is not applicable to your change.

Testing
Before we can approve your change; please submit the following in a comment:

Example configuration file for the change

PS> bin/fluent-bit -i winevtlog -p channels=application -p threshold_size=1MB -p USE_ANSI=On -p read_existing_events=On -o stdout -v

Debug log output from testing the change

After introduced threshold_size parameter, the number of collected logs per a cycle is increased around 2000. This could increase events per second.

[1734] winevtlog.0: [[1712303476.639718600, {}], {"ProviderName"=>"Microsoft-Windows-RestartManager", "ProviderGuid"=>"{0888E5EF-9B98-4695-979D-E92CE4247224}", "Qualifiers"=>"", "EventID"=>10005, "Version"=>0, "Level"=>4, "Task"=>0, "Opcode"=>0, "Keywords"=>"0x8000000000000000", "TimeCreated"=>"2024-04-05 16:49:26 +0900", "EventRecordID"=>21470, "ActivityID"=>"", "RelatedActivityID"=>"", "ProcessID"=>162420, "ThreadID"=>161096, "Channel"=>"Application", "Computer"=>"hiroshi-14-eu", "UserID"=>"S-1-5-18", "Message"=>"コンピューターの再起動が必要です。", "StringInserts"=>[0, 1, 2]}]
[1735] winevtlog.0: [[1712303476.640162200, {}], {"ProviderName"=>"Microsoft-Windows-Security-SPP", "ProviderGuid"=>"{E23B33B0-C8C9-472C-A5F9-F2BDFEA0F156}", "Qualifiers"=>16384, "EventID"=>16384, "Version"=>0, "Level"=>4, "Task"=>0, "Opcode"=>0, "Keywords"=>"0x80000000000000", "TimeCreated"=>"2024-04-05 16:49:46 +0900", "EventRecordID"=>21471, "ActivityID"=>"", "RelatedActivityID"=>"", "ProcessID"=>161772, "ThreadID"=>0, "Channel"=>"Application", "Computer"=>"hiroshi-14-eu", "UserID"=>"", "Message"=>"ソフトウェア保護サービスの 2124-03-12T07:49:46Z の再起動をスケジュールしました。理由: RulesEngine。", "StringInserts"=>["2124-03-12T07:49:46Z", "RulesEngine"]}]
[2024/04/05 16:51:24] [debug] [out flush] cb_destroy coro_id=10
[2024/04/05 16:51:24] [debug] [task] destroy task=0000020BE6A34550 (task_id=0)
[2024/04/05 16:51:37] [debug] [input:winevtlog:winevtlog.0] read 920 bytes from 'application'

Attached Valgrind output that shows no leaks or memory corruption was found

If this is a change to packaging of containers or native binaries then please confirm it works for all targets.

Run local packaging test showing all targets (including any new ones) build.
Set ok-package-test label to test for all targets (requires maintainer to do).

Documentation

Documentation required for this feature

fluent/fluent-bit-docs#1350

Backporting

Backport to latest stable release.

Fluent Bit is licensed under Apache 2.0, by submitting this pull request I understand that this code will be released under the terms of that license.

…per a cycle Previously, I had chosen to restrict with the threshold to prevent unlimited subscribing per a cycle. This should cause flood of memory comsumptions but it depends on the amound of the channels of Windows EventLog. Nowadays, this plugin is getting widely used. Also, in the fluent community slack, there is a request to make to be able to configure for this threshold per a cycle. To prevent slowing down for the subscribe the channels, it won't de able to set up below 512KiB with the newly introduced parameter. Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>

harshnasitcrest · 2024-04-07T08:03:19Z

Can someone prioritize the review and merge this PR? @edsiper @leonardo-albertovich @fujimotos @koleini

It'd resolve one part of performance issues (Ref: #8673) for winevtlog plugin and hence would be of great help!

leonardo-albertovich

Please rename the option name so it properly reflects what it does.

plugins/in_winevtlog/in_winevtlog.c

Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>

leonardo-albertovich

Please use

leonardo-albertovich · 2024-04-10T11:33:00Z

plugins/in_winevtlog/in_winevtlog.c

+#define DEFAULT_THRESHOLD_SIZE 0x7ffff /* Default reading buffer size */
+                                       /* (512kib = 524287bytes) */
+#define MINIMUM_THRESHOLD_SIZE 0x0400   /* 1024 bytes */
+#define MAXIMUM_THRESHOLD_SIZE 0x1ccccd /* 1887437 bytes (about 1.8 MiB) */


Set MAXIMUM_THRESHOLD_SIZE to (FLB_INPUT_CHUNK_FS_MAX_SIZE - (1024 * 200)) so it scales in case someone changes the default maximum chunk size.

I guess in the future when we make the chunk size configurable we'll have to do that in runtime.

Sorry about not requesting it this way the first time around, it must've slipped from my mind.

No worries. Your suggestion is also helpful. And it has more effective than just specified the fixed value for the current adequate value.

…pecification of chunk size Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>

Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>

leonardo-albertovich · 2024-04-11T12:21:13Z

plugins/in_winevtlog/in_winevtlog.c

+        flb_utils_bytes_to_human_readable_size((size_t) MINIMUM_THRESHOLD_SIZE,
+                                               human_readable_size,
+                                               sizeof(human_readable_size) - 1);
+        flb_plg_error(ctx->ins,


You probably meant to use flb_plg_warn here

Oh, good catch. I'll fix it shortly.

Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>

edsiper · 2024-04-11T16:12:52Z

thanks!

cosmo0920 requested review from edsiper, leonardo-albertovich, fujimotos and koleini as code owners April 5, 2024 07:54

github-actions bot added the docs-required label Apr 5, 2024

cosmo0920 temporarily deployed to pr April 5, 2024 07:54 — with GitHub Actions Inactive

cosmo0920 force-pushed the cosmo0920-configurable-threshold-size-per-a-cycle-on-in_winevtlog branch from 9666fe5 to 0930d59 Compare April 5, 2024 07:54

cosmo0920 changed the title ~~in_winevtlog: Make a configurable for the size of collecting threshold per a cycle~~ in_winevtlog: Make configurable for the size of collecting threshold per a cycle Apr 5, 2024

cosmo0920 temporarily deployed to pr April 5, 2024 07:55 — with GitHub Actions Inactive

cosmo0920 mentioned this pull request Apr 5, 2024

in_winevtlog: Add a description for Read_Limit_Per_Cycle fluent/fluent-bit-docs#1350

Merged

cosmo0920 force-pushed the cosmo0920-configurable-threshold-size-per-a-cycle-on-in_winevtlog branch from 0930d59 to 4dbeeb7 Compare April 5, 2024 08:13

cosmo0920 temporarily deployed to pr April 5, 2024 08:14 — with GitHub Actions Inactive

cosmo0920 had a problem deploying to pr April 5, 2024 08:14 — with GitHub Actions Failure

cosmo0920 temporarily deployed to pr April 5, 2024 08:14 — with GitHub Actions Inactive

cosmo0920 force-pushed the cosmo0920-configurable-threshold-size-per-a-cycle-on-in_winevtlog branch from 4dbeeb7 to f6a4d46 Compare April 5, 2024 10:31

cosmo0920 temporarily deployed to pr April 5, 2024 10:31 — with GitHub Actions Inactive

cosmo0920 temporarily deployed to pr April 5, 2024 10:51 — with GitHub Actions Inactive

cosmo0920 temporarily deployed to pr April 5, 2024 10:52 — with GitHub Actions Inactive

leonardo-albertovich requested changes Apr 8, 2024

View reviewed changes

plugins/in_winevtlog/in_winevtlog.c Outdated Show resolved Hide resolved

edsiper added this to the Fluent Bit v3.0.2 milestone Apr 8, 2024

in_winevtlog: Address comments

83a247c

Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>

leonardo-albertovich requested changes Apr 10, 2024

View reviewed changes

in_winevtlog: Specify the maximum cutoff value thorough the current s…

ced8b0b

…pecification of chunk size Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>

cosmo0920 temporarily deployed to pr April 11, 2024 02:29 — with GitHub Actions Inactive

cosmo0920 temporarily deployed to pr April 11, 2024 02:49 — with GitHub Actions Inactive

in_winevtlog: Display the upper limit with the calculated value

a5caba4

Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>

cosmo0920 temporarily deployed to pr April 11, 2024 09:35 — with GitHub Actions Inactive

cosmo0920 temporarily deployed to pr April 11, 2024 09:54 — with GitHub Actions Inactive

cosmo0920 temporarily deployed to pr April 11, 2024 09:55 — with GitHub Actions Inactive

leonardo-albertovich previously approved these changes Apr 11, 2024

View reviewed changes

in_winevtlog: Downgrade for the log level for lower threshold

cf0c5bf

Signed-off-by: Hiroshi Hatake <hiroshi@chronosphere.io>

cosmo0920 dismissed leonardo-albertovich’s stale review via cf0c5bf April 11, 2024 12:37

cosmo0920 temporarily deployed to pr April 11, 2024 12:37 — with GitHub Actions Inactive

cosmo0920 requested a review from leonardo-albertovich April 11, 2024 12:37

cosmo0920 temporarily deployed to pr April 11, 2024 12:55 — with GitHub Actions Inactive

cosmo0920 temporarily deployed to pr April 11, 2024 12:56 — with GitHub Actions Inactive

leonardo-albertovich approved these changes Apr 11, 2024

View reviewed changes

edsiper merged commit 65700a7 into master Apr 11, 2024
46 checks passed

edsiper deleted the cosmo0920-configurable-threshold-size-per-a-cycle-on-in_winevtlog branch April 11, 2024 16:12

BrewTestBot mentioned this pull request Apr 12, 2024

fluent-bit 3.0.2 Homebrew/homebrew-core#168826

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

in_winevtlog: Make configurable for the size of collecting threshold per a cycle #8677

in_winevtlog: Make configurable for the size of collecting threshold per a cycle #8677

cosmo0920 commented Apr 5, 2024 •

edited

Loading

harshnasitcrest commented Apr 7, 2024

leonardo-albertovich left a comment

leonardo-albertovich left a comment

leonardo-albertovich Apr 10, 2024

leonardo-albertovich Apr 10, 2024

cosmo0920 Apr 11, 2024

leonardo-albertovich Apr 11, 2024

cosmo0920 Apr 11, 2024

edsiper commented Apr 11, 2024

in_winevtlog: Make configurable for the size of collecting threshold per a cycle #8677

in_winevtlog: Make configurable for the size of collecting threshold per a cycle #8677

Conversation

cosmo0920 commented Apr 5, 2024 • edited Loading

harshnasitcrest commented Apr 7, 2024

leonardo-albertovich left a comment

Choose a reason for hiding this comment

leonardo-albertovich left a comment

Choose a reason for hiding this comment

leonardo-albertovich Apr 10, 2024

Choose a reason for hiding this comment

leonardo-albertovich Apr 10, 2024

Choose a reason for hiding this comment

cosmo0920 Apr 11, 2024

Choose a reason for hiding this comment

leonardo-albertovich Apr 11, 2024

Choose a reason for hiding this comment

cosmo0920 Apr 11, 2024

Choose a reason for hiding this comment

edsiper commented Apr 11, 2024

cosmo0920 commented Apr 5, 2024 •

edited

Loading