Merge pull request #620 from tsloughter/attribute-processing

move attribute processing to otel-attribute module
open-telemetry · Aug 31, 2023 · a673c0f · a673c0f
2 parents d88504e + 6957343
commit a673c0f
Show file tree

Hide file tree

Showing 5 changed files with 80 additions and 76 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -14,14 +14,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - [Add `otel_tracestate` module for creating and updating
   tracestate](https://github.com/open-telemetry/opentelemetry-erlang/pull/607)
 - [Attributes module `otel_attributes` moved to
-  SDK](https://github.com/open-telemetry/opentelemetry-erlang/pull/618)
+  API](https://github.com/open-telemetry/opentelemetry-erlang/pull/618)
+- [Moved attribute processing functions to `otel_attributes` from
+  `otel_span`](https://github.com/open-telemetry/opentelemetry-erlang/pull/620)
 
 ## SDK
 
 ### Changes
 
 - [Attributes module `otel_attributes` moved to
-  SDK](https://github.com/open-telemetry/opentelemetry-erlang/pull/618)
+  API](https://github.com/open-telemetry/opentelemetry-erlang/pull/618)
 
 ## Experimental API
 

diff --git a/apps/opentelemetry_api/src/opentelemetry.erl b/apps/opentelemetry_api/src/opentelemetry.erl
@@ -340,12 +340,12 @@ convert_timestamp(Timestamp, Unit) ->
 links(List) when is_list(List) ->
     lists:filtermap(fun({TraceId, SpanId, Attributes, TraceState}) when is_integer(TraceId) ,
                                                                         is_integer(SpanId) ->
-                            link_or_false(TraceId, SpanId, otel_span:process_attributes(Attributes), TraceState);
+                            link_or_false(TraceId, SpanId, otel_attributes:process_attributes(Attributes), TraceState);
                        ({#span_ctx{trace_id=TraceId,
                                    span_id=SpanId,
                                    tracestate=TraceState}, Attributes}) when is_integer(TraceId) ,
                                                                              is_integer(SpanId) ->
-                            link_or_false(TraceId, SpanId, otel_span:process_attributes(Attributes), TraceState);
+                            link_or_false(TraceId, SpanId, otel_attributes:process_attributes(Attributes), TraceState);
                        (#span_ctx{trace_id=TraceId,
                                   span_id=SpanId,
                                   tracestate=TraceState}) when is_integer(TraceId) ,
@@ -365,7 +365,7 @@ link(SpanCtx) ->
 link(#span_ctx{trace_id=TraceId,
                span_id=SpanId,
                tracestate=TraceState}, Attributes) ->
-    ?MODULE:link(TraceId, SpanId, otel_span:process_attributes(Attributes), TraceState);
+    ?MODULE:link(TraceId, SpanId, otel_attributes:process_attributes(Attributes), TraceState);
 link(_, _) ->
     undefined.
 
@@ -379,7 +379,7 @@ link(TraceId, SpanId, Attributes, TraceState) when is_integer(TraceId),
                                                    (is_list(Attributes) orelse is_map(Attributes)) ->
     #{trace_id => TraceId,
       span_id => SpanId,
-      attributes => otel_span:process_attributes(Attributes),
+      attributes => otel_attributes:process_attributes(Attributes),
       tracestate => TraceState};
 link(_, _, _, _) ->
     undefined.
@@ -401,7 +401,7 @@ event(Timestamp, Name, Attributes) when is_integer(Timestamp),
         true ->
             #{system_time_native => Timestamp,
               name => Name,
-              attributes => otel_span:process_attributes(Attributes)};
+              attributes => otel_attributes:process_attributes(Attributes)};
         false ->
             undefined
     end;

diff --git a/apps/opentelemetry_api/src/otel_attributes.erl b/apps/opentelemetry_api/src/otel_attributes.erl
@@ -21,8 +21,16 @@
          set/2,
          set/3,
          dropped/1,
-         map/1]).
+         map/1,
+         is_valid_attribute/2,
+         process_attributes/1]).
 
+-define(is_allowed_key(Key), (is_atom(Key) orelse (is_binary(Key) andalso Key =/= <<"">>))).
+-define(is_allowed_value(Value), (is_atom(Value) orelse
+                                  is_boolean(Value) orelse
+                                  is_number(Value) orelse
+                                  is_binary(Value) orelse
+                                  is_list(Value))).
 -record(attributes, {
                      count_limit :: integer(),
                      value_length_limit :: integer() | infinity,
@@ -106,3 +114,58 @@ maybe_truncate_binary(Value, ValueLengthLimit) ->
         false ->
             Value
     end.
+
+-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) ->
+    lists:all(fun is_boolean/1, Values);
+is_valid_attribute(Key, [Value1 | _Rest] = Values) when is_atom(Value1) , ?is_allowed_key(Key) ->
+    lists:all(fun is_valid_atom_value/1, Values);
+is_valid_attribute(Key, [Value1 | _Rest] = Values) when is_integer(Value1) , ?is_allowed_key(Key) ->
+    lists:all(fun is_integer/1, Values);
+is_valid_attribute(Key, [Value1 | _Rest] = Values) when is_float(Value1) , ?is_allowed_key(Key) ->
+    lists:all(fun is_float/1, Values);
+is_valid_attribute(_Key, Value) when is_list(Value) ->
+    false;
+is_valid_attribute(Key, []) when ?is_allowed_key(Key) ->
+    true;
+is_valid_attribute(Key, Value) when ?is_allowed_key(Key) , ?is_allowed_value(Value) ->
+    true;
+is_valid_attribute(_, _) ->
+    false.
+
+is_valid_atom_value(undefined) ->
+    false;
+is_valid_atom_value(nil) ->
+    false;
+is_valid_atom_value(Value) ->
+    is_atom(Value) andalso (is_boolean(Value) == false).
+
+-spec process_attributes(eqwalizer:dynamic()) -> opentelemetry:attributes_map().
+process_attributes(Attributes) when is_map(Attributes) ->
+    maps:fold(fun process_attribute/3, #{}, Attributes);
+process_attributes([]) -> #{};
+process_attributes(Attributes) when is_list(Attributes) ->
+    process_attributes(maps:from_list(Attributes));
+process_attributes(_) ->
+    #{}.
+
+process_attribute(Key, Value, Map) when is_tuple(Value) ->
+    List = tuple_to_list(Value),
+    case is_valid_attribute(Key, List) of
+        true ->
+            maps:put(Key, Value, Map);
+        false ->
+            Map
+    end;
+process_attribute(Key, Value, Map) ->
+    case is_valid_attribute(Key, Value) of
+        true ->
+            maps:put(Key, Value, Map);
+        false ->
+            Map
+    end.
diff --git a/apps/opentelemetry_api/src/otel_span.erl b/apps/opentelemetry_api/src/otel_span.erl
@@ -27,7 +27,6 @@
          is_recording/1,
          is_valid/1,
          is_valid_name/1,
-         process_attributes/1,
          validate_start_opts/1,
          set_attribute/3,
          set_attributes/2,
@@ -45,12 +44,6 @@
 -include_lib("opentelemetry_semantic_conventions/include/trace.hrl").
 
 -define(is_recording(SpanCtx), SpanCtx =/= undefined andalso SpanCtx#span_ctx.is_recording =:= true).
--define(is_allowed_key(Key), is_atom(Key) orelse (is_binary(Key) andalso Key =/= <<"">>)).
--define(is_allowed_value(Value), is_atom(Value) orelse
-                                 is_boolean(Value) orelse
-                                 is_number(Value) orelse
-                                 is_binary(Value) orelse
-                                 is_list(Value)).
 
 -type start_opts() :: #{attributes := opentelemetry:attributes_map(),
                         links := [opentelemetry:link()],
@@ -68,7 +61,7 @@ validate_start_opts(Opts) when is_map(Opts) ->
     StartTime = maps:get(start_time, Opts, opentelemetry:timestamp()),
     IsRecording = maps:get(is_recording, Opts, true),
     #{
-      attributes => process_attributes(Attributes),
+      attributes => otel_attributes:process_attributes(Attributes),
       links => Links,
       kind => Kind,
       start_time => StartTime,
@@ -89,60 +82,6 @@ is_valid(#span_ctx{trace_id=TraceId,
 is_valid(_) ->
     false.
 
--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) ->
-    lists:all(fun is_boolean/1, Values);
-is_valid_attribute(Key, [Value1 | _Rest] = Values) when is_atom(Value1) , ?is_allowed_key(Key) ->
-    lists:all(fun is_valid_atom_value/1, Values);
-is_valid_attribute(Key, [Value1 | _Rest] = Values) when is_integer(Value1) , ?is_allowed_key(Key) ->
-    lists:all(fun is_integer/1, Values);
-is_valid_attribute(Key, [Value1 | _Rest] = Values) when is_float(Value1) , ?is_allowed_key(Key) ->
-    lists:all(fun is_float/1, Values);
-is_valid_attribute(_Key, Value) when is_list(Value) ->
-    false;
-is_valid_attribute(Key, []) when ?is_allowed_key(Key) ->
-    true;
-is_valid_attribute(Key, Value) when ?is_allowed_key(Key) , ?is_allowed_value(Value) ->
-    true;
-is_valid_attribute(_, _) ->
-    false.
-
-is_valid_atom_value(undefined) ->
-    false;
-is_valid_atom_value(nil) ->
-    false;
-is_valid_atom_value(Value) ->
-    is_atom(Value) andalso (is_boolean(Value) == false).
-
--spec process_attributes(eqwalizer:dynamic()) -> opentelemetry:attributes_map().
-process_attributes(Attributes) when is_map(Attributes) ->
-    maps:fold(fun process_attribute/3, #{}, Attributes);
-process_attributes([]) -> #{};
-process_attributes(Attributes) when is_list(Attributes) ->
-    process_attributes(maps:from_list(Attributes));
-process_attributes(_) ->
-    #{}.
-
-process_attribute(Key, Value, Map) when is_tuple(Value) ->
-    List = tuple_to_list(Value),
-    case is_valid_attribute(Key, List) of
-        true ->
-            maps:put(Key, Value, Map);
-        false ->
-            Map
-    end;
-process_attribute(Key, Value, Map) ->
-    case is_valid_attribute(Key, Value) of
-        true ->
-            maps:put(Key, Value, Map);
-        false ->
-            Map
-    end.
 -spec is_valid_name(any()) -> boolean().
 is_valid_name(undefined) ->
     false;
@@ -203,14 +142,14 @@ tracestate(_) ->
       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
+    case otel_attributes: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
+    case otel_attributes:is_valid_attribute(Key, Value) of
         true ->
             Module:set_attribute(SpanCtx, Key, Value);
         false ->
@@ -224,7 +163,7 @@ set_attribute(_, _, _) ->
       SpanCtx :: opentelemetry:span_ctx().
 set_attributes(SpanCtx=#span_ctx{span_sdk={Module, _}}, Attributes) when ?is_recording(SpanCtx),
                                                                          (is_list(Attributes) orelse is_map(Attributes)) ->
-    Module:set_attributes(SpanCtx, process_attributes(Attributes));
+    Module:set_attributes(SpanCtx, otel_attributes:process_attributes(Attributes));
 set_attributes(_, _) ->
     false.
 
@@ -237,7 +176,7 @@ add_event(SpanCtx=#span_ctx{span_sdk={Module, _}}, Name, Attributes)
        (is_list(Attributes) orelse is_map(Attributes)) ->
     case is_valid_name(Name) of
         true ->
-            Module:add_event(SpanCtx, Name, process_attributes(Attributes));
+            Module:add_event(SpanCtx, Name, otel_attributes:process_attributes(Attributes));
         false ->
             false
     end;

diff --git a/apps/opentelemetry_api/test/opentelemetry_api_SUITE.erl b/apps/opentelemetry_api/test/opentelemetry_api_SUITE.erl
@@ -76,7 +76,7 @@ can_create_link_from_span(_Config) ->
 
 validations(_Config) ->
     InvalidAttributesArg = undefined,
-    ?assertMatch(#{}, otel_span:process_attributes(InvalidAttributesArg)),
+    ?assertMatch(#{}, otel_attributes:process_attributes(InvalidAttributesArg)),
 
     Attributes = [
                   {<<"key-1">>, <<"value-1">>},
@@ -102,7 +102,7 @@ validations(_Config) ->
               {untimed_event, Attributes},
               {<<"">>, Attributes},
               {123, Attributes}],
-    ProcessedAttributes = otel_span:process_attributes(Attributes),
+    ProcessedAttributes = otel_attributes:process_attributes(Attributes),
 
     ?assertMatch(#{key2 := 1,
                    key3 := true,