From 3d1f0c756cd8a63412858c2b631b7879fba5ccc1 Mon Sep 17 00:00:00 2001 From: JimMoen Date: Mon, 29 Jul 2024 15:29:01 +0800 Subject: [PATCH 1/2] feat: call plugin's app module `on_config_changed/2` callback assume the module: `[PluginName]_app` --- apps/emqx_plugins/src/emqx_plugins.erl | 30 ++++++++++++++++++++++++-- changes/ce/feat-13548.en.md | 6 ++++++ 2 files changed, 34 insertions(+), 2 deletions(-) create mode 100644 changes/ce/feat-13548.en.md diff --git a/apps/emqx_plugins/src/emqx_plugins.erl b/apps/emqx_plugins/src/emqx_plugins.erl index 253d1b4ef..56a88d7cb 100644 --- a/apps/emqx_plugins/src/emqx_plugins.erl +++ b/apps/emqx_plugins/src/emqx_plugins.erl @@ -356,13 +356,13 @@ get_config_bin(NameVsn) -> %% RPC call from Management API or CLI. %% The plugin config Json Map was valid by avro schema %% Or: if no and plugin config ALWAYS be valid before calling this function. -put_config(NameVsn, ConfigJsonMap, AvroValue) when not is_binary(NameVsn) -> +put_config(NameVsn, ConfigJsonMap, AvroValue) when (not is_binary(NameVsn)) -> put_config(bin(NameVsn), ConfigJsonMap, AvroValue); put_config(NameVsn, ConfigJsonMap, _AvroValue) -> HoconBin = hocon_pp:do(ConfigJsonMap, #{}), ok = backup_and_write_hocon_bin(NameVsn, HoconBin), - %% TODO: callback in plugin's on_config_changed (config update by mgmt API) %% TODO: callback in plugin's on_config_upgraded (config vsn upgrade v1 -> v2) + ok = maybe_call_on_config_changed(NameVsn, ConfigJsonMap), ok = persistent_term:put(?PLUGIN_PERSIS_CONFIG_KEY(NameVsn), ConfigJsonMap), ok. @@ -373,6 +373,32 @@ restart(NameVsn) -> {error, Reason} -> {error, Reason} end. +%% @doc Call plugin's callback on_config_changed/2 +maybe_call_on_config_changed(NameVsn, NewConf) -> + FuncName = on_config_changed, + maybe + {ok, PluginAppModule} ?= app_module_name(NameVsn), + true ?= erlang:function_exported(PluginAppModule, FuncName, 2), + {ok, OldConf} = get_config(NameVsn), + _ = erlang:apply(PluginAppModule, FuncName, [OldConf, NewConf]) + else + {error, Reason} -> + ?SLOG(info, #{msg => "failed_to_call_on_config_changed", reason => Reason}); + false -> + ?SLOG(info, #{msg => "on_config_changed_callback_not_exported"}); + _ -> + ok + end. + +app_module_name(NameVsn) -> + case read_plugin_info(NameVsn, #{}) of + {ok, #{<<"name">> := Name} = _PluginInfo} -> + emqx_utils:safe_to_existing_atom(<>); + {error, Reason} -> + ?SLOG(error, Reason#{msg => "failed_to_read_plugin_info"}), + {error, Reason} + end. + %% @doc List all installed plugins. %% Including the ones that are installed, but not enabled in config. -spec list() -> [plugin_info()]. diff --git a/changes/ce/feat-13548.en.md b/changes/ce/feat-13548.en.md new file mode 100644 index 000000000..75b56cd43 --- /dev/null +++ b/changes/ce/feat-13548.en.md @@ -0,0 +1,6 @@ +Optionally calls the `on_config_changed/2` callback function when the plugin configuration is updated via the REST API. + +This callback function is assumed to be exported by the `_app` module. +i.e: +Plugin NameVsn: `my_plugin-1.0.0` +This callback function is assumed to be `my_plugin_app:on_config_changed/2` From e6bfc14cc9f8f7c792dc55c96fe81e38f9acb237 Mon Sep 17 00:00:00 2001 From: JimMoen Date: Wed, 31 Jul 2024 09:26:44 +0800 Subject: [PATCH 2/2] fix: try-catch optional `on_config_changed/2` plugin app callback --- apps/emqx_plugins/src/emqx_plugins.erl | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/apps/emqx_plugins/src/emqx_plugins.erl b/apps/emqx_plugins/src/emqx_plugins.erl index 56a88d7cb..94c6aa4e1 100644 --- a/apps/emqx_plugins/src/emqx_plugins.erl +++ b/apps/emqx_plugins/src/emqx_plugins.erl @@ -380,7 +380,18 @@ maybe_call_on_config_changed(NameVsn, NewConf) -> {ok, PluginAppModule} ?= app_module_name(NameVsn), true ?= erlang:function_exported(PluginAppModule, FuncName, 2), {ok, OldConf} = get_config(NameVsn), - _ = erlang:apply(PluginAppModule, FuncName, [OldConf, NewConf]) + try erlang:apply(PluginAppModule, FuncName, [OldConf, NewConf]) of + _ -> ok + catch + Class:CatchReason:Stacktrace -> + ?SLOG(error, #{ + msg => "failed_to_call_on_config_changed", + exception => Class, + reason => CatchReason, + stacktrace => Stacktrace + }), + ok + end else {error, Reason} -> ?SLOG(info, #{msg => "failed_to_call_on_config_changed", reason => Reason});