-
-
Notifications
You must be signed in to change notification settings - Fork 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
feat: basic ipfs network diagnostics #7068
base: master
Are you sure you want to change the base?
Conversation
(needs tests) |
core/commands/netdiag.go
Outdated
To (your current IP): {{.LocalIP}} | ||
{{- end}} | ||
{{if .Gateway -}} | ||
Your router's administrative console can likely be found at: |
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.
this is less likely to be actionable in corporate / educational situations. Maybe clearer specification of "if you are on a personal home network" or similar?
a107df4
to
607ddf3
Compare
@jacobheun Is there an epic/meta issue for debugging/tools/troubleshooting? |
@autonome not at the moment, but this would be helpful. I think there is some overlap here with metrics/telemetry but they're discrete concerns. I'll open up a meta issue for tracking/discussion. |
607ddf3
to
fccd5f7
Compare
Just a note, that while I've updated this PR and it looks like it's passing tests we don't really have any tests to make sure this works (testing this is hard bc it's network level/configuration things). I've done a bit of local testing and found the results a little flaky (it wouldn't always realize if I was unreachable). We'll need to put a bit of time into review + testing before we ship this. Would love to get this in though, it should be super useful. |
cc @aarshkshah1992 you may want to modify this PR or use it as inspiration given your work on libp2p/go-libp2p#1042 |
} | ||
|
||
n.printf("Firewall Status: %s", status) | ||
if !n.hasNagged { |
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.
@Stebalien Would you remember why you put in this flag ? What's the use case ?
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 appears to be so that it only says it once, and then remembers that it has said it so it doesn't say it again, right?
n.printf("Firewall Status: %s", status) | ||
if !n.hasNagged { | ||
n.hasNagged = true | ||
n.printf("NOTE: Your node appears to be behind a firewall. Please run `ipfs diag net` to diagnose.") |
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.
How can we be sure here that we are behind a firewall ?
2089fca
to
e74e6da
Compare
@willscott @aschmahmann I've made some small changes and have updated the deps and rebased. This PR looks in pretty good shape. Please can. I get a review ? Will augment it with the NAT Type event on a separate PR as that work will time time to get merged. |
@aarshkshah1992 - looking at this conversation, the main thing this PR was waiting for was some sort of testing to gain confidence that we tell people the right things given their circumstances, and that future changes don't break this. Thoughts on any clear paths to gain confidence in this code? |
concretely: lets put in a sanity test: run a daemon. run this command. make sure daemon and command exit cleanly. |
This is needs a rebase and may be dependent on some updates in go-libp2p to support detecting the ability to do NAT traversal. |
This implements a basic command and subsystem for diagnosing various networking issues.