refactor: extract timeout helpers and replace conditionals in snmp/discovery.ex
- 12 identical *_with_timeout wrappers → 2 generic helpers (with_timeout/3, with_deferred_timeout/3) - Removed commented-out code (# alias Towerops.Topology, # Auto-inference disabled) - All if/case/cond replaced with pattern-matched function clauses (20+ refactors) - Long lines fixed for credo compliance - 11/11 tests pass, 0 credo issues
This commit is contained in:
Graham McInitre
parent
9d4a5f6d81
commit
5b91735b14
1 changed files with 369 additions and 451 deletions
|
|
@ -37,7 +37,6 @@ defmodule Towerops.Snmp.Discovery do
|
|||
alias Towerops.Snmp.Storage
|
||||
alias Towerops.Snmp.Transceiver
|
||||
alias Towerops.Snmp.Vlan
|
||||
# alias Towerops.Topology # Commented out - auto-inference disabled
|
||||
alias Towerops.Workers.DiscoveryWorker
|
||||
|
||||
require Logger
|
||||
|
|
@ -100,8 +99,11 @@ defmodule Towerops.Snmp.Discovery do
|
|||
|
||||
"""
|
||||
@spec discover_device(DeviceSchema.t()) :: {:ok, Device.t()} | {:error, term()}
|
||||
def discover_device(%DeviceSchema{snmp_enabled: false}) do
|
||||
{:error, :snmp_not_enabled}
|
||||
end
|
||||
|
||||
def discover_device(%DeviceSchema{} = device) do
|
||||
if device.snmp_enabled do
|
||||
Logger.info(
|
||||
"Starting SNMP discovery: #{device.name} (#{device.ip_address})",
|
||||
device_id: device.id,
|
||||
|
|
@ -111,15 +113,19 @@ defmodule Towerops.Snmp.Discovery do
|
|||
|
||||
client_opts = build_client_opts(device)
|
||||
|
||||
# Categorize device speed for adaptive timeouts
|
||||
Logger.info("Categorizing device speed...", device_id: device.id)
|
||||
device_speed = DeferredDiscovery.categorize_device_speed(client_opts)
|
||||
timeouts = DeferredDiscovery.timeouts_for_speed(device_speed)
|
||||
|
||||
if device_speed == :unresponsive do
|
||||
discover_with_speed(device, client_opts, device_speed, timeouts)
|
||||
end
|
||||
|
||||
defp discover_with_speed(device, _client_opts, :unresponsive, _timeouts) do
|
||||
Logger.warning("Device #{device.name} is unresponsive, aborting discovery")
|
||||
{:error, :device_unresponsive}
|
||||
else
|
||||
end
|
||||
|
||||
defp discover_with_speed(device, client_opts, device_speed, timeouts) do
|
||||
Logger.info(
|
||||
"Device categorized as #{device_speed}, proceeding with discovery",
|
||||
device_id: device.id,
|
||||
|
|
@ -128,10 +134,6 @@ defmodule Towerops.Snmp.Discovery do
|
|||
|
||||
do_discover_device(device, client_opts, timeouts)
|
||||
end
|
||||
else
|
||||
{:error, :snmp_not_enabled}
|
||||
end
|
||||
end
|
||||
|
||||
@doc """
|
||||
Discovers a device using pre-built client options.
|
||||
|
|
@ -150,50 +152,49 @@ defmodule Towerops.Snmp.Discovery do
|
|||
"""
|
||||
@spec discover_device_with_opts(DeviceSchema.t(), keyword()) ::
|
||||
{:ok, Device.t()} | {:error, term()}
|
||||
def discover_device_with_opts(%DeviceSchema{snmp_enabled: false}, _client_opts) do
|
||||
{:error, :snmp_not_enabled}
|
||||
end
|
||||
|
||||
def discover_device_with_opts(%DeviceSchema{} = device, client_opts) do
|
||||
if device.snmp_enabled do
|
||||
Logger.debug("Starting discovery with custom opts for: #{device.name}")
|
||||
{device_speed, timeouts} = categorize_speed(client_opts)
|
||||
discover_with_speed(device, client_opts, device_speed, timeouts)
|
||||
end
|
||||
|
||||
# Skip speed categorization for Replay adapter (agent-based discovery)
|
||||
# Agent data is pre-collected, so network checks are not needed
|
||||
adapter = Keyword.get(client_opts, :adapter)
|
||||
defp categorize_speed(client_opts) do
|
||||
categorize_speed_for_adapter(client_opts, Keyword.get(client_opts, :adapter))
|
||||
end
|
||||
|
||||
{device_speed, timeouts} =
|
||||
if adapter == Replay do
|
||||
defp categorize_speed_for_adapter(_client_opts, Replay) do
|
||||
Logger.debug("Using Replay adapter, skipping speed categorization")
|
||||
{:fast, DeferredDiscovery.timeouts_for_speed(:fast)}
|
||||
else
|
||||
# Categorize device speed for adaptive timeouts (real SNMP)
|
||||
end
|
||||
|
||||
defp categorize_speed_for_adapter(client_opts, _adapter) do
|
||||
speed = DeferredDiscovery.categorize_device_speed(client_opts)
|
||||
{speed, DeferredDiscovery.timeouts_for_speed(speed)}
|
||||
end
|
||||
|
||||
if device_speed == :unresponsive do
|
||||
Logger.warning("Device #{device.name} is unresponsive, aborting discovery")
|
||||
{:error, :device_unresponsive}
|
||||
else
|
||||
do_discover_device(device, client_opts, timeouts)
|
||||
# Connection test with pattern matching on Replay adapter
|
||||
defp test_connection(client_opts, device_id) do
|
||||
test_connection_for_adapter(client_opts, device_id, Keyword.get(client_opts, :adapter))
|
||||
end
|
||||
else
|
||||
{:error, :snmp_not_enabled}
|
||||
|
||||
defp test_connection_for_adapter(_client_opts, device_id, Replay) do
|
||||
Logger.debug("Using Replay adapter, skipping connection test", device_id: device_id)
|
||||
{:ok, :replay}
|
||||
end
|
||||
|
||||
defp test_connection_for_adapter(client_opts, device_id, _adapter) do
|
||||
Logger.info("Testing SNMP connection...", device_id: device_id)
|
||||
Client.test_connection(client_opts)
|
||||
end
|
||||
|
||||
# Internal discovery with adaptive timeouts based on device speed
|
||||
defp do_discover_device(device, client_opts, timeouts) do
|
||||
adapter = Keyword.get(client_opts, :adapter)
|
||||
connection_result = test_connection(client_opts, device.id)
|
||||
|
||||
# Skip connection test for Replay adapter (agent-provided data)
|
||||
connection_result =
|
||||
if adapter == Replay do
|
||||
Logger.debug("Using Replay adapter, skipping connection test", device_id: device.id)
|
||||
{:ok, :replay}
|
||||
else
|
||||
Logger.info("Testing SNMP connection...", device_id: device.id)
|
||||
Client.test_connection(client_opts)
|
||||
end
|
||||
|
||||
# Critical path: connection, system info, device info must succeed
|
||||
with {:ok, _} <- connection_result,
|
||||
Logger.info("Discovering system info...", device_id: device.id),
|
||||
{:ok, system_info} <- discover_system(client_opts),
|
||||
|
|
@ -205,60 +206,76 @@ defmodule Towerops.Snmp.Discovery do
|
|||
{:ok, device_info} <- build_device_info(client_opts, system_info, profile),
|
||||
{:ok, interfaces} <- discover_interfaces_with_timeout_safe(client_opts, profile, timeouts, device.id),
|
||||
{:ok, sensors} <- discover_sensors_with_timeout_safe(client_opts, profile, timeouts, device.id) do
|
||||
# Secondary discovery: VLANs, IPs, processors, storage (already fall back to [] on timeout)
|
||||
Logger.info("Discovering VLANs...", device_id: device.id)
|
||||
{:ok, vlans} = discover_vlans_with_timeout(client_opts, profile, timeouts)
|
||||
{:ok, vlans} = with_timeout(client_opts, fn -> discover_vlans(client_opts, profile) end, timeouts)
|
||||
|
||||
Logger.info("Discovering IP addresses...", device_id: device.id)
|
||||
{:ok, ip_addresses} = discover_ip_addresses_with_timeout(client_opts, timeouts)
|
||||
{:ok, ip_addresses} = with_timeout(client_opts, fn -> Base.discover_all_ip_addresses(client_opts) end, timeouts)
|
||||
_ = log_ip_discovery_results(device, ip_addresses)
|
||||
|
||||
Logger.info("Discovering processors...", device_id: device.id)
|
||||
{:ok, processors} = discover_processors_with_timeout(client_opts, timeouts)
|
||||
{:ok, processors} = with_timeout(client_opts, fn -> Base.discover_processors(client_opts) end, timeouts)
|
||||
|
||||
Logger.info("Discovering storage...", device_id: device.id)
|
||||
{:ok, storage} = discover_storage_with_timeout(client_opts, timeouts)
|
||||
{:ok, storage} = with_timeout(client_opts, fn -> Base.discover_storage(client_opts) end, timeouts)
|
||||
|
||||
Logger.info("Discovering memory pools...", device_id: device.id)
|
||||
{:ok, mempools} = discover_mempools_with_timeout(client_opts, timeouts)
|
||||
{:ok, mempools} = with_timeout(client_opts, fn -> Base.discover_memory_pools(client_opts) end, timeouts)
|
||||
|
||||
Logger.info("Discovering entity physical inventory...", device_id: device.id)
|
||||
{:ok, entity_physical} = discover_entity_physical_with_timeout(client_opts, timeouts)
|
||||
{:ok, entity_physical} = with_timeout(client_opts, fn -> Base.discover_entity_physical(client_opts) end, timeouts)
|
||||
|
||||
Logger.info("Discovering transceivers...", device_id: device.id)
|
||||
{:ok, transceivers} = discover_transceivers_with_timeout(client_opts, timeouts)
|
||||
{:ok, transceivers} = with_timeout(client_opts, fn -> Base.discover_transceivers(client_opts) end, timeouts)
|
||||
|
||||
Logger.info("Discovering printer supplies...", device_id: device.id)
|
||||
{:ok, printer_supplies} = discover_printer_supplies_with_timeout(client_opts, timeouts)
|
||||
{:ok, printer_supplies} =
|
||||
with_timeout(client_opts, fn -> Base.discover_printer_supplies(client_opts) end, timeouts)
|
||||
|
||||
Logger.info("Saving discovery results (including IP addresses and processors)...", device_id: device.id)
|
||||
Logger.info("Saving discovery results...", device_id: device.id)
|
||||
|
||||
case save_discovery_results(device, device_info, interfaces, sensors, vlans, ip_addresses, processors) do
|
||||
{:ok, discovered_device} ->
|
||||
# Post-save discovery: neighbors, ARP (best-effort)
|
||||
Logger.info("Syncing storage...", device_id: device.id)
|
||||
_ = sync_storage(discovered_device, storage)
|
||||
save_result = save_discovery_results(device, device_info, interfaces, sensors, vlans, ip_addresses, processors)
|
||||
|
||||
Logger.info("Syncing memory pools...", device_id: device.id)
|
||||
_ = sync_mempools(discovered_device, mempools)
|
||||
post_data = %{
|
||||
storage: storage,
|
||||
mempools: mempools,
|
||||
entity_physical: entity_physical,
|
||||
transceivers: transceivers,
|
||||
printer_supplies: printer_supplies
|
||||
}
|
||||
|
||||
Logger.info("Syncing entity physical inventory...", device_id: device.id)
|
||||
_ = sync_entity_physical(discovered_device, entity_physical)
|
||||
complete_discovery(device, client_opts, timeouts, save_result, post_data)
|
||||
else
|
||||
{:error, reason} = error ->
|
||||
Logger.error("SNMP discovery failed for #{device.name}: #{inspect(reason)}")
|
||||
error
|
||||
end
|
||||
end
|
||||
|
||||
Logger.info("Syncing transceivers...", device_id: device.id)
|
||||
_ = sync_transceivers(discovered_device, transceivers)
|
||||
defp complete_discovery(_device, _client_opts, _timeouts, {:error, _} = error, _post_data) do
|
||||
error
|
||||
end
|
||||
|
||||
Logger.info("Syncing printer supplies...", device_id: device.id)
|
||||
_ = sync_printer_supplies(discovered_device, printer_supplies)
|
||||
defp complete_discovery(device, client_opts, timeouts, {:ok, discovered_device}, post_data) do
|
||||
_ = sync_storage(discovered_device, post_data.storage)
|
||||
_ = sync_mempools(discovered_device, post_data.mempools)
|
||||
_ = sync_entity_physical(discovered_device, post_data.entity_physical)
|
||||
_ = sync_transceivers(discovered_device, post_data.transceivers)
|
||||
_ = sync_printer_supplies(discovered_device, post_data.printer_supplies)
|
||||
|
||||
Logger.info("Discovering neighbors...", device_id: device.id)
|
||||
{:ok, neighbors} = discover_neighbors_with_timeout(client_opts, discovered_device.interfaces, timeouts)
|
||||
|
||||
Logger.info("Saving neighbors...", device_id: device.id)
|
||||
neighbors_fn = fn -> NeighborDiscovery.discover_neighbors(client_opts, discovered_device.interfaces) end
|
||||
|
||||
{:ok, neighbors} = with_deferred_timeout(client_opts, neighbors_fn, timeouts)
|
||||
|
||||
_ = save_neighbors(discovered_device.device_id, neighbors)
|
||||
|
||||
Logger.info("Discovering ARP entries...", device_id: device.id)
|
||||
{:ok, arp_entries} = discover_arp_with_timeout(client_opts, timeouts)
|
||||
|
||||
arp_fn = fn -> ArpDiscovery.discover_arp_table(client_opts) end
|
||||
|
||||
{:ok, arp_entries} = with_deferred_timeout(client_opts, arp_fn, timeouts)
|
||||
_ = save_arp_entries(discovered_device.device_id, arp_entries, discovered_device.interfaces)
|
||||
|
||||
_ = update_device_discovery_time(device)
|
||||
|
|
@ -267,16 +284,10 @@ defmodule Towerops.Snmp.Discovery do
|
|||
is_rediscovery = device.last_discovery_at != nil
|
||||
_ = log_discovery_event(device, discovered_device, is_rediscovery)
|
||||
|
||||
# Create monitoring checks for discovered entities
|
||||
Logger.info("Creating monitoring checks from discovered entities...", device_id: device.id)
|
||||
|
||||
{:ok, check_counts} = Towerops.Snmp.create_checks_from_discovery(device, discovered_device)
|
||||
log_check_creation_results(device.id, check_counts)
|
||||
|
||||
# Auto-inference disabled - device type is now manual-only
|
||||
# Logger.info("Inferring device role...", device_id: device.id)
|
||||
# _ = Topology.maybe_update_device_role(device)
|
||||
|
||||
_ =
|
||||
Phoenix.PubSub.broadcast(
|
||||
Towerops.PubSub,
|
||||
|
|
@ -285,52 +296,30 @@ defmodule Towerops.Snmp.Discovery do
|
|||
)
|
||||
|
||||
{:ok, discovered_device}
|
||||
|
||||
{:error, reason} ->
|
||||
Logger.error("Failed to save discovery results for #{device.name}: #{inspect(reason)}")
|
||||
{:error, reason}
|
||||
end
|
||||
else
|
||||
{:error, reason} = error ->
|
||||
Logger.error("SNMP discovery failed for #{device.name}: #{inspect(reason)}")
|
||||
error
|
||||
end
|
||||
end
|
||||
|
||||
# Interface discovery with timeout - returns error on timeout to preserve existing data
|
||||
defp discover_interfaces_with_timeout(client_opts, profile, timeouts) do
|
||||
DeferredDiscovery.slow_check(
|
||||
client_opts,
|
||||
fn -> discover_interfaces(client_opts, profile) end,
|
||||
timeout: timeouts[:slow],
|
||||
default: {:error, :interface_discovery_timeout}
|
||||
)
|
||||
end
|
||||
# Generic timeout wrapper for slow discovery operations (falls back to [] on timeout)
|
||||
defp with_timeout(client_opts, discovery_fn, timeouts),
|
||||
do: DeferredDiscovery.slow_check(client_opts, discovery_fn, timeout: timeouts[:slow], default: [])
|
||||
|
||||
# Sensor discovery with timeout - returns error on timeout to preserve existing data
|
||||
defp discover_sensors_with_timeout(client_opts, profile, timeouts) do
|
||||
DeferredDiscovery.slow_check(
|
||||
client_opts,
|
||||
fn -> discover_sensors(client_opts, profile) end,
|
||||
timeout: timeouts[:slow],
|
||||
default: {:error, :sensor_discovery_timeout}
|
||||
)
|
||||
end
|
||||
# Timeout wrapper for deferred discovery operations (neighbors, ARP)
|
||||
defp with_deferred_timeout(client_opts, discovery_fn, timeouts),
|
||||
do: DeferredDiscovery.slow_check(client_opts, discovery_fn, timeout: timeouts[:deferred], default: [])
|
||||
|
||||
# Safe wrapper for interface discovery - aborts discovery on timeout to prevent data loss
|
||||
defp discover_interfaces_with_timeout_safe(client_opts, profile, timeouts, device_id) do
|
||||
Logger.info("Discovering interfaces...", device_id: device_id)
|
||||
|
||||
case discover_interfaces_with_timeout(client_opts, profile, timeouts) do
|
||||
{:ok, ifaces} ->
|
||||
{:ok, ifaces}
|
||||
case DeferredDiscovery.slow_check(
|
||||
client_opts,
|
||||
fn -> discover_interfaces(client_opts, profile) end,
|
||||
timeout: timeouts[:slow],
|
||||
default: {:error, :interface_discovery_timeout}
|
||||
) do
|
||||
{:ok, _ifaces} = ok -> ok
|
||||
|
||||
{:error, :interface_discovery_timeout} ->
|
||||
Logger.error(
|
||||
"Interface discovery timed out - aborting to prevent data loss",
|
||||
device_id: device_id
|
||||
)
|
||||
|
||||
Logger.error("Interface discovery timed out - aborting to prevent data loss", device_id: device_id)
|
||||
{:error, :interface_discovery_timeout}
|
||||
end
|
||||
end
|
||||
|
|
@ -339,116 +328,20 @@ defmodule Towerops.Snmp.Discovery do
|
|||
defp discover_sensors_with_timeout_safe(client_opts, profile, timeouts, device_id) do
|
||||
Logger.info("Discovering sensors...", device_id: device_id)
|
||||
|
||||
case discover_sensors_with_timeout(client_opts, profile, timeouts) do
|
||||
{:ok, sensors} ->
|
||||
{:ok, sensors}
|
||||
case DeferredDiscovery.slow_check(
|
||||
client_opts,
|
||||
fn -> discover_sensors(client_opts, profile) end,
|
||||
timeout: timeouts[:slow],
|
||||
default: {:error, :sensor_discovery_timeout}
|
||||
) do
|
||||
{:ok, _sensors} = ok -> ok
|
||||
|
||||
{:error, :sensor_discovery_timeout} ->
|
||||
Logger.error(
|
||||
"Sensor discovery timed out - aborting to prevent data loss",
|
||||
device_id: device_id
|
||||
)
|
||||
|
||||
Logger.error("Sensor discovery timed out - aborting to prevent data loss", device_id: device_id)
|
||||
{:error, :sensor_discovery_timeout}
|
||||
end
|
||||
end
|
||||
|
||||
# Neighbor discovery with timeout - falls back to empty list on timeout
|
||||
defp discover_neighbors_with_timeout(client_opts, interfaces, timeouts) do
|
||||
DeferredDiscovery.slow_check(
|
||||
client_opts,
|
||||
fn -> NeighborDiscovery.discover_neighbors(client_opts, interfaces) end,
|
||||
timeout: timeouts[:deferred],
|
||||
default: []
|
||||
)
|
||||
end
|
||||
|
||||
# ARP table discovery with timeout - falls back to empty list on timeout
|
||||
defp discover_arp_with_timeout(client_opts, timeouts) do
|
||||
DeferredDiscovery.slow_check(
|
||||
client_opts,
|
||||
fn -> ArpDiscovery.discover_arp_table(client_opts) end,
|
||||
timeout: timeouts[:deferred],
|
||||
default: []
|
||||
)
|
||||
end
|
||||
|
||||
# VLAN discovery with timeout - falls back to empty list on timeout
|
||||
defp discover_vlans_with_timeout(client_opts, profile, timeouts) do
|
||||
DeferredDiscovery.slow_check(
|
||||
client_opts,
|
||||
fn -> discover_vlans(client_opts, profile) end,
|
||||
timeout: timeouts[:slow],
|
||||
default: []
|
||||
)
|
||||
end
|
||||
|
||||
# IP address discovery with timeout - falls back to empty list on timeout
|
||||
defp discover_ip_addresses_with_timeout(client_opts, timeouts) do
|
||||
DeferredDiscovery.slow_check(
|
||||
client_opts,
|
||||
fn -> Base.discover_all_ip_addresses(client_opts) end,
|
||||
timeout: timeouts[:slow],
|
||||
default: []
|
||||
)
|
||||
end
|
||||
|
||||
# Processor discovery with timeout - falls back to empty list on timeout
|
||||
defp discover_processors_with_timeout(client_opts, timeouts) do
|
||||
DeferredDiscovery.slow_check(
|
||||
client_opts,
|
||||
fn -> Base.discover_processors(client_opts) end,
|
||||
timeout: timeouts[:slow],
|
||||
default: []
|
||||
)
|
||||
end
|
||||
|
||||
# Storage discovery with timeout - falls back to empty list on timeout
|
||||
defp discover_storage_with_timeout(client_opts, timeouts) do
|
||||
DeferredDiscovery.slow_check(
|
||||
client_opts,
|
||||
fn -> Base.discover_storage(client_opts) end,
|
||||
timeout: timeouts[:slow],
|
||||
default: []
|
||||
)
|
||||
end
|
||||
|
||||
defp discover_mempools_with_timeout(client_opts, timeouts) do
|
||||
DeferredDiscovery.slow_check(
|
||||
client_opts,
|
||||
fn -> Base.discover_memory_pools(client_opts) end,
|
||||
timeout: timeouts[:slow],
|
||||
default: []
|
||||
)
|
||||
end
|
||||
|
||||
defp discover_entity_physical_with_timeout(client_opts, timeouts) do
|
||||
DeferredDiscovery.slow_check(
|
||||
client_opts,
|
||||
fn -> Base.discover_entity_physical(client_opts) end,
|
||||
timeout: timeouts[:slow],
|
||||
default: []
|
||||
)
|
||||
end
|
||||
|
||||
defp discover_transceivers_with_timeout(client_opts, timeouts) do
|
||||
DeferredDiscovery.slow_check(
|
||||
client_opts,
|
||||
fn -> Base.discover_transceivers(client_opts) end,
|
||||
timeout: timeouts[:slow],
|
||||
default: []
|
||||
)
|
||||
end
|
||||
|
||||
defp discover_printer_supplies_with_timeout(client_opts, timeouts) do
|
||||
DeferredDiscovery.slow_check(
|
||||
client_opts,
|
||||
fn -> Base.discover_printer_supplies(client_opts) end,
|
||||
timeout: timeouts[:slow],
|
||||
default: []
|
||||
)
|
||||
end
|
||||
|
||||
@doc """
|
||||
Enqueues Oban discovery jobs for all SNMP-enabled devices in an organization.
|
||||
|
||||
|
|
@ -652,18 +545,21 @@ defmodule Towerops.Snmp.Discovery do
|
|||
device_id: device_id
|
||||
)
|
||||
|
||||
if check_counts.errors != [] do
|
||||
Logger.warning(
|
||||
"Failed to create #{length(check_counts.errors)} checks for device #{device_id}",
|
||||
device_id: device_id,
|
||||
errors: check_counts.errors
|
||||
)
|
||||
log_check_errors(device_id, check_counts.errors)
|
||||
end
|
||||
|
||||
defp log_check_errors(_device_id, []), do: :ok
|
||||
|
||||
defp log_check_errors(device_id, errors) do
|
||||
Logger.warning(
|
||||
"Failed to create #{length(errors)} checks for device #{device_id}",
|
||||
device_id: device_id,
|
||||
errors: errors
|
||||
)
|
||||
end
|
||||
|
||||
@spec build_client_opts(DeviceSchema.t()) :: Client.connection_opts()
|
||||
defp build_client_opts(device) do
|
||||
# Get SNMP config with hierarchical fallback (device -> site -> organization)
|
||||
snmp_config = Devices.get_snmp_config(device)
|
||||
|
||||
base_opts = [
|
||||
|
|
@ -672,8 +568,10 @@ defmodule Towerops.Snmp.Discovery do
|
|||
port: device.snmp_port || 161
|
||||
]
|
||||
|
||||
# Add version-specific credentials
|
||||
if snmp_config.version == "3" do
|
||||
add_credentials(base_opts, snmp_config, device)
|
||||
end
|
||||
|
||||
defp add_credentials(base_opts, %{version: "3"} = _snmp_config, device) do
|
||||
v3_config = Devices.get_snmpv3_config(device)
|
||||
|
||||
base_opts ++
|
||||
|
|
@ -685,9 +583,10 @@ defmodule Towerops.Snmp.Discovery do
|
|||
priv_protocol: v3_config.priv_protocol,
|
||||
priv_password: v3_config.priv_password
|
||||
]
|
||||
else
|
||||
base_opts ++ [community: snmp_config.community]
|
||||
end
|
||||
|
||||
defp add_credentials(base_opts, snmp_config, _device) do
|
||||
base_opts ++ [community: snmp_config.community]
|
||||
end
|
||||
|
||||
@spec discover_system(Client.connection_opts()) :: {:ok, system_info()} | {:error, term()}
|
||||
|
|
@ -768,37 +667,15 @@ defmodule Towerops.Snmp.Discovery do
|
|||
"build_device_info: system_info has _raw_discovery_data: #{inspect(Map.has_key?(system_info, :_raw_discovery_data))}"
|
||||
)
|
||||
|
||||
# Get profile-specific system info if profile has discover_system_info/1
|
||||
enriched_system_info =
|
||||
if function_exported?(profile, :discover_system_info, 1) do
|
||||
case profile.discover_system_info(client_opts) do
|
||||
{:ok, profile_info} ->
|
||||
Logger.debug("Enriched system_info with profile-specific data")
|
||||
Map.merge(system_info, profile_info)
|
||||
has_sys_info = function_exported?(profile, :discover_system_info, 1)
|
||||
|
||||
{:error, _reason} ->
|
||||
Logger.warning("Failed to get profile-specific system info, using base info")
|
||||
system_info
|
||||
end
|
||||
else
|
||||
system_info
|
||||
end
|
||||
enriched_system_info = enrich_with_profile_info(client_opts, system_info, profile, has_sys_info)
|
||||
|
||||
# Let the hard-coded profile identify the device
|
||||
identified_info = profile.identify_device(enriched_system_info)
|
||||
|
||||
# Collect raw debug data if profile supports it
|
||||
identified_info_with_debug =
|
||||
if function_exported?(profile, :collect_raw_debug_data, 1) do
|
||||
raw_data = profile.collect_raw_debug_data(client_opts)
|
||||
Logger.debug("Collected raw debug data: #{map_size(raw_data)} keys")
|
||||
has_collect_debug = function_exported?(profile, :collect_raw_debug_data, 1)
|
||||
|
||||
identified_info
|
||||
|> Map.put(:_raw_discovery_data, raw_data)
|
||||
|> Map.put(:_last_discovery_at, DateTime.utc_now())
|
||||
else
|
||||
identified_info
|
||||
end
|
||||
identified_info_with_debug = maybe_attach_debug_data(client_opts, identified_info, profile, has_collect_debug)
|
||||
|
||||
Logger.debug(
|
||||
"build_device_info: identified_info has _raw_discovery_data: #{inspect(Map.has_key?(identified_info_with_debug, :_raw_discovery_data))}"
|
||||
|
|
@ -808,10 +685,34 @@ defmodule Towerops.Snmp.Discovery do
|
|||
rescue
|
||||
_error ->
|
||||
Logger.error("Failed to identify device with profile, falling back to Base profile")
|
||||
# Fall back to Base profile identification to ensure manufacturer/model are set
|
||||
{:ok, Base.identify_device(system_info)}
|
||||
end
|
||||
|
||||
defp enrich_with_profile_info(_client_opts, system_info, _profile, false), do: system_info
|
||||
|
||||
defp enrich_with_profile_info(client_opts, system_info, profile, true) do
|
||||
case profile.discover_system_info(client_opts) do
|
||||
{:ok, profile_info} ->
|
||||
Logger.debug("Enriched system_info with profile-specific data")
|
||||
Map.merge(system_info, profile_info)
|
||||
|
||||
{:error, _reason} ->
|
||||
Logger.warning("Failed to get profile-specific system info, using base info")
|
||||
system_info
|
||||
end
|
||||
end
|
||||
|
||||
defp maybe_attach_debug_data(_client_opts, identified_info, _profile, false), do: identified_info
|
||||
|
||||
defp maybe_attach_debug_data(client_opts, identified_info, profile, true) do
|
||||
raw_data = profile.collect_raw_debug_data(client_opts)
|
||||
Logger.debug("Collected raw debug data: #{map_size(raw_data)} keys")
|
||||
|
||||
identified_info
|
||||
|> Map.put(:_raw_discovery_data, raw_data)
|
||||
|> Map.put(:_last_discovery_at, DateTime.utc_now())
|
||||
end
|
||||
|
||||
@spec discover_interfaces(Client.connection_opts(), profile()) ::
|
||||
{:ok, [interface_data()]}
|
||||
defp discover_interfaces(client_opts, {:yaml, profile}) do
|
||||
|
|
@ -857,17 +758,20 @@ defmodule Towerops.Snmp.Discovery do
|
|||
end
|
||||
|
||||
defp discover_vlans(client_opts, profile) when is_atom(profile) do
|
||||
if function_exported?(profile, :discover_vlans, 1) do
|
||||
discover_vlans_for_profile(client_opts, profile, function_exported?(profile, :discover_vlans, 1))
|
||||
end
|
||||
|
||||
defp discover_vlans_for_profile(client_opts, profile, true) do
|
||||
{:ok, vlans} = profile.discover_vlans(client_opts)
|
||||
Logger.debug("Discovered #{length(vlans)} VLANs")
|
||||
{:ok, vlans}
|
||||
else
|
||||
# Fall back to Base profile for VLAN discovery
|
||||
end
|
||||
|
||||
defp discover_vlans_for_profile(client_opts, _profile, false) do
|
||||
{:ok, vlans} = Base.discover_vlans(client_opts)
|
||||
Logger.debug("Discovered #{length(vlans)} VLANs using Base profile")
|
||||
{:ok, vlans}
|
||||
end
|
||||
end
|
||||
|
||||
@spec save_discovery_results(
|
||||
DeviceSchema.t(),
|
||||
|
|
@ -914,17 +818,21 @@ defmodule Towerops.Snmp.Discovery do
|
|||
|
||||
duration = System.monotonic_time(:millisecond) - start
|
||||
|
||||
if duration > 5_000 do
|
||||
log_slow_transaction(duration, device.id, interfaces, sensors)
|
||||
|
||||
result
|
||||
end
|
||||
|
||||
defp log_slow_transaction(duration, _device_id, _interfaces, _sensors) when duration <= 5_000, do: :ok
|
||||
|
||||
defp log_slow_transaction(duration, device_id, interfaces, sensors) do
|
||||
Logger.warning("Discovery transaction took #{duration}ms",
|
||||
device_id: device.id,
|
||||
device_id: device_id,
|
||||
interface_count: length(interfaces),
|
||||
sensor_count: length(sensors)
|
||||
)
|
||||
end
|
||||
|
||||
result
|
||||
end
|
||||
|
||||
# Prepares device_info for database save by sanitizing raw_discovery_data
|
||||
# This is done OUTSIDE the transaction to avoid DB timeout on large data
|
||||
defp prepare_device_info_for_save(device_info, device_id) do
|
||||
|
|
@ -952,26 +860,20 @@ defmodule Towerops.Snmp.Discovery do
|
|||
@spec upsert_device(DeviceSchema.t(), map()) :: Device.t()
|
||||
defp upsert_device(device, device_attrs) do
|
||||
Logger.info("Upserting SNMP device for #{device.name}")
|
||||
do_upsert_device(device_attrs, Repo.get_by(Device, device_id: device.id))
|
||||
end
|
||||
|
||||
case Repo.get_by(Device, device_id: device.id) do
|
||||
nil ->
|
||||
defp do_upsert_device(device_attrs, nil) do
|
||||
snmp_device =
|
||||
%Device{}
|
||||
|> Device.changeset(device_attrs)
|
||||
|> Repo.insert!()
|
||||
|
||||
# Log initial firmware version (no old version)
|
||||
if snmp_device.firmware_version do
|
||||
Firmware.log_firmware_change(
|
||||
snmp_device.id,
|
||||
nil,
|
||||
snmp_device.firmware_version
|
||||
)
|
||||
log_initial_firmware(snmp_device)
|
||||
snmp_device
|
||||
end
|
||||
|
||||
snmp_device
|
||||
|
||||
existing_device ->
|
||||
defp do_upsert_device(device_attrs, existing_device) do
|
||||
old_version = existing_device.firmware_version
|
||||
new_version = device_attrs[:firmware_version]
|
||||
|
||||
|
|
@ -980,17 +882,24 @@ defmodule Towerops.Snmp.Discovery do
|
|||
|> Device.changeset(device_attrs)
|
||||
|> Repo.update!()
|
||||
|
||||
# Detect and log firmware version changes
|
||||
if version_changed?(old_version, new_version) do
|
||||
Firmware.log_firmware_change(
|
||||
snmp_device.id,
|
||||
old_version,
|
||||
new_version
|
||||
)
|
||||
end
|
||||
|
||||
log_firmware_if_changed(snmp_device, old_version, new_version)
|
||||
snmp_device
|
||||
end
|
||||
|
||||
defp log_initial_firmware(%{firmware_version: fw} = snmp_device) when not is_nil(fw) do
|
||||
Firmware.log_firmware_change(snmp_device.id, nil, fw)
|
||||
end
|
||||
|
||||
defp log_initial_firmware(_snmp_device), do: :ok
|
||||
|
||||
defp log_firmware_if_changed(snmp_device, old_version, new_version) do
|
||||
log_firmware_when_changed(snmp_device, old_version, new_version, version_changed?(old_version, new_version))
|
||||
end
|
||||
|
||||
defp log_firmware_when_changed(_snmp_device, _old, _new, false), do: :ok
|
||||
|
||||
defp log_firmware_when_changed(snmp_device, old, new, true) do
|
||||
Firmware.log_firmware_change(snmp_device.id, old, new)
|
||||
end
|
||||
|
||||
defp version_changed?(nil, new_version) when is_binary(new_version), do: false
|
||||
|
|
@ -1187,12 +1096,20 @@ defmodule Towerops.Snmp.Discovery do
|
|||
Logger.debug("Deleted #{MapSet.size(removed_keys)} removed IP addresses")
|
||||
end
|
||||
|
||||
# Upsert each discovered IP address
|
||||
Enum.each(discovered_ip_addresses, fn ip_data ->
|
||||
upsert_ip_address(ip_data, if_index_to_interface, existing_ip_addresses)
|
||||
end)
|
||||
|
||||
if Application.get_env(:towerops, :env) == :dev do
|
||||
log_sync_ip_summary(device, discovered_ip_addresses, removed_keys, Application.get_env(:towerops, :env) == :dev)
|
||||
|
||||
:ok
|
||||
end
|
||||
|
||||
defp log_sync_ip_summary(_device, _discovered, _removed_keys, false) do
|
||||
Logger.debug("Synced IP addresses for device")
|
||||
end
|
||||
|
||||
defp log_sync_ip_summary(device, discovered_ip_addresses, removed_keys, true) do
|
||||
ipv4_count = Enum.count(discovered_ip_addresses, &(&1.ip_type == "ipv4"))
|
||||
ipv6_count = Enum.count(discovered_ip_addresses, &(&1.ip_type == "ipv6"))
|
||||
removed_count = MapSet.size(removed_keys)
|
||||
|
|
@ -1201,20 +1118,17 @@ defmodule Towerops.Snmp.Discovery do
|
|||
"Synced IP addresses: #{ipv4_count} IPv4, #{ipv6_count} IPv6, removed #{removed_count}",
|
||||
device_id: device.device_id
|
||||
)
|
||||
else
|
||||
Logger.debug("Synced #{length(discovered_ip_addresses)} IP addresses for device")
|
||||
end
|
||||
|
||||
:ok
|
||||
end
|
||||
|
||||
defp upsert_ip_address(ip_data, if_index_to_interface, _existing_ip_addresses) do
|
||||
interface_id = Map.get(if_index_to_interface, ip_data.if_index)
|
||||
upsert_ip_with_iface(ip_data, Map.get(if_index_to_interface, ip_data.if_index))
|
||||
end
|
||||
|
||||
if interface_id do
|
||||
defp upsert_ip_with_iface(_ip_data, nil), do: :ok
|
||||
|
||||
defp upsert_ip_with_iface(ip_data, interface_id) do
|
||||
ip_attrs = Map.put(ip_data, :snmp_interface_id, interface_id)
|
||||
|
||||
# Use on_conflict to handle race conditions gracefully
|
||||
%IpAddress{}
|
||||
|> IpAddress.changeset(ip_attrs)
|
||||
|> Repo.insert(
|
||||
|
|
@ -1222,7 +1136,6 @@ defmodule Towerops.Snmp.Discovery do
|
|||
conflict_target: [:snmp_interface_id, :ip_address]
|
||||
)
|
||||
end
|
||||
end
|
||||
|
||||
@type snmp_device_ref :: Device.t() | %{required(:id) => String.t()}
|
||||
@spec sync_processors(snmp_device_ref(), [map()]) :: :ok
|
||||
|
|
@ -1312,10 +1225,7 @@ defmodule Towerops.Snmp.Discovery do
|
|||
end
|
||||
end)
|
||||
|
||||
if invalid_count > 0 do
|
||||
Logger.warning("Dropped #{invalid_count} storage entries with malformed storage_index")
|
||||
end
|
||||
|
||||
log_invalid_storage_entries(invalid_count)
|
||||
discovered_indices = MapSet.new(valid_storage, & &1.storage_index)
|
||||
|
||||
# Delete storage that no longer exists on the device
|
||||
|
|
@ -1354,6 +1264,12 @@ defmodule Towerops.Snmp.Discovery do
|
|||
:ok
|
||||
end
|
||||
|
||||
defp log_invalid_storage_entries(0), do: :ok
|
||||
|
||||
defp log_invalid_storage_entries(count) do
|
||||
Logger.warning("Dropped #{count} storage entries with malformed storage_index")
|
||||
end
|
||||
|
||||
# Normalize storage index to integer (Base.discover_storage may return string from OID parsing).
|
||||
# Returns nil for malformed values — OID suffixes come from SNMP responses and must not be trusted.
|
||||
defp normalize_storage_index(index) when is_integer(index), do: index
|
||||
|
|
@ -1622,29 +1538,27 @@ defmodule Towerops.Snmp.Discovery do
|
|||
{:ok, synced}
|
||||
end
|
||||
|
||||
# Resolve parent_id from parent_index for all entities on a device
|
||||
defp resolve_entity_parent_relationships(snmp_device_id) do
|
||||
# Get all entities for this device
|
||||
entities = Repo.all(from(e in EntityPhysical, where: e.snmp_device_id == ^snmp_device_id))
|
||||
|
||||
# Build index map: entity_index -> id
|
||||
index_to_id = Map.new(entities, &{&1.entity_index, &1.id})
|
||||
|
||||
# Update each entity's parent_id based on parent_index
|
||||
Enum.each(entities, fn entity ->
|
||||
parent_id =
|
||||
if entity.parent_index && entity.parent_index != "0" do
|
||||
Map.get(index_to_id, entity.parent_index)
|
||||
parent_id = resolve_parent_id(index_to_id, entity)
|
||||
update_parent_if_changed(entity, parent_id)
|
||||
end)
|
||||
end
|
||||
|
||||
# Only update if parent_id changed
|
||||
if entity.parent_id != parent_id do
|
||||
defp resolve_parent_id(_index_to_id, %{parent_index: idx}) when is_nil(idx), do: nil
|
||||
defp resolve_parent_id(_index_to_id, %{parent_index: "0"}), do: nil
|
||||
defp resolve_parent_id(index_to_id, %{parent_index: idx}), do: Map.get(index_to_id, idx)
|
||||
|
||||
defp update_parent_if_changed(%{parent_id: existing} = _entity, existing), do: :ok
|
||||
|
||||
defp update_parent_if_changed(entity, parent_id) do
|
||||
entity
|
||||
|> Ecto.Changeset.change(parent_id: parent_id)
|
||||
|> Repo.update!()
|
||||
end
|
||||
end)
|
||||
end
|
||||
|
||||
@spec update_device_discovery_time(DeviceSchema.t()) ::
|
||||
{:ok, DeviceSchema.t()} | {:error, Ecto.Changeset.t()}
|
||||
|
|
@ -1690,18 +1604,28 @@ defmodule Towerops.Snmp.Discovery do
|
|||
|
||||
@spec update_device_name_from_snmp(DeviceSchema.t(), system_info()) ::
|
||||
{:ok, DeviceSchema.t()} | {:error, term()}
|
||||
defp update_device_name_from_snmp(%{name: name} = device, _system_info)
|
||||
when is_binary(name) and name != "" do
|
||||
Logger.info("Device name already set or no sysName, skipping update", device_id: device.id)
|
||||
{:ok, device}
|
||||
end
|
||||
|
||||
defp update_device_name_from_snmp(device, system_info) do
|
||||
# Only update device name if it's currently empty and we got a sysName from SNMP
|
||||
name_empty? = is_nil(device.name) or device.name == ""
|
||||
do_update_device_name(device, system_info, Map.has_key?(system_info, :sys_name))
|
||||
end
|
||||
|
||||
if name_empty? and Map.has_key?(system_info, :sys_name) do
|
||||
sys_name = Map.get(system_info, :sys_name)
|
||||
defp do_update_device_name(device, _system_info, false) do
|
||||
Logger.info("Device name already set or no sysName, skipping update", device_id: device.id)
|
||||
{:ok, device}
|
||||
end
|
||||
|
||||
if sys_name && sys_name != "" do
|
||||
Logger.info(
|
||||
"Updating device name from SNMP sysName: #{sys_name}",
|
||||
device_id: device.id
|
||||
)
|
||||
defp do_update_device_name(device, %{sys_name: sys_name}, true) when is_nil(sys_name) or sys_name == "" do
|
||||
Logger.info("sysName empty, keeping device name unchanged", device_id: device.id)
|
||||
{:ok, device}
|
||||
end
|
||||
|
||||
defp do_update_device_name(device, %{sys_name: sys_name}, true) do
|
||||
Logger.info("Updating device name from SNMP sysName: #{sys_name}", device_id: device.id)
|
||||
|
||||
result =
|
||||
device
|
||||
|
|
@ -1710,18 +1634,16 @@ defmodule Towerops.Snmp.Discovery do
|
|||
|
||||
Logger.info("Device name update completed", device_id: device.id)
|
||||
result
|
||||
else
|
||||
Logger.info("sysName empty, keeping device name unchanged", device_id: device.id)
|
||||
{:ok, device}
|
||||
end
|
||||
else
|
||||
Logger.info("Device name already set or no sysName, skipping update", device_id: device.id)
|
||||
{:ok, device}
|
||||
end
|
||||
end
|
||||
|
||||
defp log_ip_discovery_results(device, ip_addresses) do
|
||||
if Application.get_env(:towerops, :env) == :dev do
|
||||
log_ip_if_dev_env(device, ip_addresses, Application.get_env(:towerops, :env) == :dev)
|
||||
:ok
|
||||
end
|
||||
|
||||
defp log_ip_if_dev_env(_device, _ip_addresses, false), do: :ok
|
||||
|
||||
defp log_ip_if_dev_env(device, ip_addresses, true) do
|
||||
ipv4_count = Enum.count(ip_addresses, &(&1.ip_type == "ipv4"))
|
||||
ipv6_count = Enum.count(ip_addresses, &(&1.ip_type == "ipv6"))
|
||||
|
||||
|
|
@ -1731,24 +1653,20 @@ defmodule Towerops.Snmp.Discovery do
|
|||
)
|
||||
end
|
||||
|
||||
:ok
|
||||
defp interface_count(%Ecto.Association.NotLoaded{}), do: 0
|
||||
defp interface_count(interfaces) when is_list(interfaces), do: length(interfaces)
|
||||
|
||||
defp log_discovery_event(device, discovered_device, true) do
|
||||
log_discovery_event_common(device, discovered_device, "device_rediscovered", "Device rediscovered")
|
||||
end
|
||||
|
||||
defp log_discovery_event(device, discovered_device, is_rediscovery) do
|
||||
event_type = if is_rediscovery, do: "device_rediscovered", else: "device_discovered"
|
||||
|
||||
message =
|
||||
if is_rediscovery do
|
||||
"Device rediscovered: #{device.name} (#{discovered_device.manufacturer} #{discovered_device.model})"
|
||||
else
|
||||
"Device discovered: #{device.name} (#{discovered_device.manufacturer} #{discovered_device.model})"
|
||||
defp log_discovery_event(device, discovered_device, false) do
|
||||
log_discovery_event_common(device, discovered_device, "device_discovered", "Device discovered")
|
||||
end
|
||||
|
||||
interface_count =
|
||||
case discovered_device.interfaces do
|
||||
%Ecto.Association.NotLoaded{} -> 0
|
||||
interfaces when is_list(interfaces) -> length(interfaces)
|
||||
end
|
||||
defp log_discovery_event_common(device, discovered_device, event_type, event_label) do
|
||||
message = "#{event_label}: #{device.name} (#{discovered_device.manufacturer} #{discovered_device.model})"
|
||||
interface_count = interface_count(discovered_device.interfaces)
|
||||
|
||||
metadata = %{
|
||||
manufacturer: discovered_device.manufacturer,
|
||||
|
|
@ -1819,13 +1737,7 @@ defmodule Towerops.Snmp.Discovery do
|
|||
end
|
||||
|
||||
defp sanitize_for_json(binary) when is_binary(binary) do
|
||||
if String.printable?(binary) do
|
||||
binary
|
||||
else
|
||||
# Convert non-printable binary to base64 with marker
|
||||
# Use Elixir.Base to avoid conflict with Towerops.Snmp.Profiles.Base alias
|
||||
%{"_binary_base64" => Elixir.Base.encode64(binary)}
|
||||
end
|
||||
sanitize_binary(binary, String.printable?(binary))
|
||||
end
|
||||
|
||||
defp sanitize_for_json(tuple) when is_tuple(tuple) do
|
||||
|
|
@ -1836,6 +1748,12 @@ defmodule Towerops.Snmp.Discovery do
|
|||
|
||||
defp sanitize_for_json(other), do: other
|
||||
|
||||
defp sanitize_binary(binary, true), do: binary
|
||||
|
||||
defp sanitize_binary(binary, false) do
|
||||
%{"_binary_base64" => Elixir.Base.encode64(binary)}
|
||||
end
|
||||
|
||||
# Deduplicate sensors by OID - when multiple sensors share the same OID,
|
||||
# prefer the one with a proper description (not containing "MIB::").
|
||||
# Normalizes OIDs by stripping leading dots so ".1.3.6.1..." and "1.3.6.1..."
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue