Fix metrics suite

esl · Oct 10, 2022 · a8e3477 · a8e3477
1 parent 636aa93
commit a8e3477
Show file tree

Hide file tree

Showing 9 changed files with 113 additions and 96 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], 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, El, Acc) || El <- ToSend],
+    ok;
 maybe_send_xml(StateData, Acc, ToSend) ->
-    send_element(StateData, ToSend, Acc).
+    [send_element(StateData, El, Acc) || El <- ToSend],
+    ok.
 
 -spec maybe_deliver(c2s_data(), gen_hook:hook_fn_ret(mongoose_acc:t())) -> mongoose_acc:t().
 maybe_deliver(StateData, {ok, Acc}) ->

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(),

diff --git a/src/metrics/mongoose_metrics_hooks.erl b/src/metrics/mongoose_metrics_hooks.erl
@@ -11,16 +11,17 @@
 -include("mongoose.hrl").
 -include("jlib.hrl").
 
--export([get_hooks/1]).
+-export([hooks/1]).
+-export([c2s_hooks/1]).
 
 %%-------------------
 %% Internal exports
 %%-------------------
 -export([sm_register_connection_hook/5,
          sm_remove_connection_hook/5,
          auth_failed/3,
-         user_send_packet/4,
-         user_receive_packet/5,
+         user_send_packet/3,
+         user_open_session/3,
          xmpp_bounce_message/1,
          xmpp_stanza_dropped/4,
          xmpp_send_element/2,
@@ -40,34 +41,37 @@
              privacy_list_push/5, register_user/3, remove_user/3, roster_get/2,
              roster_in_subscription/5, roster_push/4, roster_set/4,
              sm_register_connection_hook/5, sm_remove_connection_hook/5,
-             user_receive_packet/5, user_send_packet/4, xmpp_bounce_message/1,
-             xmpp_send_element/2, xmpp_stanza_dropped/4]).
+             user_send_packet/3, user_open_session/3,
+             xmpp_bounce_message/1, xmpp_send_element/2, xmpp_stanza_dropped/4]).
 
 %%-------------------
 %% Implementation
 %%-------------------
 
 %% @doc Here will be declared which hooks should be registered
--spec get_hooks(_) -> [ejabberd_hooks:hook(), ...].
-get_hooks(HostType) ->
-    [ {sm_register_connection_hook, HostType, ?MODULE, sm_register_connection_hook, 50},
-      {sm_remove_connection_hook, HostType, ?MODULE, sm_remove_connection_hook, 50},
-      {auth_failed, HostType, ?MODULE, auth_failed, 50},
-      {user_send_packet, HostType, ?MODULE, user_send_packet, 50},
-      {user_receive_packet, HostType, ?MODULE, user_receive_packet, 50},
-      {xmpp_stanza_dropped, HostType, ?MODULE, xmpp_stanza_dropped, 50},
-      {xmpp_bounce_message, HostType, ?MODULE, xmpp_bounce_message, 50},
-      {xmpp_send_element, HostType, ?MODULE, xmpp_send_element, 50},
-      {roster_get, HostType, ?MODULE, roster_get, 55},
-      {roster_set, HostType, ?MODULE, roster_set, 50},
-      {roster_push, HostType, ?MODULE, roster_push, 50},
-      {roster_in_subscription, HostType, ?MODULE, roster_in_subscription, 55},
-      {register_user, HostType, ?MODULE, register_user, 50},
-      {remove_user, HostType, ?MODULE, remove_user, 50},
-      {privacy_iq_get, HostType, ?MODULE, privacy_iq_get, 1},
-      {privacy_iq_set, HostType, ?MODULE, privacy_iq_set, 1},
-      {privacy_check_packet, HostType, ?MODULE, privacy_check_packet, 55},
-      {sm_broadcast, HostType, ?MODULE, privacy_list_push, 1}].
+-spec hooks(mongooseim:host_type()) -> [ejabberd_hooks:hook()].
+hooks(HostType) ->
+    [{sm_register_connection_hook, HostType, ?MODULE, sm_register_connection_hook, 50},
+     {sm_remove_connection_hook, HostType, ?MODULE, sm_remove_connection_hook, 50},
+     {auth_failed, HostType, ?MODULE, auth_failed, 50},
+     {xmpp_stanza_dropped, HostType, ?MODULE, xmpp_stanza_dropped, 50},
+     {xmpp_bounce_message, HostType, ?MODULE, xmpp_bounce_message, 50},
+     {xmpp_send_element, HostType, ?MODULE, xmpp_send_element, 50},
+     {roster_get, HostType, ?MODULE, roster_get, 55},
+     {roster_set, HostType, ?MODULE, roster_set, 50},
+     {roster_push, HostType, ?MODULE, roster_push, 50},
+     {roster_in_subscription, HostType, ?MODULE, roster_in_subscription, 55},
+     {register_user, HostType, ?MODULE, register_user, 50},
+     {remove_user, HostType, ?MODULE, remove_user, 50},
+     {privacy_iq_get, HostType, ?MODULE, privacy_iq_get, 1},
+     {privacy_iq_set, HostType, ?MODULE, privacy_iq_set, 1},
+     {privacy_check_packet, HostType, ?MODULE, privacy_check_packet, 55},
+     {sm_broadcast, HostType, ?MODULE, privacy_list_push, 1}].
+
+-spec c2s_hooks(mongooseim:host_type()) -> gen_hook:hook_list(mongoose_c2s_hooks:hook_fn()).
+c2s_hooks(HostType) ->
+    [{user_send_packet, HostType, fun ?MODULE:user_send_packet/3, #{}, 50},
+     {user_open_session, HostType, fun ?MODULE:user_open_session/3, #{}, 50}].
 
 -spec sm_register_connection_hook(any(), mongooseim:host_type(), tuple(), jid:jid(), term()
                                  ) -> any().
@@ -92,13 +96,14 @@ auth_failed(Acc, _, Server) ->
     mongoose_metrics:update(HostType, sessionAuthFails, 1),
     Acc.
 
--spec user_send_packet(mongoose_acc:t(), jid:jid(), tuple(), tuple()
-                      ) -> mongoose_acc:t().
-user_send_packet(Acc, _, _, Packet) ->
-    HostType = mongoose_acc:host_type(Acc),
+-spec user_send_packet(mongoose_acc:t(), mongoose_c2s:hook_params(), map()) ->
+    mongoose_c2s_hooks:hook_result().
+user_send_packet(Acc, _Params, #{host_type := HostType}) ->
     mongoose_metrics:update(HostType, xmppStanzaSent, 1),
-    user_send_packet_type(HostType, Packet),
-    Acc.
+    mongoose_metrics:update(HostType, xmppStanzaCount, 1),
+    El = mongoose_acc:element(Acc),
+    user_send_packet_type(HostType, El),
+    {ok, Acc}.
 
 -spec user_send_packet_type(HostType :: mongooseim:host_type(),
                             Packet :: exml:element()) -> ok | {error, not_found}.
@@ -109,23 +114,6 @@ user_send_packet_type(HostType, #xmlel{name = <<"iq">>}) ->
 user_send_packet_type(HostType, #xmlel{name = <<"presence">>}) ->
     mongoose_metrics:update(HostType, xmppPresenceSent, 1).
 
--spec user_receive_packet(mongoose_acc:t(), jid:jid(), tuple(), tuple(), tuple()
-                         ) -> mongoose_acc:t().
-user_receive_packet(Acc, _, _, _, Packet) ->
-    HostType = mongoose_acc:host_type(Acc),
-    mongoose_metrics:update(HostType, xmppStanzaReceived, 1),
-    user_receive_packet_type(HostType, Packet),
-    Acc.
-
--spec user_receive_packet_type(HostType :: mongooseim:host_type(),
-                               Packet :: exml:element()) -> ok | {error, not_found}.
-user_receive_packet_type(HostType, #xmlel{name = <<"message">>}) ->
-    mongoose_metrics:update(HostType, xmppMessageReceived, 1);
-user_receive_packet_type(HostType, #xmlel{name = <<"iq">>}) ->
-    mongoose_metrics:update(HostType, xmppIqReceived, 1);
-user_receive_packet_type(HostType, #xmlel{name = <<"presence">>}) ->
-    mongoose_metrics:update(HostType, xmppPresenceReceived, 1).
-
 -spec xmpp_bounce_message(Acc :: mongoose_acc:t()) -> mongoose_acc:t().
 xmpp_bounce_message(Acc) ->
     HostType = mongoose_acc:host_type(Acc),
@@ -139,25 +127,48 @@ xmpp_stanza_dropped(Acc, _, _, _) ->
     mongoose_metrics:update(HostType, xmppStanzaDropped, 1),
     Acc.
 
--spec xmpp_send_element(Acc :: mongoose_acc:t(), Server :: jid:server()) -> mongoose_acc:t().
-xmpp_send_element(Acc, _El) ->
+-spec xmpp_send_element(Acc :: mongoose_acc:t(), El :: exml:element()) -> mongoose_acc:t().
+xmpp_send_element(Acc, #xmlel{name = Name} = El)
+  when Name == <<"iq">>; Name == <<"message">>; Name == <<"presence">> ->
     HostType = mongoose_acc:host_type(Acc),
     mongoose_metrics:update(HostType, xmppStanzaCount, 1),
-    case mongoose_acc:stanza_type(Acc) of
+    case exml_query:attr(El, <<"type">>) of
         <<"error">> ->
             mongoose_metrics:update(HostType, xmppErrorTotal, 1),
-            case mongoose_acc:stanza_name(Acc) of
-                <<"iq">> ->
-                    mongoose_metrics:update(HostType, xmppErrorIq, 1);
-                <<"message">> ->
-                    mongoose_metrics:update(HostType, xmppErrorMessage, 1);
-                <<"presence">> ->
-                    mongoose_metrics:update(HostType, xmppErrorPresence, 1)
-            end;
-        _ -> ok
+            xmpp_send_element_error(HostType, El);
+        _ ->
+            mongoose_metrics:update(HostType, xmppStanzaReceived, 1),
+            xmpp_send_element_type(HostType, El)
     end,
+    Acc;
+xmpp_send_element(Acc, _El) ->
     Acc.
 
+-spec xmpp_send_element_error(HostType :: mongooseim:host_type(),
+                              Packet :: exml:element()) -> ok | {error, not_found}.
+xmpp_send_element_error(HostType, #xmlel{name = <<"message">>}) ->
+    mongoose_metrics:update(HostType, xmppErrorMessage, 1);
+xmpp_send_element_error(HostType, #xmlel{name = <<"iq">>}) ->
+    mongoose_metrics:update(HostType, xmppErrorIq, 1);
+xmpp_send_element_error(HostType, #xmlel{name = <<"presence">>}) ->
+    mongoose_metrics:update(HostType, xmppErrorPresence, 1).
+
+-spec xmpp_send_element_type(HostType :: mongooseim:host_type(),
+                             Packet :: exml:element()) -> ok | {error, not_found}.
+xmpp_send_element_type(HostType, #xmlel{name = <<"message">>}) ->
+    mongoose_metrics:update(HostType, xmppMessageReceived, 1);
+xmpp_send_element_type(HostType, #xmlel{name = <<"iq">>}) ->
+    mongoose_metrics:update(HostType, xmppIqReceived, 1);
+xmpp_send_element_type(HostType, #xmlel{name = <<"presence">>}) ->
+    mongoose_metrics:update(HostType, xmppPresenceReceived, 1).
+
+-spec user_open_session(mongoose_acc:t(), mongoose_c2s:hook_params(), map()) ->
+    mongoose_c2s_hooks:hook_result().
+user_open_session(Acc, _Params, #{host_type := HostType}) ->
+    mongoose_metrics:update(HostType, xmppStanzaSent, 1),
+    mongoose_metrics:update(HostType, xmppStanzaCount, 1),
+    mongoose_metrics:update(HostType, xmppIqSent, 1),
+    {ok, Acc}.
 
 %% Roster