Merge pull request #346 from tsloughter/strict-trace-context

Tracestate: Keep last value for duplicate key

Author: tsloughter

.github/workflows/w3c_interop.yml

@@ -47,5 +47,5 @@ jobs:
         erl -noinput -pa ./_build/interop/extras/interop/ $(rebar3 as interop path) -s w3c_trace_context_interop &
         sleep 1
         cd trace-context/test
-        STRICT_LEVEL=1 python3 test.py http://127.0.0.1:5000/test
+        STRICT_LEVEL=2 python3 test.py http://127.0.0.1:5000/test
       shell: bash

apps/opentelemetry/src/otel_tracer.hrl

@@ -2,7 +2,7 @@
 %% The `tracer' contains the modules to use for span creation,
 %% the sampler to use at creation time and the processors to
 %% run `OnStart' and `OnEnd'.
-%% See OpenTelemetry Tracer spec: https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/sdk-tracing.md#tracer-creation
+%% See OpenTelemetry Tracer spec: https://github.com/open-telemetry/opentelemetry-specification/blob/v1.8.0/specification/trace/sdk.md#tracer-creation
 -record(tracer,
         {
          module                  :: module(),

apps/opentelemetry_api/lib/open_telemetry.ex

@@ -134,7 +134,7 @@ defmodule OpenTelemetry do
   defdelegate timestamp_to_nano(timestamp), to: :opentelemetry
 
   @doc """
-  Convert a native monotonic timestamp to POSIX time of any `:erlang.time_unit/0`.
+  Convert a native monotonic timestamp to POSIX time of any `t::erlang.time_unit/0`.
   Meaning the time since Epoch. Epoch is defined to be 00:00:00 UTC, 1970-01-01.
   """
   @spec convert_timestamp(integer(), :erlang.time_unit()) :: integer()

apps/opentelemetry_api/src/otel_propagator_trace_context.erl

@@ -170,13 +170,13 @@ parse_pairs([Pair | Rest], Acc) ->
     case split(string:trim(Pair)) of
         {K, V} ->
             case re:run(K, ?KEY_MP) =/= nomatch
-                andalso not lists:keymember(K, 1, Acc)
                 andalso re:run(V, ?VALUE_MP) =/= nomatch
             of
                 false ->
                     [];
                 true ->
-                    parse_pairs(Rest, Acc ++ [{K, V}])
+                    %% replace existing key value or append to the end of the list
+                    parse_pairs(Rest, lists:keystore(K, 1, Acc, {K, V}))
             end;
         undefined ->
             []

apps/opentelemetry_api/test/otel_propagators_SUITE.erl

@@ -60,13 +60,15 @@ rewrite(_Config) ->
                                                                 <<"00-10000000000000000000000000000000-1000000000000000-00">>},
                                                                {<<"tracestate">>,<<"a=b">>}])),
 
-
-    otel_propagator_text_map:extract([{<<"traceparent">>,
-                                       <<"00-10000000000000000000000000000000-1000000000000000-00">>}]),
+    otel_propagator_text_map:extract([{<<"tracestate">>,<<"a=j,c=d">>},
+                                      {<<"traceparent">>,
+                                       <<"00-10000000000000000000000000000000-1000000000000000-00">>},
+                                      {<<"tracestate">>,<<"a=b,c=d">>}]),
     ?assertEqual(RecordingSpanCtx#span_ctx{is_recording=false,
+                                           tracestate=[{<<"a">>,<<"b">>},{<<"c">>,<<"d">>}],
                                            is_remote=true}, otel_tracer:current_span_ctx()),
 
-    % should not fail on empty Carrier
+    %% should not fail on empty Carrier
     ?assertMatch(Ctx,
                  otel_propagator_trace_context:extract(Ctx, [],
                                                        fun otel_propagator_text_map:default_carrier_keys/1,

apps/opentelemetry_exporter/src/opentelemetry_exporter.erl

@@ -59,7 +59,7 @@
 %%   <li>`OTEL_EXPORTER_OTLP_TRACES_HEADERS': Additional headers to add to only trace export requests.</li>
 %%   <li>`OTEL_EXPORTER_OTLP_PROTOCOL': The transport protocol to use, supported values: `grpc' and `http_protobuf'. Defaults to `http_protobuf'.</li>
 %%   <li>`OTEL_EXPORTER_OTLP_TRACES_PROTOCOL': The transport protocol to use for exporting traces, supported values: `grpc' and `http_protobuf'. Defaults to `http_protobuf'.</li>
-%%   <li>`OTEL_EXPORTER_OTLP_COMPRESSION': Compression to use, supported value: gzip. Defaults to no compression./li>
+%%   <li>`OTEL_EXPORTER_OTLP_COMPRESSION': Compression to use, supported value: gzip. Defaults to no compression.</li>
 %%   <li>`OTEL_EXPORTER_OTLP_TRACES_COMPRESSION': Compression to use when exporting traces, supported value: gzip. Defaults to no compression.</li>
 %% </ul>
 %%
@@ -79,14 +79,6 @@
          merge_with_environment/1]).
 -endif.
 
-%% dialyzer will warn about having catch all clauses on these
-%% functions because previous clauses cover all cases. But
-%% we want to not crash if something incorrect is passed
-%% through so we ignore those warnings.
--dialyzer({nowarn_function, to_events/2}).
--dialyzer({nowarn_function, to_links/2}).
--dialyzer({nowarn_function, to_tracestate_string/1}).
-
 -include_lib("kernel/include/logger.hrl").
 -include_lib("opentelemetry_api/include/opentelemetry.hrl").
 -include_lib("opentelemetry/include/otel_span.hrl").
@@ -576,41 +568,28 @@ to_status(_) ->
 
 -spec to_events([#event{}]) -> [opentelemetry_exporter_trace_service_pb:event()].
 to_events(Events) ->
-    to_events(Events, []).
-
-to_events([], Acc)->
-    Acc;
-to_events([#event{system_time_nano=Timestamp,
-                  name=Name,
-                  attributes=Attributes} | Rest], Acc) ->
-    to_events(Rest, [#{time_unix_nano => to_unixnano(Timestamp),
-                       name => to_binary(Name),
-                       attributes => to_attributes(otel_attributes:map(Attributes))} | Acc]);
-to_events([_ | Rest], Acc) ->
-    to_events(Rest, Acc).
+    [#{time_unix_nano => to_unixnano(Timestamp),
+       name => to_binary(Name),
+       attributes => to_attributes(otel_attributes:map(Attributes))}
+     || #event{system_time_nano=Timestamp,
+               name=Name,
+               attributes=Attributes} <- Events].
 
 -spec to_links([#link{}]) -> [opentelemetry_exporter_trace_service_pb:link()].
 to_links(Links) ->
-    to_links(Links, []).
+    [#{trace_id => <<TraceId:128>>,
+       span_id => <<SpanId:64>>,
+       trace_state => to_tracestate_string(TraceState),
+       attributes => to_attributes(otel_attributes:map(Attributes)),
+       dropped_attributes_count => 0} || #link{trace_id=TraceId,
+                                               span_id=SpanId,
+                                               attributes=Attributes,
+                                               tracestate=TraceState} <- Links].
+
+-spec to_tracestate_string(opentelemetry:tracestate()) -> string().
+to_tracestate_string(List) ->
+    lists:join($,, [[Key, $=, Value] || {Key, Value} <- List]).
 
-to_links([], Acc)->
-    Acc;
-to_links([#link{trace_id=TraceId,
-                span_id=SpanId,
-                attributes=Attributes,
-                tracestate=TraceState} | Rest], Acc) ->
-    to_links(Rest, [#{trace_id => <<TraceId:128>>,
-                      span_id => <<SpanId:64>>,
-                      trace_state => to_tracestate_string(TraceState),
-                      attributes => to_attributes(otel_attributes:map(Attributes)),
-                      dropped_attributes_count => 0} | Acc]);
-to_links([_ | Rest], Acc) ->
-    to_links(Rest, Acc).
-
-to_tracestate_string(List) when is_list(List) ->
-    lists:join($,, [[Key, $=, Value] || {Key, Value} <- List]);
-to_tracestate_string(_) ->
-    [].
 
 -spec to_otlp_kind(atom()) -> opentelemetry_exporter_trace_service_pb:'span.SpanKind'().
 to_otlp_kind(?SPAN_KIND_INTERNAL) ->