From 706520e04ee00293730e8090d68c74f777ad51f5 Mon Sep 17 00:00:00 2001 From: Anton Belyaev Date: Sat, 16 Apr 2016 22:11:54 +0300 Subject: [PATCH] Rename request IDs and http headers in accordance with Dapper terms --- src/rpc_client.erl | 26 ++++++++-------- src/rpc_t.erl | 6 ++-- src/rpc_thrift_client.erl | 4 +-- src/rpc_thrift_http_handler.erl | 6 ++-- src/rpc_thrift_http_headers.hrl | 8 ++--- src/rpc_thrift_http_transport.erl | 34 ++++++++++----------- test/rpc_tests_SUITE.erl | 50 +++++++++++++++---------------- 7 files changed, 68 insertions(+), 66 deletions(-) diff --git a/src/rpc_client.erl b/src/rpc_client.erl index 29b03d7..48ee89c 100644 --- a/src/rpc_client.erl +++ b/src/rpc_client.erl @@ -16,6 +16,8 @@ -export_type([client/0, options/0, result_ok/0, result_error/0]). +-define(ROOT_REQ_PARENT_ID, <<"undefined">>). + %% Internal API -export([init_call_async/4, do_call_async/4]). @@ -31,14 +33,14 @@ %% -type client() :: #{ %% all elements are mandatory root_rpc => boolean(), - req_id => rpc_t:req_id() | undefined, - root_req_id => rpc_t:req_id(), - parent_req_id => rpc_t:req_id(), + span_id => rpc_t:req_id() | undefined, + trace_id => rpc_t:req_id(), + parent_id => rpc_t:req_id(), event_handler => rpc_t:handler(), seq => non_neg_integer() }. --type result_ok() :: {ok, _Reply, client()} | {ok, client()}. +-type result_ok() :: {ok, _Response, client()} | {ok, client()}. -type result_error() :: {error, rpc_failed, client()} | {throw, _Exception, client()}. -type request() :: any(). @@ -55,20 +57,20 @@ new(ReqId, EventHandler) -> #{ root_rpc => true, - req_id => ReqId, - root_req_id => ReqId, - parent_req_id => <<"undefined">>, + span_id => ReqId, + trace_id => ReqId, + parent_id => ?ROOT_REQ_PARENT_ID, seq => 0, event_handler => EventHandler }. -spec make_child_client(rpc_t:rpc_id(), rpc_t:handler()) -> client(). -make_child_client(#{req_id := ReqId, root_req_id := RootReqId}, EventHandler) -> +make_child_client(#{span_id := ReqId, trace_id := TraceId}, EventHandler) -> #{ root_rpc => false, - req_id => undefined, - root_req_id => RootReqId, - parent_req_id => ReqId, + span_id => undefined, + trace_id => TraceId, + parent_id => ReqId, seq => 0, event_handler => EventHandler }. @@ -78,7 +80,7 @@ next(Client = #{root_rpc := true}) -> Client; next(Client = #{root_rpc := false, seq := Seq}) -> NextSeq = Seq +1, - Client#{req_id => make_req_id(NextSeq), seq => NextSeq}. + Client#{span_id => make_req_id(NextSeq), seq => NextSeq}. -spec call(client(), request(), options()) -> result_ok() | no_return(). call(Client, Request, Options) -> diff --git a/src/rpc_t.erl b/src/rpc_t.erl index a24bcd5..f305ed9 100644 --- a/src/rpc_t.erl +++ b/src/rpc_t.erl @@ -8,9 +8,9 @@ -type req_id() :: binary(). -type rpc_id() :: #{ - req_id => rpc_id(), - root_req_id => rpc_id(), - parent_req_id => rpc_id() + span_id => rpc_id(), + trace_id => rpc_id(), + parent_id => rpc_id() }. -type options() :: map(). diff --git a/src/rpc_thrift_client.erl b/src/rpc_thrift_client.erl index eea8583..b0d01cb 100644 --- a/src/rpc_thrift_client.erl +++ b/src/rpc_thrift_client.erl @@ -33,7 +33,7 @@ stop_pool(Name) -> call(Client = #{event_handler := EventHandler}, {Service, Function, Args}, TransportOpts = #{url := Url}) -> - RpcId = maps:with([req_id, root_req_id, parent_req_id], Client), + RpcId = maps:with([span_id, trace_id, parent_id], Client), rpc_event_handler:handle_event(EventHandler, send_request, RpcId#{ rpc_role => client, direction => request, @@ -42,7 +42,7 @@ call(Client = #{event_handler := EventHandler}, function => Function, args => Args }), - Result = do_call(make_thrift_client( RpcId, Service, TransportOpts), Function, Args), + Result = do_call(make_thrift_client(RpcId, Service, TransportOpts), Function, Args), rpc_event_handler:handle_event(EventHandler, receive_response, RpcId#{ rpc_role => client, direction => response, diff --git a/src/rpc_thrift_http_handler.erl b/src/rpc_thrift_http_handler.erl index 2156c11..194870d 100644 --- a/src/rpc_thrift_http_handler.erl +++ b/src/rpc_thrift_http_handler.erl @@ -54,9 +54,9 @@ -define(event_rpc_receive, receive_request). -define(HEADERS_RPC_ID, #{ - req_id => ?HEADER_NAME_RPC_ID, - root_req_id => ?HEADER_NAME_RPC_ROOT_ID, - parent_req_id => ?HEADER_NAME_RPC_PARENT_ID + span_id => ?HEADER_NAME_RPC_ID, + trace_id => ?HEADER_NAME_RPC_ROOT_ID, + parent_id => ?HEADER_NAME_RPC_PARENT_ID }). diff --git a/src/rpc_thrift_http_headers.hrl b/src/rpc_thrift_http_headers.hrl index f7a9f23..ae4546d 100644 --- a/src/rpc_thrift_http_headers.hrl +++ b/src/rpc_thrift_http_headers.hrl @@ -3,10 +3,10 @@ -define(CONTENT_TYPE_THRIFT , <<"application/x-thrift">>). --define(HEADER_NAME_RPC_ID , <<"x-rbk-rpc-id">>). --define(HEADER_NAME_RPC_PARENT_ID , <<"x-rbk-rpc-id-parent">>). --define(HEADER_NAME_RPC_ROOT_ID , <<"x-rbk-rpc-id-root">>). --define(HEADER_NAME_ERROR_TRANSPORT , <<"x-rbk-rpc-error-transport">>). +-define(HEADER_NAME_RPC_ID , <<"x-rbk-span-id">>). +-define(HEADER_NAME_RPC_PARENT_ID , <<"x-rbk-parent-id">>). +-define(HEADER_NAME_RPC_ROOT_ID , <<"x-rbk-root-id">>). +-define(HEADER_NAME_ERROR_TRANSPORT , <<"x-rbk-rpc-error-thrift">>). -define(HEADER_NAME_ERROR_LOGIC , <<"x-rbk-rpc-error-logic">>). -endif. diff --git a/src/rpc_thrift_http_transport.erl b/src/rpc_thrift_http_transport.erl index 9c17e7e..3bae3a9 100644 --- a/src/rpc_thrift_http_transport.erl +++ b/src/rpc_thrift_http_transport.erl @@ -18,13 +18,13 @@ -export_type([url/0]). -type rpc_transport() :: #{ - req_id => rpc_t:req_id(), - root_req_id => rpc_t:req_id(), - parent_req_id => rpc_t:req_id(), - url => url(), - options => map(), - write_buffer => binary(), - read_buffer => binary() + span_id => rpc_t:req_id(), + trace_id => rpc_t:req_id(), + parent_id => rpc_t:req_id(), + url => url(), + options => map(), + write_buffer => binary(), + read_buffer => binary() }. @@ -73,13 +73,13 @@ read(Transport = #{read_buffer := RBuffer}, Len) when -spec flush(rpc_transport()) -> {rpc_transport(), ok | {error, _Reason}}. flush(Transport = #{ - url := Url, - req_id := ReqId, - root_req_id := RootReqId, - parent_req_id := PaReqId, - options := Options, - write_buffer := WBuffer, - read_buffer := RBuffer + url := Url, + span_id := SpanId, + trace_id := TraceId, + parent_id := ParentId, + options := Options, + write_buffer := WBuffer, + read_buffer := RBuffer }) when is_binary(WBuffer), is_binary(RBuffer) @@ -87,9 +87,9 @@ flush(Transport = #{ Headers = [ {<<"content-type">> , ?CONTENT_TYPE_THRIFT}, {<<"accept">> , ?CONTENT_TYPE_THRIFT}, - {?HEADER_NAME_RPC_ROOT_ID , genlib:to_binary(RootReqId)}, - {?HEADER_NAME_RPC_ID , genlib:to_binary(ReqId)}, - {?HEADER_NAME_RPC_PARENT_ID , genlib:to_binary(PaReqId)} + {?HEADER_NAME_RPC_ROOT_ID , genlib:to_binary(TraceId)}, + {?HEADER_NAME_RPC_ID , genlib:to_binary(SpanId)}, + {?HEADER_NAME_RPC_PARENT_ID , genlib:to_binary(ParentId)} ], case send(Url, Headers, WBuffer, Options) of {ok, Response} -> diff --git a/test/rpc_tests_SUITE.erl b/test/rpc_tests_SUITE.erl index 4b6b917..89f758f 100644 --- a/test/rpc_tests_SUITE.erl +++ b/test/rpc_tests_SUITE.erl @@ -105,7 +105,7 @@ all() -> call_handle_error_fails_test, call_oneway_void_test, call_async_ok_test, - check_req_ids_sequence_test, + check_span_ids_sequence_test, call_two_services_test, call_with_client_pool_test, multiplexed_transport_test @@ -209,7 +209,7 @@ call_safe_handler_throw_unexpected_test(_) -> Id = <<"call_safe_handler_throw_unexpected">>, Current = genlib_map:get(<<"Rocket Launcher">>, ?WEAPONS), Client = get_client(Id), - Expect = {error, rpc_failed, Client#{req_id => Id}}, + Expect = {error, rpc_failed, Client#{span_id => Id}}, Expect = call_safe(Client, weapons, switch_weapon, [Current, next, 1, self_to_bin()]), {ok, _} = receive_msg({Id, Current}). @@ -218,7 +218,7 @@ call_handler_throw_unexpected_test(_) -> Id = <<"call_handler_throw_unexpected">>, Current = genlib_map:get(<<"Rocket Launcher">>, ?WEAPONS), Client = get_client(Id), - Expect = {rpc_failed, Client#{req_id => Id}}, + Expect = {rpc_failed, Client#{span_id => Id}}, try call(Client, weapons, switch_weapon, [Current, next, 1, self_to_bin()]) catch error:Expect -> ok @@ -245,7 +245,7 @@ call_safe_server_transport_error_test(_) -> Id = <<"call_safe_server_transport_error">>, Armor = <<"Helmet">>, Client = get_client(Id), - Expect = {error, rpc_failed, Client#{req_id => Id}}, + Expect = {error, rpc_failed, Client#{span_id => Id}}, Expect = call_safe(Client, powerups, get_powerup, [Armor, self_to_bin()]), {ok, _} = receive_msg({Id, Armor}). @@ -259,7 +259,7 @@ call_handle_error_fails_test(_) -> do_call_server_transport_error(Id) -> Armor = <<"Helmet">>, Client = get_client(Id), - Expect = {rpc_failed, Client#{req_id => Id}}, + Expect = {rpc_failed, Client#{span_id => Id}}, try call(Client, powerups, get_powerup, [Armor, self_to_bin()]) catch error:Expect -> ok @@ -270,7 +270,7 @@ call_oneway_void_test(_) -> Id = <<"call_oneway_void_test">>, Armor = <<"Helmet">>, Client = get_client(Id), - Expect = {ok, ok, Client#{req_id => Id}}, + Expect = {ok, ok, Client#{span_id => Id}}, Expect = call(Client, powerups, like_powerup, [Armor, self_to_bin()]), {ok, _} = receive_msg({Id, Armor}). @@ -280,11 +280,11 @@ call_async_ok_test(C) -> Callback = fun(Res) -> collect(Res, Pid) end, Id1 = <<"call_async_ok1">>, Client1 = get_client(Id1), - Client11 = Client1#{req_id => Id1}, + Client11 = Client1#{span_id => Id1}, {ok, Pid1, Client11} = get_weapon(Client1, Sup, Callback, <<"Impact Hammer">>), Id2 = <<"call_async_ok2">>, Client2 = get_client(Id2), - Client22 = Client2#{req_id => Id2}, + Client22 = Client2#{span_id => Id2}, {ok, Pid2, Client22} = get_weapon(Client2, Sup, Callback, <<"Flak Cannon">>), {ok, Pid1} = receive_msg({Client11, genlib_map:get(<<"Impact Hammer">>, ?WEAPONS)}), {ok, Pid2} = receive_msg({Client22, genlib_map:get(<<"Flak Cannon">>, ?WEAPONS)}). @@ -295,11 +295,11 @@ get_weapon(Client, Sup, Cb, Gun) -> collect({ok, Result, Tag}, Pid) -> send_msg(Pid, {Tag, Result}). -check_req_ids_sequence_test(_) -> - Id = <<"check_req_ids_sequence">>, +check_span_ids_sequence_test(_) -> + Id = <<"check_span_ids_sequence">>, Current = genlib_map:get(<<"Enforcer">>, ?WEAPONS), Client = get_client(Id), - Expect = {ok, genlib_map:get(<<"Ripper">>, ?WEAPONS), Client#{req_id => Id}}, + Expect = {ok, genlib_map:get(<<"Ripper">>, ?WEAPONS), Client#{span_id => Id}}, Expect = call(Client, weapons, switch_weapon, [Current, next, 1, self_to_bin()]). @@ -309,7 +309,7 @@ call_two_services_test(_) -> Id = <<"two_services2">>, Armor = <<"Body Armor">>, Client = get_client(Id), - Expect = {ok, genlib_map:get(<<"Body Armor">>, ?POWERUPS), Client#{req_id => Id}}, + Expect = {ok, genlib_map:get(<<"Body Armor">>, ?POWERUPS), Client#{span_id => Id}}, Expect = call_safe(Client, powerups, get_powerup, [Armor, self_to_bin()]), {ok, _} = receive_msg({Id, Armor}). @@ -320,7 +320,7 @@ call_with_client_pool_test(_) -> Gun = <<"Enforcer">>, Client = get_client(Id), {Url, Service} = get_service_endpoint(weapons), - Expect = {ok, genlib_map:get(Gun, ?WEAPONS), Client#{req_id => Id}}, + Expect = {ok, genlib_map:get(Gun, ?WEAPONS), Client#{span_id => Id}}, Expect = rpc_client:call( Client, {Service, get_weapon, [Gun, self_to_bin()]}, @@ -342,7 +342,7 @@ make_thrift_multiplexed_client(Id, ServiceName, {Url, Service}) -> {ok, Protocol} = thrift_binary_protocol:new( rpc_thrift_http_transport:new( #{ - req_id => Id, root_req_id => Id, parent_req_id => Id + span_id => Id, trace_id => Id, parent_id => Id }, #{url => Url} ), @@ -366,16 +366,16 @@ init(_) -> %% %% Weapons -handle_function(switch_weapon, RpcClient = #{parent_req_id := PaReqId}, +handle_function(switch_weapon, RpcClient = #{parent_id := ParentId}, {CurrentWeapon, Direction, Shift, To}, _Opts ) -> - send_msg(To, {PaReqId, CurrentWeapon}), + send_msg(To, {ParentId, CurrentWeapon}), switch_weapon(CurrentWeapon, Direction, Shift, RpcClient); -handle_function(get_weapon, #{parent_req_id := PaReqId}, +handle_function(get_weapon, #{parent_id := ParentId}, {Name, To}, _Opts) -> - send_msg(To,{PaReqId,Name}), + send_msg(To,{ParentId,Name}), Res = case genlib_map:get(Name, ?WEAPONS) of #weapon{ammo = 0} -> throw(?weapon_failure("out of ammo")); @@ -385,14 +385,14 @@ handle_function(get_weapon, #{parent_req_id := PaReqId}, {ok, Res}; %% Powerups -handle_function(get_powerup, #{parent_req_id := PaReqId}, {Name, To}, _Opts) -> - send_msg(To, {PaReqId, Name}), +handle_function(get_powerup, #{parent_id := ParentId}, {Name, To}, _Opts) -> + send_msg(To, {ParentId, Name}), {ok, genlib_map:get(Name, ?POWERUPS, powerup_unknown)}; -handle_function(like_powerup, #{parent_req_id := PaReqId}, {Name, To}, _Opts) -> - send_msg(To, {PaReqId, Name}), +handle_function(like_powerup, #{parent_id := ParentId}, {Name, To}, _Opts) -> + send_msg(To, {ParentId, Name}), ok. -handle_error(get_powerup, #{parent_req_id := <<"call_handle_error_fails">>}, _, _) -> +handle_error(get_powerup, #{parent_id := <<"call_handle_error_fails">>}, _, _) -> error(no_more_powerups); handle_error(_Function, _RpcClient, _Reason, _Opts) -> ok. @@ -445,7 +445,7 @@ get_service_endpoint(powerups) -> gun_test_bacic(CallFun, Id, Gun, {ExpectStatus, ExpectRes}, WithMsg) -> Client = get_client(Id), - Expect = {ExpectStatus, ExpectRes, Client#{req_id => Id}}, + Expect = {ExpectStatus, ExpectRes, Client#{span_id => Id}}, Expect = ?MODULE:CallFun(Client, weapons, get_weapon, [Gun, self_to_bin()]), case WithMsg of true -> {ok, _} = receive_msg({Id, Gun}); @@ -454,7 +454,7 @@ gun_test_bacic(CallFun, Id, Gun, {ExpectStatus, ExpectRes}, WithMsg) -> gun_catch_test_basic(Id, Gun, {Class, Exception}, WithMsg) -> Client = get_client(Id), - Expect = {Exception, Client#{req_id => Id}}, + Expect = {Exception, Client#{span_id => Id}}, try call(Client, weapons, get_weapon, [Gun, self_to_bin()]) catch Class:Expect -> ok