winlogbeat/eventlog: add initial event processing metrics support #33922

efd6 · 2022-12-02T00:18:05Z

What does this PR do?

This adds metrics for discarded_events_total, errors_total, received_events_count,
received_events_total and source_lag_time, and also a histogram for non-zero event
batch period intended to make burstiness of log event creation visible.

The metrics are visible for all winlogbeat/eventlog client packages, e.g. the
filebeat winlog input.

~~source_lag_count is left for a later PR.~~ source_lag_count is in a separate commit since it was noted as being potentially not worth the trouble. The code is not too complex and with a reasonably infrequent polling should not be a significant host load (initially set to once per minute).

Why is it important?

This allows us to help users configure their systems to match the requirements that they have.

Checklist

My code follows the style guidelines of this project
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
I have made corresponding change to the default configuration files
I have added tests that prove my fix is effective or that my feature works
I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

Author's Checklist

Recommend review by commit since the first commit is colour and movement.

How to test this PR locally

Related issues

Use cases

Screenshots

Logs

Attaching sample output from the /dataset endpoint: dataset.json.txt

elasticmachine · 2022-12-02T00:56:09Z

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

Start Time: 2023-01-06T05:31:35.479+0000
Duration: 40 min 53 sec

Test stats 🧪

Test	Results
Failed	0
Passed	930
Skipped	9
Total	939

💚 Flaky test report

Tests succeeded.

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

/test : Re-trigger the build.
/package : Generate the packages and run the E2E tests.
/beats-tester : Run the installation tests with beats-tester.
run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

elasticmachine · 2022-12-02T01:32:20Z

Pinging @elastic/security-external-integrations (Team:Security-External Integrations)

This adds metrics for discarded_events_total, errors_total, received_events_count, received_events_total and source_lag_time, and also a histogram for non-zero event batch period intended to make burstiness of log event creation visible. The metrics are visible for all winlogbeat/eventlog client packages, e.g. the filebeat winlog input. source_lag_count is left for a later PR.

This only operates on Windows hosts and is best effort only.

andrewkroh · 2022-12-06T04:33:24Z

winlogbeat/eventlog/poll_windows.go

+	}
+	defer windows.CloseHandle(event)
+
+	s, err := win.Subscribe(0, event, "", "", 0, win.EvtSubscribeStartAtOldestRecord)


I think this might not be doing what I would expect. This should be attempting to get the newest event's record ID to compare with reader's current event ID. I'm not sure if this can be done exclusively through flags or if an EvtSeek is needed.

Yes, you are right. I was off-by-sign.

I think this is right now; the docs are weak, but they say that the EvtSeekd handle should have come from a call to EvtQuery.

It's not right.

andrewkroh · 2022-12-06T19:29:40Z

winlogbeat/eventlog/eventlog.go

+	if id == "" {
+		return nil
+	}
+	reg, unreg := inputmon.NewInputRegistry(name, id, nil)


Let's make the input name be a constant winlog. This way when viewing this in agent we have input values like winlog, udp, cel, lumberjack, etc.

That should happen in the filebeat end, since this will also be being called from winlogbeat.

andrewkroh · 2022-12-06T19:30:47Z

winlogbeat/eventlog/eventlog.go

+		Register("histogram", metrics.NewHistogram(out.batchPeriod))
+
+	if poll > 0 && runtime.GOOS == "windows" {
+		out.lastRecordID = &atomic.Uint64{}


out.sourceLagN is never initialized which will cause a panic when something tries to take a snapshot of the metrics. You can see this if you start Winlogbeat with logging.metrics.namespaces: [stats, dataset] and wait 30s.

And I'm thinking that since this metric updates every 1 minute that we need a smaller reservoir size like say 15 so we are looking at samples over the last 15 minutes.

Suggested change

out.lastRecordID = &atomic.Uint64{}

out.lastRecordID = &atomic.Uint64{}

out.sourceLagN = metrics.NewUniformSample(15)

andrewkroh · 2022-12-06T19:54:17Z

winlogbeat/eventlog/eventlog.go

+		case <-t.C:
+			last, err := lastEvent(name, work[:], buf)
+			if err != nil {
+				m.logError(err)


While testing I saw the errors_total going up and traced it to here with and error string of The handle is invalid.

I never saw source_lag_count get updated.

Yeah, this is currently not correct, and I don't see from the MS documentation how to make it correct.

This reverts commit 6a9e46f. Reason for revert: I have not been able to get this particular functionality to work, so we'll leave it out for now and come back to it later.

sonarqubecloud · 2022-12-08T00:13:35Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
2 Code Smells

No Coverage information
No Duplication information

…3922) This adds metrics for discarded_events_total, errors_total, received_events_count, received_events_total and source_lag_time, and also a histogram for non-zero event batch period intended to make burstiness of log event creation visible. The metrics are visible for all winlogbeat/eventlog client packages, e.g. the filebeat winlog input. source_lag_count is left for a later PR.

winlogbeat/eventlog: restructure source to improve code locality

5b92d3a

efd6 added enhancement Filebeat Filebeat Winlogbeat Team:Security-External Integrations backport-skip Skip notification from the automated backport with mergify 8.7-candidate labels Dec 2, 2022

efd6 self-assigned this Dec 2, 2022

botelastic bot added needs_team Indicates that the issue/PR needs a Team:* label and removed needs_team Indicates that the issue/PR needs a Team:* label labels Dec 2, 2022

efd6 force-pushed the st5527-winlogbeat branch 2 times, most recently from 736b923 to 18d7626 Compare December 2, 2022 00:49

efd6 marked this pull request as ready for review December 2, 2022 01:32

efd6 requested a review from a team as a code owner December 2, 2022 01:32

efd6 force-pushed the st5527-winlogbeat branch 3 times, most recently from 3e062fe to 2580c12 Compare December 4, 2022 23:54

winlogbeat/eventlog: add source_lag_count metric

6a9e46f

This only operates on Windows hosts and is best effort only.

efd6 force-pushed the st5527-winlogbeat branch from 2580c12 to 6a9e46f Compare December 5, 2022 19:34

andrewkroh mentioned this pull request Dec 6, 2022

[Metricbeat] beat module - add metricset for input metrics #33953

Closed

andrewkroh reviewed Dec 6, 2022

View reviewed changes

efd6 force-pushed the st5527-winlogbeat branch from 1b6ca5b to 3e3f63c Compare December 6, 2022 06:52

address pr comment

e790fbe

efd6 force-pushed the st5527-winlogbeat branch from 3e3f63c to e790fbe Compare December 6, 2022 10:26

andrewkroh reviewed Dec 6, 2022

View reviewed changes

efd6 added 2 commits December 7, 2022 07:25

address pr comment

9584e2c

Revert "winlogbeat/eventlog: add source_lag_count metric"

bb22421

This reverts commit 6a9e46f. Reason for revert: I have not been able to get this particular functionality to work, so we'll leave it out for now and come back to it later.

efd6 requested a review from andrewkroh December 8, 2022 19:35

efd6 added 2 commits January 6, 2023 15:59

Merge branch 'master' into st5527-winlogbeat

69c556e

address pr comment

a69716c

andrewkroh approved these changes Jan 9, 2023

View reviewed changes

efd6 merged commit 64d8649 into elastic:main Jan 10, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

winlogbeat/eventlog: add initial event processing metrics support #33922

winlogbeat/eventlog: add initial event processing metrics support #33922

efd6 commented Dec 2, 2022 •

edited by andrewkroh

Loading

elasticmachine commented Dec 2, 2022 •

edited by jenkins-beats-ci bot

Loading

Build stats

Test stats 🧪

elasticmachine commented Dec 2, 2022

andrewkroh Dec 6, 2022

efd6 Dec 6, 2022

efd6 Dec 6, 2022

efd6 Dec 6, 2022

andrewkroh Dec 6, 2022

efd6 Dec 6, 2022

andrewkroh Dec 6, 2022

efd6 Dec 6, 2022

andrewkroh Dec 6, 2022

efd6 Dec 6, 2022

sonarqubecloud bot commented Dec 8, 2022

	out.lastRecordID = &atomic.Uint64{}
	out.lastRecordID = &atomic.Uint64{}
	out.sourceLagN = metrics.NewUniformSample(15)

winlogbeat/eventlog: add initial event processing metrics support #33922

winlogbeat/eventlog: add initial event processing metrics support #33922

Conversation

efd6 commented Dec 2, 2022 • edited by andrewkroh Loading

What does this PR do?

Why is it important?

Checklist

Author's Checklist

How to test this PR locally

Related issues

Use cases

Screenshots

Logs

elasticmachine commented Dec 2, 2022 • edited by jenkins-beats-ci bot Loading

💚 Build Succeeded

Build stats

Test stats 🧪

💚 Flaky test report

🤖 GitHub comments

elasticmachine commented Dec 2, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarqubecloud bot commented Dec 8, 2022

efd6 commented Dec 2, 2022 •

edited by andrewkroh

Loading

elasticmachine commented Dec 2, 2022 •

edited by jenkins-beats-ci bot

Loading