From 7bc69b129ab16ef70888c2e492c2c344e97f16f7 Mon Sep 17 00:00:00 2001
From: Shawn <506895667@qq.com>
Date: Sat, 25 Sep 2021 21:22:31 +0800
Subject: [PATCH] fix(rules): update test cases for rule metrics

---
 .../src/emqx_rule_metrics.erl                 | 12 +--
 .../test/emqx_rule_metrics_SUITE.erl          | 89 ++-----------------
 2 files changed, 9 insertions(+), 92 deletions(-)

diff --git a/apps/emqx_rule_engine/src/emqx_rule_metrics.erl b/apps/emqx_rule_engine/src/emqx_rule_metrics.erl
index 874514a03..990911801 100644
--- a/apps/emqx_rule_engine/src/emqx_rule_metrics.erl
+++ b/apps/emqx_rule_engine/src/emqx_rule_metrics.erl
@@ -28,10 +28,6 @@
 -export([ get_rules_matched/1
         ]).
 
--export([ inc_rules_matched/1
-        , inc_rules_matched/2
-        ]).
-
 -export([ inc/2
         , inc/3
         , get/2
@@ -129,11 +125,6 @@ inc(Id, Metric, Val) ->
             counters:add(Ref, metrics_idx(Metric), Val)
     end.
 
-inc_rules_matched(Id) ->
-    inc_rules_matched(Id, 1).
-inc_rules_matched(Id, Val) ->
-    inc(Id, 'rules.matched', Val).
-
 get_rules_matched(Id) ->
     get(Id, 'rules.matched').
 
@@ -211,8 +202,9 @@ stop() ->
 create_counters(Id) ->
     case get_couters_ref(Id) of
         not_found ->
+            Counters = get_all_counters(),
             CntrRef = counters:new(max_counters_size(), [write_concurrency]),
-            persistent_term:put(?CntrRef, #{Id => CntrRef});
+            persistent_term:put(?CntrRef, Counters#{Id => CntrRef});
         _Ref -> ok
     end.
 
diff --git a/apps/emqx_rule_engine/test/emqx_rule_metrics_SUITE.erl b/apps/emqx_rule_engine/test/emqx_rule_metrics_SUITE.erl
index e7c543c91..ff654ba94 100644
--- a/apps/emqx_rule_engine/test/emqx_rule_metrics_SUITE.erl
+++ b/apps/emqx_rule_engine/test/emqx_rule_metrics_SUITE.erl
@@ -31,11 +31,8 @@ suite() ->
 
 groups() ->
     [{metrics, [sequence],
-        [ t_action
-        , t_rule
-        , t_clear
+        [ t_rule
         , t_no_creation_1
-        , t_no_creation_2
         ]},
     {speed, [sequence],
         [ rule_speed
@@ -55,59 +52,27 @@ end_per_suite(_Config) ->
 init_per_testcase(_, Config) ->
     catch emqx_rule_metrics:stop(),
     {ok, _} = emqx_rule_metrics:start_link(),
-    [emqx_metrics:set(M, 0) || M <- emqx_rule_metrics:overall_metrics()],
     Config.
 
 end_per_testcase(_, _Config) ->
     ok.
 
 t_no_creation_1(_) ->
-    ?assertEqual(ok, emqx_rule_metrics:inc_rules_matched(<<"rule1">>)).
-
-t_no_creation_2(_) ->
-    ?assertEqual(ok, emqx_rule_metrics:inc_actions_taken(<<"action:0">>)).
-
-t_action(_) ->
-    ?assertEqual(0, emqx_rule_metrics:get_actions_taken(<<"action:1">>)),
-    ?assertEqual(0, emqx_rule_metrics:get_actions_exception(<<"action:1">>)),
-    ?assertEqual(0, emqx_rule_metrics:get_actions_taken(<<"action:2">>)),
-    ok = emqx_rule_metrics:create_metrics(<<"action:1">>),
-    ok = emqx_rule_metrics:create_metrics(<<"action:2">>),
-    ok = emqx_rule_metrics:inc_actions_taken(<<"action:1">>),
-    ok = emqx_rule_metrics:inc_actions_exception(<<"action:1">>),
-    ok = emqx_rule_metrics:inc_actions_taken(<<"action:2">>),
-    ok = emqx_rule_metrics:inc_actions_taken(<<"action:2">>),
-    ?assertEqual(1, emqx_rule_metrics:get_actions_taken(<<"action:1">>)),
-    ?assertEqual(1, emqx_rule_metrics:get_actions_exception(<<"action:1">>)),
-    ?assertEqual(2, emqx_rule_metrics:get_actions_taken(<<"action:2">>)),
-    ?assertEqual(0, emqx_rule_metrics:get_actions_taken(<<"action:3">>)),
-    ?assertEqual(3, emqx_rule_metrics:get_overall('actions.taken')),
-    ?assertEqual(1, emqx_rule_metrics:get_overall('actions.exception')),
-    ok = emqx_rule_metrics:clear_metrics(<<"action:1">>),
-    ok = emqx_rule_metrics:clear_metrics(<<"action:2">>),
-    ?assertEqual(0, emqx_rule_metrics:get_actions_taken(<<"action:1">>)),
-    ?assertEqual(0, emqx_rule_metrics:get_actions_taken(<<"action:2">>)).
+    ?assertEqual(ok, emqx_rule_metrics:inc(<<"rule1">>, 'rules.matched')).
 
 t_rule(_) ->
-    ok = emqx_rule_metrics:create_rule_metrics(<<"rule:1">>),
+    ok = emqx_rule_metrics:create_rule_metrics(<<"rule1">>),
     ok = emqx_rule_metrics:create_rule_metrics(<<"rule2">>),
-    ok = emqx_rule_metrics:inc(<<"rule:1">>, 'rules.matched'),
+    ok = emqx_rule_metrics:inc(<<"rule1">>, 'rules.matched'),
     ok = emqx_rule_metrics:inc(<<"rule2">>, 'rules.matched'),
     ok = emqx_rule_metrics:inc(<<"rule2">>, 'rules.matched'),
-    ?assertEqual(1, emqx_rule_metrics:get(<<"rule:1">>, 'rules.matched')),
+    ct:pal("----couters: ---~p", [persistent_term:get(emqx_rule_metrics)]),
+    ?assertEqual(1, emqx_rule_metrics:get(<<"rule1">>, 'rules.matched')),
     ?assertEqual(2, emqx_rule_metrics:get(<<"rule2">>, 'rules.matched')),
     ?assertEqual(0, emqx_rule_metrics:get(<<"rule3">>, 'rules.matched')),
-    ?assertEqual(3, emqx_rule_metrics:get_overall('rules.matched')),
-    ok = emqx_rule_metrics:clear_rule_metrics(<<"rule:1">>),
+    ok = emqx_rule_metrics:clear_rule_metrics(<<"rule1">>),
     ok = emqx_rule_metrics:clear_rule_metrics(<<"rule2">>).
 
-t_clear(_) ->
-    ok = emqx_rule_metrics:create_metrics(<<"action:1">>),
-    ok = emqx_rule_metrics:inc_actions_taken(<<"action:1">>),
-    ?assertEqual(1, emqx_rule_metrics:get_actions_taken(<<"action:1">>)),
-    ok = emqx_rule_metrics:clear_metrics(<<"action:1">>),
-    ?assertEqual(0, emqx_rule_metrics:get_actions_taken(<<"action:1">>)).
-
 rule_speed(_) ->
     ok = emqx_rule_metrics:create_rule_metrics(<<"rule1">>),
     ok = emqx_rule_metrics:create_rule_metrics(<<"rule:2">>),
@@ -119,51 +84,11 @@ rule_speed(_) ->
     ?LET(#{max := Max, current := Current}, emqx_rule_metrics:get_rule_speed(<<"rule1">>),
          {?assert(Max =< 2),
           ?assert(Current =< 2)}),
-    ct:pal("===== Speed: ~p~n", [emqx_rule_metrics:get_overall_rule_speed()]),
-    ?LET(#{max := Max, current := Current}, emqx_rule_metrics:get_overall_rule_speed(),
-         {?assert(Max =< 3),
-          ?assert(Current =< 3)}),
     ct:sleep(2100),
     ?LET(#{max := Max, current := Current, last5m := Last5Min}, emqx_rule_metrics:get_rule_speed(<<"rule1">>),
          {?assert(Max =< 2),
           ?assert(Current == 0),
           ?assert(Last5Min =< 0.67)}),
-    ?LET(#{max := Max, current := Current, last5m := Last5Min}, emqx_rule_metrics:get_overall_rule_speed(),
-         {?assert(Max =< 3),
-          ?assert(Current == 0),
-          ?assert(Last5Min =< 1)}),
     ct:sleep(3000),
-    ?LET(#{max := Max, current := Current, last5m := Last5Min}, emqx_rule_metrics:get_overall_rule_speed(),
-         {?assert(Max =< 3),
-          ?assert(Current == 0),
-          ?assert(Last5Min == 0)}),
     ok = emqx_rule_metrics:clear_rule_metrics(<<"rule1">>),
     ok = emqx_rule_metrics:clear_rule_metrics(<<"rule:2">>).
-
-% t_create(_) ->
-%     error('TODO').
-
-% t_get(_) ->
-%     error('TODO').
-
-% t_get_overall(_) ->
-%     error('TODO').
-
-% t_get_rule_speed(_) ->
-%     error('TODO').
-
-% t_get_overall_rule_speed(_) ->
-%     error('TODO').
-
-% t_get_rule_metrics(_) ->
-%     error('TODO').
-
-% t_get_action_metrics(_) ->
-%     error('TODO').
-
-% t_inc(_) ->
-%     error('TODO').
-
-% t_overall_metrics(_) ->
-%     error('TODO').
-