Merge pull request #363 from tsloughter/zipkin-spec

improvements to zipkin exporter
open-telemetry · Feb 16, 2022 · 01e4944 · 01e4944
2 parents feb17ea + 9d4facd
commit 01e4944
Show file tree

Hide file tree

Showing 4 changed files with 60 additions and 21 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -13,6 +13,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 - [Simpler configuration of span processors](https://github.com/open-telemetry/opentelemetry-erlang/pull/357)
 
+### [Zipkin Exporter]
+
+#### Fixed
+
+- Attribute values that are lists are converted to strings in Zipkin tags
+- Status converted to Zipkin tags
+
 ## 1.0.1 - 2022-02-03
 
 ### [API]

diff --git a/apps/opentelemetry_zipkin/src/opentelemetry_zipkin.erl b/apps/opentelemetry_zipkin/src/opentelemetry_zipkin.erl
@@ -74,7 +74,7 @@ zipkin_span(Span, LocalEndpoint) ->
        kind=to_kind(Span#span.kind),
        parent_id=to_parent_id(Span#span.parent_span_id),
        annotations=to_annotations(otel_events:list(Span#span.events)),
-       tags=to_tags(Span#span.attributes),
+       tags=to_tags(Span),
        local_endpoint=LocalEndpoint
        %% remote_endpoint %% TODO: get from attributes?
      }.
@@ -103,22 +103,42 @@ to_string(Value) when is_list(Value) ;
                       is_binary(Value) ->
     Value;
 to_string(Value) ->
-    io_lib:format("~p", [Value]).
+    io_lib:format("~tp", [Value]).
 
 to_binary_string(Value) when is_function(Value) ->
     to_binary_string(Value());
-to_binary_string(Value) when is_list(Value) ->
-    list_to_binary(Value);
 to_binary_string(Value) when is_integer(Value) ->
     integer_to_binary(Value);
 to_binary_string(Value) when is_float(Value) ->
     float_to_binary(Value);
 to_binary_string(Value) when is_atom(Value) ->
     atom_to_binary(Value, utf8);
+to_binary_string(Value) when is_binary(Value) ->
+    Value;
 to_binary_string(Value) ->
-    Value.
+    %% attributes already check that the value is a binary string
+    %% and not a list string, so any other term, like a list,
+    %% can just be turned into a string of term
+    case unicode:characters_to_binary(io_lib:format("~tp", [Value])) of
+        S when is_binary(S) ->
+            S;
+        _ ->
+            %% {error, _, _} or {incomplete, _, _}
+            <<>>
+    end.
+
+to_tags(Span) ->
+    status_to_tags(Span#span.status) ++ attributes_to_tags(Span#span.attributes).
+
+status_to_tags(#status{code=?OTEL_STATUS_ERROR,
+                       message=Message}) ->
+    [{<<"otel.status_code">>, <<"ERROR">>}, {<<"error">>, Message}];
+status_to_tags(#status{code=?OTEL_STATUS_OK}) ->
+    [{<<"otel.status_code">>, <<"OK">>}];
+status_to_tags(_) ->
+    [].
 
-to_tags(Attributes) ->
+attributes_to_tags(Attributes) ->
     maps:fold(fun(Name, Value, Acc) ->
                      [{to_binary_string(Name), to_binary_string(Value)} | Acc]
               end, [], otel_attributes:map(Attributes)).

diff --git a/apps/opentelemetry_zipkin/test/opentelemetry_zipkin_SUITE.erl b/apps/opentelemetry_zipkin/test/opentelemetry_zipkin_SUITE.erl
@@ -25,21 +25,28 @@ verify_export(_Config) ->
     TraceId = otel_id_generator:generate_trace_id(),
     SpanId = otel_id_generator:generate_span_id(),
 
+    Events = otel_events:new(128, 128, 128),
+
     ParentSpan =
         #span{name = <<"span-1">>,
               trace_id = TraceId,
               span_id = SpanId,
               kind = ?SPAN_KIND_CLIENT,
               start_time = opentelemetry:timestamp(),
               end_time = opentelemetry:timestamp(),
-              events = [#event{system_time_nano=opentelemetry:timestamp(),
-                               name = <<"event-1">>,
-                               attributes = [{<<"attr-1">>, <<"value-1">>}]},
-                        #event{system_time_nano=opentelemetry:timestamp(),
-                               name = <<"event-2">>,
-                               attributes = [{<<"attr-3">>, <<"value-3">>}]}],
-              status=opentelemetry:status(?SPAN_KIND_INTERNAL, <<"some message about status">>),
-              attributes = [{<<"attr-2">>, <<"value-2">>}]},
+              links = otel_links:new([], 128, 128, 128),
+              events = otel_events:add([#event{system_time_nano=opentelemetry:timestamp(),
+                                               name = <<"event-1">>,
+                                               attributes = [{<<"attr-1">>, <<"value-1">>}]},
+                                        #event{system_time_nano=opentelemetry:timestamp(),
+                                               name = event_2,
+                                               attributes = [{<<"attr-3">>, <<"value-3">>}]}], Events),
+              attributes = otel_attributes:new([{<<"attr-2">>, <<"value-2">>},
+                                                {attr_3, true},
+                                                {<<"map-key-1">>, #{<<"map-key-1">> => 123}},
+                                                {<<"list-key-1">>, [3.14, 9.345]}
+                                                ], 128, 128),
+              status=opentelemetry:status(?SPAN_KIND_INTERNAL, <<"some message about status">>)},
     true = ets:insert(Tid, ParentSpan),
 
     ChildSpan = #span{name = <<"span-2">>,
@@ -49,13 +56,17 @@ verify_export(_Config) ->
                       kind = ?SPAN_KIND_SERVER,
                       start_time = opentelemetry:timestamp(),
                       end_time = opentelemetry:timestamp(),
-                      events = [#event{system_time_nano=opentelemetry:timestamp(),
-                                       name = <<"event-1">>,
-                                       attributes = [{<<"attr-1">>, <<"value-1">>}]},
-                                #event{system_time_nano=opentelemetry:timestamp(),
-                                       name = <<"event-2">>,
-                                       attributes = [{<<"attr-3">>, <<"value-3">>}]}],
-                      attributes = [{<<"attr-2">>, <<"value-2">>}]},
+                      events = otel_events:add([#event{system_time_nano=opentelemetry:timestamp(),
+                                                       name = <<"event-1">>,
+                                                       attributes = [{<<"attr-1">>, <<"value-1">>}]},
+                                                #event{system_time_nano=opentelemetry:timestamp(),
+                                                       name = event_2,
+                                                       attributes = [{<<"attr-3">>, <<"value-3">>}]}], Events),
+                      attributes = otel_attributes:new([{<<"attr-2">>, <<"value-2">>},
+                                                        {attr_3, true},
+                                                        {<<"map-key-1">>, #{<<"map-key-1">> => 123}},
+                                                        {<<"list-key-1">>, [3.14, 9.345]}
+                                                       ], 128, 128)},
     true = ets:insert(Tid, ChildSpan),
 
     ?assertMatch(ok, opentelemetry_zipkin:export(Tid, Resource, State)),

diff --git a/rebar.config b/rebar.config
@@ -33,3 +33,4 @@
 {cover_enabled, true}.
 {cover_export_enabled, true}.
 {covertool, [{coverdata_files, ["ct.coverdata"]}]}.
+{cover_excl_apps, [opentelemetry_api_experimental, opentelemetry_experimental]}.