From 6c6d75daa9088b0b417be668c16422a82e771b91 Mon Sep 17 00:00:00 2001 From: Tristan Sloughter Date: Fri, 3 Sep 2021 08:27:53 -0600 Subject: [PATCH] support manually specifying propagators in inject/extract Since a new option to inject/extract was needed I've added 2 new functions `inject_from` and `extract_to` that take Contexts as the first argument. Otherwise there is a lot of overloading of types of arguments required. --- .../test/opentelemetry_SUITE.erl | 96 --------- .../test/otel_propagation_SUITE.erl | 192 ++++++++++++++++++ apps/opentelemetry_api/src/opentelemetry.erl | 4 +- .../opentelemetry_api/src/otel_propagator.erl | 29 ++- .../src/otel_propagator_text_map.erl | 86 +++++--- 5 files changed, 280 insertions(+), 127 deletions(-) create mode 100644 apps/opentelemetry/test/otel_propagation_SUITE.erl diff --git a/apps/opentelemetry/test/opentelemetry_SUITE.erl b/apps/opentelemetry/test/opentelemetry_SUITE.erl index a4872013..05d74512 100644 --- a/apps/opentelemetry/test/opentelemetry_SUITE.erl +++ b/apps/opentelemetry/test/opentelemetry_SUITE.erl @@ -14,21 +14,12 @@ -include("otel_span_ets.hrl"). all() -> - [all_testcases(), - {group, w3c}, - {group, b3}]. - -all_testcases() -> [disable_auto_registration, registered_tracers, with_span, macros, child_spans, update_span_data, tracer_instrumentation_library, tracer_previous_ctx, stop_temporary_app, reset_after, attach_ctx, default_sampler, non_recording_ets_table, root_span_sampling_always_on, root_span_sampling_always_off, record_but_not_sample, record_exception_works, record_exception_with_message_works]. -groups() -> - [{w3c, [], [propagation]}, - {b3, [], [propagation]}]. - init_per_suite(Config) -> application:load(opentelemetry), Config. @@ -37,25 +28,6 @@ end_per_suite(_Config) -> application:unload(opentelemetry), ok. -init_per_group(Propagator, Config) when Propagator =:= w3c ; - Propagator =:= b3 -> - application:set_env(opentelemetry, processors, [{otel_batch_processor, #{scheduled_delay_ms => 1}}]), - {ok, _} = application:ensure_all_started(opentelemetry), - - case Propagator of - w3c -> - opentelemetry:set_text_map_propagators([otel_propagator_baggage, - otel_propagator_trace_context]); - b3 -> - opentelemetry:set_text_map_propagators([otel_propagator_baggage, - otel_propagator_b3]) - end, - - [{propagator, Propagator} | Config]. - -end_per_group(_, _Config) -> - _ = application:stop(opentelemetry). - init_per_testcase(disable_auto_registration, Config) -> application:set_env(opentelemetry, register_loaded_applications, false), {ok, _} = application:ensure_all_started(opentelemetry), @@ -232,67 +204,6 @@ update_span_data(Config) -> events=Events, _='_'})). -propagation(Config) -> - Propagator = ?config(propagator, Config), - SpanCtx=#span_ctx{trace_id=TraceId, - span_id=SpanId} = ?start_span(<<"span-1">>), - ?set_current_span(SpanCtx), - - ?assertMatch(#span_ctx{trace_flags=1}, ?current_span_ctx), - ?assertMatch(#span_ctx{is_recording=true}, ?current_span_ctx), - - - otel_baggage:set("key-1", <<"value=1">>, []), - %% TODO: should the whole baggage entry be dropped if metadata is bad? - %% drop bad metadata (the `1'). - otel_baggage:set(<<"key-2">>, <<"value-2">>, [<<"metadata">>, 1, {<<"md-k-1">>, <<"md-v-1">>}]), - %% drop baggage with bad value - otel_baggage:set(<<"key-3">>, value3), - - 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]), - - ?assertListsEqual([{<<"baggage">>, <<"key-2=value-2;metadata;md-k-1=md-v-1,key-1=value%3D1">>}, - {<<"existing-header">>, <<"I exist">>} | - trace_context(Propagator, EncodedTraceId, EncodedSpanId)], Headers), - - ?end_span(SpanCtx), - - ?assertEqual(#{<<"key-1">> => {<<"value=1">>, []}, - <<"key-2">> => {<<"value-2">>, [<<"metadata">>, {<<"md-k-1">>, <<"md-v-1">>}]}}, - otel_baggage:get_all()), - - %% ?end_span doesn't remove the span from the context - ?assertEqual(SpanCtx, ?current_span_ctx), - ?set_current_span(undefined), - ?assertEqual(undefined, ?current_span_ctx), - - %% clear our baggage from the context to test extraction - otel_baggage:clear(), - ?assertEqual(#{}, otel_baggage:get_all()), - - %% make header keys uppercase to validate the extractor is case insensitive - BinaryHeaders = [{string:uppercase(Key), iolist_to_binary(Value)} || {Key, Value} <- Headers], - otel_propagator_text_map:extract(BinaryHeaders), - - ?assertEqual(#{<<"key-1">> => {<<"value=1">>, []}, - <<"key-2">> => {<<"value-2">>, [<<"metadata">>, {<<"md-k-1">>, <<"md-v-1">>}]}}, - otel_baggage:get_all()), - - %% extracted remote spans are set to the active span - %% but with `is_recording' false - ?assertMatch(#span_ctx{is_recording=false}, ?current_span_ctx), - - #span_ctx{trace_id=TraceId2, - span_id=_SpanId2} = ?start_span(<<"span-2">>), - - %% new span should be a child of the extracted span - ?assertEqual(TraceId, TraceId2), - - ok. - tracer_instrumentation_library(Config) -> Tid = ?config(tid, Config), @@ -597,10 +508,3 @@ assert_not_exported(Tid, #span_ctx{trace_id=TraceId, span_id=SpanId, _='_'})). -trace_context(w3c, EncodedTraceId, EncodedSpanId) -> - [{<<"traceparent">>, - iolist_to_binary([<<"00">>, "-", EncodedTraceId,"-", EncodedSpanId, "-", <<"01">>])}]; -trace_context(b3, EncodedTraceId, EncodedSpanId) -> - [{<<"X-B3-Sampled">>, <<"1">>}, - {<<"X-B3-SpanId">>, iolist_to_binary(EncodedSpanId)}, - {<<"X-B3-TraceId">>, iolist_to_binary(EncodedTraceId)}]. diff --git a/apps/opentelemetry/test/otel_propagation_SUITE.erl b/apps/opentelemetry/test/otel_propagation_SUITE.erl new file mode 100644 index 00000000..c4bf1fe2 --- /dev/null +++ b/apps/opentelemetry/test/otel_propagation_SUITE.erl @@ -0,0 +1,192 @@ +-module(otel_propagation_SUITE). + +-compile(export_all). + +-include_lib("stdlib/include/assert.hrl"). +-include_lib("common_test/include/ct.hrl"). + +-include_lib("opentelemetry_api/include/opentelemetry.hrl"). +-include_lib("opentelemetry_api/include/otel_tracer.hrl"). +-include("otel_tracer.hrl"). +-include("otel_span.hrl"). +-include("otel_test_utils.hrl"). +-include("otel_sampler.hrl"). +-include("otel_span_ets.hrl"). + +all() -> + [override_propagators, + {group, w3c}, + {group, b3}]. + +groups() -> + [{w3c, [], [propagation]}, + {b3, [], [propagation]}]. + +init_per_suite(Config) -> + application:load(opentelemetry), + application:set_env(opentelemetry, processors, [{otel_batch_processor, #{scheduled_delay_ms => 1}}]), + {ok, _} = application:ensure_all_started(opentelemetry), + Config. + +end_per_suite(_Config) -> + application:unload(opentelemetry), + ok. + +init_per_group(Propagator, Config) when Propagator =:= w3c ; + Propagator =:= b3 -> + %% start in group as well since we must stop it after each group run + {ok, _} = application:ensure_all_started(opentelemetry), + + case Propagator of + w3c -> + opentelemetry:set_text_map_propagators([otel_propagator_baggage, + otel_propagator_trace_context]); + b3 -> + opentelemetry:set_text_map_propagators([otel_propagator_baggage, + otel_propagator_b3]) + end, + + [{propagator, Propagator} | Config]. + +end_per_group(_, _Config) -> + _ = application:stop(opentelemetry). + +propagation(Config) -> + Propagator = ?config(propagator, Config), + SpanCtx=#span_ctx{trace_id=TraceId, + span_id=SpanId} = ?start_span(<<"span-1">>), + ?set_current_span(SpanCtx), + + ?assertMatch(#span_ctx{trace_flags=1}, ?current_span_ctx), + ?assertMatch(#span_ctx{is_recording=true}, ?current_span_ctx), + + + otel_baggage:set("key-1", <<"value=1">>, []), + %% TODO: should the whole baggage entry be dropped if metadata is bad? + %% drop bad metadata (the `1'). + otel_baggage:set(<<"key-2">>, <<"value-2">>, [<<"metadata">>, 1, {<<"md-k-1">>, <<"md-v-1">>}]), + %% drop baggage with bad value + otel_baggage:set(<<"key-3">>, value3), + + 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]), + + ?assertListsEqual([{<<"baggage">>, <<"key-2=value-2;metadata;md-k-1=md-v-1,key-1=value%3D1">>}, + {<<"existing-header">>, <<"I exist">>} | + trace_context(Propagator, EncodedTraceId, EncodedSpanId)], Headers), + + ?end_span(SpanCtx), + + ?assertEqual(#{<<"key-1">> => {<<"value=1">>, []}, + <<"key-2">> => {<<"value-2">>, [<<"metadata">>, {<<"md-k-1">>, <<"md-v-1">>}]}}, + otel_baggage:get_all()), + + %% ?end_span doesn't remove the span from the context + ?assertEqual(SpanCtx, ?current_span_ctx), + ?set_current_span(undefined), + ?assertEqual(undefined, ?current_span_ctx), + + %% clear our baggage from the context to test extraction + otel_baggage:clear(), + ?assertEqual(#{}, otel_baggage:get_all()), + + %% make header keys uppercase to validate the extractor is case insensitive + BinaryHeaders = [{string:uppercase(Key), iolist_to_binary(Value)} || {Key, Value} <- Headers], + otel_propagator_text_map:extract(BinaryHeaders), + + ?assertEqual(#{<<"key-1">> => {<<"value=1">>, []}, + <<"key-2">> => {<<"value-2">>, [<<"metadata">>, {<<"md-k-1">>, <<"md-v-1">>}]}}, + otel_baggage:get_all()), + + %% extracted remote spans are set to the active span + %% but with `is_recording' false + ?assertMatch(#span_ctx{is_recording=false}, ?current_span_ctx), + + #span_ctx{trace_id=TraceId2, + span_id=_SpanId2} = ?start_span(<<"span-2">>), + + %% new span should be a child of the extracted span + ?assertEqual(TraceId, TraceId2), + + ok. + +override_propagators(_Config) -> + SpanCtx=#span_ctx{} = ?start_span(<<"span-1">>), + ?set_current_span(SpanCtx), + + ?assertMatch(#span_ctx{trace_flags=1}, ?current_span_ctx), + ?assertMatch(#span_ctx{is_recording=true}, ?current_span_ctx), + + + otel_baggage:set("key-1", <<"value=1">>, []), + %% TODO: should the whole baggage entry be dropped if metadata is bad? + %% drop bad metadata (the `1'). + otel_baggage:set(<<"key-2">>, <<"value-2">>, [<<"metadata">>, 1, {<<"md-k-1">>, <<"md-v-1">>}]), + %% drop baggage with bad value + otel_baggage:set(<<"key-3">>, value3), + + Headers = otel_propagator_text_map:inject([{<<"existing-header">>, <<"I exist">>}], + #{propagators => [otel_propagator_baggage]}), + + %% the manually set propagators does not include trace_context or b3 + %% so header must only have the existing-header and the baggage + ?assertListsEqual([{<<"baggage">>, <<"key-2=value-2;metadata;md-k-1=md-v-1,key-1=value%3D1">>}, + {<<"existing-header">>, <<"I exist">>}], Headers), + + ?end_span(SpanCtx), + + ?assertEqual(#{<<"key-1">> => {<<"value=1">>, []}, + <<"key-2">> => {<<"value-2">>, [<<"metadata">>, {<<"md-k-1">>, <<"md-v-1">>}]}}, + otel_baggage:get_all()), + + %% ?end_span doesn't remove the span from the context + ?assertEqual(SpanCtx, ?current_span_ctx), + ?set_current_span(undefined), + ?assertEqual(undefined, ?current_span_ctx), + + %% clear our baggage from the context to test extraction + otel_baggage:clear(), + ?assertEqual(#{}, otel_baggage:get_all()), + + %% make header keys uppercase to validate the extractor is case insensitive + BinaryHeaders = [{string:uppercase(Key), iolist_to_binary(Value)} || {Key, Value} <- Headers], + otel_propagator_text_map:extract(BinaryHeaders, #{propagators => [otel_propagator_baggage]}), + + ?assertEqual(#{<<"key-1">> => {<<"value=1">>, []}, + <<"key-2">> => {<<"value-2">>, [<<"metadata">>, {<<"md-k-1">>, <<"md-v-1">>}]}}, + otel_baggage:get_all()), + + %% no trace extractor used so current span ctx remains undefined + ?assertEqual(undefined, ?current_span_ctx), + + ok. + +%% + +assert_all_exported(Tid, SpanCtxs) -> + [assert_exported(Tid, SpanCtx) || SpanCtx <- SpanCtxs]. + +assert_exported(Tid, #span_ctx{trace_id=TraceId, + span_id=SpanId}) -> + ?UNTIL_NOT_EQUAL([], ets:match_object(Tid, #span{trace_id=TraceId, + span_id=SpanId, + _='_'})). + +assert_not_exported(Tid, #span_ctx{trace_id=TraceId, + span_id=SpanId}) -> + %% sleep so exporter has run before we check + %% since we can't do like when checking it exists with UNTIL + timer:sleep(100), + ?assertMatch([], ets:match(Tid, #span{trace_id=TraceId, + span_id=SpanId, + _='_'})). + +trace_context(w3c, EncodedTraceId, EncodedSpanId) -> + [{<<"traceparent">>, + iolist_to_binary([<<"00">>, "-", EncodedTraceId,"-", EncodedSpanId, "-", <<"01">>])}]; +trace_context(b3, EncodedTraceId, EncodedSpanId) -> + [{<<"X-B3-Sampled">>, <<"1">>}, + {<<"X-B3-SpanId">>, iolist_to_binary(EncodedSpanId)}, + {<<"X-B3-TraceId">>, iolist_to_binary(EncodedTraceId)}]. diff --git a/apps/opentelemetry_api/src/opentelemetry.erl b/apps/opentelemetry_api/src/opentelemetry.erl index 0c5f7dfb..990c1f8d 100644 --- a/apps/opentelemetry_api/src/opentelemetry.erl +++ b/apps/opentelemetry_api/src/opentelemetry.erl @@ -159,13 +159,13 @@ set_text_map_propagators(_) -> ok. set_text_map_extractors(List) when is_list(List) -> - ParsedList = otel_propagator:builtin_to_module(List), + ParsedList = otel_propagator:builtins_to_modules(List), persistent_term:put({?MODULE, text_map_extractors}, ParsedList); set_text_map_extractors(_) -> ok. set_text_map_injectors(List) when is_list(List) -> - ParsedList = otel_propagator:builtin_to_module(List), + ParsedList = otel_propagator:builtins_to_modules(List), persistent_term:put({?MODULE, text_map_injectors}, ParsedList); set_text_map_injectors(_) -> ok. diff --git a/apps/opentelemetry_api/src/otel_propagator.erl b/apps/opentelemetry_api/src/otel_propagator.erl index 0c236760..56405bdb 100644 --- a/apps/opentelemetry_api/src/otel_propagator.erl +++ b/apps/opentelemetry_api/src/otel_propagator.erl @@ -31,21 +31,42 @@ %%%------------------------------------------------------------------------- -module(otel_propagator). --export([builtin_to_module/1]). +-export([builtins_to_modules/1, + builtin_to_module/1]). %% Sets a value into a carrier --callback inject(otel_ctx:t(), carrier()) -> carrier(). +-callback inject(carrier()) -> carrier(). +-callback inject(carrier(), propagator_options()) -> carrier(). +-callback inject_from(otel_ctx:t(), carrier()) -> carrier(). +-callback inject_from(otel_ctx:t(), carrier(), propagator_options()) -> carrier(). %% extracts values from a carrier and sets them in the context --callback extract(otel_ctx:t(), carrier()) -> otel_ctx:t(). +-callback extract(carrier()) -> otel_ctx:t(). +-callback extract(carrier(), propagator_options()) -> otel_ctx:t(). +-callback extract_to(otel_ctx:t(), carrier()) -> otel_ctx:t(). +-callback extract_to(otel_ctx:t(), carrier(), propagator_options()) -> otel_ctx:t(). + +-type propagator_options() :: #{propagators => [t()]}. + +-type t() :: builtin() | module(). + +-type builtin() :: trace_context | tracecontext | b3. %% multib3 | jaeger %% a carrier can be any type -type carrier() :: term(). --export_type([carrier/0]). +-export_type([t/0, + builtin/0, + carrier/0]). %% convert the short name of a propagator to its module name if it is a builtin %% if the name doens't match a builtin it is assumed to be a module %% @hidden +-spec builtins_to_modules([t()]) -> [module()]. +builtins_to_modules(Propagators) -> + [builtin_to_module(P) || P <- Propagators]. + +%% @hidden +-spec builtin_to_module(builtin() | module()) -> module(). builtin_to_module(tracecontext) -> otel_propagator_trace_context; builtin_to_module(trace_context) -> diff --git a/apps/opentelemetry_api/src/otel_propagator_text_map.erl b/apps/opentelemetry_api/src/otel_propagator_text_map.erl index 24c71bbd..36dba3bb 100644 --- a/apps/opentelemetry_api/src/otel_propagator_text_map.erl +++ b/apps/opentelemetry_api/src/otel_propagator_text_map.erl @@ -59,10 +59,12 @@ -export([inject/1, inject/2, - inject/3, + inject_from/2, + inject_from/3, extract/1, extract/2, - extract/4]). + extract_to/2, + extract_to/3]). -export([default_carrier_get/2, default_carrier_set/3, @@ -91,34 +93,63 @@ -type default_text_map_carrier() :: [{unicode:latin1_binary(), unicode:latin1_binary()}]. +-type inject_options() :: #{carrier_set_fun => carrier_set(), + propagators => [otel_propagator:t()]}. + +-type extract_options() :: #{carrier_get_fun => carrier_get(), + carrier_keys_fun => carrier_keys(), + propagators => [otel_propagator:t()]}. -export_type([]). +-spec inject(otel_propagator:carrier()) -> otel_propagator:carrier(). inject(Carrier) -> - Context = otel_ctx:get_current(), - inject(Context, Carrier, fun ?MODULE:default_carrier_set/3). - -inject(Context, Carrier) -> - inject(Context, Carrier, fun ?MODULE:default_carrier_set/3). + inject(Carrier, #{}). -inject(Context, Carrier, CarrierSet) -> - Injectors = opentelemetry:get_text_map_injectors(), - run_injectors(Context, Carrier, Injectors, CarrierSet). - -extract(Carrier) -> +-spec inject(otel_propagator:carrier(), inject_options()) -> otel_propagator:carrier(). +inject(Carrier, InjectOptions) -> Context = otel_ctx:get_current(), - extract(Context, Carrier, fun ?MODULE:default_carrier_keys/1, fun ?MODULE:default_carrier_get/2). - -extract(Context, Carrier) -> - extract(Context, Carrier, fun ?MODULE:default_carrier_keys/1, fun ?MODULE:default_carrier_get/2). + inject_from(Context, Carrier, InjectOptions). + +-spec inject_from(otel_ctx:t(), otel_propagator:carrier()) -> otel_propagator:carrier(). +inject_from(Context, Carrier) -> + inject_from(Context, Carrier, #{}). + +-spec inject_from(otel_ctx:t(), otel_propagator:carrier(), inject_options()) -> otel_propagator:carrier(). +inject_from(Context, Carrier, InjectOptions) when is_map(InjectOptions)-> + Injectors = maps:get(propagators, InjectOptions, opentelemetry:get_text_map_injectors()), + CarrierSetFun = maps:get(carrier_set_fun, InjectOptions, fun ?MODULE:default_carrier_set/3), + run_injectors(Context, Injectors, Carrier, CarrierSetFun); +inject_from(_Context, Carrier, InjectOptions) -> + ?LOG_INFO("inject failed. InjectOptions must be a map but instead got: ~p", [InjectOptions]), + Carrier. + +-spec extract(otel_propagator:carrier()) -> otel_ctx:t(). +extract(Carrier) -> + extract(Carrier, #{}). -extract(Context, Carrier, CarrierKeysFun, CarrierGet) -> - Extractors = opentelemetry:get_text_map_extractors(), - Context1 = run_extractors(Context, Carrier, Extractors, CarrierKeysFun, CarrierGet), +-spec extract(otel_propagator:carrier(), extract_options()) -> otel_ctx:t(). +extract(Carrier, ExtractOptions) -> + Context = otel_ctx:get_current(), + Context1 = extract_to(Context, Carrier, ExtractOptions), otel_ctx:attach(Context1). -run_extractors(Context, Carrier, Extractors, CarrierKeysFun, Getter) -> +-spec extract_to(otel_ctx:t(), otel_propagator:carrier()) -> otel_ctx:t(). +extract_to(Context, Carrier) -> + extract_to(Context, Carrier, #{}). + +-spec extract_to(otel_ctx:t(), otel_propagator:carrier(), extract_options()) -> otel_ctx:t(). +extract_to(Context, Carrier, ExtractOptions) when is_map(ExtractOptions) -> + Extractors = maps:get(propagators, ExtractOptions, opentelemetry:get_text_map_extractors()), + CarrierKeysFun = maps:get(carrier_keys_fun, ExtractOptions, fun ?MODULE:default_carrier_keys/1), + CarrierGetFun = maps:get(carrier_get_fun, ExtractOptions, fun ?MODULE:default_carrier_get/2), + run_extractors(Context, Extractors, Carrier, CarrierKeysFun, CarrierGetFun); +extract_to(Context, _Carrier, ExtractOptions) -> + ?LOG_INFO("extract failed. ExtractOptions must be a map but instead got: ~p", [ExtractOptions]), + Context. + +run_extractors(Context, Extractors, Carrier, CarrierKeysFun, CarrierGetFun) when is_list(Extractors) -> lists:foldl(fun(Extract, ContextAcc) -> - try Extract:extract(ContextAcc, Carrier, CarrierKeysFun, Getter) + try Extract:extract(ContextAcc, Carrier, CarrierKeysFun, CarrierGetFun) catch C:E:S -> ?LOG_INFO("text map propagator failed to extract from carrier", @@ -126,9 +157,12 @@ run_extractors(Context, Carrier, Extractors, CarrierKeysFun, Getter) -> class => C, exception => E, stacktrace => S}), ContextAcc end - end, Context, Extractors). + end, Context, otel_propagator:builtins_to_modules(Extractors)); +run_extractors(Context, Extractors, _, _, _) -> + ?LOG_INFO("extract failed. Extractors must be a list but instead got: ~p", [Extractors]), + Context. -run_injectors(Context, Carrier, Injectors, Setter) -> +run_injectors(Context, Injectors, Carrier, Setter) when is_list(Injectors) -> lists:foldl(fun(Inject, CarrierAcc) -> try Inject:inject(Context, CarrierAcc, Setter) catch @@ -138,8 +172,10 @@ run_injectors(Context, Carrier, Injectors, Setter) -> class => C, exception => E, stacktrace => S}), CarrierAcc end - end, Carrier, Injectors). - + end, Carrier, otel_propagator:builtins_to_modules(Injectors)); +run_injectors(_, Injectors, Carrier, _) -> + ?LOG_INFO("inject failed. Injectors must be a list but instead got: ~p", [Injectors]), + Carrier. %% case-insensitive finding of a key string in a list of ASCII strings %% if there are multiple entries in the list for the same key the values