feat(otel): set span status error when authn/authz failed

apps/emqx/include/emqx_external_trace.hrl

@@ -33,12 +33,14 @@
 -if(?EMQX_RELEASE_EDITION == ee).
 
 -define(with_provider(IfRegistered, IfNotRegistered),
-    case persistent_term:get(?PROVIDER, undefined) of
-        undefined ->
-            IfNotRegistered;
-        Provider ->
-            Provider:IfRegistered
-    end
+    fun() ->
+        case persistent_term:get(?PROVIDER, undefined) of
+            undefined ->
+                IfNotRegistered;
+            Provider ->
+                Provider:IfRegistered
+        end
+    end()
 ).
 
 -define(EXT_TRACE_ANY(FuncName, Any, Attrs),
@@ -56,6 +58,27 @@
     ?with_provider(add_span_attrs(Attrs, Ctx), ok)
 ).
 
+-define(EXT_TRACE_SET_STATUS_OK(),
+    ?with_provider(
+        set_status_ok(),
+        ok
+    )
+).
+
+-define(EXT_TRACE_SET_STATUS_ERROR(),
+    ?with_provider(
+        set_status_error(),
+        ok
+    )
+).
+
+-define(EXT_TRACE_SET_STATUS_ERROR(Msg),
+    ?with_provider(
+        set_status_error(Msg),
+        ok
+    )
+).
+
 -define(EXT_TRACE_WITH_ACTION_START(FuncName, Any, Attrs),
     ?with_provider(
         FuncName(?EXT_TRACE_START, Any, Attrs),
@@ -84,6 +107,9 @@
 -define(EXT_TRACE_ANY(_FuncName, Any, _Attrs), Any).
 -define(EXT_TRACE_ADD_ATTRS(_Attrs), ok).
 -define(EXT_TRACE_ADD_ATTRS(_Attrs, _Ctx), ok).
+-define(EXT_TRACE_SET_STATUS_OK(), ok).
+-define(EXT_TRACE_SET_STATUS_ERROR(), ok).
+-define(EXT_TRACE_SET_STATUS_ERROR(_), ok).
 -define(EXT_TRACE_WITH_ACTION_START(_FuncName, Any, _Attrs), Any).
 -define(EXT_TRACE_WITH_ACTION_STOP(_FuncName, Any, _Attrs), ok).
 -define(EXT_TRACE_WITH_PROCESS_FUN(_FuncName, Any, _Attrs, ProcessFun), ProcessFun(Any)).

apps/emqx/src/emqx_channel.erl

@@ -2038,8 +2038,14 @@ authenticate(Packet, Channel) ->
         },
         fun(PacketWithTrace) ->
             Res = process_authenticate(PacketWithTrace, Channel),
-            %% TODO: which authenticator is used
             ?EXT_TRACE_ADD_ATTRS(authn_attrs(Res)),
+            case Res of
+                {ok, _, _} -> ?EXT_TRACE_SET_STATUS_OK();
+                %% TODO: Enhanced AUTH
+                {continue, _, _} -> ok;
+                {error, _} -> ?EXT_TRACE_SET_STATUS_ERROR()
+            end,
+            %% TODO: which authenticator is used
             Res
         end
     ).
@@ -2317,6 +2323,7 @@ do_check_pub_authz(
                 'authz.publish.topic' => Topic,
                 'authz.publish.result' => allow
             }),
+            ?EXT_TRACE_SET_STATUS_OK(),
             ok;
         deny ->
             ?EXT_TRACE_ADD_ATTRS(#{
@@ -2324,6 +2331,7 @@ do_check_pub_authz(
                 'authz.publish.result' => deny,
                 'authz.reason_code' => ?RC_NOT_AUTHORIZED
             }),
+            ?EXT_TRACE_SET_STATUS_ERROR(),
             {error, ?RC_NOT_AUTHORIZED}
     end.
 
@@ -2376,20 +2384,21 @@ do_check_sub_authzs(
         CheckResult
     ),
     DenyAction = emqx:get_config([authorization, deny_action], ignore),
-    case DenyAction =:= disconnect andalso HasAuthzDeny of
-        true ->
-            ?EXT_TRACE_ADD_ATTRS(
-                (subscribe_authz_result_attrs(CheckResult))#{
-                    'authz.deny_action' => disconnect
-                }
-            ),
+    case {HasAuthzDeny, DenyAction} of
+        {true, disconnect} ->
+            ?EXT_TRACE_ADD_ATTRS((subscribe_authz_result_attrs(CheckResult))#{
+                'authz.deny_action' => disconnect
+            }),
+            ?EXT_TRACE_SET_STATUS_ERROR(),
             {error, {disconnect, ?RC_NOT_AUTHORIZED}, Channel};
-        false ->
-            ?EXT_TRACE_ADD_ATTRS(
-                (subscribe_authz_result_attrs(CheckResult))#{
-                    'authz.deny_action' => ignore
-                }
-            ),
+        {true, ignore} ->
+            ?EXT_TRACE_ADD_ATTRS((subscribe_authz_result_attrs(CheckResult))#{
+                'authz.deny_action' => ignore
+            }),
+            ?EXT_TRACE_SET_STATUS_ERROR(),
+            {ok, ?SUBSCRIBE_PACKET(PacketId, SubProps, CheckResult), Channel};
+        {false, _} ->
+            ?EXT_TRACE_SET_STATUS_OK(),
             {ok, ?SUBSCRIBE_PACKET(PacketId, SubProps, CheckResult), Channel}
     end.
 

apps/emqx/src/emqx_external_trace.erl

@@ -121,10 +121,19 @@
     Attrs :: attrs(),
     Ctx :: map() | undefined.
 
+-callback set_status_ok() -> ok.
+
+-callback set_status_error() -> ok.
+
+-callback set_status_error(unicode:unicode_binary()) -> ok.
+
 -optional_callbacks(
     [
         add_span_attrs/1,
         add_span_attrs/2,
+        set_status_ok/0,
+        set_status_error/0,
+        set_status_error/1,
         client_authn/3,
         client_authz/3
     ]

apps/emqx_opentelemetry/src/emqx_otel_trace.erl

@@ -50,7 +50,11 @@
 %% Span enrichments APIs
 
 -export([
-    add_span_attrs/1
+    add_span_attrs/1,
+    add_span_attrs/2,
+    set_status_ok/0,
+    set_status_error/0,
+    set_status_error/1
 ]).
 
 -include("emqx_otel_trace.hrl").
@@ -59,6 +63,7 @@
 -include_lib("emqx/include/emqx_mqtt.hrl").
 -include_lib("emqx/include/emqx_external_trace.hrl").
 -include_lib("opentelemetry_api/include/otel_tracer.hrl").
+-include_lib("opentelemetry_api/include/opentelemetry.hrl").
 
 -define(EMQX_OTEL_CTX, emqx_otel_ctx).
 
@@ -870,6 +875,30 @@ add_span_attrs(Attrs, Ctx) ->
     otel_span:set_attributes(CurrentSpanCtx, Attrs),
     ok.
 
+-spec set_status_ok() -> ok.
+set_status_ok() ->
+    ?with_trace_mode(
+        ok,
+        begin
+            ?set_status(?OTEL_STATUS_OK),
+            ok
+        end
+    ).
+
+-spec set_status_error() -> ok.
+set_status_error() ->
+    set_status_error(<<>>).
+
+-spec set_status_error(unicode:unicode_binary()) -> ok.
+set_status_error(Msg) ->
+    ?with_trace_mode(
+        ok,
+        begin
+            ?set_status(?OTEL_STATUS_ERROR, Msg),
+            ok
+        end
+    ).
+
 msg_attrs(_Msg = #message{flags = #{sys := true}}) ->
     #{};
 msg_attrs(Msg = #message{}) ->