The Wayback Machine - https://web.archive.org/web/20260202205744/https://github.com/prometheus/procfs/pull/435
Skip to content

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

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.

Sign up for GitHub

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

Jump to bottom
Merged
pgier merged 3 commits into from
Apr 1, 2022
Merged

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

pgier merged 3 commits into from
Apr 1, 2022

Conversation

@nikakis
Copy link
Contributor

@nikakis nikakis commented Feb 14, 2022

No description provided.

@nikakis nikakis mentioned this pull request Feb 14, 2022
Closed
@nikakis nikakis force-pushed the introduce-support-for-snmp branch from 6f39a9c to a8481ba Compare February 14, 2022 14:25
@SuperQ SuperQ self-requested a review February 14, 2022 14:40
@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 •
edited
Loading

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 nikakis force-pushed the introduce-support-for-snmp branch from a8481ba to c0cd365 Compare February 18, 2022 11:57
@nikakis
Copy link
Contributor Author

nikakis commented Feb 18, 2022

@SuperQ I added also snmp6 and netstat. Have a look please!
SuperQ
SuperQ requested changes Feb 20, 2022
View reviewed changes
@nikakis nikakis force-pushed the introduce-support-for-snmp branch from c0cd365 to 978dee6 Compare February 21, 2022 10:06
@nikakis
Introduce parsing for /proc/<pid>/net/snmp, /proc/<pid>/net/snmp6…
3d753b0 
… and `/proc/<pid>/net/netstat`.

Signed-off-by: Nikos Kakavas <nikakis@protonmail.com>
@nikakis nikakis force-pushed the introduce-support-for-snmp branch from 978dee6 to 3d753b0 Compare February 21, 2022 10:10
discordianfish
discordianfish reviewed Mar 7, 2022
View reviewed changes
discordianfish
discordianfish reviewed Mar 7, 2022
View reviewed changes
@nikakis
remove mapstructure
5c12586 
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!
discordianfish
discordianfish approved these changes Mar 9, 2022
View reviewed changes
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

discordianfish commented Mar 22, 2022

Still needs a LGTM from @SuperQ or @pgier
pgier
pgier approved these changes Mar 22, 2022
View reviewed changes
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.
@nikakis nikakis force-pushed the introduce-support-for-snmp branch from 99e9c79 to 18c4ef8 Compare March 30, 2022 12:21
@nikakis
Introduce strings.TrimSuffix instead of custom split
6ff7304 
Signed-off-by: Nikos Kakavas <nikakis@protonmail.com>
@nikakis nikakis force-pushed the introduce-support-for-snmp branch from 18c4ef8 to 6ff7304 Compare March 30, 2022 12:29
@pgier pgier merged commit d917e64 into prometheus:master Apr 1, 2022
@nikakis nikakis mentioned this pull request Apr 3, 2022
Open
@nikakis nikakis deleted the introduce-support-for-snmp branch April 3, 2022 16:31
discordianfish
discordianfish reviewed Apr 14, 2022
View reviewed changes
proc_snmp6.go
// and returns a map contains those metrics.
func parseSNMP6Stats(r io.Reader) (ProcSnmp6, error) {
var (
scanner = bufio.NewScanner(r)
Copy link
Member

@discordianfish discordianfish Apr 14, 2022

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 •
edited
Loading

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

@SuperQ SuperQ Apr 14, 2022

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
@pgier
Merge pull request prometheus#435 from nikakis/introduce-support-for-…
1ab1d7d 
…snmp

Introduce parsing for `/proc/<pid>/net/snmp`, `/proc/<pid>/net/snmp6` and `/proc/<pid>/net/netstat`.
@ffyuanda ffyuanda mentioned this pull request Jan 6, 2026
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

4 participants

@nikakis @SuperQ @discordianfish @pgier