From 47fcbeb483818014c15c74ecf5267a8a9f26d952 Mon Sep 17 00:00:00 2001 From: Graham McIntire Date: Wed, 11 Feb 2026 15:58:31 -0600 Subject: [PATCH] fix: agent discovery and polling accuracy improvements - use 64-bit HC counters (ifHCInOctets/ifHCOutOctets) instead of 32-bit counters that wrap every ~34s on gigabit links - fix negative integer parsing in replay adapter so sensor scale values like -3 produce correct divisors (1000) instead of falling through to 1 - replace walking entire Cisco enterprise tree with targeted subtrees to avoid timeouts; add Juniper, HP/H3C, Fortinet, Cambium vendor OIDs - add missing discovery OIDs: ENTITY-STATE-MIB, HOST-RESOURCES-MIB tables, and UCD-SNMP-MIB Linux CPU stats fallback --- CHANGELOG.txt | 18 +++ lib/towerops/snmp/adapters/replay.ex | 4 +- lib/towerops_web/channels/agent_channel.ex | 63 ++++++++-- test/towerops/snmp/adapters/replay_test.exs | 14 +++ .../channels/agent_channel_test.exs | 114 ++++++++++++++++++ 5 files changed, 201 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.txt b/CHANGELOG.txt index 07dfed45..b743d0ac 100644 --- a/CHANGELOG.txt +++ b/CHANGELOG.txt @@ -1,6 +1,24 @@ CHANGELOG - towerops-web ======================== +2026-02-11 - fix: agent discovery and polling improvements + - File: lib/towerops_web/channels/agent_channel.ex (build_polling_queries/1, process_interface_stats/3) + Changed interface traffic OIDs from 32-bit counters (ifInOctets/ifOutOctets) to 64-bit + HC counters (ifHCInOctets/ifHCOutOctets). 32-bit counters wrap at ~4.3GB which causes + wildly inaccurate readings on gigabit+ links. + - File: lib/towerops/snmp/adapters/replay.ex (parse_value/1) + Fixed negative integer parsing regex from ^\d+$ to ^-?\d+$ so values like + entPhySensorScale=-3 are parsed as integers. Previously fell through to string, + causing sensor divisor calculation to silently return 1 instead of 1000. + - File: lib/towerops_web/channels/agent_channel.ex (build_discovery_queries/0) + Replaced walking entire Cisco enterprise tree (1.3.6.1.4.1.9) with targeted subtrees + for CPU, ENVMON, CDP, and WAP to avoid 50k+ OID responses and timeouts. Added Juniper, + HP/H3C, Fortinet, and Cambium vendor OIDs for broader device support. + - File: lib/towerops_web/channels/agent_channel.ex (build_discovery_queries/0) + Added missing discovery OIDs: ENTITY-STATE-MIB for state sensors, HOST-RESOURCES-MIB + tables for processors/storage/devices, and UCD-SNMP-MIB for Linux CPU stats fallback. + - Tests added in replay_test.exs and agent_channel_test.exs + 2026-02-11 - fix: handle RouterOS version returned as list of integers - File: lib/towerops/snmp/profiles/vendors/routeros.ex (detect_version/1) Added pattern match to handle case where SNMP version OID returns list diff --git a/lib/towerops/snmp/adapters/replay.ex b/lib/towerops/snmp/adapters/replay.ex index e4c714fd..7a37af25 100644 --- a/lib/towerops/snmp/adapters/replay.ex +++ b/lib/towerops/snmp/adapters/replay.ex @@ -161,8 +161,8 @@ defmodule Towerops.Snmp.Adapters.Replay do String.to_float(value) # Pure integer (no dots) - check FIFTH - # Example: "42" => 42 - Regex.match?(~r/^\d+$/, value) -> + # Example: "42" => 42, "-3" => -3 + Regex.match?(~r/^-?\d+$/, value) -> String.to_integer(value) # OID-like string (multiple dots) - check SIXTH diff --git a/lib/towerops_web/channels/agent_channel.ex b/lib/towerops_web/channels/agent_channel.ex index 28473678..e99b7ca4 100644 --- a/lib/towerops_web/channels/agent_channel.ex +++ b/lib/towerops_web/channels/agent_channel.ex @@ -924,16 +924,59 @@ defmodule ToweropsWeb.AgentChannel do "1.3.6.1.2.1.47.1.1.1.1.5" ] }, - # Vendor-specific MIBs (WALK) + # State sensors - ENTITY-STATE-MIB (WALK) + %SnmpQuery{ + query_type: :WALK, + oids: ["1.3.6.1.2.1.131.1.1.1.1"] + }, + # HOST-RESOURCES-MIB (WALK) - processors, storage, devices %SnmpQuery{ query_type: :WALK, oids: [ - # Cisco - "1.3.6.1.4.1.9", + # hrProcessorTable + "1.3.6.1.2.1.25.3.3", + # hrStorageTable + "1.3.6.1.2.1.25.2.3", + # hrDeviceTable (for processor descriptions) + "1.3.6.1.2.1.25.3.2" + ] + }, + # UCD-SNMP-MIB (GET) - Linux CPU stats fallback + %SnmpQuery{ + query_type: :GET, + oids: [ + # ssCpuUser + "1.3.6.1.4.1.2021.11.9.0", + # ssCpuSystem + "1.3.6.1.4.1.2021.11.10.0", + # ssCpuIdle + "1.3.6.1.4.1.2021.11.11.0" + ] + }, + # Vendor-specific MIBs (WALK) - targeted subtrees only + %SnmpQuery{ + query_type: :WALK, + oids: [ + # Cisco PROCESS-MIB (CPU) + "1.3.6.1.4.1.9.9.109", + # Cisco ENVMON-MIB (temp/voltage/fan/PSU) + "1.3.6.1.4.1.9.9.13", + # Cisco CDP + "1.3.6.1.4.1.9.9.23", + # Cisco WAP + "1.3.6.1.4.1.9.9.618", # MikroTik "1.3.6.1.4.1.14988", # Ubiquiti - "1.3.6.1.4.1.41112" + "1.3.6.1.4.1.41112", + # Juniper + "1.3.6.1.4.1.2636", + # HP/H3C/Comware + "1.3.6.1.4.1.25506", + # Fortinet + "1.3.6.1.4.1.12356", + # Cambium + "1.3.6.1.4.1.17713" ] }, # Neighbor discovery (WALK) @@ -967,10 +1010,10 @@ defmodule ToweropsWeb.AgentChannel do idx = iface.if_index [ - # ifInOctets - "1.3.6.1.2.1.2.2.1.10.#{idx}", - # ifOutOctets - "1.3.6.1.2.1.2.2.1.16.#{idx}", + # ifHCInOctets (64-bit counter) + "1.3.6.1.2.1.31.1.1.1.6.#{idx}", + # ifHCOutOctets (64-bit counter) + "1.3.6.1.2.1.31.1.1.1.10.#{idx}", # ifInErrors "1.3.6.1.2.1.2.2.1.14.#{idx}", # ifOutErrors @@ -1566,8 +1609,8 @@ defmodule ToweropsWeb.AgentChannel do Snmp.create_interface_stat(%{ interface_id: iface.id, - if_in_octets: parse_integer(Map.get(oid_values, "1.3.6.1.2.1.2.2.1.10.#{idx}")), - if_out_octets: parse_integer(Map.get(oid_values, "1.3.6.1.2.1.2.2.1.16.#{idx}")), + 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_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/test/towerops/snmp/adapters/replay_test.exs b/test/towerops/snmp/adapters/replay_test.exs index a35cd975..17806b58 100644 --- a/test/towerops/snmp/adapters/replay_test.exs +++ b/test/towerops/snmp/adapters/replay_test.exs @@ -324,5 +324,19 @@ defmodule Towerops.Snmp.Adapters.ReplayTest do assert {:ok, 4_294_967_295} = Replay.get(opts, "oid") end + + test "parses negative integer" do + oid_map = %{"oid" => "-3"} + opts = Replay.new(oid_map) + + assert {:ok, -3} = Replay.get(opts, "oid") + end + + test "parses negative large integer" do + oid_map = %{"oid" => "-1000"} + opts = Replay.new(oid_map) + + assert {:ok, -1000} = Replay.get(opts, "oid") + end end end diff --git a/test/towerops_web/channels/agent_channel_test.exs b/test/towerops_web/channels/agent_channel_test.exs index cc6f8504..cc4f4e66 100644 --- a/test/towerops_web/channels/agent_channel_test.exs +++ b/test/towerops_web/channels/agent_channel_test.exs @@ -98,5 +98,119 @@ defmodule ToweropsWeb.AgentChannelTest do assert poll_job != nil, "Expected a POLL job for device #{device.id}" assert poll_job.job_id == "poll:#{device.id}" end + + test "poll job uses 64-bit HC counters for interface octets", %{ + socket: socket, + device: device + } do + # Build discovery result with interface data so the poll job + # will include interface OIDs + discovery_result = %SnmpResult{ + device_id: device.id, + job_type: :DISCOVER, + job_id: "discover:#{device.id}", + timestamp: DateTime.to_unix(DateTime.utc_now()), + oid_values: %{ + # System info + "1.3.6.1.2.1.1.1.0" => "Test Device Description", + "1.3.6.1.2.1.1.2.0" => "1.3.6.1.4.1.9.1.1", + "1.3.6.1.2.1.1.3.0" => "123456", + "1.3.6.1.2.1.1.4.0" => "admin@test.com", + "1.3.6.1.2.1.1.5.0" => "test-device", + "1.3.6.1.2.1.1.6.0" => "Test Location", + # ifTable - interface index + "1.3.6.1.2.1.2.2.1.1.1" => "1", + # ifDescr + "1.3.6.1.2.1.2.2.1.2.1" => "GigabitEthernet0/1", + # ifType (ethernetCsmacd = 6) + "1.3.6.1.2.1.2.2.1.3.1" => "6", + # ifSpeed + "1.3.6.1.2.1.2.2.1.5.1" => "1000000000", + # ifPhysAddress + "1.3.6.1.2.1.2.2.1.6.1" => "aa:bb:cc:dd:ee:ff", + # ifAdminStatus (up = 1) + "1.3.6.1.2.1.2.2.1.7.1" => "1", + # ifOperStatus (up = 1) + "1.3.6.1.2.1.2.2.1.8.1" => "1" + } + } + + binary = SnmpResult.encode(discovery_result) + payload = %{"binary" => Base.encode64(binary)} + + push(socket, "result", payload) + + # Receive the poll job pushed after discovery + assert_push "jobs", %{binary: jobs_binary}, 5_000 + + {:ok, decoded_binary} = Base.decode64(jobs_binary) + job_list = AgentJobList.decode(decoded_binary) + + poll_job = + Enum.find(job_list.jobs, fn job -> + job.job_type == :POLL and job.device_id == device.id + end) + + assert poll_job != nil, "Expected a POLL job for device #{device.id}" + + # 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) + 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" + + 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" + end + end + + describe "discovery job OIDs" do + test "initial discovery job contains expected OIDs", %{socket: socket} do + # Trigger a fresh job push by sending :send_jobs + send(socket.channel_pid, :send_jobs) + + assert_push "jobs", %{binary: jobs_binary}, 5_000 + + {:ok, decoded_binary} = Base.decode64(jobs_binary) + job_list = AgentJobList.decode(decoded_binary) + + discover_job = + Enum.find(job_list.jobs, fn job -> + job.job_type == :DISCOVER + end) + + assert discover_job != nil, "Expected a DISCOVER job in initial jobs" + + # Collect all OIDs from all queries in the discovery job + all_oids = Enum.flat_map(discover_job.queries, & &1.oids) + + # ENTITY-STATE-MIB + assert "1.3.6.1.2.1.131.1.1.1.1" in all_oids, + "Missing ENTITY-STATE-MIB OID (1.3.6.1.2.1.131.1.1.1.1)" + + # HOST-RESOURCES-MIB: hrProcessorTable + assert "1.3.6.1.2.1.25.3.3" in all_oids, + "Missing hrProcessorTable OID (1.3.6.1.2.1.25.3.3)" + + # HOST-RESOURCES-MIB: hrStorageTable + assert "1.3.6.1.2.1.25.2.3" in all_oids, + "Missing hrStorageTable OID (1.3.6.1.2.1.25.2.3)" + + # HOST-RESOURCES-MIB: hrDeviceTable + assert "1.3.6.1.2.1.25.3.2" in all_oids, + "Missing hrDeviceTable OID (1.3.6.1.2.1.25.3.2)" + + # UCD-SNMP-MIB: ssCpuUser + assert "1.3.6.1.4.1.2021.11.9.0" in all_oids, + "Missing ssCpuUser OID (1.3.6.1.4.1.2021.11.9.0)" + end end end