Merge pull request #3800 from esl/c2s/metrics

C2s/metrics
esl · Oct 11, 2022 · e3a74ae · e3a74ae
2 parents 636aa93 + a5c97ac
commit e3a74ae
Show file tree

Hide file tree

Showing 9 changed files with 119 additions and 100 deletions.
diff --git a/big_tests/default.spec b/big_tests/default.spec
@@ -56,10 +56,10 @@
 {suites, "tests", mam_SUITE}.
 {suites, "tests", mam_proper_SUITE}.
 {suites, "tests", mam_send_message_SUITE}.
-% {suites, "tests", metrics_api_SUITE}.
-% {suites, "tests", metrics_c2s_SUITE}.
+{suites, "tests", metrics_api_SUITE}.
+{suites, "tests", metrics_c2s_SUITE}.
 {suites, "tests", metrics_register_SUITE}.
-% {suites, "tests", metrics_roster_SUITE}.
+{suites, "tests", metrics_roster_SUITE}.
 {suites, "tests", metrics_session_SUITE}.
 {suites, "tests", mod_blocking_SUITE}.
 {suites, "tests", mod_event_pusher_http_SUITE}.

diff --git a/big_tests/dynamic_domains.spec b/big_tests/dynamic_domains.spec
@@ -77,15 +77,15 @@
 {suites, "tests", mam_proper_SUITE}.
 {suites, "tests", mam_send_message_SUITE}.
 
-% {suites, "tests", metrics_c2s_SUITE}.
+{suites, "tests", metrics_c2s_SUITE}.
 
 {suites, "tests", metrics_register_SUITE}.
 
-% {suites, "tests", metrics_roster_SUITE}.
+{suites, "tests", metrics_roster_SUITE}.
 
-% {suites, "tests", metrics_session_SUITE}.
+{suites, "tests", metrics_session_SUITE}.
 
-% {suites, "tests", metrics_api_SUITE}.
+{suites, "tests", metrics_api_SUITE}.
 
 {suites, "tests", mod_blocking_SUITE}.
 

diff --git a/big_tests/tests/metrics_api_SUITE.erl b/big_tests/tests/metrics_api_SUITE.erl
@@ -110,14 +110,14 @@ one_client_just_logs_in(Config) ->
         (Config, metrics_helper:userspec(1, Config),
          fun(_User1) -> end_of_story end,
          %% A list of metrics and their expected relative increase
-         [{xmppIqSent, 0},
-          {xmppIqReceived, 0},
+         [{xmppIqSent, 0 + user_alpha(2)},
+          {xmppIqReceived, 0 + user_alpha(2)},
           {xmppMessageSent, 0},
           {xmppMessageReceived, 0},
           {xmppPresenceSent, 0 + user_alpha(1)},
           {xmppPresenceReceived, 0 + user_alpha(1)},
-          {xmppStanzaSent, 0 + user_alpha(1)},
-          {xmppStanzaReceived, 0 + user_alpha(1)},
+          {xmppStanzaSent, 0 + user_alpha(3)},
+          {xmppStanzaReceived, 0 + user_alpha(3)},
           {sessionSuccessfulLogins, 0 + user_alpha(1)},
           {sessionLogouts, 0 + user_alpha(1)}
          ]).
@@ -126,12 +126,14 @@ two_clients_just_log_in(Config) ->
     instrumented_story
         (Config, metrics_helper:userspec(1, 1, Config),
          fun(_User1, _User2) -> end_of_story end,
-         [{xmppMessageSent, 0},
+         [{xmppIqSent, 0 + user_alpha(4)},
+          {xmppIqReceived, 0 + user_alpha(4)},
+          {xmppMessageSent, 0},
           {xmppMessageReceived, 0},
-          {xmppStanzaSent, 0 + user_alpha(2)},
-          {xmppStanzaReceived, 0 + user_alpha(2)},
           {xmppPresenceSent, 0 + user_alpha(2)},
           {xmppPresenceReceived, 0 + user_alpha(2)},
+          {xmppStanzaSent, 0 + user_alpha(6)},
+          {xmppStanzaReceived, 0 + user_alpha(6)},
           {sessionSuccessfulLogins, 0 + user_alpha(2)},
           {sessionLogouts, 0 + user_alpha(2)}
          ]).
@@ -158,8 +160,8 @@ one_direct_presence_sent(Config) ->
         end,
        [{xmppPresenceSent, 1 + user_alpha(2)},
         {xmppPresenceReceived, 1 + user_alpha(2)},
-        {xmppStanzaSent, 1 + user_alpha(2)},
-        {xmppStanzaReceived, 1 + user_alpha(2)}]).
+        {xmppStanzaSent, 1 + user_alpha(6)},
+        {xmppStanzaReceived, 1 + user_alpha(6)}]).
 
 one_iq_sent(Config) ->
     instrumented_story
@@ -169,11 +171,11 @@ one_iq_sent(Config) ->
                escalus_client:send(User1, RosterIq),
                escalus_client:wait_for_stanza(User1)
         end,
-       [{xmppIqSent, 1},
-        {xmppIqReceived, 1},
+       [{xmppIqSent, 3},
+        {xmppIqReceived, 3},
         {modRosterGets, 1},
-        {xmppStanzaSent, 1 + user_alpha(1)},
-        {xmppStanzaReceived, 1 + user_alpha(1)}]).
+        {xmppStanzaSent, 1 + user_alpha(3)},
+        {xmppStanzaReceived, 1 + user_alpha(3)}]).
 
 one_message_error(Config) ->
     instrumented_story
@@ -221,11 +223,13 @@ session_counters(Config) ->
     escalus:story
       (Config, [{alice, 2}, {bob, 1}],
        fun(_User11, _User12, _User2) ->
-               %% Force update
-               lists:foreach(fun metrics_helper:sample/1, Names),
-               ?assertEqual(3, fetch_global_gauge_value(totalSessionCount, Config)),
-               ?assertEqual(2, fetch_global_gauge_value(uniqueSessionCount, Config)),
-               ?assertEqual(3, fetch_global_gauge_value(nodeSessionCount, Config))
+            %% Force update
+            lists:foreach(fun metrics_helper:sample/1, Names),
+            timer:sleep(timer:seconds(1)),
+
+            ?assertEqual(3, fetch_global_gauge_value(totalSessionCount, Config)),
+            ?assertEqual(2, fetch_global_gauge_value(uniqueSessionCount, Config)),
+            ?assertEqual(3, fetch_global_gauge_value(nodeSessionCount, Config))
        end).
 
 node_uptime(Config) ->
@@ -352,7 +356,7 @@ user_alpha(NumberOfUsers) ->
 instrumented_story(Config, UsersSpecs, StoryFun, CounterSpecs) ->
     Befores = fetch_all(Config, CounterSpecs),
     StoryResult = escalus:story(Config, UsersSpecs, StoryFun),
-    Afters =  fetch_all(Config, CounterSpecs),
+    Afters = fetch_all(Config, CounterSpecs),
     [ assert_counter_inc(Name, N, find(Name, Befores), find(Name, Afters))
       || {Name, N} <- CounterSpecs ],
     StoryResult.

diff --git a/big_tests/tests/metrics_c2s_SUITE.erl b/big_tests/tests/metrics_c2s_SUITE.erl
@@ -255,7 +255,6 @@ error_presence(Config) ->
         Presence = escalus_stanza:presence_direct(escalus_client:short_jid(Alice),
                                                   <<"error">>, [ErrorElt]),
         escalus:send(Bob, Presence),
-        escalus:wait_for_stanza(Alice),
 
         wait_for_counter(Errors + 1, xmppErrorPresence)
 

diff --git a/doc/operation-and-maintenance/MongooseIM-metrics.md b/doc/operation-and-maintenance/MongooseIM-metrics.md
@@ -7,11 +7,11 @@ MongooseIM uses [ESL's fork of this project](https://github.com/esl/exometer/tre
 All metrics are divided into the following groups:
 
 * Per host type metrics: Gathered separately for every host type supported by the cluster.
-    
+
     !!! Warning
         If a cluster supports many (thousands or more) host types, performance issues might occur.
         To avoid this, use global equivalents of the metrics with `all_metrics_are_global` config option.
-    
+
     * Hook metrics.
     They are created for every [hook](../developers-guide/Hooks-and-handlers.md) and incremented on every call to it.
 
@@ -137,7 +137,7 @@ As a result it makes more sense to maintain a list of the most relevant or usefu
 | `[HostType, xmppMessageReceived]` | spiral | A message is sent to a client. |
 | `[HostType, xmppPresenceReceived]` | spiral | A presence is sent to a client. |
 | `[HostType, xmppStanzaReceived]` | spiral | A stanza is sent to a client. |
-| `[HostType, xmppStanzaCount]` | spiral | A stanza is sent to a client. |
+| `[HostType, xmppStanzaCount]` | spiral | A stanza is sent to and by a client. |
 | `[HostType, xmppStanzaDropped]` | spiral | A stanza is dropped due to an AMP rule or a `filter_packet` processing flow. |
 
 ### Extension-specific metrics

diff --git a/src/c2s/mongoose_c2s.erl b/src/c2s/mongoose_c2s.erl
@@ -217,6 +217,7 @@ activate_socket(#c2s_data{socket = Socket}) ->
 
 -spec send_text(c2s_data(), iodata()) -> ok | {error, term()}.
 send_text(#c2s_data{socket = Socket}, Text) ->
+    mongoose_metrics:update(global, [data, xmpp, sent, xml_stanza_size], iolist_size(Text)),
     mongoose_c2s_socket:send_text(Socket, Text).
 
 -spec filter_mechanism(c2s_data(), binary()) -> boolean().
@@ -705,13 +706,16 @@ handle_state_result(StateData0, C2SState, MaybeAcc,
     maybe_send_xml(StateData2, MaybeAcc, MaybeSocketSend),
     {next_state, NextFsmState, StateData2, MaybeActions}.
 
+-spec maybe_send_xml(c2s_data(), mongoose_acc:t(), [exml:element()]) -> ok.
 maybe_send_xml(_StateData, _Acc, []) ->
     ok;
 maybe_send_xml(StateData = #c2s_data{host_type = HostType, lserver = LServer}, undefined, ToSend) ->
     Acc = mongoose_acc:new(#{host_type => HostType, lserver => LServer, location => ?LOCATION}),
-    send_element(StateData, ToSend, Acc);
+    send_element(StateData, ToSend, Acc),
+    ok;
 maybe_send_xml(StateData, Acc, ToSend) ->
-    send_element(StateData, ToSend, Acc).
+    send_element(StateData, ToSend, Acc),
+    ok.
 
 -spec maybe_deliver(c2s_data(), gen_hook:hook_fn_ret(mongoose_acc:t())) -> mongoose_acc:t().
 maybe_deliver(StateData, {ok, Acc}) ->
@@ -829,14 +833,16 @@ sm_unset_reason(_) ->
     error.
 
 %% @doc This is the termination point - from here stanza is sent to the user
--spec send_element(c2s_data(), exml:element(), mongoose_acc:t()) -> mongoose_acc:t().
+-spec send_element(c2s_data(), [exml:element()] | exml:element(), mongoose_acc:t()) -> mongoose_acc:t().
 send_element(StateData = #c2s_data{host_type = <<>>}, El, Acc) ->
     send_xml(StateData, El),
     Acc;
-send_element(StateData = #c2s_data{host_type = HostType}, El, Acc) ->
-    Res = send_xml(StateData, El),
+send_element(StateData = #c2s_data{host_type = HostType}, Els, Acc) when is_list(Els) ->
+    Res = send_xml(StateData, Els),
     Acc1 = mongoose_acc:set(c2s, send_result, Res, Acc),
-    mongoose_hooks:xmpp_send_element(HostType, Acc1, El).
+    [mongoose_hooks:xmpp_send_element(HostType, Acc1, El) || El <- Els];
+send_element(StateData, El, Acc) ->
+    send_element(StateData, [El], Acc).
 
 -spec send_xml(c2s_data(), exml_stream:element() | [exml_stream:element()]) -> maybe_ok().
 send_xml(StateData, Xml) ->

diff --git a/src/ejabberd_sm.erl b/src/ejabberd_sm.erl
@@ -448,7 +448,7 @@ terminate_session(Pid, Reason) ->
 %% Hook handlers
 %%====================================================================
 
--spec node_cleanup(Acc, Args, Extra) -> {ok, Acc} when 
+-spec node_cleanup(Acc, Args, Extra) -> {ok, Acc} when
       Acc :: any(),
       Args :: #{node := node()},
       Extra :: map().

diff --git a/src/metrics/mongoose_metrics.erl b/src/metrics/mongoose_metrics.erl
@@ -83,9 +83,8 @@ create_global_metrics() ->
 -spec init_predefined_host_type_metrics(mongooseim:host_type()) -> ok.
 init_predefined_host_type_metrics(HostType) ->
     create_metrics(HostType),
-    Hooks = mongoose_metrics_hooks:get_hooks(HostType),
-    ejabberd_hooks:add(Hooks),
-    ok.
+    ejabberd_hooks:add(mongoose_metrics_hooks:hooks(HostType)),
+    gen_hook:add_handlers(mongoose_metrics_hooks:c2s_hooks(HostType)).
 
 init_subscriptions() ->
     Reporters = exometer_report:list_reporters(),