Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[CENNSO-2033] rework logging #219

Merged
merged 4 commits into from
May 15, 2024
Merged

[CENNSO-2033] rework logging #219

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c961d15
[ct] change the outstanding_requests setting in the Rf and Gy suite
RoadRunnr May 13, 2024
56a6f71
[log] rework logging of EXIT and DOWN events in session
RoadRunnr May 13, 2024
853828c
[log] clean up logging
RoadRunnr May 13, 2024
f605150
[srv] change handling of reference counting underflow
RoadRunnr May 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 17 additions & 21 deletions src/ergw_aaa_diameter_srv.erl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ call(App, Request, #{function := Function} = Config, CallOpts) ->
encode ->
?LOG(critical, "failed to encode DIAMETER requests: ~0p", [Request]);
Other ->
?LOG(notice, "non-critical diameter API: ~p", [Other])
?LOG(notice, "non-critical diameter API return: ~p", [Other])
end,
SI = diameter:service_info(Function, applications),
handle_plain_error(Result, Request, Function, App, CallOpts, SI);
Expand Down Expand Up @@ -250,8 +250,8 @@ handle_call({start_request, SvcName, Peer, RPid}, _From, #state{peers = Peers0}
{reply, Reply, State#state{peers = Peers}};

handle_call({finish_request, SvcName, Peer, RPid}, _From, #state{peers = Peers0} = State) ->
{Reply, Peers} = finish_request_h(SvcName, Peer, RPid, Peers0),
{reply, Reply, State#state{peers = Peers}};
Peers = finish_request_h(SvcName, Peer, RPid, Peers0),
{reply, ok, State#state{peers = Peers}};

handle_call(peers_info, _, #state{peers = Peers} = State) ->
{reply, Peers, State}.
Expand All @@ -264,19 +264,11 @@ handle_cast(stop, State) ->

handle_info({'DOWN', MRef, _Type, Pid, Info}, #state{peers = Peers0} = State)
when is_map_key(MRef, Peers0) ->
?LOG(alert, "got down for pending request ~0p (~p) with ~p",
[maps:get(MRef, Peers0), Pid, Info]),
?LOG(alert, "Diameter request process terminated unexpected with ~0tp (req: ~0tp, pid: ~0p)",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://www.erlang.org/doc/apps/kernel/logger_chapter.html#log-level

which "action must be taken immediatly"? is something dead already? is everything about to be shut down?

[Info, maps:get(MRef, Peers0), Pid]),

{_, _, PeerRef, Caps} = maps:get(MRef, Peers0),
{Result, Peers} = release_peer(PeerRef, Caps, maps:without([Pid, MRef], Peers0)),
case Result of
{error, Error} ->
?LOG(critical, "release peer failed with ~0p", [Error]),
ct:fail("release peer failed with ~0p", [Error]),
ok;
ok ->
ok
end,
Peers = release_peer(PeerRef, Caps, maps:without([Pid, MRef], Peers0)),
ets:match_delete(?MODULE, {{'_', Pid}, MRef}),
{noreply, State#state{peers = Peers}};

Expand All @@ -285,7 +277,8 @@ handle_info({'DOWN', MRef, _Type, Pid, _Info}, State) ->
{noreply, State};

handle_info(Info, State) ->
?LOG(warning, "handle_info: ~p", [Info]),
?LOG(alert, "unexpected info message, something important might have been missed: ~p",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://www.erlang.org/doc/apps/kernel/logger_chapter.html#log-level

what shall the user of the software incorporating ergw_aaa do with this message?
the component of the software using this library is not visible here, thus the user of the software is unable to deduce in which area a problem is, especially when the description of the loglevel "alert" is "action must be taken immediately".

[Info]),
{noreply, State}.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It used to be recommended in a gen_server to have a cluse handle_info of unknown content. Perhaps it is no longer a good idea.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd keep it in notice, like "this is not normal but it's not causing an error". It would avoid a crash. But I don't see this proposal as much better anyway 🤷🏻

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Production code should IMHO not get call/cast/info message that are not handled. Sure, it is always possible to send something though a remote shell, but I would that not call a normal situation.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@javiermtorres @ebengt I would be ok with a policy to always add catch all clauses for those with logs at level alert (or even emergency). That would make them noisy enough to catch them, but still be somewhat safe.

Very high log levels are IMHO justified because missing a info/cast/call could mean that something important was missed and it is impossible so determine if it was important without understanding the message.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I'd be fine with a high log level. The alternative is a crash (if we omit the catchall clause) that is equally visible.

Since infos catch any sent message, it's easier to stumble upon issues of processes sending messages to the wrong pid than calls/casts. I'd agree to not using catchall clauses for calls/casts.

terminate(_Reason, _State) ->
Expand Down Expand Up @@ -389,11 +382,13 @@ start_request_h(SvcName, {PeerRef, #diameter_caps{origin_host = {_, OH}} = Caps
capacity = Cap,
tokens = Tokens} = Peer,
if Cnt >= Cap ->
?LOG(debug, "peer ~0p over capacity (~p > ~p)", [OH, Cnt, Cap]),
{{discard, rate_limit}, Peers0#{OH => inc_stats(no_tokens, Peer)}};
Tokens == 0 ->
?LOG(debug, "peer ~0p over rate", [OH]),
?LOG(notice,
"Diameter peer ~0p traffic is over the configured outstanding request capacity (~p > ~p)",
[OH, Cnt, Cap]),
{{discard, rate_limit}, Peers0#{OH => inc_stats(no_capacity, Peer)}};
Tokens == 0 ->
?LOG(notice, "Diameter peer ~0p traffic is over the configured rate", [OH]),
{{discard, rate_limit}, Peers0#{OH => inc_stats(no_tokens, Peer)}};
true ->
?LOG(debug, "outstanding inc: ~p", [Cnt + 1]),
Peers1 =
Expand All @@ -415,9 +410,10 @@ release_peer_oh(#diameter_caps{origin_host = {_, OH}}, Peers) ->
case Peer of
#peer{outstanding = Cnt} when Cnt > 0 ->
?LOG(debug, "outstanding dec #1: ~p", [Cnt - 1]),
{ok, Peers#{OH => Peer#peer{outstanding = Cnt - 1}}};
Peers#{OH => Peer#peer{outstanding = Cnt - 1}};
_ ->
{{error, underflow}, Peers#{OH => Peer}}
?LOG(emergency, "reference counting underflow for Diameter peer ~0tp", [OH]),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://www.erlang.org/doc/apps/kernel/logger_chapter.html#log-level

the whole system became just unusable because the some diameter-peers vanished?

Peers#{OH => Peer}
end.

release_peer(PeerRef, Caps, Peers) ->
Expand Down
19 changes: 9 additions & 10 deletions src/ergw_aaa_gx.erl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,14 +164,14 @@ handle_response(_Promise, _Msg, Session, Events, _Opts, State) ->
%%===================================================================

%% peer_up/3
peer_up(_SvcName, _Peer, State) ->
?LOG(debug, "peer_up: ~p~n", [_Peer]),
peer_up(_SvcName, {_, #diameter_caps{origin_host = {OH, _}}}, State) ->
?LOG(info, "Diameter peer FSM for ~p changed to state UP", [OH]),
State.

%% peer_down/3
peer_down(SvcName, Peer, State) ->
peer_down(SvcName, {_, #diameter_caps{origin_host = {OH, _}}} = Peer, State) ->
ergw_aaa_diameter_srv:peer_down(?MODULE, SvcName, Peer),
?LOG(debug, "peer_down: ~p~n", [Peer]),
?LOG(info, "Diameter peer FSM for ~p changed to state DOWN", [OH]),
State.

%% pick_peer/5
Expand Down Expand Up @@ -218,9 +218,11 @@ prepare_retransmit(_Pkt, _SvcName, _Peer, _CallOpts) ->

%% handle_answer/5
handle_answer(#diameter_packet{msg = Msg, errors = Errors},
_Request, SvcName, Peer, _CallOpts)
when length(Errors) /= 0 ->
?LOG(error, "~p: decode of answer from ~p failed, errors ~p", [SvcName, Peer, Errors]),
_Request, SvcName, {_, #diameter_caps{origin_host = {OH, _}}} = Peer, _CallOpts)
when length(Errors) /= 0 ->
%% the exact content of Errors is a bit unclear, dumping them is best we can do
?LOG(error, "~0tp: decode of Diameter answer ~0tp from ~0tp failed, errors ~0tp",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://www.erlang.org/doc/apps/kernel/logger_chapter.html#log-level

the operator of the software using ergw_aaa is bombarded with faulty encoded diameter messages. ergw_aaa deals with this by … just not accepting those messages. what is the "error"? from the perspective of the operator everything is running just fine.

from a developer perspective i can understand "error", but from a user perspective which needs to see those logs within normal operations i do not see it. in this area, an increase of an error-metric plus a log message on debug-level (which the operator can activate on demand to find out the peer involved, the message, the error itself).

[SvcName, Msg, OH, Errors]),
ok = ergw_aaa_diameter_srv:finish_request(SvcName, Peer),
case Msg of
[_ | #{'Result-Code' := RC}] -> {error, RC}; %% try to handle gracefully
Expand Down Expand Up @@ -303,7 +305,6 @@ handle_cca({error, Code} = Result, Session, Events, _Opts, State)
when is_integer(Code) ->
{Result, Session, [{stop, {?API, peer_reject}} | Events], State#state{state = stopped}};
handle_cca({error, Reason} = Result, Session, Events, _Opts, State) ->
?LOG(error, "CCA Result: ~p", [Result]),
{Result, Session, [{stop, {?API, Reason}} | Events], State#state{state = stopped}}.

%% handle_cca/7
Expand Down Expand Up @@ -625,10 +626,8 @@ to_session(Procedure, SessEvs, Avps) ->
to_session(_, 'Usage-Monitoring-Information', Value, SessEv) ->
lists:foldl(fun umi_to_session/2, SessEv, Value);
to_session(_, 'Charging-Rule-Install', V, {Session, Events}) ->
[?LOG(info, "CRI: ~p", [R]) || R <- V],
{Session, [{pcc, install, V} | Events]};
to_session(_, 'Charging-Rule-Remove', V, {Session, Events}) ->
[?LOG(info, "CRI: ~p", [R]) || R <- V],
{Session, [{pcc, remove, V} | Events]};
to_session(_, _, _, SessEv) ->
SessEv.
Expand Down
25 changes: 14 additions & 11 deletions src/ergw_aaa_nasreq.erl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ initialize_service(_ServiceId, #{function := Function, accounting := AcctModel})
{dictionary, ?DIAMETER_DICT_NASREQ},
{module, ?MODULE}]
| Appl]},
?LOG(debug, "Registering NASREQ service: ~p", [{Function, SvcOpts}]),
?LOG(info, "Registering NASREQ service: ~p", [{Function, SvcOpts}]),
ergw_aaa_diameter_srv:register_service(Function, SvcOpts),
{ok, []}.

Expand Down Expand Up @@ -150,7 +150,6 @@ invoke(_Service, interim, Session, Events, Opts, #state{state = started} = State
invoke(_Service, Procedure, Session, Events, Opts,
#state{state = started, authorized = Authorized} = State0)
when Procedure =:= stop; Procedure =:= terminate ->
?LOG(debug, "Session ~s: ~p", [Procedure, Session]),
State1 = inc_record_number(State0#state{state = stopped}),
RecType = ?'DIAMETER_SGI_ACCOUNTING-RECORD-TYPE_STOP_RECORD',
App = acct_app_alias(Opts),
Expand Down Expand Up @@ -209,14 +208,14 @@ handle_response('STR', Msg, Session, Events, Opts, State) ->
%%===================================================================

%% peer_up/3
peer_up(_SvcName, _Peer, State) ->
?LOG(debug, "peer_up: ~p~n", [_Peer]),
peer_up(_SvcName, {_, #diameter_caps{origin_host = {OH, _}}}, State) ->
?LOG(info, "Diameter peer FSM for ~p changed to state UP", [OH]),
State.

%% peer_down/3
peer_down(SvcName, Peer, State) ->
peer_down(SvcName, {_, #diameter_caps{origin_host = {OH, _}}} = Peer, State) ->
ergw_aaa_diameter_srv:peer_down(?MODULE, SvcName, Peer),
?LOG(debug, "peer_down: ~p~n", [Peer]),
?LOG(info, "Diameter peer FSM for ~p changed to state DOWN", [OH]),
State.

%% pick_peer/5
Expand Down Expand Up @@ -275,9 +274,11 @@ prepare_retransmit(_Pkt, _SvcName, _Peer, _CallOpts) ->

%% handle_answer/5
handle_answer(#diameter_packet{msg = Msg, errors = Errors},
_Request, SvcName, Peer, _CallOpts)
_Request, SvcName, {_, #diameter_caps{origin_host = {OH, _}}} = Peer, _CallOpts)
when length(Errors) /= 0 ->
?LOG(error, "~p: decode of answer ~p from ~p failed, errors ~p", [SvcName, Msg, Peer, Errors]),
%% the exact content of Errors is a bit unclear, dumping them is best we can do
?LOG(error, "~0tp: decode of Diameter answer ~0tp from ~0tp failed, errors ~0tp",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see my remarks before about error-log for diameter decoding errors.

[SvcName, Msg, OH, Errors]),
ok = ergw_aaa_diameter_srv:finish_request(SvcName, Peer),
case Msg of
[_ | #{'Result-Code' := RC}] -> {error, RC}; %% try to handle gracefully
Expand All @@ -299,8 +300,10 @@ handle_error(Reason, _Request, SvcName, Peer, CallOpts) ->
handle_request(#diameter_packet{msg = [Command | Avps]}, _SvcName, Peer)
when Command =:= 'ASR'; Command =:= 'RAR' ->
handle_common_request(Command, Avps, Peer);
handle_request(_Packet, _SvcName, _Peer) ->
erlang:error({unexpected, ?MODULE, ?LINE}).
handle_request(Packet, SvcName, {_, #diameter_caps{origin_host = {OH, _}}}) ->
?LOG(error, "~0tp: unsupported Diameter request from peer ~p, raw request ~0tp",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see my remarks about error logs for such messages before.

with this error-log entry the door is open for external parties to flood the log with whatever they want.

especially since an answer-message with 3001 is sent back to the originator of the diameter-request: an operator can see the increase of "3001" answers in the metrics and if he is interested in he will tcpdump the situation to see the involved party pooper.

[SvcName, OH, Packet]),
{answer_message, 3001}. %% DIAMETER_COMMAND_UNSUPPORTED

%%%===================================================================
%%% Options Validation
Expand Down Expand Up @@ -619,7 +622,7 @@ handle_common_request(Command, #{'Session-Id' := SessionId} = Avps, {_PeerRef, C
'Session-Id' => SessionId},
ReplyCode = diameter_reply_code(Command),
ReplyAvps = diameter_reply_avps(Result, ReplyAvps2),
?LOG(error, "~p reply Avps: ~p", [Command, ReplyAvps]),
?LOG(debug, "~p reply Avps: ~p", [Command, ReplyAvps]),
{reply, [ReplyCode | ReplyAvps]}.

filter_reply_avps('RAR', Avps) ->
Expand Down
17 changes: 9 additions & 8 deletions src/ergw_aaa_radius.erl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,8 @@ validate_host(Opt, Host)
{ok, #hostent{h_addr_list = [IP | _]}} ->
IP;
_ ->
?LOG(error, "can't resolve remote RADIUS server name '~s'", [Host]),
?LOG(emergency, "unable to resolve remote RADIUS server name '~s' to IPv4 address",
[Host]),
erlang:error(badarg, [Opt, Host])
end;
validate_host(_Opt, {_,_,_,_} = IP) ->
Expand Down Expand Up @@ -810,26 +811,26 @@ to_session_opts({#attribute{name = "X_" ++ Name} = Attr, Value}, SOpts) ->
to_session_opts({#attribute{name = Name} = Attr, Value}, SOpts) ->
to_session_opts(Attr, catch (list_to_existing_atom(Name)), Value, SOpts);
to_session_opts({Attr, Value}, SOpts) ->
?LOG(debug, "unhandled undecoded reply AVP: ~0p: ~0p", [Attr, Value]),
?LOG(warning, "unsupported, undecoded AVP in reply: ~0tp: ~0tp", [Attr, Value]),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://www.erlang.org/doc/apps/kernel/logger_chapter.html#log-level

i rather see a metric here and the log-level being "debug": an operator sees the increase in the metrics and can then activate debug-level as soon as he is ready.

the warning would be what? "dear operator, i warn you about that i will … continue to not decode the unsupported AVP in whatever that other party sent". this is on a per-message level and thus out of the scope of someone using the software incorporating ergw_aaa.

the component / context is missing in the log message ("aaa", "ergw_aaa", "aaa:radius" etc)

SOpts.

%% Service-Type = Framed-User
to_session_opts(_Attr, 'Service-Type', 2, SOpts) ->
SOpts#{'Service-Type' => 'Framed-User'};

to_session_opts(_Attr, 'Service-Type', Value, _SOpts) ->
?LOG(debug, "unexpected Value in Service-Type: ~p", [Value]),
throw(?AAA_ERR(?FATAL));
to_session_opts(_Attr, 'Service-Type', Value, SOpts) ->
?LOG(warning, "unsupported value for RADIUS Service-Type in reply: ~p", [Value]),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see my remarks as before.

debug-level is fine imho.

SOpts;

%% Framed-Protocol = PPP
to_session_opts(_Attr, 'Framed-Protocol', 1, SOpts) ->
SOpts#{'Framed-Protocol' => 'PPP'};
%% Framed-Protocol = GPRS-PDP-Context
to_session_opts(_Attr, 'Framed-Protocol', 7, SOpts) ->
SOpts#{'Framed-Protocol' => 'GPRS-PDP-Context'};
to_session_opts(_Attr, 'Framed-Protocol', Value, _SOpts) ->
?LOG(debug, "unexpected Value in Framed-Protocol: ~p", [Value]),
throw(?AAA_ERR(?FATAL));
to_session_opts(_Attr, 'Framed-Protocol', Value, SOpts) ->
?LOG(warning, "unsupported value for RADIUS Framed-Protocol in reply: ~p", [Value]),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see my remarks as before.

debug-level is fine.

SOpts;

%% Alc-Primary-Dns
to_session_opts(_Attr, 'Alc-Primary-DNS', DNS, SOpts) ->
Expand Down
6 changes: 3 additions & 3 deletions src/ergw_aaa_radius_handler.erl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,9 @@ radius_request(#radius_request{cmd = discreq, attrs = Attrs} = Req, _NasProp, _A
end,
{reply, Req#radius_request{cmd = Cmd}};

radius_request(#radius_request{} = Request, _NasProp, _Args) ->
?LOG(warning, "Unhandled radius request: ~p", [Request]),
{reply, Request}.
radius_request(#radius_request{}, _NasProp, _Args) ->
%% unsupported requests, eradius_server will logs the `noreply` as an error, no need to duplicate that
noreply.

to_session_id(HexSessionId) ->
H = fun(N) when N < 58 -> N - 48;
Expand Down
14 changes: 8 additions & 6 deletions src/ergw_aaa_rf.erl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,14 +190,14 @@ handle_response(_Promise, _Msg, Session, Events, _Opts, State) ->
%%===================================================================

%% peer_up/3
peer_up(_SvcName, _Peer, State) ->
?LOG(debug, "peer_up: ~p~n", [_Peer]),
peer_up(_SvcName, {_, #diameter_caps{origin_host = {OH, _}}}, State) ->
?LOG(info, "Diameter peer FSM for ~p changed to state UP", [OH]),
State.

%% peer_down/3
peer_down(SvcName, Peer, State) ->
peer_down(SvcName, {_, #diameter_caps{origin_host = {OH, _}}} = Peer, State) ->
ergw_aaa_diameter_srv:peer_down(?MODULE, SvcName, Peer),
?LOG(debug, "peer_down: ~p~n", [Peer]),
?LOG(info, "Diameter peer FSM for ~p changed to state DOWN", [OH]),
State.

%% pick_peer/5
Expand Down Expand Up @@ -231,9 +231,11 @@ prepare_retransmit(_Pkt, _SvcName, _Peer, _CallOpts) ->

%% handle_answer/5
handle_answer(#diameter_packet{msg = Msg, errors = Errors},
_Request, SvcName, Peer, _CallOpts)
_Request, SvcName, {_, #diameter_caps{origin_host = {OH, _}}} = Peer, _CallOpts)
when length(Errors) /= 0 ->
?LOG(error, "~p: decode of answer from ~p failed, errors ~p", [SvcName, Peer, Errors]),
%% the exact content of Errors is a bit unclear, dumping them is best we can do
?LOG(error, "~0tp: decode of Diameter answer ~0tp from ~0tp failed, errors ~0tp",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see my previous comments on that

[SvcName, Msg, OH, Errors]),
ok = ergw_aaa_diameter_srv:finish_request(SvcName, Peer),
case Msg of
[_ | #{'Result-Code' := RC}] -> {error, RC}; %% try to handle gracefully
Expand Down
25 changes: 12 additions & 13 deletions src/ergw_aaa_ro.erl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,14 +190,14 @@ handle_response(_Promise, _Msg, Session, Events, _Opts, State) ->
%%===================================================================

%% peer_up/3
peer_up(_SvcName, _Peer, State) ->
?LOG(debug, "peer_up: ~p~n", [_Peer]),
peer_up(_SvcName, {_, #diameter_caps{origin_host = {OH, _}}}, State) ->
?LOG(info, "Diameter peer FSM for ~p changed to state UP", [OH]),
State.

%% peer_down/3
peer_down(SvcName, Peer, State) ->
peer_down(SvcName, {_, #diameter_caps{origin_host = {OH, _}}} = Peer, State) ->
ergw_aaa_diameter_srv:peer_down(?MODULE, SvcName, Peer),
?LOG(debug, "peer_down: ~p~n", [Peer]),
?LOG(info, "Diameter peer FSM for ~p changed to state DOWN", [OH]),
State.

%% pick_peer/5
Expand Down Expand Up @@ -232,9 +232,11 @@ prepare_retransmit(_Pkt, _SvcName, _Peer, _CallOpts) ->

%% handle_answer/5
handle_answer(#diameter_packet{msg = Msg, errors = Errors},
_Request, SvcName, Peer, CallOpts)
_Request, SvcName, {_, #diameter_caps{origin_host = {OH, _}}} = Peer, CallOpts)
when length(Errors) /= 0 ->
?LOG(error, "~p: decode of answer from ~p failed, errors ~p", [SvcName, Peer, Errors]),
%% the exact content of Errors is a bit unclear, dumping them is best we can do
?LOG(error, "~0tp: decode of Diameter answer ~0tp from ~0tp failed, errors ~0tp",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see my previous comment on that

[SvcName, Msg, OH, Errors]),
ok = ergw_aaa_diameter_srv:finish_request(SvcName, Peer),
Code =
case Msg of
Expand Down Expand Up @@ -276,9 +278,9 @@ maybe_retry(Reason, SvcName, Peer, CallOpts) ->
handle_request(#diameter_packet{msg = [Command | Avps]}, _SvcName, Peer)
when Command =:= 'ASR'; Command =:= 'RAR' ->
handle_common_request(Command, Avps, Peer);
handle_request(_Packet, _SvcName, {_PeerRef, _Caps} = _Peer) ->
?LOG(error, "~p:handle_request(~p, ~p, ~p)",
[?MODULE, _Packet, _SvcName, _Caps]),
handle_request(Packet, SvcName, {_, #diameter_caps{origin_host = {OH, _}}}) ->
?LOG(error, "~0tp: unsupported Diameter request from peer ~p, raw request ~0tp",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see my previous comment on that

[SvcName, OH, Packet]),
{answer_message, 3001}. %% DIAMETER_COMMAND_UNSUPPORTED

%%%===================================================================
Expand Down Expand Up @@ -359,7 +361,6 @@ handle_cca({error, Code} = Result, Session, Events, _Opts, State)
when is_integer(Code) ->
{Result, Session, [{stop, {?API, peer_reject}} | Events], State#state{state = stopped}};
handle_cca({error, Reason} = Result, Session, Events, _Opts, State) ->
?LOG(error, "CCA Result: ~p", [Result]),
{Result, Session, [{stop, {?API, Reason}} | Events], State#state{state = stopped}}.

handle_common_request(Command, #{'Session-Id' := SessionId} = Avps, {_PeerRef, Caps}) ->
Expand Down Expand Up @@ -768,11 +769,9 @@ request_credits(Session, MSCC) ->
Credits = maps:get(credits, Session, #{}),
maps:fold(
fun(RatingGroup, empty, Request) ->
?LOG(warning, "Ro Charging Key: ~p", [RatingGroup]),
javiermtorres marked this conversation as resolved.
Show resolved Hide resolved
RSU = [{'Requested-Service-Unit', #{}}],
merge_mscc(RatingGroup, RSU, Request);
(RatingGroup, _, Request) ->
?LOG(error, "unknown Ro Rating Group: ~p", [RatingGroup]),
(_, _, Request) ->
javiermtorres marked this conversation as resolved.
Show resolved Hide resolved
Request
end, MSCC, Credits).

Expand Down
Loading