From bc3e909d2b1160e53247365f928c34bb88afbd76 Mon Sep 17 00:00:00 2001 From: Tristan Sloughter Date: Tue, 16 May 2023 15:09:30 -0600 Subject: [PATCH 01/10] update API to use binary trace and span id --- apps/opentelemetry/src/otel_id_generator.erl | 4 ++-- apps/opentelemetry_api/src/opentelemetry.erl | 20 +++++++++---------- apps/opentelemetry_api/src/otel_span.erl | 18 ++++------------- .../src/otel_tracer_noop.erl | 9 ++++++--- .../test/opentelemetry_api_SUITE.erl | 15 +++++++------- 5 files changed, 30 insertions(+), 36 deletions(-) diff --git a/apps/opentelemetry/src/otel_id_generator.erl b/apps/opentelemetry/src/otel_id_generator.erl index e7035ea4..54da4fcb 100644 --- a/apps/opentelemetry/src/otel_id_generator.erl +++ b/apps/opentelemetry/src/otel_id_generator.erl @@ -48,9 +48,9 @@ generate_span_id(Module) -> %% @doc Generates a 128 bit random integer to use as a trace id. -spec generate_trace_id() -> opentelemetry:trace_id(). generate_trace_id() -> - rand:uniform(?assert_type(2 bsl 127 - 1, pos_integer())). %% 2 shifted left by 127 == 2 ^ 128 + rand:bytes(16). %% @doc Generates a 64 bit random integer to use as a span id. -spec generate_span_id() -> opentelemetry:span_id(). generate_span_id() -> - rand:uniform(?assert_type(2 bsl 63 - 1, pos_integer())). %% 2 shifted left by 63 == 2 ^ 64 + rand:bytes(8). diff --git a/apps/opentelemetry_api/src/opentelemetry.erl b/apps/opentelemetry_api/src/opentelemetry.erl index e018cf43..0b885829 100644 --- a/apps/opentelemetry_api/src/opentelemetry.erl +++ b/apps/opentelemetry_api/src/opentelemetry.erl @@ -93,8 +93,8 @@ -type instrumentation_scope() :: #instrumentation_scope{}. --type trace_id() :: non_neg_integer(). --type span_id() :: non_neg_integer(). +-type trace_id() :: binary(). +-type span_id() :: binary(). -type hex_trace_id() :: binary(). -type hex_span_id() :: binary(). @@ -336,20 +336,20 @@ convert_timestamp(Timestamp, Unit) -> Attributes :: attributes_map(), TraceState :: tracestate(). links(List) when is_list(List) -> - lists:filtermap(fun({TraceId, SpanId, Attributes, TraceState}) when is_integer(TraceId) , - is_integer(SpanId) , + lists:filtermap(fun({TraceId, SpanId, Attributes, TraceState}) when is_binary(TraceId) , + is_binary(SpanId) , is_list(TraceState) -> link_or_false(TraceId, SpanId, otel_span:process_attributes(Attributes), TraceState); ({#span_ctx{trace_id=TraceId, span_id=SpanId, - tracestate=TraceState}, Attributes}) when is_integer(TraceId) , - is_integer(SpanId) , + tracestate=TraceState}, Attributes}) when is_binary(TraceId) , + is_binary(SpanId) , is_list(TraceState) -> link_or_false(TraceId, SpanId, otel_span:process_attributes(Attributes), TraceState); (#span_ctx{trace_id=TraceId, span_id=SpanId, - tracestate=TraceState}) when is_integer(TraceId) , - is_integer(SpanId) , + tracestate=TraceState}) when is_binary(TraceId) , + is_binary(SpanId) , is_list(TraceState) -> link_or_false(TraceId, SpanId, [], TraceState); (_) -> @@ -375,8 +375,8 @@ link(_, _) -> SpanId :: span_id(), Attributes :: attributes_map(), TraceState :: tracestate(). -link(TraceId, SpanId, Attributes, TraceState) when is_integer(TraceId), - is_integer(SpanId), +link(TraceId, SpanId, Attributes, TraceState) when is_binary(TraceId), + is_binary(SpanId), (is_list(Attributes) orelse is_map(Attributes)), is_list(TraceState) -> #{trace_id => TraceId, diff --git a/apps/opentelemetry_api/src/otel_span.erl b/apps/opentelemetry_api/src/otel_span.erl index 649cc2a8..ce5ccaac 100644 --- a/apps/opentelemetry_api/src/otel_span.erl +++ b/apps/opentelemetry_api/src/otel_span.erl @@ -167,29 +167,19 @@ span_id(#span_ctx{span_id=SpanId}) -> hex_span_ctx(#span_ctx{trace_id=TraceId, span_id=SpanId, trace_flags=TraceFlags}) -> - #{otel_trace_id => io_lib:format("~32.16.0b", [TraceId]), - otel_span_id => io_lib:format("~16.16.0b", [SpanId]), + #{otel_trace_id => binary:encode_hex(TraceId), + otel_span_id => binary:encode_hex(SpanId), otel_trace_flags => case TraceFlags band 1 of 1 -> "01"; _ -> "00" end}; hex_span_ctx(_) -> #{}. -spec hex_trace_id(opentelemetry:span_ctx()) -> opentelemetry:hex_trace_id(). hex_trace_id(#span_ctx{trace_id=TraceId}) -> - case otel_utils:format_binary_string("~32.16.0b", [TraceId]) of - {ok, Binary} -> - Binary; - _ -> - <<>> - end. + binary:encode_hex(TraceId). -spec hex_span_id(opentelemetry:span_ctx()) -> opentelemetry:hex_span_id(). hex_span_id(#span_ctx{span_id=SpanId}) -> - case otel_utils:format_binary_string("~16.16.0b", [SpanId]) of - {ok, Binary} -> - Binary; - _ -> - <<>> - end. + binary:encode_hex(SpanId). -spec tracestate(opentelemetry:span_ctx() | undefined) -> opentelemetry:tracestate(). tracestate(#span_ctx{tracestate=Tracestate}) -> diff --git a/apps/opentelemetry_api/src/otel_tracer_noop.erl b/apps/opentelemetry_api/src/otel_tracer_noop.erl index d4528a6b..c742ee62 100644 --- a/apps/opentelemetry_api/src/otel_tracer_noop.erl +++ b/apps/opentelemetry_api/src/otel_tracer_noop.erl @@ -27,8 +27,11 @@ -include("opentelemetry.hrl"). -include("otel_tracer.hrl"). --define(NOOP_SPAN_CTX, #span_ctx{trace_id=0, - span_id=0, +-define(ZERO_TRACE_ID, <<0:128>>). +-define(ZERO_SPAN_ID, <<0:64>>). + +-define(NOOP_SPAN_CTX, #span_ctx{trace_id = ?ZERO_TRACE_ID, + span_id = ?ZERO_SPAN_ID, trace_flags=0, tracestate=[], is_valid=false, @@ -41,7 +44,7 @@ start_span(Ctx, _, _SpanName, _) -> %% Spec: Behavior of the API in the absence of an installed SDK case otel_tracer:current_span_ctx(Ctx) of - Parent=#span_ctx{trace_id=TraceId} when TraceId =/= 0 -> + Parent=#span_ctx{trace_id=TraceId} when TraceId =/= ?ZERO_TRACE_ID -> Parent; _ -> %% If the parent Context contains no valid Span, diff --git a/apps/opentelemetry_api/test/opentelemetry_api_SUITE.erl b/apps/opentelemetry_api/test/opentelemetry_api_SUITE.erl index cd10ef87..2ad01850 100644 --- a/apps/opentelemetry_api/test/opentelemetry_api_SUITE.erl +++ b/apps/opentelemetry_api/test/opentelemetry_api_SUITE.erl @@ -96,7 +96,7 @@ validations(_Config) -> {<<"boolean-list-invalid">>, [true, 1.1]}, {<<"float-list-invalid">>, [1.1, 2]}, {<<"int-list-invalid">>, [1, 2.0]}], - Links = [{0, 0, Attributes, []}], + Links = [{<<0:128>>, <<0:64>>, Attributes, []}], Events = [{opentelemetry:timestamp(), <<"timed-event-name">>, Attributes}, {untimed_event, Attributes}, {<<"">>, Attributes}, @@ -132,8 +132,8 @@ validations(_Config) -> attributes := ProcessedAttributes}], opentelemetry:events(Events)), - ?assertMatch([#{trace_id := 0, - span_id := 0, + ?assertMatch([#{trace_id := <<0:128>>, + span_id := <<0:64>>, attributes := ProcessedAttributes, tracestate := []}], opentelemetry:links(Links)), @@ -141,7 +141,7 @@ validations(_Config) -> StartOpts = #{attributes => Attributes, links => opentelemetry:links(Links)}, ?assertMatch(#{attributes := ProcessedAttributes, - links := [#{trace_id := 0, span_id := 0, attributes := ProcessedAttributes, tracestate := []}]}, + links := [#{trace_id := <<0:128>>, span_id := <<0:64>>, attributes := ProcessedAttributes, tracestate := []}]}, otel_span:validate_start_opts(StartOpts)), %% names @@ -246,7 +246,8 @@ noop_with_span(_Config) -> ok. hex_trace_ids(_Config) -> - SpanCtx=#span_ctx{trace_id=41394, span_id=50132}, - ?assertEqual(<<"0000000000000000000000000000a1b2">>, otel_span:hex_trace_id(SpanCtx)), - ?assertEqual(<<"000000000000c3d4">>, otel_span:hex_span_id(SpanCtx)), + SpanCtx=#span_ctx{trace_id = <<19,25,104,206,49,198,60,63,69,245,232,137,234,183,74,97>>, + span_id = <<161,20,190,33,18,16,115,223>>}, + ?assertEqual(<<"131968CE31C63C3F45F5E889EAB74A61">>, otel_span:hex_trace_id(SpanCtx)), + ?assertEqual(<<"A114BE21121073DF">>, otel_span:hex_span_id(SpanCtx)), ok. From d731ba87e56a0b6a7c0acaba6b13736273458393 Mon Sep 17 00:00:00 2001 From: Tristan Sloughter Date: Tue, 16 May 2023 16:25:02 -0600 Subject: [PATCH 02/10] update SDK to use binary trace and span id --- .../src/otel_resource_detector.erl | 4 +- .../test/opentelemetry_SUITE.erl | 16 ++++---- .../test/otel_propagation_SUITE.erl | 4 +- .../src/otel_propagator_b3multi.erl | 21 +++++----- .../src/otel_propagator_b3single.erl | 21 +++++----- .../src/otel_propagator_trace_context.erl | 12 +++--- apps/opentelemetry_api/src/otel_span.erl | 8 ++-- .../test/opentelemetry_api_SUITE.erl | 9 +++-- .../test/otel_propagator_b3_SUITE.erl | 40 +++++++++---------- .../test/otel_propagators_SUITE.erl | 12 +++--- .../src/otel_otlp_traces.erl | 11 +++-- 11 files changed, 81 insertions(+), 77 deletions(-) diff --git a/apps/opentelemetry/src/otel_resource_detector.erl b/apps/opentelemetry/src/otel_resource_detector.erl index 9c4b2072..6781e6ae 100644 --- a/apps/opentelemetry/src/otel_resource_detector.erl +++ b/apps/opentelemetry/src/otel_resource_detector.erl @@ -253,7 +253,7 @@ add_service_instance(Resource) -> false -> case erlang:node() of nonode@nohost -> - ServiceInstanceId = otel_id_generator:generate_trace_id(), + ServiceInstanceId = string:lowercase(binary:encode_hex(otel_id_generator:generate_trace_id())), ServiceInstanceResource = otel_resource:create([{?SERVICE_INSTANCE_ID, ServiceInstanceId}]), otel_resource:merge(ServiceInstanceResource, Resource); ServiceInstance -> @@ -263,7 +263,7 @@ add_service_instance(Resource) -> ServiceInstanceResource = otel_resource:create([{?SERVICE_INSTANCE_ID, ServiceInstance1}]), otel_resource:merge(ServiceInstanceResource, Resource); _Match -> - ServiceInstanceId = otel_id_generator:generate_trace_id(), + ServiceInstanceId = string:lowercase(binary:encode_hex(otel_id_generator:generate_trace_id())), ServiceInstanceResource = otel_resource:create([{?SERVICE_INSTANCE_ID, ServiceInstanceId}]), otel_resource:merge(ServiceInstanceResource, Resource) end diff --git a/apps/opentelemetry/test/opentelemetry_SUITE.erl b/apps/opentelemetry/test/opentelemetry_SUITE.erl index c41db277..7e72d39d 100644 --- a/apps/opentelemetry/test/opentelemetry_SUITE.erl +++ b/apps/opentelemetry/test/opentelemetry_SUITE.erl @@ -404,8 +404,8 @@ shutdown_sdk_noop_spans(_Config) -> SpanCtx2 = ?start_span(<<"span-2">>), - ?assertMatch(#span_ctx{trace_id=0, - span_id=0}, SpanCtx2), + ?assertMatch(#span_ctx{trace_id = <<0:128>>, + span_id = <<0:64>>}, SpanCtx2), ?assertEqual(false, otel_span:set_attribute(SpanCtx1, a, 1)), @@ -784,8 +784,8 @@ stop_temporary_app(_Config) -> Tracer = opentelemetry:get_tracer(), SpanCtx1 = ?start_span(<<"span-1">>), - ?assertNotMatch(#span_ctx{trace_id=0, - span_id=0}, SpanCtx1), + ?assertNotMatch(#span_ctx{trace_id = <<0:128>>, + span_id = <<0:64>>}, SpanCtx1), ok = application:stop(opentelemetry), @@ -793,8 +793,8 @@ stop_temporary_app(_Config) -> %% so eventually newly started spans will be no-ops because %% inserting a new span will fail ?UNTIL(case ?start_span(<<"span-2">>) of - #span_ctx{trace_id=0, - span_id=0} -> + #span_ctx{trace_id = <<0:128>>, + span_id = <<0:64>>} -> true; _ -> false @@ -1074,8 +1074,8 @@ too_many_attributes(Config) -> disabled_sdk(_Config) -> SpanCtx1 = ?start_span(<<"span-1">>), - ?assertMatch(#span_ctx{trace_id=0, - span_id=0}, SpanCtx1), + ?assertMatch(#span_ctx{trace_id = <<0:128>>, + span_id = <<0:64>>}, SpanCtx1), ok. no_exporter(_Config) -> diff --git a/apps/opentelemetry/test/otel_propagation_SUITE.erl b/apps/opentelemetry/test/otel_propagation_SUITE.erl index 34103318..f5d51975 100644 --- a/apps/opentelemetry/test/otel_propagation_SUITE.erl +++ b/apps/opentelemetry/test/otel_propagation_SUITE.erl @@ -77,8 +77,8 @@ propagation(Config) -> Headers = otel_propagator_text_map:inject([{<<"existing-header">>, <<"I exist">>}]), - EncodedTraceId = io_lib:format("~32.16.0b", [TraceId]), - EncodedSpanId = io_lib:format("~16.16.0b", [SpanId]), + EncodedTraceId = string:lowercase(binary:encode_hex(TraceId)), + EncodedSpanId = string:lowercase(binary:encode_hex(SpanId)), ?assertListsEqual([{<<"baggage">>, <<"key-2=value-2;metadata;md-k-1=md-v-1,key-1=value%3D1">>}, {<<"existing-header">>, <<"I exist">>} | diff --git a/apps/opentelemetry_api/src/otel_propagator_b3multi.erl b/apps/opentelemetry_api/src/otel_propagator_b3multi.erl index eaa5e2ab..cf3d1305 100644 --- a/apps/opentelemetry_api/src/otel_propagator_b3multi.erl +++ b/apps/opentelemetry_api/src/otel_propagator_b3multi.erl @@ -45,10 +45,10 @@ inject(Ctx, Carrier, CarrierSet, _Options) -> case otel_tracer:current_span_ctx(Ctx) of #span_ctx{trace_id=TraceId, span_id=SpanId, - trace_flags=TraceOptions} when TraceId =/= 0, SpanId =/= 0 -> + trace_flags=TraceOptions} when TraceId =/= <<0:128>>, SpanId =/= <<0:64>> -> Options = case TraceOptions band 1 of 1 -> <<"1">>; _ -> <<"0">> end, - EncodedTraceId = io_lib:format("~32.16.0b", [TraceId]), - EncodedSpanId = io_lib:format("~16.16.0b", [SpanId]), + EncodedTraceId = string:lowercase(binary:encode_hex(TraceId)), + EncodedSpanId = string:lowercase(binary:encode_hex(SpanId)), case {unicode:characters_to_binary(EncodedTraceId), unicode:characters_to_binary(EncodedSpanId)} of {BinTraceId, BinSpanId} when is_binary(BinTraceId) , is_binary(BinSpanId) -> @@ -92,8 +92,10 @@ extract(Ctx, Carrier, _CarrierKeysFun, CarrierGet, _Options) -> % Trace ID is a 32 or 16 lower-hex character binary. parse_trace_id(TraceId) when is_binary(TraceId) -> case string:length(TraceId) =:= 32 orelse string:length(TraceId) =:= 16 of - true -> string_to_integer(TraceId, 16); - _ -> throw(invalid) + true -> + binary:decode_hex(TraceId); + _ -> + throw(invalid) end; parse_trace_id(_) -> throw(invalid). @@ -101,8 +103,10 @@ parse_trace_id(_) -> % Span ID is a 16 lower-hex character binary. parse_span_id(SpanId) when is_binary(SpanId) -> case string:length(SpanId) =:= 16 of - true -> string_to_integer(SpanId, 16); - _ -> throw(invalid) + true -> + binary:decode_hex(SpanId); + _ -> + throw(invalid) end; parse_span_id(_) -> throw(invalid). @@ -130,6 +134,3 @@ parse_is_sampled(undefined) -> 0; parse_is_sampled(_) -> throw(invalid). - -string_to_integer(S, Base) when is_binary(S) -> - binary_to_integer(S, Base). diff --git a/apps/opentelemetry_api/src/otel_propagator_b3single.erl b/apps/opentelemetry_api/src/otel_propagator_b3single.erl index b079e7fe..24ef9e8a 100644 --- a/apps/opentelemetry_api/src/otel_propagator_b3single.erl +++ b/apps/opentelemetry_api/src/otel_propagator_b3single.erl @@ -43,10 +43,10 @@ inject(Ctx, Carrier, CarrierSet, _Options) -> case otel_tracer:current_span_ctx(Ctx) of #span_ctx{trace_id=TraceId, span_id=SpanId, - trace_flags=TraceOptions} when TraceId =/= 0, SpanId =/= 0 -> + trace_flags=TraceOptions} when TraceId =/= <<0:128>>, SpanId =/= <<0:64>> -> Options = case TraceOptions band 1 of 1 -> <<"1">>; _ -> <<"0">> end, - EncodedTraceId = io_lib:format("~32.16.0b", [TraceId]), - EncodedSpanId = io_lib:format("~16.16.0b", [SpanId]), + EncodedTraceId = string:lowercase(binary:encode_hex(TraceId)), + EncodedSpanId = string:lowercase(binary:encode_hex(SpanId)), B3Context = otel_utils:assert_to_binary([EncodedTraceId, "-", EncodedSpanId, "-", Options]), CarrierSet(?B3_CONTEXT_KEY, B3Context, Carrier); @@ -103,8 +103,10 @@ decode_b3_context(_) -> % Trace ID is a 32 or 16 lower-hex character binary. parse_trace_id(TraceId) when is_binary(TraceId) -> case string:length(TraceId) =:= 32 orelse string:length(TraceId) =:= 16 of - true -> string_to_integer(TraceId, 16); - _ -> throw(invalid) + true -> + binary:decode_hex(TraceId); + _ -> + throw(invalid) end; parse_trace_id(_) -> throw(invalid). @@ -112,8 +114,10 @@ parse_trace_id(_) -> % Span ID is a 16 lower-hex character binary. parse_span_id(SpanId) when is_binary(SpanId) -> case string:length(SpanId) =:= 16 of - true -> string_to_integer(SpanId, 16); - _ -> throw(invalid) + true -> + binary:decode_hex(SpanId); + _ -> + throw(invalid) end; parse_span_id(_) -> throw(invalid). @@ -136,6 +140,3 @@ parse_is_sampled(Sampled) when is_binary(Sampled) -> end; parse_is_sampled(_) -> throw(invalid). - -string_to_integer(S, Base) when is_binary(S) -> - binary_to_integer(S, Base). diff --git a/apps/opentelemetry_api/src/otel_propagator_trace_context.erl b/apps/opentelemetry_api/src/otel_propagator_trace_context.erl index 89567415..6958c214 100644 --- a/apps/opentelemetry_api/src/otel_propagator_trace_context.erl +++ b/apps/opentelemetry_api/src/otel_propagator_trace_context.erl @@ -63,7 +63,7 @@ fields(_) -> inject(Ctx, Carrier, CarrierSet, _Options) -> case otel_tracer:current_span_ctx(Ctx) of SpanCtx=#span_ctx{trace_id=TraceId, - span_id=SpanId} when TraceId =/= 0 andalso SpanId =/= 0 -> + span_id=SpanId} when TraceId =/= <<0:128>> andalso SpanId =/= <<0:64>> -> {TraceParent, TraceState} = encode_span_ctx(SpanCtx), Carrier1 = CarrierSet(?HEADER_KEY, TraceParent, Carrier), case TraceState of @@ -109,8 +109,8 @@ encode_span_ctx(#span_ctx{trace_id=TraceId, encode_traceparent(TraceId, SpanId, TraceOptions) -> Options = case TraceOptions band 1 of 1 -> <<"01">>; _ -> <<"00">> end, - {ok, EncodedTraceId} = otel_utils:format_binary_string("~32.16.0b", [TraceId]), - {ok, EncodedSpanId} = otel_utils:format_binary_string("~16.16.0b", [SpanId]), + EncodedTraceId = string:lowercase(binary:encode_hex(TraceId)), + EncodedSpanId = string:lowercase(binary:encode_hex(SpanId)), otel_utils:assert_to_binary([?VERSION, "-", EncodedTraceId, "-", EncodedSpanId, "-", Options]). @@ -148,9 +148,9 @@ decode(_) -> to_span_ctx(Version, TraceId, SpanId, Opts) -> try %% verify version is hexadecimal - _ = binary_to_integer(Version, 16), - otel_tracer:from_remote_span(?assert_type(binary_to_integer(TraceId, 16), non_neg_integer()), - ?assert_type(binary_to_integer(SpanId, 16), non_neg_integer()), + _ = binary:decode_hex(Version), + otel_tracer:from_remote_span(binary:decode_hex(TraceId), + binary:decode_hex(SpanId), case Opts of <<"01">> -> 1; <<"00">> -> 0; _ -> error(badarg) end) catch %% to integer from base 16 string failed diff --git a/apps/opentelemetry_api/src/otel_span.erl b/apps/opentelemetry_api/src/otel_span.erl index ce5ccaac..923c93c5 100644 --- a/apps/opentelemetry_api/src/otel_span.erl +++ b/apps/opentelemetry_api/src/otel_span.erl @@ -167,19 +167,19 @@ span_id(#span_ctx{span_id=SpanId}) -> hex_span_ctx(#span_ctx{trace_id=TraceId, span_id=SpanId, trace_flags=TraceFlags}) -> - #{otel_trace_id => binary:encode_hex(TraceId), - otel_span_id => binary:encode_hex(SpanId), + #{otel_trace_id => string:lowercase(binary:encode_hex(TraceId)), + otel_span_id => string:lowercase(binary:encode_hex(SpanId)), otel_trace_flags => case TraceFlags band 1 of 1 -> "01"; _ -> "00" end}; hex_span_ctx(_) -> #{}. -spec hex_trace_id(opentelemetry:span_ctx()) -> opentelemetry:hex_trace_id(). hex_trace_id(#span_ctx{trace_id=TraceId}) -> - binary:encode_hex(TraceId). + string:lowercase(binary:encode_hex(TraceId)). -spec hex_span_id(opentelemetry:span_ctx()) -> opentelemetry:hex_span_id(). hex_span_id(#span_ctx{span_id=SpanId}) -> - binary:encode_hex(SpanId). + string:lowercase(binary:encode_hex(SpanId)). -spec tracestate(opentelemetry:span_ctx() | undefined) -> opentelemetry:tracestate(). tracestate(#span_ctx{tracestate=Tracestate}) -> diff --git a/apps/opentelemetry_api/test/opentelemetry_api_SUITE.erl b/apps/opentelemetry_api/test/opentelemetry_api_SUITE.erl index 2ad01850..e7c9bb9e 100644 --- a/apps/opentelemetry_api/test/opentelemetry_api_SUITE.erl +++ b/apps/opentelemetry_api/test/opentelemetry_api_SUITE.erl @@ -141,7 +141,10 @@ validations(_Config) -> StartOpts = #{attributes => Attributes, links => opentelemetry:links(Links)}, ?assertMatch(#{attributes := ProcessedAttributes, - links := [#{trace_id := <<0:128>>, span_id := <<0:64>>, attributes := ProcessedAttributes, tracestate := []}]}, + links := [#{trace_id := <<0:128>>, + span_id := <<0:64>>, + attributes := ProcessedAttributes, + tracestate := []}]}, otel_span:validate_start_opts(StartOpts)), %% names @@ -248,6 +251,6 @@ noop_with_span(_Config) -> hex_trace_ids(_Config) -> SpanCtx=#span_ctx{trace_id = <<19,25,104,206,49,198,60,63,69,245,232,137,234,183,74,97>>, span_id = <<161,20,190,33,18,16,115,223>>}, - ?assertEqual(<<"131968CE31C63C3F45F5E889EAB74A61">>, otel_span:hex_trace_id(SpanCtx)), - ?assertEqual(<<"A114BE21121073DF">>, otel_span:hex_span_id(SpanCtx)), + ?assertEqual(<<"131968ce31c63c3f45f5e889eab74a61">>, otel_span:hex_trace_id(SpanCtx)), + ?assertEqual(<<"a114be21121073df">>, otel_span:hex_span_id(SpanCtx)), ok. diff --git a/apps/opentelemetry_api/test/otel_propagator_b3_SUITE.erl b/apps/opentelemetry_api/test/otel_propagator_b3_SUITE.erl index 6946c117..485bb447 100644 --- a/apps/opentelemetry_api/test/otel_propagator_b3_SUITE.erl +++ b/apps/opentelemetry_api/test/otel_propagator_b3_SUITE.erl @@ -37,49 +37,49 @@ end_per_group(_, _Config) -> extract(_Config) -> % Single: Common B3 format run_extract([{<<"b3">>, <<"0000008c3defb1edb984fe2ac71c71c7-0007e5196e2ae38e-1">>}]), - ?assertMatch(#span_ctx{trace_id=11111111111111111111111111111111, - span_id=2222222222222222, + ?assertMatch(#span_ctx{trace_id = <<0,0,0,140,61,239,177,237,185,132,254,42,199,28,113,199>>, + span_id = <<0,7,229,25,110,42,227,142>>, trace_flags=1}, otel_tracer:current_span_ctx()), % Single: B3 format with 16 character trace ID run_extract([{<<"b3">>, <<"0007e5196e2ae38e-0007e5196e2ae38e-1">>}]), - ?assertMatch(#span_ctx{trace_id=2222222222222222, - span_id=2222222222222222, + ?assertMatch(#span_ctx{trace_id = <<0,7,229,25,110,42,227,142>>, + span_id = <<0,7,229,25,110,42,227,142>>, trace_flags=1}, otel_tracer:current_span_ctx()), % Single: B3 format with parent span id run_extract([{<<"b3">>, <<"0000008c3defb1edb984fe2ac71c71c7-0007e5196e2ae38e-1-0101010101010101">>}]), - ?assertMatch(#span_ctx{trace_id=11111111111111111111111111111111, - span_id=2222222222222222, + ?assertMatch(#span_ctx{trace_id = <<0,0,0,140,61,239,177,237,185,132,254,42,199,28,113,199>>, + span_id = <<0,7,229,25,110,42,227,142>>, trace_flags=1}, otel_tracer:current_span_ctx()), % Single: B3 format without trace flags run_extract([{<<"b3">>, <<"0000008c3defb1edb984fe2ac71c71c7-0007e5196e2ae38e">>}]), - ?assertMatch(#span_ctx{trace_id=11111111111111111111111111111111, - span_id=2222222222222222, + ?assertMatch(#span_ctx{trace_id = <<0,0,0,140,61,239,177,237,185,132,254,42,199,28,113,199>>, + span_id = <<0,7,229,25,110,42,227,142>>, trace_flags=0}, otel_tracer:current_span_ctx()), % Multi: B3 format with 32 character trace ID run_extract([{<<"X-B3-TraceId">>, <<"0000008c3defb1edb984fe2ac71c71c7">>}, {<<"X-B3-SpanId">>, <<"0007e5196e2ae38e">>}, {<<"X-B3-Sampled">>, <<"1">>}]), - ?assertMatch(#span_ctx{trace_id=11111111111111111111111111111111, - span_id=2222222222222222, + ?assertMatch(#span_ctx{trace_id = <<0,0,0,140,61,239,177,237,185,132,254,42,199,28,113,199>>, + span_id = <<0,7,229,25,110,42,227,142>>, trace_flags=1}, otel_tracer:current_span_ctx()), % Multi: B3 format with 16 character trace ID run_extract([{<<"X-B3-TraceId">>, <<"0007e5196e2ae38e">>}, {<<"X-B3-SpanId">>, <<"0007e5196e2ae38e">>}, {<<"X-B3-Sampled">>, <<"1">>}]), - ?assertMatch(#span_ctx{trace_id=2222222222222222, - span_id=2222222222222222, + ?assertMatch(#span_ctx{trace_id = <<0,7,229,25,110,42,227,142>>, + span_id = <<0,7,229,25,110,42,227,142>>, trace_flags=1}, otel_tracer:current_span_ctx()), % Multi: B3 format without Sampled header run_extract([{<<"X-B3-TraceId">>, <<"0000008c3defb1edb984fe2ac71c71c7">>}, {<<"X-B3-SpanId">>, <<"0007e5196e2ae38e">>}]), - ?assertMatch(#span_ctx{trace_id=11111111111111111111111111111111, - span_id=2222222222222222, + ?assertMatch(#span_ctx{trace_id = <<0,0,0,140,61,239,177,237,185,132,254,42,199,28,113,199>>, + span_id = <<0,7,229,25,110,42,227,142>>, trace_flags=0}, otel_tracer:current_span_ctx()), % Combined: Single header format is preferred when both are present @@ -87,8 +87,8 @@ extract(_Config) -> {<<"X-B3-TraceId">>, <<"75694700a50b5df51a28412ea368a592">>}, {<<"X-B3-SpanId">>, <<"b6431ccfe6d2ea8f">>}, {<<"X-B3-Sampled">>, <<"0">>}]), - ?assertMatch(#span_ctx{trace_id=11111111111111111111111111111111, - span_id=2222222222222222, + ?assertMatch(#span_ctx{trace_id = <<0,0,0,140,61,239,177,237,185,132,254,42,199,28,113,199>>, + span_id = <<0,7,229,25,110,42,227,142>>, trace_flags=1}, otel_tracer:current_span_ctx()), ok. @@ -215,8 +215,8 @@ extract_invalid_span_id(_Config) -> ok. inject_single(_Config) -> - otel_tracer:set_current_span(#span_ctx{trace_id=11111111111111111111111111111111, - span_id=2222222222222222, + otel_tracer:set_current_span(#span_ctx{trace_id = <<0,0,0,140,61,239,177,237,185,132,254,42,199,28,113,199>>, + span_id = <<0,7,229,25,110,42,227,142>>, trace_flags=1}), Headers = otel_propagator_text_map:inject([]), @@ -226,8 +226,8 @@ inject_single(_Config) -> inject_multi(_Config) -> % Span with all fields - otel_tracer:set_current_span(#span_ctx{trace_id=11111111111111111111111111111111, - span_id=2222222222222222, + otel_tracer:set_current_span(#span_ctx{trace_id = <<0,0,0,140,61,239,177,237,185,132,254,42,199,28,113,199>>, + span_id = <<0,7,229,25,110,42,227,142>>, trace_flags=1}), Headers = otel_propagator_text_map:inject([]), ?assertListsEqual([{<<"X-B3-TraceId">>, <<"0000008c3defb1edb984fe2ac71c71c7">>}, diff --git a/apps/opentelemetry_api/test/otel_propagators_SUITE.erl b/apps/opentelemetry_api/test/otel_propagators_SUITE.erl index 641e0a54..aef30bc2 100644 --- a/apps/opentelemetry_api/test/otel_propagators_SUITE.erl +++ b/apps/opentelemetry_api/test/otel_propagators_SUITE.erl @@ -41,8 +41,8 @@ end_per_suite(_Config) -> rewrite(_Config) -> otel_ctx:clear(), - RecordingSpanCtx = #span_ctx{trace_id=21267647932558653966460912964485513216, - span_id=1152921504606846976, + RecordingSpanCtx = #span_ctx{trace_id = <<21267647932558653966460912964485513216:128>>, + span_id = <<1152921504606846976:64>>, is_valid=true, is_recording=true}, otel_tracer:set_current_span(RecordingSpanCtx), @@ -85,8 +85,8 @@ invalid_span_no_sdk_propagation(_Config) -> "and no SDK results in the same invalid span as the child"), otel_ctx:clear(), - InvalidSpanCtx = #span_ctx{trace_id=0, - span_id=0, + InvalidSpanCtx = #span_ctx{trace_id = <<0:128>>, + span_id = <<0:64>>, trace_flags=0, tracestate=[], is_valid=false, @@ -111,8 +111,8 @@ nonrecording_no_sdk_propagation(_Config) -> otel_ctx:clear(), - NonRecordingSpanCtx = #span_ctx{trace_id=21267647932558653966460912964485513216, - span_id=1152921504606846976, + NonRecordingSpanCtx = #span_ctx{trace_id = <<16,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0>>, + span_id = <<16,0,0,0,0,0,0,0>>, is_valid=true, is_recording=false}, ?set_current_span(NonRecordingSpanCtx), diff --git a/apps/opentelemetry_exporter/src/otel_otlp_traces.erl b/apps/opentelemetry_exporter/src/otel_otlp_traces.erl index 9b06997a..af3f2b87 100644 --- a/apps/opentelemetry_exporter/src/otel_otlp_traces.erl +++ b/apps/opentelemetry_exporter/src/otel_otlp_traces.erl @@ -77,12 +77,11 @@ to_proto(#span{trace_id=TraceId, links=Links, status=Status, trace_flags=_TraceFlags, - is_recording=_IsRecording}) when is_integer(TraceId), - is_integer(SpanId) -> + is_recording=_IsRecording}) -> ParentSpanId = case MaybeParentSpanId of _ when is_integer(MaybeParentSpanId) -> <>; _ -> <<>> end, #{name => otel_otlp_common:to_binary(Name), - trace_id => <>, - span_id => <>, + trace_id => TraceId, + span_id => SpanId, parent_span_id => ParentSpanId, %% eqwalizer:ignore have to have tracestate as type '_' for matchspecs trace_state => to_tracestate_string(TraceState), @@ -120,8 +119,8 @@ to_events(Events) -> -spec to_links([#link{}]) -> [opentelemetry_exporter_trace_service_pb:link()]. to_links(Links) -> - [#{trace_id => <>, - span_id => <>, + [#{trace_id => TraceId, + span_id => SpanId, trace_state => to_tracestate_string(TraceState), attributes => otel_otlp_common:to_attributes(Attributes), dropped_attributes_count => 0} || #link{trace_id=TraceId, From 629f5cfc721805c64aaaa2610d1291103b890294 Mon Sep 17 00:00:00 2001 From: Tristan Sloughter Date: Wed, 17 May 2023 14:24:11 -0600 Subject: [PATCH 03/10] use OTP-26 encode_hex/2 when available --- apps/opentelemetry/src/otel_resource_detector.erl | 4 ++-- apps/opentelemetry/test/otel_propagation_SUITE.erl | 4 ++-- .../opentelemetry_api/src/otel_propagator_b3multi.erl | 4 ++-- .../src/otel_propagator_b3single.erl | 4 ++-- .../src/otel_propagator_trace_context.erl | 4 ++-- apps/opentelemetry_api/src/otel_span.erl | 8 ++++---- apps/opentelemetry_api/src/otel_utils.erl | 11 ++++++++++- 7 files changed, 24 insertions(+), 15 deletions(-) diff --git a/apps/opentelemetry/src/otel_resource_detector.erl b/apps/opentelemetry/src/otel_resource_detector.erl index 6781e6ae..93c269ca 100644 --- a/apps/opentelemetry/src/otel_resource_detector.erl +++ b/apps/opentelemetry/src/otel_resource_detector.erl @@ -253,7 +253,7 @@ add_service_instance(Resource) -> false -> case erlang:node() of nonode@nohost -> - ServiceInstanceId = string:lowercase(binary:encode_hex(otel_id_generator:generate_trace_id())), + ServiceInstanceId = otel_utils:encode_hex(otel_id_generator:generate_trace_id()), ServiceInstanceResource = otel_resource:create([{?SERVICE_INSTANCE_ID, ServiceInstanceId}]), otel_resource:merge(ServiceInstanceResource, Resource); ServiceInstance -> @@ -263,7 +263,7 @@ add_service_instance(Resource) -> ServiceInstanceResource = otel_resource:create([{?SERVICE_INSTANCE_ID, ServiceInstance1}]), otel_resource:merge(ServiceInstanceResource, Resource); _Match -> - ServiceInstanceId = string:lowercase(binary:encode_hex(otel_id_generator:generate_trace_id())), + ServiceInstanceId = otel_utils:encode_hex(otel_id_generator:generate_trace_id()), ServiceInstanceResource = otel_resource:create([{?SERVICE_INSTANCE_ID, ServiceInstanceId}]), otel_resource:merge(ServiceInstanceResource, Resource) end diff --git a/apps/opentelemetry/test/otel_propagation_SUITE.erl b/apps/opentelemetry/test/otel_propagation_SUITE.erl index f5d51975..c835ac53 100644 --- a/apps/opentelemetry/test/otel_propagation_SUITE.erl +++ b/apps/opentelemetry/test/otel_propagation_SUITE.erl @@ -77,8 +77,8 @@ propagation(Config) -> Headers = otel_propagator_text_map:inject([{<<"existing-header">>, <<"I exist">>}]), - EncodedTraceId = string:lowercase(binary:encode_hex(TraceId)), - EncodedSpanId = string:lowercase(binary:encode_hex(SpanId)), + EncodedTraceId = otel_utils:encode_hex(TraceId), + EncodedSpanId = otel_utils:encode_hex(SpanId), ?assertListsEqual([{<<"baggage">>, <<"key-2=value-2;metadata;md-k-1=md-v-1,key-1=value%3D1">>}, {<<"existing-header">>, <<"I exist">>} | diff --git a/apps/opentelemetry_api/src/otel_propagator_b3multi.erl b/apps/opentelemetry_api/src/otel_propagator_b3multi.erl index cf3d1305..d51c0e1c 100644 --- a/apps/opentelemetry_api/src/otel_propagator_b3multi.erl +++ b/apps/opentelemetry_api/src/otel_propagator_b3multi.erl @@ -47,8 +47,8 @@ inject(Ctx, Carrier, CarrierSet, _Options) -> span_id=SpanId, trace_flags=TraceOptions} when TraceId =/= <<0:128>>, SpanId =/= <<0:64>> -> Options = case TraceOptions band 1 of 1 -> <<"1">>; _ -> <<"0">> end, - EncodedTraceId = string:lowercase(binary:encode_hex(TraceId)), - EncodedSpanId = string:lowercase(binary:encode_hex(SpanId)), + EncodedTraceId = otel_utils:encode_hex(TraceId), + EncodedSpanId = otel_utils:encode_hex(SpanId), case {unicode:characters_to_binary(EncodedTraceId), unicode:characters_to_binary(EncodedSpanId)} of {BinTraceId, BinSpanId} when is_binary(BinTraceId) , is_binary(BinSpanId) -> diff --git a/apps/opentelemetry_api/src/otel_propagator_b3single.erl b/apps/opentelemetry_api/src/otel_propagator_b3single.erl index 24ef9e8a..bd49d75a 100644 --- a/apps/opentelemetry_api/src/otel_propagator_b3single.erl +++ b/apps/opentelemetry_api/src/otel_propagator_b3single.erl @@ -45,8 +45,8 @@ inject(Ctx, Carrier, CarrierSet, _Options) -> span_id=SpanId, trace_flags=TraceOptions} when TraceId =/= <<0:128>>, SpanId =/= <<0:64>> -> Options = case TraceOptions band 1 of 1 -> <<"1">>; _ -> <<"0">> end, - EncodedTraceId = string:lowercase(binary:encode_hex(TraceId)), - EncodedSpanId = string:lowercase(binary:encode_hex(SpanId)), + EncodedTraceId = otel_utils:encode_hex(TraceId), + EncodedSpanId = otel_utils:encode_hex(SpanId), B3Context = otel_utils:assert_to_binary([EncodedTraceId, "-", EncodedSpanId, "-", Options]), CarrierSet(?B3_CONTEXT_KEY, B3Context, Carrier); diff --git a/apps/opentelemetry_api/src/otel_propagator_trace_context.erl b/apps/opentelemetry_api/src/otel_propagator_trace_context.erl index 6958c214..364808bf 100644 --- a/apps/opentelemetry_api/src/otel_propagator_trace_context.erl +++ b/apps/opentelemetry_api/src/otel_propagator_trace_context.erl @@ -109,8 +109,8 @@ encode_span_ctx(#span_ctx{trace_id=TraceId, encode_traceparent(TraceId, SpanId, TraceOptions) -> Options = case TraceOptions band 1 of 1 -> <<"01">>; _ -> <<"00">> end, - EncodedTraceId = string:lowercase(binary:encode_hex(TraceId)), - EncodedSpanId = string:lowercase(binary:encode_hex(SpanId)), + EncodedTraceId = otel_utils:encode_hex(TraceId), + EncodedSpanId = otel_utils:encode_hex(SpanId), otel_utils:assert_to_binary([?VERSION, "-", EncodedTraceId, "-", EncodedSpanId, "-", Options]). diff --git a/apps/opentelemetry_api/src/otel_span.erl b/apps/opentelemetry_api/src/otel_span.erl index 923c93c5..2e9d7c87 100644 --- a/apps/opentelemetry_api/src/otel_span.erl +++ b/apps/opentelemetry_api/src/otel_span.erl @@ -167,19 +167,19 @@ span_id(#span_ctx{span_id=SpanId}) -> hex_span_ctx(#span_ctx{trace_id=TraceId, span_id=SpanId, trace_flags=TraceFlags}) -> - #{otel_trace_id => string:lowercase(binary:encode_hex(TraceId)), - otel_span_id => string:lowercase(binary:encode_hex(SpanId)), + #{otel_trace_id => otel_utils:encode_hex(TraceId), + otel_span_id => otel_utils:encode_hex(SpanId), otel_trace_flags => case TraceFlags band 1 of 1 -> "01"; _ -> "00" end}; hex_span_ctx(_) -> #{}. -spec hex_trace_id(opentelemetry:span_ctx()) -> opentelemetry:hex_trace_id(). hex_trace_id(#span_ctx{trace_id=TraceId}) -> - string:lowercase(binary:encode_hex(TraceId)). + otel_utils:encode_hex(TraceId). -spec hex_span_id(opentelemetry:span_ctx()) -> opentelemetry:hex_span_id(). hex_span_id(#span_ctx{span_id=SpanId}) -> - string:lowercase(binary:encode_hex(SpanId)). + otel_utils:encode_hex(SpanId). -spec tracestate(opentelemetry:span_ctx() | undefined) -> opentelemetry:tracestate(). tracestate(#span_ctx{tracestate=Tracestate}) -> diff --git a/apps/opentelemetry_api/src/otel_utils.erl b/apps/opentelemetry_api/src/otel_utils.erl index e76091f9..a3ee4362 100644 --- a/apps/opentelemetry_api/src/otel_utils.erl +++ b/apps/opentelemetry_api/src/otel_utils.erl @@ -21,7 +21,8 @@ format_binary_string/2, format_binary_string/3, assert_to_binary/1, - unicode_to_binary/1]). + unicode_to_binary/1, + encode_hex/1]). -if(?OTP_RELEASE >= 24). format_exception(Kind, Reason, StackTrace) -> @@ -56,3 +57,11 @@ unicode_to_binary(String) -> _ -> {error, bad_binary_conversion} end. + +-if(?OTP_RELEASE >= 26). +encode_hex(Bin) -> + binary:encode_hex(Bin, lowercase). +-else. +encode_hex(Bin) -> + string:lowercase(binary:encode_hex(Bin)). +-endif. From a5a38222902534e7c8b96eb39760a0f704391877 Mon Sep 17 00:00:00 2001 From: Tristan Sloughter Date: Wed, 17 May 2023 16:19:43 -0600 Subject: [PATCH 04/10] fix zipkin exporter for use of binary trace/span ids --- apps/opentelemetry/src/otel_links.erl | 4 ++-- .../src/otel_sampler_trace_id_ratio_based.erl | 5 ++--- apps/opentelemetry_exporter/src/otel_otlp_traces.erl | 2 +- .../opentelemetry_zipkin/src/opentelemetry_zipkin.erl | 11 +++-------- rebar.config | 1 - 5 files changed, 8 insertions(+), 15 deletions(-) diff --git a/apps/opentelemetry/src/otel_links.erl b/apps/opentelemetry/src/otel_links.erl index 37cf3dae..bc58d326 100644 --- a/apps/opentelemetry/src/otel_links.erl +++ b/apps/opentelemetry/src/otel_links.erl @@ -69,8 +69,8 @@ create_links([L | Rest], Limit, AttributePerLinkLimit, AttributeValueLengthLimit %% new_link({TraceId, SpanId, Attributes, TraceState}, AttributePerLinkLimit, AttributeValueLengthLimit) - when is_integer(TraceId), - is_integer(SpanId), + when is_binary(TraceId), + is_binary(SpanId), (is_list(Attributes) orelse is_map(Attributes)), is_list(TraceState) -> #link{trace_id=TraceId, diff --git a/apps/opentelemetry/src/otel_sampler_trace_id_ratio_based.erl b/apps/opentelemetry/src/otel_sampler_trace_id_ratio_based.erl index a9e31410..e10f70e2 100644 --- a/apps/opentelemetry/src/otel_sampler_trace_id_ratio_based.erl +++ b/apps/opentelemetry/src/otel_sampler_trace_id_ratio_based.erl @@ -61,9 +61,8 @@ decide(undefined, _IdUpperBound) -> ?DROP; decide(0, _IdUpperBound) -> ?DROP; -decide(TraceId, IdUpperBound) -> - Lower64Bits = TraceId band ?MAX_VALUE, - case erlang:abs(Lower64Bits) < IdUpperBound of +decide(<<_:65, LowerBits:63>>, IdUpperBound) -> + case erlang:abs(LowerBits) < IdUpperBound of true -> ?RECORD_AND_SAMPLE; false -> ?DROP end. diff --git a/apps/opentelemetry_exporter/src/otel_otlp_traces.erl b/apps/opentelemetry_exporter/src/otel_otlp_traces.erl index af3f2b87..e644ea5b 100644 --- a/apps/opentelemetry_exporter/src/otel_otlp_traces.erl +++ b/apps/opentelemetry_exporter/src/otel_otlp_traces.erl @@ -78,7 +78,7 @@ to_proto(#span{trace_id=TraceId, status=Status, trace_flags=_TraceFlags, is_recording=_IsRecording}) -> - ParentSpanId = case MaybeParentSpanId of _ when is_integer(MaybeParentSpanId) -> <>; _ -> <<>> end, + ParentSpanId = case MaybeParentSpanId of undefined -> <<>>; _ -> MaybeParentSpanId end, #{name => otel_otlp_common:to_binary(Name), trace_id => TraceId, span_id => SpanId, diff --git a/apps/opentelemetry_zipkin/src/opentelemetry_zipkin.erl b/apps/opentelemetry_zipkin/src/opentelemetry_zipkin.erl index b9708140..9570576e 100644 --- a/apps/opentelemetry_zipkin/src/opentelemetry_zipkin.erl +++ b/apps/opentelemetry_zipkin/src/opentelemetry_zipkin.erl @@ -75,15 +75,15 @@ zipkin_span(Span, LocalEndpoint) -> Timestamp = ?assert_type(opentelemetry:convert_timestamp(StartTime, microsecond), non_neg_integer()), Duration = ?assert_type(erlang:convert_time_unit(EndTime - StartTime, native, microsecond), non_neg_integer()), #zipkin_span{ - trace_id = <<(?assert_type(Span#span.trace_id, opentelemetry:trace_id())):128>>, + trace_id = Span#span.trace_id, name=to_binary_string(Span#span.name), - id = <<(?assert_type(Span#span.span_id, opentelemetry:span_id())):64>>, + id = Span#span.span_id, timestamp=Timestamp, duration=Duration, %% debug=false, %% TODO: get from attributes? %% shared=false, %% TODO: get from attributes? kind=to_kind(Span#span.kind), - parent_id=to_parent_id(Span#span.parent_span_id), + parent_id=Span#span.parent_span_id, annotations=to_annotations(otel_events:list(Span#span.events)), tags=to_tags(Span), local_endpoint=LocalEndpoint @@ -154,11 +154,6 @@ attributes_to_tags(Attributes) -> [{to_binary_string(Name), to_binary_string(Value)} | Acc] end, [], otel_attributes:map(Attributes)). -to_parent_id(undefined) -> - undefined; -to_parent_id(ParentId) -> - <>. - to_kind(undefined) -> 'SPAN_KIND_UNSPECIFIED'; to_kind(?SPAN_KIND_INTERNAL) -> diff --git a/rebar.config b/rebar.config index d3058734..fab6bc88 100644 --- a/rebar.config +++ b/rebar.config @@ -60,7 +60,6 @@ opentelemetry_exporter_logs_service_pb, opentelemetry_zipkin_pb]}. - {dialyzer, [{warnings, [no_unknown]}]}. {cover_enabled, true}. From 371b2cb0ad730455aa7c6864a035be478a8870ad Mon Sep 17 00:00:00 2001 From: Tristan Sloughter Date: Thu, 18 May 2023 10:15:49 -0600 Subject: [PATCH 05/10] update sampler trace id ratio based tests for binary ids --- apps/opentelemetry/test/otel_samplers_SUITE.erl | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/apps/opentelemetry/test/otel_samplers_SUITE.erl b/apps/opentelemetry/test/otel_samplers_SUITE.erl index 865d6141..7a828b93 100644 --- a/apps/opentelemetry/test/otel_samplers_SUITE.erl +++ b/apps/opentelemetry/test/otel_samplers_SUITE.erl @@ -48,8 +48,8 @@ get_description(_Config) -> trace_id_ratio_based(_Config) -> SpanName = <<"span-prob-sampled">>, Probability = 0.5, - DoSample = 120647249294066572380176333851662846319, - DoNotSample = 53020601517903903921384168845238205400, + DoSample = <<120647249294066572380176333851662846319:128>>, + DoNotSample = <<53020601517903903921384168845238205400:128>>, Ctx = otel_ctx:new(), @@ -140,8 +140,8 @@ trace_id_ratio_based(_Config) -> parent_based(_Config) -> SpanName = <<"span-prob-sampled">>, Probability = 0.5, - DoSample = 120647249294066572380176333851662846319, - DoNotSample = 53020601517903903921384168845238205400, + DoSample = <<120647249294066572380176333851662846319:128>>, + DoNotSample = <<53020601517903903921384168845238205400:128>>, Ctx = otel_ctx:new(), From f541f41159da54a2c91b6d763ed4580628c82682 Mon Sep 17 00:00:00 2001 From: Tristan Sloughter Date: Thu, 18 May 2023 10:20:34 -0600 Subject: [PATCH 06/10] fix elixir trace_id/span_id types to reference the Erlang types --- apps/opentelemetry_api/lib/open_telemetry.ex | 6 +++--- apps/opentelemetry_api/test/open_telemetry_test.exs | 10 +++++----- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/apps/opentelemetry_api/lib/open_telemetry.ex b/apps/opentelemetry_api/lib/open_telemetry.ex index 0bca4ba2..1b05f8b7 100644 --- a/apps/opentelemetry_api/lib/open_telemetry.ex +++ b/apps/opentelemetry_api/lib/open_telemetry.ex @@ -44,14 +44,14 @@ defmodule OpenTelemetry do the same `trace_id`. The ID is a 16-byte array. An ID with all zeroes is considered invalid. """ - @type trace_id() :: non_neg_integer() + @type trace_id() :: :opentelemetry.trace_id() @typedoc """ SpanId is a unique identifier for a span within a trace, assigned when the span is created. The ID is an 8-byte array. An ID with all zeroes is considered invalid. """ - @type span_id() :: non_neg_integer() + @type span_id() :: :opentelemetry.span_id() @type attribute_key() :: :opentelemetry.attribute_key() @type attribute_value() :: :opentelemetry.attribute_value() @@ -167,7 +167,7 @@ defmodule OpenTelemetry do Creates a list of `t:link/0` from a list of 4-tuples. """ @spec links([ - {integer(), integer(), attributes_map(), tracestate()} + {trace_id(), span_id(), attributes_map(), tracestate()} | span_ctx() | {span_ctx(), attributes_map()} ]) :: [link()] diff --git a/apps/opentelemetry_api/test/open_telemetry_test.exs b/apps/opentelemetry_api/test/open_telemetry_test.exs index 0a24eb6a..4a81a331 100644 --- a/apps/opentelemetry_api/test/open_telemetry_test.exs +++ b/apps/opentelemetry_api/test/open_telemetry_test.exs @@ -24,20 +24,20 @@ defmodule OpenTelemetryTest do end test "link creation" do - ctx = span_ctx(trace_id: 1, span_id: 2, tracestate: []) + ctx = span_ctx(trace_id: <<1::128>>, span_id: <<2::64>>, tracestate: []) %{trace_id: t, span_id: s, attributes: a, tracestate: ts} = OpenTelemetry.link(ctx) - assert 1 == t - assert 2 == s + assert <<1::128>> == t + assert <<2::64>> == s assert [] == ts assert %{} == a %{trace_id: t, span_id: s, attributes: a, tracestate: ts} = OpenTelemetry.link(ctx, [{"attr-1", "value-1"}]) - assert 1 == t - assert 2 == s + assert <<1::128>> == t + assert <<2::64>> == s assert [] == ts assert %{"attr-1" => "value-1"} == a end From fcf0b9a8264443101d355bbaecf0ccf290089ef9 Mon Sep 17 00:00:00 2001 From: Tristan Sloughter Date: Thu, 18 May 2023 11:17:41 -0600 Subject: [PATCH 07/10] expand trace id ratio based tests --- .../test/otel_samplers_SUITE.erl | 176 ++++++++++++------ 1 file changed, 117 insertions(+), 59 deletions(-) diff --git a/apps/opentelemetry/test/otel_samplers_SUITE.erl b/apps/opentelemetry/test/otel_samplers_SUITE.erl index 7a828b93..deeb161c 100644 --- a/apps/opentelemetry/test/otel_samplers_SUITE.erl +++ b/apps/opentelemetry/test/otel_samplers_SUITE.erl @@ -58,82 +58,140 @@ trace_id_ratio_based(_Config) -> %% checks the trace id is under the upper bound ?assertMatch( - {?RECORD_AND_SAMPLE, [], []}, - Sampler:should_sample( - otel_tracer:set_current_span(Ctx, undefined), - DoSample, - [], - SpanName, - undefined, - [], - Opts + {?RECORD_AND_SAMPLE, [], []}, + Sampler:should_sample( + otel_tracer:set_current_span(Ctx, undefined), + DoSample, + [], + SpanName, + undefined, + [], + Opts ) - ), + ), %% checks the trace id is is over the upper bound ?assertMatch( - {?DROP, [], []}, - Sampler:should_sample( - otel_tracer:set_current_span(Ctx, undefined), - DoNotSample, - [], - SpanName, - undefined, - [], - Opts + {?DROP, [], []}, + Sampler:should_sample( + otel_tracer:set_current_span(Ctx, undefined), + DoNotSample, + [], + SpanName, + undefined, + [], + Opts ) - ), + ), %% ignores the parent span context trace flags ?assertMatch( - {?DROP, [], []}, - Sampler:should_sample( - otel_tracer:set_current_span(Ctx, #span_ctx{ - trace_flags = 1, - is_remote = true - }), - DoNotSample, - [], - SpanName, - undefined, - [], - Opts + {?DROP, [], []}, + Sampler:should_sample( + otel_tracer:set_current_span(Ctx, #span_ctx{ + trace_flags = 1, + is_remote = true + }), + DoNotSample, + [], + SpanName, + undefined, + [], + Opts ) - ), + ), %% ignores the parent span context trace flags ?assertMatch( - {?RECORD_AND_SAMPLE, [], []}, - Sampler:should_sample( - otel_tracer:set_current_span(Ctx, #span_ctx{ - trace_flags = 0, - is_remote = false - }), - DoSample, - [], - SpanName, - undefined, - [], - Opts + {?RECORD_AND_SAMPLE, [], []}, + Sampler:should_sample( + otel_tracer:set_current_span(Ctx, #span_ctx{ + trace_flags = 0, + is_remote = false + }), + DoSample, + [], + SpanName, + undefined, + [], + Opts ) - ), + ), %% trace id is under the upper bound ?assertMatch( - {?RECORD_AND_SAMPLE, [], []}, - Sampler:should_sample( - otel_tracer:set_current_span(Ctx, #span_ctx{ - trace_flags = 0, - is_remote = true - }), - DoSample, - [], - SpanName, - undefined, - [], - Opts + {?RECORD_AND_SAMPLE, [], []}, + Sampler:should_sample( + otel_tracer:set_current_span(Ctx, #span_ctx{ + trace_flags = 0, + is_remote = true + }), + DoSample, + [], + SpanName, + undefined, + [], + Opts ) - ), + ), + + %% everything is dropped + {ZeroSampler, _, ZeroOpts} = otel_sampler:new({trace_id_ratio_based, 0.0}), + ?assertMatch( + {?DROP, [], []}, + ZeroSampler:should_sample( + otel_tracer:set_current_span(Ctx, undefined), + DoNotSample, + [], + SpanName, + undefined, + [], + ZeroOpts + ) + ), + + %% everything is sampled + {OneSampler, _, OneOpts} = otel_sampler:new({trace_id_ratio_based, 1.0}), + ?assertMatch( + {?RECORD_AND_SAMPLE, [], []}, + OneSampler:should_sample( + otel_tracer:set_current_span(Ctx, undefined), + DoSample, + [], + SpanName, + undefined, + [], + OneOpts + ) + ), + + %% drop for undefined trace id + ?assertMatch( + {?DROP, [], []}, + OneSampler:should_sample( + otel_tracer:set_current_span(Ctx, undefined), + undefined, + [], + SpanName, + undefined, + [], + OneOpts + ) + ), + + %% drop for 0 trace id + ?assertMatch( + {?DROP, [], []}, + OneSampler:should_sample( + otel_tracer:set_current_span(Ctx, undefined), + undefined, + [], + SpanName, + undefined, + [], + OneOpts + ) + ), ok. From 61379117f204d6dc4ce52ad3511ae92342cf7ef9 Mon Sep 17 00:00:00 2001 From: Tristan Sloughter Date: Thu, 18 May 2023 11:33:22 -0600 Subject: [PATCH 08/10] add test of propagator fields function --- apps/opentelemetry/test/otel_propagation_SUITE.erl | 1 + 1 file changed, 1 insertion(+) diff --git a/apps/opentelemetry/test/otel_propagation_SUITE.erl b/apps/opentelemetry/test/otel_propagation_SUITE.erl index c835ac53..9751de29 100644 --- a/apps/opentelemetry/test/otel_propagation_SUITE.erl +++ b/apps/opentelemetry/test/otel_propagation_SUITE.erl @@ -67,6 +67,7 @@ propagation(Config) -> ?assertMatch(#span_ctx{trace_flags=1}, ?current_span_ctx), ?assertMatch(#span_ctx{is_recording=true}, ?current_span_ctx), + ?assertMatch([_ | _], otel_propagator_text_map:fields(opentelemetry:get_text_map_injector())), otel_baggage:set("key-1", <<"value=1">>, []), %% TODO: should the whole baggage entry be dropped if metadata is bad? From e3ac4bec21ef7923ee5cfa38147b915585ccceab Mon Sep 17 00:00:00 2001 From: Tristan Sloughter Date: Thu, 18 May 2023 11:46:50 -0600 Subject: [PATCH 09/10] cleanup b3single code to remove unnecessary guards --- .../src/otel_propagator_b3single.erl | 25 ++++++++----------- 1 file changed, 11 insertions(+), 14 deletions(-) diff --git a/apps/opentelemetry_api/src/otel_propagator_b3single.erl b/apps/opentelemetry_api/src/otel_propagator_b3single.erl index bd49d75a..d0bee698 100644 --- a/apps/opentelemetry_api/src/otel_propagator_b3single.erl +++ b/apps/opentelemetry_api/src/otel_propagator_b3single.erl @@ -101,26 +101,22 @@ decode_b3_context(_) -> throw(invalid). % Trace ID is a 32 or 16 lower-hex character binary. -parse_trace_id(TraceId) when is_binary(TraceId) -> +parse_trace_id(TraceId) -> case string:length(TraceId) =:= 32 orelse string:length(TraceId) =:= 16 of true -> binary:decode_hex(TraceId); _ -> throw(invalid) - end; -parse_trace_id(_) -> - throw(invalid). + end. % Span ID is a 16 lower-hex character binary. -parse_span_id(SpanId) when is_binary(SpanId) -> +parse_span_id(SpanId) -> case string:length(SpanId) =:= 16 of true -> binary:decode_hex(SpanId); _ -> throw(invalid) - end; -parse_span_id(_) -> - throw(invalid). + end. % Sampling State is encoded as a single hex character for all states except % Defer. Defer is absence of the sampling field. @@ -132,11 +128,12 @@ parse_span_id(_) -> % % Before the specification was written, some tracers propagated X-B3-Sampled as % true or false. -parse_is_sampled(Sampled) when is_binary(Sampled) -> - case Sampled of - S when S =:= <<"1">> orelse S =:= <<"d">> orelse S =:= <<"true">> -> 1; - S when S =:= <<"0">> orelse S =:= <<"false">> -> 0; - _ -> throw(invalid) - end; +parse_is_sampled(Sampled) when Sampled =:= <<"1">> orelse + Sampled =:= <<"d">> orelse + Sampled =:= <<"true">> -> + 1; +parse_is_sampled(Sampled) when Sampled =:= <<"0">> orelse + Sampled =:= <<"false">> -> + 0; parse_is_sampled(_) -> throw(invalid). From 1d0d7c50c0542625539321c5c41e4111549f2a98 Mon Sep 17 00:00:00 2001 From: Tristan Sloughter Date: Thu, 18 May 2023 12:02:41 -0600 Subject: [PATCH 10/10] update changelog for PR-590 --- CHANGELOG.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2b3486e4..70251325 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## API + +### Changed + +- [`trace_id` and `span_id` now represented as binaries instead of + integers](https://github.com/open-telemetry/opentelemetry-erlang/pull/590/) + ## SDK ### Added