From 338b11ab95f879116a90dcd70415c06dcec00ed0 Mon Sep 17 00:00:00 2001 From: Shawn <506895667@qq.com> Date: Mon, 10 Oct 2022 17:11:57 +0800 Subject: [PATCH 1/2] fix: cannot reset metrics for fallback actions --- .../src/emqx_rule_metrics.erl | 13 ++- .../test/emqx_rule_engine_SUITE.erl | 79 +++++++++++++++---- 2 files changed, 73 insertions(+), 19 deletions(-) diff --git a/apps/emqx_rule_engine/src/emqx_rule_metrics.erl b/apps/emqx_rule_engine/src/emqx_rule_metrics.erl index 624032056..2e30f40dc 100644 --- a/apps/emqx_rule_engine/src/emqx_rule_metrics.erl +++ b/apps/emqx_rule_engine/src/emqx_rule_metrics.erl @@ -131,16 +131,15 @@ clear_metrics(Id) -> -spec(reset_metrics(rule_id()) -> ok). reset_metrics(Id) -> reset_speeds(Id), - reset_metrics(Id, rule_metrics()), + do_reset_metrics(Id, rule_metrics()), case emqx_rule_registry:get_rule(Id) of not_found -> ok; {ok, #rule{actions = Actions}} -> - [ reset_metrics(ActionId, action_metrics()) - || #action_instance{ id = ActionId} <- Actions], + reset_action_metrics(Actions), ok end. -reset_metrics(Id, Metrics) -> +do_reset_metrics(Id, Metrics) -> case couters_ref(Id) of not_found -> ok; Ref -> [counters:put(Ref, metrics_idx(Idx), 0) @@ -148,6 +147,12 @@ reset_metrics(Id, Metrics) -> ok end. +reset_action_metrics(Actions) -> + lists:foreach(fun(#action_instance{id = ActionId, fallbacks = FallbackActions}) -> + do_reset_metrics(ActionId, action_metrics()), + reset_action_metrics(FallbackActions) + end, Actions). + reset_speeds(Id) -> gen_server:call(?MODULE, {reset_speeds, Id}). diff --git a/apps/emqx_rule_engine/test/emqx_rule_engine_SUITE.erl b/apps/emqx_rule_engine/test/emqx_rule_engine_SUITE.erl index c8f66ea00..ec9717a75 100644 --- a/apps/emqx_rule_engine/test/emqx_rule_engine_SUITE.erl +++ b/apps/emqx_rule_engine/test/emqx_rule_engine_SUITE.erl @@ -50,6 +50,7 @@ groups() -> t_unregister_provider, t_create_rule, t_reset_metrics, + t_reset_metrics_fallbacks, t_create_resource ]}, {actions, [], @@ -379,18 +380,14 @@ t_inspect_action(_Config) -> t_reset_metrics(_Config) -> ok = emqx_rule_engine:load_providers(), - {ok, #resource{id = ResId}} = emqx_rule_engine:create_resource( - #{type => built_in, - config => #{}, - description => <<"debug resource">>}), - {ok, #rule{id = Id}} = emqx_rule_engine:create_rule( - #{rawsql => "select clientid as c, username as u " - "from \"t1\" ", - actions => [#{name => 'inspect', - args => #{'$resource' => ResId, a=>1, b=>2}}], - type => built_in, - description => <<"Inspect rule">> - }), + {ok, #rule{id = Id, actions = [#action_instance{id = ActId0}]}} = + emqx_rule_engine:create_rule( + #{rawsql => "select clientid as c, username as u " + "from \"t1\" ", + actions => [#{name => 'inspect', args => #{a=>1, b=>2}}], + type => built_in, + description => <<"Inspect rule">> + }), {ok, Client} = emqtt:start_link([{username, <<"emqx">>}]), {ok, _} = emqtt:connect(Client), [ begin @@ -398,16 +395,68 @@ t_reset_metrics(_Config) -> timer:sleep(100) end || _ <- lists:seq(1,10)], + ?assertMatch(#{exception := 0, failed := 0, + matched := 10, no_result := 0, passed := 10}, + emqx_rule_metrics:get_rule_metrics(Id)), + ?assertMatch(#{failed := 0, success := 10, taken := 10}, + emqx_rule_metrics:get_action_metrics(ActId0)), emqx_rule_metrics:reset_metrics(Id), ?assertEqual(#{exception => 0,failed => 0, matched => 0,no_result => 0,passed => 0, speed => 0.0,speed_last5m => 0.0,speed_max => 0.0}, emqx_rule_metrics:get_rule_metrics(Id)), - ?assertEqual(#{failed => 0,success => 0,taken => 0}, - emqx_rule_metrics:get_action_metrics(ResId)), + ?assertEqual(#{failed => 0, success => 0, taken => 0}, + emqx_rule_metrics:get_action_metrics(ActId0)), emqtt:stop(Client), emqx_rule_registry:remove_rule(Id), - emqx_rule_registry:remove_resource(ResId), + ok. + +t_reset_metrics_fallbacks(_Config) -> + ok = emqx_rule_engine:load_providers(), + ok = emqx_rule_registry:add_action( + #action{name = 'crash_action', app = ?APP, + module = ?MODULE, on_create = crash_action, + types=[], params_spec = #{}, + title = #{en => <<"Crash Action">>}, + description = #{en => <<"This action will always fail!">>}}), + {ok, #rule{id = Id, actions = [#action_instance{id = ActId0, fallbacks = [ + #action_instance{id = ActId1}, + #action_instance{id = ActId2} + ]}]}} = + emqx_rule_engine:create_rule( + #{rawsql => "select clientid as c, username as u " + "from \"t1\" ", + actions => [#{name => 'crash_action', args => #{a=>1, b=>2}, fallbacks => [ + #{name => 'inspect', args => #{}, fallbacks => []}, + #{name => 'inspect', args => #{}, fallbacks => []} + ]}], + type => built_in, + description => <<"Inspect rule">> + }), + {ok, Client} = emqtt:start_link([{username, <<"emqx">>}]), + {ok, _} = emqtt:connect(Client), + [ begin + emqtt:publish(Client, <<"t1">>, <<"{\"id\": 1, \"name\": \"ha\"}">>, 0), + timer:sleep(100) + end + || _ <- lists:seq(1,10)], + ?assertMatch(#{exception := 0, failed := 0, + matched := 10, no_result := 0, passed := 10}, + emqx_rule_metrics:get_rule_metrics(Id)), + [?assertMatch(#{failed := 10, success := 0, taken := 10}, + emqx_rule_metrics:get_action_metrics(AId)) || AId <- [ActId0]], + [?assertMatch(#{failed := 0, success := 10, taken := 10}, + emqx_rule_metrics:get_action_metrics(AId)) || AId <- [ ActId1, ActId2]], + emqx_rule_metrics:reset_metrics(Id), + ?assertEqual(#{exception => 0,failed => 0, + matched => 0,no_result => 0,passed => 0, + speed => 0.0,speed_last5m => 0.0,speed_max => 0.0}, + emqx_rule_metrics:get_rule_metrics(Id)), + [?assertEqual(#{failed => 0, success => 0, taken => 0}, + emqx_rule_metrics:get_action_metrics(AId)) || AId <- [ActId0, ActId1, ActId2]], + emqtt:stop(Client), + emqx_rule_registry:remove_rule(Id), + ok = emqx_rule_registry:remove_action('crash_action'), ok. t_republish_action(_Config) -> From 6d52f908d1689a2fe2a4636cec4e7e19a05ed5bc Mon Sep 17 00:00:00 2001 From: Shawn <506895667@qq.com> Date: Mon, 10 Oct 2022 17:17:21 +0800 Subject: [PATCH 2/2] chore: update emqx_rule_engine.appup.src --- CHANGES-4.3.md | 2 ++ .../src/emqx_rule_engine.appup.src | 14 ++++++++++++++ 2 files changed, 16 insertions(+) diff --git a/CHANGES-4.3.md b/CHANGES-4.3.md index 22a909780..7762f5eb1 100644 --- a/CHANGES-4.3.md +++ b/CHANGES-4.3.md @@ -71,6 +71,8 @@ File format: subscriber from another node in the cluster. Fixed in [#9122](https://github.com/emqx/emqx/pull/9122) +- Fix cannot reset metrics for fallback actions. [#9125](https://github.com/emqx/emqx/pull/9125) + ## v4.3.20 ### Bug fixes diff --git a/apps/emqx_rule_engine/src/emqx_rule_engine.appup.src b/apps/emqx_rule_engine/src/emqx_rule_engine.appup.src index 6aa35d2d6..130e5d4d4 100644 --- a/apps/emqx_rule_engine/src/emqx_rule_engine.appup.src +++ b/apps/emqx_rule_engine/src/emqx_rule_engine.appup.src @@ -5,12 +5,14 @@ [{load_module,emqx_rule_sqltester,brutal_purge,soft_purge,[]}, {load_module,emqx_rule_registry,brutal_purge,soft_purge,[]}, {load_module,emqx_rule_engine_api,brutal_purge,soft_purge,[]}, + {load_module,emqx_rule_metrics,brutal_purge,soft_purge,[]}, {load_module,emqx_rule_engine,brutal_purge,soft_purge,[]}]}, {"4.3.14", [{load_module,emqx_rule_sqltester,brutal_purge,soft_purge,[]}, {load_module,emqx_rule_engine_api,brutal_purge,soft_purge,[]}, {load_module,emqx_rule_registry,brutal_purge,soft_purge,[]}, {load_module,emqx_rule_runtime,brutal_purge,soft_purge,[]}, + {load_module,emqx_rule_metrics,brutal_purge,soft_purge,[]}, {load_module,emqx_rule_engine,brutal_purge,soft_purge,[]}]}, {"4.3.13", [{load_module,emqx_rule_sqltester,brutal_purge,soft_purge,[]}, @@ -19,6 +21,7 @@ {load_module,emqx_rule_actions,brutal_purge,soft_purge,[]}, {load_module,emqx_rule_utils,brutal_purge,soft_purge,[]}, {load_module,emqx_rule_runtime,brutal_purge,soft_purge,[]}, + {load_module,emqx_rule_metrics,brutal_purge,soft_purge,[]}, {load_module,emqx_rule_registry,brutal_purge,soft_purge,[]}]}, {"4.3.12", [{load_module,emqx_rule_sqltester,brutal_purge,soft_purge,[]}, @@ -27,6 +30,7 @@ {load_module,emqx_rule_actions,brutal_purge,soft_purge,[]}, {load_module,emqx_rule_utils,brutal_purge,soft_purge,[]}, {load_module,emqx_rule_runtime,brutal_purge,soft_purge,[]}, + {load_module,emqx_rule_metrics,brutal_purge,soft_purge,[]}, {load_module,emqx_rule_registry,brutal_purge,soft_purge,[]}]}, {"4.3.11", [{load_module,emqx_rule_sqltester,brutal_purge,soft_purge,[]}, @@ -36,6 +40,7 @@ {load_module,emqx_rule_runtime,brutal_purge,soft_purge,[]}, {load_module,emqx_rule_registry,brutal_purge,soft_purge,[]}, {load_module,emqx_rule_validator,brutal_purge,soft_purge,[]}, + {load_module,emqx_rule_metrics,brutal_purge,soft_purge,[]}, {load_module,emqx_rule_engine_api,brutal_purge,soft_purge,[]}]}, {"4.3.10", [{load_module,emqx_rule_sqltester,brutal_purge,soft_purge,[]}, @@ -45,6 +50,7 @@ {load_module,emqx_rule_engine,brutal_purge,soft_purge,[]}, {load_module,emqx_rule_runtime,brutal_purge,soft_purge,[]}, {load_module,emqx_rule_registry,brutal_purge,soft_purge,[]}, + {load_module,emqx_rule_metrics,brutal_purge,soft_purge,[]}, {load_module,emqx_rule_engine_api,brutal_purge,soft_purge,[]}]}, {"4.3.9", [{load_module,emqx_rule_sqltester,brutal_purge,soft_purge,[]}, @@ -56,6 +62,7 @@ {load_module,emqx_rule_funcs,brutal_purge,soft_purge,[]}, {load_module,emqx_rule_events,brutal_purge,soft_purge,[]}, {load_module,emqx_rule_engine_api,brutal_purge,soft_purge,[]}, + {load_module,emqx_rule_metrics,brutal_purge,soft_purge,[]}, {load_module,emqx_rule_engine,brutal_purge,soft_purge,[]}, {add_module,emqx_rule_date}, {load_module,emqx_rule_registry,brutal_purge,soft_purge,[]}]}, @@ -202,12 +209,14 @@ [{load_module,emqx_rule_sqltester,brutal_purge,soft_purge,[]}, {load_module,emqx_rule_registry,brutal_purge,soft_purge,[]}, {load_module,emqx_rule_engine_api,brutal_purge,soft_purge,[]}, + {load_module,emqx_rule_metrics,brutal_purge,soft_purge,[]}, {load_module,emqx_rule_engine,brutal_purge,soft_purge,[]}]}, {"4.3.14", [{load_module,emqx_rule_sqltester,brutal_purge,soft_purge,[]}, {load_module,emqx_rule_engine_api,brutal_purge,soft_purge,[]}, {load_module,emqx_rule_registry,brutal_purge,soft_purge,[]}, {load_module,emqx_rule_runtime,brutal_purge,soft_purge,[]}, + {load_module,emqx_rule_metrics,brutal_purge,soft_purge,[]}, {load_module,emqx_rule_engine,brutal_purge,soft_purge,[]}]}, {"4.3.13", [{load_module,emqx_rule_sqltester,brutal_purge,soft_purge,[]}, @@ -216,6 +225,7 @@ {load_module,emqx_rule_actions,brutal_purge,soft_purge,[]}, {load_module,emqx_rule_utils,brutal_purge,soft_purge,[]}, {load_module,emqx_rule_runtime,brutal_purge,soft_purge,[]}, + {load_module,emqx_rule_metrics,brutal_purge,soft_purge,[]}, {load_module,emqx_rule_registry,brutal_purge,soft_purge,[]}]}, {"4.3.12", [{load_module,emqx_rule_sqltester,brutal_purge,soft_purge,[]}, @@ -224,6 +234,7 @@ {load_module,emqx_rule_actions,brutal_purge,soft_purge,[]}, {load_module,emqx_rule_utils,brutal_purge,soft_purge,[]}, {load_module,emqx_rule_runtime,brutal_purge,soft_purge,[]}, + {load_module,emqx_rule_metrics,brutal_purge,soft_purge,[]}, {load_module,emqx_rule_registry,brutal_purge,soft_purge,[]}]}, {"4.3.11", [{load_module,emqx_rule_sqltester,brutal_purge,soft_purge,[]}, @@ -233,6 +244,7 @@ {load_module,emqx_rule_registry,brutal_purge,soft_purge,[]}, {load_module,emqx_rule_validator,brutal_purge,soft_purge,[]}, {load_module,emqx_rule_runtime,brutal_purge,soft_purge,[]}, + {load_module,emqx_rule_metrics,brutal_purge,soft_purge,[]}, {load_module,emqx_rule_engine_api,brutal_purge,soft_purge,[]}]}, {"4.3.10", [{load_module,emqx_rule_sqltester,brutal_purge,soft_purge,[]}, @@ -242,6 +254,7 @@ {load_module,emqx_rule_runtime,brutal_purge,soft_purge,[]}, {load_module,emqx_rule_actions,brutal_purge,soft_purge,[]}, {load_module,emqx_rule_registry,brutal_purge,soft_purge,[]}, + {load_module,emqx_rule_metrics,brutal_purge,soft_purge,[]}, {load_module,emqx_rule_engine_api,brutal_purge,soft_purge,[]}]}, {"4.3.9", [{load_module,emqx_rule_sqltester,brutal_purge,soft_purge,[]}, @@ -254,6 +267,7 @@ {load_module,emqx_rule_events,brutal_purge,soft_purge,[]}, {load_module,emqx_rule_engine_api,brutal_purge,soft_purge,[]}, {load_module,emqx_rule_engine,brutal_purge,soft_purge,[]}, + {load_module,emqx_rule_metrics,brutal_purge,soft_purge,[]}, {load_module,emqx_rule_registry,brutal_purge,soft_purge,[]}, {delete_module,emqx_rule_date}]}, {"4.3.8",