Merge pull request #62 from RIPE-NCC/TracerouteIsErrorFlag

Enhancement of last_hop_responded flag

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.

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",

ripe/atlas/sagan/version.py

@@ -1 +1 @@
-__version__ = "1.1.6"
+__version__ = "1.1.7"

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'])