Adding protocols collector #1921

juancb · 2021-01-05T01:01:49Z

This patch adds a /proc/net/protocols collector. By default it only exports protocols matching the ^tcp.* regex in order to limit cardinality. This results in 12 additional metrics and are diagnostically useful in cases where the system is experiencing memory pressure. After looking through the repo and previous PRs it appeared to me that it was preferred to have the procfs parsing done in the procfs module. I have made the necessary commit there and this PR depends on prometheus/procfs#347

Signed-off-by: Juan Bran [email protected]

juancb · 2021-01-05T01:14:49Z

Even if the exporter is disabled by default a user might want to filter the protocols that they are exporting. For my particular use case I'm only interested in the stats that experience memory pressure, namely TCP. I imagined some users might want to track a subset of all protocols and figured some filtering mechanism would offer that flexibility.

roidelapluie · 2021-01-05T01:30:49Z

Even if the exporter is disabled by default a user might want to filter the protocols that they are exporting. For my particular use case I'm only interested in the stats that experience memory pressure, namely TCP. I imagined some users might want to track a subset of all protocols and figured some filtering mechanism would offer that flexibility.

Yes, I agree

roidelapluie · 2021-01-07T16:39:58Z

Does it make sense to have it in https://github.com/prometheus/procfs proper first?

juancb · 2021-01-08T18:30:23Z

Does it make sense to have it in https://github.com/prometheus/procfs proper first?

Yes that makes sense. What's the best way to go about that? Since the commit was accepted I figured I would wait for it to get included in a release so I can update the procfs version in the modules files for the PR. I'm happy to go about it another way if it's less noisy or easier for anyone involved.

roidelapluie · 2021-01-10T23:34:27Z

Once it will be merged in procfs and release you will be able to update this PR.

juancb · 2021-01-11T01:13:01Z

Sounds good, I'll keep an eye on the procfs release and update this PR accordingly. Thanks!

Signed-off-by: Juan Bran <[email protected]>

juancb · 2021-01-13T06:38:35Z

@roidelapluie Sorry for the mess here, I'm not used to the fork and pull model and I should have merged instead of rebasing. I can recreate the PR if that helps.

Also looks like an xfs bug fix in procfs v0.3.0 required updating the end-to-end tests, so I've updated that to fix CI.

roidelapluie · 2021-01-13T08:57:43Z

@roidelapluie Sorry for the mess here, I'm not used to the fork and pull model and I should have merged instead of rebasing. I can recreate the PR if that helps.

You could also force push, that would update this PR.

juancb · 2021-01-13T09:43:39Z

Force push to which repo/branch? I rebased to master and then force-pushed to my fork's branch juancb:net-protocols.

$ git status
On branch net-protocols
Your branch is up to date with 'origin/net-protocols'.

nothing to commit, working tree clean

$ git remote -v
origin  [email protected]:prometheus/node_exporter.git (fetch)
origin  [email protected]:juancb/node_exporter.git (push)

juancb · 2021-01-14T01:22:59Z

The juancb force-pushed the juancb:net-protocols branch from ... to ... messages were throwing me off, but I see under the commit tab what I expect to see. Let me know if there's anything else you'd like from me.

roidelapluie · 2021-01-19T12:36:34Z

I have found the time to review this.

I suggest tcpv6, tcp should go in labels and not be part of the metric names. We could also include "net" in the metric names.

node_net_protocols_sockets{protocol="ipv6"}

SuperQ · 2021-01-19T13:13:26Z

collector/protocols_linux.go

+					fmt.Sprintf("%s_memory", p.Name),
+				),
+				fmt.Sprintf("Total number of 4KB pages allocated by %s", p.Name),


This should be converted to bytes.

SuperQ · 2021-01-19T13:20:48Z

collector/protocols_linux.go

@@ -0,0 +1,165 @@
+// Copyright 2015 The Prometheus Authors


Suggested change

// Copyright 2015 The Prometheus Authors

// Copyright 2021 The Prometheus Authors

SuperQ · 2021-01-19T13:23:46Z

collector/protocols_linux.go

+					subsystem,
+					fmt.Sprintf("%s_pressure", p.Name),
+				),
+				fmt.Sprintf("Indicates whether %s is experiencing memory pressure; 1 = true, 0 = false, -1 = not implemented", p.Name),


This should be split into two metrics.

node_net_prototocols_implemented{protocol="TCPv6"} 1 node_net_prototocols_pressure{protocol="TCPv6"} 0

It looks like this is an issue in the procfs implementation. This should be fixed to return the proper state options as an iota.

discordianfish · 2021-11-12T10:52:56Z

@juancb Do you still want to finish this or should we close it?

McSim85 · 2024-07-24T19:23:01Z

Bump this.
The idea is nice, but my go-fu is not strong enough to complete this :)

juancb force-pushed the net-protocols branch from 2757698 to 4384586 Compare January 13, 2021 05:07

Juan Bran added 3 commits January 12, 2021 21:47

Adding protocols collector

0f4cdbd

Signed-off-by: Juan Bran <[email protected]>

upadting procfs to include dependency

580b14f

Signed-off-by: Juan Bran <[email protected]>

Cleaning up

0088e59

Signed-off-by: Juan Bran <[email protected]>

juancb force-pushed the net-protocols branch from 4384586 to 0088e59 Compare January 13, 2021 05:54

Updating e2e to match bug fix in procfs (see: procfs/pull/343)

69d2305

Signed-off-by: Juan Bran <[email protected]>

juancb force-pushed the net-protocols branch from 4bedd9c to 69d2305 Compare January 13, 2021 06:30

SuperQ reviewed Jan 19, 2021

View reviewed changes

SuperQ requested changes Jan 19, 2021

View reviewed changes

discordianfish added the pending/close label Nov 12, 2021

berzaza27 approved these changes Jan 29, 2022

View reviewed changes

discordianfish closed this Jan 30, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding protocols collector #1921

Adding protocols collector #1921

juancb commented Jan 5, 2021

juancb commented Jan 5, 2021

roidelapluie commented Jan 5, 2021

roidelapluie commented Jan 7, 2021

juancb commented Jan 8, 2021

roidelapluie commented Jan 10, 2021

juancb commented Jan 11, 2021

juancb commented Jan 13, 2021

roidelapluie commented Jan 13, 2021

juancb commented Jan 13, 2021 •

edited

Loading

juancb commented Jan 14, 2021

roidelapluie commented Jan 19, 2021 •

edited

Loading

SuperQ Jan 19, 2021

SuperQ Jan 19, 2021

SuperQ Jan 19, 2021

discordianfish commented Nov 12, 2021

McSim85 commented Jul 24, 2024

	// Copyright 2015 The Prometheus Authors
	// Copyright 2021 The Prometheus Authors

Adding protocols collector #1921

Adding protocols collector #1921

Conversation

juancb commented Jan 5, 2021

juancb commented Jan 5, 2021

roidelapluie commented Jan 5, 2021

roidelapluie commented Jan 7, 2021

juancb commented Jan 8, 2021

roidelapluie commented Jan 10, 2021

juancb commented Jan 11, 2021

juancb commented Jan 13, 2021

roidelapluie commented Jan 13, 2021

juancb commented Jan 13, 2021 • edited Loading

juancb commented Jan 14, 2021

roidelapluie commented Jan 19, 2021 • edited Loading

SuperQ Jan 19, 2021

Choose a reason for hiding this comment

SuperQ Jan 19, 2021

Choose a reason for hiding this comment

SuperQ Jan 19, 2021

Choose a reason for hiding this comment

discordianfish commented Nov 12, 2021

McSim85 commented Jul 24, 2024

juancb commented Jan 13, 2021 •

edited

Loading

roidelapluie commented Jan 19, 2021 •

edited

Loading