Merge pull request #384 from tsloughter/rm-select-replace

set_status: use update_element since select_replace doesn't use index
open-telemetry · Apr 27, 2022 · 8b355ee · 8b355ee
2 parents f9996de + 6cf7b36
commit 8b355ee
Show file tree

Hide file tree

Showing 2 changed files with 22 additions and 15 deletions.
diff --git a/apps/opentelemetry/src/otel_span_ets.erl b/apps/opentelemetry/src/otel_span_ets.erl
@@ -138,21 +138,23 @@ add_events(#span_ctx{span_id=SpanId}, NewEvents) ->
 
 -spec set_status(opentelemetry:span_ctx(), opentelemetry:status()) -> boolean().
 set_status(#span_ctx{span_id=SpanId}, Status=#status{code=NewCode}) ->
-    MS = ets:fun2ms(fun(Span=#span{span_id=Id,
-                                   status=#status{code=?OTEL_STATUS_ERROR}}) when Id =:= SpanId ,
-                                                                                  NewCode =:= ?OTEL_STATUS_OK ->
-                            %% can only set status to OK if it has been set to ERROR before
-                            Span#span{status=#status{code=?OTEL_STATUS_OK}};
-                       (Span=#span{span_id=Id,
-                                   status=#status{code=?OTEL_STATUS_UNSET}}) when Id =:= SpanId ->
-                            %% if UNSET then the status can be updated to OK or ERROR
-                            Span#span{status=Status};
-                       (Span=#span{span_id=Id,
-                                   status=undefined}) when Id =:= SpanId ->
-                            %% if undefined then the status can be updated to anything
-                            Span#span{status=Status}
-                    end),
-    ets:select_replace(?SPAN_TAB, MS) =:= 1;
+    try ets:lookup_element(?SPAN_TAB, SpanId, #span.status) of
+         #status{code=?OTEL_STATUS_ERROR} when NewCode =:= ?OTEL_STATUS_OK ->
+            %% can only set status to OK if it has been set to ERROR before
+            ets:update_element(?SPAN_TAB, SpanId, {#span.status, Status});
+        #status{code=?OTEL_STATUS_UNSET} ->
+            %% if UNSET then the status can be updated to OK or ERROR
+            ets:update_element(?SPAN_TAB, SpanId, {#span.status, Status});
+        undefined ->
+            %% if undefined then the status can be updated to anything
+            ets:update_element(?SPAN_TAB, SpanId, {#span.status, Status});
+        _ ->
+            %% nothing to do since status code is either
+            %% OK or ERROR but NewCode is not OK
+            false
+    catch error:badarg ->
+            false
+    end;
 set_status(_, _) ->
     false.
 

diff --git a/apps/opentelemetry/test/opentelemetry_SUITE.erl b/apps/opentelemetry/test/opentelemetry_SUITE.erl
@@ -394,6 +394,11 @@ update_span_data(Config) ->
                          message = <<>>}, UnsetStatus),
     ?assertEqual(UnsetStatus, opentelemetry:status(?OTEL_STATUS_UNSET)),
 
+    %% successfully set to unset status, replacing `undefined'
+    ?assert(otel_span:set_status(SpanCtx1, UnsetStatus)),
+    %% successfully set to an error status
+    ?assert(otel_span:set_status(SpanCtx1, ErrorStatus)),
+    %% successfully set to OK after it was an error already
     ?assert(otel_span:set_status(SpanCtx1, OkStatus)),
     %% spec does not allow setting status to error/unset after it is ok
     ?assertNot(otel_span:set_status(SpanCtx1, ErrorStatus)),