From a8e34772fe58d88533e20db80dc9bf3eb40e6301 Mon Sep 17 00:00:00 2001 From: Kamil Waz Date: Fri, 7 Oct 2022 14:18:40 +0200 Subject: [PATCH] Fix metrics suite --- big_tests/default.spec | 6 +- big_tests/dynamic_domains.spec | 8 +- big_tests/tests/metrics_api_SUITE.erl | 42 +++--- big_tests/tests/metrics_c2s_SUITE.erl | 1 - .../MongooseIM-metrics.md | 6 +- src/c2s/mongoose_c2s.erl | 8 +- src/ejabberd_sm.erl | 2 +- src/metrics/mongoose_metrics.erl | 5 +- src/metrics/mongoose_metrics_hooks.erl | 131 ++++++++++-------- 9 files changed, 113 insertions(+), 96 deletions(-) diff --git a/big_tests/default.spec b/big_tests/default.spec index 34b16dfb11d..ee9965e06cf 100644 --- 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 index 94c6c9912f3..4b38fc345ae 100644 --- 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 index b07805052ed..39016129ade 100644 --- 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 index 9ad17726738..f23fc3f5c10 100644 --- 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 index fe31040394e..a8146429225 100644 --- 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 index b02a9dd9a1f..70145f6c527 100644 --- 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 index 05a7a538fe4..7f748b532de 100644 --- 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 index 393a4792675..a4df7ce2315 100644 --- 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 index 8b5479415eb..17282dafa35 100644 --- a/src/metrics/mongoose_metrics_hooks.erl +++ b/src/metrics/mongoose_metrics_hooks.erl @@ -11,7 +11,8 @@ -include("mongoose.hrl"). -include("jlib.hrl"). --export([get_hooks/1]). +-export([hooks/1]). +-export([c2s_hooks/1]). %%------------------- %% Internal exports @@ -19,8 +20,8 @@ -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