Merge pull request #3585 from esl/fix_auth_metrics

Fix metrics for auth backend not being reported
esl · Mar 11, 2022 · abb2701 · abb2701
2 parents ef8b27c + 7e08200
commit abb2701
Show file tree

Hide file tree

Showing 3 changed files with 74 additions and 27 deletions.
diff --git a/big_tests/tests/auth_methods_for_c2s_SUITE.erl b/big_tests/tests/auth_methods_for_c2s_SUITE.erl
@@ -1,38 +1,50 @@
 -module(auth_methods_for_c2s_SUITE).
-
 -compile([export_all, nowarn_export_all]).
 
--include_lib("common_test/include/ct.hrl").
 -include_lib("eunit/include/eunit.hrl").
 -include_lib("exml/include/exml.hrl").
 
 -import(distributed_helper, [mim/0, rpc/4]).
 
 all() ->
-    [{group, two_methods_enabled}].
+    [
+     {group, two_methods_enabled},
+     {group, metrics}
+    ].
 
 groups() ->
-    [{two_methods_enabled, [parallel],
-      [can_login_with_allowed_method,
+    [
+     {two_methods_enabled, [parallel],
+      [
+       can_login_with_allowed_method,
        cannot_login_with_not_allowed_method,
-       can_login_to_another_listener]}].
+       can_login_to_another_listener
+      ]},
+     {metrics, [],
+      [
+       metrics_incremented_on_user_connect
+      ]}
+    ].
 
 init_per_suite(Config) ->
-    Config0 = escalus:init_per_suite(Config),
-    Config1 = ejabberd_node_utils:init(Config0),
-    ejabberd_node_utils:backup_config_file(Config1),
-    Config1.
+    escalus:init_per_suite(Config).
 
 end_per_suite(Config) ->
-    ejabberd_node_utils:restore_config_file(Config),
-    ejabberd_node_utils:restart_application(mongooseim),
     escalus:end_per_suite(Config).
 
-init_per_group(_, Config) ->
-    modify_config_and_restart(Config),
-    escalus_cleaner:start(Config).
+init_per_group(metrics, Config) ->
+    Config;
+init_per_group(_, Config0) ->
+    Config1 = ejabberd_node_utils:init(Config0),
+    ejabberd_node_utils:backup_config_file(Config1),
+    modify_config_and_restart(Config1),
+    escalus_cleaner:start(Config1).
 
-end_per_group(_, _Config) ->
+end_per_group(metrics, _Config) ->
+    escalus_fresh:clean();
+end_per_group(_, Config) ->
+    ejabberd_node_utils:restore_config_file(Config),
+    ejabberd_node_utils:restart_application(mongooseim),
     escalus_fresh:clean().
 
 init_per_testcase(TC, Config) ->
@@ -66,8 +78,19 @@ can_login_to_another_listener(Config) ->
              {password, <<"wrong">>}|Spec],
     {ok, _, _} = escalus_connection:start(Spec2).
 
-%% Helpers
+metrics_incremented_on_user_connect(ConfigIn) ->
+    F = fun(Alice, Bob) ->
+                Body = <<"Hello Bob">>,
+                escalus:send(Alice, escalus_stanza:chat_to(Bob, Body)),
+                escalus:assert(is_chat_message, [Body], escalus:wait_for_stanza(Bob))
+        end,
+    HostType = domain_helper:host_type(),
+    HostTypePrefix = domain_helper:make_metrics_prefix(HostType),
+    MongooseMetrics = [{[HostTypePrefix, backends, auth, authorize], changed}],
+    Config = [{mongoose_metrics, MongooseMetrics} | ConfigIn],
+    escalus_fresh:story(Config, [{alice, 1}, {bob, 1}], F).
 
+%% Helpers
 %% If dummy backend is enabled, it is not possible to create new users
 %% (we check if an user does exist before registering the user).
 register_internal_user(Spec) ->

diff --git a/big_tests/tests/mongooseimctl_SUITE.erl b/big_tests/tests/mongooseimctl_SUITE.erl
@@ -439,11 +439,21 @@ check_password_hash(Config) ->
 
 check_password(Config) ->
     {User, Domain, Pass} = get_user_data(alice, Config),
-
+    MetricName = [backends, auth, check_password],
+    OldValue = get_metric(MetricName),
     {_, 0} = mongooseimctl("check_password", [User, Domain, Pass], Config),
     {_, ErrCode} = mongooseimctl("check_password", [User, Domain, <<Pass/binary, "Bad">>], Config),
+    mongoose_helper:wait_until(
+      fun() -> get_metric(MetricName) end, true,
+      #{validator => fun(NewValue) -> OldValue =/= NewValue end, name => ?FUNCTION_NAME}),
     true = (ErrCode =/= 0). %% Must return code other than 0
 
+get_metric(MetricName) ->
+    HostType = domain_helper:host_type(mim),
+    HostTypePrefix = domain_helper:make_metrics_prefix(HostType),
+    {ok, Value} = rpc(mim(), mongoose_metrics, get_metric_value, [[HostTypePrefix | MetricName]]),
+    Value.
+
 check_account(Config) ->
     {User, Domain, _Pass} = get_user_data(alice, Config),
 

diff --git a/src/auth/ejabberd_auth.erl b/src/auth/ejabberd_auth.erl
@@ -295,8 +295,7 @@ get_passterm_with_authmodule(HostType, #jid{luser = LUser, lserver = LServer}) -
 -spec does_user_exist(JID :: jid:jid() | error) -> boolean().
 does_user_exist(#jid{luser = LUser, lserver = LServer}) ->
     F = fun(HostType, Mod) -> does_user_exist_in_module(HostType, LUser, LServer, Mod) end,
-    case call_auth_modules_for_domain(LServer, F,
-                                      #{default => false, metric => does_user_exist}) of
+    case call_auth_modules_for_domain(LServer, F, #{default => false, metric => does_user_exist}) of
         {error, _Error} -> false;
         Result -> Result
     end;
@@ -314,10 +313,8 @@ does_user_exist(HostType, Jid, RequestType) ->
 does_user_exist(false, HostType, Jid, stored) ->
     true =:= does_stored_user_exist(HostType, Jid);
 does_user_exist(false, HostType, #jid{luser = LUser, lserver = LServer}, with_anonymous) ->
-    F = fun(Mod) ->
-                does_user_exist_in_module(HostType, LUser, LServer, Mod)
-        end,
-    call_auth_modules_for_host_type(HostType, F, #{default => false});
+    F = fun(Mod) -> does_user_exist_in_module(HostType, LUser, LServer, Mod) end,
+    call_auth_modules_for_host_type(HostType, F, #{default => false, metric => does_user_exist});
 does_user_exist(Status, _, _, _) ->
     Status.
 
@@ -329,7 +326,7 @@ does_stored_user_exist(HostType, #jid{luser = LUser, lserver = LServer}) ->
     F = fun(Mod) when Mod =/= ejabberd_auth_anonymous ->
                 does_user_exist_in_module(HostType, LUser, LServer, Mod)
         end,
-    call_auth_modules_for_host_type(HostType, F, #{default => false});
+    call_auth_modules_for_host_type(HostType, F, #{default => false, metric => does_user_exist});
 does_stored_user_exist(_HostType, error) ->
     false.
 
@@ -511,14 +508,31 @@ bind_host_type(HostType, F) when is_function(F, 2) ->
           mod_res() | [mod_res()].
 call_auth_modules_for_host_type(HostType, F, Opts) ->
     Modules = auth_modules_for_host_type(HostType),
-    call_auth_modules(Modules, F, Opts).
+    case maps:take(metric, Opts) of
+        {Metric, NewOpts} ->
+            {Time, Result} = timer:tc(fun call_auth_modules/3, [Modules, F, NewOpts]),
+            mongoose_metrics:update(HostType, ?METRIC(Metric), Time),
+            Result;
+        error ->
+            call_auth_modules(Modules, F, Opts)
+    end.
 
 -spec call_auth_modules_with_creds(mongoose_credentials:t(),
                                    mod_fun() | mod_fold_fun(), call_opts()) ->
           mod_res() | [mod_res()].
 call_auth_modules_with_creds(Creds, F, Opts) ->
     Modules = mongoose_credentials:auth_modules(Creds),
-    call_auth_modules(Modules, F, Opts).
+    case maps:take(metric, Opts) of
+        {Metric, NewOpts} ->
+            HostType = mongoose_credentials:host_type(Creds),
+            {Time, Result} = timer:tc(fun call_auth_modules/3,
+                                      [Modules, F, NewOpts]),
+            mongoose_metrics:update(HostType, ?METRIC(Metric), Time),
+            Result;
+        error ->
+            call_auth_modules(Modules, F, Opts)
+    end.
+
 
 %% @doc Perform a map or a fold operation with function F over the provided Modules
 -spec call_auth_modules([authmodule()], mod_fun() | mod_fold_fun(), call_opts()) ->