Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce parsing for /proc/<pid>/net/snmp, /proc/<pid>/net/snmp6 and /proc/<pid>/net/netstat. #435

Merged
merged 3 commits into from
Apr 1, 2022

Conversation

nikakis
Copy link
Contributor

@nikakis nikakis commented Feb 14, 2022

No description provided.

@SuperQ
Copy link
Member

SuperQ commented Feb 14, 2022

Nice, do we have to worry about missing fields in various kernel versions? We like to be able to support very old kernels in this library, since Go will run on kernels as old as 2.6.23.

@nikakis
Copy link
Contributor Author

nikakis commented Feb 17, 2022

@SuperQ I believe according to this RFC it seems that we will be fine.

@SuperQ
Copy link
Member

SuperQ commented Feb 17, 2022

Any chance I can also get you to implement snmp6 parsing in this PR too?

@nikakis
Copy link
Contributor Author

nikakis commented Feb 17, 2022

Sure i will implement snmp6 and netstat also.
Maybe i can also do the refactoring on the node_exporter side when this is done.

@nikakis nikakis changed the title Introduce parsing for /proc/<pid>/net/snmp. Introduce parsing for /proc/<pid>/net/snmp, /proc/<pid>/net/snmp6 and /proc/<pid>/net/netstat. Feb 18, 2022
@nikakis
Copy link
Contributor Author

nikakis commented Feb 18, 2022

@SuperQ I added also snmp6 and netstat. Have a look please!

.gitignore Outdated Show resolved Hide resolved
proc_netstat.go Outdated Show resolved Hide resolved
proc_snmp.go Outdated Show resolved Hide resolved
… and `/proc/<pid>/net/netstat`.

Signed-off-by: Nikos Kakavas <nikakis@protonmail.com>
proc_netstat.go Outdated Show resolved Hide resolved
proc_snmp.go Outdated Show resolved Hide resolved
Signed-off-by: Nikos Kakavas <nikakis@protonmail.com>
@nikakis nikakis force-pushed the introduce-support-for-snmp branch from dc5e963 to 5c12586 Compare March 9, 2022 15:23
@nikakis
Copy link
Contributor Author

nikakis commented Mar 9, 2022

@discordianfish @SuperQ Have a look again please!

Copy link
Member

@discordianfish discordianfish left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, LGTM!

@nikakis nikakis requested a review from SuperQ March 10, 2022 08:31
@nikakis
Copy link
Contributor Author

nikakis commented Mar 15, 2022

Hello! @discordianfish @SuperQ
Sorry for spamming but any update with merging this?
Thanks!

@discordianfish
Copy link
Member

Still needs a LGTM from @SuperQ or @pgier

Copy link
Collaborator

@pgier pgier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just had a couple of optional in-line suggestions.

proc_netstat.go Outdated Show resolved Hide resolved
proc_netstat.go Show resolved Hide resolved
proc_snmp.go Outdated Show resolved Hide resolved
Signed-off-by: Nikos Kakavas <nikakis@protonmail.com>
@pgier pgier merged commit d917e64 into prometheus:master Apr 1, 2022
@nikakis nikakis deleted the introduce-support-for-snmp branch April 3, 2022 16:31
// and returns a map contains those metrics.
func parseSNMP6Stats(r io.Reader) (ProcSnmp6, error) {
var (
scanner = bufio.NewScanner(r)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How did this gofmt issue not surfaced in the PR? @SuperQ @pgier any idea?

Copy link
Contributor Author

@nikakis nikakis Apr 14, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will fix this fmt issue and also try to investigate why this didn't worked properly

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Weird. Maybe out of date source branch?

remijouannet pushed a commit to remijouannet/procfs that referenced this pull request Oct 20, 2022
…snmp

Introduce parsing for `/proc/<pid>/net/snmp`, `/proc/<pid>/net/snmp6` and `/proc/<pid>/net/netstat`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants