diff --git a/CHANGES.rst b/CHANGES.rst index 16de957..3c309ad 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -1,6 +1,9 @@ Changelog ========= +* 1.1.7 + * Change condition of traceroute's ``last_hop_responded`` flag. + * Add couple of more traceroute's properties. ``is_success`` and ``last_hop_errors``. * 1.1.6 * Fix for `Issue #56`_ a case where the ``qbuf`` value wasn't being properly captured. diff --git a/ripe/atlas/sagan/traceroute.py b/ripe/atlas/sagan/traceroute.py index ee47969..d6bca96 100644 --- a/ripe/atlas/sagan/traceroute.py +++ b/ripe/atlas/sagan/traceroute.py @@ -26,7 +26,7 @@ def __init__(self, data, **kwargs): self.version = self.ensure("version", int) self.rfc4884 = self.ensure("rfc4884", bool) - self.objects = self.ensure("obj", list) + self.objects = self.ensure("obj", list) class Packet(ParsingDict): @@ -124,11 +124,14 @@ def __init__(self, data, **kwargs): self.hops = [] self.total_hops = 0 self.last_median_rtt = None - self._parse_hops(**kwargs) # Sets hops, last_median_rtt, and total_hops # Used by a few response tests below - self._destination_ip_responded = None - self._last_hop_responded = None + self.destination_ip_responded = False + self.last_hop_responded = False + self.is_success = False + self.last_hop_errors = [] + + self._parse_hops(**kwargs) # Sets hops, last_median_rtt, and total_hops @property def last_rtt(self): @@ -145,36 +148,35 @@ def target_responded(self): ) return self.destination_ip_responded - @property - def destination_ip_responded(self): - - if self._destination_ip_responded is not None: - return self._destination_ip_responded - - self._destination_ip_responded = False - if self.hops and self.hops[-1].packets: - destination_address = IP(self.destination_address) - for packet in self.hops[-1].packets: - if packet.origin and destination_address == IP(packet.origin): - self._destination_ip_responded = True - break - - return self.destination_ip_responded - - @property - def last_hop_responded(self): - - if self._last_hop_responded is not None: - return self._last_hop_responded - - self._last_hop_responded = False - if self.hops and self.hops[-1].packets: - for packet in self.hops[-1].packets: - if packet.origin: - self._last_hop_responded = True - break - - return self.last_hop_responded + def set_destination_ip_responded(self, last_hop): + """Sets the flag if destination IP responded.""" + destination_address = IP(self.destination_address) + for packet in last_hop.packets: + if packet.origin and destination_address == IP(packet.origin): + self.destination_ip_responded = True + break + + def set_last_hop_responded(self, last_hop): + """Sets the flag if last hop responded.""" + for packet in last_hop.packets: + if packet.rtt: + self.last_hop_responded = True + break + + def set_is_success(self, last_hop): + """Sets the flag if traceroute result is successfull or not.""" + for packet in last_hop.packets: + if packet.rtt and not packet.is_error: + self.is_success = True + break + else: + self.set_last_hop_errors(last_hop) + + def set_last_hop_errors(self, last_hop): + """Sets the last hop's errors..""" + for packet in last_hop.packets: + if packet.is_error: + self.last_hop_errors.append(packet.error_message) @property def end_time_timestamp(self): @@ -199,7 +201,8 @@ def _parse_hops(self, **kwargs): self._handle_malformation("Legacy formats not supported") return - for hop in hops: + hops_number = len(hops) + for index, hop in enumerate(hops): hop = Hop(hop, **kwargs) @@ -209,6 +212,12 @@ def _parse_hops(self, **kwargs): self.hops.append(hop) self.total_hops += 1 + # If last hop set several usefull properties + if index + 1 == hops_number: + self.set_destination_ip_responded(hop) + self.set_last_hop_responded(hop) + self.set_is_success(hop) + __all__ = ( "TracerouteResult", diff --git a/ripe/atlas/sagan/version.py b/ripe/atlas/sagan/version.py index 1436d8f..bf78826 100644 --- a/ripe/atlas/sagan/version.py +++ b/ripe/atlas/sagan/version.py @@ -1 +1 @@ -__version__ = "1.1.6" +__version__ = "1.1.7" diff --git a/tests/traceroute.py b/tests/traceroute.py index 8be8649..348e014 100644 --- a/tests/traceroute.py +++ b/tests/traceroute.py @@ -265,6 +265,7 @@ def test_traceroute_4610(): assert(result.last_median_rtt == 273.494) assert(result.destination_ip_responded is False) assert(result.last_hop_responded is False) + assert(result.is_success is False) assert(result.ip_path[3][1] == "137.164.47.14") assert(result.hops[0].packets[0].destination_option_size is None) assert(result.hops[0].packets[0].hop_by_hop_option_size is None) @@ -298,6 +299,7 @@ def test_traceroute_responding_target(): assert(result.last_median_rtt == 217.103) assert(result.destination_ip_responded is True) assert(result.last_hop_responded is True) + assert(result.is_success is True) assert(result.ip_path[3][1] == "68.85.214.113") assert(result.hops[0].packets[0].destination_option_size is None) assert(result.hops[0].packets[0].hop_by_hop_option_size is None) @@ -377,6 +379,33 @@ def test_traceroute_with_late_packets(): assert(len(result.hops[1].packets) == 3) assert(result.hops[1].packets[0].origin is None) assert(result.hops[1].packets[0].rtt is None) + assert(result.is_success is False) # Newer firmwares # No sample available yet + + +def test_last_responded_hop_with_error_packets(): + """" + Case where traceroute has a single hop and all of its packets have error + in them. + """ + + result = Result.get({'lts': 117, 'msm_id': 3059023, 'endtime': 1449152831, 'from': '2001:470:1883:1:6666:b3ff:feb0:dcda', 'dst_name': '2001:980:1284:10:beee:7bff:fe87:9eda', 'fw': 4720, 'timestamp': 1449152831, 'proto': 'ICMP', 'paris_id': 1, 'prb_id': 14468, 'af': 6, 'result': [{'result': [{'rtt': 0.714, 'size': 96, 'from': '2001:470:1883:1::1', 'err': 'N', 'ttl': 64}, {'rtt': 0.495, 'size': 96, 'from': '2001:470:1883:1::1', 'err': 'N', 'ttl': 64}, {'rtt': 0.48, 'size': 96, 'from': '2001:470:1883:1::1', 'err': 'N', 'ttl': 64}], 'hop': 1}], 'dst_addr': '2001:980:1284:10:beee:7bff:fe87:9eda', 'src_addr': '2001:470:1883:1:6666:b3ff:feb0:dcda', 'group_id': 3059023, 'type': 'traceroute', 'msm_name': 'Traceroute', 'size': 48}) + + assert(len(result.hops[0].packets) == 3) + assert(result.last_hop_responded is True) + assert(result.is_success is False) + + +def test_is_success_with_error_packets(): + """" + Case where traceroute has a single hop and all of its packets have error + in them. + """ + + result = Result.get({'lts': 117, 'msm_id': 3059023, 'endtime': 1449152831, 'from': '2001:470:1883:1:6666:b3ff:feb0:dcda', 'dst_name': '2001:980:1284:10:beee:7bff:fe87:9eda', 'fw': 4720, 'timestamp': 1449152831, 'proto': 'ICMP', 'paris_id': 1, 'prb_id': 14468, 'af': 6, 'result': [{'result': [{'rtt': 0.714, 'size': 96, 'from': '2001:470:1883:1::1', 'err': 'N', 'ttl': 64}, {'rtt': 0.495, 'size': 96, 'from': '2001:470:1883:1::1', 'err': 'N', 'ttl': 64}, {'rtt': 0.48, 'size': 96, 'from': '2001:470:1883:1::1', 'err': 'N', 'ttl': 64}], 'hop': 1}], 'dst_addr': '2001:980:1284:10:beee:7bff:fe87:9eda', 'src_addr': '2001:470:1883:1:6666:b3ff:feb0:dcda', 'group_id': 3059023, 'type': 'traceroute', 'msm_name': 'Traceroute', 'size': 48}) + + assert(len(result.hops[0].packets) == 3) + assert(result.is_success is False) + assert(result.last_hop_errors == ['Network unreachable', 'Network unreachable', 'Network unreachable'])