refactor(metrics): more understandable auth metrics

`client.auth.success`/`client.auth.failure`
`client.auth.success.anonymous`

src/emqx_access_control.erl

@@ -34,11 +34,15 @@
 -spec(authenticate(emqx_types:clientinfo()) -> {ok, result()} | {error, term()}).
 authenticate(ClientInfo = #{zone := Zone}) ->
     AuthResult = default_auth_result(Zone),
-    case emqx_zone:get_env(Zone, bypass_auth_plugins, false) of
+    case
+        begin ok = emqx_metrics:inc('client.authenticate'),
+              emqx_zone:get_env(Zone, bypass_auth_plugins, false)
+        end
+    of
         true ->
             return_auth_result(AuthResult);
         false ->
-            return_auth_result(run_hooks('client.authenticate', [ClientInfo], AuthResult))
+            return_auth_result(emqx_hooks:run_fold('client.authenticate', [ClientInfo], AuthResult))
     end.
 
 %% @doc Check ACL
@@ -51,6 +55,10 @@ check_acl(ClientInfo, PubSub, Topic) ->
     end,
     inc_acl_metrics(Result), Result.
 
+%%--------------------------------------------------------------------
+%% Internal functions
+%%--------------------------------------------------------------------
+%% ACL
 check_acl_cache(ClientInfo, PubSub, Topic) ->
     case emqx_acl_cache:get_acl_cache(PubSub, Topic) of
         not_found ->
@@ -64,21 +72,16 @@ check_acl_cache(ClientInfo, PubSub, Topic) ->
 
 do_check_acl(ClientInfo = #{zone := Zone}, PubSub, Topic) ->
     Default = emqx_zone:get_env(Zone, acl_nomatch, deny),
-    case run_hooks('client.check_acl', [ClientInfo, PubSub, Topic], Default) of
+    case
+        begin
+            ok = emqx_metrics:inc('client.check_acl'),
+            emqx_hooks:run_fold('client.check_acl', [ClientInfo, PubSub, Topic], Default)
+        end
+    of
         allow  -> allow;
         _Other -> deny
     end.
 
-default_auth_result(Zone) ->
-    case emqx_zone:get_env(Zone, allow_anonymous, false) of
-        true  -> #{auth_result => success, anonymous => true};
-        false -> #{auth_result => not_authorized, anonymous => false}
-    end.
-
--compile({inline, [run_hooks/3]}).
-run_hooks(Name, Args, Acc) ->
-    ok = emqx_metrics:inc(Name), emqx_hooks:run_fold(Name, Args, Acc).
-
 -compile({inline, [inc_acl_metrics/1]}).
 inc_acl_metrics(allow) ->
     emqx_metrics:inc('client.acl.allow');
@@ -87,8 +90,26 @@ inc_acl_metrics(deny) ->
 inc_acl_metrics(cache_hit) ->
     emqx_metrics:inc('client.acl.cache_hit').
 
+%% Auth
+default_auth_result(Zone) ->
+    case emqx_zone:get_env(Zone, allow_anonymous, false) of
+        true  -> #{auth_result => success, anonymous => true};
+        false -> #{auth_result => not_authorized, anonymous => false}
+    end.
+
 -compile({inline, [return_auth_result/1]}).
-return_auth_result(Result = #{auth_result := success}) ->
-    {ok, Result};
-return_auth_result(Result) ->
-    {error, maps:get(auth_result, Result, unknown_error)}.
+return_auth_result(AuthResult = #{auth_result := success}) ->
+    inc_auth_success_metrics(AuthResult),
+    {ok, AuthResult};
+return_auth_result(AuthResult) ->
+    emqx_metrics:inc('client.auth.failure'),
+    {error, maps:get(auth_result, AuthResult, unknown_error)}.
+
+-compile({inline, [inc_auth_success_metrics/1]}).
+inc_auth_success_metrics(AuthResult) ->
+    is_anonymous(AuthResult) andalso
+        emqx_metrics:inc('client.auth.success.anonymous'),
+    emqx_metrics:inc('client.auth.success').
+
+is_anonymous(#{anonymous := true}) -> true;
+is_anonymous(_AuthResult)          -> false.

src/emqx_channel.erl

@@ -1285,8 +1285,6 @@ auth_connect(#mqtt_packet_connect{password  = Password},
       username := Username} = ClientInfo,
     case emqx_access_control:authenticate(ClientInfo#{password => Password}) of
         {ok, AuthResult} ->
-            is_anonymous(AuthResult) andalso
-                emqx_metrics:inc('client.auth.anonymous'),
             NClientInfo = maps:merge(ClientInfo, AuthResult),
             {ok, Channel#channel{clientinfo = NClientInfo}};
         {error, Reason} ->
@@ -1295,9 +1293,6 @@ auth_connect(#mqtt_packet_connect{password  = Password},
             {error, emqx_reason_codes:connack_error(Reason)}
     end.
 
-is_anonymous(#{anonymous := true}) -> true;
-is_anonymous(_AuthResult)          -> false.
-
 %%--------------------------------------------------------------------
 %% Enhanced Authentication
 

src/emqx_metrics.erl

@@ -68,8 +68,10 @@
 %% BACKW
 -export([%% v4.3.0
          upgrade_retained_delayed_counter_type/0,
-         %% e4.4.0, e4.3.0-e4.3.6, v4.3.0-v4.3.11
-         assign_acl_stats_from_ets_to_counter/0
+         %% v4.3.0-v4.3.11, e4.3.0-e4.3.6; v4.4.0, e4.4.0
+         assign_acl_stats_from_ets_to_counter/0,
+         %% v4.3.0-v4.3.14, e4.3.0-e4.3.9; v4.4.0-v4.4.3, e4.4.0-e4.4.3,
+         assign_auth_stats_from_ets_to_counter/0
         ]).
 
 -export_type([metric_idx/0]).
@@ -174,7 +176,6 @@
          {counter, 'client.connack'},
          {counter, 'client.connected'},
          {counter, 'client.authenticate'},
-         {counter, 'client.auth.anonymous'},
          {counter, 'client.check_acl'},
          {counter, 'client.subscribe'},
          {counter, 'client.unsubscribe'},
@@ -189,8 +190,16 @@
          {counter, 'session.discarded'},
          {counter, 'session.terminated'}
         ]).
-%% Statistic metrics for ACL checking
--define(STASTS_ACL_METRICS,
+
+%% Statistic metrics for auth checking
+-define(STATS_AUTH_METRICS,
+        [ {counter, 'client.auth.success'},
+          {counter, 'client.auth.success.anonymous'},
+          {counter, 'client.auth.failure'}
+        ]).
+
+%% Statistic metrics for ACL checking stats
+-define(STATS_ACL_METRICS,
         [ {counter, 'client.acl.allow'},
           {counter, 'client.acl.deny'},
           {counter, 'client.acl.cache_hit'}
@@ -228,6 +237,21 @@ assign_acl_stats_from_ets_to_counter() ->
         ok = counters:put(CRef, Idx, Val)
     end, Names).
 
+%% BACKW: %% v4.3.0-v4.3.14, e4.3.0-e4.3.9; v4.4.0-v4.4.3, e4.4.0-e4.4.3,
+assign_auth_stats_from_ets_to_counter() ->
+    CRef = persistent_term:get(?MODULE),
+    Names = ['client.auth.success', 'client.auth.success.anonymous', 'client.auth.failure'],
+    lists:foreach(fun(Name) ->
+        Val = case emqx_metrics:val(Name) of
+            undefined -> 0;
+            Val0 -> Val0
+        end,
+        Idx = reserved_idx(Name),
+        Metric = #metric{name = Name, type = counter, idx = Idx},
+        ok = gen_server:call(?SERVER, {set, Metric}),
+        ok = counters:put(CRef, Idx, Val)
+    end, Names).
+
 %%--------------------------------------------------------------------
 %% Metrics API
 %%--------------------------------------------------------------------
@@ -458,7 +482,8 @@ init([]) ->
                             ?DELIVERY_METRICS,
                             ?CLIENT_METRICS,
                             ?SESSION_METRICS,
-                            ?STASTS_ACL_METRICS
+                            ?STATS_AUTH_METRICS,
+                            ?STATS_ACL_METRICS
                            ]),
     % Store reserved indices
     ok = lists:foreach(fun({Type, Name}) ->
@@ -592,7 +617,6 @@ reserved_idx('client.connack')               -> 201;
 reserved_idx('client.connected')             -> 202;
 reserved_idx('client.authenticate')          -> 203;
 reserved_idx('client.enhanced_authenticate') -> 204;
-reserved_idx('client.auth.anonymous')        -> 205;
 reserved_idx('client.check_acl')             -> 206;
 reserved_idx('client.subscribe')             -> 207;
 reserved_idx('client.unsubscribe')           -> 208;
@@ -604,9 +628,13 @@ reserved_idx('session.takeovered')           -> 222;
 reserved_idx('session.discarded')            -> 223;
 reserved_idx('session.terminated')           -> 224;
 %% Stats metrics
+%% ACL
 reserved_idx('client.acl.allow')             -> 300;
 reserved_idx('client.acl.deny')              -> 301;
 reserved_idx('client.acl.cache_hit')         -> 302;
+%% Auth
+reserved_idx('client.auth.success')          -> 310;
+reserved_idx('client.auth.success.anonymous') -> 311;
+reserved_idx('client.auth.failure')          -> 312;
 
 reserved_idx(_)                              -> undefined.
-