diff --git a/src/emqx_access_control.erl b/src/emqx_access_control.erl index fe12ce7e6..93dde28c2 100644 --- a/src/emqx_access_control.erl +++ b/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. diff --git a/src/emqx_channel.erl b/src/emqx_channel.erl index 530b04250..63a67592f 100644 --- a/src/emqx_channel.erl +++ b/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 diff --git a/src/emqx_metrics.erl b/src/emqx_metrics.erl index 9bb19b30a..7bec83465 100644 --- a/src/emqx_metrics.erl +++ b/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. -