diff --git a/apps/emqx/src/emqx_trace/emqx_trace.erl b/apps/emqx/src/emqx_trace/emqx_trace.erl index e55367530..eaa5bd67b 100644 --- a/apps/emqx/src/emqx_trace/emqx_trace.erl +++ b/apps/emqx/src/emqx_trace/emqx_trace.erl @@ -145,7 +145,7 @@ list(Enable) -> ets:match_object(?TRACE, #?TRACE{enable = Enable, _ = '_'}). -spec create([{Key :: binary(), Value :: binary()}] | #{atom() => binary()}) -> - ok | {error, {duplicate_condition, iodata()} | {already_existed, iodata()} | iodata()}. +{ok, #?TRACE{}} | {error, {duplicate_condition, iodata()} | {already_existed, iodata()} | iodata()}. create(Trace) -> case mnesia:table_info(?TRACE, size) < ?MAX_SIZE of true -> @@ -291,7 +291,9 @@ insert_new_trace(Trace) -> #?TRACE{start_at = StartAt, type = Type, filter = Filter} = Trace, Match = #?TRACE{_ = '_', start_at = StartAt, type = Type, filter = Filter}, case mnesia:match_object(?TRACE, Match, read) of - [] -> mnesia:write(?TRACE, Trace, write); + [] -> + ok = mnesia:write(?TRACE, Trace, write), + {ok, Trace}; [#?TRACE{name = Name}] -> mnesia:abort({duplicate_condition, Name}) end; [#?TRACE{name = Name}] -> mnesia:abort({already_existed, Name}) diff --git a/apps/emqx/test/emqx_trace_SUITE.erl b/apps/emqx/test/emqx_trace_SUITE.erl index a1cad9d27..b5d761cc1 100644 --- a/apps/emqx/test/emqx_trace_SUITE.erl +++ b/apps/emqx/test/emqx_trace_SUITE.erl @@ -62,7 +62,7 @@ t_base_create_delete(_Config) -> end_at => End }, AnotherTrace = Trace#{name => <<"anotherTrace">>}, - ok = emqx_trace:create(Trace), + {ok, _} = emqx_trace:create(Trace), ?assertEqual({error, {already_existed, Name}}, emqx_trace:create(Trace)), ?assertEqual({error, {duplicate_condition, Name}}, emqx_trace:create(AnotherTrace)), [TraceRec] = emqx_trace:list(), @@ -95,13 +95,13 @@ t_create_size_max(_Config) -> Name = list_to_binary("name" ++ integer_to_list(Seq)), Trace = [{name, Name}, {type, topic}, {topic, list_to_binary("/x/y/" ++ integer_to_list(Seq))}], - ok = emqx_trace:create(Trace) + {ok, _} = emqx_trace:create(Trace) end, lists:seq(1, 30)), Trace31 = [{<<"name">>, <<"name31">>}, {<<"type">>, topic}, {<<"topic">>, <<"/x/y/31">>}], {error, _} = emqx_trace:create(Trace31), ok = emqx_trace:delete(<<"name30">>), - ok = emqx_trace:create(Trace31), + {ok, _} = emqx_trace:create(Trace31), ?assertEqual(30, erlang:length(emqx_trace:list())), ok. @@ -145,7 +145,7 @@ t_create_failed(_Config) -> t_create_default(_Config) -> {error, "name required"} = emqx_trace:create([]), - ok = emqx_trace:create([{<<"name">>, <<"test-name">>}, + {ok, _} = emqx_trace:create([{<<"name">>, <<"test-name">>}, {<<"type">>, clientid}, {<<"clientid">>, <<"good">>}]), [#emqx_trace{name = <<"test-name">>}] = emqx_trace:list(), ok = emqx_trace:clear(), @@ -166,7 +166,7 @@ t_create_default(_Config) -> {<<"end_at">>, to_rfc3339(Now + 3)} ], {error, "failed by start_at >= end_at"} = emqx_trace:create(Trace2), - ok = emqx_trace:create([{<<"name">>, <<"test-name">>}, + {ok, _} = emqx_trace:create([{<<"name">>, <<"test-name">>}, {<<"type">>, topic}, {<<"topic">>, <<"/x/y/z">>}]), [#emqx_trace{start_at = Start, end_at = End}] = emqx_trace:list(), ?assertEqual(10 * 60, End - Start), @@ -182,7 +182,7 @@ t_create_with_extra_fields(_Config) -> {<<"clientid">>, <<"dev001">>}, {<<"ip_address">>, <<"127.0.0.1">>} ], - ok = emqx_trace:create(Trace), + {ok, _} = emqx_trace:create(Trace), ?assertMatch([#emqx_trace{name = <<"test-name">>, filter = <<"/x/y/z">>, type = topic}], emqx_trace:list()), ok. @@ -191,7 +191,7 @@ t_update_enable(_Config) -> Name = <<"test-name">>, Now = erlang:system_time(second), End = list_to_binary(calendar:system_time_to_rfc3339(Now + 2)), - ok = emqx_trace:create([{<<"name">>, Name}, {<<"type">>, topic}, + {ok, _} = emqx_trace:create([{<<"name">>, Name}, {<<"type">>, topic}, {<<"topic">>, <<"/x/y/z">>}, {<<"end_at">>, End}]), [#emqx_trace{enable = Enable}] = emqx_trace:list(), ?assertEqual(Enable, true), @@ -219,8 +219,8 @@ t_load_state(_Config) -> Finished = [{<<"name">>, <<"Finished">>}, {<<"type">>, topic}, {<<"topic">>, <<"/x/y/3">>}, {<<"start_at">>, to_rfc3339(Now - 5)}, {<<"end_at">>, to_rfc3339(Now)}], - ok = emqx_trace:create(Running), - ok = emqx_trace:create(Waiting), + {ok, _} = emqx_trace:create(Running), + {ok, _} = emqx_trace:create(Waiting), {error, "end_at time has already passed"} = emqx_trace:create(Finished), Traces = emqx_trace:format(emqx_trace:list()), ?assertEqual(2, erlang:length(Traces)), @@ -241,7 +241,7 @@ t_client_event(_Config) -> Now = erlang:system_time(second), Start = to_rfc3339(Now), Name = <<"test_client_id_event">>, - ok = emqx_trace:create([{<<"name">>, Name}, + {ok, _} = emqx_trace:create([{<<"name">>, Name}, {<<"type">>, clientid}, {<<"clientid">>, ClientId}, {<<"start_at">>, Start}]), ok = emqx_trace_handler_SUITE:filesync(Name, clientid), {ok, Client} = emqtt:start_link([{clean_start, true}, {clientid, ClientId}]), @@ -250,7 +250,7 @@ t_client_event(_Config) -> ok = emqtt:publish(Client, <<"/test">>, #{}, <<"1">>, [{qos, 0}]), ok = emqtt:publish(Client, <<"/test">>, #{}, <<"2">>, [{qos, 0}]), ok = emqx_trace_handler_SUITE:filesync(Name, clientid), - ok = emqx_trace:create([{<<"name">>, <<"test_topic">>}, + {ok, _} = emqx_trace:create([{<<"name">>, <<"test_topic">>}, {<<"type">>, topic}, {<<"topic">>, <<"/test">>}, {<<"start_at">>, Start}]), ok = emqx_trace_handler_SUITE:filesync(<<"test_topic">>, topic), {ok, Bin} = file:read_file(emqx_trace:log_file(Name, Now)), @@ -279,7 +279,7 @@ t_get_log_filename(_Config) -> {<<"start_at">>, list_to_binary(Start)}, {<<"end_at">>, list_to_binary(End)} ], - ok = emqx_trace:create(Trace), + {ok, _} = emqx_trace:create(Trace), ?assertEqual({error, not_found}, emqx_trace:get_trace_filename(<<"test">>)), ?assertEqual(ok, element(1, emqx_trace:get_trace_filename(Name))), ct:sleep(3000), diff --git a/apps/emqx_management/src/emqx_mgmt_api_trace.erl b/apps/emqx_management/src/emqx_mgmt_api_trace.erl index 17fcafe24..d32e9f7cd 100644 --- a/apps/emqx_management/src/emqx_mgmt_api_trace.erl +++ b/apps/emqx_management/src/emqx_mgmt_api_trace.erl @@ -261,7 +261,7 @@ trace(get, _Params) -> end; trace(post, #{body := Param}) -> case emqx_trace:create(Param) of - ok -> {200}; + {ok, Trace0} -> {200, format_trace(Trace0)}; {error, {already_existed, Name}} -> {400, #{ code => 'ALREADY_EXISTED', @@ -280,11 +280,27 @@ trace(post, #{body := Param}) -> end; trace(delete, _Param) -> ok = emqx_trace:clear(), - {200}. + {204}. + +format_trace(Trace0) -> + [ + #{start_at := Start, end_at := End, + enable := Enable, type := Type, filter := Filter} = Trace1 + ] = emqx_trace:format([Trace0]), + Now = erlang:system_time(second), + LogSize = lists:foldl(fun(Node, Acc) -> Acc#{Node => 0} end, #{}, + mria_mnesia:running_nodes()), + Trace2 = maps:without([enable, filter], Trace1), + Trace2#{log_size => LogSize + , Type => iolist_to_binary(Filter) + , start_at => list_to_binary(calendar:system_time_to_rfc3339(Start)) + , end_at => list_to_binary(calendar:system_time_to_rfc3339(End)) + , status => status(Enable, Start, End, Now) + }. delete_trace(delete, #{bindings := #{name := Name}}) -> case emqx_trace:delete(Name) of - ok -> {200}; + ok -> {204}; {error, not_found} -> ?NOT_FOUND(Name) end. diff --git a/apps/emqx_management/src/emqx_mgmt_cli.erl b/apps/emqx_management/src/emqx_mgmt_cli.erl index cd5474261..34aff22fb 100644 --- a/apps/emqx_management/src/emqx_mgmt_cli.erl +++ b/apps/emqx_management/src/emqx_mgmt_cli.erl @@ -490,7 +490,7 @@ trace_cluster_on(Name, Type, Filter, DurationS0) -> , end_at => list_to_binary(calendar:system_time_to_rfc3339(Now + DurationS)) }, case emqx_trace:create(Trace) of - ok -> + {ok, _} -> emqx_ctl:print("cluster_trace ~p ~s ~s successfully~n", [Type, Filter, Name]); {error, Error} -> emqx_ctl:print("[error] cluster_trace ~s ~s=~s ~p~n", diff --git a/apps/emqx_management/test/emqx_mgmt_api_trace_SUITE.erl b/apps/emqx_management/test/emqx_mgmt_api_trace_SUITE.erl index 8e6c278ae..c73d1d046 100644 --- a/apps/emqx_management/test/emqx_mgmt_api_trace_SUITE.erl +++ b/apps/emqx_management/test/emqx_mgmt_api_trace_SUITE.erl @@ -69,7 +69,7 @@ t_http_test(_Config) -> ], {ok, Create} = request_api(post, api_path("trace"), Header, Trace), - ?assertEqual(<<>>, Create), + ?assertMatch(#{<<"name">> := Name}, json(Create)), {ok, List} = request_api(get, api_path("trace"), Header), [Data] = json(List), @@ -107,7 +107,7 @@ t_http_test(_Config) -> %% clear {ok, Create1} = request_api(post, api_path("trace"), Header, Trace), - ?assertEqual(<<>>, Create1), + ?assertMatch(#{<<"name">> := Name}, json(Create1)), {ok, Clear} = request_api(delete, api_path("trace"), Header), ?assertEqual(<<>>, Clear), @@ -141,7 +141,7 @@ create_trace(Name, ClientId, Start) -> ?check_trace( #{timetrap => 900}, begin - ok = emqx_trace:create([{<<"name">>, Name}, + {ok, _} = emqx_trace:create([{<<"name">>, Name}, {<<"type">>, clientid}, {<<"clientid">>, ClientId}, {<<"start_at">>, Start}]), ?block_until(#{?snk_kind := update_trace_done}) end, @@ -206,7 +206,7 @@ do_request_api(Method, Request) -> {error, {shutdown, server_closed}} -> {error, server_closed}; {ok, {{"HTTP/1.1", Code, _}, _Headers, Return}} - when Code =:= 200 orelse Code =:= 201 -> + when Code =:= 200 orelse Code =:= 201 orelse Code =:= 204 -> {ok, Return}; {ok, {Reason, _Header, Body}} -> {error, Reason, Body}