Merge pull request #10847 from zhongwencool/telemetry-conf-update

feat: support emqx_conf:update([telemetry],Conf).

apps/emqx_telemetry/src/emqx_telemetry.erl

@@ -121,7 +121,7 @@ start_reporting() ->
 
 %% @doc Stop the reporting timer.
 %% This is an async notification which never fails.
-%% This is a no-op in enterprise eidtion.
+%% This is a no-op in enterprise edition.
 stop_reporting() ->
     gen_server:cast(?MODULE, stop_reporting).
 

apps/emqx_telemetry/src/emqx_telemetry_config.erl

@@ -32,11 +32,12 @@
 -export([
     on_server_start/0,
     on_server_stop/0,
-    is_official_version/1
+    is_official_version/1,
+    is_official_version/0
 ]).
 
 is_enabled() ->
-    IsOfficial = ?MODULE:is_official_version(emqx_release:version()),
+    IsOfficial = ?MODULE:is_official_version(),
     emqx_conf:get([telemetry, enable], IsOfficial).
 
 on_server_start() ->
@@ -53,7 +54,9 @@ set_telemetry_status(Status) ->
     end.
 
 pre_config_update(_, {set_telemetry_status, Status}, RawConf) ->
-    {ok, RawConf#{<<"enable">> => Status}}.
+    {ok, RawConf#{<<"enable">> => Status}};
+pre_config_update(_, NewConf, _OldConf) ->
+    {ok, NewConf}.
 
 post_config_update(
     _,
@@ -65,6 +68,11 @@ post_config_update(
     case Status of
         true -> emqx_telemetry:start_reporting();
         false -> emqx_telemetry:stop_reporting()
+    end;
+post_config_update(_, _UpdateReq, NewConf, _OldConf, _AppEnvs) ->
+    case maps:get(enable, NewConf, ?MODULE:is_official_version()) of
+        true -> emqx_telemetry:start_reporting();
+        false -> emqx_telemetry:stop_reporting()
     end.
 
 cfg_update(Path, Action, Params) ->
@@ -74,6 +82,9 @@ cfg_update(Path, Action, Params) ->
         #{override_to => cluster}
     ).
 
+is_official_version() ->
+    is_official_version(emqx_release:version()).
+
 is_official_version(Version) ->
     Pt = "^\\d+\\.\\d+(?:\\.\\d+)?(?:(-(?:alpha|beta|rc)\\.[1-9][0-9]*))?$",
     match =:= re:run(Version, Pt, [{capture, none}]).

apps/emqx_telemetry/test/emqx_telemetry_api_SUITE.erl

@@ -30,6 +30,7 @@ all() ->
 
 init_per_suite(Config) ->
     ok = emqx_common_test_helpers:load_config(emqx_modules_schema, ?BASE_CONF),
+    ok = emqx_common_test_helpers:load_config(emqx_telemetry_schema, ?BASE_CONF),
     ok = emqx_mgmt_api_test_util:init_suite(
         [emqx_conf, emqx_authn, emqx_authz, emqx_telemetry],
         fun set_special_configs/1
@@ -52,32 +53,26 @@ end_per_suite(_Config) ->
     ok.
 
 init_per_testcase(t_status_non_official, Config) ->
-    meck:new(emqx_telemetry, [non_strict, passthrough]),
+    meck:new(emqx_telemetry_config, [non_strict, passthrough]),
-    meck:expect(emqx_telemetry, official_version, 1, false),
+    meck:expect(emqx_telemetry_config, is_official_version, 0, false),
+    %% check non-official telemetry is disable by default
+    {ok, _} = emqx:update_config([telemetry], #{}),
     Config;
-init_per_testcase(t_status, Config) ->
+init_per_testcase(t_status_official, Config) ->
-    meck:new(emqx_telemetry, [non_strict, passthrough]),
+    meck:new(emqx_telemetry_config, [non_strict, passthrough]),
-    meck:expect(emqx_telemetry, enable, fun() -> ok end),
+    meck:expect(emqx_telemetry_config, is_official_version, 0, true),
-    {ok, _, _} =
+    %% check official telemetry is enable by default
-        request(
+    {ok, _} = emqx:update_config([telemetry], #{}),
-            put,
-            uri(["telemetry", "status"]),
-            #{<<"enable">> => true}
-        ),
     Config;
 init_per_testcase(_TestCase, Config) ->
-    {ok, _, _} =
+    %% Force enable telemetry to check data.
-        request(
+    {ok, _} = emqx:update_config([telemetry], #{<<"enable">> => true}),
-            put,
-            uri(["telemetry", "status"]),
-            #{<<"enable">> => true}
-        ),
     Config.
 
 end_per_testcase(t_status_non_official, _Config) ->
-    meck:unload(emqx_telemetry);
+    meck:unload(emqx_telemetry_config);
 end_per_testcase(t_status, _Config) ->
-    meck:unload(emqx_telemetry);
+    meck:unload(emqx_telemetry_config);
 end_per_testcase(_TestCase, _Config) ->
     ok.
 
@@ -95,39 +90,50 @@ set_special_configs(_App) ->
 %% Tests
 %%------------------------------------------------------------------------------
 
-t_status(_) ->
+%% official's telemetry is enabled by default
+t_status_official(_) ->
+    check_status(true).
+
+%% non official's telemetry is disabled by default
+t_status_non_official(_) ->
+    check_status(false).
+
+check_status(Default) ->
+    ct:pal("Check telemetry status:~p~n", [emqx_telemetry_config:is_official_version()]),
+    ?assertEqual(Default, is_telemetry_process_enabled()),
     ?assertMatch(
         {ok, 200, _},
         request(
             put,
             uri(["telemetry", "status"]),
-            #{<<"enable">> => false}
+            #{<<"enable">> => (not Default)}
         )
     ),
 
     {ok, 200, Result0} =
         request(get, uri(["telemetry", "status"])),
-
     ?assertEqual(
-        #{<<"enable">> => false},
+        #{<<"enable">> => (not Default)},
         emqx_utils_json:decode(Result0)
     ),
+    ?assertEqual((not Default), is_telemetry_process_enabled()),
 
     ?assertMatch(
         {ok, 400, _},
         request(
             put,
             uri(["telemetry", "status"]),
-            #{<<"enable">> => false}
+            #{<<"enable">> => (not Default)}
         )
     ),
+    ?assertEqual((not Default), is_telemetry_process_enabled()),
 
     ?assertMatch(
         {ok, 200, _},
         request(
             put,
             uri(["telemetry", "status"]),
-            #{<<"enable">> => true}
+            #{<<"enable">> => Default}
         )
     ),
 
@@ -135,30 +141,24 @@ t_status(_) ->
         request(get, uri(["telemetry", "status"])),
 
     ?assertEqual(
-        #{<<"enable">> => true},
+        #{<<"enable">> => Default},
         emqx_utils_json:decode(Result1)
     ),
+    ?assertEqual(Default, is_telemetry_process_enabled()),
 
     ?assertMatch(
         {ok, 400, _},
         request(
             put,
             uri(["telemetry", "status"]),
-            #{<<"enable">> => true}
+            #{<<"enable">> => Default}
         )
-    ).
+    ),
-
+    ?assertEqual(Default, is_telemetry_process_enabled()),
-t_status_non_official(_) ->
+    ok.
-    ?assertMatch(
-        {ok, 200, _},
-        request(
-            put,
-            uri(["telemetry", "status"]),
-            #{<<"enable">> => false}
-        )
-    ).
 
 t_data(_) ->
+    ?assert(is_telemetry_process_enabled()),
     {ok, 200, Result} =
         request(get, uri(["telemetry", "data"])),
 
@@ -191,3 +191,23 @@ t_data(_) ->
         request(get, uri(["telemetry", "data"])),
 
     ok.
+
+%% Support emqx:update_config([telemetry], Conf).
+t_conf_update(_) ->
+    Conf = emqx:get_raw_config([telemetry]),
+    ?assert(is_telemetry_process_enabled()),
+    {ok, 200, Result1} = request(get, uri(["telemetry", "status"])),
+    ?assertEqual(#{<<"enable">> => true}, emqx_utils_json:decode(Result1)),
+    {ok, _} = emqx:update_config([telemetry], Conf#{<<"enable">> => false}),
+    {ok, 200, Result2} = request(get, uri(["telemetry", "status"])),
+    ?assertEqual(#{<<"enable">> => false}, emqx_utils_json:decode(Result2)),
+    ?assertNot(is_telemetry_process_enabled()),
+    %% reset to true
+    {ok, _} = emqx:update_config([telemetry], Conf#{<<"enable">> => true}),
+    ?assert(is_telemetry_process_enabled()),
+    ok.
+
+is_telemetry_process_enabled() ->
+    %% timer is not undefined.
+    Timer = element(6, sys:get_state(emqx_telemetry)),
+    is_reference(Timer).