-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
fix netdev collector for linux #890
fix netdev collector for linux #890
Conversation
for some reason the test promt worning
but looks like the problem not in this changes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for catching this bug.
collector/netdev_linux.go
Outdated
@@ -50,12 +50,14 @@ func parseNetDevStats(r io.Reader, ignore *regexp.Regexp) (map[string]map[string | |||
scanner.Text()) | |||
} | |||
|
|||
header := strings.Fields(parts[1]) | |||
header_recieve := strings.Fields(parts[1]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use go-style variable names. For example this should be recieveHeader
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's also receive, not recieve
collector/netdev_linux.go
Outdated
netDev := map[string]map[string]string{} | ||
for scanner.Scan() { | ||
line := strings.TrimLeft(scanner.Text(), " ") | ||
parts := procNetDevFieldSep.Split(line, -1) | ||
if len(parts) != 2*len(header)+1 { | ||
if len(parts) != len(header_recieve)+len(header_transmit)+1 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's move len(header_recieve)+len(header_transmit)+1
up to line 55 as something like this:
headerLength := len(recieveHeader)+len(transmitHeader)+1
Then we can test len(parts) != headerLength
.
The actual test failure is below what you posted.
|
collector/netdev_linux.go
Outdated
} | ||
|
||
for i := 0; i < len(header_transmit); i++ { | ||
netDev[dev]["transmit_"+header_transmit[i]] = parts[i+1] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The offset on parts
here is incorrect. It should be parts[i+1+len(recieveHeader)]
.
3a050f5
to
e6fc076
Compare
@SuperQ fixed, please review |
@dmitriy-lukyanchikov the indentation is off. Please ensure that |
The end-to-end fixture needs to be updated. You can run Please also use |
Please make sure to |
3259656
to
25703b5
Compare
fix variable name, fix transmitHeader extracting modify fixtures to run tests with updated netdev_linux collector Signed-off-by: dmitriy-lukyanchikov <d.lukyanchikov@anchorfree.com>
25703b5
to
3420bc7
Compare
@SuperQ thank you for helping with this) now looks like all tests are passsed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, Awesome.
Ping @discordianfish / @grobie, any other comments? |
LGTM |
fix variable name, fix transmitHeader extracting modify fixtures to run tests with updated netdev_linux collector Signed-off-by: dmitriy-lukyanchikov <d.lukyanchikov@anchorfree.com>
header for transmit traffic is differs from recieve, so we cannot use recieve header as transmit, now we can see carier and colls metrics in prometheus