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

Add parsing /proc/net/snmp6 file for netstat-linux #615

Merged
merged 8 commits into from
Jul 8, 2017

Conversation

alezkv
Copy link
Contributor

@alezkv alezkv commented Jul 7, 2017

No description provided.

@SuperQ
Copy link
Member

SuperQ commented Jul 7, 2017

Please add a sample data file to the collector/fixtures/proc/net/ directory.

@SuperQ SuperQ self-requested a review July 7, 2017 15:24
return parseSNMP6Stats(file, fileName)
}

func parseSNMP6Stats(r io.Reader, fileName string) (map[string]map[string]string, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

fileName isn't actually used in this function.


for scanner.Scan() {
stat := strings.Fields(scanner.Text())
protocol := stat[0][:strings.Index(stat[0], "6") + 1]
Copy link
Contributor

Choose a reason for hiding this comment

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

You'll want to do bounds-checks on all of these slice index accesses.

stat := strings.Fields(scanner.Text())
protocol := stat[0][:strings.Index(stat[0], "6") + 1]
name := stat[0][strings.Index(stat[0], "6") + 1:]
if _, present := netStats[protocol]; ! present {
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like this isn't gofmt'd.

# HELP node_netstat_Ip6_InECT1Pkts Protocol Ip6 statistic InECT1Pkts.
# TYPE node_netstat_Ip6_InECT1Pkts untyped
node_netstat_Ip6_InECT1Pkts 0
# HELP node_netstat_Ip6_InHdrErrors Protocol Ip6 statistic InHdrErrors.
Copy link
Contributor

Choose a reason for hiding this comment

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

This is Ip6InHdrErrors, and I believe keeping the name together would make it easier for users to search for it online.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This format is the same as previews stats.
Have different naming convention among the same module will be very confusing.

Copy link
Contributor

Choose a reason for hiding this comment

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

The other source files are formatted differently. Most of these are obscure kernel stats that if you're lucky are mentioned on LKML somewhere, so keeping the original names increases the chances the users can find what little information there is about these stats.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is it ok if I change description to this format?
# HELP node_netstat_Ip6_InHdrErrors Statistic Ip6InHdrErrors.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think that'd be okay. These sort of metrics are always a tradeoff.

@alezkv
Copy link
Contributor Author

alezkv commented Jul 7, 2017

@SuperQ - add fixture and e2e output
@mdlayher - try to comply yours review

Copy link
Member

@SuperQ SuperQ left a comment

Choose a reason for hiding this comment

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

LGTM

@mdlayher
Copy link
Contributor

mdlayher commented Jul 8, 2017

LGTM as well.

@SuperQ SuperQ merged commit 7a914e5 into prometheus:master Jul 8, 2017
oblitorum pushed a commit to shatteredsilicon/node_exporter that referenced this pull request Apr 9, 2024
* Add parsing /proc/net/snmp6 file

* add /proc/net/snmp6 fixture

* fix e2e test

* gofmt

* remove unuser variable

* safe checks

* add tests

* change help format
tamcore pushed a commit to gitgrave/node_exporter that referenced this pull request Oct 22, 2024
Signed-off-by: prombot <prometheus-team@googlegroups.com>
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