Add /proc/net/udp parsing especially for the tx_queue and rx_queue lengths #235

peterbueschel · 2019-11-19T12:50:44Z

Hi @pgier,

this belongs to the requested of @discordianfish in prometheus/node_exporter#1503.

Signed-off-by: Peter Bueschel peter.bueschel@logmein.com

peterbueschel · 2019-11-19T13:00:02Z

I would update prometheus/node_exporter#1503 as soon as this PR is merged.

SuperQ · 2019-11-19T14:02:00Z

Nice. It would also be good to include /proc/net/udp6 as well.

SuperQ · 2019-11-19T14:08:19Z

net_udp.go

+
+// newNetUDP creates a new NetUDP from the contents of the given file.
+func newNetUDP(file string) (*NetUDP, error) {
+	f, err := os.Open(file)


We're trying to make use of the procfs utli.ReadFileNoStat(), in order to read this data more atomically from the kernel.

But, look at this file, it seems like it could be very big in a real-world environment. Do you have any data on what size this file can grow to?

We have a hard limit on our hosts of 1300 concurrent connections. I measured locally that one line is about 150 Byte. So, 1300x150 Byte = 19.5 KByte =~ 19 Kibibyte.

If we play a bit with the numbers and say we will use the number of possible ports (and forget about other limits like open files), which is limited to 16 bits for the source port field in a UDPv4 header, we could have 2^16 = 65536 lines for a server listening on a single IP.

--> So, with 150 Byte per line we theoretically have a filesize of 65536 * 150 Byte ~ 10 MByte

But lets have a look at some numbers available at the internet:

Whatsapp (published 2012: https://web.archive.org/web/20140501234954/https://blog.whatsapp.com/196/1-million-is-so-2011) they were able to handle 2M TCP connections at this time. Just for the fun lets say 2M UDP connections will then create a /proc/net/udp file with size of 300 MByte

MigratoryData WebSocket Server (published: 2013 https://mrotaru.wordpress.com/2013/06/20/12-million-concurrent-connections-with-migratorydata-websocket-server/) they can handle 12M concurrent connections. Even this is not UDP related and more socket descriptor magic, let's use this number and we have a filesize of 1.8 GByte...

Means, we have a filesize range from more realistic KBytes to unusual GBytes. As an engineer I would choose the middle and estimate a maximum of multiple MByte per IP.

Thanks for the followup, good stuff.

I guess we should stick to this streaming reader for large file safety. What do you think @mdlayher?

I think it's most reasonable to stick to a streaming read but would also encourage using an io.LimitReader when initializing the scanner and set some sane upper bound like 4GiB of file.

SuperQ · 2019-11-19T14:10:31Z

net_udp.go

+)
+
+// NetUDP returns kernel/networking statistics for udp datagrams read from /proc/net/udp.
+func (fs FS) NetUDP() (*NetUDP, error) {


Since this is a more specialty parser than the typical procfs reader, I might suggest we have a name like NetUDPSummary(), or NetUDPStats(). What do you think @pgier?

If I am allowed to add my 2 cents: The name should represents the location of the file in the proc filesystem. If someone else needs to extract some other data from this file, she/he/it could extend the function/struct instead of creating another file/reader. From my perspective, there is also not much more you can get from the content of the lines.

Most of our other parsers dump the raw contents of the file into a struct, rather than parse and produce some derived metrics. I agree, that the function should be named after the filepath, but since this isn't a raw dump, we should either have it be a different function name, or have a struct method to summarize.

So, the NetUDP type struct is a full contents of the file, and then you would call netUdp.Summary() to get TxQueueLength, etc.

+1. We should provide Go representations of the raw data and allow the caller to determine how the data is used. The convenience method could be reasonable if the logic is nontrivial.

mdlayher · 2019-11-22T23:28:56Z

net_udp.go

@@ -0,0 +1,94 @@
+// Copyright 2018 The Prometheus Authors


mdlayher · 2019-11-22T23:29:35Z

net_udp.go

+type (
+	// NetUDPLine is a line parsed from /proc/net/udp
+	// For the proc file format details, see https://linux.die.net/man/5/proc
+	NetUDPLine struct {


Unexport this type and add a doc comment to NetUDP.

Actually if we decide to expose this directly it should remain exported.

mdlayher · 2019-11-22T23:30:54Z

net_udp.go

+)
+
+// NetUDP returns kernel/networking statistics for udp datagrams read from /proc/net/udp.
+func (fs FS) NetUDP() (*NetUDP, error) {


+1. We should provide Go representations of the raw data and allow the caller to determine how the data is used. The convenience method could be reasonable if the logic is nontrivial.

mdlayher · 2019-11-22T23:32:21Z

net_udp.go

+
+// newNetUDP creates a new NetUDP from the contents of the given file.
+func newNetUDP(file string) (*NetUDP, error) {
+	f, err := os.Open(file)


I think it's most reasonable to stick to a streaming read but would also encourage using an io.LimitReader when initializing the scanner and set some sane upper bound like 4GiB of file.

mdlayher · 2019-11-22T23:34:29Z

net_udp.go

+		)
+	}
+	q := strings.Split(fields[4], ":")
+	if len(q) < 2 {


Based on what I'm seeing on my machine, I think this should be != 2.

mdlayher · 2019-11-22T23:35:48Z

net_udp_test.go

+)
+
+func Test_parseNetUDPLine(t *testing.T) {
+	type args struct {


I'm not sure this extra struct is necessary since there's only one field.

mdlayher · 2019-11-22T23:35:54Z

net_udp_test.go

@@ -0,0 +1,131 @@
+// Copyright 2018 The Prometheus Authors


mdlayher · 2019-11-22T23:36:14Z

net_udp_test.go

+}
+
+func Test_newNetUDP(t *testing.T) {
+	type args struct {


mdlayher

I would also like to see udp6 parsed as well. It appears much of the logic can be reused and we can add NetUDP6 method that returns a NetUDP type (the fields are identical yeah?).

I don't like the UDP/UDP6 convention personally but I guess we should do what the kernel does.

mdlayher · 2019-11-23T05:33:48Z

You can see how I handled sockstat/sockstat6 in #237.

@pgier

…ngths. @pgier this belongs to the requested of @discordianfish in prometheus/node_exporter#1503. Signed-off-by: Peter Bueschel <peter.bueschel@logmein.com>

Add UDPv6 support. Limit file read to 4GiB. Signed-off-by: Peter Bueschel <peter.bueschel@logmein.com>

peterbueschel · 2019-11-26T17:18:30Z

Hi @SuperQ , Hi @mdlayher ,

as discussed UDP6 support and a read limitation were added. Also the NetUDP represents now the lines parsed from the /proc/net/udp{,6} file, while the summary struct contains the total lengths. A user can decide if he/she/it wants to collect directly the queue lengths without storing the parsed lines or if he/she/it needs all lines for further calculations.

pgier

LGTM, just had a couple of suggestions related to the comments

pgier · 2019-12-03T21:46:12Z

net_udp.go

+)
+
+const (
+	readLimit = 4294967296 // Bytes


I think it would be helpful to add a comment with some info for future reference about why we need the limit and how the number was chosen.

pgier · 2019-12-03T22:00:57Z

net_udp.go

+	return netUDP, nil
+}
+
+// newNetUDP creates a new NetUDP{,6} from the contents of the given file.


Comment should be updated to newNetUDPSummary to match the function name.

Signed-off-by: Peter Bueschel <peter.bueschel@logmein.com>

SuperQ requested a review from pgier November 19, 2019 14:00

SuperQ reviewed Nov 19, 2019

View reviewed changes

mdlayher reviewed Nov 22, 2019

View reviewed changes

Peter Bueschel added 2 commits November 26, 2019 17:59

Add /proc/net/udp parsing especially for the tx_queue and rx_queue le…

5ac4381

…ngths. @pgier this belongs to the requested of @discordianfish in prometheus/node_exporter#1503. Signed-off-by: Peter Bueschel <peter.bueschel@logmein.com>

Add extra summary struct for /proc/net/upd{,6}.

1632cd9

Add UDPv6 support. Limit file read to 4GiB. Signed-off-by: Peter Bueschel <peter.bueschel@logmein.com>

pgier approved these changes Dec 3, 2019

View reviewed changes

Add explanation to the readLimit. Fix comment on newNetUDPSummary.

dee3ff5

Signed-off-by: Peter Bueschel <peter.bueschel@logmein.com>

pgier merged commit fa4d6ce into prometheus:master Dec 9, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add /proc/net/udp parsing especially for the tx_queue and rx_queue lengths #235

Add /proc/net/udp parsing especially for the tx_queue and rx_queue lengths #235

peterbueschel commented Nov 19, 2019

peterbueschel commented Nov 19, 2019

SuperQ commented Nov 19, 2019

SuperQ Nov 19, 2019

peterbueschel Nov 20, 2019

SuperQ Nov 22, 2019

mdlayher Nov 22, 2019

SuperQ Nov 19, 2019

peterbueschel Nov 20, 2019

SuperQ Nov 22, 2019

mdlayher Nov 22, 2019

mdlayher Nov 22, 2019

mdlayher Nov 22, 2019

mdlayher Nov 22, 2019

mdlayher Nov 22, 2019

mdlayher Nov 22, 2019

mdlayher Nov 22, 2019

mdlayher Nov 22, 2019

mdlayher Nov 22, 2019

mdlayher Nov 22, 2019

mdlayher left a comment

mdlayher commented Nov 23, 2019

peterbueschel commented Nov 26, 2019

pgier left a comment

pgier Dec 3, 2019

pgier Dec 3, 2019

Add /proc/net/udp parsing especially for the tx_queue and rx_queue lengths #235

Add /proc/net/udp parsing especially for the tx_queue and rx_queue lengths #235

Conversation

peterbueschel commented Nov 19, 2019

peterbueschel commented Nov 19, 2019

SuperQ commented Nov 19, 2019

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

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

mdlayher left a comment

Choose a reason for hiding this comment

mdlayher commented Nov 23, 2019

peterbueschel commented Nov 26, 2019

pgier left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment