Merge pull request #737 from tsloughter/attribute-iolist

fix support for attribute lists of strings to not flatten them
open-telemetry · Jul 10, 2024 · 4510254 · 4510254
2 parents c81e99d + d1e392b
commit 4510254
Show file tree

Hide file tree

Showing 5 changed files with 43 additions and 21 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -7,6 +7,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+## Exporter 
+
+### Fixes
+
+- [BREAKING: Fixes support for attribute values that are lists when the elements
+  are strings. Lists of strings in attribute values are no longer flattened but
+  remain lists. Meaning to use an Erlang charlist string or iolist as a value in
+  an attribute you must convert with `unicode:characters_to_binary` before
+  adding to the
+  attributes](https://github.com/open-telemetry/opentelemetry-erlang/pull/737)
+
 ## Experimental API 0.5.1 - 2024-03-18
 
 ### Added

diff --git a/apps/opentelemetry/src/otel_links.erl b/apps/opentelemetry/src/otel_links.erl
@@ -76,14 +76,8 @@ new_link({TraceId, SpanId, Attributes, TraceState}, AttributePerLinkLimit, Attri
           span_id=SpanId,
           tracestate=TraceState,
           attributes=otel_attributes:new(Attributes, AttributePerLinkLimit, AttributeValueLengthLimit)};
-new_link(#link{trace_id=TraceId,
-               span_id=SpanId,
-               tracestate=TraceState,
-               attributes=Attributes}, AttributePerLinkLimit, AttributeValueLengthLimit) ->
-    #link{trace_id=TraceId,
-          span_id=SpanId,
-          tracestate=TraceState,
-          attributes=otel_attributes:new(Attributes, AttributePerLinkLimit, AttributeValueLengthLimit)};
+new_link(Link=#link{}, _AttributePerLinkLimit, _AttributeValueLengthLimit) ->
+    Link;
 new_link(#{trace_id := TraceId,
            span_id := SpanId,
            tracestate := TraceState,

diff --git a/apps/opentelemetry_api/src/otel_attributes.erl b/apps/opentelemetry_api/src/otel_attributes.erl
@@ -50,7 +50,7 @@
 %% `Pairs' can be a list of key-value pairs or a map. If `Pairs' is not a list or map, the
 %% function returns an empty `Attributes'.
 -spec new(
-    [opentelemetry:attribute()] | opentelemetry:attributes_map() | term(),
+    [opentelemetry:attribute()] | opentelemetry:attributes_map(),
     integer(),
     integer() | infinity
 ) -> t().
@@ -72,7 +72,7 @@ new(_, CountLimit, ValueLengthLimit) ->
 %%
 %% `NewListOrMap' can be a list of key-value pairs or a map. If `NewListOrMap' is not a list
 %% or map, the function returns `Attributes' as is. Returns the updated `Attributes'.
--spec set([opentelemetry:attribute()] | opentelemetry:attributes_map() | term(), t()) -> t().
+-spec set([opentelemetry:attribute()] | opentelemetry:attributes_map(), t()) -> t().
 set(NewListOrMap, Attributes) when is_list(NewListOrMap) ->
     set(maps:from_list(NewListOrMap), Attributes);
 set(NewMap, Attributes) when is_map(NewMap) ->

diff --git a/apps/opentelemetry_exporter/src/otel_otlp_common.erl b/apps/opentelemetry_exporter/src/otel_otlp_common.erl
@@ -73,16 +73,7 @@ to_any_value(Value) when is_map(Value) ->
 to_any_value(Value) when is_tuple(Value) ->
     #{value => {array_value, to_array_value(tuple_to_list(Value))}};
 to_any_value(Value) when is_list(Value) ->
-    try unicode:characters_to_binary(Value) of
-        {Failure, _, _} when Failure =:= error ;
-                             Failure =:= incomplete ->
-            to_array_or_kvlist(Value);
-        String ->
-            #{value => {string_value, String}}
-    catch
-        _:_ ->
-            to_array_or_kvlist(Value)
-    end;
+    to_array_or_kvlist(Value);
 to_any_value(Value) ->
     #{value => {string_value, to_binary(io_lib:format("~p", [Value]))}}.
 

diff --git a/apps/opentelemetry_exporter/test/opentelemetry_exporter_SUITE.erl b/apps/opentelemetry_exporter/test/opentelemetry_exporter_SUITE.erl
@@ -14,7 +14,7 @@ all() ->
      {group, grpc}, {group, grpc_gzip}].
 
 groups() ->
-    [{functional, [], [configuration, span_round_trip, ets_instrumentation_info]},
+    [{functional, [], [configuration, span_round_trip, ets_instrumentation_info, to_attributes]},
      {grpc, [], [verify_export]},
      {grpc_gzip, [], [verify_export]},
      {http_protobuf, [], [verify_export, user_agent]},
@@ -332,6 +332,32 @@ span_round_trip(_Config) ->
 
     ok.
 
+%% test conversion of attributes to the map representation of the OTLP protobuf
+to_attributes(_Config) ->
+    %% this tests the removal of support for iolists as values in attributes
+    %% a list of strings remains a list when converted to OTLP instead of being
+    %% flattened into a string value
+    Attributes = otel_attributes:new(#{world => [<<"hello">>, <<"there">>],
+                                      charlist => "ints"}, 100, 100),
+    Converted = otel_otlp_common:to_attributes(Attributes),
+
+    ?assertMatch([#{value :=
+                        #{value :=
+                              {array_value,
+                               #{values :=
+                                     [#{value := {int_value,105}},
+                                      #{value := {int_value,110}},
+                                      #{value := {int_value,116}},
+                                      #{value := {int_value,115}}]}}},
+                    key := <<"charlist">>},
+                  #{value :=
+                        #{value :=
+                              {array_value,
+                               #{values :=
+                                     [#{value := {string_value,<<"hello">>}},
+                                      #{value := {string_value,<<"there">>}}]}}},
+                    key := <<"world">>}], lists:sort(Converted)).
+
 %% insert a couple spans and export to locally running otel collector
 verify_export(Config) ->
     os:putenv("OTEL_RESOURCE_ATTRIBUTES", "service.name=my-test-service,service.version=98da75ea6d38724743bf42b45565049238d86b3f"),