Feedback

Author: bryannaegele

apps/opentelemetry/src/otel_attributes.erl

@@ -82,7 +82,7 @@ update_attribute(Key, [Value1 | _Rest] = Value, Attributes=#attributes{count_lim
     Attributes#attributes{map=Map#{Key => [maybe_truncate_binary(V, ValueLengthLimit) || V <- Value]}};
 %% already in the map and not a binary so update
 update_attribute(Key, Value, Attributes=#attributes{map=Map}) when is_map_key(Key, Map) ->
-    Attributes#attributes{map=Map#{Key => Value}};
+    Attributes#attributes{map=Map#{Key := Value}};
 %% we've already started dropping, so just increment
 update_attribute(_Key, _Value, Attributes=#attributes{dropped=Dropped})
   when Dropped > 0 ->

apps/opentelemetry/test/opentelemetry_SUITE.erl

@@ -728,7 +728,8 @@ too_many_attributes(Config) ->
 
     ?set_current_span(SpanCtx),
 
-    ?set_attribute(<<"attr-1">>, <<"attr-value-1">>),
+    %% tuple tests cover lists, as well.
+    ?set_attribute(attr1, {homogenous, tuple}),
 
     %% dropped because of non-homogenous
     ?set_attribute(<<"attr-2-dropped">>, {non_homogenous, <<"attributes">>}),
@@ -748,17 +749,17 @@ too_many_attributes(Config) ->
     otel_span:end_span(SpanCtx),
     [Span] = assert_exported(Tid, SpanCtx),
 
-    ?assertEqual(#{<<"attr-1">> => <<"attr-value-5">>,
+    ?assertEqual(#{attr1 => [homogenous, tuple],
                    <<"attr-3">> => 4}, otel_attributes:map(Span#span.attributes)),
-    ?assertEqual(2, otel_attributes:dropped(Span#span.attributes)),
+    ?assertEqual(3, otel_attributes:dropped(Span#span.attributes)),
 
     %% test again using the `set_attributes' macro
     SpanCtx2 = ?start_span(<<"span-2">>),
 
     ?set_current_span(SpanCtx2),
 
     ?set_attributes(#{<<"attr-1">> => <<"attr-value-1">>,
-                      <<"attr-2">> => {non_homogenous, <<"attributes">>},
+                      <<"attr-2">> => {homogenous, attribute},
                       <<"attr-3">> => attr_3_value,
                       <<"attr-4">> => <<"attr-value-4">>}),
 
@@ -770,7 +771,7 @@ too_many_attributes(Config) ->
 
 
     %% order isn't guaranteed so just verify the number dropped is right
-    ?assertEqual(1, otel_attributes:dropped(Span2#span.attributes)),
+    ?assertEqual(2, otel_attributes:dropped(Span2#span.attributes)),
 
     ok.
 

apps/opentelemetry_api/src/otel_span.erl

@@ -89,6 +89,7 @@ is_valid(_) ->
 -spec is_valid_attribute(opentelemetry:attribute_key(), opentelemetry:attribute_value()) -> boolean().
 is_valid_attribute(Key, Value) when is_tuple(Value) , ?is_allowed_key(Key) ->
     is_valid_attribute(Key, tuple_to_list(Value));
+%% lists as attribute values must be primitive types and homogeneous
 is_valid_attribute(Key, [Value1 | _Rest] = Values) when is_binary(Value1) , ?is_allowed_key(Key) ->
     lists:all(fun is_binary/1, Values);
 is_valid_attribute(Key, [Value1 | _Rest] = Values) when is_boolean(Value1) , ?is_allowed_key(Key) ->
@@ -174,6 +175,14 @@ tracestate(_) ->
       Key :: opentelemetry:attribute_key(),
       Value :: opentelemetry:attribute_value(),
       SpanCtx :: opentelemetry:span_ctx().
+set_attribute(SpanCtx=#span_ctx{span_sdk={Module, _}}, Key, Value) when ?is_recording(SpanCtx) , is_tuple(Value) ->
+    List = tuple_to_list(Value),
+    case is_valid_attribute(Key, List) of
+        true ->
+            Module:set_attribute(SpanCtx, Key, List);
+        false ->
+            false
+    end;
 set_attribute(SpanCtx=#span_ctx{span_sdk={Module, _}}, Key, Value) when ?is_recording(SpanCtx) ->
     case is_valid_attribute(Key, Value) of
         true ->

apps/opentelemetry_api/src/otel_tracer.erl

@@ -43,46 +43,46 @@
 -callback start_span(otel_ctx:t(),
                      opentelemetry:tracer(),
                      opentelemetry:span_name(),
-                     otel_span:start_opts()) -> opentelemetry:span_ctx() | undefined.
+                     otel_span:start_opts()) -> opentelemetry:span_ctx().
 -callback with_span(otel_ctx:t(), opentelemetry:tracer(),
-                    opentelemetry:span_name(), otel_span:start_opts(), traced_fun(T)) -> T | any().
+                    opentelemetry:span_name(), otel_span:start_opts(), traced_fun(T)) -> T.
 
 -spec start_span(opentelemetry:tracer(), opentelemetry:span_name(), otel_span:start_opts())
-                -> opentelemetry:span_ctx() | undefined.
+                -> opentelemetry:span_ctx().
 start_span(Tracer={Module, _}, SpanName, Opts) ->
     case otel_span:is_valid_name(SpanName) of
         true ->
             Module:start_span(otel_ctx:get_current(), Tracer, SpanName, otel_span:validate_start_opts(Opts));
         false ->
-            undefind
+            otel_tracer_noop:noop_span_ctx()
     end.
 
 -spec start_span(otel_ctx:t(), opentelemetry:tracer(), opentelemetry:span_name(), otel_span:start_opts())
-                -> opentelemetry:span_ctx() | undefined.
+                -> opentelemetry:span_ctx().
 start_span(Ctx, Tracer={Module, _}, SpanName, Opts) ->
     case otel_span:is_valid_name(SpanName) of
         true ->
             Module:start_span(Ctx, Tracer, SpanName, otel_span:validate_start_opts(Opts));
         false ->
-            undefined
+            otel_tracer_noop:noop_span_ctx()
     end.
 
--spec with_span(opentelemetry:tracer(), opentelemetry:span_name(), otel_span:start_opts(), traced_fun(T)) -> T | any().
+-spec with_span(opentelemetry:tracer(), opentelemetry:span_name(), otel_span:start_opts(), traced_fun(T)) -> T.
 with_span(Tracer={Module, _}, SpanName, Opts, Fun) when is_atom(Module) ->
     case otel_span:is_valid_name(SpanName) of
         true ->
             Module:with_span(otel_ctx:get_current(), Tracer, SpanName, otel_span:validate_start_opts(Opts), Fun);
         false ->
-            Fun()
+            Fun(otel_tracer_noop:noop_span_ctx())
     end.
 
--spec with_span(otel_ctx:t(), opentelemetry:tracer(), opentelemetry:span_name(), otel_span:start_opts(), traced_fun(T)) -> T | any().
+-spec with_span(otel_ctx:t(), opentelemetry:tracer(), opentelemetry:span_name(), otel_span:start_opts(), traced_fun(T)) -> T.
 with_span(Ctx, Tracer={Module, _}, SpanName, Opts, Fun) when is_atom(Module) ->
     case otel_span:is_valid_name(SpanName) of
         true ->
             Module:with_span(Ctx, Tracer, SpanName, otel_span:validate_start_opts(Opts), Fun);
         false ->
-            Fun()
+            Fun(otel_tracer_noop:noop_span_ctx())
     end.
 
 %% @doc Returns a `span_ctx' record with `is_recording' set to `false'. This is mainly

apps/opentelemetry_api/src/otel_tracer_noop.erl

@@ -21,7 +21,8 @@
 
 -export([start_span/4,
          with_span/5,
-         end_span/2]).
+         end_span/2,
+         noop_span_ctx/0]).
 
 -include("opentelemetry.hrl").
 -include("otel_tracer.hrl").
@@ -64,3 +65,7 @@ with_span(Ctx, Tracer, SpanName, Opts, Fun) ->
               -> boolean() | {error, term()}.
 end_span(_, _) ->
     true.
+
+-spec noop_span_ctx() -> opentelemetry:span_ctx().
+noop_span_ctx() ->
+    ?NOOP_SPAN_CTX.