wip

src/eradius_client.erl

@@ -62,17 +62,13 @@ send_request(NAS, #{cmd := _, payload := _} = Req) ->
           {ok, eradius_req:req()} | {error, 'timeout' | 'socket_down'}.
 send_request(ServerName, #{cmd := Cmd} = Req, Opts)
   when ?GOOD_CMD(Cmd), is_map(Opts), is_list(ServerName) ->
-    ct:pal("about to send-#1"),
     do_send_request(ServerName, [], Req, Opts);
 send_request(ServerName, Req, Opts) when not is_list(ServerName) ->
-    ct:pal("about to send-#2"),
     send_request([ServerName], Req, Opts).
 
 do_send_request([], _Tried, _Req, _Opts) ->
-    ct:pal("about to send-#3"),
     {error, no_active_servers};
 do_send_request(Peers, Tried, Req0, Opts0) ->
-    ct:pal("about to send-#4"),
     TS1 = erlang:monotonic_time(),
 
     case eradius_client_mngr:wanna_send(Peers, Tried) of
@@ -84,7 +80,6 @@ do_send_request(Peers, Tried, Req0, Opts0) ->
             Opts = maps:merge(ServerOpts, Opts0),
 
             Req1 = maps:merge(Req0, ReqInfo),
-            ct:pal("Client-M#1: ~p", [Req1]),
             Req2 = eradius_req:record_metric(request, #{}, Req1),
 
             {Response, Req} =
@@ -116,9 +111,7 @@ maybe_failover(_, Req, Response, _) ->
 
 %% send_request_loop/4
 send_request_loop(Socket, ReqId, Req0, Opts) ->
-    ct:pal("Packet-#0: ~p", [Req0]),
     {Packet, Req} = eradius_req:packet(Req0),
-    ct:pal("Packet-#1: ~p", [Packet]),
     send_request_loop(Socket, ReqId, Packet, Opts, Req).
 
 %% send_request_loop/8

src/eradius_client_mngr.erl

@@ -97,7 +97,6 @@ start_client(ServerName, Opts) ->
     maybe
         ok ?= check_already_started(ServerName),
         {ok, Config} = client_config(maps:merge(default_client_opts(), Opts)),
-        ct:pal("Mngr Opts: ~p~nConfig: ~p", [Opts, Config]),
         eradius_client_top_sup:start_client([ServerName, Config])
     end.
 

src/eradius_lib.erl

@@ -28,7 +28,11 @@ pad_to(Width, Binary) ->
 timestamp() ->
     erlang:system_time(milli_seconds).
 
--spec printable_peer({inet:ip_address() | any, eradius_server:port_number()}) -> io_lib:chars().
+-spec printable_peer(server_name() | {inet:ip_address() | any, eradius_server:port_number()}) -> io_lib:chars().
+printable_peer(Atom) when is_atom(Atom) ->
+    [atom_to_list(Atom)];
+printable_peer(Binary) when is_binary(Binary) ->
+    [Binary];
 printable_peer({IP, Port}) ->
     printable_peer(IP, Port).
 

src/eradius_metrics_prometheus.erl

@@ -9,6 +9,7 @@
 -ignore_xref([init/1, reset/0]).
 -ignore_xref([client_metrics_callback/3, server_metrics_callback/3]).
 
+-include_lib("kernel/include/logger.hrl").
 -include("dictionary.hrl").
 -include("eradius_lib.hrl").
 -include("eradius_dict.hrl").
@@ -36,7 +37,6 @@ init_client_metrics(#{histogram_buckets := Buckets, client_metrics := true}) ->
     %%
     %% Client Side Metrics
     %%
-    ct:pal("Doing Client Metrics"),
 
     %% Server Status
     prometheus_boolean:declare(
@@ -159,14 +159,12 @@ init_client_metrics(#{histogram_buckets := Buckets, client_metrics := true}) ->
        {help, "Disconnect execution time"}]),
     ok;
 init_client_metrics(_Config) ->
-    ct:pal("NOT doing Client Metrics: ~p", [_Config]),
     ok.
 
 init_server_metrics(#{histogram_buckets := Buckets, server_metrics := true}) ->
     %%
     %% Server Side Metrics
     %%
-    ct:pal("Doing Server Metrics"),
 
     %% this need a collector...
     %% {uptime_milliseconds, gauge, "RADIUS server uptime"},
@@ -296,7 +294,6 @@ init_server_metrics(#{histogram_buckets := Buckets, server_metrics := true}) ->
        {help, "Disconnect-Request execution time"}]),
     ok;
 init_server_metrics(_Config) ->
-    ct:pal("NOT doing Server Metrics ~p", [_Config]),
     ok.
 
 %%%=========================================================================
@@ -312,8 +309,8 @@ client_metrics_callback(Event, MetaData,
                           server := Server, server_addr := {ServerIP, ServerPort},
                           client := Client, client_addr := ClientIP
                          } = Req) ->
-    ct:pal("Client-Metric:~nEvent: ~p~nMetaData: ~p~nReq: ~p~n",
-           [Event, MetaData, Req]),
+    ?LOG(debug, "Client-Metric:~nEvent: ~p~nMetaData: ~p~nReq: ~p~n",
+         [Event, MetaData, Req]),
 
     Labels = [ServerIP, ServerPort, Server, Client, ClientIP],
     case Event of
@@ -329,8 +326,8 @@ client_metrics_callback(Event, MetaData,
     end,
     Req;
 client_metrics_callback(Event, MetaData, Req) ->
-    ct:pal("BROKEN Client-Metric:~nEvent: ~p~nMetaData: ~p~nReq: ~p~n",
-           [Event, MetaData, Req]),
+    ?LOG(error, "BROKEN Client-Metric:~nEvent: ~p~nMetaData: ~p~nReq: ~p~n",
+         [Event, MetaData, Req]),
     Req.
 
 
@@ -385,8 +382,8 @@ client_reply_metrics(_MetaData, Labels,
                               Req :: eradius_req:req()) -> eradius_req:req().
 server_metrics_callback(Event, MetaData,
                         #{cmd := Cmd, server := Server} = Req) ->
-    ct:pal("Server-Metric:~nEvent: ~p~nMetaData: ~p~nReq: ~p~n",
-           [Event, MetaData, Req]),
+    ?LOG(debug, "Server-Metric:~nEvent: ~p~nMetaData: ~p~nReq: ~p~n",
+         [Event, MetaData, Req]),
     case Event of
         request ->
             prometheus_counter:inc(eradius_requests_total, [Server], 1),
@@ -454,16 +451,14 @@ server_metrics_callback(invalid_request, #{server := Server} = _MetaData, _) ->
     prometheus_counter:inc(eradius_invalid_requests_total, [Server], 1),
     ok;
 server_metrics_callback(Event, MetaData, Req) ->
-    ct:pal("BROKEN Server-Metric:~nEvent: ~p~nMetaData: ~p~nReq: ~p~n",
-           [Event, MetaData, Req]),
+    ?LOG(error, "BROKEN Server-Metric:~nEvent: ~p~nMetaData: ~p~nReq: ~p~n",
+         [Event, MetaData, Req]),
     Req.
 
 
 acct_status_type(#{attrs := Attrs}) when is_list(Attrs) ->
-    ct:pal("AcctStatusTypeList: ~p", [Attrs]),
     acct_status_type_list(Attrs);
 acct_status_type(#{body := Body}) when is_binary(Body) ->
-    ct:pal("AcctStatusTypeBin: ~p", [Body]),
     acct_status_type_scan(Body);
 acct_status_type(_) ->
     invalid.
@@ -478,10 +473,8 @@ acct_status_type_list([_|Next]) ->
     acct_status_type_list(Next).
 
 acct_status_type_scan(<<?Acct_Status_Type, 6, Type:32, _/binary>>) ->
-    ct:pal("acct_status_type_scan-#1: ~p", [Type]),
     acct_status_type_label(Type);
 acct_status_type_scan(<<_, Len, Rest/binary>>) ->
-    ct:pal("acct_status_type_scan-#2: ~p, ~p", [Len, Rest]),
     case Rest of
         <<_:(Len-2)/bytes, Next/binary>> ->
             acct_status_type_scan(Next);

src/eradius_req.erl

@@ -149,28 +149,37 @@ packet(Req) ->
     erlang:error(badarg, [Req]).
 
 -spec attrs(req()) -> {eradius_lib:attribute_list(), req()} | no_return().
-attrs(#{attrs := Attrs, is_valid := true} = Req)
-  when is_list(Attrs) ->
+attrs(#{attrs := Attrs, is_valid := IsValid} = Req)
+  when is_list(Attrs), IsValid =/= false ->
     {Attrs, Req};
-attrs(#{attrs := Attrs, is_valid := IsValid})
-  when is_list(Attrs), IsValid =/= true ->
-    throw({bad_pdu, invalid});
-attrs(#{body := Body, secret := _} = Req)
+attrs(#{body := Body, secret := _} = Req0)
   when is_binary(Body) ->
-    try decode_body(Body, Req)
+    try
+        #{attrs := Attrs} = Req = decode_body(Body, Req0),
+        {Attrs, Req}
     catch
         exit:_ ->
             throw({bad_pdu, decoder_error})
     end;
 attrs(Req) ->
     erlang:error(badarg, [Req]).
 
-attr(Id, #{attrs := Attrs, is_valid := true})
-  when is_list(Attrs) ->
-    proplists:get_value(Id, Attrs, undefined);
+attr(Id, #{attrs := Attrs, is_valid := IsValid})
+  when is_list(Attrs), IsValid =/= false ->
+    get_attr(Id, Attrs);
 attr(_, _) ->
     undefined.
 
+get_attr(_Id, []) ->
+    undefined;
+get_attr(Id, [Head|Tail]) ->
+    case Head of
+        {#attribute{id = Id}, Value} -> Value;
+        {Id, Value} -> Value;
+        _ -> get_attr(Id, Tail)
+    end.
+
+
 -spec new(command()) -> req().
 new(Command) ->
     new(Command, undefined).
@@ -198,10 +207,14 @@ request(<<Cmd, ReqId, Len:16, Authenticator:16/bytes>> = Header, Body,
            Req#{req_id => ReqId, request_authenticator => Authenticator,
                 nas_id => NasId, secret => Secret}).
 
--spec response(binary(), binary(), req()) -> req() | no_return().
+-spec response(binary(), binary(), req()) -> req() | no_return();
+              (command(), undefined | eradius_lib:attribute_list(), req()) -> req().
 response(<<Cmd, ReqId, Len:16, Authenticator:16/bytes>> = Header, Body,
          #{req_id := ReqId, secret := _} = Req) ->
-    mk_req(Cmd, ReqId, Len, Authenticator, Header, Body, Req).
+    mk_req(Cmd, ReqId, Len, Authenticator, Header, Body, Req);
+
+response(Response, Attrs, Req) when is_atom(Response) ->
+    Req#{cmd := Response, body := undefined, attrs := Attrs, is_valid := undefined}.
 
 -spec set_secret(req(), eradius_lib:secret()) -> req().
 set_secret(Req, Secret) ->

src/eradius_server.erl

@@ -69,7 +69,6 @@
                 server         :: {inet:ip_address() | any, inet:port_number()}, % IP and port to which this socket is bound
                 active_n       :: non_neg_integer(),
                 transacts      :: ets:tid(),         % ETS table containing current transactions
-                counter        :: #server_counter{}, % statistics counter,
                 handler        :: eradius_server_mon:handler(),
                 metrics_callback :: eradius_req:metrics_callback(),
                 clients        :: #{inet:ip_address() => eradius_client()}
@@ -146,7 +145,6 @@ init([#{server_name := ServerName,
                    handler = Handler,
                    clients = Clients,
                    transacts = ets:new(transacts, []),
-                   counter = eradius_counter:init_counter({IP, Port, ServerName}),
                    metrics_callback = MetricsCallback
                   },
             {ok, State};
@@ -157,12 +155,8 @@ init([#{server_name := ServerName,
     end.
 
 %% @private
-handle_call({stats, pull}, _From, State = #state{counter = Counter}) ->
-    {reply, Counter, State#state{counter = eradius_counter:reset_counter(Counter)}};
-handle_call({stats, read}, _From, State = #state{counter = Counter}) ->
-    {reply, Counter, State};
-handle_call({stats, reset}, _From, State = #state{counter = Counter}) ->
-    {reply, ok, State#state{counter = eradius_counter:reset_counter(Counter)}}.
+handle_call(_Call, _From, State) ->
+    {reply, ok, State}.
 
 %% @private
 handle_cast(_Msg, State) ->
@@ -178,7 +172,6 @@ handle_info({udp, Socket, FromIP, FromPortNo, <<Header:20/bytes, Body/binary>>}
                    handler = Handler, clients = Clients,
                    metrics_callback = MetricsCallback} = State)
   when is_map_key(FromIP, Clients) ->
-    ct:pal("Good-UDP: ~p", [Msg]),
     NAS = maps:get(FromIP, Clients),
     <<_, ReqId:8, _/binary>> = Header,
     Req0 = eradius_req:request(Header, Body, NAS, MetricsCallback),
@@ -192,7 +185,6 @@ handle_info({udp, Socket, FromIP, FromPortNo, <<Header:20/bytes, Body/binary>>}
         [] ->
             HandlerPid =
                 proc_lib:spawn_link(?MODULE, do_radius, [self(), Handler, ReqKey, Req1]),
-            ct:pal("do_radius-pid: ~p", [HandlerPid]),
             ets:insert(Transacts, {ReqKey, {handling, HandlerPid}}),
             ets:insert(Transacts, {HandlerPid, ReqKey}),
             %% eradius_counter:inc_counter(pending, Req);
@@ -221,7 +213,6 @@ handle_info({udp, Socket, FromIP, FromPortNo, <<Header:20/bytes, Body/binary>>}
 handle_info({udp, _Socket, _FromIP, _FromPortNo, _Packet} = Msg,
             #state{name = ServerName, metrics_callback = MetricsCallback} = State) ->
     %% TBD: this should go into a malformed counter
-    ct:pal("Invalid-UDP: ~p", [Msg]),
     eradius_req:metrics_callback(MetricsCallback, invalid_request, #{server => ServerName}),
     flow_control(State),
     {noreply, State};
@@ -234,7 +225,6 @@ handle_info({replied, ReqKey, HandlerPid}, State = #state{transacts = Transacts}
     {noreply, State};
 
 handle_info({'EXIT', HandlerPid, _Reason}, State = #state{transacts = Transacts}) ->
-    ct:pal("do_radius_exit: ~p", [_Reason]),
     [ets:delete(Transacts, ReqKey) || {_, ReqKey} <- ets:take(Transacts, HandlerPid)],
     {noreply, State};
 
@@ -243,7 +233,6 @@ handle_info(_Info, State) ->
 
 %% @private
 terminate(_Reason, State) ->
-    ct:pal("Terminate: ~p", [_Reason]),
     gen_udp:close(State#state.socket),
     ok.
 
@@ -259,7 +248,6 @@ code_change(_OldVsn, State, _Extra) -> {ok, State}.
 do_radius(ServerPid, {HandlerMod, HandlerArg}, ReqKey,
           #{arrival_time := TS1, socket := Socket, server := Server,
             nas := {NasIP, NasPort} = Nas} = Req0) ->
-    ct:pal("do_radius: ~p", [Req0]),
     case apply_handler_mod(HandlerMod, HandlerArg, Req0) of
         {reply, Packet, Resp0, Req} ->
             ?LOG(debug, "~s From: ~s INF: Sending response for request ~0p",
@@ -313,22 +301,22 @@ apply_handler_mod(HandlerMod, HandlerArg,
             {Packet, Resp} = eradius_req:packet(Resp0),
             {reply, Packet, Resp, Req};
         noreply ->
-            ?LOG(error, "~s INF: Noreply for request ~p from handler ~p: returned value: ~p",
+            ?LOG(error, "~ts INF: Noreply for request ~tp from handler ~tp: returned value: ~tp",
                  [printable_peer(Server), ReqId, HandlerArg, noreply]),
             {discard, handler_returned_noreply};
         {error, timeout} ->
             ReqType = eradius_log:format_cmd(Cmd),
             ReqId = integer_to_list(ReqId),
-            ?LOG(error, "~s INF: Timeout after waiting for response to ~s(~s) from RADIUS NAS: ~s",
+            ?LOG(error, "~ts INF: Timeout after waiting for response to ~ts(~ts) from RADIUS NAS: ~s",
                  [printable_peer(Server), ReqType, ReqId, inet:ntoa(NasIP)]),
             {discard, {bad_return, {error, timeout}}};
         OtherReturn ->
-            ?LOG(error, "~s INF: Unexpected return for request ~0p from handler ~p: returned value: ~p",
+            ?LOG(error, "~ts INF: Unexpected return for request ~0tp from handler ~tp: returned value: ~tp",
                  [printable_peer(Server), ReqId, HandlerArg, OtherReturn]),
             {discard, {bad_return, OtherReturn}}
     catch
         Class:Reason:S ->
-            ?LOG(error, "~s INF: Handler crashed after request ~p, radius handler class: ~p, reason of crash: ~p, stacktrace: ~p",
+            ?LOG(error, "~ts INF: Handler crashed after request ~0tp, radius handler class: ~0tp, reason of crash: ~0tp, stacktrace: ~0tp",
                  [printable_peer(Server), ReqId, Class, Reason, S]),
             {exit, {Class, Reason}}
     end.

src/eradius_sup.erl

@@ -20,8 +20,6 @@ init([]) ->
     SupFlags = {RestartStrategy, MaxRestarts, MaxSecondsBetweenRestarts},
 
     DictServer   = {dict, {eradius_dict, start_link, []}, permanent, brutal_kill, worker, [eradius_dict]},
-    StatsServer  = {counter, {eradius_counter, start_link, []}, permanent, brutal_kill, worker, [eradius_counter]},
-    StatsCollect = {aggregator, {eradius_counter_aggregator, start_link, []}, permanent, brutal_kill, worker, [eradius_counter_aggregator]},
     RadiusLog    = {radius_log, {eradius_log, start_link, []}, permanent, brutal_kill, worker, [eradius_log]},
     ServerSup =
         #{id => server_sup,
@@ -38,5 +36,4 @@ init([]) ->
           type => supervisor,
           modules => [eradius_client_top_sup]},
 
-    {ok, {SupFlags, [DictServer, StatsServer, StatsCollect, RadiusLog,
-                     ServerSup, ClientTopSup]}}.
+    {ok, {SupFlags, [DictServer, RadiusLog, ServerSup, ClientTopSup]}}.