Adding protocols collector

[juancb]

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 juan.bran@verizondigitalmedia.com

[juancb]

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]

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]

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

[juancb]

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]

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

[juancb]

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

[juancb]

@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]

@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]

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  git@github.com:prometheus/node_exporter.git (fetch)
origin  git@github.com:juancb/node_exporter.git (push)

[juancb]

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]

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]

This should be converted to bytes.

[SuperQ]

Suggested change

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

[SuperQ]

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]

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

[McSim85]

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