diff --git a/lib/mix/tasks/towerops.fix_mikrotik_temperature_scaling.ex b/lib/mix/tasks/towerops.fix_mikrotik_temperature_scaling.ex new file mode 100644 index 00000000..2499d6cc --- /dev/null +++ b/lib/mix/tasks/towerops.fix_mikrotik_temperature_scaling.ex @@ -0,0 +1,58 @@ +defmodule Mix.Tasks.Towerops.FixMikrotikTemperatureScaling do + @shortdoc "Renormalise stored temperature sensor readings using SensorScale" + + @moduledoc """ + One-shot backfill that re-runs `Towerops.Snmp.SensorScale.normalize/2` + over every stored temperature reading. Catches historical mis-scaled + rows from MikroTik `mtxrGaugeValue` (some models report deci-degrees) + and any other sensor whose YAML profile didn't include the right + divisor at discovery time. + + Run from inside a release pod: + + /app/bin/towerops eval 'Mix.Tasks.Towerops.FixMikrotikTemperatureScaling.run([])' + + Or via mix during dev: + + mix towerops.fix_mikrotik_temperature_scaling + """ + + use Mix.Task + + import Ecto.Query + + alias Towerops.Repo + alias Towerops.Snmp.Sensor + alias Towerops.Snmp.SensorScale + + @impl Mix.Task + def run(_argv) do + {:ok, _} = Application.ensure_all_started(:towerops) + + sensors = + Repo.all( + from(s in Sensor, where: s.sensor_type == "temperature" and not is_nil(s.last_value) and s.last_value > 150.0) + ) + + {fixed, skipped} = + Enum.reduce(sensors, {0, 0}, fn sensor, {fixed, skipped} -> + normalized = SensorScale.normalize(sensor.sensor_type, sensor.last_value) + + if normalized == sensor.last_value do + {fixed, skipped + 1} + else + {:ok, _} = + sensor + |> Sensor.changeset(%{last_value: Float.round(normalized / 1.0, 2)}) + |> Repo.update() + + {fixed + 1, skipped} + end + end) + + Mix.shell().info( + "Temperature backfill complete: rescaled #{fixed} rows, " <> + "left #{skipped} unchanged." + ) + end +end diff --git a/lib/towerops/recommendations/rules/device_overheating.ex b/lib/towerops/recommendations/rules/device_overheating.ex index df5af171..76ca51d4 100644 --- a/lib/towerops/recommendations/rules/device_overheating.ex +++ b/lib/towerops/recommendations/rules/device_overheating.ex @@ -26,6 +26,11 @@ defmodule Towerops.Recommendations.Rules.DeviceOverheating do @warning_threshold_c 65.0 @critical_threshold_c 75.0 @qoe_concern_threshold 60.0 + # Anything above this is implausible for routing/wireless gear and + # almost always indicates a missing divisor in the SNMP profile. Skip + # those readings rather than emit a "5,692 °C" alert — the underlying + # scaling bug should be fixed in the profile, not surfaced as ops noise. + @sanity_max_c 150.0 @spec evaluate(Ecto.UUID.t()) :: [map()] def evaluate(organization_id) when is_binary(organization_id) do @@ -46,7 +51,8 @@ defmodule Towerops.Recommendations.Rules.DeviceOverheating do d.organization_id == ^organization_id and s.sensor_type == "temperature" and not is_nil(s.last_value) and - s.last_value >= ^@warning_threshold_c, + s.last_value >= ^@warning_threshold_c and + s.last_value <= ^@sanity_max_c, order_by: [asc: d.id, desc: s.last_value], select: {d, s} ) diff --git a/lib/towerops/snmp/sensor_scale.ex b/lib/towerops/snmp/sensor_scale.ex new file mode 100644 index 00000000..400cc492 --- /dev/null +++ b/lib/towerops/snmp/sensor_scale.ex @@ -0,0 +1,49 @@ +defmodule Towerops.Snmp.SensorScale do + @moduledoc """ + Post-processing scale correction for SNMP sensor readings whose YAML + profile reports a unit but not the per-model scale factor. + + The motivating case is MikroTik `mtxrGaugeValue` for temperature + (`MIKROTIK-MIB` OID `1.3.6.1.4.1.14988.1.1.3.100.1.3.*`, unit 1 = °C). + Some models — CRS317 in upstream test data — return whole degrees + (`36`); others return deci-degrees (`569` → 56.9 °C). The MIB doesn't + expose this difference, so we normalize at write time by + divide-by-10ing implausibly hot temperature readings until they fall + in a believable range. + + This is intentionally a narrow heuristic — only `temperature` sensors, + only correcting *upward* implausibility. A well-scaled value passes + through untouched. + """ + + # Routing/wireless gear specs cap around 70-90 °C; firmware crashes + # well before 150 °C. Anything beyond is a unit mistake, not a real + # reading. + @sane_max_c 150.0 + @max_iterations 3 + + @doc """ + Normalises a sensor reading. + + iex> normalize(:temperature, 5692.0) + 56.92 + + iex> normalize(:temperature, 36.0) + 36.0 + + iex> normalize(:other, 5692.0) + 5692.0 + """ + @spec normalize(String.t() | atom(), number() | nil) :: number() | nil + def normalize(_type, nil), do: nil + + def normalize(type, value) when type in ["temperature", :temperature] and is_number(value) do + rescale(value, 0) + end + + def normalize(_type, value), do: value + + defp rescale(value, n) when n >= @max_iterations, do: value + defp rescale(value, _n) when value <= @sane_max_c, do: value + defp rescale(value, n), do: rescale(value / 10.0, n + 1) +end diff --git a/lib/towerops/workers/device_poller_worker.ex b/lib/towerops/workers/device_poller_worker.ex index 36abc09d..435e98ef 100644 --- a/lib/towerops/workers/device_poller_worker.ex +++ b/lib/towerops/workers/device_poller_worker.ex @@ -36,6 +36,7 @@ defmodule Towerops.Workers.DevicePollerWorker do alias Towerops.Snmp.NeighborDiscovery alias Towerops.Snmp.RadioState alias Towerops.Snmp.SensorChangeDetector + alias Towerops.Snmp.SensorScale alias Towerops.Snmp.WirelessClientDiscovery alias Towerops.Workers.PollingOffset @@ -1102,8 +1103,9 @@ defmodule Towerops.Workers.DevicePollerWorker do decoded_value = decode_snmp_value(raw_value) if is_number(decoded_value) do - value = Float.round(decoded_value / sensor.sensor_divisor, 1) - {:ok, value} + divided = decoded_value / sensor.sensor_divisor + normalized = SensorScale.normalize(sensor.sensor_type, divided) + {:ok, Float.round(normalized / 1.0, 2)} else {:error, :non_numeric} end diff --git a/priv/profiles/os_discovery/routeros.yaml b/priv/profiles/os_discovery/routeros.yaml index d629b161..cdec1f6d 100644 --- a/priv/profiles/os_discovery/routeros.yaml +++ b/priv/profiles/os_discovery/routeros.yaml @@ -28,6 +28,13 @@ modules: num_oid: '.1.3.6.1.4.1.14988.1.1.3.100.1.3.{{ $index }}' descr: MIKROTIK-MIB::mtxrGaugeName group: System + # mtxrGaugeValue (mtxrGaugeUnit=1) reports °C, but the + # raw scale varies by model — CRS317 returns whole + # degrees (36), some RB-series devices return + # deci-degrees (569). The poller normalises any + # implausibly large temperature reading at write time + # (see Towerops.Snmp.SensorScale) so we leave divisor + # at the literal protocol value here. index: 'mtxrGaugeTemperature.{{ $index }}' skip_values: - oid: MIKROTIK-MIB::mtxrGaugeUnit diff --git a/test/towerops/recommendations/rules/device_overheating_test.exs b/test/towerops/recommendations/rules/device_overheating_test.exs index 32f860be..7f2d1ce2 100644 --- a/test/towerops/recommendations/rules/device_overheating_test.exs +++ b/test/towerops/recommendations/rules/device_overheating_test.exs @@ -71,6 +71,15 @@ defmodule Towerops.Recommendations.Rules.DeviceOverheatingTest do assert insight.urgency == "critical" end + test "ignores implausible readings (likely missing-divisor scaling bug)", + %{org: org, snmp: snmp} do + # 5692 °C is what a MikroTik mtxrGaugeValue temperature looks like + # without divisor:10 — defense in depth so a profile bug never produces + # a "5,692 °C" critical alert. + insert_temp(snmp, 5692.0, descr: "Temperature 6") + assert DeviceOverheating.evaluate(org.id) == [] + end + test "uses the hottest sensor when multiple are present", %{org: org, snmp: snmp} do insert_temp(snmp, 60.0, descr: "CPU") insert_temp(snmp, 70.0, descr: "Enclosure") diff --git a/test/towerops/snmp/sensor_scale_test.exs b/test/towerops/snmp/sensor_scale_test.exs new file mode 100644 index 00000000..59079425 --- /dev/null +++ b/test/towerops/snmp/sensor_scale_test.exs @@ -0,0 +1,43 @@ +defmodule Towerops.Snmp.SensorScaleTest do + use ExUnit.Case, async: true + + alias Towerops.Snmp.SensorScale + + describe "normalize/2 — temperature" do + test "passes plausible whole-degree readings through unchanged" do + assert SensorScale.normalize("temperature", 36.0) == 36.0 + assert SensorScale.normalize("temperature", 75.4) == 75.4 + assert SensorScale.normalize("temperature", 150.0) == 150.0 + end + + test "rescales deci-degree readings (one /10 step)" do + # MikroTik mtxrGaugeValue on RB-series: 569 → 56.9 + assert SensorScale.normalize("temperature", 569.0) == 56.9 + end + + test "rescales centi-degree readings (two /10 steps)" do + # 5692 → 569.2 → 56.92 + assert SensorScale.normalize("temperature", 5692.0) == 56.92 + end + + test "still bails out after max iterations even on absurd inputs" do + # 5_000_000 / 10^3 = 5000 — three iterations, capped, returned as-is + assert SensorScale.normalize("temperature", 5_000_000.0) == 5000.0 + end + + test "treats nil as nil" do + assert SensorScale.normalize("temperature", nil) == nil + end + end + + describe "normalize/2 — non-temperature sensors" do + test "leaves voltage / current / power readings alone" do + assert SensorScale.normalize("voltage", 12.5) == 12.5 + assert SensorScale.normalize("current", 0.85) == 0.85 + assert SensorScale.normalize("power", 5.4) == 5.4 + # Even an implausibly large voltage stays untouched — only + # temperature gets the auto-scale heuristic. + assert SensorScale.normalize("voltage", 9999.0) == 9999.0 + end + end +end