From 7605fa5e64bd38dbe881b18c69bdc4a22a15374d Mon Sep 17 00:00:00 2001 From: EMQ-YangM Date: Tue, 11 Jan 2022 01:12:15 -0800 Subject: [PATCH] fix(rule_engine_metric): remove unused metrics 'overall_metrics' --- .../src/emqx_rule_metrics.erl | 43 ++----------------- .../test/emqx_rule_metrics_SUITE.erl | 25 ----------- 2 files changed, 3 insertions(+), 65 deletions(-) diff --git a/apps/emqx_rule_engine/src/emqx_rule_metrics.erl b/apps/emqx_rule_engine/src/emqx_rule_metrics.erl index a77c56745..bb8313f59 100644 --- a/apps/emqx_rule_engine/src/emqx_rule_metrics.erl +++ b/apps/emqx_rule_engine/src/emqx_rule_metrics.erl @@ -58,14 +58,11 @@ -export([ inc/2 , inc/3 , get/2 - , get_overall/1 , get_rule_speed/1 - , get_overall_rule_speed/0 , create_rule_metrics/1 , create_metrics/1 , clear_rule_metrics/1 , clear_metrics/1 - , overall_metrics/0 ]). -export([ get_rule_metrics/1 @@ -137,18 +134,10 @@ get(Id, Metric) -> Ref -> counters:get(Ref, metrics_idx(Metric)) end. --spec(get_overall(atom()) -> number()). -get_overall(Metric) -> - emqx_metrics:val(Metric). - -spec(get_rule_speed(rule_id()) -> map()). get_rule_speed(Id) -> gen_server:call(?MODULE, {get_rule_speed, Id}). --spec(get_overall_rule_speed() -> map()). -get_overall_rule_speed() -> - gen_server:call(?MODULE, get_overall_rule_speed). - -spec(get_rule_metrics(rule_id()) -> map()). get_rule_metrics(Id) -> #{max := Max, current := Current, last5m := Last5M} = get_rule_speed(Id), @@ -186,12 +175,7 @@ inc(Id, Metric, Val) -> counters:add(couters_ref(Id), metrics_idx(Metric), Val); Ref -> counters:add(Ref, metrics_idx(Metric), Val) - end, - inc_overall(Metric, Val). - --spec(inc_overall(atom(), pos_integer()) -> ok). -inc_overall(Metric, Val) -> - emqx_metrics:inc(Metric, Val). + end. inc_actions_taken(Id) -> inc_actions_taken(Id, 1). @@ -280,8 +264,6 @@ start_link() -> init([]) -> erlang:process_flag(trap_exit, true), - %% the overall counters - [ok = emqx_metrics:ensure(Metric)|| Metric <- overall_metrics()], %% the speed metrics erlang:send_after(timer:seconds(?SAMPLING), self(), ticking), {ok, #state{overall_rule_speed = #rule_speed{}}}. @@ -294,9 +276,6 @@ handle_call({get_rule_speed, Id}, _From, State = #state{rule_speeds = RuleSpeeds Speed -> format_rule_speed(Speed) end, State}; -handle_call(get_overall_rule_speed, _From, State = #state{overall_rule_speed = RuleSpeed}) -> - {reply, format_rule_speed(RuleSpeed), State}; - handle_call({create_metrics, Id}, _From, State = #state{metric_ids = MIDs}) -> {reply, create_counters(Id), State#state{metric_ids = sets:add_element(Id, MIDs)}}; @@ -333,17 +312,14 @@ handle_info(ticking, State = #state{rule_speeds = undefined}) -> erlang:send_after(timer:seconds(?SAMPLING), self(), ticking), {noreply, State}; -handle_info(ticking, State = #state{rule_speeds = RuleSpeeds0, - overall_rule_speed = OverallRuleSpeed0}) -> +handle_info(ticking, State = #state{rule_speeds = RuleSpeeds0}) -> RuleSpeeds = maps:map( fun(Id, RuleSpeed) -> calculate_speed(get_rules_matched(Id), RuleSpeed) end, RuleSpeeds0), - OverallRuleSpeed = calculate_speed(get_overall('rules.matched'), OverallRuleSpeed0), async_refresh_resource_status(), erlang:send_after(timer:seconds(?SAMPLING), self(), ticking), - {noreply, State#state{rule_speeds = RuleSpeeds, - overall_rule_speed = OverallRuleSpeed}}; + {noreply, State#state{rule_speeds = RuleSpeeds}}; handle_info(_Info, State) -> {noreply, State}. @@ -493,16 +469,3 @@ metrics_idx('rules.passed') -> 8; metrics_idx('rules.exception') -> 9; metrics_idx('rules.no_result') -> 10; metrics_idx(_) -> 11. - -overall_metrics() -> - [ 'rules.matched' - , 'actions.success' - , 'actions.error' - , 'actions.taken' - , 'actions.exception' - , 'actions.retry' - , 'rules.failed' - , 'rules.passed' - , 'rules.exception' - , 'rules.no_result' - ]. 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 342dcddac..5a8d99d0e 100644 --- a/apps/emqx_rule_engine/test/emqx_rule_metrics_SUITE.erl +++ b/apps/emqx_rule_engine/test/emqx_rule_metrics_SUITE.erl @@ -55,7 +55,6 @@ 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) -> @@ -81,8 +80,6 @@ t_action(_) -> ?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">>)), @@ -105,7 +102,6 @@ t_rule(_) -> ?assertEqual(1, emqx_rule_metrics:get(<<"rule:1">>, 'rules.failed')), ?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(<<"rule2">>). @@ -127,24 +123,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">>). @@ -154,14 +137,10 @@ rule_speed(_) -> % 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'). @@ -171,7 +150,3 @@ rule_speed(_) -> % t_inc(_) -> % error('TODO'). - -% t_overall_metrics(_) -> -% error('TODO'). -