Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update propagators with composite #273

Merged
merged 12 commits into from
Sep 24, 2021
Merged
29 changes: 29 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
# Changelog

## Unreleased

### API

#### Context

- Propagators must now be implementations of a propagator type's behaviour. At
this time only the `otel_propagator_text_map` behaviour exists. Callbacks for
inject and extract take an optional "set" and "get" function for working with
a carrier.
- Configuration of propagators is now a list of atoms representing either the
name of a builtin propagator (at this time those are, `trace_context`, `b3` and
`baggage`) or the name of a module implementing the propagator's behaviour.
- Default configuration: `{text_map_propagators, [trace_context, baggage]}`
- Injectors and extractors can be configured separately instead of using the
same list of propagators for both by configuring `text_map_injectors` and
`text_map_extractors`.
- For example you may want your service to support receiving `b3multi` headers
but have no need for it including `b3multi` headers when it is propagating to
other services:

```
{text_map_injectors, [trace_context, baggage]},
{text_map_extractors, [b3multi, trace_context, baggage]}
```
- `b3` propagator renamed `b3multi` to properly convey it is the version of the
B3 spec that creates multiple headers
2 changes: 1 addition & 1 deletion apps/opentelemetry/include/otel_span.hrl
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
%% 64 bit int span id
span_id :: opentelemetry:span_id() | undefined,

tracestate :: opentelemetry:tracestate() | undefined,
tracestate = [] :: opentelemetry:tracestate(),

%% 64 bit int parent span
parent_span_id :: opentelemetry:span_id() | undefined,
Expand Down
3 changes: 1 addition & 2 deletions apps/opentelemetry/src/opentelemetry.app.src
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,7 @@
]},
{env, [{sampler, {parent_based, #{root => always_on}}}, % default sampler

{text_map_propagators, [fun otel_baggage:get_text_map_propagators/0,
fun otel_tracer_default:w3c_propagators/0]},
{text_map_propagators, [trace_context, baggage]},

%% list of disabled tracers
{deny_list, []},
Expand Down
13 changes: 3 additions & 10 deletions apps/opentelemetry/src/opentelemetry_app.erl
Original file line number Diff line number Diff line change
Expand Up @@ -53,16 +53,9 @@ stop(_State) ->
%% internal functions

setup_text_map_propagators(Opts) ->
Propagators = proplists:get_value(text_map_propagators, Opts, []),

{Extractors, Injectors} =
lists:foldl(fun(F, {ExtractorsAcc, InjectorsAcc}) ->
{Extractor, Injector} = F(),
{[Extractor | ExtractorsAcc], [Injector | InjectorsAcc]}
end, {[], []}, Propagators),

opentelemetry:set_text_map_extractors(Extractors),
opentelemetry:set_text_map_injectors(Injectors).
List = proplists:get_value(text_map_propagators, Opts, []),
CompositePropagator = otel_propagator_text_map_composite:create(List),
opentelemetry:set_text_map_propagator(CompositePropagator).

register_loaded_application_tracers(Opts) ->
RegisterLoadedApplications = proplists:get_value(register_loaded_applications, Opts, true),
Expand Down
15 changes: 10 additions & 5 deletions apps/opentelemetry/src/otel_configuration.erl
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ merge_with_environment(ConfigMappings, Opts) ->

config_mappings(general_sdk) ->
[{"OTEL_LOG_LEVEL", log_level, "info", existing_atom},
{"OTEL_PROPAGATORS", propagators, "tracecontext,baggage", propagators},
{"OTEL_PROPAGATORS", text_map_propagators, "tracecontext,baggage", propagators},
{"OTEL_TRACES_EXPORTER", traces_exporter, "otlp", exporter},
{"OTEL_METRICS_EXPORTER", metrics_exporter, undefined, exporter}];
config_mappings(otel_batch_processor) ->
Expand Down Expand Up @@ -173,11 +173,16 @@ transform(propagators, PropagatorsString) when is_list(PropagatorsString) ->
end, Propagators);

transform(propagator, "tracecontext") ->
fun otel_tracer_default:w3c_propagators/0;
transform(propagator, "b3multi") ->
fun otel_tracer_default:b3_propagators/0;
trace_context;
transform(propagator, "baggage") ->
fun otel_baggage:get_text_map_propagators/0;
baggage;
transform(propagator, "b3multi") ->
b3multi;
%% TODO: support b3multi and jager propagator formats
%% transform(propagator, "b3") ->
%% b3;
%% transform(propagator, "jaeger") ->
%% jaeger;
transform(propagator, Propagator) ->
?LOG_WARNING("Ignoring uknown propagator ~ts in OS environment variable $OTEL_PROPAGATORS",
[Propagator]),
Expand Down
12 changes: 2 additions & 10 deletions apps/opentelemetry/src/otel_tracer_default.erl
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,8 @@
-behaviour(otel_tracer).

-export([start_span/4,
with_span/5,
b3_propagators/0,
w3c_propagators/0]).
with_span/5
]).

-include_lib("opentelemetry_api/include/opentelemetry.hrl").
-include("otel_tracer.hrl").
Expand Down Expand Up @@ -59,10 +58,3 @@ with_span(Ctx, Tracer, SpanName, Opts, Fun) ->
otel_ctx:detach(Ctx)
end.

-spec b3_propagators() -> {otel_propagator:text_map_extractor(), otel_propagator:text_map_injector()}.
b3_propagators() ->
otel_tracer:text_map_propagators(otel_propagator_http_b3).

-spec w3c_propagators() -> {otel_propagator:text_map_extractor(), otel_propagator:text_map_injector()}.
w3c_propagators() ->
otel_tracer:text_map_propagators(otel_propagator_http_w3c).
152 changes: 54 additions & 98 deletions apps/opentelemetry/test/opentelemetry_SUITE.erl
Original file line number Diff line number Diff line change
Expand Up @@ -14,20 +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]}].
record_but_not_sample, record_exception_works, record_exception_with_message_works,
propagator_configuration, propagator_configuration_with_os_env].

init_per_suite(Config) ->
application:load(opentelemetry),
Expand All @@ -37,30 +29,20 @@ 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),

{BaggageTextMapExtractor, BaggageTextMapInjector} = otel_baggage:get_text_map_propagators(),
{TraceTextMapExtractor, TraceTextMapInjector} = case Propagator of
w3c -> otel_tracer_default:w3c_propagators();
b3 -> otel_tracer_default:b3_propagators()
end,
opentelemetry:set_text_map_extractors([BaggageTextMapExtractor,
TraceTextMapExtractor]),
opentelemetry:set_text_map_injectors([BaggageTextMapInjector,
TraceTextMapInjector]),

[{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),
Config;
init_per_testcase(propagator_configuration, Config) ->
os:unsetenv("OTEL_PROPAGATORS"),
application:set_env(opentelemetry, text_map_propagators, [b3multi, baggage]),
{ok, _} = application:ensure_all_started(opentelemetry),
Config;
init_per_testcase(propagator_configuration_with_os_env, Config) ->
os:putenv("OTEL_PROPAGATORS", "tracecontext"),
application:set_env(opentelemetry, text_map_propagators, [b3multi, baggage]),
{ok, _} = application:ensure_all_started(opentelemetry),
Config;
init_per_testcase(_, Config) ->
application:set_env(opentelemetry, processors, [{otel_batch_processor, #{scheduled_delay_ms => 1}}]),
{ok, _} = application:ensure_all_started(opentelemetry),
Expand All @@ -74,6 +56,10 @@ end_per_testcase(disable_auto_registration, _Config) ->
_ = application:stop(opentelemetry),
_ = application:unload(opentelemetry),
ok;
end_per_testcase(propagator_configuration_with_os_env, _Config) ->
os:unsetenv("OTEL_PROPAGATORS"),
_ = application:stop(opentelemetry),
ok;
end_per_testcase(_, _Config) ->
_ = application:stop(opentelemetry),
ok.
Expand All @@ -94,6 +80,44 @@ registered_tracers(_Config) ->
?assertEqual(<<"fake-version">>, NewLibrary#instrumentation_library.version),
ok.

propagator_configuration(_Config) ->
?assertEqual({otel_propagator_text_map_composite,
[otel_propagator_b3multi, otel_propagator_baggage]}, opentelemetry:get_text_map_extractor()),
?assertEqual({otel_propagator_text_map_composite,
[otel_propagator_b3multi, otel_propagator_baggage]}, opentelemetry:get_text_map_injector()),

opentelemetry:set_text_map_extractor({otel_propagator_baggage, []}),

?assertEqual({otel_propagator_baggage, []}, opentelemetry:get_text_map_extractor()),
?assertEqual({otel_propagator_text_map_composite,
[otel_propagator_b3multi, otel_propagator_baggage]}, opentelemetry:get_text_map_injector()),

opentelemetry:set_text_map_injector({otel_propagator_b3multi, []}),

?assertEqual({otel_propagator_baggage, []}, opentelemetry:get_text_map_extractor()),
?assertEqual({otel_propagator_b3multi, []}, opentelemetry:get_text_map_injector()),

ok.

propagator_configuration_with_os_env(_Config) ->
?assertEqual({otel_propagator_text_map_composite,
[otel_propagator_trace_context]}, opentelemetry:get_text_map_extractor()),
?assertEqual({otel_propagator_text_map_composite,
[otel_propagator_trace_context]}, opentelemetry:get_text_map_injector()),

opentelemetry:set_text_map_extractor({otel_propagator_baggage, []}),

?assertEqual({otel_propagator_baggage, []}, opentelemetry:get_text_map_extractor()),
?assertEqual({otel_propagator_text_map_composite,
[otel_propagator_trace_context]}, opentelemetry:get_text_map_injector()),

opentelemetry:set_text_map_injector({otel_propagator_b3multi, []}),

?assertEqual({otel_propagator_baggage, []}, opentelemetry:get_text_map_extractor()),
?assertEqual({otel_propagator_b3multi, []}, opentelemetry:get_text_map_injector()),

ok.

macros(Config) ->
Tid = ?config(tid, Config),

Expand Down Expand Up @@ -233,67 +257,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),

Expand Down Expand Up @@ -598,10 +561,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)}].
7 changes: 3 additions & 4 deletions apps/opentelemetry/test/otel_configuration_SUITE.erl
Original file line number Diff line number Diff line change
Expand Up @@ -114,8 +114,7 @@ end_per_testcase(_, Config) ->

empty_os_environment(_Config) ->
?assertIsSubset([{log_level,info},
{propagators,[fun otel_tracer_default:w3c_propagators/0,
fun otel_baggage:get_text_map_propagators/0]},
{text_map_propagators,[trace_context, baggage]},
{sampler,{parent_based,#{root => always_on}}}],
otel_configuration:merge_with_os([])),

Expand Down Expand Up @@ -170,14 +169,14 @@ log_level(_Config) ->
propagators(_Config) ->
%% TODO: can make this a better error message when it fails with a custom assert macro
?assertIsSubset([{log_level, error},
{propagators, [fun otel_baggage:get_text_map_propagators/0]}],
{text_map_propagators, [baggage]}],
otel_configuration:merge_with_os([{log_level, error}])),

ok.

propagators_b3multi(_Config) ->
?assertIsSubset([{log_level, error},
{propagators, [fun otel_tracer_default:b3_propagators/0]}],
{text_map_propagators, [b3multi]}],
otel_configuration:merge_with_os([{log_level, error}])),

ok.
Expand Down
Loading