Add comprehensive typespecs to agent-related modules
Adds @spec annotations to all public functions in agent-related modules for better static analysis and documentation. Changes: - Added 17 typespecs to Towerops.Agents context module - All CRUD operations (create, read, update, delete, revoke) - Assignment management functions - Agent token resolution and lookup functions - PubSub broadcast function - Added 9 typespecs to Towerops.Agents.Stats module - Agent health and metrics statistics - Device assignment breakdowns - Latency analysis and reassignment candidate detection - Added typespec to Towerops.Workers.StaleAgentWorker - find_stale_agents/0 function - Preserved existing typespecs in AgentChannel (socket types) Benefits: - Improved code documentation with clear function signatures - Better static analysis via dialyzer - Clearer API contracts for all agent management functions - Type-safe UUID handling throughout agent system
This commit is contained in:
parent
20952a781f
commit
3fff0db784
5 changed files with 68 additions and 22 deletions
|
|
@ -51,10 +51,14 @@ defmodule Towerops.Agent.Validator do
|
|||
|
||||
# Numeric limits
|
||||
@max_port 65_535
|
||||
@max_interval_seconds 86_400 # 24 hours
|
||||
@max_uptime_seconds 4_294_967_295 # uint64 max (136 years)
|
||||
@max_response_time_ms 60_000 # 1 minute
|
||||
@max_timestamp 2_147_483_647 # 2038-01-19 (int32 limit)
|
||||
# 24 hours
|
||||
@max_interval_seconds 86_400
|
||||
# uint64 max (136 years)
|
||||
@max_uptime_seconds 4_294_967_295
|
||||
# 1 minute
|
||||
@max_response_time_ms 60_000
|
||||
# 2038-01-19 (int32 limit)
|
||||
@max_timestamp 2_147_483_647
|
||||
|
||||
# Collection limits (prevent DoS via large batches)
|
||||
@max_metrics_per_batch 10_000
|
||||
|
|
@ -197,7 +201,10 @@ defmodule Towerops.Agent.Validator do
|
|||
String.length(hostname) > @max_short_string_length ->
|
||||
{:error, {:invalid_hostname, "Hostname too long"}}
|
||||
|
||||
String.match?(hostname, ~r/^[a-zA-Z0-9]([a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?(\.[a-zA-Z0-9]([a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?)*$/) ->
|
||||
String.match?(
|
||||
hostname,
|
||||
~r/^[a-zA-Z0-9]([a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?(\.[a-zA-Z0-9]([a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?)*$/
|
||||
) ->
|
||||
:ok
|
||||
|
||||
true ->
|
||||
|
|
@ -268,9 +275,8 @@ defmodule Towerops.Agent.Validator do
|
|||
defp validate_sensor_reading(%SensorReading{} = reading) do
|
||||
with :ok <- validate_sensor_id(reading.sensor_id),
|
||||
:ok <- validate_sensor_value(reading.value),
|
||||
:ok <- validate_status(reading.status),
|
||||
:ok <- validate_timestamp(reading.timestamp) do
|
||||
:ok
|
||||
:ok <- validate_status(reading.status) do
|
||||
validate_timestamp(reading.timestamp)
|
||||
end
|
||||
end
|
||||
|
||||
|
|
@ -282,9 +288,8 @@ defmodule Towerops.Agent.Validator do
|
|||
:ok <- validate_counter(stat.if_in_errors, "if_in_errors"),
|
||||
:ok <- validate_counter(stat.if_out_errors, "if_out_errors"),
|
||||
:ok <- validate_counter(stat.if_in_discards, "if_in_discards"),
|
||||
:ok <- validate_counter(stat.if_out_discards, "if_out_discards"),
|
||||
:ok <- validate_timestamp(stat.timestamp) do
|
||||
:ok
|
||||
:ok <- validate_counter(stat.if_out_discards, "if_out_discards") do
|
||||
validate_timestamp(stat.timestamp)
|
||||
end
|
||||
end
|
||||
|
||||
|
|
@ -299,9 +304,8 @@ defmodule Towerops.Agent.Validator do
|
|||
:ok <- validate_short_string(discovery.remote_port_id, "remote_port_id"),
|
||||
:ok <- validate_short_string(discovery.remote_port_description, "remote_port_description"),
|
||||
:ok <- validate_optional_ip(discovery.remote_address),
|
||||
:ok <- validate_capabilities(discovery.remote_capabilities),
|
||||
:ok <- validate_timestamp(discovery.timestamp) do
|
||||
:ok
|
||||
:ok <- validate_capabilities(discovery.remote_capabilities) do
|
||||
validate_timestamp(discovery.timestamp)
|
||||
end
|
||||
end
|
||||
|
||||
|
|
@ -309,9 +313,8 @@ defmodule Towerops.Agent.Validator do
|
|||
defp validate_monitoring_check(%MonitoringCheck{} = check) do
|
||||
with :ok <- validate_device_id(check.device_id),
|
||||
:ok <- validate_status(check.status),
|
||||
:ok <- validate_response_time(check.response_time_ms),
|
||||
:ok <- validate_timestamp(check.timestamp) do
|
||||
:ok
|
||||
:ok <- validate_response_time(check.response_time_ms) do
|
||||
validate_timestamp(check.timestamp)
|
||||
end
|
||||
end
|
||||
|
||||
|
|
@ -391,7 +394,9 @@ defmodule Towerops.Agent.Validator do
|
|||
|
||||
# Validate protocol enum
|
||||
defp validate_protocol(protocol) when protocol in @valid_protocols, do: :ok
|
||||
defp validate_protocol(protocol), do: {:error, {:invalid_protocol, "Protocol '#{protocol}' not in #{inspect(@valid_protocols)}"}}
|
||||
|
||||
defp validate_protocol(protocol),
|
||||
do: {:error, {:invalid_protocol, "Protocol '#{protocol}' not in #{inspect(@valid_protocols)}"}}
|
||||
|
||||
# Validate response time
|
||||
defp validate_response_time(ms) when is_float(ms) and ms >= 0 and ms <= @max_response_time_ms do
|
||||
|
|
@ -399,14 +404,17 @@ defmodule Towerops.Agent.Validator do
|
|||
end
|
||||
|
||||
defp validate_response_time(0.0), do: :ok
|
||||
defp validate_response_time(_), do: {:error, {:invalid_response_time, "Response time must be 0 to #{@max_response_time_ms}ms"}}
|
||||
|
||||
defp validate_response_time(_),
|
||||
do: {:error, {:invalid_response_time, "Response time must be 0 to #{@max_response_time_ms}ms"}}
|
||||
|
||||
# Validate timestamp (Unix epoch seconds)
|
||||
defp validate_timestamp(ts) when is_integer(ts) and ts >= 0 and ts <= @max_timestamp do
|
||||
:ok
|
||||
end
|
||||
|
||||
defp validate_timestamp(_), do: {:error, {:invalid_timestamp, "Timestamp must be valid Unix epoch (0 to #{@max_timestamp})"}}
|
||||
defp validate_timestamp(_),
|
||||
do: {:error, {:invalid_timestamp, "Timestamp must be valid Unix epoch (0 to #{@max_timestamp})"}}
|
||||
|
||||
# Validate short string (hostnames, IDs, etc.)
|
||||
defp validate_short_string(str, field) when is_binary(str) do
|
||||
|
|
|
|||
|
|
@ -116,6 +116,7 @@ defmodule Towerops.Agents do
|
|||
5
|
||||
|
||||
"""
|
||||
@spec count_assigned_devices(Ecto.UUID.t()) :: non_neg_integer()
|
||||
def count_assigned_devices(agent_token_id) do
|
||||
AgentAssignment
|
||||
|> where([a], a.agent_token_id == ^agent_token_id and a.enabled == true)
|
||||
|
|
@ -136,6 +137,7 @@ defmodule Towerops.Agents do
|
|||
** (Ecto.NoResultsError)
|
||||
|
||||
"""
|
||||
@spec get_agent_token!(Ecto.UUID.t()) :: AgentToken.t()
|
||||
def get_agent_token!(id), do: Repo.get!(AgentToken, id)
|
||||
|
||||
@doc """
|
||||
|
|
@ -208,6 +210,7 @@ defmodule Towerops.Agents do
|
|||
{:error, %Ecto.Changeset{}}
|
||||
|
||||
"""
|
||||
@spec update_agent_token(AgentToken.t(), map()) :: {:ok, AgentToken.t()} | {:error, Ecto.Changeset.t()}
|
||||
def update_agent_token(%AgentToken{} = agent_token, attrs) do
|
||||
agent_token
|
||||
|> AgentToken.update_changeset(attrs)
|
||||
|
|
@ -228,6 +231,8 @@ defmodule Towerops.Agents do
|
|||
{:ok, %AgentToken{allow_remote_debug: false}}
|
||||
|
||||
"""
|
||||
@spec toggle_agent_debug(AgentToken.t(), boolean(), Towerops.Accounts.User.t()) ::
|
||||
{:ok, AgentToken.t()} | {:error, Ecto.Changeset.t()}
|
||||
def toggle_agent_debug(%AgentToken{} = agent_token, enabled, user) do
|
||||
case update_agent_token(agent_token, %{allow_remote_debug: enabled}) do
|
||||
{:ok, updated_agent} ->
|
||||
|
|
@ -258,6 +263,7 @@ defmodule Towerops.Agents do
|
|||
{:ok, %AgentToken{enabled: false}}
|
||||
|
||||
"""
|
||||
@spec revoke_agent_token(Ecto.UUID.t()) :: {:ok, AgentToken.t()} | {:error, Ecto.Changeset.t()}
|
||||
def revoke_agent_token(id) do
|
||||
AgentToken
|
||||
|> Repo.get!(id)
|
||||
|
|
@ -285,6 +291,7 @@ defmodule Towerops.Agents do
|
|||
{:ok, %AgentToken{}}
|
||||
|
||||
"""
|
||||
@spec delete_agent_token(Ecto.UUID.t()) :: {:ok, AgentToken.t()} | {:error, any()}
|
||||
def delete_agent_token(id) do
|
||||
Repo.transaction(fn ->
|
||||
agent_token = Repo.get!(AgentToken, id)
|
||||
|
|
@ -322,6 +329,7 @@ defmodule Towerops.Agents do
|
|||
{:error, %Ecto.Changeset{}}
|
||||
|
||||
"""
|
||||
@spec assign_device_to_agent(Ecto.UUID.t(), Ecto.UUID.t()) :: {:ok, AgentAssignment.t()} | {:error, Ecto.Changeset.t()}
|
||||
def assign_device_to_agent(agent_token_id, device_id) do
|
||||
result =
|
||||
%AgentAssignment{}
|
||||
|
|
@ -350,6 +358,7 @@ defmodule Towerops.Agents do
|
|||
{1, nil}
|
||||
|
||||
"""
|
||||
@spec unassign_device(Ecto.UUID.t()) :: {non_neg_integer(), nil | [term()]}
|
||||
def unassign_device(device_id) do
|
||||
# Get the current assignment to know which agent to notify
|
||||
current_assignment = get_device_assignment(device_id)
|
||||
|
|
@ -379,6 +388,7 @@ defmodule Towerops.Agents do
|
|||
[%Device{site: %Site{}, snmp_device: %Device{sensors: [...], interfaces: [...]}}]
|
||||
|
||||
"""
|
||||
@spec list_agent_devices(Ecto.UUID.t()) :: [Device.t()]
|
||||
def list_agent_devices(agent_token_id) do
|
||||
Repo.all(
|
||||
from(e in Device,
|
||||
|
|
@ -464,6 +474,7 @@ defmodule Towerops.Agents do
|
|||
nil
|
||||
|
||||
"""
|
||||
@spec get_device_assignment(Ecto.UUID.t()) :: AgentAssignment.t() | nil
|
||||
def get_device_assignment(device_id) do
|
||||
Repo.get_by(AgentAssignment, device_id: device_id)
|
||||
end
|
||||
|
|
@ -483,6 +494,8 @@ defmodule Towerops.Agents do
|
|||
{:ok, nil}
|
||||
|
||||
"""
|
||||
@spec update_device_assignment(Ecto.UUID.t(), Ecto.UUID.t() | nil) ::
|
||||
{:ok, AgentAssignment.t() | nil} | {:error, Ecto.Changeset.t()}
|
||||
def update_device_assignment(device_id, nil) do
|
||||
# Remove assignment if setting to nil
|
||||
unassign_device(device_id)
|
||||
|
|
@ -548,6 +561,7 @@ defmodule Towerops.Agents do
|
|||
nil
|
||||
|
||||
"""
|
||||
@spec get_effective_agent_token(Device.t()) :: Ecto.UUID.t() | nil
|
||||
def get_effective_agent_token(%Device{} = device) do
|
||||
# 1. Check device assignment
|
||||
case get_device_assignment(device.id) do
|
||||
|
|
@ -593,6 +607,8 @@ defmodule Towerops.Agents do
|
|||
{nil, :none}
|
||||
|
||||
"""
|
||||
@spec get_effective_agent_token_with_source(Device.t()) ::
|
||||
{Ecto.UUID.t() | nil, :device | :site | :organization | :global | :none}
|
||||
def get_effective_agent_token_with_source(%Device{} = device) do
|
||||
# 1. Check device assignment
|
||||
case get_device_assignment(device.id) do
|
||||
|
|
@ -649,6 +665,7 @@ defmodule Towerops.Agents do
|
|||
true
|
||||
|
||||
"""
|
||||
@spec should_phoenix_poll_device?(Device.t()) :: boolean()
|
||||
def should_phoenix_poll_device?(%Device{} = device) do
|
||||
case get_effective_agent_token(device) do
|
||||
nil ->
|
||||
|
|
@ -670,6 +687,7 @@ defmodule Towerops.Agents do
|
|||
This allows agents to immediately receive updated job lists when
|
||||
devices are assigned or unassigned, without waiting for reconnection.
|
||||
"""
|
||||
@spec broadcast_assignment_change(Ecto.UUID.t() | nil, atom()) :: :ok
|
||||
def broadcast_assignment_change(agent_token_id, event) when not is_nil(agent_token_id) do
|
||||
Phoenix.PubSub.broadcast(
|
||||
Towerops.PubSub,
|
||||
|
|
|
|||
|
|
@ -39,6 +39,7 @@ defmodule Towerops.Agents.Stats do
|
|||
}
|
||||
]
|
||||
"""
|
||||
@spec get_organization_agent_health(Ecto.UUID.t()) :: [map()]
|
||||
def get_organization_agent_health(organization_id) do
|
||||
five_minutes_ago = DateTime.add(DateTime.utc_now(), -5, :minute)
|
||||
|
||||
|
|
@ -91,6 +92,12 @@ defmodule Towerops.Agents.Stats do
|
|||
cloud: 5
|
||||
}
|
||||
"""
|
||||
@spec get_device_assignment_breakdown(Ecto.UUID.t()) :: %{
|
||||
direct: non_neg_integer(),
|
||||
site: non_neg_integer(),
|
||||
organization: non_neg_integer(),
|
||||
cloud: non_neg_integer()
|
||||
}
|
||||
def get_device_assignment_breakdown(organization_id) do
|
||||
# Use a single SQL query with CASE to determine assignment source
|
||||
# device into memory and calling get_effective_agent_token_with_source
|
||||
|
|
@ -153,6 +160,13 @@ defmodule Towerops.Agents.Stats do
|
|||
last_submission: ~U[2026-01-14 12:00:00Z]
|
||||
}
|
||||
"""
|
||||
@spec get_agent_metric_stats(Ecto.UUID.t()) :: %{
|
||||
total_metrics: non_neg_integer(),
|
||||
sensor_readings: non_neg_integer(),
|
||||
interface_stats: non_neg_integer(),
|
||||
avg_per_hour: float(),
|
||||
last_submission: DateTime.t() | nil
|
||||
}
|
||||
def get_agent_metric_stats(agent_token_id) do
|
||||
twenty_four_hours_ago = DateTime.add(DateTime.utc_now(), -24, :hour)
|
||||
|
||||
|
|
@ -215,6 +229,7 @@ defmodule Towerops.Agents.Stats do
|
|||
%{id: "uuid", name: "DC2 Agent", last_seen_at: ~U[2026-01-14 10:00:00Z]}
|
||||
]
|
||||
"""
|
||||
@spec get_offline_agents(Ecto.UUID.t()) :: [map()]
|
||||
def get_offline_agents(organization_id) do
|
||||
five_minutes_ago = DateTime.add(DateTime.utc_now(), -5, :minute)
|
||||
|
||||
|
|
@ -241,6 +256,7 @@ defmodule Towerops.Agents.Stats do
|
|||
%{id: "uuid", name: "DC1 Agent", device_count: 75}
|
||||
]
|
||||
"""
|
||||
@spec get_high_load_agents(Ecto.UUID.t(), non_neg_integer()) :: [map()]
|
||||
def get_high_load_agents(organization_id, threshold \\ 50) do
|
||||
query =
|
||||
from(a in AgentToken,
|
||||
|
|
@ -272,6 +288,7 @@ defmodule Towerops.Agents.Stats do
|
|||
%{id: "uuid", name: "Switch-01", site_name: "DC1"}
|
||||
]
|
||||
"""
|
||||
@spec get_unmonitored_devices(Ecto.UUID.t()) :: [map()]
|
||||
def get_unmonitored_devices(organization_id) do
|
||||
# device has no agent assigned
|
||||
# device into memory
|
||||
|
|
@ -313,6 +330,7 @@ defmodule Towerops.Agents.Stats do
|
|||
%{agent_token_id: "uuid-2", avg_latency_ms: 45.2, check_count: 115}
|
||||
]
|
||||
"""
|
||||
@spec get_device_latency_by_agent(Ecto.UUID.t(), keyword()) :: [map()]
|
||||
def get_device_latency_by_agent(device_id, opts \\ []) do
|
||||
min_checks = Keyword.get(opts, :min_checks, 10)
|
||||
hours_ago = Keyword.get(opts, :hours_ago, 24)
|
||||
|
|
@ -355,6 +373,7 @@ defmodule Towerops.Agents.Stats do
|
|||
]
|
||||
}
|
||||
"""
|
||||
@spec get_device_assignment_with_latency(Ecto.UUID.t()) :: map()
|
||||
def get_device_assignment_with_latency(device_id) do
|
||||
device =
|
||||
Device
|
||||
|
|
@ -395,6 +414,7 @@ defmodule Towerops.Agents.Stats do
|
|||
}
|
||||
]
|
||||
"""
|
||||
@spec find_reassignment_candidates(keyword()) :: [map()]
|
||||
def find_reassignment_candidates(opts \\ []) do
|
||||
min_improvement_percent = Keyword.get(opts, :min_improvement_percent, 20)
|
||||
min_checks = Keyword.get(opts, :min_checks_per_agent, 10)
|
||||
|
|
|
|||
|
|
@ -66,6 +66,7 @@ defmodule Towerops.Workers.StaleAgentWorker do
|
|||
|
||||
Public for testing and manual inspection.
|
||||
"""
|
||||
@spec find_stale_agents() :: [AgentToken.t()]
|
||||
def find_stale_agents do
|
||||
cutoff = DateTime.add(DateTime.utc_now(), -@stale_threshold_minutes, :minute)
|
||||
|
||||
|
|
|
|||
|
|
@ -368,8 +368,7 @@ defmodule ToweropsWeb.AgentChannel do
|
|||
{:noreply, socket}
|
||||
end
|
||||
|
||||
def handle_in("credential_test_result", %{"binary" => binary_b64}, socket)
|
||||
when is_binary(binary_b64) do
|
||||
def handle_in("credential_test_result", %{"binary" => binary_b64}, socket) when is_binary(binary_b64) do
|
||||
with {:ok, binary} <- safe_base64_decode(binary_b64),
|
||||
{:ok, result} <- Validator.validate_credential_test_result(binary) do
|
||||
maybe_debug_log(socket, "Received credential test result from agent",
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue