From 9eca2e7069c9d1e7f004c5da8cb2fa33369b5030 Mon Sep 17 00:00:00 2001 From: Graham McIntire Date: Mon, 5 Jan 2026 08:26:16 -0600 Subject: [PATCH] Fix database schema mismatches preventing SNMP and monitoring data collection This fixes critical issues where SNMP sensor readings, interface stats, and monitoring checks were not being saved to the database due to schema mismatches. Database schema fixes: - Recreate snmp_sensor_readings table with binary_id primary key - Recreate snmp_interface_stats table with binary_id primary key - Recreate monitoring_checks table with binary_id primary key The original migrations used default integer primary keys, but the schemas expected binary_id (UUID). This caused Ecto to generate UUID strings that Postgrex tried to encode as binaries, resulting in silent insert failures. SNMP interface stats fixes: - Fix field name mismatch in get_interface_stats (if_in_octets vs in_octets) - Remove unnecessary tuple handling code (Client already unwraps SNMPKit tuples) - Clean up decode_snmp_value function and improve error messages Monitoring check fixes: - Fix response_time type from float to integer in Poller.check_device - Add error logging to catch and report check creation failures These changes enable: - SNMP sensor readings to be collected and stored (disk usage, memory, CPU, etc.) - Interface statistics to be collected (Counter64 octets, errors, discards) - Equipment availability metrics to update on the dashboard --- lib/towerops/monitoring/equipment_monitor.ex | 18 +++-- lib/towerops/snmp/poller.ex | 4 +- lib/towerops/snmp/poller_worker.ex | 74 ++++++++++--------- .../live/equipment_live/show.html.heex | 4 +- ...134321_fix_sensor_readings_primary_key.exs | 49 ++++++++++++ ...134404_fix_interface_stats_primary_key.exs | 55 ++++++++++++++ ...2046_fix_monitoring_checks_primary_key.exs | 46 ++++++++++++ 7 files changed, 207 insertions(+), 43 deletions(-) create mode 100644 priv/repo/migrations/20260105134321_fix_sensor_readings_primary_key.exs create mode 100644 priv/repo/migrations/20260105134404_fix_interface_stats_primary_key.exs create mode 100644 priv/repo/migrations/20260105142046_fix_monitoring_checks_primary_key.exs diff --git a/lib/towerops/monitoring/equipment_monitor.ex b/lib/towerops/monitoring/equipment_monitor.ex index 0002fcd9..76cdfa58 100644 --- a/lib/towerops/monitoring/equipment_monitor.ex +++ b/lib/towerops/monitoring/equipment_monitor.ex @@ -91,12 +91,18 @@ defmodule Towerops.Monitoring.EquipmentMonitor do end # Save the check result - Monitoring.create_check(%{ - equipment_id: equipment_id, - status: status, - response_time_ms: response_time, - checked_at: now - }) + case Monitoring.create_check(%{ + equipment_id: equipment_id, + status: status, + response_time_ms: response_time, + checked_at: now + }) do + {:ok, _check} -> + :ok + + {:error, changeset} -> + Logger.error("Failed to create monitoring check for equipment #{equipment_id}: #{inspect(changeset.errors)}") + end # Update equipment status if it changed new_status = if status == :success, do: :up, else: :down diff --git a/lib/towerops/snmp/poller.ex b/lib/towerops/snmp/poller.ex index ff171051..c2af5987 100644 --- a/lib/towerops/snmp/poller.ex +++ b/lib/towerops/snmp/poller.ex @@ -28,7 +28,7 @@ defmodule Towerops.Snmp.Poller do {:error, :timeout} """ @impl true - @spec check_device(Client.connection_opts()) :: {:ok, float()} | {:error, term()} + @spec check_device(Client.connection_opts()) :: {:ok, integer()} | {:error, term()} def check_device(client_opts) do start_time = System.monotonic_time(:millisecond) @@ -36,7 +36,7 @@ defmodule Towerops.Snmp.Poller do {:ok, _uptime} -> end_time = System.monotonic_time(:millisecond) response_time = end_time - start_time - {:ok, response_time / 1.0} + {:ok, response_time} {:error, reason} = error -> Logger.debug("SNMP check failed: #{inspect(reason)}") diff --git a/lib/towerops/snmp/poller_worker.ex b/lib/towerops/snmp/poller_worker.ex index 1233e7a9..11f00805 100644 --- a/lib/towerops/snmp/poller_worker.ex +++ b/lib/towerops/snmp/poller_worker.ex @@ -222,12 +222,12 @@ defmodule Towerops.Snmp.PollerWorker do defp get_interface_stats(client_opts, if_index) do oids = [ - in_octets: "1.3.6.1.2.1.2.2.1.10.#{if_index}", - out_octets: "1.3.6.1.2.1.2.2.1.16.#{if_index}", - in_errors: "1.3.6.1.2.1.2.2.1.14.#{if_index}", - out_errors: "1.3.6.1.2.1.2.2.1.20.#{if_index}", - in_discards: "1.3.6.1.2.1.2.2.1.13.#{if_index}", - out_discards: "1.3.6.1.2.1.2.2.1.19.#{if_index}" + if_in_octets: "1.3.6.1.2.1.2.2.1.10.#{if_index}", + if_out_octets: "1.3.6.1.2.1.2.2.1.16.#{if_index}", + if_in_errors: "1.3.6.1.2.1.2.2.1.14.#{if_index}", + if_out_errors: "1.3.6.1.2.1.2.2.1.20.#{if_index}", + if_in_discards: "1.3.6.1.2.1.2.2.1.13.#{if_index}", + if_out_discards: "1.3.6.1.2.1.2.2.1.19.#{if_index}" ] results = @@ -242,45 +242,51 @@ defmodule Towerops.Snmp.PollerWorker do end # Decode SNMP values, handling Counter64 and other binary types - defp decode_snmp_value(value) when is_number(value), do: value + # Note: Client.get already unwraps SNMPKit tuples via extract_snmp_value + defp decode_snmp_value(value) when is_number(value) do + value + end - # Try to decode based on byte size + # Handle binary Counter64 values (8 bytes, big-endian unsigned integer) defp decode_snmp_value(value) when is_binary(value) do - case byte_size(value) do - 8 -> - # Standard Counter64 (8 bytes, big-endian unsigned integer) - <> = value - counter + size = byte_size(value) - 16 -> - # Some SNMP implementations return 16-byte values - # Try reading last 8 bytes as Counter64 - <<_prefix::binary-size(8), counter::unsigned-big-integer-size(64)>> = value - counter + result = + case size do + 8 -> + # Standard Counter64 (8 bytes, big-endian unsigned integer) + <> = value + counter - size when size > 8 -> - # Large binary, try reading last 8 bytes - offset = size - 8 - <<_prefix::binary-size(offset), counter::unsigned-big-integer-size(64)>> = value - Logger.debug("Decoded #{size}-byte SNMP value as Counter64 from last 8 bytes: #{counter}") - counter + 16 -> + # Some SNMP implementations return 16-byte values + # Try reading last 8 bytes as Counter64 + <<_prefix::binary-size(8), counter::unsigned-big-integer-size(64)>> = value + counter - _ -> - # Unknown binary format or too small - Logger.warning("Unknown SNMP binary value format, size: #{byte_size(value)}, bytes: #{inspect(value)}") + s when s > 8 -> + # Large binary, try reading last 8 bytes + offset = s - 8 + <<_prefix::binary-size(offset), counter::unsigned-big-integer-size(64)>> = value + counter - nil - end + _ -> + # Unknown binary format or too small + Logger.warning("Unknown SNMP binary value format, size: #{size}") + nil + end + + result rescue error -> - Logger.error( - "Failed to decode SNMP binary value (size: #{byte_size(value)}): #{inspect(error)}, bytes: #{inspect(value)}" - ) - + Logger.error("Failed to decode SNMP binary value (size: #{byte_size(value)}): #{inspect(error)}") nil end - defp decode_snmp_value(_), do: nil + defp decode_snmp_value(other) do + Logger.warning("Unexpected SNMP value type: #{inspect(other)}") + nil + end defp build_client_opts(equipment) do [ diff --git a/lib/towerops_web/live/equipment_live/show.html.heex b/lib/towerops_web/live/equipment_live/show.html.heex index f45c1206..97e40cf6 100644 --- a/lib/towerops_web/live/equipment_live/show.html.heex +++ b/lib/towerops_web/live/equipment_live/show.html.heex @@ -55,7 +55,9 @@
Last Check
{time_ago(@equipment.last_checked_at)} diff --git a/priv/repo/migrations/20260105134321_fix_sensor_readings_primary_key.exs b/priv/repo/migrations/20260105134321_fix_sensor_readings_primary_key.exs new file mode 100644 index 00000000..6c6e154c --- /dev/null +++ b/priv/repo/migrations/20260105134321_fix_sensor_readings_primary_key.exs @@ -0,0 +1,49 @@ +defmodule Towerops.Repo.Migrations.FixSensorReadingsPrimaryKey do + use Ecto.Migration + + def up do + # Drop and recreate the table with correct primary key type + # This is safe because sensor readings are time-series data that regenerates + drop table(:snmp_sensor_readings) + + create table(:snmp_sensor_readings, primary_key: false) do + add :id, :binary_id, primary_key: true + + add :sensor_id, references(:snmp_sensors, type: :binary_id, on_delete: :delete_all), + null: false + + add :value, :float + add :status, :string, null: false + add :checked_at, :utc_datetime, null: false + + timestamps(type: :utc_datetime, updated_at: false) + end + + # Recreate indexes + create index(:snmp_sensor_readings, [:sensor_id]) + create index(:snmp_sensor_readings, [:sensor_id, :checked_at]) + create index(:snmp_sensor_readings, [:status]) + create index(:snmp_sensor_readings, [:checked_at]) + end + + def down do + # Recreate with old integer primary key + drop table(:snmp_sensor_readings) + + create table(:snmp_sensor_readings) do + add :sensor_id, references(:snmp_sensors, type: :binary_id, on_delete: :delete_all), + null: false + + add :value, :float + add :status, :string, null: false + add :checked_at, :utc_datetime, null: false + + timestamps(type: :utc_datetime, updated_at: false) + end + + create index(:snmp_sensor_readings, [:sensor_id]) + create index(:snmp_sensor_readings, [:sensor_id, :checked_at]) + create index(:snmp_sensor_readings, [:status]) + create index(:snmp_sensor_readings, [:checked_at]) + end +end diff --git a/priv/repo/migrations/20260105134404_fix_interface_stats_primary_key.exs b/priv/repo/migrations/20260105134404_fix_interface_stats_primary_key.exs new file mode 100644 index 00000000..78e246ec --- /dev/null +++ b/priv/repo/migrations/20260105134404_fix_interface_stats_primary_key.exs @@ -0,0 +1,55 @@ +defmodule Towerops.Repo.Migrations.FixInterfaceStatsPrimaryKey do + use Ecto.Migration + + def up do + # Drop and recreate the table with correct primary key type + # This is safe because interface stats are time-series data that regenerates + drop table(:snmp_interface_stats) + + create table(:snmp_interface_stats, primary_key: false) do + add :id, :binary_id, primary_key: true + + add :interface_id, references(:snmp_interfaces, type: :binary_id, on_delete: :delete_all), + null: false + + add :if_in_octets, :bigint + add :if_out_octets, :bigint + add :if_in_errors, :bigint + add :if_out_errors, :bigint + add :if_in_discards, :bigint + add :if_out_discards, :bigint + add :checked_at, :utc_datetime, null: false + + timestamps(type: :utc_datetime, updated_at: false) + end + + # Recreate indexes + create index(:snmp_interface_stats, [:interface_id]) + create index(:snmp_interface_stats, [:interface_id, :checked_at]) + create index(:snmp_interface_stats, [:checked_at]) + end + + def down do + # Recreate with old integer primary key + drop table(:snmp_interface_stats) + + create table(:snmp_interface_stats) do + add :interface_id, references(:snmp_interfaces, type: :binary_id, on_delete: :delete_all), + null: false + + add :if_in_octets, :bigint + add :if_out_octets, :bigint + add :if_in_errors, :bigint + add :if_out_errors, :bigint + add :if_in_discards, :bigint + add :if_out_discards, :bigint + add :checked_at, :utc_datetime, null: false + + timestamps(type: :utc_datetime, updated_at: false) + end + + create index(:snmp_interface_stats, [:interface_id]) + create index(:snmp_interface_stats, [:interface_id, :checked_at]) + create index(:snmp_interface_stats, [:checked_at]) + end +end diff --git a/priv/repo/migrations/20260105142046_fix_monitoring_checks_primary_key.exs b/priv/repo/migrations/20260105142046_fix_monitoring_checks_primary_key.exs new file mode 100644 index 00000000..1cb83aa7 --- /dev/null +++ b/priv/repo/migrations/20260105142046_fix_monitoring_checks_primary_key.exs @@ -0,0 +1,46 @@ +defmodule Towerops.Repo.Migrations.FixMonitoringChecksPrimaryKey do + use Ecto.Migration + + def up do + # Drop and recreate the table with correct primary key type + drop table(:monitoring_checks) + + create table(:monitoring_checks, primary_key: false) do + add :id, :binary_id, primary_key: true + + add :equipment_id, references(:equipment, type: :binary_id, on_delete: :delete_all), + null: false + + add :status, :string, null: false + add :response_time_ms, :integer + add :checked_at, :utc_datetime, null: false + + timestamps(type: :utc_datetime, updated_at: false) + end + + # Recreate indexes + create index(:monitoring_checks, [:equipment_id]) + create index(:monitoring_checks, [:equipment_id, :checked_at]) + create index(:monitoring_checks, [:checked_at]) + end + + def down do + # Recreate with old integer primary key + drop table(:monitoring_checks) + + create table(:monitoring_checks) do + add :equipment_id, references(:equipment, type: :binary_id, on_delete: :delete_all), + null: false + + add :status, :string, null: false + add :response_time_ms, :integer + add :checked_at, :utc_datetime, null: false + + timestamps(type: :utc_datetime, updated_at: false) + end + + create index(:monitoring_checks, [:equipment_id]) + create index(:monitoring_checks, [:equipment_id, :checked_at]) + create index(:monitoring_checks, [:checked_at]) + end +end