wip

travelping · Apr 26, 2024 · f1da28a · f1da28a
1 parent 823d0d1
commit f1da28a
Show file tree

Hide file tree

Showing 4 changed files with 60 additions and 35 deletions.
diff --git a/src/eradius_client.erl b/src/eradius_client.erl
@@ -60,10 +60,9 @@ send_request(NAS, #{cmd := _, payload := _} = Req) ->
 %%   If no answer is received within the specified timeout, the request will be sent again.
 -spec send_request(eradius_client_mngr:server_name(), eradius_req:req(), options()) ->
           {ok, eradius_req:req()} | {error, 'timeout' | 'socket_down'}.
-send_request(ServerName, #{cmd := Cmd} = Req, Opts0)
-  when ?GOOD_CMD(Cmd), is_map(Opts0), is_list(ServerName) ->
+send_request(ServerName, #{cmd := Cmd} = Req, Opts)
+  when ?GOOD_CMD(Cmd), is_map(Opts), is_list(ServerName) ->
     ct:pal("about to send-#1"),
-    Opts = maps:merge(?DEFAULT_REQUEST_OPTS, Opts0),
     do_send_request(ServerName, [], Req, Opts);
 send_request(ServerName, Req, Opts) when not is_list(ServerName) ->
     ct:pal("about to send-#2"),
@@ -72,14 +71,18 @@ send_request(ServerName, Req, Opts) when not is_list(ServerName) ->
 do_send_request([], _Tried, _Req, _Opts) ->
     ct:pal("about to send-#3"),
     {error, no_active_servers};
-do_send_request(Peers, Tried, Req0, Opts) ->
+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
         {ok, {Socket, ReqId, ServerName, Server, ReqInfo}} ->
             #{secret := Secret, ip := IP, port := Port} = Server,
 
+            ServerOpts0 = maps:with([retries, timeout], Server),
+            ServerOpts = maps:merge(?DEFAULT_REQUEST_OPTS, ServerOpts0),
+            Opts = maps:merge(ServerOpts, Opts0),
+
             Req1 = maps:merge(Req0, ReqInfo),
             ct:pal("Client-M#1: ~p", [Req1]),
             Req2 = eradius_req:record_metric(request, #{}, Req1),
@@ -90,7 +93,7 @@ do_send_request(Peers, Tried, Req0, Opts) ->
             proceed_response([ServerName | Tried], Req, Response, ServerName, Server, TS1, Opts);
 
         {error, _} = Error ->
-            maybe_failover(Tried, Req0, Error, Opts)
+            maybe_failover(Tried, Req0, Error, Opts0)
     end.
 
 proceed_response(_Tried, Req, {ok, Resp0}, _ServerName, _, TS1, _Opts) ->

diff --git a/src/eradius_metrics_prometheus.erl b/src/eradius_metrics_prometheus.erl
@@ -8,6 +8,7 @@
 
 -include("dictionary.hrl").
 -include("eradius_lib.hrl").
+-include("eradius_dict.hrl").
 
 -define(DEFAULT_BUCKETS, [10, 30, 50, 75, 100, 1000, 2000]).
 -define(CONFIG, #{histogram_buckets => ?DEFAULT_BUCKETS,
@@ -317,8 +318,7 @@ client_metrics_callback(Event, MetaData,
             prometheus_counter:inc(eradius_client_requests_total, Labels, 1),
             client_request_metrics(MetaData, Labels, Req);
         retransmission ->
-            prometheus_counter:inc(eradius_client_retransmissions_total, Labels, 1),
-            client_request_metrics(MetaData, Labels, Req);
+            prometheus_counter:inc(eradius_client_retransmissions_total, Labels, 1);
         reply ->
             client_reply_metrics(MetaData, Labels, Req);
         _ ->
@@ -401,6 +401,24 @@ server_metrics_callback(Event, MetaData,
                 _ ->
                     prometheus_counter:inc(eradius_unknown_type_request_total, [Server], 1)
             end;
+        retransmission ->
+            prometheus_counter:inc(eradius_requests_total, [Server], 1),
+            prometheus_counter:inc(eradius_retransmissions_total, [Server], 1);
+        drop ->
+            prometheus_counter:inc(eradius_requests_total, [Server], 1),
+            case MetaData of
+                #{reason := duplicate} ->
+                    prometheus_counter:inc(eradius_duplicated_requests_total, [Server], 1);
+                _ ->
+                    ok
+            end;
+        %% eradius_malformed_requests_total
+        %% eradius_invalid_requests_total
+        %% eradius_retransmissions_total
+        %% eradius_duplicated_requests_total
+        %% eradius_pending_requests_total
+        %% eradius_packets_dropped_total
+        %% eradius_bad_authenticator_request_total
         reply ->
             prometheus_counter:inc(eradius_replies_total, [Server], 1),
             case Cmd of
@@ -425,35 +443,39 @@ server_metrics_callback(Event, MetaData,
                 _ ->
                     ok
             end;
-        %% eradius_malformed_requests_total
-        %% eradius_invalid_requests_total
-        %% eradius_retransmissions_total
-        %% eradius_duplicated_requests_total
-        %% eradius_pending_requests_total
-        %% eradius_packets_dropped_total
-        %% eradius_bad_authenticator_request_total
         _ ->
             ok
     end,
+    Req;
+server_metrics_callback(Event, MetaData, Req) ->
+    ct:pal("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~nFind: ~p",
-           [Attrs, lists:keyfind(?Acct_Status_Type, 1, Attrs)]),
-    case lists:keyfind(?Acct_Status_Type, 1, Attrs) of
-        {_, V} -> acct_status_type_label(V);
-        _ -> invalid
-    end;
+    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.
 
+acct_status_type_list([]) ->
+    invalid;
+acct_status_type_list([{?Acct_Status_Type, Type}|_]) ->
+    acct_status_type_label(Type);
+acct_status_type_list([{#attribute{id = ?Acct_Status_Type}, Type}|_]) ->
+    acct_status_type_label(Type);
+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);

diff --git a/src/eradius_server.erl b/src/eradius_server.erl
@@ -188,16 +188,16 @@ handle_info({udp, Socket, FromIP, FromPortNo, <<Header:20/bytes, Body/binary>>}
                  nas => {FromIP, FromPortNo}},
     ReqKey = {FromIP, FromPortNo, ReqId},
     %% eradius_counter:inc_counter(requests, Req),
-    Req = eradius_req:record_metric(request, #{}, Req1),
 
     case ets:lookup(Transacts, ReqKey) of
         [] ->
             HandlerPid =
-                proc_lib:spawn_link(?MODULE, do_radius, [self(), Handler, ReqKey, Req]),
+                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);
+            Req = eradius_req:record_metric(request, #{}, Req1),
             eradius_req:record_metric(pending, #{}, Req);
 
         [{_ReqKey, {handling, HandlerPid}}] ->
@@ -207,7 +207,7 @@ handle_info({udp, Socket, FromIP, FromPortNo, <<Header:20/bytes, Body/binary>>}
                  [printable_peer(Server),
                   printable_peer(FromIP, FromPortNo), HandlerPid, ReqKey]),
             %% eradius_counter:inc_counter(dupRequests, Req);
-            eradius_req:record_metric(drop, #{reason => duplicate}, Req);
+            eradius_req:record_metric(drop, #{reason => duplicate}, Req1);
         [{_ReqKey, {replied, HandlerPid}}] ->
             %% handler process waiting for resend message
             HandlerPid ! {self(), resend, Socket},
@@ -217,7 +217,7 @@ handle_info({udp, Socket, FromIP, FromPortNo, <<Header:20/bytes, Body/binary>>}
                   printable_peer(FromIP, FromPortNo), HandlerPid, ReqKey]),
             %% eradius_counter:inc_counter(dupRequests, Req),
             %% eradius_counter:inc_counter(retransmissions, Req)
-            eradius_req:record_metric(retransmissions, #{reason => duplicate}, Req)
+            eradius_req:record_metric(retransmissions, #{reason => duplicate}, Req1)
     end,
     flow_control(State),
     {noreply, State};

diff --git a/test/eradius_metrics_SUITE.erl b/test/eradius_metrics_SUITE.erl
@@ -44,11 +44,10 @@ all() -> [good_requests, bad_requests, error_requests, request_with_attrs_as_rec
 init_per_suite(Config) ->
     application:load(eradius),
     application:set_env(eradius, unreachable_timeout, 2),
-    {ok, _} = application:ensure_all_started([prometheus, eradius]),
+    {ok, _} = application:ensure_all_started([eradius]),
 
     ok = start_client(Config),
     ok = start_servers(Config),
-    eradius_metrics_prometheus:init(#{}),
 
     Config.
 
@@ -58,7 +57,9 @@ end_per_suite(_Config) ->
     ok.
 
 init_per_testcase(_, Config) ->
-    eradius_metrics_prometheus:reset(),
+    application:stop(prometheus),
+    {ok, _} = application:ensure_all_started([prometheus]),
+    eradius_metrics_prometheus:init(#{}),
     Config.
 
 %%%===================================================================
@@ -138,7 +139,7 @@ error_requests(_Config) ->
 request_with_attrs_as_record(_Config) ->
     ok = send_request(good, accreq, ?ATTRS_AS_RECORD,
                       #{server_name => good, client_name => test_records}),
-    check_metric(accreq, eradius_client_accounting_requests_total, [{"server_name", good}, {client_name, test_records}, {"acct_type", start}], 1).
+    check_metric(accreq, eradius_client_accounting_requests_total, [{"server_name", good}, {"acct_type", start}], 1).
 
 %% helpers
 check_single_request(good, EradiusRequestType, _RequestType, _ResponseType) ->
@@ -170,18 +171,18 @@ check_single_request(bad, EradiusRequestType, _RequestType, _ResponseType) ->
 check_single_request(error, EradiusRequestType, _RequestType, _ResponseType) ->
     ok = send_request(error, EradiusRequestType, ?ATTRS_ERROR,
                       #{server_name => error, client_name => test, timeout => 100,
-                        failover => [good]}),
+                        failover => []}),
     check_metric(eradius_client_access_requests_total, [{"server_name", error}], 1),
     check_metric(eradius_client_retransmissions_total, [{"server_name", error}], 1),
     check_metric(eradius_access_requests_total, [{"server_name", error}], 1),
     check_metric(eradius_accept_responses_total, [{"server_name", error}], 1),
     check_metric(eradius_duplicated_requests_total, [{"server_name", error}], 1),
     check_metric(eradius_client_requests_total, [{"server_name", error}], 1),
     check_metric(eradius_requests_total, [{"server_name", error}], 2),
-    check_metric(eradius_server_status, false, [eradius_test_lib:localhost(tuple), 1812]),
-    check_metric(eradius_server_status, false, [eradius_test_lib:localhost(tuple), 1813]),
+    check_metric(eradius_server_status, undefined, [eradius_test_lib:localhost(tuple), 1812]),
+    check_metric(eradius_server_status, undefined, [eradius_test_lib:localhost(tuple), 1813]),
     check_metric(eradius_server_status, true, [eradius_test_lib:localhost(tuple), 1814]),
-    check_metric(eradius_server_status, undefined, [eradius_test_lib:localhost(tuple), 1815]).
+    ok.
 
 check_total_requests(good, N) ->
     check_metric(eradius_requests_total, [{"server_name", good}], N),
@@ -222,8 +223,7 @@ check_metric(_, _, _, _) ->
 
 check_metric(eradius_server_status = Id, Value, LabelValues) ->
     ct:pal("check_metric-#0 ~p: ~p, ~p", [Id, LabelValues, Value]),
-    ?assertEqual(Value, prometheus_boolean:value(Id, LabelValues)),
-    ok;
+    ?assertEqual(Value, prometheus_boolean:value(Id, LabelValues));
 check_metric(Id, Labels, Count) ->
     Values = prometheus_counter:values(default, Id),
     Filtered =
@@ -272,5 +272,5 @@ radius_request(#{cmd := discreq, nas_id := <<"bad_nas">>} = Req, _) ->
 
 %% RADIUS NAS callbacks for 'error' requests
 radius_request(#{cmd := request, nas_id := <<"error_nas">>} = Req, _) ->
-    timer:sleep(1500), %% this will by default trigger one resend
+    timer:sleep(150), %% this will by default trigger one resend
     {reply, Req#{cmd := accept}}.