fix: correct 32-bit SNMP counter wrap detection for all traffic calculations (#163)

The wrap detection only applied the 2^32 correction when the previous
counter was in the upper half of the 32-bit range (> 2^31). On fast
links, counters wrap regardless of where in the range they sit. Three
graph rendering paths had no wrap detection at all, just max(0).

Consolidated all bps calculations into a single public
Capacity.calculate_bps/3 that always tries the 2^32 correction for
negative deltas. 64-bit HC counter resets remain clamped to zero since
the corrected delta stays negative.

Reviewed-on: graham/towerops-web#163

lib/towerops/capacity.ex

@@ -161,18 +161,27 @@ defmodule Towerops.Capacity do
     end
   end
 
-  defp calculate_bps(nil, _, _), do: 0.0
-  defp calculate_bps(_, nil, _), do: 0.0
+  @doc """
+  Calculates bits per second from two consecutive octet counter readings.
 
-  defp calculate_bps(prev_octets, curr_octets, time_diff) do
+  Handles 32-bit SNMP counter wraps (ifInOctets/ifOutOctets wrap at 2^32)
+  and 64-bit HC counter resets (device reboot). Always tries a 2^32
+  correction when the delta is negative — this works because:
+
+  - 32-bit wrap: corrected delta is positive and correct
+  - 64-bit reset: corrected delta is still very negative, clamped to 0
+  """
+  def calculate_bps(nil, _, _), do: 0.0
+  def calculate_bps(_, nil, _), do: 0.0
+
+  def calculate_bps(prev_octets, curr_octets, time_diff) do
     delta = curr_octets - prev_octets
 
-    # When prev is in the upper half of 32-bit range and delta is negative,
-    # the 32-bit counter likely wrapped around 2^32. Add 2^32 to recover
-    # the correct delta. If it's still negative after the correction, it's
-    # a device reboot/reset on 64-bit HC counters — clamp to 0.
     delta =
-      if delta < 0 and prev_octets > 2_147_483_648 do
+      if delta < 0 do
+        # Try 32-bit counter wrap correction. For 64-bit HC counter resets
+        # (e.g. 10 TB → 1 MB), the corrected delta stays negative and gets
+        # clamped to 0 below.
         delta + 4_294_967_296
       else
         delta

lib/towerops_web/graphql/resolvers/time_series.ex

@@ -158,8 +158,8 @@ defmodule ToweropsWeb.GraphQL.Resolvers.TimeSeries do
   defp compute_bps(_, nil, _time_diff), do: 0.0
 
   defp compute_bps(octets1, octets2, time_diff) do
-    ((octets2 - octets1) * 8 / time_diff)
-    |> max(0)
+    octets1
+    |> Towerops.Capacity.calculate_bps(octets2, time_diff)
     |> Float.round(2)
   end
 

lib/towerops_web/live/device_live/show.ex

@@ -1263,16 +1263,7 @@ defmodule ToweropsWeb.DeviceLive.Show do
   end
 
   defp calculate_counter_bps(prev_octets, curr_octets, time_diff) do
-    delta = curr_octets - prev_octets
-
-    delta =
-      if delta < 0 and prev_octets > 2_147_483_648 do
-        delta + 4_294_967_296
-      else
-        delta
-      end
-
-    max(0.0, delta * 8 / time_diff)
+    Towerops.Capacity.calculate_bps(prev_octets, curr_octets, time_diff)
   end
 
   defp build_traffic_json_datasets(all_stats, device) do

lib/towerops_web/live/graph_live/show.ex

@@ -887,21 +887,15 @@ defmodule ToweropsWeb.GraphLive.Show do
     stats
     |> Enum.chunk_every(2, 1, :discard)
     |> Enum.map(fn [stat1, stat2] ->
-      # Skip if octets are nil
       if is_nil(stat1.if_in_octets) or is_nil(stat2.if_in_octets) do
         nil
       else
         time_diff = stat2.checked_at |> DateTime.diff(stat1.checked_at, :second) |> max(1)
-
-        bps =
-          ((stat2.if_in_octets - stat1.if_in_octets) * 8 / time_diff)
-          |> max(0)
-          |> :erlang.float()
-          |> Float.round(2)
+        bps = Towerops.Capacity.calculate_bps(stat1.if_in_octets, stat2.if_in_octets, time_diff)
 
         %{
           x: DateTime.to_unix(stat2.checked_at, :millisecond),
-          y: bps
+          y: Float.round(bps, 2)
         }
       end
     end)
@@ -912,21 +906,15 @@ defmodule ToweropsWeb.GraphLive.Show do
     stats
     |> Enum.chunk_every(2, 1, :discard)
     |> Enum.map(fn [stat1, stat2] ->
-      # Skip if octets are nil
       if is_nil(stat1.if_out_octets) or is_nil(stat2.if_out_octets) do
         nil
       else
         time_diff = stat2.checked_at |> DateTime.diff(stat1.checked_at, :second) |> max(1)
-
-        bps =
-          ((stat2.if_out_octets - stat1.if_out_octets) * 8 / time_diff)
-          |> max(0)
-          |> :erlang.float()
-          |> Float.round(2)
+        bps = Towerops.Capacity.calculate_bps(stat1.if_out_octets, stat2.if_out_octets, time_diff)
 
         %{
           x: DateTime.to_unix(stat2.checked_at, :millisecond),
-          y: -bps
+          y: -Float.round(bps, 2)
         }
       end
     end)
@@ -1381,16 +1369,15 @@ defmodule ToweropsWeb.GraphLive.Show do
 
       previous ->
         time_diff = current.polled_at |> DateTime.diff(previous.polled_at, :second) |> max(1)
-        in_bps = calculate_bps(current.if_in_octets, previous.if_in_octets, time_diff)
-        out_bps = calculate_bps(current.if_out_octets, previous.if_out_octets, time_diff)
+        in_bps = calculate_live_bps(current.if_in_octets, previous.if_in_octets, time_diff)
+        out_bps = calculate_live_bps(current.if_out_octets, previous.if_out_octets, time_diff)
         [%{interface_id: current.interface_id, in_bps: in_bps, out_bps: out_bps}]
     end
   end
 
-  # Calculate BPS from octets difference
-  defp calculate_bps(current_octets, previous_octets, time_diff) do
+  defp calculate_live_bps(current_octets, previous_octets, time_diff) do
     if current_octets && previous_octets do
-      max((current_octets - previous_octets) * 8 / time_diff, 0)
+      Towerops.Capacity.calculate_bps(previous_octets, current_octets, time_diff)
     else
       0.0
     end

test/towerops/capacity_test.exs

@@ -83,7 +83,7 @@ defmodule Towerops.CapacityTest do
       assert_in_delta result.max_bps, 133_333.33, 1.0
     end
 
-    test "clamps to zero for small counter decreases (device reset or glitch)", %{interface: interface} do
+    test "applies 32-bit wrap correction for small counter decreases", %{interface: interface} do
       now = DateTime.utc_now()
       t1 = DateTime.add(now, -120, :second)
       t2 = DateTime.add(now, -60, :second)
@@ -94,7 +94,9 @@ defmodule Towerops.CapacityTest do
         checked_at: t1
       })
 
-      # Small decrease - treated as reset/glitch, not a 32-bit wrap
+      # Counter decreased — could be 32-bit wrap on a fast link.
+      # We always apply 2^32 correction for negative deltas within 32-bit range.
+      # Corrected delta: (4,294,967,296 - 5,000,000) + 1,000,000 = 4,290,967,296
       insert_interface_stat(interface.id, %{
         if_in_octets: 1_000_000,
         if_out_octets: 6_000_000,
@@ -103,11 +105,43 @@ defmodule Towerops.CapacityTest do
 
       result = Capacity.calculate_throughput(interface.id, DateTime.add(now, -180, :second))
 
-      assert result.in_bps == 0.0
+      # 4,290,967,296 * 8 / 60 = 572,128,972.8 bps
+      assert_in_delta result.in_bps, 572_128_972.8, 1.0
       assert_in_delta result.out_bps, 133_333.33, 1.0
     end
 
-    test "recovers correct rate when 32-bit counter wraps", %{interface: interface} do
+    test "recovers correct rate when 32-bit counter wraps with prev in lower half of range", %{
+      interface: interface
+    } do
+      now = DateTime.utc_now()
+      t1 = DateTime.add(now, -120, :second)
+      t2 = DateTime.add(now, -60, :second)
+
+      # Counter at 1.36B (lower half of 32-bit range, < 2^31)
+      # This is the exact scenario from production data
+      insert_interface_stat(interface.id, %{
+        if_in_octets: 1_360_303_152,
+        if_out_octets: 2_961_644_622,
+        checked_at: t1
+      })
+
+      # Counter wrapped — high-bandwidth link pushed past 2^32
+      insert_interface_stat(interface.id, %{
+        if_in_octets: 105_884_342,
+        if_out_octets: 3_220_277_392,
+        checked_at: t2
+      })
+
+      result = Capacity.calculate_throughput(interface.id, DateTime.add(now, -180, :second))
+
+      # in: (4,294,967,296 - 1,360,303,152) + 105,884,342 = 3,040,548,486 bytes
+      # bps = 3,040,548,486 * 8 / 60 = 405,406,464.8
+      assert_in_delta result.in_bps, 405_406_464.8, 1.0
+      # out: positive delta, no wrap needed = (3,220,277,392 - 2,961,644,622) * 8 / 60
+      assert_in_delta result.out_bps, 34_484_369.33, 1.0
+    end
+
+    test "recovers correct rate when 32-bit counter wraps near max", %{interface: interface} do
       now = DateTime.utc_now()
       t1 = DateTime.add(now, -120, :second)
       t2 = DateTime.add(now, -60, :second)