diff --git a/big_tests/default.spec b/big_tests/default.spec index 3f84fbf8dd0..c7018062d90 100644 --- a/big_tests/default.spec +++ b/big_tests/default.spec @@ -62,7 +62,6 @@ {suites, "tests", mam_send_message_SUITE}. {suites, "tests", metrics_api_SUITE}. {suites, "tests", metrics_c2s_SUITE}. -{suites, "tests", metrics_register_SUITE}. {suites, "tests", metrics_roster_SUITE}. {suites, "tests", metrics_session_SUITE}. {suites, "tests", mod_blocking_SUITE}. diff --git a/big_tests/dynamic_domains.spec b/big_tests/dynamic_domains.spec index fe95e729e14..08cbcff5400 100644 --- a/big_tests/dynamic_domains.spec +++ b/big_tests/dynamic_domains.spec @@ -85,8 +85,6 @@ {suites, "tests", metrics_c2s_SUITE}. -{suites, "tests", metrics_register_SUITE}. - {suites, "tests", metrics_roster_SUITE}. {suites, "tests", metrics_session_SUITE}. diff --git a/big_tests/tests/bosh_SUITE.erl b/big_tests/tests/bosh_SUITE.erl index 840de7acee0..d9e3448083a 100644 --- a/big_tests/tests/bosh_SUITE.erl +++ b/big_tests/tests/bosh_SUITE.erl @@ -954,7 +954,7 @@ wait_for_zero_bosh_sessions() -> instrumentation_events() -> instrument_helper:declared_events(mod_bosh, []) ++ instrument_helper:declared_events(mongoose_c2s, [global]) - ++ instrument_helper:declared_events(mongoose_c2s). %% For host_type() + ++ [{c2s_message_processing_time, #{host_type => host_type()}}]. negative_instrumentation_events() -> [{Name, #{}} || Name <- negative_instrumentation_events_names()]. diff --git a/big_tests/tests/connect_SUITE.erl b/big_tests/tests/connect_SUITE.erl index 72d91e9a1aa..f5eefbc40c3 100644 --- a/big_tests/tests/connect_SUITE.erl +++ b/big_tests/tests/connect_SUITE.erl @@ -818,4 +818,4 @@ proxy_info() -> instrumentation_events() -> instrument_helper:declared_events(mongoose_c2s_listener, [#{}]) ++ instrument_helper:declared_events(mongoose_c2s, [global]) - ++ instrument_helper:declared_events(mongoose_c2s). %% For host_type() + ++ [{c2s_message_processing_time, #{host_type => domain_helper:host_type()}}]. diff --git a/big_tests/tests/instrument_helper.erl b/big_tests/tests/instrument_helper.erl index 03ec6058422..7149789df49 100644 --- a/big_tests/tests/instrument_helper.erl +++ b/big_tests/tests/instrument_helper.erl @@ -5,7 +5,7 @@ -export([declared_events/1, declared_events/2, start/1, start/2, stop/0, - assert/3, assert/4, + assert/3, assert/4, filter/2, assert_not_emitted/1, assert_not_emitted/2, wait_for/2, wait_for_new/2, lookup/2, take/2]). @@ -67,9 +67,7 @@ assert(EventName, Labels, CheckF) -> %% This is for convenience - you only have to code one clause. -spec assert(event_name(), labels(), [measurements()], fun((measurements()) -> boolean())) -> ok. assert(EventName, Labels, MeasurementsList, CheckF) -> - case lists:filter(fun(Measurements) -> - try CheckF(Measurements) catch error:function_clause -> false end - end, MeasurementsList) of + case filter(CheckF, MeasurementsList) of [] -> ct:log("All measurements for event ~p with labels ~p:~n~p", [EventName, Labels, MeasurementsList]), @@ -91,6 +89,12 @@ assert_not_emitted(EventName, Labels) -> assert_not_emitted(Events) -> [assert_not_emitted(Event, Label) || {Event, Label} <- Events]. +-spec filter(fun((measurements()) -> boolean()), [measurements()]) -> [measurements()]. +filter(CheckF, MeasurementsList) -> + lists:filter(fun(Measurements) -> + try CheckF(Measurements) catch error:function_clause -> false end + end, MeasurementsList). + %% @doc Remove previous events, and wait for a new one. Use for probes only. -spec wait_for_new(event_name(), labels()) -> [measurements()]. wait_for_new(EventName, Labels) -> diff --git a/big_tests/tests/last_SUITE.erl b/big_tests/tests/last_SUITE.erl index 745376a0962..0c4d37c0470 100644 --- a/big_tests/tests/last_SUITE.erl +++ b/big_tests/tests/last_SUITE.erl @@ -38,7 +38,7 @@ groups() -> valid_test_cases() -> [online_user_query, last_online_user, last_offline_user, - last_server, sessions_cleanup]. + last_server]. invalid_test_cases() -> [user_not_subscribed_receives_error]. @@ -170,7 +170,10 @@ user_not_subscribed_receives_error(Config) -> ok end). -sessions_cleanup(Config) -> +%% This test is disabled on CI, because it puts the system in an inconsistent state. +%% The sessions are created with Pids from the test node, which is incorrect. +%% This incorrectly generates 'sm_session' events, leading to inconsistent session count metrics. +sessions_cleanup(_Config) -> N = distributed_helper:mim(), HostType = domain_helper:host_type(), Server = domain_helper:domain(), @@ -197,10 +200,8 @@ sessions_cleanup(Config) -> {ok, #{timestamp := TS, status := Status} = Data} = distributed_helper:rpc(N, mod_last_api, get_last, [Jid3]), ?assertNotEqual(TS, 1714000000, Data), ?assertEqual(Status, <<>>, Data), - distributed_helper:rpc(N, mongoose_metrics, update, [HostType, sessionCount, -345]), {ok, _} = distributed_helper:rpc(N, mongoose_account_api, unregister_user, [<<"user3">>, Server]). - %%----------------------------------------------------------------- %% Helpers %%----------------------------------------------------------------- diff --git a/big_tests/tests/login_SUITE.erl b/big_tests/tests/login_SUITE.erl index ac498ff536a..2bb61bee1b9 100644 --- a/big_tests/tests/login_SUITE.erl +++ b/big_tests/tests/login_SUITE.erl @@ -277,7 +277,7 @@ log_one_scram_sha224(Config) -> log_one_scram_sha256(Config) -> log_one([{escalus_auth_method, <<"SCRAM-SHA-256">>} | Config]). - log_one_scram_sha384(Config) -> +log_one_scram_sha384(Config) -> log_one([{escalus_auth_method, <<"SCRAM-SHA-384">>} | Config]). log_one_scram_sha512(Config) -> diff --git a/big_tests/tests/metrics_api_SUITE.erl b/big_tests/tests/metrics_api_SUITE.erl index 3a7231cde17..f3972c81d63 100644 --- a/big_tests/tests/metrics_api_SUITE.erl +++ b/big_tests/tests/metrics_api_SUITE.erl @@ -123,8 +123,8 @@ one_client_just_logs_in(Config) -> {xmppPresenceReceived, 0 + user_alpha(1)}, {xmppStanzaSent, 0 + user_alpha(3)}, {xmppStanzaReceived, 0 + user_alpha(3)}, - {sessionSuccessfulLogins, 0 + user_alpha(1)}, - {sessionLogouts, 0 + user_alpha(1)} + {'sm_session.logins', 0 + user_alpha(1)}, + {'sm_session.logouts', 0 + user_alpha(1)} ]). two_clients_just_log_in(Config) -> @@ -139,8 +139,8 @@ two_clients_just_log_in(Config) -> {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)} + {'sm_session.logins', 0 + user_alpha(2)}, + {'sm_session.logouts', 0 + user_alpha(2)} ]). one_message_sent(Config) -> diff --git a/big_tests/tests/metrics_roster_SUITE.erl b/big_tests/tests/metrics_roster_SUITE.erl index 7031a45207e..996bf868266 100644 --- a/big_tests/tests/metrics_roster_SUITE.erl +++ b/big_tests/tests/metrics_roster_SUITE.erl @@ -290,7 +290,7 @@ declared_events() -> declared_backend_events() ++ declared_sm_events() ++ instrument_helper:declared_events(mod_roster). declared_sm_events() -> - [{sm_presence_subscription, #{}}]. + [{sm_presence_subscription, #{host_type => host_type()}}]. declared_backend_events() -> BackendMod = backend_mod(), diff --git a/big_tests/tests/metrics_session_SUITE.erl b/big_tests/tests/metrics_session_SUITE.erl index af0becf44f7..4569a31878a 100644 --- a/big_tests/tests/metrics_session_SUITE.erl +++ b/big_tests/tests/metrics_session_SUITE.erl @@ -17,16 +17,15 @@ -module(metrics_session_SUITE). -compile([export_all, nowarn_export_all]). --include_lib("escalus/include/escalus.hrl"). --include_lib("common_test/include/ct.hrl"). - --define(RT_WINDOW, 3). % seconds +-include_lib("stdlib/include/assert.hrl"). -import(metrics_helper, [assert_counter/2, assert_counter/3, get_counter_value/1, wait_for_counter/2, wait_for_counter/3]). +-import(domain_helper, [host_type/0]). + %%-------------------------------------------------------------------- %% Suite configuration %%-------------------------------------------------------------------- @@ -36,7 +35,7 @@ all() -> {group, session_global}]. groups() -> - [{session, [sequence], [login_one, + [{session, [parallel], [login_one, login_many, auth_failed]}, {session_global, [sequence], [session_global, @@ -50,10 +49,14 @@ suite() -> %%-------------------------------------------------------------------- init_per_suite(Config) -> + instrument_helper:start([{sm_session, #{host_type => host_type()}}, + {c2s_auth_failed, #{host_type => host_type()}}]), escalus:init_per_suite(Config). end_per_suite(Config) -> - escalus:end_per_suite(Config). + escalus_fresh:clean(), + escalus:end_per_suite(Config), + instrument_helper:stop(). init_per_group(_GroupName, Config) -> escalus:create_users(Config, escalus:get_users([alice, bob])). @@ -71,39 +74,31 @@ end_per_testcase(CaseName, Config) -> %% Tests %%-------------------------------------------------------------------- - login_one(Config) -> - {value, Logins} = get_counter_value(sessionSuccessfulLogins), - escalus:story(Config, [{alice, 1}], fun(Alice) -> - - assert_counter(1, sessionCount), - assert_counter(Logins + 1, sessionSuccessfulLogins), - - {value, Logouts} = get_counter_value(sessionLogouts), - escalus_client:stop(Config, Alice), - wait_for_counter(0, sessionCount), - wait_for_counter(Logouts + 1, sessionLogouts) + escalus:fresh_story(Config, [{alice, 1}], fun login_one_story/1). - end). +login_one_story(Alice) -> + assert_sm_login_event(Alice), + sm_helper:stop_client_and_wait_for_termination(Alice), + assert_sm_logout_event(Alice). login_many(Config) -> - {value, Logins} = get_counter_value(sessionSuccessfulLogins), - escalus:story(Config, [{alice, 1}, {bob, 1}], fun(_Alice, _Bob) -> + escalus:fresh_story(Config, [{alice, 1}, {bob, 1}], fun login_many_story/2). - assert_counter(2, sessionCount), - assert_counter(Logins + 2, sessionSuccessfulLogins) - - end). +login_many_story(Alice, Bob) -> + assert_sm_login_event(Alice), + assert_sm_login_event(Bob), + sm_helper:stop_client_and_wait_for_termination(Alice), + assert_sm_logout_event(Alice), + sm_helper:stop_client_and_wait_for_termination(Bob), + assert_sm_logout_event(Bob). auth_failed(Config) -> - {value, AuthFails} = get_counter_value(sessionAuthFails), - - [{_, UserSpec} | _] = escalus_config:get_config(escalus_users, Config), + UserSpec = escalus_fresh:create_fresh_user(Config, alice), UserSpecM = proplists:delete(password, UserSpec) ++ [{password, <<"mazabe">>}], - {error, _} = escalus_client:start(Config, UserSpecM, <<"res1">>), - assert_counter(0, sessionCount), - assert_counter(AuthFails + 1, sessionAuthFails). + assert_no_sm_login_event(UserSpec), + assert_c2s_auth_failed(UserSpec). %% Global @@ -120,3 +115,27 @@ session_unique(Config) -> wait_for_counter(global, 1, uniqueSessionCount), wait_for_counter(global, 2, totalSessionCount) end). + +%% Instrumentation events + +assert_sm_login_event(Client) -> + JID = jid:from_binary(escalus_client:full_jid(Client)), + F = fun(M) -> M =:= #{logins => 1, count => 1, jid => JID} end, + instrument_helper:assert(sm_session, #{host_type => host_type()}, F). + +assert_no_sm_login_event(UserSpec) -> + LUser = jid:nodeprep(proplists:get_value(username, UserSpec)), + LoginEvents = instrument_helper:lookup(sm_session, #{host_type => host_type()}), + F = fun(#{jid := JID}) -> jid:luser(JID) =:= LUser end, + ?assertEqual([], instrument_helper:filter(F, LoginEvents)). + +assert_sm_logout_event(Client) -> + JID = jid:from_binary(escalus_client:full_jid(Client)), + F = fun(M) -> M =:= #{logouts => 1, count => -1, jid => JID} end, + instrument_helper:assert(sm_session, #{host_type => host_type()}, F). + +assert_c2s_auth_failed(UserSpec) -> + Server = proplists:get_value(server, UserSpec), + UserName = proplists:get_value(username, UserSpec), + F = fun(M) -> M =:= #{count => 1, server => Server, username => UserName} end, + instrument_helper:assert(c2s_auth_failed, #{host_type => host_type()}, F). diff --git a/big_tests/tests/roster_helper.erl b/big_tests/tests/roster_helper.erl index ae9efd6705a..9ed7ecfae8f 100644 --- a/big_tests/tests/roster_helper.erl +++ b/big_tests/tests/roster_helper.erl @@ -11,7 +11,7 @@ set_versioning(Versioning, VersionStore, Config) -> store_current_id => VersionStore}}]), Config. -%% Intrumentation events +%% Instrumentation events assert_roster_event(Client, Event) -> ClientJid = jid:from_binary(escalus_utils:get_jid(Client)), @@ -23,7 +23,7 @@ assert_subscription_event(FromClient, ToClient, CheckF) -> FromClientJid = jid:from_binary(escalus_utils:get_short_jid(FromClient)), ToClientJid = jid:from_binary(escalus_utils:get_short_jid(ToClient)), instrument_helper:assert( - sm_presence_subscription, #{}, + sm_presence_subscription, #{host_type => host_type()}, fun(#{from_jid := FromJid, to_jid := ToJid} = M) -> FromClientJid =:= FromJid andalso ToClientJid =:= ToJid andalso CheckF(M) end). diff --git a/big_tests/tests/websockets_SUITE.erl b/big_tests/tests/websockets_SUITE.erl index 80cf94d8541..a44dc5b639b 100644 --- a/big_tests/tests/websockets_SUITE.erl +++ b/big_tests/tests/websockets_SUITE.erl @@ -171,7 +171,7 @@ escape_attrs(Config) -> instrumentation_events() -> instrument_helper:declared_events(mod_websockets, []) ++ instrument_helper:declared_events(mongoose_c2s, [global]) - ++ instrument_helper:declared_events(mongoose_c2s). %% For host_type() + ++ [{c2s_message_processing_time, #{host_type => domain_helper:host_type()}}]. negative_instrumentation_events() -> [{Name, #{}} || Name <- negative_instrumentation_events_names()]. diff --git a/doc/modules/mod_register.md b/doc/modules/mod_register.md index bdf91806c80..a8943a0ef86 100644 --- a/doc/modules/mod_register.md +++ b/doc/modules/mod_register.md @@ -85,8 +85,8 @@ If you'd like to learn more about metrics in MongooseIM, please visit [MongooseI | Name | Type | Description (when it gets incremented) | | ---- | ---- | -------------------------------------- | -| `[Host, modRegisterCount]` | spiral | A user registers via `mod_register` module. | -| `[Host, modUnregisterCount]` | spiral | A user unregisters via `mod_register` module. | +| `[Host, auth_register, count]` | spiral | A user registers via `mod_register` module. | +| `[Host, auth_unregister, count]` | spiral | A user unregisters via `mod_register` module. | ## Entropy calculation algorithm diff --git a/doc/operation-and-maintenance/Logging-&-monitoring.md b/doc/operation-and-maintenance/Logging-&-monitoring.md index 2105d327200..1a0333635f7 100644 --- a/doc/operation-and-maintenance/Logging-&-monitoring.md +++ b/doc/operation-and-maintenance/Logging-&-monitoring.md @@ -119,8 +119,8 @@ For time-based metrics, you can choose to display multiple calculated values for Session count: .global.totalSessionCount.value XMPP messages received: ..xmppMessageReceived.one XMPP messages sent: ..xmppMessageSent.one -Successful logins: ..sessionSuccessfulLogins.one -Logouts: ..sessionLogouts.one +Successful logins: ..sm_session.logins.one +Logouts: ..sm_session.logouts.one Authorization time: ..backends.auth.authorize. RDBMS "simple" query time: ..backends.mongoose_rdbms.query. RDBMS prepared query time: ..backends.mongoose_rdbms.execute. diff --git a/doc/operation-and-maintenance/MongooseIM-metrics.md b/doc/operation-and-maintenance/MongooseIM-metrics.md index 51c909cf810..d8d292a03df 100644 --- a/doc/operation-and-maintenance/MongooseIM-metrics.md +++ b/doc/operation-and-maintenance/MongooseIM-metrics.md @@ -96,33 +96,33 @@ As a result it makes more sense to maintain a list of the most relevant or usefu | Name | Type | Description (when it gets incremented) | | ---- | ---- | -------------------------------------- | -| `[HostType, modPresenceSubscriptions]` | spiral | Presence subscription is processed. | -| `[HostType, modPresenceUnsubscriptions]` | spiral | Presence unsubscription is processed. | -| `[HostType, modRosterGets]` | spiral | User's roster is fetched. | -| `[HostType, modRosterPush]` | spiral | A roster update is pushed to a single session. | -| `[HostType, modRosterSets]` | spiral | User's roster is updated. | +| `[HostType, sm_presence_subscription, subscription_count]` | spiral | Presence subscription is processed. | +| `[HostType, sm_presence_subscription, unsubscription_count]` | spiral | Presence unsubscription is processed. | +| `[HostType, mod_roster_get, count]` | spiral | User's roster is fetched. | +| `[HostType, mod_roster_push, count]` | spiral | A roster update is pushed to a single session. | +| `[HostType, mod_roster_set, count]` | spiral | User's roster is updated. | ### Privacy lists | Name | Type | Description (when it gets incremented) | | ---- | ---- | -------------------------------------- | -| `[HostType, modPrivacyGets]` | spiral | IQ privacy `get` is processed. | -| `[HostType, modPrivacyPush]` | spiral | Privacy list update is sent to a single session. | -| `[HostType, modPrivacySets]` | spiral | IQ privacy `set` is processed. | -| `[HostType, modPrivacySetsActive]` | spiral | Active privacy list is changed. | -| `[HostType, modPrivacySetsDefault]` | spiral | Default privacy list is changed. | -| `[HostType, modPrivacyStanzaAll]` | spiral | A packet is checked against the privacy list. | -| `[HostType, modPrivacyStanzaDenied]` | spiral | Privacy list check resulted in `deny`. | -| `[HostType, modPrivacyStanzaBlocked]` | spiral | Privacy list check resulted in `block`. | +| `[HostType, mod_privacy_get, count]` | spiral | IQ privacy `get` is processed. | +| `[HostType, mod_privacy_push_item, count]` | spiral | Privacy list update is sent to a single session. | +| `[HostType, mod_privacy_set, count]` | spiral | IQ privacy `set` is processed. | +| `[HostType, mod_privacy_set, active_count]` | spiral | Active privacy list is changed. | +| `[HostType, mod_privacy_set, default_count]` | spiral | Default privacy list is changed. | +| `[HostType, mod_privacy_check_packet, count]` | spiral | A packet is checked against the privacy list. | +| `[HostType, mod_privacy_check_packet, denied_count]` | spiral | Privacy list check resulted in `deny`. | +| `[HostType, mod_privacy_check_packet, blocked_count]` | spiral | Privacy list check resulted in `block`. | ### Other | Name | Type | Description (when it gets incremented) | | ---- | ---- | -------------------------------------- | -| `[HostType, sessionAuthFails]` | spiral | A client failed to authenticate. | -| `[HostType, sessionCount]` | counter | Number of active sessions. | -| `[HostType, sessionLogouts]` | spiral | A client session is closed. | -| `[HostType, sessionSuccessfulLogins]` | spiral | A client session is opened. | +| `[HostType, c2s_auth_failed, count]` | spiral | A client failed to authenticate. | +| `[HostType, sm_session, count]` | counter | Number of active sessions. | +| `[HostType, sm_session, logouts]` | spiral | A client session is closed. | +| `[HostType, sm_session, logins]` | spiral | A client session is opened. | | `[HostType, xmppErrorIq]` | spiral | An `error` IQ is sent to a client. | | `[HostType, xmppErrorMessage]` | spiral | An `error` message is sent to a client. | | `[HostType, xmppErrorPresence]` | spiral | An `error` presence is sent to a client. | diff --git a/doc/rest-api/Administration-backend_swagger.yml b/doc/rest-api/Administration-backend_swagger.yml index a96c27216e7..830004db15f 100644 --- a/doc/rest-api/Administration-backend_swagger.yml +++ b/doc/rest-api/Administration-backend_swagger.yml @@ -842,7 +842,7 @@ paths: metrics: type: object example: - modRosterPush: + mod_roster_push.count: one: 0 count: 0 /metrics/all/{metric}: @@ -881,7 +881,7 @@ paths: metrics: type: object example: - modRosterPush: + mod_roster_push.count: one: 0 count: 0 404: diff --git a/src/c2s/mongoose_c2s.erl b/src/c2s/mongoose_c2s.erl index 1b9f9d871f9..42b3229ab2e 100644 --- a/src/c2s/mongoose_c2s.erl +++ b/src/c2s/mongoose_c2s.erl @@ -87,7 +87,9 @@ instrumentation(global) -> #{metrics => #{byte_size => histogram}}}]; instrumentation(HostType) -> [{c2s_message_processing_time, #{host_type => HostType}, - #{metrics => #{byte_size => histogram}}}]. + #{metrics => #{byte_size => histogram}}}, + {c2s_auth_failed, #{host_type => HostType}, + #{metrics => #{count => spiral}}}]. %%%---------------------------------------------------------------------- %%% gen_statem @@ -497,6 +499,8 @@ handle_sasl_failure(#c2s_data{host_type = HostType, lserver = LServer} = StateDa ?LOG_INFO(#{what => auth_failed, text => <<"Failed SASL authentication">>, username => Username, lserver => LServer, c2s_state => StateData}), mongoose_hooks:auth_failed(HostType, LServer, Username), + mongoose_instrument:execute(c2s_auth_failed, #{host_type => HostType}, + #{count => 1, server => LServer, username => Username}), El = mongoose_c2s_stanzas:sasl_failure_stanza(ServerOut), NewSaslAcc = send_acc_from_server_jid(StateData, SaslAcc, El), maybe_retry_state(StateData, {wait_for_feature_before_auth, NewSaslAcc, Retries}). diff --git a/src/ejabberd_sm.erl b/src/ejabberd_sm.erl index cdae7f9dff5..65759e9a5d1 100644 --- a/src/ejabberd_sm.erl +++ b/src/ejabberd_sm.erl @@ -190,6 +190,8 @@ make_new_sid() -> open_session(HostType, SID, JID, Priority, Info) -> set_session(SID, JID, Priority, Info), ReplacedPIDs = check_for_sessions_to_replace(HostType, JID), + mongoose_instrument:execute(sm_session, #{host_type => HostType}, + #{jid => JID, logins => 1, count => 1}), mongoose_hooks:sm_register_connection(HostType, SID, JID, Info), ReplacedPIDs. @@ -203,6 +205,9 @@ open_session(HostType, SID, JID, Priority, Info) -> close_session(Acc, SID, JID, Reason, Info) -> #jid{luser = LUser, lserver = LServer, lresource = LResource} = JID, ejabberd_sm_backend:delete_session(SID, LUser, LServer, LResource), + HostType = mongoose_acc:host_type(Acc), + mongoose_instrument:execute(sm_session, #{host_type => HostType}, + #{jid => JID, logouts => 1, count => -1}), mongoose_hooks:sm_remove_connection(Acc, SID, JID, Info, Reason). -spec store_info(jid:jid(), sid(), info_key(), any()) -> ok. @@ -471,9 +476,11 @@ init([]) -> ets:new(sm_iqtable, [named_table, protected, {read_concurrency, true}]), gen_hook:add_handler(node_cleanup, global, fun ?MODULE:node_cleanup/3, #{}, 50), - lists:foreach(fun(HostType) -> gen_hook:add_handlers(hooks(HostType)) end, + lists:foreach(fun(HostType) -> + gen_hook:add_handlers(hooks(HostType)), + mongoose_instrument:set_up(instrumentation(HostType)) + end, ?ALL_HOST_TYPES), - mongoose_instrument:set_up(instrumentation()), %% Create metrics after backend has started, otherwise probe could have null value create_metrics(), {ok, #state{}}. @@ -564,7 +571,8 @@ handle_info(_Info, State) -> %%-------------------------------------------------------------------- -spec terminate(_, state()) -> 'ok'. terminate(_Reason, _State) -> - mongoose_instrument:tear_down(instrumentation()). + [mongoose_instrument:tear_down(instrumentation(HostType)) || HostType <- ?ALL_HOST_TYPES], + ok. %%-------------------------------------------------------------------- %% Func: code_change(OldVsn, State, Extra) -> {ok, NewState} @@ -632,20 +640,21 @@ do_route(Acc, From, To, El) -> do_route_no_resource_presence_prv(From, To, Acc, Packet, Type, Reason) -> case is_privacy_allow(From, To, Acc, Packet) of true -> - execute_subscription_instrumentation(From, To, Type), + HostType = mongoose_acc:host_type(Acc), + execute_subscription_instrumentation(HostType, From, To, Type), Res = mongoose_hooks:roster_in_subscription(Acc, To, From, Type, Reason), mongoose_acc:get(hook, result, false, Res); false -> false end. -execute_subscription_instrumentation(From, To, subscribed) -> - mongoose_instrument:execute(sm_presence_subscription, #{}, +execute_subscription_instrumentation(HostType, From, To, subscribed) -> + mongoose_instrument:execute(sm_presence_subscription, #{host_type => HostType}, #{from_jid => From, to_jid => To, subscription_count => 1}); -execute_subscription_instrumentation(From, To, unsubscribed) -> - mongoose_instrument:execute(sm_presence_subscription, #{}, +execute_subscription_instrumentation(HostType, From, To, unsubscribed) -> + mongoose_instrument:execute(sm_presence_subscription, #{host_type => HostType}, #{from_jid => From, to_jid => To, unsubscription_count => 1}); -execute_subscription_instrumentation(_From, _To, _Type) -> +execute_subscription_instrumentation(_HostType, _From, _To, _Type) -> ok. -spec do_route_no_resource_presence(Type, From, To, Acc, Packet) -> boolean() when @@ -982,7 +991,9 @@ get_cached_unique_count() -> sm_backend() -> mongoose_backend:get_backend_module(global, ?MODULE). --spec instrumentation() -> [mongoose_instrument:spec()]. -instrumentation() -> - [{sm_presence_subscription, #{}, +-spec instrumentation(mongooseim:host_type()) -> [mongoose_instrument:spec()]. +instrumentation(HostType) -> + [{sm_session, #{host_type => HostType}, + #{metrics => #{logins => spiral, logouts => spiral, count => counter}}}, + {sm_presence_subscription, #{host_type => HostType}, #{metrics => #{subscription_count => spiral, unsubscription_count => spiral}}}]. diff --git a/src/instrument/mongoose_instrument.erl b/src/instrument/mongoose_instrument.erl index 330f86e41f8..5537851a8d3 100644 --- a/src/instrument/mongoose_instrument.erl +++ b/src/instrument/mongoose_instrument.erl @@ -28,7 +28,11 @@ -type label_value() :: mongooseim:host_type() | atom(). % to be extended -type metrics() :: #{metric_name() => metric_type()}. -type metric_name() :: atom(). --type metric_type() :: gauge | spiral | histogram. % to be extended + +-type metric_type() :: gauge % last value (integer) + | counter % sum of integers + | spiral % sum of non-negative integers - total and in a time window + | histogram. % statistics of integers -type measurements() :: #{atom() => term()}. -type spec() :: {event_name(), labels(), config()}. -type config() :: #{metrics => metrics(), diff --git a/src/instrument/mongoose_instrument_exometer.erl b/src/instrument/mongoose_instrument_exometer.erl index 1909e918104..397be6e85c1 100644 --- a/src/instrument/mongoose_instrument_exometer.erl +++ b/src/instrument/mongoose_instrument_exometer.erl @@ -68,6 +68,8 @@ handle_metric_event(EventName, Labels, MetricName, MetricType, Measurements) -> -spec update_metric(exometer:name(), spiral | histogram, integer()) -> ok. update_metric(Name, gauge, Value) when is_integer(Value) -> ok = exometer:update(Name, Value); +update_metric(Name, counter, Value) when is_integer(Value) -> + ok = exometer:update(Name, Value); update_metric(Name, spiral, Value) when is_integer(Value), Value >= 0 -> ok = exometer:update(Name, Value); update_metric(Name, histogram, Value) when is_integer(Value) -> diff --git a/src/instrument/mongoose_instrument_prometheus.erl b/src/instrument/mongoose_instrument_prometheus.erl index 0693b00fe2f..bd86a46df91 100644 --- a/src/instrument/mongoose_instrument_prometheus.erl +++ b/src/instrument/mongoose_instrument_prometheus.erl @@ -46,6 +46,8 @@ set_up_metric(EventName, Labels, MetricName, MetricType) -> -spec declare_metric(proplists:proplist(), mongoose_instrument:metric_type()) -> boolean(). declare_metric(MetricSpec, gauge) -> prometheus_gauge:declare(MetricSpec); +declare_metric(MetricSpec, counter) -> + prometheus_gauge:declare(MetricSpec); declare_metric(MetricSpec, spiral) -> prometheus_counter:declare(MetricSpec); declare_metric(MetricSpec, histogram) -> @@ -55,6 +57,8 @@ declare_metric(MetricSpec, histogram) -> mongoose_instrument:metric_type()) -> boolean(). reset_metric(Name, LabelValues, gauge) -> prometheus_gauge:remove(Name, LabelValues); +reset_metric(Name, LabelValues, counter) -> + prometheus_gauge:remove(Name, LabelValues); reset_metric(Name, LabelValues, spiral) -> prometheus_counter:remove(Name, LabelValues); reset_metric(Name, LabelValues, histogram) -> @@ -66,6 +70,9 @@ initialize_metric(Name, LabelValues, spiral) -> %% Initialize counter, because it has a meaningful initial value of zero %% Additionally, leaving it undefined would delay rate calculation in Prometheus prometheus_counter:inc(Name, LabelValues, 0); +initialize_metric(Name, LabelValues, counter) -> + %% Initialize the gauge, because as a counter it has a meaningful initial value of zero + prometheus_gauge:inc(Name, LabelValues, 0); initialize_metric(_Name, _LabelValues, _) -> %% Don't initialize, because no meaningful value could be provided ok. @@ -119,6 +126,8 @@ labels_to_values(Labels) -> mongoose_instrument:metric_type(), integer()) -> ok. update_metric(Name, Labels, gauge, Value) when is_integer(Value) -> ok = prometheus_gauge:set(Name, Labels, Value); +update_metric(Name, Labels, counter, Value) when is_integer(Value) -> + ok = prometheus_gauge:inc(Name, Labels, Value); update_metric(Name, Labels, spiral, Value) when is_integer(Value), Value >= 0 -> ok = prometheus_counter:inc(Name, Labels, Value); update_metric(Name, Labels, histogram, Value) when is_integer(Value) -> diff --git a/src/metrics/mongoose_metrics.erl b/src/metrics/mongoose_metrics.erl index 1a7ee309856..f96de0ee684 100644 --- a/src/metrics/mongoose_metrics.erl +++ b/src/metrics/mongoose_metrics.erl @@ -338,8 +338,7 @@ create_host_type_metrics() -> -spec create_host_type_metrics(mongooseim:host_type()) -> 'ok'. create_host_type_metrics(HostType) -> - lists:foreach(fun(Name) -> ensure_metric(HostType, Name, spiral) end, ?GENERAL_SPIRALS), - lists:foreach(fun(Name) -> ensure_metric(HostType, Name, counter) end, ?TOTAL_COUNTERS). + lists:foreach(fun(Name) -> ensure_metric(HostType, Name, spiral) end, ?GENERAL_SPIRALS). -spec create_host_type_hook_metrics() -> ok. create_host_type_hook_metrics() -> diff --git a/src/metrics/mongoose_metrics_definitions.hrl b/src/metrics/mongoose_metrics_definitions.hrl index aa125b95232..a7db7e3a175 100644 --- a/src/metrics/mongoose_metrics_definitions.hrl +++ b/src/metrics/mongoose_metrics_definitions.hrl @@ -1,7 +1,4 @@ -define(GENERAL_SPIRALS, [ - sessionSuccessfulLogins, - sessionAuthFails, - sessionLogouts, xmppMessageSent, xmppMessageReceived, xmppMessageBounced, @@ -16,22 +13,7 @@ xmppErrorTotal, xmppErrorIq, xmppErrorMessage, - xmppErrorPresence, - modRosterSets, - modRosterGets, - modPresenceSubscriptions, - modPresenceUnsubscriptions, - modRosterPush, - modRegisterCount, - modUnregisterCount, - modPrivacySets, - modPrivacySetsActive, - modPrivacySetsDefault, - modPrivacyPush, - modPrivacyGets, - modPrivacyStanzaDenied, - modPrivacyStanzaBlocked, - modPrivacyStanzaAll + xmppErrorPresence ]). -define(GLOBAL_SPIRALS, [ @@ -41,10 +23,6 @@ [data, xmpp, sent, component] ]). --define(TOTAL_COUNTERS, [ - sessionCount -]). - -define(REPORT_INTERVAL, mongoose_metrics:get_report_interval()). -define(PROBE(Name, Module), diff --git a/src/metrics/mongoose_metrics_hooks.erl b/src/metrics/mongoose_metrics_hooks.erl index b02566dda8e..7961c1b605a 100644 --- a/src/metrics/mongoose_metrics_hooks.erl +++ b/src/metrics/mongoose_metrics_hooks.erl @@ -15,10 +15,7 @@ %%------------------- %% Internal exports %%------------------- --export([sm_register_connection/3, - sm_remove_connection/3, - auth_failed/3, - user_send_packet/3, +-export([user_send_packet/3, user_open_session/3, xmpp_bounce_message/3, xmpp_stanza_dropped/3, @@ -32,10 +29,7 @@ %% @doc Here will be declared which hooks should be registered -spec get_hooks(_) -> [gen_hook:hook_tuple(), ...]. get_hooks(HostType) -> - [ {sm_register_connection, HostType, fun ?MODULE:sm_register_connection/3, #{}, 50}, - {sm_remove_connection, HostType, fun ?MODULE:sm_remove_connection/3, #{}, 50}, - {auth_failed, HostType, fun ?MODULE:auth_failed/3, #{}, 50}, - {xmpp_stanza_dropped, HostType, fun ?MODULE:xmpp_stanza_dropped/3, #{}, 50}, + [ {xmpp_stanza_dropped, HostType, fun ?MODULE:xmpp_stanza_dropped/3, #{}, 50}, {xmpp_bounce_message, HostType, fun ?MODULE:xmpp_bounce_message/3, #{}, 50}, {xmpp_send_element, HostType, fun ?MODULE:xmpp_send_element/3, #{}, 50} | c2s_hooks(HostType)]. @@ -45,34 +39,6 @@ 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(Acc, Params, Extra) -> {ok, Acc} when - Acc :: any(), - Params :: map(), - Extra :: #{host_type := mongooseim:host_type()}. -sm_register_connection(Acc, _, #{host_type := HostType}) -> - mongoose_metrics:update(HostType, sessionSuccessfulLogins, 1), - mongoose_metrics:update(HostType, sessionCount, 1), - {ok, Acc}. - --spec sm_remove_connection(Acc, Params, Extra) -> {ok, Acc} when - Acc :: mongoose_acc:t(), - Params :: map(), - Extra :: #{host_type := mongooseim:host_type()}. -sm_remove_connection(Acc, _, #{host_type := HostType}) -> - mongoose_metrics:update(HostType, sessionLogouts, 1), - mongoose_metrics:update(HostType, sessionCount, -1), - {ok, Acc}. - --spec auth_failed(Acc, Params, Extra) -> {ok, Acc} when - Acc :: any(), - Params :: #{server := jid:server()}, - Extra :: map(). -auth_failed(Acc, #{server := Server}, _) -> - LServer = jid:nameprep(Server), - {ok, HostType} = mongoose_domain_api:get_host_type(LServer), - mongoose_metrics:update(HostType, sessionAuthFails, 1), - {ok, Acc}. - -spec user_send_packet(mongoose_acc:t(), mongoose_c2s_hooks:params(), map()) -> mongoose_c2s_hooks:result(). user_send_packet(Acc, _Params, #{host_type := HostType}) -> diff --git a/test/ejabberd_sm_SUITE.erl b/test/ejabberd_sm_SUITE.erl index e14be2a4489..fdce27ea218 100644 --- a/test/ejabberd_sm_SUITE.erl +++ b/test/ejabberd_sm_SUITE.erl @@ -30,7 +30,7 @@ end_per_suite(Config) -> opts() -> #{instrumentation => config_parser_helper:default_config([instrumentation]), - hosts => [<<"localhost">>], + hosts => [<<"localhost">>, <<"otherhost">>], host_types => [], all_metrics_are_global => false}. diff --git a/test/mongoose_instrument_metrics_SUITE.erl b/test/mongoose_instrument_metrics_SUITE.erl index c55658717e9..8641dffa95c 100644 --- a/test/mongoose_instrument_metrics_SUITE.erl +++ b/test/mongoose_instrument_metrics_SUITE.erl @@ -1,6 +1,7 @@ -module(mongoose_instrument_metrics_SUITE). -compile([export_all, nowarn_export_all]). +-include("log_helper.hrl"). -include_lib("eunit/include/eunit.hrl"). -include_lib("common_test/include/ct.hrl"). @@ -22,7 +23,10 @@ groups() -> [{prometheus, [parallel], [prometheus_skips_non_metric_event, prometheus_gauge_is_created_and_updated, prometheus_gauge_is_updated_separately_for_different_labels, + prometheus_gauge_counter_is_created_and_updated, + prometheus_gauge_counter_is_updated_separately_for_different_labels, prometheus_counter_is_created_and_updated, + prometheus_counter_cannot_be_decreased, prometheus_counter_is_updated_separately_for_different_labels, prometheus_histogram_is_created_and_updated, prometheus_histogram_is_updated_separately_for_different_labels, @@ -30,7 +34,10 @@ groups() -> {exometer, [parallel], [exometer_skips_non_metric_event, exometer_gauge_is_created_and_updated, exometer_gauge_is_updated_separately_for_different_labels, + exometer_counter_is_created_and_updated, + exometer_counter_is_updated_separately_for_different_labels, exometer_spiral_is_created_and_updated, + exometer_spiral_cannot_be_decreased, exometer_spiral_is_updated_separately_for_different_labels, exometer_histogram_is_created_and_updated, exometer_histogram_is_updated_separately_for_different_labels, @@ -39,6 +46,13 @@ groups() -> {prometheus_and_exometer, [parallel], [prometheus_and_exometer_metrics_are_updated]} ]. +init_per_suite(Config) -> + log_helper:set_up(), + Config. + +end_per_suite(_Config) -> + log_helper:tear_down(). + init_per_group(Group, Config) -> [application:ensure_all_started(App) || App <- apps(Group)], mongoose_config:set_opts(#{hosts => [?HOST_TYPE], @@ -53,10 +67,11 @@ end_per_group(_Group, Config) -> mongoose_config:erase_opts(). init_per_testcase(Case, Config) -> + log_helper:subscribe(), [{event, join_atoms(Case, event)} | Config]. end_per_testcase(_Case, _Config) -> - ok. + log_helper:unsubscribe(). apps(prometheus) -> [prometheus]; apps(exometer) -> [exometer_core]; @@ -88,8 +103,8 @@ prometheus_gauge_is_created_and_updated(Config) -> ?assertEqual(undefined, prometheus_gauge:value(Metric, [?HOST_TYPE])), ok = mongoose_instrument:execute(Event, ?LABELS, #{count => 1}), ?assertEqual(1, prometheus_gauge:value(Metric, [?HOST_TYPE])), - ok = mongoose_instrument:execute(Event, ?LABELS, #{count => 2}), - ?assertEqual(2, prometheus_gauge:value(Metric, [?HOST_TYPE])). + ok = mongoose_instrument:execute(Event, ?LABELS, #{count => -2}), + ?assertEqual(-2, prometheus_gauge:value(Metric, [?HOST_TYPE])). prometheus_gauge_is_updated_separately_for_different_labels(Config) -> Event = ?config(event, Config), @@ -101,6 +116,28 @@ prometheus_gauge_is_updated_separately_for_different_labels(Config) -> ?assertEqual(1, prometheus_gauge:value(Metric, [?HOST_TYPE])), ?assertEqual(2, prometheus_gauge:value(Metric, [?HOST_TYPE2])). +prometheus_gauge_counter_is_created_and_updated(Config) -> + Event = ?config(event, Config), + Metric = prom_name(Event, count), + + %% Prometheus gauge used as a counter starts at zero and can be incremented/decremented + ok = mongoose_instrument:set_up(Event, ?LABELS, #{metrics => #{count => counter}}), + ?assertEqual(0, prometheus_gauge:value(Metric, [?HOST_TYPE])), + ok = mongoose_instrument:execute(Event, ?LABELS, #{count => 1}), + ?assertEqual(1, prometheus_gauge:value(Metric, [?HOST_TYPE])), + ok = mongoose_instrument:execute(Event, ?LABELS, #{count => -2}), + ?assertEqual(-1, prometheus_gauge:value(Metric, [?HOST_TYPE])). + +prometheus_gauge_counter_is_updated_separately_for_different_labels(Config) -> + Event = ?config(event, Config), + Metric = prom_name(Event, count), + ok = mongoose_instrument:set_up(Event, ?LABELS, #{metrics => #{count => counter}}), + ok = mongoose_instrument:set_up(Event, ?LABELS2, #{metrics => #{count => counter}}), + ok = mongoose_instrument:execute(Event, ?LABELS, #{count => 1}), + ok = mongoose_instrument:execute(Event, ?LABELS2, #{count => 2}), + ?assertEqual(1, prometheus_gauge:value(Metric, [?HOST_TYPE])), + ?assertEqual(2, prometheus_gauge:value(Metric, [?HOST_TYPE2])). + prometheus_counter_is_created_and_updated(Config) -> Event = ?config(event, Config), Metric = prom_name(Event, count), @@ -113,6 +150,16 @@ prometheus_counter_is_created_and_updated(Config) -> ok = mongoose_instrument:execute(Event, ?LABELS, #{count => 2}), ?assertEqual(3, prometheus_counter:value(Metric, [?HOST_TYPE])). +prometheus_counter_cannot_be_decreased(Config) -> + Event = ?config(event, Config), + Metric = prom_name(Event, count), + + %% Prometheus counter starts at zero, and cannot be decreased + ok = mongoose_instrument:set_up(Event, ?LABELS, #{metrics => #{count => spiral}}), + ok = mongoose_instrument:execute(Event, ?LABELS, #{count => -1}), + ?assertEqual(0, prometheus_counter:value(Metric, [?HOST_TYPE])), + ?assertLog(error, #{what := event_handler_failed}). + prometheus_counter_is_updated_separately_for_different_labels(Config) -> Event = ?config(event, Config), Metric = prom_name(Event, count), @@ -197,6 +244,29 @@ exometer_gauge_is_updated_separately_for_different_labels(Config) -> ?assertEqual({ok, [{value, 1}]}, exometer:get_value(Metric1, value)), ?assertEqual({ok, [{value, 2}]}, exometer:get_value(Metric2, value)). +exometer_counter_is_created_and_updated(Config) -> + Event = ?config(event, Config), + Metric = [?HOST_TYPE, Event, count], + + %% Exometer counter starts at zero, and can be incremented/decremented + ok = mongoose_instrument:set_up(Event, ?LABELS, #{metrics => #{count => counter}}), + ?assertEqual({ok, [{value, 0}]}, exometer:get_value(Metric, value)), + ok = mongoose_instrument:execute(Event, ?LABELS, #{count => 1}), + ?assertEqual({ok, [{value, 1}]}, exometer:get_value(Metric, value)), + ok = mongoose_instrument:execute(Event, ?LABELS, #{count => -2}), + ?assertEqual({ok, [{value, -1}]}, exometer:get_value(Metric, value)). + +exometer_counter_is_updated_separately_for_different_labels(Config) -> + Event = ?config(event, Config), + Metric1 = [?HOST_TYPE, Event, count], + Metric2 = [<<"test_type">>, Event, count], + ok = mongoose_instrument:set_up(Event, ?LABELS, #{metrics => #{count => counter}}), + ok = mongoose_instrument:set_up(Event, ?LABELS2, #{metrics => #{count => counter}}), + ok = mongoose_instrument:execute(Event, ?LABELS, #{count => 1}), + ok = mongoose_instrument:execute(Event, ?LABELS2, #{count => 2}), + ?assertEqual({ok, [{value, 1}]}, exometer:get_value(Metric1, value)), + ?assertEqual({ok, [{value, 2}]}, exometer:get_value(Metric2, value)). + exometer_spiral_is_created_and_updated(Config) -> Event = ?config(event, Config), Metric = [?HOST_TYPE, Event, count], @@ -209,6 +279,16 @@ exometer_spiral_is_created_and_updated(Config) -> ok = mongoose_instrument:execute(Event, ?LABELS, #{count => 2}), ?assertEqual({ok, [{count, 3}]}, exometer:get_value(Metric, count)). +exometer_spiral_cannot_be_decreased(Config) -> + Event = ?config(event, Config), + Metric = [?HOST_TYPE, Event, count], + + %% Exometer spiral starts at zero, and cannot be decreased + ok = mongoose_instrument:set_up(Event, ?LABELS, #{metrics => #{count => spiral}}), + ok = mongoose_instrument:execute(Event, ?LABELS, #{count => -1}), + ?assertEqual({ok, [{count, 0}]}, exometer:get_value(Metric, count)), + ?assertLog(error, #{what := event_handler_failed}). + exometer_spiral_is_updated_separately_for_different_labels(Config) -> Event = ?config(event, Config), Metric1 = [?HOST_TYPE, Event, count],