Add support for RIPE Atlas Commands #459
Conversation
Codecov Report
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. @@ Coverage Diff @@
## master #459 +/- ##
==========================================
- Coverage 82.19% 81.54% -0.65%
==========================================
Files 129 134 +5
Lines 5908 6134 +226
==========================================
+ Hits 4856 5002 +146
- Misses 929 999 +70
- Partials 123 133 +10
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
|
I guess it needs some tests :D |
command/ripe_atlas/traceroute.go
Outdated
|
|
||
| var af int | ||
| address, err := netip.ParseAddr(destination) | ||
| if err != nil { |
There was a problem hiding this comment.
You could simplify the if by adding an || address.Is6() here. BTW why are we defaulting to IPv6 when the destination could not be parsed? Is this an implicit "is a hostname" check?
There was a problem hiding this comment.
This is more of a: if it can't be parsed, it's a hostname and if it's a hostname, use IPv6. The AF defined here is later used for the MeasurementRequest.
I need to actually test what would happen in the case a hostname has IPv4 only.
There was a problem hiding this comment.
As an idea we could also launch probes for both families
command/ripe_atlas/traceroute.go
Outdated
| } | ||
| defer response.Body.Close() | ||
|
|
||
| if response.StatusCode >= 300 { |
There was a problem hiding this comment.
Hmm 3xx codes are not errors, they are redirects.
Maybe you could try to centralise some of the request making / checking logic because some parts are very similar.
There was a problem hiding this comment.
I changed it to >= 400.
I will look into making a common function for the API requests.
lamaral
force-pushed
the
l_ripe_atlas
branch
from
3022e4a to
ad62786
Compare
|
resolved some conflicts and merged into master |
This MR adds initial support for tracerouting via the RIPE Atlas API.
This can aid network troubleshooting in a manner that is visible on Slack for all parties involved.
If the codestyle for this is good, I will further extend the traceroute command to allow for selecting a set of origin probes as well as adding some more commands.