From 50c213f6a09e09f7e2b1c6bf42caf47cfddb7afa Mon Sep 17 00:00:00 2001 From: Elin Olsson Date: Fri, 11 Oct 2024 09:59:56 +0200 Subject: [PATCH 1/4] Add metrics filtering to devices query --- lib/nerves_hub/devices.ex | 40 ++++++++++++++++++++++++ lib/nerves_hub/devices/device.ex | 2 ++ lib/nerves_hub_web/live/devices/index.ex | 28 +++++++++++++++-- 3 files changed, 67 insertions(+), 3 deletions(-) diff --git a/lib/nerves_hub/devices.ex b/lib/nerves_hub/devices.ex index dafc826b6..93d5ca9c0 100644 --- a/lib/nerves_hub/devices.ex +++ b/lib/nerves_hub/devices.ex @@ -18,6 +18,7 @@ defmodule NervesHub.Devices do alias NervesHub.Devices.Device alias NervesHub.Devices.DeviceCertificate alias NervesHub.Devices.DeviceHealth + alias NervesHub.Devices.DeviceMetric alias NervesHub.Devices.SharedSecretAuth alias NervesHub.Devices.InflightUpdate alias NervesHub.Devices.UpdatePayload @@ -174,6 +175,39 @@ defmodule NervesHub.Devices do defp sort_devices(sort), do: sort + def filter_on_metric(query, key, value, operator) do + query + |> where( + [d], + d.id in subquery(metrics_query(key, value, operator)) + ) + |> preload_metric(key) + end + + def metrics_query(key, value, operator) do + DeviceMetric + |> from + |> select([d], d.device_id) + |> where([dm], dm.key == ^key) + |> gt_or_lt(value, operator) + |> order_by(desc: :inserted_at) + |> distinct(:device_id) + end + + defp gt_or_lt(query, value, :gt), do: where(query, [dm], dm.value > ^value) + defp gt_or_lt(query, value, :lt), do: where(query, [dm], dm.value < ^value) + + defp preload_metric(query, key) do + preload_query = + DeviceMetric + |> distinct(:device_id) + |> where(key: ^key) + |> order_by([:device_id, desc: :inserted_at]) + + query + |> preload(device_metrics: ^preload_query) + end + defp filtering(query, filters) do Enum.reduce(filters, query, fn {key, value}, query -> case {key, value} do @@ -199,6 +233,12 @@ defmodule NervesHub.Devices do d.id in subquery(Connections.query_devices_with_connection_status(value)) ) + {:metrics, _} -> + query + + {:connection, _value} -> + where(query, [d], d.connection_status == ^String.to_atom(value)) + {:connection_type, value} -> where(query, [d], ^value in d.connection_types) diff --git a/lib/nerves_hub/devices/device.ex b/lib/nerves_hub/devices/device.ex index a12db4474..f769ba13f 100644 --- a/lib/nerves_hub/devices/device.ex +++ b/lib/nerves_hub/devices/device.ex @@ -6,6 +6,7 @@ defmodule NervesHub.Devices.Device do alias NervesHub.Accounts.Org alias NervesHub.Devices.DeviceCertificate alias NervesHub.Devices.DeviceConnection + alias NervesHub.Devices.DeviceMetric alias NervesHub.Deployments.Deployment alias NervesHub.Firmwares.FirmwareMetadata alias NervesHub.Products.Product @@ -40,6 +41,7 @@ defmodule NervesHub.Devices.Device do embeds_one(:firmware_metadata, FirmwareMetadata, on_replace: :update) has_many(:device_certificates, DeviceCertificate, on_delete: :delete_all) has_many(:device_connections, DeviceConnection, on_delete: :delete_all) + has_many(:device_metrics, DeviceMetric, on_delete: :delete_all) field(:identifier, :string) field(:description, :string) diff --git a/lib/nerves_hub_web/live/devices/index.ex b/lib/nerves_hub_web/live/devices/index.ex index a699eef96..604cd8fdb 100644 --- a/lib/nerves_hub_web/live/devices/index.ex +++ b/lib/nerves_hub_web/live/devices/index.ex @@ -30,7 +30,8 @@ defmodule NervesHubWeb.Live.Devices.Index do updates: "", has_no_tags: false, alarm_status: "", - alarm: "" + alarm: "", + metrics: %{} } @filter_types %{ @@ -44,7 +45,8 @@ defmodule NervesHubWeb.Live.Devices.Index do updates: :string, has_no_tags: :boolean, alarm_status: :string, - alarm: :string + alarm: :string, + metrics: :map } @default_page 1 @@ -88,7 +90,11 @@ defmodule NervesHubWeb.Live.Devices.Index do end def handle_params(unsigned_params, _uri, socket) do - filters = Map.merge(@default_filters, filter_changes(unsigned_params)) + filters = + Map.merge(@default_filters, filter_changes(unsigned_params)) + |> filter_on_metrics_if_provided(unsigned_params) + |> dbg() + pagination_opts = Map.merge(socket.assigns.paginate_opts, pagination_changes(unsigned_params)) socket @@ -102,6 +108,22 @@ defmodule NervesHubWeb.Live.Devices.Index do |> noreply() end + def filter_on_metrics_if_provided(filters, %{ + "metric" => metric_type, + "metric_value" => metric_value, + "metric_operator" => operator + }) do + Map.put(filters, :metrics, %{ + key: metric_type, + value: String.to_float(metric_value), + operator: String.to_existing_atom(operator) + }) + end + + def filter_on_metrics_if_provided(filters, _) do + filters + end + defp self_path(socket, extra) do params = Enum.into(stringify_keys(extra), socket.assigns.params) pagination = pagination_changes(params) From 79898f81a923c13e5e3f333254404aab9d4ace86 Mon Sep 17 00:00:00 2001 From: Elin Olsson Date: Fri, 15 Nov 2024 17:00:35 +0100 Subject: [PATCH 2/4] Fix broken query + add test for metrics filtering --- lib/nerves_hub/devices.ex | 32 +++++------------ lib/nerves_hub_web/live/devices/index.ex | 18 +++++----- .../live/devices/index_test.exs | 35 +++++++++++++++++++ 3 files changed, 52 insertions(+), 33 deletions(-) diff --git a/lib/nerves_hub/devices.ex b/lib/nerves_hub/devices.ex index 93d5ca9c0..d51a158bf 100644 --- a/lib/nerves_hub/devices.ex +++ b/lib/nerves_hub/devices.ex @@ -177,36 +177,20 @@ defmodule NervesHub.Devices do def filter_on_metric(query, key, value, operator) do query - |> where( - [d], - d.id in subquery(metrics_query(key, value, operator)) - ) - |> preload_metric(key) + |> join(:inner, [d], m in DeviceMetric, on: d.id == m.device_id) + |> where([_, m], m.inserted_at == subquery(latest_metric_for_key(key))) + |> where([d, m], m.key == ^key) + |> gt_or_lt(value, operator) end - def metrics_query(key, value, operator) do + defp latest_metric_for_key(key) do DeviceMetric - |> from - |> select([d], d.device_id) + |> select([dm], max(dm.inserted_at)) |> where([dm], dm.key == ^key) - |> gt_or_lt(value, operator) - |> order_by(desc: :inserted_at) - |> distinct(:device_id) end - defp gt_or_lt(query, value, :gt), do: where(query, [dm], dm.value > ^value) - defp gt_or_lt(query, value, :lt), do: where(query, [dm], dm.value < ^value) - - defp preload_metric(query, key) do - preload_query = - DeviceMetric - |> distinct(:device_id) - |> where(key: ^key) - |> order_by([:device_id, desc: :inserted_at]) - - query - |> preload(device_metrics: ^preload_query) - end + defp gt_or_lt(query, value, :gt), do: where(query, [_, dm], dm.value > ^value) + defp gt_or_lt(query, value, :lt), do: where(query, [_, dm], dm.value < ^value) defp filtering(query, filters) do Enum.reduce(filters, query, fn {key, value}, query -> diff --git a/lib/nerves_hub_web/live/devices/index.ex b/lib/nerves_hub_web/live/devices/index.ex index 604cd8fdb..d17f777b9 100644 --- a/lib/nerves_hub_web/live/devices/index.ex +++ b/lib/nerves_hub_web/live/devices/index.ex @@ -92,8 +92,10 @@ defmodule NervesHubWeb.Live.Devices.Index do def handle_params(unsigned_params, _uri, socket) do filters = Map.merge(@default_filters, filter_changes(unsigned_params)) + # Until UI selectors for metrics are implemented, + # params can be added to the URL and be merged with filters. + # Example: ?metric=cpu_temp&metric_operator=gt&metric_value=37.0 |> filter_on_metrics_if_provided(unsigned_params) - |> dbg() pagination_opts = Map.merge(socket.assigns.paginate_opts, pagination_changes(unsigned_params)) @@ -108,11 +110,11 @@ defmodule NervesHubWeb.Live.Devices.Index do |> noreply() end - def filter_on_metrics_if_provided(filters, %{ - "metric" => metric_type, - "metric_value" => metric_value, - "metric_operator" => operator - }) do + defp filter_on_metrics_if_provided(filters, %{ + "metric" => metric_type, + "metric_value" => metric_value, + "metric_operator" => operator + }) do Map.put(filters, :metrics, %{ key: metric_type, value: String.to_float(metric_value), @@ -120,9 +122,7 @@ defmodule NervesHubWeb.Live.Devices.Index do }) end - def filter_on_metrics_if_provided(filters, _) do - filters - end + defp filter_on_metrics_if_provided(filters, _), do: filters defp self_path(socket, extra) do params = Enum.into(stringify_keys(extra), socket.assigns.params) diff --git a/test/nerves_hub_web/live/devices/index_test.exs b/test/nerves_hub_web/live/devices/index_test.exs index 29d77cd34..74a5c54ea 100644 --- a/test/nerves_hub_web/live/devices/index_test.exs +++ b/test/nerves_hub_web/live/devices/index_test.exs @@ -1,6 +1,7 @@ defmodule NervesHubWeb.Live.Devices.IndexTest do use NervesHubWeb.ConnCase.Browser, async: false + alias NervesHub.Devices alias NervesHub.Fixtures alias NervesHubWeb.Endpoint @@ -85,6 +86,40 @@ defmodule NervesHubWeb.Live.Devices.IndexTest do device2.identifier end + test "filters devices by metrics", %{conn: conn, fixture: fixture} do + %{device: device, firmware: firmware, org: org, product: product} = fixture + + device2 = Fixtures.device_fixture(org, product, firmware, %{}) + {:ok, _view, html} = live(conn, device_index_path(fixture)) + assert html =~ device.identifier + assert html =~ device2.identifier + + # Add metrics for device2, sleep between to secure order. + Devices.Metrics.save_metric(%{device_id: device2.id, key: "cpu_temp", value: 36}) + :timer.sleep(100) + Devices.Metrics.save_metric(%{device_id: device2.id, key: "cpu_temp", value: 42}) + :timer.sleep(100) + Devices.Metrics.save_metric(%{device_id: device2.id, key: "load_1min", value: 3}) + + greater_than_filter = + device_index_path(fixture) <> "?metric=cpu_temp&metric_operator=gt&metric_value=37.0" + + {:ok, _view, html} = live(conn, greater_than_filter) + + # Show only show device2, which has a value greater than 37 on most recent cpu_temp metric. + assert html =~ device2.identifier + refute html =~ device.identifier + + less_than_filter = + device_index_path(fixture) <> "?metric=cpu_temp&metric_operator=lt&metric_value=37.0" + + {:ok, _view, html} = live(conn, less_than_filter) + + # Should not show any device since the query is for values less than 37 + refute html =~ device2.identifier + refute html =~ device.identifier + end + test "filters devices by several tags", %{conn: conn, fixture: fixture} do %{device: _device, firmware: firmware, org: org, product: product} = fixture From 929e5330b969b7b2b80787020b5b62024499e619 Mon Sep 17 00:00:00 2001 From: Elin Olsson Date: Mon, 18 Nov 2024 14:13:45 +0100 Subject: [PATCH 3/4] Add metrics filters to UI --- lib/nerves_hub/devices.ex | 51 +++++++++++-------- lib/nerves_hub_web/live/devices/index.ex | 28 +++------- .../live/devices/index.html.heex | 24 +++++++++ .../live/devices/index_test.exs | 30 ++++++----- 4 files changed, 80 insertions(+), 53 deletions(-) diff --git a/lib/nerves_hub/devices.ex b/lib/nerves_hub/devices.ex index d51a158bf..7a2b031e5 100644 --- a/lib/nerves_hub/devices.ex +++ b/lib/nerves_hub/devices.ex @@ -175,23 +175,6 @@ defmodule NervesHub.Devices do defp sort_devices(sort), do: sort - def filter_on_metric(query, key, value, operator) do - query - |> join(:inner, [d], m in DeviceMetric, on: d.id == m.device_id) - |> where([_, m], m.inserted_at == subquery(latest_metric_for_key(key))) - |> where([d, m], m.key == ^key) - |> gt_or_lt(value, operator) - end - - defp latest_metric_for_key(key) do - DeviceMetric - |> select([dm], max(dm.inserted_at)) - |> where([dm], dm.key == ^key) - end - - defp gt_or_lt(query, value, :gt), do: where(query, [_, dm], dm.value > ^value) - defp gt_or_lt(query, value, :lt), do: where(query, [_, dm], dm.value < ^value) - defp filtering(query, filters) do Enum.reduce(filters, query, fn {key, value}, query -> case {key, value} do @@ -217,9 +200,6 @@ defmodule NervesHub.Devices do d.id in subquery(Connections.query_devices_with_connection_status(value)) ) - {:metrics, _} -> - query - {:connection, _value} -> where(query, [d], d.connection_status == ^String.to_atom(value)) @@ -268,10 +248,41 @@ defmodule NervesHub.Devices do else query end + + {:metrics_value, _value} -> + filter_on_metric(query, filters) + + {_, _} -> + query end end) end + defp filter_on_metric( + query, + %{metrics_key: key, metrics_operator: operator, metrics_value: value} + ) + when key != "" do + {value_as_float, _} = Float.parse(value) + + query + |> join(:inner, [d], m in DeviceMetric, on: d.id == m.device_id) + |> where([_, m], m.inserted_at == subquery(latest_metric_for_key(key))) + |> where([d, m], m.key == ^key) + |> gt_or_lt(value_as_float, operator) + end + + defp filter_on_metric(query, _), do: query + + defp latest_metric_for_key(key) do + DeviceMetric + |> select([dm], max(dm.inserted_at)) + |> where([dm], dm.key == ^key) + end + + defp gt_or_lt(query, value, "gt"), do: where(query, [_, dm], dm.value > ^value) + defp gt_or_lt(query, value, "lt"), do: where(query, [_, dm], dm.value < ^value) + def get_device_count_by_org_id(org_id) do q = from( diff --git a/lib/nerves_hub_web/live/devices/index.ex b/lib/nerves_hub_web/live/devices/index.ex index d17f777b9..38a49c8bc 100644 --- a/lib/nerves_hub_web/live/devices/index.ex +++ b/lib/nerves_hub_web/live/devices/index.ex @@ -6,6 +6,7 @@ defmodule NervesHubWeb.Live.Devices.Index do alias NervesHub.AuditLogs alias NervesHub.Devices alias NervesHub.Devices.Alarms + alias NervesHub.Devices.Metrics alias NervesHub.Firmwares alias NervesHub.Products.Product alias NervesHub.Tracker @@ -31,7 +32,9 @@ defmodule NervesHubWeb.Live.Devices.Index do has_no_tags: false, alarm_status: "", alarm: "", - metrics: %{} + metrics_key: "", + metrics_operator: "gt", + metrics_value: "" } @filter_types %{ @@ -46,7 +49,9 @@ defmodule NervesHubWeb.Live.Devices.Index do has_no_tags: :boolean, alarm_status: :string, alarm: :string, - metrics: :map + metrics_key: :string, + metrics_operator: :string, + metrics_value: :string } @default_page 1 @@ -85,6 +90,7 @@ defmodule NervesHubWeb.Live.Devices.Index do |> assign(:device_tags, "") |> assign(:total_entries, 0) |> assign(:current_alarms, Alarms.get_current_alarm_types(product.id)) + |> assign(:metrics_keys, Metrics.default_metric_types()) |> subscribe_and_refresh_device_list_timer() |> ok() end @@ -92,10 +98,6 @@ defmodule NervesHubWeb.Live.Devices.Index do def handle_params(unsigned_params, _uri, socket) do filters = Map.merge(@default_filters, filter_changes(unsigned_params)) - # Until UI selectors for metrics are implemented, - # params can be added to the URL and be merged with filters. - # Example: ?metric=cpu_temp&metric_operator=gt&metric_value=37.0 - |> filter_on_metrics_if_provided(unsigned_params) pagination_opts = Map.merge(socket.assigns.paginate_opts, pagination_changes(unsigned_params)) @@ -110,20 +112,6 @@ defmodule NervesHubWeb.Live.Devices.Index do |> noreply() end - defp filter_on_metrics_if_provided(filters, %{ - "metric" => metric_type, - "metric_value" => metric_value, - "metric_operator" => operator - }) do - Map.put(filters, :metrics, %{ - key: metric_type, - value: String.to_float(metric_value), - operator: String.to_existing_atom(operator) - }) - end - - defp filter_on_metrics_if_provided(filters, _), do: filters - defp self_path(socket, extra) do params = Enum.into(stringify_keys(extra), socket.assigns.params) pagination = pagination_changes(params) diff --git a/lib/nerves_hub_web/live/devices/index.html.heex b/lib/nerves_hub_web/live/devices/index.html.heex index 82cede03d..3ee7b9660 100644 --- a/lib/nerves_hub_web/live/devices/index.html.heex +++ b/lib/nerves_hub_web/live/devices/index.html.heex @@ -155,6 +155,30 @@
+
+ +
+ +
+
+
+
+ + +
+
+
+ + +
diff --git a/test/nerves_hub_web/live/devices/index_test.exs b/test/nerves_hub_web/live/devices/index_test.exs index 74a5c54ea..cf3bead62 100644 --- a/test/nerves_hub_web/live/devices/index_test.exs +++ b/test/nerves_hub_web/live/devices/index_test.exs @@ -90,7 +90,7 @@ defmodule NervesHubWeb.Live.Devices.IndexTest do %{device: device, firmware: firmware, org: org, product: product} = fixture device2 = Fixtures.device_fixture(org, product, firmware, %{}) - {:ok, _view, html} = live(conn, device_index_path(fixture)) + {:ok, view, html} = live(conn, device_index_path(fixture)) assert html =~ device.identifier assert html =~ device2.identifier @@ -101,23 +101,27 @@ defmodule NervesHubWeb.Live.Devices.IndexTest do :timer.sleep(100) Devices.Metrics.save_metric(%{device_id: device2.id, key: "load_1min", value: 3}) - greater_than_filter = - device_index_path(fixture) <> "?metric=cpu_temp&metric_operator=gt&metric_value=37.0" - - {:ok, _view, html} = live(conn, greater_than_filter) + change = + render_change(view, "update-filters", %{ + "metrics_key" => "cpu_temp", + "metrics_operator" => "gt", + "metrics_value" => "37" + }) # Show only show device2, which has a value greater than 37 on most recent cpu_temp metric. - assert html =~ device2.identifier - refute html =~ device.identifier - - less_than_filter = - device_index_path(fixture) <> "?metric=cpu_temp&metric_operator=lt&metric_value=37.0" + assert change =~ device2.identifier + refute change =~ device.identifier - {:ok, _view, html} = live(conn, less_than_filter) + change2 = + render_change(view, "update-filters", %{ + "metrics_key" => "cpu_temp", + "metrics_operator" => "lt", + "metrics_value" => "37" + }) # Should not show any device since the query is for values less than 37 - refute html =~ device2.identifier - refute html =~ device.identifier + refute change2 =~ device2.identifier + refute change2 =~ device.identifier end test "filters devices by several tags", %{conn: conn, fixture: fixture} do From 4ac3549dc75dfb20a26311d81e4d322a6c84b6c5 Mon Sep 17 00:00:00 2001 From: Elin Olsson Date: Mon, 18 Nov 2024 15:27:22 +0100 Subject: [PATCH 4/4] Remove deprecated connection filter --- lib/nerves_hub/devices.ex | 3 --- 1 file changed, 3 deletions(-) diff --git a/lib/nerves_hub/devices.ex b/lib/nerves_hub/devices.ex index 7a2b031e5..30b79d7ff 100644 --- a/lib/nerves_hub/devices.ex +++ b/lib/nerves_hub/devices.ex @@ -200,9 +200,6 @@ defmodule NervesHub.Devices do d.id in subquery(Connections.query_devices_with_connection_status(value)) ) - {:connection, _value} -> - where(query, [d], d.connection_status == ^String.to_atom(value)) - {:connection_type, value} -> where(query, [d], ^value in d.connection_types)