Merge pull request #8177 from thalesmg/fix-connack-auth-err-mqtt3-43

fix(metrics): inc `connack.auth_error` when using MQTT 3.1

CHANGES-4.3.md

@@ -18,6 +18,11 @@ File format:
   password-protected private key files used for dashboard and
   management HTTPS listeners. [#8129]
 
+### Bug-fixes
+
+- Correctly tally `connack.auth_error` metrics when a client uses MQTT
+  3.1. [#8177]
+
 ## v4.3.15
 
 ### Enhancements

src/emqx.app.src

@@ -6,7 +6,7 @@
   %% the emqx `release' version, which in turn is comprised of several
   %% apps, one of which is this.  See `emqx_release.hrl' for more
   %% info.
-  {vsn, "4.3.16"}, % strict semver, bump manually!
+  {vsn, "4.3.17"}, % strict semver, bump manually!
   {modules, []},
   {registered, []},
   {applications, [ kernel

src/emqx.appup.src

@@ -1,7 +1,8 @@
 %% -*- mode: erlang -*-
 %% Unless you know what you are doing, DO NOT edit manually!!
 {VSN,
-  [{"4.3.15",
+  [{"4.3.16",[{load_module,emqx_metrics,brutal_purge,soft_purge,[]}]},
+   {"4.3.15",
     [{add_module,emqx_calendar},
      {load_module,emqx_logger_textfmt,brutal_purge,soft_purge,[]},
      {load_module,emqx_packet,brutal_purge,soft_purge,[]},
@@ -555,7 +556,8 @@
      {load_module,emqx_message,brutal_purge,soft_purge,[]},
      {load_module,emqx_limiter,brutal_purge,soft_purge,[]}]},
    {<<".*">>,[]}],
-  [{"4.3.15",
+  [{"4.3.16",[{load_module,emqx_metrics,brutal_purge,soft_purge,[]}]},
+   {"4.3.15",
     [{delete_module,emqx_calendar},
      {load_module,emqx_logger_textfmt,brutal_purge,soft_purge,[]},
      {load_module,emqx_packet,brutal_purge,soft_purge,[]},

src/emqx_metrics.erl

@@ -429,8 +429,12 @@ inc_sent(Packet) ->
 
 do_inc_sent(?CONNACK_PACKET(ReasonCode)) ->
     (ReasonCode == ?RC_SUCCESS) orelse inc('packets.connack.error'),
-    (ReasonCode == ?RC_NOT_AUTHORIZED) andalso inc('packets.connack.auth_error'),
+    ((ReasonCode == ?RC_NOT_AUTHORIZED)
-    (ReasonCode == ?RC_BAD_USER_NAME_OR_PASSWORD) andalso inc('packets.connack.auth_error'),
+     orelse (ReasonCode == ?CONNACK_AUTH))
+        andalso inc('packets.connack.auth_error'),
+    ((ReasonCode == ?RC_BAD_USER_NAME_OR_PASSWORD)
+     orelse (ReasonCode == ?CONNACK_CREDENTIALS))
+        andalso inc('packets.connack.auth_error'),
     inc('packets.connack.sent');
 
 do_inc_sent(?PUBLISH_PACKET(QoS)) ->

test/emqx_broker_SUITE.erl

@@ -277,6 +277,38 @@ t_stats_fun({'end', _Config}) ->
     ok = emqx_broker:unsubscribe(<<"topic">>),
     ok = emqx_broker:unsubscribe(<<"topic2">>).
 
+t_connack_auth_error({init, Config}) ->
+    process_flag(trap_exit, true),
+    emqx_ct_helpers:stop_apps([]),
+    emqx_ct_helpers:boot_modules(all),
+    Handler =
+        fun(emqx) ->
+                application:set_env(emqx, acl_nomatch, deny),
+                application:set_env(emqx, allow_anonymous, false),
+                application:set_env(emqx, enable_acl_cache, false),
+                ok;
+           (_) ->
+                ok
+        end,
+    emqx_ct_helpers:start_apps([], Handler),
+    Config;
+t_connack_auth_error({'end', _Config}) ->
+    emqx_ct_helpers:stop_apps([]),
+    emqx_ct_helpers:boot_modules(all),
+    emqx_ct_helpers:start_apps([]),
+    ok;
+t_connack_auth_error(Config) when is_list(Config) ->
+    %% MQTT 3.1
+    ?assertEqual(0, emqx_metrics:val('packets.connack.auth_error')),
+    {ok, C0} = emqtt:start_link([{proto_ver, v4}]),
+    ?assertEqual({error, {unauthorized_client, undefined}}, emqtt:connect(C0)),
+    ?assertEqual(1, emqx_metrics:val('packets.connack.auth_error')),
+    %% MQTT 5.0
+    {ok, C1} = emqtt:start_link([{proto_ver, v5}]),
+    ?assertEqual({error, {not_authorized, #{}}}, emqtt:connect(C1)),
+    ?assertEqual(2, emqx_metrics:val('packets.connack.auth_error')),
+    ok.
+
 recv_msgs(Count) ->
     recv_msgs(Count, []).