diff --git a/lib/towerops_web/channels/agent_channel.ex b/lib/towerops_web/channels/agent_channel.ex index f2eba536..d9bf7ecf 100644 --- a/lib/towerops_web/channels/agent_channel.ex +++ b/lib/towerops_web/channels/agent_channel.ex @@ -1329,10 +1329,14 @@ defmodule ToweropsWeb.AgentChannel do idx = iface.if_index [ - # ifHCInOctets (64-bit counter) + # ifHCInOctets (64-bit counter, preferred) "1.3.6.1.2.1.31.1.1.1.6.#{idx}", - # ifHCOutOctets (64-bit counter) + # ifHCOutOctets (64-bit counter, preferred) "1.3.6.1.2.1.31.1.1.1.10.#{idx}", + # ifInOctets (32-bit fallback for devices without HC counter support) + "1.3.6.1.2.1.2.2.1.10.#{idx}", + # ifOutOctets (32-bit fallback for devices without HC counter support) + "1.3.6.1.2.1.2.2.1.16.#{idx}", # ifInErrors "1.3.6.1.2.1.2.2.1.14.#{idx}", # ifOutErrors @@ -1963,6 +1967,13 @@ defmodule ToweropsWeb.AgentChannel do if map_size(oid_values) > length(snmp_device.sensors) + length(snmp_device.interfaces) * 6 do process_additional_polling_data(device, oid_values) end + + # Notify LiveViews that new sensor/interface data is available for graphing + Phoenix.PubSub.broadcast( + Towerops.PubSub, + "device:#{device.id}", + {:state_sensors_updated, device.id} + ) end # Process neighbors, ARP, MAC, IP addresses, processors, storage from agent polling @@ -2184,10 +2195,19 @@ defmodule ToweropsWeb.AgentChannel do Enum.map(interfaces, fn iface -> idx = iface.if_index + # Prefer 64-bit HC counters; fall back to 32-bit for devices that don't support ifXTable + in_octets = + parse_integer(Map.get(oid_values, "1.3.6.1.2.1.31.1.1.1.6.#{idx}")) || + parse_integer(Map.get(oid_values, "1.3.6.1.2.1.2.2.1.10.#{idx}")) + + out_octets = + parse_integer(Map.get(oid_values, "1.3.6.1.2.1.31.1.1.1.10.#{idx}")) || + parse_integer(Map.get(oid_values, "1.3.6.1.2.1.2.2.1.16.#{idx}")) + %{ interface_id: iface.id, - if_in_octets: parse_integer(Map.get(oid_values, "1.3.6.1.2.1.31.1.1.1.6.#{idx}")), - if_out_octets: parse_integer(Map.get(oid_values, "1.3.6.1.2.1.31.1.1.1.10.#{idx}")), + if_in_octets: in_octets, + if_out_octets: out_octets, if_in_errors: parse_integer(Map.get(oid_values, "1.3.6.1.2.1.2.2.1.14.#{idx}")), if_out_errors: parse_integer(Map.get(oid_values, "1.3.6.1.2.1.2.2.1.20.#{idx}")), if_in_discards: parse_integer(Map.get(oid_values, "1.3.6.1.2.1.2.2.1.13.#{idx}")), diff --git a/lib/towerops_web/live/device_live/show.html.heex b/lib/towerops_web/live/device_live/show.html.heex index c89b225b..4c3a9324 100644 --- a/lib/towerops_web/live/device_live/show.html.heex +++ b/lib/towerops_web/live/device_live/show.html.heex @@ -1347,6 +1347,7 @@ Utilization In Out + Errors @@ -1518,6 +1519,26 @@ do: format_bytes(interface.latest_stat.if_out_octets), else: "-"} + + <% total_errors = + interface.latest_stat && + (interface.latest_stat.if_in_errors || 0) + + (interface.latest_stat.if_out_errors || 0) + + (interface.latest_stat.if_in_discards || 0) + + (interface.latest_stat.if_out_discards || 0) %> + <%= if total_errors && total_errors > 0 do %> + <.link + navigate={ + ~p"/devices/#{@device.id}/graph/interface_errors?interface_id=#{interface.id}" + } + class="text-red-600 dark:text-red-400 hover:underline" + > + {total_errors} + + <% else %> + - + <% end %> + <% end %> diff --git a/lib/towerops_web/live/graph_live/show.ex b/lib/towerops_web/live/graph_live/show.ex index 761c6bda..7ad29dc0 100644 --- a/lib/towerops_web/live/graph_live/show.ex +++ b/lib/towerops_web/live/graph_live/show.ex @@ -352,10 +352,17 @@ defmodule ToweropsWeb.GraphLive.Show do defp get_title_suffix(assigns) do cond do - assigns[:sensor_id] -> get_sensor_name(assigns[:sensor_id]) - assigns[:interface_id] && assigns[:sensor_type] == "traffic" -> get_interface_name(assigns[:interface_id]) - assigns[:storage_id] && assigns[:sensor_type] == "storage_volume" -> get_storage_name(assigns[:storage_id]) - true -> nil + assigns[:sensor_id] -> + get_sensor_name(assigns[:sensor_id]) + + assigns[:interface_id] && assigns[:sensor_type] in ["traffic", "interface_errors"] -> + get_interface_name(assigns[:interface_id]) + + assigns[:storage_id] && assigns[:sensor_type] == "storage_volume" -> + get_storage_name(assigns[:storage_id]) + + true -> + nil end end @@ -368,6 +375,11 @@ defmodule ToweropsWeb.GraphLive.Show do {load_interface_traffic_chart_data(interface_id, range), get_interface_name(interface_id)} end + defp load_chart_data_for_type(%{sensor_type: "interface_errors", interface_id: interface_id}, range) + when not is_nil(interface_id) do + {load_interface_errors_chart_data(interface_id, range), get_interface_name(interface_id)} + end + defp load_chart_data_for_type(%{sensor_type: "traffic", device_id: device_id}, range) do {load_traffic_chart_data(device_id, range), nil} end @@ -403,6 +415,7 @@ defmodule ToweropsWeb.GraphLive.Show do defp get_chart_config("temperature"), do: {"Temperature", "°C", true} defp get_chart_config("voltage"), do: {"Voltage", "V", true} defp get_chart_config("traffic"), do: {"Overall Traffic", "bps", true} + defp get_chart_config("interface_errors"), do: {"Interface Errors & Discards", "", true} defp get_chart_config("count"), do: {"Count", "", true} defp get_chart_config("pppoe_sessions"), do: {"PPPoE Sessions", "", true} defp get_chart_config("connections"), do: {"Connections", "", true} @@ -557,6 +570,58 @@ defmodule ToweropsWeb.GraphLive.Show do end end + defp load_interface_errors_chart_data(interface_id, range) do + case Snmp.get_interface(interface_id) do + nil -> nil + interface -> build_interface_errors_chart_json(interface, range) + end + end + + defp build_interface_errors_chart_json(interface, range) do + since = get_datetime_from_range(range) + limit = get_limit_for_range(range) + + stats = + interface.id + |> Snmp.get_interface_stats(since: since, limit: limit) + |> Enum.reverse() + + if Enum.empty?(stats) do + nil + else + datasets = + Enum.reject( + [ + %{label: "In Errors", data: build_error_rate_data(stats, :if_in_errors)}, + %{label: "Out Errors", data: build_error_rate_data(stats, :if_out_errors)}, + %{label: "In Discards", data: build_error_rate_data(stats, :if_in_discards)}, + %{label: "Out Discards", data: build_error_rate_data(stats, :if_out_discards)} + ], + fn ds -> Enum.empty?(ds.data) end + ) + + if Enum.empty?(datasets), do: nil, else: safe_encode_chart_data(%{datasets: datasets}) + end + end + + # Calculates rate-of-change per minute for a cumulative error/discard counter + defp build_error_rate_data(stats, field) do + stats + |> Enum.chunk_every(2, 1, :discard) + |> Enum.flat_map(fn [stat1, stat2] -> + v1 = Map.get(stat1, field) + v2 = Map.get(stat2, field) + + if is_nil(v1) or is_nil(v2) do + [] + else + time_diff = stat2.checked_at |> DateTime.diff(stat1.checked_at, :second) |> max(1) + rate = ((v2 - v1) / time_diff * 60) |> max(0) |> Float.round(2) + [%{x: DateTime.to_unix(stat2.checked_at, :millisecond), y: rate}] + end + end) + end + defp get_interface_name(interface_id) do case Snmp.get_interface(interface_id) do nil -> "Unknown Interface" @@ -642,7 +707,7 @@ defmodule ToweropsWeb.GraphLive.Show do # Add capacity datasets for backhaul devices datasets = if device.device.device_role == "backhaul" && !Enum.empty?(datasets) do - add_capacity_to_full_graph(datasets, interfaces, since, limit) + add_capacity_to_full_graph(datasets, interfaces, since, limit, device.sensors) else datasets end @@ -672,23 +737,94 @@ defmodule ToweropsWeb.GraphLive.Show do end) end - # Add capacity reference lines for backhaul devices on full-page graph - # Calculates capacity per timestamp based on which interfaces have data at that point - defp add_capacity_to_full_graph(datasets, interfaces, since, limit) do - # Build a map of interface_id -> capacity for quick lookup - # Includes all interfaces to match the total device traffic + # Add capacity reference lines for backhaul devices on full-page graph. + # Uses Rx/Tx Capacity sensors when available (e.g. AirFiber adaptive modulation), + # otherwise falls back to interface configured_capacity_bps or if_speed. + defp add_capacity_to_full_graph(datasets, interfaces, since, _limit, sensors) do + rx_sensor = Enum.find(sensors, &(&1.sensor_descr == "Rx Capacity")) + tx_sensor = Enum.find(sensors, &(&1.sensor_descr == "Tx Capacity")) + + capacity_data = resolve_capacity_data(datasets, interfaces, since, rx_sensor, tx_sensor) + + max_capacity = capacity_data |> Enum.map(fn {_, rx, _} -> rx end) |> Enum.max(fn -> 0 end) + + if max_capacity > 0 do + datasets ++ + [ + %{ + label: "Capacity (In)", + data: Enum.map(capacity_data, fn {ts, rx, _} -> %{x: ts, y: Float.round(rx * 1.0, 2)} end), + borderDash: [5, 5], + borderWidth: 2, + pointRadius: 0, + fill: false + }, + %{ + label: "Capacity (Out)", + data: Enum.map(capacity_data, fn {ts, _, tx} -> %{x: ts, y: -Float.round(tx * 1.0, 2)} end), + borderDash: [5, 5], + borderWidth: 2, + pointRadius: 0, + fill: false + } + ] + else + datasets + end + end + + defp resolve_capacity_data(datasets, interfaces, since, rx_sensor, tx_sensor) + when rx_sensor != nil or tx_sensor != nil do + rx_readings = load_capacity_sensor_readings_for_graph(rx_sensor, since) + tx_readings = load_capacity_sensor_readings_for_graph(tx_sensor, since) + + if map_size(rx_readings) > 0 || map_size(tx_readings) > 0 do + all_ts = + (Map.keys(rx_readings) ++ Map.keys(tx_readings)) + |> MapSet.new() + |> Enum.sort() + + Enum.map(all_ts, fn ts -> + rx = Map.get(rx_readings, ts, capacity_sensor_fallback_bps(rx_sensor)) + tx = Map.get(tx_readings, ts, capacity_sensor_fallback_bps(tx_sensor)) + {ts, rx, tx} + end) + else + build_interface_speed_capacity(datasets, interfaces, since) + end + end + + defp resolve_capacity_data(datasets, interfaces, since, _rx_sensor, _tx_sensor) do + build_interface_speed_capacity(datasets, interfaces, since) + end + + defp load_capacity_sensor_readings_for_graph(nil, _since), do: %{} + + defp load_capacity_sensor_readings_for_graph(sensor, since) do + sensor.id + |> Snmp.get_sensor_readings(since: since, limit: 1000) + |> Map.new(fn r -> + bps = if r.value, do: round(r.value * 1_000_000), else: 0 + {DateTime.to_unix(r.checked_at, :millisecond), bps} + end) + end + + defp capacity_sensor_fallback_bps(nil), do: 0 + defp capacity_sensor_fallback_bps(sensor), do: if(sensor.last_value, do: round(sensor.last_value * 1_000_000), else: 0) + + # Fallback: derive capacity from interface configured_capacity_bps or if_speed + defp build_interface_speed_capacity(datasets, interfaces, since) do interface_capacity_map = Map.new(interfaces, fn interface -> capacity = interface.configured_capacity_bps || interface.if_speed || 0 {interface.id, capacity} end) - # Track which interfaces have data at each timestamp interface_activity_by_timestamp = interfaces |> Enum.flat_map(fn interface -> interface.id - |> Snmp.get_interface_stats(since: since, limit: limit) + |> Snmp.get_interface_stats(since: since, limit: 1000) |> Enum.map(fn stat -> ts_ms = DateTime.to_unix(stat.checked_at, :millisecond) {ts_ms, interface.id} @@ -696,7 +832,6 @@ defmodule ToweropsWeb.GraphLive.Show do end) |> Enum.group_by(fn {ts, _} -> ts end, fn {_, iface_id} -> iface_id end) - # Get all unique timestamps from the datasets all_timestamps = datasets |> Enum.flat_map(fn dataset -> @@ -708,63 +843,20 @@ defmodule ToweropsWeb.GraphLive.Show do |> Enum.uniq() |> Enum.sort() - # Calculate capacity at each timestamp based on active interfaces - capacity_data = - Enum.map(all_timestamps, fn ts -> - # Get interfaces that had data at this timestamp (within 1 minute tolerance) - active_interfaces = - interface_activity_by_timestamp - |> Enum.filter(fn {stat_ts, _} -> abs(stat_ts - ts) < 60_000 end) - |> Enum.flat_map(fn {_, iface_ids} -> iface_ids end) - |> Enum.uniq() + Enum.map(all_timestamps, fn ts -> + active_interfaces = + interface_activity_by_timestamp + |> Enum.filter(fn {stat_ts, _} -> abs(stat_ts - ts) < 60_000 end) + |> Enum.flat_map(fn {_, iface_ids} -> iface_ids end) + |> Enum.uniq() - # Sum capacity of active interfaces - total_capacity = - Enum.reduce(active_interfaces, 0, fn iface_id, acc -> - capacity = Map.get(interface_capacity_map, iface_id, 0) - acc + capacity - end) - - {ts, total_capacity} - end) - - # Only add capacity datasets if we have at least some capacity data - max_capacity = capacity_data |> Enum.map(fn {_, cap} -> cap end) |> Enum.max(fn -> 0 end) - - if max_capacity > 0 do - capacity_in_data = - Enum.map(capacity_data, fn {ts, cap} -> - %{x: ts, y: Float.round(cap * 1.0, 2)} + total_capacity = + Enum.reduce(active_interfaces, 0, fn iface_id, acc -> + acc + Map.get(interface_capacity_map, iface_id, 0) end) - capacity_out_data = - Enum.map(capacity_data, fn {ts, cap} -> - %{x: ts, y: -Float.round(cap * 1.0, 2)} - end) - - # Add capacity datasets with dashed styling - datasets ++ - [ - %{ - label: "Capacity (In)", - data: capacity_in_data, - borderDash: [5, 5], - borderWidth: 2, - pointRadius: 0, - fill: false - }, - %{ - label: "Capacity (Out)", - data: capacity_out_data, - borderDash: [5, 5], - borderWidth: 2, - pointRadius: 0, - fill: false - } - ] - else - datasets - end + {ts, total_capacity, total_capacity} + end) end defp build_interface_traffic_chart_json(interface, range) do diff --git a/test/towerops_web/channels/agent_channel_test.exs b/test/towerops_web/channels/agent_channel_test.exs index f608f125..9cd35224 100644 --- a/test/towerops_web/channels/agent_channel_test.exs +++ b/test/towerops_web/channels/agent_channel_test.exs @@ -991,6 +991,47 @@ defmodule ToweropsWeb.AgentChannelTest do assert stat.if_out_octets == 2_000_000 end + test "poll result falls back to 32-bit octets when HC counters are absent", %{ + socket: socket, + device: device, + interface: interface + } do + idx = interface.if_index + + # Device doesn't support HC counters — only standard 32-bit ifInOctets/ifOutOctets + result = %SnmpResult{ + device_id: device.id, + job_type: :POLL, + job_id: "poll:#{device.id}", + timestamp: DateTime.to_unix(DateTime.utc_now()), + oid_values: %{ + # HC counters absent (empty string = noSuchObject from device) + "1.3.6.1.2.1.31.1.1.1.6.#{idx}" => "", + "1.3.6.1.2.1.31.1.1.1.10.#{idx}" => "", + # Standard 32-bit counters present + "1.3.6.1.2.1.2.2.1.10.#{idx}" => "5000000", + "1.3.6.1.2.1.2.2.1.16.#{idx}" => "3000000", + "1.3.6.1.2.1.2.2.1.14.#{idx}" => "2", + "1.3.6.1.2.1.2.2.1.20.#{idx}" => "1", + "1.3.6.1.2.1.2.2.1.13.#{idx}" => "0", + "1.3.6.1.2.1.2.2.1.19.#{idx}" => "0" + } + } + + push(socket, "result", encode_payload(result)) + + stats = + poll_until(fn -> + result = Towerops.Snmp.get_interface_stats(interface.id) + if result != [], do: result + end) + + assert stats != [] + stat = hd(stats) + assert stat.if_in_octets == 5_000_000 + assert stat.if_out_octets == 3_000_000 + end + test "poll result with leading dot OIDs is normalized", %{ socket: socket, device: device, @@ -1333,7 +1374,7 @@ defmodule ToweropsWeb.AgentChannelTest do assert poll_job.job_id == "poll:#{device.id}" end - test "poll job uses 64-bit HC counters for interface octets", %{ + test "poll job includes both HC 64-bit and 32-bit fallback octets for interface stats", %{ socket: socket, device: device } do @@ -1390,19 +1431,19 @@ defmodule ToweropsWeb.AgentChannelTest do # Collect all OIDs from all queries in the poll job all_oids = Enum.flat_map(poll_job.queries, & &1.oids) - # Must use 64-bit HC counters (ifHCInOctets / ifHCOutOctets) + # Must include 64-bit HC counters (preferred, for devices that support ifXTable) assert Enum.any?(all_oids, &String.starts_with?(&1, "1.3.6.1.2.1.31.1.1.1.6.")), "Expected ifHCInOctets OIDs (1.3.6.1.2.1.31.1.1.1.6.x) but found: #{inspect(all_oids)}" assert Enum.any?(all_oids, &String.starts_with?(&1, "1.3.6.1.2.1.31.1.1.1.10.")), "Expected ifHCOutOctets OIDs (1.3.6.1.2.1.31.1.1.1.10.x) but found: #{inspect(all_oids)}" - # Must NOT use old 32-bit counters (ifInOctets / ifOutOctets) - refute Enum.any?(all_oids, &String.starts_with?(&1, "1.3.6.1.2.1.2.2.1.10.")), - "Found old 32-bit ifInOctets OIDs (1.3.6.1.2.1.2.2.1.10.x) - should use HC counters" + # Must also include 32-bit fallback counters (for devices that don't support HC counters) + assert Enum.any?(all_oids, &String.starts_with?(&1, "1.3.6.1.2.1.2.2.1.10.")), + "Expected ifInOctets fallback OIDs (1.3.6.1.2.1.2.2.1.10.x) but found: #{inspect(all_oids)}" - refute Enum.any?(all_oids, &String.starts_with?(&1, "1.3.6.1.2.1.2.2.1.16.")), - "Found old 32-bit ifOutOctets OIDs (1.3.6.1.2.1.2.2.1.16.x) - should use HC counters" + assert Enum.any?(all_oids, &String.starts_with?(&1, "1.3.6.1.2.1.2.2.1.16.")), + "Expected ifOutOctets fallback OIDs (1.3.6.1.2.1.2.2.1.16.x) but found: #{inspect(all_oids)}" end end diff --git a/test/towerops_web/live/graph_live/show_test.exs b/test/towerops_web/live/graph_live/show_test.exs index 353e8c39..83267002 100644 --- a/test/towerops_web/live/graph_live/show_test.exs +++ b/test/towerops_web/live/graph_live/show_test.exs @@ -625,6 +625,57 @@ defmodule ToweropsWeb.GraphLive.ShowTest do end end + describe "Interface errors graph" do + setup %{snmp_device: snmp_device} do + interface = + %Interface{} + |> Interface.changeset(%{ + snmp_device_id: snmp_device.id, + if_index: 1, + if_descr: "eth0", + if_name: "eth0", + if_oper_status: "up", + if_speed: 1_000_000_000 + }) + |> Repo.insert!() + + %{interface: interface} + end + + test "renders interface errors graph page", %{ + conn: conn, + device: device, + interface: interface + } do + Snmp.create_interface_stat(%{ + interface_id: interface.id, + if_in_errors: 5, + if_out_errors: 2, + if_in_discards: 1, + if_out_discards: 0, + checked_at: DateTime.utc_now() + }) + + {:ok, _view, html} = + live(conn, ~p"/devices/#{device.id}/graph/interface_errors?interface_id=#{interface.id}") + + assert html =~ "Interface Errors & Discards" + assert html =~ device.name + assert html =~ interface.if_name + end + + test "renders interface errors graph without data", %{ + conn: conn, + device: device, + interface: interface + } do + {:ok, _view, html} = + live(conn, ~p"/devices/#{device.id}/graph/interface_errors?interface_id=#{interface.id}") + + assert html =~ "Interface Errors & Discards" + end + end + describe "access control" do test "redirects when device not found", %{conn: conn} do fake_id = Ecto.UUID.generate()