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

support to read from /proc/net/tcp{,6} #332

Merged
merged 7 commits into from
Feb 5, 2021
Merged

Conversation

hubt
Copy link
Contributor

@hubt hubt commented Sep 20, 2020

This provides support to read from /proc/net/tcp and /proc/net/tcp6. Which had originally been done with #20, but that was considered out of date, so @SuperQ suggested I open a new PR.

This change is not as extensive as it looks.

Because /proc/net/tcp{,6} is identical to /proc/net/udp{,6} and udp support already exists in net_udp.go, I went and refactored the net_udp.go to create a new set of base classes in net_ip_socket.go that could be used for both net_udp.go and net_tcp.go.

I moved most of the test code from net_udp_test.go and into net_ip_socket_test.go. I replicated the existing net_udp_test.go tests into net_tcp_test.go, and added a new tcp fixture identical to the udp one. I validated that the old UDP tests still work and new identical TCP tests also work.

I'm happy to rewrite this in a different way if people don't think this is a good idea. I had considered doing a copy of the existing net_udp.go code and just changing names to support TCP, but thought this was a bit more reusable if formats change or bugs are uncovered they would likely apply to both tcp and udp.

net_ip_socket.go Outdated Show resolved Hide resolved
net_ip_socket.go Outdated Show resolved Hide resolved
net_ip_socket.go Outdated Show resolved Hide resolved
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.

Thanks! LGTM beside minor nits.

@SuperQ
Copy link
Member

SuperQ commented Jan 11, 2021

Can you fix the DCO with git commit -s --amend?

wolandr and others added 7 commits January 12, 2021 05:02
Signed-off-by: Andrei Volnin <wolandr@gmail.com>
Signed-off-by: Hubert Chen <hubert@branchmetrics.io>
Signed-off-by: Hubert Chen <hubert@branchmetrics.io>
Signed-off-by: Hubert Chen <hubert@branchmetrics.io>
Signed-off-by: Hubert Chen <hubert@branchmetrics.io>
Signed-off-by: Hubert Chen <hubert@branchmetrics.io>
Signed-off-by: Hubert Chen <hubert@branchmetrics.io>
Signed-off-by: Hubert Chen <hubert@branchmetrics.io>
@SuperQ SuperQ merged commit 8c052e3 into prometheus:master Feb 5, 2021
@SuperQ
Copy link
Member

SuperQ commented Feb 5, 2021

This is causing the node_exporter to crash. :-(

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x8f6929]

goroutine 79 [running]:
github.com/prometheus/procfs.FS.NetUDP6Summary(0x7ffc346651b1, 0x17, 0x0, 0xc00040dca0, 0x2)
	/go/pkg/mod/github.com/prometheus/procfs@v0.4.0/net_udp.go:50 +0x169
github.com/prometheus/node_exporter/collector.(*udpQueuesCollector).Update(0xc00036c330, 0xc0003854a0, 0x110ea20, 0x0)
	/home/circleci/project/collector/udp_queues_linux.go:72 +0x26a
github.com/prometheus/node_exporter/collector.execute(0xc2f01f, 0xa, 0xd09900, 0xc00036c330, 0xc0003854a0, 0xd08fa0, 0xc0001d90e0)
	/home/circleci/project/collector/collector.go:153 +0x84
github.com/prometheus/node_exporter/collector.NodeCollector.Collect.func1(0xc0003854a0, 0xc0001d92f0, 0xd08fa0, 0xc0001d90e0, 0xc0001162a0, 0xc2f01f, 0xa, 0xd09900, 0xc00036c330)
	/home/circleci/project/collector/collector.go:144 +0x71
created by github.com/prometheus/node_exporter/collector.NodeCollector.Collect
	/home/circleci/project/collector/collector.go:143 +0x13c

@hubt
Copy link
Contributor Author

hubt commented Feb 5, 2021

Sorry. Will take a look today.

@hubt
Copy link
Contributor Author

hubt commented Feb 6, 2021

@SuperQ I built and ran the node_exporter master on amd64 linux with 3.x and 4.x kernels and didn't see any issues.

I noticed that node_exporter end-to-end-test.sh was skipping the test because there was no udp6 fixtures. When I add the fixtures from procfs it also didn't crash.

I did a circleci build and it passed tests and built all the cross compiled binaries fine.

What environment did you see the crash in? How can I replicate it?

@SuperQ
Copy link
Member

SuperQ commented Feb 6, 2021

We should not return a nil pointer if the fixtures don't exist. That edge case needs to be handled.

remijouannet pushed a commit to remijouannet/procfs that referenced this pull request Oct 20, 2022
* Add GIDs to ProcStatus struct

Signed-off-by: Andrei Volnin <wolandr@gmail.com>

* support to read from /proc/net/tcp{,6}

Signed-off-by: Hubert Chen <hubert@branchmetrics.io>
Co-authored-by: Andrei Volnin <wolandr@gmail.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