filebeat/input/udp: add initial UDP metrics support #33870

efd6 · 2022-11-30T00:45:22Z

What does this PR do?

This adds metrics for UDP packet count and total bytes, and histograms for time required to process UDP packets prior to acking from a publication and time between UDP packet arrivals.

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

[ ]

How to test this PR locally

Related issues

Use cases

Screenshots

Logs

efd6 · 2022-11-30T01:20:06Z

filebeat/input/udp/input.go

 	}, nil
 }

+func configID(config *conf.C) (string, error) {


This is probably a good thing to factor out into inputmon.

andrewkroh · 2022-11-30T01:50:07Z

filebeat/input/udp/input.go

 	}, nil
 }

+func configID(config *conf.C) (string, error) {


I didn't realize that this input is not converted to the v2.Input interface that provides an ID. Can you please scope what the effort would be to convert it to the new interface.

It looks reasonably straight forward. The only inputs here that are v2 are filestream, journald, kafka, unix and winlog. The update would be better in another PR.

elasticmachine · 2022-11-30T01:57:10Z

💚 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: 2022-12-01T03:51:21.881+0000
Duration: 70 min 1 sec

Test stats 🧪

Test	Results
Failed	0
Passed	7133
Skipped	738
Total	7871

💚 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!)

This adds metrics for UDP packet count and total bytes, and histograms for time required to process UDP packets prior to acking from a publication and time between UDP packet arrivals.

elasticmachine · 2022-11-30T03:37:01Z

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

andrewkroh

This looks good. I have some naming suggestions. Can you please a table to the UDP input docs that describes the metrics (the aws-s3 input doc has an example).

andrewkroh · 2022-12-01T01:45:20Z

filebeat/input/udp/input.go

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


For the case of Fleet where an id should be present in the config, my desire would be to have same value represented in the metrics. Could we keep the ID as is in that case (without appending a device)

andrewkroh · 2022-12-01T02:15:41Z

filebeat/input/udp/input.go

+	reg, unreg := inputmon.NewInputRegistry("udp", id+"::"+device, nil)
+	out := &inputMetrics{
+		unregister:     unreg,
+		bufferLen:      monitoring.NewUint(reg, "udp_read_buffer_length"),


Our monitoring package does not have the concept of a gauge so I have been suffixing new gauges with _gauge. This gives you a quick hint how this value should be interpreted. It also hints to the monitoring log reporter that it should not compute a delta between the previous sample when logging this value. So can you add a suffix to this one.

andrewkroh · 2022-12-01T02:24:10Z

filebeat/input/udp/input.go

+		Register("histogram", metrics.NewHistogram(out.arrivalPeriod))
+	_ = adapter.NewGoMetrics(reg, "udp_processing_time", adapter.Accept).
+		Register("histogram", metrics.NewHistogram(out.processingTime))
+


Let me propose some slightly more generic names. Before we ship this in the next release I plan to look across the inputs to see if there are opportunities for alignment on naming.

received_bytes_total

received_events_total - My thinking is that s/packet/events/ will be more generic and applicable across more inputs.

arrival_period

processing_time

received_packets_total

received_events_total - My thinking is that s/packet/events/ will be more generic and applicable across more

Is the first of these intended to be received_bytes_total?

Yep. That should have been received_bytes_total. (updated original comment)

sonarcloud · 2022-12-01T03:58:41Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
2 Code Smells

No Coverage information
No Duplication information

efd6 · 2022-12-01T23:32:56Z

filebeat/input/udp/input.go

+}
+
+func (m *inputMetrics) close() {
+	if m != nil {


Ooops. Will send a fix. #33920

This adds metrics for UDP packet count and total bytes, and histograms for time required to process UDP packets prior to acking from a publication and time between UDP packet arrivals.

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

efd6 requested a review from andrewkroh November 30, 2022 00:45

efd6 self-assigned this Nov 30, 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 Nov 30, 2022

efd6 force-pushed the st-5524-filebeat branch from daf72d2 to cdb37c6 Compare November 30, 2022 00:46

efd6 requested a review from a team November 30, 2022 00:46

efd6 commented Nov 30, 2022

View reviewed changes

andrewkroh reviewed Nov 30, 2022

View reviewed changes

filebeat/input/udp: add initial UDP metrics support

6cd27d6

This adds metrics for UDP packet count and total bytes, and histograms for time required to process UDP packets prior to acking from a publication and time between UDP packet arrivals.

efd6 force-pushed the st-5524-filebeat branch from cdb37c6 to 6cd27d6 Compare November 30, 2022 02:06

efd6 marked this pull request as ready for review November 30, 2022 03:36

efd6 requested a review from a team as a code owner November 30, 2022 03:37

efd6 requested review from belimawr and rdner and removed request for a team November 30, 2022 03:37

andrewkroh reviewed Dec 1, 2022

View reviewed changes

address pr comments

923a290

efd6 force-pushed the st-5524-filebeat branch from 1feb8f3 to 923a290 Compare December 1, 2022 03:51

andrewkroh approved these changes Dec 1, 2022

View reviewed changes

efd6 merged commit f2e20e9 into elastic:main Dec 1, 2022

efd6 commented Dec 1, 2022

View reviewed changes

efd6 mentioned this pull request Dec 4, 2022

filebeat/input/udp: convert to v2 input #33930

Merged

6 tasks

andrewkroh mentioned this pull request Dec 6, 2022

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

Closed

efd6 mentioned this pull request Dec 17, 2022

filebeat/input/udp: add support for /proc/net/udp metrics #34070

Merged

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

filebeat/input/udp: add initial UDP metrics support #33870

filebeat/input/udp: add initial UDP metrics support #33870

efd6 commented Nov 30, 2022

efd6 Nov 30, 2022

andrewkroh Nov 30, 2022

efd6 Nov 30, 2022 •

edited

Loading

elasticmachine commented Nov 30, 2022 •

edited by jenkins-beats-ci bot

Loading

Build stats

Test stats 🧪

elasticmachine commented Nov 30, 2022

andrewkroh left a comment

andrewkroh Dec 1, 2022

andrewkroh Dec 1, 2022

andrewkroh Dec 1, 2022 •

edited

Loading

efd6 Dec 1, 2022

andrewkroh Dec 1, 2022 •

edited

Loading

sonarcloud bot commented Dec 1, 2022

efd6 Dec 1, 2022 •

edited

Loading

filebeat/input/udp: add initial UDP metrics support #33870

filebeat/input/udp: add initial UDP metrics support #33870

Conversation

efd6 commented Nov 30, 2022

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

efd6 Nov 30, 2022

Choose a reason for hiding this comment

andrewkroh Nov 30, 2022

Choose a reason for hiding this comment

efd6 Nov 30, 2022 • edited Loading

Choose a reason for hiding this comment

elasticmachine commented Nov 30, 2022 • edited by jenkins-beats-ci bot Loading

💚 Build Succeeded

Build stats

Test stats 🧪

💚 Flaky test report

🤖 GitHub comments

elasticmachine commented Nov 30, 2022

andrewkroh left a comment

Choose a reason for hiding this comment

andrewkroh Dec 1, 2022

Choose a reason for hiding this comment

andrewkroh Dec 1, 2022

Choose a reason for hiding this comment

andrewkroh Dec 1, 2022 • edited Loading

Choose a reason for hiding this comment

efd6 Dec 1, 2022

Choose a reason for hiding this comment

andrewkroh Dec 1, 2022 • edited Loading

Choose a reason for hiding this comment

sonarcloud bot commented Dec 1, 2022

efd6 Dec 1, 2022 • edited Loading

Choose a reason for hiding this comment

efd6 Nov 30, 2022 •

edited

Loading

elasticmachine commented Nov 30, 2022 •

edited by jenkins-beats-ci bot

Loading

andrewkroh Dec 1, 2022 •

edited

Loading

andrewkroh Dec 1, 2022 •

edited

Loading

efd6 Dec 1, 2022 •

edited

Loading