Improve metrics for broken routes (#321)

* Improve metrics for broken routes

* Remove record skfs

Author: macpie

include/hpr_metrics.hrl

@@ -1,7 +1,7 @@
 -ifdef(TEST).
 -define(METRICS_TICK_INTERVAL, timer:seconds(1)).
 -else.
--define(METRICS_TICK_INTERVAL, timer:seconds(10)).
+-define(METRICS_TICK_INTERVAL, timer:seconds(30)).
 -endif.
 -define(METRICS_TICK, '_hpr_metrics_tick').
 
@@ -13,7 +13,7 @@
 -define(METRICS_ROUTES_GAUGE, "hpr_routes_gauge").
 -define(METRICS_EUI_PAIRS_GAUGE, "hpr_eui_pairs_gauge").
 -define(METRICS_SKFS_GAUGE, "hpr_skfs_gauge").
--define(METRICS_WEIRD_ROUTES_GAUGE, "hpr_weird_routes_gauge").
+-define(METRICS_BROKEN_ROUTES_GAUGE, "hpr_broken_routes_gauge").
 -define(METRICS_PACKET_REPORT_HISTOGRAM, "hpr_packet_report_histogram").
 -define(METRICS_MULTI_BUY_GET_HISTOGRAM, "hpr_multi_buy_get_histogram").
 -define(METRICS_FIND_ROUTES_HISTOGRAM, "hpr_find_routes_histogram").
@@ -36,7 +36,7 @@
     {?METRICS_ROUTES_GAUGE, prometheus_gauge, [], "Number of Routes"},
     {?METRICS_EUI_PAIRS_GAUGE, prometheus_gauge, [], "Number of EUI Pairs"},
     {?METRICS_SKFS_GAUGE, prometheus_gauge, [], "Number of SKFs"},
-    {?METRICS_WEIRD_ROUTES_GAUGE, prometheus_gauge, [], "Number of weird routes"},
+    {?METRICS_BROKEN_ROUTES_GAUGE, prometheus_gauge, [oui], "Number of broken routes"},
     {?METRICS_PACKET_REPORT_HISTOGRAM, prometheus_histogram, [status], "Packet Reports"},
     {?METRICS_MULTI_BUY_GET_HISTOGRAM, prometheus_histogram, [status], "Multi Buy Service Get"},
     {?METRICS_FIND_ROUTES_HISTOGRAM, prometheus_histogram, [], "Find Routes"},

src/cli/hpr_cli_config.erl

@@ -57,6 +57,7 @@ config_usage() ->
             "    [--display_skfs] default: false (SKFs not included)\n",
             "config route refresh_all                    - Refresh all routes\n",
             "    [--minimum] default: 1 (need a minimum of 1 SKFs ro be updated)\n",
+            "config route refresh_broken                 - Refresh broken routes\n",
             "config route refresh <route_id>             - Refresh route's EUIs, SKFs, DevAddrRanges\n",
             "config route activate <route_id>            - Activate route\n",
             "config route deactivate <route_id>          - Deactivate route\n",
@@ -103,6 +104,12 @@ config_cmd() ->
             ],
             fun config_route_refresh_all/3
         ],
+        [
+            ["config", "route", "refresh_broken"],
+            [],
+            [],
+            fun config_route_refresh_broken/3
+        ],
         [
             ["config", "route", "refresh", '*'],
             [],
@@ -299,44 +306,78 @@ config_route_refresh_all(["config", "route", "refresh_all"], [], Flags) ->
         RouteIDs = [ID || {ID, _} <- Sorted],
         Total = erlang:length(RouteIDs),
         lager:info("Found ~p routes to update", [Total]),
-        route_refresh_all(
-            Total, RouteIDs, backoff:init(timer:seconds(1), timer:minutes(1))
-        )
+        routes_refresh(Total, RouteIDs)
     end),
-    c_text("command spawned @ ~p, look at logs and tail hpr_cli_config", [Pid]);
+    c_text(
+        "command spawned @ ~p, look at logs and `tail -F /opt/hpr/log/info.log | grep hpr_cli_config`",
+        [Pid]
+    );
 config_route_refresh_all(_, _, _) ->
     usage.
 
-route_refresh_all(_Total, [], _Backoff0) ->
+config_route_refresh_broken(["config", "route", "refresh_broken"], [], _Flags) ->
+    Pid = erlang:spawn(fun() ->
+        RouteIDsWithDevAddr =
+            hpr_devaddr_range_storage:foldl(
+                fun({_, RouteID}, Acc) ->
+                    sets:add_element(RouteID, Acc)
+                end,
+                sets:new()
+            ),
+        RouteIDs = hpr_route_storage:foldl(
+            fun(RouteETS, RouteIDs) ->
+                SKFCount =
+                    case ets:info(hpr_route_ets:skf_ets(RouteETS), size) of
+                        undefined -> 0;
+                        N -> N
+                    end,
+                Route = hpr_route_ets:route(RouteETS),
+                RouteID = hpr_route:id(Route),
+                case SKFCount > 0 andalso not sets:is_element(RouteID, RouteIDsWithDevAddr) of
+                    true ->
+                        lager:warning(
+                            [{route_id, RouteID}, {oui, hpr_route:oui(Route)}],
+                            "BROKEN_ROUTES route has no devaddr ranges but has (~p) skfs",
+                            [SKFCount]
+                        ),
+                        [RouteID | RouteIDs];
+                    false ->
+                        RouteIDs
+                end
+            end,
+            []
+        ),
+        Total = erlang:length(RouteIDs),
+        lager:info("Found ~p routes to fix", [Total]),
+        routes_refresh(Total, RouteIDs)
+    end),
+    c_text(
+        "command spawned @ ~p, look at logs and `tail -F /opt/hpr/log/info.log | grep hpr_cli_config`",
+        [Pid]
+    );
+config_route_refresh_broken(_, _, _) ->
+    usage.
+
+routes_refresh(_Total, []) ->
     lager:info("All done!");
-route_refresh_all(Total, [RouteID | T] = RouteIDs, Backoff0) ->
+routes_refresh(Total, [RouteID | RouteIDs]) ->
     CurrTotal = erlang:length(RouteIDs),
     IDX = Total - CurrTotal + 1,
     lager:info("~p/~p===== ~s =====", [
         IDX, Total, RouteID
     ]),
     Start = erlang:system_time(millisecond),
-    case try_refresh_route(RouteID) of
-        ok ->
-            End = erlang:system_time(millisecond),
-            lager:info("took ~pms", [End - Start]),
-            {_, Backoff1} = backoff:succeed(Backoff0),
-            route_refresh_all(Total, T, Backoff1);
-        error ->
-            End = erlang:system_time(millisecond),
-            lager:info("took ~pms", [End - Start]),
-            {Delay, Backoff1} = backoff:fail(Backoff0),
-            lager:info("sleeping ~pms", [Delay]),
-            timer:sleep(Delay),
-            route_refresh_all(Total, RouteIDs, Backoff1)
-    end,
-    ok.
-
-try_refresh_route(RouteID) ->
-    try hpr_route_stream_worker:refresh_route(RouteID) of
+    try hpr_route_stream_worker:refresh_route(RouteID, 3) of
         {ok, Map} ->
             lager:info("| Type | Before  |  After  | Removed |  Added  |"),
             lager:info("|------|---------|---------|---------|---------|"),
+            lager:info("| ~4w | ~7w | ~7w | ~7w | ~7w |", [
+                addr,
+                maps:get(devaddr_before, Map),
+                maps:get(devaddr_after, Map),
+                maps:get(devaddr_removed, Map),
+                maps:get(devaddr_added, Map)
+            ]),
             lager:info("| ~4w | ~7w | ~7w | ~7w | ~7w |", [
                 eui,
                 maps:get(eui_before, Map),
@@ -350,23 +391,16 @@ try_refresh_route(RouteID) ->
                 maps:get(skf_after, Map),
                 maps:get(skf_removed, Map),
                 maps:get(skf_added, Map)
-            ]),
-            lager:info("| ~4w | ~7w | ~7w | ~7w | ~7w |", [
-                addr,
-                maps:get(devaddr_before, Map),
-                maps:get(devaddr_after, Map),
-                maps:get(devaddr_removed, Map),
-                maps:get(devaddr_added, Map)
-            ]),
-            ok;
+            ]);
         {error, _R} ->
-            lager:info("ERROR ~p", [_R]),
-            error
+            lager:error("error ~p", [_R])
     catch
         _E:_R ->
-            lager:info("CRASHED ~p", [_R]),
-            error
-    end.
+            lager:critical("crashed ~p", [_R])
+    end,
+    End = erlang:system_time(millisecond),
+    lager:info("took ~pms", [End - Start]),
+    routes_refresh(Total, RouteIDs).
 
 config_route_refresh(["config", "route", "refresh", RouteID], [], _Flags) ->
     case hpr_route_stream_worker:refresh_route(RouteID, 3) of

src/grpc/iot_config/hpr_devaddr_range_storage.erl

@@ -4,6 +4,7 @@
     init_ets/0,
     checkpoint/0,
 
+    foldl/2,
     lookup/1,
     insert/1,
     delete/1,
@@ -38,6 +39,10 @@ checkpoint() ->
         ok = dets:from_ets(?DETS, ?ETS)
     end).
 
+-spec foldl(Fun :: function(), Acc :: any()) -> any().
+foldl(Fun, Acc) ->
+    ets:foldl(Fun, Acc, ?ETS).
+
 -spec lookup(DevAddr :: non_neg_integer()) -> [hpr_route_ets:route()].
 lookup(DevAddr) ->
     MS = [

src/grpc/iot_config/hpr_route_storage.erl

@@ -4,6 +4,7 @@
     init_ets/0,
     checkpoint/0,
 
+    foldl/2,
     insert/1, insert/2, insert/3,
     delete/1,
     lookup/1,
@@ -60,6 +61,10 @@ lookup(ID) ->
             {error, not_found}
     end.
 
+-spec foldl(Fun :: function(), Acc :: any()) -> any().
+foldl(Fun, Acc) ->
+    ets:foldl(Fun, Acc, ?ETS).
+
 -spec insert(Route :: hpr_route:route()) -> ok.
 insert(Route) ->
     RouteID = hpr_route:id(Route),

src/metrics/hpr_metrics.erl

@@ -186,8 +186,6 @@ handle_info(?METRICS_TICK, State) ->
         fun record_grpc_connections/0,
         fun record_routes/0,
         fun record_eui_pairs/0,
-        fun record_skfs/0,
-        fun record_weird_routes/0,
         fun record_ets/0,
         fun record_queues/0,
         fun record_devices/0
@@ -246,12 +244,47 @@ declare_metrics() ->
 
 -spec record_routes() -> ok.
 record_routes() ->
-    case ets:info(hpr_routes_ets, size) of
-        undefined ->
-            _ = prometheus_gauge:set(?METRICS_ROUTES_GAUGE, [], 0);
-        N ->
-            _ = prometheus_gauge:set(?METRICS_ROUTES_GAUGE, [], N)
-    end,
+    RouteIDsWithDevAddr =
+        hpr_devaddr_range_storage:foldl(
+            fun({_, RouteID}, Acc) ->
+                sets:add_element(RouteID, Acc)
+            end,
+            sets:new()
+        ),
+    {RoutesCount, SKFsCount, BrokenMap} = hpr_route_storage:foldl(
+        fun(RouteETS, {RoutesCount, SKFsCount, BrokenMap}) ->
+            SKFCount =
+                case ets:info(hpr_route_ets:skf_ets(RouteETS), size) of
+                    undefined -> 0;
+                    N -> N
+                end,
+            Route = hpr_route_ets:route(RouteETS),
+            RouteID = hpr_route:id(Route),
+            OUI = hpr_route:oui(Route),
+            NewBrokenMap =
+                case SKFCount > 0 andalso not sets:is_element(RouteID, RouteIDsWithDevAddr) of
+                    true ->
+                        lager:warning(
+                            [{route_id, RouteID}, {oui, OUI}],
+                            "BROKEN_ROUTES route has no devaddr ranges but has (~p) skfs",
+                            [SKFCount]
+                        ),
+                        maps:update_with(OUI, fun(Count) -> Count + 1 end, 1, BrokenMap);
+                    false ->
+                        BrokenMap
+                end,
+            {RoutesCount + 1, SKFsCount + SKFCount, NewBrokenMap}
+        end,
+        {0, 0, #{}}
+    ),
+    _ = prometheus_gauge:set(?METRICS_ROUTES_GAUGE, [], RoutesCount),
+    _ = prometheus_gauge:set(?METRICS_SKFS_GAUGE, [], SKFsCount),
+    maps:foreach(
+        fun(OUI, Count) ->
+            _ = prometheus_gauge:set(?METRICS_BROKEN_ROUTES_GAUGE, [OUI], Count)
+        end,
+        BrokenMap
+    ),
     ok.
 
 -spec record_eui_pairs() -> ok.
@@ -264,51 +297,6 @@ record_eui_pairs() ->
     end,
     ok.
 
--spec record_skfs() -> ok.
-record_skfs() ->
-    Count = lists:foldl(
-        fun(RouteETS, Acc) ->
-            case ets:info(hpr_route_ets:skf_ets(RouteETS), size) of
-                undefined -> Acc;
-                N -> N + Acc
-            end
-        end,
-        0,
-        ets:tab2list(hpr_routes_ets)
-    ),
-    _ = prometheus_gauge:set(?METRICS_SKFS_GAUGE, [], Count),
-    ok.
-
--spec record_weird_routes() -> ok.
-record_weird_routes() ->
-    Count = lists:foldl(
-        fun(RouteETS, Acc) ->
-            Route = hpr_route_ets:route(RouteETS),
-            RouteID = hpr_route:id(Route),
-            SKFCount =
-                case ets:info(hpr_route_ets:skf_ets(RouteETS), size) of
-                    undefined -> 0;
-                    N -> N
-                end,
-            DevAddrRangesCount = hpr_devaddr_range_storage:count_for_route(RouteID),
-            case SKFCount > 0 andalso DevAddrRangesCount == 0 of
-                true ->
-                    lager:critical(
-                        [{route_id, RouteID}, {oui, hpr_route:oui(Route)}],
-                        "route has no devaddr ranges but has (~p) skfs",
-                        [SKFCount]
-                    ),
-                    Acc + 1;
-                false ->
-                    Acc
-            end
-        end,
-        0,
-        ets:tab2list(hpr_routes_ets)
-    ),
-    _ = prometheus_gauge:set(?METRICS_WEIRD_ROUTES_GAUGE, [], Count),
-    ok.
-
 -spec record_grpc_connections() -> ok.
 record_grpc_connections() ->
     Opts = application:get_env(grpcbox, listen_opts, #{}),