8000 Merge pull request #9125 from emqx/cannot_reset_metrics_for_fallback_… · emqx/emqx@9a2300e · GitHub
[go: up one dir, main page]

Skip to content

Commit 9a2300e

Browse files
authored
Merge pull request #9125 from emqx/cannot_reset_metrics_for_fallback_actions
Cannot reset metrics for fallback actions
2 parents 373bd41 + 6d52f90 commit 9a2300e

File tree

4 files changed

+89
-19
lines changed

4 files changed

+89
-19
lines changed

CHANGES-4.3.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,8 @@ File format:
7171
subscriber from another node in the cluster.
7272
Fixed in [#9122](https://github.com/emqx/emqx/pull/9122)
7373

74+
- Fix cannot reset metrics for fallback actions. [#9125](https://github.com/emqx/emqx/pull/9125)
75+
7476
## v4.3.20
7577

7678
### Bug fixes

apps/emqx_rule_engine/src/emqx_rule_engine.appup.src

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,14 @@
55
[{load_module,emqx_rule_sqltester,brutal_purge,soft_purge,[]},
66
{load_module,emqx_rule_registry,brutal_purge,soft_purge,[]},
77
{load_module,emqx_rule_engine_api,brutal_purge,soft_purge,[]},
8+
{load_module,emqx_rule_metrics,brutal_purge,soft_purge,[]},
89
{load_module,emqx_rule_engine,brutal_purge,soft_purge,[]}]},
910
{"4.3.14",
1011
[{load_module,emqx_rule_sqltester,brutal_purge,soft_purge,[]},
1112
{load_module,emqx_rule_engine_api,brutal_purge,soft_purge,[]},
1213
{load_module,emqx_rule_registry,brutal_purge,soft_purge,[]},
1314
{load_module,emqx_rule_runtime,brutal_purge,soft_purge,[]},
15+
{load_module,emqx_rule_metrics,brutal_purge,soft_purge,[]},
1416
{load_module,emqx_rule_engine,brutal_purge,soft_purge,[]}]},
1517
{"4.3.13",
1618
[{load_module,emqx_rule_sqltester,brutal_purge,soft_purge,[]},
@@ -19,6 +21,7 @@
1921
{load_module,emqx_rule_actions,brutal_purge,soft_purge,[]},
2022
{load_module,emqx_rule_utils,brutal_purge,soft_purge,[]},
2123
{load_module,emqx_rule_runtime,brutal_purge,soft_purge,[]},
24+
{load_module,emqx_rule_metrics,brutal_purge,soft_purge,[]},
2225
{load_module,emqx_rule_registry,brutal_purge,soft_purge,[]}]},
2326
D28D {"4.3.12",
2427
[{load_module,emqx_rule_sqltester,brutal_purge,soft_purge,[]},
@@ -27,6 +30,7 @@
2730
{load_module,emqx_rule_actions,brutal_purge,soft_purge,[]},
2831
{load_module,emqx_rule_utils,brutal_purge,soft_purge,[]},
2932
{load_module,emqx_rule_runtime,brutal_purge,soft_purge,[]},
33+
{load_module,emqx_rule_metrics,brutal_purge,soft_purge,[]},
3034
{load_module,emqx_rule_registry,brutal_purge,soft_purge,[]}]},
3135
{"4.3.11",
3236
[{load_module,emqx_rule_sqltester,brutal_purge,soft_purge,[]},
@@ -36,6 +40,7 @@
3640
{load_module,emqx_rule_runtime,brutal_purge,soft_purge,[]},
3741
{load_module,emqx_rule_registry,brutal_purge,soft_purge,[]},
3842
{load_module,emqx_rule_validator,brutal_purge,soft_purge,[]},
43+
{load_module,emqx_rule_metrics,brutal_purge,soft_purge,[]},
3944
{load_module,emqx_rule_engine_api,brutal_purge,soft_purge,[]}]},
4045
{"4.3.10",
4146
[{load_module,emqx_rule_sqltester,brutal_purge,soft_purge,[]},
@@ -45,6 +50,7 @@
4550
{load_module,emqx_rule_engine,brutal_purge,soft_purge,[]},
4651
{load_module,emqx_rule_runtime,brutal_purge,soft_purge,[]},
4752
{load_module,emqx_rule_registry,brutal_purge,soft_purge,[]},
53+
{load_module,emqx_rule_metrics,brutal_purge,soft_purge,[]},
4854
{load_module,emqx_rule_engine_api,brutal_purge,soft_purge,[]}]},
4955
{"4.3.9",
5056
[{load_module,emqx_rule_sqltester,brutal_purge,soft_purge,[]},
@@ -56,6 +62,7 @@
5662
{load_module,emqx_rule_funcs,brutal_purge,soft_purge,[]},
5763
{load_module,emqx_rule_events,brutal_purge,soft_purge,[]},
5864
{load_module,emqx_rule_engine_api,brutal_purge,soft_purge,[]},
65+
{load_module,emqx_rule_metrics,brutal_purge,soft_purge,[]},
5966
{load_module,emqx_rule_engine,brutal_purge,soft_purge,[]},
6067
{add_module,emqx_rule_date},
6168
{load_module,emqx_rule_registry,brutal_purge,soft_purge,[]}]},
@@ -202,12 +209,14 @@
202209
[{load_module,emqx_rule_sqltester,brutal_purge,soft_purge,[]},
203210
{load_module,emqx_rule_registry,brutal_purge,soft_purge,[]},
204211
{load_module,emqx_rule_engine_api,brutal_purge,soft_purge,[]},
212+
{load_module,emqx_rule_metrics,brutal_purge,soft_purge,[]},
205213
{load_module,emqx_rule_engine,brutal_purge,soft_purge,[]}]},
206214
{"4.3.14",
207215
[{load_module,emqx_rule_sqltester,brutal_purge,soft_purge,[]},
208216
{load_module,emqx_rule_engine_api,brutal_purge,soft_purge,[]},
209217
{load_module,emqx_rule_registry,brutal_purge,soft_purge,[]},
210218
{load_module,emqx_rule_runtime,brutal_purge,soft_purge,[]},
219+
{load_module,emqx_rule_metrics,brutal_purge,soft_purge,[]},
211220
{load_module,emqx_rule_engine,brutal_purge,soft_purge,[]}]},
212221
{"4.3.13",
213222
[{load_module,emqx_rule_sqltester,brutal_purge,soft_purge,[]},
@@ -216,6 +225,7 @@
216225
{load_module,emqx_rule_actions,brutal_purge,soft_purge,[]},
217226
{load_module,emqx_rule_utils,brutal_purge,soft_purge,[]},
218227
{load_module,emqx_rule_runtime,brutal_purge,soft_purge,[]},
228+
{load_module,emqx_rule_metrics,brutal_purge,soft_purge,[]},
219229
{load_module,emqx_rule_registry,brutal_purge,soft_purge,[]}]},
220230
{"4.3.12",
221231
[{load_module,emqx_rule_sqltester,brutal_purge,soft_purge,[]},
@@ -224,6 +234,7 @@
224234
{load_module,emqx_rule_actions,brutal_purge,soft_purge,[]},
225235
{load_module,emqx_rule_utils,brutal_purge,soft_purge,[]},
226236
{load_module,emqx_rule_runtime,brutal_purge,soft_purge,[]},
237+
{load_module,emqx_rule_metrics,brutal_purge,soft_purge,[]},
227238
{load_module,emqx_rule_registry,brutal_purge,soft_purge,[]}]},
228239
{"4.3.11",
229240
[{load_module,emqx_rule_sqltester,brutal_purge,soft_purge,[]},
@@ -233,6 +244,7 @@
233244
{load_module,emqx_rule_registry,brutal_purge,soft_purge,[]},
234245
{load_module,emqx_rule_validator,brutal_purge,soft_purge,[]},
235246
{load_module,emqx_rule_runtime,brutal_purge,soft_purge,[]},
247+
{load_module,emqx_rule_metrics,brutal_purge,soft_purge,[]},
236248
{load_module,emqx_rule_engine_api,brutal_purge,soft_purge,[]}]},
237249
{"4.3.10",
238250
[{load_module,emqx_rule_sqltester,brutal_purge,soft_purge,[]},
@@ -242,6 +254,7 @@
242254
{load_module,emqx_rule_runtime,brutal_purge,soft_purge,[]},
243255
{load_module,emqx_rule_actions,brutal_purge,soft_purge,[]},
244256
{load_module,emqx_rule_registry,brutal_purge,soft_purge,[]},
257+
{load_module,emqx_rule_metrics,brutal_purge,soft_purge,[]},
245258
{load_module,emqx_rule_engine_api,brutal_purge,soft_purge,[]}]},
246259
{"4.3.9",
247260
[{load_module,emqx_rule_sqltester,brutal_purge,soft_purge,[]},
@@ -254,6 +267,7 @@
254267
{load_module,emqx_rule_events,brutal_purge,soft_purge,[]},
255268
{load_module,emqx_rule_engine_api,brutal_purge,soft_purge,[]},
256269
{load_module,emqx_rule_engine,brutal_purge,soft_purge,[]},
270+
{load_module,emqx_rule_metrics,brutal_purge,soft_purge,[]},
257271
{load_module,emqx_rule_registry,brutal_purge,soft_purge,[]},
258272
{delete_module,emqx_rule_date}]},
259273
{"4.3.8",

apps/emqx_rule_engine/src/emqx_rule_metrics.erl

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -131,23 +131,28 @@ clear_metrics(Id) ->
131131
-spec(reset_metrics(rule_id()) -> ok).
132132
reset_metrics(Id) ->
133133
reset_speeds(Id),
134-
reset_metrics(Id, rule_metrics()),
134+
do_reset_metrics(Id, rule_metrics()),
135135
case emqx_rule_registry:get_rule(Id) of
136136
not_found -> ok;
137137
{ok, #rule{actions = Actions}} ->
138-
[ reset_metrics(ActionId, action_metrics())
139-
|| #action_instance{ id = ActionId} <- Actions],
138+
reset_action_metrics(Actions),
140139
ok
141140
end.
142141

143-
reset_metrics(Id, Metrics) ->
142+
do_reset_metrics(Id, Metrics) ->
144143
case couters_ref(Id) of
145144
not_found -> ok;
146145
Ref -> [counters:put(Ref, metrics_idx(Idx), 0)
147146
|| Idx <- Metrics],
148147
ok
149148
end.
150149

150+
reset_action_metrics(Actions) ->
151+
lists:foreach(fun(#action_instance{id = ActionId, fallbacks = FallbackActions}) ->
152+
do_reset_metrics(ActionId, action_metrics()),
153+
reset_action_metrics(FallbackActions)
154+
end, Actions).
155+
151156
reset_speeds(Id) ->
152157
gen_server:call(?MODULE, {reset_speeds, Id}).
153158

apps/emqx_rule_engine/test/emqx_rule_engine_SUITE.erl

Lines changed: 64 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ groups() ->
5050
t_unregister_provider,
5151
t_create_rule,
5252
t_reset_metrics,
53+
t_reset_metrics_fallbacks,
5354
t_create_resource
5455
]},
5556
{actions, [],
@@ -379,35 +380,83 @@ t_inspect_action(_Config) ->
379380

380381
t_reset_metrics(_Config) ->
381382
ok = emqx_rule_engine:load_providers(),
382-
{ok, #resource{id = ResId}} = emqx_rule_engine:create_resource(
383-
#{type => built_in,
384-
config => #{},
385-
description => <<"debug resource">>}),
386-
{ok, #rule{id = Id}} = emqx_rule_engine:create_rule(
387-
#{rawsql => "select clientid as c, username as u "
388-
"from \"t1\" ",
389-
actions => [#{name => 'inspect',
390-
args => #{'$resource' => ResId, a=>1, b=>2}}],
391-
type => built_in,
392-
description => <<"Inspect rule">>
393-
}),
383+
{ok, #rule{id = Id, actions = [#action_instance{id = ActId0}]}} =
384+
emqx_rule_engine:create_rule(
385+
#{rawsql => "select clientid as c, username as u "
386+
"from \"t1\" ",
387+
actions => [#{name => 'inspect', args => #{a=>1, b=>2}}],
388+
type => built_in,
389+
description => <<"Inspect rule">>
390+
}),
394391
{ok, Client} = emqtt:start_link([{username, <<"emqx">>}]),
395392
{ok, _} = emqtt:connect(Client),
396393
[ begin
397394
emqtt:publish(Client, <<"t1">>, <<"{\"id\": 1, \"name\": \"ha\"}">>, 0),
398395
timer:sleep(100)
399396
end
400397
|| _ <- lists:seq(1,10)],
398+
?assertMatch(#{exception := 0, failed := 0,
399+
matched := 10, no_result := 0, passed := 10},
400+
emqx_rule_metrics:get_rule_metrics(Id)),
401+
?assertMatch(#{failed := 0, success := 10, taken := 10},
402+
emqx_rule_metrics:get_action_metrics(ActId0)),
401403
emqx_rule_metrics:reset_metrics(Id),
402404
?assertEqual(#{exception => 0,failed => 0,
403405
matched => 0,no_result => 0,passed => 0,
404406
speed => 0.0,speed_last5m => 0.0,speed_max => 0.0},
405407
emqx_rule_metrics:get_rule_metrics(Id)),
406-
?assertEqual(#{failed => 0,success => 0,taken => 0},
407-
emqx_rule_metrics:get_action_metrics(ResId)),
408+
?assertEqual(#{failed => 0, success => 0, taken => 0},
409+
emqx_rule_metrics:get_action_metrics(ActId0)),
408410
emqtt:stop(Client),
409411
emqx_rule_registry:remove_rule(Id),
410-
emqx_rule_registry:remove_resource(ResId),
412+
ok.
413+
414+
t_reset_metrics_fallbacks(_Config) ->
415+
ok = emqx_rule_engine:load_providers(),
416+
ok = emqx_rule_registry:add_action(
417+
#action{name = 'crash_action', app = ?APP,
418+
module = ?MODULE, on_create = crash_action,
419+
types=[], params_spec = #{},
420+
title = #{en => <<"Crash Action">>},
421+
description = #{en => <<"This action will always fail!">>}}),
422+
{ok, #rule{id = Id, actions = [#action_instance{id = ActId0, fallbacks = [
423+
#action_instance{id = ActId1},
424+
#action_instance{id = ActId2}
425+
]}]}} =
426+
emqx_rule_engine:create_rule(
427+
#{rawsql => "select clientid as c, username as u "
428+
"from \"t1\" ",
429+
actions => [#{name => 'crash_action', args => #{a=>1, b=>2}, fallbacks => [
430+
#{name => 'inspect', args => #{}, fallbacks => []},
431+
#{name => 'inspect', args => #{}, fallbacks => []}
432+
]}],
433+
type => built_in,
434+
description => <<"Inspect rule">>
435+
}),
436+
{ok, Client} = emqtt:start_link([{username, <<"emqx">>}]),
437+
{ok, _} = emqtt:connect(Client),
438+
[ begin
439+
emqtt:publish(Client, <<"t1">>, <<"{\"id\": 1, \"name\": \"ha\"}">>, 0),
440+
timer:sleep(100)
441+
end
442+
|| _ <- lists:seq(1,10)],
443+
?assertMatch(#{exception := 0, failed := 0,
444+
matched := 10, no_result := 0, passed := 10},
445+
emqx_rule_metrics:get_rule_metrics(Id)),
446+
[?assertMatch(#{failed := 10, success := 0, taken := 10},
447+
emqx_rule_metrics:get_action_metrics(AId)) || AId <- [ActId0]],
448+
[?assertMatch(#{failed := 0, success := 10, taken := 10},
449+
emqx_rule_metrics:get_action_metrics(AId)) || AId <- [ ActId1, ActId2]],
450+
emqx_rule_metrics:reset_metrics(Id),
451+
?assertEqual(#{exception => 0,failed => 0,
452+
matched => 0,no_result => 0,passed => 0,
453+
speed => 0.0,speed_last5m => 0.0,speed_max => 0.0},
454+
emqx_rule_metrics:get_rule_metrics(Id)),
455+
[?assertEqual(#{failed => 0, success => 0, taken => 0},
456+
emqx_rule_metrics:get_action_metrics(AId)) || AId <- [ActId0, ActId1, ActId2]],
457+
emqtt:stop(Client),
458+
emqx_rule_registry:remove_rule(Id),
459+
ok = emqx_rule_registry:remove_action('crash_action'),
411460
ok.
412461

413462
t_republish_action(_Config) ->

0 commit comments

Comments
 (0)
0